Skip to content

fix(session): align forked context to display for large (compacted) s… - #5563

Closed
b3nw wants to merge 1 commit into
nesquena:masterfrom
b3nw:fix/fork-context-alignment
Closed

b3nw wants to merge 1 commit into
nesquena:masterfrom
b3nw:fix/fork-context-alignment

Conversation

@b3nw

@b3nw b3nw commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Thinking Path

  • Hermes WebUI aims for near 1:1 parity with the Hermes CLI, including forking a conversation from any point ("fork from here").
  • A session keeps two parallel arrays: the visible messages transcript and the context_messages the model actually receives. In large sessions the latter is trimmed/compacted, so it is shorter than and structurally divergent from the display.
  • Forking copies a display-indexed prefix of both and asks truncate_context_for_display_keep to cut the model context at the matching point so the two stay aligned.
  • The bug: when context_messages was shorter than messages (every trimmed large session), the function short-circuited on len(ctx) <= len(msgs) and returned ctx[:keep] — slicing the shorter context at the display index. The matcher that exists precisely to align divergent arrays never ran.
  • Result: the fork's visible transcript and the model's context described different points in the conversation, and the context often ended mid-turn on a dangling assistant tool_use. This is the "broken / disjointed fork."
  • The fix routes the shorter-context case through the existing signature matcher so the cut lands on a real turn boundary — restoring the invariant in docs/rfcs/webui-run-state-consistency-contract.md that the model context must not "contradict what the user can see."

What Changed

