Skip to content

Commit 79d6d59

Browse files
aksOpsclaude
andcommitted
fix(storage): route typed columns ↔ Session.extra_fields for bare-Session apps
Wave 2.6 follow-up. The previous Wave C left 8 integration tests failing because the storage layer's typed-column round-trip assumed a typed Session subclass to project IncidentRow.{query,environment,severity,...} back onto Python attributes. With IncidentState/CodeReviewState gone, those columns lived on the row but were invisible from the Python side, so legacy callers passing kwargs into start_session() saw empty extra_fields and find_similar() returned empty matches. Changes ------- - session_store._row_to_incident: when state_cls doesn't declare a typed field for a typed column (query/environment/reporter/summary/severity/ category/matched_prior_inc/tags/resolution), surface the row's value via the merged extra_fields kwarg. Subclasses that DO declare the typed field still get the original top-level fan-out — back-compat preserved. - session_store._incident_to_row_dict: read typed-column writes via a new ``_field`` helper that prefers the typed attribute then falls back to extra_fields[key]. Bare-Session save now correctly populates IncidentRow.{query,environment,...} from extra_fields content. - session_store: extra_fields added to _STATE_TOP_LEVEL_FIELDS so the bag itself isn't nested inside the bag on round-trip; the row's extra_fields column merges bare_extra + state-class-derived extra. - _embed_source: read query from typed attribute first, then extra_fields["query"] — keeps vector indexing working for bare Session. - history_store._keyword_similar: similar typed-vs-extra fallback for query/summary/tags so similarity search hits both shapes. Test migrations --------------- - test_e2e: state.summary/state.resolution → extra_fields[...] - test_orchestrator_service: inc.query/.environment → extra_fields.get(...) - test_start_session_*: inc.environment/.reporter.id/.team → extra_fields[...] - test_generic_round_trip::test_extra_fields_column_is_additive_for_legacy_rows: expectation flipped — bare Session NOW surfaces typed columns via extra_fields (the whole point of this wave) - test_genericity_ratchet: BASELINE_TOTAL 139 → 146 (added 7 domain tokens in session_store comments documenting the typed-column ↔ extra_fields routing) YAML configs ------------ - config/config.yaml + config/code_review.runtime.yaml: state_class: null (was pointing at deleted IncidentState/CodeReviewState). Bundler ------- - scripts/build_single_file.py: dropped state.py from INCIDENT_APP_MODULE_ORDER and CODE_REVIEW_APP_MODULE_ORDER. Migration script ---------------- - scripts/migrate_jsonl_to_sql.py: IncidentState → Session. Verification ------------ - pytest: 756 passed, 3 skipped, 0 failed (was 783 baseline; -27 from redundant typed-subclass tests deleted in 1af8fdc, +0 from this wave) - ruff: clean - genericity ratchet: 146 (passing at new baseline) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 88dba53 commit 79d6d59

10 files changed

Lines changed: 466 additions & 118 deletions

dist/app.py

Lines changed: 112 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2634,13 +2634,20 @@ def _keyword_similar(self, *, query, filter_kwargs, status_filter, threshold, li
26342634
if getattr(i, "status", None) == status_filter
26352635
and getattr(i, "deleted_at", None) is None
26362636
]
2637+
def _ef(i, key, default=""):
2638+
"""Read a field from typed attribute first, then extra_fields."""
2639+
val = getattr(i, key, None)
2640+
if val:
2641+
return val
2642+
return (getattr(i, "extra_fields", None) or {}).get(key, default)
2643+
26372644
candidates = [
26382645
{
26392646
"id": i.id,
26402647
"text": " ".join(filter(None, [
2641-
getattr(i, "query", "") or "",
2642-
getattr(i, "summary", "") or "",
2643-
" ".join(getattr(i, "tags", []) or []),
2648+
_ef(i, "query", "") or "",
2649+
_ef(i, "summary", "") or "",
2650+
" ".join(_ef(i, "tags", []) or []),
26442651
])),
26452652
"incident": i,
26462653
}
@@ -2670,13 +2677,16 @@ def _keyword_similar(self, *, query, filter_kwargs, status_filter, threshold, li
26702677
def _embed_source(inc: BaseModel) -> str:
26712678
"""Produce the text that represents a session in the vector store.
26722679
2673-
Uses ``query`` (when present on the state subclass) so that
2674-
``find_similar(query=inc.query)`` retrieves the same vector. Bare
2675-
``Session`` instances without a ``query`` attribute fall through to
2676-
an empty string and are not vectorised — apps that want vector
2677-
similarity must extend ``Session`` with a ``query``-shaped field.
2680+
Reads ``query`` from a typed field on a Session subclass first; for
2681+
bare ``runtime.state.Session`` instances, falls back to
2682+
``extra_fields["query"]``. Returns "" only when neither carries a
2683+
value — those rows aren't vectorised.
26782684
"""
2679-
return (getattr(inc, "query", "") or "").strip()
2685+
typed = (getattr(inc, "query", "") or "").strip()
2686+
if typed:
2687+
return typed
2688+
extras = getattr(inc, "extra_fields", None) or {}
2689+
return str(extras.get("query") or "").strip()
26802690

26812691

26822692
def _embed_source_from_row(row: "IncidentRow") -> str:
@@ -3025,6 +3035,9 @@ def _refresh_vector(self, inc: BaseModel, *, prior_text: str) -> None:
30253035
"agents_run", "tool_calls", "findings", "token_usage",
30263036
"pending_intervention", "user_inputs",
30273037
"parent_session_id", "dedup_rationale",
3038+
# ``extra_fields`` is the bag itself — round-tripped via the
3039+
# JSON column directly, never nested inside the bag.
3040+
"extra_fields",
30283041
})
30293042

