20260914 - Record what the filter predicted, so R and Q can be separated

[Purple10101]

R and Q are both wrong, in opposite directions, and they cancel. Measured this morning against recorded detections on jn1 (n=402): R is about 45x too large in delay and 6.3x in Doppler, while the model error it is standing in for grows as L^1.96 rather than the L^5 that white jerk implies. No single jerk value fits, so the adaptive scale cannot converge. The sum comes out plausible, which is why nothing has complained.

Neither constant can be fitted from the events file. That records the detections which were associated, never what the filter predicted before it saw them, and the difference between those is the only quantity either one answers to. NIS alone will not do either: a single scalar moves one way for an oversized R and the other for an undersized Q, which is exactly how they have been hiding from it.

Reconstructing innovations offline by replaying recorded detections works, and is how the figures above were obtained, but it cannot see the filter's real state. A replay has to guess at the covariance, the adaptive scale and the coasting history that shaped each prediction. This records them instead.

What is in here

• Residual, a NamedTuple of (innovation, S, nis). kalman.update() computed all three and returned only the NIS, discarding the two that separate R from Q. Residual.degenerate() keeps the singular-S path honest rather than letting it pass NaNs off as a measurement.
• InnovationWriter, subclassing TrackEventWriter for its rotation, writing one JSONL record per update: track_id, birth, timestamp, dt, snr, delay, doppler, n_missed, q_scale, innovation, s_diag, nis.
• --innovations PATH on the CLI, threaded through both the TCP server and the file path. Absent, nothing is built and nothing is written.
• tests/test_innovations.py, new: record shape, the degenerate path, rotation, and that the writer stays off by default.

Each record carries its own measurement deliberately. Most tracks at an interfered site are built on a fixed-Doppler tone, 70% on jn1 and 76% on fairforest B, and have to be excluded before anything is fitted to them. A track that never confirmed has no id and never reaches the events file, so the Doppler has to travel with the record rather than be joined back to it.

Cost when off

innovation_writer defaults to None and the call site is guarded, so an unflagged node builds no record rather than building one and dropping it. This is the same shape as detection_sink.

Cost when on

Bounded by OUTPUT_MAX_BYTES / OUTPUT_BACKUP_COUNT, the same rotation as the events file, so a node left recording cannot fill its disk.

Why now

Phase 0 of today's tracker plan. This afternoon's data window is a simultaneous daylight capture on both nodes, and it is expensive to repeat. Landing this beforehand is what makes Phase 2's calibration direct and repeatable rather than an offline reconstruction of state the tracker already knew.

Verification

ruff check (E, F, W) clean, ruff format --check clean, 271 passed / 1 xfailed.

Merges cleanly with #31, the other Phase 0 branch.

🤖 Generated with Claude Code

[claude]

Claude finished @Purple10101's task in 2m 23s —— View job

---

Claude finished reviewing PR #32

Task list

• Gather context (PR body, changed files list)
• Read the diff for kalman.py, track.py, tracker.py, output.py, cli.py, server.py, tests
• Post review findings

---

Summary

The core plumbing (Residual NamedTuple, InnovationWriter, the --innovations CLI flag, rotation reuse, and the off-by-default guard) is sound and well-tested. One correctness bug undermines a specific goal stated in the PR description, though.

Bug: n_missed is always recorded as 0

retina_tracker/track.py:726-760, called from retina_tracker/tracker.py:174-179:

def update(self, detection, timestamp, frame=0):
    ...
    self.last_residual = residual
    self.last_q_scale = q_scale
    ...
    self.n_missed = 0          # line 760 — reset happens inside update()
    ...

track.update(det, timestamp, frame=self.frame_count)
if self.innovation_writer:
    self.innovation_writer.write_residual(track.id, timestamp, track, det)   # reads track.n_missed *after* it was reset

write_residual (output.py) reads track.n_missed to populate the record, but Track.update() already zeroes self.n_missed at line 760, before control returns to tracker.py and write_residual is called. So every recorded n_missed will be 0, regardless of how many frames a track was coasting before this update landed.

This directly contradicts the PR's stated rationale: "a replay has to guess at ... the coasting history that shaped each prediction. This records them instead." Right now it doesn't — the coasting count is lost before it's written. Notably, q_scale avoids exactly this trap by being captured into a local variable (q_scale = self.process_noise_scale(), line 728) before it could go stale; n_missed needs the same treatment, e.g. capturing it into self.last_n_missed before the reset at line 760 (mirroring last_q_scale), or by reading track.n_missed in tracker.py before calling track.update().

None of the new tests (tests/test_innovations.py) catch this because they never exercise a track that coasts (misses a detection) before being re-associated — I confirmed the bug by tracing the control flow rather than by a failing test, since the sandbox here couldn't execute Python. Recommend adding a test that coasts a track for a frame or two and then re-associates it, asserting n_missed > 0 on that record.

