diff --git a/CHANGELOG.md b/CHANGELOG.md index d7091d8b..a1b2447b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ ### Fixed +- **Forking or truncating a large/compacted conversation no longer misaligns the model context (occasional "dangling tool-call" API rejection).** WebUI keeps two arrays per session — the visible transcript and the trimmed model context — and forking a big session translated a display cut-point into the matching context cut-point using a per-message identity. But the model-context rows carried no stable id (a 989-session scan found zero), so on large sessions full of look-alike rows the aligner went ambiguous and could cut mid-turn — leaving the fork's context ending on a tool-call whose result was dropped, which the provider then rejected. Each message now gets a stable id shared between the transcript and model-context copies (the key the aligner already prefers, shipped in the previous release), so large-session forks/truncations align exactly. The id is additive, stripped before it ever reaches the model provider, and legacy id-less sessions keep working (they just fall back to the previous matching). Thanks @b3nw. (#5564) - **The sidebar now re-sorts a reactivated conversation to the top even if the tab was in the background.** When a conversation became active again from another device (or another tab) while your current tab was unfocused or backgrounded, the sidebar didn't bump it up to "Today" / the top until you manually refreshed. The session-list refresh coalescing only preserved the refresh *reason* and dropped its options, so a `force` refresh queued behind an in-flight one silently lost its force flag and got skipped on a hidden tab (`refreshSessionList` early-returns while `document.hidden`). Refresh options are now preserved through coalescing (force / refresh-active are OR-merged), and returning focus to the sidebar forces a catch-up — so cross-device recency re-sorts land on their own. The extra refresh coalesces into the existing render-skip no-op path, so there's no flicker and no background rebuild storm. Thanks @rodboev. (#5562, #5551) - **Transparent Stream: the final reply stays a visible answer after the turn settles.** In Transparent Stream mode, the moment a turn settled its final reply folded into the collapsed worklog/activity disclosure group — the answer vanished from view until you refreshed the page (the stored transcript was always correct, so a reload fixed it). The live-anchor scene renderer was hardcoding the compact-worklog layout mode when it projected the settling turn, regardless of your active display mode, so the final answer got compact-worklog display hints and was grouped away. It now reads your actual active mode and applies per-mode display hints, so in Transparent Stream the answer settles as a visible chronological reply. Compact Worklog is unchanged (bit-identical projection). Thanks @rodboev. (#5558, #5550) - **iOS/Android PWA: streaming stays smooth on long chats, and a dropped connection recovers instead of erroring.** Two mobile reliability fixes. (1) On touch devices, the chat now skips layout and paint for off-screen message rows while a response streams (`content-visibility` scoped to `pointer:coarse`), so a long conversation no longer degrades into a freeze on iOS WKWebView — the live streaming turn (targeted by its stable `#liveAssistantTurn` id so every render mode is covered) stays fully rendered so it never blanks mid-stream and the new-message cue still works, off-screen rows keep a tiny size estimate to preserve flick-scroll momentum, and desktop find-in-page is untouched. (2) The SSE reconnect ladder is extended (4→6 backoff steps) with a last-ditch full-session poll (an 8s-watchdog-guarded `_restoreSettledSession`, with a "Restoring session…" affordance) after the retries are exhausted, so a response that finished while the connection was flapping (e.g. an iOS Tailscale/VPN reconnect) is recovered without showing an error banner. Deliberately does not touch `overflow-anchor` — that would re-open the #4856/#5338 mobile jump-to-top regression. Thanks @luperrypf. (#5541) diff --git a/api/gateway_chat.py b/api/gateway_chat.py index 9329e6f2..b0bc7aaa 100644 --- a/api/gateway_chat.py +++ b/api/gateway_chat.py @@ -843,6 +843,19 @@ def _run_gateway_chat_streaming( if saved_reasoning: assistant_msg["reasoning"] = saved_reasoning previous_context = list(getattr(s, "context_messages", None) or getattr(s, "messages", None) or []) + # Stamp stable ids on the two new rows (shared with the display merge + # below) so display and model-context copies share an id for the + # fork/truncate aligner (#context-message-stable-id). + try: + from api.streaming import _assign_stable_message_ids + + _assign_stable_message_ids( + [user_msg, assistant_msg], + previous_context, + list(getattr(s, "messages", None) or []), + ) + except Exception: + logger.debug("Failed to stamp stable ids on gateway turn rows", exc_info=True) s.context_messages = previous_context + [user_msg, assistant_msg] try: from api.streaming import _is_context_compression_marker diff --git a/api/routes.py b/api/routes.py index e1b258af..13ced3bf 100644 --- a/api/routes.py +++ b/api/routes.py @@ -20282,6 +20282,7 @@ def _handle_chat_sync(handler, body): ) from api.streaming import ( _WEBUI_PROGRESS_PROMPT, + _assign_stable_message_ids, _dedupe_replayed_context_messages, _merge_display_messages_after_agent_result, _restore_display_reasoning_metadata, @@ -20346,6 +20347,11 @@ def _handle_chat_sync(handler, body): _previous_context_messages, _result_messages, ) + # Mint ids on the shared result rows BEFORE dedupe deep-copies any + # stale-user boundary row, so both arrays share the id (#5564). + _assign_stable_message_ids( + _result_messages, _previous_messages, _previous_context_messages + ) _next_context_messages = _dedupe_replayed_context_messages( _previous_context_messages, _next_context_messages, diff --git a/api/streaming.py b/api/streaming.py index 3425f1cb..2903e80f 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -4170,6 +4170,46 @@ def _deduplicate_context_messages(messages): return deduped +def _assign_stable_message_ids(result_messages, *existing_arrays): + """Mint a stable, session-unique integer ``id`` on model-result rows lacking one. + + Both ``messages`` (display transcript) and ``context_messages`` (model-facing + history) are derived from the *same* per-turn ``result['messages']`` dicts. + Stamping the id on those shared dicts here makes each logical row carry an + identical id in both arrays, so the fork/truncate aligner + (``session_ops.truncate_context_for_display_keep``) can match rows by its + preferred ``id`` key instead of the fragile content-signature fallback that + goes blind on large sessions full of structurally-identical rows. + + Ids are monotonic within a session (max existing id + 1, seeded from the + result rows plus any ``existing_arrays`` passed for collision-avoidance). + Across sessions ids may repeat, which is safe: alignment only ever compares + rows within one session, and a fork copies both arrays together so the child + stays internally consistent. Rows whose historical id was carried forward by + ``_restore_reasoning_metadata`` keep it; only genuinely new rows are minted. + + Returns the number of rows newly stamped. Mutates ``result_messages`` in place. + """ + if not result_messages: + return 0 + seed = 0 + for arr in (result_messages, *existing_arrays): + for m in arr or []: + if isinstance(m, dict): + mid = m.get('id') + # bool is an int subclass; exclude it so a stray True/False id + # can never seed the counter. + if isinstance(mid, int) and not isinstance(mid, bool) and mid > seed: + seed = mid + stamped = 0 + for m in result_messages: + if isinstance(m, dict) and m.get('id') is None: + seed += 1 + m['id'] = seed + stamped += 1 + return stamped + + def _prune_context_tool_results_after_compression(agent, context_messages): """Run the active compressor's cheap tool-result pruning on model context. @@ -4235,6 +4275,13 @@ def _restore_reasoning_metadata(previous_messages, updated_messages): if isinstance(prev_msg, dict) and isinstance(cur_msg, dict) and _safe_projection(prev_msg) == _safe_projection(cur_msg): if prev_msg.get('role') == 'assistant' and prev_msg.get('reasoning') and not cur_msg.get('reasoning'): cur_msg['reasoning'] = prev_msg['reasoning'] + # Carry the stable per-message id (#context-message-stable-id) forward + # the same way timestamp is carried. The agent rebuilds result rows + # without our id every turn; without this, historical context rows + # would be re-minted a fresh id each turn and drift out of alignment + # with their display counterpart. + if prev_msg.get('id') is not None and cur_msg.get('id') is None: + cur_msg['id'] = prev_msg['id'] if prev_msg.get('timestamp') and not cur_msg.get('timestamp'): cur_msg['timestamp'] = prev_msg['timestamp'] elif prev_msg.get('_ts') and not cur_msg.get('_ts') and not cur_msg.get('timestamp'): @@ -5217,6 +5264,18 @@ def _merge_display_messages_after_agent_result(previous_display, previous_contex # before the agent runs. When the agent returns that same user turn # in result_messages, keep the durable checkpoint and append only # the assistant/tool delta. + # The eager checkpoint was written before _assign_stable_message_ids + # stamped the result rows, so it has no `id` while the context copy + # does — which would silently defeat id-based fork/truncate alignment + # for eager-mode users. Carry the minted id onto the kept checkpoint + # so display and context share it (#5564). + if ( + isinstance(msg, dict) + and msg.get('id') is not None + and isinstance(merged[-1], dict) + and merged[-1].get('id') is None + ): + merged[-1]['id'] = msg['id'] continue if ( key is not None @@ -8181,6 +8240,16 @@ def _run_agent_streaming( _previous_context_messages, _result_messages, ) + # Stamp stable ids on the shared result rows AFTER the context + # restore (so carried-forward ids survive) and BEFORE the + # dedupe/merge build both arrays — including before + # _dedupe_replayed_context_messages deep-copies any + # stale-user repaired boundary row — so display and + # model-context copies of each row share an id for the + # fork/truncate aligner. + _assign_stable_message_ids( + _result_messages, _previous_messages, _previous_context_messages + ) _next_context_messages = _dedupe_replayed_context_messages( _previous_context_messages, _next_context_messages, @@ -8491,6 +8560,12 @@ def _run_agent_streaming( _previous_context_messages, _result_messages, ) + # Mint ids on the shared result rows BEFORE dedupe + # deep-copies any stale-user boundary row, so both + # arrays share the id (#5564). + _assign_stable_message_ids( + _result_messages, _previous_messages, _previous_context_messages + ) _next_context_messages = _dedupe_replayed_context_messages( _previous_context_messages, _next_context_messages, @@ -9547,6 +9622,12 @@ def _run_agent_streaming( _next_context_messages = _restore_reasoning_metadata( _previous_context_messages, _result_messages, ) + # Mint ids on the shared result rows BEFORE dedupe + # deep-copies any stale-user boundary row, so both + # arrays share the id (#5564). + _assign_stable_message_ids( + _result_messages, _previous_messages, _previous_context_messages + ) _next_context_messages = _dedupe_replayed_context_messages( _previous_context_messages, _next_context_messages, diff --git a/tests/test_context_message_stable_ids.py b/tests/test_context_message_stable_ids.py new file mode 100644 index 00000000..79275ae8 --- /dev/null +++ b/tests/test_context_message_stable_ids.py @@ -0,0 +1,183 @@ +"""Stable per-message ids keep the fork/truncate aligner from going blind. + +Regression coverage for the systemic gap documented in +local-ops/docs/bug-context-messages-lost-timestamps.md: ``context_messages`` +rows carried neither a stable ``id`` nor a ``timestamp``, so +``truncate_context_for_display_keep`` degraded to fragile content-signature +matching and mis-cut large sessions on fork/truncate. + +The fix mints a stable, session-unique integer ``id`` on the per-turn result +rows that both ``messages`` (display) and ``context_messages`` (model) derive +from, and carries it forward across turns. These tests prove: + +1. the minting helper is monotonic and non-destructive, +2. ``_restore_reasoning_metadata`` carries ids forward like timestamps, +3. the real turn transforms give a logical row the SAME id in both arrays, +4. the aligner resolves an otherwise-ambiguous boundary via id alone. +""" + +from api.streaming import ( + _assign_stable_message_ids, + _restore_reasoning_metadata, + _deduplicate_context_messages, + _dedupe_replayed_context_messages, + _merge_display_messages_after_agent_result, +) +from api.session_ops import truncate_context_for_display_keep + + +def test_assign_stable_ids_mints_monotonic_ints(): + rows = [ + {"role": "user", "content": "u1"}, + {"role": "assistant", "content": "a1"}, + {"role": "tool", "content": "t1", "tool_call_id": "c1"}, + ] + stamped = _assign_stable_message_ids(rows) + assert stamped == 3 + assert [r["id"] for r in rows] == [1, 2, 3] + + +def test_assign_stable_ids_seeds_from_existing_max_and_skips_present(): + prev = [{"role": "user", "content": "old", "id": 41}] + rows = [ + {"role": "assistant", "content": "keep", "id": 7}, # already has id -> untouched + {"role": "tool", "content": "new", "tool_call_id": "c"}, + ] + stamped = _assign_stable_message_ids(rows, prev) + assert stamped == 1 + # kept its own id; new row minted above the global max (41), not above 7 + assert rows[0]["id"] == 7 + assert rows[1]["id"] == 42 + + +def test_assign_stable_ids_ignores_bool_and_empty(): + assert _assign_stable_message_ids([]) == 0 + assert _assign_stable_message_ids(None) == 0 + rows = [{"role": "user", "content": "u", "id": True}] # bool must not seed + other = [{"role": "assistant", "content": "a"}] + _assign_stable_message_ids(other, rows) + assert other[0]["id"] == 1 # seed ignored the bool, started at 1 + + +def test_restore_reasoning_metadata_carries_id_forward(): + previous = [ + {"role": "user", "content": "u1", "id": 10, "timestamp": 1}, + {"role": "assistant", "content": "a1", "id": 11, "timestamp": 2}, + ] + # Agent rebuilds the same history without our id (and without timestamp). + updated = [ + {"role": "user", "content": "u1"}, + {"role": "assistant", "content": "a1"}, + {"role": "user", "content": "u2"}, # genuinely new tail row + ] + out = _restore_reasoning_metadata(previous, updated) + assert out[0]["id"] == 10 + assert out[1]["id"] == 11 + assert "id" not in out[2] # new row left for the minting pass + + +def test_both_arrays_share_id_for_same_logical_row(): + """The real turn transforms must give a row the SAME id in both arrays.""" + prev_display = [] + prev_context = [] + # The two arrays are built from the SAME result dicts (as in streaming.py). + result_messages = [ + {"role": "user", "content": "hello"}, + {"role": "assistant", "content": "", "tool_calls": [{"id": "c1", "name": "x"}]}, + {"role": "tool", "content": "out", "tool_call_id": "c1"}, + {"role": "assistant", "content": "done"}, + ] + + # --- context path (mirrors streaming.py commit site) --- + next_ctx = _restore_reasoning_metadata(prev_context, result_messages) + next_ctx = _dedupe_replayed_context_messages(prev_context, next_ctx, "hello") + _assign_stable_message_ids(result_messages, prev_display, prev_context) + context_out = _deduplicate_context_messages(next_ctx) + + # --- display path (mirrors streaming.py commit site) --- + display_out = _merge_display_messages_after_agent_result( + prev_display, + prev_context, + _restore_reasoning_metadata(prev_display, result_messages), + "hello", + ) + + # every context row must have an id, and the matching display row (by + # role+content) must carry the identical id. + assert all(isinstance(m.get("id"), int) for m in context_out) + disp_by_key = {(m["role"], str(m.get("content"))): m.get("id") for m in display_out} + for m in context_out: + assert disp_by_key.get((m["role"], str(m.get("content")))) == m["id"] + + +def _dup_ambiguous_shapes(with_ids): + """Large-session shape: display longer than context, full of structurally + identical assistant(tool_call)/tool(result) pairs so content signatures + collide. Context is the compressed/shorter model view. + """ + dup_call = [{"id": "c", "name": "run"}] + + def row(role, content, **kw): + r = {"role": role, "content": content} + r.update(kw) + return r + + def maybe_id(r, i): + if with_ids and i is not None: + r["id"] = i + return r + + # 8 display rows, three identical assistant/tool pairs then a trailing user. + display = [ + maybe_id(row("user", "u1"), 1), + maybe_id(row("assistant", "", tool_calls=dup_call), 2), + maybe_id(row("tool", "same", tool_call_id="c"), 3), + maybe_id(row("assistant", "", tool_calls=dup_call), 4), + maybe_id(row("tool", "same", tool_call_id="c"), 5), + maybe_id(row("assistant", "", tool_calls=dup_call), 6), + maybe_id(row("tool", "same", tool_call_id="c"), 7), + maybe_id(row("user", "u2"), 8), + ] + # 7 context rows (shorter -> len(ctx) < len(msgs), the documented signature): + # a leading compaction summary with no display match, then display ids 1..6. + context = [ + row("assistant", "[compaction summary]"), # leading, unmatched, no id + maybe_id(row("user", "u1"), 1), + maybe_id(row("assistant", "", tool_calls=dup_call), 2), + maybe_id(row("tool", "same", tool_call_id="c"), 3), + maybe_id(row("assistant", "", tool_calls=dup_call), 4), + maybe_id(row("tool", "same", tool_call_id="c"), 5), + maybe_id(row("assistant", "", tool_calls=dup_call), 6), + ] + return display, context + + +def test_aligner_resolves_ambiguous_boundary_via_id(): + """With shared ids the aligner cuts at the exact boundary even though the + duplicate rows make content signatures ambiguous.""" + display, context = _dup_ambiguous_shapes(with_ids=True) + # Keep the first 4 display rows (u1, a, t, a=id4). Correct cut keeps the + # leading summary + ids 1..4 (5 rows), stopping before id5. + out = truncate_context_for_display_keep(context, display, keep=4) + assert len(out) == 5 + assert [m.get("id") for m in out] == [None, 1, 2, 3, 4] + + +def test_aligner_without_ids_underkeeps_baseline(): + """Same shape with ids/timestamps stripped (pre-fix live data): the duplicate + rows defeat signature matching, so the aligner cannot resolve the exact + boundary and errs toward UNDER-keeping (the #5563 matcher no longer raw-slices + the compacted case). It keeps fewer rows than the id-aware path, which is why + the shared stable id is the durable fix — with ids the same shape resolves to + the exact 5-row cut (see test_aligner_resolves_ambiguous_boundary_via_id).""" + display, context = _dup_ambiguous_shapes(with_ids=False) + out = truncate_context_for_display_keep(context, display, keep=4) + # Without ids the matcher can't disambiguate the identical assistant/tool + # pairs, so it under-keeps (3 rows) rather than reaching the exact id-aware + # boundary (5 rows). It never over-keeps or slices mid-turn at the raw index. + assert len(out) < 5 + assert [m.get("content") for m in out] == [ + "[compaction summary]", + "u1", + "", + ] diff --git a/tests/test_issue1217_transcript_compaction.py b/tests/test_issue1217_transcript_compaction.py index 93fb058f..78953ec6 100644 --- a/tests/test_issue1217_transcript_compaction.py +++ b/tests/test_issue1217_transcript_compaction.py @@ -793,11 +793,30 @@ def test_handle_chat_sync_writeback_dedupes_full_context_replay(tmp_path, monkey assert handler.status == 200 reloaded = Session.load(session.session_id) - assert reloaded.context_messages == previous_context + [ + + # Stable per-message ids (#context-message-stable-id) are now stamped on + # context rows; compare on the semantic fields so the dedup invariant is + # still what is under test. Build ``expected`` from fresh literals: the id + # stamping mutates the shared result dicts in place, and this test reuses the + # same objects for its result and its previous_context. + def _no_id(rows): + return [{k: v for k, v in m.items() if k != "id"} for m in rows] + + expected = [ + {"role": "assistant", "content": "cron banner"}, + {"role": "user", "content": "[Session Arc Summary (d1, node 39)]\n" + "old context\n" * 400}, + {"role": "assistant", "content": "previous answer"}, {"role": "user", "content": "simple follow-up"}, {"role": "assistant", "content": "short answer"}, ] - assert reloaded.context_messages.count(previous_context[0]) == 1 + assert _no_id(reloaded.context_messages) == expected + assert _no_id(reloaded.context_messages).count(expected[0]) == 1 + # The new turn's rows carry unique stable ids. (This session preloads + # id-less legacy rows, which stay id-less until they age out of context; + # brand-new sessions get full id coverage since previous_context is empty.) + new_ids = [m.get("id") for m in reloaded.context_messages[-2:]] + assert all(isinstance(i, int) for i in new_ids) + assert len(set(new_ids)) == 2 def test_session_context_falls_back_to_display_messages_for_legacy_sessions(tmp_path):