Skip to content

fix(chat): stop branching a rewrite-free chat from forking the card - #386

Merged
hydall merged 1 commit into
nightlyfrom
claude/card-disabled-studio-bug-mktbhx
Sep 5, 2026
Merged

fix(chat): stop branching a rewrite-free chat from forking the card#386
hydall merged 1 commit into
nightlyfrom
claude/card-disabled-studio-bug-mktbhx

Conversation

@hydall

@hydall hydall commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Branching a chat always forked a character variant, so a chat that never ran the Card Rewriter — Studio off, no rewrite ever applied — left a Branch from X card behind on every branch and buried the character list under variants nothing had ever edited.

The fork is not needed there: ManualRewriteApplyRepo._forkSessionCharacter already forks a root card (variantOrder == 0) lazily, on the first rewrite that would edit it, and rebinds that session to the fork. Forking up front only mattered for a session whose card can already differ from the character it points at.

Changes

  • ChatSessionBranchRepo.requiresCardForkInTransaction decides whether a branch needs its own card: a canon checkpoint past the root (a rewrite apply or a rollback), a session lorebook overlay, or a source card that is itself a session-owned variant (variantOrder != 0) — a variant is edited in place by a rewrite, so two sessions must never share one.
  • ChatSessionService.branchSession takes the forked path only when that returns true; the forked branch is unchanged (card rooted at the latest surviving checkpoint, own canon root, ${newCharacterId}_0).
  • ChatSessionService._insertBranchSession handles the other case: the branch is a plain extra session on the same character — ${charId}_$nextIndex with the retained slice, branchedAt, and the author's note, claiming its index the way _insertNewSession does and moving the character's current session onto it. No character row, no revision row, no canon checkpoint.
  • An unforked branch keeps the source session's own baseline evidence (CharacterSessionBaselineRepo.copyForSessionBranch with a null card/hash), and canon transitions are inherited only by a forked branch, which is the only one with a timeline to inherit.
  • The Branch action in message_actions.dart opens ?session=${branch.sessionIndex} instead of a hardcoded ?session=0, which would otherwise land on the wrong chat.
  • docs/rules/database.md § Session branch policy documents the fork rule.

The Studio enabled flag deliberately does not enter the condition: with Studio on but no rewrite applied, the card has still not diverged, and the first rewrite forks it on its own.

Verification

Flutter 3.44.9 installed in the agent container, dart run build_runner build and the easy_localization generator run as CI does.

  • flutter analyze --no-fatal-infos --no-fatal-warnings — no issues on the changed files; the tree's 9 pre-existing infos/warnings are untouched.
  • flutter test test/chat_session_branch_transaction_test.dart — 6/6. The existing "durable, current, provenance-filtered, and isolated" case now asserts the no-fork outcome (branch on c1, no second character row, no revision row, no checkpoint, source baseline kept), and two cases were added for the fork that must stay: a session-owned variant (new card in the same group, variantOrder 2, own canon root) and an evolved lorebook entry. The checkpoint-timeline case is unchanged and still forks.
  • Full flutter test — green. The first run of this PR was red on prompt_build_architecture_test.dart › "Studio request assembly excludes ordinary prompt artifacts", which was failing on the base commit too and is fixed by fix(studio): keep prompt_builder.dart out of the Studio assembly files #387; this branch has since been rebased onto nightly carrying that fix, and all three checks pass on the current head.
  • The WebView render suite was not run locally — no assets in assets/chat_webview/ are involved — and it is green in CI.

🤖 Generated with Claude Code

https://claude.ai/code/session_0137NTHNxY3UPZ4iwrnyW1jU

hydall commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

CI status on 8029fb6: WebView render tests ✅, Bug-triage agent (unit tests) ✅, Analyze + test ❌ — the analyzer step passed, and the test step ended +3792 -1 with exactly one failure:

Failing tests:
  test/prompt_build_architecture_test.dart: Studio request assembly excludes ordinary prompt artifacts

This one is not this PR's. It fails identically on the base commit (824e4ef, the current head of nightly), which I reproduced locally before pushing — the test reads source text, so it is deterministic and a re-run cannot change it. Nothing in this diff touches lib/core/llm/; the branch only changes chat_session_branch_repo.dart, chat_session_service.dart, message_actions.dart, one test file and one doc.

The cause: that architecture test forbids the literal string prompt_builder.dart anywhere in six Studio files, and lib/core/llm/studio_message_builder.dart names it in two comments introduced by #385 — line 115 (// classic non-Studio pipeline — see 'applyAppendToLastMessage' in 'prompt_builder.dart') and line 559 (/// classic 'applyAppendToLastMessage' in 'prompt_builder.dart'). There is no import and no PromptPayload / PromptResult use, so it is the wording alone that trips the contract.

No fix for it exists anywhere yet, so there is nothing to port into this PR. The proposal, kept to the two comments so the test's intent (Studio assembly must not reach into the ordinary prompt builder) stays exactly as strict:

-    // classic non-Studio pipeline — see `applyAppendToLastMessage` in
-    // `prompt_builder.dart`). Their macros are expanded once here so the merged
+    // classic non-Studio pipeline — see `applyAppendToLastMessage` in the
+    // ordinary prompt builder). Their macros are expanded once here so the merged

-  /// classic `applyAppendToLastMessage` in `prompt_builder.dart`. Studio history
+  /// classic `applyAppendToLastMessage` in the ordinary prompt builder. Studio history

That belongs on nightly rather than in this PR, so it is not landing here on my own. Say the word and I will open it as its own PR into nightly and port the same change here so this branch goes green without waiting for that merge.


Generated by Claude Code

Branching always forked a character variant, so a chat that never ran the
Card Rewriter — Studio off, no rewrite applied — left a "Branch from X"
card behind on every branch and buried the character list under variants
nothing had ever edited.

- `ChatSessionBranchRepo.requiresCardForkInTransaction` decides it: a
  branch forks only when the session's card can already differ from the
  character it points at — a canon checkpoint past the root (a rewrite
  apply or a rollback), a session lorebook overlay, or a source card that
  is itself a session-owned variant, which a rewrite would edit in place
  and so must not be shared by two sessions
- otherwise the branch is a plain extra session on the same card
  (`${charId}_$nextIndex`, retained slice, no character/revision/checkpoint
  row); the first rewrite in either session forks the root card by itself
  through `ManualRewriteApplyRepo._forkSessionCharacter`, so the source
  session's card stays protected
- the branch keeps the source session's baseline evidence when it does not
  fork, and inherits canon transitions only when it does
- the Branch action now opens the branch's own session index instead of
  hardcoding `?session=0`
- document the fork policy in `docs/rules/database.md` § Session branch
  policy

Verified: `flutter analyze` clean on the changed files; the branch suite
covers both sides — no fork without a rewrite (no character row, no
revision, no checkpoint, source baseline kept), fork retained for a
session-owned variant, an evolved lorebook entry, and a checkpoint
timeline. Full `flutter test` is green apart from
`prompt_build_architecture_test.dart`, which already fails on the base
commit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0137NTHNxY3UPZ4iwrnyW1jU
@hydall
hydall force-pushed the claude/card-disabled-studio-bug-mktbhx branch from 8029fb6 to 2002db2 Compare September 4, 2026 20:26
@hydall
hydall merged commit e13ebc5 into nightly Sep 5, 2026
3 checks passed
@hydall
hydall deleted the claude/card-disabled-studio-bug-mktbhx branch September 6, 2026 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants