Show the debrief its frame tallies, and name them so they cannot be misread - #690
Merged
Conversation
RunDebriefPayload never carried the terminal event's capture snapshot, so a debrief could not reach DataSuspect on the "missing frames" ground the choice set has always listed. The handler already reads that event; the tallies were simply never passed on. They travel with `reading_age_seconds_before_terminal`, and that pairing is the point rather than a nicety. CaptureProgressSnapshot deliberately exposes no `all_counts_matched`, because these are the last counts that REACHED the record before the terminal: a count short of its total may mean frames were lost, or may only mean the last reading arrived early. Its docstring names the comparison a reader should make instead, saved_at against the event's observed_at, and refuses to make it in the domain. 2-BM's record shows why that refusal is load-bearing. Across 609 completions, a 14.5-hour window (2026-08-18 21:55 to 2026-08-19 12:29) carries a consistent 11-frame shortfall, and the Runs showing it have a mean reading lag of 70 s against 12 s for those that do not. The shortfall tracks the staleness, which reads as telemetry going quiet rather than frames going missing. An earlier draft of this change told the model a shortfall "means frames were lost and is sufficient on its own"; on that corpus it would have reported data loss across 255 Runs from what looks like a reporting gap. So the guidance says the opposite. A shortfall with a large reading age is a reporting gap, stays NominalCompletion, and is noted as unconfirmed. A shortfall with a small reading age is worth DataSuspect, still phrased as a shortfall in what was reported rather than as a count of lost frames. Whether files on disk are complete is not in this payload and cannot be concluded from it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The guidance added in the previous commit told the model that a shortfall paired with a large reading age was a reporting gap, and to keep NominalCompletion while noting it as unconfirmed. That was my inference, not a measured fact, and counting frames in the files refuted it. The affected scans at 2-BM hold 1530 frames against 1541: 1500 projections, 10 flat fields, 20 darks, versus 1501/20/20 on either side of the window. Ten of twenty flats were never collected. CORA's saved_count was correct all along; the correlation I built on (reading lag 70 s against 12 s) most likely runs the other way, since a scan that skips half its flat collection finishes that phase earlier and leaves a longer gap before the terminal. Acting on the wrong version, the agent produced "more likely a reporting lapse than data loss" about real scans that had genuinely lost half their flat fields, in an operator's own vocabulary, into the record. It reasoned fluently from a false premise it had been handed. A better model would have done the same. The guidance now says to report the shortfall and select DataSuspect, never to discount it via reading age, and equally never to assert the files are damaged: a debrief reads counters, and only the files settle what the files contain. reading_age stays in the payload as context for a reader rather than as an acquittal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The snapshot holds two independently sourced pairs, FPNumCapture and CamNumImages, whose totals are not the same quantity. Calling the first total `frames_expected` while the second was `frames_collected_expected` made the first read like a grand total the collected count fell short of. A model duly compared 1541 saved against 10 collected, called it a "substantial data shortfall", and returned DataSuspect on ten of ten scans that were complete on both counters. Renaming to `frames_saved` / `frames_saved_expected` puts the pairing in the names, where the reader cannot miss it, rather than in prose the reader may not apply. The guidance also now states the rule outright and adds the one the value object already documents: `frames_collected` slightly under its own total is the normal ending of a healthy scan, about one poll interval's worth, and is not evidence of anything. Third iteration on this prompt, and the failures have been instructive in the same direction each time. Blind, then explaining a real defect away, then flagging healthy scans. Only the first was a missing capability; the other two were the agent faithfully applying a framing it was handed. What each cost was measured against the beamline's own files rather than argued. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
The diff gate caught `_reading_lag_seconds` returning None untested on both paths: timestamps that are not strings, and strings that will not parse. Neither is reachable through `CaptureProgressSnapshot`, which is typed, so the tests build the payload by hand. That is the case the branches exist for, an event written before the snapshot carried timestamps or a row a migration touched. Reading the file to place them turned up something worse. The docstring still argued the inference the prompt guidance retracted two commits ago: that a shortfall paired with a large reading lag reads as telemetry going quiet rather than frames being lost. Counting frames in the files refuted that, and the correlation most likely runs the other way, since a scan that skips half its flat collection finishes that phase early and leaves a longer gap before the terminal. A retraction that lands in the prompt and not in the docstring next to it is half a retraction, and the docstring is what the next reader will believe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The 2-BM pilot's own scan files settled a question the debrief could not
answer: a beamline scientist reported missing frames, and CORA's record
could pin the onset to the minute while the debriefing agent stayed
blind, because the payload never carried the counts.
Three commits, and the middle one is a correction of the first.
capture_progressreaches the RunDebrief payload, carrying bothindependently sourced pairs and how stale the reading was.
The first guidance told the model that a shortfall paired with a
large reading age was a reporting gap. That was an inference, not a
measurement, and counting frames in the files refuted it: the
affected scans hold 1530 against 1541, having collected 10 flat
fields instead of 20. Acting on it, the agent wrote "more likely a
reporting lapse than data loss" about scans that had genuinely lost
half their flats. The guidance now says to report the shortfall and
pick DataSuspect, and equally not to claim the files are damaged: a
debrief reads counters, and only the files settle what the files
contain.
The counters are renamed into symmetric pairs.
frames_expectedsitting beside
frames_collected_expectedread as a grand total, sothe model compared 1541 saved against 10 collected and reported a
substantial shortfall on a complete scan. The pairing now lives in
the names rather than in prose the model has to remember to apply.
Measured against ground truth established by counting frames on disk,
over 20 Runs from the pilot record: 10/10 complete scans NominalCompletion,
10/10 short scans DataSuspect.
Gates: 930 agent unit tests, 31173 architecture tests, green.
🤖 Generated with Claude Code