diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cbdf586..b37fbcff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ ### Fixed +- **Forking a long or compacted conversation now keeps a clean turn boundary.** "Fork from here" on a large session (one big enough to have been context-compacted) could cut the copied context in the middle of a turn — or right after a tool call, before its result — leaving the new conversation with a malformed context that the model choked on. The fork now aligns its cut to a real turn boundary in the same display coordinate space `/api/session` uses (matching both the compacted and non-compacted cases), erring toward keeping slightly less rather than slicing a turn, with send-time sanitization as a backstop. Small, non-compacted forks are unchanged. Thanks @b3nw. (#5563) - **Clearing a conversation now actually clears it — history no longer comes back after a refresh (P0 data-loss).** `Clear conversation` wiped the on-screen transcript but never recorded a truncation watermark, so the messages resurrected from the agent's state database on the next load: the cleared history reappeared after a refresh, and continuing the conversation still fed the full pre-clear context to the model. Clear now routes through the same truncate-to-empty path as the truncate action (setting the watermark that blocks state-db replay), verifies the empty state actually persisted, removes the stale backup, and — for a compressed-continuation session — detaches the compression-snapshot parent link so the archived transcript can't be stitched back in either (ordinary fork links are preserved). Thanks @rodboev and the maintainer fix. (#5556, #5553, #5532) - **Transparent Stream now shows a running tool as running, not already-done.** The live anchor-scene renderer hardcoded the tool-status as "settled" even while the tool was still executing, so a tool call flashed straight to its finished state in Transparent Stream instead of showing its in-progress status. It now uses the row's actual settled state, so a running tool reads as running until it genuinely completes. Thanks @rodboev. (#5547, #5523) - **The composer no longer repopulates the tail of a message you already sent.** On mobile/slow connections, a just-sent message could momentarily reappear in the composer because a stale server-side `composer_draft` restored before the empty-draft write caught up. A short per-session suppression window now blocks that stale restore right after send — but scoped to the specific sent payload, so a genuinely new draft you (or another tab) start for the same conversation still restores immediately instead of being swallowed for the window. Drafts are also cleared on the busy send paths (queue / interrupt / steer). Thanks @ruizanthony. (#5471) diff --git a/api/session_ops.py b/api/session_ops.py index 3d47a8a7..29432a31 100644 --- a/api/session_ops.py +++ b/api/session_ops.py @@ -110,10 +110,25 @@ def truncate_context_for_display_keep( msgs = full_messages if isinstance(full_messages, list) else [] if not ctx: return [] - if len(ctx) <= len(msgs): - return ctx[:keep] if len(msgs) == 0: return [] + # Only the perfectly-parallel case (display and context row-for-row) can be + # sliced at the raw display index. When the two arrays differ in length — + # in EITHER direction — they have diverged and need alignment: + # * context LONGER than display → an injected summary/system prefix, etc. + # * context SHORTER than display → large-session context trimming dropped + # turns from the model context that the display still shows. + # The shorter-context case is the one that broke forked large sessions: the + # old ``len(ctx) <= len(msgs)`` guard short-circuited to ``ctx[:keep]``, + # slicing the shorter context at the display index (landing mid-turn, e.g. + # on an assistant tool_call whose result was past the cut). Fall through to + # the signature matcher for both divergent cases so the cut lands on a real + # turn boundary. Any residual dangling tool_use in the persisted context is + # made wire-safe on the send path (streaming: ``_sanitize_messages_for_api`` + # strips unanswered tool_calls; gateway: it forwards no tool_calls/tool rows + # at all), so we do not re-do that trimming here. + if len(ctx) == len(msgs): + return ctx[:keep] def _row_signature(row: Any) -> tuple[str, ...] | None: if not isinstance(row, dict): @@ -197,7 +212,27 @@ def truncate_context_for_display_keep( return ctx[:ambiguous_first_unkept] return ctx[:last_kept + 1] - # Final fallback preserves #5096 behavior when alignment is unreliable. + # Both boundary rows were ambiguous/unmatched (common in large sessions + # where context rows have lost their id/timestamp so the matcher can't + # disambiguate structurally-identical rows). Only for the shorter-context + # case: cut just past the LAST display row in the kept prefix that + # resolved to a context index — preferring an exact match but accepting + # an ambiguous (weak) one, mirroring how the sibling branches above fold + # ``ambiguous_matches`` into the boundary. Accepting the weak match keeps + # the forked boundary turn's own context (often exactly that ambiguous + # row) instead of dropping back to an earlier exact match. It still errs + # toward UNDER-keeping rather than slicing at the raw display index, + # which would over-keep and mis-attribute later context rows to the kept + # display turns. The context-longer case (injected summary prefix) is + # left to the #5096 fallback below, which preserves that prefix. + if len(ctx) < len(msgs): + for i in range(keep - 1, -1, -1): + resolved = matches[i] if matches[i] is not None else ambiguous_matches[i] + if resolved is not None: + return ctx[:resolved + 1] + + # Final fallback preserves #5096 behavior when alignment is unreliable + # (no display row resolved to a context index, or keep >= len(msgs)). prefix_len = max(0, len(ctx) - len(msgs)) prefix = ctx[:prefix_len] suffix = ctx[prefix_len:] diff --git a/tests/test_issue_branch_context_at_fork.py b/tests/test_issue_branch_context_at_fork.py index fae95e82..a3d5f589 100644 --- a/tests/test_issue_branch_context_at_fork.py +++ b/tests/test_issue_branch_context_at_fork.py @@ -107,6 +107,15 @@ def test_truncate_context_for_display_keep_drops_unkept_tool_rows_after_user_bou def test_truncate_context_for_display_keep_prefers_compact_summary_fallback(): + # Context is SHORTER than display (a compaction summary replaced earlier + # turns) and the display has trailing turns beyond the kept prefix. Keeping + # display [u1, a1] must map to the context that represents [u1, a1] — the + # leading compaction row PLUS both u1 and a1. The old behaviour sliced the + # shorter context at the raw display index (returning [compact, u1]), which + # dropped a1 — the assistant reply to a kept user turn — from the model + # context while the display still showed it. That mismatch is the forked- + # large-session disjointedness bug; the result must now match the sibling + # ``preserves_leading_compaction_row`` case regardless of trailing turns. msgs = [ {"role": "user", "content": "u1", "id": "u1", "timestamp": 1.0}, {"role": "assistant", "content": "a1", "id": "a1", "timestamp": 2.0}, @@ -119,7 +128,7 @@ def test_truncate_context_for_display_keep_prefers_compact_summary_fallback(): {"role": "assistant", "content": "a1", "id": "a1", "timestamp": 2.0}, ] out = truncate_context_for_display_keep(ctx, msgs, 2) - assert out == ctx[:2] + assert [row["content"] for row in out] == ["compact", "u1", "a1"] def test_truncate_context_for_display_keep_keeps_real_user_turn_when_duplicate_rows_lack_identity(): @@ -135,6 +144,99 @@ def test_truncate_context_for_display_keep_keeps_real_user_turn_when_duplicate_r out = truncate_context_for_display_keep(ctx, msgs, 1) assert [row["content"] for row in out] == ["u1", "u1"] +def test_truncate_context_shorter_than_display_aligns_to_turn_boundary(): + """Large-session fork regression. + + Mirrors the live shape that broke forks: the model context has been trimmed + so it is SHORTER than the display transcript, and its rows carry no ``id`` + (and mostly no ``timestamp``). The old ``len(ctx) <= len(msgs)`` + short-circuit returned ``ctx[:keep]`` verbatim — slicing the shorter context + at the *display* index, which lands mid-turn on an assistant ``tool_use`` + whose result is past the cut. The fix routes the shorter-context case + through the signature matcher, which cuts on the last aligned turn boundary + (a completed ``tool`` result), so the context never ends on an unmatched + ``tool_use``. (Any residual dangling ``tool_use`` on the harder no-match + paths is stripped tool-id-aware by ``_sanitize_messages_for_api`` at send.) + """ + msgs = [ + {"role": "user", "content": "u1", "timestamp": 1.0}, + {"role": "assistant", "content": "a1", "timestamp": 2.0, + "tool_calls": [{"id": "c1"}]}, + {"role": "tool", "content": "r1", "tool_call_id": "c1", "timestamp": 3.0}, + {"role": "assistant", "content": "done", "timestamp": 4.0}, # keep boundary + {"role": "user", "content": "u2", "timestamp": 5.0}, + {"role": "assistant", "content": "a2", "timestamp": 6.0, + "tool_calls": [{"id": "c2"}]}, + ] + # Context: no ids, no timestamps (matcher must fall back to signature), the + # "done" summary turn was compressed out, so ctx is shorter and diverges. + ctx = [ + {"role": "user", "content": "u1"}, + {"role": "assistant", "content": "a1", "tool_calls": [{"id": "c1"}]}, + {"role": "tool", "content": "r1", "tool_call_id": "c1"}, + {"role": "assistant", "content": "a2", "tool_calls": [{"id": "c2"}]}, + {"role": "tool", "content": "r2", "tool_call_id": "c2"}, + ] + out = truncate_context_for_display_keep(ctx, msgs, 4) + contents = [row["content"] for row in out] + # Old raw ctx[:4] would be ["u1","a1","r1","a2"] — "a2" is a dangling + # tool_use. Alignment instead cuts at the last resolved turn boundary (r1). + assert contents == ["u1", "a1", "r1"] + assert not (out and out[-1].get("role") == "assistant" and out[-1].get("tool_calls")) + + +def test_shorter_context_ambiguous_boundary_keeps_forked_turn_via_weak_match(): + """Shorter context, forked boundary turn matches only ambiguously. + + The kept boundary turn (a1) has no id/timestamp and appears twice in the + shorter context, so the matcher records it in ambiguous_matches (not + matches). Both boundary rows are then unmatched, so the shorter-context + fallback loop runs. It must accept the weak (ambiguous) match for the + boundary turn and KEEP a1's context, rather than dropping back to the + earlier exact match (u1) and losing the very turn the user forked at. + """ + msgs = [ + {"role": "user", "content": "u1", "id": "u1"}, + {"role": "assistant", "content": "a1"}, # keep boundary, no id (ambiguous) + {"role": "user", "content": "u2"}, + {"role": "assistant", "content": "a2"}, + ] + # Shorter context (3 < 4); a1 appears twice with no id → ambiguous match. + ctx = [ + {"role": "user", "content": "u1", "id": "u1"}, + {"role": "assistant", "content": "a1"}, + {"role": "assistant", "content": "a1"}, + ] + out = truncate_context_for_display_keep(ctx, msgs, 2) + # Must include the forked boundary turn a1 (weak match at ctx[1]), + # not stop at the earlier exact match u1. + assert [row["content"] for row in out] == ["u1", "a1"] + + +def test_shorter_context_zero_match_falls_back_to_best_effort_prefix(): + """Shorter context where NO kept-prefix row aligns (unalignable last resort). + + When the kept display prefix lies entirely inside a summarized region whose + rows share no signature with the display, the matcher resolves nothing and + neither the sibling branches nor the ambiguous-aware fallback loop fire. + Control reaches the #5096 last-resort fallback, which returns a best-effort + prefix of the (shorter) context. Alignment is genuinely impossible here; + wire-safety of any dangling tool_use is handled at send time. This test + pins the documented last-resort behavior so it is not changed unknowingly. + """ + msgs = [ + {"role": "user", "content": "u1", "id": "u1"}, + {"role": "assistant", "content": "a1", "id": "a1"}, + {"role": "user", "content": "u2", "id": "u2"}, + ] + ctx = [ + {"role": "user", "content": "summary-1"}, + {"role": "assistant", "content": "summary-2"}, + ] + out = truncate_context_for_display_keep(ctx, msgs, 2) + assert [row["content"] for row in out] == ["summary-1", "summary-2"] + + def test_truncate_context_for_display_keep_keeps_tool_tail_before_ambiguous_unkept_anchor(): msgs = [ {"role": "user", "content": "u1", "id": "u1"},