fix(core): guard compression request admission - #9541
Conversation
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Thanks for the fix, @AaronZ345 — the linked issue (#9455) is a confirmed P1 bug (compression admission skipped when the compression model is the main model, stale cache-sharing anchor, thoughtSignature undercount), so this direction is worth pursuing. The blocker here is form, not substance: the PR description doesn't follow the repository's PR template. It currently has only free-form ## Summary and ## Test plan sections, and none of the template's required headings are present.
Please edit the PR body to fill in the template at https://github.com/QwenLM/qwen-code/blob/main/.github/pull_request_template.md — in particular:
- What this PR does and Why it's needed — prose sections (your Summary bullets are a good starting point; "Why" should reference the observed overflow sequence from the issue)
- Reviewer Test Plan — How to verify with expected vs observed behavior. This changes compression admission behavior, so describe a concrete scenario a reviewer can run (e.g. how to exercise the same-model compression path and what should now fail locally instead of being sent). If nothing is user-visible, write N/A under Evidence (Before & After) rather than omitting it
- Tested on — the OS table
- Risk & Scope — main risk/tradeoff, and what is not validated
- Linked Issues —
Fixes #9455 - The Chinese translation in a
<details>block, section by section as the template asks
Once the description follows the template, a maintainer — or @qwen-code /triage — can re-run the gate and the review will proceed to the code.
中文说明
感谢提交这个修复，@AaronZ345 —— 关联的 issue（#9455）是已确认的 P1 bug（压缩模型即主模型时准入检查被跳过、缓存共享锚点过期、thoughtSignature 计数遗漏），方向是对的。当前的阻碍是格式而非内容：PR 描述没有遵循仓库的 PR 模板，目前只有自由格式的 ## Summary 和 ## Test plan 两节，模板要求的各个标题均缺失。
请编辑 PR 正文，补全模板 https://github.com/QwenLM/qwen-code/blob/main/.github/pull_request_template.md 中的各项，尤其是：
- What this PR does 与 Why it's needed —— 用 prose 描述（现有的 Summary 列表可以作为起点；"Why" 部分应引用 issue 中观测到的超限过程）
- Reviewer Test Plan —— How to verify，写明期望行为与实际行为。本 PR 修改了压缩准入行为，请给出评审者可运行的具体场景（例如如何触发"压缩模型即主模型"路径、现在应当本地失败而不是发出请求）。如果没有用户可见变化，Evidence (Before & After) 一栏写 N/A，而不是省略
- Tested on —— 操作系统表格
- Risk & Scope —— 主要风险/取舍，以及未验证的内容
- Linked Issues ——
Fixes #9455 - 按模板要求在
<details>块中逐节附上中文翻译
描述补全后，maintainer —— 或 @qwen-code /triage —— 可以重新运行准入检查，评审会继续进入代码审查。
— Qwen Code · qwen3.8-max
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Test Plan (not a blocker): src/services/compactionInputSlimming.test.ts — no such file or directory; src/core/geminiChat.test.ts — no such file or directory.
— qwen3.8-max via Qwen Code /review (v0.21.14)
| const reduced = microcompactHistory( | ||
| slim.slimmedHistory, | ||
| null, | ||
| config.getClearContextOnIdle?.() ?? {}, | ||
| { force: true }, | ||
| ); |
There was a problem hiding this comment.
[Critical] This new admission-time microcompact is the only microcompactHistory call site that omits preserveReadFileResult — the managed-memory retention exemption all three existing callers pass (client.ts pre-send, geminiChat.ts compressFast, memoryPressureMonitor.ts, each with (filePath) => isManagedMemoryPath(filePath, projectRoot, targetDir)). Without it, a managed-memory read_file result (an auto-memory file) that sits outside the keepRecent tail is blanked out of the summarizer input, and the resulting summary permanently drops that content from the model's context: on COMPRESSED the entire history is replaced by the summary, and post-compact restoration does not cover this case — it re-attaches at most the 5 most recent reads filtered by isInsideWorkspace, while managed-memory roots live outside the workspace by default.
A session that reads an auto-memory file early, runs many tool turns, and then hits this admission path (exactly the oversize case this code exists for) gets its loaded memory silently evicted after a successful compression — no diagnostic, while every pre-existing microcompaction path deliberately retains that content. Before this PR no microcompaction ran on the admission path, so the loss route is PR-born.
Verified by probe against the unmodified PR: with an early auto-memory read_file plus six later tool results, the summarizer payload arrived with the memory content replaced by [Old tool result content cleared] (memory marker present: false, three cleared placeholders) and compression returned COMPRESSED; with the fix below patched in, the memory content is present in the payload (marker present: true, two placeholders) — the probe assertion flips, and reverting re-confirms the PR behavior.
One note for the fix: preserving a large memory read can leave the post-reduction estimate above the admission gate, so compression fails locally (COMPRESSION_FAILED_INPUT_TOO_LARGE) instead of being sent. That is the safe failure mode (history intact), but it deserves a test or a comment so it is not read later as a regression of this PR's admission improvement.
const projectRoot = config.getProjectRoot();
const targetDir = config.getTargetDir?.() ?? projectRoot;
const reduced = microcompactHistory(
slim.slimmedHistory,
null,
config.getClearContextOnIdle?.() ?? {},
{
force: true,
preserveReadFileResult: (filePath) =>
isManagedMemoryPath(filePath, projectRoot, targetDir),
},
);— qwen3.8-max via Qwen Code /review (v0.21.14)
| status === CompressionStatus.COMPRESSION_FAILED_OUTPUT_TRUNCATED || | ||
| status === CompressionStatus.COMPRESSION_FAILED_INPUT_TOO_LARGE |
There was a problem hiding this comment.
[Suggestion] isCompressionFailureStatus now exists as two hand-maintained copies — this one and the identical predicate in packages/core/src/core/geminiChat.ts — while the CompressionStatus enum lives in core (turn.ts) and is already re-exported through core's index.ts, and this file imports CompressionStatus from core. This PR had to add the new member to both copies in lockstep, and the copies already drift cosmetically (member order differs between them). On top of that, no test exercises this ACP-side copy's classification of the new status — the Session.test.ts guard tests only drive COMPRESSION_FAILED_EMPTY_SUMMARY and NOOP.
The cost is concrete: the next COMPRESSION_FAILED_* status added to the enum and the core copy but missed here is silently classified as non-failure in ACP sessions — compressionFailed stays false, the stop-guard proceeds as if compression had not failed, and no test turns red, while the TUI path counts the same status toward its breaker.
Consider exporting a single isCompressionFailureStatus from core next to the enum and importing it at both sites, plus adding a Session.test.ts case mirroring the existing COMPRESSION_FAILED_EMPTY_SUMMARY guard tests with COMPRESSION_FAILED_INPUT_TOO_LARGE.
— qwen3.8-max via Qwen Code /review (v0.21.14)
| const directiveTokenCount = Math.ceil( | ||
| COMPRESSION_REQUEST_DIRECTIVE.length / CHARS_PER_TOKEN, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The hoisted-estimate comment this admission gate joins (around lines 609-613) still enumerates the consumers of getColdInputEstimate() as two checks ("keeping the leading terms in one place so the two checks cannot drift"), but this diff adds a third consumer: the admission gate, which adds the directive plus margin and minimum output, invalidates the memo, and recomputes after microcompaction (call sites now at ~640, 677, 694, 729).
The comment exists solely so future editors audit every consumer before changing the estimate's composition; as written, it invites auditing two of the three and reintroducing exactly the drift it promises is impossible. Updating it to name the admission gate (three checks) keeps that guarantee true.
— qwen3.8-max via Qwen Code /review (v0.21.14)
| const sharedCurrentRouteTokenEstimate = | ||
| estimateContentTokens( | ||
| sideQueryHistory, | ||
| slimmingConfig.imageTokenEstimate, | ||
| ) + |
There was a problem hiding this comment.
[Suggestion] These shared-cache admission estimates — a full O(history) walk of the unslimmed history here, plus the JSON.stringify of the entire generation config (system prompt and all tool schemas) just above — are computed before the cheap booleans (usesMainModel, providerSupportsCacheSharing, hasProviderTokenCount) that decide whether cache sharing is even possible. Before this PR sharedRequestFits was a cheap arithmetic expression; the unconditional cost is new.
A session with a distinct compaction model configured, or on a provider that fails supportsCompressionCacheSharing, pays both the full-history estimation walk and a transient hundreds-of-KB serialization on every compaction — at exactly the memory-pressure moment compaction runs — even though canShareCache is already statically false and the only other consumer of these numbers (the "shared request exceeds context window" debug branch) is unreachable in that case. Computing them only when all three booleans hold (the debug reason string needs them in that same branch) removes the waste.
— qwen3.8-max via Qwen Code /review (v0.21.14)
| coldInput = { ...slim, slimmedHistory: reduced.history }; | ||
| cachedColdInputEstimate = undefined; | ||
| slim = coldInput; | ||
| coldRequestInputTokens = getColdInputEstimate() + directiveTokenCount; |
There was a problem hiding this comment.
[Suggestion] The compaction-model window guard (around lines 628-657) demotes effectiveCompactionModel to the main model using the pre-microcompaction memoized estimate, but the bounded reduction this PR adds runs later, inside runColdCompression — nothing re-evaluates the guard or budgetWindow afterwards. Before this PR no such reduction existed, so the guard's estimate was final; the discrepancy is PR-born.
With an explicitly configured cheaper/smaller-window compaction model — the configuration the guard was built for — a tool-heavy session whose post-microcompaction payload would fit the compaction window still demotes silently to the more expensive main model (with the "context window too small; using the main model" warning), and since budgetWindow then stays at the larger main window the reduction may be skipped entirely. Running the slimming + microcompaction before the guard, or re-evaluating the guard against the post-reduction estimate, restores the intended model choice.
— qwen3.8-max via Qwen Code /review (v0.21.14)
| null, | ||
| config.getClearContextOnIdle?.() ?? {}, |
There was a problem hiding this comment.
[Suggestion] This passes the live session's clearContextOnIdle retention settings into the admission microcompact, so toolResultsNumToKeep (default 5, and the QWEN_MC_KEEP_RECENT env override) now caps how much the admission pass may clear — coupling an unrelated user setting to whether compression can run at all. The compressFast precedent does not argue for the reuse: there the history being mutated is the live history, while here the copy is throwaway side-query input.
Verified by probe: with identical 8×30K-char tool-result history in a 50K window, toolResultsNumToKeep: 50 makes compress return COMPRESSION_FAILED_INPUT_TOO_LARGE with no request sent, while toolResultsNumToKeep: 1 proceeds; patching in a fixed minimal retention flips the first case to COMPRESSED. A user who raised the knob to retain live-session context would hit this wall on every compression attempt — breaker trips after three strikes, and manual /compress never gets through.
If reusing the live setting is intentional, a comment saying so would prevent this reading; otherwise a fixed minimal retention fits this pass's purpose:
| null, | |
| config.getClearContextOnIdle?.() ?? {}, | |
| null, | |
| { toolResultsNumToKeep: 1 }, |
— qwen3.8-max via Qwen Code /review (v0.21.14)
| compactionWarning = | ||
| `Compression input too large: estimated input ` + |
There was a problem hiding this comment.
[Suggestion] This locally-decidable rejection runs after firePreCompactEvent, so hooks with side effects execute for compressions that are guaranteed to send nothing — the case the pre-hook cheap gates say they exist to avoid. The other failure statuses genuinely cannot be known before the request; this one can, since its inputs (the slimmed estimate and budgetWindow) do not depend on hook output beyond the 4000-char-capped hookExtraInstructions.
Verified by probe: an irreducible oversized history plus a hook system produced COMPRESSION_FAILED_INPUT_TOO_LARGE with no request sent and firePreCompactEvent called once — a hook fired for a compression guaranteed to fail locally. An irreducible session crossing the auto threshold pays up to three phantom hook fires before the breaker trips, and manual /compress re-fires the hook on every attempt indefinitely since force bypasses the breaker. A conservative admission pre-check before the hook (base system prompt without hook instructions, plus MAX_HOOK_INSTRUCTIONS_CHARS as slack) would close this; if the post-hook placement is a deliberate tradeoff, saying so in the hook-gate comment would settle it.
— qwen3.8-max via Qwen Code /review (v0.21.14)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Test Plan (not a blocker): src/services/compactionInputSlimming.test.ts — no such file or directory; src/core/geminiChat.test.ts — no such file or directory.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/core/src/core/turn.ts:383 — [review] New INPUT_TOO_LARGE status has no rendering/handling case in the unchanged /compress consumers (CompressionMessage renders empty; compressCommand reports success-shaped output)
中文说明
Test Plan（非阻断）：src/services/compactionInputSlimming.test.ts — no such file or directory; src/core/geminiChat.test.ts — no such file or directory。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| let preHookInputTokens = estimateColdRequestInput( | ||
| preHookSystemInstruction, | ||
| ); | ||
| if (coldRequestCannotFit(preHookInputTokens, contextLimit)) { |
There was a problem hiding this comment.
[Critical] This pre-hook gate compares the payload against the main model's contextLimit even when a distinct compaction model is configured — but the window that actually receives the side-query in that case is the compaction model's window, resolved only later by the compaction-model guard (budgetWindow = window). Because canAttemptSharedRequestBeforeHook is always false when a distinct compaction model is configured, this gate always runs in that config — and a compaction model with a LARGER window than the main model is a supported configuration (the guard's budgetWindow = window branch exists precisely to use it, and no config validation relates the two windows).
Failure: small main window (e.g. 65,536 — the issue #7960 vLLM shape) plus a larger compaction window (e.g. 200K). An oversized text-heavy session triggers compression (/compress or the reactive overflow rescue); the slimmed estimate stays above main window − 2048 even after microcompaction, so this gate rejects locally with COMPRESSION_FAILED_INPUT_TOO_LARGE although the compaction model would serve the request — pre-PR the guard compared against the compaction window and compression succeeded. Each retry accrues a breaker strike (INPUT_TOO_LARGE is counted by isCompressionFailureStatus), the breaker latches after 3, and auto-compression is permanently off: the session can never compact — reintroducing exactly the stall #9455 set out to remove, for this config. No test covers the larger-window direction; the only distinct-model test uses a SMALLER compaction window.
Verified by probe against the unmodified PR (main window 65,536 + compaction window 200,000 + 255,000-char history, force: true):
PR: COMPRESSION_FAILED_INPUT_TOO_LARGE, 0 side queries sent,
warning "estimated input 64,667 tokens cannot leave 1,024 usable
output tokens within the 65,536-token context window"
PATCH: gate admits against max(main, compaction window) → COMPRESSED,
side query sent to the compaction model
Suggested fix: resolve the receiving window the same way the guard does and gate against Math.max(contextLimit, compactionModelWindow) — or skip the pre-hook local failure when a distinct compaction model is configured, deferring to the guard and the runColdCompression admission, which both already use budgetWindow. Please add a test with compaction window > main window.
中文说明
该预钩子准入检查即使在配置了独立压缩模型时，也只按主模型的 contextLimit 判断——但此时真正接收 side-query 的窗口是压缩模型的窗口，要到下方的压缩模型守卫才会解析（budgetWindow = window）。由于配置了独立压缩模型时 canAttemptSharedRequestBeforeHook 恒为 false，此检查在该配置下必然执行；而压缩模型窗口大于主模型是受支持的配置（守卫的 budgetWindow = window 分支正是为此存在，且配置层没有任何校验约束两者关系）。
失败场景：主窗口较小（如 65,536——issue #7960 的 vLLM 部署形态）+ 压缩窗口较大（如 200K）。超大的文本为主的会话触发压缩（/compress 或溢出救援）时，即使 microcompaction 后估算仍高于 主窗口 − 2048，此检查会以 COMPRESSION_FAILED_INPUT_TOO_LARGE 本地拒绝——尽管压缩模型本可以处理该请求；PR 之前守卫会按压缩模型窗口比较并成功压缩。每次重试都会累计熔断计数（INPUT_TOO_LARGE 被 isCompressionFailureStatus 计入），3 次后熔断器锁死，自动压缩永久关闭：会话永远无法再压缩——对该配置重新引入了 #9455 要消除的卡死。现有测试没有覆盖"压缩窗口更大"的方向，唯一的独立模型测试用的是更小的压缩窗口。
已用探针在未修改的 PR 上验证（主窗口 65,536 + 压缩窗口 200,000 + 255,000 字符历史，force: true）：
PR: COMPRESSION_FAILED_INPUT_TOO_LARGE，0 次 side query，
警告 "estimated input 64,667 tokens cannot leave 1,024 usable
output tokens within the 65,536-token context window"
补丁: 准入改为 max(主窗口, 压缩窗口) → COMPRESSED，
side query 发往压缩模型
建议修复：按守卫相同的方式解析接收窗口，并以 Math.max(contextLimit, compactionModelWindow) 作为准入窗口；或在配置了独立压缩模型时跳过预钩子本地失败，交由守卫与 runColdCompression 准入决定（二者都已使用 budgetWindow）。请补充"压缩窗口 > 主窗口"的测试。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| const reduceColdInputForAdmission = () => { | ||
| const slim = getColdInput(); | ||
| const reduced = microcompactHistory( |
There was a problem hiding this comment.
[Critical] This reduction step strips tokens out of the side-query input, but the usage-based post-compression token math (compressionInputTokenCount - 1000 - pendingToolResultTokenCount, ~line 1284) still assumes the side-query input covered the FULL visible history — which is no longer true once admission has reduced it. The blanked bulk is invisible to compressedHistoryTokenCount, so newTokenCount = originalTokenCount − compressedHistoryTokenCount + summary + restoration (restoration alone can add up to maxRecentFiles × ~5K tokens per the comment near line 1294) can exceed originalTokenCount, tripping the inflation guard (~line 1379) and discarding a perfectly valid summary.
Failure: a tool-heavy session (old read_file/grep results dominating the window) on a provider that reports side-query usage: admission reduces the payload to the last tool result, the cold request succeeds with a small reported prompt count and a valid summary — then the math trips and the return is COMPRESSION_FAILED_INFLATED_TOKEN_COUNT with newHistory: null. That status strikes the breaker; the session stays oversized, every rescue repeats the same arithmetic, the breaker latches, and the session can never recover — the exact #9455 symptom, reintroduced for the tool-heavy sessions this reduction path targets. When the inequality does not trip, the same skew still over-reports newTokenCount into telemetry by the blanked amount.
Verified by probe against the unmodified PR (tool-heavy history, 50K window, originalTokenCount 45,000, provider-reported usage 1,200/3,000):
PR: COMPRESSION_FAILED_INFLATED_TOKEN_COUNT, newHistory null,
newTokenCount 47,800 > originalTokenCount 45,000 — valid summary
discarded although the reduced payload was admitted
PATCH: add reduced.meta.tokensSaved back into the math → COMPRESSED,
summary persisted (probe flips; patch reverted)
Suggested fix: accumulate reduced.meta?.tokensSaved ?? 0 across reduceColdInputForAdmission() calls (the meta field exists for exactly this bookkeeping) and add it back into compressedHistoryTokenCount at ~line 1284 — or take the already-present local-estimate branch whenever the cold input was reduced.
中文说明
该缩减步骤会从 side-query 输入中去掉 token，但压缩后基于 usage 的 token 计算（compressionInputTokenCount - 1000 - pendingToolResultTokenCount，约第 1284 行）仍假设 side-query 输入覆盖完整可见历史——准入缩减之后这不再成立。被清掉的部分对 compressedHistoryTokenCount 不可见，于是 newTokenCount = originalTokenCount − compressedHistoryTokenCount + 摘要 + 恢复项（仅恢复项最多可达 maxRecentFiles × 约 5K token，见约第 1294 行注释）可能超过 originalTokenCount，触发膨胀守卫（约第 1379 行）并丢弃一份完全有效的摘要。
失败场景：工具结果为主的会话（旧的 read_file/grep 结果占满窗口）且 provider 上报 side-query 用量：准入将payload缩减到仅剩最近一个工具结果，冷请求以较小的上报 prompt 数成功并得到有效摘要——随后上述计算触发，返回 COMPRESSION_FAILED_INFLATED_TOKEN_COUNT 且 newHistory: null。该状态计入熔断；会话仍然超大，每次救援都重复同样的算式，熔断器锁死，会话永远无法恢复——正是 #9455 的症状，被这条缩减路径重新引入到它本要服务的高工具负载会话上。即使不等式未触发，同样的偏差也会让上报的 newTokenCount 虚高被清掉的那部分。
已用探针在未修改的 PR 上验证（工具为主的历史、50K 窗口、originalTokenCount 45,000、上报用量 1,200/3,000）：
PR: COMPRESSION_FAILED_INFLATED_TOKEN_COUNT，newHistory 为 null，
newTokenCount 47,800 > originalTokenCount 45,000——缩减后的payload
已被准入，有效摘要仍被丢弃
补丁: 把 reduced.meta.tokensSaved 加回计算 → COMPRESSED，摘要保留
（探针翻转；补丁已回退）
建议修复：在多次 reduceColdInputForAdmission() 调用间累计 reduced.meta?.tokensSaved ?? 0（该 meta 字段正是为此记录而存在），并加回约第 1284 行的 compressedHistoryTokenCount；或在冷输入曾被缩减时改走已有的本地估算分支。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| // minimum payload smaller. Skip this cold-path work while a cache-sharing | ||
| // request is still possible; that path deliberately preserves the full | ||
| // history and may succeed without any slimming. | ||
| if (!canAttemptSharedRequestBeforeHook) { |
There was a problem hiding this comment.
[Suggestion] This skip condition is a cache-sharing precondition check, not a fit check — so side-effecting PreCompact hooks still fire for inputs where neither the shared request nor the cold request can fit, contradicting the invariant the comment above states ("Do not fire side-effecting hooks for an input that cannot fit even with zero hook output"). This is the sibling entrance of round-1 R1-7: the cold-path ordering was fixed (the rejection now runs before the hook when cache sharing is impossible), but this skip path still fires hooks before a locally-decidable rejection.
Failure: an Anthropic/Gemini session (cache-sharing-capable, provider-reported anchor present, no distinct compaction model) with a text-dominated history that fills the window (~170K tokens in a 200K window): this block is skipped, the PreCompact hook fires (hooks do transcript dumps / external notifications), then the shared request fails its fit check and the cold admission also fails (nothing to slim or microcompact) → COMPRESSION_FAILED_INPUT_TOO_LARGE. The hook's side effects ran for a compression that never happened; an irreducible session crossing the auto threshold pays up to three phantom hook fires before the breaker trips, and manual /compress re-fires indefinitely since force bypasses the breaker.
Verified by probe (cache-sharing provider, 200K window, ~200K-token history, provider anchor 200,000): hook calls 1, then COMPRESSION_FAILED_INPUT_TOO_LARGE with 0 side queries. All inputs needed for a shared-fit lower bound are available pre-hook, and hook output can only inflate the shared prompt, so a pre-hook check is safe.
Suggested fix: pre-hook, compute the shared request's fit with the pre-hook system prompt and run the existing early-rejection check when the shared request already cannot fit, keeping the current skip only while a shared request is still possible — or document that PreCompact may fire for inputs later rejected locally when cache-sharing preconditions hold.
中文说明
该跳过条件是缓存共享的前置条件检查，而不是"能放下"的检查——因此当共享请求与冷请求都放不下时，带副作用的 PreCompact hook 仍会触发，与上方注释声明的不变量（"对于即使 hook 输出为零也放不下的输入，不触发带副作用的 hook"）矛盾。这是上一轮 R1-7 的兄弟入口：冷路径的顺序已修复（缓存共享不可能时，拒绝已移到 hook 之前），但这条跳过路径仍会在本地可判定的拒绝之前触发 hook。
失败场景：Anthropic/Gemini 会话（支持缓存共享、有 provider 上报锚点、无独立压缩模型），文本为主的历史占满窗口（200K 窗口中约 170K token）：此分支被跳过，PreCompact hook 触发（hook 会做转录转储/外部通知），随后共享请求适配检查失败、冷路径准入也失败（无可缩减内容）→ COMPRESSION_FAILED_INPUT_TOO_LARGE。hook 的副作用为一次从未发生的压缩执行了；自动阈值附近不可缩减的会话在熔断前最多白触发三次 hook，而手动 /compress 因 force 绕过熔断会无限重复触发。
已用探针验证（支持缓存共享的 provider、200K 窗口、约 200K token 历史、provider 锚点 200,000）：hook 调用 1 次，随后 COMPRESSION_FAILED_INPUT_TOO_LARGE，0 次 side query。共享适配下界所需的全部输入在 hook 之前都可用，且 hook 输出只会增大共享 prompt，因此预钩子检查是安全的。
建议修复：在 hook 之前用预钩子系统提示词计算共享请求适配性，若共享请求已确定放不下则执行现有的提前拒绝检查；仅在共享请求仍可能时保留当前跳过——或者明确注释说明：当缓存共享前置条件成立时，PreCompact 可能为之后被本地拒绝的压缩而触发。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| reduceColdInputForAdmission(); | ||
| slimmedTokenEstimate = | ||
| getColdInputEstimate() + COMPACT_MAX_OUTPUT_TOKENS; |
There was a problem hiding this comment.
[Suggestion] The guard microcompacts the cold input BEFORE deciding whether the compaction model can be kept, and reduceColdInputForAdmission() mutates the persistent coldInput closure — so when the re-estimate still does not fit and the guard falls back to the main model, the main-model request is sent with old tool results blanked even when the full slimmed input fits the main window fine.
Failure: distinct compaction model with a small window (e.g. 32K), main model with a large window (e.g. 262K), payload ~150K tokens. Guard: 150K + 20K > 32K → reduction blanks every old tool result except the last → re-estimate still > 32K → fallback to the main model. The cold request then goes to the 262K-window main model using the microcompacted input, although the unreduced input would have fit with a full 20K budget. The summary is produced from degraded input (pre-PR this scenario sent the complete slimmed input), losing old tool outputs from the summarizer's view — the content whose preservation is compression's whole purpose.
Verified by probe (compact-model 32K, main 262K, ~171K-token payload with one old tool result): guard warning "context window (32,000 tokens) is too small for the current payload (~170,990 tokens); using the main model", then the side query went to the main model with the cleared-marker present in the payload.
Suggested fix: snapshot coldInput and cachedColdHistoryEstimate before calling reduceColdInputForAdmission() in the guard and restore them in the fallback branch (or decide the fallback before mutating the input).
中文说明
守卫在判断压缩模型是否可保留之前就对冷输入做 microcompaction，而 reduceColdInputForAdmission() 会修改持久化的 coldInput 闭包——因此当重新估算仍放不下、守卫回退到主模型时，即使完整的瘦身后输入完全放得进主窗口，发给主模型的请求也已经是清掉了旧工具结果的版本。
失败场景：独立压缩模型窗口较小（如 32K），主模型窗口较大（如 262K），payload 约 150K token。守卫：150K + 20K > 32K → 缩减把除最近一个之外的旧工具结果全部清空 → 重新估算仍 > 32K → 回退到主模型。随后冷请求带着被 microcompact 的输入发往 262K 窗口的主模型，尽管未缩减的输入本可以带着完整 20K 预算放下。摘要基于降级的输入生成（PR 之前该场景发送完整瘦身输入），旧工具输出从摘要器视野中丢失——而保留这些内容正是压缩的目的。
已用探针验证（压缩模型 32K、主模型 262K、约 171K token 且含一个旧工具结果）：守卫警告 "context window (32,000 tokens) is too small for the current payload (~170,990 tokens); using the main model"，随后 side query 发往主模型，且payload中带有清除标记。
建议修复：在守卫中调用 reduceColdInputForAdmission() 之前快照 coldInput 与 cachedColdHistoryEstimate，并在回退分支中恢复（或先决定是否回退，再决定是否修改输入）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| if (coldRequestCannotFit(coldRequestInputTokens, budgetWindow)) { | ||
| compactionWarning = buildInputTooLargeWarning( | ||
| coldRequestInputTokens, |
There was a problem hiding this comment.
[Suggestion] The cold-path admission rejection (here, plus return undefined) and its !summaryResult → COMPRESSION_FAILED_INPUT_TOO_LARGE mapping (~line 1011) have no test: every INPUT_TOO_LARGE assertion in this PR exercises only the pre-hook early return, because all four such tests use fixtures with getLastPromptTokenCount = 0 or absent, forcing canAttemptSharedRequestBeforeHook false.
Failure: this cold-side branch is the only guard for a session with a provider-reported anchor on a cache-sharing-capable provider whose irreducible history exceeds window − 2048 — pre-hook is skipped, the shared request fails its fit check, and this rejection must fire. Deleting either this block or the final mapping keeps every current test green and restores the exact #7960 degenerate behaviour this PR removes (a floored 1-token-budget send whose fragment is rejected as OUTPUT_TRUNCATED, plus a real API call) for that session shape.
Verified by mutation on the restored baseline (145/145 green): deleting this rejection block → suite still 145/145 green; changing the !summaryResult mapping's status to EMPTY_SUMMARY → suite still 145/145 green (both reverted).
Suggested fix: add one admission test whose fixture returns a non-zero getLastPromptTokenCount (and cache-sharing-capable authType) with an irreducibly oversized history and a small window; assert COMPRESSION_FAILED_INPUT_TOO_LARGE, the warning, and that neither generateText nor runSideQuery is called.
中文说明
冷路径准入拒绝（此处加 return undefined）及其 !summaryResult → COMPRESSION_FAILED_INPUT_TOO_LARGE 映射（约第 1011 行）没有任何测试：本 PR 所有 INPUT_TOO_LARGE 断言都只覆盖预钩子提前返回，因为这四个测试的 fixture 都让 getLastPromptTokenCount 为 0 或缺失，从而 canAttemptSharedRequestBeforeHook 恒为 false。
失败场景：对于"支持缓存共享的 provider、有 provider 上报锚点、且不可缩减历史超过 窗口 − 2048"的会话，这条冷路径分支是唯一的守卫——预钩子被跳过、共享请求适配失败，必须由该拒绝兜底。删除此块或最终映射，现有测试全部仍为绿，并对该会话形态恢复本 PR 要消除的 #7960 退化行为（以 1-token 兜底预算发出请求、其片段被 OUTPUT_TRUNCATED 拒绝，外加一次真实 API 调用）。
已在恢复的基线上做变异验证（145/145 绿）：删除该拒绝块 → 测试仍 145/145 绿；把 !summaryResult 映射的状态改为 EMPTY_SUMMARY → 仍 145/145 绿（均已回退）。
建议修复：新增一个准入测试，fixture 返回非零 getLastPromptTokenCount（及支持缓存共享的 authType），历史不可缩减且超大、窗口较小；断言 COMPRESSION_FAILED_INPUT_TOO_LARGE、警告内容，且 generateText 与 runSideQuery 均未被调用。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| if (coldRequestCannotFit(coldRequestInputTokens, budgetWindow)) { | ||
| reduceColdInputForAdmission(); | ||
| slim = getColdInput(); |
There was a problem hiding this comment.
[Suggestion] The admission invariant this block enforces (input + COMPACTION_BUDGET_SAFETY_MARGIN + MIN_COMPACTION_OUTPUT_TOKENS <= window before any send) guarantees coldOutputBudget >= 1024 on every path that reaches the truncation guard — which makes that guard's coldOutputBudget > 1 floor-regime branch and its two long comment paragraphs about the budget-1 regime (~lines 1075-1097) unreachable and stale. summaryResult is only ever set by the cache-sharing path (guard skipped via !usedCacheSharing) or by runColdCompression after admission, so at the guard coldOutputBudget ∈ [1024, 20000] and the ternary always takes its first operand. The updated computeCompactionOutputBudget docstring already says the caller's admission check rejects such requests, contradicting the guard's comments.
Cost: a future change to MIN_COMPACTION_OUTPUT_TOKENS, the admission check, or the guard must reason about documented-but-unreachable budget-1 semantics; the stale comments assert active protections for a regime that can no longer occur.
Suggested fix: simplify the threshold to outputCountIsEstimated ? COMPACT_MAX_OUTPUT_TOKENS : coldOutputBudget and delete the two floor-regime paragraphs from the guard's comment block.
中文说明
此块强制的准入不变量（任何发送前 输入 + COMPACTION_BUDGET_SAFETY_MARGIN + MIN_COMPACTION_OUTPUT_TOKENS <= 窗口）保证到达截断守卫的每条路径都有 coldOutputBudget >= 1024——这使得截断守卫中 coldOutputBudget > 1 的"预算 1"分支及其两段关于 budget-1 机制的长注释（约第 1075-1097 行）不可达且过期。summaryResult 只会由缓存共享路径（经 !usedCacheSharing 跳过守卫）或 runColdCompression 准入后设置，因此在守卫处 coldOutputBudget ∈ [1024, 20000]，三元表达式恒取第一个操作数。更新后的 computeCompactionOutputBudget 文档注释已说明调用方准入会拒绝此类请求，与守卫注释相互矛盾。
代价：未来修改 MIN_COMPACTION_OUTPUT_TOKENS、准入检查或守卫时，必须去推理一份"有文档但不可达"的 budget-1 语义；过期注释还在声称对一个不可能再发生的机制提供保护。
建议修复：将阈值简化为 outputCountIsEstimated ? COMPACT_MAX_OUTPUT_TOKENS : coldOutputBudget，并删除守卫注释块中的两段 floor-regime 描述。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| expect(coldSpy).toHaveBeenCalledTimes(1); | ||
| expect(JSON.stringify(coldSpy.mock.calls[0]![1].contents)).toContain( | ||
| managedMemoryMarker, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] This test asserts only the positive case — nothing verifies that a large read_file result of a NON-managed file is still cleared by reduceColdInputForAdmission. The preservation wiring (preserveReadFileResult: (filePath) => isManagedMemoryPath(...)) is new in this PR; if it regressed to preserving every read_file result regardless of path (e.g. the callback simplified to () => true), no test turns red: test 1's history contains only run_shell_command parts, this test asserts only that the managed marker survives, and isManagedMemoryPath's own unit tests test the predicate, not this call site.
Cost: a read_file-heavy session (large source/doc reads — a very common history shape) whose old read_file outputs dominate the window would then reduce by ~nothing at admission; the estimate stays above window − margin − 1024, compress returns COMPRESSION_FAILED_INPUT_TOO_LARGE, the breaker accrues, and the session becomes permanently unable to compact — silently reintroducing the #9455 stall class for the most common oversized-session shape.
Suggested fix: add a negative case to this describe block: a history with an old, oversized read_file functionResponse whose file_path is a plain workspace file (e.g. /tmp/test-workspace/src/main.ts), same fixture as this test, asserting the serialized cold payload contains [Old tool result content cleared] and not that file's content.
中文说明
该测试只断言了正向情况——没有测试验证非托管文件的大 read_file 结果仍会被 reduceColdInputForAdmission 清除。保留接线（preserveReadFileResult: (filePath) => isManagedMemoryPath(...)）是本 PR 新增的；如果它退化为无论路径一律保留所有 read_file 结果（例如回调被简化成 () => true），没有任何测试会变红：测试 1 的历史只含 run_shell_command，本测试只断言托管标记存活，而 isManagedMemoryPath 的单测测的是谓词本身而非此调用点。
代价：以 read_file 为主的会话（大量源码/文档读取——非常常见的历史形态）中，旧 read_file 输出占满窗口时，准入几乎缩减不了任何内容；估算保持在 窗口 − margin − 1024 之上，compress 返回 COMPRESSION_FAILED_INPUT_TOO_LARGE，熔断累计，会话永久无法压缩——对最常见的超大会话形态悄悄重新引入 #9455 的卡死类别。
建议修复：在此 describe 块中补充反向用例：历史中包含一个旧的、超大的 read_file functionResponse，其 file_path 是普通工作区文件（如 /tmp/test-workspace/src/main.ts），fixture 与本测试相同，断言序列化后的冷payload包含 [Old tool result content cleared] 且不含该文件内容。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| { toolResultsNumToKeep: 1 }, | ||
| { | ||
| force: true, | ||
| preserveReadFileResult: (filePath) => |
There was a problem hiding this comment.
[Suggestion] The "fixed minimal retention" for admission microcompaction is not fixed: microcompactHistory resolves keepRecent via resolveKeepRecent(process.env['QWEN_MC_KEEP_RECENT'], settings.toolResultsNumToKeep), which gives the env knob precedence over the hardcoded toolResultsNumToKeep: 1 here — so a session-tuning env var silently overrides the admission path's retention. The new test "uses fixed minimal retention for admission microcompaction" pins independence from config.getClearContextOnIdle() but cannot see the env path (unset in CI), so the pinned "fixed" behavior is green in CI and gone at runtime. This is the residual sibling of round-1 R1-6, whose settings-coupling was fixed.
Failure: a user who set QWEN_MC_KEEP_RECENT=50 to tune ordinary idle/size microcompaction (a supported knob with dedicated precedence tests in microcompact.test.ts) hits an oversized tool-heavy session: admission keeps up to 50 recent tool results — exactly the bulk that must go — the reduced estimate stays above window − 2048, compress returns COMPRESSION_FAILED_INPUT_TOO_LARGE, the breaker latches, and the session loses compression recovery — the fix this PR exists for silently doesn't apply to that user.
Verified by A/B probe (identical history and window, only the env var varied):
env unset: reduction fires (cleared marker present in payload)
QWEN_MC_KEEP_RECENT=50: COMPRESSION_FAILED_INPUT_TOO_LARGE
("estimated input 61,007 tokens…"), 0 side queries
Suggested fix: make the admission reduction immune to the knob — e.g. an explicit keepRecentOverride consulted before process.env['QWEN_MC_KEEP_RECENT'] in resolveKeepRecent, passed as 1 from here — or at minimum document that the env knob also governs admission retention.
中文说明
准入 microcompaction 的"固定最小保留"并不固定：microcompactHistory 通过 resolveKeepRecent(process.env['QWEN_MC_KEEP_RECENT'], settings.toolResultsNumToKeep) 解析保留数量，环境变量优先于这里硬编码的 toolResultsNumToKeep: 1——因此一个用于调会话行为的静默覆盖了准入路径的保留策略。新测试"uses fixed minimal retention for admission microcompaction"只钉住了对 config.getClearContextOnIdle() 的独立性，看不到环境变量路径（CI 中未设置），所以被钉住的"固定"行为在 CI 里是绿的、在运行时却消失了。这是上一轮 R1-6 的残留兄弟项（其设置耦合已修复）。
失败场景：用户为调节常规空闲/体积 microcompaction 设置了 QWEN_MC_KEEP_RECENT=50（受支持的调节项，microcompact.test.ts 有专门的优先级测试），遇到超大的工具为主会话：准入保留多达 50 个最近工具结果——恰恰是必须清掉的大头——缩减后估算仍高于 窗口 − 2048，compress 返回 COMPRESSION_FAILED_INPUT_TOO_LARGE，熔断锁死，会话失去压缩恢复能力——本 PR 要提供的修复对该用户静默失效。
已用 A/B 探针验证（历史与窗口完全相同，仅环境变量变化）：
未设置环境变量: 缩减生效（payload中出现清除标记）
QWEN_MC_KEEP_RECENT=50: COMPRESSION_FAILED_INPUT_TOO_LARGE
("estimated input 61,007 tokens…")，0 次 side query
建议修复：让准入缩减不受该调节项影响——例如在 resolveKeepRecent 中增加一个先于 process.env['QWEN_MC_KEEP_RECENT'] 的显式 keepRecentOverride，此处传 1；至少也应注释说明该环境变量同样管辖准入保留。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| compressionStatus: | ||
| CompressionStatus.COMPRESSION_FAILED_INPUT_TOO_LARGE, | ||
| warning, |
There was a problem hiding this comment.
[Suggestion] The new pre-send rejection paths (this pre-hook return and the cold-admission !summaryResult return) bypass logChatCompression — its single call site (~line 1348) is reached only after a side-query result exists — so a breaker-counting compression failure is invisible to telemetry. Of the sibling failure statuses, EMPTY_SUMMARY, TOKEN_COUNT_ERROR and INFLATED return after the telemetry call and are telemetered; OUTPUT_TRUNCATED also bypasses it (its two returns sit before the call — pre-existing, unchanged by this PR), so the new status joins that exception rather than being unique. It is, however, the only failure that accrues breaker strikes while emitting zero events.
Failure: an oversized tool-heavy session — the class this PR targets, and the one the findings above show can latch the breaker after 3 strikes — that is locally rejected pre-send produces zero chat_compression events while stalling permanently; production telemetry cannot distinguish "compression never attempted" from "compression locally rejected and breaker latched", so the frequency of the new rejection path is unmeasurable in the field.
Verified by probe on the unmodified PR: INPUT_TOO_LARGE rejection → logChatCompression calls 0, while the EMPTY_SUMMARY comparison arm → 1; adding a telemetry call at this return flips it to 1 (fix reverted).
Suggested fix: emit the compression telemetry event on the INPUT_TOO_LARGE returns as well (tokens_after = originalTokenCount, no input/output counts), or explicitly document that pre-send rejections are deliberately untelemetered like OUTPUT_TRUNCATED.
中文说明
新的发送前拒绝路径（此预钩子返回与冷准入的 !summaryResult 返回）绕过了 logChatCompression——它唯一的调用点（约第 1348 行）只有在 side-query 结果存在后才会到达——因此一个计入熔断的压缩失败对遥测不可见。兄弟失败状态中，EMPTY_SUMMARY、TOKEN_COUNT_ERROR、INFLATED 都在遥测调用之后返回、会被记录；OUTPUT_TRUNCATED 同样绕过（它的两个返回位于调用之前——既有行为，本 PR 未改动），所以新状态是加入该例外而非独有。但它是唯一一个"累计熔断计数却零事件"的失败。
失败场景：超大的工具为主会话——正是本 PR 针对的类别，也是上述发现表明 3 次失败即锁死熔断的类别——若在发送前被本地拒绝，则会产生零条 chat_compression 事件并永久卡死；生产遥测无法区分"从未尝试压缩"与"压缩被本地拒绝且熔断锁死"，新拒绝路径的发生频率在线上不可测量。
已用探针在未修改的 PR 上验证：INPUT_TOO_LARGE 拒绝 → logChatCompression 调用 0 次，而 EMPTY_SUMMARY 对照分支 → 1 次；在此返回处补一条遥测调用后翻转为 1（修复已回退）。
建议修复：在 INPUT_TOO_LARGE 返回处也发出压缩遥测事件（tokens_after = originalTokenCount，不带输入/输出计数），或明确注释说明发送前拒绝与 OUTPUT_TRUNCATED 一样有意不做遥测。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| expect(coldSpy).toHaveBeenCalledTimes(1); | ||
| const contents = coldSpy.mock.calls[0]![1].contents as Content[]; | ||
| const serialized = JSON.stringify(contents); |
There was a problem hiding this comment.
[Suggestion] All five admission success-path tests in this block (here, plus the tests at ~4691, ~4780, ~4856, ~4905) assert only the cold-request shape; none asserts result.info.compressionStatus === COMPRESSED or that newHistory is non-null, and the mocked usage is inconsistent with the reduced payload — so any post-side-query failure ships green in the very suite that pins the #9455 fix. The failure statuses (INFLATED_TOKEN_COUNT / EMPTY_SUMMARY / OUTPUT_TRUNCATED) carry no warning, so even test 4's expect(result.info.warning).toBeUndefined() passes on them.
This is not hypothetical: on the CURRENT code the "does not serialize shared-route config" test already terminates in COMPRESSION_FAILED_INFLATED_TOKEN_COUNT — its mocked usage (promptTokenCount 1,000 with originalTokenCount 1,000) makes newTokenCount 1,500 > 1,000 — and it passes green because nothing reads the result. One of the suite's success-path tests doesn't currently exercise the success path at all.
Verified by probe: a one-line mutation forcing every summary to be dropped as INFLATED → "Tests 7 passed" (the regression ships green); the same mutation plus status/newHistory assertions → 5 failed (expected 2 to be 1); those same assertions on the unmodified PR → the 1 failure named above.
Suggested fix: in the five success tests assert expect(result.info.compressionStatus).toBe(CompressionStatus.COMPRESSED) and expect(result.newHistory).not.toBeNull(); fix the "does not serialize shared-route config" fixture so its mocked usage is consistent with its originalTokenCount (promptTokenCount below originalTokenCount minus output).
中文说明
本块中全部五个准入成功路径测试（此处以及约 4691、4780、4856、4905 处的测试）都只断言冷请求的形状；没有一个断言 result.info.compressionStatus === COMPRESSED 或 newHistory 非空，且 mock 的用量与缩减后的payload不一致——因此任何 side-query 之后的失败都会在钉住 #9455 修复的测试套件里以绿灯通过。失败状态（INFLATED_TOKEN_COUNT / EMPTY_SUMMARY / OUTPUT_TRUNCATED）都不带 warning，所以连测试 4 的 expect(result.info.warning).toBeUndefined() 也会在这些状态上通过。
这并非假设：在当前代码上，"does not serialize shared-route config" 测试实际已经以 COMPRESSION_FAILED_INFLATED_TOKEN_COUNT 结束——其 mock 用量（promptTokenCount 1,000 而 originalTokenCount 1,000）使 newTokenCount 为 1,500 > 1,000——它仍然绿灯，因为没有代码读取结果。套件中的一个成功路径测试目前根本没有走成功路径。
已用探针验证：一行变异强制所有摘要被 INFLATED 丢弃 → "Tests 7 passed"（回归以绿灯放行）；同样的变异加上状态/newHistory 断言 → 5 个失败（expected 2 to be 1）；同样的断言放在未修改的 PR 上 → 出现上述 1 个失败。
建议修复：在五个成功测试中断言 expect(result.info.compressionStatus).toBe(CompressionStatus.COMPRESSED) 与 expect(result.newHistory).not.toBeNull()；修正 "does not serialize shared-route config" 的 fixture，使其 mock 用量与 originalTokenCount 一致（promptTokenCount 低于 originalTokenCount 减去输出）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: reverse audit — an auditor ran and opened its brief, but no agent was launched with the prompt the CLI built — the launch was written by hand, and what the agent was actually asked is not what this skill certifies.
Test Plan (not a blocker): src/services/compactionInputSlimming.test.ts — no such file or directory; src/core/geminiChat.test.ts — no such file or directory.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：反向审计——有审计 agent 运行并打开了自己的 brief，但没有 agent 是用 CLI 构建的 prompt 启动的——启动 prompt 是手写的，agent 实际被要求做的并不是本 skill 所认证的内容。
Test Plan（非阻断）：src/services/compactionInputSlimming.test.ts — no such file or directory; src/core/geminiChat.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| const preHookReceivingWindow = Math.max( | ||
| contextLimit, | ||
| configuredCompactionWindow ?? 0, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R3-1: The pre-hook gate admits the cold input against Math.max(contextLimit, configuredCompactionWindow ?? 0) with only the 2,048-token reserve, but the compaction-model guard below requires a 20,000-token reserve (COMPACT_MAX_OUTPUT_TOKENS) against that same compaction window to keep the model — so with a larger compaction window configured, the gate can approve an input that no model will accept, letting the side-effecting PreCompact hook fire for a compression that then fails locally with COMPRESSION_FAILED_INPUT_TOO_LARGE, contradicting the gate's own comment ("Do not fire side-effecting hooks for an input that cannot fit even with zero hook output").
Failure band: a compaction model whose window exceeds the main model's (a supported configuration) plus an irreducible text-dominated input in (compactionWindow − 20K, compactionWindow − 2,048] that also exceeds the main window. The gate admits, the hook fires (transcript dumps / checkpoints / notifications), the guard's reduction cannot recover ~18K tokens and falls back to the main model, and the final admission rejects. Verified by probe at this commit (main window 50K, compaction model 200K, ~185,920-token irreducible history):
PR: COMPRESSION_FAILED_INPUT_TOO_LARGE, preCompactHookCalls 1, sideQueryCalls 0,
warning names the 50,000-token main window (fallback happened post-hook)
FIX: gate predicate = input + 2,048 <= contextLimit
OR input + COMPACT_MAX_OUTPUT_TOKENS <= configuredCompactionWindow
→ preCompactHookCalls 0, same local rejection at the gate
The flip loses no rescue path: gate and guard invoke the identical deterministic reduceColdInputForAdmission, so anything the guard's reduction could rescue, the gate's own reduce-then-recheck already passes.
Suggested fix: model the per-model reserves in the gate instead of a single max() window — admit only when estimateColdRequestInput(...) + COMPACTION_BUDGET_SAFETY_MARGIN + MIN_COMPACTION_OUTPUT_TOKENS <= contextLimit OR estimateColdRequestInput(...) + COMPACT_MAX_OUTPUT_TOKENS <= configuredCompactionWindow, matching what the guard and the final admission actually enforce.
中文说明
预钩子准入仅以 2,048 token 的余量对 Math.max(contextLimit, configuredCompactionWindow ?? 0) 放行冷输入，但下方的压缩模型守卫需要对该压缩窗口保留 20,000 token（COMPACT_MAX_OUTPUT_TOKENS）才能保留该模型——因此当配置了更大的压缩窗口时，此检查会放行一个任何模型都无法接收的输入，导致带副作用的 PreCompact hook 为一次随后以 COMPRESSION_FAILED_INPUT_TOO_LARGE 本地失败的压缩而触发，与该检查自身的注释（"对于即使 hook 输出为零也放不下的输入，不触发带副作用的 hook"）矛盾。
失败区间：压缩模型窗口大于主模型（受支持的配置），且输入为不可缩减的文本为主、落在 (压缩窗口 − 20K, 压缩窗口 − 2,048] 且超过主窗口的区间。此时检查放行、hook 触发（转录转储/检查点/外部通知）、守卫的缩减无法回收约 18K token 而回退到主模型，最终准入拒绝。已在本提交上用探针验证（主窗口 50K、压缩模型 200K、约 185,920 token 的不可缩减历史）：PR 行为为 hook 触发 1 次、0 次 side query，警告文案提到 50,000 token 的主窗口（说明回退发生在 hook 之后）；应用建议的检查谓词后，hook 不再触发，同样的输入在检查处即被本地拒绝。翻转不丢失任何救援路径：检查与守卫调用同一个确定性的 reduceColdInputForAdmission，守卫缩减能救回的输入，检查自身的"缩减后再判断"同样会放行。
建议修复：在检查中按模型分别建模余量，而不是单一 max() 窗口——仅当 estimateColdRequestInput(...) + COMPACTION_BUDGET_SAFETY_MARGIN + MIN_COMPACTION_OUTPUT_TOKENS <= contextLimit 或 estimateColdRequestInput(...) + COMPACT_MAX_OUTPUT_TOKENS <= configuredCompactionWindow 时才放行，与守卫和最终准入实际执行的约束保持一致。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| let coldRequestInputTokens = | ||
| getColdInputEstimate() + compressionDirectiveTokenCount; | ||
| if (coldRequestCannotFit(coldRequestInputTokens, budgetWindow)) { |
There was a problem hiding this comment.
[Suggestion] R2-5: The cold-path admission rejection (this second admission site inside runColdCompression) and its !summaryResult → COMPRESSION_FAILED_INPUT_TOO_LARGE mapping still have no test: every INPUT_TOO_LARGE test in this PR terminates at the pre-hook gate, so the final safety net remains unpaired.
All four INPUT_TOO_LARGE tests take the pre-hook early return — none reaches runColdCompression. The deferred check is reachable when the pre-hook gate passes narrowly and a PreCompact hook then adds up to ~1K tokens (MAX_HOOK_INSTRUCTIONS_CHARS = 4000 chars) of additionalContext, pushing the post-hook estimate over budgetWindow. Verified by mutation probe at this commit: deleting this admission block keeps the whole suite green, while the suggested test flips between the PR and the mutant — a regression here would send a request that cannot leave the 1K minimum output (the issue #7960 shape) uncaught.
mutated suite (admission block deleted): Tests 148 passed (148)
probe vs mutated: AssertionError: expected 2 to be 7 (request sent despite the floored budget)
probe vs PR: Tests 1 passed (COMPRESSION_FAILED_INPUT_TOO_LARGE, no request sent)
Suggested fix: add one test where the pre-hook gate passes but firePreCompactEvent returns additionalContext large enough to push the post-hook cold estimate over budgetWindow, asserting COMPRESSION_FAILED_INPUT_TOO_LARGE, newHistory null, and that logChatCompression received the event per the !summaryResult block.
中文说明
冷路径准入拒绝（runColdCompression 内的第二个准入检查点）及其 !summaryResult → COMPRESSION_FAILED_INPUT_TOO_LARGE 映射仍然没有测试：本 PR 中所有 INPUT_TOO_LARGE 测试都在预钩子检查处终止，最后这道安全网依然没有被覆盖。
四个 INPUT_TOO_LARGE 测试全部走预钩子提前返回——没有一个能到达 runColdCompression。该延迟检查在如下场景可达：预钩子检查以较小余量通过，随后 PreCompact hook 追加最多约 1K token（MAX_HOOK_INSTRUCTIONS_CHARS = 4000 字符）的 additionalContext，使钩子后的估算超过 budgetWindow。已在本提交上用变异探针验证：删除该准入检查块后整个套件仍为绿灯；而补充上述建议测试后，探针在 PR 与变异体之间翻转——此处的回归会发出一个连 1K 最小输出都无法保留的请求（即 issue #7960 的形态），且不会被任何测试捕获。
建议修复：新增一个测试，让预钩子检查通过、但 firePreCompactEvent 返回足够大的 additionalContext，使钩子后的冷请求估算超过 budgetWindow，断言 COMPRESSION_FAILED_INPUT_TOO_LARGE、newHistory 为 null，且 logChatCompression 按 !summaryResult 分支收到事件。
— qwen3.8-max via Qwen Code /review (v0.21.14)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No blocking issues. LGTM! ✅
Test Plan (not a blocker): src/services/compactionInputSlimming.test.ts — no such file or directory; src/core/geminiChat.test.ts — no such file or directory.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/core/src/services/chatCompressionService.ts:784 — [review] Orphaned guard/coalesce comment block above getColdInputEstimate — the effectiveCompactionModel declaration it explains moved to ~line 621packages/core/src/services/chatCompressionService.ts:655 — [review] Pre-hook sharedRequestCouldFitBeforeHook math duplicates the post-hook sharedRequestFits block near-verbatim; a term changed in only one copy makes the gates disagree
中文说明
无阻断问题。LGTM！✅
Test Plan（非阻断）：src/services/compactionInputSlimming.test.ts — no such file or directory; src/core/geminiChat.test.ts — no such file or directory。
收敛姿态下延后（第 4 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
Qwen precheck requires maintainer approval before automated triage/review. Head SHA: Reason:
A maintainer with write access can inspect the PR and manually request a run with |
|
/review |
70c4ef0 to
bc91335
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
|
@qwen-code /review |
|
@qwen-code /triage |
|
CI is green on the latest head |
e4ab81c to
17f8aae
Compare
|
@qwen-code /review |
|
Qwen Code review request accepted. Review is running in workflow run. A command-triggered review is not listed under the checks of this PR; the result is posted here as a review when it finishes. |
Local runtime validation of #9541 (real build A/B, real tokenizer as oracle)I built and ran both sides of this PR locally rather than reading the diff, so the numbers below are measured, not inferred. Setup
Verdict: the fix works, but it introduces a worse regression for non-ASCII sessions. Requesting changes. 1. What the PR fixes — confirmed end to endReproducing #9455 on the real CLI (128,000-token window, session at ~116k real tokens):
Driving the compiled service directly confirms the rest of the claims (BEFORE → AFTER, 60,000-token window, ~100k-token history):
Mutation sweep: 10 of 11 mutants of the new production code are caught by the PR's own tests (336 tests pass at baseline). Only reverting the 2. Blocking: the admission estimator over-counts non-ASCII text by ~5x, so sessions that fit are refused
The base End-to-end consequence. Same CLI, same mock provider, same Chinese session, only the build differs.
Sweeping session content against real window usage (each cell is BEFORE → AFTER; provider verdicts come from the real tokenizer). On a provider without compression cache sharing —
With cache sharing and a provider anchor the shared path masks it until the anchor stops fitting — i.e. precisely the 85%+ region where auto-compaction lives:
This is not just a lost
The PR's Risk section says conservative estimates "can choose cold compression or local failure earlier than a provider tokenizer". A 5x factor on the product's primary language is a different thing: it converts the issue's failure mode ("compression sends a request that fails") into a strictly worse one ("compression refuses, and the session cannot be recovered at all"). Suggested directions (any one of these unblocks it):
3. Secondary findingsa. The two admission paths score the same history 5x apart. b. The new status renders as an empty line in the interactive TUI. c. Cost, for the record (not a blocker). The new estimate walks 4. SummaryThe core of this PR is right and well tested — the same-model admission gap, the stale cache-sharing anchor, the Reproduction artifacts, probes and raw logs are in branch 中文版对 #9541 的本地运行时验证（真实构建 A/B + 真实分词器作为判据）我没有只读 diff，而是把 PR 的两侧都在本地构建并实际运行，下面的数字都是实测值。 环境
结论：修复本身是有效的，但它为非 ASCII 会话引入了一个更严重的回归。建议 request changes。 1. PR 修复的问题——已端到端确认在真实 CLI 上复现 #9455（128,000 token 窗口，会话约 116k 真实 token）：
直接驱动编译产物验证其余主张（BEFORE → AFTER，60,000 token 窗口，约 100k token 历史）：
对携带 4,096 字符 变异扫描：新增生产代码的 11 个变异体中 10 个被 PR 自带测试捕获（基线 336 个测试通过）。唯一存活的是回退 2. 阻塞问题：准入估算对非 ASCII 文本高估约 5 倍，导致本来放得下的会话被拒绝
基线 端到端后果。 同一个 CLI、同一个 mock provider、同一个中文会话，只有构建不同。 按会话内容与真实窗口占用做扫描（每格为 BEFORE → AFTER，provider 判定来自真实分词器）：在没有压缩缓存共享的 provider 上—— 这不只是丢一次 PR 的 Risk 部分写的是保守估算"可能比 provider 分词器更早选择冷压缩或本地失败"。但在产品的主要语言上高估 5 倍是另一回事：它把 issue 描述的失败模式（"压缩发出了注定失败的请求"）换成了更糟的一种（"压缩直接拒绝，且会话完全无法恢复"）。 可选的修改方向（任意一条即可解除阻塞）：
3. 次要问题a. 两条准入路径对同一份历史的打分相差 5 倍。 b. 新状态在交互式 TUI 中渲染为空行。 c. 开销（非阻塞，仅作记录）。 新估算会逐字符遍历 4. 小结这个 PR 的主体是正确且测试充分的——同模型准入缺口、陈旧的缓存共享 anchor、 复现用的产物、探针与原始日志见分支 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- duplicated pre-hook/post-hook shared-request admission math — already reported (round-4 deferred list, review 4985130953)
- INPUT_TOO_LARGE status rendering gap in the unchanged /compress consumers — already reported (round-2 deferred list, review 4981833353)
Not explored to full depth (tool budget reached): "agent 1b": none — no check was cut short.; chunk 6: running the three affected vitest suites — the review worktree had no node_modules / dist , and npm ci (required before vitest's globalSetup guard passes) ….
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Test Plan (not a blocker): src/services/compactionInputSlimming.test.ts — no such file or directory; src/services/microcompaction/microcompact.test.ts — no such file or directory; src/core/turn.test.ts — no such file or directory; src/core/geminiChat.test.ts — no such file or directory; src/ui/components/messages/CompressionMessage.test.tsx — no such file or directory; and 1 more.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）："agent 1b"：none — no check was cut short.；chunk 6：running the three affected vitest suites — the review worktree had no node_modules / dist , and npm ci (required before vitest's globalSetup guard passes) …。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
Test Plan（非阻断）：src/services/compactionInputSlimming.test.ts — no such file or directory; src/services/microcompaction/microcompact.test.ts — no such file or directory; src/core/turn.test.ts — no such file or directory; src/core/geminiChat.test.ts — no such file or directory; src/ui/components/messages/CompressionMessage.test.tsx — no such file or directory; and 1 more。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| newTokenCountIsEstimated: usedEstimatedVisibleDelta, | ||
| compressionStatus: CompressionStatus.COMPRESSED, |
There was a problem hiding this comment.
[Critical] R5-8: [certifies-falsely] [regression] The COMPRESSED return derives newTokenCountIsEstimated solely from usedEstimatedVisibleDelta, so a newTokenCount that still contains locally-estimated components is stamped as authoritative false. Two estimated components flow into this number while the stamp says authoritative: (1) an estimate-derived baseline — CompressOptions carries only originalTokenCount: number with no provenance, and tryCompress (llm-chat.ts:2342-2352) passes a local content estimate documented to miss the system prompt and tool definitions by ~15-20K tokens whenever the baseline is estimate-derived; (2) the restoration addend — newTokenCount += Math.ceil(restorationChars / CHARS_PER_TOKEN) (~line 1458) charges restored file/image blocks at the unadjusted chars/4 estimator, documented (tokenEstimation.ts) as under-counting CJK-dense content by 39-54%. Pre-PR this branch stamped a hardcoded true. A session compressing from an estimated baseline (every prior turn aborted before usage arrived, or a transcript resume), or restoring recently-read CJK-dense files on the common success path (up to 5 files x 20,000 chars, charged ~25K tokens while costing ~40-55K), gets setLastPromptTokenCount(count, false) (llm-chat.ts:2426-2429) — so the next send's output clamp skips ESTIMATE_CLAMP_OVERHEAD_PAD = 20,000 tokens (llm-chat.ts:565, applied at :3058-3060, sized for exactly this under-estimate class per issue #5950): the first post-compression send is granted an output budget ~15-29K tokens too large and 400s on context overflow, rescued only by reactive overflow recovery. The false stamp also persists to the transcript and re-seeds on resume (session-resume-token-counts.ts:44), misattributing the estimate as API-reported.
Witness:
Baseline probe: INTACT stamps newTokenCountIsEstimated=false for an estimated baseline with side-query usage present; WITH provenance plumbing -> true, and the API-baseline pin (chatCompressionService.test.ts:619) stays green.
Restoration probe (5 recently-read CJK files, 5 x 19,000 chars, usage present):
PR: { newTokenCount: 86378, newTokenCountIsEstimated: false,
observedAddend: 23878 (= ceil(95510/4)),
bytes_div_4_lower_bound: 71407, utf8Adjusted_repoEstimator: 285128 }
FIX: { newTokenCount: 86378, newTokenCountIsEstimated: true }
Merge base stamps true on this branch with the identical restoration addend.
Fix — plumb both provenance channels into the stamp: add originalTokenCountIsEstimated?: boolean to CompressOptions, populate it at llm-chat.ts:2353, hoist hasEstimatedRestoration = restorationChars > 0 out of the provider branch, and stamp the OR on the COMPRESSED branch (mirror the INFLATED conditional):
newTokenCountIsEstimated:
usedEstimatedVisibleDelta ||
Boolean(opts.originalTokenCountIsEstimated) ||
restorationChars > 0,Keep false for a purely authoritative count (API baseline, no restoration) — that improvement over the pre-PR hardcoded true is intentional.
Fix constraint: llm-chat.ts:2405-2408 — "Keep a conservative fallback for older/custom implementations that omit the field, but preserve an explicit authoritative false" (info.newTokenCountIsEstimated ??= true): the ORs must apply only when the count actually has an estimated component; ESTIMATE_CLAMP_OVERHEAD_PAD = 20_000 (llm-chat.ts:565) is the pad sized to this under-estimate class.
Fix witness: please add two tests and prove them by mutation — an estimated baseline passed via opts with a usage-returning side query asserting result.info.newTokenCountIsEstimated is true, and a usage-metadata COMPRESSED run with a restorable recent CJK file asserting true while a no-restoration sibling keeps false; removing either OR must turn the corresponding test red, and the API-baseline pin at chatCompressionService.test.ts:619 must stay green.
中文说明
COMPRESSED 返回值中的 newTokenCountIsEstimated 仅由 usedEstimatedVisibleDelta 决定，因此一个仍包含本地估算成分的 newTokenCount 会被标记为权威值 false。有两个估算成分会在该标记为权威值时流入该数字：(1) 估算来源的基线——CompressOptions 只有 originalTokenCount: number，没有来源信息；当基线是估算值时，tryCompress（llm-chat.ts:2342-2352）传入的本地内容估算按文档说明会漏掉系统提示词与工具定义约 15-20K token；(2) 恢复项加数——newTokenCount += Math.ceil(restorationChars / CHARS_PER_TOKEN)（约第 1458 行）按未调整的 chars/4 估算器计费恢复的文件/图片块，而仓库文档（tokenEstimation.ts）明确该估算器对 CJK 高密度内容低估 39-54%。PR 之前此分支硬编码为 true。
失败场景：从估算基线发起压缩的会话（此前每轮都在 usage 到达前中止，或会话恢复时无 usage 记录），或在常见成功路径上恢复了近期读取的 CJK 高密度文件（最多 5 个文件 × 20,000 字符，按约 25K token 计费、实际成本约 40-55K），都会经过 setLastPromptTokenCount(count, false)（llm-chat.ts:2426-2429）——于是下一次发送的输出钳制会跳过 ESTIMATE_CLAMP_OVERHEAD_PAD = 20,000 token（llm-chat.ts:565，应用于 :3058-3060，正是为这类低估按 issue #5950 预留的余量）：压缩后的首次发送会多拿约 15-29K token 的输出预算，因上下文溢出而 400，只能靠反应式溢出救援兜底。错误的 false 标记还会持久化到会话记录并在恢复时重新播种（session-resume-token-counts.ts:44），把估算值误标为 API 上报值。
修复：把两个来源通道都接入该标记——在 CompressOptions 增加 originalTokenCountIsEstimated?: boolean（在 llm-chat.ts:2353 处填充），把 hasEstimatedRestoration = restorationChars > 0 提出 provider 分支，在 COMPRESSED 分支标记三者的或（INFLATED 分支同步处理）。对纯权威计数（API 基线、无恢复项）保留 false——相对 PR 前硬编码 true 的这一改进是有意的。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| utf8Bytes += codePoint <= 0x7ff ? 2 : codePoint <= 0xffff ? 3 : 4; | ||
| } | ||
| return Math.ceil(utf8Bytes - utf16CodeUnits / CHARS_PER_TOKEN); | ||
| } |
There was a problem hiding this comment.
[Critical] R5-6: [fails-closed] [new-surface] estimateNonAsciiUtf8Adjustment adds ~1 token per non-ASCII UTF-8 byte — 3.0 tokens per 3-byte CJK char on top of the chars/4 base — versus this PR's own fixture calibration of 1.25 tokens/char (48,000 保 chars against originalTokenCount: 60_000 at chatCompressionService.test.ts:5205) and the repo's documented conservative calibration (tokenEstimation.ts: char/4 under-counts CJK-dense tool output by 39-54%, and the 1.5x factor covers the observed cases). This estimator feeds the cold admission gates, and the gates never consult a provider anchor — hasProviderTokenCount feeds only the cache-sharing decision, not estimateColdRequestInput. A 65,536-window deployment (the vLLM class this PR targets) with a CJK-dense text-only history trips this immediately: ~26K CJK chars is ~78K UTF-8 bytes but only ~32.5K real tokens — exactly the auto-trigger threshold — so the ~78K estimate exceeds the 63,488 admission budget and compress() rejects locally with COMPRESSION_FAILED_INPUT_TOO_LARGE even though the real side-query (~32.5K prompt + full 20K output budget) fits with >12K tokens of headroom. Text-only CJK history has no tool results for reduceColdInputForAdmission to clear; each non-forced rejection strikes the breaker (llm-chat.ts:2455-2466, latches at 3), forced /compress runs the same unconditional gate and is rejected identically (the pinned test at test.ts:5205 itself uses force: true), and compressFast is a NOOP on text-only history — so the session can never be compacted again: the overflow-stall class this PR exists to fix (issue #9455), made permanent for CJK-dense sessions on no-cache-sharing/no-anchor configurations. The Risk & Scope disclosure covers "fails earlier than a provider tokenizer", not "fails forever" — a state where the forced path is rejected by the same gate and the breaker can never reset is not the disclosed tradeoff.
Witness:
Probe (26K CJK chars, window 65,536, provider anchor 33,000 — a side-query of
~33K prompt + 20K output fits with ~12.5K headroom):
INTACT: status=8 (COMPRESSION_FAILED_INPUT_TOO_LARGE), coldSpyCalls=0,
warning "estimated input 78,929 tokens cannot leave 1,024 usable
output tokens within the 65,536-token context window"
WITH recalibration ((utf8Bytes - utf16CodeUnits) / 2):
status=1 (COMPRESSED), coldSpyCalls=1, newHistory present
| utf8Bytes += codePoint <= 0x7ff ? 2 : codePoint <= 0xffff ? 3 : 4; | |
| } | |
| return Math.ceil(utf8Bytes - utf16CodeUnits / CHARS_PER_TOKEN); | |
| } | |
| utf8Bytes += codePoint <= 0x7ff ? 2 : codePoint <= 0xffff ? 3 : 4; | |
| } | |
| return Math.ceil((utf8Bytes - utf16CodeUnits) / 2); | |
| } |
The recalibration puts 3-byte CJK at ~1.25 tokens/char, matching this PR's own fixture calibration; alternatively, gate admission on the provider-reported prompt count when hasProviderTokenCount is true. Either way, re-tune the CJK rejection tests (chatCompressionService.test.ts:5205-5268) to the new boundary and add an acceptance test just below it.
Fix constraint: the same adjustment function feeds the admission side and estimateSummaryOutputTokens' truncation guard (chatCompressionService.ts:179-185) — a recalibration moves both boundaries at once and must keep them consistent; and the test helper estimateUtf8AdjustedVisibleTokens (chatCompressionService.test.ts:40) replicates the formula verbatim and must change in lockstep or the visible-delta assertions silently diverge from production accounting.
Fix witness: please add a test — window 65,536, ~26K chars of 保, force: true — that must reach runSideQuery (spy called, status COMPRESSED) instead of COMPRESSION_FAILED_INPUT_TOO_LARGE, and prove it by mutation: removing the recalibration must turn it red while the re-tuned rejection tests keep pinning the far side of the boundary.
中文说明
estimateNonAsciiUtf8Adjustment 对每个非 ASCII 的 UTF-8 字节加约 1 个 token——3 字节的 CJK 字符在 chars/4 基础之上合计约 3.0 token/字符——而本 PR 自己的测试标定是 1.25 token/字符（chatCompressionService.test.ts:5205 用 48,000 个 保 对应 originalTokenCount: 60_000），仓库文档化的保守标定也只有 1.5 倍 char/4（tokenEstimation.ts：char/4 对 CJK 高密度工具输出低估 39-54%，1.5 倍已覆盖观测案例）。该估算器驱动冷路径准入检查，而准入检查从不参考 provider 锚点——hasProviderTokenCount 只参与缓存共享判断，不进入 estimateColdRequestInput。
失败场景：65,536 窗口的部署（本 PR 针对的 vLLM 形态）+ CJK 高密度纯文本历史：约 26K 个 CJK 字符约为 78K UTF-8 字节、但真实只有约 32.5K token——恰好是自动触发阈值——于是约 78K 的估算超过 63,488 的准入预算，compress() 在本地以 COMPRESSION_FAILED_INPUT_TOO_LARGE 拒绝，尽管真实 side-query（约 32.5K prompt + 完整 20K 输出预算）还有 12K 以上余量。纯文本 CJK 历史没有可被 reduceColdInputForAdmission 清理的工具结果；每次非强制拒绝都计入熔断（llm-chat.ts:2455-2466，3 次锁死），强制 /compress 走同一无条件检查并被同样拒绝（test.ts:5205 的固定测试本身就用 force: true），compressFast 对纯文本历史是 NOOP——会话从此永远无法压缩：本 PR 要修复的溢出卡死（issue #9455）在无缓存共享/无锚点配置的 CJK 高密度会话上变成永久状态。Risk & Scope 披露的是"比 provider 分词器更早失败"，而不是"永远失败"——强制路径被同一检查拒绝、熔断永远无法重置的状态不在披露范围内。
修复：按上面 suggestion 重新标定（3 字节 CJK 约 1.25 token/字符，与本 PR 自身标定一致），或在 hasProviderTokenCount 为真时以 provider 上报的 prompt 数作为准入依据；无论哪种，都请按新边界重新标定 CJK 拒绝测试（chatCompressionService.test.ts:5205-5268）并在边界下方补一个接受测试。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| const estimatedOriginalVisibleTokenCount = | ||
| estimateUtf8AdjustedContentTokens( | ||
| curatedHistory, | ||
| slimmingConfig.imageTokenEstimate, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R5-3: The visible-delta branch now stringifies the full unslimmed history — including all inline media base64 — on the common success path: estimateUtf8AdjustedContentTokens (new in this PR, ~line 165) computes estimateNonAsciiUtf8Adjustment(JSON.stringify(contents)), serializing the entire curatedHistory into one string and walking every code point. The merge base used per-part estimateContentTokens here, so the allocation is PR-born, and this branch runs on every successful cache-sharing compression plus the usage-missing and admission-reduced paths. Base64 is pure ASCII, so media bytes contribute zero to the UTF-8 adjustment — the cost buys no signal. The screenshot-overflow sessions this feature targets hold 20+ tool-returned images (each ~1,600 tokens toward thresholds, hundreds of KB to MB of base64), so accounting allocates a string comparable in size to the whole history and scans it — at exactly the compaction moment the file's own comment warns is lowest on V8 heap headroom (:514-518): transient memory spike / GC stall on media-heavy sessions, risking OOM when compaction is the only relief valve.
Witness:
Probe (unmodified PR, 2MB base64 inlineData in history, usage-missing path):
intact: maxStringifyArg=2000631, sizesOver100k=[2000141, 2000631], base64Len=2000000
reverted: maxStringifyArg=121, sizesOver100k=[]
both arms: newTokenCount=100112 (accounting identical)
Apply the UTF-8 adjustment only to text-bearing payloads instead — walk parts the way estimatePartChars does and run estimateNonAsciiUtf8Adjustment over the text / functionCall-args / functionResponse-output strings only, e.g.:
const estimatedOriginalVisibleTokenCount =
estimateUtf8AdjustedTextTokensFromContents(
curatedHistory,
slimmingConfig.imageTokenEstimate,
);(where the helper sums per-part text estimates plus the image estimate, then applies the adjustment to the concatenated text payload only).
Fix constraint: the delta comment at chatCompressionService.ts:1477-1479 — "Both sides use the same UTF-8-adjusted estimator; provider prompt counts and admission savings are intentionally excluded from this local delta" — so curatedHistory and extraHistory must keep identical estimation; the heap-headroom warning at chatCompressionService.ts:514-518 is the documented constraint any per-compression allocation must respect.
Fix witness: please extend the visible-delta tests with an inline-media fixture asserting the identical newTokenCount after the change, and prove it by mutation: a probe asserting no JSON.stringify argument exceeds the largest text part must go red if the whole-history stringify returns.
中文说明
可见增量分支现在会在常见成功路径上把完整的未瘦身历史（包括所有内联媒体的 base64）序列化成一个字符串：estimateUtf8AdjustedContentTokens（本 PR 新增，约第 165 行）计算 estimateNonAsciiUtf8Adjustment(JSON.stringify(contents))，把整个 curatedHistory 序列化为单个字符串并逐码点扫描。合并基线在此处使用按 part 的 estimateContentTokens，因此该内存分配是 PR 引入的；该分支在每次缓存共享成功压缩、usage 缺失、准入缩减路径上都会执行。base64 是纯 ASCII，媒体字节对 UTF-8 调整的贡献为零——开销买不到任何信号。本功能针对的截图溢出会话说历史里有 20+ 张工具返回的图片（每张按阈值约 1,600 token，base64 却有数百 KB 到 MB），于是记账时会分配一个与整个历史相当的字符串并扫描它——恰好发生在文件自身注释警告的 V8 堆余量最低的时刻（:514-518）：媒体密集会话上出现瞬时内存尖峰 / GC 停顿，而压缩本身就是唯一的泄压阀，存在 OOM 风险。
修复：只对承载文本的载荷应用 UTF-8 调整——像 estimatePartChars 一样遍历 parts，仅对 text / functionCall 参数 / functionResponse 输出字符串运行 estimateNonAsciiUtf8Adjustment，两侧增量保持同一估算器（约束见 chatCompressionService.ts:1477-1479 的注释）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| return Math.max( | ||
| estimateContentTokens( | ||
| [{ role: 'model', parts: [{ text: summary }] }], | ||
| imageTokenEstimate, | ||
| ), | ||
| estimateUtf8AdjustedTextTokens(summary), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R5-4: The new UTF-8-adjusted summary-output estimate moved the false-truncation boundary for complete summaries from ~13,333 to ~6,667 CJK chars at the fixed 20K threshold (truncationThreshold, :1252), and the truncation guard does not check well-formedness — so COMPLETE summaries are what get dropped. A provider that omits usage metadata on the side-query (the OpenAI-compatible case documented in this file) returning a complete CJK-dense summary of ~7,000 chars estimates ~21,000 >= 20,000 and drops it as COMPRESSION_FAILED_OUTPUT_TRUNCATED; that status strikes the breaker (turn.ts:404-412), retries drop it identically, and after MAX_CONSECUTIVE_FAILURES = 3 auto-compaction NOOPs — the overflow-stall class this PR fixes, re-introduced for CJK sessions on such providers. No acceptance-side CJK test exists: the only CJK output-side test (chatCompressionService.test.ts:1336) pins rejection at cap size and was calibrated to the old 1.5x formula.
Witness:
Probe (usage missing, closed <state_snapshot>, deterministic 1-char split, 21 runs):
6,663 CJK chars -> compressionStatus: 1 (COMPRESSED), newTokenCountIsEstimated: true, persisted
6,664 CJK chars -> compressionStatus: 6 (COMPRESSION_FAILED_OUTPUT_TRUNCATED), newHistory null
scan: n=6,655-6,663 all pass the guard; n=6,664-6,675 all dropped
Add a usage-missing test asserting a complete CJK summary whose adjusted estimate sits just below COMPACT_MAX_OUTPUT_TOKENS is persisted (status COMPRESSED, newTokenCountIsEstimated: true), or a boundary test pinning the chosen cutoff so the narrower acceptance band stays deliberate:
it('accepts a complete CJK summary just under the estimated truncation threshold', async () => {
// size the summary so estimateUtf8AdjustedTextTokens(summary) < COMPACT_MAX_OUTPUT_TOKENS
// mockVllmBackend(WINDOW, false, { omitUsage: true });
// assert compressionStatus COMPRESSED and newTokenCountIsEstimated: true
});Note: recalibrating estimateNonAsciiUtf8Adjustment under R5-6 moves this boundary too — land the acceptance test against whichever calibration ships.
Fix constraint: const truncationThreshold = outputCountIsEstimated ? COMPACT_MAX_OUTPUT_TOKENS : coldOutputBudget; at chatCompressionService.ts:1252 — the estimated path must compare against the fixed ceiling, not the clamped budget, so the new fixture must keep the clamped budget below the ceiling.
Fix witness: the added acceptance test itself — it must go red if the >= truncationThreshold comparison regresses or the estimated-path threshold stops using the fixed ceiling.
中文说明
新的 UTF-8 调整后摘要输出估算把完整摘要的误截断边界从约 13,333 个 CJK 字符移到约 6,667 个（固定 20K 阈值，truncationThreshold，:1252），且截断守卫不检查格式完整性——被丢弃的恰恰是完整摘要。对省略 side-query usage 元数据的 provider（本文件中已有文档的 OpenAI 兼容形态），若返回约 7,000 字符的 CJK 高密度完整摘要，估算约 21,000 >= 20,000，会以 COMPRESSION_FAILED_OUTPUT_TRUNCATED 丢弃；该状态计入熔断（turn.ts:404-412），重试同样被丢弃，3 次后自动压缩 NOOP——本 PR 要修复的溢出卡死在这类 provider 的 CJK 会话上被重新引入。目前没有接受侧的 CJK 测试：唯一的 CJK 输出侧测试（chatCompressionService.test.ts:1336）固定的是上限处的拒绝，且按旧的 1.5 倍公式标定。
修复：补一个 usage 缺失的测试，断言调整后估算刚好低于 COMPACT_MAX_OUTPUT_TOKENS 的完整 CJK 摘要被持久化（状态 COMPRESSED、newTokenCountIsEstimated: true），或补一个边界测试固定所选截断点。注意：若按 R5-6 重新标定 estimateNonAsciiUtf8Adjustment，此边界也会移动——请按最终落地的标定补测试。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| expect(result.info.compressionStatus).not.toBe( | ||
| CompressionStatus.COMPRESSION_FAILED_INPUT_TOO_LARGE, | ||
| ); | ||
| expect(capturedModel).toBe('test-model'); |
There was a problem hiding this comment.
[Suggestion] R5-7: 'falls back when a distinct model cannot fit the full request plus safety and 20K output' — the only coverage of the compaction-model->main-model fallback branch — pins the outcome only negatively (.not.toBe(COMPRESSION_FAILED_INPUT_TOO_LARGE)) plus the requested model/budget; it never asserts the fallback actually compressed, unlike its sibling 'keys the budget to the compaction model window…' (~line 5926) which asserts toBe(COMPRESSED) under the same mock response shape. The gap is not hypothetical: replicating this exact fixture and these exact assertions on the unmodified PR code, the real outcome is a FAILED compression — COMPRESSION_FAILED_INFLATED_TOKEN_COUNT with newHistory: null and newTokenCount 102,000 (originalTokenCount: 100_000 against a ~0-token payload makes the usage accounting compute 100,000 - 0 + 2,000 > 100,000) — and all three existing assertions still pass. The fallback branch's happy ending is pinned by no test today, so a regression in the fallback path's result handling ships green.
Witness:
Exact-fixture replica probe on unmodified code:
{ status: 2 (COMPRESSION_FAILED_INFLATED_TOKEN_COUNT), newHistory: null,
capturedModel: 'test-model', capturedMaxOutputTokens: 20000,
newTokenCount: 102000 } — every existing assertion green, no mutation needed
Re-tune the fixture the way the sibling test's is (the current 100,000-vs-tiny-payload shape inflates under usage accounting), then assert the positive outcome:
expect(result.info.compressionStatus).toBe(CompressionStatus.COMPRESSED);
expect(result.newHistory).not.toBeNull();Fix constraint: the fixture must keep the fallback trigger intact — estimate + COMPACTION_BUDGET_SAFETY_MARGIN + COMPACT_MAX_OUTPUT_TOKENS > 21,500 (the configured compaction window) at chatCompressionService.ts:836-871 — or the test stops exercising the fallback branch.
Fix witness: the strengthened assertions — mutate the fallback path so the cold summary maps to COMPRESSION_FAILED_OUTPUT_TRUNCATED; the current test stays green, the strengthened one must go red.
中文说明
'distinct 模型装不下完整请求 + 安全边际 + 20K 输出时回退'是压缩模型→主模型回退分支的唯一覆盖，但它只用否定式断言固定结果（.not.toBe(COMPRESSION_FAILED_INPUT_TOO_LARGE)）外加请求的模型/预算，从未断言回退真的压缩成功——而兄弟测试 '…把预算固定到压缩模型窗口'（约第 5926 行）在相同的 mock 响应下断言 toBe(COMPRESSED)。这个缺口不是假设：在未修改的 PR 代码上用完全相同的 fixture 与断言复现，真实结果是压缩失败——COMPRESSION_FAILED_INFLATED_TOKEN_COUNT、newHistory: null、newTokenCount 102,000（originalTokenCount: 100_000 对约 0 token 的载荷使 usage 记账算出 100,000 - 0 + 2,000 > 100,000）——而三个现有断言全部通过。回退分支的成功结局目前没有任何测试固定，结果处理一旦回归会静默合入。
修复：按兄弟测试的方式调整 fixture，然后改用肯定断言；fixture 必须保留回退触发条件（估算 + 安全边际 + 20,000 > 21,500 的压缩窗口），否则测试不再走回退分支。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| mockVllmBackend(WINDOW, true, { omitUsage: true }); | ||
| vi.mocked(logChatCompression).mockClear(); |
There was a problem hiding this comment.
[Suggestion] R5-9: The cache_sharing_attempted: false / cache_sharing_used: false fields of the pre-hook rejection telemetry event (chatCompressionService.ts:751-758) are asserted by no test — the only assertion on that path (chatCompressionService.test.ts:5196) pins tokens_before/tokens_after only — and this test carries a dangling vi.mocked(logChatCompression).mockClear() with no assertion following it, indicating one was intended. Deleting those two fields from the rejection event turns no test red, so corrupted cache-sharing fields would silently flow into compression telemetry, while the sibling truncation test at ~line 6064 deliberately pins its equivalent event.
Witness:
Mutation (scratch tree): removed cache_sharing_attempted / cache_sharing_used
from the pre-hook rejection event, ran every test reaching that path:
'fails locally when an irreducible cold request cannot leave usable output room' -> 1 passed
'rejects locally before provider usage metadata can matter' -> 1 passed
'rejects CJK-dense cold input that only fits under the char/4 lower bound' -> 1 passed
After the status assertion, add (keep the mockClear):
expect(logChatCompression).toHaveBeenCalledWith(
mockConfig,
expect.objectContaining({
tokens_before: 65_000,
tokens_after: 65_000,
cache_sharing_attempted: false,
cache_sharing_used: false,
}),
);Fix constraint: the service emits tokens_before: originalTokenCount, tokens_after: originalTokenCount, cache_sharing_attempted: false, cache_sharing_used: false on this path (chatCompressionService.ts:751-758); this fixture's originalTokenCount is 65_000, so the values above must match that shape.
Fix witness: the new assertion itself — it must go red if the logChatCompression call at chatCompressionService.ts:750-758 is deleted or its cache-sharing fields change.
中文说明
预钩子拒绝遥测事件（chatCompressionService.ts:751-758）的 cache_sharing_attempted: false / cache_sharing_used: false 两个字段没有任何测试断言——该路径上唯一的断言（chatCompressionService.test.ts:5196）只固定 tokens_before/tokens_after——而本测试里有一个悬空的 vi.mocked(logChatCompression).mockClear()，后面没有跟随断言，说明原本打算断言。删除这两个字段不会让任何测试变红，损坏的缓存共享字段会静默流入压缩遥测，而约第 6064 行的兄弟截断测试却刻意固定了它的等价事件。
修复：在状态断言后补上上面的断言（保留 mockClear）；断言取值必须与该路径实际发出的事件形状一致（此 fixture 的 originalTokenCount 为 65_000）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| coldInput = unreducedColdInput; | ||
| cachedColdHistoryEstimate = unreducedColdHistoryEstimate; | ||
| coldInputReducedForAdmission = wasUnreducedForAdmission; |
There was a problem hiding this comment.
[Suggestion] R5-10: This PR-born fallback restore of coldInput / cachedColdHistoryEstimate / coldInputReducedForAdmission has zero test coverage. The existing test 'restores full cold input when a small compaction model falls back' (chatCompressionService.test.ts:5066) pins the restored payload but asserts nothing about the result — and it never performs a real reduction at all: it holds one clearable tool result, and keepRecentOverride: 1 protects the last clearable ref (buildKeepRefs, microcompact.ts:322-324), so microcompactHistory returns the same array and the flag never sets. If a refactor drops the coldInputReducedForAdmission restore, the flag stays true while the sent payload is unreduced: post-compression accounting silently switches to the estimated visible-delta branch, stamping newTokenCountIsEstimated: true with a divergent count. The wrong true stamp flows into setLastPromptTokenCount (llm-chat.ts:2426-2429), so the next send's clamp adds the 20,000-token ESTIMATE_CLAMP_OVERHEAD_PAD to an API-anchored count — needlessly shrinking the output budget — and mislabels the count's provenance in the transcript.
Witness:
Probe (two clearable tool results, 32K compaction window, 262K main window;
guard reduces with toolsCleared: 1, reduced estimate still exceeds the window,
falls back, restore fires):
INTACT: { newTokenCount: 21500, newTokenCountIsEstimated: false, compressionStatus: 1 }
MUTANT (restore line dropped): { newTokenCount: 63, newTokenCountIsEstimated: true,
compressionStatus: 1 } — identical sent payload
Under the mutation the ENTIRE chatCompressionService.test.ts stays green:
Test Files 1 passed (1) / Tests 165 passed (165)
Anchor fixture check: test.ts:5066's history returns { reducedSameRef: true, meta: null }
— it never reduces, so assertions added there pin nothing.
Pin it where a real reduction happens — a fixture with at least two clearable tool results whose reduced payload still misses the compaction-model window:
expect(result.info.compressionStatus).toBe(CompressionStatus.COMPRESSED);
expect(result.info.newTokenCountIsEstimated).toBe(false);
expect(result.info.newTokenCount).toBe(21_500); // exact restored accountingFix constraint: llm-chat.ts:2405-2408 preserves an explicit authoritative false — the restore must keep stamping false for this API-anchored shape; and keepRecentOverride: 1 protects the last clearable ref, so the fixture needs at least two clearable tool results for any reduction to happen at all.
Fix witness: the added assertions — delete the coldInputReducedForAdmission restore at chatCompressionService.ts:865 and they must go red (stamp flips to true, count diverges) while every current assertion stays green.
中文说明
这段 PR 引入的回退恢复（coldInput / cachedColdHistoryEstimate / coldInputReducedForAdmission）完全没有测试覆盖。现有测试 '小压缩模型装不下时回退并恢复完整冷输入'（chatCompressionService.test.ts:5066）固定了恢复后的载荷，但对结果没有任何断言——而且它根本没有执行真正的缩减：只有一个可清理的工具结果，keepRecentOverride: 1 会保护最后一个可清理引用（buildKeepRefs，microcompact.ts:322-324），microcompactHistory 原样返回同一数组，标志从未被置位。若重构删掉 coldInputReducedForAdmission 的恢复，标志会保持为 true 而发送的载荷并未缩减：压缩后记账会静默切换到估算可见增量分支，把 newTokenCountIsEstimated 标成 true 且数值发散。错误的 true 标记流入 setLastPromptTokenCount（llm-chat.ts:2426-2429），下一次发送的钳制会给 API 锚定的计数加上 20,000 token 的 ESTIMATE_CLAMP_OVERHEAD_PAD——白白压缩输出预算——并在会话记录里误标计数来源。
修复：在真正发生缩减的 fixture 上固定它——至少两个可清理的工具结果、且缩减后的载荷仍超出压缩模型窗口——补上对 compressionStatus、newTokenCountIsEstimated: false 与精确 newTokenCount 的断言。
— qwen3.8-max via Qwen Code /review (v0.22.3)
c7d33c8 to
0504834
Compare
Keep managed-memory reads and shared failure semantics intact while bounding cold-request admission work. Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Use the actual receiving window, preserve unreduced fallback input, and account for admission-cleared tokens so valid summaries are not rejected or silently degraded. Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Model each receiving window with its actual output reserve and pin the post-hook cold-request safety gate. Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Use conservative multilingual admission, consistent estimated accounting, and explicit side-query failures so compression degrades without corrupting context or UI state. Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
0504834 to
758c406
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Refresh the branch against the current CI contract while preserving the compression regression coverage. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
4 participants
What this PR does
This change applies complete request admission to shared-cache and cold compression requests. It combines provider anchors with current-route estimates, includes thought signatures, runs bounded microcompaction before cold requests, and rejects requests locally when they cannot leave a usable output budget.
The latest hardening also:
Why it's needed
Issue #9455 captured compression requests whose estimated input plus reserved output exceeded the receiving model window. Those requests were knowingly sent to the provider and failed with context-window errors instead of being reduced or rejected locally. Additional review found that multilingual inputs, side-query failures, and mixed exact/estimated accounting could still bypass or corrupt that safety boundary.
Reviewer Test Plan
Run:
Verified: 663 core tests and 693 CLI tests, plus core/CLI build, lint, typecheck, Prettier, and
git diff --check.The regression matrix covers same-model cache admission, route overhead, thought signatures, bounded microcompaction, multilingual dense text, compaction-model budget boundaries, side-query exceptions, truncation telemetry, estimated token deltas, and failure rendering.
Risk & Scope
Linked Issues
Fixes #9455
中文说明
本改动为共享缓存和冷压缩请求补齐完整准入检查，并进一步修复多语种高密度文本、独立压缩模型预算、side-query 异常、截断遥测、精确/估算 token 混算以及 CLI 失败展示问题。
验证通过 663 项 core 测试和 693 项 CLI 测试，以及 build、lint、typecheck、Prettier 和
git diff --check。残余风险：provider 未返回可比 usage 时，token 数仍是启发式估算，并会显式标记为 estimated；不同 tokenizer 可能存在偏差。