Skip to content

fix(state): stop branch replay at fork snapshot - #77396

Open
QuarkAssistant wants to merge 4 commits into
NousResearch:mainfrom
QuarkAssistant:fix/desktop-branch-resume-boundary-77375
Open

QuarkAssistant wants to merge 4 commits into
NousResearch:mainfrom
QuarkAssistant:fix/desktop-branch-resume-boundary-77375

Conversation

@QuarkAssistant

@QuarkAssistant QuarkAssistant commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • walk transcript replay through compression continuations, but stop at the nearest explicit branch edge
  • keep presentation lineage unchanged for branch trees and conversation-root resolution
  • make resume counting and safety guards use the same branch-aware lineage as the actual resume read

Why this remains needed after #86775

#86775 fixed direct branch resumes on main, but its tip-only branch check still walks through the original parent after a branch is compressed. The result is a duplicated fork snapshot plus post-fork parent turns in the resumed continuation.

This PR detects a branch boundary on each traversed edge. Inherited _branched_from provenance on compression continuations is not treated as a new fork unless it names that row’s immediate parent.

Regression coverage

Real SessionDB tests cover:

  • direct branch resume excludes parent turns written after the fork
  • nested branch resume excludes parent turns written after the nested fork
  • ordinary compression continuation still replays ancestors
  • compression after a branch replays from the branch boundary, including inherited branch provenance
  • resume counts and size guards match the branch-aware transcript actually materialized

TDD evidence

  • current main + the regression file: 3 passed / 1 failed at test_compression_after_branch_replays_from_branch_boundary
  • inherited-marker variant before the edge comparison: 3 passed / 1 failed
  • resume-count regression before guard alignment: 4 passed / 1 failed (8 != 6)
  • final focused file: 5 passed

Validation

  • scripts/run_tests.sh tests/hermes_state tests/test_hermes_state.py -q → 375 passed, 0 failed, 2 skipped
  • uv run ruff check hermes_state.py tests/hermes_state/test_branch_replay_lineage.py → clean
  • python -m py_compile, git diff --check, and ancestry check against current origin/main → clean

Closes #77375

@QuarkAssistant
QuarkAssistant force-pushed the fix/desktop-branch-resume-boundary-77375 branch 3 times, most recently from 19a5fd4 to 029e362 Compare August 3, 2026 06:30
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 3, 2026
@QuarkAssistant
QuarkAssistant marked this pull request as ready for review August 3, 2026 06:40
@QuarkAssistant
QuarkAssistant force-pushed the fix/desktop-branch-resume-boundary-77375 branch from 029e362 to ecac0aa Compare August 5, 2026 02:13
@QuarkAssistant
QuarkAssistant force-pushed the fix/desktop-branch-resume-boundary-77375 branch from ecac0aa to d13ee0b Compare August 15, 2026 10:18
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(state): stop branch replay at fork snapshot

  1. hermes_state.py:10479-10500_session_replay_lineage_root_to_tip caps the walk at 100 iterations (parity with _session_lineage_root_to_tip, which also caps at 100). The new call sites change the consequences of that cap: get_resume_message_count and assert_resume_safe now count the capped lineage, so a lineage deeper than ~100 compression continuations would undercount and assert_resume_safe could green-light a resume that then exceeds the message cap at materialization time. Pre-existing cap, but now it gates a safety check — worth a comment or a shared constant so the limit is visible in both functions.

  2. The boundary condition branch_parent == parent_id (line 10492-10494) depends on _branched_from always naming the immediate parent edge. The compression-inheritance case is well covered by tests, but if any branch-creation path ever writes _branched_from with a non-parent value (or a legacy row records it differently), the boundary is not recognized and live parent turns leak into the branch — the exact bug class this PR fixes. Consider asserting the invariant (marker == parent) at branch creation time, or documenting that the marker must name the fork edge.

  3. get_conversation_root still uses the old _session_lineage_root_to_tip full walk, which crosses fork edges — so a branch session's "conversation root" resolves into the live parent chain while replay stops at the fork. If any UI derives breadcrumbs/display lineage from get_conversation_root, it may show the parent turns that resume no longer includes. Confirm this split (presentation lineage vs replay lineage) is intended, or align the two functions.

Verified: _is_explicit_branch_session has no remaining callers after removal, and the new tests (nested branches, compression-after-branch) cover the tricky boundary cases well.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Desktop branch cold resume includes parent turns written after the fork

3 participants