Skip to content

Sweep multinode_tracks under the solver's track lock - #300

Merged
jehanazad merged 36 commits into
mainfrom
fix/feed-sweep-under-tracks-lock
Sep 6, 2026
Merged

jehanazad merged 36 commits into
mainfrom
fix/feed-sweep-under-tracks-lock

Conversation

@jehanazad

Copy link
Copy Markdown
Contributor

Why

build_combined_aircraft_json snapshots state.multinode_tracks and pops expired entries without _MN_TRACKS_LOCK, while the solver worker iterates the same dict in multinode_key_decision holding that lock. On the test droplet today this raised RuntimeError: dictionary changed size during iteration in the keying scan and the solve was lost. It became visible once MN_DARK_EXPIRY_S dropped to 30 s (#297) and the dark track-following lane (#299) raised the publish rate, but the race is older than both.

What

Take the lock for the snapshot and for each pop; lazy import of the solver module because it owns the lock and a module-level import would cycle through the task modules.

Verified

test_feed_multinode.py test_mn_lifetime.py test_aircraft_flush.py green; pre-commit clean on the file.

🤖 Generated with Claude Code

claude and others added 22 commits September 5, 2026 06:25
Bumps libs/retina-analytics to the branch commit that stops register_node and
rebuild_zones_for building an overlap zone between two nodes in different
worlds (offworldlabs/retina-analytics#25).

This fleet is the reason: 50 synthetic nodes and 8 receivers share one
footprint over Greenville, so the associator held a zone for all 400 sim/real
pairs — 39 of them a live adjacency edge — and a pairing drawn from one of
those grids can only ever match a simulated tracklet against a real echo.
Real node ids showed up in 48 of 156 synthetic dark solver records over a
7-minute window.  node_world is already the single authority for the question
and is already injected as node_world_provider, so nothing new is wired here.

/api/radar/association/status gains assoc_world_skipped_pairs next to
overlap_zones: without it a fleet whose cross-world pairs are being refused
reads exactly like a fleet whose pairs never overlapped.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The ghost rate cannot see the failure the cluster-merge rework targets.  A
solver input that merges two aircraft usually still solves within MATCH_KM
of one of them, so it counts as *matched* while carrying 4-5 km of position
error and a node that never saw the aircraft it was published as.  Nothing
in the bench reported that, so the fix had no acceptance criterion.

--mode track now scores every solver input association emits, ahead of the
solve and ahead of every gate below it, against a truth side-channel: which
aircraft actually produced each detection.  That channel cannot come from
the frame's own ADS-B list -- the simulator appends None there for every
aircraft with has_adsb False, and dark aircraft are exactly the population
under study -- so each detection is matched back to the aircraft whose
noiseless (delay, doppler) it is nearest, at a gate ~5 sigma wider than the
simulator's own measurement noise.  It is built before _strip_adsb and never
reaches association, so the blind discipline is intact.

Reports contaminated_inputs_pct and foreign_nodes_per_input, per seed and
pooled, alongside the ghost rate and matched error already there.

Also adds the cluster-merge knobs (--merge-dist-km, --pair-vel-exclusive,
--merge-vel-consistent and the two velocity thresholds) so each sub-step of
the rework can be swept on its own; each defaults to None, meaning "leave
the library's default alone", so a plain run measures what the library
currently ships rather than freezing today's values into the bench.

Pins retina-analytics at fix/cluster-contamination.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Two independent changes to the same enqueue-and-solve path.

1. SOLVER_ALT_MODE (core/state.py, default "sweep" — nothing changes until
   it is set).  The n>=3 altitude sweep solves once per fixed layer and keeps
   the lowest rms_delay.  Its layers are 2 km apart, so the altitude it pins
   is systematically up to 1 km wrong, and on noise-free replay of this
   fleet's geometry that quantisation ALONE left rms_delay at a 1.76 us
   median against the 3.0 us reject gate — most of the gate's budget spent on
   an altitude the measurements themselves determine, and the residual left
   over then blamed on nodes, so _trim_and_resolve drops measurements that
   were never the problem.

   In "free" mode _solve_best_altitude makes ONE pool call to the
   geolocator's solve_multinode_multistart with three start layers (the one
   nearest the association guess and its two neighbours), altitude solved as
   a sixth unknown — against the sweep's six round trips, each of which
   pickles the node configs for a child.  n=2 is untouched: altitude is
   unobservable there, so _solve_best_altitude_n2 keeps its single pinned
   solve and the geolocator pins regardless of the flag.  Trimming re-solves
   through _solve_best_altitude, so a trim round inherits the mode its first
   solve used — mixing a swept altitude with a free one would make the rms it
   compares between rounds a different quantity.

   Both modes stamp altitude_mode on every history record, published or
   rejected, and free adds the per-start residuals and z_saturated.  That is
   the whole comparison channel: one deploy of each, read off
   /api/test/mlat-history.  There is deliberately no shadow mode — the two
   produce the same shape of result, so running both would double the
   solver's cost to learn what a deploy of each already says.

2. The frame path shipped ALL 58 node configs with every solver input.  The
   pool is a spawn pool, so that whole set is pickled per solve while a
   candidate carries 2-8 measurements.  configs_for_solver_input restricts it
   to the measurement node ids; nothing downstream needs the rest — the
   solver builds NodeSetups from the measurements, trimming and consensus
   only ever narrow that set, the beam gate iterates contributing_node_ids,
   and cv_epochs is built from the same matched nodes in all three input
   shapes association emits.  The known lane fetches its own configs and is
   unaffected.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
ruff format wrapped the truth-channel lookup and SIM910 wanted the
redundant None default off dict.get.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Three of the signals the dark-lane recovery work is judged on could not be
read from a running server, so every judgement about it has been offline
replay.  This adds no behaviour: nothing about which candidates solve, which
solves publish, or what reaches the map changes.

1. /api/test/mlat-history caps per lane.  The known lane writes ~16x the dark
   lane's volume, so the flat records[:1000] answered a 30 min request with
   the newest ~6 min of dark records and the rest of the window read as a
   quiet period — the same failure the shared deque had before #289 split it,
   moved into the reader.  ?lane= narrows to one lane, ?limit= (max 5 000) is
   applied per lane, and lane_counts is reported pre-cap so truncation stays
   legible.

2. Resolve-slot skips are recorded, not just counted.  Live the rule refuses
   ~1 537 candidates per 646 dark attempts per 30 min, and nothing said WHICH
   claim did it — so a skip that suppressed a duplicate and one that
   suppressed a different aircraft (tracker track ids are shared across
   candidates) were indistinguishable.  A separate 500-entry deque carries
   the blocking claims; solver-stats windows it as resolve_skips, and
   ?kind=resolve_skips dumps it.  Its own deque and not the solve history:
   skips outrun dark records two to one and would evict them.

3. Contamination is measured live.  A dark record matched to ground truth now
   says which of its own contributing nodes could not see that aircraft
   (foreign_node_ids/contaminated), using the associator's own visibility
   predicate — the same gate known-lane claiming uses, so the two cannot
   drift apart.  The GT trail lookup already happened; this costs one cone
   test per node.  Records nothing could be asked about stay out of the
   denominator rather than counting as clean.

4. NODE_FRAME_MIN_INTERVAL_S drops are counted.  frames_dropped is published
   but is the queue-saturation counter and reads zero throughout; the frames
   the per-node rate limiter refuses were uncounted entirely, so "the tracker
   sees what this node sent" looked true from every metric.

docs/solverflow.md's file:line references are replaced with file + symbol —
every one of them had drifted — and gain a section on reading these endpoints.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The suppression rule reads "this aircraft is already on the map at this
width, at every track it is built from". _claim_resolve_slot recorded the
claim BEFORE the solve, so a candidate that never reached the map still made
that statement — and never released it.

Two consequences, both live:

* A rejected candidate blacked out its own identical twin for the full 12 s.
  24% of dark attempts are rejected, and the retry that would have published
  was suppressed by the failure.
* Tracker track ids are shared across the association candidates of DIFFERENT
  aircraft (74 of 178 ids in a 6 min window appeared in solves of more than
  one ground-truth aircraft — the finding that forced _supersession_match's
  spatial guard in #290). So a contaminated superset that the gates sank also
  suppressed the clean subsets behind it, including its neighbour's only
  candidate.

Measured on the test droplet: ~1,537 skips against 646 dark attempts per
30 min. The mechanism refused more than twice as many candidates as it
solved, for aircraft it had put nowhere.

_claim_resolve_slot splits into a pure _resolve_slot_covered (read-only,
run before the solve, also returning the blocking claims for the skip record)
and _record_resolve_slot, called only on the publish path with the POST-TRIM
survivors — result["source_track_ids"], which _filter_s_in_to_nodes rebuilds
from the surviving track_ids_by_node. A trimmed node's track is deliberately
left unclaimed: it contributed nothing to the published position and was
probably another aircraft's, so claiming it would suppress that aircraft on
the strength of a measurement this solve threw away.

The rule itself is unchanged: every track covered at no fewer nodes within
_SOLVER_RESOLVE_INTERVAL_S, widest claim wins, same pruning sweep. No
negative claim for rejects — measure first.

The cost is that the check no longer claims under the same lock, so two
workers can both solve duplicates that arrived together. That is one extra
solve, arbitrated downstream by keying and supersession, against the
starvation above.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The candidate-level rate has a denominator association itself moves.
Splitting one contaminated cluster into two clean sub-clusters plus the
false pairing it was hiding emits three inputs where there was one, and the
false pairing on its own scores as 100% contaminated -- so the rate rises
while what reaches the map gets cleaner.  Measured on the 50-node scatter
scene: 20.3% -> 25.7% of candidates, with the ghost rate by solve falling
3.2% -> 2.4% over the same change.

So the same score is now also taken at the publish point, over solves that
cleared every gate and bound to a real aircraft.  That is the population the
live audit sampled (45% of published dark solves carried a foreign node),
and its denominator is fixed by the aircraft in the sky rather than by how
many hypotheses association chose to emit.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
/api/radar/association/status filed track_pairs_superseded under
"track_pairs_inline_only", documented as permanently zero in production
because the only exclusivity stage ran on a chi2 that cv_fit=None never
computes.  The deferred path now has an exclusivity stage of its own, so the
counter moves on a live fleet and that grouping would be a lie.  Moved to
the live block alongside the new cluster_splits.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Picks up offworldlabs/retina-analytics@7e5414a, which extends the cluster
partition from same-node conflicts only to every group, bounding each
emitted solver input at the merge distance.  Foreign nodes per published
solve 0.80 -> 0.32 on the 15-node ring bench scene.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Two independent corrections to the same failure: legitimately in-cone
nodes being trimmed out of dark solves because their measurement did not
describe the moment the solve claims to.

Stale-track exclusion (TRACK_MAX_STALE_S, default 3.0 s).  A COASTING
track survives N_DELETE=10 frames past its last association, and at the
fleet's 0.74-1 Hz per-node cadence that is up to ~13 s of dead
reckoning.  confirmed_track_views hands association the track's newest
REAL sample and association hands the solver that sample as the node's
current measurement, so an aircraft that has flown out of a node's beam
keeps contributing seconds-old delay to n>=3 solves.  Measured on the
test droplet: 230 out-of-cone nodes reached published dark solves in
20 min, median 4 deg outside the beam edge (p90 22 deg) and 1.8 km
beyond max range.  The freshness test reads the newest entry from
get_recent_detections, which returns only ASSOCIATED samples, so it is
the last real detection's time rather than a coast count — and it is
compared against the frame timestamp being processed, never wall clock,
so replays and backfills are unaffected.

Epoch alignment (SOLVER_EPOCH_ALIGN, default on).  The solver's residual
model evaluates every measurement against one target state; nodes sample
at independent phases, so it is fitting a set that spans up to a frame
interval.  align_measurement_epochs dead-reckons each delay onto the
newest sample's time along its own measured Doppler, using
d(delay_us)/dt = -doppler_hz * 1e6 / fc_hz — the rate implied by the
geolocator's own residual model, in which positive Doppler is a closing
target whose delay is decreasing.  All-or-nothing per input: a partially
aligned set just relocates the error, so a missing t_s, doppler_hz or
fc_hz skips the input and counts solver_epoch_align_skipped.

Both flags exist to be turned off live rather than rolled back.  The
known lane reuses the same helper: it does NOT have one epoch either
(_CLAIM_SPREAD_S admits claims up to 5 s apart, ~1.5 km of motion at
300 m/s), and it is the lane whose residual is the fleet's accuracy
measurement.

Pins retina-analytics feat/measurement-epochs (PR #27), which carries
t_s on each solver-input measurement.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…r more

The free mode shipped with three start altitudes on the reasoning that
freeing z removes the ladder's quantisation but not the LM's locality, so
several starts are still what keeps a solve off the wrong side of a
bistatic ellipse.  On this fleet's geometry there was almost nothing for
them to keep it off.  Over a 20-minute window on test, 1019 free-mode
solves: the three starts' rms_delay differed by more than 0.1 us in 13 of
them (1.3%), and the nearest-layer start — the one the sweep would have
pinned at — was more than 0.5 us worse than the best start in 2.  So the
extra two starts bought ~0.2% of solves a rounding error at three times
the solver CPU, and solver CPU is now the constraint: ~1.7 attempts/s
against a 2.0 s average latency on two pool workers.

The count becomes SOLVER_FREE_ALT_STARTS, default 1, read per call beside
the mode flag and clamped into [1, len(layers)] by _free_alt_starts
against the ladder that module owns.  One start is the layer nearest the
association guess, or the guess altitude itself when the input carries a
non-layer one (ADS-B) — the same splice the sweep does, so the one exact
altitude a candidate has is still what it starts from.  Above 1 the
window is unchanged, so 3 reproduces what was measured; the knob stays
because locality is a property of the geometry, not of this fleet, and a
deployment whose nodes sit nearer an ellipse should not need a code
change to buy the starts back.

alt_starts_km / alt_start_rms_us keep being recorded — a one-element list
rather than three — so the live comparison channel is unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The sign of the Doppler-to-delay-rate conversion is the part of this
that cannot be checked by reading it, so it is checked against the
simulator's own _bistatic_delay / _bistatic_doppler: a target is flown
in a straight line, sampled at two times, and the correction applied to
the older sample must land on the newer sample's true delay.  Three
velocities (inbound, outbound, crossing-with-climb), plus a direct
assertion that the correction and the true delay change share a sign, so
a failure says "the sign is wrong" rather than "the error did not shrink
enough".

The staleness tests express the coast as a gap between the newest
ASSOCIATED sample and the frame time, which is exactly what the tracker
produces — get_recent_detections skips the None measurements mark_missed
appends — and pin that TENTATIVE exclusion is unchanged and does not
feed the stale counter, so the counter keeps meaning "an aircraft left
this node's cone".

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Picks up offworldlabs/retina-analytics@2b83871.  The 6.0 / 4.5 / 3.0 km
sweep on the association bench is monotone with the real-track count flat
at every point, so 3.0 is taken: published contamination 32.0% -> 25.1%,
foreign nodes per published solve 0.76 -> 0.52.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Picks up offworldlabs/retina-analytics@67f1488 (ruff-format only, no
behaviour change).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Two map complaints, both traceable to numbers chosen when a dark aircraft
was re-solved every ~12 s.  Dark solves now land every 1-3 s while a track
is held, which changes what a gap means: it is a lost track, not a cadence
gap, and every budget sized as an allowance for the old cadence now buys
wrong pixels instead of coverage.

Measured on the test droplet over 20 minutes (dark multinode feed entries
against ground truth), median position error by solve age: 1.05 km under
3 s, 1.21 km at 3-8 s, 1.50 km at 8-15 s, 2.02 km at 15-30 s (7% more than
5 km off), 3.99 km at 30-60 s (12% of all displayed dark entries, 32% more
than 5 km off).

Dead reckoning is cut at that curve's two knees.  MN_DR_CAP_S (15 s,
was a hardcoded 30) is how far an entry is extrapolated before it holds its
last point; MN_DARK_EXPIRY_S (30 s) is when a dark entry leaves the feed
entirely.  mn-adsb-* entries keep the 60 s expiry -- a transponder hex
anchors them, so the same gap there is the ADS-B feed breathing.  The
frontend's matching budgets move with them: the dark icon-hide distance
6 km -> 3 km (6 km was 90%-drawable at the 12 s cadence; now it is the
width of the error it hides) and the uncertainty disc's growth cap
60 s -> 30 s, which is the age past which a dark entry cannot exist.

The selected track's per-solve dots came from a 30 s poll, so at a 1-3 s
solve cadence they showed a decomposition of the track that was mostly
missing.  The poll is now MLAT_HISTORY_REFRESH_MS (3 s), and a fall in the
selected entry's `seen` -- the feed announcing a fresh solve -- refetches
inside it.  newSolveArrived is the pure predicate for that, unit-tested.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The test droplet threw 91 ValueError: math domain error tracebacks in
40 minutes out of learned_velocity's sqrt, from two callers that each
lose real work: solver.py's multinode_key_decision (that solve is
dropped) and aircraft_feed's multinode_to_aircraft (the whole feed
broadcast for the tick is dropped).  One sick track was taking down the
map.

