correlator+summarizer: per-scene call-doc storage, fixes #96 and #114's real fix #132

Merged
logan merged 2 commits from fix/96-114-per-scene-call-doc into main 2026-09-13 13:34:19 -04:00
Owner

Fixes #96 and #114.

Shared root cause

A single radio transmission can extract into 2+ scenes, and upload.py's for scene in scenes: loop (two near-identical copies — main upload path and the reprocess path) correlates each scene independently against the SAME call doc. Two different bugs came from the same missing structure:

  • #96: every scene's correlation decision wrote corr_debug flat onto the call doc via incident_correlator._apply_and_log -> fstore.doc_set("calls", call_id, corr_debug). Scene 2's write silently overwrote scene 1's corr_path/corr_consensus/corr_llm_action/etc, so the doc described a splice of decisions, not any one of them, and admin.py's tally blended them into one data point per call instead of one per scene — invalidating any per-link rate computed from it (e.g. #35's LLM-tier-share figure).
  • #114: summarizer.py read doc["transcript"] (the whole call, raw) for every linked call, so an incident's summary mixed in text from scenes it wasn't part of, and ignored transcript_corrected entirely.

#80/#95/#102 fixed the same root gap for embedding/severity/coords/LLM-prompt-transcript by threading the scene's own value through ctx at correlation time. That approach doesn't help here because both #96 and #114 need to read the value back after correlation, per scene — nothing before this PR persisted scene-scoped data at all.