Fix this →

Everything else looks solid

• Residual.degenerate() correctly preserves the old nis = float(MEASUREMENT_DIM) sentinel used to decay nis_ema, while now also surfacing NaN innovation/S so a caller can't mistake it for a real measurement.
• q_scale capture timing is correct — read from self.nis_ema before update() mutates it, so it matches what was actually used in the preceding predict() call (verified against test_q_scale_is_the_one_in_force_for_that_prediction).
• All existing kf.update() call sites (track.py, both test files) were updated consistently to the 3-tuple (state, covariance, Residual) return shape — no stale 3-tuple unpacking left assuming nis was the third element.
• InnovationWriter correctly reuses TrackEventWriter's rotation via the extracted _write_line helper, and --innovations is threaded through both the CLI file path and the TCP server path, defaulting to None with a guarded call site — matches the detection_sink pattern as claimed.
• Doppler is recorded on the record itself (not joined from the events file), which is needed since unconfirmed tracks (track_id is None, fixed-Doppler-tone clutter) never reach the events file.

Note: I wasn't able to execute ruff/pytest in this sandboxed environment (Bash execution required approval that wasn't available), so the "271 passed / 1 xfailed" claim in the PR description is taken on faith; the above finding was confirmed by static control-flow tracing, not a failing test run.

[claude]

Claude finished @Purple10101's task in 1m 15s —— View job

---

Review of PR #32

Tasks

• Gather context (PR body, changed files, prior review comment)
• Re-review the full diff (origin/main...HEAD)
• Verify the previously-flagged n_missed bug against the new commit
• Post final review feedback

---

Previously flagged bug is fixed

My earlier pass on this PR flagged that Track.update() zeroed self.n_missed before write_residual read it, so every recorded n_missed came out 0. Commit 3121ef0 ("Latch how long a track coasted, before the update forgets it") fixes this correctly:

• retina_tracker/track.py:120 adds self.last_n_missed = 0 to __init__.
• retina_tracker/track.py:735 captures self.last_n_missed = self.n_missed inside update(), right next to the existing self.last_q_scale = q_scale latch, and before n_missed gets reset later in the method — same pattern, same reasoning.
• retina_tracker/output.py:166 now reads track.last_n_missed instead of track.n_missed.
• tests/test_innovations.py gained TestTheCoastingHistoryIsNotLost with two tests that directly exercise this: one coasts a track for 2 frames before re-association and asserts n_missed == 2 on the record, the other asserts an uninterrupted update still records 0. This is exactly the regression test my earlier review was missing.

Nice, minimal fix — no unrelated churn.

Rest of the PR (re-checked against current HEAD)

• Residual NamedTuple (kalman.py) and its degenerate() classmethod are correct: the singular-S path still returns nis = float(MEASUREMENT_DIM) (preserving the old nis_ema decay behavior) while now surfacing NaN innovation/S instead of silently omitting them.
• q_scale and now n_missed/last_dt are all captured before they'd go stale, consistent with each other — predict() sets self.last_dt = dt once per frame (track.py:717), before update() is called, so last_dt correctly reflects the interval the innovation was actually measured against.
• The single innovation_writer.write_residual(...) call site (tracker.py:178) only fires on the association/update path, never on track birth — matches the PR's claim that unconfirmed tracks never produce a record, and confirms residual is None guard in write_residual is defensive rather than a masked bug.
• --innovations is threaded consistently through cli.py (both the direct-file and --tcp paths) and server.py, defaults to None, and the call site is guarded — same shape as detection_sink, as claimed.
• Rotation is correctly reused via the extracted _write_line helper on TrackEventWriter, and TestTheFileStaysBounded verifies the bound is honored.
• tests/test_adaptive_process_noise.py updates are mechanical unpacking changes (nis → residual.nis) with no behavior change.

I did not find any further issues. tests/test_innovations.py is thorough — it covers record shape, the degenerate/singular-S path, rotation bounds, the off-by-default guard, and (now) the coasting-history latch.

Note on verification: I could not execute ruff or pytest in this sandboxed environment (Bash command execution required approval that wasn't available here), so the PR's "271 passed / 1 xfailed" and clean-lint claims are taken on faith rather than independently re-run. The above was confirmed by reading the diff and tracing control flow, not by a test run.

[Purple10101]

Fixed in 3121ef0. last_n_missed is latched next to last_q_scale, before update() zeroes the counter, and write_residual reads that instead. Added a coasting test to tests/test_innovations.py: it asserts 2 on a record that followed two coasted frames, and it fails with assert 0 == 2 against the old code, so the column is now pinned rather than merely correct today.

273 passed / 1 xfailed, ruff and format clean.