Root cause is upstream of the sqrt.  cov_en_km2 is the top-left 2x2 of
s2 * inv(JtJ) for the solver's 5-state fit, and the solver falls back to
pinv only on an outright LinAlgError -- an ill-conditioned-but-singular
JtJ (near-parallel baselines, the degenerate tail that puts the formal
sigma's p99 at 3.8e6 km) inverts to numerical garbage that is INDEFINITE
while still having both diagonals positive.  _measurement_R checked only
shape, finiteness and positive diagonals, so that matrix became R.

An indefinite R is not survivable: _kf_correct's Joseph form preserves
PSD for any gain, but only GIVEN PSD P and R -- its K R K^T term
inherits R's negative eigenvalue -- and _init_entry seeds P's position
block straight from R, so a sick covariance poisons the filter at birth
as well as on every update.  Once P's velocity diagonals go negative,
learned_velocity's sqrt raises.

Three changes, defence in depth:
- _measurement_R now requires the cov to be positive-semidefinite (both
  diagonals > 0 AND determinant >= 0), falling back to the base floor
  otherwise.  That is already this function's documented answer for a
  degenerate cov, and a covariance this broken carries no trustworthy
  relative weighting worth repairing by eigenvalue clipping.
- The predict step re-symmetrises P, as _kf_correct already did.
- learned_velocity clamps its sqrt at 0, so a pathological state degrades
  to "sigma 0" rather than taking out a solve or a broadcast.  The feed's
  per-entry build is now failure-isolated for the same reason: one entry
  raising logs once a minute and skips that aircraft instead of aborting
  the whole tick.

Measured on origin/main across a 180-point sweep of indefinite
covariances (correlation 1.2-10, formal variance 0.01-100 km^2, 1.4/5/20 s
cadence, 0/100/250 m/s): 89 of 180 reproduce the exact math domain error
through the public API, the fastest in 3 solves.  All 180 pass after this
change, with the worst P diagonal over the sweep at +71.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Dark aircraft are solved bottom-up all the way down: tracker tracks, delay
grid, cluster, solve, and only then a key chosen by proximity to whatever
multinode entry is nearest.  Measured on the test droplet: consecutive
solves of one aircraft <5 s apart land on a different key 15% of the time,
any gap re-mints the key, neighbours 3 km apart share keys, and no solve is
ever told where its aircraft is expected to be — the initial guess is a 3 km
grid centroid.

The known lane already inverted that order for ADS-B aircraft.  This adds
the missing half of the analogy for dark ones: an established mn-dark-*
track with a Kalman state predicts (delay, Doppler) at any node exactly the
way a dead-reckoned transponder fix does, so the same claiming machinery
works against it.  The follow solve then carries anchor_key = the followed
key, so multinode_key_decision's anchor branch lands it back on the SAME
track — continuity by construction rather than by the proximity scan
happening to pick the right neighbour.

Node-track ids were the obvious alternative and are not safe: attaching each
solve to the newest key sharing a source_track_id linked the wrong aircraft
12% of the time in a dense metro cluster, for the same reason
_supersession_match stopped trusting a bare shared id.

Following is a feedback loop, so the ghost guard is load-bearing rather than
tidy: two rejected follow-solves in a row, or a filter velocity sigma past
the threshold, drop a key for a cooldown and hand the aircraft back to the
bottom-up lane.  Default mode is shadow — the lane decides what the map
believes in with no transponder to check itself against.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
solve_uncertainty.grown_sigma_m is the backend twin of the frontend's
uncertainty.ts and exists so the two can be pinned to the same shape.  Its
horizon stays with UNCERTAINTY_DR_CAP_S: 60 -> 30 s.  Its docstring claimed
the frontend stops dead-reckoning at 60 s, which stopped being true in the
previous commit, and a dark entry no longer reaches 60 s of age at all.

No production caller -- the feed ships pos_sigma_m at the solve epoch and
the frontend does the growing.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Every test here fails on origin/main and passes on this branch.

test_track_filter.TestIndefiniteCovariance:
- the indefinite cov is rejected and R falls back to the base floor, while
  a strongly-correlated-but-valid cov (corr 0.9, det > 0) still gets
  through — the check must reject only the impossible matrices, or it
  silently discards the relative weighting _KF_R_INFLATE exists for
- 200 solves carrying an indefinite cov at both a 1.4 s and a 20 s
  cadence keep P PSD and symmetric and learned_velocity answering.  On
  origin/main the 20 s case raises at solve 16 with the droplet's exact
  traceback, track_filter.py:278 ValueError: math domain error
- a 1 m and a 1e-6 m formal sigma both compose to the 1200 m floor, so a
  metre-scale R never reaches the update
- 200 updates at an effectively-zero cov with alternating positions stay
  PSD
- a hand-poisoned negative P[1,1]/P[3,3] returns sigma 0.0 rather than
  raising

test_feed_multinode.TestMultinodeEntryFailureIsolation:
- three multinode keys, the middle one raising ValueError("math domain
  error") from learned_velocity: the other two are still served, only the
  sick one is missing, and one log line names it.  On origin/main the
  exception escapes build_combined_aircraft_json and the whole tick is
  lost, which is what "Aircraft flush failed" was
- 20 consecutive failures produce one log line, not 20

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The anchor branch's distance check compares a solve against where the entry
was last STORED.  For a follow input that premise is false by construction —
its guess IS the prediction of where the anchor drifted to — and the numbers
bite: the dark displacement cap and the flat anchor gate are both 6.0 km, so
a solve at the edge of the gate that admitted it is at the edge of the gate
that must key it before any drift is added, and at the lane's 20 s staleness
limit a 270 m/s target adds 5.4 km more.  anchor_dr, set only by a follow
input, dead-reckons the anchor and applies the proximity scan's own
age-scaled gate.  Every other anchored input is unchanged.

Also documents the lane in docs/solverflow.md 3d and backend/.env.example.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
lane_split and the counters block are asserted by exact equality, which is
what makes them a contract; the new lane's five counters and its lane_split
bucket move those pins.  Adds the case they exist for: a follow record is
keyed mn-dark-* by design, so only the lane stamp keeps it out of a
bottom-up funnel it is not one of.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
aircraft_feed's feed build snapshotted state.multinode_tracks and popped
expired entries without _MN_TRACKS_LOCK while the solver worker iterates
the same dict inside multinode_key_decision under that lock.  Live on the
test droplet (2026-09-05) this raised "dictionary changed size during
iteration" and lost the solve; it surfaced once the dark expiry fell to
30 s and the follow lane raised the publish rate, but the race predates
both.

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

This comment has been minimized.

jehanazad and others added 7 commits September 5, 2026 18:48
Following an established dark track fixed the lane's own continuity, but
the bottom-up lane kept solving the same sky and keying by proximity onto
whatever entry was nearest.  Measured on test with the lane binding (20
min, 625 six-plus-node dark samples): 224 follow solves published, all
anchor-keyed, position error median 0.24 km / p90 1.35 km, and 6+-node
dark aircraft on the map 89% of the time against 76% before.  The
residual misses are all the other lane — of 425 bottom-up solves keyed by
proximity onto an existing key, 90 landed on a key owned by a DIFFERENT
aircraft (21%), 12 of them onto a key the follow lane had published on
within the previous 6 s.  A cross-keyed solve moves the entry 5+ km,
corrupts the KF velocity it feeds, and can supersede the right key.

A tighter spatial gate cannot separate the two cases: same-aircraft
re-key distances are p50 1.5 km / p90 4.3 km, mostly n=2 bottom-up solves
whose own error is ~2.4 km median, which overlaps the wrong-aircraft
population entirely.  What can separate them is that the follow lane
already supplies every solve an established track needs, so a bottom-up
solve arriving at a freshly-followed key is either a duplicate of that
aircraft — competing with the anchored solve and dragging the filter — or
a different one stealing the key.  Neither should join.

So in binding mode a key the lane published on within DARK_FOLLOW_OWN_S
(6 s, three follow-solve intervals) leaves the proximity scan's candidate
set, and a solve landing within DARK_FOLLOW_SHADOW_KM (2.0 km) of the
nearest such key is refused outright rather than keyed: outcome
shadowed_by_follow, counter dark_bottomup_shadowed, no publish, no KF
update, no entry written.  Farther out the solve falls through to the
non-followed candidates and mints as before.  The anchor and ADS-B
branches are untouched, which is what keeps the lane's own solves — they
are anchored — from shadowing themselves off the map.

The ghost guard is deliberately not fed from these records.  A shadowed
record names a followed key but was produced by the bottom-up lane, and
two of them in a row would otherwise drop the very track that refused
them, so the guard now reads the solver INPUT's follow_key rather than
the record's.

Inert unless DARK_FOLLOW_MODE is binding; the default is still shadow.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…r-observability

docs/solverflow.md: keep this branch's symbol-named 'Defined in' column and
add #292's SOLVER_ALT_MODE / SOLVER_FREE_ALT_STARTS rows and section in it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…is merged there

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ix/cluster-contamination

test_analytics_routes.py: keep both new status tests (live counters from this
branch, world-skipped pairs from #291). libs/retina-analytics: point at
analytics main (#25 world gate + #26 cluster split + #27 epochs merged).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…urement-epochs

state.py / routes/test.py / solver.py / test_solver_stats.py: adjacent
additions from both sides kept (epoch-align flag + counters next to the
free-altitude flags, resolve_skips_dark and altitude_mode stamps).
libs/retina-analytics: analytics main.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ts-and-dr-budget

.env.example: both new comment blocks, the shared commented knobs once.
docs/solverflow.md: symbol-named constants table from the stack plus this
branch's MN_DR_CAP_S / MN_DARK_EXPIRY_S rows; section 7 kept.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…covariance-psd

aircraft_feed.py: #297's MN_DR_CAP_S cap and KF learned-velocity dead
reckoning live inside #298's per-entry _multinode_entry helper, as resolved
on deploy/dark-solver-stack (cedbe69), which ran this exact PR set.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ck-following

state.py / test_solver_stats.py: adjacent additions from both sides kept
(DARK_FOLLOW_MODE beside the epoch-align and altitude-mode flags; follow
counters beside the stale/epoch/skip counters).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Records the follow lane writes carry lane="dark_follow"; the history
endpoint's per-lane cap and lane_counts index by that name, so without it a
single follow record turned /api/test/mlat-history into a KeyError (seen live
on the test droplet the first time the lane ran in binding mode).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
jehanazad pushed a commit that referenced this pull request Sep 6, 2026
…supersession-altitude-gate

solver.py / state.py taken from deploy/dark-solver-stack b0a289f, which merged
this exact PR set: the altitude-gated supersession block re-indented inside
#299's restructured publish path, and mn_superseded_blocked_alt reset beside
the other counters.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@jehanazad
jehanazad marked this pull request as ready for review September 6, 2026 02:39
@claude

claude Bot commented Sep 6, 2026

Copy link
Copy Markdown

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


Code Review — PR #300

  • Gather context (PR diff, description, prior review comment)
  • Understand the request (invoke /code-review skill on this PR)
  • Run the code-review skill against the current diff
  • Post findings

Summary

The core fix (taking _MN_TRACKS_LOCK for the snapshot and each pop in build_combined_aircraft_json) is correct and matches the diagnosis: the solver's unwrapped for key, prev in tracks.items(): in multinode_key_decision (solver.py) races an unlocked .pop() from the feed thread, and this PR closes that gap. The prior automated review already confirmed the lock ordering and lazy-import rationale are sound. On top of that, I found the following:

  1. backend/services/aircraft_feed.py:479-488 — stale-eviction pop doesn't re-check freshness under the lock.
    The staleness snapshot is taken and released at aircraft_feed.py:425-426, then the loop (which can touch many entries — history appends, _multinode_entry calls, etc.) runs without holding the lock. If the solver refreshes the same key with a new timestamp while that loop is still running, the eviction loop at line 487-488 does state.multinode_tracks.pop(k, None) against the current dict value, not the stale snapshot value — popping a solve the solver just confirmed. This won't crash (the original bug is fixed), but it can silently drop a just-refreshed aircraft for a tick. Low severity given the eviction loop is fast and the entry, being genuinely fresh, will very shortly reappear once multinode_to_aircraft republishes — but it's a narrower version of the same TOCTOU class this PR targets. Consider re-checking state.multinode_tracks.get(k)'s timestamp is still stale immediately before the pop, still under the lock.

  2. No regression test pins the lock usage in aircraft_feed.py.
    test_solver_stats.py:775-791 (test_multinode_tracks_iterated_under_the_solver_lock) monkeypatches _MN_TRACKS_LOCK and asserts it's held during the _solver_window_stats iteration. No equivalent test exists for build_combined_aircraft_json's snapshot (line 425-426) or per-key pop (line 487-488). A future refactor that accidentally drops the lock here — the exact live incident this PR fixes — would pass the full suite, including the three tests named in the PR description, since none of them assert lock usage directly.

  3. The with solver._MN_TRACKS_LOCK: list(state.multinode_tracks.items()) + lazy-import idiom is now duplicated three times (aircraft_feed.py:423-426, routes/test.py:1118-1127, known_lane.py:367-368), each independently working around the same task-module import cycle. Not a bug, but a future change to the locking scheme (renaming the lock, switching to an RLock, etc.) requires updating all three by hand — missing one silently reintroduces this same class of race. A shared solver.snapshot_multinode_tracks() helper would remove the duplication.

None of these block merging; #1 is worth a look since it's a subtler recurrence of the exact bug class this PR fixes, and #2 would make that regression visible in CI going forward.

@jehanazad
jehanazad merged commit cbe5a8b into main Sep 6, 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.

2 participants