The fix

  1. Scene index threading. Both for scene in scenes: loops in upload.py become for scene_index, scene in enumerate(scenes):, passing scene_index into _correlate_with_consensus -> incident_correlator.preview_correlation/correlate_call -> _build_context -> ctx["scene_index"] (default 0 for every caller with no scene concept — the recorrelation sweep, the no-scenes-extracted orphan-check path).

  2. New additive schema. incident_correlator._apply_and_log now writes, in the same Firestore call:

    • the existing flat corr_* fields, unchanged — last-scene-wins, kept as the safe backward-compatible default for any reader that doesn't know about scenes yet.
    • a new nested map: scenes.<scene_index> = {"transcript": ..., "incident_id": ..., "corr_debug": {...}}, written via doc_set(..., merge=True).

    Confirmed (not assumed): Firestore's DocumentReference.set(data, merge=True) recursively merges nested map fields by key — a write of {"scenes": {"1": {...}}} merges into an existing {"scenes": {"0": {...}}} to produce both keys side by side, rather than replacing the whole scenes map. The real google-cloud-firestore SDK isn't installed in the sandboxed test environment (it's runtime-only, stubbed out in tests/conftest.py), so this is verified against the documented SDK contract and pinned by a hand-written recursive-merge fake in the new tests, rather than exercised against live Firestore.

    scene_index defaults to 0, so a plain single-scene call still gets a scenes map with one entry — equivalent to reading its flat fields today, not a behaviour change for that population.

  3. #96's fix. admin.py's _call_summary gains a scenes field (list of per-scene records using the same corr_* field names as the flat fields, so the two shapes are interchangeable). The summary tally now iterates scenes when present, else falls back to the call's own flat fields for old-schema docs — a 2-scene call with two different corr_path values now counts as two data points instead of one blend. New scene_decision_count sits next to the unchanged linked_call_count to make that distinction visible.

  4. #114's fix. summarizer.py's new _scene_text_for_incident reads a linked call's scenes map for the scene(s) whose incident_id matches the incident being summarized (joining more than one if several scenes happened to land in the same incident), and falls back to transcript_corrected or transcript for a call doc with no scenes field. That fallback is the one-liner half of #114 and applies even to old-schema docs, so raw-transcript summaries stop regardless of schema.

Does not touch #80/#95/#102's existing ctx-threading fixes (embedding/severity/coords/LLM-prompt-transcript) — correct as-is, out of scope here.

Tests

14 new tests:

  • tests/test_per_scene_call_doc.py — a 2-scene call lands distinct corr_path/corr_consensus in scenes.0/scenes.1 while the flat fields show last-scene-wins; the scene entry records the incident_id it resolved to; a single-scene call still gets an equivalent one-entry scenes map; an empty corr_debug writes nothing at all (unchanged short-circuit). Uses a hand-written Firestore-merge-semantics fake, since the real SDK isn't available in this sandbox.
  • tests/test_admin_debug_correlation.py (additions) — the tally counts a 2-scene call as 2 data points, a plain single-scene call as 1, and an old-schema call doc (no scenes field) as 1 — no errors, no regressions to the existing flat-field exposure.
  • tests/test_summarizer_scene_transcript.py — _scene_text_for_incident picks the right scene's text per incident, joins multiple matching scenes, and falls back correctly for old-schema docs; an end-to-end _summarize_incident test confirms a 2-scene call only contributes its incident-relevant scene's text to the model call, not the whole-call blend.

Full sandboxed suite (drb-c2-core, PYTHONPATH=.../site python3 -m pytest -q): 364 -> 378 passed, all green, no regressions.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Tbknwttzou4s46PAykmtix

Fixes #96 and #114. ## Shared root cause A single radio transmission can extract into 2+ scenes, and `upload.py`'s `for scene in scenes:` loop (two near-identical copies — main upload path and the reprocess path) correlates each scene independently against the SAME call doc. Two different bugs came from the same missing structure: - **#96**: every scene's correlation decision wrote `corr_debug` flat onto the call doc via `incident_correlator._apply_and_log` -> `fstore.doc_set("calls", call_id, corr_debug)`. Scene 2's write silently overwrote scene 1's `corr_path`/`corr_consensus`/`corr_llm_action`/etc, so the doc described a splice of decisions, not any one of them, and `admin.py`'s tally blended them into one data point per call instead of one per scene — invalidating any per-link rate computed from it (e.g. #35's LLM-tier-share figure). - **#114**: `summarizer.py` read `doc["transcript"]` (the whole call, raw) for every linked call, so an incident's summary mixed in text from scenes it wasn't part of, and ignored `transcript_corrected` entirely. #80/#95/#102 fixed the same root gap for embedding/severity/coords/LLM-prompt-transcript by threading the scene's own value through `ctx` at *correlation time*. That approach doesn't help here because both #96 and #114 need to read the value back *after* correlation, per scene — nothing before this PR persisted scene-scoped data at all. ## The fix 1. **Scene index threading.** Both `for scene in scenes:` loops in `upload.py` become `for scene_index, scene in enumerate(scenes):`, passing `scene_index` into `_correlate_with_consensus` -> `incident_correlator.preview_correlation`/`correlate_call` -> `_build_context` -> `ctx["scene_index"]` (default 0 for every caller with no scene concept — the recorrelation sweep, the no-scenes-extracted orphan-check path). 2. **New additive schema.** `incident_correlator._apply_and_log` now writes, in the same Firestore call: - the existing flat `corr_*` fields, **unchanged** — last-scene-wins, kept as the safe backward-compatible default for any reader that doesn't know about `scenes` yet. - a new nested map: `scenes.<scene_index> = {"transcript": ..., "incident_id": ..., "corr_debug": {...}}`, written via `doc_set(..., merge=True)`. Confirmed (not assumed): Firestore's `DocumentReference.set(data, merge=True)` recursively merges nested map fields by key — a write of `{"scenes": {"1": {...}}}` merges into an existing `{"scenes": {"0": {...}}}` to produce both keys side by side, rather than replacing the whole `scenes` map. The real `google-cloud-firestore` SDK isn't installed in the sandboxed test environment (it's runtime-only, stubbed out in `tests/conftest.py`), so this is verified against the documented SDK contract and pinned by a hand-written recursive-merge fake in the new tests, rather than exercised against live Firestore. `scene_index` defaults to 0, so a plain single-scene call still gets a `scenes` map with one entry — equivalent to reading its flat fields today, not a behaviour change for that population. 3. **#96's fix.** `admin.py`'s `_call_summary` gains a `scenes` field (list of per-scene records using the same `corr_*` field names as the flat fields, so the two shapes are interchangeable). The summary tally now iterates `scenes` when present, else falls back to the call's own flat fields for old-schema docs — a 2-scene call with two different `corr_path` values now counts as two data points instead of one blend. New `scene_decision_count` sits next to the unchanged `linked_call_count` to make that distinction visible. 4. **#114's fix.** `summarizer.py`'s new `_scene_text_for_incident` reads a linked call's `scenes` map for the scene(s) whose `incident_id` matches the incident being summarized (joining more than one if several scenes happened to land in the same incident), and falls back to `transcript_corrected or transcript` for a call doc with no `scenes` field. That fallback is the one-liner half of #114 and applies even to old-schema docs, so raw-transcript summaries stop regardless of schema. Does **not** touch #80/#95/#102's existing ctx-threading fixes (embedding/severity/coords/LLM-prompt-transcript) — correct as-is, out of scope here. ## Tests 14 new tests: - `tests/test_per_scene_call_doc.py` — a 2-scene call lands distinct `corr_path`/`corr_consensus` in `scenes.0`/`scenes.1` while the flat fields show last-scene-wins; the scene entry records the `incident_id` it resolved to; a single-scene call still gets an equivalent one-entry `scenes` map; an empty `corr_debug` writes nothing at all (unchanged short-circuit). Uses a hand-written Firestore-merge-semantics fake, since the real SDK isn't available in this sandbox. - `tests/test_admin_debug_correlation.py` (additions) — the tally counts a 2-scene call as 2 data points, a plain single-scene call as 1, and an old-schema call doc (no `scenes` field) as 1 — no errors, no regressions to the existing flat-field exposure. - `tests/test_summarizer_scene_transcript.py` — `_scene_text_for_incident` picks the right scene's text per incident, joins multiple matching scenes, and falls back correctly for old-schema docs; an end-to-end `_summarize_incident` test confirms a 2-scene call only contributes its incident-relevant scene's text to the model call, not the whole-call blend. Full sandboxed suite (`drb-c2-core`, `PYTHONPATH=.../site python3 -m pytest -q`): **364 -> 378 passed**, all green, no regressions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Tbknwttzou4s46PAykmtix
logan added 1 commit 2026-09-13 13:26:11 -04:00
Every scene of a multi-scene call correlates independently in upload.py's
scene loop, but every scene's corr_debug was written flat onto the same
shared call doc — scene 2's write silently clobbered scene 1's
corr_path/corr_consensus/etc (#96), and summarizer.py read the whole call's
raw transcript per linked call, mixing text from scenes the incident had
nothing to do with, while ignoring transcript_corrected entirely (#114).

Fix: thread a scene_index from both `for scene in scenes:` loops in
upload.py down through _correlate_with_consensus ->
incident_correlator.preview_correlation/correlate_call -> _build_context ->
ctx["scene_index"]. incident_correlator._apply_and_log now writes, in the
same Firestore call:
  - the existing flat corr_* fields, unchanged (last-scene-wins, the safe
    backward-compatible default for any reader that doesn't know about
    `scenes` yet)
  - a new nested `scenes.<scene_index>` entry with {transcript, incident_id,
    corr_debug}, via doc_set(..., merge=True). Firestore's
    DocumentReference.set(data, merge=True) recursively merges nested map
    fields by key (documented SDK behaviour, not assumed) — a write to
    scenes.1 merges alongside an existing scenes.0 instead of replacing the
    whole `scenes` map.
scene_index defaults to 0 for every caller with no scene concept (the
recorrelation sweep, the no-scenes-extracted orphan-check path), so a plain
single-scene call still gets a one-entry `scenes` map equivalent to reading
its flat fields today.

admin.py's _call_summary exposes the new `scenes` list per call (each entry
carrying the same corr_* field names as the flat fields, so the two shapes
are interchangeable to the tally); the summary tally now iterates each
call's scenes-if-present, else its own flat fields, so a 2-scene call with
two different corr_path values counts as two data points instead of one
blend. New `scene_decision_count` sits next to `linked_call_count` to make
that distinction visible.

summarizer.py's _scene_text_for_incident reads a linked call's `scenes` map
to find the scene(s) whose corr_debug recorded a link into the specific
incident being summarized, joining more than one if several scenes landed
in the same incident. Falls back to transcript_corrected-or-transcript for a
call doc with no `scenes` field (predates this change) — the one-liner half
of #114, worth doing regardless since it stops raw-transcript summaries even
for old-schema docs.

Does not touch #80/#95/#102's existing ctx-threading fixes
(embedding/severity/coords/LLM-prompt-transcript) — correct as-is, out of
scope here.

Tests: 14 new (test_per_scene_call_doc.py, test_summarizer_scene_transcript.py,
additions to test_admin_debug_correlation.py) covering the merge shape,
last-scene-wins flat-field backward compat, the admin tally's per-scene vs
per-call counting (including old-schema fallback), and the summarizer's
scene-specific text selection (including old-schema fallback). Full sandboxed
suite: 364 -> 378 passed, all green.

Fixes server-26#96, server-26#114

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tbknwttzou4s46PAykmtix
logan added 1 commit 2026-09-13 13:33:55 -04:00
Review of #132 found a blocker: PATCH /calls/{id}/transcript wipes tags/severity/location/units/embedding before re-extraction, but not the new scenes map, and doc_set(merge=True) can only add/overwrite nested map keys, never remove one. A call corrected from 3 scenes to 1 kept scenes.1/scenes.2 with pre-correction transcripts and incident_ids forever -- corrupting the exact per-scene tally #96 exists to make trustworthy, and able to re-feed stale text into #114's summarizer fix if a stale scene's incident_id still names a real incident.

Fix: fstore.doc_update(...,{"scenes": fstore.DELETE_FIELD}) -- a real delete, not a merge over an empty map. Added fstore.DELETE_FIELD (re-exports the real firebase_admin sentinel) and stubbed it in the sandboxed test conftest, which didn't have it. Also softened an overclaiming docstring: the Firestore nested-merge behavior is verified against the doc_set wrapper's pass-through, not against live Firestore.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Claude-Session: https://claude.ai/code/session_01Tbknwttzou4s46PAykmtix
logan merged commit 833cfade4e into main 2026-09-13 13:34:19 -04:00
logan deleted branch fix/96-114-per-scene-call-doc 2026-09-13 13:34:19 -04:00
Sign in to join this conversation.