fix(core): share compression cache for equivalent model selectors - #15318

✅ Qwen Triage finished — view run. See the stage comments in this thread for the result.

✅ Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

Thanks for the PR — and for filing #15197 first with the reproduction attached. That is the right order, and it made this gate quick to run.

Template looks good ✓ — every required heading is present, including the Chinese section.

Problem: observed, not theoretical. #15197 carries an executed source-level reproduction that calls the real ChatCompressionService.compress() and holds every input constant except the selector: test-model gets 1 shared / 0 cold, while openai:test-model and openai:test-model\0https://same.invalid/v1 get 0 shared / 1 cold — with a separate control confirming resolveForModel hands back the identical generator and config objects for all three. The issue's own caveat (no real-provider cache hit, latency, or cost measured) is about the size of the benefit, not about whether the route divergence happens. It happens.

Direction: aligned, and the codebase already agrees with it. sideQueryOnMainRoute has driven compression budget sizing since #13432/#13876, so base already treats openai:test-model as the main route when picking a window — the cache gate was the lone holdout still comparing raw strings, and the two checks disagreed about the same value inside one function. docs/design/chat-compression-cache-sharing.md puts the boundary at cache identity ("Sessions using a distinct compaction model stay on the existing path because their cache identity differs"), not at selector spelling, so this makes the implementation match the stated design rather than adding a new rule.

