intelligence: don't reject a geocode just because it is far from the node (#159)
_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>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
6c095083fc
commit
66bbf5b473
@@ -368,18 +368,20 @@ async def extract_scenes(
|
||||
# the country.
|
||||
location_coords: Optional[dict] = None
|
||||
if location:
|
||||
parts = [location]
|
||||
if tg_area.get("municipality") or tg_area.get("county") or tg_area.get("state"):
|
||||
parts += [tg_area[f] for f in area_context.PLACE_FIELDS if tg_area.get(f)]
|
||||
elif node_lat is not None and node_lon is not None:
|
||||
muni = _municipality_from_tg(talkgroup_name)
|
||||
state = await _get_node_state(node_id or "", node_lat, node_lon) if node_id else ""
|
||||
county = _node_county_cache.get(node_id or "") if node_id else ""
|
||||
parts += [p for p in (muni, county, state) if p]
|
||||
node_state, node_county = "", ""
|
||||
if not area_context.has_place(tg_area) and node_id and node_lat is not None and node_lon is not None:
|
||||
# Only worth the (cached-after-first-call) reverse-geocode
|
||||
# when nothing better already describes this talkgroup.
|
||||
node_state = await _get_node_state(node_id, node_lat, node_lon)
|
||||
node_county = _node_county_cache.get(node_id) or ""
|
||||
parts, tg_named_region = _location_query_parts(
|
||||
location, tg_area, talkgroup_name, node_state, node_county,
|
||||
)
|
||||
query = ", ".join(parts)
|
||||
if tg_anchor or (node_lat is not None and node_lon is not None):
|
||||
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
|
||||
@@ -514,6 +516,7 @@ async def _geocode_location(
|
||||
node_lat: Optional[float] = None,
|
||||
node_lon: Optional[float] = None,
|
||||
anchor: Optional[dict] = None,
|
||||
trust_named_region: bool = False,
|
||||
) -> Optional[dict]:
|
||||
"""
|
||||
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
|
||||
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.
|
||||
|
||||
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
|
||||
from app.config import settings
|
||||
@@ -594,11 +617,21 @@ async def _geocode_location(
|
||||
lat, lng = float(loc["lat"]), float(loc["lng"])
|
||||
dist_km = _geo_dist_km(ref_lat, ref_lon, lat, lng)
|
||||
if dist_km > max_km:
|
||||
logger.warning(
|
||||
f"Geocoding rejected '{location_str}' → ({lat:.4f}, {lng:.4f}) "
|
||||
f"— {dist_km:.1f}km from {ref_label} exceeds {max_km:.1f}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(
|
||||
f"Geocoding rejected '{location_str}' → ({lat:.4f}, {lng:.4f}) "
|
||||
f"— {dist_km:.1f}km from {ref_label} exceeds {max_km:.1f}km"
|
||||
)
|
||||
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"
|
||||
)
|
||||
return None
|
||||
coords = {"lat": lat, "lng": lng}
|
||||
logger.info(
|
||||
f"Geocoded '{location_str}' → {coords} "
|
||||
@@ -624,6 +657,43 @@ def _municipality_from_tg(tg_name: Optional[str]) -> Optional[str]:
|
||||
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:
|
||||
"""Format transcript as numbered transmissions if segments are available."""
|
||||
if segments and len(segments) > 1:
|
||||
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user