Skip to content

sim separation + honest GT error lines + deploy-verification rule - #228

Merged
jehanazad merged 2 commits into
mainfrom
fix/sim-separation-and-map-error-lines
Aug 24, 2026
Merged

jehanazad merged 2 commits into
mainfrom
fix/sim-separation-and-map-error-lines

Conversation

@jehanazad

@jehanazad jehanazad commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Three related fixes extracted from the droplet's deploy branch (deploy/fixes-on-compose-rename), where they have been running on retina-test since 2026-08-21, now rebased cleanly onto main.

Depends on offworldlabs/retina-simulation#10 merging first — this PR pins libs/retina-simulation to 7c5f6f1, the rebased tip of that PR's fix/aircraft-separation branch.

What & why

  • Simulator aircraft separation — bumps libs/retina-simulation to fix/aircraft-separation: spawn poses resample away from live traffic, and in-flight conflicts slow the later-created aircraft toward 70% of cruise. The hub-radial planner was stacking traffic on the metro core, making association ambiguity a property of the simulator rather than of realistic traffic.
  • Honest GT error lines on the map — in LiveAircraftMap.tsx, the matched-GT error line for an arc-only track now requires the arc to actually be present. Beam-gate suppression keeps the track entry but withholds its arc (the track's only map presence), so the midpoint fallback drew dashed yellow lines ending at empty ground. The GT dot stays; the line only draws with an honest endpoint.
  • Deploy-verification rule — CLAUDE.md now instructs verifying on the environment's live map in a real browser after deploying; green tests don't cover the compose/env/frontend seams.

The second commit re-pins the submodule from the original droplet tip ee7f2be to 7c5f6f1, the same change rebased onto retina-simulation's main (old tip preserved at backup/aircraft-separation-ee7f2be).

Testing

Frontend CI suite (node 20, matching .github/workflows/ci.yml) on the rebased branch:

  • npm run lint — clean
  • npm run typecheck — clean
  • npm test — 9 files, 95 tests, all passed
  • npm run build — succeeded

Backend sim-path tests against the re-pinned submodule (RADAR_TCP_PORT=13012 for this droplet's known port flake): test_simulation.py test_metro_and_arcs.py test_stonesoup_metrics.py — 20 passed, 1 skipped.

Rebase onto origin/main (6b6fd3c) applied without conflicts.

🤖 Generated with Claude Code

- libs/retina-simulation → fix/aircraft-separation (ee7f2be): spawn poses
  resample away from live traffic and in-flight conflicts slow the
  later-created aircraft toward 70% of cruise — the hub-radial planner
  stacked traffic on the metro core, making association ambiguity a
  property of the simulator rather than of realistic traffic.
- LiveAircraftMap: the matched-GT error line for an arc-only track now
  requires the arc to actually be present.  Beam-gate suppression keeps
  the track entry but withholds its arc — the track's only map presence —
  so the midpoint fallback drew dashed yellow lines ending at empty
  ground.  The GT dot stays; the line only draws with an honest endpoint.
- CLAUDE.md: after deploying, verify on the environment's live map in a
  real browser; green tests don't cover the compose/env/frontend seams.

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

claude Bot commented Aug 24, 2026

Copy link
Copy Markdown

Claude finished @jehanazad's task in 4m 43s —— View job


Code Review

  • Gather context (PR diff, files, description)
  • Understand the request (custom instruction: run /code-review on this PR)
  • Review changed files for correctness, simplification, efficiency
  • Post final review feedback

Findings

1. computeError (detail panel) wasn't updated alongside MatchedGroundTruthLayer — the panel and map now disagree in exactly the case this PR set out to fix

frontend/src/components/LiveAircraftMap.tsx:1579-1585:

if (ac.position_source === POSITION_SOURCE_ARC_ONLY) {
  const near = arcNearestPoint(ac, nodesByIdRef.current?.[ac.node_id], gtLat, gtLon);
  if (near) return near.distKm;
}
return distanceKm(ac.lat, ac.lon, gtLat, gtLon);  // ← dishonest midpoint fallback

When an arc-only track is beam-gate-suppressed this tick, arcNearestPoint returns null. In MatchedGroundTruthLayer (lines 210-227) this now correctly sets hasAnchor = false and hides the yellow line — the fix this PR ships. But computeError (called from AircraftDetailPanel.tsx:12 as computeError(ac.hex, ac), feeding the "Pos Error" field at AircraftDetailPanel.tsx:270-275/400) falls through to the same arc-midpoint distanceKm(ac.lat, ac.lon, ...) fallback it always used. A user who clicks that aircraft still sees a fabricated km value with an accuracy badge — contradicting the shared-invariant comment in arcErrors.ts:8-9 ("The error line and the panel 'Pos Error' both use this helper so they always agree") and computeError's own comment at line 1559-1560. Suggest mirroring the map layer's fix: return null (or similar "no estimate" sentinel) from computeError when near is null, and have AircraftDetailPanel render "—" / hide the field in that case, matching the map's "no honest endpoint → no line" behavior.

Fix this →

2. Minor: wasted work in MatchedGroundTruthLayer when hasAnchor is false

frontend/src/components/LiveAircraftMap.tsx:228-229errKm/label are still computed via the midpoint-distance fallback even when hasAnchor was just set to false (line 225) and no line will be drawn or updated this tick. Harmless (happens once per suppressed arc-only track per ~250ms tick) but easy to skip with an early branch once hasAnchor is known false, since neither value is used in that path.

Other notes

  • Simulator submodule bump and CLAUDE.md deploy-verification addition look fine and match the PR description.
  • No backend files touched, consistent with the description.
    · branch fix/sim-separation-and-map-error-lines

The submodule branch was rebased onto its main and force-pushed; new tip
7c5f6f1 (old ee7f2be preserved at backup/aircraft-separation-ee7f2be).
See offworldlabs/retina-simulation#10.

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

claude Bot commented Aug 24, 2026

Copy link
Copy Markdown

Claude finished @jehanazad's task in 1m 10s —— View job


Code Review

  • Gather context (PR diff, files, description)
  • Understand the request (custom instruction: run /code-review on this PR)
  • Review changed files for correctness, simplification, efficiency
  • Post final review feedback

What changed since the last review

Only one new commit landed: 706f9e0 re-pins the libs/retina-simulation submodule from ee7f2be to 7c5f6f1, because the fix/aircraft-separation branch in that repo was rebased and force-pushed (old tip preserved at backup/aircraft-separation-ee7f2be, per offworldlabs/retina-simulation#10). CLAUDE.md and frontend/src/components/LiveAircraftMap.tsx are byte-identical to what was already reviewed (same blob SHAs 1f337eb / ac2c1d4).

  • The re-pin itself is a mechanical pointer bump with no independent logic to review here — I don't have submodule history checked out in this sandbox to diff ee7f2be vs 7c5f6f1 content directly, but a rebase (vs. a rewrite) should be content-preserving, and the commit message documents the reason and where the old commit is recoverable. Nothing concerning about the change itself.

Still open from the previous review (unaddressed — file unchanged)

computeError (detail panel) still disagrees with MatchedGroundTruthLayer (map line) for suppressed arc-only tracks

frontend/src/components/LiveAircraftMap.tsx:1579-1585:

if (ac.position_source === POSITION_SOURCE_ARC_ONLY) {
  const near = arcNearestPoint(ac, nodesByIdRef.current?.[ac.node_id], gtLat, gtLon);
  if (near) return near.distKm;
}
return distanceKm(ac.lat, ac.lon, gtLat, gtLon);  // ← dishonest midpoint fallback

When beam-gate suppression causes arcNearestPoint to return null, MatchedGroundTruthLayer (lines 210-227) now correctly hides the yellow line (hasAnchor = false). But computeError — which feeds the "Pos Error" field in AircraftDetailPanel.tsx (~line 270-275/400) — still falls through to the same midpoint distanceKm fallback the map line was just fixed to stop using. A user who opens that aircraft's detail panel still sees a fabricated km value, contradicting the shared-invariant comment in arcErrors.ts:8-9 ("the error line and the panel 'Pos Error' both use this helper so they always agree") and computeError's own comment above it. Suggest returning null from computeError in that branch and rendering a placeholder (e.g. "—") in AircraftDetailPanel when the value is null, mirroring the map's "no honest endpoint → no line" behavior.

Fix this →

Minor (unchanged, low priority): MatchedGroundTruthLayer (LiveAircraftMap.tsx:228-229) still computes errKm/label even when hasAnchor is false and neither value ends up used — harmless, easy to skip with an early branch.

Other notes

  • No backend files touched, consistent with the description.
    ·

@jehanazad
jehanazad merged commit a16ed55 into main Aug 24, 2026
27 checks passed
@jehanazad
jehanazad deleted the fix/sim-separation-and-map-error-lines branch August 25, 2026 05:29
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