fix: generated content stops overwriting GM authorship (#400, #401)

claude-bot commented

2026-08-31 02:39:30 +00:00

Contributor

Closes #400 and #401 (both HIGH). Two paths, one principle — and they compound, so they ship together.

#400 — the storyline rebuild

storyline.body = new_body was unconditional, and the task fires on every transcription, every summary edit and every canonical-name pick, for any session in the campaign. An hour spent turning six AI summaries into a readable chronicle survived until the next recording. There is no storyline history table, so it was not merely overwritten — it was unrecoverable.

The full_regenerate flag did not help: both of its branches compute the identical string, so no code path preserved an existing body at all.

Adds campaign_storylines.manually_edited_at (migration c1d2e3f4a5b7), set by the manual-edit endpoint. Its presence is the generated-versus-authored distinction the rebuild never had. The automatic trigger now leaves an authored body alone; the GM's explicit Regenerate rebuilds and clears the mark — which is the first behaviour full_regenerate has ever actually had.

A timestamp rather than a boolean because it costs the same and answers "when", which is what a GM asking "why did my storyline stop updating?" wants. Exposed on the response so the UI can say so.

#401 — canonical-name selection

Picking a name for a tavern re-summarised the whole session and committed straight over session.summary. A GM who had corrected the AI's mistakes by hand got those mistakes back, from an action whose stated purpose is naming, with nothing in the UI to suggest it would rewrite the recap.

Removed entirely, which is option (a) in the issue and the smaller change. The picked name lands in session.canonical_names, which every later generation already reads — so it applies going forward without rewriting prose a GM has approved.

The compounding: the regeneration also fired on_session_summary_available, which rebuilt the storyline. One canonical-name pick could destroy the corrected summary and the campaign chronicle. Fixing either alone leaves half of that.

Three existing tests asserted the removed behaviour

One (test_select_canonical_name_refreshes_summary_for_transcribed_session) required the summary to be refreshed — the defect written down as the specification. Inverted, with the history in its docstring so nobody restores it.

Two used the canonical-name route as a vehicle to check system-prompt threading into summarise. Their vehicle is gone. Rather than delete them silently, the note left in their place records what is now uncovered: summarise's only remaining call site is process_audio, and nothing drives _process_audio_async in the suite today, so a call-site test there needs a harness that does not exist. The behaviour they checked (linked vs free-form resolution) is still covered by the direct unit tests and two other call sites.

Verification — the first attempt was worthless

Mutation-checking caught both test files being vacuous before I trusted them:

#401's tests passed with the fix reverted. The removed code sat behind if llm_cfg is not None, and the suite configures no LLM, so the block never ran either way. They now seed a config and a summariser that would fire; reverting the fix fails two of them, including the compounding storyline case.
#400's tests needed task_session patched to the test's own session. Without it the task found no sessions and returned before reaching the guard — so "the storyline was not overwritten" held for a task that never ran.

With both fixed: removing the storyline guard fails the hand-written-storyline test; reverting the canonical-name change fails two more. There is also a test that a generated storyline is still rebuilt, so the guard cannot pass by freezing the feature.

1,455 backend tests pass. Migration verified applying and downgrading over the full chain. Lint clean at pinned ruff 0.4.4.

Note on this branch

This commit was briefly pushed on top of #454's branch by mistake. It has been rebased onto main and force-pushed, so this PR is a single independent commit; PR #455 is back to #454 alone. Worth a fresh look at #455's diff if you had already reviewed it.

🤖 Generated with Claude Code

Closes #400 and #401 (both HIGH). Two paths, one principle — and they compound, so they ship together. ## #400 — the storyline rebuild `storyline.body = new_body` was unconditional, and the task fires on every transcription, every summary edit and every canonical-name pick, **for any session in the campaign**. An hour spent turning six AI summaries into a readable chronicle survived until the next recording. There is no storyline history table, so it was not merely overwritten — it was unrecoverable. The `full_regenerate` flag did not help: **both of its branches compute the identical string**, so no code path preserved an existing body at all. Adds `campaign_storylines.manually_edited_at` (migration `c1d2e3f4a5b7`), set by the manual-edit endpoint. Its presence is the generated-versus-authored distinction the rebuild never had. The automatic trigger now leaves an authored body alone; the GM's explicit **Regenerate** rebuilds and clears the mark — which is the first behaviour `full_regenerate` has ever actually had. A timestamp rather than a boolean because it costs the same and answers "when", which is what a GM asking "why did my storyline stop updating?" wants. Exposed on the response so the UI can say so. ## #401 — canonical-name selection Picking a name for a tavern re-summarised the whole session and committed straight over `session.summary`. A GM who had corrected the AI's mistakes by hand got those mistakes back, from an action whose stated purpose is naming, with nothing in the UI to suggest it would rewrite the recap. Removed entirely, which is option (a) in the issue and the smaller change. The picked name lands in `session.canonical_names`, which every later generation already reads — so it applies going forward without rewriting prose a GM has approved. **The compounding:** the regeneration also fired `on_session_summary_available`, which rebuilt the storyline. One canonical-name pick could destroy the corrected summary *and* the campaign chronicle. Fixing either alone leaves half of that. ## Three existing tests asserted the removed behaviour One (`test_select_canonical_name_refreshes_summary_for_transcribed_session`) *required* the summary to be refreshed — the defect written down as the specification. Inverted, with the history in its docstring so nobody restores it. Two used the canonical-name route as a vehicle to check system-prompt threading into `summarise`. Their vehicle is gone. Rather than delete them silently, the note left in their place records what is now uncovered: `summarise`'s only remaining call site is `process_audio`, and **nothing drives `_process_audio_async` in the suite today**, so a call-site test there needs a harness that does not exist. The behaviour they checked (linked vs free-form resolution) is still covered by the direct unit tests and two other call sites. ## Verification — the first attempt was worthless Mutation-checking caught both test files being vacuous before I trusted them: - **#401's tests passed with the fix reverted.** The removed code sat behind `if llm_cfg is not None`, and the suite configures no LLM, so the block never ran either way. They now seed a config *and* a summariser that would fire; reverting the fix fails two of them, including the compounding storyline case. - **#400's tests needed `task_session` patched** to the test's own session. Without it the task found no sessions and returned before reaching the guard — so "the storyline was not overwritten" held for a task that never ran. With both fixed: removing the storyline guard fails the hand-written-storyline test; reverting the canonical-name change fails two more. There is also a test that a *generated* storyline is still rebuilt, so the guard cannot pass by freezing the feature. **1,455 backend tests pass.** Migration verified applying and downgrading over the full chain. Lint clean at pinned ruff 0.4.4. ## Note on this branch This commit was briefly pushed on top of #454's branch by mistake. It has been rebased onto `main` and force-pushed, so this PR is a single independent commit; PR #455 is back to #454 alone. Worth a fresh look at #455's diff if you had already reviewed it. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 1 commit

2026-08-31 02:39:31 +00:00

fix: generated content stops overwriting GM authorship (#400 , #401 )

CI / Summarisation accuracy eval harness (stub provider) (pull_request) Successful in 54s

Details

CI / Bot tests and audit (pull_request) Successful in 1m22s

Details

CI / Frontend tests, audit, and build (pull_request) Successful in 1m27s

Details

CI / Bot/backend version sync (pull_request) Successful in 52s

Details

CI / Backend lint (ruff) (pull_request) Successful in 57s

Details

CI / Docker image build (pull_request) Successful in 1m17s

Details

CI / Backend migration, tests, and audit (pull_request) Successful in 5m16s

Details

eb561eb957

Two paths, one principle, and they compound — a canonical-name pick
regenerated the summary and fired the fan-out that rebuilt the storyline, so
one click could destroy both. Neither keeps history, so the GM's work was
not merely replaced; it was unrecoverable.

transcription, summary edit and canonical-name pick, for any session in the
campaign, and `storyline.body = new_body` was unconditional. Adds
campaign_storylines.manually_edited_at (c1d2e3f4a5b7), set by the manual
edit endpoint; the automatic trigger now leaves an authored body alone.
The GM's explicit regenerate still rebuilds and clears the mark — which is
the first behaviour full_regenerate has ever had, since both of its
branches compute the identical body.

straight over the GM's corrections, reintroducing the very mistakes they
had just fixed, from an action whose stated purpose is naming. Removed
entirely: the name lands in canonical_names, which every later generation
reads, so nothing is gained by rewriting prose already approved.

Three existing tests asserted the removed behaviour. One required the
summary to be refreshed — the defect written down as the specification —
and is inverted. Two used the canonical-name route as a vehicle to check
system-prompt threading into summarise; their vehicle is gone, and the note
left in their place records that summarise's remaining call site
(process_audio) has no call-site test rather than hiding the gap.

Mutation-checked. The first attempt did not hold: the removed code sat
behind `if llm_cfg is not None`, and the suite configures no LLM, so the
canonical-name tests passed with or without the fix. They now seed a config
and a summariser that would fire, and reverting the fix fails two of them.
The storyline tests likewise needed task_session patched to the test's own
session, or the task found no sessions and returned before reaching the
guard.

1,458 backend tests pass. Migration verified up and down over the full chain.

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

claude-bot referenced this pull request

2026-08-31 02:45:04 +00:00

[Tests] Nothing exercises process_audio's body — the pipeline's own orchestration is untested #457

rbrooks force-pushed fix/400-401-gm-authorship from eb561eb957

CI / Summarisation accuracy eval harness (stub provider) (pull_request) Successful in 54s

Details

CI / Bot tests and audit (pull_request) Successful in 1m22s

Details

CI / Frontend tests, audit, and build (pull_request) Successful in 1m27s

Details

CI / Bot/backend version sync (pull_request) Successful in 52s

Details