Size: core path touched (packages/core/src/services/**). Production logic 12 lines (5 in chatCompressionService.ts, 7 docs), tests 63, generated/schema 0. Well under both the 500-line escalation and the 1000-line advisory — no maintainer escalation on size.

Approach: this is the minimal change, and it matches what I would have written from the title alone. Reusing the existing predicate instead of introducing a second identity scheme is the right instinct — a new normalizer here would have given the function two competing notions of "same route". Deleting usesMainModel outright rather than OR-ing it in is also right, since the predicate subsumes it. No drive-by refactors or unrelated edits. The settings.md paragraph earns its place: the eligibility boundary is something a user can hit from the picker.

Risk: no high-risk path match (Stage 1e clean). The real risk here is evidentiary rather than structural, and I want to flag it up front because it decides what happens next: the fork CI is sitting at action_required, so none of Qwen Code CI, tui-parity, or Managed Agent gate has executed on 0b62b93e. There is no test evidence for this change yet, and the PR body itself says "Full preflight is not green." A maintainer has to approve the workflow run before those 63 new test lines have been executed anywhere.

Moving on to code review. 🔍

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at 0b62b93ecb1484f1e741f76347562d8d298bf3d5 · re-run with @qwen-code /triage}

Code review

Independent proposal first. From the title and the "why" alone, before reading the diff: the picker persists provider-qualified selectors, the gate compares them against a bare model ID, so the fix has to compare resolved routes rather than strings — and the authority on when a selector maps back to the main generator is the send layer's resolveForModel. The repo already has a predicate that mirrors exactly those terms, so the change should reuse it at the gate instead of inventing a second normalizer. That is what this PR does. It matches my proposal, and I did not find a simpler path it missed.

The load-bearing claim holds. Everything rests on sideQueryOnMainRoute === true implying the shared request lands on the main generator, so I checked the equivalence term by term against resolveForModel (packages/core/src/core/baseLlmClient.ts:844-871):

• model half — the predicate calls resolveModelId(selector, buildModelIdContext(config)), which is character-for-character what BaseLlmClient.resolveModelSelector does (baseLlmClient.ts:1070-1072). Same function, same context builder.
• pin half — effectiveCompactionModel.split('\0') matches splitModelBaseUrl (baseLlmClient.ts:61-70): same \0 separator, same "undefined means unpinned" semantics, both compared against contentGeneratorConfig.baseUrl. (parseModelIdSelector strips the pin the same way before parsing, so the two never disagree about what the model half is.)
• authType half — identical shape: !authType || authType === main.

And the shared path really goes through that resolver: generateText (baseLlmClient.ts:656) calls resolveForModel(model, …) at line 682, so the model: effectiveCompactionModel the gate passes is normalized the same way the cold path's runSideQuery normalizes it. Widening the gate therefore cannot send a shared request to a different generator or with different credentials — which is the failure mode that would have made this unsafe.

It fails toward cold, never toward false sharing. There is exactly one divergence between the two conditions. When resolveModelId returns undefined, the resolver falls back to config.getModel() (→ main route) while the predicate falls back to the raw selector string (→ main route only if it literally equals the model ID). The predicate is the stricter of the two, so an unparseable selector keeps the cold path. Wrong in the safe direction.

No new evaluation path, no new throw. sideQueryOnMainRoute is already computed unconditionally at line 788 for budget sizing; the gate at 946 only reads it. Same const, same method scope (compress() starts at 474), defined well before use — no TDZ, no ordering hazard. resolveModelId can throw on a malformed authType: selector with an empty ID, but that call already happens on every compression in base, so this PR adds no new failure surface. usesMainModel had exactly three occurrences in the file (declaration 925, gate 946, diagnostic 958) and the diff rewrites all three — no dangling reference, so no type break.

Static regression sweep. CI never ran, so I walked every existing test that could flip instead of trusting a green suite:

• clamps the cold output budget when the compaction selector names the main route (#13432) (line 1930) is the only existing test using openai:test-model. It stays on the cold path because it sets lastPromptTokenCountIsEstimated: true, and hasProviderTokenCount gates sharing independently of the model check. The PR body's claim about this test is accurate — I confirmed it in the code rather than taking it on faith.
• The keeps $name on the cold path table (line 1800) has two entries: compact-model (different modelId → predicate false) and enableCacheControl: false (unrelated conjunct). Neither flips.
• openai:compact-model\0OTHER_ENDPOINT (2052), compact-model (2106), and the same-endpoint/different-model case (2149) all resolve to a modelId that differs from test-model → predicate false → unchanged.

So the only behavior that changes is the intended one: equivalent spellings of the main route regain sharing, genuinely distinct model/auth/endpoint stay cold.

New test well-formedness. I can't execute it, so I checked it against the fixtures it uses: makeFixture is synchronous (line 1391), so the non-awaited destructure is correct; expectCacheLog(config, attempted, used) takes the three args it's given; FixtureOptions declares registryBaseUrl; compressionDebugLines is vi.hoisted (line 54) and reset the same way as the existing use at line 2290; compressWith defaults to force: true, originalTokenCount: 180_000, and with the fixture's 220 000 window sharedRequestFits has headroom, so the sharesCache: true arms aren't vacuously passing on an unrelated conjunct. The seven cases line up with the truth table I derived from the predicate by hand.

Two non-blocking notes. The diagnostic string is still 'distinct compaction model', but the condition is now route-based — a same-model/different-endpoint-pin selector reports "distinct model" when the distinction is really the endpoint. Pre-existing wording, unchanged by this diff, so not worth blocking on. Separately, docs/design/chat-compression-cache-sharing.md states the eligibility bullet as "the compression model is the current main model"; the gate now means "on the main route". A one-line clarification there would keep the design doc honest, and the settings.md paragraph you added is a good model for the wording. (That doc has no zh-CN counterpart, but that gap predates this PR — not yours to fix here.)

CI test evidence

The PR's own CI has not executed. All three pull_request-triggered workflows on the head SHA completed with conclusion action_required, which is GitHub holding a first-time fork contribution until a maintainer approves the run. The 90 check-runs on this commit are all bot orchestration (assign, label, precheck-pr, review-pr, triage, verify, tmux-testing) — none of them is a build, lint, typecheck, or unit-test job. I did not run the PR's tests myself; the review above is static.

Not verified, and why:

• not verified: that the 63 new test lines pass. No CI job has run them, and I do not execute PR-derived code. The fixture analysis above shows they are well-formed and consistent with the predicate's truth table; it does not show them green.
• not verified: build, typecheck, lint, format. Same reason — no executed check exists to read.
• not verified: the author's reported numbers (288 focused tests, 298 under independent rerun, the 33 workspace failures reproducing on pristine upstream). These are the author's claim, cited here as context only, not as evidence. The PR carries the review/self-reported label and states plainly that full preflight is not green.
• not verified: real-provider behavior. Neither the issue nor the PR claims a measured cache hit, latency, or cost change, so nothing here overstates the benefit.

Sandboxed verification would settle this: @qwen-code /verify — the load-bearing claim is that a provider-qualified same-route selector now actually reaches the shared request path while a different model, auth type, or endpoint pin stays cold, and right now nothing has executed that: the fork CI is action_required, so the new seven-case matrix has never run and my regression sweep is static reading, not a green suite. The author has read-only access, so /tmux is unavailable (it executes the author's code and gates on the author); /verify is still available as a sponsored run — a maintainer's @qwen-code /verify comment approves the head it was written against, and that run additionally carries a pre-execution risk screen and a full workspace wipe before any PR code runs. Read the resulting report with the same skepticism as the fork's own CI logs: the code under verification is adversarial input, and a crafted PR can shape what the report says even though the sandbox bounds what it can do.

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at 0b62b93ecb1484f1e741f76347562d8d298bf3d5 · re-run with @qwen-code /triage}

Confidence: 3/5 — the code is clean and I could not break it, but nothing has executed it, so approving now would attest to test results that do not exist.

Stepping back. My independent proposal and this diff are the same change, which is the strongest signal I have that the approach is right rather than merely plausible. The five production lines delete a string comparison and substitute a predicate that the same function already computes and already trusts for budget sizing — so this is less "new behavior" than "removing an inconsistency between two checks that were answering the same question differently." I verified the predicate against resolveForModel term by term, confirmed the shared path goes through that same resolver, confirmed the one divergence fails toward the cold path, and walked every existing test that a widened gate could flip. There is exactly one such test and it is held cold by an independent conjunct. Six months from now I would thank the author for this, not curse them: there is one notion of "same route" in the function instead of two.

So why not approve? Because the honest answer to "did I verify it works?" is no. Qwen Code CI, tui-parity, and Managed Agent gate all sit at action_required — held pending maintainer approval of a first-time fork contribution. No build, no typecheck, no lint, no unit test has run on this commit. My regression sweep is a substitute for reasoning about the change, not a substitute for running it: I confirmed the new test is well-formed against its fixtures and consistent with the truth table I derived by hand, but I have not seen it pass, and a wrong assertion in those 63 lines lands broken on main just as easily as a right one. The author says so themselves — "Full preflight is not green" — and the PR carries review/self-reported. That candor is a point in their favor, not a reason to wave it through.

I am deliberately not emitting a deferred-approval marker here. The PR's CI runs are already completed (as action_required), so there is no in-flight run for a finalize pass to wait on, and approving against a head with zero executed checks would be exactly the gap the deferral exists to close.

What unblocks this: a maintainer approves the pending workflow run on 0b62b93e. If Qwen Code CI lands green, this is a merge in my judgment — the code review found nothing blocking, and the two notes above (diagnostic wording, design-doc bullet) are follow-ups, not conditions. If green CI is not enough to settle the behavioral claim, @qwen-code /verify as a sponsored run would, per Stage 2.

⏸️ Deferring to the maintainer. Reason, specifically: this is a core-path behavioral change (packages/core/src/services/**) and Stage 0 Tier 2 asks for 100% confidence with every downstream consumer named. I can name the consumers — the shared-versus-cold compression path, the distinct compaction model diagnostic, and the cache_sharing_attempted/cache_sharing_used telemetry — and I am confident about the production logic. What I cannot do from a static read is certify 63 test lines that have never been executed in any environment I can observe. That is a missing-evidence call, not a code objection, and it needs a human to approve the CI run. No @mention here because the deterministic resolver found no matching area owner for this PR's labels and no human has reviewed it yet — I would rather leave it unaddressed than guess a login.

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at 0b62b93ecb1484f1e741f76347562d8d298bf3d5 · re-run with @qwen-code /triage}

Refreshed the branch with current main (head 6005d65b) so the required Lint & Static lane can validate under the current gate definition — the Qwen Code CI run is underway: https://github.com/QwenLM/qwen-code/actions/runs/40170351822. Also addressed the R1-1/R1-2 review suggestions in 27339451ce: the window guard is now keyed on the send-layer route, and the main-route equivalence is pinned by per-model resolution tests.

merge-ready — 33/33 scripted assertions passed (0 failed) at head b2c8b648b55ec0a113e37adee8f71b97bc541f48 (base 06c1f989715fa812fc26258525ef97627c15b855).

Central claim and A/B

The PR's claim: a compaction selector that routes to the main model (provider-qualified, or endpoint-pinned at the main base URL) should regain compression cache sharing, while a genuinely different model, auth type or endpoint stays cold.

Both arms run ChatCompressionService.compress() through the real service with one input varied per cell; the fake peers are BaseLlmClient.generateText (the shared path) and runSideQuery (the cold path), so route is read off the wire rather than inferred. Harness: ab-matrix.test.ts, copied byte-identically into both arms, run with AB_OUT=… npx vitest run src/services/__ab.test.ts.

Every shared cell also posted the unslimmed history ahead of the directive, and both cache_sharing_attempted and the route flip together — so the change moves the request, not just a flag. Witness: 01-ab-route-flip.png.

C9/C10 are listed as harness-only on purpose: the harness feeds raw selectors into a stubbed getCompactionModel(), which production cannot do (see the corrections item). C11 is the sharpest consequence checked beyond the gate claim: a widened gate means a rejected shared request now costs one wasted request before the existing cold fallback, which the run confirms happens exactly once and reports cache_sharing_used=false.

Corrections to the prior review round

The stage-3 findings were written against 0b62b93e; the head has since moved to b2c8b648 (three merges of upstream/main). Two statements in R2-1 are inaccurate about reachability and are corrected here — this is a correction to the description, not a request to change the code.

R2-1 axis 1 (empty pin) and axis 2 (multi-NUL) are not reachable. reachability.test.ts drives the real Config.getCompactionModel() with each persisted form:

The predicate and the send layer agree on all four. So the two copies cannot drift on those axes through any settings value — only a future edit to one side can, which is exactly the maintenance argument R2-1 makes, and it survives. C10 flipping in the harness is a harness artifact and is not claimed as a product behaviour.

Findings

No blocking findings. Two suggestions:

S1 (Suggestion, pre-existing): the predicate is a hand-maintained copy of the send layer's condition. R2-1's proposed extraction (isMainRouteSelector beside splitModelBaseUrl, called from both resolveForModel's early return and this service) remains worth doing, but for a reason narrower than R2-1 stated: the copies agree on every reachable input today, so the risk is future edits, not current drift. Extracting it also lets the baseLlmClient.ts:861-871 line citation in the comment go away.

S2 (Suggestion, pre-existing wording): the skip diagnostic still says distinct compaction model. The condition is now route-based, so an endpoint-pin mismatch reports "distinct model" when the distinction is the endpoint. Not introduced by this diff.

Mutation matrix

Independent third tree at head; runner run-mutations.mjs; suite = chatCompressionService.test.ts + baseLlmClient.test.ts. Baseline ran through the same runner first and was green (283/283), and every row restored the tree (FINAL_PORCELAIN=clean, SERVICE_VS_HEAD=identical). Witness: 02-mutation-matrix.png.

The report claims per-guard coverage only, and the control row shows the harness can kill: 12 named tests go red. The first M9 I wrote only deleted a debug line and survived; that row was discarded and re-run as the behavioural mutation above, which is why the matrix is trustworthy at all.

Survivor adjudication — both are dead clauses, not coverage gaps. For M8c the instrumented run shows sideQueryOnMainRoute === true at the ceiling read with the guard in place, because the guard's only assignment (effectiveCompactionModel = config.getModel()) makes it a main-route selector by construction, and the guard is the only assignment between the initial selector and the read. So the term cannot be false there under head; M7 (which makes it false) is killed. For M6, the identical expression survives via the || short-circuit (effectiveCompactionModel !== config.getModel() is false whenever !sideQueryOnMainRoute is false). Reading-only for M6's || branch: no build demonstrates it. Both are deletable redundancy, not missing tests.

Vacuity check. The PR's own new tests are not vacuous: run against base source they fail exactly 3 cases (openai:test-model, the pinned-endpoint case, and does not slim the history for an equivalent selector), and they pass 283/283 at head.

Gates

• tsc --noEmit -p packages/core: exit 0 at head.
• chatCompressionService.test.ts + baseLlmClient.test.ts: 283/283 at head, 271/271 at base (the 12 extra are the PR's new cases).
• The PR's own CI has now executed on this head: Test (ubuntu-latest, Node 22.x) pass (48m), Lint & Static pass, Integration Tests (no-AK, No Sandbox) pass, web-shell E2E Smoke pass.

Not covered

• Real-provider cache hits, latency and cost. The issue and the PR both decline to claim them; nothing here measures a provider.
• The rest of packages/cli and the other workspaces. Not run here; the PR's own Test (ubuntu) job is green at this head, which I cite as the PR's CI, not as my measurement.
• Windows and macOS. Not run.
• Behaviour outside the compression gate (budget sizing beyond the cells above, media paths beyond C8's neighbours, /compress manual trigger end to end).
• The || short-circuit branch of the M6 survivor — inference from reading, not a driven build.

Methodology

Three detached worktrees from this clone: head b2c8b648, base 06c1f989, and a separate mutation tree (also head), each with its own pnpm install --frozen-lockfile --ignore-scripts and its own tsc --build packages/core; readlink -f node_modules/@qwen-code/qwen-code-core pointed into its own tree for each arm, so no arm could load another's code. The A/B harness and the reachability probe are single .mjs-style vitest files copied byte-identically into both arms; results land in logs/ab-{base,head}.json and logs/reach-head.json. Raw per-cell logs, the mutation runner and its per-row logs are in logs/. Every number above is read from those files.

Evidence

— DeepSeek-V4.1-Flash-1017 via Qwen Code /verify-pr (v0.26.4; skill c5273cc80df5)

Qwen Code review did not complete successfully. Qwen review exited with status 1. A transient error is retried automatically; if you are seeing this, retry with @qwen-code /review. See workflow logs.

@doudouOUC Done — the complete zh-CN counterpart is added at cb188f27c6 (docs/design/chat-compression-cache-sharing.zh-CN.md), with reciprocal language links below both titles and the route-based model/auth-type/endpoint eligibility wording aligned across the two versions. Thanks for catching the docs/design/README.md requirement.

For the assigned reviewers (@yiliang114 @wenshao) — the current head cb188f27c6 is code-identical to the one @wenshao verified as merge-ready; the only change since is the zh-CN design counterpart @doudouOUC asked for. The actual CI picture on this head:

• Required Lint & Static: SUCCESS (freshness validated under the current gate), plus Test (ubuntu) and the no-AK integration lane green and the Managed Agent gate green — the rollup is green apart from the two items below.
• web-shell E2E Smoke failed 1 of 423 (files.spec.ts:806) with TCP connect errors in the log and 2 flaky retries — the lane's documented network flakiness (ci(smoke): web-shell E2E Smoke lane — settings-exclusions and lazy-session-preview @smoke tests fail under suite load, PR-independent #16065, fix(ci): isolate web-shell smoke from shared runner networks #15886/Revert "fix(ci): isolate web-shell smoke from shared runner networks" #15961). This diff touches packages/core compression plus docs only, no web-shell files.
• review-pr did not produce a review on this head: the Nov-1 attempts hit the repo-wide pipeline incident, the Nov-3 run exhausted the default 180-minute budget (3h0m46s), and the latest attempt was skipped. None of these is a code verdict.

If anything else is blocking from your side, happy to address it; otherwise an approve + merge whenever convenient would be much appreciated — happy to help with any follow-up in a separate PR (e.g. the R2-1 predicate extraction wenshao suggested keeping as a follow-up).

⚠️ Failed to process this request. Please re-mention the bot to retry.

@qwen-code /review --timeout=360

@qwen-code Retrying the request from #15318 (comment) — the previous attempt failed to process. Summary unchanged: head cb188f27c6 has the required Lint & Static green (plus Test/integration/gate), the two non-required reds are the web-shell smoke network flake (#16065) and the review-pipeline infra failures; the only change from the merge-ready-verified b2c8b648 is the zh-CN design doc. Requesting the reviewers confirm nothing else blocks, then approve and merge.

@wenshao Sorry to ping again — it looks like your agent run failed twice at its final step: once at the end of the full verification report on b2c8b648 (the report itself landed complete — 33/33, no blocking findings — but its footer shows the failure), and once when you tried to process our request above.

We did try re-mentioning the bot as the footer suggests (the two @qwen-code comments above), but the review workflow's authorize gate only accepts comments from write-access principals, so our attempts were skipped — that retry can only come from your side.

If it saves a step: your verification already reads as complete, the only change since b2c8b648 is the zh-CN design doc @doudouOUC asked for, and the required checks are green on the current head. A direct approve would be all that's left from our reading — but however you'd like to handle it, happy to help.

⚠️ Failed to process this request. Please re-mention the bot to retry.

@yiliang114 as assignee, a quick summary now that this PR is fully green at 4c698c47: every check passes (required Lint & Static included — 31 pass / 0 fail), the automated review completed on this head with no new findings (R1-1/R1-2 confirmed fixed; R2-1 corrected by wenshao as unreachable and deferred), wenshao's own verification verdict is merge-ready with no blocking findings (33/33, A/B + mutation matrix), and @doudouOUC's zh-CN documentation requirement is satisfied. The only thing left is two approvals — would you have a window to review? Happy to walk through any part.

从验证侧的复核已完成，所有声明均已对照实时状态核实。以下是公开回复：

Confirming from the verification side — nothing is blocking from my side at the current head 4c698c47. Note the head has moved since the comment above (cb188f27c6 → 4c698c47, a merge of main), so I re-checked the delta:

• The merge-ready verdict carries over. Blob-for-blob, the three code/test files (chatCompressionService.ts, chatCompressionService.test.ts, baseLlmClient.test.ts) are identical to the head I verified (b2c8b648, 33/33 scripted assertions). The only branch-authored commit since then is cb188f27c6 — the zh-CN design counterpart plus reciprocal links — and the net diff against base is still the same six files.
• Both items open on Nov 4 are closed on this head. web-shell E2E Smoke is green on all four shards plus the rollup (no rerun or flake attribution needed), and the automated review did post on this head (Nov 5, partial with disclosed gaps) with zero new findings — its only confirmed item is the already-reported R2-1 suggestion.
• Gate status: all four required checks are green (Lint & Static, Classify PR, Test (ubuntu), Integration Tests (no-AK)). The only remaining requirement is two approving reviews on this head; there are currently none.

R2-1 stays a deferred, non-blocking suggestion as discussed — its thread is still open, but thread resolution is not part of the merge gate.

@wenshao Thank you for the thorough re-verification — that closes every open question on this side. Since your verification already covers everything a review would: would you convert that conclusion into one of the two formal approvals on this head? The second we'll ask of @yiliang114 or @doudouOUC.

Conclusion unchanged, but it no longer covers the review head — my round ran on b2c8b648; the head is now 4c698c47.

The added surface between those two commits is documentation only, and I verified that at blob level rather than by reading the diff stat:

The PR's only own commit after my round is cb188f27c6 docs(core): add the zh-CN counterpart of the cache-sharing design; everything else in that range is upstream/main. The new Chinese design doc has three matching section headings against the English one, so the AGENTS.md bilingual requirement is now met — that gap was open when I ran and it is closed, which is the one thing about my report that has gone stale.

On the current head, CI is fully green: all eight Linux unit tests shards, Lint & Static, Test (ubuntu-latest), Integration Tests (no-AK), web-shell E2E Smoke, OpenTUI no-flicker gate, Managed Agent gate.

Two things I want to be precise about rather than quietly helpful:

• I do not submit formal approvals. Verifying a change and attesting it as a maintainer sign-off are different acts, and the second one is the human's to make. My round produced a verdict, not an approval, and that holds on this head too.
• An approval has to be cast on 4c698c47. A stamp on the old head would be dismissed as stale the moment the branch moves, so the sign-off has to land on the head the merge will use.

The one unresolved thread is still R2-1, a Suggestion. My round's finding on it stands: the two divergent axes it names (empty pin, multi-NUL) cannot reach onMainRoute() through a real Config.getCompactionModel(), which normalizes both, so the drift is a maintenance concern for the next edit to baseLlmClient.ts, not a defect here. Resolving that thread as a follow-up is reasonable if you want the PR clean; the extraction itself does not need to gate the merge.

— DeepSeek-V4.1-Flash-1017 via Qwen Code /verify-pr (v0.26.4)

Understood — thanks for the blob-level re-verification and the precise verdict/approval distinction. The formal approvals then rest with the human reviewers on 4c698c47: @yiliang114 (pinged above) or @doudouOUC.

@doudouOUC following up on this one for review when you have a window. Where things stand on head 4c698c47:

• @wenshao's verification round concluded 33/33 merge-ready; his latest pass re-verified at blob level and confirmed the delta to the current head is documentation only (his verdict is explicitly not a formal approval, so the two write-access approvals are still open).
• Required checks are green on this head.
• The Chinese documentation you asked for in the earlier round is included at this head.

The functional change stays concentrated in the compression service's main-route predicate extraction; the rest is tests. Happy to walk through any part of the diff.

One correction to the summary above, because it cites my round:

The change is not a predicate extraction. onMainRoute is added locally in chatCompressionService.ts as a duplicate of the send layer's condition — packages/core/src/core/baseLlmClient.ts is untouched (only its test file changed, baseLlmClient.test.ts). Nothing was moved or shared; the send layer keeps its own copy, and the comment at the new predicate cites baseLlmClient.ts:861-871 by line number, which will drift on the next edit to that file.

That distinction is exactly the open R2-1 thread, so a reviewer reading this for the first time should see "local predicate introduced, duplication retained", not "duplication removed". Extraction is what R2-1 suggests and what has not been done here.

It does not change my verdict, and I want to be clear that it is not a request for a code change in this PR: I verified that the two copies agree on every input reachable through Config.getCompactionModel(), which is why I rated the extraction maintenance-only rather than blocking.

— DeepSeek-V4.1-Flash-1017 via Qwen Code /verify-pr (v0.26.4)

Understood, and thanks for catching it before a reviewer reads the thread cold — the "predicate extraction" wording in my summary was wrong. For the record: this PR introduces onMainRoute locally in chatCompressionService.ts as a duplicate of the send layer's condition, baseLlmClient.ts keeps its own copy (only its test changed), and the extraction itself is the R2-1 follow-up (#16473), not done here. No code change requested or made.

merge-ready — 85/85 scripted assertions passed (0 failed) at head 4c698c473a742931ca826e6d38f8d95a552c956e (base 2be58e52eef2cf74305ec4dc69c1c7ec48b11526). Round 2: the three code/test blobs are byte-identical to the head round 1 verified (b2c8b648b55ec0a113e37adee8f71b97bc541f48); the delta is one docs commit plus a merge of main.

Previous round status

Round 1 ran at b2c8b648b55ec0a113e37adee8f71b97bc541f48 (base 06c1f989) and reported merge-ready with two suggestions.

The gap round 1 left open — its round predated the zh-CN design counterpart — is closed: docs/design/chat-compression-cache-sharing.zh-CN.md is present and the English doc links to it.

Central claim and A/B

The PR's claim: a compaction selector that routes to the main model — provider-qualified (openai:test-model) or endpoint-pinned at the main base URL — should regain compression cache sharing, while a genuinely different model, auth type or endpoint stays cold.

Both arms run the real ChatCompressionService.compress(); the peers are BaseLlmClient.generateText (shared path) and runSideQuery (cold path), so the route is read off which peer served the call, not off a flag. The harness is byte-identical in both arms (__ab_matrix.test.ts, copied into each worktree, run with AB_OUT=… npx vitest run --coverage.enabled=false).

Witness: 01-ab-route-matrix.png.

The 4 target flips are C3-qualified-equivalent, C4-pinned-equivalent, C11-colon-main-id-qualified, C18-qualified-equivalent-other-auth-entry (the last of those has a main model id containing a colon). C13 is the widened gate's sharpest consequence: a rejected shared request now costs one wasted request before the existing cold fallback, which the run confirms happens exactly once and reports cache_sharing_used=false. The remaining flip, C17, is the description correction below.

Every shared cell in both arms posted the unslimmed history (sizeDelta>0 against the stored history) with promptCacheSharing:true, so the change moves the request, not just a diagnostic.

C9/C10 are the window-guard interaction: C9 (genuinely distinct model, tiny window) still warns and downgrades to the main model, while C10 (equivalent selector, same tiny registry window) now skips the guard entirely and no longer pays for the slimming estimate (base: 1 call, head: 0) — that is what the hoist is for.

Corrections

To the PR description, not to the code. The evidence table says "Default or bare main model: 1 / 0 before and after". That holds when the main model is registered under the session's auth type (C1, C2), but not universally: in C17 the session's main model is registered only under a different auth type, and the bare-main selector goes shared → cold. That is the correct outcome, and it is not a regression this PR introduces: the same config makes the real BaseLlmClient.resolveForModel() build a per-model generator instead of returning the main one, so round 1's gate was sharing the cache on a request that had already left the main route. The description should say "a selector the send layer actually routes to the main generator", which is what the code now does, rather than enumerating "default / bare main model" as unconditionally unchanged. Reachability of that registry shape from a real settings file is not established here; the direction is safe either way, because the gate can only become more conservative.

The two-ends table below is the measurement behind that statement. A cold gate on a main route (C12, unmatched endpoint pin) is the conservative direction and is pre-existing — it is listed for completeness, not as a defect.

PR 15318 two ends: compression cache gate vs BaseLlmClient.resolveForModel
(gate read off which peer served compress(); send layer is the real resolver)

cell                                           gate       send layer   agree?
----------------------------------------------------------------------------------
C2-bare-main                                   shared     main         yes
C3-qualified-equivalent                        shared     main         yes
C4-pinned-equivalent                           shared     main         yes
C5-distinct-model                              cold       other route  yes
C6-distinct-auth                               cold       other route  yes
C12-pin-trailing-slash                         cold       main         NO (cold gate, main route)
C17-bare-main-registered-under-other-auth      cold       other route  yes
C18-qualified-equivalent-other-auth-entry      shared     main         yes

disagreements: 1 — a cold gate on a main route is conservative (missed sharing);
a shared gate on another route would be the unsafe direction.

Witness: 02-two-ends-gate-vs-send-layer.png.

Findings

No blocking findings.

F1 (Suggestion — description accuracy). Narrow the "default or bare main model is unchanged" line to "a selector that the send layer routes to the main generator". Evidence: C17 in the A/B table plus the two-ends row for the same config. Not a code change request: the code's verdict matches the send layer, and reachability of the registry shape is unproven.

F2 (informational, pre-existing). An endpoint pin that the registry cannot match (C12, trailing slash) stays cold even though resolveForModel falls back to the main generator for it. That is a missed sharing opportunity, not a wrong route, and it behaves identically on both arms.

Mutation matrix

Independent runs of the two suites that pin this behaviour (chatCompressionService.test.ts + baseLlmClient.test.ts), each mutation applied with a single-match anchor, run, then restored with git checkout HEAD -- and verified clean before the next one. Baseline through the same runner first: 283/283 green, so the kills below are not a pre-red suite. Positive control: a mutation that forces onMainRoute to false (a behaviour several tests pin) is killed by 33 tests, so the runner can make this suite fail.

Final tree state after the matrix: FINAL_PORCELAIN=clean. Witness: 03-mutation-matrix.png.

Every row's expectation was fixed before the run from the PR's own claims; M6 is the one row expected to survive, because round 1 measured the same wording unprotected and the blob is unchanged. A survivor there is a coverage gap for a diagnostic string, not a behavioural hole.

Not covered

• Real-provider cache-hit rate, latency and cost. The PR does not claim them either; the harness oracle is the route taken, not a provider's cache accounting.
• Re-running the matrix on the trial-merge tree. git merge-tree --write-tree 4c698c47 origin/main is conflict-free (tree b1f92469a05e), but main moved this PR's file after the base: packages/core/src/services/chatCompressionService.ts gains movePromptContextAheadOfUserText(sideQueryHistory) on the shared payload, and its test gains the matching case. The merged tree therefore contains both changes; the route decision measured here is upstream of that payload construction, but I did not re-run the matrix on the merged tree.
• The previous round's "dead clause" observations (the sideQueryOnMainRoute term in the reactive-ceiling ternary and its diagnostic sibling) were not re-measured this round; they are not part of the central claim.
• Reachability of the C17 registry shape from a real settings file — measured only as a config shape handed to the gate and the resolver.
• Windows/macOS-specific behaviour, hosted/managed-agent paths, and the CLI suites (the PR's CI covers Test (ubuntu), all eight Linux unit shards, Lint & Static and Integration green on this head).

Methodology

macOS arm64 host, Node 22.23.4, vitest 3.2.7. Two detached worktrees at 2be58e52 (base) and 4c698c47 (head), each with its own seeded node_modules; readlink -f node_modules/@qwen-code/qwen-code-core resolves inside each arm's own tree (not the enclosing checkout), so neither arm can load the other's source. packages/core was emitted into each arm with tsc --build --noCheck per arm. The A/B harness imports the service from source and stubs only the two peers, so the unit under test is the shipped code path. Raw logs, per-cell JSONL, vitest JSON reports per mutant and the harness scripts are in this artifact directory; logs/ab-ledger.json is the assertion source for the counts above.

Evidence

image	what it shows
	18-cell A/B route matrix, base vs head, same harness in both arms
	the cache gate against the real send-layer resolver on the same configs
	mutation matrix as it printed, including the positive control

Released in v0.27.1.