Snapshot track history deques before off-thread iteration

[jehanazad]

Summary

retina-tracker#25 turned the five Track.history[*] buffers from lists into deque(maxlen=600). A deque raises RuntimeError: deque mutated during iteration if it is appended to while a Python-level loop walks it; a list tolerated that silently. Only the frame worker appends to a given node's tracker, but several readers run on other threads — and one of them was failing live:

task_error_counts {'aircraft_flush': 4}      # over 7 h
RuntimeError: deque mutated during iteration
  at backend/services/aircraft_feed.py:538

Each failure aborted the whole 1 s feed tick, so every websocket client lost that frame of aircraft, arcs and ground truth.

list(some_deque) is a single C call and therefore cannot be interleaved by another Python thread: it is an atomic snapshot under the GIL. That is already the idiom elsewhere on this path (reversed(list(dq)) in analytics_refresh.py). This PR applies it at the one server-side site and pins the library fix for the rest.

Changes

backend/services/aircraft_feed.py:535 — the single-node arc builder inside build_combined_aircraft_json now snapshots before the reverse scan:

meas = list(track.history.get("measurements") or ())

with a comment recording why. Nothing else about the arc path changes; the or () keeps the existing "no history, skip this track" behaviour for a track that has none.

libs/retina-tracker — bumped to the merged retina-tracker#26, which makes Track.get_recent_detections and Track.to_dict snapshot their buffers the same way.

Audit: readers of track.history / get_recent_detections / to_dict in backend/

Site	Thread	Status
services/aircraft_feed.py:535 (history["measurements"])	aircraft-flush executor	Fixed here — the live traceback
services/tasks/analytics_refresh.py:536 (get_recent_detections)	analytics executor	Fixed by the library bump
services/state_snapshot.py:43 + routes/custody.py:175 (to_dict)	snapshot / request threads	Custody Identity.to_dict, not Track.to_dict; no history deque involved. Track.to_dict is reachable only from the library's own surfaces, and is fixed by the library bump
pipeline/passive_radar.py:160, 572 (get_recent_detections)	frame worker (the mutator)	Left alone — cannot race itself
services/frame_processor.py:364 (get_recent_detections)	frame worker (process_one_frame)	Left alone — same
services/tasks/analytics_refresh.py:558 (reversed(list(dq)) on known_claims)	analytics executor	Already correct; this is the reference idiom

Test coverage

New backend/tests/test_feed_history_snapshot.py: a background thread appends to a real deque history (including the None entries the coast path writes) while the main thread drives build_combined_aircraft_json in a loop for ~0.6 s, with the arc-refresh timer cleared each pass so every build really walks the history. Asserts no exception and that the loop did meaningful work. ~0.7 s wall.

• Without the fix it reproduced the live traceback at aircraft_feed.py:538 on 3 of 5 runs.
• With the fix: 2992 backend tests, all passing (2991 before this PR; 1 skipped either way), no failures and no flakes on the full run.
• Library side: 109 tracker tests passing (108 before), and the new tracker stress test failed 5 of 5 runs without the library fix.
• pre-commit run --all-files clean in both repos (ruff, ruff-format, vulture, ruff-config).

Review notes

• Behaviour is unchanged for every caller; the only new cost is one list() copy of a ≤600-entry deque per node-track per arc refresh, which happens at most once per ARC_REFRESH_S, not once per flush.
• The library's zip(reversed(meas), reversed(ts)) is deliberately left as it is. The mutator appends timestamps before measurements, so a reader landing mid-append can see one extra timestamp and zip truncates to the shorter buffer — the same behaviour these buffers had as lists. Aligning them properly is a separate question and not a regression.
• The tracker PR (Snapshot track history deques before off-thread iteration retina-tracker#26) is already merged; this PR pins its merge commit 2d31f00.

🤖 Generated with Claude Code

[claude]

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

---

Claude's Review

Tasks

• Gather context (PR description, diff, changed files)
• Review backend/services/aircraft_feed.py change
• Review new test backend/tests/test_feed_history_snapshot.py
• Check libs/retina-tracker pin bump
• Verify audit table claims (other reader sites)
• Post final review

Summary

This is a small, well-scoped fix and the fix itself is correct. I verified the audit table's claims directly rather than taking them on faith, and every line reference and thread attribution checks out:

• backend/services/aircraft_feed.py:540 (meas = list(track.history.get("measurements") or ())) runs inside build_combined_aircraft_json, which is only ever invoked from _aircraft_flush_executor (backend/services/tasks/aircraft_flush.py:215-218, a dedicated single-thread ThreadPoolExecutor) — a different thread than whatever mutates the tracker's deque. list(deque) being an atomic snapshot under the GIL is accurate, and matches the existing idiom at analytics_refresh.py:558 (reversed(list(dq))).
• The or () correctly preserves the pre-existing "no history → skip" behavior for a track whose history["measurements"] key is missing (.get() returning None).
• analytics_refresh.py:536 (get_recent_detections) is a library-side call, fixed by the retina-tracker bump — consistent with the audit table.
• state_snapshot.py:43 and custody.py:175 both call .to_dict() on Identity objects from state.node_identities, not on Track — confirmed by reading both call sites. Correctly excluded from this fix.
• passive_radar.py:160,572 and frame_processor.py:364 call get_recent_detections from the frame-processing path itself (the mutator), so no cross-thread race — consistent with "left alone, cannot race itself."

Test

test_feed_history_snapshot.py is a solid regression test: it builds a real deque(maxlen=600), spins a writer thread appending (including None entries to exercise the coast-path skip in the reverse scan), and drives build_combined_aircraft_json in a loop with _arcs_last_ts reset each iteration so the arc section is actually re-walked every call rather than served from cache. Passing a SimpleNamespace(geolocated_tracks={}, config={}) as the fake pipeline is fine — those fields are only touched by the earlier (untested-here) sections of the builder, both of which correctly no-op on empty inputs.

Minor, non-blocking observations:

• The test description says it "failed 3 of 5 runs" without the fix — inherently a bit flaky by nature of being a race reproduction. That's expected and acceptable for this kind of regression test, and the 0.6s stress window with builds > 10 assertion is a reasonable tradeoff between reliability and CI runtime.
• The comment at aircraft_feed.py:535-539 is a bit long for the change it documents, but it's justified here — it records a non-obvious cross-thread hazard that isn't visible from the code alone, which is exactly the kind of thing worth a comment.

No correctness issues found. The library pin bump to 2d31f00 (retina-tracker#26, already merged) is the appropriate complement to the server-side fix, and the audit table's coverage of all other track.history/get_recent_detections/to_dict call sites in backend/ appears complete and accurate.