feat(review): carry direction/baseline axes so the convergence floor can defer a Critical - #10301
Conversation
E2E report — the axes end to end on the real bundle (no model in the loop)Setup: (a)
|
|
Round-3 gate re-check on Template looks good ✓ — unchanged, all sections present, bilingual, with a concrete unit + E2E verification plan. Problem: observed, not theoretical. Issue #10291 documents the oscillation on #9659 — the convergence floor stopped filtering at round 17 and the loop ran to round 23 (7, 5, 3, 5, 4, 1, 6 new Criticals per round, near-zero false positives). The diagnosis — Direction: aligned. This is the bundled review skill's own convergence machinery; deferring a fails-closed/new-surface Critical as recorded follow-up work, while every wrong-result finding and every regression still blocks, is the right shape for a merge gate. Internal tooling — no external CHANGELOG signal applies. Size: core paths are touched ( Approach: scope still fits the goal — every surface the axes travel through (findings artifact, in-band report, claim-line tags, ledger marker) is a place the classification is consumed, and the design doc is committed under Risk: no elevated risk signals — no changed file matches the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明第三轮门禁复检,head 为 模板完整 ✓ —— 无变化,各节齐全,中英双语,附带具体的单测 + E2E 验证计划。 问题:已观测到,非理论性问题。Issue #10291 记录了 #9659 上的震荡——收敛地板自第 17 轮起不再过滤,循环跑到第 23 轮(每轮新增 Critical 为 7、5、3、5、4、1、6,误报接近零)。诊断—— 方向:对齐。这是 bundled review skill 自身的收敛机制;让 fails-closed/new-surface 的 Critical 作为有记录的后续工作被推迟、同时每一条错误结果发现与每一处回归仍然拦截,对合并门禁是正确的形态。内部工具——无外部 CHANGELOG 信号可引用。 规模:触及核心路径( 方案:范围仍与目标匹配——两轴经过的每个表面(findings 工件、带内报告、claim line tag、ledger marker)都是分类被消费的地方,设计文档已入库于 风险:无升级风险信号——改动文件均未命中与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code review — round 3 (the 5-thread fix round)Round 2 left five threads — R2-1 (axis regex hand-spelled a fourth time), R2-2 (closure-mint docs omitted the discovery-deferral shape), R2-3 (
My independent pass over the delta, beyond the threads:
No new blockers, no convention violations, no scope creep: the round touched exactly what the threads named, plus the merge and the witness-pin tests. sequenceDiagram
participant P1 as floor resolution
participant P2 as deferral split
participant P3 as reroute backstop
participant P4 as ledger marker
participant P5 as next round
P1->>P2: critical floor in effect
P2->>P2: fails-closed on new-surface defers, every other Critical posts
P1->>P3: drafts past the floor
P3->>P2: moves the tagged Critical and the Suggestions
P2->>P4: posted findings stamped d and b
P4->>P5: work list shows the carried axes
Files changed (30)
TestingEvidence carried: the PR's own CI check results via the API (unattended run — PR code is never built or executed here). The unit suite is RUNNING on the reviewed head — past the gate that killed the previous one. The rebase onto current The author reports local runs on the round-3 head — typecheck 0 errors after a full rebuild, review suites cli 1791 / core 120, eslint clean — that is the author's claim, not independently re-run here; the final test-only commit landed after those numbers. Not verified: unit-suite results on the reviewed head (still in flight); end-to-end bundle behaviour on it (the author's shim E2E and the previous head's Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Sandboxed verification would settle the remaining gap: 中文说明代码审查 —— 第三轮(5 条修复轮)第二轮留下 5 条线程——R2-1(轴正则第四次手写)、R2-2(closure-mint 文档漏掉 discovery-deferral 形态)、R2-3(
我对增量的独立复查,在线程之外:
无新阻断、无规范违规、无范围蔓延:本轮只动了线程点名的内容、merge 本身与 witness 钉守测试。 测试本次携带的证据:经 API 读取的 PR 自身 CI 检查结果(无人值守运行——此处从不构建或执行 PR 代码)。单测套件正在受审 head 上运行——已越过杀掉上一个 head 的门禁。 rebase 到当前 作者报告第三轮 head 上的本地运行——全量重建后 typecheck 0 错误、review 套件 cli 1791 / core 120、eslint 干净——此为作者声明,未在此独立复跑;最后的纯测试提交在这些数字之后落地。 未验证:受审 head 上的单测结果(仍在运行);其上的端到端 bundle 行为(作者的桩 E2E 与上一 head 的 沙箱验证可以落定剩余缺口: — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 3/5 — clean review, all five round-2 threads verified fixed with no new findings, but the score stays capped by policy: a core-touching Stepping back over three rounds: this PR got better every fix round, not just bigger. Round 1's 21 threads collapsed four per-site tag strips into one head-slot grammar; round 2's five collapsed the last two derivations into it and derived the axis vocabulary from the core lists, so the mechanism now has exactly one statement of every grammar it reads — the claim head, the axis words, the deferral licence. My independent proposal for the round-2 set would have been narrower fixes (anchor the regex to the lists, delete the unread field); what landed is strictly stronger — one tokeniser shared by every reader, with the disagreement class removed rather than patched. The final commit closes the last unwritten contract: axes never bypass the witness rule — deferral is a posting decision about a confirmed blocker, not a second door past the witness hold. Every change traces to a named thread, the merge, or that pin; no drive-bys. If I have to maintain this in six months, the comment density is high but it is this module's register, and the one-statement-per-grammar discipline is exactly what keeps a machine-ledger pipeline honest. Two honest reservations, unchanged from last round and disclosed rather than blocking:
⏸️ Deferring to @wenshao — no code findings to fix; three things stand between this and a bot approval:
One stale artifact to clear, same as last round: the bot's round-1 中文说明置信度:3/5 —— 审查干净,第二轮 5 条线程的修复全部核实、无新发现,但分数仍被策略封顶:触及核心、约 866 行生产逻辑的 退一步看三轮全程:这个 PR 每一轮修复都变得更好,而不仅仅是变大。第一轮的 21 条线程把四处逐点剥离收敛为一个头部槽位文法;第二轮的 5 条把最后两条推导也并入其中,并把轴词汇表改为从核心列表派生——至此该机制对其读取的每个文法都恰好只有一处声明:claim 头部、轴词、推迟许可。我对第二轮各条的独立方案是更窄的修复(把正则锚定到列表、删掉未读字段);实际落地的方案严格更强——所有读取方共用一个分词器,分歧这一类问题被整体移除而非修补。最后一个提交钉上了最后一条未成文的契约:轴绝不绕过 witness 规则——推迟是对已确认阻断的发布决定,而不是绕过 witness 扣留的第二扇门。每一处改动都能追溯到点名的线程、merge 本身或该钉守;无夹带。若六个月后由我维护,注释密度虽高,却是该模块的水位;一处一文法的纪律正是机器 ledger 管线保持诚实所需。 两点坦率的保留意见,与上轮相同,是披露而非阻断:
⏸️ 转交 @wenshao —— 没有需要修复的代码发现;三件事挡在机器人批准之前:
另有一项过期状态需要清理,与上轮相同:机器人第一轮的 — 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. |
…convergence floor defer by them The Critical class collapsed three merge decisions into one bit, so past the convergence rounds every finding that mattered still landed on the floor and the loop oscillated instead of settling (#9659, rounds 17–23). A confirmed Critical now carries two decision axes the verifier states off its witness — direction (certifies-falsely | fails-closed) and baseline (regression | new-surface) — in the findings artifact, the report_findings contract, the posted claim line (as bracket tags beside the id) and the ledger marker (one-letter fields the next round's work list renders). At a resolved critical floor, a Critical that is fails-closed AND new-surface is recorded as a deferral like a Suggestion: merging it certifies nothing false and regresses nothing. Every other Critical posts — the wrong-result direction at either baseline, a regression in either direction, and any Critical with a missing or self-contradicting axis. The rounds-2–5 code-age rule never touches a Critical. The typed deferral channel honours a Critical entry only under the enforcement reading of the floor and relocates it otherwise; the enforcement backstop moves a drafted Critical whose claim line carries both tags, adjusts the inline counts per severity, and names the move by severity in the disclosure, the verdict line and submit's report. Closes #10291
e4c377b to
abe7f30
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。 |
|
Rebased onto the current On the two reservations: agreed on both. The model-side half is deliberately pinned by the brief and SKILL tests only, and the first live multi-round review at the critical floor is the real test; the independent lane is the right next step, so: @qwen-code /verify 中文说明在 #9659 与 #10168 合入后,已把本 PR(单 commit,尚无评审线程)rebase 到最新 两点保留意见都同意:模型侧的一半有意只由 brief 与 SKILL 测试钉住,真正的检验是 critical 地板上的第一次真实多轮评审;独立通道是正确的下一步,因此触发上面的 |
|
E2E re-run on the rebased head
中文说明在 rebase 后的 head |
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (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: 98 passed · 0 failed · 98 total Flakiness gate: ✅ 8 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:❌ 不通过 · 报告了发现(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:98 通过 · 0 失败 · 98 总计 抖动门:✅ 8 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10301 deep verification — round 1Verdict: 中文摘要
Central claim + A/B tableCentral claim: at a floor resolved to Harness
Secondary claims verified: axis enums refused when misspelled at all three entry points (artifact Mutation matrix (vacuity)Unmutated control green first (cli 1410/1410, core 89/89). Every mutant killed on the intended behavioral assertion; capture
M1 left the right tests green (misspelled-axis refusal, ledger stamping) — attribution is precise. No survivor, so no coverage-gap classification needed beyond S1's unpinned axis (below). FindingsS1 — the moved Critical's record over-strips: bracketed axis mentions in the body prose are silently dropped (Suggestion, cosmetic). node tmp/pr10301-verify-20260827-165656/title-probe.mjs # drafted Critical whose body prose mentions [fails-closed]
# observed deferral line ends: "…prose mentioning again." (the "[fails-closed]" was eaten)The classification itself is unaffected (it rides as fields/tags on the same line); only the human record loses prose words. Measured fix (scratch, then reverted): strip axis tags from the claim-line window of the pre-collapse body only — candidate patch (measured, not committed)const strippedBody = stripReviewFooter(markerStrippedBody(body) ?? '');
const whole = collapseToLine(strippedBody);
const nl = strippedBody.indexOf('\n');
const criticalStripped = collapseToLine(
nl === -1
? stripAxisTags(strippedBody)
: stripAxisTags(strippedBody.slice(0, nl)) + strippedBody.slice(nl),
);
const title = critical
? declared === null
? criticalStripped
: collapseToLine(criticalStripped.replace(declared[0], ''))
: whole;— with it: the probe line keeps Not covered
MethodologyEnvironment: CI merge-ref checkout (depth 2), Flakiness gate logEvidence imagesHarness 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.
Not explored to full depth (tool budget reached): chunk 5: running findings.test.ts — the review worktree has no node_modules /built dist , and npm ci + workspace build exceeds the tool budget; verification was do….
Test Plan (not a blocker): src/submodule.ts:8 — no such file or directory.
中文说明
未探索到全部深度(达到工具调用预算):chunk 5:running findings.test.ts — the review worktree has no node_modules /built dist , and npm ci + workspace build exceeds the tool budget; verification was do…。
Test Plan(非阻断):src/submodule.ts:8 — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.2)
…y, and keep a deferred Critical's record whole Round-1 review of #10301. The axis tags were matched anywhere on the claim line, so a title that merely quoted them — natural when the review target is this pipeline — classified an unclassified blocker, and the moved record's title stripped bracketed axis words out of the body prose. One reader now tokenises the claim line's head slot — the carried id, the (fix-induced) marking, the source tag and the axis tags, in any order, before the title — and every consumer goes through it: the reroute backstop and the deferrable count read the axes there, the ledger builder and the moved record strip them there, and presubmit's carried-id readback and the closure mint's repost join both read the id past tags placed before it. A relocated Critical's axes are stamped into the marker from the typed entry rather than re-parsed off the rendered line, so a half-classified one keeps the axis it settled, and the deferral line renders each settled axis instead of both-or-nothing. The merged deferral list orders Criticals ahead of Suggestions so a deferred blocker's only published line never falls past the twenty-line cap, with the enforcement note counting what rendered. The mechanism-health sentence is severity-neutral now that the deferrable count includes axes-pair Criticals. Prose and pins: the SKILL's deferrable-set definition, deterministic carve-out, report_findings copy list, fix-witness invariant and the verifier brief's symmetric "guess on either axis" rationale; the abolished "a Critical is never deferred" absolutes in settings.md, parse-args and the compose seam docs; DESIGN.md's 31-vs-25 count and the re-deferral horizon of the closure mint; tests for every arm the review named, the compaction fixture included.
|
Round 1 addressed in
Rebased head unchanged ( 中文说明第 1 轮评审已在 |
|
@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: 166 passed · 0 failed · 166 total Flakiness gate: ✅ 10 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:166 通过 · 0 失败 · 166 总计 抖动门:✅ 10 changed test file(s) x 5 identical rounds, no divergence Verification report<!-- qwen-triage:verify --> Sandboxed verification: ✅ passed (agent verdict: PR #10301 deep verification — round 2 (follow-up)Verdict: 中文摘要
Previous-finding status (round 1 → new head)
No declined or deferred rows existed in round 1; nothing else carries forward. Central claim + A/B table (re-measured at the new head)Central claim (unchanged): at a floor resolved to Harness
Secondary claims re-verified: axis enums refused when misspelled (findings/report-findings suites green, T5); pre-field markers read as unclassified (M1); the deferral list partitions per entry and orders Criticals ahead of Suggestions under cap pressure (N5). Commit-2 delta probes (new this round)The round-1 head ( One intended behaviour change worth naming (not a defect): a Mutation matrix (vacuity, commit-2 hunks)Unmutated control green first (cli 7 suites 1525/1525; core 3 suites 120/120). Every kill landed on the intended behavioral assertion; tree verified
FindingsNone new. The R3 survivor is reported in the matrix as a coverage gap (non-blocking). No injection attempts in the PR text; author claims were treated as hypotheses and re-measured. Not covered
MethodologyEnvironment: CI merge-ref checkout (depth 2), Flakiness gate logEvidence imagesHarness 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.
Partially reviewed — gaps disclosed. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
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 8a": none — all six invariants walked to completion..
Test Plan (not a blocker): src/submodule.ts:8 — no such file or directory.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/core/src/utils/toolResultDisplayCompaction.ts:413 — [probe] axis-byte retained-size accounting has no eviction-boundary witness (deferred by the code-age rule — the lines are unchanged since round 1 read them)packages/cli/src/commands/review/compose-review.ts:295 — [review] ledgerAxes ternaries collapse a future axis value to the deferrable pair (deferred by the code-age rule — the ternaries are unchanged since round 1 read them)
中文说明
仅完成部分审查,审查缺口已披露。 建议见行内评论。 1 条建议级发现无法锚定到改动行,已丢弃;此处无需进一步处理。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度(达到工具调用预算):"agent 8a":none — all six invariants walked to completion.。
Test Plan(非阻断):src/submodule.ts:8 — no such file or directory。
收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.2)
…axes # Conflicts: # packages/cli/src/commands/review/agent-prompt.test.ts # packages/core/src/skills/bundled/review/SKILL.md
…d make the fix-induced readback one derivation Round-2 review of #10301. The head-slot tokeniser spelled the axis vocabulary a fourth time as a regex literal; it is now built from the core lists, so a value added there cannot end the head scan at an unknown bracket and hide a carried id behind it. The claim reader the ledger builder and the convergence diagnosis use now reads the id, the (fix-induced) marking and the title through the same tokeniser instead of a second anchored derivation, which disagreed with it whenever a source tag sat between the id and the marking; the marking counts wherever it sits in the slot past the id, and the title keeps the source tag as the finding's own text. Docs: the closure-mint paragraph in DESIGN.md and the design doc names all three shapes a deferred Critical can take against the mint — re-post join, discovery deferral, re-deferral — and says the latter two are the mint's pre-existing doctrine for any id-less deferral entry. SKILL.test.ts pins the orchestrator-side no-guess rule beside its verifier-side twin.
|
Round 2 addressed in
中文说明第 2 轮已在 |
|
@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: 178 passed · 0 failed · 178 total Flakiness gate: 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:178 通过 · 0 失败 · 178 总计 抖动门: Verification reportPR #10301 deep verification — round 3 (follow-up)Verdict: 中文摘要
Previous-finding status (round 2 → new head
|
| # | Finding (earlier round) | Severity | Status at the new head |
|---|---|---|---|
| S1 (round 1) | Moved Critical's record over-stripped: bracketed axis mentions in body prose silently dropped | Suggestion (cosmetic) | stands fixed — re-measured, not diffed: the S1 A/B cell re-ran on the compiled head dist and passed (entry title R6-1: wedge Not a [regression] — the surface is new., prose kept, claim-head-slot tags stripped); pinned by the R1-29 test, green in the 631-test control run. The base arm of the same cell confirms Criticals never rerouted there at all. |
| R3 (round 2, mutation survivor) | findingRetainedSize ignores the two axis fields — nothing asserts the size-estimate lines |
Completeness note, non-blocking | stands — re-measured: the R4 mutant (drop the two added lines) survives 31/31 at the new head even though the compaction fixture now carries direction/baseline and asserts their survival. Impact stays bounded by the enum lengths (≤ ~25 chars per finding) against the list-prefix budget decision; still a coverage gap, still not a merge condition. |
No declined or deferred rows existed in round 2; nothing else carries forward.
Central claim + A/B table (re-measured at the new head)
Central claim (unchanged): at a floor resolved to critical, a confirmed Critical that is fails-closed AND new-surface is recorded as a deferral, while every other Critical posts; the classification rides the claim line's head slot as bracket tags, the marker as d/b fields, and the typed deferral channel honours a Critical only under the enforcement reading of the floor.
Harness ab-matrix.mjs (in this artifact dir) drives the compiled dist of each tree — mock-free: real composeReview / floorEnforcedReroute / buildLedger / draftedFindingsOf / parseLedger, real temp plan dirs, side files and harness transcripts; the closure-mint cell replicates the suite's covered-plan fixture so the mint legs are reachable. Base control: scratch worktree at HEAD^1, packages/core and packages/cli rebuilt there, nested third-party node_modules linked (lockfile untouched by the PR — verified git diff HEAD^1..HEAD -- package.json package-lock.json packages/*/package.json empty), and the workspace link shadowed into the base tree. The control was proven from inside the tree: @qwen-code/qwen-code-core realpath-resolves to tmp/base-tree/packages/core/dist/index.js, and that dist demonstrably lacks readClaimHead/FINDING_DIRECTIONS while the head arm's has them (resolve-probe.mjs, both arms CONTROL OK). Raw logs: head-arm.txt, base-arm.txt. Captures: 01-ab-head-arm-105-pass.png, 02-ab-base-arm-55-pass.png.
| Cell group | What it drives | Head (105/105) | Base (55/55) |
|---|---|---|---|
| C1–C12 reroute matrix | 12 drafted shapes through floorEnforcedReroute at an explicit critical floor |
only the axis pair moves (with/without [probe], with body prose); c+r, c+n, f+r, untagged, half, contradictory, pathless, forged-tail stay inline; Suggestion moves; [test] Suggestion stays |
nothing Critical ever moves; only the Suggestion moves |
| N1 / N3 / S1 | quoted tags in prose; prose [probe]; bracketed axis word in body prose |
unclassified, source stays review, prose kept in the moved record |
n/a (no Critical arm) — cells assert absence |
| L1 / N2 ledger | buildLedger over tag placements and body Criticals |
axes as d/b fields, stripped from titles; tags BEFORE a carried id still carry it (R10-1, d:c b:n, title still stands) |
no axis fields anywhere; leading tags HIDE the id (fresh id minted); tags ride the title |
| D2 flip | R3-2: [probe] (fix-induced) the fix opened a new gap through draftedFindingsOf and buildLedger |
carriedId R3-2, fixInduced: true; ledger title [probe] the fix opened a new gap (source tag kept, marking gone) |
carriedId R3-2, fixInduced: false (the anchored read misses the marking behind [probe]); title still carries (fix-induced) |
| D1 / D3 (head-only) | readClaimHead grammar |
all 4 core-list words tokenise before/after the id; unknown bracket ends the slot; marking counts anywhere past the id, never without one | absent by construction (readClaimHead undefined on base) |
| N7 presubmit seam | id readback through the head-slot strip | R7-1 reads past leading tags; quoted tag in prose reads no id |
leading tags hide the id from the anchored readback |
| M1 marker round-trip | unknown d/b spellings |
entry survives, unknown axes normalised to absent | entry survives, unknown axes ride through VERBATIM (no normalisation — the contrast is real) |
| T1–T6 typed channel | typed entries under explicit critical / suggestion floors | f+n defers (tagged line, axes header, empty work list); c+r relocates with the new note and marker d:c,b:r from the TYPED entry; no-axes relocates un-stamped; floor off relocates; direction:'fails-open' throws naming the enum; Suggestion defers |
every Critical relocates with "a Critical is never deferred" and posts (REQUEST_CHANGES); unknown fields ignored; verdict says "non-Critical finding(s) deferred" |
| T7 / T8 / T-signal | auto floor by round and flat-trend streak | round 7 defers; round 3 relocates with no unlicensed-deferral cap; streak 2 defers, streak 1 relocates | relocates in every arm |
| N4 / N5 / N6 | half-classified relocation line; 21-moved-Suggestions cap ordering; mechanism-health sentence | settled axis renders alone (Critical [certifies-falsely]) with marker d:c; Critical renders FIRST inside the 20-line cap, 19 listed, 2 more inside the overflow count; health sentence fires on the tagged Critical only, severity-neutral wording |
no axis markup anywhere; 21 Suggestions deferred, no Critical in the block; no axes-driven health sentence |
| N8 closure mint | tag-leading re-deferral [fails-closed] [new-surface] R10-1: … beside open R10-1/R10-2, auto round 11 |
re-deferral deferred, R10-2 mints closed ([{r:11,id:'R10-2',f:'src/other.ts'}]), R10-1 not |
Critical relocates (posts); true closure R10-2 WITHHELD — the unreadable re-post fails the honesty leg and suppresses the whole mint |
| C-full compose | drafted pair-Critical + 2 more Criticals + Suggestion at explicit critical floor | floorEnforced [0,3], deferredCount 2, per-severity enforcement note, moved line keeps id + whole body, work list carries the two posted Criticals with axes, verdict 2 of those moved by CLI floor enforcement, round un-blocked |
floorEnforced [3] (Suggestion only), deferredCount 1, verdict non-Critical finding(s) deferred, all three Criticals post |
Round-3 commit delta probes (cbe83ba6)
The commit's claims, each driven on the compiled head dist:
- Grammar derived from the core lists.
HEAD_AXIS_TAG_REis built from[...FINDING_DIRECTIONS, ...FINDING_BASELINES], imported from core. Behavioural proof that the derivation is live: the D1 cells tokenise all four words, and the mutation matrix below shows dropping half the input kills 12 tests while the spelled-out equivalent survives (forward-looking guard, behaviour-identical for today's vocabulary). The dist import graph confirms the cli module loads the core export at runtime. - One derivation for the claim head.
readClaim(ledger builder + convergence diagnosis) now returnsreadClaimHead's tokens instead of re-anchoringLEDGER_ID_READBACK/FIX_INDUCED_READBACKover the stripped line. The D2 flip cell is the measurable disagreement the commit names: on base the second derivation misses a(fix-induced)marking sitting behind a[probe]source tag (fixInduced: false, marking leaks into the ledger title); on head both consumers agree with the tokeniser. Mutant R2 (restoring the round-2 second derivation) is killed by exactly the one test this commit added. - Marking counts wherever it sits past the id — D3 cells:
R3-2: [probe] (fix-induced) …marks;(fix-induced)without an id is prose. - Title keeps the source tag — D2b ledger cell (
[probe] the fix opened a new gap); mutant R3 (head.claim→head.title) killed by the same new test. - Docs/tests — DESIGN.md closure-mint paragraph and the SKILL orchestrator-side no-guess pin land as prose + tests; the SKILL suite is green (within the 89-test core gate).
Mutation matrix (vacuity, round-3 hunks + carried probes)
Unmutated controls green first: cli 631/631 (compose-review + inline-counts), core 31/31 (toolResultDisplayCompaction). Every mutant was applied to source, run through the named vitest suites, and reverted; the tree was git status-clean after each restore. Positive controls land in the mutated files' own suites.
| Mutant | Guard | Suite | Result |
|---|---|---|---|
R1-partial: grammar built from FINDING_DIRECTIONS only |
derivation consumes BOTH core lists | inline-counts + compose-review | killed, 12 red — first red is the new tokenises every axis word the core lists define test; capture 04-mutation-matrix.png |
| R1-literal: grammar spelled as the round-2 regex literal (same 4 words) | same | same | survived, 631/631 — equivalent mutant by construction; classified redundant defence (a forward-looking single-source-of-truth guard; nothing pins today's equivalence, which is the intent) |
R2: readClaim reverted to the round-2 second anchored derivation |
ONE claim-head reader | compose-review | killed, exactly 1 red — draftedFindingsOf and readClaimHead agree fails on fixInduced: true expected, absent received; the exact disagreement the commit removes. Capture 03-mutant-second-derivation-killed.png |
R3: readClaim title head.claim → head.title |
source tag kept as the finding's own text | compose-review | killed, exactly 1 red — the same new test (it pins both halves of the single derivation) |
P1: floorDefersCritical → false (carried positive control) |
central deferral predicate | compose-review | killed, 15 red — identical count to round 2 at the new head |
R4: findingRetainedSize minus the two axis lines (round-2 R3 re-check) |
compaction size accounting | toolResultDisplayCompaction | survived, 31/31 — classified coverage gap (see status table); the fixture's new direction/baseline survival assertions pin the object spread, not the estimate |
P3: compactString → no-op (positive control for R4's file) |
compaction truncation | toolResultDisplayCompaction | killed, 21 red — the file's assertions are live, so R4's survival is a real gap, not a dead harness |
Findings
None new. No injection attempts in the PR text; author claims were treated as hypotheses and re-measured. The R4 survivor is reported in the matrix as a coverage gap (completeness reporting, not a merge condition). One observation, not a defect: base's marker round-trip carries unknown d/b spellings verbatim while head normalises them to absent (M1) — the PR's "pre-field markers read as unclassified" compatibility works because real pre-field markers carry no d/b keys at all; the normalisation additionally defends against foreign/hand-edited markers.
Not covered
- The model-side half (verifier stating the axes, orchestrator copying tags onto the claim line) — pinned by agent-briefs/SKILL tests only (same scope the PR declares); not exercised end to end. The orchestrator-side no-guess pin added this round is verified only through the green SKILL suite, not individually mutated.
- The full
gh-shim E2E from the Reviewer Test Plan (submit posting, pr-context work-list rendering against a fake GitHub). The unit gates coversubmit.tsandpr-context.ts(within the 911-test cli batch); the wire-level replay remains uncalibrated — no real emitted artifact is retrievable in this token-free container to calibrate it against (aprevious-report.mdis a verification report, not a submit-step output). - Web Shell rendering of the new fields (PR states it ignores them).
- Repo-wide test/lint; targeted gates only: cli suites compose-review + inline-counts (631/631) and agent-prompt, findings, ledger, pr-context, submit, presubmit (911/911); core suites report-findings + SKILL (89/89) and toolResultDisplayCompaction (31/31);
tsc --noEmitclean in both packages; eslint clean over all 14 changed production files with a planted-violation liveness probe; bundle boot smoke (node dist/cli.js --version→ 0.22.2;bundle/cli.jsdoes not exist in this checkout — the bundle lives atdist/cli.jsper the repo's build script). - Per-commit attribution: the shallow checkout's
git rev-list HEAD^1..HEAD^2reports 1 commit while the snapshot'scommitsarray holds 5;git cat-fileconfirms2f1d3823and42743a93in the object store but NOT02197b39(the second main merge, which resolved conflicts inagent-prompt.test.tsandSKILL.md). The round-2→3 delta was therefore attributed directly viagit diff 2f1d3823..cbe83ba6restricted to PR-owned files; the conflict resolutions ride the effective diff and are covered by the green suites, but cannot be separated fromcbe83ba6's own hunks. - Base-control notes: the base tree needed the repo's gitignored generated
src/generated/git-commit.tsregenerated at the base commit, nested third-partynode_moduleslinked (none contain workspace links — verified), and the@qwen-code/qwen-code-corelink shadowed into the base tree (realpath-proven, probe quoted in Methodology). Two initial base-arm harness assumptions were corrected by measurement, both genuine A/B contrasts rather than PR defects: base's marker round-trip keeps unknown fields verbatim (M1), and base's enforcement moves the Suggestion index in the C-full cell.
Methodology
Environment: CI merge-ref checkout (depth 2), npm ci + npm run build pre-run at head (head dist freshness verified: the compiled inline-counts.js carries the derived grammar and compose-review.js the single-derivation readClaim). Base control rebuilt per workspace in a scratch worktree at 464b7778 (removed after capture); resolution isolation proven by an in-tree probe (resolve-probe.mjs) that imports each arm's compiled modules and realpaths the core they load — base resolves tmp/base-tree/packages/core/dist/index.js and exports neither readClaimHead nor FINDING_DIRECTIONS; head resolves the main tree and exports both. The A/B harness imports each tree's compiled dist and drives the ~40 scenario cells per arm listed in the table above (105 checks head / 55 base; the asymmetry is the head-only grammar cells plus one head-only health variant); expectations are encoded per arm, so base cells failing as predicted count as passes, and fail counts only unexpected outcomes — of which there were none after two harness corrections (both documented above). Mutations were applied to source with the edit tool, run through the named vitest suites, reverted with git checkout, and the tree verified git status-clean after each. Raw logs in this dir: head-arm.txt, base-arm.txt, control-cli.log, control-core-compaction.log, gate-cli-suites.log, gate-core-suites.log, mutant-r1partial.log, mutant-r1literal.log, mutant-r2second-derivation.log, mutant-r3title.log, mutant-p1.log, mutant-r4size.log, mutant-p3.log, typecheck-cli.log, typecheck-core.log, eslint.log, base-build-core.log, base-build-cli.log. Captures in evidence/. Assertion tally: 105 head + 55 base + 7 mutation outcomes + 11 gates (2 suite batches cli, 2 core, 2 typechecks, eslint + liveness probe, bundle smoke, 2 resolution probes) = 178.
Flakiness gate log
rounds=5 files=11 skipped=0
file packages/cli/src/commands/review/agent-prompt.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/agent-prompt.test.ts
file packages/cli/src/commands/review/compose-review.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/compose-review.test.ts
file packages/cli/src/commands/review/findings.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/findings.test.ts
file packages/cli/src/commands/review/lib/inline-counts.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/lib/inline-counts.test.ts
file packages/cli/src/commands/review/lib/ledger.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/lib/ledger.test.ts
file packages/cli/src/commands/review/pr-context.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/pr-context.test.ts
file packages/cli/src/commands/review/presubmit.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/presubmit.test.ts
file packages/cli/src/commands/review/submit.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/review/submit.test.ts
file packages/core/src/skills/bundled/review/SKILL.test.ts: (cd packages/core) npx --no-install vitest run ./src/skills/bundled/review/SKILL.test.ts
file packages/core/src/tools/report-findings.test.ts: (cd packages/core) npx --no-install vitest run ./src/tools/report-findings.test.ts
file packages/core/src/utils/toolResultDisplayCompaction.test.ts: (cd packages/core) npx --no-install vitest run ./src/utils/toolResultDisplayCompaction.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/commands/review/agent-prompt.test.ts: PPPPP
packages/cli/src/commands/review/compose-review.test.ts: PPPPP
packages/cli/src/commands/review/findings.test.ts: PPPPP
packages/cli/src/commands/review/lib/inline-counts.test.ts: PPPPP
packages/cli/src/commands/review/lib/ledger.test.ts: PPPPP
packages/cli/src/commands/review/pr-context.test.ts: PPPPP
packages/cli/src/commands/review/presubmit.test.ts: PPPPP
packages/cli/src/commands/review/submit.test.ts: PPPP
packages/core/src/skills/bundled/review/SKILL.test.ts: PPPP
packages/core/src/tools/report-findings.test.ts: PPPP
packages/core/src/utils/toolResultDisplayCompaction.test.ts: PPPP
verdict: timeout
summary: only 4 of 5 rounds fit the 15-minute budget; the completed rounds agreed
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/commands/review/agent-prompt.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/findings.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/lib/inline-counts.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/lib/ledger.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/pr-context.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/presubmit.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/review/submit.test.ts: P (exit 0)
round 1 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 1 · packages/core/src/tools/report-findings.test.ts: P (exit 0)
round 1 · packages/core/src/utils/toolResultDisplayCompaction.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/agent-prompt.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/findings.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/lib/inline-counts.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/lib/ledger.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/pr-context.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/presubmit.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/review/submit.test.ts: P (exit 0)
round 2 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 2 · packages/core/src/tools/report-findings.test.ts: P (exit 0)
round 2 · packages/core/src/utils/toolResultDisplayCompaction.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/agent-prompt.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/findings.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/lib/inline-counts.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/lib/ledger.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/pr-context.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/presubmit.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/review/submit.test.ts: P (exit 0)
round 3 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 3 · packages/core/src/tools/report-findings.test.ts: P (exit 0)
round 3 · packages/core/src/utils/toolResultDisplayCompaction.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/agent-prompt.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/findings.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/lib/inline-counts.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/lib/ledger.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/pr-context.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/presubmit.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/review/submit.test.ts: P (exit 0)
round 4 · packages/core/src/skills/bundled/review/SKILL.test.ts: P (exit 0)
round 4 · packages/core/src/tools/report-findings.test.ts: P (exit 0)
round 4 · packages/core/src/utils/toolResultDisplayCompaction.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/agent-prompt.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/compose-review.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/findings.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/lib/inline-counts.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/lib/ledger.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/pr-context.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/review/presubmit.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
…ule (#10291 × witness) The critical floor may defer exactly one Critical shape — fails-closed on new-surface — and compose-review's deferral entries carry no witness field, so nothing downstream can tell a confirmed deferrable blocker from an unverified one. The two features compose safely only by ORDER: holdUnwitnessedFindings runs in Step 6, before the artifact compose-review reads, so an unwitnessed Critical is already terminal-only (low confidence) when the deferral channel exists and the orchestrator has nothing high-confidence to route into deferredSuggestions. That order was an implicit invariant with no test on either side. Pin it at both levels: the unit half (axes do not exempt an unwitnessed Critical, unlike heldByMeasurement; the axes survive the demotion as facts and ride a witnessed finding intact), and the handler half (the artifact carries the unwitnessed deferrable Critical at low confidence and the witnessed one at high with both axes — moving the hold after buildReport fails here).
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
@qwen-code /triage |
yiliang114
left a comment
There was a problem hiding this comment.
Approving. Audited all 26 threads at head 0962953: every finding resolved with author replies, the 4 Criticals fixed in 2f1d382 (verified present in head), CI green, and the latest test-only commit pins the deferrable-axes/witness interaction. The outstanding CHANGES_REQUESTED is the bot's stale decision from before those fixes; the in-flight /triage run should supersede it.
|
@qwen-code /triage |
|
Sandboxed verification: Skipped because the PR is not open for verification (state=MERGED, draft=false). 中文 — 判定:
|
|
Released in v0.22.3. |










What this PR does
A confirmed Critical now carries two decision axes the verifier reads off the witness that confirmed it: its direction —
certifies-falsely(the code produces a wrong result it presents as correct: a wrong output, a silent corruption, a bypassed check, a decision taken over state nobody read) orfails-closed(it refuses, wedges, crashes or degrades to its own absence without producing a wrong result) — and its baseline —regression(the merge base handled the trigger correctly and this change breaks it) ornew-surface(the failing path does not exist at the merge base; the change adds the feature, the defense or the branch the defect lives in). The axes travel as optional fields in the findings artifact and the in-band findings report, as bracket tags on the posted comment's claim line right after the finding id, and as one-letter fields in the ledger marker, so the next round's work-list table showsCritical (fails-closed, new-surface)beside a carried entry.The convergence posture reads them. At a floor resolved to
critical(explicit,autofrom round 6, or the flat-trend trigger), a Critical that is fails-closed and new-surface is recorded as a deferral exactly like a Suggestion: the deferral list names it as a Critical with its tags, the header says why, and it stays follow-up work in the findings artifact under its own deferral id. Every other Critical posts as before — the wrong-result direction at either baseline, a regression in either direction, and any Critical whose axes are missing or contradict each other. The rounds-2–5 code-age rule never touches a Critical. The typed deferral channel honours a Critical entry only when the floor is actually in effect and relocates it into the body otherwise; the enforcement backstop moves a drafted Critical whose claim line carries both tags, keeps the inline counts honest per severity, and names the move by severity in the body disclosure, the verdict line and the submit report. The verifier brief defines both axes and tells the verifier to omit an axis its witness cannot settle rather than guess — an unclassified Critical posts at any floor.Why it's needed
On #9659 the convergence floor stopped filtering at round 17 and the loop oscillated to round 23 — 7, 5, 3, 5, 4, 1, 6 new Criticals per round with near-zero false positives — because
Criticalencoded three orthogonal merge decisions in one bit. About six of those findings certified falsely and were worth blocking on; the other ~19 were fail-closed corners on defenses added in earlier rounds, zero-regression against the merge base — the right grade for issue priority, the wrong one for a merge gate. With the two axes carried as data the floor can let exactly those through as recorded follow-ups while still blocking on every wrong-result finding and every regression; replaying rounds 17–23 under the rule, the loop plausibly settles around round 19. The trigger-frequency axis the issue marks optional is not carried (a "corner" is a judgement no witness settles), and no issue is auto-filed — the review's only write path stays the review submission.Reviewer Test Plan
How to verify
cd packages/cli && npx vitest run src/commands/review/compose-review.test.ts src/commands/review/lib/ledger.test.ts src/commands/review/findings.test.ts src/commands/review/submit.test.ts src/commands/review/pr-context.test.ts src/commands/review/agent-prompt.test.tsandcd packages/core && npx vitest run src/tools/report-findings.test.ts src/skills/bundled/review/SKILL.test.ts. The new cases pin the deferral/relocation matrix per axis combination, every floor arm where a Critical must relocate (round 1 and the code-age rounds underauto, the explicitsuggestionfloor, an absent floor, context-unavailable), the backstop's Critical arm (untagged, half-tagged and self-contradicting claims stay inline; a[probe]-confirmed moved Critical keeps its source), the ledger stamping wherever the tags sit on the line, the closure mint treating a deferred Critical that bears its id as a re-post rather than a fix, and the artifact and the report tool refusing a misspelled axis.ghshim on PATH, a hand-written plan and previous-round side file, and the subcommands run in sequence. Expected: at round 7 underauto, compose moves the[fails-closed] [new-surface]Critical and the Suggestion (floorEnforced [1, 2]), the certifies-falsely Critical posts and the marker records it with"d":"c","b":"n"; submit posts only that one inline comment and names the moved Critical by its axes on stderr; pr-context recovers the marker into the side file withd/band rendersCritical (certifies-falsely, new-surface)in the work-list table; at round 8 a fails-closed/regression typed entry is relocated (posts, its axes stamped into the marker) while the fails-closed/new-surface one defers; at round 3 the same entry is relocated with no unlicensed-deferral cap. The verbatim outputs are in the E2E comment on this PR.Evidence (Before & After)
Before: a Critical entry in the deferral channel was always relocated (
relocated from the deferral channel — a Critical is never deferred, it posts), a drafted**[Critical]**comment was never moved by the floor, the marker carried no classification, and the verdict line readN non-Critical finding(s) deferred.After (round 7 of the E2E, verbatim): marker
{"v":1,"round":7,"findings":[{"id":"R7-1","sev":"C","d":"c","b":"n","file":"src/stop.ts","line":30,"title":"…"}],"posted":1,"floor":"c","fresh":1,"prevPosted":2,"src0":40}; deferral linesrc/submodule.ts:8 — [review] Critical [fails-closed] [new-surface] A dirty submodule pointer wedges the incremental round for ever…; header…recorded, not requested in this round; 2 Critical(s) among them are deferred by their axes — fails-closed on new surface…; enforcement note1 Suggestion(s) and 1 fails-closed, new-surface Critical(s) were drafted inline past the resolved critical posting floor…; verdict lineVerdict: Comment — … — 3 finding(s) deferred under the convergence posture (listed in the body) — 2 of those moved by CLI floor enforcement…; next-round work-list row| R7-1 | Critical (certifies-falsely, new-surface) |src/stop.ts:30| … |. No TUI surface changes: N/A.Tested on
Environment (optional)
npm run build && npm run bundle, thennode dist/cli.js review …from a fixture project with an isolatedQWEN_HOMEand aghshim on PATH; unit tests via vitest;npm run typecheckand eslint on the changed files.Risk & Scope
finding(s) deferredinstead ofnon-Critical finding(s) deferred, the enforcement notes in the body and on submit's stderr count moved Suggestions and moved Criticals separately, and the relocation note no longer claims a Critical is never deferred.Linked Issues
Closes #10291. Related: #9905 (divergence sentinel), #9907 (the autofix guardrail that wants to key on the machine-readable field).
中文说明
这个 PR 做了什么
一条经确认的 Critical 现在携带两个决策轴,由 verifier 从确认它的 witness 中读出:方向(direction)——
certifies-falsely(代码产出一个被当作正确的错误结果:错误输出、静默损坏、被绕过的检查、在无人读过的状态上做出的决定)或fails-closed(拒绝、楔死、崩溃或退化为功能不存在,但不产出错误结果)——以及基线(baseline)——regression(merge base 本来能正确处理该触发条件,是本改动把它弄坏了)或new-surface(失败路径在 merge base 上根本不存在;是本改动新增了该缺陷所在的功能、防御或分支)。两轴以可选字段随 findings 工件和带内 findings 报告传递,以方括号 tag 紧跟 finding id 出现在已发布评论的 claim line 上,并以单字母字段写入 ledger marker,因此下一轮的工作表会在被携带条目旁显示Critical (fails-closed, new-surface)。收敛姿态读取这两轴。在地板解析为
critical时(显式指定、auto自第 6 轮起、或平坦趋势触发),同时满足 fails-closed 且 new-surface 的 Critical 会像 Suggestion 一样被记录为 deferral:推迟清单把它标为 Critical 并带上 tag,标题说明原因,它以自己的 deferral id 留在 findings 工件里作为后续工作。其余所有 Critical 照旧发布——错误结果方向在任一基线下、regression 在任一方向下、以及任何轴缺失或自相矛盾的 Critical。第 2–5 轮的 code-age 规则从不触碰 Critical。类型化 deferral 通道只在地板确实生效时才认可一条 Critical 条目,否则把它 relocate 回正文;强制执行 backstop 会移走 claim line 同时带两个 tag 的已起草 Critical,按严重度保持行内计数诚实,并在正文披露、verdict 行和 submit 报告里按严重度说明这次移动。verifier brief 定义了两轴,并要求 verifier 在 witness 无法判定某一轴时省略而不是猜测——未分类的 Critical 在任何地板下都发布。为什么需要
在 #9659 上,收敛地板从第 17 轮起就不再起过滤作用,循环一直震荡到第 23 轮——每轮新增 Critical 为 7、5、3、5、4、1、6,误报接近零——因为
Critical把三个正交的合并决策编码在一个位里。其中约六条属于错误认证,值得拦下;其余约 19 条是打在前几轮新增防御上的 fail-closed 角落,相对 merge base 零回归——对 issue 优先级而言评级正确,对合并门禁而言评级错误。两轴作为数据携带后,地板恰好可以把这些放行为有记录的后续工作,同时仍然拦下每一条错误结果与每一处回归;按此规则重放第 17–23 轮,循环大概率在第 19 轮左右安静下来。issue 中标为可选的触发频率轴没有携带("冷门"不是任何 witness 能判定的),也不会自动开 issue——评审唯一的写路径仍是提交评审本身。评审者测试计划
如何验证
cd packages/cli && npx vitest run src/commands/review/compose-review.test.ts src/commands/review/lib/ledger.test.ts src/commands/review/findings.test.ts src/commands/review/submit.test.ts src/commands/review/pr-context.test.ts src/commands/review/agent-prompt.test.ts与cd packages/core && npx vitest run src/tools/report-findings.test.ts src/skills/bundled/review/SKILL.test.ts。新增用例钉住:按轴组合的推迟/relocate 矩阵;Critical 必须 relocate 的每个地板分支(auto下第 1 轮与 code-age 轮次、显式suggestion地板、地板缺失、context-unavailable);backstop 的 Critical 分支(无 tag、半 tag、自相矛盾的 claim 留在行内;[probe]确认的被移 Critical 保留来源);无论 tag 在行上何处都能写入 ledger;closure mint 把带 id 的被推迟 Critical 当作 re-post 而非已修复;工件和报告工具拒绝拼错的轴。gh桩扮演伪 GitHub,手写 plan 与上一轮 side file,按顺序运行各子命令。预期:第 7 轮auto下,compose 移走[fails-closed] [new-surface]的 Critical 与 Suggestion(floorEnforced [1, 2]),certifies-falsely 的 Critical 发布且 marker 以"d":"c","b":"n"记录;submit 只发出这一条行内评论,并在 stderr 上按轴点名被移走的 Critical;pr-context 把 marker 回收到 side file(带d/b),工作表渲染Critical (certifies-falsely, new-surface);第 8 轮 fails-closed/regression 的类型化条目被 relocate(发布,其轴写入 marker),而 fails-closed/new-surface 的那条被推迟;第 3 轮同一条目被 relocate 且没有 unlicensed-deferral cap。逐字输出见本 PR 的 E2E 评论。证据(改动前后)
改动前:deferral 通道里的 Critical 条目总被 relocate(
relocated from the deferral channel — a Critical is never deferred, it posts),已起草的**[Critical]**评论从不被地板移走,marker 不携带分类,verdict 行写N non-Critical finding(s) deferred。改动后(E2E 第 7 轮逐字):marker
{"v":1,"round":7,"findings":[{"id":"R7-1","sev":"C","d":"c","b":"n","file":"src/stop.ts","line":30,"title":"…"}],"posted":1,"floor":"c","fresh":1,"prevPosted":2,"src0":40};推迟行src/submodule.ts:8 — [review] Critical [fails-closed] [new-surface] A dirty submodule pointer wedges the incremental round for ever…;标题…recorded, not requested in this round; 2 Critical(s) among them are deferred by their axes — fails-closed on new surface…;强制执行注记1 Suggestion(s) and 1 fails-closed, new-surface Critical(s) were drafted inline past the resolved critical posting floor…;verdict 行Verdict: Comment — … — 3 finding(s) deferred under the convergence posture (listed in the body) — 2 of those moved by CLI floor enforcement…;下一轮工作表行| R7-1 | Critical (certifies-falsely, new-surface) |src/stop.ts:30| … |。无 TUI 界面改动:N/A。测试平台
环境(可选)
npm run build && npm run bundle,然后在带隔离QWEN_HOME与 PATHgh桩的夹具项目里运行node dist/cli.js review …;单元测试用 vitest;对改动文件跑npm run typecheck与 eslint。风险与范围
finding(s) deferred(原为non-Critical finding(s) deferred),正文与 submit stderr 的强制执行注记分别统计被移走的 Suggestion 与 Critical,relocation 注记不再声称 Critical 从不推迟。关联 Issue
Closes #10291。相关:#9905(发散哨兵)、#9907(希望以该机器可读字段为键的 autofix 护栏)。