fix(desktop): branch long sessions without renderer transcript load

[benjaminbrumbaugh]

Bug Description

Fixes #103139

Desktop whole-session branching fails for long conversations with:

Session transcript exceeds the Desktop safe-load limit; use the Web Dashboard export for this session.

This blocks the primary reason to branch a conversation: preserving a large amount of accumulated context.

Root Cause

The Desktop renderer fetched and materialized the complete persisted transcript before sending a branch request. The generic renderer safety guard rejects the JSON payload at 32,000,000 characters. The gateway already owns the canonical SQLite transcript and can copy it in bounded batches, so the renderer was doing unnecessary work and hitting the wrong memory boundary.

Fix

• Whole-session Desktop branches use explicit session.branch_whole and session.branch_stored RPCs.
• The gateway reads the authoritative persisted display transcript and creates the child without returning the full transcript to Desktop.
• Stored-session branching preserves the owning connection/profile route and never sends transcript JSON through the ambient renderer.
• Older gateways are detected by their missing-method response and fall back to the existing transcript-backed path, preventing a mixed-version client from silently creating an empty branch.
• Message-level branching keeps its existing behavior; the separate durable-row selection fix is tracked in fix(desktop): branch no longer drops the conversation tail #98025.

Priority

P1 — major feature broken, no acceptable Desktop workaround. This is a complete failure of a core session operation for the long-context conversations where branching is most valuable. The suggested Dashboard export path is not equivalent to branching in Desktop: it changes products and does not preserve the user’s normal branch workflow. The failure also forces users to choose between abandoning the branch and materializing a potentially enormous renderer payload, with avoidable memory pressure as a secondary impact.

How to Verify

1. Open or resume a conversation whose persisted transcript is large enough to approach or exceed the Desktop 32,000,000-character safe-load threshold.
2. Use the conversation-level Branch action, not a branch-at-message action.
3. Confirm the branch is created and becomes available without the Desktop safe-load error.
4. Confirm a stored/sidebar parent is routed to its owning backend and that a mixed-version fallback still sends the complete transcript only through the legacy path.

Test Plan

• Added regression coverage for renderer-free whole-session branching, backend-authoritative parent copying, omitted branch responses, and older-gateway fallback.
• npx vitest run src/app/session/hooks/use-session-actions.test.tsx — 98 passed.
• npx tsc -p tsconfig.json --noEmit.
• Scoped ESLint and Prettier checks for changed Desktop files.
• scripts/run_tests.sh tests/test_tui_gateway_server.py focused branch selection — 3 passed.
• Manual verification against a production-sized transcript — not available in this local test run.

Risk Assessment

Low / Medium — the new RPCs are additive, the existing safe-load guard remains for general transcript hydration and legacy fallback, and message-level branching is unchanged. The principal compatibility risk is mixed-version client/backend behavior; it is covered by an explicit missing-method fallback regression test.

Related Work

• Related to [Bug]: Desktop branch drops the conversation tail (count mismatch between merged frontend transcript and raw backend history) #80973 (Desktop branching correctness for compacted/nested history).
• Deliberately does not duplicate fix(desktop): branch no longer drops the conversation tail #98025 (durable row-ID selection for message-level branches).

Review Notes

The authoritative layer is the gateway’s persisted session database: the Desktop client now sends session identity and branch intent, while the backend owns transcript selection, copying, and persistence. The response is metadata-only unless the legacy fallback is required.

[JoaoMarcos44]

Independent validation — premise confirmed on origin/main, fix is at the right layer

I verified the issue premise line by line against origin/main before reading this PR, then audited the diff and ran the backend suite. Summary: the bug is real, this PR fixes the root cause at the correct architectural layer, and it is the only open PR doing so. I found one confirmed defect worth fixing before merge, plus a staleness condition.

1. The premise holds (verified on origin/main)

apps/desktop/src/api/sessions.ts:524-564 — getAllSessionMessages pages 500 rows at a time, accumulates jsonChars += (JSON.stringify(page.messages) ?? '').length at :545, and throws the reported string at :547-551 once the accumulation passes 32_000_000 (:531).

The user-visible failure is the stored/sidebar path: use-session-actions/index.ts:2367 (branchStoredSession) calls getAllSessionMessages inside a try, and the catch at :2385-2388 surfaces it verbatim through notifyError(err, copy.branchFailed). That is exactly the reported toast.

Worth naming explicitly, because it is a second symptom of the same root cause: the open-chat path branchCurrentSession:2243-2332 swallows the identical exception at :2285-2287. It shows no toast, but it silently degrades the branch to the compacted live atom instead of the durable display projection. Quieter, not healthier.

And the round-trip really was unnecessary: tui_gateway/methods_session.py:1874-1888 (_branch_source_history) already reads the authoritative display projection via db.get_resume_conversations(...). The renderer was being used as transport for a dataset the backend owns — which is what apps/desktop/AGENTS.md rules out ("the backend is authoritative for anything another Hermes surface can also change; treat the renderer's copy as a cache of that truth").

2. Confirmed defect — omit_messages leaks onto the message-level call

In forkBranch the new branchParams object carries omit_messages: true unconditionally, and the same object feeds both arms of the ternary:

const branchParams = {
  session_id: sourceSessionId,
  omit_messages: true,
  ...(branchCount !== undefined ? { count: branchCount } : {})
}
// ...
sourceSessionId
  ? requestBranchGateway(branchCount === undefined ? 'session.branch_whole' : 'session.branch', branchParams)

