docs(review): name the axis when two of them both call it "rank 3" - #9759
Merged
Conversation
The trim-rank move gave `trim` a rank 3, and the file already used a bare
"rank 3" for the `keep` default on the last-resort cut's axis. The two mean
opposite things — `trim: 3` is the LAST rank the ladder sheds, `keep: 3` is
the FIRST thing the cut spends — so a reader landing on the cannot-tell
block's "Deliberately untagged (rank 3, spent first by the last-resort cut)"
now reads it against a comment three hundred lines up saying rank 3 goes
last.
Every mention of the number on either axis now says which axis it is on:
the cannot-tell block states the collision outright, and the four trim-side
mentions and the one keep-side test comment are qualified. The `keep`
comment that already disambiguated itself ("No `trim` rank rides here") is
unchanged.
Comments only, no behaviour. Raised by the review as a deferred, non-blocking
item; taken now because it is the same class of drift the previous commit
closed, and shipping the ambiguity would have seeded the next one.
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: confirmed real, not theoretical. The current tree carries the collision exactly as described: Direction: aligned. This ladder is maintained through its comments — the retuning instructions for the sized overflow tests live in them, and #9715's own review caught five stale rank-order assertions — so a number meaning opposite things on two axes is the next instance of exactly that failure mode, already sitting in the tree. Size: not applicable — Approach: scope feels exactly right. Every edit serves the stated goal — each comment mention of the number names its axis, the cannot-tell block states the collision outright rather than only qualifying itself, and the one already-disambiguated comment ("No Risk: no elevated risk signals — no high-risk paths matched, and every changed line is a comment (verified mechanically against the diff). Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：确认是真实存在的，不是理论性的。当前代码树里就有这个撞名： 方向：对齐。这个阶梯靠注释维护——定容溢出测试的重新调参说明就写在注释里，而 #9715 自身的评审抓到过五处过时的档位顺序断言——所以一个数字在两条轴上含义相反，正是同一失效模式已经躺在树里的下一个实例。 规模：不适用—— 方案：范围恰好。每一处改动都服务于目标——每处出现该数字的注释都点明所属轴，cannot-tell 块直接把撞名写出来而不只是自我限定，已经自行消歧的那处注释（"No 风险：无升级风险信号——未命中高风险路径，且每一处改动行都是注释（已对 diff 机械校验）。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewBefore reading the diff, my independent proposal for this problem was exactly what this PR does: annotate every comment mention of the shared number with its axis, and let the cannot-tell block state the collision outright — because the root fix (renaming one axis or giving What I verified against the code:
No blockers, no findings. TestingUnattended CI run — per policy I do not build or execute PR code; the evidence below is this PR's own CI on the reviewed commit, fetched via the API. At review time the substantive check — This PR carries no behavioural claim — comments only, no runtime or user-visible surface — so no sandboxed lane ( Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 中文说明代码审查读 diff 之前，我对这个问题的独立方案与本 PR 一致：给每处提到该共用数字的注释标明所属轴，并让 cannot-tell 块直接写出撞名——因为根上修复（重命名一条轴或给 对照代码核实的内容：
无阻断项，无发现。 测试无人值守 CI 运行——按规则不构建、不执行 PR 代码；以下证据来自该 PR 自身在受审提交上的 CI，经 API 获取。 审查时实质性检查—— 本 PR 不含行为性声明——纯注释、无运行时或用户可见面——因此没有沙箱通道（ （CI 明细见上方表格） — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 5/5 — comments-only, the collision is real and verified in-tree, every new sentence checks out accurate against the code; nothing left to want. Stepping back: this is the tail of the review on #9715 — an item raised there, deferred as non-blocking, and carried here so it is not lost. That provenance matters — it is not a theoretical-tidiness PR but the completion of an observed cleanup, on a ladder that is maintained through its comments (#9715's own review caught five stale rank-order assertions in them). The implementation does the minimal thing: seven comment sites name their axis, the cannot-tell block states the collision outright instead of only qualifying itself, and the one comment that already disambiguated itself is left alone. My independent proposal before reading the diff was the same, and I found no simpler path inside the deferred scope — renaming an axis would be the root fix and is correctly left to its own change. Risk is nil: no executable line changed (verified mechanically), no behavioural surface, and CI is the only test evidence that matters. One caveat — and the reason this comment does not approve yet: the Linux unit suite ( 中文说明置信度：5/5 —— 纯注释改动，撞名真实存在且已在代码树中核实，每一句新注释对照代码都准确无误；没有可再挑剔之处。 退一步看：这是 #9715 评审的收尾——当时作为非阻断项延后，由本 PR 承接以免丢失。这个出处很重要——它不是追求理论整洁的 PR，而是一次已观察到问题的清理的完成，且这个阶梯正是靠注释维护的（#9715 自身的评审就在其中抓到过五处过时的档位顺序断言）。实现做的是最小改动：七处注释点明所属轴，cannot-tell 块直接把撞名写出来而不只是自我限定，已经自行消歧的那处注释保持不动。我在读 diff 之前的独立方案与之相同，在延后项的范围内也没有找到更简的路径——重命名某条轴是根上修复，正确地留到单独的改动。 风险为零：没有任何可执行行被改动（已机械校验），无行为面，CI 是唯一需要的测试证据。 一个保留——也是本条暂不附带批准的原因：审查时 Linux 单测套件仍在运行。纯注释 diff 不可能改变测试结果，但认证规则不开口子——批准延后至该提交的 CI 全绿后自动落章；若有任何检查变红则予以搁置。 — Qwen Code · qwen3.8-max Reviewed at |
Contributor
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
approved these changes
Aug 23, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
approved these changes
Aug 23, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
Test Plan (not a blocker): 650 passing — this review observed 23337 passed.
中文说明
未发现问题。LGTM！✅
Test Plan（非阻断）：650 passing — this review observed 23337 passed。
— qwen3.8-max via Qwen Code /review (v0.22.0)
wenshao
enabled auto-merge
August 23, 2026 02:00
wenshao
added a commit
that referenced
this pull request
Aug 23, 2026
main landed #9759, a comments-only pass that names the axis wherever a comment says "rank N", because `trim` and `keep` now both use small integers for opposite orderings — `trim: 3` sheds LAST, `keep: 3` is spent FIRST. Resolutions: - `noteTrimmedRanks` docblock: #9759 edited the sentence this branch had already replaced wholesale. This branch's content stays and adopts the discipline — every number in it now says `trim rank`. - `lib/convergence.test.ts`: main added an `isFreshDraft` describe (#9674's fix-induced marking) where this branch's fixtures sit, and both sides were cut mid-`it` sharing one terminator. main's block goes first, where main put it; this branch's follows. Two comments of this branch's own were STALE from last merge's renumber, and neither is reachable by a test — found by reading, in exactly the class #9759 exists to prevent: - the artifact-pointer note still said "Ranks 2 and 3 can drop alone — rank 3 does on any run with disclosures", written when disclosures were rank 3; they are trim rank 2 now and the advisory is trim rank 0. - `Bi.trim`'s docstring still described the pre-renumber order (deferral list 1 -> advisory 2 -> disclosures 3). It now states the current order and why it runs that way, and says outright that `keep` above it is a different axis. One of main's own comments became ambiguous because of this branch, so it is qualified rather than left: the observation's "Shed second (it was rank 0)" is historically true, and rank 0 is now the residual-risk advisory's slot. `isFreshDraft` gaining the fix-induced arm changes what `postedFresh` counts, which is the number this branch's window runs on. That is the intended direction — a NEW defect wearing an old id is first-time work — and both ends of the window are computed the same way in successive rounds, so the comparison stays like-for-like. Mutation matrix re-run after the merge: the advisory sharing trim rank 1, the tail clause keying on the advisory, reverting the window to totals, deleting the backlog veto, deleting the enforcement-evidence guard, and disabling the pure-foreign gate each still turn the suite red. packages/cli 4692 pass / 1 skipped; packages/core skills 384 pass; typecheck, ESLint and Prettier clean on both.
Collaborator
|
Released in v0.22.2. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Names the axis wherever a comment says "rank 3", because two different orderings in
compose-review.tsnow use that number for opposite things.trim: 3is the LAST rank the body-budget ladder sheds — that landed in #9715.keep: 3is the default on the last-resort cut's axis, and it is the FIRST thing that cut spends. Both were already written as a bare "rank 3", so the cannot-tell block's// Deliberately untagged (rank 3, spent first by the last-resort cut)now reads directly against a comment three hundred lines above it saying rank 3 goes last. Both sentences are true; they contradict each other only because the number is shared.Every mention of the number on either axis now says which axis it is on. The cannot-tell block states the collision outright rather than only qualifying itself. The one
keepcomment that already disambiguated itself — "Notrimrank rides here" — is untouched.Why it's needed
This ladder is maintained through its comments: the retuning instructions for the sized overflow tests live in them, and #9715's own review found five separate comments still asserting the pre-#9715 order, two of which had been stale since earlier rank additions. A number that means "shed last" in one paragraph and "spent first" in another is the next instance of exactly that, already sitting in the tree.
The item was raised by the review on #9715 and deferred there as non-blocking; #9715 merged before this commit was pushed, so it is carried here rather than lost.
Reviewer Test Plan
How to verify
Comments only — the diff contains no executable line.
git show --unified=0 -- packages/cli/src/commands/review/on this commit shows every changed line beginning with//or a JSDoc*.npx vitest run packages/cli/src/commands/review— 99 files, 4,650 passing on this base. That count is this directory only, which is the whole of what a comments-only change can reach; a full-repo run reports ~23,300 and is the same signal at a wider scope.To confirm the collision is gone:
grep -rn "rank 3\|keep: 3\|keep ?? 3" packages/cli/src/commands/review/*.ts— every hit now namestrimorkeep, or is thekeep ?? 3expression itself.Evidence (Before & After)
N/A — comments only, no user-visible or runtime surface.
Tested on
Environment (optional)
Unit tests only. Note for the reviewer:
tsc --noEmit -p packages/clireports pre-existing errors inpackages/cli/src/acp-integration/**on this base — verified identical on pristineorigin/main, and caused by stale build artifacts resolving@qwen-code/acp-bridge/bridgeTypesto an old.d.ts(the symbols do exist inpackages/acp-bridge/src/bridgeTypes.ts). Unrelated to this change, which cannot introduce a type error.Risk & Scope
keepa named constant) would remove the collision at its root rather than annotating it; that is a larger change than the deferred item asked for.Linked Issues
Follow-up to #9715. Refs #9278.
中文说明
本 PR 做了什么
凡是注释里写「rank 3」的地方都标明它属于哪条轴，因为
compose-review.ts里现在有两套不同的排序都用这个数字，且含义相反。trim: 3是正文预算阶梯最后丢的一档——这是 #9715 落地的。keep: 3是兜底截断那条轴上的默认值，而它是那次截断最先花掉的东西。两者原本都写成裸的「rank 3」，于是 cannot-tell 块的// Deliberately untagged (rank 3, spent first by the last-resort cut)与其上三百行"rank 3 最后才丢"的注释直接对撞。两句都为真，矛盾只源于共用了一个数字。现在两条轴上出现的这个数字，每一处都标明所属。cannot-tell 块直接把这次撞名写出来，而不只是给自己加限定。已经自行消歧的那处
keep注释（"Notrimrank rides here"）未动。为什么需要
这个阶梯是靠注释维护的：定容溢出测试的重新调参说明就写在注释里，而 #9715 自身的评审发现了五处仍在断言 #9715 之前顺序的注释，其中两处从更早的档位新增起就已过时。一个数字在一段里意为"最后才丢"、在另一段里意为"最先花掉"，就是同一问题的下一个实例，且已经躺在代码树里。
该条由 #9715 的评审提出并在那里作为非阻断项延后；#9715 在本提交推送前就已合入，所以改由本 PR 承载，而不是让它丢失。
复核方式
纯注释——diff 中没有任何可执行行。本提交上执行
git show --unified=0 -- packages/cli/src/commands/review/可见每一处改动行都以//或 JSDoc 的*开头。npx vitest run packages/cli/src/commands/review——本基线上 99 个文件、4,650 条通过。该计数仅限本目录（纯注释改动能触及的全部范围）；全仓运行约 2.33 万条，是同一个信号的更大口径。确认撞名已消除：
grep -rn "rank 3\|keep: 3\|keep ?? 3" packages/cli/src/commands/review/*.ts——每一处命中要么点明trim或keep，要么就是keep ?? 3表达式本身。证据（前后对比）
N/A——纯注释，无用户可见面或运行时表面。
环境说明
仅单元测试。提请评审注意：本基线上
tsc --noEmit -p packages/cli会报packages/cli/src/acp-integration/**的既有错误——已在纯净的origin/main上验证完全相同，成因是陈旧构建产物让@qwen-code/acp-bridge/bridgeTypes解析到了旧的.d.ts（这些符号在packages/acp-bridge/src/bridgeTypes.ts中确实存在）。与本改动无关，纯注释改动也不可能引入类型错误。风险与范围
keep一个具名常量）才是从根上消除撞名，但那超出了这条延后项的请求范围。关联 issue
#9715 的跟进。Refs #9278。