fix(cli): map rewind turns after compression#4242
Conversation
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Fix correctly diagnoses #4046 and the refactor is clean. Two blocking concerns and a few nits inline.
Out-of-band concern (not inline because it's in a different file): packages/cli/src/acp-integration/session/Session.ts:344 (#computeApiTruncationIndexForUserTurn) is a near-duplicate of the pre-fix logic and still has the same compression bug for the ACP rewind path. Consider consolidating Session.ts onto computeApiTruncationIndex (preferred — removes the duplication), applying the same fix there, or filing a follow-up so #4046 doesn't half-close.
Test gaps: the three new tests cover the happy path but miss (a) compression + startup-context-pair together (startIndex = 2 then summary at 2/3), (b) targetOrdinal === 1 when the first turn was compressed away (should return -1; behavior is correct but no regression net), and (c) a tail containing tool-call entries (functionCall + functionResponse) between the bridge and the next user prompt.
| const COMPRESSION_SUMMARY_MODEL_ACK = | ||
| 'Got it. Thanks for the additional context!'; | ||
| const COMPRESSION_CONTINUATION_BRIDGE = | ||
| 'Continue with the prior task using the context above.'; |
There was a problem hiding this comment.
Magic-string coupling between cli and core (blocking). These two literals are also hardcoded in packages/core/src/services/chatCompressionService.ts:420,432. If either side edits the text (typo fix, i18n, model-prompt tuning), this mapper silently regresses to the original bug — and CI will pass because the unit tests use the same literals.
Compare to STARTUP_CONTEXT_MODEL_ACK, which is exported from core and imported on line 9. Please export COMPRESSION_SUMMARY_MODEL_ACK and COMPRESSION_CONTINUATION_BRIDGE from chatCompressionService.ts (or a shared constants module) and import them here. Bonus: a core-side test that asserts the emitted Content uses those exact constants would make future renames break loudly.
| if ( | ||
| skipContinuationBridge && | ||
| hasTextPart(content, COMPRESSION_CONTINUATION_BRIDGE) |
There was a problem hiding this comment.
Bridge detection by exact text match is over-broad. If a user happens to type exactly Continue with the prior task using the context above., that genuine prompt is silently dropped from the tail mapping and shifts every subsequent ordinal — so rewinding to any post-bridge turn lands one slot off.
Unlikely in practice but easy to harden: mark the synthetic bridge with something that can never collide with user input — either a sentinel flag on the Content (e.g., a private extension property), or a zero-width character / unique marker in the text — and match on that instead of the human-readable string.
| apiHistory[startIndex + 1]?.role === 'model' && | ||
| hasTextPart(apiHistory[startIndex + 1], COMPRESSION_SUMMARY_MODEL_ACK) |
There was a problem hiding this comment.
nit: the apiHistory[startIndex + 1]?.role === 'model' check is redundant — hasTextPart on the next line already short-circuits to false if the Content is missing/has no parts/has no matching text part. The role check only guards against a user-role Content with a text part equal to the ack literal, which is a separate (and unlikely) coupling concern. Consider dropping it, or roll the role check into a hasModelTextPart helper for symmetry with isUserTextContent.
| return -1; | ||
| } | ||
|
|
||
| return apiTailUserIndices[targetOrdinal - compressedTurnCount - 1] ?? -1; |
There was a problem hiding this comment.
nit: the ?? -1 is unreachable by construction. targetOrdinal ≤ totalRealUserTurns (the ordinal can only be set while iterating real user turns), and apiTailUserIndices.length = totalRealUserTurns - compressedTurnCount, so targetOrdinal - compressedTurnCount - 1 is always in [0, apiTailUserIndices.length - 1] once we've passed the targetOrdinal <= compressedTurnCount guard on line 176.
Not worth blocking on — defensive ?? -1 is fine — but if you prefer it to read as intentional, a one-liner comment ("defensive: shouldn't fire given the guard above") would help future readers.
| }); | ||
|
|
||
| it('keeps compressed turns unreachable after a compression summary pair', () => { | ||
| const ui: HistoryItem[] = [ |
There was a problem hiding this comment.
Suggest adding a sibling test for the targetOrdinal === 1 + compression case (rewind to the very first UI turn after it was absorbed into the summary). The current code returns -1 correctly because the compression branch on line 161 runs before the targetOrdinal === 1 early-return on line 183, but that ordering is load-bearing and easy to flip during a future refactor without any test catching it.
There was a problem hiding this comment.
Covered in the current test file by keeps the first UI turn unreachable when compression absorbed it, which asserts the targetOrdinal === 1 compressed case returns -1.
| @@ -55,6 +55,8 @@ import { | |||
| getSubagentSystemReminder, | |||
There was a problem hiding this comment.
[Suggestion] ACP 压缩路径使用 this.turn 推导 totalUserTurns，但 this.turn 可能偏离实际的 user-text entry 数量——slash-command prompt 会递增 turn 但不产生 user-text 条目，cron prompt 会产生条目但不递增 turn。相比之下 historyMapping.ts 直接遍历 UI history 计数更为可靠。
— DeepSeek/deepseek-v4-pro via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
/review 补充发现
PR 正确诊断了 #4046，核心思路无误。所有测试通过（155 tests），无类型错误。
以下是在 @wenshao 已有 5 条 inline comment 之外的补充发现：
Suggestion: Session.ts 使用 this.turn 计算 compressedTurnCount，可能偏离实际值
Session.#computeApiTruncationIndexForUserTurn 用 Math.max(this.turn, targetTurnIndex + 1) 作为总轮次数。但 this.turn 在 slash-command prompt（递增 turn 但不产生 user-text 条目）和 cron prompt（产生条目但不递增 turn）场景下会偏离实际值。UI 路径 computeApiTruncationIndex 直接遍历 UI history 计数，更为可靠。建议从 API history 直接统计 user-text entry 总数。
Suggestion: 四个辅助函数在 Session.ts 和 historyMapping.ts 中重复实现
hasTextPart、hasModelTextPart、hasCompressionSummaryPair、getApiUserTextIndices 在两处逻辑完全一致。建议提取到共享模块，避免压缩格式变更时遗漏同步。
Suggestion: 错误信息缺少诊断上下文
rewindToTurn 抛出的 "Cannot rewind to the requested turn. It may have been compressed or does not exist." 不含 targetTurnIndex 或 compressedTurnCount，调试不便。
Suggestion: 整个 rewind 流水线零日志
rewindToTurn、computeApiTruncationIndex 等关键函数中无任何日志，缺少操作审计踪迹。
— DeepSeek/deepseek-v4-pro via Qwen Code /review
Jerry2003826
force-pushed
the
codex/fix-rewind-compressed-tail
branch
2 times, most recently
from
fcb706a to
a956001
Compare
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
补充发现（在 @wenshao 已有 6 条 inline comment 之外）：
[Suggestion] 代码重复：Session.ts 与 historyMapping.ts — hasTextPart、hasModelTextPart、hasCompressionSummaryPair、getApiUserTextIndices 等辅助函数在两处近乎相同的实现。未来压缩格式变更需双文件同步维护。建议提取到公共工具模块。
[Suggestion] 缺少测试覆盖：targetOrdinal < 0 guard 分支 — getUiTurnOrdinals 新增的提前返回分支（historyMapping.test.ts）无测试用例覆盖。
[Suggestion] #recordModelFacingUserTurn 仅通过 mock 测试 — 计数器递增从未通过真实的 send 路径验证（Session.test.ts），如果调用点被意外移除无法检测。
— DeepSeek/deepseek-v4-pro via Qwen Code /review
| true, | ||
| ); | ||
| const totalUserTurns = Math.max( | ||
| this.modelFacingUserTurnCount, |
There was a problem hiding this comment.
[Critical] modelFacingUserTurnCount 在 rewind 后不回退，导致 rewind→send→rewind 循环后可达轮次变为不可达。
#computeApiTruncationIndexForUserTurn 用 Math.max(this.modelFacingUserTurnCount, targetTurnIndex + 1) 计算 compressedTurnCount，但 modelFacingUserTurnCount 只增不减。rewind 到早期轮次后发送新消息会使计数器膨胀，后续 rewind 时被错误地判定为「unreachable turn」。
| this.modelFacingUserTurnCount, | |
| // 截断后重置计数器为实际保留的轮次数 | |
| const truncatedHistory = this.#chat.getHistory(); | |
| this.modelFacingUserTurnCount = this.#getApiUserTextIndices( | |
| truncatedHistory, | |
| this.#hasStartupContext(truncatedHistory) ? 2 : 0, | |
| false, | |
| ).length; | |
| const totalUserTurns = Math.max( | |
| this.modelFacingUserTurnCount, | |
| targetTurnIndex + 1, | |
| ); |
— DeepSeek/deepseek-v4-pro via Qwen Code /review
| return false; | ||
| } | ||
| if (record.subtype === 'cron') { | ||
| return true; |
There was a problem hiding this comment.
[Suggestion] isModelFacingUserPromptRecord 未排除 ? 前缀命令，导致 modelFacingUserTurnCount 与 UI 轮次计数背离。
isRealUserTurn（historyMapping.ts）排除 ?-prefixed 命令，但此处不排除。当用户输入 ?help 时 ACP 路径的计数器会额外 +1，压缩轮次计算出现偏差。
| return true; | |
| if (record.subtype === 'cron') { | |
| return true; | |
| } | |
| const fullText = textParts.join(' '); | |
| if (fullText.startsWith('?')) { | |
| return false; | |
| } | |
| return !isSlashCommand(fullText); |
— DeepSeek/deepseek-v4-pro via Qwen Code /review
There was a problem hiding this comment.
Already covered in the current implementation: isModelFacingUserPromptRecord now returns !fullText.startsWith('?') && !isSlashCommand(fullText), so ? commands are excluded from the resumed model-facing counter.
Jerry2003826
force-pushed
the
codex/fix-rewind-compressed-tail
branch
from
a956001 to
6c8b555
Compare
| // targetTurnIndex is zero-based; after truncating before that turn, | ||
| // exactly targetTurnIndex model-facing user turns remain. | ||
| this.modelFacingUserTurnCount = targetTurnIndex; | ||
|
|
There was a problem hiding this comment.
[Critical] restoreHistory() restores only the API Content[], but this rewind path also mutates modelFacingUserTurnCount. ACP's session/restoreSessionHistory returns the saved history after an edit/cancel flow, but the counter remains at the rewound value. After compression, later rewind mapping uses that stale counter to derive compressedTurnCount, so a restored session can incorrectly make absorbed turns reachable or reject valid tail turns.
Please snapshot and restore the counter together with the chat history, e.g. return an object like { history, modelFacingUserTurnCount } from captureHistorySnapshot() and have restoreHistory() restore both fields. Recomputing from Content[] after compression is not enough because the absorbed turn count is not recoverable from the summary pair.
— gpt-5.5 via Qwen Code /review
There was a problem hiding this comment.
Addressed. restoreHistory() restores modelFacingUserTurnCount from HistorySnapshot, and 4188d3dce adds an assertion that the restored counter is used before the next compressed rewind.
|
|
||
| let nextMessage: Content | null = { role: 'user', parts }; | ||
| this.#recordModelFacingUserTurn(nextMessage); | ||
|
|
There was a problem hiding this comment.
[Critical] This increments modelFacingUserTurnCount before the prompt is actually accepted by #sendMessageStreamWithAutoCompression(). If the send is skipped by the session token limit (max_tokens), #preserveUnsentMessageHistory(..., false) drops the non-functionResponse user text from API history, but the counter has already advanced.
After compression, ACP rewind derives compressedTurnCount from this counter, so a skipped/unsent prompt can silently inflate the total turn count and later make valid tail turns unreachable or map them to the wrong API entry. Move the counter increment until after sendResult.responseStream is present, or roll it back on every skipped-send path where the user text is not preserved as model-facing API history.
— gpt-5.5 via Qwen Code /review
| function isModelFacingUserPromptRecord(record: ChatRecord): boolean { | ||
| if (record.type !== 'user') { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
[Suggestion] mid_turn_user_message records are counted as independent model-facing user turns when initializing modelFacingUserTurnCount from resumed history. However, resume/API reconstruction merges these records into the previous user Content rather than treating them as standalone rewind turn boundaries.
That inflates the resumed counter relative to the API user-text entries used by compressed-tail mapping, which can make ACP rewind reject reachable tail turns after a resume. Please exclude record.subtype === 'mid_turn_user_message' here, matching the boundary logic used when rebuilding turn boundaries.
— gpt-5.5 via Qwen Code /review
There was a problem hiding this comment.
Already covered in the current implementation. isModelFacingUserPromptRecord excludes both notification and mid_turn_user_message, so resumed mid-turn user messages do not increment the model-facing counter.
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Requested changes based on the inline comments. — gpt-5.5 via Qwen Code /review
Jerry2003826
force-pushed
the
codex/fix-rewind-compressed-tail
branch
from
6c8b555 to
ee819d6
Compare
| ); | ||
| } | ||
| if (!Array.isArray(history)) { | ||
| const isHistorySnapshot = |
There was a problem hiding this comment.
[Critical] isHistorySnapshot guard accepts NaN, Infinity, -Infinity, negative, and non-integer values for modelFacingUserTurnCount. typeof NaN === 'number' is true in JS. A malformed ACP client can send {history: [...], modelFacingUserTurnCount: NaN} which permanently poisons the session — Math.max(NaN, ...) propagates NaN through all downstream arithmetic in #computeApiTruncationIndexForUserTurn, making every future rewindToTurn fail silently.
| const isHistorySnapshot = | |
| const isHistorySnapshot = | |
| !!history && | |
| typeof history === 'object' && | |
| !Array.isArray(history) && | |
| Array.isArray((history as { history?: unknown }).history) && | |
| Number.isInteger( | |
| (history as { modelFacingUserTurnCount?: unknown }) | |
| .modelFacingUserTurnCount, | |
| ) && | |
| ((history as { modelFacingUserTurnCount?: unknown }) | |
| .modelFacingUserTurnCount as number) >= 0; |
Number.isInteger() rejects NaN, Infinity, floats, and the >= 0 rejects negatives — matching the existing validation pattern for targetTurnIndex at line 1004.
— mimo-v2.5-pro via Qwen Code /review
There was a problem hiding this comment.
Already covered in the current guard: snapshot counts must be integer, finite, non-negative, and <= Number.MAX_SAFE_INTEGER. The invalid-count test covers NaN, infinities, negative, fractional, and oversized values.
| return this.config.getGeminiClient()!.getChat().getHistory(); | ||
| captureHistorySnapshot(): HistorySnapshot { | ||
| return { | ||
| history: structuredClone( |
There was a problem hiding this comment.
[Suggestion] Redundant structuredClone. GeminiChat.getHistory() already returns structuredClone(history) (see packages/core/src/core/geminiChat.ts:1225). The outer structuredClone here deep-clones an already-deep-cloned result — a redundant O(n) allocation over the entire conversation.
| history: structuredClone( | |
| history: this.config.getGeminiClient()!.getChat().getHistory(), |
— mimo-v2.5-pro via Qwen Code /review
| const history = Array.isArray(snapshot) ? snapshot : snapshot.history; | ||
| this.config | ||
| .getGeminiClient()! | ||
| .getChat() |
There was a problem hiding this comment.
[Suggestion] When restoreHistory receives the backward-compatible Content[] format, modelFacingUserTurnCount is left at its previous value — potentially desynchronized from the restored history. While the current producer always sends HistorySnapshot, older ACP clients or direct API callers could hit this path. A stale counter causes #computeApiTruncationIndexForUserTurn to compute wrong compressedTurnCount, producing incorrect rewind results with an unhelpful error message.
Consider computing a fallback counter from the restored history in the legacy branch:
if (Array.isArray(snapshot)) {
// Legacy path: derive count from restored history
this.modelFacingUserTurnCount = history.filter(c => c.role === 'user').length;
} else {
this.modelFacingUserTurnCount = snapshot.modelFacingUserTurnCount;
}— mimo-v2.5-pro via Qwen Code /review
| @@ -409,10 +647,47 @@ describe('Session', () => { | |||
| const snapshot = session.captureHistorySnapshot(); | |||
| session.restoreHistory(snapshot); | |||
|
|
|||
There was a problem hiding this comment.
[Suggestion] Missing test coverage for the new HistorySnapshot behavior. Consider adding:
-
Verify
modelFacingUserTurnCountis actually restored — this test checks the snapshot shape andsetHistorycall, but doesn't assert the internal counter changed (if theif (!Array.isArray(snapshot))branch were removed, the test would still pass). -
Legacy
Content[]happy path — no test callsrestoreHistorywith a plainContent[]and verifies it works (the onlyContent[]tests are error-path guards that throw before reaching the history-setting code). -
Deep clone verification — no test confirms that mutating the returned snapshot doesn't affect the session's internal history (the
structuredCloneaddition). -
NaN/Infinity rejection — add a test for
extMethod('restoreSessionHistory', { history: { history: [], modelFacingUserTurnCount: NaN } })to verify it throws after the guard fix.
— mimo-v2.5-pro via Qwen Code /review
There was a problem hiding this comment.
Expanded in 4188d3dce: tests now assert snapshot counter restoration, non-compressed count/history consistency, compressed visible-tail consistency, empty snapshot rejection, and restore-side history cloning.
Jerry2003826
force-pushed
the
codex/fix-rewind-compressed-tail
branch
2 times, most recently
from
7d4c088 to
3519336
Compare
| modelFacingUserTurnCount: number; | ||
| } | ||
|
|
||
| function computeVisibleModelFacingUserTurnCount(apiHistory: Content[]): number { |
There was a problem hiding this comment.
[Suggestion] computeVisibleModelFacingUserTurnCount counts the synthetic compression summary user entry as a real user turn.
When the legacy Content[] path receives a history with an active compression summary pair, this function counts the summary user entry (at startIndex) because getApiUserTextIndices does not skip it. This inflates modelFacingUserTurnCount by 1, which feeds into #computeApiTruncationIndexForUserTurn's compressedTurnCount calculation and can make one tail turn unreachable during rewind.
Both historyMapping.ts:86 and #computeApiTruncationIndexForUserTurn (line 472) detect hasCompressionSummaryPair and shift to startIndex + 2. This helper should do the same:
| function computeVisibleModelFacingUserTurnCount(apiHistory: Content[]): number { | |
| function computeVisibleModelFacingUserTurnCount(apiHistory: Content[]): number { | |
| const startIndex = hasStartupContext(apiHistory) ? 2 : 0; | |
| if (hasCompressionSummaryPair(apiHistory, startIndex)) { | |
| return getApiUserTextIndices(apiHistory, startIndex + 2, true).length; | |
| } | |
| return getApiUserTextIndices(apiHistory, startIndex, true).length; | |
| } |
Adding a test with a compression summary pair in the legacy array derivation test would also catch this.
— mimo-v2.5-pro via Qwen Code /review_
There was a problem hiding this comment.
Already covered in the current implementation. computeVisibleModelFacingUserTurnCount() skips the compression summary pair by counting from startIndex + 2, and the startup+compression legacy restore test asserts only the visible tail is counted.
Jerry2003826
force-pushed
the
codex/fix-rewind-compressed-tail
branch
from
3519336 to
17ebc6c
Compare
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
The core logic is correct — compression-aware tail alignment works as intended. All 217 tests pass, build succeeds, no type errors. The refactor cleanly extracts shared utilities.
New findings (not covered in prior reviews)
1. [Suggestion] Stop hook continuation missing #recordModelFacingUserTurn
(pre-existing code, not in diff). The stop hook continuation loop sends messages via #sendMessageStreamWithAutoCompression but never calls #recordModelFacingUserTurn, unlike the main send loop (line 734) and cron path (line 1474). After a stop hook continuation, modelFacingUserTurnCount is deflated relative to the API history's actual user-text entries. This inflates compressedTurnCount, potentially making reachable tail turns appear unreachable.
2. [Suggestion] No dedicated tests for apiHistoryUtils.ts
The new shared utility module exports 6 functions with no dedicated test file. Key edge-case branches (empty parts, mixed text + functionResponse, boundary conditions) are tested only indirectly.
3. [Suggestion] No test for #rollbackModelFacingUserTurn path
The rollback method (send fails → counter decremented) has zero test coverage.
Low-confidence finding (needs human review)
[Critical — low confidence] Prompt injection via restoreSessionHistory
acpAgent.ts:1024-1060 accepts any Content[] and passes it to chat.setHistory(). A malicious ACP client could replace the startup context pair with a poisoned version matching the model ack text but with adversarial user entry. Practical risk is low (ACP runs on localhost).
— mimo-v2.5-pro via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
本审查在 @wenshao 已有 16 条 inline comments + 4 轮 review 之外，聚焦遗漏的 gap：
Critical
- rewind 失败路径缺少诊断日志 —
#computeApiTruncationIndexForUserTurn返回 -1 时不记录compressedTurnCount、apiTailUserIndices.length等关键状态，导致生产环境无法定位根因。 - 空历史快照被接受 —
isHistorySnapshot接受{ history: [], modelFacingUserTurnCount: 0 }，可静默清空整个会话。 modelFacingUserTurnCount被赋值为索引而非剩余计数 —this.modelFacingUserTurnCount = targetTurnIndex（基于 0 的索引）在语义上不等价于剩余回合数，多次 rewind 循环后累积误差。
Suggestion（非文件级）
historyMapping.ts与Session.ts的压缩感知倒回实现应提取到apiHistoryUtils.ts统一维护apiHistoryUtils.ts（6 个导出函数）和chatCompressionConstants.ts（常量值验证）缺少专用测试文件
— DeepSeek/deepseek-v4-pro via Qwen Code /review
| targetTurnIndex + 1, | ||
| ); | ||
| const compressedTurnCount = Math.max( | ||
| 0, |
There was a problem hiding this comment.
[Critical] rewind 失败路径缺少诊断日志，生产环境无法定位根因。
#computeApiTruncationIndexForUserTurn 在压缩感知路径中返回 -1 时仅抛出通用错误 'Cannot rewind to the requested turn'，不记录 modelFacingUserTurnCount、compressedTurnCount、apiTailUserIndices.length、targetTurnIndex 等关键中间值。凌晨 3 点收到 rewind 不工作的上报时，无法从日志中判断是计数器偏移、压缩回合数计算错误还是 tail 索引越界。
| 0, | |
| // 在返回 -1 前添加： | |
| this.debugLogger?.warn('rewind failed', { | |
| targetTurnIndex, | |
| modelFacingUserTurnCount: this.modelFacingUserTurnCount, | |
| compressedTurnCount, | |
| apiTailUserIndicesLength: apiTailUserIndices.length, | |
| hasCompressionSummaryPair: true, | |
| totalUserTurns, | |
| }); |
— DeepSeek/deepseek-v4-pro via Qwen Code /review
There was a problem hiding this comment.
Fixed in 4188d3dce. Compressed rewind rejection now logs targetTurnIndex, modelFacingUserTurnCount, compressedTurnCount, apiTailUserIndices.length, apiHistory.length, and startIndex.
| !Array.isArray(history) && | ||
| Array.isArray((history as { history?: unknown }).history) && | ||
| Number.isInteger( | ||
| (history as { modelFacingUserTurnCount?: unknown }) |
There was a problem hiding this comment.
[Critical] 空历史快照被接受，可静默清空整个会话。
isHistorySnapshot 接受 { history: [], modelFacingUserTurnCount: 0 }。该快照会通过 session.restoreHistory 调用 chat.setHistory([]) 静默清空整个会话历史。当前校验没有对 history 数组设最小长度限制。
| (history as { modelFacingUserTurnCount?: unknown }) | |
| // 在 restoreHistory 或 isHistorySnapshot 后添加： | |
| if (Array.isArray(history) && history.length === 0 && modelFacingUserTurnCount > 0) { | |
| throw new Error('Inconsistent snapshot: empty history with non-zero turn count'); | |
| } |
— DeepSeek/deepseek-v4-pro via Qwen Code /review
There was a problem hiding this comment.
Fixed in 4188d3dce. isHistorySnapshot now requires a non-empty history array before forwarding to Session.restoreHistory(), and both agent/session tests cover empty snapshot rejection.
|
|
||
| chat.truncateHistory(apiTruncateIndex); | ||
| chat.stripThoughtsFromHistory(); | ||
| // targetTurnIndex is zero-based; after truncating before that turn, |
There was a problem hiding this comment.
[Critical] modelFacingUserTurnCount 被赋值为索引（0-based）而非剩余回合计数。
rewindToTurn 中 this.modelFacingUserTurnCount = targetTurnIndex 将基于 0 的索引赋给表示剩余回合数的计数器。例如：3 个回合（索引 0,1,2），rewindToTurn(2) 截断后仅剩 1 个回合，但计数器被设为 2。当前被 Math.max 防御掩盖，但多次 rewind+resend 循环后累积误差可能导致有效回合被错误拒绝。
| // targetTurnIndex is zero-based; after truncating before that turn, | |
| // 从截断后的历史重新计算： | |
| this.modelFacingUserTurnCount = computeVisibleModelFacingUserTurnCount( | |
| chat.getHistory(), | |
| ); |
— DeepSeek/deepseek-v4-pro via Qwen Code /review
There was a problem hiding this comment.
No code change for the assignment itself: targetTurnIndex is zero-based, and rewinding to turn N truncates before that turn, leaving exactly N model-facing user turns. 4188d3dce adds an explicit assertion that rewindToTurn(2) leaves the counter at 2.
| modelFacingUserTurnCount: number; | ||
| } | ||
|
|
||
| function computeVisibleModelFacingUserTurnCount(apiHistory: Content[]): number { |
There was a problem hiding this comment.
[Suggestion] computeVisibleModelFacingUserTurnCount 在压缩活跃时低估计数器，导致 compressedTurnCount = 0 退化。
旧版数组恢复路径下，该函数仅统计尾部可见回合数，不包含被压缩摘要代表的压缩回合。当后续 compressedTurnCount = Math.max(0, totalUserTurns - apiTailUserIndices.length) 计算时，totalUserTurns 偏低，使得 compressedTurnCount = 0，让本应不可达的已压缩回合变为可达。
| function computeVisibleModelFacingUserTurnCount(apiHistory: Content[]): number { | |
| // 至少为压缩摘要保留 1 个回合： | |
| if (hasCompressionSummaryPair(apiHistory, startIndex)) { | |
| const tailCount = getApiUserTextIndices(apiHistory, startIndex + 2, true).length; | |
| return tailCount + 1; // 摘要至少代表 1 个压缩回合 | |
| } |
— DeepSeek/deepseek-v4-pro via Qwen Code /review
There was a problem hiding this comment.
For the legacy Content[] restore path the compressed turn count is not recoverable because older callers did not send modelFacingUserTurnCount. The snapshot path preserves the full count; the legacy path intentionally derives only visible model-facing turns rather than inventing a compressed count.
| : snapshot.modelFacingUserTurnCount; | ||
| } | ||
|
|
||
| #computeApiTruncationIndexForUserTurn( |
There was a problem hiding this comment.
[Suggestion] Math.max(this.modelFacingUserTurnCount, targetTurnIndex + 1) 静默掩盖计数器不一致。
当 modelFacingUserTurnCount 偏低时，Math.max 使用 targetTurnIndex + 1 作为下限，让计算跑通，但压缩回合数被低估。建议在此分支发出 warning 日志标记可能的状态不一致，而非静默修正。
— DeepSeek/deepseek-v4-pro via Qwen Code /review
There was a problem hiding this comment.
Addressed in 4188d3dce with a warning for inconsistent compressed rewind state when the model-facing counter is lower than the visible API tail, instead of silently relying on the Math.max fallback.
| @@ -0,0 +1,16 @@ | |||
| /** | |||
| * @license | |||
| * Copyright 2025 Google LLC | |||
There was a problem hiding this comment.
[Suggestion] License 头错误：Copyright 2025 Google LLC 应改为 Copyright 2025 Qwen Code。
同 PR 新增的 apiHistoryUtils.ts 使用 Copyright 2025 Qwen Code，此文件为 copy-paste 遗留。
| * Copyright 2025 Google LLC | |
| * Copyright 2025 Qwen Code |
— DeepSeek/deepseek-v4-pro via Qwen Code /review
| export const COMPRESSION_SUMMARY_MODEL_ACK = | ||
| 'Got it. Thanks for the additional context!'; | ||
|
|
||
| const COMPRESSION_CONTINUATION_BRIDGE_MARKER = '\u200B\u200C\u200D\u2060'; |
There was a problem hiding this comment.
[Suggestion] COMPRESSION_CONTINUATION_BRIDGE 的不可见 Unicode 前缀可能被中间件剥离。
零宽字符（\u200B\u200C\u200D\u2060）在某些日志系统、序列化层或 hook 链中可能被规范化去除，导致 bridge 检测静默失败，用户 prompt 被误认为桥接条目。建议在检测路径添加 debug 级别日志。
— DeepSeek/deepseek-v4-pro via Qwen Code /review
| #rollbackModelFacingUserTurn(recorded: boolean): void { | ||
| if (recorded) { | ||
| this.modelFacingUserTurnCount = Math.max( | ||
| 0, |
There was a problem hiding this comment.
[Suggestion] #rollbackModelFacingUserTurn 的非取消停止原因路径缺少测试覆盖。
当 sendResult.stopReason !== 'cancelled'（如 max_tokens、session_token_limit_exceeded）时，回滚逻辑无测试验证。建议添加测试使 mock 返回 { responseStream: undefined, stopReason: 'max_tokens' } 并验证计数器正确回滚。
— DeepSeek/deepseek-v4-pro via Qwen Code /review
There was a problem hiding this comment.
Covered in 4188d3dce by rolls back model-facing turn count when a non-compressed send is stopped before streaming, which exercises the non-cancel max_tokens stop path and verifies the counter is rolled back.
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Qwen Code Review — PR #4242
Verdict: REQUEST CHANGES
Summary
This PR fixes #4046 by adding a modelFacingUserTurnCount field and compression-aware mapping in Session.#resolveApiIndex. The approach is sound in principle, but there are three high-confidence issues with the counter lifecycle and validation that can cause incorrect rewind targeting.
Critical Issues (3 high-confidence)
1. Stop hook continuation skips #recordModelFacingUserTurn
#recordModelFacingUserTurn is called in the main send path (line 734) and ACP send path (line 1474), but NOT in the stop hook continuation path (lines 980-1001). When a stop hook creates a synthetic user message and continues the conversation, the counter is never incremented. After N stop hook continuations, modelFacingUserTurnCount undercounts by N, causing Math.max in #resolveApiIndex to mask the drift until the error becomes large enough to produce wrong mappings.
→ See inline comment on Session.ts:1129.
2. Math.max safety net amplifies counter drift
totalUserTurns = Math.max(this.modelFacingUserTurnCount, targetTurnIndex + 1) ensures the value never drops below targetTurnIndex + 1, but this masks a too-low counter. With N missing turns from Finding 1, compressedTurnCount becomes max(counter, targetTurnIndex+1) - tailLen, which can be too small, causing the mapping to target an earlier turn than intended.
→ See inline comment on Session.ts:481.
3. No upper bound validation for modelFacingUserTurnCount
The isHistorySnapshot check in acpAgent.ts:1052 validates modelFacingUserTurnCount >= 0 but not <= actualTurnCount. An external caller can pass an arbitrarily large value, which would inflate compressedTurnCount and make all rewind targets resolve to -1 (blocked).
→ See inline comment on acpAgent.ts:1052.
Needs Human Review
4. Dual compression-aware mapping implementations (Critical/low-confidence)
Session.#resolveApiIndex (lines 472-507) and historyMapping.ts:computeApiTruncationIndex (lines 73-126) both handle compression-aware turn mapping but use different data sources (API-level counter vs UI-level ordinals). The Session version uses modelFacingUserTurnCount and targetTurnIndex; the historyMapping version uses totalRealUserTurns from UI history and targetOrdinal. This duplication creates a risk of future divergence.
Info
- Deterministic analysis: 102 TS4111 errors (all pre-existing from root tsconfig), 0 eslint errors
- Build: OK
- Tests: 217/217 pass
- Autofix: skipped (Critical issues are architectural decisions)
Reviewed by mimo-v2.5-pro
| } | ||
|
|
||
| #recordModelFacingUserTurn(message: Content): boolean { | ||
| if (isApiUserTextContent(message)) { |
There was a problem hiding this comment.
Critical: Stop hook continuation path skips #recordModelFacingUserTurn
This method increments modelFacingUserTurnCount and is correctly called in the main send path (~line 734) and ACP send path (~line 1474). However, the stop hook continuation path (lines 980-1001) creates a synthetic user message and sends it via #sendMessageStreamWithAutoCompression without calling #recordModelFacingUserTurn.
After N stop hook continuations, modelFacingUserTurnCount undercounts by N. This causes Math.max in #resolveApiIndex to silently mask the drift, eventually producing wrong rewind mappings.
Fix: Add this.#recordModelFacingUserTurn(nextMessage) before the #sendMessageStreamWithAutoCompression call at ~line 995, matching the pattern in the main send path (line 734).
Reviewed by mimo-v2.5-pro
There was a problem hiding this comment.
Already covered in the current implementation. The Stop-hook continuation loop calls #recordModelFacingUserTurn(nextMessage) before #sendMessageStreamWithAutoCompression, and the forced-continuation test asserts the counter reaches 2 after the initial prompt plus Stop-hook continuation.
| true, | ||
| ); | ||
| const totalUserTurns = Math.max( | ||
| this.modelFacingUserTurnCount, |
There was a problem hiding this comment.
Critical: Math.max safety net amplifies counter drift
totalUserTurns = Math.max(this.modelFacingUserTurnCount, targetTurnIndex + 1) ensures the value never drops below targetTurnIndex + 1, but this masks a too-low counter from the stop hook bug (see comment on #recordModelFacingUserTurn).
With N missing turns from stop hook continuations:
compressedTurnCount = max(counter, targetTurnIndex+1) - tailLen- When
targetTurnIndex + 1 > counter,Math.maxusestargetTurnIndex + 1instead of the true count - This can make
compressedTurnCounttoo small, causing the mapping to target an earlier (wrong) turn
Fix: Remove the Math.max safety net and instead ensure the counter is always correct (fix Finding 1). If a fallback is needed, prefer returning -1 (blocked) rather than guessing.
Reviewed by mimo-v2.5-pro
| ) && | ||
| ((history as { modelFacingUserTurnCount?: unknown }) | ||
| .modelFacingUserTurnCount as number) >= 0; | ||
| if (!Array.isArray(history) && !isHistorySnapshot) { |
There was a problem hiding this comment.
Critical: No upper bound validation for modelFacingUserTurnCount
The validation checks Number.isInteger(modelFacingUserTurnCount) && >= 0 but does NOT check an upper bound. An external caller (e.g., IDE companion extension) can pass an arbitrarily large value.
If modelFacingUserTurnCount > actualTurnCount, then:
compressedTurnCount = modelFacingUserTurnCount - apiTailUserIndices.lengthbecomes too large- All rewind targets resolve to
-1(blocked), making rewind completely non-functional
Fix: Add an upper bound check, e.g.:
&& ((history as { modelFacingUserTurnCount?: unknown })
.modelFacingUserTurnCount as number) <= MAX_REASONABLE_TURN_COUNTOr validate against history.history.length at restore time.
Reviewed by mimo-v2.5-pro
There was a problem hiding this comment.
Already covered in the current guard: both isHistorySnapshot and Session validation reject values above Number.MAX_SAFE_INTEGER.
| } | ||
|
|
||
| #recordModelFacingUserTurn(message: Content): boolean { | ||
| if (isApiUserTextContent(message)) { |
There was a problem hiding this comment.
Critical: Stop hook continuation path skips #recordModelFacingUserTurn
This method increments modelFacingUserTurnCount and is correctly called in the main send path (~line 734) and ACP send path (~line 1474). However, the stop hook continuation path (lines 980-1001) creates a synthetic user message and sends it via #sendMessageStreamWithAutoCompression without calling #recordModelFacingUserTurn.
After N stop hook continuations, modelFacingUserTurnCount undercounts by N. This causes Math.max in #resolveApiIndex to silently mask the drift, eventually producing wrong rewind mappings.
Fix: Add this.#recordModelFacingUserTurn(nextMessage) before the #sendMessageStreamWithAutoCompression call at ~line 995, matching the pattern in the main send path (line 734).
Reviewed by mimo-v2.5-pro
There was a problem hiding this comment.
Already covered in the current implementation. The Stop-hook continuation loop records the synthetic continuation user message before sending, and the Stop-hook auto-compression test asserts the counter includes that continuation.
| true, | ||
| ); | ||
| const totalUserTurns = Math.max( | ||
| this.modelFacingUserTurnCount, |
There was a problem hiding this comment.
Critical: Math.max safety net amplifies counter drift
totalUserTurns = Math.max(this.modelFacingUserTurnCount, targetTurnIndex + 1) ensures the value never drops below targetTurnIndex + 1, but this masks a too-low counter from the stop hook bug (see comment on #recordModelFacingUserTurn).
With N missing turns from stop hook continuations:
compressedTurnCount = max(counter, targetTurnIndex+1) - tailLen- When
targetTurnIndex + 1 > counter,Math.maxusestargetTurnIndex + 1instead of the true count - This can make
compressedTurnCounttoo small, causing the mapping to target an earlier (wrong) turn
Fix: Remove the Math.max safety net and instead ensure the counter is always correct (fix Finding 1). If a fallback is needed, prefer returning -1 (blocked) rather than guessing.
Reviewed by mimo-v2.5-pro
| ) && | ||
| ((history as { modelFacingUserTurnCount?: unknown }) | ||
| .modelFacingUserTurnCount as number) >= 0; | ||
| if (!Array.isArray(history) && !isHistorySnapshot) { |
There was a problem hiding this comment.
Critical: No upper bound validation for modelFacingUserTurnCount
The validation checks Number.isInteger(modelFacingUserTurnCount) && >= 0 but does NOT check an upper bound. An external caller (e.g., IDE companion extension) can pass an arbitrarily large value.
If modelFacingUserTurnCount > actualTurnCount, then:
compressedTurnCount = modelFacingUserTurnCount - apiTailUserIndices.lengthbecomes too large- All rewind targets resolve to
-1(blocked), making rewind completely non-functional
Fix: Add an upper bound check, e.g.:
&& ((history as { modelFacingUserTurnCount?: unknown })
.modelFacingUserTurnCount as number) <= MAX_REASONABLE_TURN_COUNTOr validate against history.history.length at restore time.
Reviewed by mimo-v2.5-pro
There was a problem hiding this comment.
Already covered in the current guard: both ACP agent shape validation and Session validation reject oversized modelFacingUserTurnCount values.
Jerry2003826
force-pushed
the
codex/fix-rewind-compressed-tail
branch
from
17ebc6c to
c923bbe
Compare
|
|
||
| try { | ||
| const recordedModelFacingTurn = | ||
| this.#recordModelFacingUserTurn(nextMessage); |
There was a problem hiding this comment.
[Suggestion] Catch blocks do not rollback modelFacingUserTurnCount after #recordModelFacingUserTurn increments it.
Three send paths increment the counter at the top of try but never call #rollbackModelFacingUserTurn in catch:
- Main send (line 738 → catch ~795): exception re-thrown, counter dirty
- Stop hook (line 1002 → catch ~1058): same pattern
- Cron ACP (line 1483 → catch ~1563): error swallowed, permanent drift
The try-internal early-return paths correctly rollback. The exception paths were missed.
| this.#recordModelFacingUserTurn(nextMessage); | |
| } catch (error) { | |
| + this.#rollbackModelFacingUserTurn(recordedModelFacingTurn); |
— DeepSeek/deepseek-v4-pro via Qwen Code /review
There was a problem hiding this comment.
Fixed in 4188d3dce. The send-path catch blocks now track whether this send compressed history and only roll back the counter when the failed send did not replace history via compression.
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
No new review findings beyond the 14 existing inline comments from prior review rounds. Downgraded from Approve to Comment: CI failing (Test windows-latest, Node 22.x).
Deterministic checks: Build passes. All 275 PR-affected tests pass (6 test files). 102 pre-existing TS4111 type errors in vscode-ide-companion (not from this PR — diff only added a type alias).
— DeepSeek/deepseek-v4-pro via Qwen Code /review
| const promptId = | ||
| this.config.getSessionId() + '########cron' + Date.now(); | ||
|
|
||
| let recordedModelFacingTurn = false; |
There was a problem hiding this comment.
[Critical] recordedModelFacingTurn is scoped to the whole cron execution instead of each send iteration. After the initial cron prompt is recorded, a later tool-response iteration can leave this flag true even though #recordModelFacingUserTurn(nextMessage) returns false. If that later tool-response send throws, the catch block rolls back the previously successful cron user turn, drifting modelFacingUserTurnCount too low and breaking compression-aware rewind mapping.
Reset the flag inside the while (nextMessage !== null) loop so each iteration only rolls back its own attempted send.
| let recordedModelFacingTurn = false; | |
| while (nextMessage !== null) { | |
| if (ac.signal.aborted) return; | |
| const functionCalls: FunctionCall[] = []; | |
| let usageMetadata: GenerateContentResponseUsageMetadata | null = | |
| null; | |
| const streamStartTime = Date.now(); | |
| let recordedModelFacingTurn = false; |
— gpt-5.5 via Qwen Code /review
There was a problem hiding this comment.
Fixed in 4188d3dce. Cron send bookkeeping now resets recordedModelFacingTurn and sendHistoryCompressed at the start of each loop iteration, so a previous cron send cannot leak state into a later tool-response iteration.
Jerry2003826
force-pushed
the
codex/fix-rewind-compressed-tail
branch
from
004dc80 to
c24c8cc
Compare
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Re-review at 49203b9 (updated to 79f46e9) — no new high-confidence Critical or Suggestion issues found.
The compression-aware rewind mapping logic is sound after 23+ rounds of iterative review. All 472 targeted tests pass, CI is green (24/24). The core changes (modelFacingUserTurnCount tracking, HistorySnapshot shape, apiHistoryUtils shared utilities, send-path rollback bookkeeping) are well-tested and correctly handle the compressed/non-compressed boundary cases.
5 open inline Critical comments from prior review rounds remain for author attention (notification parity, counter drift, cross-package prefix coupling, isHistorySnapshot extraction, compression+max_tokens rollback).
— qwen3.7-max via Qwen Code /review
✅ Local real-build verification (tmux + built-function A/B)Maintainer verification before merge. I built the real Verdict: LGTM. The fix makes rewind-after-compression behave correctly end-to-end. Pre-fix, the same mapping silently does the wrong thing in three distinct ways (rejects a valid recent turn, maps a compressed turn to the wrong API index, or rewinds to the start). The refactor is behavior-preserving on every non-compression path. 1. Real-TUI A/B (tmux) — the user-visible payoff of #4046Scenario in the real TUI: send 3 turns → After (this PR) — correctly refuses and tells the user why: Debug ( Before (pre-fix) — same 3-turn setup, rewinding to the same compressed turn #1: Debug ( So the pre-fix build silently "succeeds": it guts the API history ( 2. Built-function A/B (
|
| compressed-history scenario | rewind target | BASE (pre-fix) | FIXED (this PR) | pre-fix harm |
|---|---|---|---|---|
| tail turn after summary (turns 1,3 absorbed) | 5 | -1 | 2 | rejects a valid recent rewind |
| compressed turn (absorbed by summary) | 3 | 2 | -1 | maps to the wrong API index (truncates at the tail prompt!) |
| first turn absorbed by summary | 1 | 0 | -1 | rewinds to the start instead of refusing |
| full tail absorbed, turn 1 / turn 3 | 1 / 3 | 0 / -1 | -1 / -1 | turn 1 wrongly reachable |
| post-compact attachment, compressed turn 3 | 3 | 2 | -1 | wrongly reachable |
| post-compact attachment, tail turn 5 | 5 | 4 | 4 | (coincidentally equal) |
| startup-context + compressed tail | 5 | -1 | 4 | rejects a valid rewind |
6 of 8 scenarios diverge — exactly the "reject a valid recent rewind or truncate to the wrong API-history index" failure the PR describes.
3. Mutation test (the new tests are not vacuous)
Reverted historyMapping.ts to the merge-base, kept the PR's historyMapping.test.ts, ran vitest:
- 7 tests fail — all 7 are the new
compression fallbackcases:expected -1 to be 2,expected 2 to be -1,expected +0 to be -1,expected -1 to be 4, … - 26 tests pass — every non-compression case (first/second/third-turn mapping, startup context, mid-history system-reminders, tool-call/functionResponse skipping, slash-command filtering, single-turn).
→ the refactor is behavior-preserving off the compression path, and the 7 new tests precisely guard the fix.
中文版（点击展开）
✅ 本地真实构建验证（tmux + 内置函数 A/B）
合并前的维护者验证。我在 PR HEAD（79f46e9）构建了真实 qwen 二进制，并与修复前构建（仅把 historyMapping.ts 回退到 merge-base 6fa1320 后重新构建 cli）做了 A/B 对比。环境：macOS arm64，Node v22.22.2，模型 qwen3.7-max。
结论：可以合并。 这个修复让"压缩后 rewind"端到端表现正确。修复前，同样的映射会以三种不同方式静默出错（拒绝一个有效的近期轮次、把已压缩轮次映射到错误的 API index、或直接 rewind 到开头）。重构在所有非压缩路径上行为保持不变。
1. 真实 TUI A/B（tmux）—— #4046 对用户可见的结果
真实 TUI 场景：发 3 个轮次 → /compress（强制压缩）→ /rewind → 选第 #1 个轮次（已被压缩摘要吸收）。
修复后（本 PR）—— 正确拒绝并告知原因：
✦ Chat history compressed from 21589 to 19719 tokens.
Rewind Conversation (3 turns)
✕ Cannot rewind to a turn that was compressed. Try a more recent turn.
Debug（HISTORY_MAPPING）：Rewind target turn 1 is unreachable: compressed 3 of 3 total turns, tail has 0 entries。
修复前—— 同样的 3 轮次设置，rewind 到同一个已压缩的 #1 轮次：
● Conversation rewound. Edit your prompt and press Enter to continue.
* Compute 100 plus 11 ... ← 输入框被错误地预填成了 #1 轮次
Debug（CLIENT）：clear after truncateHistory(keep=1, prev=3, new=1) —— 它静默地把 API history 从 3 条截断成了 1 条。
所以修复前的构建静默"成功"：它把 API history 掏空（truncateHistory(keep=1, prev=3)）并把输入框预填成 #1 轮次的 prompt，UI 显示 rewind 到了一个其上下文已不在 API history 中的轮次 —— 正是 #4046 的损坏。而修复后的构建会用清晰提示拒绝，并保持会话状态不被破坏。
2. 内置函数 A/B（computeApiTruncationIndex，从构建出的 cli dist 导入）
用 PR 自己 historyMapping.test.ts 的同款 fixture。该函数把 UI 轮次映射到 API history 截断 index（-1 = "不可达 / 拒绝"）。
| 压缩历史场景 | rewind 目标 | BASE（修复前） | FIXED（本 PR） | 修复前的危害 |
|---|---|---|---|---|
| 摘要后的 tail 轮次（轮次 1,3 被吸收） | 5 | -1 | 2 | 拒绝一个有效的近期 rewind |
| 已压缩轮次（被摘要吸收） | 3 | 2 | -1 | 映射到错误的 API index（截断到了 tail prompt！） |
| 第一个轮次被摘要吸收 | 1 | 0 | -1 | rewind 到开头而不是拒绝 |
| 整个 tail 被吸收，turn 1 / turn 3 | 1 / 3 | 0 / -1 | -1 / -1 | turn 1 被错误判为可达 |
| post-compact attachment，已压缩 turn 3 | 3 | 2 | -1 | 被错误判为可达 |
| post-compact attachment，tail turn 5 | 5 | 4 | 4 | （恰好相等） |
| startup-context + 压缩 tail | 5 | -1 | 4 | 拒绝一个有效 rewind |
8 个场景里 6 个发散 —— 正是 PR 描述的"拒绝有效的近期 rewind 或 截断到错误的 API history index"。
3. 变异测试（证明新增测试不是空过）
把 historyMapping.ts 回退到 merge-base，保留 PR 的 historyMapping.test.ts，跑 vitest：
- 7 个测试失败 —— 全是新增的
compression fallback用例：expected -1 to be 2、expected 2 to be -1、expected +0 to be -1、expected -1 to be 4…… - 26 个测试通过 —— 所有非压缩用例（首/二/三轮映射、startup context、history 中部的 system-reminder、tool-call/functionResponse 跳过、slash 命令过滤、单轮）。
→ 重构在压缩路径之外行为保持不变，且这 7 个新测试精确守住了修复。
|
@qwen-code /resolve |
|
Qwen Code resolved the merge conflicts and pushed the branch update. Conflict Resolution SummaryPR
Conflicted File
What ConflictedThe PR branch changed Meanwhile, origin/main added a new method The conflict block sat at the boundary where both changes touched the same ResolutionKept both contributions and adapted the new method to the new return type:
No other files required manual resolution; all other auto-merged files were |
| if (record.subtype === 'cron') { | ||
| return true; | ||
| } | ||
| return !fullText.startsWith('?') && !isResumeSlashCommand(fullText); |
There was a problem hiding this comment.
[Critical] This filter drops every slash-looking user record when reconstructing API history, but ACP records the raw prompt before slash-command expansion. For a model-facing submit_prompt command, #processSlashCommandResult sends expanded content to the model and the assistant reply is recorded, but resume now skips the recorded /... user turn while keeping the following model turn. That rebuilds API history with an orphaned model response and also undercounts model-facing turns. Please distinguish fully-handled slash commands from submit_prompt commands, or persist the expanded model-bound content/metadata so resume can keep the model-facing turn.
— GPT-5 Codex via Qwen Code /review
| .map((part) => part.text.trim()); | ||
| if (textParts.length === 0) return false; | ||
|
|
||
| const fullText = textParts.join(' '); |
There was a problem hiding this comment.
[Critical] fullText joins every text part before checking for <task-notification. Live background notifications are built as [...notificationReminders, ...notificationParts], so in plan/arena mode the joined text starts with <system-reminder> and this notification is counted as a real user turn. That shifts getApiUserTextIndices() and can make rewind truncate to a notification, or reject a valid compressed-tail target. Please detect notification parts before joining, or ignore structural reminder parts before applying the task-notification check.
— GPT-5 Codex via Qwen Code /review
| let sendDispatched = false; | ||
|
|
||
| try { | ||
| recordedModelFacingTurn = |
There was a problem hiding this comment.
[Critical] The retry path strips the previous failed send's trailing user entries from chat history before this new counter increment runs for the retry, but it never decrements modelFacingUserTurnCount for the stripped entry. After a failed send whose user entry reached chat history, the successful retry counts the same logical turn again; compressed rewind then uses an inflated total and can reject or mis-map valid tail turns. Please make stripOrphanedUserEntriesFromHistory() report removed model-facing entries, or compute the before/after user-entry count and roll back the counter before retrying.
— GPT-5 Codex via Qwen Code /review
| getRewindableUserTurnCount(): number { | ||
| const apiHistory = this.captureHistorySnapshot(); | ||
| const { history: apiHistory } = this.captureHistorySnapshot(); | ||
| const startIndex = getStartupContextLength(apiHistory); |
There was a problem hiding this comment.
[Critical] Build-breaking merge regression: getRewindableUserTurnCount() at line 1159 still calls this.#isUserTextContent(apiHistory[i]!), but the private method #isUserTextContent was removed by this PR (replaced by the shared isApiUserTextContent from api-history-utils.ts). The merge conflict resolution in a9bf5fcb8 brought in getRewindableUserTurnCount from main without updating its call site.
tsc emits TS2339, esbuild rejects the orphaned private name (so all Session.test.ts tests fail to collect), and the CI test job fails.
| const startIndex = getStartupContextLength(apiHistory); | |
| const { history: apiHistory } = this.captureHistorySnapshot(); | |
| const startIndex = getStartupContextLength(apiHistory); | |
| let count = 0; | |
| for (let i = startIndex; i < apiHistory.length; i++) { | |
| if (isApiUserTextContent(apiHistory[i]!)) { |
Note: isApiUserTextContent additionally excludes ?-prefixed queries and <task-notification entries compared to the old #isUserTextContent. This is likely correct for rewind counting (those are not real model-facing user turns).
— qwen3.7-max via Qwen Code /review
| modelFacingUserTurnCount: this.modelFacingUserTurnCount, | ||
| }; | ||
| } | ||
|
|
There was a problem hiding this comment.
[Critical] Even after the build break is fixed, this method is not compression-aware. It counts every user-role text entry from startIndex onwards, which includes the compression summary, post-compact attachment content, and continuation bridge entries. After compression with 2 real tail turns, this method returns ~5 instead of 2.
The ACP rewindSession endpoint (acpAgent.ts:5082) calls this to determine which file history snapshots are available for rewind — an inflated count exposes unreachable turns.
computeVisibleModelFacingUserTurnCount (defined in this same file) already handles compression correctly via getCompressionTailStartIndex + skipContinuationBridge. Consider delegating:
| getRewindableUserTurnCount(): number { | |
| const { history: apiHistory } = this.captureHistorySnapshot(); | |
| return computeVisibleModelFacingUserTurnCount(apiHistory); | |
| } |
— qwen3.7-max via Qwen Code /review
| if (targetOrdinal <= compressedTurnCount) { | ||
| debugLogger.info( | ||
| `Rewind target turn ${targetOrdinal} is unreachable: compressed ${compressedTurnCount} of ${totalRealUserTurns} total turns, tail has ${apiTailUserIndices.length} entries`, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] Test gap: no compression test exercises rewind to a middle tail turn. All 9 compression tests use 0 or 1 tail turn in the API history. The index arithmetic apiTailUserIndices[targetOrdinal - compressedTurnCount - 1] is only tested at offset 0 of the tail array.
An off-by-one error in the tail offset calculation would be invisible because the only exercised positions are "compressed turn (returns -1)" and "last tail turn (offset = length - 1)."
Suggested test: 4+ UI user turns, compression absorbing the first 2, leaving 2 tail turns in API history. Rewind targeting the first (not last) tail turn — verify the returned index truncates at the correct position.
— qwen3.7-max via Qwen Code /review
| expect(history).toEqual([ | ||
| recordA1.message, | ||
| notification.message, | ||
| assistantReply.message, |
There was a problem hiding this comment.
[Suggestion] shouldIncludeUserRecordInApiHistory has an explicit if (record.subtype === 'cron') return true; branch, but no test here verifies this. The new tests cover slash exclusion, question-prefix exclusion, and notification inclusion — but never construct a ChatRecord with subtype: 'cron'.
If the cron filter regresses (e.g., reordering checks so the slash-command exclusion catches cron records first), cron prompts silently disappear from model context after session restart.
Suggested test: Add a ChatRecord with subtype: 'cron' and plain text, assert it appears in buildApiHistoryFromConversation output.
— qwen3.7-max via Qwen Code /review
| | { | ||
| responseStream: AsyncGenerator<StreamEvent>; | ||
| stopReason?: never; | ||
| historyCompressed?: boolean; |
There was a problem hiding this comment.
[Suggestion] historyCompressed is populated on all 6 return paths in #sendMessageStreamWithAutoCompression (lines 2237, 2254, 2269, 2294, 2313, 2327) but never read by any caller. No send-path consumer destructures or branches on sendResult.historyCompressed — the rollback logic exclusively uses sendDispatched.
This was likely introduced during iterative development and superseded by the sendDispatched flag. It inflates the type and creates an obligation to keep it consistent across all return sites.
Fix: Remove historyCompressed from both AutoCompressionSendResult variants and all 6 return-object literals.
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
This branch does not build on the current HEAD — see the inline comment on getRewindableUserTurnCount. CI Test (ubuntu-latest, Node 22.x) is red for the same reason (the prior "CI green" comments were on earlier heads; the bot merge-conflict commit re-broke it).
One non-inline follow-up (the failing line is outside the diff hunk, so noting it here): in Session.test.ts, the test can rewind the conversation without restoring file history (~L736) still asserts rewindRecording(1, { truncatedCount: 2 }, undefined), but rewindToTurn now always includes maxModelFacingUserTurnCount in that payload. Its sibling test (~L705) was updated to expect the new field; this one was missed, so it fails once the build is fixed (here the expected value is { truncatedCount: 2, maxModelFacingUserTurnCount: 0 }, since this test never seeds the counter).
I checked the other still-open review threads against this exact HEAD and am deliberately not re-posting them to avoid duplicating the existing threads — they remain valid but minor (dead hasStartupContext export, unused historyCompressed field, the continuation-bridge detection that no production path emits, and the middle-tail / isResumeSlashCommand test gaps).
中文
当前 HEAD 无法编译——见 getRewindableUserTurnCount 上的行内评论；CI Test (ubuntu-latest, Node 22.x) 因同样原因变红（此前"CI 绿"的评论都是更早的 head，bot 解冲突的合并 commit 把它重新弄坏了）。
一个无法行内锚定的后续项（失败行不在 diff hunk 内，故写在这里）：Session.test.ts 中的 can rewind the conversation without restoring file history（约 L736）仍断言 rewindRecording(1, { truncatedCount: 2 }, undefined)，但 rewindToTurn 现在总会在该 payload 里带上 maxModelFacingUserTurnCount。它的兄弟测试（约 L705）已更新为新字段，这个被漏掉了，所以构建修好后它会失败（此处期望值应为 { truncatedCount: 2, maxModelFacingUserTurnCount: 0 }，因为该测试没有给计数器赋初值）。
其余仍处于 open 的 review 线程我已对照本 HEAD 逐一核验，为避免与既有线程重复，刻意不再行内重复张贴——它们依然成立但属次要（死代码 hasStartupContext 导出、未被读取的 historyCompressed 字段、没有任何生产路径会产出的 continuation-bridge 检测，以及 middle-tail / isResumeSlashCommand 的测试缺口）。
— claude-opus-4-8 via Claude Code /qreview
|
|
||
| getRewindableUserTurnCount(): number { | ||
| const apiHistory = this.captureHistorySnapshot(); | ||
| const { history: apiHistory } = this.captureHistorySnapshot(); |
There was a problem hiding this comment.
[Critical] This method does not compile, and the obvious one-line fix still leaves it wrong.
Line 1159 below calls this.#isUserTextContent(...), but this PR deleted that private method (it was replaced repo-wide by the shared isApiUserTextContent). The current HEAD does not build:
tsc --noEmit→Session.ts(1159,16): error TS2339: Property '#isUserTextContent' does not exist on type 'Session'.- vitest/esbuild can't even load the module (
Private name "#isUserTextContent" must be declared in an enclosing class) → the entireSession.test.tssuite collects 0 tests. - CI agrees:
Test (ubuntu-latest, Node 22.x)is failing on this commit with the same error at line 1159. This regressed in themerge: resolve conflict with origin/main in Session.tscommit, after the earlier green-CI verifications.
Minimal compile fix (the helper is already imported at the top of the file):
// line 1159
if (isApiUserTextContent(apiHistory[i]!)) {But that alone leaves a second, real bug: getRewindableUserTurnCount is the only turn-counter in this file the PR did not make compression-aware (computeVisibleModelFacingUserTurnCount, validateModelFacingUserTurnCountForHistory, and #computeApiTruncationIndexForUserTurn all gained the hasCompressionSummaryPair/getCompressionTailStartIndex branch — this one didn't). After compression it counts the synthetic summary entry plus the absorbed turns, so its sole consumer — the web-shell rewind list (acpAgent.ts:5082, filter(idx < rewindableTurnCount)) — surfaces absorbed turns the server then rejects and hides the valid uncompressed tail turn. Please make it compression-aware (skip the prelude via getCompressionTailStartIndex when hasCompressionSummaryPair, mirroring #computeApiTruncationIndexForUserTurn) and add a compressed-history test.
中文
[严重] 这个方法无法编译，而且最直接的一行改法仍然是错的。
下方第 1159 行调用了 this.#isUserTextContent(...)，但本 PR 已删除该私有方法（全仓库改用共享的 isApiUserTextContent）。当前 HEAD 编译失败：
tsc --noEmit→Session.ts(1159,16): error TS2339: Property '#isUserTextContent' does not exist on type 'Session'.- vitest/esbuild 连模块都无法加载（
Private name "#isUserTextContent" must be declared in an enclosing class）→ 整个Session.test.ts套件收集到 0 个测试。 - CI 也证实：
Test (ubuntu-latest, Node 22.x)在该 commit 上失败，错误相同（第 1159 行）。该回归出现在merge: resolve conflict with origin/main in Session.ts合并 commit 中，发生在之前 CI 全绿验证之后。
最小编译修复（helper 已在文件顶部 import）：
// 第 1159 行
if (isApiUserTextContent(apiHistory[i]!)) {但仅此还遗留第二个 bug：getRewindableUserTurnCount 是本文件中唯一没有被改成压缩感知的轮次计数器（computeVisibleModelFacingUserTurnCount、validateModelFacingUserTurnCountForHistory、#computeApiTruncationIndexForUserTurn 都已加入 hasCompressionSummaryPair/getCompressionTailStartIndex 分支，唯独这个没有）。压缩后它会把合成的 summary 条目以及被吸收的轮次一并计入，于是其唯一消费方——web-shell 的 rewind 列表（acpAgent.ts:5082，filter(idx < rewindableTurnCount)）——会显示服务端随后会拒绝的"已吸收"轮次，并隐藏真正可达的未压缩尾部轮次。请将其改为压缩感知（命中 hasCompressionSummaryPair 时用 getCompressionTailStartIndex 跳过前导，与 #computeApiTruncationIndexForUserTurn 一致），并补一个压缩历史的测试。
— claude-opus-4-8 via Claude Code /qreview
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
No new issues found beyond the existing 63 inline comments from prior review rounds (23+ rounds). Build passes, 710 tests pass across all affected packages, ESLint clean. The compression-aware rewind mapping logic and shared api-history-utils extraction are well-structured.
— qwen3.7-max via Qwen Code /review
DragonnZhang
left a comment
DragonnZhang
left a comment
There was a problem hiding this comment.
Reviewed diff at a9c7146. No high-confidence findings.
— reviewed via DragonnZhang /review
Generated by Claude Code
| @@ -3158,65 +3477,78 @@ export class Session implements SessionContext { | |||
| null; | |||
There was a problem hiding this comment.
Bug: const recordedModelFacingTurn = false — #recordModelFacingUserTurn is never called in the notification send path, making the rollback calls dead code.
In every other send path (main loop, stop-hook continuation, cron), the pattern is:
let recordedModelFacingTurn = false;
// ...
recordedModelFacingTurn = this.#recordModelFacingUserTurn(nextMessage);In this background-notification path the variable is const false and #recordModelFacingUserTurn is never called, so both #rollbackModelFacingUserTurn(recordedModelFacingTurn) calls (in the !sendResult.responseStream branch and the catch block) are permanent no-ops.
Currently this is functionally correct because notification Content entries fail isApiUserTextContent (tested), so #recordModelFacingUserTurn would have returned false anyway and the counter would be unchanged. But the misleading structure means a future editor who adds a notification variant that does pass isApiUserTextContent will silently get counter drift — exactly the class of bug this PR is fixing elsewhere.
Fix: either call #recordModelFacingUserTurn before the send (matching the other three paths and letting it decide), or add a comment explaining that the notification send path intentionally skips counter tracking because notification content is never model-facing.
Generated by Claude Code
| throw RequestError.invalidParams( | ||
| undefined, | ||
| `modelFacingUserTurnCount ${validatedCount} exceeds visible model-facing user entries ${visibleTurnCount}`, | ||
| ); |
There was a problem hiding this comment.
Bug: validateModelFacingUserTurnCountForHistory rejects valid compressed snapshots when maxKnownModelFacingUserTurnCount is 0 (fresh session, no prior rewinds loaded).
For the compressed-history branch (lines ~2984–2988), the upper-bound check is:
if (validatedCount > maxKnownModelFacingUserTurnCount) {
throw RequestError.invalidParams(...)
}maxKnownModelFacingUserTurnCount is this.maxModelFacingUserTurnCount, which starts at 0 and is only bumped by #recordModelFacingUserTurn or replayHistory. On a fresh session that has not yet replayed any history, this value is 0.
Failure scenario: an ACP client creates a new session and immediately calls restoreSessionHistory with a compressed snapshot { history: [summaryUser, summaryAck, thirdUser], modelFacingUserTurnCount: 3 }. The validation computes visibleTailTurnCount = 1, validatedCount = 3 >= 1 (first check passes), then 3 > 0 (second check throws 'exceeds known model-facing user entries 0'). The restore is rejected even though the snapshot is perfectly valid.
The guard is intended to catch a client lying about its count, but using an uninitialized 0 as the upper bound makes it too strict for the first restore into a session whose maxModelFacingUserTurnCount has never been seeded.
Fix options:
- Skip the upper-bound check when
maxKnownModelFacingUserTurnCount === 0(treat 0 as "unknown"). - Seed
maxKnownModelFacingUserTurnCountfrom the incomingmodelFacingUserTurnCountbefore the check (i.e., allow the first restore to establish the baseline). - Document that callers must invoke
replayHistorybeforerestoreHistoryfor compressed sessions and enforce this with a more explicit error.
Generated by Claude Code
DragonnZhang
left a comment
DragonnZhang
left a comment
There was a problem hiding this comment.
Found 1 issue: background-notification send path in Session.ts declares recordedModelFacingTurn as const false instead of calling #recordModelFacingUserTurn(nextMessage), causing modelFacingUserTurnCount to never be incremented for notification turns and making all rollback calls on that path no-ops.
Generated by Claude Code
| null; | ||
| let responseText = ''; | ||
| const streamStartTime = Date.now(); | ||
| const recordedModelFacingTurn = false; |
There was a problem hiding this comment.
Issue: recordedModelFacingTurn is declared with const and hardcoded to false instead of being assigned via this.#recordModelFacingUserTurn(nextMessage). This is the background-notification send path. Every other send path in this file (main prompt loop, stop-hook continuation, cron) correctly does:
let recordedModelFacingTurn = false;
// ...
recordedModelFacingTurn = this.#recordModelFacingUserTurn(nextMessage);But this path uses const recordedModelFacingTurn = false and never calls #recordModelFacingUserTurn. Consequences:
modelFacingUserTurnCountis never incremented after a successful background-notification turn, so the ACP rewind index mapping will be off by N (where N is the number of background notification turns).- The
#rollbackModelFacingUserTurn(recordedModelFacingTurn)calls in the null-stream and catch branches are permanently no-ops, which is benign only because the counter was never incremented — but it means both the rollback and the increment are silently absent.
Suggested fix: Change const to let and add the recording call immediately before the sendMessageStreamWithAutoCompression call, matching the other three paths:
let recordedModelFacingTurn = false;
let sendDispatched = false;
try {
recordedModelFacingTurn = this.#recordModelFacingUserTurn(nextMessage);
const sendResult = await this.#sendMessageStreamWithAutoCompression(...);
...
}Generated by Claude Code
|
@qwen-code /triage |
Code ReviewIndependent proposal (before reading diff): I'd detect compression by looking for the summary/ack pair at the API history boundary after startup context. Then count the remaining uncompressed user-text entries (the tail), and align them against the end of the UI turn list. Turns absorbed by compression become unreachable (return -1). I'd extract the shared detection/counting logic into a utility module used by both TUI and ACP paths. For the ACP side, I'd track a Comparison: the PR's approach matches this proposal closely. The tail-aligned mapping in Findings — no blockers: The The The No security concerns. No over-abstraction. No AGENTS.md violations. TestsUnit tests (all on the PR head, from the worktree):
Typecheck: CI: all checks successful (Test on macOS/Ubuntu/Windows Node 22, lint, PR review bot — all green). Tmux Smoke TestDev build starts and responds correctly on the PR head: The rewind-after-compression scenario requires an interactive TUI session (multi-turn conversation →
中文说明代码审查独立方案（阅读 diff 前）： 我会在 startup context 之后检测压缩（通过 summary/ack 对），然后计算剩余的未压缩 user-text 条目（tail），并将它们与 UI 轮次列表末端对齐。被压缩吸收的轮次变为不可达（返回 -1）。我会将共享的检测/计数逻辑提取为 TUI 和 ACP 共用的工具模块。ACP 端，我会跟踪 对比： PR 的方案与我的提案高度一致。 发现 — 无阻塞项：
无安全问题。无过度抽象。无 AGENTS.md 违规。 测试所有 710 个单元测试通过，typecheck 零错误，CI 全绿。 Tmux 烟雾测试Dev build 在 PR head 上正常启动并正确响应。压缩后 rewind 场景需要交互式 TUI 会话，无法通过 — Qwen Code · qwen3.7-max |
ReflectionStepping back, this is a well-executed bug fix that solves a real user problem. The rewind-after-compression bug was a silent correctness issue — the old code didn't crash, it just quietly truncated API history to the wrong position or rejected a valid rewind. That's arguably worse than a crash, because the user loses context without any indication something went wrong. The PR's approach matches my independent proposal. The tail-aligned mapping is the right strategy, and the implementation is clean. The shared Every change in the diff serves the stated goal. The prettier-only formatting in ~5 files is cosmetic rebase noise, not scope creep. The 710 unit tests pass. Two maintainers independently verified the fix with real tmux A/B testing, including mutation tests proving the new tests are non-vacuous. CI is green across all platforms. The one thing I'd flag for ongoing maintenance: the Verdict: this is a focused, correct, well-tested fix for a real user-facing bug. The previous scope concerns have been fully addressed. Ready to merge. 中文说明反思退后一步看，这是一个执行良好的 bug 修复，解决了真实的用户问题。压缩后 rewind 的 bug 是一个静默的正确性问题 — 旧代码不会崩溃，只是悄悄地把 API 历史截断到错误位置，或拒绝有效的 rewind。这 arguably 比崩溃更糟，因为用户在没有任何提示的情况下丢失了上下文。 PR 的方案与我的独立提案一致。尾对齐策略是正确的，实现干净。共享的 diff 中的每个改动都服务于既定目标。约 5 个文件的纯 prettier 格式化是 rebase 带来的装饰性噪音，不是范围蔓延。 710 个单元测试通过。两位维护者独立用真实 tmux A/B 测试验证了修复，包括变异测试证明新测试非空过。CI 在所有平台上全绿。 唯一标记的持续维护关注点： 结论： 这是一个聚焦、正确、测试充分的修复，解决了真实的用户可见 bug。之前的范围问题已完全解决。可以合并。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
@qwen-code /resolve |
|
Qwen Code resolved the merge conflicts and pushed the branch update. Conflict Resolution SummaryPR: #4242 — fix(cli): map rewind turns after compression Conflicts1.
|
qwen-code-dev-bot
dismissed stale reviews from qwen-code-ci-bot and wenshao
via
64f9c4f
| ): boolean { | ||
| if (!content || content.role !== 'user') return false; | ||
|
|
||
| return ( |
There was a problem hiding this comment.
[Critical] This predicate treats any user message that starts with one of the post-compact attachment prefixes as synthetic restoration content. After compression, the next real user prompt can legitimately begin with one of those strings, and getCompressionTailStartIndex() will then skip that real first tail turn. That makes the first reachable post-compression turn unreachable by rewind and shifts every later tail mapping by one. Please make the synthetic attachment block distinguishable by a non-user-forgeable shape/sentinel, or otherwise only skip the exact structure produced by composePostCompactHistory().
— GPT-5 Codex via Qwen Code /review
|
|
||
| return count; | ||
| const { history: apiHistory } = this.captureHistorySnapshot(); | ||
| return computeVisibleModelFacingUserTurnCount(apiHistory); |
There was a problem hiding this comment.
[Critical] After compression this now returns only the count of visible tail turns, but the sessionRewindSnapshots status endpoint still uses that value as a prefix cap (idx < rewindableTurnCount) for file-history snapshots. If the first three turns are compressed and two tail turns remain, this returns 2, so the endpoint exposes snapshots 0 and 1 (absorbed/unrewindable) while hiding the actually reachable snapshots 3 and 4. Clients using /session/:id/rewind/snapshots can offer invalid targets and miss valid recent rewinds. Please expose absolute reachable turn indexes/bounds from Session and filter snapshots against those instead of treating the tail count as a prefix length.
— GPT-5 Codex via Qwen Code /review
9 participants
What this PR does
Maps rewind targets correctly after conversation compression. The current revision is rebased onto
mainand squashed to the focused #4046 fix only: ACP model-facing turn counting, history snapshots, restore rollback, shared compression-aware API-history helpers, and compressed legacy history boundaries.Why it's needed
Compressed summaries collapse earlier user turns, so rewind must keep absorbed turns unreachable while preserving valid uncompressed tail turns. Without tail-aware mapping, sessions can reject a valid recent rewind or truncate to the wrong API-history index.
Reviewer Test Plan
How to verify
Run:
cd packages/cli && npx vitest run src/utils/api-history-utils.test.ts src/ui/utils/historyMapping.test.ts;cd packages/cli && npx vitest run src/acp-integration/session/Session.test.ts;cd packages/cli && npx vitest run src/acp-integration/acpAgent.test.ts;cd packages/core && npx vitest run src/services/sessionService.test.ts src/services/chatCompressionService.test.ts src/services/postCompactAttachments.test.ts;cd packages/vscode-ide-companion && npx vitest run src/webview/handlers/SessionMessageHandler.test.ts;npm run typecheck --workspace=packages/cli;npm run check-types --workspace=packages/vscode-ide-companion;npm run lint --workspace=packages/cli;npm run lint --workspace=@qwen-code/qwen-code-core;npx prettier --checkon the changed files.Evidence (Before & After)
Before: rewind mapping could treat compressed or restored-history turns as reachable incorrectly, or reject a valid uncompressed tail turn after compression. After: tests cover compressed-tail mapping, first-turn rejection after compression, snapshot restore, invalid snapshot rejection, send-failure rollback, stop-hook/cron/notification counters, slash-command resume parity, legacy compressed history lower-bound handling, and VS Code ACP history snapshot rollback. This is session/history behavior, so TUI screenshots are not required for the code-level paths; maintainer verification has already reproduced the TUI before/after behavior for #4046.
Tested on
Environment (optional)
Local macOS checkout with repository npm workspaces. The branch is a single focused commit on current
main; unrelated cache-diagnostics commits, merge commits, and review-ping commit were removed from the PR history.Risk & Scope
Linked Issues
Fixes #4046
中文说明
这个 PR 做了什么
修复压缩对话后的 rewind 映射。当前版本已基于最新
main重新整理并 squash，只保留 #4046 相关修复：ACP 模型可见轮次计数、history snapshot、restore rollback、共享的压缩感知 API history helper，以及 legacy 压缩历史边界处理。为什么需要
压缩摘要会折叠早期 user turns，rewind 必须拒绝已被压缩吸收的轮次，同时保留可达的未压缩 tail turns。否则会误拒绝有效 rewind，或截断到错误的 API history index。
Reviewer Test Plan
本地 macOS 跑了 CLI/core/VS Code 相关单测、CLI typecheck、VS Code check-types、CLI/core lint、changed-files prettier check；Windows/Linux 依赖 GitHub CI。
风险和范围
只影响 ACP/session rewind mapping 和历史快照 bookkeeping，不改压缩算法或 UI。无关 cache diagnostics、provider、extension、merge commit 和催 review commit 已从 PR 历史中移除。