Skip to content

Retire orphaned dark keys behind DARK_FOLLOW_ORPHAN_MODE - #303

Closed
jehanazad wants to merge 1 commit into
feat/dark-track-followingfrom
feat/dark-follow-orphan-rule
Closed

jehanazad wants to merge 1 commit into
feat/dark-track-followingfrom
feat/dark-follow-orphan-rule

Conversation

@jehanazad

Copy link
Copy Markdown
Contributor

Stacked on #299 (feat/dark-track-following) — review that first; this branch's own diff is the last commit.

What

A new rule in the dark-follow lane, behind a new env flag DARK_FOLLOW_ORPHAN_MODE (off|on, default off): when an established (followed) dark key A publishes a solve built from the same node tracks that a younger key B was last built from, B has no evidence of its own left, so it is retired at publish time instead of drifting to its 30 s expiry.

Since #302 stopped supersession from popping another aircraft's key on a bare shared source-track id, fragment keys that used to be popped now live out that full expiry beside the real track. This is the narrow, provable slice of them.

Why it is flagged off

Offline replay of this exact predicate over three 20-min captures says the benefit is small:

  • cap17: 7 of 38 ghost keys retired, 0–34 ghost frames removed, 13–21 correct frames lost.
  • cap15: 5 ghost frames removed, 21 correct frames lost.

So it ships default off, to be measured live on the test droplet without changing any default behaviour. The predicate is implemented as specified and deliberately not tuned to look better — the live capture is the verdict. With the flag off nothing about the publish path changes.

The predicate — dark_follow.find_orphans(tracks, key, new_ids, ts_s, solve_count, n_nodes)

Returns [] unless orphan_mode() == "on", mode() == "binding" (the rule is a key-ownership rule, and ownership only exists when the lane binds), key is mn-dark-*, new_ids is non-empty, and A is established (solve_count >= DARK_FOLLOW_MIN_SOLVES, n_nodes >= DARK_FOLLOW_MIN_NODES, both as stored on the entry after this publish).

A key B is then retired iff its source_track_ids are non-empty; B is strictly older than the solve and within DARK_FOLLOW_ORPHAN_WINDOW_S (default 30 s) of it; at least _ORPHAN_MIN_SHARED = 2 of B's ids are in new_ids and they are a majority of B's; B has no more solves than A; and B is not recently_followed (a key the lane is itself refreshing has evidence that is not in tracks at all).

Changes

  • services/dark_follow.pyorphan_mode() accessor (shaped like mode(), same defensive parsing), find_orphans(), the two constants with their WHY comments, and a docstring paragraph placing the rule against ownership.
  • services/tasks/solver.py — new _forget_mn_key() factors the four-store removal (entry, anomaly hex, smoother position history, KF state) that supersession already did; the supersession loop now calls it, with no behaviour change. The orphan call site sits in _process_solver_item under _MN_TRACKS_LOCK, right after the entry is stored, so the counts it judges by are the ones this publish wrote; each orphan is removed, put in follow cooldown (drop_target), and counted. _record_solve_history gains an orphaned_keys kwarg plumbed like superseded_keys.
  • core/state.pyDARK_FOLLOW_ORPHAN_MODE parsed beside DARK_FOLLOW_MODE; dark_follow_orphaned counter beside dark_follow_dropped, reset with it.
  • routes/test.pydark_follow_orphaned in the fragmentation block of /api/test/solver-stats, beside the supersession counters it is read against.
  • tests/test_dark_follow.pyTestOrphanRule (14 predicate tests: off by default, unreadable flag, both inert follow modes, the positive case, 1-of-3 and 2-of-5 majority refusals, newer-than-solve, past the window, better-established B, recently-followed B, unestablished A by solves and by nodes, ADS-B keys on both sides, no provenance) and TestOrphanRuleInTheSolver (end-to-end through _process_solver_item: with the flag on the fragment leaves multinode_tracks, dark_follow_orphaned bumps, the key enters follow cooldown and the history record lists it in orphaned_keys with superseded_keys empty; with the flag off the same publish leaves it in place).
  • tests/test_solver_stats.py — the two exact-key assertions on the fragmentation block updated for the new counter.

Verified

  • tests/test_dark_follow.py tests/test_mn_lifetime.py tests/test_solver_anchor.py tests/test_solver_stats.py: 168 passed.
  • Full backend suite (pytest tests/ -n 2 -m "not external"): 2834 passed, 2 skipped in 164.88s.
  • pre-commit run --all-files: all five hooks passed (ruff check, ruff format, vulture, both ruff-config checks).

🤖 Generated with Claude Code

Since supersession stopped popping another aircraft's key on a bare
shared source-track id, a fragment key that the guard refuses now lives
out its full 30 s expiry beside the real track.  When an established
followed key publishes a solve built from the very node tracks a younger
key was last built from, that younger key has no evidence of its own
left, so it can be retired at publish time instead.

Offline replay of this exact predicate over three 20-min captures says
the benefit is small and two-sided (cap17: 7 of 38 ghost keys retired,
0-34 ghost frames removed against 13-21 correct frames lost; cap15: 5
ghost frames removed against 21 correct frames lost), so the rule ships
behind DARK_FOLLOW_ORPHAN_MODE, default off, to be measured live rather
than assumed.  Nothing changes with the flag off.

The removal is factored into solver._forget_mn_key so the orphan path
erases exactly the four stores supersession does; supersession's own
behaviour is untouched.

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

claude Bot commented Sep 5, 2026

Copy link
Copy Markdown

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


Claude is reviewing this PR

  • Gather context (PR body, changed files, repo guidelines)
  • Review services/dark_follow.py (orphan_mode(), find_orphans())
  • Review services/tasks/solver.py (_forget_mn_key(), orphan call site, history plumbing)
  • Review core/state.py and routes/test.py changes
  • Review test coverage in tests/test_dark_follow.py and tests/test_solver_stats.py
  • Post final review

Review

Overall this is a careful, well-documented change: the predicate is small and pure, correctly gated behind orphan_mode() == "on" and mode() == "binding", defaults to off with a shipped-off replay case made in the WHY comments, and the tests (TestOrphanRule + TestOrphanRuleInTheSolver) cover the predicate's boundary conditions thoroughly (window edges, majority refusal, established tie-break, recently_followed, ADS-B exclusion on both sides, no-provenance). I traced the lock discipline (_MN_TRACKS_LOCK held throughout, find_orphans itself takes no lock, _forget_mn_key preserves the exact four-store removal order the old inline supersession code had) and didn't find a correctness or concurrency issue.

A few things worth a look:

