Measure ADS-B freshness at capture, not at handling - #275
Merged
Merged
Conversation
Every staleness gate in the pipeline keys off an entry's last_seen_ms, and three places set it to the moment the backend handled a position rather than the moment it was measured. The gates then read stale data as current, which is worst exactly when the data is worst. The external cache was the sharpest case: fresh_adsb fabricated int(now * 1000) for entries carrying no timestamp at all, so calibration's CAL_MAX_ADSB_AGE_S (10 s) and its fix-vs-detection skew rule both saw an age of zero. A fix up to a poll interval old (120 s, or 420 s behind a rate-limit backoff) entered the learned FOV as if it were current, which is 30 km at 250 m/s, and the coverage polygon is what judges solves. Both feeds publish the age of each position, so the capture time is available rather than inferred: OpenSky's time_position, adsb.lol's seen_pos. The poll wall-time is a fallback now, not the source. Node frames carry frame["timestamp"], which both ingest paths already had in hand for association and simply did not use for the ADS-B store. Honest stamps alone were not enough, because nothing ever removed an entry: both fetch paths only replace the cache wholesale, so if every source failed the last good snapshot persisted for the length of the outage behind a log line. Two expiries close that, one per layer. external_adsb_cache is pruned every poll cycle, outside the fetch because the fetch returns early when no real node is connected and those are exactly the runs that would otherwise leave a stale cache untouched. AdsbLolClient's per-area last-good cache ages out too: bounding only the outer one would leave stale positions arriving from underneath it. Node clocks are trusted within ADSB_CAPTURE_MAX_SKEW_S and no further. Past that the clock is broken rather than the frame late, and a stamp minutes into the future is never stale to any gate at all. Those frames fall back to receipt time and are counted, surfaced as adsb_capture_ts_fallback, so the condition is visible rather than silent. Cross-validation of node ADS-B reports against external truth is gated on the same timestamps. It has been dead, the cache being near-empty for an unrelated reason, and it is not safe to wake as written: the penalty is 0.1 against a block threshold of 0.2, it is persisted across restarts, and apply_reward is a no-op once blocked. Comparing positions measured minutes apart, re-charging the same sample every cycle, or reading the report route's default (0, 0) as a position claim would each block a truthful node. Gating both sides on freshness, judging each sample once, and ignoring null island leave the 10 km bar measuring disagreement rather than elapsed time. That confines the check to samples captured near a poll. Widening its reach means dead-reckoning the entry to the sample's time, not a larger budget; raised separately rather than folded in here. ClickUp 86cb9br6k. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The two writers of external_adsb_cache disagreed on units for velocity. OpenSky's field is metres per second and was stored as it arrived, correctly. adsb.lol serves tar1090, whose gs is knots, and it was stored raw. Both consumers read the field as m/s: analytics_refresh assigns it straight to gs_ms under a comment explicitly asserting this is not the tar1090 schema, and track_gates multiplies by 1.94384 to recover knots for the display feed. So every adsb.lol-sourced ground speed was about 1.94x high wherever it was consumed: verification's velocity-error statistics, and the solver's ADS-B-anchored initial guess and its dead-reckoning. OpenSky returns 429 for anonymous access, so adsb.lol is the only path that runs in production and this was every external truth entry rather than a fallback minority. It has not been observed in the wild only because the cache has been near-empty for an unrelated reason (86cb5p1jy site 1: the query bbox collapses to a single point). It would surface the moment that lands, which is why the schema is corrected here first rather than in the branch that fills the cache. The same function has always converted alt_baro from feet, so tar1090's units were understood when this was written; only the speed was missed. ClickUp 86cb9br6k. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review of the two commits below it. Making last_seen_ms a capture time was right, but three of its consequences were not, and two of them disabled the very defences the change exists to restore. The client, not the poller, owns capture time. tar1090's seen_pos is an age relative to the fetch that produced the row, and AdsbLolClient serves last-good rows again on a later failed fetch, so resolving it against the current poll re-stamped a frozen snapshot as seconds old on every cycle. Nine minutes into an adsb.lol outage a 542 s position read as 2 s: the prune dropped nothing, fresh_adsb served it as current, calibration's 10 s gate accepted it, and the cross-validator compared honest nodes against nine-minute-old truth and charged them 0.1 of persisted reputation a cycle. AdsbLolClient now resolves seen_pos at fetch time and publishes an absolute captured_at, which stays true however long the row is served. Records carry recv_ms alongside. record_adsb_calibration bounds a fix against the node's last detection at 2 s, and that detection stamp is server wall clock; comparing it against a node-clock capture stamp turned a co-timing rule into an NTP test, and a board 3 s out recorded no calibration points at all. The age gate keeps the capture stamp, the skew rule gets the server one. Writes are guarded against regression. adsb_aircraft is keyed by hex alone and written from two paths for the same frame, so once the stamp came from the frame rather than the clock, a queued replay walked it backwards by the queue latency and feed_gc evicted aircraft that were being reported at 50 Hz. The node-supplied stamp is bounded asymmetrically now, in both directions it matters: the past gets the width of a real backlog because that is honest, the future only jitter, because a stamp ahead of us is not stale to any gate and never becomes one. The same rule reaches the two writers that were missed, the sim push (forwards only, so replay still works) and the feeds themselves. Cross-validation loses its sample watermark entirely. It was meant to stop one bad fix being re-charged until a node blocked, but it keyed on an unvalidated node-supplied timestamp: one future-dated report pinned it and bought that node permanent immunity, an omitted timestamp_ms defaulted to 0 and skipped every sample forever, samples sharing a frame's millisecond collapsed to one judgement, and a sample deferred by any other gate was never judged again. The guarantee it was reaching for already exists — the admission window is far shorter than the interval between cycles — so the watermark is gone and the coupling is stated instead. The route stamps timestamp_ms when a node omits it. Cross-validation and the prune also move into _adsb_truth_cycle's finally: a fault inside cross-validation was being caught as an OpenSky failure, which churned the connection pool, ran cross-validation twice in one cycle, and left the prune skipped — freezing the cache the prune exists to age. Remaining: consumers scoring solves against external truth now gate on EXTERNAL_TRUTH_MAX_AGE_S rather than trusting the poller's retention bound, which runs on the fetch cadence and is ten minutes wide; an empty adsb.lol result clears the cache instead of leaving the previous one to chain the two budgets into a 19-minute stale window; feed values go through is_num, so a bare NaN drops its own row rather than raising and blanking the whole poll; and fresh_adsb uses valid_latlon, which it needs on the prime meridian. /api/v1/ground-truth/real gains a per-entry captured_at, since it invites callers to measure solver accuracy against positions it was presenting as current. Its "source" field still says opensky_network whatever fed it; that is a published field and the per-entry provenance it wants belongs to 86cb9btw9. ClickUp 86cb9br6k. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
|
Claude finished @Babissimo's task in 1m 23s —— View job Claude's review of #275 (re-check after sync)Tasks
SummaryThis trigger fired from a Nothing new to review here, so the prior review stands as-is. For reference, its one open finding is still unaddressed:
No other issues found; nothing further to check since the code itself hasn't changed. |
Babissimo
added a commit
that referenced
this pull request
Aug 28, 2026
86cb9br6k landed as #275 while this branch was open, and the two changed the same cache from opposite ends: it made every entry carry the time its position was captured and fixed adsb.lol's ground speed from knots to m/s, while this branch replaced the fleet-wide bounding box with one query region per node cluster. Both survive. The client keeps this branch's request handling and area pruning and gains their per-row capture time and last-good expiry; their _cache_ts joins the dicts the areas setter prunes, or it grows for the process lifetime the way the other three would have. The fetch keeps per-region querying and gains their cycle wrapper, cache ageing and unit conversions, so cross- validation runs once per cycle outside the fetch, which is what lets its age window double as the guarantee that no sample is judged twice. Their gated cross-validation replaces the log-only stopgap this branch carried while the penalty was unsafe, since the capture timestamps it was waiting for now exist. It keeps this branch's pair of position predicates: their inline (0, 0) test reads a bool as absent, and with the penalty restored a bool reaching haversine_km would score 8,000 km and charge a truthful node for it. Their tests move to the region-taking signature with every assertion intact, and this branch's OpenSky vector helper gains a capture time: it stamped index 3 with 0, which the merged code reads as epoch 0 rather than as absent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes ClickUp 86cb9br6k. Branch
fix/adsb-capture-timestamps.What this fixes
Three places stamped an ADS-B position's freshness with the time it was handled rather than the time it was captured, and one fabricated the timestamp outright for external-cache entries that carried none. Every 60 s staleness gate keys off that stamp, so the defences switched off exactly when the data was stalest.
The sharpest case was calibration:
record_adsb_calibrationrefuses a fix older thanCAL_MAX_ADSB_AGE_S(10 s), but withfresh_adsbreportingnowthe age was always zero, so a fix up to a poll interval old (120 s, or 420 s behind a rate-limit backoff) entered the learned FOV as if it were current. That is 30 km at 250 m/s, feeding the polygon that judges solves.Commits
7cc9e2242c806ab226719b3c1b85main(see below)mainis merged in rather than rebased, so the three commits above stay reviewable in isolation. It was not housekeeping: the branch was 17 commits behind and still carried the pre-fixdeploy/tower-contract.sh, so itstower-service-contractcheck failed on an open-meteo rate limit thatb459167and8e84750had already fixed onmain. CI runs that script from the PR's own branch, so a bare re-run could not have passed.The units defect is isolated in its own commit: it is a separate bug that merely shares a location, and it was ~1.94x high on every external truth entry, since OpenSky 429s anonymous access and adsb.lol is the only path that runs in production.
Notes for review
b226719is the interesting one. A review of the first two commits found that making freshness honest broke two things that had been quietly relying on every ADS-B stamp coming off the server clock:CAL_FIX_DETECTION_SKEW_Sbounds a fix against the node's last detection, and that detection stamp is server wall clock — so comparing it against a node-clock capture stamp turned a co-timing rule into an NTP test. Records now carryrecv_ms(server clock) alongsidelast_seen_ms(capture); the skew rule uses one, the age gates the other.seen_posis an age relative to the fetch that produced the row, and the client serves last-good rows again on a later failed fetch, so resolving it against the current poll re-stamped a frozen snapshot as seconds old every cycle.AdsbLolClientnow publishes an absolutecaptured_at, resolved at fetch time.last_seen_mscould go backwards, becauseadsb_aircraftis keyed by hex alone and written from two paths for the same frame. A queued replay walked the stamp back by the queue latency andfeed_gcevicted aircraft being reported at 50 Hz. Writes are guarded now.Cross-validation's sample watermark is deleted rather than patched — it caused four separate defects (a future-dated
timestamp_msbought a node permanent immunity; an omitted one skipped every sample; samples sharing a frame's millisecond collapsed to one judgement; deferred samples were never re-judged). The guarantee it reached for already holds: the admission window is far shorter than the interval between cycles, and there is now a test pinning that coupling.Deliberately not done
/api/v1/ground-truth/real'ssourcefield still saysopensky_networkwhatever fed it. It is a published field and the per-entry provenance it wants belongs to 86cb9btw9. It does gain an additive per-entrycaptured_at, since its docstring invites callers to measure solver accuracy against those positions.Sequencing
Site 1 of 86cb5p1jy holds behind this, per the decision recorded on the ticket: it rewrites the same
external_adsb_cachewrite sites, so this one owns the entry schema outright. It will add asourcefield to the same dict, which should merge cleanly alongsidelast_seen_msandrecv_ms.Verification
Full backend suite green (exit 0, zero failures, coverage 83.15% against a 55% gate) and
pre-commit run --all-filesclean. 51 new tests across four files, including a regression file with one test per review finding. Re-run after mergingmain: still exit 0.Not yet verified on a live map. Green tests do not cover the compose/env/frontend seams, so this wants a look at
testmap.retina.fmonce it is deployed.Worth a baseline either side, because two numbers people watch will move by design:
/api/test/solver-statsand/api/test/mlat-accuracy. External ADS-B fixes stop feeding calibration, which is the point of the change, and adsb.lol-sourced velocities drop by 1.94x.🤖 Generated with Claude Code