[Backend] Stop canonical-name selection from silently regenerating the summary #401

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

Severity: HIGH

Found in the August 2026 session lifecycle review (#319).

Picking a canonical name for something mentioned in a session (e.g. finally deciding what to call "that tavern we visited") silently throws away anything the GM wrote in the summary and replaces it with a brand-new LLM-generated draft — an action the GM never asked for and has no way to anticipate from the UI, since the endpoint's stated purpose is naming, not rewriting the summary.

Evidence

  • webapp/backend/app/routers/sessions.py:313-372 (select_canonical_name) — after persisting the picked name, if session.transcript is set (:343), it calls session.summary = await audio_service.summarise(...) (:350), replacing the entire summary field with a fresh LLM generation, with no check of whether the GM has already hand-edited the current summary and no snapshot of the prior value taken first.
  • Same handler at :364-370 then fires on_session_summary_available(..., first_time=False), which (per the storyline-overwrite issue tracked separately) rebuilds the campaign storyline too — so a canonical-name pick compounds into a second silent overwrite downstream.

Failure scenario
A GM reviews the raw AI summary after a session, fixes several factual mistakes (wrong NPC name, wrong outcome of a fight), and saves it. Later, while cleaning up the wiki, the GM opens the session's "pick a canonical name" flow to settle what a location should be called going forward. The instant they submit that pick, the summary they hand-corrected is silently replaced by a fresh LLM regeneration — which reintroduces the same mistakes the GM just fixed, with the GM's corrections gone and unrecoverable.

Proposed fix
Never regenerate the summary implicitly from an unrelated action. Either (a) drop the automatic re-summarisation from select_canonical_name entirely — the newly-picked canonical name is already applied to future generations via session.canonical_names, so there's no correctness reason to force an immediate rewrite of an already-approved summary — or (b) if regeneration is genuinely wanted (e.g. to backfill the new name into the existing prose), stage it the way the wiki draft pipeline already does: produce a draft the GM must explicitly review and approve rather than committing straight to session.summary. Prefer (a): it is the smaller, safer change and matches the review's "never regenerate summary implicitly" recommendation.

Acceptance criteria

  • Selecting a canonical name no longer silently overwrites session.summary.
  • If re-summarisation after a canonical-name pick is still desired, it is staged as a reviewable draft rather than committed directly.
  • A GM-edited summary survives a canonical-name selection with no data loss.
  • Regression test: hand-edit a session summary, then call POST /sessions/{id}/canonical-names, and assert the summary is unchanged (or, if staged, that the original is still readable/approvable separately from the draft).
**Severity: HIGH** Found in the August 2026 session lifecycle review (#319). Picking a canonical name for something mentioned in a session (e.g. finally deciding what to call "that tavern we visited") silently throws away anything the GM wrote in the summary and replaces it with a brand-new LLM-generated draft — an action the GM never asked for and has no way to anticipate from the UI, since the endpoint's stated purpose is naming, not rewriting the summary. **Evidence** - `webapp/backend/app/routers/sessions.py:313-372` (`select_canonical_name`) — after persisting the picked name, if `session.transcript` is set (`:343`), it calls `session.summary = await audio_service.summarise(...)` (`:350`), replacing the entire summary field with a fresh LLM generation, with no check of whether the GM has already hand-edited the current summary and no snapshot of the prior value taken first. - Same handler at `:364-370` then fires `on_session_summary_available(..., first_time=False)`, which (per the storyline-overwrite issue tracked separately) rebuilds the campaign storyline too — so a canonical-name pick compounds into a second silent overwrite downstream. **Failure scenario** A GM reviews the raw AI summary after a session, fixes several factual mistakes (wrong NPC name, wrong outcome of a fight), and saves it. Later, while cleaning up the wiki, the GM opens the session's "pick a canonical name" flow to settle what a location should be called going forward. The instant they submit that pick, the summary they hand-corrected is silently replaced by a fresh LLM regeneration — which reintroduces the same mistakes the GM just fixed, with the GM's corrections gone and unrecoverable. **Proposed fix** Never regenerate the summary implicitly from an unrelated action. Either (a) drop the automatic re-summarisation from `select_canonical_name` entirely — the newly-picked canonical name is already applied to future generations via `session.canonical_names`, so there's no correctness reason to force an immediate rewrite of an already-approved summary — or (b) if regeneration is genuinely wanted (e.g. to backfill the new name into the existing prose), stage it the way the wiki draft pipeline already does: produce a draft the GM must explicitly review and approve rather than committing straight to `session.summary`. Prefer (a): it is the smaller, safer change and matches the review's "never regenerate `summary` implicitly" recommendation. **Acceptance criteria** - [ ] Selecting a canonical name no longer silently overwrites `session.summary`. - [ ] If re-summarisation after a canonical-name pick is still desired, it is staged as a reviewable draft rather than committed directly. - [ ] A GM-edited summary survives a canonical-name selection with no data loss. - [ ] Regression test: hand-edit a session summary, then call `POST /sessions/{id}/canonical-names`, and assert the summary is unchanged (or, if staged, that the original is still readable/approvable separately from the draft).
Author
Contributor

Shipped in PR #456 alongside #400 (merged, CI green) - the canonical-name endpoint no longer re-summarises, so a GM's hand-corrected summary survives a name pick.

Left open by accident: the PR said Closes #400 and #401 and Forgejo only acted on the first of the pair. Worth knowing for future multi-issue PRs - reference each one on its own line if you want both closed automatically.

Closing now.

Shipped in PR #456 alongside #400 (merged, CI green) - the canonical-name endpoint no longer re-summarises, so a GM's hand-corrected summary survives a name pick. Left open by accident: the PR said `Closes #400 and #401` and Forgejo only acted on the first of the pair. Worth knowing for future multi-issue PRs - reference each one on its own line if you want both closed automatically. Closing now.
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#401
No description provided.