Skip to content

20260910 - Own the record, and serve it (re-target of #28) - #30

Merged
Purple10101 merged 8 commits into
mainfrom
20260909-couple-delay-doppler-state
Sep 10, 2026
Merged

Purple10101 merged 8 commits into
mainfrom
20260909-couple-delay-doppler-state

Conversation

@Purple10101

Copy link
Copy Markdown
Contributor

#28 was approved and merged, but it landed on 20260909-couple-delay-doppler-state rather than on main.

It was a stacked PR based on #27. #27 merged to main at 12:53:51 and #28 merged twenty seconds later, before GitHub had re-targeted it, so it merged into its original base branch. gh reports it MERGED, which is true, just not into main.

The result is that main carries #27 alone. retina_tracker/control.py and retina_tracker/history.py do not exist on it, along with the ingest rewrite, the detection classification work and five test files.

That is load-bearing right now: retina-gui and retina-node have both merged and both depend on the control surface this adds. retina-node passes --control-port to the tracker container, and main's CLI does not accept it, so argparse would exit 2 and the container would crash-loop.

What is in here

Nothing new. This is #28's content, unchanged, carried on the branch it was merged into:

  • control.py, the loopback HTTP surface: POST /reset, POST /history/clear, GET /health, and GET /events as an SSE stream with chunked framing
  • history.py, four hours of detections held in array.array columns across three classes, with snapshot and delta reads
  • the ingest rewrite, so the newest detection feed takes the socket
  • per-detection classification, so the record says what became of every detection rather than only the tracked ones
  • the events file carrying only what changed, rather than repeating each track's whole window

Verification

This merge produces no conflicts, and the resulting tree is byte-identical to the branch tree already reviewed on #28. There is no new code to look at.

Both gates pass on the branch head. claude-review will fail until CLAUDE_CODE_OAUTH_TOKEN is rotated: every review run across the org has failed since the token went stale yesterday evening, and it fails before any model call, so it is not reading this diff.

🤖 Generated with Claude Code

Purple10101 and others added 8 commits September 9, 2026 18:47
The one control operation the tracker supports, clearing state between search
geometries, has only ever been reachable as a {"type": "RESET"} message mixed
into the detection socket. That works while one process sends both the
detections and the controls, and that is exactly the arrangement being unwound.
blah2_api is taking over the detection socket so retina-gui stops being the
transport between blah2 and here; run_tcp_server accepts one connection at a
time, so the auto-calibration search cannot simply open its own alongside it.

So control gets its own door: POST /reset on a loopback HTTP surface, plus GET
/health, which reports frames seen and tracks held and is the one question
worth asking a node while the feeder is being switched over. Nothing here knows
who is calling. The auto-calibration search and the Tracker page are equal
consumers of a tracker that does not distinguish between them.

Loopback by default, for the same reason the ingest socket is: the sidecar runs
with network_mode host, where 0.0.0.0 would publish this on the LAN. Stdlib
only. This is a handful of routes with one or two clients, and a framework
would be a dependency and an image layer for nothing.

A reset from a request thread can now land in the middle of a frame, so both
paths take one lock. It is held for the duration of the reset, so a 200 means
the tracker is already clear rather than scheduled to be: a caller resetting
between candidate geometries waits on the next frame, and that frame must not
be able to associate into pre-reset state. Deferring to a flag the frame loop
reads would have made the response a promise, and would never have applied at
all on a node whose detections had stopped.

The socket RESET keeps working. It goes when retina-gui stops feeding this
socket, not before.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The ingest loop accepted one connection and then read it to EOF, with a
backlog of one behind it. A peer that vanishes without closing — a killed
container, a dropped link — leaves a half-open socket that never returns
from recv and never reaches EOF, so the loop sat there for as long as the
kernel took to notice while the next peer waited unserved.

That is not a hypothetical here. The handover from retina-gui to blah2_api
is exactly a new peer arriving while the old one may still be holding the
slot, and getting it wrong means a tracker that silently receives nothing.

So the loop selects on the listener as well as the connection, and the
newest connection wins: accepting closes the one it replaces and drops
whatever was half-read, since a partial line belongs to the peer that was
sending it. Replacing rather than multiplexing is deliberate. The tracker
does not deduplicate frames, so two live feeders would hand it every frame
twice at dt=0 and give every track twice the evidence it earned.

Two smaller things the rewrite fixes on the way past. The buffer holds bytes
and decodes per line, so a multi-byte character split across a recv boundary
no longer raises out of the loop. And one malformed frame can no longer end
detection ingest until the container is restarted: a line that is not JSON,
not an object, or missing what process_frame needs is logged and dropped.
Both were reachable from anything with access to the port.

serve_detections is split out from run_tcp_server and takes a stop_event, so
the tests drive it over real sockets on an ephemeral port. Nothing here is
observable against a stub: every case is about what the socket layer does.
Reverting to the old "stop watching the listener once connected" behaviour
fails three of them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every track event carried the track's whole rolling window, up to
detection_window points, to communicate the one point that was new. On a
300-frame run with three targets that is 1.38 MB where 277 KB says the same
thing: 5x, measured by running the same input through the tracker twice with
the filtering off and on.

Not the 20x the window size suggests, because at one detection per event the
per-event metadata is most of the line. If this file needs to shrink further,
that is where the bytes are now, not in the detections.

Safe because neither consumer reads an event as the track's state now.
live_score.load_tracks unions detections by timestamp across every event
mentioning a track, and retina-gui's buffer appends only timestamps it has
not seen; both reconstruct the same history from a delta stream as from a
repeating one, which the tests assert directly by loading both and comparing.

A high-water mark per track is enough. A track's history only grows forwards
for as long as it can emit: merging is the one thing that splices older
points in, and it operates on all_tracks, the post-mortem archive, after the
track has been deleted from self.tracks and can no longer produce an event.
The map is an LRU bounded at 512 tracks, since ids are unique for the life of
a run and this process runs for weeks. Evicting one costs a repeated window,
never a dropped detection, which is the right way round given both consumers
dedupe.

The event is still written when nothing is new. length, the anomaly flags and
shadow_fraction all move over a track's life, and a consumer that missed
those updates would be holding a stale opinion of a live track.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The tracker consumed detections and reported only the ones that ended up
inside a confirmed track. Two exits were silent. Anything below MIN_SNR was
dropped at the gate before the tracker looked at it, and anything that
started a tentative track which never promoted vanished with it. So nothing
downstream could tell "the tracker rejected everything it saw" from "there
was nothing to see", which are the two explanations for an empty page and
want opposite responses.

Every detection now leaves through an optional sink, classified as
associated, unassociated, or below_snr. The gate becomes a partition rather
than a filter, in one loop rather than two comprehensions so a NaN snr — which
fails both comparisons — still lands in exactly one bucket instead of
disappearing from the accounting.

The hard part is when a verdict is final. "Associated" means the detection
ended up in a track that was confirmed, and that is not knowable when the
detection arrives: one that starts a tentative track is unassociated at that
instant and becomes associated a few frames later if the track promotes.
Answering on arrival would be wrong for exactly the detections that matter
most. So a frame waits until each of its tracks has settled, promoted or
deleted, and is released with a final answer. A detection joining an
already-confirmed track settles immediately, so a steady feed is not delayed;
on a mixed 200-frame scene the queue sat 6 frames behind, which is N_WINDOW.

Frames are released only from the front. Out-of-order release would break the
one assumption a consumer buffering these wants to make, and it is why a
below-gate detection waits for its frame despite its own verdict being
obvious. One call per frame, in order, is worth a few frames of lag.

ever_confirmed latches in the state_status setter rather than at the
promotion site. Three routes reach ACTIVE — M-of-N promotion, a coasting
track re-associating, and tracklet initiation — and latching at one of them
was silently wrong for the other two: every track in a steady feed read as
never confirmed. Current status cannot stand in for the flag either, since a
track that was ACTIVE and has since been deleted reads the same as one that
never got there.

No sink means no bookkeeping, so the CLI and any node without a consumer pay
nothing. With one attached the cost was 3.8% on 200 frames.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The tracker's own Track objects keep a bounded per-track ring and drop
completed tracks after a short merge window, so nothing in it can answer what
was seen three hours ago. This is that record: every detection the tracker was
given, classified, plus the points and metadata of every track.

In memory, not on disk. It does not need to survive a restart, and the
alternative was several hundred megabytes a day onto SD cards the fleet cannot
afford to wear out.

Stored as array.array columns. A CPython tuple of four floats costs about 192
bytes once the list slot, the tuple header and four boxed floats are counted;
the same point here is 20, so a buffer that would have been 83 MB is 8.7.
Columns are also what goes on the wire, so serving a snapshot is a slice rather
than a transposition, and one point-set per class means it is a slice rather
than a filter over everything held.

Bounded twice. The window is the intent, but a window alone makes the footprint
a function of how busy the sky is, which is not something a node can promise.
max_points is a hard ceiling underneath it, defaulting to about 10 MB per
class, and whichever binds first wins. Tracks are capped too, evicting whichever
has the stalest last point so a live one is reached for last.

Values are rounded on the way out, to the precision the measurement has. That
is not only about wire size: 16.1 held as a float32 reads back as
16.100000381469727, which would be eighteen bytes for four bytes of meaning.

GET /events serves it: a snapshot on connect, then only what has been appended.
The cursor lives in the connection's own thread and nowhere else, so there is
no per-consumer state on the server to expire and nothing to negotiate — a
reconnect takes a fresh snapshot, which is always a valid place to start.
Sending the snapshot down the same stream is what removes the race between what
it contained and where the deltas began. Because pruning drops from the front,
a position is a monotonic count rather than an index, and clear() bumps a
generation that voids every outstanding cursor at once.

/health now reports the footprint, which is the question worth being able to
ask of a node whose memory matters.

Verified end to end against a running tracker: three detections a frame, one of
each class, arriving over a real SSE connection as a snapshot followed by
deltas, with the track's own points alongside.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…cord be cleared

Two additions the page needs.

POST /history/clear wipes the record without touching the tracker. That is
the distinction "Clear buffer" has always carried — clear what I am being
shown, keep tracking — and it survives the record moving here from
retina-gui, which no longer has a buffer to clear.

And the stream is chunked. It was sending "Connection: close" with no length,
which is legal and which curl and EventSource both handle, so it looked fine.
urllib3 does not: with no framing it reads to EOF to find a message boundary,
and on a stream that never ends that means blocking until the read timeout
instead of delivering each event as it arrives. retina-gui proxies this with
requests, so every message would have arrived only when the connection
finally gave up. Framing each message as its own HTTP/1.1 chunk makes it
stream to any client rather than only the forgiving ones.

Found by proxying it for real rather than by reading the spec, which is the
only way this one shows up: every direct client I had tried worked.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…work hooks

Two CI gates, neither of them a code problem.

ruff-format reflows the lines this branch added: joined argument lists that
fit, an implicit string concatenation collapsed to one literal, trailing
commas, and a space after a docstring's opening quotes. Token-for-token
identical otherwise.

The dead-code gate reported seven names in control.py. Six are called by
http.server and socketserver rather than by us: do_GET and do_POST are looked
up by name from the request line, log_message is an override the base class
calls, and protocol_version, daemon_threads and allow_reuse_address are class
attributes the servers read. Those go in the whitelist's contracts section,
which had been empty, each with the reason it cannot be deleted.

The seventh was real. log_message's fmt parameter went unused because the
override discards everything; the base class calls it positionally, so *args
covers the format string and the parameter can go.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Sep 10, 2026

Copy link
Copy Markdown

Claude finished @Purple10101's task in 4m 12s —— View job


Claude finished the review

Todo list:

  • Gather context (PR body, changed files, git diff against origin/main)
  • Confirm this PR's tree is really just 20260910 - Own the record, and serve it #28's content, re-targeted
  • Review control.py — HTTP control surface (reset, health, history/clear, SSE /events)
  • Review history.py — bounded in-memory detection/track record
  • Review tracker.py / output.py / track.py — classification pipeline, ever_confirmed/retired, dedup
  • Review server.py — newest-connection-wins ingest rewrite
  • Review cli.py / vulture_whitelist.py
  • Review new test files for coverage and correctness
  • Post findings

Summary

Confirmed the branch story: origin/main (2298f78, #27 only) and this PR's tip (ee9362f) share full history up to 17a64f4; the diff against origin/main is exactly the 13 files described (2384/-40), matching #28's merge. No conflicts, nothing new to review that wasn't already reviewed on #28 — I read it fresh anyway rather than taking that on faith.

Findings

I did not find any correctness, security, or style bugs. Specifically checked and confirmed correct:

  • Units consistency: history.py treats all timestamps as milliseconds (prune's now_ms - window_s * 1000 cutoff), which matches tracker.process_frame's documented ms timestamps and the test fixtures (BASE = 1789030000000). No unit mismatch.
  • Track.state_status property (track.py:56-87): the ever_confirmed latch fires on every path into ACTIVE (M-of-N promotion, coasting re-association, tracklet initiation), and all existing read/write sites in tracker.py and track.py go through the property transparently — no direct _state_status access anywhere.
  • Classification vs. shadow-rejection interaction: a shadowed track can accumulate n_associated >= M_THRESHOLD() without ever reaching ACTIVE (track.py:816-822, promote_if_ready's not self.is_shadowed() guard) and still land in all_tracks via the pre-existing tracker.py:263 condition. Its detections correctly stream as unassociated since ever_confirmed never latches — that's the shadow-rejection working as designed (see commit 32bf051), not a new inconsistency from this PR.
  • Control surface: loopback-only default (127.0.0.1), proper 404/503 handling, lock-guarded /reset (verified against test_reset_waits_for_an_in_flight_frame), SSE cursor/clear()/generation handling all check out against history.py's _gen bump.
  • Ingest rewrite: newest-connection-wins logic in serve_detections correctly discards the replaced peer's straddling buffer; malformed/partial frames can't take the feed down (_handle_line's narrow excepts plus the outer except Exception around process_streaming_frame).
  • output.py dedup: _new_detections's high-water-mark + bounded LRU (EMITTED_MEMORY) is exercised well by test_event_deltas.py, including the eviction-repeats-rather-than-drops case.

Test coverage across test_classification.py, test_control.py, test_event_deltas.py, test_history.py, and test_ingest.py is thorough — real sockets/HTTP rather than stubs for the parts where that matters (connection handover, SSE framing), and the classification tests specifically pin the subtle "verdict isn't knowable on arrival" behavior.

Not independently verified

I could not execute ruff check, ruff format --check, or pytest myself — this session's sandbox blocks all python/subprocess execution (even python -c "1+1" requires approval that isn't available non-interactively). The PR body states both gates pass on the branch head; I have no reason to doubt that given the diff is unchanged from the already-reviewed #28, but flagging per CLAUDE.md's mandatory pre-commit verification so a human/CI run confirms it before merge.

No blocking issues found. This looks safe to merge as a re-target of already-approved work.

@Purple10101
Purple10101 merged commit f814e8a into main Sep 10, 2026
2 checks passed
@Purple10101
Purple10101 deleted the 20260909-couple-delay-doppler-state branch September 10, 2026 14:24
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