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
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
b7701b6d49
commit
fae84a45c3
@@ -0,0 +1,124 @@
|
||||
"""
|
||||
server-26#114 — the incident summarizer used to read doc["transcript"] (the
|
||||
WHOLE call, raw) for every linked call, so a multi-scene call contributed
|
||||
text from scenes it wasn't part of into an incident's summary, and
|
||||
transcript_corrected was never consulted at all.
|
||||
|
||||
Fix: _scene_text_for_incident reads the server-26#96 `scenes` map to find the
|
||||
scene(s) that actually resolved into a given incident_id, and falls back to
|
||||
transcript_corrected-or-transcript for a call doc with no `scenes` field
|
||||
(predates #96).
|
||||
"""
|
||||
import pytest
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
from app.internal import summarizer
|
||||
from app.internal.summarizer import _scene_text_for_incident
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _scene_text_for_incident — pure function, no Firestore
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_picks_the_scene_that_linked_to_this_incident():
|
||||
doc = {
|
||||
"transcript": "whole raw transcript blend",
|
||||
"transcript_corrected": "whole corrected transcript blend",
|
||||
"scenes": {
|
||||
"0": {"transcript": "scene zero text", "incident_id": "inc-A", "corr_debug": {}},
|
||||
"1": {"transcript": "scene one text", "incident_id": "inc-B", "corr_debug": {}},
|
||||
},
|
||||
}
|
||||
assert _scene_text_for_incident(doc, "inc-A") == "scene zero text"
|
||||
assert _scene_text_for_incident(doc, "inc-B") == "scene one text"
|
||||
|
||||
|
||||
def test_joins_multiple_scenes_linked_to_the_same_incident_in_scene_order():
|
||||
doc = {
|
||||
"scenes": {
|
||||
"1": {"transcript": "second", "incident_id": "inc-A"},
|
||||
"0": {"transcript": "first", "incident_id": "inc-A"},
|
||||
},
|
||||
}
|
||||
assert _scene_text_for_incident(doc, "inc-A") == "first\nsecond"
|
||||
|
||||
|
||||
def test_old_schema_doc_falls_back_to_transcript_corrected_over_transcript():
|
||||
doc = {"transcript": "raw", "transcript_corrected": "corrected"}
|
||||
assert _scene_text_for_incident(doc, "inc-A") == "corrected"
|
||||
|
||||
|
||||
def test_old_schema_doc_with_only_raw_transcript_still_returns_it():
|
||||
doc = {"transcript": "raw only"}
|
||||
assert _scene_text_for_incident(doc, "inc-A") == "raw only"
|
||||
|
||||
|
||||
def test_scenes_present_but_none_match_falls_back_defensively():
|
||||
"""Should not happen for a call_id genuinely in this incident's call_ids,
|
||||
but silently dropping the call's contribution would be worse than a
|
||||
whole-call fallback."""
|
||||
doc = {
|
||||
"transcript": "raw",
|
||||
"transcript_corrected": "corrected",
|
||||
"scenes": {"0": {"transcript": "x", "incident_id": "inc-OTHER"}},
|
||||
}
|
||||
assert _scene_text_for_incident(doc, "inc-A") == "corrected"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _summarize_incident — end to end with fstore/Gemini mocked
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_summarize_incident_uses_scene_specific_text_for_a_multiscene_call():
|
||||
"""
|
||||
call-1 is a 2-scene call: scene 0 linked into inc-OTHER, scene 1 linked
|
||||
into inc-1 (the incident being summarized). Only scene 1's text may reach
|
||||
the model.
|
||||
"""
|
||||
call_1 = {
|
||||
"call_id": "call-1",
|
||||
"transcript": "scene zero text scene one text", # the old, wrong, whole-call blend
|
||||
"scenes": {
|
||||
"0": {"transcript": "scene zero text", "incident_id": "inc-OTHER"},
|
||||
"1": {"transcript": "scene one text", "incident_id": "inc-1"},
|
||||
},
|
||||
}
|
||||
|
||||
async def fake_doc_get(collection, doc_id):
|
||||
assert collection == "calls"
|
||||
return call_1 if doc_id == "call-1" else None
|
||||
|
||||
with patch("app.internal.feature_flags.get_flags",
|
||||
AsyncMock(return_value={"summaries_enabled": True})), \
|
||||
patch.object(summarizer, "fstore") as fs, \
|
||||
patch.object(summarizer, "_sync_summarize", return_value="a summary") as sync:
|
||||
fs.doc_get = AsyncMock(side_effect=fake_doc_get)
|
||||
fs.doc_set = AsyncMock()
|
||||
await summarizer._summarize_incident({"incident_id": "inc-1", "call_ids": ["call-1"]})
|
||||
|
||||
sync.assert_called_once()
|
||||
_inc_arg, transcripts_arg = sync.call_args.args
|
||||
assert transcripts_arg == ["scene one text"]
|
||||
assert "scene zero text scene one text" not in transcripts_arg
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_summarize_incident_falls_back_for_old_schema_call_doc():
|
||||
"""A call doc with no `scenes` field at all — summarizer must still work,
|
||||
using transcript_corrected over raw transcript."""
|
||||
call_1 = {"call_id": "call-1", "transcript": "raw", "transcript_corrected": "corrected"}
|
||||
|
||||
async def fake_doc_get(collection, doc_id):
|
||||
return call_1 if doc_id == "call-1" else None
|
||||
|
||||
with patch("app.internal.feature_flags.get_flags",
|
||||
AsyncMock(return_value={"summaries_enabled": True})), \
|
||||
patch.object(summarizer, "fstore") as fs, \
|
||||
patch.object(summarizer, "_sync_summarize", return_value="a summary") as sync:
|
||||
fs.doc_get = AsyncMock(side_effect=fake_doc_get)
|
||||
fs.doc_set = AsyncMock()
|
||||
await summarizer._summarize_incident({"incident_id": "inc-1", "call_ids": ["call-1"]})
|
||||
|
||||
_inc_arg, transcripts_arg = sync.call_args.args
|
||||
assert transcripts_arg == ["corrected"]
|
||||
Reference in New Issue
Block a user