diff --git a/drb-c2-core/app/internal/incident_correlator.py b/drb-c2-core/app/internal/incident_correlator.py index 1032944..2654c1e 100644 --- a/drb-c2-core/app/internal/incident_correlator.py +++ b/drb-c2-core/app/internal/incident_correlator.py @@ -943,6 +943,13 @@ def _run_decision(ctx: dict) -> dict: corr_debug = { "corr_path": "fast/thin", "corr_incident_idle_min": round(_incident_idle_minutes(matched_incident, now), 1), + # This path is 63% of all links and was the only one writing + # no fit signal, so the admin debug view's "fit_signal + # distribution" panel read empty on 95% of calls and looked + # broken. Name what actually decided it: recency on this + # talkgroup, with no content to check a fit against. + "corr_fit_signal": "thin_recency", + "corr_candidates": len(thin_pool), } logger.info( f"Correlator fast-path (thin→last TGID incident): " diff --git a/drb-c2-core/app/internal/mqtt_handler.py b/drb-c2-core/app/internal/mqtt_handler.py index 1e01bc1..494dd4c 100644 --- a/drb-c2-core/app/internal/mqtt_handler.py +++ b/drb-c2-core/app/internal/mqtt_handler.py @@ -6,6 +6,7 @@ import paho.mqtt.client as mqtt from app.config import settings from app.internal.logger import logger from app.internal import firestore as fstore +from app.internal import talkgroups from app.internal.tenancy import FOUNDING_ORG_ID @@ -219,16 +220,12 @@ class MQTTHandler: else datetime.now(timezone.utc) ) - # Prefer the name from OP25 metadata; fall back to the system config - tgid_name = payload.get("tgid_name") or "" - if not tgid_name and system_id and payload.get("tgid"): - system_doc = await fstore.doc_get_cached("systems", system_id) - if system_doc: - tgid_int = int(payload["tgid"]) - for tg in system_doc.get("config", {}).get("talkgroups", []): - if int(tg.get("id", -1)) == tgid_int: - tgid_name = tg.get("name", "") - break + # Prefer the name from OP25 metadata; fall back to the system config. + # The lookup lives in internal/talkgroups.py because /upload needs the + # identical resolution and used to go without it — see server-26#34. + tgid_name = await talkgroups.resolve( + system_id, payload.get("tgid"), hint=payload.get("tgid_name") or None + ) or "" doc = { "call_id": call_id, diff --git a/drb-c2-core/app/internal/talkgroups.py b/drb-c2-core/app/internal/talkgroups.py new file mode 100644 index 0000000..662c0dd --- /dev/null +++ b/drb-c2-core/app/internal/talkgroups.py @@ -0,0 +1,77 @@ +""" +Talkgroup name resolution. + +C2 owns the `systems` collection, and a system's config carries the full +talkgroup table — id and human name for every channel the node scans. The edge +node only knows the name when OP25 happened to have it in the loaded tags file, +so `tgid_name` on a call_start, and the `talkgroup_name` form field on /upload, +are both frequently empty for a talkgroup C2 can name perfectly well. + +This resolver is the single place that closes that gap. `mqtt_handler` had its +own copy of the lookup on the call_start path, so calls got a name written to +their document while the /upload path — the one that drives transcription, +correlation and, critically, the incident *title* — kept whatever empty string +the node sent. The result was 84 of 100 incidents named "Ems — TGID 9048" +instead of "Ems — Ossining Police Dispatch" (server-26#34). + +Order of preference: whatever the caller was given, then the call document +(written at call_start), then the system config. Returns None when nothing +knows the name, so callers keep their existing "TGID {id}" fallback. +""" + +from typing import Optional + +from app.internal import firestore as fstore +from app.internal.logger import logger + + +async def name_from_system(system_id: Optional[str], talkgroup_id: Optional[int]) -> Optional[str]: + """Look a talkgroup's name up in its system's config. None if unknown.""" + if not system_id or talkgroup_id is None: + return None + try: + tgid_int = int(talkgroup_id) + except (TypeError, ValueError): + return None + + system_doc = await fstore.doc_get_cached("systems", system_id) + if not system_doc: + return None + + for tg in system_doc.get("config", {}).get("talkgroups", []): + try: + if int(tg.get("id", -1)) == tgid_int: + return tg.get("name") or None + except (TypeError, ValueError): + continue + return None + + +async def resolve( + system_id: Optional[str], + talkgroup_id: Optional[int], + hint: Optional[str] = None, + call_doc: Optional[dict] = None, +) -> Optional[str]: + """ + Best available human name for a talkgroup. + + `hint` is whatever the caller already had (OP25 metadata, a form field). + `call_doc` is an already-fetched call document, if the caller has one — + passing it avoids a second read. + """ + if hint: + return hint + + if call_doc: + from_doc = call_doc.get("talkgroup_name") + if from_doc: + return from_doc + + resolved = await name_from_system(system_id, talkgroup_id) + if resolved: + logger.info( + f"Resolved talkgroup name from system config: " + f"TGID {talkgroup_id} → {resolved!r}" + ) + return resolved diff --git a/drb-c2-core/app/routers/upload.py b/drb-c2-core/app/routers/upload.py index b62707c..dff626d 100644 --- a/drb-c2-core/app/routers/upload.py +++ b/drb-c2-core/app/routers/upload.py @@ -244,9 +244,27 @@ async def _run_intelligence_pipeline( 3. Correlate each scene with existing incidents (or create new ones) 4. Check alert rules and dispatch notifications """ - from app.internal import transcription, intelligence, incident_correlator, alerter + from app.internal import transcription, intelligence, incident_correlator, alerter, talkgroups from app.internal.feature_flags import get_flags + # The node only sends talkgroup_name when OP25 had it in the loaded tags + # file, so it arrives empty for exactly the talkgroups C2 can name from the + # system config. Resolve it once, here, at the single funnel both /upload + # and /calls/{id}/reprocess pass through — everything downstream (the + # dispatch-channel test, scene extraction, and the incident title) then + # gets a real name instead of "TGID 9048". server-26#34. + _call_doc = await fstore.doc_get("calls", call_id) + talkgroup_name = await talkgroups.resolve( + system_id, talkgroup_id, hint=talkgroup_name, call_doc=_call_doc, + ) + # Backfill the call document too, so the archive and the orphan panel stop + # showing a bare TGID for a channel we can now name. + if talkgroup_name and _call_doc is not None and not _call_doc.get("talkgroup_name"): + try: + await fstore.doc_set("calls", call_id, {"talkgroup_name": talkgroup_name}) + except Exception as e: + logger.warning(f"Could not backfill talkgroup_name on call {call_id}: {e}") + flags = await get_flags() # Resolve per-system overrides: system flag=False beats global flag=True, diff --git a/drb-c2-core/tests/test_talkgroups.py b/drb-c2-core/tests/test_talkgroups.py new file mode 100644 index 0000000..66fb907 --- /dev/null +++ b/drb-c2-core/tests/test_talkgroups.py @@ -0,0 +1,111 @@ +""" +Unit tests for talkgroup name resolution. + +From the 2026-08-23 correlation dump: 84 of 100 incidents were titled +"Ems — TGID 9048" rather than "Ems — Ossining Police Dispatch", because +/upload took `talkgroup_name` from a multipart form field the node only fills +when OP25 already had the name — and never fell back to the system config the +way mqtt_handler's call_start path did. server-26#34. + +resolve() is the single implementation both paths now share. These tests pin +its preference order, since the whole bug was one caller skipping a step. +""" +import pytest +from unittest.mock import AsyncMock, patch + +from app.internal import talkgroups + +SYSTEM = { + "config": { + "talkgroups": [ + {"id": 9048, "name": "Ossining - Police Dispatch"}, + {"id": 9600, "name": "MTA PD Districts 6/7/11 - Police Dispatch"}, + {"id": 9563, "name": ""}, # present but unnamed + {"id": "9211", "name": "Ardsley"}, # id stored as a string + ] + } +} + + +def _system(doc=SYSTEM): + return patch.object(talkgroups.fstore, "doc_get_cached", AsyncMock(return_value=doc)) + + +@pytest.mark.asyncio +async def test_hint_wins_over_everything(): + """OP25 knew the name — no Firestore read at all.""" + with patch.object(talkgroups.fstore, "doc_get_cached", AsyncMock()) as m: + got = await talkgroups.resolve("sys-1", 9048, hint="Whatever OP25 Said") + assert got == "Whatever OP25 Said" + m.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_call_doc_used_before_system_config(): + """The call document already carries the name written at call_start.""" + with patch.object(talkgroups.fstore, "doc_get_cached", AsyncMock()) as m: + got = await talkgroups.resolve( + "sys-1", 9048, hint=None, call_doc={"talkgroup_name": "From Call Doc"} + ) + assert got == "From Call Doc" + m.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_falls_back_to_system_config(): + """The case that was broken: nothing upstream knew the name, C2 did.""" + with _system(): + got = await talkgroups.resolve("sys-1", 9048, hint=None, call_doc={}) + assert got == "Ossining - Police Dispatch" + + +@pytest.mark.asyncio +async def test_empty_call_doc_name_does_not_block_the_lookup(): + """A falsy talkgroup_name on the call doc must not short-circuit.""" + with _system(): + got = await talkgroups.resolve( + "sys-1", 9600, hint=None, call_doc={"talkgroup_name": ""} + ) + assert got == "MTA PD Districts 6/7/11 - Police Dispatch" + + +@pytest.mark.asyncio +async def test_string_talkgroup_ids_in_config_still_match(): + with _system(): + assert await talkgroups.resolve("sys-1", 9211) == "Ardsley" + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "system_id, tgid", + [(None, 9048), ("sys-1", None), (None, None)], +) +async def test_missing_inputs_return_none_without_reading(system_id, tgid): + with patch.object(talkgroups.fstore, "doc_get_cached", AsyncMock()) as m: + assert await talkgroups.resolve(system_id, tgid) is None + m.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_unknown_talkgroup_returns_none_so_caller_keeps_tgid_fallback(): + with _system(): + assert await talkgroups.resolve("sys-1", 1234) is None + + +@pytest.mark.asyncio +async def test_named_entry_with_empty_string_returns_none(): + """An entry that exists but has no name is not a name.""" + with _system(): + assert await talkgroups.resolve("sys-1", 9563) is None + + +@pytest.mark.asyncio +async def test_missing_system_document_returns_none(): + with _system(doc=None): + assert await talkgroups.resolve("sys-1", 9048) is None + + +@pytest.mark.asyncio +async def test_unparseable_talkgroup_id_returns_none(): + with _system(): + assert await talkgroups.resolve("sys-1", "not-a-number") is None