[Backend] Validate beats in code before any prose is written #333

Closed
opened 2026-08-25 20:38:52 +00:00 by claude-bot · 1 comment
Contributor

Severity: CRITICAL. Found in the August 2026 session lifecycle review (#319). This is the issue that makes accuracy independent of model size.

Why

The two reported failure modes — scrambled chronology and misattributed actions — are both currently trusted to the model. This makes them checked.

Crucially, the check is pure code and free. It runs the same way on a 9B local model and on a frontier hosted one. Given that self-hosted is a first-class target, this is the only design whose correctness floor does not ride on model size.

Proposed fix

After the map phase, before any prose is generated:

  1. Evidence exists — every evidence timestamp must correspond to a real transcript line.
  2. Actor is grounded — every named actor must be the resolved speaker (per the code-side legend) of at least one cited line. For an action described by the GM rather than performed on-mic, the beat is tagged narrated and the actor must appear in the text of a cited GM line.
  3. Time range is sanet_start <= t_end, both inside the session, and consistent with the cited evidence.
  4. Sort by time in code. Chronology becomes a sorted() guarantee rather than a model behaviour. This is the entire point.

Beats that fail validation are flagged, not silently dropped — surface them to the GM as "unverified" so a real event with a bad citation is recoverable rather than invisible. Silent dropping would trade one invisible failure for another.

Acceptance criteria

  • Validator is pure code with no model call
  • All four checks implemented, each independently unit-tested
  • Failing beats are flagged and surfaced, never silently discarded
  • Validated beats are ordered by sorted(), not by the model
  • Validation results are recorded per session so accuracy can be tracked over time
  • A deliberately misattributed beat and a hallucinated-citation beat are both caught in tests
**Severity: CRITICAL.** Found in the August 2026 session lifecycle review (#319). This is the issue that makes accuracy independent of model size. ## Why The two reported failure modes — scrambled chronology and misattributed actions — are both currently *trusted* to the model. This makes them *checked*. Crucially, the check is **pure code and free**. It runs the same way on a 9B local model and on a frontier hosted one. Given that self-hosted is a first-class target, this is the only design whose correctness floor does not ride on model size. ## Proposed fix After the map phase, before any prose is generated: 1. **Evidence exists** — every `evidence` timestamp must correspond to a real transcript line. 2. **Actor is grounded** — every named actor must be the resolved speaker (per the code-side legend) of at least one cited line. For an action described by the GM rather than performed on-mic, the beat is tagged `narrated` and the actor must appear *in the text* of a cited GM line. 3. **Time range is sane** — `t_start <= t_end`, both inside the session, and consistent with the cited evidence. 4. **Sort by time in code.** Chronology becomes a `sorted()` guarantee rather than a model behaviour. This is the entire point. Beats that fail validation are **flagged, not silently dropped** — surface them to the GM as "unverified" so a real event with a bad citation is recoverable rather than invisible. Silent dropping would trade one invisible failure for another. ## Acceptance criteria - [ ] Validator is pure code with no model call - [ ] All four checks implemented, each independently unit-tested - [ ] Failing beats are flagged and surfaced, never silently discarded - [ ] Validated beats are ordered by `sorted()`, not by the model - [ ] Validation results are recorded per session so accuracy can be tracked over time - [ ] A deliberately misattributed beat and a hallucinated-citation beat are both caught in tests
Author
Contributor

Verified against acceptance criteria, and five defects fixed in 152048e. This is the issue whose stated purpose is that the accuracy floor "does not ride on model size" because the check is pure code — so a check that does not run matters more here than anywhere else in the pipeline.

Criteria

  • Validator is pure code with no model callvalidate_beats is synchronous and makes no request.
  • All four checks implemented, each independently unit-tested — checks 1, 2 and 4 were genuinely done. Check 3 was not, three ways over:
    • t_start <= t_end was repaired in _coerce_beat, never recorded.
    • "inside the session" was unreachable dead code: _coerce_beat clamped t_start to transcript_seconds, then validate_beats tested beat.t_start > transcript_seconds. "starts after the session ends" could not be appended under any input.
    • "consistent with the cited evidence" was never implemented at all.
  • Failing beats are flagged and surfaced, never silently discarded — plus two silent discards found and fixed, below.
  • Validated beats are ordered by sorted() — and the sort key is now derived from verified data rather than a field nothing checked.
  • Validation results recorded per sessionused_beats was computed every run and never persisted, so the runs that skipped validation were the ones leaving no trace. Now every run gets a row.
  • A misattributed beat and a hallucinated-citation beat are both caught — already true.

The test that could not fail

test_a_beat_starting_after_the_session_ends_is_flagged asserted only result.beat.t_start <= max(index), which the clamp guarantees unconditionally. It never touched result.ok or result.problems. Its own comment hedged — "the range problem may resolve" — which reads like the author noticing and writing a weaker assertion rather than chasing it down.

Why the evidence check re-anchors instead of rejecting

My first implementation flagged a beat whose range contained none of its citations. The scrambled_chronology eval fixture immediately showed that was wrong: all five of its beats failed, so five real, correctly-attributed events dropped out of the summary. That fixture exists precisely to demonstrate this gap — its notes say so, and its expected chronology_tau: -1.0 encoded the defect.

The evidence has already been checked against the transcript; the declared range has been checked by nothing. When they disagree, the verified value wins. So the range is re-anchored onto the citations, which makes the chronology guarantee stronger than it was — the sort key now comes from transcript-verified timestamps rather than a model-supplied field.

That fixture now scores tau = +1.0 with coverage, attribution and validation all 1.0. Its expectations and notes are rewritten to record the fix rather than the gap; the five reversed timestamps still make it hand-checkable in both directions.

A hallucinated citation is never used as an anchor — that would trade a wrong time the validator reported for a wrong time it invented.

BeatValidation.repairs carries corrections, separate from problems and not affecting ok: a beat the code could fix did not fail. Persisted to session_beats.repairs and exposed through #424's API, because a silent correction is indistinguishable from a correct answer.

dedupe_beats preferred hallucinations

It kept whichever duplicate had more citations, with no reference to the verdict — and gather_validated_beats validates before merging, so both sides already carried one. A second-pass beat citing the same real line plus one invented stamp beat a clean single-citation beat. render_beats_for_compose skips failing beats, so the event then vanished from the summary having been both correctly extracted and correctly checked. Verified beats now win outright.

The loop's docstring described a filter that did not exist

"'Did this pass find anything new' has to mean anything new that survives checking" sat three lines above gained = len(merged) - len(validations), which counted flagged beats. A pass of fresh hallucinations looked like progress and ran to the cap — a full re-extraction over every window, paid for nothing.

test_a_pass_of_pure_hallucination_does_not_keep_the_loop_running passed for the wrong reason: its invented beat was identical every pass, so dedupe_beats collapsed it. It would have gone on passing if the .ok filter it is named for were deleted — which it was, because it never existed. It now varies actor and stamp per pass, and fails against the old code.

Verified by mutation, not by reading

Reverting gained and the dedupe rule fails exactly the three tests named above and nothing else. _coerce_beat's silent discards are now counted and logged.

1,306 → 1,324 passing. Migration f7a8b0c1d2e3, applied and rolled back against a real database. Closing.

**Verified against acceptance criteria, and five defects fixed in `152048e`.** This is the issue whose stated purpose is that the accuracy floor "does not ride on model size" because the check is pure code — so a check that does not run matters more here than anywhere else in the pipeline. ## Criteria - [x] **Validator is pure code with no model call** — [`validate_beats`](webapp/backend/app/services/beat_service.py#L216) is synchronous and makes no request. - [x] **All four checks implemented, each independently unit-tested** — checks 1, 2 and 4 were genuinely done. **Check 3 was not**, three ways over: - `t_start <= t_end` was *repaired* in `_coerce_beat`, never recorded. - "inside the session" was **unreachable dead code**: `_coerce_beat` clamped `t_start` to `transcript_seconds`, then `validate_beats` tested `beat.t_start > transcript_seconds`. `"starts after the session ends"` could not be appended under any input. - "consistent with the cited evidence" **was never implemented at all**. - [x] **Failing beats are flagged and surfaced, never silently discarded** — plus two silent discards found and fixed, below. - [x] **Validated beats are ordered by `sorted()`** — and the sort key is now derived from verified data rather than a field nothing checked. - [x] **Validation results recorded per session** — `used_beats` was computed every run and never persisted, so the runs that *skipped* validation were the ones leaving no trace. Now every run gets a row. - [x] **A misattributed beat and a hallucinated-citation beat are both caught** — already true. ## The test that could not fail `test_a_beat_starting_after_the_session_ends_is_flagged` asserted only `result.beat.t_start <= max(index)`, which the clamp guarantees unconditionally. It never touched `result.ok` or `result.problems`. Its own comment hedged — *"the range problem may resolve"* — which reads like the author noticing and writing a weaker assertion rather than chasing it down. ## Why the evidence check re-anchors instead of rejecting My first implementation flagged a beat whose range contained none of its citations. The **`scrambled_chronology` eval fixture immediately showed that was wrong**: all five of its beats failed, so five real, correctly-attributed events dropped out of the summary. That fixture exists precisely to demonstrate this gap — its notes say so, and its expected `chronology_tau: -1.0` encoded the defect. The evidence has already been checked against the transcript; the declared range has been checked by nothing. When they disagree, the verified value wins. So the range is **re-anchored onto the citations**, which makes the chronology guarantee *stronger* than it was — the sort key now comes from transcript-verified timestamps rather than a model-supplied field. That fixture now scores **`tau = +1.0`** with coverage, attribution and validation all 1.0. Its expectations and notes are rewritten to record the fix rather than the gap; the five reversed timestamps still make it hand-checkable in both directions. A hallucinated citation is never used as an anchor — that would trade a wrong time the validator *reported* for a wrong time it *invented*. `BeatValidation.repairs` carries corrections, separate from `problems` and not affecting `ok`: a beat the code could fix did not fail. Persisted to `session_beats.repairs` and exposed through #424's API, because a silent correction is indistinguishable from a correct answer. ## dedupe_beats preferred hallucinations It kept whichever duplicate had more citations, with **no reference to the verdict** — and `gather_validated_beats` validates *before* merging, so both sides already carried one. A second-pass beat citing the same real line plus one invented stamp beat a clean single-citation beat. `render_beats_for_compose` skips failing beats, so the event then vanished from the summary having been both correctly extracted **and** correctly checked. Verified beats now win outright. ## The loop's docstring described a filter that did not exist *"'Did this pass find anything new' has to mean anything new that survives checking"* sat three lines above `gained = len(merged) - len(validations)`, which counted flagged beats. A pass of fresh hallucinations looked like progress and ran to the cap — a full re-extraction over every window, paid for nothing. `test_a_pass_of_pure_hallucination_does_not_keep_the_loop_running` passed for the wrong reason: its invented beat was identical every pass, so `dedupe_beats` collapsed it. It would have gone on passing if the `.ok` filter it is named for were deleted — which it was, because it never existed. It now varies actor and stamp per pass, and fails against the old code. ## Verified by mutation, not by reading Reverting `gained` and the dedupe rule fails exactly the three tests named above and nothing else. `_coerce_beat`'s silent discards are now counted and logged. 1,306 → 1,324 passing. Migration `f7a8b0c1d2e3`, applied and rolled back against a real database. Closing.
Sign in to join this conversation.
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
rbrooks/Quest-Board#333
No description provided.