rbrooks/Quest-Board

Watch

Fork 0

[Backend] Emit structured, timestamped beats with evidence references #332

New issue

Closed

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

claude-bot commented

2026-08-25 20:38:51 +00:00

Collaborator

Severity: CRITICAL. Found in the August 2026 session lifecycle review (#319). The core of the accuracy re-architecture.

Why

The summary is currently free prose with no timestamps, no citations, and no required evidence — nothing lets a reader or a downstream check verify a claimed action against the transcript. That is why a total corruption of the input produced output that read fine to a human for months.

The codebase already knows how to do this properly: extract_highlights validates every quote by normalised substring match against the transcript and carries a timestamp_ref (audio_service.py:954-1101). The summary path has no equivalent. This issue generalises that proven pattern.

Proposed fix

The map phase over each time window emits grammar-enforced JSON:

{"beats": [{"t_start": "01:12:30", "t_end": "01:15:05",
            "actors": ["Kira"],
            "type": "combat|decision|reveal|travel|social|other",
            "summary": "Kira disarms the shrine trap",
            "evidence": ["[01:12:41]", "[01:13:02]"]}]}

Constrain the shape at the decoder, not by asking politely — llama.cpp supports json_schema to GBNF grammar (see the constrained-decoding spike, #281), and Anthropic/OpenAI support structured outputs. response_format: {"type":"json_object"} alone is advisory on the current deployment and is not sufficient.

Beats become reusable structure, not a throwaway intermediate: a timeline UI, the input to highlights, the input to the lore extract phase (replacing today's second full-transcript pass — a net token saving across the pipeline), and an indexable event store for /ask.

Acceptance criteria

A beat schema exists and is enforced by grammar or structured output on every provider that supports it
Beats carry a time range, actors, type, summary, and at least one evidence timestamp
Beats are persisted, not discarded after the summary is written
Providers without schema enforcement fall back to the tolerant parser and are flagged as degraded
Tests cover a well-formed response, a malformed one, and a truncated one

**Severity: CRITICAL.** Found in the August 2026 session lifecycle review (#319). The core of the accuracy re-architecture. ## Why The summary is currently free prose with **no timestamps, no citations, and no required evidence** — nothing lets a reader or a downstream check verify a claimed action against the transcript. That is why a total corruption of the input produced output that read fine to a human for months. The codebase already knows how to do this properly: `extract_highlights` validates every quote by normalised substring match against the transcript and carries a `timestamp_ref` (`audio_service.py:954-1101`). The summary path has no equivalent. This issue generalises that proven pattern. ## Proposed fix The map phase over each time window emits grammar-enforced JSON: ```json {"beats": [{"t_start": "01:12:30", "t_end": "01:15:05", "actors": ["Kira"], "type": "combat|decision|reveal|travel|social|other", "summary": "Kira disarms the shrine trap", "evidence": ["[01:12:41]", "[01:13:02]"]}]} ``` Constrain the shape at the decoder, not by asking politely — llama.cpp supports `json_schema` to GBNF grammar (see the constrained-decoding spike, #281), and Anthropic/OpenAI support structured outputs. `response_format: {"type":"json_object"}` alone is advisory on the current deployment and is not sufficient. Beats become reusable structure, not a throwaway intermediate: a timeline UI, the input to highlights, the input to the lore extract phase (replacing today's second full-transcript pass — a net token saving across the pipeline), and an indexable event store for `/ask`. ## Acceptance criteria - [ ] A beat schema exists and is enforced by grammar or structured output on every provider that supports it - [ ] Beats carry a time range, actors, type, summary, and at least one evidence timestamp - [ ] Beats are persisted, not discarded after the summary is written - [ ] Providers without schema enforcement fall back to the tolerant parser and are flagged as degraded - [ ] Tests cover a well-formed response, a malformed one, and a truncated one

claude-bot added this to the v4.0.0 - Session Pipeline Accuracy milestone

2026-08-25 20:38:51 +00:00

claude-bot added the

bug

backend

labels

2026-08-25 20:38:51 +00:00

rbrooks referenced this issue from a commit

2026-08-26 01:30:27 +00:00

feat(backend): verify session summaries against the transcript (#332, #333, #334)

rbrooks referenced this issue from a commit

2026-08-28 00:45:01 +00:00

feat(backend): persist the events a summary was built from (#332, #333, #329)

rbrooks referenced this issue from a commit

2026-08-28 18:39:30 +00:00

fix(backend): make the beat validator actually check what it claims (#332, #333)

claude-bot commented

2026-08-28 19:14:22 +00:00

Author

Collaborator

Verified; gaps fixed in 152048e.

Criteria

A beat schema exists and is enforced by grammar or structured output on every provider that supports it — EXTRACT_SCHEMA, shaped per provider by _apply_json_schema, request shapes tested per provider.
Beats carry a time range, actors, type, summary, and at least one evidence timestamp — see the note on minItems below.
Beats are persisted, not discarded — and now for every run, not only ones that produced beats.
Providers without schema enforcement fall back to the tolerant parser and are flagged as degraded — was not built at all. Now is.
Tests cover a well-formed response, a malformed one, and a truncated one — at the beat layer, which had none.

"Flagged as degraded" had never existed

grep -rn degraded across the backend returned only unrelated docstrings. The tolerant fallback was there; nothing recorded that it had been used.

The observable signature is that the response needed structural repair — on llama.cpp and Ollama the schema compiles to a decoding grammar, so a conforming server cannot emit malformed JSON. If repair was necessary, the grammar was not applied and the beats came from the tolerant parser rather than a guarantee.

This matters concretely because of #281: llama.cpp's own documented response_format shape is silently ignored by real servers, so every json_schema call in production was doing nothing at all — with green unit tests throughout. That is exactly the condition this criterion asks to be made visible, and it still would not have been.

Recorded on summarisation_runs.schema_degraded and exposed through the API. Implemented as a callback rather than a changed return type, because a dozen tests monkeypatch extract_beats and the signal was worth having without churning all of them.

extract_beats was never invoked by any test

Every test in the suite patches it out, and the doubles return pre-parsed dicts — so the parsing it exists to do had zero coverage. "Tests cover a well-formed response, a malformed one, and a truncated one" was met one layer down at the transport, never here. Now covered directly: well-formed, unparseable, no beats key, repaired, and clean.

The best-effort promise was not actually kept

_persist_summarisation_run flushed without a savepoint, inside the transaction that later commits the transcript and summary. Catching the exception does not clear a session needing rollback, so the next commit() would die of PendingRollbackError — meaning a failure to record provenance would destroy the artifact the GM actually asked for. Precisely backwards, and the attendance block a few hundred lines away already had this right.

The guarding test could not catch it: test_a_failure_to_record_never_costs_the_summary passes identities=[object()], which raises inside the legend comprehension before any DB call, so it exercises the one failure mode that structurally cannot poison the transaction. There is now one that writes a NUL byte — Postgres rejects \x00 in a text column, and a model can emit one — and asserts the session is still committable afterwards. Verified by mutation: it fails without the savepoint, and the original test passes either way.

One criterion deliberately not met

minItems: 1 on the evidence array. That constraint compiles into a decoding grammar, so a model that genuinely cannot cite a line would be forced to emit one — converting a visible "cites no transcript line" flag into an invisible fabricated citation. In a milestone built on not trusting the model, that trade is backwards. The validator already flags empty evidence, and _coerce_beat accepts it so the beat is recorded rather than dropped.

Also

unsupported_sentences was computed on every run and dropped on the floor, despite #334's commit message claiming it landed on the record. Now persisted alongside used_beats and schema_degraded.

Migration f7a8b0c1d2e3. Closing.

**Verified; gaps fixed in `152048e`.** ## Criteria - [x] **A beat schema exists and is enforced by grammar or structured output on every provider that supports it** — `EXTRACT_SCHEMA`, shaped per provider by `_apply_json_schema`, request shapes tested per provider. - [x] **Beats carry a time range, actors, type, summary, and at least one evidence timestamp** — see the note on `minItems` below. - [x] **Beats are persisted, not discarded** — and now for *every* run, not only ones that produced beats. - [x] **Providers without schema enforcement fall back to the tolerant parser and are flagged as degraded** — **was not built at all.** Now is. - [x] **Tests cover a well-formed response, a malformed one, and a truncated one** — at the *beat* layer, which had none. ## "Flagged as degraded" had never existed `grep -rn degraded` across the backend returned only unrelated docstrings. The tolerant fallback was there; nothing recorded that it had been used. The observable signature is that the response **needed structural repair** — on llama.cpp and Ollama the schema compiles to a decoding grammar, so a conforming server *cannot* emit malformed JSON. If repair was necessary, the grammar was not applied and the beats came from the tolerant parser rather than a guarantee. This matters concretely because of #281: llama.cpp's own documented `response_format` shape is **silently ignored** by real servers, so every `json_schema` call in production was doing nothing at all — with green unit tests throughout. That is exactly the condition this criterion asks to be made visible, and it still would not have been. Recorded on `summarisation_runs.schema_degraded` and exposed through the API. Implemented as a callback rather than a changed return type, because a dozen tests monkeypatch `extract_beats` and the signal was worth having without churning all of them. ## extract_beats was never invoked by any test Every test in the suite patches it out, and the doubles return pre-parsed dicts — so the parsing it exists to do had **zero coverage**. "Tests cover a well-formed response, a malformed one, and a truncated one" was met one layer down at the transport, never here. Now covered directly: well-formed, unparseable, no `beats` key, repaired, and clean. ## The best-effort promise was not actually kept `_persist_summarisation_run` flushed **without a savepoint**, inside the transaction that later commits the transcript and summary. Catching the exception does not clear a session needing rollback, so the *next* `commit()` would die of `PendingRollbackError` — meaning a failure to record **provenance** would destroy the artifact the GM actually asked for. Precisely backwards, and the attendance block a few hundred lines away already had this right. The guarding test could not catch it: `test_a_failure_to_record_never_costs_the_summary` passes `identities=[object()]`, which raises inside the legend comprehension *before any DB call*, so it exercises the one failure mode that structurally cannot poison the transaction. There is now one that writes a NUL byte — Postgres rejects `\x00` in a text column, and a model can emit one — and asserts the session is still committable afterwards. Verified by mutation: it fails without the savepoint, and the original test passes either way. ## One criterion deliberately not met **`minItems: 1` on the evidence array.** That constraint compiles into a decoding grammar, so a model that genuinely cannot cite a line would be *forced* to emit one — converting a visible `"cites no transcript line"` flag into an **invisible fabricated citation**. In a milestone built on not trusting the model, that trade is backwards. The validator already flags empty evidence, and `_coerce_beat` accepts it so the beat is recorded rather than dropped. ## Also `unsupported_sentences` was computed on every run and dropped on the floor, despite #334's commit message claiming it landed on the record. Now persisted alongside `used_beats` and `schema_degraded`. Migration `f7a8b0c1d2e3`. Closing.

claude-bot closed this issue

2026-08-28 19:16:03 +00:00