Skip to content

Land the seeding world gate on main and pin merged submodules - #269

Merged
jehanazad merged 4 commits into
mainfrom
chore/land-seed-wiring-and-pins
Aug 27, 2026
Merged

jehanazad merged 4 commits into
mainfrom
chore/land-seed-wiring-and-pins

Conversation

@jehanazad

Copy link
Copy Markdown
Contributor

Summary

Recovery PR for a stacked-merge race: #268 merged into its base branch feat/adsb-world-provenance after #267 had already merged that branch to main, so the seeding-pool world gate never reached main. This merges the stranded wiring in and bumps both submodule pins to the upstream merge commits.

Changes

Test coverage

  • Targeted backend suites green on this branch against the merge-commit pins: test_known_claiming.py, test_adsb_seed_backend.py, test_solver_stats.py, test_sim_ingest.py (145 passed).
  • Pre-commit gate clean.

Review notes

🤖 Generated with Claude Code

jehanazad and others added 4 commits August 27, 2026 04:15
Claiming's world gate closed one consumer of the mixed sim/real ADS-B
cache; this closes the other two.  The associator's seed round gets the
node-world resolver (retina-analytics #23's node_world_provider) so a
tracklet's tag is never verified against a state from the other world,
and the frame processor's auto-tag pass — a cache-wide assignment for a
node with no receiver, the same decoy exposure claiming had — filters
the snapshot to the node's own world before the node-agnostic lib call.

node_world moves to core/state.py as the single authority: claiming,
the seed gate and the auto-tag filter all key on one resolver, where
two could disagree and let one consumer accept what another rejects.
The lib's adsb_seed_world_rejects counter joins the adsb_seed block of
/api/radar/association/status (getattr with a zero default, so an old
lib pin reads as zero rather than breaking the route).  Submodule bump
to the retina-analytics commit that carries the provider hook.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wire the world gate into ADS-B seeding
PR #268 merged into its stacked base branch after #267 had already
merged, so the seeding wiring never reached main.  This merges it in
and bumps both submodule pins to the upstream merge commits:
retina-analytics #23 (node_world_provider seed gate) and
retina-simulation #12 (--real-adsb opt-in relay gate).

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

claude Bot commented Aug 27, 2026

Copy link
Copy Markdown

Claude finished @jehanazad's task in 5m 32s —— View job


Code review — PR #269

  • Gather context (PR diff, description, CLAUDE.md)
  • Understand request: code review (/code-review:code-review offworldlabs/retina-server/pull/269)
  • Run code review over the PR diff
  • Post review findings

Scope: Traced the retina-server diff line-by-line (this PR is mostly the stranded #268 content landing, plus two submodule pin bumps) and cross-checked call sites for the _node_worldstate.node_world() move.

What looks solid:

  • backend/core/state.py: node_world() is a clean move of known_claiming._node_world, correctly ordered before node_associator construction, wired via node_world_provider=node_world.
  • known_claiming.py / frame_processor.py: all call sites updated to state.node_world(...); grepped the backend for stale _node_world references — none remain.
  • frame_processor.py's new cross-world filter mirrors the existing gate in known_claiming.py and is covered by a new, well-constructed test (test_cross_world_state_is_not_attached).
  • routes/analytics.py's new world_rejects key follows the existing getattr(..., 0) pattern used for every other counter in that block.
  • Import ordering/circularity for the relocated function is safe — the function-local import inside core/state.py is preserved, matching the pre-existing pattern.
  • Submodule pin bumps (libs/retina-analytics, libs/retina-simulation) couldn't be inspected directly (no network access to the submodule repos in this sandbox), but test_associator_gets_the_state_world_resolver self-verifies the node_world_provider kwarg name — a mismatch there would raise TypeError at import and fail the whole suite, so that risk is effectively covered by construction.

No correctness bugs found in the retina-server-side diff — it's a faithful move/rename plus a correctly-gated, well-tested new filter. Two observability gaps worth a look:

1. backend/routes/analytics.py:175world_rejects counter is untested against a real reject

"world_rejects": getattr(_a, "adsb_seed_world_rejects", 0),

This reads adsb_seed_world_rejects via getattr(..., 0), but no test exercises an actual cross-world seed-reject to confirm that attribute name matches what the pinned retina-analytics lib exposes. If the pinned lib's counter is named differently — now or after a future submodule bump — getattr silently falls back to 0: /api/radar/association/status would report zero world-rejects forever with no test failure, masking exactly the cross-world contamination this PR was built to surface. The existing test_association_status_carries_the_adsb_seed_block only checks key presence, not that the value reflects a real reject.

2. backend/services/frame_processor.py:379-380 — silent filtering in the predictive-tagging world gate

_nw = state.node_world(node_id)
_states = {h: s for h, s in state._adsb_for_seeding().items() if s.get("world") in (None, _nw)}

This new filter drops other-world ADS-B states with no counter, unlike its sibling world gates in this same PR (known_claiming's known_claims_world_rejects, the associator's adsb_seed_world_rejects) which are the documented single source of truth for this kind of rejection. A node with heavy real/sim cache overlap could have most of its predictive auto-tag candidates filtered out every frame with zero operator-visible signal — there'd be no way to distinguish "no aircraft nearby" from "world gate is silently discarding everything" via the existing /api/radar/association/status telemetry.

Neither is a correctness bug in this PR — both are pre-existing-style gaps in observability that this PR's own stated goal (making cross-world contamination visible) would benefit from closing.

@jehanazad
jehanazad merged commit e0fc41a into main Aug 27, 2026
15 checks passed
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