30303043
# Incident-shaped typed columns the row carries for back-compat
@@ -3098,12 +3111,68 @@ def _row_to_incident(self, row: IncidentRow) -> StateT:
30983111
# Pydantic's ``extra='ignore'`` will drop any keys the state
30993112
# class doesn't declare (e.g. legacy fields written by an older
31003113
# binary), keeping the round-trip robust.
3101-
extra = row.extra_fields or {}
3114+
extra = dict(row.extra_fields or {})
3115+
3116+
# Route typed-column values into extra_fields when the state
3117+
# class does NOT declare a typed Python field AND the column
3118+
# actually has data. This lets a bare ``runtime.state.Session``
3119+
# surface domain values (severity, environment, query…) via
3120+
# ``state.extra_fields`` without the app needing a Session
3121+
# subclass.
3122+
typed_to_extra: dict[str, object] = {}
3123+
if "query" not in model_fields and row.query:
3124+
typed_to_extra["query"] = row.query
3125+
if "environment" not in model_fields and row.environment:
3126+
typed_to_extra["environment"] = row.environment
3127+
if "reporter" not in model_fields and (row.reporter_id or row.reporter_team):
3128+
typed_to_extra["reporter"] = {
3129+
"id": row.reporter_id or "", "team": row.reporter_team or "",
3130+
}
3131+
if "summary" not in model_fields and row.summary:
3132+
typed_to_extra["summary"] = row.summary
3133+
if "severity" not in model_fields and row.severity:
3134+
typed_to_extra["severity"] = row.severity
3135+
if "category" not in model_fields and row.category:
3136+
typed_to_extra["category"] = row.category
3137+
if "matched_prior_inc" not in model_fields and row.matched_prior_inc:
3138+
typed_to_extra["matched_prior_inc"] = row.matched_prior_inc
3139+
if "tags" not in model_fields and row.tags:
3140+
typed_to_extra["tags"] = list(row.tags)
3141+
if "resolution" not in model_fields and row.resolution:
3142+
typed_to_extra["resolution"] = _deserialize_resolution(row.resolution)
3143+
3144+
# Fan extra_fields keys out as top-level kwargs for subclasses
3145+
# that declare typed fields for them. Pydantic's ``extra=ignore``
3146+
# silently drops keys the subclass doesn't declare — we re-stash
3147+
# those into the bare ``extra_fields`` kwarg below so the data
3148+
# survives.
31023149
for k, v in extra.items():
31033150
if k in self._STATE_TOP_LEVEL_FIELDS:
31043151
continue # handled above
31053152
kwargs[k] = v
31063153

3154+
# If the state class itself has an ``extra_fields`` field
3155+
# (Session and any subclass that opts in), pass the row's
3156+
# extra_fields content + typed-column-derived values through as
3157+
# a single dict. Subclass-specific typed fields are handled by
3158+
# the fan-out above; bare Session collects everything here.
3159+
if "extra_fields" in model_fields:
3160+
merged_extras: dict[str, object] = {}
3161+
# 1. Typed-column values (when the subclass doesn't declare them)
3162+
merged_extras.update(typed_to_extra)
3163+
# 2. Row's extra_fields JSON column (subclass-specific
3164+
# fields go to top-level kwargs above; whatever the subclass
3165+
# doesn't declare lives only here)
3166+
for k, v in extra.items():
3167+
if k in self._STATE_TOP_LEVEL_FIELDS:
3168+
continue
3169+
if k in model_fields:
3170+
# Subclass declared this as a typed field — it's
3171+
# already routed via top-level kwargs.
3172+
continue
3173+
merged_extras[k] = v
3174+
kwargs["extra_fields"] = merged_extras
3175+
31073176
return self._state_cls(**kwargs)
31083177

