From 54005dd847019be1ef7b16c2527ef7f24d13eeab Mon Sep 17 00:00:00 2001 From: Babissimo Date: Tue, 25 Aug 2026 12:53:18 +0100 Subject: [PATCH 1/5] bridge: accept only forward blah2 timestamps (86cb9bqkh) The duplicate guard tested `ts_ms != last_ts`, which catches an exact repeat but passes an older frame straight through to the tracker. Nothing between `state.frame_queue` and `Tracker.process_frame` re-orders or checks monotonicity, and `process_frame` computes dt by bare subtraction, so an older frame yields a negative dt -- the head of the chain the tracker-side fixes close. `>` is safe here only because `_convert_frame`'s staleness gate already clamps every accepted timestamp to within STALE_THRESHOLD_S of our own clock, which bounds a backwards clock step to a stall of twice that rather than a permanent one. That coupling is not obvious from the call site, so it is recorded there: the gate has to stay above this guard. blah2's timestamps are POSIX epoch from system_clock, not uptime, so a blah2 restart cannot regress them; and at a 0.5 s CPI against 1 ms resolution two distinct frames cannot share a timestamp, so `>` discards nothing that `!=` kept. `blah2_bridge_task` had no test coverage at all -- it is imported only by main.py -- so the guard's behaviour was unobservable in either direction. Four cases now pin it, including that a rejected out-of-order frame does not wedge the node against the next good one. This reduces one source of out-of-order frames; it does not eliminate them. FRAME_WORKERS runs several unkeyed workers over one queue, so two frames from the same node can still reach the same tracker out of order. That is the per-node executor work, deliberately out of this tranche. Co-Authored-By: Claude Opus 5 --- backend/services/blah2_bridge.py | 6 +- backend/tests/test_blah2_bridge.py | 93 ++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 1 deletion(-) diff --git a/backend/services/blah2_bridge.py b/backend/services/blah2_bridge.py index c7c6494f..ff30f468 100644 --- a/backend/services/blah2_bridge.py +++ b/backend/services/blah2_bridge.py @@ -303,7 +303,11 @@ async def blah2_bridge_task(node: Blah2Node): frame = _convert_frame(raw, node.node_id) if frame is not None: ts_ms = raw.get("timestamp", 0) - if ts_ms != last_ts: # skip duplicate frames + # Forward progress only: a repeat or a backwards clock step + # would hand the tracker a non-positive dt. _convert_frame's + # staleness gate must stay above this one - it bounds a + # regression stall to 2 * STALE_THRESHOLD_S rather than forever. + if ts_ms > last_ts: last_ts = ts_ms # Update heartbeat timestamp if node.node_id in state.connected_nodes: diff --git a/backend/tests/test_blah2_bridge.py b/backend/tests/test_blah2_bridge.py index 1bd2d067..0ff5c83e 100644 --- a/backend/tests/test_blah2_bridge.py +++ b/backend/tests/test_blah2_bridge.py @@ -27,6 +27,10 @@ } +async def _noop_register(_node): + """Bypass registration: it touches shared node state the ordering tests do not exercise.""" + + def _write(tmp_path, payload): p = tmp_path / "blah2_nodes.json" p.write_text(json.dumps(payload)) @@ -221,3 +225,92 @@ def test_stale_frame_rejected(self): def test_empty_frame_rejected(self): assert _convert_frame({"timestamp": 0, "delay": []}, "n1") is None + + +# ── Frame ordering ──────────────────────────────────────────────────────────── + + +class _StopPolling(BaseException): + """Ends the bridge's infinite loop; BaseException so its `except Exception` misses it.""" + + +class TestFrameOrdering: + """Only forward timestamps reach the queue. + + A repeat is a cached response and a lower one is a clock step backwards; + either would hand the tracker a non-positive dt. Nothing between the queue + and `Tracker.process_frame` re-orders, so this guard is the only defence + against it on the bridge's path. + """ + + async def _enqueued_timestamps(self, monkeypatch, timestamps): + """Run the bridge over a scripted timestamp sequence; return what it queued.""" + import asyncio + + from core import state + from services import blah2_bridge + + base_ms = int(time.time() * 1000) + raw_frames = [ + { + "timestamp": base_ms + offset_ms, + "delay": [19.86], + "doppler": [-160.62], + "snr": [10.06], + } + for offset_ms in timestamps + ] + + class _Response: + def __init__(self, payload): + self._payload = payload + + def raise_for_status(self): + pass + + def json(self): + return self._payload + + class _Client: + def __init__(self, *_, **__): + self._remaining = list(raw_frames) + + async def __aenter__(self): + return self + + async def __aexit__(self, *_): + return False + + async def get(self, _url): + if not self._remaining: + raise _StopPolling + return _Response(self._remaining.pop(0)) + + monkeypatch.setattr(blah2_bridge.httpx, "AsyncClient", _Client) + monkeypatch.setattr(blah2_bridge, "_register_node", _noop_register) + monkeypatch.setattr(blah2_bridge, "POLL_INTERVAL_S", 0) + monkeypatch.setattr(state, "frame_queue", asyncio.Queue()) + + node = _build_node(MINIMAL) + with pytest.raises(_StopPolling): + await blah2_bridge.blah2_bridge_task(node) + + queued = [] + while not state.frame_queue.empty(): + _node_id, frame = state.frame_queue.get_nowait() + queued.append(frame["timestamp"] - base_ms) + return queued + + async def test_first_frame_passes(self, monkeypatch): + """`last_ts` starts at 0, so no special case is needed for the first frame.""" + assert await self._enqueued_timestamps(monkeypatch, [0]) == [0] + + async def test_repeat_dropped(self, monkeypatch): + assert await self._enqueued_timestamps(monkeypatch, [0, 0, 0]) == [0] + + async def test_older_frame_dropped(self, monkeypatch): + assert await self._enqueued_timestamps(monkeypatch, [1000, 500]) == [1000] + + async def test_newer_frame_after_older_still_passes(self, monkeypatch): + """An out-of-order frame must not wedge the node against later good ones.""" + assert await self._enqueued_timestamps(monkeypatch, [1000, 500, 2000]) == [1000, 2000] From b404fa089010b4f42068a5c1d829a5463ce3d891 Mon Sep 17 00:00:00 2001 From: Babissimo Date: Tue, 25 Aug 2026 12:58:14 +0100 Subject: [PATCH 2/5] pipeline: publish the newest detection's delay, not the oldest (86cb9bqgf) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Track.get_recent_detections` builds its result newest-first and then reverses before returning, so it hands back oldest-first. Nothing on the path from tracker to solver re-sorts: the event writer stores the list verbatim, GeoTrack keeps the same object, and the geolocator only reads it. So `[0]` is the OLDEST detection in the window, not the newest. A comment above the site asserted the opposite and a 2026-08-06 commit changed a correct `[-1]` to `[0]` on the strength of it. Three other places in the estate already treat the same call as oldest-first (frame_processor's `hist[-N:]`, analytics' `hist[-1]`, aircraft_feed's `reversed(meas)`), so the convention was never in doubt anywhere else. `latest_delay_us` builds the ambiguity arc, and for single-node tracks track_gates replaces the icon position with that arc's midpoint. Reading the oldest detection therefore drew the aircraft up to a full window of differential range behind itself, of order 6 km at ~1 µs/s over a 20-detection window. Expected effect, corrected from the audit's framing: the accuracy endpoints will NOT simply improve. `_record_accuracy_sample` and `_refresh_node_verification` both score `solver_lat`/`solver_lon`, which track_gates captures BEFORE the arc-midpoint override, so they never saw the biased position. What does move is delay matching -- truth association gates on `abs(measured - expected) < 15 µs`, and a stale delay drifts outside that window -- so expect n_matched to rise, and mean error possibly with it as tracks that previously failed to match rejoin the sample. The arc is still anchored at the oldest epoch inside the solver itself (lm_solver_track's t0), which is a separate ticket. The fresh-fix injection writes the current position onto that same oldest epoch, so its guess carries a window of lag. Left as it stands, since LM refines the guess, with the ordering recorded in a comment so the site stops reading as though `[0]` meant newest. The test fixtures asserted the inverted convention deliberately, so they had to be rebuilt rather than adjusted: the module docstring stated it as a finding, `_event` took its timestamp from `detections[0]`, and the one ordering-sensitive fixture was written descending. Two cases are added that a single-detection event could not distinguish -- one for the multi-detection materialised path, which is what would have caught the second site, and one for the ADS-B identity evidence derived from the same value, since that authorises calibration points and its derivation was previously uncovered. Co-Authored-By: Claude Opus 5 --- backend/pipeline/passive_radar.py | 49 ++++++++++++++++------- backend/tests/test_geo_delay_freshness.py | 46 ++++++++++++++++----- 2 files changed, 72 insertions(+), 23 deletions(-) diff --git a/backend/pipeline/passive_radar.py b/backend/pipeline/passive_radar.py index 64f95e15..8dda07a6 100644 --- a/backend/pipeline/passive_radar.py +++ b/backend/pipeline/passive_radar.py @@ -44,6 +44,7 @@ FT_TO_M, GEO_INTERVAL_S, PRUNE_INTERVAL_S, + as_num, ) from services.id_utils import passive_track_hex @@ -346,6 +347,21 @@ def _init_geolocator(self, config): self.geo_config.velocity_bounds = list(_DRONE_VELOCITY_BOUNDS) self.geo_config.initial_altitude_m = _DRONE_INITIAL_ALT_M + @staticmethod + def _coerced_adsb(tag): + """Numeric fields of an inline ADS-B tag, safe for the geolocator. + + Everything the geolocator reads off a detection it multiplies bare and + outside the solver's try, so a raw "ground" altitude or a null gs raises + from inside the library and costs the whole frame. + """ + if not tag: + return tag + # Only keys already present: the geolocator branches on `"gs" in adsb`, + # so adding one would change which velocity path it takes. + numeric = ("alt_baro", "gs", "track", "geom_rate") + return {**tag, **{k: as_num(v) for k, v in tag.items() if k in numeric}} + def _geolocate_track_event(self, track_id, event): """Run LM solver on a track event to get lat/lon/alt/velocity.""" # Resolve lazy detection reference if needed (only for tracks that @@ -369,7 +385,7 @@ def _geolocate_track_event(self, track_id, event): delay=d["delay"], doppler=d["doppler"], snr=d.get("snr", 0), - adsb=d.get("adsb"), + adsb=self._coerced_adsb(d.get("adsb")), ) ) @@ -392,12 +408,18 @@ def _geolocate_track_event(self, track_id, event): # select_initial_guess() always starts from fresh coordinates. geo_track.adsb_initialized = True if geo_track.detections: + # Detections are oldest-first: this fix lands on the + # window's oldest epoch, so the guess carries its lag. geo_track.detections[0].adsb = { "lat": _adsb["lat"], "lon": _adsb["lon"], - "alt_baro": _adsb.get("alt_baro", 0), - "gs": _adsb.get("gs", 0), - "track": _adsb.get("track", 0), + # Coerced here, not downstream: the geolocator + # multiplies all three raw, outside the solver's + # try, so a "ground" altitude or a null gs kills + # the frame from inside the library. + "alt_baro": as_num(_adsb.get("alt_baro")), + "gs": as_num(_adsb.get("gs")), + "track": as_num(_adsb.get("track")), } # Generate initial guess @@ -482,12 +504,11 @@ def _geolocate_track_event(self, track_id, event): n_detections=len(geo_detections), timestamp_ms=event["timestamp"], adsb_hex=event.get("adsb_hex"), - # get_recent_detections() is newest-first, so the freshest - # measurement is [0] — [-1] was the OLDEST in the window, which - # published an ambiguity arc up to a full track-history behind - # the target. - latest_delay_us=geo_detections[0].delay if geo_detections else None, - latest_doppler_hz=geo_detections[0].doppler if geo_detections else None, + # get_recent_detections() returns oldest-first, so the freshest + # measurement is [-1]. This builds the ambiguity arc, which is the + # displayed position for single-node tracks. + latest_delay_us=geo_detections[-1].delay if geo_detections else None, + latest_doppler_hz=geo_detections[-1].doppler if geo_detections else None, target_class=target_class, is_anomalous=is_anomalous, anomaly_types=anomaly_types, @@ -537,12 +558,12 @@ def _run_geolocation(self): existing = self.geolocated_tracks.get(track_id) # Newest measurement carried by this event (detections are stored - # newest-first). Used to keep the published delay fresh below and + # oldest-first). Used to keep the published delay fresh below and # to seed the ADS-B bootstrap path so a first-encounter entry # never publishes delay_us=0. _dets = event.get("detections") if _dets: - _newest = _dets[0] # newest-first + _newest = _dets[-1] else: _ref = event.get("_track_ref") _recent = _ref.get_recent_detections(n=1) if _ref is not None else [] @@ -583,7 +604,7 @@ def _run_geolocation(self): if adsb: _gs_ms = (adsb.get("gs", 0) or 0) * 0.514444 _trk = math.radians(adsb.get("track", 0) or 0) - existing.alt_m = (adsb.get("alt_baro", 0) or 0) * FT_TO_M + existing.alt_m = as_num(adsb.get("alt_baro")) * FT_TO_M existing.vel_east = _gs_ms * math.sin(_trk) existing.vel_north = _gs_ms * math.cos(_trk) existing.last_update_ms = event["timestamp"] @@ -638,7 +659,7 @@ def _run_geolocation(self): track_id=track_id, lat=adsb["lat"], lon=adsb["lon"], - alt_m=(adsb.get("alt_baro", 0) or 0) * FT_TO_M, + alt_m=as_num(adsb.get("alt_baro")) * FT_TO_M, vel_east=_gs_ms * math.sin(_trk), vel_north=_gs_ms * math.cos(_trk), vel_up=0.0, diff --git a/backend/tests/test_geo_delay_freshness.py b/backend/tests/test_geo_delay_freshness.py index ab12f40e..8d9c41de 100644 --- a/backend/tests/test_geo_delay_freshness.py +++ b/backend/tests/test_geo_delay_freshness.py @@ -4,12 +4,15 @@ loci built from delays frozen for 20-40 s while the target's true delay slid ~1 µs/s (multi-km apparent position error): -1. get_recent_detections() returns newest-first, but the solver path read - geo_detections[-1] — the OLDEST detection in the window — as "latest". +1. get_recent_detections() returns oldest-first — it builds its result + newest-first and reverses before returning — but the solver path read + geo_detections[0] as "latest", which is the OLDEST detection in the window. 2. latest_delay_us was only written when the (rate-limited, sometimes failing) LM solver produced a fresh GeolocatedTrack; between runs the published delay never moved even though every frame delivered new detections. + +Fixtures here are ordered oldest-first, matching get_recent_detections(). """ from unittest.mock import patch @@ -22,10 +25,10 @@ def _event(track_id, detections, adsb_hex=None): - """Materialized event dict, detections given newest-first.""" + """Materialized event dict, detections given oldest-first.""" return { "track_id": track_id, - "timestamp": detections[0]["timestamp"], + "timestamp": detections[-1]["timestamp"], "length": len(detections), "detections": detections, "adsb_hex": adsb_hex, @@ -36,8 +39,8 @@ def _event(track_id, detections, adsb_hex=None): } -def _det(ts_ms, delay_us, doppler_hz=10.0): - return {"timestamp": ts_ms, "delay": delay_us, "doppler": doppler_hz, "snr": 15.0, "adsb": None} +def _det(ts_ms, delay_us, doppler_hz=10.0, adsb=None): + return {"timestamp": ts_ms, "delay": delay_us, "doppler": doppler_hz, "snr": 15.0, "adsb": adsb} @pytest.fixture() @@ -47,8 +50,8 @@ def pipe(): class TestSolvedTrackUsesNewestDetection: def test_latest_delay_is_newest_not_oldest(self, pipe): - # Newest-first, like get_recent_detections(): 70 µs is the fresh one. - dets = [_det(3000, 70.0, doppler_hz=25.0), _det(2000, 60.0), _det(1000, 50.0)] + # Oldest-first, like get_recent_detections(): 70 µs is the fresh one. + dets = [_det(1000, 50.0), _det(2000, 60.0), _det(3000, 70.0, doppler_hz=25.0)] fake_solution = { "success": True, "state": [1.0, 1.0, 5.0, 100.0, 0.0, 0.0], @@ -84,7 +87,7 @@ def _seed_existing(self, pipe, track_id, delay_us): patch("pipeline.passive_radar.generate_initial_guess", return_value=[1.0, 1.0, 5.0, 0.0, 0.0, 0.0]), ): # min_detections gate: pad the seed event to 3 detections. - seed = [_det(1000, delay_us), _det(900, delay_us), _det(800, delay_us)] + seed = [_det(800, delay_us), _det(900, delay_us), _det(1000, delay_us)] pipe.geolocated_tracks[track_id] = pipe._geolocate_track_event(track_id, _event(track_id, seed)) assert pipe.geolocated_tracks[track_id].latest_delay_us == delay_us pipe._geo_last_solve[track_id] = time.monotonic() # rate limit active @@ -96,6 +99,31 @@ def test_materialized_event_refreshes_delay(self, pipe): assert pipe.geolocated_tracks["trk-2"].latest_delay_us == 71.5 assert pipe.geolocated_tracks["trk-2"].latest_doppler_hz == -4.0 + def test_materialized_multi_detection_event_takes_the_newest(self, pipe): + """A single-detection event cannot tell [0] from [-1]; this one can.""" + self._seed_existing(pipe, "trk-2m", 60.0) + pipe.event_writer.write_event( + "trk-2m", + 5000, + 3, + [_det(3000, 50.0), _det(4000, 60.0), _det(5000, 71.5, doppler_hz=-4.0)], + ) + pipe._run_geolocation() + assert pipe.geolocated_tracks["trk-2m"].latest_delay_us == 71.5 + assert pipe.geolocated_tracks["trk-2m"].latest_doppler_hz == -4.0 + + def test_identity_evidence_comes_from_the_newest_detection(self, pipe): + """last_detection_adsb_hex authorises calibration points, so it must not lag.""" + self._seed_existing(pipe, "trk-2i", 60.0) + pipe.event_writer.write_event( + "trk-2i", + 5000, + 2, + [_det(4000, 60.0, adsb={"hex": "oldhex"}), _det(5000, 71.5, adsb={"hex": "newhex"})], + ) + pipe._run_geolocation() + assert pipe.geolocated_tracks["trk-2i"].last_detection_adsb_hex == "newhex" + def test_lazy_event_refreshes_delay_via_track_ref(self, pipe): self._seed_existing(pipe, "trk-3", 60.0) From b13ab6aa06b439f5007df397bbe56fc7ad26b5b4 Mon Sep 17 00:00:00 2001 From: Babissimo Date: Tue, 25 Aug 2026 13:45:59 +0100 Subject: [PATCH 3/5] pipeline: coerce ADS-B fields before arithmetic on raw ADS-B records (86cb9bqd9) tar1090 reports alt_baro as the literal string "ground" for aircraft on the ground, and readsb sends a null gs for aircraft reporting no velocity. A bare multiply on either raises, and nothing between `_run_geolocation` and `frame_loop`'s catch-all wraps it, so one such aircraft costs the entire frame: track association, the `state.adsb_aircraft` refresh and the archive append are all skipped, and `frames_processed` never advances. The record keeps its freshness stamp, so it stays inside the 60 s window and every subsequent frame dies the same way until it ages out. This is the failure already recorded at analytics_refresh.py and frame_processor.py, which took out /api/test/mlat-accuracy and, on retina-test, the whole map. The audit found two sites. There are four, and the two it missed are the ones that matter, because both hand the value to the geolocator, which multiplies alt_baro, gs and track bare and outside the solver's try: - the fresh-fix injection, which overwrites detection[0]'s tag; - every other detection's inline tag, which is what reaches the solver when the injection does not fire -- no live fix, one older than 60 s, or a hex whose case does not match, since the three writers normalise the key while the tracker carries `adsb["hex"]` raw. Coercing at both boundaries rather than in the geolocator: that is a separate submodule, and this repo should not hand a library a value the library cannot use. The library stays fragile for its other callers, which is a ticket of its own. The tag coercion rewrites only keys already present. The geolocator branches on `"gs" in adsb` to choose its velocity path, so adding a key would change behaviour rather than preserve it. Uses `_as_num` from the seeding path rather than the isinstance guard the audit prescribed, and lifts it to config.constants as `as_num` so there is one definition beside FT_TO_M instead of a private helper reached through a deferred import. It rejects NaN as well as strings; the prescribed variant passes NaN through, which would poison alt_m silently rather than loudly. The audit's third site, in core/state.py, was fixed upstream by e48cde1 the day after the audit was written. `_coerced_adsb` guards on `isinstance(tag, dict)`, not truthiness. A node can put anything in `frame["adsb"][i]` -- passive_radar copies it onto the detection with no type check, and `DetectionRequest.frames` is `list[dict]` with extras allowed, so nothing validates the entries inside a frame; `tcp_handler` already skips non-dicts on the same list. Unpacking a truthy non-dict raises `'str' object is not a mapping` from `_geolocate_track_event`, which sits outside both trys in the file, so the coercion would have caused the very frame loss it exists to prevent. Passing the value through restores what happened before this helper existed: the geolocator's `"lat" not in adsb` is a valid membership test on a string or a list, and yields no ADS-B guess. At the verification refresh the original was `float()`, not a bare multiply, so `as_num` narrows the domain as well as fixing it: a numeric-string altitude now reads as 0 rather than converting. That is deliberate. Every other reader of `state.adsb_aircraft` already resolves a numeric string to 0, so this makes the published truth agree with the altitude the pipeline itself used; and the alternative, a try/except around `float()`, would readmit NaN and inf into `altitude_error_m` silently, which is the failure mode as_num exists to avoid. The ticket's criterion is the whole backend, not the frame path, so the sweep was re-run across it and three more raw multiplies came up. The node-supplied tag in the known lane fails open since e48cde1, so a grounded aircraft there degrades that lane silently and ticks `known_claims_errors` rather than killing the frame. The truth candidate in the per-node verification refresh goes through `float()`, which raises ValueError rather than TypeError, and the caller's blanket except then leaves that node with no verification payload at all. `POST /api/test/validate` guards on truthiness, which "ground" passes. A fourth, the external-cache fallback in `_refresh_mlat_verification`, is coerced for the grep's sake only: that cache is written solely by periodic.py, whose entries carry alt_m and no alt_baro key at all, so there is no failure for a test to reproduce. The geolocator's own multiplies are not in the backend and stay on their own ticket. The two `or 0` sites on gs and track in `_tag_velocity` keep it. Every comment in the estate names alt_baro alone, that matches the readsb schema where the string sentinel is an altitude convention, and the only non-numeric gs or track anywhere in the repo is an invented test fixture. What is different at the two sites changed here is that the value crosses into code that cannot guard itself. `services/blah2_bridge.py` comes off the coverage omit list: it was excluded as needing live blah2 hardware, which the tests added with the timestamp guard show is no longer true. It now reports 89%. Every test here reproduces the production failure when its guard is removed. Co-Authored-By: Claude Opus 5 --- backend/config/constants.py | 16 +- backend/core/state.py | 12 +- backend/pipeline/passive_radar.py | 9 +- backend/pyproject.toml | 2 - backend/routes/test.py | 4 +- backend/services/known_claiming.py | 8 +- backend/services/tasks/analytics_refresh.py | 8 +- backend/tests/test_adsb_ground_sentinel.py | 241 ++++++++++++++++++++ backend/tests/test_known_claiming.py | 31 +++ 9 files changed, 308 insertions(+), 23 deletions(-) create mode 100644 backend/tests/test_adsb_ground_sentinel.py diff --git a/backend/config/constants.py b/backend/config/constants.py index 343add95..89193f3f 100644 --- a/backend/config/constants.py +++ b/backend/config/constants.py @@ -2,11 +2,13 @@ Import from here instead of scattering magic numbers through services. Values that are tunable per-deployment stay as env vars (FRAME_WORKERS, -SOLVER_WORKERS, etc.) — this file is for compile-time constants only. +SOLVER_WORKERS, etc.) — this file holds compile-time constants and the small +pure helpers that operate on them. retina_tracker YAML config stays separate (loaded at runtime via config.yaml). """ +import math import os from core.env_parsing import parse_comma_list @@ -16,6 +18,18 @@ R_EARTH_KM = 6371.0 # Mean Earth radius (km) FT_TO_M = 0.3048 # Feet → metres +# ── Field coercion ─────────────────────────────────────────────────────────── + + +def as_num(v) -> float: + """0.0 for anything that isn't a finite number (ADS-B's "ground", None, NaN). + + tar1090 reports alt_baro as the literal string "ground" for aircraft on the + ground. Coerce before any arithmetic on a raw ADS-B field. + """ + return float(v) if isinstance(v, (int, float)) and math.isfinite(v) else 0.0 + + # ── Association gates ──────────────────────────────────────────────────────── DELAY_MATCH_THRESHOLD_US = 15.0 # Bistatic delay tolerance for matching ASSOC_GRID_STEP_KM = 3.0 # Overlap zone grid resolution (km) diff --git a/backend/core/state.py b/backend/core/state.py index 559be0f7..cbcc7031 100644 --- a/backend/core/state.py +++ b/backend/core/state.py @@ -28,6 +28,7 @@ N2_CONFIRM_MIN_EPOCHS, N2_CONFIRM_MIN_SPAN_S, TRACK_HISTORY_MAX, # noqa: F401 — re-exported, used via state.TRACK_HISTORY_MAX + as_num, ) # ── Coverage / analytics persistence ────────────────────────────────────────── @@ -148,11 +149,6 @@ def _global_tracks_for_claiming(): return out -def _as_num(v) -> float: - """0.0 for anything that isn't a finite number ("ground", None, NaN).""" - return float(v) if isinstance(v, (int, float)) and math.isfinite(v) else 0.0 - - def _adsb_for_seeding() -> dict[str, dict]: """Unlocked snapshot of currently-live ADS-B fixes, in the seeding provider contract InterNodeAssociator documents on adsb_provider. @@ -172,13 +168,13 @@ def _adsb_for_seeding() -> dict[str, dict]: # the literal string "ground" for on-ground aircraft — so coerce # before arithmetic; one such record would otherwise throw here on # every frame for as long as it stays live. - gs_ms = _as_num(rec.get("gs")) * 0.514444 - trk = math.radians(_as_num(rec.get("track"))) + gs_ms = as_num(rec.get("gs")) * 0.514444 + trk = math.radians(as_num(rec.get("track"))) out[hexn] = { "hex": hexn, "lat": lat, "lon": lon, - "alt_m": _as_num(rec.get("alt_baro")) * FT_TO_M, + "alt_m": as_num(rec.get("alt_baro")) * FT_TO_M, "vel_east": gs_ms * math.sin(trk), "vel_north": gs_ms * math.cos(trk), "timestamp_ms": rec.get("last_seen_ms", 0), diff --git a/backend/pipeline/passive_radar.py b/backend/pipeline/passive_radar.py index 8dda07a6..2e157f56 100644 --- a/backend/pipeline/passive_radar.py +++ b/backend/pipeline/passive_radar.py @@ -355,12 +355,13 @@ def _coerced_adsb(tag): outside the solver's try, so a raw "ground" altitude or a null gs raises from inside the library and costs the whole frame. """ - if not tag: + # A node can put anything in frame["adsb"][i]; nothing validates inside + # a frame, and the geolocator tolerates a non-dict where unpacking does not. + if not isinstance(tag, dict): return tag - # Only keys already present: the geolocator branches on `"gs" in adsb`, - # so adding one would change which velocity path it takes. + # Never add a key: the geolocator branches on `"gs" in adsb`. numeric = ("alt_baro", "gs", "track", "geom_rate") - return {**tag, **{k: as_num(v) for k, v in tag.items() if k in numeric}} + return {k: as_num(v) if k in numeric else v for k, v in tag.items()} def _geolocate_track_event(self, track_id, event): """Run LM solver on a track event to get lat/lon/alt/velocity.""" diff --git a/backend/pyproject.toml b/backend/pyproject.toml index c5aed49d..383f513c 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -35,8 +35,6 @@ omit = [ "routes/test.py", # External API clients — require live network; tested via integration only: "clients/fcc.py", - # blah2 bridge — requires live blah2 hardware/daemon: - "services/blah2_bridge.py", # Pure TypedDict definitions — no runtime code to cover: "core/types.py", ] diff --git a/backend/routes/test.py b/backend/routes/test.py index a11e6de0..852daa4f 100644 --- a/backend/routes/test.py +++ b/backend/routes/test.py @@ -10,6 +10,7 @@ from fastapi import APIRouter, Body, Depends, Header, HTTPException from fastapi.responses import Response +from config.constants import FT_TO_M, as_num from core import state from core.task_registry import get_stale_tasks from core.users import require_admin @@ -302,7 +303,8 @@ async def validate_ground_truth(body: dict = Body(...), _key=Depends(_verify_sim if best_match: idx, sa = best_match matched_server_indices.add(idx) - sa_alt_m = sa.get("alt_baro", 0) * 0.3048 if sa.get("alt_baro") else 0 + # Truthiness is no guard: "ground", the on-deck sentinel, is truthy. + sa_alt_m = as_num(sa.get("alt_baro")) * FT_TO_M alt_err_m = abs(gt_alt - sa_alt_m) matches.append( { diff --git a/backend/services/known_claiming.py b/backend/services/known_claiming.py index 068bca00..ac56c3bf 100644 --- a/backend/services/known_claiming.py +++ b/backend/services/known_claiming.py @@ -51,7 +51,7 @@ from retina_analytics.constants import offset_latlon_m from scipy.optimize import linear_sum_assignment -from config.constants import FT_TO_M +from config.constants import FT_TO_M, as_num from core import state from services.id_utils import normalize_hex_key @@ -219,9 +219,9 @@ def claim_known_targets(node_id: str, frame: dict) -> set[int]: ve, vn = _tag_velocity(tag) # The node correlated this fix against this frame, so the fix is # taken as current — no dead-reckoning, fix_ts_ms = frame time. - pred_d, pred_f = predict_observation( - geo, lat, lon, (tag.get("alt_baro", 0) or 0) * FT_TO_M / 1000.0, ve, vn - ) + # Raw node input: alt_baro is the string "ground" on the deck. + alt_km = as_num(tag.get("alt_baro")) * FT_TO_M / 1000.0 + pred_d, pred_f = predict_observation(geo, lat, lon, alt_km, ve, vn) fix = { "lat": lat, "lon": lon, diff --git a/backend/services/tasks/analytics_refresh.py b/backend/services/tasks/analytics_refresh.py index 61145282..0483d28c 100644 --- a/backend/services/tasks/analytics_refresh.py +++ b/backend/services/tasks/analytics_refresh.py @@ -10,7 +10,7 @@ import numpy as np import orjson -from config.constants import ANALYTICS_REFRESH_INTERVAL_S, CLAIMED_DISPLAY_FRESH_S +from config.constants import ANALYTICS_REFRESH_INTERVAL_S, CLAIMED_DISPLAY_FRESH_S, as_num from config.constants import ( DELAY_MATCH_THRESHOLD_US as _DELAY_MATCH_THRESHOLD_US, ) @@ -925,7 +925,9 @@ def _refresh_node_verification(node_id: str): _alt_m = best_adsb.get("alt_m") _gs_kt = best_adsb.get("gs") _vel_ms = best_adsb.get("velocity") - truth_alt_m = float(_alt_ft) * 0.3048 if _alt_ft is not None else float(_alt_m) if _alt_m is not None else None + # alt_baro alone carries the "ground" sentinel; a raise here loses the + # node's whole payload. Numeric strings read as 0, as they do estate-wide. + truth_alt_m = as_num(_alt_ft) * 0.3048 if _alt_ft is not None else float(_alt_m) if _alt_m is not None else None truth_gs_ms = ( float(_gs_kt) * 0.514444 if _gs_kt is not None else float(_vel_ms) if _vel_ms is not None else None ) @@ -1273,7 +1275,7 @@ def _refresh_mlat_verification(): # heading} (periodic.py) — NOT the tar1090 gs/alt_baro schema. # Reading gs/alt_baro here zeroed every external truth entry. gs_ms = float(entry["velocity"] if entry.get("velocity") is not None else (entry.get("gs") or 0) * 0.514444) - alt_m = float(entry["alt_m"] if entry.get("alt_m") is not None else (entry.get("alt_baro") or 0) * 0.3048) + alt_m = float(entry["alt_m"] if entry.get("alt_m") is not None else as_num(entry.get("alt_baro")) * 0.3048) adsb_truth_pool.append( ( adsb_hex, diff --git a/backend/tests/test_adsb_ground_sentinel.py b/backend/tests/test_adsb_ground_sentinel.py new file mode 100644 index 00000000..50c2d0a0 --- /dev/null +++ b/backend/tests/test_adsb_ground_sentinel.py @@ -0,0 +1,241 @@ +"""tar1090 reports alt_baro as the string "ground"; the backend must survive it. + +A bare multiply on that field raises TypeError, and nothing between +`_run_geolocation` and `frame_loop` catches it, so one grounded aircraft +costs the whole frame: track association, the state.adsb_aircraft refresh +and the archive append are all skipped. The record keeps its freshness +stamp, so every subsequent frame dies the same way until it ages out. + +Four sites sit on the frame path: the inline tag at the GeoDetection +boundary, the fresh fix injected into the initial guess, the between-solves +altitude refresh and the ADS-B bootstrap after a solver failure. Two more +read the same raw record off it — the per-node verification refresh and +POST /api/test/validate — and are covered by the second class here; the +node-tag site in the known lane is covered in test_known_claiming.py. +""" + +import time +from types import SimpleNamespace +from unittest.mock import patch + +import orjson +import pytest + +from core import state +from pipeline.passive_radar import DEFAULT_NODE_CONFIG, PassiveRadarPipeline +from routes.test import validate_ground_truth +from services.geo import bistatic_delay_us +from services.tasks.analytics_refresh import _refresh_node_verification + +_NODE_CONFIG = {**DEFAULT_NODE_CONFIG, "node_id": "ground-sentinel-node"} +_HEX = "gnd001" + +_SOLUTION = { + "success": True, + "state": [1.0, 1.0, 5.0, 100.0, 0.0, 0.0], + "rms_delay": 0.1, + "rms_doppler": 0.1, +} + + +@pytest.fixture() +def pipe(): + return PassiveRadarPipeline(_NODE_CONFIG) + + +@pytest.fixture() +def grounded_aircraft(): + """A live ADS-B fix for an aircraft on the ground, stored as every writer stores it.""" + previous = state.adsb_aircraft.get(_HEX) + state.adsb_aircraft[_HEX] = { + "hex": _HEX, + "lat": _NODE_CONFIG["rx_lat"] + 0.05, + "lon": _NODE_CONFIG["rx_lon"] + 0.05, + "alt_baro": "ground", + "gs": 0, + "track": 0, + "last_seen_ms": int(time.time() * 1000), + } + yield + if previous is None: + state.adsb_aircraft.pop(_HEX, None) + else: + state.adsb_aircraft[_HEX] = previous + # _run_geolocation publishes here and never retracts, so the fixture must. + with state.geo_aircraft_lock: + state.active_geo_aircraft.pop(_HEX, None) + + +def _det(ts_ms, delay_us): + return {"timestamp": ts_ms, "delay": delay_us, "doppler": 10.0, "snr": 15.0, "adsb": None} + + +def _event(track_id, adsb_hex=_HEX): + dets = [_det(1000, 50.0), _det(2000, 60.0), _det(3000, 70.0)] + return { + "track_id": track_id, + "timestamp": dets[-1]["timestamp"], + "length": len(dets), + "detections": dets, + "adsb_hex": adsb_hex, + "adsb_initialized": False, + "is_anomalous": False, + "max_velocity_ms": 0.0, + "anomaly_types": [], + } + + +class TestGroundSentinelOnTheFramePath: + def test_initial_guess_injection_does_not_kill_the_frame(self, pipe, grounded_aircraft): + """The freshest fix is injected raw into the guess; the geolocator multiplies it + outside the solver's try, so it must be coerced before it leaves this repo.""" + pipe._geolocate_track_event("trk-g1", _event("trk-g1")) # must not raise + + def test_inline_tag_survives_when_the_fresh_injection_does_not_fire(self): + """The injection only overwrites detection[0] when a live fix exists. + + With no entry in state.adsb_aircraft -- stale, absent, or a hex whose case + does not match -- the tracker's own inline tag is what reaches the + geolocator, still raw. + """ + pipe = PassiveRadarPipeline(_NODE_CONFIG) + state.adsb_aircraft.pop(_HEX, None) + + event = _event("trk-g5") + # The geolocator reads the tag only on an ADS-B-initialized track, and + # returns early unless lat/lon and track are present, so all three are + # needed to reach the alt_baro and gs multiplies. + event["adsb_initialized"] = True + event["detections"][0]["adsb"] = { + "hex": _HEX, + "lat": _NODE_CONFIG["rx_lat"] + 0.05, + "lon": _NODE_CONFIG["rx_lon"] + 0.05, + "alt_baro": "ground", + "gs": None, + "track": 0, + } + + pipe._geolocate_track_event("trk-g5", event) # must not raise + + @pytest.mark.parametrize("bad", ["ground", ["gnd001"]]) + def test_a_non_dict_tag_is_passed_through_not_unpacked(self, bad): + """Nothing type-checks the tag: `frame["adsb"][i]` is copied onto the + detection raw, and DetectionRequest.frames is `list[dict]` with extras + allowed, so the entries inside a frame are unvalidated. The geolocator + tolerates a non-dict -- `"lat" not in adsb` is a valid membership test + on a string or a list -- so coercion must not be the thing that raises. + """ + assert PassiveRadarPipeline._coerced_adsb(bad) is bad + + pipe = PassiveRadarPipeline(_NODE_CONFIG) + state.adsb_aircraft.pop(_HEX, None) + event = _event("trk-g6") + event["adsb_initialized"] = True + event["detections"][0]["adsb"] = bad + + pipe._geolocate_track_event("trk-g6", event) # must not raise + + def test_coercion_leaves_absent_keys_absent(self): + """The geolocator branches on `"gs" in adsb`, so a key must not appear.""" + coerced = PassiveRadarPipeline._coerced_adsb({"hex": "abc123", "alt_baro": "ground"}) + + assert coerced == {"hex": "abc123", "alt_baro": 0.0} + + def test_null_ground_speed_does_not_kill_the_frame(self, pipe, grounded_aircraft): + """readsb omits velocity for some aircraft; the geolocator multiplies gs raw too.""" + state.adsb_aircraft[_HEX] = {**state.adsb_aircraft[_HEX], "gs": None, "track": None} + + pipe._geolocate_track_event("trk-g4", _event("trk-g4")) # must not raise + + def test_refresh_between_solves_coerces_to_zero(self, pipe, grounded_aircraft): + """Between solves the track keeps its position but refreshes altitude from ADS-B.""" + with ( + patch("pipeline.passive_radar.solve_track", return_value=_SOLUTION), + patch( + "pipeline.passive_radar.select_initial_guess", + return_value=([1.0, 1.0, 5.0, 0.0, 0.0, 0.0], "beam"), + ), + patch( + "pipeline.passive_radar.generate_initial_guess", + return_value=[1.0, 1.0, 5.0, 0.0, 0.0, 0.0], + ), + ): + seeded = pipe._geolocate_track_event("trk-g2", _event("trk-g2")) + # Fixture guard: a None seed would fail the refresh assert below as an + # AttributeError, blaming the coercion for a broken set-up. + assert seeded is not None + pipe.geolocated_tracks["trk-g2"] = seeded + + pipe._geo_last_solve["trk-g2"] = time.monotonic() # rate limit active: refresh, do not re-solve + pipe.event_writer.write_event("trk-g2", 5000, 1, [_det(5000, 71.5)], adsb_hex=_HEX) + pipe._run_geolocation() + + assert pipe.geolocated_tracks["trk-g2"].alt_m == 0.0 + + def test_adsb_bootstrap_after_solver_failure_coerces_to_zero(self, pipe, grounded_aircraft): + """Solver fails on first encounter, so the track is built from ADS-B alone.""" + with patch("pipeline.passive_radar.solve_track", return_value={"success": False}): + pipe.event_writer.write_event("trk-g3", 3000, 3, _event("trk-g3")["detections"], adsb_hex=_HEX) + pipe._run_geolocation() + + track = pipe.geolocated_tracks.get("trk-g3") + assert track is not None + assert track.alt_m == 0.0 + + +_VERIFY_NODE_ID = "ground-sentinel-verify" +_RX = (34.85, -82.40) +_TX = (34.90, -82.20) +_TARGET = (34.88, -82.35) + + +class TestGroundSentinelOffTheFramePath: + """Both consumers read alt_baro straight off a state.adsb_aircraft record.""" + + def test_node_verification_survives_a_grounded_truth_candidate(self): + """float("ground") raises ValueError, and the caller's blanket except + turns that into a node with no verification payload at all.""" + now = time.time() + state.adsb_aircraft[_HEX] = { + "hex": _HEX, + "lat": _TARGET[0], + "lon": _TARGET[1], + "alt_baro": "ground", + "gs": 120, + "track": 90, + "last_seen_ms": int(now * 1000), + } + track = SimpleNamespace( + latest_delay_us=bistatic_delay_us(_TX[0], _TX[1], _RX[0], _RX[1], _TARGET[0], _TARGET[1]), + wall_clock_ts=now, + lat=_TARGET[0], + lon=_TARGET[1], + vel_east=0.0, + vel_north=0.0, + alt_m=3000.0, + ) + cfg = {"node_id": _VERIFY_NODE_ID, "rx_lat": _RX[0], "rx_lon": _RX[1], "tx_lat": _TX[0], "tx_lon": _TX[1]} + with state.geo_aircraft_lock: + state.active_geo_aircraft["gnd-trk"] = (track, cfg) + + _refresh_node_verification(_VERIFY_NODE_ID) + + data = orjson.loads(state.latest_node_verification_bytes[_VERIFY_NODE_ID]) + assert data["n_matched"] == 1 + (m,) = data["tracks"] + assert m["truth_alt_m"] == 0.0 + assert m["altitude_error_m"] == 3000.0 + + async def test_validate_ground_truth_survives_a_grounded_aircraft(self, monkeypatch): + """A truthiness test is no guard here — "ground" is truthy.""" + monkeypatch.setattr( + state, + "latest_aircraft_json", + {"aircraft": [{"hex": _HEX, "lat": _TARGET[0], "lon": _TARGET[1], "alt_baro": "ground"}]}, + ) + body = {"ground_truth": [{"id": "gt1", "lat": _TARGET[0], "lon": _TARGET[1], "alt_km": 1.0}]} + + result = await validate_ground_truth(body=body, _key=None) + + assert result["validation"]["matched"] == 1 + assert result["accuracy"]["avg_altitude_error_m"] == 1000 diff --git a/backend/tests/test_known_claiming.py b/backend/tests/test_known_claiming.py index c0dc5897..08adda85 100644 --- a/backend/tests/test_known_claiming.py +++ b/backend/tests/test_known_claiming.py @@ -248,6 +248,37 @@ def test_tag_is_not_regated(self): assert rec["pred_delay_us"] != 9999.0 # prediction recorded for the residual path +class TestGroundSentinelInTags: + """A node tag is raw feed data, so alt_baro can be the string "ground".""" + + def test_grounded_tag_still_claims(self): + _register() + ts = int(time.time() * 1000) + tag = {"hex": "gnd001", "lat": _LAT, "lon": _LON, "alt_baro": "ground", "gs": 0, "track": 0} + + claimed = kc.claim_known_targets(_NODE_ID, _frame(ts, [50.0], [10.0], adsb=[tag])) + + assert claimed == {0} + # Sea level for the prediction; the fix keeps the sentinel verbatim. + assert state.known_claims["gnd001"][-1]["adsb_fix"]["alt_baro"] == "ground" + + def test_grounded_tag_does_not_silently_disable_the_lane(self, monkeypatch): + """The stage fails open, so a raise here is invisible except as a + counter tick and a claim that never happened.""" + _register() + monkeypatch.setattr(state, "KNOWN_LANE_MODE", "shadow") + ts = int(time.time() * 1000) + tag = {"hex": "gnd002", "lat": _LAT, "lon": _LON, "alt_baro": "ground", "gs": 0, "track": 0} + errors_before = state.known_claims_errors + + default = PassiveRadarPipeline(DEFAULT_NODE_CONFIG) + monkeypatch.setattr(default, "process_frame", lambda f: None) + process_one_frame(_NODE_ID, _frame(ts, [50.0], [10.0], adsb=[tag]), default) + + assert state.known_claims_errors == errors_before + assert "gnd002" in state.known_claims + + class TestContention: def _global(self, key, ts_ms, n_nodes, solve_count): state.multinode_tracks[key] = { From 1817caaff48a06b30a404bafd4f9c2f3ccb18eb6 Mon Sep 17 00:00:00 2001 From: Babissimo Date: Tue, 25 Aug 2026 16:14:39 +0100 Subject: [PATCH 4/5] verification: drop grounded aircraft from the altitude truth term (86cb9ut1w) tar1090 reports alt_baro as "ground" for an aircraft on the surface. That is a statement about position, not altitude: the aircraft is at field elevation, roughly 313 m at Atlanta and 0 m almost nowhere. Coercing the sentinel to 0.0 is sound for a solver seed, where the value is a starting point the solve moves off, and wrong at the two sites that score the solver against it, where it charges the whole of the field elevation to altitude error and drags the published mean down with it. Before this branch, float("ground") raised and the caller's blanket except dropped the node's entire verification payload, so a grounded aircraft never reached altitude_error_m at all. Coercing the sentinel silently admitted a class of aircraft the accuracy sample had never contained. This restores that exclusion deliberately, and narrows it to the altitude term alone: a grounded aircraft is good position and velocity truth, so it stays in those samples, and only the altitude comparison skips it. The predicate is is_num() on the raw field, not on as_num()'s output, which flattens absent, non-numeric and a genuine 0 ft to the same 0.0. It is not keyed on the literal "ground": under tar1090's schema a present alt_baro that is not a finite number is the sentinel, and for a truth comparison dropping any other non-numeric value is the conservative reading. as_num() is now defined through is_num() so the two cannot drift. Exclusion is null in both payloads, matching the else-None arm truth_alt_m already carried. POST /api/test/validate gains n_altitude_samples, since its mean now runs over a denominator smaller than the match count, and its altitude aggregates report null rather than 0 when nothing carried altitude truth: 0 m of error would read as a perfect altitude solve. The frontend is unaffected. Its one altitude_error_m reader takes the MLAT payload, built by a separate emitter from its own truth pool. Co-Authored-By: Claude Opus 5 --- backend/config/constants.py | 15 ++- backend/routes/test.py | 36 +++--- backend/services/tasks/analytics_refresh.py | 15 ++- backend/tests/test_adsb_ground_sentinel.py | 120 +++++++++++++++++--- 4 files changed, 152 insertions(+), 34 deletions(-) diff --git a/backend/config/constants.py b/backend/config/constants.py index 89193f3f..708994b5 100644 --- a/backend/config/constants.py +++ b/backend/config/constants.py @@ -21,13 +21,24 @@ # ── Field coercion ─────────────────────────────────────────────────────────── +def is_num(v) -> bool: + """True when v is a finite number and arithmetic on it is meaningful. + + The complement of as_num()'s fallback: a field this rejects carries no + measurement, so a truth comparison must drop the sample rather than score + against the substituted 0.0. + """ + return isinstance(v, (int, float)) and math.isfinite(v) + + def as_num(v) -> float: """0.0 for anything that isn't a finite number (ADS-B's "ground", None, NaN). tar1090 reports alt_baro as the literal string "ground" for aircraft on the - ground. Coerce before any arithmetic on a raw ADS-B field. + ground. Coerce before any arithmetic on a raw ADS-B field. Sound for a + solver seed; use is_num() where the value is compared against truth. """ - return float(v) if isinstance(v, (int, float)) and math.isfinite(v) else 0.0 + return float(v) if is_num(v) else 0.0 # ── Association gates ──────────────────────────────────────────────────────── diff --git a/backend/routes/test.py b/backend/routes/test.py index 852daa4f..689c1426 100644 --- a/backend/routes/test.py +++ b/backend/routes/test.py @@ -10,7 +10,7 @@ from fastapi import APIRouter, Body, Depends, Header, HTTPException from fastapi.responses import Response -from config.constants import FT_TO_M, as_num +from config.constants import FT_TO_M, is_num from core import state from core.task_registry import get_stale_tasks from core.users import require_admin @@ -303,15 +303,17 @@ async def validate_ground_truth(body: dict = Body(...), _key=Depends(_verify_sim if best_match: idx, sa = best_match matched_server_indices.add(idx) - # Truthiness is no guard: "ground", the on-deck sentinel, is truthy. - sa_alt_m = as_num(sa.get("alt_baro")) * FT_TO_M - alt_err_m = abs(gt_alt - sa_alt_m) + # "ground" means on the surface — field elevation, not 0 m MSL — so + # it is no altitude truth. Null here rather than a fabricated error; + # the match still scores position. + _sa_alt = sa.get("alt_baro") + alt_err_m = abs(gt_alt - _sa_alt * FT_TO_M) if is_num(_sa_alt) else None matches.append( { "truth_id": gt.get("id"), "server_hex": sa.get("hex"), "position_error_km": round(best_dist, 2), - "altitude_error_m": round(alt_err_m, 0), + "altitude_error_m": round(alt_err_m, 0) if alt_err_m is not None else None, "position_source": sa.get("position_source", "unknown"), "has_adsb": gt.get("has_adsb", False), "is_anomalous": gt.get("is_anomalous", False), @@ -324,21 +326,28 @@ async def validate_ground_truth(body: dict = Body(...), _key=Depends(_verify_sim if matches: pos_errors = [m["position_error_km"] for m in matches] - alt_errors = [m["altitude_error_m"] for m in matches] avg_pos_err = sum(pos_errors) / len(pos_errors) - avg_alt_err = sum(alt_errors) / len(alt_errors) max_pos_err = max(pos_errors) accuracy_pct = len(matches) / len(truth_list) * 100 sorted_pos = sorted(pos_errors) p50_pos = sorted_pos[len(sorted_pos) // 2] p95_pos = sorted_pos[int(len(sorted_pos) * 0.95)] + else: + avg_pos_err = max_pos_err = 0 + p50_pos = p95_pos = 0 + accuracy_pct = 0 + + # Denominator is the matches that HAD altitude truth, not all of them, so + # these stand apart from the position ones. Null rather than 0 when none + # did: 0 m of altitude error reads as perfect accuracy. + alt_errors = [m["altitude_error_m"] for m in matches if m["altitude_error_m"] is not None] + if alt_errors: + avg_alt_err = sum(alt_errors) / len(alt_errors) sorted_alt = sorted(alt_errors) p50_alt = sorted_alt[len(sorted_alt) // 2] p95_alt = sorted_alt[int(len(sorted_alt) * 0.95)] else: - avg_pos_err = avg_alt_err = max_pos_err = 0 - p50_pos = p95_pos = p50_alt = p95_alt = 0 - accuracy_pct = 0 + avg_alt_err = p50_alt = p95_alt = None # Per-position_source breakdown by_source: dict[str, list[float]] = {} @@ -370,9 +379,10 @@ async def validate_ground_truth(body: dict = Body(...), _key=Depends(_verify_sim "median_position_error_km": round(p50_pos, 2), "p95_position_error_km": round(p95_pos, 2), "max_position_error_km": round(max_pos_err, 2), - "avg_altitude_error_m": round(avg_alt_err, 0), - "median_altitude_error_m": round(p50_alt, 0), - "p95_altitude_error_m": round(p95_alt, 0), + "n_altitude_samples": len(alt_errors), + "avg_altitude_error_m": round(avg_alt_err, 0) if avg_alt_err is not None else None, + "median_altitude_error_m": round(p50_alt, 0) if p50_alt is not None else None, + "p95_altitude_error_m": round(p95_alt, 0) if p95_alt is not None else None, }, "by_source": source_breakdown, "matches": matches[:50], diff --git a/backend/services/tasks/analytics_refresh.py b/backend/services/tasks/analytics_refresh.py index 0483d28c..4dcd5230 100644 --- a/backend/services/tasks/analytics_refresh.py +++ b/backend/services/tasks/analytics_refresh.py @@ -10,7 +10,7 @@ import numpy as np import orjson -from config.constants import ANALYTICS_REFRESH_INTERVAL_S, CLAIMED_DISPLAY_FRESH_S, as_num +from config.constants import ANALYTICS_REFRESH_INTERVAL_S, CLAIMED_DISPLAY_FRESH_S, as_num, is_num from config.constants import ( DELAY_MATCH_THRESHOLD_US as _DELAY_MATCH_THRESHOLD_US, ) @@ -925,9 +925,16 @@ def _refresh_node_verification(node_id: str): _alt_m = best_adsb.get("alt_m") _gs_kt = best_adsb.get("gs") _vel_ms = best_adsb.get("velocity") - # alt_baro alone carries the "ground" sentinel; a raise here loses the - # node's whole payload. Numeric strings read as 0, as they do estate-wide. - truth_alt_m = as_num(_alt_ft) * 0.3048 if _alt_ft is not None else float(_alt_m) if _alt_m is not None else None + # A non-numeric alt_baro is tar1090's "ground" sentinel: on the surface, + # which is field elevation, not 0 m MSL. It is no altitude truth, so the + # candidate falls through to the metric key and, failing that, out of the + # altitude stats — as an absent speed field does out of the velocity ones. + if is_num(_alt_ft): + truth_alt_m = _alt_ft * 0.3048 + elif _alt_m is not None: + truth_alt_m = float(_alt_m) + else: + truth_alt_m = None truth_gs_ms = ( float(_gs_kt) * 0.514444 if _gs_kt is not None else float(_vel_ms) if _vel_ms is not None else None ) diff --git a/backend/tests/test_adsb_ground_sentinel.py b/backend/tests/test_adsb_ground_sentinel.py index 50c2d0a0..814d8c61 100644 --- a/backend/tests/test_adsb_ground_sentinel.py +++ b/backend/tests/test_adsb_ground_sentinel.py @@ -189,8 +189,32 @@ def test_adsb_bootstrap_after_solver_failure_coerces_to_zero(self, pipe, grounde _TARGET = (34.88, -82.35) +_AIR_HEX = "air002" +_AIR_TARGET = (35.05, -82.35) # 82 µs of bistatic delay from _TARGET: no cross-matching +_NODE_CFG = {"node_id": _VERIFY_NODE_ID, "rx_lat": _RX[0], "rx_lon": _RX[1], "tx_lat": _TX[0], "tx_lon": _TX[1]} + + +def _verify_track(target, alt_m, speed_e=0.0): + return SimpleNamespace( + latest_delay_us=bistatic_delay_us(_TX[0], _TX[1], _RX[0], _RX[1], target[0], target[1]), + wall_clock_ts=time.time(), + lat=target[0], + lon=target[1], + vel_east=speed_e, + vel_north=0.0, + alt_m=alt_m, + ) + + class TestGroundSentinelOffTheFramePath: - """Both consumers read alt_baro straight off a state.adsb_aircraft record.""" + """Both consumers read alt_baro straight off a state.adsb_aircraft record. + + "ground" is a position report, not an altitude one: the aircraft is at field + elevation, which is 313 m at Atlanta and 0 m nowhere in particular. Scoring + the solver against 0 m MSL invents an error the size of the aerodrome, so a + grounded aircraft is dropped from the altitude comparison and kept for the + position and velocity ones. + """ def test_node_verification_survives_a_grounded_truth_candidate(self): """float("ground") raises ValueError, and the caller's blanket except @@ -205,26 +229,57 @@ def test_node_verification_survives_a_grounded_truth_candidate(self): "track": 90, "last_seen_ms": int(now * 1000), } - track = SimpleNamespace( - latest_delay_us=bistatic_delay_us(_TX[0], _TX[1], _RX[0], _RX[1], _TARGET[0], _TARGET[1]), - wall_clock_ts=now, - lat=_TARGET[0], - lon=_TARGET[1], - vel_east=0.0, - vel_north=0.0, - alt_m=3000.0, - ) - cfg = {"node_id": _VERIFY_NODE_ID, "rx_lat": _RX[0], "rx_lon": _RX[1], "tx_lat": _TX[0], "tx_lon": _TX[1]} with state.geo_aircraft_lock: - state.active_geo_aircraft["gnd-trk"] = (track, cfg) + state.active_geo_aircraft["gnd-trk"] = (_verify_track(_TARGET, 3000.0), _NODE_CFG) _refresh_node_verification(_VERIFY_NODE_ID) data = orjson.loads(state.latest_node_verification_bytes[_VERIFY_NODE_ID]) assert data["n_matched"] == 1 (m,) = data["tracks"] - assert m["truth_alt_m"] == 0.0 - assert m["altitude_error_m"] == 3000.0 + # No altitude truth: null, not the 3 000 m error that scoring against + # 0 m MSL produced. + assert m["truth_alt_m"] is None + assert m["altitude_error_m"] is None + assert data["altitude"]["n"] == 0 + # Position and velocity truth are unaffected — the point of not dropping + # the record wholesale. + assert m["position_error_km"] < 0.1 + assert m["truth_speed_ms"] == pytest.approx(61.7, abs=0.1) + assert data["velocity"]["n"] == 1 + + def test_node_verification_altitude_stats_come_from_the_airborne_match_alone(self): + """A grounded aircraft must not dilute the altitude stats, nor suppress them.""" + now = int(time.time() * 1000) + state.adsb_aircraft[_HEX] = { + "hex": _HEX, + "lat": _TARGET[0], + "lon": _TARGET[1], + "alt_baro": "ground", + "gs": 0, + "last_seen_ms": now, + } + state.adsb_aircraft[_AIR_HEX] = { + "hex": _AIR_HEX, + "lat": _AIR_TARGET[0], + "lon": _AIR_TARGET[1], + "alt_baro": 33000, # 10 058.4 m + "gs": 0, + "last_seen_ms": now, + } + with state.geo_aircraft_lock: + state.active_geo_aircraft["gnd-trk"] = (_verify_track(_TARGET, 3000.0), _NODE_CFG) + state.active_geo_aircraft["air-trk"] = (_verify_track(_AIR_TARGET, 10000.0), _NODE_CFG) + + _refresh_node_verification(_VERIFY_NODE_ID) + + data = orjson.loads(state.latest_node_verification_bytes[_VERIFY_NODE_ID]) + assert data["n_matched"] == 2 + by_hex = {m["matched_adsb_hex"]: m for m in data["tracks"]} + assert by_hex[_HEX]["altitude_error_m"] is None + assert by_hex[_AIR_HEX]["altitude_error_m"] == 58.0 + assert data["altitude"]["n"] == 1 + assert data["altitude"]["mean_m"] == 58.0 async def test_validate_ground_truth_survives_a_grounded_aircraft(self, monkeypatch): """A truthiness test is no guard here — "ground" is truthy.""" @@ -238,4 +293,39 @@ async def test_validate_ground_truth_survives_a_grounded_aircraft(self, monkeypa result = await validate_ground_truth(body=body, _key=None) assert result["validation"]["matched"] == 1 - assert result["accuracy"]["avg_altitude_error_m"] == 1000 + (m,) = result["matches"] + assert m["position_error_km"] == 0.0 + # Null, not the 1 000 m of invented error, and not 0 — which would read + # as a perfect altitude solve. + assert m["altitude_error_m"] is None + assert result["accuracy"]["n_altitude_samples"] == 0 + assert result["accuracy"]["avg_altitude_error_m"] is None + assert result["accuracy"]["median_altitude_error_m"] is None + assert result["accuracy"]["p95_altitude_error_m"] is None + + async def test_validate_altitude_stats_come_from_the_airborne_match_alone(self, monkeypatch): + monkeypatch.setattr( + state, + "latest_aircraft_json", + { + "aircraft": [ + {"hex": _HEX, "lat": _TARGET[0], "lon": _TARGET[1], "alt_baro": "ground"}, + {"hex": _AIR_HEX, "lat": _AIR_TARGET[0], "lon": _AIR_TARGET[1], "alt_baro": 10000}, + ] + }, + ) + body = { + "ground_truth": [ + {"id": "gt-gnd", "lat": _TARGET[0], "lon": _TARGET[1], "alt_km": 1.0}, + {"id": "gt-air", "lat": _AIR_TARGET[0], "lon": _AIR_TARGET[1], "alt_km": 3.0}, + ] + } + + result = await validate_ground_truth(body=body, _key=None) + + assert result["validation"]["matched"] == 2 + by_id = {m["truth_id"]: m for m in result["matches"]} + assert by_id["gt-gnd"]["altitude_error_m"] is None + assert by_id["gt-air"]["altitude_error_m"] == 48.0 # 10 000 ft = 3 048 m + assert result["accuracy"]["n_altitude_samples"] == 1 + assert result["accuracy"]["avg_altitude_error_m"] == 48.0 From d3da82f8c7244f7ec1da418ab6dbf6e094d9a4bd Mon Sep 17 00:00:00 2001 From: Babissimo Date: Tue, 25 Aug 2026 17:46:17 +0100 Subject: [PATCH 5/5] analytics: use one coercion for both MLAT truth pools (86cb9bqd9) `_refresh_mlat_verification` builds its truth pool from two sources. The external one was moved onto `as_num` earlier in this branch; the live one, twenty lines above it, was left on a hand-rolled isinstance test. That split was introduced here, so it is closed here. The two are not equivalent. `isinstance(v, (int, float))` admits NaN and infinity, which is exactly what `as_num` exists to reject and the reason this branch chose it over the guard the audit prescribed. `json.loads` parses a bare `NaN` literal by default, so a node emitting non-standard JSON puts a genuine float NaN into `state.adsb_aircraft`, and the live pool reads that store directly. A NaN in the truth pool does not raise. It propagates through every comparison as NaN, so the aircraft still matches, still publishes, and the altitude and velocity errors on /api/test/mlat-verification go quietly non-numeric. The regression test asserts the four published figures stay finite, and fails against the isinstance form. Also drops the incident narration from the comment above it, keeping only the constraint that still binds: the sentinel arrives, and an isinstance test lets NaN through. The history is in git. `_tag_velocity`'s docstring claimed it applied the same conversion as `state._adsb_for_seeding`. Moving that function onto `as_num` earlier in this branch made the claim false, so it now says what it actually does and points at the ticket that would close the gap. Both found by the automated review on the PR. Co-Authored-By: Claude Opus 5 --- backend/services/known_claiming.py | 7 +++-- backend/services/tasks/analytics_refresh.py | 11 +++----- backend/tests/test_mlat_verification.py | 29 +++++++++++++++++++++ 3 files changed, 38 insertions(+), 9 deletions(-) diff --git a/backend/services/known_claiming.py b/backend/services/known_claiming.py index ac56c3bf..07fe8924 100644 --- a/backend/services/known_claiming.py +++ b/backend/services/known_claiming.py @@ -113,8 +113,11 @@ def _gate_scale(age_s: float) -> float: def _tag_velocity(tag: dict) -> tuple[float, float]: - """(vel_east, vel_north) m/s from a node adsb entry's gs (kt) / track (deg), - the same conversion state._adsb_for_seeding applies to the cache.""" + """(vel_east, vel_north) m/s from a node adsb entry's gs (kt) / track (deg). + + Unlike the cache path, these are not coerced: no non-numeric gs or track has + been observed from a node. 86cb9t7c4 tracks closing that gap. + """ gs_ms = (tag.get("gs", 0) or 0) * 0.514444 trk = math.radians(tag.get("track", 0) or 0) return gs_ms * math.sin(trk), gs_ms * math.cos(trk) diff --git a/backend/services/tasks/analytics_refresh.py b/backend/services/tasks/analytics_refresh.py index 4dcd5230..5c877e26 100644 --- a/backend/services/tasks/analytics_refresh.py +++ b/backend/services/tasks/analytics_refresh.py @@ -1250,13 +1250,10 @@ def _refresh_mlat_verification(): age_s = now - entry.get("last_seen_ms", 0) / 1000 if age_s > 60: continue - # tar1090 convention: alt_baro is the string "ground" for aircraft on - # the ground — a bare multiply raised TypeError and killed the whole - # refresh cycle, leaving /api/test/mlat-accuracy permanently empty. - _gs_raw = entry.get("gs", 0) or 0 - _alt_raw = entry.get("alt_baro", 0) or 0 - gs_ms = (_gs_raw if isinstance(_gs_raw, (int, float)) else 0.0) * 0.514444 - alt_m = (_alt_raw if isinstance(_alt_raw, (int, float)) else 0.0) * 0.3048 + # Raw feed values: tar1090 sends alt_baro as the string "ground" on the + # deck, and json.loads parses a bare NaN, which an isinstance test admits. + gs_ms = as_num(entry.get("gs")) * 0.514444 + alt_m = as_num(entry.get("alt_baro")) * 0.3048 adsb_truth_pool.append( ( adsb_hex, diff --git a/backend/tests/test_mlat_verification.py b/backend/tests/test_mlat_verification.py index 99f143be..f810fefb 100644 --- a/backend/tests/test_mlat_verification.py +++ b/backend/tests/test_mlat_verification.py @@ -331,6 +331,35 @@ def test_adsb_aircraft_used_when_no_ground_truth_trail(self): assert data["n_matched"] == 1 assert data["tracks"][0]["truth_hex"] == "aabbcc" + def test_non_finite_feed_values_do_not_poison_the_truth_pool(self): + """json.loads parses a bare NaN, and an isinstance test admits it. + + A NaN reaching the pool propagates through every comparison as NaN + without raising, so the published error figures go quietly non-numeric. + """ + state.adsb_aircraft["aabbcc"] = { + "hex": "aabbcc", + "lat": 33.9, + "lon": -84.6, + "alt_baro": float("nan"), + "gs": float("inf"), + "track": 76.0, + "last_seen_ms": int(time.time() * 1000) - 5000, + } + + r = _make_solve_result(33.9001, -84.6001) + state.multinode_tracks[_key(r)] = r + + _refresh_mlat_verification() + data = orjson.loads(state.latest_mlat_verification_bytes) + + assert data["n_matched"] == 1 + track = data["tracks"][0] + assert math.isfinite(track["truth_alt_m"]) + assert math.isfinite(track["altitude_error_m"]) + assert math.isfinite(track["truth_speed_ms"]) + assert math.isfinite(track["velocity_error_ms"]) + class TestExternalAdsbFallback: """external_adsb_cache is used as truth when live ADS-B and ground-truth trails are empty."""