api/session_ops.py — truncate_context_for_display_keep:

  1. Narrowed the naive-slice guard from len(ctx) <= len(msgs) to len(ctx) == len(msgs). Only perfectly-parallel arrays may be sliced at the raw display index; both divergent directions now fall through to the signature matcher.
  2. Added a shorter-context fallback for when both boundary rows are ambiguous/unmatched (the id/timestamp gap in large sessions): cut just past the last kept display row that resolved to a context index, preferring an exact match but accepting an ambiguous one (mirroring the sibling branches). Gated to len(ctx) < len(msgs) so the context-longer (fix(session): rewind/fork leaves stale model context (branch, truncate, edit index) #5096 summary-prefix) path is unchanged.
  3. Reordered the len(msgs) == 0 guard above the length check (no behavior change).

tests/test_issue_branch_context_at_fork.py:

  • Corrected test_..._prefers_compact_summary_fallback: the old assertion [compact, u1] dropped a1 (the assistant reply to a kept user turn) — see Contract Routing. New assertion [compact, u1, a1] matches the sibling preserves_leading_compaction_row case.
  • Added test_truncate_context_shorter_than_display_aligns_to_turn_boundary, test_shorter_context_ambiguous_boundary_keeps_forked_turn_via_weak_match, and test_shorter_context_zero_match_falls_back_to_best_effort_prefix.

No new dependencies; no build-step/bundler/framework changes; server logic stays in api/.

Why It Matters

Forking a large (trimmed/compacted) session produced a child whose model context was misaligned with its visible transcript and frequently malformed (dangling tool_use). Users saw the fork "continue from the wrong place" or behave erratically on the first message. Small sessions were unaffected (arrays equal length), which masked the bug. The measured live case (618 display / 600 context) now forks to the correct context[:425] (ending on a completed tool result) instead of the broken context[:440] (ending on a dangling tool_use, ~15 turns out of sync).

Verification

  • ./scripts/test.sh full suite: 11,781 passed, 133 skipped, 2 xfailed, 1 xpassed. The two failures are pre-existing / unrelated (confirmed): test_gateway_lifecycle_controls…profile_scoped_agent_cli… also fails on the base commit (sandbox resolves the agent CLI to true not echo), and test_issue4536_service_tier…roundtrip… passes in isolation on this branch (order-dependent provider-config-state flake). This diff touches no gateway/provider/config code.
  • Targeted fork/branch/truncate/context/lineage/compression/materialize files all green (re-run after rebasing onto current master).
  • Live-data replay of truncate_context_for_display_keep against the real parent session returns context[:425], ends on a completed tool result, zero dangling tool_calls (was context[:440] with a dangling call_0366).
  • Reviewed via multi-model, multi-angle adversarial code review; findings drove removal of an earlier redundant trim (see Risks) and the ambiguous-match refinement.

No browser/UI surface changed, so no before/after images apply.

Contract Routing

  • Task type: bug fix (model-context reconstruction on fork/truncate)
  • Touched areas: api/session_ops.py truncate_context_for_display_keep / truncate_session_at_keep; product-semantics tests in tests/test_issue_branch_context_at_fork.py
  • Relevant public docs: AGENTS.md, CONTRIBUTING.md, docs/CONTRACTS.md, docs/rfcs/webui-run-state-consistency-contract.md (Runtime/durability/state family — model-context reconstruction), docs/rfcs/stable-assistant-turn-anchors.md (root-cause context)
  • Scope boundaries: keeps the existing WebUI execution path; no adapter/runner/anchor implementation; no doc/contract redefinition.
  • Evidence: green full suite; alignment regression tests on the shorter-context shape; live-data confirmation.

Risks / Follow-ups

  • Shorter-context + zero-match case (kept prefix entirely inside a summarized region with no signature overlap) still returns a best-effort ctx[:keep]; alignment is genuinely impossible there. Any residual dangling tool_use is made wire-safe at send (_sanitize_messages_for_api on the streaming path; the gateway path forwards no tool_calls/tool rows). Pinned by a regression test.
  • Root cause is upstream: context rows lose their id/timestamp, forcing fuzzy signature matching that stable-assistant-turn-anchors.md explicitly warns against. Restoring stable per-message identity would make alignment exact. Tracked separately (144 affected sessions observed in a live store).
  • An earlier revision added a _drop_trailing_unsatisfied_tool_calls trim; review showed it was redundant with the send-path sanitizer, missed Anthropic-native tool_use content blocks and partial multi-tool tails, and could empty the context (making the send path fall back to the untrimmed display messages). Removed.

Release note

Fixed forking a large (compacted) session producing a child whose model context was misaligned with the visible transcript and could start mid-tool-call.

Model Used / AI Usage Disclosure

  • Provider: Anthropic
  • Model: claude-opus-4-8 (Claude Opus 4.8)
  • Mode/tools: Claude Code agentic session; multi-agent adversarial code review (parallel finder subagents + verification) drove the removal of the redundant trim and the ambiguous-match refinement.

…essions

truncate_context_for_display_keep short-circuited on `len(ctx) <= len(msgs)`
and returned ctx[:keep], slicing a trimmed (shorter) model context at the
DISPLAY index. For large/compacted sessions this misaligned the forked context
from the visible transcript and left a dangling assistant tool_use at the tail,
producing broken/disjointed forks. Small sessions were unaffected (equal-length
arrays), which masked the bug.

Narrow the naive-slice guard to `len(ctx) == len(msgs)` so both divergent
directions fall through to the signature matcher and the cut lands on a real
turn boundary. Add an ambiguous-match-aware shorter-context fallback, gated to
`len(ctx) < len(msgs)` so the context-longer (nesquena#5096 summary-prefix) path is
unchanged.

Upholds the model-context invariant in
docs/rfcs/webui-run-state-consistency-contract.md (context must not contradict
what the user can see). Live case b88a443d2f2b (618 display / 600 context) now
forks to context[:425] (completed tool result) instead of context[:440]
(dangling call_0366, ~15 turns out of sync).

Tests: correct the compact-summary-fallback assertion (it encoded the
contract-violating drop of a kept user turn's assistant reply) and add
shorter-context alignment, ambiguous-boundary, and zero-match regressions.
@greptile-apps

greptile-apps Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Fixes a session-fork misalignment that surfaced in large (compacted) sessions where context_messages is shorter than messages. The root cause was a <= guard that short-circuited to ctx[:keep] for the shorter-context case, slicing the model context at the display index — potentially landing mid-turn on a dangling tool_use.

  • api/session_ops.py: Narrows the naive-slice guard to len(ctx) == len(msgs) so only perfectly-parallel arrays bypass the signature matcher; adds a shorter-context fallback loop inside the keep < len(msgs) branch that walks backward through the kept prefix to find the last aligned turn boundary (preferring exact over ambiguous match), mirroring the existing sibling branches.
  • tests/test_issue_branch_context_at_fork.py: Corrects a stale assertion in prefers_compact_summary_fallback (previously expected the buggy ctx[:2]) and adds three targeted regression tests covering turn-boundary alignment, ambiguous-boundary weak-match acceptance, and zero-match last-resort fallback.

Confidence Score: 5/5

Safe to merge — the change is confined to a single alignment function and its tests, with no new dependencies or API surface changes.

The guard narrowing from <= to == is minimal and precisely targets the shorter-context divergence. The new fallback loop is correctly scoped inside keep < len(msgs), iterates over a bounds-safe range of the pre-built matches/ambiguous_matches arrays, and can only return ctx[:resolved+1] where resolved < len(ctx). All three documented sub-cases (turn-boundary alignment, weak-match acceptance, zero-match last-resort) are exercised by new regression tests that complement the pre-existing suite of 11,000+ passing tests. Wire-safety of any residual dangling tool_use is already handled downstream by _sanitize_messages_for_api, so the fix does not need to duplicate that logic.

No files require special attention — both changed files are self-contained and well-covered by the updated test suite.

Important Files Changed

Filename Overview
api/session_ops.py Narrows the naive-slice guard from len(ctx) <= len(msgs) to len(ctx) == len(msgs) and adds a shorter-context fallback loop inside the keep < len(msgs) branch; all return paths are reachable and index-safe.
tests/test_issue_branch_context_at_fork.py Fixes a wrong assertion in the pre-existing prefers_compact_summary_fallback test and adds three new regression tests covering the shorter-context alignment path, weak-match preference, and zero-match best-effort fallback.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[truncate_context_for_display_keep] --> B{keep <= 0?}
    B -- yes --> RET0[return empty]
    B -- no --> C{ctx empty or msgs empty?}
    C -- yes --> RET1[return empty]
    C -- no --> D{len ctx == len msgs?}
    D -- yes --> NAIVE[return ctx keep\nnew: only perfectly-parallel arrays]
    D -- no --> MATCHER[Run signature matcher\nbuild matches + ambiguous_matches]
    MATCHER --> E{keep < len msgs?}
    E -- no --> FALLBACK5096[5096 fallback\nprefix + suffix keep]
    E -- yes --> F{first_unkept resolved?}
    F -- yes --> G{last_kept is user row?}
    G -- yes --> R1[return ctx last_kept+1]
    G -- no --> R2[return ctx first_unkept]
    F -- no --> H{last_kept resolved?}
    H -- yes --> I{ambiguous first_unkept\nand last_kept not user?}
    I -- yes --> R3[return ctx ambiguous_unkept]
    I -- no --> R4[return ctx last_kept+1]
    H -- no --> J{len ctx < len msgs?\nnew fallback}
    J -- no --> FALLBACK5096
    J -- yes --> K[Walk kept prefix backward\nfor last resolved ctx index]
    K -- found --> R5[return ctx resolved+1]
    K -- none found --> FALLBACK5096
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[truncate_context_for_display_keep] --> B{keep <= 0?}
    B -- yes --> RET0[return empty]
    B -- no --> C{ctx empty or msgs empty?}
    C -- yes --> RET1[return empty]
    C -- no --> D{len ctx == len msgs?}
    D -- yes --> NAIVE[return ctx keep\nnew: only perfectly-parallel arrays]
    D -- no --> MATCHER[Run signature matcher\nbuild matches + ambiguous_matches]
    MATCHER --> E{keep < len msgs?}
    E -- no --> FALLBACK5096[5096 fallback\nprefix + suffix keep]
    E -- yes --> F{first_unkept resolved?}
    F -- yes --> G{last_kept is user row?}
    G -- yes --> R1[return ctx last_kept+1]
    G -- no --> R2[return ctx first_unkept]
    F -- no --> H{last_kept resolved?}
    H -- yes --> I{ambiguous first_unkept\nand last_kept not user?}
    I -- yes --> R3[return ctx ambiguous_unkept]
    I -- no --> R4[return ctx last_kept+1]
    H -- no --> J{len ctx < len msgs?\nnew fallback}
    J -- no --> FALLBACK5096
    J -- yes --> K[Walk kept prefix backward\nfor last resolved ctx index]
    K -- found --> R5[return ctx resolved+1]
    K -- none found --> FALLBACK5096
Loading

Reviews (1): Last reviewed commit: "fix(session): align forked context to di..." | Re-trigger Greptile

@nesquena-hermes nesquena-hermes added the size:M Medium PR (≤10 files, ≤250 LOC) label Jul 4, 2026
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

🔬 Gate certification — GREEN ✅ (fork context alignment for compacted sessions)

Certified head: sha:57fc513a (clean rebase, branch gate-rebase/5563-fork-context-align) · PR: #5563 · b3nw, fix(session): align forked context to display for large (compacted) sessions
Verdict: Full gate GREEN. A well-reasoned data-correctness fix — forking a large/compacted session no longer slices the (shorter) model-context at the display index (which landed mid-turn); both divergent cases now route through the signature matcher so the fork cut lands on a real turn boundary. Codex SAFE, suite fully green.

What I ran (rebased worktree /tmp/wt-rebase-5563)

Gate Result
Rebase onto current master ✅ git apply clean
Codex (reproduce) SAFE TO SHIP — 0 findings
Full pytest suite ✅ 11988 passed, 0 failed
PR's own test ✅ 11/11 (test_issue_branch_context_at_fork.py)

Findings

✅ Correct alignment fix: truncate_context_for_display_keep — the old len(ctx) <= len(msgs) guard short-circuited to ctx[:keep], slicing the SHORTER model-context at the DISPLAY index (landing mid-turn, e.g. on an assistant tool_call whose result was past the cut) → forked compacted sessions got misaligned context. Fix narrows the fast-path to len(ctx) == len(msgs) (perfectly parallel) and falls through to the signature matcher for BOTH divergent directions (context longer = injected summary/prefix; context shorter = large-session trim dropped turns), so the cut lands on a real turn boundary. Ambiguous/weak-match fallback for large sessions where context rows lost id/timestamp errs toward UNDER-keeping (not a raw-display-index slice), preserving #5096 behavior for unreliable alignment. Any residual dangling tool_use is made wire-safe on the send path (_sanitize_messages_for_api strips unanswered tool_calls; gateway forwards none) — Codex confirmed no dangling tool_use escapes to the API. 11 branch-context tests + full suite green.

Recommendation to the next agent

Ready to merge — use branch gate-rebase/5563-fork-context-align (sha:57fc513a), NOT the PR's stale head d5b357cd. A surgical, well-documented data-correctness fix (forked compacted sessions get context aligned to display on a real turn boundary, no mid-turn/dangling-tool slice), Codex SAFE + 11 targeted tests + full suite green (0 failures). Backend/logic — no visible surface. concept 4/5 (real fork-alignment correctness for large sessions). Author @b3nw (T2). crit=3, data-correctness.


Gate-certifier layer (warm-up → gate → release). I do not merge/tag/deploy. Rebased onto current master; verified the fast-path narrowed to len(ctx)==len(msgs) with both divergent cases routed to the signature matcher (real turn-boundary cut), ambiguous-fallback errs under-keep (#5096 preserved), dangling tool_use wire-safe on send path (Codex), Codex SAFE + 11/11 + full suite green (0 failed). Cert valid for sha:57fc513a.

@nesquena-hermes nesquena-hermes added the gate-pass Full gate passed (Codex+Opus+suite+browser); queued Tier 1 for release agent label Jul 4, 2026
nesquena-hermes added a commit that referenced this pull request Jul 4, 2026
release #5563: align forked context to display for large/compacted sessions
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Shipped in v0.51.861 — thanks @b3nw! 🎉

Your fix for fork context alignment on large/compacted sessions is live. Forking now aligns the cut to a real turn boundary in the same display coordinate space /api/session uses (handling both the compacted and non-compacted cases), so a fork no longer slices a turn in half or drops a tool result — erring toward keeping slightly less, with send-time sanitization as a backstop.

Gate summary before release:

  • Regression gate (Codex): no real finding (the one flag was a stale-base artifact from an out-of-date staging branch; rebuilt fresh on master, and this change doesn't touch the clear path).
  • Architecture/correctness (Opus): SHIP — correct, conservative, well-tested; the turn-boundary matcher handles both session shapes and never leaves an unsendable tool pair.
  • Full suite: 12000 passed; your 11 fork tests all green.

Merged via release PR #5576 with your authorship preserved. Appreciate the clean, well-tested fix.

franksong2702 pushed a commit to franksong2702/hermes-webui-fork that referenced this pull request Jul 4, 2026
…ted) sessions

Fork ("Fork from here") on a large/compacted session could cut context off a
real turn boundary — slicing mid-turn or leaving a dangling tool call, feeding
a malformed context to the model. Align the fork cut to a resolved turn boundary
via a signature matcher handling both compacted and non-compacted display
coordinate spaces; errs toward under-keeping with send-time sanitization backstop.

Co-authored-by: b3nw <b3nw@users.noreply.github.com>
franksong2702 pushed a commit to franksong2702/hermes-webui-fork that referenced this pull request Jul 4, 2026
franksong2702 pushed a commit to franksong2702/hermes-webui-fork that referenced this pull request Jul 4, 2026
…s align

WebUI keeps two parallel arrays per session: messages (display transcript) and
context_messages (what's sent to the model). In large/compacted sessions they
diverge. Forking / in-place truncation copies a prefix of both and relies on
truncate_context_for_display_keep() to translate a display keep-index into the
matching context index — an aligner that prefers a stable per-row `id`. But the
model-context rows carried neither id nor timestamp (a 989-session live scan
found 0 with any id), so alignment fell back to fragile content-signature
matching that goes ambiguous on repeated tool calls / empty assistant turns.

This closes the DATA gap (the alignment logic itself already prefers `id` and
handles the compacted case via the matcher, shipped in nesquena#5563): mint a monotonic,
session-unique integer `id` on the per-turn result rows AFTER the context restore
and BEFORE both arrays are built, so the display and model-context copies of a
logical row share the same id.

- api/streaming.py: new _assign_stable_message_ids(); _restore_reasoning_metadata
  carries `id` forward across turns (as it already does timestamp); wired into all
  three streaming commit sites (main turn, retry, self-heal).
- api/routes.py: same mint on the runs/MoA _handle_chat_sync commit path.
- api/gateway_chat.py: mints ids on the two new rows of a gateway turn.
- The `id` is stripped before the provider API call (not in _API_SAFE_MSG_KEYS),
  same treatment as timestamp — nothing new reaches the provider.

session_ops.py alignment was already delivered by nesquena#5563 (id-preferring matcher +
compacted-case fall-through), so no change is needed there now; tests updated to
assert the post-nesquena#5563 behavior (id-bearing = exact cut; id-less = errs toward
under-keeping, never the old raw-index mis-cut).

Co-authored-by: b3nw <b3nw@users.noreply.github.com>
@claw-io
claw-io deleted the fix/fork-context-alignment branch July 6, 2026 02:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gate-pass Full gate passed (Codex+Opus+suite+browser); queued Tier 1 for release agent size:M Medium PR (≤10 files, ≤250 LOC)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants