3 Commits
Author SHA1 Message Date
Logan CusanoandClaude Sonnet 5 66bbf5b473 intelligence: don't reject a geocode just because it is far from the node (#159)
Build & Deploy / Build & push images (push) Successful in 4m9s
Build & Deploy / Deploy Firestore rules & indexes (push) Failing after 2s
Build & Deploy / Deploy to VM (push) Failing after 2m4s
Build & Deploy / Report a failed deploy (push) Successful in 1s
_geocode_location's node-distance fallback (geocode_max_km, 40km) is a
proxy for "is this plausible" that only makes sense when the node's own
position is the best guess we have at the area. It doesn't hold for a
citywide/patched feed: node-002 sits in Westchester but relays "New York
City - NYPD Citywide 2 Patch", ~56km from the addresses on it. Every real,
correctly-geocoded address on that talkgroup was rejected by this check,
every time -- location_coords stayed permanently null for the whole
system, which killed the location_proximity correlation signal and let
duplicate incidents form for the same event reported at two nearby
addresses two minutes apart ("Shots Fired at Jackson Avenue" /
"Shots Fired at 1108 Jackson Avenue", 2026-09-20 ~23:00 UTC, merged by
hand via the Archive page's attach/detach while this fix went in).

trust_named_region skips the node-distance rejection exactly when the
query already carries a place name that isn't the node's own position --
operator-set area_context, or a municipality parsed from the talkgroup's
own name. The anchor path is untouched; an anchor's own radius is always
authoritative when one has been resolved.

Also fixes a compounding defect found while verifying: the query was
grafting the node's own COUNTY onto an already-self-named region
("...New York City..., Westchester, New York"), which is self-contradictory
and could degrade the geocode independent of the distance check. The
node's county is now used only when nothing else names the place; state
stays in both branches since it's coarse enough to be correct either way.
Extracted the query-assembly logic into a pure, unit-tested helper
(_location_query_parts) rather than testing it only through the full
extraction pipeline.

Filed #160 as a follow-up: place_verifier.py's verify() has the identical
no-anchor gap for transcript place-name correction, not fixed here.

Verified: 422 pass, 0 fail (9 new tests).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-20 20:22:42 -04:00
Logan CusanoandClaude Sonnet 5 6c095083fc correlator: use srcaddr for thin-call disambiguation instead of pure recency (#158)
The fast/thin path (short acks like "10-4", enabled to attempt linking at
all by 1ffff25) had no identity signal available -- transcript_too_short
skips GPT extraction entirely, so call_units is always empty for this
population. It fell back to "most recently updated incident on this
talkgroup", which silently misattaches a short ack to the wrong incident
whenever two are live on the same busy dispatch channel at once.

metadata_watcher.py already captures the P25 source radio ID (srcaddr) on
every call independent of transcript content, and it already reaches the
call doc (models.py, mqtt_handler.py:245) -- it was just never read by the
correlator. Thread it through _build_context, check it against the
srcaddrs already seen on each TG-matched incident before falling back to
recency, and accumulate it on the incident (_update_incident/_create_incident)
so later calls from the same radio can match.

Verified: 422 pass, 0 fail (4 new tests).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-20 20:22:06 -04:00
Logan CusanoandClaude Sonnet 5 2e67d1bad6 ci: deploy Firestore rules/indexes from the runner, not the VM (server-26#51)
The rules/indexes deploy step already existed here, gated on `command -v
firebase` over SSH on the deploy VM. It never found one (no node on the
VM), so it silently warned-and-skipped on every single deploy for weeks —
PR #124 even auto-closed #13/#51 as if this were fixed, when it wasn't.

New standalone deploy-firestore-rules job runs on the Gitea runner itself
(always has node), authenticated via a new FIREBASE_TOKEN secret (from
`firebase login:ci`) instead of anything pre-installed on the VM. It's
independent of the deploy job's health-check/rollback chain on purpose —
a rules deploy failure has nothing to roll back and must not trigger that
logic. notify-failure now distinguishes which job actually failed so the
Discord alert doesn't misreport "production is unchanged" when the app
deployed fine and only the rules push failed.

Needs FIREBASE_TOKEN added as a Gitea Actions secret before this actually
runs — it will fail loudly (by design) until then, which is the whole
point: a loud failure beats a silent skip.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-20 19:08:58 -04:00
5 changed files with 441 additions and 57 deletions
+61 -23
View File
@@ -115,27 +115,14 @@ jobs:
# Update compose files + mosquitto config # Update compose files + mosquitto config
git pull origin main git pull origin main
# server-26#51: Firestore rules + composite indexes had no deploy # server-26#51: Firestore rules/indexes deploy used to be attempted
# path and regressed silently after every fix (the alert_events and # HERE, over SSH, gated on the VM having firebase-tools installed.
# calls(org_id,started_at) indexes among them). The VM runs as the # It never did (no node on the VM), so this silently warned and
# project service account, so firebase-tools authenticates via ADC # skipped on every deploy for weeks -- PR #124 even auto-closed
# with no key file, and infra/firestore/firebase.json pins database # #13/#51 as if it were fixed. Moved to a standalone
# c2-server. Indexes go on additively -- no --force -- so a stray # deploy-firestore-rules job below that runs on the Gitea runner
# edit to firestore.indexes.json can never delete a live index; # itself (which always has node), so it no longer depends on
# rules are a full replace, which is the intent. --non-interactive # anything being pre-installed on this VM.
# means the FIRST run after a drift still needs a one-time manual
# `firebase deploy` on the VM to clear pending deletions (it aborts
# rather than guess). A failure here warns but does NOT fail the
# deploy: a transient Firebase API error must not roll back a good
# app build.
if command -v firebase >/dev/null 2>&1; then
( cd /opt/drb/infra/firestore \
&& firebase deploy --only firestore:rules,firestore:indexes \
--project ${{ secrets.FIREBASE_PROJECT_ID }} --non-interactive ) \
|| echo "WARNING: firestore deploy failed (server-26#51) -- rules/indexes may be stale"
else
echo "WARNING: firebase CLI not on the VM -- skipped firestore deploy (server-26#51); install once with: npm i -g firebase-tools"
fi
# server-26#65: capture what is actually live BEFORE switching, so # server-26#65: capture what is actually live BEFORE switching, so
# a bad deploy has something concrete to fall back to. This reads # a bad deploy has something concrete to fall back to. This reads
@@ -291,9 +278,48 @@ jobs:
echo "status=success" >> "$GITHUB_OUTPUT" echo "status=success" >> "$GITHUB_OUTPUT"
echo "rolled_back_to=$PREV_TAG" >> "$GITHUB_OUTPUT" echo "rolled_back_to=$PREV_TAG" >> "$GITHUB_OUTPUT"
deploy-firestore-rules:
name: Deploy Firestore rules & indexes
needs: build
runs-on: ubuntu-latest
# Deliberately independent of the `deploy` job (app containers) and its
# health-check/rollback chain above: a rules/indexes deploy failure has
# nothing to roll back (there is no previous "build" of a ruleset to
# revert to via this pipeline) and must never be conflated with an app
# deploy failure by triggering that job's rollback logic. This job
# failing is its own, separate red run -- picked up by notify-failure
# below -- not a signal to touch the running containers.
steps:
- uses: actions/checkout@v4
- name: Deploy firestore rules and indexes
env:
FIREBASE_TOKEN: ${{ secrets.FIREBASE_TOKEN }}
run: |
set -e
# server-26#51: this used to run over SSH on the deploy VM, gated
# on the VM having firebase-tools installed. It never did, so it
# silently warned-and-skipped on every single deploy for weeks.
# Running it here instead means the only prerequisite is a secret
# -- FIREBASE_TOKEN, from `firebase login:ci` -- rather than
# something installed by hand on a machine this pipeline doesn't
# otherwise touch. A missing token now fails this job LOUDLY
# (picked up by notify-failure) instead of a buried warning line
# nobody reads in the app deploy's logs.
if [ -z "$FIREBASE_TOKEN" ]; then
echo "FIREBASE_TOKEN secret is not set -- cannot deploy Firestore rules/indexes." >&2
echo "Generate one with 'firebase login:ci' and add it as a Gitea Actions secret." >&2
exit 1
fi
npm install -g firebase-tools
cd infra/firestore
firebase deploy --only firestore:rules,firestore:indexes \
--project ${{ secrets.FIREBASE_PROJECT_ID }} \
--token "$FIREBASE_TOKEN" --non-interactive
notify-failure: notify-failure:
name: Report a failed deploy name: Report a failed deploy
needs: [build, deploy] needs: [build, deploy, deploy-firestore-rules]
if: failure() if: failure()
runs-on: ubuntu-latest runs-on: ubuntu-latest
@@ -310,6 +336,8 @@ jobs:
SHA: ${{ gitea.sha }} SHA: ${{ gitea.sha }}
ROLLBACK_STATUS: ${{ needs.deploy.outputs.rollback_status }} ROLLBACK_STATUS: ${{ needs.deploy.outputs.rollback_status }}
ROLLBACK_SHA: ${{ needs.deploy.outputs.rollback_sha }} ROLLBACK_SHA: ${{ needs.deploy.outputs.rollback_sha }}
DEPLOY_RESULT: ${{ needs.deploy.result }}
RULES_RESULT: ${{ needs.deploy-firestore-rules.result }}
run: | run: |
if [ -z "$WEBHOOK" ]; then if [ -z "$WEBHOOK" ]; then
echo "DEPLOY_ALERT_WEBHOOK is not set - skipping notification." echo "DEPLOY_ALERT_WEBHOOK is not set - skipping notification."
@@ -321,6 +349,16 @@ jobs:
run_url = os.environ["RUN_URL"] run_url = os.environ["RUN_URL"]
status = os.environ.get("ROLLBACK_STATUS", "") status = os.environ.get("ROLLBACK_STATUS", "")
rollback_sha = os.environ.get("ROLLBACK_SHA", "") rollback_sha = os.environ.get("ROLLBACK_SHA", "")
deploy_result = os.environ.get("DEPLOY_RESULT", "")
rules_result = os.environ.get("RULES_RESULT", "")
# deploy-firestore-rules runs independent of the app deploy/rollback
# chain (see its own job comment), so its failure needs its own
# branch here -- otherwise this fell through to the generic "Build
# failed before any deploy was attempted" text even when the app
# deployed fine and only the Firestore rules/indexes push failed.
if deploy_result != "failure" and rules_result == "failure":
detail = "App deploy succeeded; Firestore rules/indexes deploy FAILED (server-26#51). Rules may be stale — check FIREBASE_TOKEN and the job log."
# server-26#65: the old text here unconditionally claimed # server-26#65: the old text here unconditionally claimed
# "production is still running the previous build" -- true only # "production is still running the previous build" -- true only
@@ -329,7 +367,7 @@ jobs:
# class of bug the correlator instrumentation exists to catch), or # class of bug the correlator instrumentation exists to catch), or
# once the deploy job's own rollback path has run. Say what # once the deploy job's own rollback path has run. Say what
# actually happened instead. # actually happened instead.
if status == "success": elif status == "success":
detail = "Automatic rollback to `%s` succeeded. Production is back on the previous good build." % rollback_sha[:8] detail = "Automatic rollback to `%s` succeeded. Production is back on the previous good build." % rollback_sha[:8]
elif status == "failed": elif status == "failed":
detail = ("Automatic rollback to `%s` FAILED. Production state is UNKNOWN -- " detail = ("Automatic rollback to `%s` FAILED. Production state is UNKNOWN -- "
@@ -875,11 +875,17 @@ async def _build_context(
is_thin_call = _is_thin_call( is_thin_call = _is_thin_call(
call_units, call_vehicles, coords, tags, location, call_severity, reassignment call_units, call_vehicles, coords, tags, location, call_severity, reassignment
) )
# server-26#158: the P25 source radio ID. Captured on every call by the
# edge node's metadata_watcher.py independent of transcript content, so
# it survives even when transcript_too_short skips GPT extraction
# entirely and leaves call_units empty — exactly the population the
# thin-call path below has no other identity signal for.
call_srcaddr = call_doc.get("srcaddr")
return { return {
"call_id": call_id, "org_id": org_id, "all_active": all_active, "recent": recent, "call_id": call_id, "org_id": org_id, "all_active": all_active, "recent": recent,
"call_doc": call_doc, "call_embedding": call_embedding, "call_doc": call_doc, "call_embedding": call_embedding,
"scene_transcript": scene_transcript, "scene_transcript": scene_transcript, "call_srcaddr": call_srcaddr,
"call_units": call_units, "call_vehicles": call_vehicles, "call_units": call_units, "call_vehicles": call_vehicles,
"call_cleared": call_cleared, "call_severity": call_severity, "call_cleared": call_cleared, "call_severity": call_severity,
"coords": coords, "is_thin_call": is_thin_call, "now": now, "coords": coords, "is_thin_call": is_thin_call, "now": now,
@@ -933,6 +939,7 @@ def _run_decision(ctx: dict) -> dict:
call_severity = ctx["call_severity"] call_severity = ctx["call_severity"]
coords = ctx["coords"] coords = ctx["coords"]
is_thin_call = ctx["is_thin_call"] is_thin_call = ctx["is_thin_call"]
call_srcaddr = ctx.get("call_srcaddr")
system_id = ctx["system_id"] system_id = ctx["system_id"]
talkgroup_id = ctx["talkgroup_id"] talkgroup_id = ctx["talkgroup_id"]
talkgroup_name = ctx["talkgroup_name"] talkgroup_name = ctx["talkgroup_name"]
@@ -1008,8 +1015,28 @@ def _run_decision(ctx: dict) -> dict:
# incident idle up to tg_fast_path_idle_minutes (90) with no # incident idle up to tg_fast_path_idle_minutes (90) with no
# single-candidate requirement and no fit test of any kind. Four # single-candidate requirement and no fit test of any kind. Four
# hours is not a bound, and neither is ninety minutes. # hours is not a bound, and neither is ninety minutes.
# server-26#158: identity beats guesswork. A thin call has no
# extracted units (GPT never ran), but it still carries the P25
# radio ID that transmitted it — stronger, cheaper evidence than
# "most recently active" and immune to the exact failure this
# path exists to guard against: two incidents both live on one
# busy dispatch channel. If the radio that sent this call already
# has calls on one of the TG-matched incidents, that IS the
# thread, regardless of which incident is more recently updated
# or how many candidates are in the window.
srcaddr_matches = [
inc for inc in tg_recent
if call_srcaddr and call_srcaddr in (inc.get("srcaddrs") or [])
]
THIN_CONVERSATIONAL_SECS = 30 THIN_CONVERSATIONAL_SECS = 30
thin_window_min = settings.tg_dispatch_thin_idle_minutes thin_window_min = settings.tg_dispatch_thin_idle_minutes
if srcaddr_matches:
thin_pool = [max(srcaddr_matches, key=lambda inc: inc.get("updated_at", ""))]
logger.info(
f"Correlator fast-path thin (srcaddr match): radio {call_srcaddr} "
f"already on {len(srcaddr_matches)} candidate(s) for call {call_id}"
)
else:
very_recent = [ very_recent = [
inc for inc in tg_recent inc for inc in tg_recent
if _idle_gate_minutes(inc, now) * 60 <= THIN_CONVERSATIONAL_SECS if _idle_gate_minutes(inc, now) * 60 <= THIN_CONVERSATIONAL_SECS
@@ -1049,8 +1076,10 @@ def _run_decision(ctx: dict) -> dict:
# no fit signal, so the admin debug view's "fit_signal # no fit signal, so the admin debug view's "fit_signal
# distribution" panel read empty on 95% of calls and looked # distribution" panel read empty on 95% of calls and looked
# broken. Name what actually decided it: recency on this # broken. Name what actually decided it: recency on this
# talkgroup, with no content to check a fit against. # talkgroup, with no content to check a fit against — or,
"corr_fit_signal": "thin_recency", # when the same radio ID already touched a candidate
# (server-26#158), that identity match instead of a guess.
"corr_fit_signal": "thin_srcaddr_match" if srcaddr_matches else "thin_recency",
"corr_candidates": len(thin_pool), "corr_candidates": len(thin_pool),
} }
logger.info( logger.info(
@@ -1511,6 +1540,7 @@ async def _apply_decision(decision: dict, ctx: dict) -> Optional[str]:
call_cleared = ctx["call_cleared"] call_cleared = ctx["call_cleared"]
coords = ctx["coords"] coords = ctx["coords"]
now = ctx["now"] now = ctx["now"]
call_srcaddr = ctx.get("call_srcaddr")
incident_type = decision["incident_type"] incident_type = decision["incident_type"]
if action == "link": if action == "link":
@@ -1522,7 +1552,7 @@ async def _apply_decision(decision: dict, ctx: dict) -> Optional[str]:
location, location_coords, call_units, call_vehicles, call_embedding, now, location, location_coords, call_units, call_vehicles, call_embedding, now,
talkgroup_name=talkgroup_name, incident_type=incident_type, talkgroup_name=talkgroup_name, incident_type=incident_type,
cleared_units=call_cleared, refresh_activity=not thin_link, cleared_units=call_cleared, refresh_activity=not thin_link,
call_severity=call_severity, call_severity=call_severity, call_srcaddr=call_srcaddr,
) )
return matched_incident["incident_id"] return matched_incident["incident_id"]
@@ -1554,6 +1584,7 @@ async def _apply_decision(decision: dict, ctx: dict) -> Optional[str]:
call_id, org_id, incident_type, talkgroup_id, talkgroup_name, system_id, call_id, org_id, incident_type, talkgroup_id, talkgroup_name, system_id,
tags, location, location_coords, tags, location, location_coords,
call_units, call_vehicles, call_embedding, call_severity, now, call_units, call_vehicles, call_embedding, call_severity, now,
call_srcaddr=call_srcaddr,
) )
if existing_master_id: if existing_master_id:
@@ -1599,6 +1630,7 @@ async def _apply_decision(decision: dict, ctx: dict) -> Optional[str]:
call_id, org_id, incident_type, talkgroup_id, talkgroup_name, system_id, call_id, org_id, incident_type, talkgroup_id, talkgroup_name, system_id,
tags, location, location_coords, tags, location, location_coords,
call_units, call_vehicles, call_embedding, call_severity, now, call_units, call_vehicles, call_embedding, call_severity, now,
call_srcaddr=call_srcaddr,
) )
decision["corr_debug"]["corr_path"] = "new" decision["corr_debug"]["corr_path"] = "new"
@@ -1945,6 +1977,7 @@ async def _update_incident(
cleared_units: Optional[list[str]] = None, cleared_units: Optional[list[str]] = None,
refresh_activity: bool = True, refresh_activity: bool = True,
call_severity: Optional[str] = None, call_severity: Optional[str] = None,
call_srcaddr: Optional[str] = None,
) -> None: ) -> None:
incident_id = inc["incident_id"] incident_id = inc["incident_id"]
@@ -1963,6 +1996,12 @@ async def _update_incident(
merged_tags = list(dict.fromkeys((inc.get("tags") or []) + tags)) merged_tags = list(dict.fromkeys((inc.get("tags") or []) + tags))
merged_units = list(dict.fromkeys((inc.get("units") or []) + call_units)) merged_units = list(dict.fromkeys((inc.get("units") or []) + call_units))
merged_vehicles = list(dict.fromkeys((inc.get("vehicles") or []) + call_vehicles)) merged_vehicles = list(dict.fromkeys((inc.get("vehicles") or []) + call_vehicles))
# server-26#158: accumulate every radio ID that has transmitted on this
# incident, so a later thin call from the same radio can identity-match
# instead of guessing off recency alone.
merged_srcaddrs = list(dict.fromkeys(
(inc.get("srcaddrs") or []) + ([call_srcaddr] if call_srcaddr else [])
))
# Unit activity tracking: units_active / units_cleared # Unit activity tracking: units_active / units_cleared
# units_active = units currently on scene; units_cleared = units back in service # units_active = units currently on scene; units_cleared = units back in service
@@ -1993,6 +2032,7 @@ async def _update_incident(
"tags": merged_tags, "tags": merged_tags,
"units": merged_units, "units": merged_units,
"vehicles": merged_vehicles, "vehicles": merged_vehicles,
"srcaddrs": merged_srcaddrs,
"units_active": units_active, "units_active": units_active,
"units_cleared": units_cleared, "units_cleared": units_cleared,
"location_mentions": location_mentions, "location_mentions": location_mentions,
@@ -2059,6 +2099,7 @@ async def _create_incident(
call_embedding: Optional[list], call_embedding: Optional[list],
call_severity: str, call_severity: str,
now: datetime, now: datetime,
call_srcaddr: Optional[str] = None,
) -> str: ) -> str:
incident_id = str(uuid.uuid4()) incident_id = str(uuid.uuid4())
tg_label = ( tg_label = (
@@ -2102,6 +2143,7 @@ async def _create_incident(
"units_active": list(call_units), "units_active": list(call_units),
"units_cleared": [], "units_cleared": [],
"vehicles": call_vehicles, "vehicles": call_vehicles,
"srcaddrs": [call_srcaddr] if call_srcaddr else [],
"severity": call_severity, "severity": call_severity,
"summary": None, "summary": None,
"summary_stale": True, "summary_stale": True,
+79 -9
View File
@@ -368,18 +368,20 @@ async def extract_scenes(
# the country. # the country.
location_coords: Optional[dict] = None location_coords: Optional[dict] = None
if location: if location:
parts = [location] node_state, node_county = "", ""
if tg_area.get("municipality") or tg_area.get("county") or tg_area.get("state"): if not area_context.has_place(tg_area) and node_id and node_lat is not None and node_lon is not None:
parts += [tg_area[f] for f in area_context.PLACE_FIELDS if tg_area.get(f)] # Only worth the (cached-after-first-call) reverse-geocode
elif node_lat is not None and node_lon is not None: # when nothing better already describes this talkgroup.
muni = _municipality_from_tg(talkgroup_name) node_state = await _get_node_state(node_id, node_lat, node_lon)
state = await _get_node_state(node_id or "", node_lat, node_lon) if node_id else "" node_county = _node_county_cache.get(node_id) or ""
county = _node_county_cache.get(node_id or "") if node_id else "" parts, tg_named_region = _location_query_parts(
parts += [p for p in (muni, county, state) if p] location, tg_area, talkgroup_name, node_state, node_county,
)
query = ", ".join(parts) query = ", ".join(parts)
if tg_anchor or (node_lat is not None and node_lon is not None): if tg_anchor or (node_lat is not None and node_lon is not None):
location_coords = await _geocode_location( location_coords = await _geocode_location(
query, node_lat, node_lon, anchor=tg_anchor query, node_lat, node_lon, anchor=tg_anchor,
trust_named_region=tg_named_region,
) )
# Embed this scene's content # Embed this scene's content
@@ -514,6 +516,7 @@ async def _geocode_location(
node_lat: Optional[float] = None, node_lat: Optional[float] = None,
node_lon: Optional[float] = None, node_lon: Optional[float] = None,
anchor: Optional[dict] = None, anchor: Optional[dict] = None,
trust_named_region: bool = False,
) -> Optional[dict]: ) -> Optional[dict]:
""" """
Geocode using Google Maps Geocoding API, biased toward the channel's area. Geocode using Google Maps Geocoding API, biased toward the channel's area.
@@ -529,6 +532,26 @@ async def _geocode_location(
talkgroup has a resolved anchor, that is the reference and its own radius is talkgroup has a resolved anchor, that is the reference and its own radius is
the bound. Distance-from-node stays only as the fallback for a system nobody the bound. Distance-from-node stays only as the fallback for a system nobody
has described yet — it was always a stand-in for this. has described yet — it was always a stand-in for this.
server-26#159: "a system nobody has described yet" turned out to include
systems that describe themselves — "New York City - NYPD Citywide 2 Patch"
names its own coverage area right in the talkgroup name, parsed into the
query by `_municipality_from_tg`, but a large aggregated/patched feed like
this is routinely received 40-70km from an antenna that happens to sit
wherever the node owner lives. Real, correctly-geocoded addresses on that
feed were being rejected by the node-distance check every single time —
location_coords stayed permanently null for the whole system, which killed
the location_proximity correlation signal and let duplicate incidents form
for the same event reported at two nearby addresses two minutes apart.
`trust_named_region` is True exactly when the query already carries a place
name that isn't the node's own position — operator-set area_context, or a
municipality parsed from the talkgroup's own name. In that case a distant
node is not evidence of a bad geocode, so the node-distance check is
skipped and precision is judged by `location_type` alone (still required
to be ROOFTOP/RANGE_INTERPOLATED/GEOMETRIC_CENTER, below). This does not
touch the anchor path at all — an anchor's own radius is always authoritative
when one has been resolved.
""" """
import httpx import httpx
from app.config import settings from app.config import settings
@@ -594,11 +617,21 @@ async def _geocode_location(
lat, lng = float(loc["lat"]), float(loc["lng"]) lat, lng = float(loc["lat"]), float(loc["lng"])
dist_km = _geo_dist_km(ref_lat, ref_lon, lat, lng) dist_km = _geo_dist_km(ref_lat, ref_lon, lat, lng)
if dist_km > max_km: if dist_km > max_km:
# server-26#159: the node-distance bound is a proxy for "is
# this plausible" that only makes sense when the node's own
# position is our best guess at the area — never when the
# query already names a different region on its own terms.
if not (ref_label == "node" and trust_named_region):
logger.warning( logger.warning(
f"Geocoding rejected '{location_str}' → ({lat:.4f}, {lng:.4f}) " f"Geocoding rejected '{location_str}' → ({lat:.4f}, {lng:.4f}) "
f"— {dist_km:.1f}km from {ref_label} exceeds {max_km:.1f}km" f"— {dist_km:.1f}km from {ref_label} exceeds {max_km:.1f}km"
) )
return None return None
logger.info(
f"Geocoding '{location_str}' → ({lat:.4f}, {lng:.4f}) is "
f"{dist_km:.1f}km from the receiving node, past {max_km:.1f}km — "
f"accepted anyway: the query names its own region, not the node's"
)
coords = {"lat": lat, "lng": lng} coords = {"lat": lat, "lng": lng}
logger.info( logger.info(
f"Geocoded '{location_str}' → {coords} " f"Geocoded '{location_str}' → {coords} "
@@ -624,6 +657,43 @@ def _municipality_from_tg(tg_name: Optional[str]) -> Optional[str]:
return cleaned return cleaned
def _location_query_parts(
location: str,
tg_area: dict,
talkgroup_name: Optional[str],
node_state: str,
node_county: str,
) -> tuple[list[str], bool]:
"""
Build the geocode query parts for `location`, plus whether the query names
a region the *talkgroup itself* covers (operator-set area_context, or a
municipality parsed from the talkgroup's own name) rather than one guessed
from wherever the receiving node happens to sit (server-26#159).
That distinction matters downstream: `_geocode_location`'s node-distance
sanity check is only a valid proxy for "is this plausible" when the node's
own position is the best guess we have at the area. A citywide/patched
feed ("New York City - NYPD Citywide 2 Patch") names its own coverage area
right in the talkgroup name — grafting the node's own county onto that
(Ossining-style: valid when the feed genuinely is local to the node,
actively wrong when it names a distant region of its own) would make the
query self-contradictory, so the node's COUNTY is used only when nothing
better names the place. The node's STATE is coarse enough to still be
correct either way and is kept in both branches.
"""
parts = [location]
if area_context.has_place(tg_area):
parts += [tg_area[f] for f in area_context.PLACE_FIELDS if tg_area.get(f)]
return parts, True
muni = _municipality_from_tg(talkgroup_name)
if muni:
parts += [p for p in (muni, node_state) if p]
else:
parts += [p for p in (node_county, node_state) if p]
return parts, muni is not None
def _build_transcript_block(transcript: str, segments: Optional[list[dict]]) -> str: def _build_transcript_block(transcript: str, segments: Optional[list[dict]]) -> str:
"""Format transcript as numbered transmissions if segments are available.""" """Format transcript as numbered transmissions if segments are available."""
if segments and len(segments) > 1: if segments and len(segments) > 1:
@@ -180,6 +180,55 @@ def test_tactical_thin_call_is_ambiguous_with_two_candidates():
assert decision["action"] == "orphan" assert decision["action"] == "orphan"
# ---------------------------------------------------------------------------
# server-26#158: srcaddr identity beats recency guesswork for thin calls
# ---------------------------------------------------------------------------
def test_thin_call_srcaddr_match_resolves_tier2_ambiguity():
"""
Same fixture as test_tactical_thin_call_is_ambiguous_with_two_candidates —
two candidates, tier-2 window, no unit ID parsed (transcript_too_short
skipped GPT). Without srcaddr this orphans. With it, the radio that sent
the call already touched inc-b, so that's the thread — not a guess.
"""
a = _incident(idle_minutes=3.0, incident_id="inc-a", srcaddrs=["9001"])
b = _incident(idle_minutes=4.0, incident_id="inc-b", srcaddrs=["9002"])
decision = _run_decision(_ctx(
all_active=[a, b], recent=[a, b], talkgroup_name=TACTICAL_TG,
call_srcaddr="9002",
))
assert decision["action"] == "link"
assert decision["matched_incident"]["incident_id"] == "inc-b"
assert decision["corr_debug"]["corr_fit_signal"] == "thin_srcaddr_match"
def test_thin_call_srcaddr_match_overrides_recency_in_tier1():
"""
Both candidates are inside the 30s conversational window, where recency
alone would pick inc-a (more recently updated) even though the radio that
sent this call has only ever touched inc-b — the exact busy-channel,
two-concurrent-incidents misattach server-26#158 was filed for.
"""
a = _incident(idle_minutes=0.1, incident_id="inc-a", srcaddrs=["9001"])
b = _incident(idle_minutes=0.2, incident_id="inc-b", srcaddrs=["9002"])
decision = _run_decision(_ctx(
all_active=[a, b], recent=[a, b], call_srcaddr="9002",
))
assert decision["action"] == "link"
assert decision["matched_incident"]["incident_id"] == "inc-b"
def test_thin_call_with_no_srcaddr_match_falls_back_to_recency():
"""A radio ID that matches nothing on this talkgroup behaves exactly as
before — no regression for the ordinary case."""
a = _incident(idle_minutes=0.1, incident_id="inc-a", srcaddrs=["9001"])
decision = _run_decision(_ctx(
all_active=[a], recent=[a], call_srcaddr="unrelated-radio",
))
assert decision["action"] == "link"
assert decision["corr_debug"]["corr_fit_signal"] == "thin_recency"
def test_call_with_unit_overlap_does_attach(): def test_call_with_unit_overlap_does_attach():
""" """
Positive control: real evidence still links. Carrying units also means the Positive control: real evidence still links. Carrying units also means the
@@ -0,0 +1,185 @@
"""
server-26#159: a citywide/patched feed can be received far from its own
coverage area — "New York City - NYPD Citywide 2 Patch" was ~56km from the
receiving node, well past geocode_max_km (40km). Real, correctly-geocoded
addresses on that talkgroup were rejected by intelligence._geocode_location's
node-distance sanity check every time, so location_coords never populated for
the whole system: location_proximity correlation was permanently dead there,
and the same real event reported at two nearby addresses two minutes apart
became two separate incidents instead of one.
`trust_named_region` fixes this narrowly: the node-distance check is a proxy
for "is this plausible" that only makes sense when the node's own position is
the best guess we have at the area. It must not apply when the query already
names a different region on its own terms (operator-set area_context, or a
municipality parsed straight from the talkgroup's own name) — and it must
never touch the anchor path, whose own radius is always authoritative.
"""
from unittest.mock import patch
import pytest
import app.internal.intelligence as intel
from app.config import settings
def _maps_result(lat: float, lng: float, location_type: str = "ROOFTOP"):
payload = {
"status": "OK",
"results": [{
"geometry": {
"location": {"lat": lat, "lng": lng},
"location_type": location_type,
},
}],
}
class _Resp:
def raise_for_status(self): pass
def json(self): return payload
class _Client:
async def __aenter__(self): return self
async def __aexit__(self, *a): return False
async def get(self, *a, **k): return _Resp()
return patch("httpx.AsyncClient", lambda *a, **k: _Client())
@pytest.fixture(autouse=True)
def _api_key():
# intelligence.py imports settings locally per-function (`from app.config
# import settings`), which binds the same cached singleton — patching the
# module-level object here reaches it, but `intel.settings` itself does
# not exist as an attribute.
with patch.object(settings, "google_maps_api_key", "test-key"):
yield
# Node at (0, 0); result at (1, 0) is ~111km away — well past the 40km default.
NODE_LAT, NODE_LON = 0.0, 0.0
FAR_LAT, FAR_LNG = 1.0, 0.0
@pytest.mark.asyncio
async def test_named_region_geocode_accepted_beyond_node_distance():
with _maps_result(FAR_LAT, FAR_LNG):
coords = await intel._geocode_location(
"1108 Jackson Avenue, New York City - NYPD Citywide 2 Patch",
node_lat=NODE_LAT, node_lon=NODE_LON,
trust_named_region=True,
)
assert coords == {"lat": FAR_LAT, "lng": FAR_LNG}
@pytest.mark.asyncio
async def test_local_geocode_still_rejected_beyond_node_distance_without_named_region():
"""Regression guard: a bare street name with no named region still uses
the node as its only plausibility check, exactly as before this fix."""
with _maps_result(FAR_LAT, FAR_LNG):
coords = await intel._geocode_location(
"Main Street",
node_lat=NODE_LAT, node_lon=NODE_LON,
trust_named_region=False,
)
assert coords is None
@pytest.mark.asyncio
async def test_anchor_path_ignores_trust_named_region():
"""The anchor's own radius is always authoritative — trust_named_region
is only a statement about the node fallback, never a way to widen an
anchor that was itself deliberately sized to discriminate."""
anchor = {"lat": NODE_LAT, "lng": NODE_LON, "radius_km": 10.0}
with _maps_result(FAR_LAT, FAR_LNG):
coords = await intel._geocode_location(
"1108 Jackson Avenue, New York City - NYPD Citywide 2 Patch",
node_lat=NODE_LAT, node_lon=NODE_LON,
anchor=anchor, trust_named_region=True,
)
assert coords is None
@pytest.mark.asyncio
async def test_named_region_geocode_within_node_distance_is_unaffected():
"""A close result is accepted the same way regardless of the flag."""
near_lat, near_lng = 0.05, 0.0 # ~5.5km from the node
with _maps_result(near_lat, near_lng):
coords = await intel._geocode_location(
"Main Street, Ossining, New York",
node_lat=NODE_LAT, node_lon=NODE_LON,
trust_named_region=True,
)
assert coords == {"lat": near_lat, "lng": near_lng}
@pytest.mark.asyncio
async def test_imprecise_result_still_rejected_regardless_of_trust():
"""trust_named_region relaxes the distance check only — the location_type
precision filter (server-26#37) still applies unconditionally."""
with _maps_result(FAR_LAT, FAR_LNG, location_type="APPROXIMATE"):
coords = await intel._geocode_location(
"1108 Jackson Avenue, New York City - NYPD Citywide 2 Patch",
node_lat=NODE_LAT, node_lon=NODE_LON,
trust_named_region=True,
)
assert coords is None
# ---------------------------------------------------------------------------
# _location_query_parts — pure query assembly, no HTTP involved
# ---------------------------------------------------------------------------
def test_operator_configured_area_wins_and_is_named_region():
parts, named = intel._location_query_parts(
"High Street", {"municipality": "Yorktown", "state": "New York"},
"Tac 1", node_state="New York", node_county="Westchester",
)
assert parts == ["High Street", "Yorktown", "New York"]
assert named is True
def test_local_talkgroup_name_gets_node_state_but_not_node_county():
"""
"Ossining PD" is genuinely local to the node, so appending the node's own
state is correct. Its COUNTY is dropped even here — server-26#159's fix
applies uniformly once a municipality is derived, since there is no way
to tell "local" and "distant-but-node-adjacent" apart from the string
alone, and the county was never necessary for a bare municipality name
that already disambiguates via the state.
"""
parts, named = intel._location_query_parts(
"High Street", {}, "Ossining PD",
node_state="New York", node_county="Westchester",
)
assert parts == ["High Street", "Ossining", "New York"]
assert named is True
def test_citywide_patched_feed_does_not_get_the_nodes_county_grafted_on():
"""
server-26#159's actual production case: the talkgroup names its own
(distant) region, so the node's county (Westchester, ~56km away) must not
be appended — it would make the query self-contradictory ("...New York
City..., Westchester, New York") and risks degrading the geocode result's
precision independently of the distance check this issue also fixes.
"""
parts, named = intel._location_query_parts(
"1108 Jackson Avenue", {}, "New York City - NYPD Citywide 2 Patch",
node_state="New York", node_county="Westchester",
)
assert "Westchester" not in parts
assert parts == ["1108 Jackson Avenue", "New York City - NYPD Citywide 2 Patch", "New York"]
assert named is True
def test_uninformative_talkgroup_name_falls_back_to_node_county_and_state():
"""A tactical channel or bare code gives _municipality_from_tg nothing —
the only remaining evidence really is where the node sits, so the
original node-county-and-state fallback is preserved for this case."""
parts, named = intel._location_query_parts(
"High Street", {}, "Tac 1",
node_state="New York", node_county="Westchester",
)
assert parts == ["High Street", "Westchester", "New York"]
assert named is False