adsb truth: query one region per node cluster, not one box for the fleet (86cb5p1jy site 1)

[Babissimo]

Fixes site 1 of 86cb5p1jy. Design decided on 86cb9bmmk.

Merge gate: satisfied

This was held behind 86cb9br6k, which owns the knots-into-an-m/s-field defect on the same cache entries. That landed as #275 and is merged in here, so the gate is lifted.

Merging still deploys to staging and production, so it remains a deploy decision.

The problem

state.external_adsb_cache held 3 aircraft on production, against roughly 2,990 when the fleet sat in one metro. It is the truth source for verification matching, fresh_adsb dead reckoning, _cross_validate_adsb_reports, and the public endpoint.

_fetch_external_adsb reduced the whole fleet's geography to one lat/lon bounding box, and the adsb.lol fallback collapsed that box to its centre point. With nodes in Atlanta, Massachusetts, Sacramento and one unpositioned placeholder, the centre landed at 42.7 N, 60.7 W — the open North Atlantic. adsb_truth_fetcher reported healthy throughout, because the fetch itself succeeded.

Deleting the placeholder does not fix it: the centre moves to central Kansas, which returns 331 aircraft, none within 150 km of any node. A loud failure becomes a silent one.

The change

Nodes are grouped on a fixed 400 km sinusoidal lattice; each group's query geometry is derived from its own members, then rendered per provider. Cell ids are stable, so the per-area cache key is correct by construction.

Both providers are chunked, not just adsb.lol. The bounding box site 1 names is literally the OpenSky one, and OpenSky charges credits by box area: the fleet-wide box cost 4 credits a request for a query that was mostly ocean, against 1 for a box around a single cluster. Cheaper per request and only aircraft the nodes can see.

Today's fleet collapses to four regions, each centred on its own metro at 81-87 nm.

Also here:

• adsb.lol prerequisites before any rate increase: 5 s request spacing, explicit 429 handling with one retry, a User-Agent carrying contact info, and gzip.
• The silent-failure branch is gone. The cache is replaced whenever a fetch genuinely succeeded — an empty answer from a working query included — and left stale only when none did.
• Each entry records which provider supplied it, unblocking 86cb9btw9.

Found on the way, and handled

Filling the cache wakes _cross_validate_adsb_reports, which applies persisted reputation penalties and had never run against real data. Three paths would have ground honest nodes to blocked: samples defaulting to (0, 0) scoring an 8,000 km mismatch, cache staleness outrunning the 10 km bar, and the same samples being re-judged every cycle.

This branch disabled the penalty while that was unsafe. #275 supplies the capture timestamps the comparison needs, so the merge restores it behind #275's temporal gates rather than keeping the stopgap. It keeps this branch's pair of position predicates on top: #275's inline (0, 0) test reads a bool as absent, and False == 0.0, so a node posting {"adsb_lat": false} through the unvalidated analytics route would otherwise reach haversine_km and be charged for the 8,000 km it appears to be off.

Deliberately not here

• 86cb9m6wc — OpenSky's 429 backoff. A credit-exhausted OpenSky is re-asked each cycle; the fix is its own X-Rate-Limit-Retry-After-Seconds (measured at 5,929 s), not the fixed 300 s guess.
• 86cb9bty8 — OpenSky authentication. Noted there that the free tier supports about five 1-credit regions at the current cadence, not the eight this caps at.
• 86cb9t2pg — task_last_success is stamped even on a cycle that left the cache stale.
• 86cb9uut4 — a pre-existing intermittent test_mlat_history timeout under full-suite load. Not from this branch; the suite was green on the base commit.

Verification

Full backend suite green (pytest exit 0), coverage 83.07% against the 55% gate, pre-commit run --all-files clean.

Four review rounds were run over the branch; every finding is either fixed or ticketed above. Tests pin the regressions that recur: three separated nodes give three regions, a distant node does not displace the existing ones, request count does not grow with node count, and every member sits at least its detection-range margin inside its box at any latitude.

Not yet verified on a live map — that follows the deploy, per the repo's rule.

🤖 Generated with Claude Code

[claude]

Claude finished @Babissimo's task in 5m 21s —— View job

---

Code review