When branchCount !== undefined this is the message-level branch, so session.branch now receives omit_messages: true. On an updated gateway _branch_live therefore returns messages_omitted and no messages array — and the renderer's downstream preference at index.ts:2125-2128 is:

const responseBranchMessages =
  sourceSessionId && branched.messages?.length ? toBranchMessages(toChatMessages(branched.messages)) : []
const effectiveBranchMessages = responseBranchMessages.length ? responseBranchMessages : branchMessages

With the echo suppressed, message-level branches silently fall back to the renderer's local prefix for the child's painted transcript and preview, while the server persisted its own reconciled _branch_source_history rows (carrying the _BRANCH_COPY_FIELDS reasoning/display_kind/timestamp data, and applying history[:count] against the server's list, not the client's). The two prefixes can disagree — the exact class of divergence #98025 exists to fix. It also contradicts this PR's own stated non-goal, "Message-level branching keeps its existing behavior."

Minimal fix: scope the flag to the whole-session call only, e.g. keep branchParams free of it and pass { ...branchParams, omit_messages: true } on the session.branch_whole arm, leaving session.branch untouched.

3. Points I checked and am satisfied with

• Legacy fallback is sound, not a silent-empty-branch risk. isMissingRpcMethod (apps/desktop/src/lib/gateway-rpc.ts:1-6) matches /method not found|-32601|unknown method|no such method/i, and the backend's own reply is tui_gateway/server.py:732 → _err(rid, -32601, f"unknown method: {method}"). Exact match; timeouts and 5xx do not match, so a new gateway failing for another reason rethrows instead of falling through.
• Separate methods, not an overloaded session.create flag. Correct call — an older gateway ignoring an unknown copy_parent_history parameter would have created an empty branch. The docstring makes that reasoning explicit.
• Owner routing preserved. branchStoredSession still resolves ownerRoute from the parent row and ensureGatewayAgents before the RPC, so a foreign-owned parent still branches on its owning backend.
• The 32M guard stays for general hydration, export (lib/session-export.ts:29-36) and artifacts (app/artifacts/index.tsx:141-149). Only the branch path stops needing it.
• Test quality. The three added desktop tests are behavior contracts, not source-reading or change-detector tests.

4. Verification I actually ran

cd <worktree of pr-103154>
HERMES_PYTHON='.../.venv/Scripts/python.exe' scripts/run_tests.sh tests/test_tui_gateway_server.py -q
→ 637 passed, 1 failed in 223.4s

The single failure is pre-existing and unrelated: test_model_options_preserves_canonical_custom_row_after_agent_init. This PR's added gateway tests passed.

Not verified: the vitest suite. npm ci failed with EBADENGINE on Node v22.16.0 / npm 11.4.1 in my environment, so use-session-actions.test.tsx was not executed here. Stating that plainly rather than implying green.

5. Staleness

git rev-list --left-right --count origin/main...pr-103154
→ 6  2

Six commits behind main. Per the repo's squash-merge rule, refresh onto main before merging so the squash cannot silently revert recent fixes.

6. Landscape — no duplication

This is the only open PR that claims to fix #103139; there is no competing patch and nothing to supersede. Related but distinct work: #98025 (durable row-ID addressing for message-level branches — overlaps use-session-actions/index.ts, methods_session.py, tests/test_tui_gateway_server.py, so expect a merge-order conflict), #80970 (competing duplicate of #98025), #80973 (post-branch history truncation), #100303 (authoritative persisted paging for transcript opening — overlaps api/sessions.ts and types/hermes.ts).

Recommendation

MERGE_WITH_FIXES. Two conditions: (1) drop omit_messages from the message-level session.branch call; (2) refresh onto current main before squash-merging. The architecture — client sends identity, backend owns transcript selection and returns bounded metadata — is the right answer to this bug class.

_{Reviewed against origin/main; PR tests executed in an isolated worktree.}

[benjaminbrumbaugh]

Addressed in c12ae62: omit_messages is now sent only for whole-session session.branch_whole; message-level session.branch keeps its existing response contract. The branch is rebased onto current main. Verified Desktop Vitest (98/98), TypeScript, ESLint, scoped Prettier, and git diff --check on the pushed head.

[benjaminbrumbaugh]

Maintainer action requested: please replace the current P2 label with P1 on this PR. The PR and linked issue document why this is a major Desktop feature failure with no equivalent user-facing workaround; this account lacks GitHub permission to mutate labels.

[benjaminbrumbaugh]

Resolved the main conflict with forward merge e476fa90b10f8191233ddac79b4a5e94c3413add, preserving gateway-side whole-session transcript omission and message-level branch responses. Final-tree verification: Python gateway tests 644 passed (one unrelated timing test passed on automatic retry and 3/3 isolated reruns); desktop hook Vitest 98 passed; desktop typecheck passed; Ruff passed; desktop lint passed with warnings only.

[OutThisLife]

Thanks @benjaminbrumbaugh. Your fix is carried forward in #121239 with your commits and authorship kept, rebased onto current main. I couldn't push to your branch because your fork's workflow files differ from main. One change on top: main now validates RPC params against typed contracts, so the copy_parent_history / omit_messages flags would have been rejected. The two new methods (session.branch_stored, session.branch_whole) now have their own contracts instead. Closing this PR in favour of that one.