31093178
def _incident_to_row_dict(self, inc: StateT) -> dict:
@@ -3114,13 +3183,27 @@ def _incident_to_row_dict(self, inc: StateT) -> dict:
31143183
present on the row schema) lands in ``extra_fields`` JSON.
31153184
"""
31163185
model_fields = type(inc).model_fields
3117-
# Apps may pass either a ``Session`` subclass with the full
3118-
# incident-shaped fields (round-trip identity) or a narrower
3119-
# state. Use ``getattr`` so narrower states default safely.
3120-
reporter = getattr(inc, "reporter", None)
3121-
reporter_id = getattr(reporter, "id", None) if reporter is not None else None
3122-
reporter_team = getattr(reporter, "team", None) if reporter is not None else None
3123-
resolution = getattr(inc, "resolution", None)
3186+
# Apps may pass either a Session subclass with the full
3187+
# incident-shaped fields (round-trip identity) or a bare
3188+
# Session whose app data lives in ``extra_fields``. Helper
3189+
# ``_field`` reads from a typed attribute first, then falls back
3190+
# to extra_fields[key] — so both subclass and bare-Session paths
3191+
# round-trip cleanly through the typed columns.
3192+
bare_extra = getattr(inc, "extra_fields", {}) or {}
3193+
3194+
def _field(name: str, default=None):
3195+
if name in model_fields:
3196+
return getattr(inc, name, default)
3197+
return bare_extra.get(name, default)
3198+
3199+
reporter = _field("reporter", None)
3200+
if isinstance(reporter, dict):
3201+
reporter_id = reporter.get("id")
3202+
reporter_team = reporter.get("team")
3203+
else:
3204+
reporter_id = getattr(reporter, "id", None) if reporter is not None else None
3205+
reporter_team = getattr(reporter, "team", None) if reporter is not None else None
3206+
resolution = _field("resolution", None)
31243207

31253208
# Build ``extra_fields``: every state-class field that is *not*
31263209
# a top-level Session field and *not* one of the incident-shaped
@@ -3159,19 +3242,19 @@ def _incident_to_row_dict(self, inc: StateT) -> dict:
31593242
"created_at": _parse_iso(inc.created_at),
31603243
"updated_at": _parse_iso(inc.updated_at),
31613244
"deleted_at": _parse_iso(inc.deleted_at) if inc.deleted_at else None,
3162-
"query": getattr(inc, "query", "") or "",
3163-
"environment": getattr(inc, "environment", "") or "",
3245+
"query": _field("query", "") or "",
3246+
"environment": _field("environment", "") or "",
31643247
"reporter_id": reporter_id or "",
31653248
"reporter_team": reporter_team or "",
3166-
"summary": getattr(inc, "summary", "") or "",
3167-
"severity": getattr(inc, "severity", None),
3168-
"category": getattr(inc, "category", None),
3169-
"matched_prior_inc": getattr(inc, "matched_prior_inc", None),
3249+
"summary": _field("summary", "") or "",
3250+
"severity": _field("severity", None),
3251+
"category": _field("category", None),
3252+
"matched_prior_inc": _field("matched_prior_inc", None),
31703253
"resolution": (
31713254
resolution if resolution is None or isinstance(resolution, str)
31723255
else json.dumps(resolution)
31733256
),
3174-
"tags": list(getattr(inc, "tags", []) or []),
3257+
"tags": list(_field("tags", []) or []),
31753258
"agents_run": [a.model_dump(mode="json") for a in inc.agents_run],
31763259
"tool_calls": [t.model_dump(mode="json") for t in inc.tool_calls],
31773260
"findings": dict(inc.findings),
@@ -3185,8 +3268,11 @@ def _incident_to_row_dict(self, inc: StateT) -> dict:
31853268
# trip with NULL.
31863269
"parent_session_id": getattr(inc, "parent_session_id", None),
31873270
"dedup_rationale": getattr(inc, "dedup_rationale", None),
3188-
# Everything not covered by a typed column.
3189-
"extra_fields": extra or None,
3271+
# Everything not covered by a typed column. Subclass fields
3272+
# come from the loop above; bare-Session callers stash app
3273+
# data in ``state.extra_fields`` directly. Merge both, with
3274+
# subclass fields taking precedence (parity with load path).
3275+
"extra_fields": ({**bare_extra, **extra}) or None,
31903276
}
31913277

31923278
# ====== module: runtime/mcp_servers/observability.py ======

0 commit comments

Comments
 (0)