Skip to content

fix(tui): preserve chat roles after compaction - #6286

Merged
Hmbown merged 5 commits into
Hmbown:mainfrom
zhuowp:fix/compaction-checkpoint-chat-roles
Sep 17, 2026
Merged

Hmbown merged 5 commits into
Hmbown:mainfrom
zhuowp:fix/compaction-checkpoint-chat-roles

Conversation

@zhuowp

@zhuowp zhuowp commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Summary

When compaction follows a tool result, the saved summary is a user message after the tool exchange. Chat Completions then sends user → assistant(tool_calls) → tool → user, which strict paired chat templates reject. Move only generated summaries before the retained prompt on the outbound wire, place Agent topology checkpoints before the active tool chain, and keep tool-call IDs and persisted history intact. On session restore, retain the position of a generated checkpoint so later turns stay later.

A second compaction must not mistake the runtime-owned topology checkpoint for a real user turn. Keep the actual user prompt and tool pair inside the protected round even when the prompt exceeds the minimum verbatim retention budget; runtime text cannot stand in for the user's prompt in the coverage check.

Testing

  • Focused TUI lib tests: 14 Chat wire, 16 runtime handoff, 13 last-round compaction, 4 retained-user, 1 checkpoint restore, and 1 SyncSession restore passed (49 total).
  • cargo fmt --all -- --check and git diff --check passed.
  • Local Rust 1.96 compilation required a temporary crate-level allow for unrelated existing unfulfilled_lint_expectations; the allow was removed before both commits. Full workspace checks and a live provider call were not run.

No-Issue: reproduced downstream bug on upstream main; this PR provides the report and fix.

Signed-off-by: zhuowp <zwp-1989@qq.com>
@github-actions

Copy link
Copy Markdown
Contributor

Thanks @zhuowp for taking the time to contribute.

This repository is observing a maintainer-managed PR intake gate in dry-run mode, so this pull request is staying open. This note helps maintainers prepare the allowlist before any enforcement is considered.

Please read CONTRIBUTING.md for the expected contribution shape. A maintainer can grant recurring PR access by commenting /lgtm on a pull request.

@zhuowp

zhuowp commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

Independent review, round 1: changes requested.

P1 — crates/tui/src/compaction/last_round.rs still treats the new runtime-owned topology checkpoint as a plain user turn. After the first compaction, a history shaped as [user(question), topology, assistant(tool_use), user(tool_result), summary] makes last_round_start anchor on the topology instead of the real question. The configured verbatim retention budget is clamped to at least 2,000 tokens; a question longer than that budget can therefore be truncated on the second compaction while the survival check passes. Exclude runtime-owned user messages from last-round prompt/coverage selection and add a second-compaction regression using the minimum effective budget and a sufficiently long question. A direct zero-budget unit case may also isolate the algorithm, but is not a production configuration.

No other blocking issue found in this round. The new wire ordering, provenance check, and restore-position behavior otherwise match the intended fix.

Signed-off-by: zhuowp <zwp-1989@qq.com>
@zhuowp

zhuowp commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

Independent review, round 2: PASS.

Reviewed the complete diff against upstream main, including the follow-up for the long-user-prompt recompaction case. The protected round now anchors on the real user prompt, runtime-owned topology cannot consume verbatim retention budget or satisfy user-prompt coverage, and mixed genuine user text/tool results remain eligible. Independently ran the focused long-prompt, runtime-text coverage, and existing retention tests; each passed.

No remaining blocking code-review findings. A live strict-provider call and migration of pre-fix sessions remain outside this PR's verification and scope.

@zhuowp
zhuowp marked this pull request as ready for review September 16, 2026 14:15
@zhuowp
zhuowp requested a review from Hmbown as a code owner September 16, 2026 14:15
@Hmbown
Hmbown merged commit 21282f1 into Hmbown:main Sep 17, 2026
20 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants