Skip to content

map: one marker per site, node_ref labels, measured coverage only - #358

Merged
jehanazad merged 10 commits into
mainfrom
feat/node-site-markers-empirical-coverage
Sep 13, 2026
Merged

jehanazad merged 10 commits into
mainfrom
feat/node-site-markers-empirical-coverage

Conversation

@jehanazad

Copy link
Copy Markdown
Contributor

Depends on offworldlabs/retina-analytics#28 — the submodule bump in the first commit points at that branch's commit, so merge that first (CI checks out submodules recursively and needs the commit reachable on the remote; it is, on the feature branch).

Summary

Three map fixes requested 2026-09-06: co-located nodes were unclickable behind each other, node popups and panels printed raw node_ids, and the "detection area" was the declared beam (theoretical cone for nodes with no data, and a beam-clipped pie slice for nodes with data). Now: one marker per receive site with a count badge and a per-node popup, every node named by a public node_ref, and the only detection area ever drawn is the measured (ADS-B-calibrated) polygon, unclipped.

Changes

Backend — coverage polygon (submodule bump, retina-analytics#28)

  • manager.get_node_summary under FOV_MODE=off publishes to_polygon(evidence_only=True) instead of clipping to beam_azimuth_deg/beam_width_deg. radar3 on test has evidence in all 72 bearing bins (reach 17–65 km) and was served a 120° slice.

Backend — node_ref for every node

  • New services/node_ref.py: public_node_ref(node_id) returns the registry's Node.node_ref when the node registered through /v1/nodes, else "nde" + 12 base36 chars derived by HMAC-SHA256 over the node fuzz salt with a node_ref| domain prefix — same shape as mint_node_ref, stable across restarts, never the raw id. The nodes table on the test droplet is empty (blah2 bridge and plain-TCP nodes never register), so without the derivation no live node would have a public name.
  • Wired into /api/radar/analytics (both variants, as copies of the summaries), /api/radar/analytics/{node_id} and /api/radar/nodes. The node API contract is untouched.

Frontend

  • map/nodeSites.ts: groupNodesBySite (exact published-coordinate equality, matching how node_sites.py co-locates), nodeLabel (node_ref, else "unlisted node" — never the id), polygonMaxReachKm.
  • NodeMarkersLayer renders sites: one uncertainty disc per site, a count badge on shared sites (nodeSiteIcon), popup lists each node with node_ref and a measured-coverage line; shared sites get per-node Show/Hide buttons, single-node sites keep click-to-select. Beam/range lines removed from popups.
  • All theoretical Yagi cones removed: always-on CoverageLayer, selected-node overlay, contributing-node highlights. Nodes without a polygon draw no area.
  • InBeamDiagnostic tests membership with pointInPolygon against the empirical polygon (new helper in geo.ts), skips nodes without one; toolbar button renamed "Coverage gaps".
  • AircraftDetailPanel "Claiming node" and detecting-node list print labels via a nodeLabelFor prop; illuminator popup lists labels.
  • RadarNode gains node_ref; the beam_*/max_range_km fields stay, commented as arc-rebuild parity only (bistaticArc.ts mirrors the backend arc geometry).

Docspipeline.md §7 (evidence-only publication, node_ref), architecture.md, arc-display.md; CALIBRATION_SCHEMA note corrected 5→6.

Test coverage

  • retina-analytics: 445 passed (11 new — see ci: prune docker builder cache before staging build to fix snapshot corruption #28).
  • Backend: new tests/test_node_ref.py (10: shape, determinism, id/salt sensitivity, registry row wins, no raw-id fallback), test_analytics_refresh.py (+node_ref in both payload variants and the nodes block), test_public_location.py fixture spread over six bearings so its 30 points still form an area. Full suite: 2953 passed, 1 skipped, coverage 85% (gate 55%).
  • Frontend: nodeSites.test.ts (grouping, labels, reach), geo.test.ts (+pointInPolygon inside/outside/north-wrap); tsc, eslint, vite build, vitest 184 passed. Run twice independently (implementer and reviewer).
  • pre-commit --all-files: all hooks pass.
  • Offline check on radar3/radar3a bins copied from the test droplet: 73-vertex simple polygon, reach 16.8–58.3 / 16.8–60.8 km, beam args ignored.
  • Not verified in a browser. Nothing was deployed (another session is live on the test droplet). Owed after merge+deploy: two-node badge and popup at the Atlanta and Greenville sites, nde… labels everywhere, radar3's all-round polygon, no cones on the five unmeasured nodes.

Review notes

  • Wire still carries node_id and beam_*. Anonymity here is display-level: node_id remains the join key in the aircraft feed (node_id, contributing_node_ids, detecting_nodes) and analytics keys. Replacing the keys on the wire is a larger change across feeds, dashboard and e2e; flagged as follow-up.
  • Beam still used by the pipeline (unchanged, deliberately): solver beam gate (tasks/solver.py ~L2408), arc clipping to the wedge (track_gates._build_single_node_arc, bistaticArc.ts), _refresh_missed_detections, association grid, and the diagnostic /api/test/node/{id}/detection-range. FOV_MODE=active is the existing evidence-based path for the gates; these need their own measurement before changing.
  • Derived refs are keyed on the fuzz salt, so rotating NODE_FUZZ_SALT renames every unregistered node — the same effect rotation already has on positions. e2e nodes.spec.ts asserts beam_* fields in detection_area; they are still served, so no change there.

🤖 Generated with Claude Code

jehanazad and others added 6 commits September 6, 2026 04:18
…bump)

Bumps retina-analytics to the evidence-only polygon: under FOV_MODE=off
the published empirical_coverage.polygon is what the node has been SEEN
to detect, with the declared beam wedge no longer clipping it away.

test_public_location's fixture spread 30 calibration points along a
single line, which opened one bin — enough for the old beam-clipped
sector, not an area under the new rule.  Six adjacent bearings give it a
lobe with both a measured arc and the RX apex its displacement
assertions read.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Every public payload that names a node named it by node_id: the map's
node popups, the aircraft detail panel, the illuminator popup.  A
node_id comes off the board and is the name its owner chose — a run of
them is a naming convention — so printing it hands a stranger a
correlation key nobody agreed to publish.  core.nodes.Node already
carries the intended public handle, node_ref, but only nodes that
registered through /v1/nodes have a row: the blah2 bridge's receivers
and everything on the plain TCP protocol have none, which on the test
deployment is all seven live nodes.

services/node_ref.public_node_ref returns the stored ref when there is
one and otherwise derives HMAC-SHA256(fuzz salt, "node_ref|" + node_id)
rendered in mint_node_ref's own base36 alphabet, to the same 15-char
shape — so registered and legacy nodes are not tellable apart by form.
The salt is already this deployment's anonymity key, secret and stable
across restarts, and the domain prefix keeps this frame clear of the
location HMAC's.  There is no fallback to the raw id: a lookup that
fails derives, because "publish the id when the database is down" makes
the leak conditional on exactly the moment nobody is watching.

Wired into both /api/radar/analytics variants, /api/radar/nodes and the
fresh per-node analytics route.  No node API route or model changes, so
contracts/nodes-v1.openapi.yaml does not move.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Three display bugs, one theme: the map was showing configuration as if it
were measurement, and machine names as if they were public handles.

One marker per receive SITE.  Co-located receivers are published at
exactly equal coordinates so the pair leaks one sample of its position
instead of two; the map drew a marker and an uncertainty disc per node,
so the two glyphs stacked (only the top one could be clicked) and the
doubled fill made a shared site read as better located than a lone node.
A site now carries one disc, a count badge on the glyph, and a popup
block per node with its own Show/Hide control — a marker click at a
shared site has no single node to mean.  Single-node sites behave as
before.

Nodes are named by node_ref.  Node popups, the detail panel's claiming
node and detecting-node list, and the illuminator popup all printed the
raw node_id, which is the name the operator gave the machine.  They
print nodeLabel() now; node_id stays the join key throughout.

No theoretical beam is drawn anywhere.  The dashed Yagi sector was a
declared azimuth and width — configuration nobody surveyed — drawn as a
detection area, and the node with the LEAST evidence got the boldest
wedge (full-strength cone when it had no measured polygon at all).  The
coverage layer, the selected-node overlay and the contributing-node
highlights now draw the empirical polygon or nothing, node popups quote
measured reach instead of declared range and beam, and the gap
diagnostic tests membership with point-in-polygon against the measured
coverage rather than isInBeam against the declared wedge.  geo.ts keeps
yagiSectorPositions and its tests; nothing draws with it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
pipeline.md §7 gains what FOV_MODE=off actually publishes now (an
evidence-only polygon: own-count open bins, ≤2-bin holes bridged, no
theoretical clip) and the public node handle every payload carries.
arc-display.md said selecting an arc track draws the detecting node's
beam wedge; it draws the measured polygon, or nothing.  architecture.md
gets a pointer to services/node_ref.py beside the other services.

Also corrects CALIBRATION_SCHEMA in the same paragraph: 6 since the
bin_pos_ts bump, not 5.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
With the fuzz off, public_node_summaries returns the analytics manager's own
cached summary dicts; adding a key to those grew the manager's cache from the
publisher. Same payload, new dicts.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Carries the site markers, node_ref labels and measured-only coverage over
main's map palette/theme refactor: the site icon is built from the active
palette like nodeIcon() now is, the coverage layer takes its colour from the
palette, and the toolbar's regrouped overflow menu keeps the renamed
"Coverage gaps" button.  Submodule bumped to retina-analytics
feat/evidence-only-coverage merged with its main.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@claude

This comment has been minimized.

main's identity guard scans every tracked file for real node ids; the
module's own explanation named one.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@claude

This comment has been minimized.

…ixtures

main's identity guard bans real node ids in tracked files; the fixtures
named two co-located real nodes.  Same scenarios, invented ids.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@claude

This comment has been minimized.

…f snapshot on a failed read, memoise site grouping

- /api/radar/analytics built an owner's private node summary fresh without
  the node_ref the cached listing carries, so the owner's own node was the
  one unnamed marker on their map.
- node_ref._snapshot cleared the registered-ref cache before the fallible
  read; a transient database failure renamed every registered node to a
  derived ref until the next good refresh.  Fetch first, swap second.
- NodeMarkersLayer re-grouped the whole fleet by site on every selection
  change; the grouping is now memoised on the node list alone.
- test_publication's owner-copy check compared uptime_s byte-for-byte across
  two summaries taken at different instants; compared with a tolerance now.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@claude

This comment has been minimized.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@claude

claude Bot commented Sep 11, 2026

Copy link
Copy Markdown

Claude finished @jehanazad's task in 17m 18s —— View job


Claude finished @jehanazad's task in 6m — code review

Todo list

  • Read repo instructions (CLAUDE.md) and understand PR context
  • Run /code-review on the current diff (origin/main...HEAD)
  • Verify findings directly against the code before posting
  • Post final review feedback

Summary

I re-ran a full review of this sync against origin/main...HEAD and independently verified each candidate finding by reading the actual code rather than trusting the diff/commit description. Two are worth fixing; the rest are minor/optional. I discarded a couple of speculative findings from the raw pass that didn't hold up under verification (noted at the bottom for transparency).

Findings worth addressing

1. /api/radar/nodes still leaks the raw node_id via "name", right next to the new node_ref fieldbackend/services/tasks/analytics_refresh.py:384-386

"status": info.get("status"),
"node_ref": public_node_ref(nid),
"name": info.get("config", {}).get("name", nid),

This is the exact block the PR edited to add node_ref. "name" falls back to the raw node_id whenever config has no name key — which, per this PR's own node_ref.py docstring, is precisely the case for "the blah2 bridge and plain-TCP nodes [that] never register" and make up the whole fleet on the test deployment. So the unauthenticated /api/radar/nodes payload publishes nde… in one field and the operator's raw node id in the very next one for the same node — undercutting the stated goal that "every node [is] named by a public node_ref." (The current frontend, PhysicsSettings.tsx, doesn't render .name today, so this isn't yet visible in the UI, but the raw JSON is public.) Worth either dropping the nid fallback or replacing it with the node_ref.
Fix this →

2. A shared synthetic site never shows the count badgefrontend/src/components/LiveAircraftMap.tsx:925-939

if (site.isSynth) {
  return (
    <React.Fragment key={`site-${site.key}`}>
      {disc}
      <CircleMarker ... radius={5} ...>{popup}</CircleMarker>
    </React.Fragment>
  );
}

nodeSiteIcon(site.nodes.length) (the new count badge) is wired only into the real-node Marker branch below this one. A multi-node synthetic site — plausible given the fleet includes dozens of synthetic nodes per the association-status docstring — renders as a plain 5px circle indistinguishable from a single-node site, which is exactly the ambiguity this PR's own comment says the badge exists to fix ("Sites carry a count badge instead"). Since isSynth requires every member to be synthetic, this is a real gap for the synthetic fleet specifically, separate from the real Atlanta/Greenville sites called out as "owed after merge+deploy" verification.

Minor / optional

  • groupNodesBySite has no null guard on coordinates (nodeSites.ts:64): n.rx_lat.toFixed(5) will throw if a node ever reaches this function with a null/undefined coordinate. hooks.ts's upstream null-island guard (Math.abs(rxLat) < 1e-6 && Math.abs(rxLon) < 1e-6) only skips when both are near-zero, and is explicitly documented there as defense-in-depth rather than the primary guard. Low likelihood given backend invariants, but a single bad node would take down the whole NodeMarkersLayer, not just that marker.
  • nodesById (line 1150) duplicates nodesByIdRef's build (line 1141-1146) — both build the same {node_id: node} map from the same nodes array (one via useEffect+ref for the render-loop consumer, one via useMemo for nodeLabelFor). Not a bug, but the two copies could silently diverge under a future edit since nothing ties them together.
  • Dead code: yagiSectorPositions and isInBeam in geo.ts have no remaining callers outside their own tests and the index.ts re-export, now that every theoretical-cone caller has been removed. Fine to leave for now, but worth deleting in a follow-up since nothing exercises them at runtime.

Discarded during verification

Two findings from the initial pass didn't hold up and I'm dropping them rather than reporting noise:

  • A claim that node_ref.py's _derived cache grows unboundedly across salt rotations — checked node_fuzz_salt()/_persisted_salt(): the salt is read from an env var or a file cached once per process, so it can't change without a process restart, which also resets the module-level _derived dict. Not reproducible within a running process.
  • A claim that splicing node_ref onto summaries at 4 call sites (instead of inside public_node_summary) risks future consumers missing it — true in the abstract, but the PR's own comments at each call site already call this tradeoff out explicitly, so it's a documented deliberate choice, not a latent gap.

No other issues found in the rest of the diff (node_ref.py's HMAC derivation, pointInPolygon, icons.ts's badge rendering, the doc updates). This is a well-documented, carefully-scoped PR; the two findings above are worth a follow-up commit but neither looks blocking on their own — your call given the "not verified in a browser" caveat already in the PR description covers the badge/count verification anyway.
· branch feat/node-site-markers-empirical-coverage

@jehanazad
jehanazad merged commit 39b41da into main Sep 13, 2026
16 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>
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.

1 participant