• Gather context (PR description, comments, CLAUDE.md)
• Determine request type — code review, per the repo's /code-review:code-review custom instructions
• Review diff against origin/main
• Check previously-open findings (feat: add staging environment with synthetic data pipeline #2 source mislabeling, test: add comprehensive node registration E2E suite (52 tests) #4 missing lock) for current status
• Post review feedback

Current HEAD (02df973) contains three real commits over origin/main (c8baaf9, e6a0e82, aad5ea4) plus a no-op merge; I reviewed the full diff fresh rather than relying on the prior round's commit range, which is no longer reachable in this checkout.

Finding: the PR's stated safety mitigation isn't in the diff

backend/services/tasks/periodic.py, _cross_validate_adsb_reports, lines 677-689.

The PR description states:

Null-island samples are now skipped and the penalty is demoted to a log; restoring it is on 86cb9br6k's acceptance criteria.

Diffing this function against origin/main shows only two additions — the is_position_absent() null-island guard and the is_usable() bool/NaN/out-of-range guard (both covered by the new TestCrossValidationRejectsUnusableFixes tests in test_node_bias.py). The actual mismatch branch is untouched:

if dist_km > 10.0:
    rep = state.node_analytics.reputations.get(node_id)
    if rep:
        rep.apply_penalty(0.1, f"ADS-B position mismatch: ...")
        logging.warning(...)

apply_penalty(0.1, ...) is still called unconditionally for a genuine >10 km mismatch — it is not demoted to a log-only warning anywhere in this diff (and can't be elsewhere, since NodeReputation lives in the external retina_analytics package, untouched by this PR's file list). The function's own docstring is also unchanged and still asserts "The penalty here is severe and sticky... every gate below exists to keep a truthful node out of it."

This matters because it's exactly the risk the PR's own "do not merge before 86cb9br6k" banner calls out: 86cb9br6k's dormant velocity-unit defect can drive a node's self-reported position (via fresh_adsb dead reckoning) away from true position, and at a 0.1-per-mismatch penalty against a 0.2 block threshold, two such mismatches block an honest node — before this PR's own stated mitigation is in place. Worth confirming whether this demotion was dropped in a rebase/merge, or whether the PR description is just stale and the real intent is "defer to 86cb9br6k, don't merge first" (which the banner already says) — but as written, the description and the code disagree on a point the PR frames as safety-critical.

Fix this →

Still open from earlier rounds

_fetch_external_adsb reads connected_nodes without the lock — backend/services/tasks/periodic.py:258-262:

active_nodes = [
    info
    for info in list(state.connected_nodes.values())
    if info.get("status") != "disconnected" and info.get("config")
]

Every other cross-thread reader of state.connected_nodes takes state.connected_nodes_lock (routes/output.py:34, services/tasks/analytics_refresh.py:322,331,1351,1735, this same file's prune_synthetic_nodes at line 119, etc.). This is the one outlier, still unguarded against a RuntimeError: dictionary changed size during iteration if a node connects/disconnects mid-scan.

/api/v1/ground-truth/real hardcodes "source": "opensky_network" — backend/routes/output.py:202. Unlike the lock issue, this one is now explicitly acknowledged in a code comment deferring the fix to 86cb9btw9 (per-entry provenance), so it's a deliberate, ticketed deferral rather than an oversight — flagging for visibility only, not blocking.

Everything else reviewed and looked solid

• services/adsb_regions.py's lattice math (cell_of, _dlon_for_margin, antimeridian/pole handling in _region_from_members) is internally consistent and the edge cases (pole-adjacent rows, antimeridian-straddling boxes, 250 nm schema cap) are exercised by test_adsb_regions.py.
• clients/adsb_lol.py's new request spacing, 429 retry, User-Agent, and gzip handling match the PR description and the adsb.lol requirements it cites.
• _fetch_opensky/_fetch_opensky_region/_fetch_opensky_box's per-box failure isolation and the "replace cache on any real answer, including empty" logic in _fetch_external_adsb line up with the docstrings and are covered by test_adsb_truth_regions.py.
• task_registry.py's new 400 s expected interval has its arithmetic spelled out in-comment and is consistent with the worst-case backoff path.
• No new CLAUDE.md issues: no /v1/nodes contract changes, no new env vars needing .env.example entries, no secrets/addresses introduced.
  · Branch