1. docs/solverflow.md's dark-follow settings table isn't updated (moderate).
docs/solverflow.md:439-446 maintains an explicit table of every DARK_FOLLOW_* constant (DARK_FOLLOW_MODE, _OWN_S, _SHADOW_KM, _MIN_SOLVES/_MIN_NODES, etc.) with its default and defining file — clearly meant to be kept in sync, since section 3d was updated for the ownership rule that landed just before this PR (#302/#299). This PR adds DARK_FOLLOW_ORPHAN_MODE, DARK_FOLLOW_ORPHAN_WINDOW_S, and _ORPHAN_MIN_SHARED to services/dark_follow.py but doesn't add a row for them or a paragraph describing the orphan rule alongside the existing binding-mode flow description. Since the flag is off by default and the PR's whole point is "measure it live," a reader of that doc has no way to discover the rule exists.
Fix this →

2. Minor: dark_follow_dropped is also incremented for every orphan (by design, but worth flagging).
solver.py's orphan loop calls dark_follow.drop_target(old_key, ...) to seat the cooldown, and drop_target unconditionally bumps state.dark_follow_dropped (services/dark_follow.py:275) as a side effect. So turning the flag on will inflate the existing dark_follow_dropped counter (a metric that has meant "the ghost guard gave up on a key it was following") with orphan retirements too. This is already asserted in test_the_publish_retires_the_fragment (tests/test_dark_follow.py:1059), so it's clearly intentional and not a bug — just flagging in case the live-replay analysis this PR is meant to enable reads dark_follow_dropped anywhere as a clean signal; it will need to be read net of dark_follow_orphaned once the flag is on.

3. Minor nit: the "majority" check allows an exact tie.
find_orphans's majority test (services/dark_follow.py, shared * 2 < len(old_ids)) accepts shared == len(old_ids) / 2 (e.g. 2 of 4) as sufficient, which is a tie rather than a majority as the docstring/PR description describe it. Given _ORPHAN_MIN_SHARED = 2 already requires at least 2 overlapping ids, this only matters at exactly 4 shared ids, and given the whole feature is explicitly a to-be-measured, not-yet-tuned predicate, this is very low priority — just noting the wording vs. behavior gap.

4. Not flagged as an issue, but noted for completeness: backend/.env.example isn't updated with DARK_FOLLOW_ORPHAN_MODE/DARK_FOLLOW_ORPHAN_WINDOW_S, which CLAUDE.md calls for generally ("Add new keys to backend/.env.example"). However, the base branch (feat/dark-track-following) already didn't list several sibling constants (DARK_FOLLOW_OWN_S, DARK_FOLLOW_SHADOW_KM, DARK_FOLLOW_MIN_NODES) either, so this PR is consistent with existing (if incomplete) practice in this area — optional to fix.

Verification note: I was not able to run pytest or pre-commit in this sandbox (no network/interpreter access without approval that isn't available in this headless run), so I relied on static review of the diff plus the existing test suite structure rather than executing it. The PR body's reported results (168 passed on the targeted files, 2834 passed / 2 skipped full suite, all pre-commit hooks green) were not independently re-verified here.

No blocking issues found — the docs-table gap (#1) is the only thing I'd actually ask to see addressed before/after merge.

jehanazad added a commit that referenced this pull request Sep 5, 2026
Live measurement of PR #303's orphan rule against the cap17 overlay.

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

Copy link
Copy Markdown
Contributor Author

Live measurement on the test droplet (cap19, 20 min, stack 6593a6e = cap17 overlay + DARK_FOLLOW_ORPHAN_MODE=on, MN_DARK_EXPIRY_S back at default):

  • The rule fired twice in 20 minutes. Both retirements removed a key belonging to a different ground-truth aircraft than the publisher (one of those keys had a good last solve, one a bad one). Zero same-aircraft fragments retired.
  • Ghost rate was 3.3% (24 of 737 dark feed entries) vs 13.9% in cap17, but the fleet was ~4x thinner (737 vs 2,988 entries), and two firings cannot account for the difference. Remaining ghosts: 58% bad two/three-node solves, 38% stale entries after a good solve.
  • Shown rate 6+/4–5/2–3: 100%/89%/43% (217/245/408 samples).

Verdict: consistent with the offline replay in the description. The rule is inert at best and its rare firings were cross-aircraft. Recommend leaving this closed/unmerged; the ghost lever is upstream (bad low-node solves), not key lifecycle.

🤖 Generated with Claude Code

@jehanazad

Copy link
Copy Markdown
Contributor Author

Closing per the 2026-09-10 review against main 886a342. The cap19 measurement above stands (two firings in 20 min, both cross-aircraft), and #331 now joins bottom-up solves onto followed keys on the same >=2-shared-node-track signal at keying time, so the fragment population this rule targeted is mostly never minted. The predicate is also stale against main: it judges A by the last solve's n_nodes while the follow lane's bar uses max(max_n_nodes, n_nodes) (dark_follow.py:410). The _forget_mn_key refactor is being carried forward on a fresh branch.

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