fix(core): invalidate token counts recorded for a switched model route - #9506
Conversation
`/model` switches rebuild the content generator but keep the same GeminiChat instance, so API-reported prompt/output token counts from the previous route survived and anchored admission, output clamping, and compression decisions for a different serialization (#9454). Attribute the counts to the route that produced them (Config.getModelRouteIdentity) and invalidate them on a route change so all safety decisions fall back to the history-walk estimate. Closes #9454
|
Re-run after rounds 8–9 ( Template looks good ✓ Problem: Unchanged and still strong — the linked #9454 is a P1 observed in a dogfood session with concrete numbers (~691k input tokens on an OpenAI Responses route, first request after switching to an Anthropic route exceeding the window by ~101k, reactive and manual compression both trusting the stale count). Not theoretical hardening. Direction: Aligned, as before — one route's API-reported size must not drive another route's admission, output clamping, and compression decisions. Additive-only Config API ( Size: 384 production lines ( Approach: Scope remains right. The two commits since the last reviewed head map one-to-one onto confirmed Criticals: round 8 closes the mid-send flip cluster ( Risk: Moving on to code review. 🔍 中文说明本次为第八、九轮( 模板完整 ✓ 问题: 不变且证据充分 —— 关联的 #9454 是试用会话中观测到的 P1,有具体数字(OpenAI Responses 路由约 691k 输入 token,切换到 Anthropic 路由后首个请求超出窗口约 101k,响应式与手动压缩都信任了旧计数)。不是理论性加固。 方向: 与之前一致 —— 一种路由的 API 上报大小不应驱动另一种路由的准入、输出钳制和压缩决策。Config 为纯增量 API( 规模: 384 行生产代码( 方案: 范围仍然合理。自上次审查 head 以来的两个提交与已确认的 Critical 一一对应:第八轮关闭发送中途翻转簇( 风险: 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRe-reviewed at My independent baseline for this cluster: any single-slot store that a compression service reads through keyless getters adopting the ACTIVE route will flip mid-send on non-exact override sends, and the closure is structural — re-adopt the request route after any call that reads keyless on your behalf, retain-before-overwrite whenever a foreign-keyed slot is about to be clobbered, clear ALL retention when the shared history is rewritten (every retained entry sizes the pre-rewrite history), and have the hard-rescue snapshot/rollback capture the entire count state, not just the prompt half. The PR lands precisely those pieces. What I verified at the final state, fix by fix:
Invariants re-verified at this head: every count writer stamps a route key and supersede-deletes (or clears); The round-10 TestingUnattended CI run — this review does not execute the PR's code. Evidence below is the PR's own CI on the reviewed commit CI on
Skipped on this head: Test (macos-latest / windows-latest), Integration Tests (CLI, No Sandbox). / 本 head 上跳过:macOS/Windows 单测矩阵、集成测试(CLI、无沙箱)。 Sandboxed verification would settle the remaining gap: Not verified: live behaviour on macOS/Windows (author tested Linux only) — the mechanism is engine-internal with no platform-specific path, and there is no user-visible TUI surface change, so real-scenario tmux evidence is N/A for this run. 中文说明代码审查在 我对本簇的独立基线:任何单槽存储,只要压缩服务经由会 adopt 活动路由的无 key getter 读取,就会在非精确覆盖发送中途被翻转;闭合方式是结构性的 —— 在任何替你做了无 key 读取的调用返回后重新 adopt 请求路由;外来 key 槽位将被覆盖前先保留;共享历史被重写时清空全部保留项(每条保留项度量的都是重写前的历史);硬救援快照/回滚必须捕获全部计数状态而不只是 prompt 一半。PR 落地的正是这些。 逐修复核验(R8-1 至 R9-2,详见英文部分): 第十轮 测试无人值守 CI 运行 —— 本审查不执行 PR 代码。证据为被审提交 沙箱验证可闭合剩余缺口: 未验证:macOS/Windows 实机行为(作者仅在 Linux 测试)—— 机制为引擎内部、无平台特定路径,且无用户可见 TUI 界面变化,故本次运行的真实场景 tmux 证据为 N/A。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — the round-8/9 commits close the last mechanism cluster, a path-by-path audit of the final state finds no new issues, the round-10 review at this exact head reports zero findings, and CI is green. What keeps it from a 5: the integration suite is skipped in CI, the author tested Linux only, and a handful of Suggestion-level test-pin items remain deferred — none of it blocking. Stepping back: my independent baseline for this round's problem was structural — a single-slot count store read through keyless getters that adopt the ACTIVE route will flip mid-send on non-exact override sends, and any fix needs re-adoption after the service returns, retain-before-overwrite at the stamp, a full retention clear on shared-history rewrites, and a complete snapshot/rollback. The PR lands exactly that, and nothing besides. Ten rounds in, the growth each round has been the findings themselves — no accretion, no drive-bys, no speculative machinery. The retention map is more machinery than the original one-liner invalidation, but it is what the gate's keyed read needs to stay non-destructive: without it, steady route alternation silently disabled the session token limit, which would have made the #9454 fix a no-op in exactly the multi-route sessions it exists for. The two commits under review this run each map one-to-one onto a confirmed Critical, and each is pinned by a regression test that drives the real mechanism rather than the mock surface — the R9-2 test runs a genuine Verdict: approve. CI is green on this head and no PR CI runs are pending, so the approval lands now, pinned to the reviewed commit. The sandboxed A/B report for this head will follow in the thread when its run finishes; nothing else is needed from a maintainer on the code itself. 中文说明退一步看:我对本轮问题的独立基线是结构性的 —— 单槽计数存储经由会 adopt 活动路由的无 key getter 读取,在非精确覆盖发送时必然中途翻转;任何修复都需要服务返回后重新 adopt、盖章处先保留再覆盖、共享历史重写时全量清空保留、以及完整的快照/回滚。PR 落地的恰好是这些,别无其他。十轮下来,每轮增量都是发现本身 —— 无堆积、无顺手改动、无臆测机制。保留映射比最初一行式失效多了机制,但它是门禁按 key 读取保持非破坏性所必需的:没有它,稳定的路由交替会悄悄禁用会话 token 上限,使 #9454 的修复在它本要服务的多路由会话里形同虚设。 本次审查的两个提交各自与一个已确认的 Critical 一一对应,且各自由驱动真实机制(而非 mock 表面)的回归测试钉住 —— R9-2 测试跑了真实 结论:批准。该 head 的 CI 全绿且无进行中的 PR CI,批准即刻发出并绑定被审提交。该 head 的沙箱 A/B 报告运行结束后跟发本线程;维护者在代码本身无需再做别的。 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
|
@qwen-code /review --timeout=360 |
|
Qwen Code review request accepted. Review is queued in workflow run. |
|
@qwen-code /review --timeout=360 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): src/core/geminiChat.test.ts — no such file or directory; 1 passed — this review observed 20641, 1590, 22030, 1612, 494, 3799, 542 passed; 4 passed — this review observed 20641, 1590, 22030, 1612, 494, 3799, 542 passed; 332 passed — this review observed 20641, 1590, 22030, 1612, 494, 3799, 542 passed; 283 passed — this review observed 20641, 1590, 22030, 1612, 494, 3799, 542 passed; and 1 more.
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan(非阻断):src/core/geminiChat.test.ts — no such file or directory; 1 passed — this review observed 20641, 1590, 22030, 1612, 494, 3799, 542 passed; 4 passed — this review observed 20641, 1590, 22030, 1612, 494, 3799, 542 passed; 332 passed — this review observed 20641, 1590, 22030, 1612, 494, 3799, 542 passed; 283 passed — this review observed 20641, 1590, 22030, 1612, 494, 3799, 542 passed; and 1 more。
— qwen3.8-max via Qwen Code /review (v0.21.14)
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-5 stale lastCachedContentTokenCount telemetry mirror after a route switch — already reported (comment 3817177583)
Not explored to full depth (tool budget reached): "agent 6b": verify whether UiTelemetryService's lastCachedContentTokenCount mirror is left stale across a route switch (invalidation zeroes only the prompt count) and whe….
Test Plan (not a blocker): src/core/geminiChat.test.ts — no such file or directory; 1 passed — this review observed 1590, 22027, 1612, 494, 3799, 542 passed; 4 passed — this review observed 1590, 22027, 1612, 494, 3799, 542 passed; 332 passed — this review observed 1590, 22027, 1612, 494, 3799, 542 passed; 283 passed — this review observed 1590, 22027, 1612, 494, 3799, 542 passed; and 1 more.
中文说明
本轮确认的 1 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未探索到全部深度(达到工具调用预算):"agent 6b":verify whether UiTelemetryService's lastCachedContentTokenCount mirror is left stale across a route switch (invalidation zeroes only the prompt count) and whe…。
Test Plan(非阻断):src/core/geminiChat.test.ts — no such file or directory; 1 passed — this review observed 1590, 22027, 1612, 494, 3799, 542 passed; 4 passed — this review observed 1590, 22027, 1612, 494, 3799, 542 passed; 332 passed — this review observed 1590, 22027, 1612, 494, 3799, 542 passed; 283 passed — this review observed 1590, 22027, 1612, 494, 3799, 542 passed; and 1 more。
— qwen3.8-max via Qwen Code /review (v0.21.14)
Closeout
中文摘要已修复 session-limit 首次发送死锁,以及视觉/回退路由计数误记到主路由的问题;聚焦测试与构建通过。未扩大为计数存储重构或 mutation 测试矩阵,等待当前 head CI。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent 4": none — no check was cut short..
Test Plan (not a blocker): src/core/geminiChat.test.ts — no such file or directory.
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度(达到工具调用预算):"agent 4":none — no check was cut short.。
Test Plan(非阻断):src/core/geminiChat.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.21.14)
- C1: client.test.ts session-token-limit gate test now seeds the chat mock's getLastPromptTokenCount (the gate's new source) instead of relying on the telemetry stub alone. - C2: goal-turn-integration.test.ts passes the routeKey positional added to processStreamResponse and widens the local cast. - S1: fix stale rationale comment — no session token-limit gate reads the telemetry mirror anymore; UI context counters and compression banners do. - S2: pin tryCompress's entry invalidation with a focused test; manual /compress reaches it without sendMessageStream's entry reset. - S3: direct config tests for getModelRouteIdentity — call stability, model@<sha-prefix> shape, and the guard keeping the registry baseUrl out of non-active model identities. - R2-3: sendMessageStream now resolves the request route first and invalidates counts against IT (deriving the key from the actual model param on the non-exact branch), so an active-route count can no longer anchor an exact `\0` route's output clamp; regression test added.
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 32 passed · 0 failed · 32 total Flakiness gate: ✅ 4 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:32 通过 · 0 失败 · 32 总计 抖动门:✅ 4 changed test file(s) x 5 identical rounds, no divergence Verification reportVerification report — PR #9506
|
| cell | oracle | result |
|---|---|---|
head 6c6d80ff1d |
7 route-scoped tests | 7 passed / 0 failed |
base 3b3818db87 |
same tests | 6 failed / 1 passed — expected 691000 to be +0 (×3), expected 321 to be +0, expected 4000 to be 8000, plus the pre-send and manual-compress invalidation tests; the only base pass is the same-route control ("keeps counts authoritative while the route is unchanged"), which is correct on both arms |
All base failures are the intended assertion mismatches (expected-vs-actual quoted above), not load errors. The expected 4000 to be 8000 cell shows the foreign 691k count flooring an exact-\0-route request's maxOutputTokens at MIN_CLAMPED_OUTPUT_TOKENS on base.
A/B #2 — mock-free dist harness (harness-route-ab.mjs)
Real compiled Config + GeminiChat + real UiTelemetryService from each tree's dist/, fake content generator as the wire peer (records requests, yields usageMetadata). Counts are seeded through the real stream path (sendMessageStream → processStreamResponse), the route is switched by rebuilding the generator config (as /model does), then every safety surface is read. Witness: evidence/02-dist-harness-ab-cells.png.
| cell | head | base |
|---|---|---|
identity API present & stable (model-a@74348bc4), differs across routes (model-b@cab82cd2) |
pass | absent (control) |
| route A count recorded via real stream (691000) | pass | pass |
getLastPromptTokenCount() after switch |
0 | 691000 (stale) |
| telemetry mirror after switch | 0 | 691000 (stale) |
| first post-switch send on route B | succeeds, wire shows maxOutputTokens: 8000 (full ceiling) |
throws Context is too large to send safely after automatic compression. Estimated prompt tokens: 690808; hard limit: 41000 — hard-tier rescue and compression both trust the foreign count; the #9454 failure sequence reproduced end-to-end |
Head 8/8, base 6/6 (base assertions encode "the bug reproduces"). The base throw is the sharpest consequence of the bug: not just a floored clamp, but a rejected first send after every /model switch with a large prior count.
Reader census (sibling sweep)
Every reader of lastPromptTokenCount/lastOutputTokenCount/mirror classified by reading the call sites:
- Covered by invalidation: public getters (lazy),
sendMessageStreamentry (compares against the request route — exact\0route andmodel-param override included),tryCompressentry,compressFastentry, session-token-limit gate (client.ts:3135, now via chat getter;getChat()is already dereferenced earlier in the same method, so no new null path),chatCompressionServicesharedRequestFits/hasProviderTokenCount(via getters), all in-sendMessageStreamestimate sites (hard rescue 2544/2600, output clamp 2748, reactive recovery 3698 — the latter also takesmaxwith a fresh walk). - Writes attributed:
setLastPromptTokenCount,seedResumeTokenCounts,processStreamResponse(requestrouteKeythreaded through, incl. fallback retry'sfallbackRouteKey), post-compression and resume seeds. - Display-lag only: direct mirror readers (UI context counters; Stop-hook
buildContextUsageatclient.ts:3694) — see Findings. - Out of scope (declared):
--resumeroute drift at save time; subagentagent-core.lastPromptTokenCount(separate short-lived mechanism).
processStreamResponse has exactly one production caller and makeApiCallAndProcessStream two, all passing a route key; goal-turn-integration.test.ts (the only direct test caller) was updated for the new positional.
Vacuity and gates
Witness: evidence/03-vacuity-and-gates.png.
- No-op mutation of
invalidateTokenCountsIfRouteChanged(interface-preserving, scratch edit, restored viagit checkout): route-scoped suite flips to 6 failed / 1 passed with the intended assertion mismatches, and the new client gate test fails (expected [...] to not deep equally contain SessionTokenLimitExceeded). The tests pin the change; tree verified clean afterwards. - Targeted gates on head: 12 affected suites (
geminiChat,client,goal-turn-integration,chatCompressionService,tokenEstimation,sessionService,uiTelemetry,modelsConfig,config,config-session-env,agent-core,forkedAgent.cache) — 1741/1741 tests, 12/12 files passed. The session-limit gate remains exercised in both directions (fire at limit 100/1; no-fire in the new foreign-count test).
Findings (non-blocking)
- Telemetry-mirror display lag between switch and next send. Direct mirror readers (UI context counters, Stop-hook
buildContextUsage) are not invalidated eagerly; a/modelswitch leaves the foreign count visible until the nextsendMessageStream/getter/tryCompressentry resets the mirror. Self-heals on the next interaction and no safety decision reads the mirror directly anymore (the gate now reads the chat getter). Inherent to the lazy-invalidation design the PR documents; an eager reset on switch would be the alternative. Informational. --resumesave-time route drift is not detected (seeded counts are attributed to the resume-time active route). Declared out of scope in the description with a code comment tracking the follow-up; acceptable — the failure mode degrades to the documented first-send estimate fallback only if the route also changes after resume.
Not covered
- No live-provider E2E (no credentials in this job): the harness's fake peer reproduces the engine-level behavior and the bug(core): model switches reuse prompt and output token counts from the previous route #9454 wire shape (usage metadata → count → clamp/compression), not the model-side degradation that produced the original 691k report.
coreToolScheduler.test.ts— PR claims a pre-existing "Plan shell routing" failure on clean main; file untouched by the diff, not run.- Per-commit attribution: checkout is depth 2 (merge, base tip, PR head only); the aggregate
HEAD^1..HEADdiff was verified. The round-3 commit message lists C1/C2/S1-S3/R2-3 responses; their effects are covered by the aggregate A/B and the reader census, not exercised commit-by-commit. - Repo-wide lint/typecheck not re-run here; the base-worktree
tscdiagnostics I hit (@lydell/node-pty,mime/lite,fdir,ignore) are worktree artifacts —packages/core/tsconfig.jsonmaps node-pty types via../../node_modules/..., which a nested worktree lacks — and are absent from the files the diff touches. Headdist/was pre-built by CI and used as-is. - TUI/CLI surfaces: change is engine-internal; no user-visible rendering path was exercised.
Methodology
Environment: node:22-bookworm CI verify container, merge-ref checkout (HEAD merge, HEAD^1=3b3818db87 base tip, HEAD^2=6c6d80ff1d verified head). Base arm: scratch git worktree at baseRefOid under tmp/, core dist/ built there (npm run build -w), nested packages/core/node_modules deps symlinked from the head tree after proving the base tree's collection failure was my worktree's missing nested deps (root hoists ajv@6; core needs nested ajv@8). Realpath asserted: the repro-test closure imports only relative modules, so neither arm crosses the workspace-symlink boundary (node_modules/@qwen-code/qwen-code-core → head tree, unused by the closure). The dist harness imports compiled modules by absolute file URL from each tree; the only stub is the content generator (the wire peer), never the unit under test. Raw logs in logs/ (ab-head-repro.log, ab-base-repro.log, harness-head.log, harness-base.log, vacuity-*.log, gates-head.log, base-build*.log); harness rerunnable via node harness-route-ab.mjs <dist-root> head|base. Base worktree removed after capture.
Flakiness gate log
rounds=5 files=4 skipped=0
file packages/core/src/config/config-session-env.test.ts: (cd packages/core) npx --no-install vitest run ./src/config/config-session-env.test.ts
file packages/core/src/core/client.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/client.test.ts
file packages/core/src/core/geminiChat.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/geminiChat.test.ts
file packages/core/src/core/goal-turn-integration.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/goal-turn-integration.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/core/src/config/config-session-env.test.ts: PPPPP
packages/core/src/core/client.test.ts: PPPPP
packages/core/src/core/geminiChat.test.ts: PPPPP
packages/core/src/core/goal-turn-integration.test.ts: PPPPP
verdict: pass
summary: 4 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/core/src/config/config-session-env.test.ts: P (exit 0)
round 1 · packages/core/src/core/client.test.ts: P (exit 0)
round 1 · packages/core/src/core/geminiChat.test.ts: P (exit 0)
round 1 · packages/core/src/core/goal-turn-integration.test.ts: P (exit 0)
round 2 · packages/core/src/config/config-session-env.test.ts: P (exit 0)
round 2 · packages/core/src/core/client.test.ts: P (exit 0)
round 2 · packages/core/src/core/geminiChat.test.ts: P (exit 0)
round 2 · packages/core/src/core/goal-turn-integration.test.ts: P (exit 0)
round 3 · packages/core/src/config/config-session-env.test.ts: P (exit 0)
round 3 · packages/core/src/core/client.test.ts: P (exit 0)
round 3 · packages/core/src/core/geminiChat.test.ts: P (exit 0)
round 3 · packages/core/src/core/goal-turn-integration.test.ts: P (exit 0)
round 4 · packages/core/src/config/config-session-env.test.ts: P (exit 0)
round 4 · packages/core/src/core/client.test.ts: P (exit 0)
round 4 · packages/core/src/core/geminiChat.test.ts: P (exit 0)
round 4 · packages/core/src/core/goal-turn-integration.test.ts: P (exit 0)
round 5 · packages/core/src/config/config-session-env.test.ts: P (exit 0)
round 5 · packages/core/src/core/client.test.ts: P (exit 0)
round 5 · packages/core/src/core/geminiChat.test.ts: P (exit 0)
round 5 · packages/core/src/core/goal-turn-integration.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): src/core/geminiChat.test.ts — no such file or directory; 1 passed — this review observed 20648, 1590, 22027, 1612, 494, 3799, 542 passed; 4 passed — this review observed 20648, 1590, 22027, 1612, 494, 3799, 542 passed; 332 passed — this review observed 20648, 1590, 22027, 1612, 494, 3799, 542 passed; 283 passed — this review observed 20648, 1590, 22027, 1612, 494, 3799, 542 passed; and 1 more.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/core/src/core/geminiChat.ts:4916 — [review] Telemetry mirror is written with foreign-route counts on exact-route sends (deferred by the code-age rule)packages/core/src/core/geminiChat.ts:1974 — [review] currentRouteKey carries a test-shaped ?. / ?? '' escape hatch that silently disables invalidation (deferred by the code-age rule)
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan(非阻断):src/core/geminiChat.test.ts — no such file or directory; 1 passed — this review observed 20648, 1590, 22027, 1612, 494, 3799, 542 passed; 4 passed — this review observed 20648, 1590, 22027, 1612, 494, 3799, 542 passed; 332 passed — this review observed 20648, 1590, 22027, 1612, 494, 3799, 542 passed; 283 passed — this review observed 20648, 1590, 22027, 1612, 494, 3799, 542 passed; and 1 more。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.14)
- R3-1: hard-rescue rollback now restores tokenCountsRouteKey alongside the counts; tryCompress re-stamps the key to the active route mid-rescue, which left the resurrected request-route count riding the active key past the next entry invalidation (regression test: active-route read after failed rescue must not inherit the override count). - R3-2(2): invalidation zeroes the telemetry cached-content mirror together with the prompt mirror so /context stops rendering a foreign cached count beside a zeroed prompt count; the mirror is documented as best-effort display state between a switch and the next guarded read (R3-2(1)). - R3-4: collapse the requestRouteKey ternary into one getModelRouteIdentity call (the non-exact arm passed exactly the default parameter value). - R3-5: resolve the active-route default lazily after the zero-count fast path instead of eagerly in the default parameter. - R1-3: add the missing compressFast route-invalidation test (third entrypoint; mutation-verified).
|
Closeout for the round-3 review (04:57Z): 5 findings fixed in deca686, 1 deferred to a follow-up issue. One non-force push on top of 6c6d80f.
Verification: full geminiChat.test.ts 338/338, core @qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): src/core/geminiChat.test.ts — no such file or directory; 1 passed — this review observed 20578, 1590, 22030, 1612, 494, 3799, 542 passed; 4 passed — this review observed 20578, 1590, 22030, 1612, 494, 3799, 542 passed; 332 passed — this review observed 20578, 1590, 22030, 1612, 494, 3799, 542 passed; 283 passed — this review observed 20578, 1590, 22030, 1612, 494, 3799, 542 passed; and 1 more.
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan(非阻断):src/core/geminiChat.test.ts — no such file or directory; 1 passed — this review observed 20578, 1590, 22030, 1612, 494, 3799, 542 passed; 4 passed — this review observed 20578, 1590, 22030, 1612, 494, 3799, 542 passed; 332 passed — this review observed 20578, 1590, 22030, 1612, 494, 3799, 542 passed; 283 passed — this review observed 20578, 1590, 22030, 1612, 494, 3799, 542 passed; and 1 more。
— qwen3.8-max via Qwen Code /review (v0.21.15)
|
Round-7 closeout (head 52a1628):
|
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 133 passed · 0 failed · 133 total Flakiness gate: ✅ 4 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:133 通过 · 0 失败 · 133 总计 抖动门:✅ 4 changed test file(s) x 5 identical rounds, no divergence Verification reportVerification report — PR #9506
|
| # | finding | severity | status at head 52a1628b87 |
|---|---|---|---|
| 1 | Telemetry-mirror display lag between a /model switch and the next chat touch |
informational | stands — re-measured: the mirror is only resynchronized inside adoptTokenCountsForRoute (harness cells C2/C5/G1d/G1e); between the switch and the next chat touch it still shows the previous route's value. No safety decision reads the mirror. |
| 2 | --resume save-time route drift not detected |
informational (declared out of scope) | stands — unchanged area; the code comment tracking the follow-up is present at the new head (seedResumeTokenCounts doc). |
| 3 | One-shot modelOverride sends invalidate the active route's count |
informational | improved — the delta's retention supersedes the destructive half of this: the override send now retains the active count under its own key and a keyed/return read restores it (harness G-cells). Residual cost is only the override route's own first-send anchor loss (as for any route without counts) plus FIFO-eviction exposure under 8+ route churn. I agree with the direction; the delta made this cheaper, not worse. |
| 4 | Base-side gate tests fail with a post-gate TypeError (unmocked Turn continuation) rather than a clean assertion diff |
note | stands — re-observed on the new base (A/B #1 base/client-gate: 2 TypeError: Cannot read properties of undefined (reading 'Symbol(Symbol.asyncIterator)') + 2 assertion mismatches). The cell discriminates either way. |
| 5 | PR's claimed pre-existing coreToolScheduler.test.ts "Plan shell routing" failure |
observation | stands — still does not reproduce in this container: 369/369 pass at head; the file is untouched by the diff. |
| 6 | Predelta-f515e339 gate-bypass: fallback-served counts stamped under the fallback key, destroyed by the request-route gate read |
informational | fixed (by 46ef1c8f, re-confirmed at the new head: harness F3 — 99999 readable via the request-route key after a real fallback chain). |
| 7 | Fallback-served counts anchor the request route's decisions (cross-tokenizer anchor, declared tradeoff) | informational | stands — the delta does not change fallback stamping; the code comment still states the tradeoff and reactive overflow recovery remains the safety net. |
Delta since the previously verified head
One commit: 52a1628b (parent verified as 46ef1c8f24 via git rev-parse 52a1628b^1 after deepening the PR ref; the depth-2 checkout alone could not see it). Diff: geminiChat.ts +157/−25, geminiChat.test.ts +35, client.test.ts +51 — no other files, no lockfile.
Mechanism: invalidateTokenCountsIfRouteChanged (destructive zero) is replaced by adoptTokenCountsForRoute (non-destructive crossing). The current slots are retained in a new tokenCountsByRouteKey map (cap 8, FIFO eviction) under their owning route key; if the target route has a retained entry it is adopted back — prompt, output, provenance, and both telemetry mirrors. Zero-slot crossings are attributed to the TARGET key by raw assignment so they can never collide with the just-retained entry. Four supersession sites delete the retained entry on fresh writes (API report in processStreamResponse, resume seed, compression-adjusted write, hard-rescue rollback). tryCompress gains options.requestRouteKey so in-send compression adopts the request route's retained counts and never re-adopts the active route's mid-send; between-sends callers (manual /compress) still compress the active route.
Base movement since the previous round (5715782279 → 9f2342d3, and since the metadata snapshot's 3b3818d → 9f2342d3) touches package.json/lockfile but none of this PR's seven files; the PR leaves the lockfile untouched relative to HEAD^1, so the root node_modules (installed at HEAD) is a clean control for both worktrees. Realpath audit: packages/core has zero @qwen-code/* dependencies and zero bare @qwen-code/ imports in non-test dist/ modules, so the symlinked node_modules/@qwen-code/qwen-code-core → head tree cannot leak head code into the control arms — the harness imports each arm's dist/ by absolute path with all-internal relative resolution.
Central claim and A/B
Central claim (carried): after a model/auth/endpoint switch, API-reported counts recorded for the previous route never anchor the new route's admission, output clamping, or compression decisions. Delta claim (new): crossings are non-destructive — a route's counts survive foreign-route touches and are adopted back intact on return, so the session-token-limit gate (the only SessionTokenLimitExceeded yield site) keeps firing across route alternation instead of reading a destructively zeroed slot.
A/B #1 — PR repro tests, head vs predelta vs base worktrees (head test files copied verbatim into each control tree; identical vitest filters). Witnesses: 01-ab1-head-24-of-24.png, 02-ab1-predelta-3-delta-tests-red.png, 03-ab1-base-20-fail-as-predicted.png.
| cell | head 52a1628b |
predelta 46ef1c8f |
base 9f2342d3 |
|---|---|---|---|
geminiChat route-scoped token counts (14) |
14 passed | 12 passed / 2 failed — exactly the two delta tests: keeps a foreign count intact across a keyless display read (#9506), restores retained counts when the route switches back (#9506) |
12 failed / 2 passed — the two passes are the same-route control and the cached-mirror write-path control, correct on both arms by design; sharpest red: invalidates a stale count before sending on the new route (the base send throws the #9454 error itself) |
geminiChat stamps fallback-served counts under the request route key (1) |
1 passed | 1 passed | 1 failed |
| client session-limit gate (6) | 6 passed | 5 passed / 1 failed — exactly keeps the session limit enforced when turns alternate routes (#9506) |
4 failed / 2 passed (finding-4 TypeError shape ×2) |
config-session-env getModelRouteIdentity (3) |
3 passed | 3 passed | 3 failed (config.getModelRouteIdentity is not a function — API absent on base) |
Head 24/24; predelta red = exactly the three delta tests (attribution verified by name); base 20 red as predicted / 4 controls green. All control-cell reds are intended outcomes and counted as passing expectations (ab1-assert.mjs: 12/12).
A/B #2 — mock-free dist harness, three arms (node harness-route-ab.mjs <tree> head|predelta|base). Witnesses: 04-ab2-dist-harness-head-45-of-45.png, 05-ab2-dist-harness-predelta-defect-cells.png, 06-ab2-dist-harness-base-bug-reproduces.png.
Real compiled Config + GeminiChat + UiTelemetryService + estimatePromptTokens from each tree's dist/ (controls rebuilt, BUILD EXIT: 0); counts seeded through the real stream path, routes switched via the real Config.setModel path, fallback driven through the real retry-exhaustion → MODEL_FALLBACK chain. The only stubs are the wire peers (fake content generators recording stream and non-stream requests separately) and a thin interposition on Config.getBaseLlmClient().resolveForModel for the fallback model id.
| cell | head | predelta | base |
|---|---|---|---|
| identity API present, stable, bare-model shape | pass | pass | absent (control) |
| route A via real stream: 691000 + output 77 + mirrors (cached 555), repeat reads authoritative | pass | pass | pass |
| cross to B: keyless read / mirror | 0 / 0 | 0 / 0 | 691000 leaks (#9454) |
| keyed read for original route after crossing | 691000 restored | 0 — destroyed | n/a (route-blind 691000) |
| retention-map invariant (no entry for the slot key, size ≤ 8) | pass | n/a | n/a |
| first post-switch send on B | succeeds, 1234 stamped | succeeds | throws Context is too large to send safely after automatic compression. Estimated prompt tokens: 690826; hard limit: 177000 — exactly one wire request on B (the compression-rescue query); the send's user message never reached the wire |
orphan guard (prompt-0 response with cachedContentTokenCount: 42) |
mirror stays 0 | mirror stays 0 | mirror 42 (unconditional write) |
real fallback chain: MODEL_FALLBACK event, 1 fallback request, 99999 readable via request-route key |
pass | pass | pass (route-blind) |
| request-route read after crossing away from a fallback-served count | 99999 retained | 0 — destroyed | 99999 (route-blind) |
| G1 crossing back restores prompt/output/provenance + both mirrors | 691000 / 77 / false / 691000 / 555 | 0 (destroyed at first crossing) | foreign 691k also blocks route-B's send (#9454 again); 691000 still rides the slot on return |
G2 keyless display read non-destructive: 500000 survives /context-style reads |
0 → keyed 500000 → 0 | 0 → keyed 0 (destroyed) → 0 | 500000 leaks everywhere |
| G3 alternation vs the gate: A=101 > limit 100, intervening turn on X | gate(X)=0 admit, gate(A)=101 fires | gate(X)=0, gate(A)=0 — limit bypassed (the delta defect) | gate(X)=101 fires on the WRONG route, gate(A)=101 |
| G4 fresh API report supersedes the retained entry | restored 100 → fresh turn 200 → cross → keyed read 200 | keyed read 0 | 200 (route-blind) |
| G5 cap/FIFO: 10 routes × stamped counts | m2=3000 restored; m0=0 evicted at cap; m1=0 evicted by the m2 crossing (FIFO witnessed twice); size ≤ 8, invariant holds | nothing retained | route-blind last write |
G6 in-send compression adopts the request route: slot inspected directly after tryCompress(..., {requestRouteKey: A}) |
slot=1000@model-a (retained count adopted for sizing) | slot=0 (active-route adoption; the count was already destroyed) | route-blind 1000 anchors compression |
| G7 stale-while-absent quantification (see finding 1) | restored 52000; anchor 54004 < walk 92004 | 0; walk 92004 | 74000 (B's count anchors A) |
| H1 setter vs retained entry (edge probe) | transient invariant gap real but unobservable; next crossing supersedes (777, not stale 1000) | n/a | n/a |
Head 45/45, predelta 35/35 (predelta assertions encode "the destructive-zero defect reproduces"), base 29/29 (base assertions encode "the bug reproduces").
Delta-focused mutation matrix (live replay at head; witness 07-mutation-matrix-6-killed-2-survivors-classified.png)
| row | mutation (guard removed) | result |
|---|---|---|
| M1 | retainCurrentTokenCounts no-op (retention disabled) |
killed — the two #9506 geminiChat tests + the client alternation test red |
| M2 | adoption restore branch disabled (crossing always zeroes) | killed — same three tests red |
| M3 | tryCompress ignores options.requestRouteKey (adopts the active route) |
survived — 14/14 + 6/6 green; classified coverage gap (behaviour proven correct by harness G6; no suite asserts it) |
| M3-ctrl | head + the named fixture, unmutated | green — 15/15 (fixture passes on the shipped code) |
| M3b | M3 + fixture | killed by the fixture — adopts the request route retained counts for in-send compression sizing (#9506) red (originalTokenCount forwarded to the compression service drops from 250000 to the active route's ≈0 estimate); 14 siblings green |
| M4 | processStreamResponse no longer supersedes the retained entry on a fresh report |
survived — 14/14 + 1/1 green; classified coverage gap (behaviour proven correct by harness G4, which returns the fresh 200 rather than the stale retained 100) |
| M5 | CONTROL adoptTokenCountsForRoute full no-op |
killed — 11 route-scoped tests red (mechanism pinned; the 3 green are the no-crossing same-route control and two write-path-only mirror tests) |
| M6 | CONTROL client gate keyed read dropped (keyless read) | killed — 3 gate tests red |
Positive controls landed in the same files as their mutants (M5 in geminiChat.ts, M6 in client.ts). Survivors are completeness reporting, not merge conditions: both guards are behaviourally proven by harness cells; the fixtures that would pin them are named above (M3's is written and proven to kill). Tree verified clean after the replay.
Targeted gates (witness 08-targeted-gates-2143-of-2143.png)
13 suites at head — the four changed files plus the PR's declared regression surface: 2143 passed / 2143 (geminiChat 343, client 362, config 534, coreToolScheduler 369, sessionService 176, chatCompressionService 137, modelsConfig 81, uiTelemetry 47, agent-core 33, forkedAgent.cache 21, config-session-env 19, tokenEstimation 18, goal-turn-integration 3). The PR's Reviewer Test Plan steps reproduce per step: the repro filters fail on base/predelta exactly where claimed (A/B #1), pass at head, and the regression list is green.
Corrections
None to prior rounds. One description note (not a code request): see finding 2 — the PR body's "after any switch … fall back to the char/4 estimate" sentence predates this delta and no longer describes returning switches, which restore the exact retained counts.
Findings (non-blocking)
- (informational, new) Restored anchors are stale-low by exactly the absent-route turns. When route A regains the slots after turns happened on route B, A's restored count still measures A's last prompt — the shared history grew by B's turns in between, and the anchor branch of
estimatePromptTokens(lastPrompt + lastOutput + new user message) does not see them. Measured at head (harness G7): A's restored count 52,000; the send-time anchor estimate 54,004 versus the history-walk estimate 92,004 over the same history — the anchor misses ≈38k tokens of B-turn content. Both numbers are documented lower bounds (the walk misses system prompt/tools by ~15–20k; the anchor misses invisible overhead the API count already carried), and reactive overflow recovery remains the documented safety net; for the session-limit gate the delta is strictly better than predelta (which read 0 and admitted unconditionally after any alternation). The tradeoff is consistent with the code's own doc comment but is not spelled out there or in the PR body — worth a one-line note if alternating-route sessions are common. - (informational, new) Description drift. The body's "after any model/auth/endpoint switch, safety decisions … fall back to the documented char/4 history-walk estimate" now holds only for routes without retained counts; returning routes read the exact API-reported values back (the delta's stated intent). Suggest a sentence in the body/release note so the next reader doesn't reason from the stale wording.
- (informational, new) Two delta guards have no test coverage (M3, M4). Both behaviours are correct (harness G6/G4) but nothing in the suite pins them:
tryCompress'srequestRouteKeyadoption andprocessStreamResponse's supersession delete each survived deletion with every test green. The M3 fixture is written and proven to kill (row M3b); the M4 fixture is a small variant of the existing route-scoped tests (seed a count, cross away and back to adopt it, stream a fresh report, cross away, keyed-read must return the fresh value). Completeness reporting, not merge conditions. - (informational, new)
setLastPromptTokenCountneither retains the slot it overwrites nor deletes the retained entry for the key it stamps (unlikeseedResumeTokenCountsand the other three supersession sites), transiently contradicting the map's documented invariant. Measured unobservable (harness H1): while the slot holds the key, keyed reads early-return without consulting the map, and the next crossing overwrites the stale entry — no read ever saw the stale 1000. All production callers are fresh-chat paths (resume/inheritance, empty map), so the edge is reachable only from tests or future callers; adding thedeletewould make the invariant true rather than merely unobservable. - (carried) Telemetry-mirror display lag between a switch and the next chat touch — stands (status table row 1).
- (carried)
--resumesave-time route drift — stands, declared out of scope, code comment tracks it (row 2). - (carried, improved) One-shot
modelOverridesends — retention removes the destructive half of this tradeoff (row 3). - (observation) The PR description's pre-existing-failure claim (
coreToolScheduler.test.ts"Plan shell routing") does not reproduce in this container (369/369 at head); the file is untouched by the diff (row 5).
Not covered
- No live-provider E2E (no credentials in this job): the harness reproduces the engine-level behaviour and the bug(core): model switches reuse prompt and output token counts from the previous route #9454 wire shape (usage metadata → count → clamp/compression/hard-rescue, and the real fallback chain), not the model-side degradation that produced the original 691k report.
- The
client.tsgate is exercised by A/B pre-release: fix ci #1 vitest cells and the mutation matrix, not by the dist harness (driving the realGeminiClientturn machinery mock-free was out of budget; disclosed rather than implied). The exact\0full-turn send path was likewise not driven in the harness (no provider registry); its gate-side selector resolution is covered by the client vitest cells. - Repo-wide lint not re-run; CI's pre-built
npm run buildat HEAD includestsc(typecheck implicitly green at head). - Per-commit attribution: the delta commit verified individually this round (exact diff + three-cell A/B + matrix); commits 1–8 were verified in aggregate by rounds 1–3 (their OIDs and chain are confirmed in this round's
HEAD^1..HEAD^2listing after deepening the PR ref). - TUI/CLI surfaces: change is engine-internal; no user-visible rendering path was exercised. The flakiness gate is run by the workflow itself and published separately.
Methodology
Environment: node:22-bookworm CI verify container, merge-ref checkout (HEAD = 913fcf4 merge, HEAD^1 = 9f2342d3 base tip, HEAD^2 = 52a1628b verified head; depth 2, deepened once read-only over the public PR ref to confirm the delta's parent). Control arms: scratch git worktrees at HEAD^1 (base) and 46ef1c8f (predelta) under tmp/, core dist/ rebuilt in each (npm run build -w @qwen-code/qwen-code-core, exit 0) with node_modules symlinked to the head tree's installs (lockfile untouched by the PR and identical between base tip and head — a clean control); realpath audit confirmed zero @qwen-code/* imports in core dist/, and the harness imported each arm by absolute dist/ path; worktrees removed after capture. The dist harness's only stubs are the wire peers and the fallback-resolution interposition (disclosed in the A/B #2 header); Config, GeminiChat, UiTelemetryService, and the estimator are the real compiled units driven through the real setModel/stream/fallback/tryCompress paths, with runtime field inspection for the retention-map invariant and post-tryCompress slot state. Mutations were scratch source edits applied and restored by matrix-replay.mjs (tree verified clean). Assertion accounting: 109 harness (harness-{head,predelta,base}.log: 45+35+29) + 12 A/B #1 cell checks (ab1-assert.log) + 12 matrix/gate checks (final-assert.log) = 133, all passing, every base/predelta red encoded as an expectation before the run. Raw logs in logs/; harnesses rerunnable via node harness-route-ab.mjs <tree-root> head|predelta|base, node ab1-assert.mjs, node matrix-replay.mjs, node final-assert.mjs, bash ab1-vitest.sh <tree> <arm> <logdir>, bash gates.sh <logdir>.
Flakiness gate log
rounds=5 files=4 skipped=0
file packages/core/src/config/config-session-env.test.ts: (cd packages/core) npx --no-install vitest run ./src/config/config-session-env.test.ts
file packages/core/src/core/client.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/client.test.ts
file packages/core/src/core/geminiChat.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/geminiChat.test.ts
file packages/core/src/core/goal-turn-integration.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/goal-turn-integration.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/core/src/config/config-session-env.test.ts: PPPPP
packages/core/src/core/client.test.ts: PPPPP
packages/core/src/core/geminiChat.test.ts: PPPPP
packages/core/src/core/goal-turn-integration.test.ts: PPPPP
verdict: pass
summary: 4 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/core/src/config/config-session-env.test.ts: P (exit 0)
round 1 · packages/core/src/core/client.test.ts: P (exit 0)
round 1 · packages/core/src/core/geminiChat.test.ts: P (exit 0)
round 1 · packages/core/src/core/goal-turn-integration.test.ts: P (exit 0)
round 2 · packages/core/src/config/config-session-env.test.ts: P (exit 0)
round 2 · packages/core/src/core/client.test.ts: P (exit 0)
round 2 · packages/core/src/core/geminiChat.test.ts: P (exit 0)
round 2 · packages/core/src/core/goal-turn-integration.test.ts: P (exit 0)
round 3 · packages/core/src/config/config-session-env.test.ts: P (exit 0)
round 3 · packages/core/src/core/client.test.ts: P (exit 0)
round 3 · packages/core/src/core/geminiChat.test.ts: P (exit 0)
round 3 · packages/core/src/core/goal-turn-integration.test.ts: P (exit 0)
round 4 · packages/core/src/config/config-session-env.test.ts: P (exit 0)
round 4 · packages/core/src/core/client.test.ts: P (exit 0)
round 4 · packages/core/src/core/geminiChat.test.ts: P (exit 0)
round 4 · packages/core/src/core/goal-turn-integration.test.ts: P (exit 0)
round 5 · packages/core/src/config/config-session-env.test.ts: P (exit 0)
round 5 · packages/core/src/core/client.test.ts: P (exit 0)
round 5 · packages/core/src/core/geminiChat.test.ts: P (exit 0)
round 5 · packages/core/src/core/goal-turn-integration.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
doudouOUC
left a comment
There was a problem hiding this comment.
Not reviewed: coverage — could not read the agents' transcripts (no subagent transcripts at C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\78b1e6fc-e590-4ca5-8752-45dfee6eecce (ENOENT: no such file or directory, scandir 'C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\78b1e6fc-e590-4ca5-8752-45dfee6eecce'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.), so this run cannot show that any of the diff was read.
Not reviewed: verification — could not check that Step 4 and Step 5 ran (no subagent transcripts at C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\78b1e6fc-e590-4ca5-8752-45dfee6eecce (ENOENT: no such file or directory, scandir 'C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\78b1e6fc-e590-4ca5-8752-45dfee6eecce'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.).
中文说明
未审查:覆盖情况——无法读取 agent 的运行记录(no subagent transcripts at C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\78b1e6fc-e590-4ca5-8752-45dfee6eecce (ENOENT: no such file or directory, scandir 'C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\78b1e6fc-e590-4ca5-8752-45dfee6eecce'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.),本次运行无法证明 diff 的任何部分被读过。
未审查:验证——无法检查步骤 4 与步骤 5 是否运行(no subagent transcripts at C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\78b1e6fc-e590-4ca5-8752-45dfee6eecce (ENOENT: no such file or directory, scandir 'C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\78b1e6fc-e590-4ca5-8752-45dfee6eecce'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.)。
— qwen-code-review via Qwen Code /review (v0.21.10)
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent 6b": none — no check was cut short..
Test Plan (not a blocker): src/core/geminiChat.test.ts — no such file or directory; 1 passed — this review observed 20659, 1590, 22027, 1612, 494, 3799, 542 passed; 4 passed — this review observed 20659, 1590, 22027, 1612, 494, 3799, 542 passed; 332 passed — this review observed 20659, 1590, 22027, 1612, 494, 3799, 542 passed; 283 passed — this review observed 20659, 1590, 22027, 1612, 494, 3799, 542 passed; and 1 more.
Deferred under the convergence posture (round 8, not a blocker) — recorded, not requested in this round:
packages/core/src/core/geminiChat.ts:2527 — [review] The request-route-key derivation is hand-written twice (gate + send entry); drift silently disables sessionTokenLimit (R5-1 precedent)packages/core/src/core/geminiChat.ts:2114 — [review] The FIFO-eviction branch of retainCurrentTokenCounts (MAX_RETAINED_ROUTE_COUNTS=8) has no testpackages/core/src/core/geminiChat.ts:2126 — [review] cachedContentTokenCount retention/restore round-trip never exercised with a non-zero valuepackages/core/src/core/geminiChat.ts:2124 — [probe] Retained entries capture cached-content from the mirror, but re-stamping writers never re-sync it — foreign cached count restored on adoption (/context display only)packages/core/src/core/geminiChat.ts:2116 — [probe] FIFO eviction can drop an over-limit retained count under a 10+ route rotation — session limit silently never fires
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度(达到工具调用预算):"agent 6b":none — no check was cut short.。
Test Plan(非阻断):src/core/geminiChat.test.ts — no such file or directory; 1 passed — this review observed 20659, 1590, 22027, 1612, 494, 3799, 542 passed; 4 passed — this review observed 20659, 1590, 22027, 1612, 494, 3799, 542 passed; 332 passed — this review observed 20659, 1590, 22027, 1612, 494, 3799, 542 passed; 283 passed — this review observed 20659, 1590, 22027, 1612, 494, 3799, 542 passed; and 1 more。
收敛姿态下延后(第 8 轮,非阻断)——已记录,本轮不要求修改:共 5 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
|
Round-8 closeout — all 4 Criticals verified real at The four findings were one mechanism cluster:
Verification: 5 new collocated regression tests in All 4 review threads replied with evidence and resolved. The push auto-triggers the review-pr workflow. |
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:
- The FIFO-eviction branch of retainCurrentTokenCounts (geminiChat.ts, MAX_RETAINED_ROUTE_COUNTS) has no test — already disclosed in the round-8 deferral list (review 4990124024), still untested at this commit
- The cachedContentTokenCount retention/restore round-trip is never exercised with a non-zero value (geminiChat.ts:2126) — already disclosed in the round-8 deferral list (review 4990124024), still untested at this commit
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): src/core/geminiChat.test.ts — no such file or directory; 1 passed — this review observed 1590, 22027, 1612, 494, 3799, 542 passed; 4 passed — this review observed 1590, 22027, 1612, 494, 3799, 542 passed; 332 passed — this review observed 1590, 22027, 1612, 494, 3799, 542 passed; 283 passed — this review observed 1590, 22027, 1612, 494, 3799, 542 passed; and 1 more.
Deferred under the convergence posture (round 9, not a blocker) — recorded, not requested in this round:
packages/core/src/core/geminiChat.test.ts:15624 — [review] Restore-branch prompt-mirror write is asserted by no test (mutation survives)packages/core/src/core/geminiChat.test.ts:16099 — [review] Restore-branch retainCurrentTokenCounts never tested with a non-zero slot occupant (mutation survives)packages/core/src/core/geminiChat.ts:2322 — [probe] Compression cache-sharing decision runs mid-flip with the wrong route's countspackages/core/src/core/geminiChat.ts:2060 — [review] Retained counts stay live while other routes grow the shared history
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan(非阻断):src/core/geminiChat.test.ts — no such file or directory; 1 passed — this review observed 1590, 22027, 1612, 494, 3799, 542 passed; 4 passed — this review observed 1590, 22027, 1612, 494, 3799, 542 passed; 332 passed — this review observed 1590, 22027, 1612, 494, 3799, 542 passed; 283 passed — this review observed 1590, 22027, 1612, 494, 3799, 542 passed; and 1 more。
收敛姿态下延后(第 9 轮,非阻断)——已记录,本轮不要求修改:共 4 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
6 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- Telemetry-mirror resync in the adopt restore branch (geminiChat.ts:2072) has no test — already disclosed in the round-9 deferral list (review 4992490025, geminiChat.test.ts:15624), still untested at this commit
- FIFO-eviction branch of retainCurrentTokenCounts (geminiChat.ts, MAX_RETAINED_ROUTE_COUNTS) has no test — already disclosed in the round-8 deferral list (review 4990124024), re-confirmed in round 9, still untested at this commit
- Usage-stamp retain-before-overwrite branch never executed by its named test (geminiChat.test.ts:16084) — already disclosed in the round-9 deferral list (review 4992490025, geminiChat.test.ts:16099), still untested at this commit
- cachedContentTokenCount retention/restore round-trip never exercised with a non-zero value (geminiChat.ts:2126) — already disclosed in the round-8 deferral list (review 4990124024), re-confirmed in round 9, still untested at this commit
- Compression cache-sharing decision runs mid-flip with the wrong route's counts (geminiChat.ts:2322) — already disclosed in the round-9 deferral list (review 4992490025); re-verified this round as efficiency-only (both harm directions degrad…
- Retained counts stay live while other routes grow the shared history (geminiChat.ts:2060) — already disclosed in the round-9 deferral list (review 4992490025); re-confirmed by probe this round (45_005 gate / 50_006 sizing token under-count)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": none — full chunk 3 walked (diff lines 981-1379), plus surrounding source needed to judge it; no check cut short..
Test Plan (not a blocker): src/core/geminiChat.test.ts — no such file or directory; 1 passed — this review observed 20666, 1590, 22027, 1612, 494, 3799, 542 passed; 4 passed — this review observed 20666, 1590, 22027, 1612, 494, 3799, 542 passed; 332 passed — this review observed 20666, 1590, 22027, 1612, 494, 3799, 542 passed; 283 passed — this review observed 20666, 1590, 22027, 1612, 494, 3799, 542 passed; and 1 more.
Deferred under the convergence posture (round 10, not a blocker) — recorded, not requested in this round:
packages/core/src/core/geminiChat.ts:2830 — [review] Rollback route-key restore unreachable-as-different; its mutation survives the whole suitepackages/core/src/core/geminiChat.ts:3728 — [review] Escalation/output-recovery retry requestRouteKey unpinned; undefined-mutation ships greenpackages/core/src/core/geminiChat.test.ts:7007 — [review] Exact-route retry test assertion one-directional; no keyed read for the vision route anywherepackages/core/src/core/geminiChat.test.ts:15908 — [review] Rollback prompt-mirror restore unasserted; deleting the write ships greenpackages/core/src/core/geminiChat.test.ts:7278 — [review] Multi-hop fallback stamping unpinned; past-the-first-hop mutant ships greenpackages/core/src/core/geminiChat.ts:3494 — [review] Reactive-overflow tryCompress requestRouteKey unpinned; undefined-mutation ships greenpackages/core/src/core/geminiChat.test.ts:6952 — [review] getModelRouteIdentity mocks collapse the second arg; gate/send key drift ships green
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 6 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 2)":none — full chunk 3 walked (diff lines 981-1379), plus surrounding source needed to judge it; no check cut short.。
Test Plan(非阻断):src/core/geminiChat.test.ts — no such file or directory; 1 passed — this review observed 20666, 1590, 22027, 1612, 494, 3799, 542 passed; 4 passed — this review observed 20666, 1590, 22027, 1612, 494, 3799, 542 passed; 332 passed — this review observed 20666, 1590, 22027, 1612, 494, 3799, 542 passed; 283 passed — this review observed 20666, 1590, 22027, 1612, 494, 3799, 542 passed; and 1 more。
收敛姿态下延后(第 10 轮,非阻断)——已记录,本轮不要求修改:共 7 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 142 passed · 0 failed · 142 total Flakiness gate: ✅ 4 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:142 通过 · 0 失败 · 142 总计 抖动门:✅ 4 changed test file(s) x 5 identical rounds, no divergence Verification reportVerification report — PR #9506
|
| # | finding | severity | status at head 71bbb45fa0 |
|---|---|---|---|
| 1 | Telemetry-mirror display lag between a /model switch and the next chat touch |
informational | stands — re-verified at the new head: adoptTokenCountsForRoute remains the only mirror-resync point (code inspection; the delta's six hunks touch no mirror-sync path). Harness cells I2/I3 re-measured the mirror write/zero behaviour (all pass). No safety decision reads the mirror. |
| 2 | --resume save-time route drift not detected |
informational (declared out of scope) | stands — the tracking comment is present at the new head (seedResumeTokenCounts doc, geminiChat.ts:2256–2260); area untouched by the delta. |
| 3 | One-shot modelOverride sends invalidate the active route's count |
informational | improved (carried) — retention (round-4 delta) already removed the destructive half; this delta's compression guards additionally keep the override send's compressed count readable under both routes (harness P10/P11 cells). |
| 4 | Base-side gate tests fail with a post-gate TypeError (unmocked Turn continuation) rather than a clean assertion diff |
note | stands — 6 TypeError occurrences in this round's base-arm junit (logs/base-junit.xml), same shape; the cells discriminate either way. |
| 5 | PR's claimed pre-existing coreToolScheduler.test.ts "Plan shell routing" failure |
observation | stands — still does not reproduce: the suite is green at head inside the 13-suite gate (2158/2158); the file is untouched by the diff. |
| 6 | Predelta-f515e339 gate-bypass: fallback-served counts stamped under the fallback key |
informational | fixed (carried) — stamps fallback-served counts under the request route key (#9454) is green at the new head (A/B #1); the delta does not touch the fallback stamp path. |
| 7 | Fallback-served counts anchor the request route's decisions (cross-tokenizer anchor, declared tradeoff) | informational | stands — the delta diff does not touch the fallback stamping or its tradeoff comment; reactive overflow recovery remains the documented safety net. |
| 8 | (R4-1) Restored anchors are stale-low by exactly the absent-route turns | informational | stands — the delta touches compression/rescue interactions only; estimatePromptTokens' anchor branch is unchanged (verified against the delta diff), so the tradeoff is identical to round 4's measurement. |
| 9 | (R4-2) Description drift: "after any switch … fall back to the char/4 estimate" | informational | stands — the PR body still contains the sentence; returning switches restore exact retained counts and compressed counts now anchor both routes after in-send compression, so the wording is still stale. |
| 10 | (R4-3a) tryCompress requestRouteKey adoption unpinned |
informational | fixed by this delta — the request-route stamping is now pinned: mutation M4 (re-key block removed) is killed by stamps the compressed count under the request route when the send ends without usage (#9506). |
| 11 | (R4-3b) processStreamResponse supersession delete unpinned |
informational | stands — the delete line is unchanged by the delta and still has no dedicated test; not re-mutated this round (out of delta scope). Its reachability analysis is unchanged from round 4. |
| 12 | (R4-4) setLastPromptTokenCount transiently contradicts the map invariant |
informational | fixed (hunk 1 of this delta) — the writer now deletes the retained entry for the key it stamps. Mutation M1 shows the delete is behaviourally unobservable today (classified redundant defence — see Findings 2), which is the correct outcome: the invariant is now true for future callers rather than merely unobservable. |
Delta since the previously verified head
Two commits: 52a1628b → f3328335 → 71bbb45f (chain confirmed after deepening the PR ref read-only; all 11 commit OIDs in the metadata snapshot are present in the local HEAD^1..HEAD^2 listing). Diff: geminiChat.ts +91/−10, geminiChat.test.ts +402 — no other files, no lockfile. The two commits are interleaved by concern (compression stamps, rescue rollback, usage stamp), so they were verified as one aggregate delta.
Mechanism, six hunks: (1) setLastPromptTokenCount deletes the retained entry for the key it stamps (invariant fix = R4 finding 4); (2) tryCompress re-adopts the request route after the compression service returns — the real service reads the keyless count getters (verified in chatCompressionService.ts:741,754–755), which adopt the ACTIVE route and flip the slots mid-compression; (3) on COMPRESSED, the retention map is cleared (compression rewrote the shared history every retained entry sizes) and the fresh count is re-keyed to the request route while retained under the active key — the compressed history is shared, so both routes' next gate reads anchor on it; (4) compressFast clears the whole map for the same reason; (5) hard-rescue rollback restores lastOutputTokenCount and the full pre-rescue retention-map snapshot (the rescue's compression consumes/clears entries mid-flight); (6) the usage stamp retains a foreign-keyed slot occupant before overwriting it.
Base movement since the previous round (9f2342d3 → 04886c4354): the new base tip's own commit is review-tooling (fix(review): … #9630 per its message). The merge-result diff (HEAD^1..HEAD) touches only the PR's seven files and leaves the lockfile untouched relative to HEAD^1, so the base worktree is a clean control for the root node_modules. The predelta arm carries a named confound — see Findings 3.
Central claim and A/B
Central claim (carried): after a model/auth/endpoint switch, API-reported counts recorded for the previous route never anchor the new route's admission, output clamping, or compression decisions. Delta claim (new): compression and hard-rescue keep the route-scoping invariants — history rewrites drop every retained count, compressed counts anchor both the active and the request route, and failed rescues restore counts, output halves, and the retention map intact.
A/B #1 — PR-marked vitest tests, head vs predelta vs base worktrees (head test files copied verbatim into each control tree; identical -t "(#9454|#9506)" filter). Witnesses: 01-ab1-head-26-of-26.png, 02-ab1-predelta-7-delta-tests-red.png, 03-ab1-base-23-red.png.
| cell | head 71bbb45f |
predelta 52a1628b |
base 04886c4354 |
|---|---|---|---|
| PR-marked tests (26 at head) | 26 passed | 19 passed / 7 failed — exactly the seven delta tests, attributed by name | 3 passed / 23 failed — all route-scoping behaviour absent |
head assertion checks (ab1-assert.mjs) |
15/15 | (encoded expectations) | (encoded expectations) |
Base greens, examined individually: keeps counts authoritative while the route is unchanged and mirrors cached content alongside a route-stamped prompt count are the same by-design controls as round 4; the third — stamps the compressed count under the request route when the send ends without usage (#9506) — is a route-blind coincidence: on base every keyed read degenerates to the single slot, which holds exactly the asserted 60 000 (see Findings 4). The same test is red on predelta and green on head, so the discrimination chain is intact.
A/B #2 — mock-free dist harness, three arms (node harness-route-ab.mjs <tree> head|predelta|base). Witnesses: 04-ab2-dist-harness-head-32-of-32.png, 05-ab2-dist-harness-predelta-defect-cells.png, 06-ab2-dist-harness-base-bug-reproduces.png.
Real compiled Config + GeminiChat + ChatCompressionService + UiTelemetryService from each tree's dist/; counts seeded through the real stream path, routes switched via the real Config.setModel path, compression driven through the real service's summarization flow (fake wire peer answers the <state_snapshot> side-query with controlled usage, which the real service's token math turns into newTokenCount). The only stubs are the wire peer and the auth-flow resolution of ContentGeneratorConfig (no credentials in this container; disclosed in the harness header and Methodology). Thresholds come from the real computeThresholds over real model windows (MiniMax-M2.5 196 608 → hard = 173 608, the same regime as #9454's 177 000).
| cell | head | predelta | base |
|---|---|---|---|
| identity API present, stable, model-named | pass | pass | absent (control) |
| route A via real stream: 691 000 + output 77 + mirrors (cached 555) | pass | pass | pass |
| cross to B: keyless read / mirrors | 0 / 0 | 0 / 0 | 691 000 leaks (#9454) |
| keyed read for A after crossing / cross-back restore | 691 000 restored / 691 000+77 | same | route-blind 691 000 |
| first post-switch send on B | succeeds, reaches wire, 1234 stamped | succeeds | throws Context is too large to send safely; at most the rescue side-query reached B — the user turn never did |
P8 successful tryCompress(force) between sends |
keyed(A) = 0 (all retained entries dropped) | keyed(A) = 691 000 — stale pre-compression count still adoptable | route-blind compressed count |
P9 compressFast with thinking-strip |
keyed(A) = 0 | keyed(A) = 691 000 — stale entry survived fast compression | no retention map at all |
| P10 override send, hard-rescue compresses below hard, response ends without usage | request route reads the compressed count; active route reads it via the retained entry | request route gate read passes with 0 (session-limit bypass shape) | route-blind slot count |
| P11 same flow, response carries usage 61 000 | request = 61 000; foreign-keyed occupant retained (0 < v < hard) | request = 61 000; occupant destroyed (active = 0) | route-blind (observational) |
| P12 failed hard-rescue rollback (third route seeded in map, anchor 190 000/8 000 on request route) | anchor resurrected; output 8 000 restored; third route's 150 000 survived via the snapshot | anchor resurrected; output lost (0) | anchor resurrected; output 0 (pre-delta) |
| P13 retention-map invariant + cap (≤ 8) | pass | n/a | n/a |
Head 32/32, predelta 29/29 (the five defect cells above are encoded expectations — fail counts only unexpected outcomes), base 21/21 (the #9454 reproduction cells are encoded expectations).
Attribution note (P11): the predelta-vs-head contrast on the occupant is produced by the COMPRESSED re-key block (hunk 3b), not by the stamp's retain (hunk 6) — the dist-level M8 replay below shows head-minus-hunk-6 still retains the occupant. This is exactly why the P11 contrast alone is not cited as proof of hunk 6.
Delta mutation matrix (live replay at head; witness 07-mutation-matrix-6-killed-2-redundant-defences.png; rerun for the capture produced identical rows)
| row | mutation (guard removed) | result |
|---|---|---|
| M1 | setLastPromptTokenCount map-invariant delete |
survived (23/23 green) — classified redundant defence: every production caller operates on a fresh chat (empty map: client.ts:487,514, agent-core.ts:573, client.ts:4386) or immediately after the COMPRESSED-branch clear() (geminiChat.ts:2349–2350). The invariant is now true rather than merely unobservable; correct as it stands. |
| M2 | tryCompress post-service request-route re-adoption |
killed — exactly re-adopts the request route after the compression service flips the slots (#9506) red |
| M3 | COMPRESSED-branch retention-map clear | killed — exactly drops stale retained counts when a successful compression rewrites the history (#9506) red |
| M4 | COMPRESSED-branch request-route re-key block | killed — exactly stamps the compressed count under the request route when the send ends without usage (#9506) red |
| M5 | compressFast retention-map clear |
killed — exactly drops all retained counts when fast compression rewrites the history (#9506) red |
| M6 | rollback output-count restore | killed — exactly restores the output token count when a failed hard-rescue rolls counts back (#9506) red |
| M7 | rollback retention-map snapshot restore | killed — 2 red: restores the retention map when a failed hard-rescue rolls counts back (#9506) + restores the request route key when a failed hard-rescue rolls counts back |
| M8 | usage-stamp retain-before-overwrite | survived vitest (23/23) and a dist-level replay (harness re-run with the block removed from compiled output: 32/32, occupant still retained) — classified redundant defence; see Findings 1 |
| M2M4 | COMBO: re-adoption AND re-key removed | killed — both rows' tests red together; the two attribution guards defend different status paths (failed/empty vs success), neither redundant with the other |
| C1 | CONTROL: adoptTokenCountsForRoute full no-op (same file as mutants) |
killed — 18 route-scoped tests red; the harness demonstrably collects geminiChat.ts mutations |
| — | unmutated control | green — 23/23 |
Survivors are completeness reporting, not merge conditions: both are redundant defences proven unobservable by two independent mechanisms (call-site census for M1; vitest + dist-level mutation replays for M8), not coverage gaps on reachable behaviour. Tree verified clean after the replay; the dist mutation was restored byte-identical (cmp).
Targeted gates (witness 08-targeted-gates-2158-of-2158.png)
13 suites at head — the changed files plus the PR's declared regression surface: 2158 passed / 0 failed (geminiChat, client, config, coreToolScheduler, sessionService, chatCompressionService, modelsConfig, uiTelemetry, agent-core, forkedAgent.cache, config-session-env, tokenEstimation, goal-turn-integration). +15 over round 4's 2143, consistent with the seven new delta tests plus base movement.
Corrections
None to prior rounds.
Findings (non-blocking)
- (informational, new) Hunk 6 (retain-before-overwrite in the usage stamp) is a redundant defence, and its delta test passes for a different reason. Removing the guard survived both the vitest suite (23/23) and the dist-level harness replay (32/32, occupant retained): with hunks 2+3b present, every compression path re-keys or re-adopts the slots to the request route before any usage stamp, so the guard's
tokenCountsRouteKey !== routeKeycondition is unreachable under current code. Correspondingly,retains a foreign-keyed slot occupant when the usage stamp re-keys (#9506)is pinned by hunk 3b's COMPRESSED-branch retain, not by hunk 6 — removing hunk 6 alone leaves it green. The guard is defence-in-depth for future code paths and is correct as it stands; the fixture that would pin it is a variant of test 4 whose mockedgenerateContentStreamperforms a keyless count read on the chat before yielding usage (a/context-style touch between compression and the stamp flips the slots and makes the retain load-bearing). Named, not shipped. - (informational, new) Hunk 1's invariant delete is likewise unobservable today (redundant defence). Call-site census above (M1 row): fresh chats or post-
clear()contexts only. The change is still the right one — it makes the documented map invariant true for future writers instead of merely unobservable (round-4 finding 4 closed). - (note, new) Predelta control arm carries a dependency-drift confound, made visible by a build failure. The PR branch's lockfile (frozen at its Aug-20 main merge) differs from head's by ~2.9k lines; compiling predelta source against head's newer
node_modulesfailstscon unrelated telemetry types (@opentelemetrygained aforceFlushrequirement the older source doesn't implement). The JS still emitted (noEmitOnErrorunset) and every predelta cell ran against it; the base arm's lockfile is identical to head's (clean control). Named rather than installed-around: a full predeltanpm cidid not fit the budget, and the drift touches no code path under test (telemetry exporter types only). - (note, new) One delta test is accidentally green on route-blind base.
stamps the compressed count under the request route when the send ends without usage (#9506)passes on base because keyed reads degenerate to the single slot, which holds the asserted value. It still discriminates where it matters (red on predelta, green on head); recorded so the base-arm green set is not misread as coverage. - (note, carried) Base-arm failure shape. The base client-gate tests still fail with post-gate
TypeErrors (6 occurrences inlogs/base-junit.xml) rather than clean assertion diffs — same as round 4; the cells discriminate either way.
Not covered
- No live-provider E2E (no credentials in this job): the harness reproduces the engine-level behaviour and the bug(core): model switches reuse prompt and output token counts from the previous route #9454 wire shape (usage metadata → count → clamp/compression/hard-rescue, real summarization flow, real thresholds), not the model-side degradation that produced the original 691k report.
- The
client.tsgate and the exact\0full-turn send path were exercised by the A/B pre-release: fix ci #1 vitest cells (included in the 26 head greens), not by the dist harness — driving the realGeminiClientturn machinery mock-free was out of budget; disclosed rather than implied. - Repo-wide lint not re-run; CI's pre-built
npm run buildat HEAD includestsc(typecheck implicitly green at head). The predeltatscfailure is the named confound of Finding 3, not a PR-code result. - Per-commit attribution: the two delta commits were verified as one aggregate delta (exact diff + three-cell A/B + matrix); commits 1–9 were verified in aggregate by rounds 1–4. All 11 snapshot commit OIDs are present in the locally reachable
HEAD^1..HEAD^2listing after deepening the PR ref. - TUI/CLI surfaces: the change is engine-internal; no user-visible rendering path was exercised. The flakiness gate is run by the workflow itself and published separately.
Methodology
Environment: node:22-bookworm CI verify container, merge-ref checkout (HEAD = c52e9c2a5d merge, HEAD^1 = 04886c4354 base tip, HEAD^2 = 71bbb45fa0 verified head; depth 2, deepened once read-only over the public PR ref to confirm the delta chain 52a1628b → f3328335 → 71bbb45f). Control arms: scratch git worktrees at HEAD^1 (base) and 52a1628b (predelta) under tmp/, core dist/ rebuilt in each (npm run build -w @qwen-code/qwen-code-core; base exit 0, predelta exit 1 on the unrelated telemetry type errors of Finding 3 with JS still emitted), node_modules symlinked to the head tree's installs — clean for base (lockfile identical to head's), named-confound for predelta. Realpath audit: zero @qwen-code/* dependencies in packages/core and zero bare @qwen-code/ imports in non-test dist/ modules; the harness imports each arm by absolute dist/ path. Worktrees removed after capture. The dist harness's only stubs are the wire peer (fake content generator recording every request) and the auth-flow ContentGeneratorConfig resolution (fixed authType/baseUrl — route identity hashes exactly those two fields per resolvedModelIdentity, so keys stay stable — plus per-model contextWindowSize from the real preset values); Config, GeminiChat, the compression service, and the telemetry service are the real compiled units driven through the real setModel/stream/rescue/tryCompress/compressFast paths, with runtime field inspection for the retention-map invariant. Mutations were exact-string scratch edits applied and restored by matrix-replay.mjs (source verified clean after; the M8 dist mutation restored cmp-identical). Assertion accounting: 15 A/B #1 checks (ab1-assert.log) + 82 harness assertions (harness-{head,predelta,base}.log: 32+29+21) + 32 M8-dist replay assertions (harness-head-M8dist.log) + 12 matrix checks (matrix-results.json) + 1 gate parse check (gates-junit.xml: 2158 tests, 0 failures) = 142, all passing; every base/predelta defect cell was encoded as an expectation before its run, so fail counts only unexpected outcomes. Raw logs in logs/; harnesses rerunnable via node harness-route-ab.mjs <tree-root> head|predelta|base, bash ab1-vitest.sh <tree> <arm> <logdir>, node ab1-assert.mjs <logdir>, node matrix-replay.mjs.
Flakiness gate log
rounds=5 files=4 skipped=0
file packages/core/src/config/config-session-env.test.ts: (cd packages/core) npx --no-install vitest run ./src/config/config-session-env.test.ts
file packages/core/src/core/client.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/client.test.ts
file packages/core/src/core/geminiChat.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/geminiChat.test.ts
file packages/core/src/core/goal-turn-integration.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/goal-turn-integration.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/core/src/config/config-session-env.test.ts: PPPPP
packages/core/src/core/client.test.ts: PPPPP
packages/core/src/core/geminiChat.test.ts: PPPPP
packages/core/src/core/goal-turn-integration.test.ts: PPPPP
verdict: pass
summary: 4 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/core/src/config/config-session-env.test.ts: P (exit 0)
round 1 · packages/core/src/core/client.test.ts: P (exit 0)
round 1 · packages/core/src/core/geminiChat.test.ts: P (exit 0)
round 1 · packages/core/src/core/goal-turn-integration.test.ts: P (exit 0)
round 2 · packages/core/src/config/config-session-env.test.ts: P (exit 0)
round 2 · packages/core/src/core/client.test.ts: P (exit 0)
round 2 · packages/core/src/core/geminiChat.test.ts: P (exit 0)
round 2 · packages/core/src/core/goal-turn-integration.test.ts: P (exit 0)
round 3 · packages/core/src/config/config-session-env.test.ts: P (exit 0)
round 3 · packages/core/src/core/client.test.ts: P (exit 0)
round 3 · packages/core/src/core/geminiChat.test.ts: P (exit 0)
round 3 · packages/core/src/core/goal-turn-integration.test.ts: P (exit 0)
round 4 · packages/core/src/config/config-session-env.test.ts: P (exit 0)
round 4 · packages/core/src/core/client.test.ts: P (exit 0)
round 4 · packages/core/src/core/geminiChat.test.ts: P (exit 0)
round 4 · packages/core/src/core/goal-turn-integration.test.ts: P (exit 0)
round 5 · packages/core/src/config/config-session-env.test.ts: P (exit 0)
round 5 · packages/core/src/core/client.test.ts: P (exit 0)
round 5 · packages/core/src/core/geminiChat.test.ts: P (exit 0)
round 5 · packages/core/src/core/goal-turn-integration.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Local deep verification — current-base follow-upMaintainer verdict: ✅ merge-ready — 75/75 scripted assertions passed, 0 unexpected failures. Verified PR head The central behavior is load-bearing: the current base reproduces the original cross-route 中文摘要 — 判定:✅ 可合入
EvidenceCompiled dist A/B Mutation matrix Targeted gates and A/A control Full verification reportVerification report — PR #9506
|
| # | previous observation | status at 71bbb45fa0 on current base |
|---|---|---|
| 1 | Telemetry display mirror can show the prior route until the next route-aware chat touch | stands, informational — decision paths use the route-aware chat counters; the mirror remains display-only. The A/B re-measured mirror zeroing on the guarded read. |
| 2 | --resume cannot detect a route that already differed at save time because route identity is not persisted |
stands, declared out of scope — the tracking note remains in seedResumeTokenCounts; no persisted-state change is in this PR. |
| 3 | Returning to a previously used route restores its retained authoritative count rather than always falling back to char/4 | stands, accepted design — re-measured by the keyed restore/cross-back cells. The PR body sentence saying “after any switch” always falls back remains broader than the implemented returning-route behavior. |
| 4 | setLastPromptTokenCount map delete and usage-stamp retain are currently redundant defenses |
stands, non-blocking — M1 and M8 survived again; the other six guards were killed individually. |
| 5 | Author-reported coreToolScheduler “Plan shell routing” failure |
corrected for this environment — current base passes 19/19. Head only failed when reusing persistent .vite-temp/path state; matched clean environments pass on both arms. This is an environment-cache artifact, not a PR regression or a current-base pre-existing failure. |
Central claim and A/B proof
Central claim: API-reported prompt/output counts belong to the model route that produced them. After model/auth/endpoint/request-route changes, a foreign count must not anchor admission, output clamping, compression, fast compression, or the session-token-limit gate; counts from the new route must become authoritative after its response.
A/B #1 — PR-marked tests copied verbatim into the control
Identical command/filter on both arms: vitest ... -t "(#9454|#9506)" with the head test files copied into the current-base tree.
| arm | active tests | result | oracle |
|---|---|---|---|
head 71bbb45fa0 |
26 | 26 passed | every route-scoping assertion green |
current base 3b3818db87 |
26 | 23 failed / 3 passed | the 23 expected reds demonstrate the new tests are load-bearing; the 3 greens are route-blind/by-design controls |
A/B #2 — real compiled core state machine
Witness: 01-dist-ab-head-vs-base.png.
The harness imports each arm by its absolute packages/core/dist path and drives real Config, GeminiChat, ChatCompressionService, threshold calculation, telemetry mirror, setModel, streaming usage stamps, compression, fast compression, override sends, and hard-rescue rollback. The only substitutions are a recording wire peer and content-generator auth/config resolution, because the sandbox intentionally has no provider credentials.
| observable | head | current base |
|---|---|---|
| Route identity API | present, stable, model-qualified | absent (control) |
After A reports prompt 691000, switch to B |
keyless count and prompt/cached mirrors become 0 |
A's 691000 leaks into B |
| First B user turn | reaches wire and B usage 1234 is stamped |
rejected as too large; user turn does not reach wire |
Successful tryCompress |
all retained pre-compression entries are dropped | route-blind single slot |
compressFast |
stale retained entries are dropped | no per-route map exists |
| Override send after rescue compression | request and active route can read the compressed count through the intended slots/map | route-blind slot |
| Failed hard-rescue rollback | prompt 190000, output 8000, route key, and third-route 150000 map entry restored |
output half remains 0 pre-fix |
| Harness totals | 32/32 | 21/21, with defects encoded as expected control outcomes |
This is load-bearing: removing the PR returns the original oversized-request failure, not merely a different internal representation.
Mutation matrix
Witness: 02-mutation-matrix.png.
| mutation | outcome |
|---|---|
| M2 post-compression request-route re-adoption | killed by its named test |
| M3 successful-compression retention-map clear | killed |
| M4 successful-compression request-route re-key | killed |
| M5 fast-compression retention-map clear | killed |
| M6 hard-rescue output-count restore | killed |
| M7 hard-rescue retention-map snapshot restore | killed by two tests |
| M2+M4 combined removal | killed by both attribution tests |
C1 positive control: make adoptTokenCountsForRoute a no-op |
killed, 18 tests red — proves the mutation runner is live |
| M1 fresh-writer map delete | survived, redundant defense under current callers |
| M8 usage-stamp retain-before-overwrite | survived, redundant defense under current compression/re-key ordering |
| Unmutated control | 23/23 green |
The two survivors are completeness reporting, not merge blockers: the behavior is already enforced by adjacent current paths. The source file was restored and verified with host git diff --quiet after the matrix.
Targeted gates and environment control
Witness: 03-targeted-gates-and-aa-control.png.
packages/corebuild: passed for the current trial merge and current base in Node 22 Linux, with dependency-tree identity verified (package.jsonandpackage-lock.jsonbyte-identical across arms).- Current affected/regression surface: 1975 passed / 0 failed across 9 collected suites:
geminiChat,client,config,coreToolScheduler,sessionService,chatCompressionService,uiTelemetry,config-session-env, andgoal-turn-integration. - The first gate run reused the writable install's persistent
.vite-tempand produced twocoreToolSchedulerfailures. They repeated 3/3 on head, while current base passed. Source and test hashes were byte-identical. In a matched clean environment (same/basepath, read-only dependency bytes, fresh tmpfs.vite-temp) head and base both passed 19/19, and the full clean gate passed 1975/1975. The initial two reds are therefore an environment-cache/path artifact and are excluded from the PR verdict. - Four paths named by the previous report no longer exist on the current base:
modelsConfig.test.ts,agent-core.test.ts,forkedAgent.cache.test.ts, andtokenEstimation.test.ts. They are not silently counted as passing.
Findings
No blocking or correctness finding was reproduced on the current merge.
- Informational — two defensive clauses remain unpinned by reachable behavior. M1 and M8 survived again. Keeping them is reasonable defense-in-depth; removing them does not change current behavior. A future regression fixture should make the foreign-slot condition reachable directly if maintainers want each clause independently pinned.
- Informational — PR description overstates universal fallback. A route with no retained count falls back to the history estimate, but returning to a previously counted route restores that route's retained API count. This is the implemented and tested design; the phrase “after any model/auth/endpoint switch ... fall back” is broader than the actual returning-route behavior.
- Environment note — persistent Vite temp state can create deterministic false gate failures. The two scheduler failures were 3/3 in the dirty path and 0/19 in matched clean arms. The clean-path result is the gate evidence used above.
Not covered
- No live-provider E2E: the sandbox has no credentials. The harness reproduces the reported engine/wire shape (provider usage → retained count → route switch → safety decisions), not the upstream model degradation that emitted approximately 691k tokens.
- No persisted route identity at session-save time;
--resumeroute drift before restoration remains explicitly outside this PR. - No TUI/CLI visual behavior changed, so no user interaction flow was exercised.
- The four old regression test paths listed above were removed/renamed by current main and were not replaced speculatively.
- Per-commit attribution across all 11 PR commits was not repeated; the current aggregate head and its trial merge were verified. The last-round delta mutation coverage was fully re-run.
Methodology
Host metadata was fetched with gh for QwenLM/qwen-code; untrusted PR code was executed only in credential-free node:22-bookworm containers with --network none for build/test/harness phases. The tested tree is a clean local trial merge of the exact current baseRefOid and headRefOid. Head and base core workspaces were built separately. Because npm workspaces install both root and packages/core/node_modules, both byte-identical dependency directories were mounted into the base control so internal workspace links resolved to the base tree; omitting the package-level directory was independently shown to produce unrelated false TypeScript resolution errors. Raw logs, JUnit files, harnesses, checks.json, and image evidence are under tmp/pr9506-verify-20260824-093839/.
|
Released in v0.22.2. |
…enLM#9862) * fix(acp): route-scope the session token-limit cache in Session.ts The ACP Session keeps a private `lastPromptTokenCount` fed from streamed `usageMetadata`, reset only when the chat instance changes (#syncPromptTokenCountWithCurrentChat). ACP model switches (unstable_setSessionModel -> setModel -> config.switchModel) rebuild the content generator but keep the same GeminiChat, so a count recorded on the previous route survived and anchored the session-token-limit gate for the new route: any modelOverride send (compression skipped) or any send whose compression attempt throws reached #getPostCompressionTokenCount(null) with the stale pre-switch count and was wrongly dropped with SessionTokenLimitExceeded / stopReason 'max_tokens'. Attribute the cached count to the route that produced it (Config.getModelRouteIdentity) and invalidate it on a route change, mirroring the route-scoping QwenLM#9506 applied to the GeminiChat counts. Same-route counting and the chat-instance reset are unchanged. Fixes QwenLM#9529 * fix(cli): retain acp token counts per route * fix(cli): retain token counts for request route * test(acp): pin override-route token recording for the QwenLM#9529 gate Add a QwenLM#9529 regression test that drives a route override through the full-turn vision selector (fullTurnModelOverride): the first override send streams usage metadata over the session token limit, and a second same-override send whose compression falls back to the cache must then resolve max_tokens — proving the first count was recorded under the override route key, not the active route's. Reverting the record site to the default route key makes the test fail. Also make the hoisted requestRouteKey initialization in #executePromptInner and #runStopContinuation use optional chaining (this.config.getModelRouteIdentity?.(...) ?? ''), matching the #currentRouteKey convention for partial Config mocks; the unguarded call threw for every prompt in the ~340 Session tests whose mock config does not define getModelRouteIdentity. * fix(cli): bound acp route token cache * fix(cli): cover acp route token eviction * fix(acp): drop dead request route key initializers in the send loops The hoisted requestRouteKey initializer in #executePromptInner and #runStopContinuation computed a route identity that was discarded on every turn: the null-stream paths return before any record site, and every path that reaches a record site first assigns requestRouteKey from the send result. Replace both with a plain empty initializer. * test(acp): factor the QwenLM#9529 over-limit usage stream setup into a helper The ~25-line mock setup that streams a 101-token usage metadata chunk on the first send and an empty stream on the second was pasted verbatim in eight QwenLM#9529 session-token-limit tests. Extract it into createOverLimitUsageSendStream next to the existing stream helpers and migrate all eight copies. * test(acp): cover route count eviction in the session token cache The evict-oldest branch in #setLastPromptTokenCount had no coverage: existing tests exercise at most three route identities, so deleting the eviction block, flipping the size comparison, or evicting the newest entry all survived silently. Drive nine distinct route identities (one past MAX_RETAINED_SESSION_ROUTE_COUNTS) through session.prompt, then assert the evicted oldest route reads back no cached count (its send goes out) while a retained route still trips the gate. * test(acp): pin the stop-continuation token record route scope The existing override-route recording test only exercises the primary prompt record site in #executePromptInner; the only Stop-hook gate test drops the continuation send before streaming and never mocks getModelRouteIdentity, so the #runStopContinuation record site was unpinned. Drive a Stop-hook continuation whose send streams over-limit usage under a \0 exact-route override, then assert a second same-override send trips the gate from the cached count — reverting the continuation record site to the default route key makes the test fail. * test(acp): factor the QwenLM#9529 vision-override mock setup into a helper * fix(acp): invalidate the session token cache on every compression rewrite The route-keyed fallback cache was only cleared on a chat-instance change and re-stamped by the pre-send compression hook, so compressions inside GeminiChat.sendMessageStream (hard-tier rescue, reactive overflow — surfaced as StreamEventType.COMPRESSED, which the session loops ignored) left it holding pre-compression counts sized against destroyed history. A returning route's send could then be false-dropped with 'Session token limit exceeded' when tryCompressChat failed. Handle StreamEventType.COMPRESSED in all four session send loops and clear every retained route count on any COMPRESSED result (pre-send or in-send), re-stamping the fresh count under the request route and the active route when they differ — mirroring GeminiChat clearing its keyed counts in the COMPRESSED branch of tryCompress. Move the pre-send record after request-route resolution so the invalidation keys correctly. Update the zero-newTokenCount COMPRESSED test to pin the corrected semantics: after a successful rewrite the pre-compression count must not gate the send (owner-side parity). * fix(acp): re-check abort after the route-key await (QwenLM#9529) * fix(acp): key cron and background-notification usage records by the request route (QwenLM#9529) The cron/loop-tick and background-notification send loops captured the request route key and threaded it into the COMPRESSED handler, but their post-stream usage record still called #recordPromptTokenCount(usageMetadata), whose default route key is the record-time active route. A model switch landing between request and record stored the outgoing route's API-reported count under the incoming route's key, so the next new-route send whose pre-send compression failed was false-dropped with 'Session token limit exceeded' (and, on the cron path, could permanently disable cron via #stopCronAfterTokenLimit). Pass the captured requestRouteKey into the usage record at both call sites, matching the interactive prompt loops. Add collocated tests pinning that each loop records usage under the request route even when the route switches mid-stream. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>






















What this PR does
This PR scopes GeminiChat's API-reported prompt/output token counts to the model route that produced them, and invalidates them when the active route changes. Each count write now records the current route identity (model id qualified by auth type and endpoint, reusing the existing
resolvedModelIdentitymachinery via a new publicConfig.getModelRouteIdentity()), and every safety-decision entrypoint (sendMessageStream,tryCompress,compressFast) plus the public count getters drop counts whose recorded route no longer matches — resetting the telemetry mirror at the same time so the session token-limit gate and compression banners don't keep reading the foreign value.Why it's needed
/modelswitches rebuild the content generator but keep the sameGeminiChatinstance (history is intentionally preserved). The retainedlastPromptTokenCount/lastOutputTokenCountare bare numbers with no association to the route that produced them, andestimatePromptTokenstreats any nonzero count as an API-authoritative anchor that skips the history walk. So after switching models, auth types, or endpoints, one route's serialization size drives another route's admission, output clamping, compression admission, and fast-compression sizing — exactly the oversized-request and failed-compression sequence reported in #9454 (~691k input tokens on an OpenAI Responses route, then the first request after switching to an Anthropic route exceeding its window by ~101k, with reactive and manual compression both trusting the stale count).Reviewer Test Plan
How to verify
Unit-level reproduction (no provider credentials needed):
npx vitest run src/core/geminiChat.test.ts -t "route-scoped token counts"inpackages/core— the three invalidation tests fail becausegetLastPromptTokenCount()still returns the pre-switch count (691000) after the route identity changes.npx vitest run src/core/geminiChat.test.ts(332 tests) pluschatCompressionService.test.ts,tokenEstimation.test.ts,uiTelemetry.test.ts,modelsConfig.test.ts,config.test.ts,client.test.ts,agent-core.test.ts,sessionService.test.ts,forkedAgent.cache.test.ts— all pass. (coreToolScheduler.test.ts"Plan shell routing" fails identically on clean main in my environment — pre-existing, unrelated.)Expected behavior after the fix: after any model/auth/endpoint switch, safety decisions (pre-send hard rescue, output-window clamp, compression admission/fast gate, cache-sharing
sharedRequestFits) fall back to the documented char/4 history-walk estimate, with reactive overflow recovery as the safety net; counts reported by the new route become authoritative again after its first response.Evidence (Before & After)
N/A for user-visible UI — internal engine state. Test evidence:
Before (repro tests fail on the unfixed parent commit):
After (this branch):
Tested on
Environment (optional)
Node.js v24, vitest unit tests and
tsc --buildinpackages/core; no live provider calls.Risk & Scope
--resume(requires persisting route identity in the session transcript — flagged in code as a follow-up); per-turn full-turn-route sends keep their existing count semantics.getModelRouteIdentity), no persisted-state changes.Linked Issues
Fixes #9454
中文说明
这个 PR 做了什么
本 PR 把 GeminiChat 的 API 上报 prompt/output token 计数归属到产生它的模型路由,并在活动路由变化时将其作废。每次写入计数时都会记录当前路由身份(模型 id + 认证类型 + 端点的限定标识,复用现有
resolvedModelIdentity机制,通过新增的公开方法Config.getModelRouteIdentity()暴露);所有安全决策入口(sendMessageStream、tryCompress、compressFast)以及公开的计数 getter 都会丢弃记录路由与当前路由不匹配的计数,并同时重置遥测镜像,避免会话 token 上限检查和压缩横幅继续读取外来数值。为什么需要
/model切换会重建内容生成器,但保留同一个GeminiChat实例(历史是有意保留的)。保留下来的lastPromptTokenCount/lastOutputTokenCount只是裸数字,与产生它们的路由没有任何关联,而estimatePromptTokens把任何非零计数都当作 API 权威锚点、跳过历史遍历。因此切换模型、认证方式或端点后,一种序列化的大小会驱动另一种序列化的准入判断、输出窗口钳制、压缩准入和快速压缩估算——正是 #9454 报告的超大请求与压缩失败序列(OpenAI Responses 路由上约 691k 输入 token,切换到 Anthropic 路由后第一个请求超出窗口约 101k,响应式压缩和手动 /compress 都信任了旧计数)。审阅测试计划
如何验证
单测级复现(无需提供商凭据):
packages/core运行npx vitest run src/core/geminiChat.test.ts -t "route-scoped token counts"—— 三条失效断言会失败,因为路由身份变化后getLastPromptTokenCount()仍返回切换前的计数(691000)。npx vitest run src/core/geminiChat.test.ts(332 条)以及chatCompressionService.test.ts、tokenEstimation.test.ts、uiTelemetry.test.ts、modelsConfig.test.ts、config.test.ts、client.test.ts、agent-core.test.ts、sessionService.test.ts、forkedAgent.cache.test.ts全部通过。(coreToolScheduler.test.ts的 "Plan shell routing" 在我环境的干净 main 上同样失败——属既有问题,与本 PR 无关。)修复后的预期行为:任何模型/认证/端点切换之后,安全决策(发送前强制压缩、输出窗口钳制、压缩准入/快速门控、缓存共享
sharedRequestFits)回退到文档中的 char/4 历史遍历估算,以响应式溢出恢复作为兜底;新路由上第一个响应返回后,计数重新成为权威值。前后对比证据
无用户可见 UI 变化 —— 引擎内部状态。测试证据:
修复前(复现测试在未修复的父提交上失败):
修复后(本分支):
测试环境
环境(可选)
Node.js v24,
packages/core下的 vitest 单测与tsc --build;无真实提供商调用。风险与范围
--resume时检测会话保存时即已不同的路由(需要在会话 transcript 中持久化路由身份——代码注释中已标注为后续跟进);每轮 full-turn-route 发送保持既有计数语义。getModelRouteIdentity),不涉及持久化状态变更。关联 Issue
Fixes #9454