Skip to content

Withhold receiver-relative geometry from the public diagnostic dumps - #355

Merged
jehanazad merged 1 commit into
mainfrom
split/2-withhold-receiver-geometry
Sep 10, 2026
Merged

jehanazad merged 1 commit into
mainfrom
split/2-withhold-receiver-geometry

Conversation

@Babissimo

@Babissimo Babissimo commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

/api/test/* published per-record quantities measured from a node's TRUE receiver to a point the same payload gives the position of. services/public_location.py displaces every receiver coordinate the API publishes, and each of these handed that displacement back: a beam-gate range and bearing to a located aircraft, a bistatic range against a transmitter published untranslated, an FOV read-out at the true bearing off a curve /api/radar/analytics also publishes, a per-track bistatic delay. Enough of them intersect well inside the fuzz, which is 0.5–1 km, so defeating it resolves an address rather than a neighbourhood.

Second of five, split out of #349. Closes 123zgec25me.

The rule, in one place

services/public_geometry.py. A per-node constant is the envelope and may be published; a per-record value that varies with the true geometry is a measurement and is withheld.

Each field carries its own reason beside it, because the argument differs per field. bistatic_km goes because the transmitter is published untranslated, so the differential range fixes the receiver on a hyperbola through two known foci. fov_limit_km goes because the curve it is read off is itself published as empirical_polygon, so a value inverts to the bearing it was read at. The FOV shadow verdicts go because they are stamped for the nodes that PASSED as well as the ones that failed, so each bounds the receiver to a region where a failure would only exclude one.

The pass is structural over containers, so a field is withheld at any nesting depth without a per-route list. It matches on the leaf key name alone, which is its one real limit and is stated in the module: the same quantity under a name the module has not seen stays published.

Where it is applied

At each payload's serialisation edge rather than per record, so a key added to one of these payloads later cannot ship raw by being one the author forgot to wrap.

/api/test/mlat-verification is the exception. Both readers of its store are unauthenticated, so _publish_mlat_verification strips once on write rather than on every request, and both writers go through it — the early-return path guarded nothing before, which is a maintenance trap rather than a live bug, since that literal carries none of the withheld fields today.

/api/test/node/{id}/verification is node-scoped, so it also withholds solver_lat/solver_lon: every track entry is measured from that one node's receiver, and the aircraft feed publishes the same solve position displaced under the same hex, so the true frame beside it differences to the offset. A multinode payload keeps them, since no single receiver is behind that position. Stripped at the route rather than in the store, because it is the route's one-node addressing that makes the solve position receiver-relative.

node_detection_range dropped furthest_detections with its own dict comprehension. That becomes a call to the same module, so the rule has one owner. public_location.public_node_summary still hand-rolls the third copy; different module, out of this PR's seam.

That summary's keys come from the retina_analytics submodule rather than this repo, and the pass matches on leaf key name, so TestTheDetectionAreaSummary pins the overlap: the intersection is exactly {furthest_detections} today, and an upgrade that adds a colliding name fails there instead of quietly dropping the field.

The tests are the deliverable

backend/tests/test_public_geometry.py, 260 lines against 133 of production code.

Unit tests on the helper prove nothing about whether a route calls it, so six go through the routes. Verified by mutation rather than by assertion: strip the call from node_verification, from any mlat_history return, or from _publish_mlat_verification, and exactly those six fail, then pass again on restore.

Left deliberately

  • position_error_km stays. It inverts against the aircraft feed under same-epoch cross-surface pairing, which needs a decision about what the route is for rather than a field drop. 123zgec25a0.
  • No private_node_ids() filter. 123zgec25bc.
  • truth_lat/truth_lon stay. Public ADS-B positions, and removing measured_delay_us already breaks the bistatic ranging equation they sat in.

Verification

Backend 3286 passed, 2 skipped. pre-commit run --all-files green.

No client reads a withheld name: the solver_lat/solver_lon that LiveAircraftMap uses come from /test/mlat-verification, which is multinode and keeps them. max_range_km, max_bistatic_range_km and estimated_range_km are separate keys from the withheld bare range_km and are untouched.

One first run failed test_known_lane.py::test_interval_gate_holds_between_passes, the flake documented on main since b0bbeb4. Not this branch: it passes alone, passes after this file, and passed on the re-run, and the six new TestClient(app) calls leak zero threads (measured: 1 before, 1 after), so they add none of the solver daemons that fixture's docstring names as the mechanism.


🤖 Generated with Claude Code

@claude

This comment has been minimized.

Every receiver coordinate the API publishes is displaced
(services/public_location.py), but the unauthenticated /api/test/* payloads
carried quantities measured from the TRUE receiver to a point the same payload
gives the position of: a beam-gate range and bearing, a bistatic range against
a transmitter published untranslated, an FOV read-out at the true bearing off a
curve /api/radar/analytics also publishes, a per-track bistatic delay. Each is
a locus the receiver sits on, and enough of them intersect well inside the
0.5-1 km fuzz, so the displacement was decorative on any node appearing in one
of these dumps.

services/public_geometry.py puts the rule in one place: a per-node constant is
the envelope and may be published, a per-record value that varies with the true
geometry is a measurement and is withheld. The pass is structural over
containers rather than a per-route field list, so a field is withheld at any
nesting depth and a new record shape needs no list widened. It matches on the
leaf key alone, which is its one real limit and is stated in the module.

Applied to each payload at its serialisation edge rather than per record, so a
key added to one of these payloads later cannot ship raw by being one the
author forgot to wrap. /api/test/mlat-verification is the exception: both
readers of its store are unauthenticated, so _publish_mlat_verification strips
once on write rather than on every request, and both writers now go through it.

/api/test/node/{id}/verification is node-scoped, so it also withholds
solver_lat and solver_lon: every track entry is measured from that one node's
receiver, and the aircraft feed publishes the same solve position displaced
under the same hex, so the true frame beside it differences to the offset. A
multinode payload keeps them, since no single receiver is behind that position.

node_detection_range's hand-rolled furthest_detections drop becomes a call to
the same module, so that rule has one owner rather than two. That summary's
keys come from the retina_analytics submodule, so a test pins its overlap
with the withheld set: an upgrade that adds a colliding name fails there
instead of quietly dropping the field from the payload.

Closes 123zgec25me. Leaves 123zgec25a0 (position_error_km, which needs a
decision about what the route is for) and 123zgec25bc (the private-node
opt-out) open, both already tracked.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Babissimo
Babissimo force-pushed the split/2-withhold-receiver-geometry branch from cb62151 to cce9b99 Compare September 10, 2026 20:15
@Babissimo

Copy link
Copy Markdown
Contributor Author

Closed the review's one finding, folded into the existing commit rather than stacked.

It was a consequence of consolidating node_detection_range's hand-rolled furthest_detections drop into the module: that removed a duplicated rule, but it also put an unvendored external schema through a name-matching filter. DetectionAreaState.summary() comes from the retina_analytics submodule, so a field added there under one of the 14 withheld names would vanish from the public payload with nothing saying so.

TestTheDetectionAreaSummary pins the overlap rather than narrowing the withhold set at that call site, which would re-fragment the rule. Two assertions: the summary's key set intersects the withheld set in exactly {furthest_detections}, and the pass over a populated summary drops that and nothing else. Checked by mutation, as with the route tests: adding max_range_km to the withheld set (a real key on that summary) fails both, and both pass on restore.

No collision today. Current keys are node_id, rx, tx, beam_azimuth_deg, beam_width_deg, max_range_km, max_bistatic_range_km, observed_delay_range_us, observed_doppler_range_hz, estimated_max_range_km, observed_max_differential_km, n_detections, furthest_detections.

Backend 3286 passed, 2 skipped. Lint gate green.

One run in between failed test_publication.py once and did not reproduce across three further full runs or when run alone (62 passed). This branch changes four files and none of them is services/publication.py; the two fuzz/publication hits in its diff are unchanged context lines.

@claude

claude Bot commented Sep 10, 2026

Copy link
Copy Markdown

Claude finished @Babissimo's task in 3m 35s —— View job


Reviewing PR #355


Review

Traced all four call sites (node_verification, the three mlat_history return points, _publish_mlat_verification's two writers, and node_detection_range) against backend/services/public_geometry.py and backend/tests/test_public_geometry.py. This is a clean, well-reasoned change; no correctness or security issues found.

Confirmed correct:

  • _stripped() is a non-mutating structural copy (dict/list/tuple only); isinstance(x, list | tuple) matches the existing convention already used in routes/node_schemas.py, so no Python-version concern.
  • node_verification (backend/routes/test.py:733-746) strips at the route with node_scoped=True, correctly leaving state.latest_node_verification_bytes un-stripped for its other internal readers (test_radar3_truth_units.py, test_adsb_ground_sentinel.py read the store dict directly, not the HTTP route, so they're unaffected).
  • _publish_mlat_verification (backend/services/tasks/analytics_refresh.py) is genuinely the sole writer of state.latest_mlat_verification_bytes in production code — verified via grep, both call sites route through it, and _mlat_verification_summary() (routes/test.py:714-727) only reads aggregate top-level keys (n_solves, position.mean_km, etc.) that are never in the withheld set, so pre-stripping the store doesn't corrupt the dashboard summary.
  • max_range_km/max_bistatic_range_km/estimated_range_km are confirmed as distinct keys from the withheld bare range_km throughout the codebase — no accidental collision.
  • No /v1/nodes routes or models touched, so contracts/nodes-v1.openapi.yaml correctly does not need regeneration per CLAUDE.md.

On the concern raised in the prior review pass (leaf-name collision between without_receiver_geometry and the externally-defined DetectionArea.summary() schema from retina_analytics): this is already mitigated in this same commit. backend/tests/test_public_geometry.py:254-255 (test_only_furthest_detections_collides) pins the exact current intersection between area.summary()'s keys and _RECEIVER_RELATIVE to {"furthest_detections"}. If a future retina_analytics upgrade adds a field under one of the 14 withheld names, this assertion fails loudly rather than silently dropping data — so the prior review's suggested regression test already exists here. No further action needed on that point.

Minor, non-blocking observation: node_verification now round-trips every response through orjson.loads + orjson.dumps instead of returning stored bytes directly. Correct and necessary to enforce stripping at the route, and orjson's float round-tripping is lossless, so no behavioral concern — just a small, justified perf cost on a diagnostic endpoint.

Could not execute pre-commit run --all-files or the test suite directly in this sandbox (no approval available for shell execution in this headless run), so verification here is by static reading of the diff and tests rather than by running them — consistent with the PR's own reported 3284 passed, 2 skipped.
· branch split/2-withhold-receiver-geometry

@jehanazad
jehanazad merged commit 54cee9e into main Sep 10, 2026
31 checks passed
jehanazad added a commit that referenced this pull request Sep 13, 2026
Resolves 21 conflicting files against 53 commits on main, including the
two pieces already split out of this PR (#351 identity scrub, #355
receiver-geometry withholding), #358 site markers / empirical coverage,
#348 location privacy, #350/#353/#354 owner contact and retnode links,
and the #343/#346/#347/#357 palette work.

Rules applied: main's version is the base for the add/add files
(public_geometry.py, test_no_real_identities.py); the PR's ref-keying is
layered on top of main's rework everywhere else; substitution still runs
last on every publication path and fails closed for unregistered real
nodes.

Follow-on edits outside the conflicted set, needed to compile or to keep
main's tests honest under ref addressing: frontend nodeSites.ts (+test)
now reads is_synthetic; backend test_node_ref.py and
test_public_geometry.py address routes by ref / synthetic id;
analytics.py's owner-private listing merge keys on the owner identity.

Verified: backend 3495 passed / 2 skipped, ruff clean; frontend tsc,
lint, vite build, vitest 232 passed; dashboard tsc, eslint, vitest 53
passed, build.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
jehanazad added a commit that referenced this pull request Sep 13, 2026
The merge with main left /api/test/mlat-history running the geometry
pass twice: per record inside _published_records (this branch) and again
over the whole payload at the three return sites (#355). The per-record
pass is the one that has to stay, because it must precede the identity
walk (foreign_node_ids is both a withheld measurement and an identity
field the walk renames). The outer pass stripped nothing beyond it: every
record list already goes through _published_records and the remaining
top-level keys are window parameters and counts.

tests/test_public_geometry.py still fetches all three shapes of the route
and asserts nothing from the withheld set appears at any depth, so a
future field added outside `records` fails there regardless.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants