From 221a061e28c2a120501aa23aedf55a556d14f83b Mon Sep 17 00:00:00 2001 From: Josh Poole Date: Mon, 14 Sep 2026 09:54:53 +0100 Subject: [PATCH] 20260914 - Give every track its own identity, not the aircraft's Track._generate_id built the id from adsb_hex whenever the aircraft was known, so every pass of that aircraft in a day answered to one identity. That hid the failure we most need to see. live_score.fragmentation counts distinct ids per hex, so an id derived from the hex can only ever yield one: a site splitting an aircraft four ways reported 1.05 where the truth was 4.57. It cost more than a blind metric. TrackEventWriter keeps a per-track high-water mark keyed by id, so two tracks of one aircraft shared it, and whichever emitted first silently suppressed the other's earlier detections. The events file was missing data nobody could see was missing. The id becomes a plain daily counter. The label is not lost: it travels in the event's own adsb_hex field, which is where every consumer already reads it. One assertion in test_adsb_features.py inverts, because it pinned the old behaviour of carrying the hex in the id. This matters now rather than later because fairforest B started producing labelled tracks today, and fragmentation is one of the measures that was blind while its ADS-B feed was misconfigured. Co-Authored-By: Claude Opus 5 (1M context) --- retina_tracker/track.py | 15 +++-- retina_tracker/tracker.py | 4 +- tests/test_adsb_features.py | 4 +- tests/test_track_identity.py | 125 +++++++++++++++++++++++++++++++++++ 4 files changed, 140 insertions(+), 8 deletions(-) create mode 100644 tests/test_track_identity.py diff --git a/retina_tracker/track.py b/retina_tracker/track.py index 283a50c..4fda8e4 100644 --- a/retina_tracker/track.py +++ b/retina_tracker/track.py @@ -679,13 +679,20 @@ def _init_from_delay_doppler(self, detection): ) @classmethod - def _generate_id(cls, timestamp_ms, adsb_hex=None): + def _generate_id(cls, timestamp_ms): + """A fresh identity per track, never one derived from the aircraft. + + An id built from adsb_hex gives every pass of an aircraft the same + identity, which costs twice over: fragmentation becomes unmeasurable, + because live_score counts distinct ids per hex and would only ever + find one, and two tracks of the same aircraft share OutputWriter's + per-track high-water mark, so whichever emits first silently suppresses + the other's earlier detections. The label already travels in the + event's own adsb_hex field, so nothing downstream needs it here. + """ dt = datetime.fromtimestamp(timestamp_ms / 1000.0) date_str = dt.strftime("%y%m%d") - if adsb_hex: - return f"{date_str}-{adsb_hex.upper()}" - if cls._last_date != date_str: cls._daily_counter = 0 cls._last_date = date_str diff --git a/retina_tracker/tracker.py b/retina_tracker/tracker.py index ba04507..ba5fff1 100644 --- a/retina_tracker/tracker.py +++ b/retina_tracker/tracker.py @@ -216,7 +216,7 @@ def process_frame(self, detections, timestamp): for track in self.tracks: promoted = track.promote_if_ready() if promoted: - track.id = Track._generate_id(timestamp, adsb_hex=track.adsb_hex) + track.id = Track._generate_id(timestamp) if self.event_writer: _det_n = min(track.n_associated, self.detection_window) if _lazy_write: @@ -475,7 +475,7 @@ def _initiate_tracklets(self, timestamp): track.state[2] = doppler_to_range_rate(doppler_velocity) - track.id = Track._generate_id(timestamp, adsb_hex=track.adsb_hex) + track.id = Track._generate_id(timestamp) if self.event_writer: detections_list = track.get_recent_detections(n=track.n_associated) diff --git a/tests/test_adsb_features.py b/tests/test_adsb_features.py index 9b18277..8db1673 100644 --- a/tests/test_adsb_features.py +++ b/tests/test_adsb_features.py @@ -118,10 +118,10 @@ def test_adsb_tracking(): assert len(confirmed_tracks) >= 1, "Should have at least 1 confirmed track" assert len(adsb_tracks) >= 1, "Should have at least 1 ADS-B track" - # Verify ADS-B track has ICAO hex in ID + # Verify the ADS-B label reaches the track without claiming its identity adsb_track = adsb_tracks[0] assert adsb_track.adsb_hex == "a12345", f"Expected hex a12345, got {adsb_track.adsb_hex}" - assert "A12345" in adsb_track.id, f"Track ID should contain ICAO hex: {adsb_track.id}" + assert "A12345" not in adsb_track.id, f"Track ID must not carry the ICAO hex: {adsb_track.id}" # Verify ADS-B track has lower covariance if len(non_adsb_tracks) > 0: diff --git a/tests/test_track_identity.py b/tests/test_track_identity.py new file mode 100644 index 0000000..3d2a607 --- /dev/null +++ b/tests/test_track_identity.py @@ -0,0 +1,125 @@ +"""A track's identity is its own, never the aircraft's. + +The id used to be built from adsb_hex whenever the aircraft was known, so every +pass of that aircraft in a day answered to a single identity. That hid the +failure we most need to see: live_score.fragmentation counts distinct ids per +hex, and an id derived from the hex can only ever yield one, so a site splitting +an aircraft four ways still reported close to one. It also let two tracks of one +aircraft share TrackEventWriter's per-track high-water mark, where whichever +emitted first suppressed the other's earlier detections outright. + +The label is not lost by this. It travels in the event's own adsb_hex field, +which is where every consumer already reads it. +""" + +import json +import re + +import pytest + +from retina_tracker.live_score import fragmentation, load_tracks +from retina_tracker.output import TrackEventWriter +from retina_tracker.track import Track + +COUNTER_ID = re.compile(r"^\d{6}-[0-9A-F]{6}$") +BASE_TS = 1718747745000 +DAY_MS = 86400000 + + +@pytest.fixture(autouse=True) +def _fresh_counter(): + """The counter is class state, so it outlives a test without this.""" + Track._daily_counter = 0 + Track._last_date = None + yield + Track._daily_counter = 0 + Track._last_date = None + + +def detection(ts, delay, doppler=-120.0): + return {"timestamp": ts, "delay": delay, "doppler": doppler, "snr": 16.0, "adsb": None} + + +class TestEveryTrackGetsItsOwnIdentity: + def test_successive_tracks_never_share_an_id(self): + ids = [Track._generate_id(BASE_TS) for _ in range(5)] + assert len(set(ids)) == 5 + + def test_two_tracks_born_in_the_same_millisecond_still_differ(self): + """Concurrent tracks of one aircraft are the case that used to collide.""" + assert Track._generate_id(BASE_TS) != Track._generate_id(BASE_TS) + + def test_an_id_is_always_drawn_from_the_counter(self): + assert COUNTER_ID.match(Track._generate_id(BASE_TS)) + + def test_the_counter_resets_on_a_new_day(self): + first = Track._generate_id(BASE_TS) + second = Track._generate_id(BASE_TS + DAY_MS) + + assert first.endswith("-000000") + assert second.endswith("-000000") + assert first.split("-")[0] != second.split("-")[0] + + def test_the_date_still_leads_the_id(self): + """Operators read the date off the id, and the archive sorts on it.""" + assert Track._generate_id(BASE_TS).split("-")[0].isdigit() + + +class TestTwoTracksOfOneAircraft: + """The fragmentation case: one aircraft, two identities, both intact.""" + + def _events(self, tmp_path): + path = tmp_path / "events.jsonl" + writer = TrackEventWriter(str(path), max_bytes=0) + first = Track._generate_id(BASE_TS) + second = Track._generate_id(BASE_TS) + + writer.write_event( + first, + BASE_TS + 4000, + 2, + [detection(BASE_TS + 3500, 18.0), detection(BASE_TS + 4000, 18.2)], + adsb_hex="abc123", + ) + writer.write_event( + second, + BASE_TS + 4000, + 2, + [detection(BASE_TS + 1000, 21.0), detection(BASE_TS + 1500, 21.2)], + adsb_hex="abc123", + ) + writer.close() + return path, first, second + + def test_both_identities_reach_the_events_file(self, tmp_path): + path, first, second = self._events(tmp_path) + + assert set(load_tracks(str(path))) == {first, second} + + def test_the_later_track_does_not_suppress_the_earlier_one(self, tmp_path): + """A shared id would put these behind one high-water mark, and the + second track's older detections would never be written.""" + path, _, second = self._events(tmp_path) + + assert [d["timestamp"] for d in load_tracks(str(path))[second]["detections"]] == [ + BASE_TS + 1000, + BASE_TS + 1500, + ] + + def test_fragmentation_can_finally_see_the_split(self, tmp_path): + path, _, _ = self._events(tmp_path) + + result = fragmentation(load_tracks(str(path))) + + assert result["aircraft_identified"] == 1 + assert result["mean"] == pytest.approx(2.0) + assert result["split"] == ["abc123"] + + def test_the_label_still_reaches_the_consumer(self, tmp_path): + path, first, _ = self._events(tmp_path) + + with open(path) as f: + events = [json.loads(line) for line in f if line.strip()] + + assert {e["adsb_hex"] for e in events} == {"abc123"} + assert load_tracks(str(path))[first]["adsb_hex"] == "abc123"