Skip to content

fix(cli): word review gap disclosures so they cannot read as contradictions - #8857

Merged
yiliang114 merged 9 commits into
QwenLM:mainfrom
yiliang114:fix/review-gap-disclosure-wording
Aug 12, 2026
Merged

fix(cli): word review gap disclosures so they cannot read as contradictions#8857
yiliang114 merged 9 commits into
QwenLM:mainfrom
yiliang114:fix/review-gap-disclosure-wording

Conversation

@yiliang114

@yiliang114 yiliang114 commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Review-body wording fixes for runs that cannot certify the whole diff. Coverage gaps use the non-directional opener "Partially reviewed — gaps disclosed."; surviving unverified findings use an explicit incomplete-review opener. Agent-derived disclosure subjects are quoted on both the "Not reviewed" and tool-budget surfaces because they are names, not sentences, and their truncation now reuses the existing word-boundary summary helper. The Chinese unopened-agent reason remains natural.

Why it's needed

On #8811 the posted body opened Reviewed. Not reviewed: This PR narrows the daemon-marker check from a truthy tes... — pointed at diff lines it never opened: it made tool calls, but none of them read the diff. — and a reader could not tell whether the PR had been reviewed at all. The opener was accurate (part of the diff was certified; the disclosure named what was not) and the disclosure was accurate (one agent never opened the diff), but the pair read as a self-contradiction, and the subject — a prompt fragment cut mid-word — looked like a claim about the whole PR rather than the name of the one agent that failed. A first cut announced the gaps ("Reviewed, with gaps disclosed below.") and still confused readers: any opener starting with "Reviewed" reads against the "Not reviewed:" clauses below it. This is a presentation defect, not a verdict defect: every cap and certification check worked exactly as designed, and none of them is touched here.

Reviewer Test Plan

How to verify

  • Run cd packages/cli && npx vitest run src/commands/review/compose-review.test.ts src/commands/review/check-coverage.test.ts and confirm all 261 tests pass, including the new regressions pinning the partial opener (English and Chinese halves), the quoted word-boundary-truncated agent label, and the natural-Chinese reason.
  • Confirm the unchanged openers still pin: Reviewed — no blockers. (certifying), Reviewed. (caps without gap disclosures) and the zero-certified ⚠️ warning all keep their exact wording — their tests are untouched and pass.

Evidence (Before & After)

Before (posted body on #8811): Reviewed. Not reviewed: This PR narrows the daemon-marker check from a truthy tes... — pointed at diff lines it never opened: it made tool calls, but none of them read the diff.

After (same verdict inputs): Partially reviewed — gaps disclosed. Not reviewed: "This PR narrows the daemon-marker check from a truthy test…" — pointed at diff lines it never opened: it made tool calls, but none of them read the diff. — and the Chinese fold carries 仅完成部分审查,审查缺口已披露。 in place of 已审查。, with the reason now reading 启动 prompt 为它指定了 diff 中的行,但它从未打开:有工具调用,却没有一次读取 diff.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

Node.js 22; focused compose-review unit tests. The full exact-head repository build was not rerun.

Risk & Scope

  • Main risk or tradeoff: posted-body wording only. Verdicts, caps, dedup and the certification check are untouched — the INTERNAL disclosure subject stays the unquoted label, and every structural check keys on it.
  • Not validated / out of scope: no end-to-end /review run against a live PR; the pre-existing stale-bundle test failure on main (verified against the clean base) is unrelated and left as is.
  • Breaking changes / migration notes: none. Anything matching the literal opener Reviewed. over a body that carries disclosures sees the new sentence; inside this repo the only such consumers are the tests, updated here.

Linked Issues

Follow-up to the reader confusion on #8811.

中文说明

这个 PR 做了什么

修正 /review 在无法证明整个 diff 都被审查时发布的正文措辞。覆盖缺口使用无方向性的开头 "Partially reviewed — gaps disclosed.";仍未验证的 finding 使用明确的“审查未完成”开头。agent 推导出的披露主语在 "Not reviewed" 和工具预算两条展示路径上都会加引号,因为它们是名称而不是句子;截断复用现有的词边界摘要 helper。未打开 diff 的 agent 中文原因保持自然表达。

为什么需要

#8811 上,发布的正文以 Reviewed. Not reviewed: This PR narrows the daemon-marker check from a truthy tes... — pointed at diff lines it never opened: it made tool calls, but none of them read the diff. 开头——读者无法判断这个 PR 到底有没有被审查。开头本身是准确的(diff 的一部分已被认证,披露指出了未覆盖的部分),披露也是准确的(有一个 agent 从未打开 diff),但两者并排读起来像自相矛盾;而那个主语——一段在单词中间被截断的 prompt 片段——看起来像是对整个 PR 的论断,而不是某一个失败 agent 的名字。第一版修复尝试宣告缺口的存在("Reviewed, with gaps disclosed below."),读者仍然困惑:任何以 "Reviewed" 开头的 opener 和下方的 "Not reviewed:" 并排都会形成对撞。这是呈现缺陷,不是结论缺陷:所有上限(cap)和认证检查都按设计工作,本 PR 也不改动它们。

Reviewer Test Plan

如何验证

  • 运行 cd packages/cli && npx vitest run src/commands/review/compose-review.test.ts src/commands/review/check-coverage.test.ts,确认全部 261 个测试通过,包括钉住 partial 开头(中英文两半)、加引号且按词边界截断的 agent 标签、自然中文原因的新回归测试。
  • 确认未改动的开头措辞仍被钉住:Reviewed — no blockers.(认证)、Reviewed.(有 cap 但无缺口披露)和零认证 ⚠️ 警告都保持原文案——它们的测试未被改动且通过。

前后对比证据

修复前(#8811 上发布的正文):Reviewed. Not reviewed: This PR narrows the daemon-marker check from a truthy tes... — pointed at diff lines it never opened: it made tool calls, but none of them read the diff.

修复后(同样的结论输入):Partially reviewed — gaps disclosed. Not reviewed: "This PR narrows the daemon-marker check from a truthy test…" — pointed at diff lines it never opened: it made tool calls, but none of them read the diff.——中文折叠区相应位置由 已审查。 变为 仅完成部分审查,审查缺口已披露。,原因措辞变为 启动 prompt 为它指定了 diff 中的行,但它从未打开:有工具调用,却没有一次读取 diff

测试环境

OS 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

环境(可选)

Node.js 22;已运行 compose-review 聚焦单测,未重跑 exact-head 全仓 build。

风险与范围

  • 主要风险或权衡:仅改动发布正文的措辞。结论、上限、去重和认证检查均未触碰——内部披露主语仍是未加引号的标签,所有结构性检查都基于它。
  • 未验证 / 超出范围:未在真实 PR 上跑端到端 /reviewmain 上预存的 stale-bundle 测试失败(已在干净基线上确认)与本 PR 无关,保持原样。
  • 破坏性变更 / 迁移说明:无。任何对"携带披露的正文以 Reviewed. 开头"做字面匹配的地方会看到新句子;本仓库内这类消费者只有测试,已同步更新。

关联 Issue

跟进 #8811 上出现的读者困惑。

…ctions

A posted review body on QwenLM#8811 opened "Reviewed. Not reviewed: This PR
narrows the daemon-marker check from a truthy tes..." and readers could
not tell whether the PR had been reviewed at all. The opener certified a
part of the diff while the disclosure named an agent that never opened
it — both true, but the pair read as a self-contradiction, and the
disclosure subject (a launch-prompt fragment truncated mid-word) looked
like a claim about the whole PR.

- When "Not reviewed:" clauses follow, the opener now says "Reviewed,
  with gaps disclosed below." The certifying and zero-certified openers
  keep their exact wording.
- Agent-derived subjects (the truncated prompt line used when an agent
  is tied to no chunk) render quoted — they are names, not sentences.
- That truncation stops at a word boundary instead of mid-word.

Verdicts, caps, dedup and the certification check are untouched: the
internal subject stays the unquoted label and every structural check
keys on it.
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

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

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR! Re-running triage on the current head — five commits and four /review rounds since the last pass.

Template looks good ✓

Problem: observed, not theoretical. The body posted on #8811 opened Reviewed. two lines above Not reviewed: This PR narrows the daemon-marker check from a truthy tes... — the before/after is quoted in the description, and the defect is still reproducing live: this PR's own round-5 review body (composed by pre-fix main code) opens Reviewed. above its own Not reviewed: reverse audit — … disclosure.

Direction: aligned. This is the compose path for /review's posted bodies — the wording defect undermines the trustworthiness of the review pipeline itself. CHANGELOG: not applicable — internal review tooling with no upstream counterpart to check.

Size: not applicable — no core paths touched (packages/cli/src/commands/review/** only). For scale: ~137 production lines vs 135 test lines.

Approach: minimal and reuse-first — word-boundary truncation reuses compressSummary, the quoting rides the existing mdField inert-span helper and the existing publicSubject field, and the certification conditions are folded into one boolean rather than rewritten. Every hunk serves the stated goal; no drive-by changes.

Risk: no high-risk path matches. No elevated risk signals.

Moving on to code review. 🔍

中文说明

感谢贡献!本次在最新 head 上重跑 triage——距上次审查已有 5 个 commit、4 轮 /review

模板完整 ✓

问题:已观测到,不是理论问题。#8811 上发布的正文以 Reviewed. 开头,两行之下就是 Not reviewed: This PR narrows... ——描述中引用了 before/after,而且缺陷仍在实时复现:本 PR 自己的第 5 轮 review 正文(由修复前的 main 代码生成)就是 Reviewed. 之下跟着 Not reviewed: reverse audit — … 披露。

方向:对齐。这是 /review 发布正文的组合路径——措辞缺陷损害的是 review 流水线本身的公信力。CHANGELOG:不适用——内部 review 工具,无上游对照可查。

规模:不适用——未触及核心路径(仅 packages/cli/src/commands/review/**)。体量参考:生产代码约 137 行,测试 135 行。

方案:最小化且优先复用——按词边界截断复用 compressSummary,引号渲染复用现有 mdField 惰性 span helper 与现有 publicSubject 字段,认证条件合并为一个布尔量而非重写。每个 hunk 都服务于既定目标,无顺手改动。

风险:未命中高风险路径。无升级风险信号。

进入代码审查 🔍

Qwen Code · qwen3.8-max

Reviewed at 664d10c8a55460d4e09875c71dbfd509a4aeb47e · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Code review

Reviewed against 664d10c8a5. No blockers found.

Independent baseline. Before opening the diff, my proposal for the #8811 shape was: branch the opener on coverage state (zero-certified keeps the ⚠️ warning, partial gets a non-Reviewed opener), quote agent-derived labels because they are names, not sentences, truncate on a word boundary with the existing helper, and keep every structural check keyed on the untouched internal label. The PR does all of that, plus two things I had missed that the review rounds shook out: an (×N) count for when two distinct agents' labels truncate to the same public subject, and Review incomplete — openers for the two findings-file shapes that previously opened Reviewed. while carrying a ⚠️ clause — the same contradiction shape, one clause down.

What I verified in the diff:

  • The opener tree is exhaustive and single-directional: ⚠️ zero-certified → Partially reviewed — gaps disclosed.Reviewed — no blockers. → the two Review incomplete — openers → Reviewed.. The folded !hasCoverageGaps in canCertify is logically identical to the four conditions it replaces (uncoverable, unreviewed + coverageEntries, missingReceipts), so certification semantics are unchanged — including the empty-coveredChunks vacuous-truth edge.
  • The internal/public subject split holds. Every .subject read is structural keying (dedup set, chunk partition, certification); every posted surface routes through publicSubjectpublicAgentSubject: a backtick code span (mdField) around a JSON.stringify-escaped, compressSummary-cut (60 chars, word boundary) label. Byte-checked the test expectations — straight-quoted "…", inner quotes escaped — exactly what that pipeline emits. This also hardens the posted body against prompt-derived labels carrying Markdown, @mentions, or newlines on fork PRs, which the bare rendering did not.
  • APPROVE path untouched and safe: coverage gaps cap the event before it can be reached. Budget-gap bodies keep the plain Reviewed. opener — a disclosed budget gap is a disclosure, not a coverage gap — pinned by the budget-gap test (toContain('Reviewed.') + not.toContain('Partially reviewed')).
  • REQUEST_CHANGES and context-unavailable bodies now lead with the coverage opener; both orderings are pinned by the new tests.
  • label() in coverage.ts drops the 60-char internal cap but keeps whitespace collapse; chunk N labels stay bare through publicAgentSubject so the chunk collapse still sees them.

Non-blocking nits (for the record — R5-1 in the /review round raised the first):

  1. The two ordering assertions (indexOf('Partially reviewed') < indexOf('Reviewed diff-only')) pass vacuously if the partial opener were ever absent (−1 is less than any found index). A paired toContain('Partially reviewed — gaps disclosed.') on those two shapes would close the hole; the code itself is unambiguous (if (coverageOpener) clauses.push(coverageOpener)).
  2. With the internal cap gone, operator stderr (remediation lines) can now carry full-length prompt first lines. Public surfaces stay capped at 60 chars; the author scoped this out intentionally.

Testing (CI evidence — unattended run)

Per the gate rules this run does not execute PR code; the evidence below is the PR's own CI on the reviewed head, fetched via the API. All checks completed — nothing pending, nothing failing.

Check Conclusion
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Test (macos-latest, Node 22.x) ⏭️ skipped (fork-PR policy)
Test (windows-latest, Node 22.x) ⏭️ skipped (fork-PR policy)
Integration Tests (CLI, No Sandbox) ⏭️ skipped (fork-PR policy)

The central claim — composed body wording — is pinned by exact-string assertions (every opener in both languages, the quoted/truncated labels, the (×2) count, the inert-Markdown rendering) inside the green ubuntu suite, so CI substantiates the change directly; a sandboxed verify run for this triage is already in flight and will post its own report. Not verified here: macOS/Windows/integration runs (skipped for fork PRs; the author reports macOS tested — author's claim, not independently re-run; the change is pure string composition with no platform-dependent path). This is an unattended CI run, so no tmux real-scenario pass was driven (Stage 2c does not apply).

中文说明

代码审查

基于 664d10c8a5 审查,未发现阻断问题。

独立基线:我的方案是——按覆盖状态分支选择 opener(零认证保留 ⚠️ 警告、部分审查改用非 Reviewed 开头)、给 agent 衍生的 label 加引号(它是名字不是句子)、用现有 helper 按词边界截断、所有结构性检查继续以未改动的内部 label 为键。PR 全部做到,还补上了两点我漏掉的(也是 review 各轮打磨出来的):两个不同 agent 的 label 截断后公开主语相同时加 (×N) 计数;findings 文件两种异常形态改用 Review incomplete — 开头(之前它们顶着 ⚠️ 子句仍以 Reviewed. 开头——同一种自相矛盾,只是低了一层)。

diff 中核实的内容:opener 决策树穷尽且单向;canCertify 合并出的 !hasCoverageGaps 与原先四个条件逻辑等价,认证语义不变(含 coveredChunks 为空时的空真边界)。内部/公开主语切分完好:所有 .subject 读取都是结构性键控(去重集合、chunk 划分、认证),所有发布面都经过 publicSubject → 反引号 code span + JSON.stringify 转义 + 60 字符词边界截断(已逐字节核对测试期望:直引号 "…"、内层引号转义——与该管线输出完全一致);这也顺带加固了对 fork PR 上 prompt 衍生 label 注入 Markdown/@mention/换行的防护。APPROVE 路径未触碰且安全:覆盖缺口会在到达之前封顶事件。预算缺口正文保留普通 Reviewed. opener(披露不是覆盖缺口),有预算缺口测试钉住。REQUEST_CHANGES 与 context-unavailable 正文现在以覆盖 opener 领头,顺序均有新测试钉住。coverage.tslabel() 取消内部 60 字符上限但保留空白折叠;chunk N 标签保持裸出,chunk 折叠不受影响。

非阻断建议(记录在此,第 1 条即 /review 轮次的 R5-1):两处顺序断言(indexOf 比较)在 partial opener 缺失时会空真通过(−1 小于任何找到的下标),补一个 toContain 即可堵上——代码本身无歧义。内部上限取消后运维 stderr 可能出现整行 prompt 首行,公开面仍有 60 字符上限,作者已明确列为范围外。

测试(CI 证据——无人值守运行)

按门禁规则,本次运行不执行 PR 代码;以下证据为被审 head 上 PR 自身 CI(经 API 获取)。所有检查已完成——无 pending、无失败。核心结论(正文措辞)由绿色 ubuntu 套件中的精确字符串断言(双语每个 opener、加引号截断的 label、(×2) 计数、惰性 Markdown 渲染)直接钉住,CI 已实质验证该变更;本次 triage 的沙箱验证运行已在进行,将单独发布报告。此处未验证:macOS/Windows/集成运行(fork PR 跳过;作者称已在 macOS 测试——为作者声明,未独立复跑;改动为纯字符串组合,无平台相关路径)。无人值守 CI 运行不驱动 tmux 真实场景(Stage 2c 不适用)。

Qwen Code · qwen3.8-max

Reviewed at 664d10c8a55460d4e09875c71dbfd509a4aeb47e · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — every stage landed clean and the fix is pinned by exact-string tests in a green suite; the fifth point is the two non-blocking nits (vacuous ordering assertions, unbounded operator-stderr labels), neither of which touches the posted-body guarantee.

Reflection, honestly: this one earned its merge. The problem is not hypothetical — it reproduced on #8811, and it reproduced again on this PR's own round-5 review body while I was triaging it (pre-fix main code opening Reviewed. above its own Not reviewed: disclosure). The fix attacks the shape, not the instance: every opener that carries a disclosure now reads in one direction, the certifying and zero-certified openers keep their exact wording, and the agent-derived labels — which on fork PRs are attacker-influenced text — now render inside an escaped, backtick-bounded code span instead of bare prose. The injection hardening was not even the stated goal; it fell out of doing the quoting right.

The process worked as designed too: four /review rounds shook out the INTERNAL-label regression (round 2's Critical), the context-unavailable ordering (round 4's Critical), and the truncation collisions — each fixed and re-verified, round 5 closing with Suggestion-level findings only. The final diff is ~137 production lines, every hunk load-bearing, reusing compressSummary / mdField / publicSubject instead of adding parallel machinery. If I'm maintaining this in six months, the opener decision tree is one exhaustive, single-directional chain with the #8811 rationale in the comments — I'd thank the author.

Approving, pinned to the reviewed commit. The two nits are worth a follow-up if the author wants, but neither blocks.

中文说明

反思(如实说):这个 PR 配得上合入。问题不是假设——它在 #8811 复现过,在我 triage 期间又在本 PR 第 5 轮 review 正文上复现了一次(修复前的 main 代码在自己的 Not reviewed: 披露上方顶着 Reviewed.)。修复针对的是这一形态而非单个实例:所有携带披露的 opener 现在都朝一个方向读,认证型与零认证型 opener 措辞保持不变,而 agent 衍生的 label(在 fork PR 上是可被攻击者影响的文本)现在渲染在转义过、反引号包裹的 code span 里,不再是裸文本。注入加固甚至不是既定目标——它是把引号做对的副产品。

流程也按设计运转:4 轮 /review 分别揪出了 INTERNAL-label 回归(第 2 轮 Critical)、context-unavailable 顺序(第 4 轮 Critical)和截断撞名——逐一修复并复核,第 5 轮收敛到仅剩 Suggestion 级发现。最终 diff 约 137 行生产代码,每个 hunk 都承重,复用 compressSummary / mdField / publicSubject 而非另造轮子。六个月后维护这段代码时,opener 决策树是一条穷尽、单向的链,注释里带着 #8811 的缘由——我会感谢作者。

已按被审 commit 钉住批准。两个建议值得后续跟进,但均不阻断。

Qwen Code · qwen3.8-max

Reviewed at 664d10c8a55460d4e09875c71dbfd509a4aeb47e · re-run with @qwen-code /triage

@yiliang114

yiliang114 commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator Author

For anyone wondering what a posted review body actually looks like after this change, here is the exact #8811 case.

Before (what #8811 received):

Reviewed. Not reviewed: This PR narrows the daemon-marker check from a truthy tes... — pointed at diff lines it never opened: it made tool calls, but none of them read the diff.

After (same verdict inputs):

Partially reviewed — gaps disclosed below. Not reviewed: "This PR narrows the daemon-marker check from a truthy..." — pointed at diff lines it never opened: it made tool calls, but none of them read the diff.

The opener does not start with "Reviewed" at all in this shape — an earlier cut tried "Reviewed, with gaps disclosed below." and still read as contradicting the "Not reviewed:" clauses below it. On a PR whose description carries Han characters, the Chinese fold opens with 仅完成部分审查,未覆盖部分见下方披露。 instead of 已审查。, and the reason reads as natural Chinese (启动 prompt 为它指定了 diff 中的行,但它从未打开:有工具调用,却没有一次读取 diff) rather than a literal translation.

Two things stay exactly as they are: the verdict (same caps, same COMMENT event — wording only), and every gap-free body — Reviewed — no blockers. and the bare Reviewed. (caps without disclosures) keep their current wording.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship — CI landed green after the review. ✅

@yiliang114
yiliang114 requested a review from wenshao August 10, 2026 09:42
@yiliang114
yiliang114 enabled auto-merge August 10, 2026 09:42
…e one reason

Feedback on the first wording: "Reviewed, with gaps disclosed below."
still starts with the word the "Not reviewed:" clauses take back, and
readers had to reconcile the pair anyway. The opener now says the review
is PARTIAL instead — "Partially reviewed — gaps disclosed below." — so
opener and disclosures read in one direction (Chinese half:
仅完成部分审查,未覆盖部分见下方披露。).

Also replaces the translationese Chinese rendering of the unopened-agent
reason ("它被指向 diff 的行却从未打开") with natural phrasing.
Comment thread packages/cli/src/commands/review/lib/coverage.ts Outdated
Comment thread packages/cli/src/commands/review/compose-review.ts Outdated
Comment thread packages/cli/src/commands/review/lib/coverage.ts
Comment thread packages/cli/src/commands/review/compose-review.ts
Comment thread packages/cli/src/commands/review/compose-review.ts
Comment thread packages/cli/src/commands/review/compose-review.ts Outdated
Comment thread packages/cli/src/commands/review/compose-review.ts Outdated
Comment thread packages/cli/src/commands/review/compose-review.ts Outdated
Comment thread packages/cli/src/commands/review/compose-review.ts Outdated

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed. Suggestions are inline. Not explored to full depth (tool budget reached): Context: PR #8857 fixes the wording of the body a /review...: tests were not executed — the shared worktree was concurrently dirty, so a run would have tested the wrong bytes; all conclusions rest on committed-blob tracing…. Test Plan (not a blocker): 261 tests pass — this review observed 18774 passed.

中文说明

已审查。 建议见行内评论。 未探索到全部深度(达到工具调用预算):Context: PR #8857 fixes the wording of the body a /review...:tests were not executed — the shared worktree was concurrently dirty, so a run would have tested the wrong bytes; all conclusions rest on committed-blob tracing…。 Test Plan(非阻断):261 tests pass — this review observed 18774 passed

— qwen3.8-max via Qwen Code /review (v0.21.8)

Comment thread packages/cli/src/commands/review/compose-review.ts
Comment thread packages/cli/src/commands/review/compose-review.ts Outdated
Comment thread packages/cli/src/commands/review/compose-review.ts Outdated
Comment thread packages/cli/src/commands/review/lib/coverage.ts Outdated
Comment thread packages/cli/src/commands/review/compose-review.ts
Comment thread packages/cli/src/commands/review/compose-review.ts Outdated
@yiliang114
yiliang114 requested a review from wenshao August 10, 2026 12:39
@yiliang114

yiliang114 commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator Author

Handled the current feedback and CI follow-up at 1007daa1e64.

Changed:

  • Removed directional wording from partial-review disclosures.
  • Quoted prompt-derived labels on both coverage and tool-budget surfaces using the existing summary helper.
  • Added an explicit incomplete-review opener for surviving unverified findings.
  • Updated the PR body to match the exact behavior.
  • Merged the latest main normally after an unrelated exact-head CI failure.

Verified:

  • compose-review.test.ts: 184/184 passed after the base refresh.

Intentionally not changed:

  • Unrelated-agent admission changes coverage eligibility and certification semantics, outside this presentation-only PR.
  • The failed Ubuntu run covered unrelated CLI/core suites and showed broad timeout/worker-load failures; no failure pointed to review-gap disclosure.

Pending:

  • New exact-head CI and human re-review. The full exact-head repository build was not rerun.
中文摘要

已修正 partial opener、两条 agent 标签展示路径和未验证 finding 的开头,同步 PR 正文,并因无关 runner 不稳定普通合入 latest main;刷新后 compose-review.test.ts 184/184 通过。未改变 coverage 资格与认证语义,等待新 head CI 与人工复审。

@yiliang114

Copy link
Copy Markdown
Collaborator Author

@qwen-code-review-bot pls review this pr and approve it if no blockers

Comment thread packages/cli/src/commands/review/lib/coverage.ts
Comment thread packages/cli/src/commands/review/compose-review.ts
Comment on lines +1443 to +1445
: findingsUnverifiedAtCompose
? {
en: 'Review incomplete — unverified findings disclosed.',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] This opener overclaims on the findings-file-unreadable shape. findingsUnverifiedAtCompose is set on TWO shapes: surviving — [unverified] tags (~line 791) and the unreadable-file catch (~line 802, which also sets findingsFileUnreadable). On the second shape the body discloses an unreadable file — not unverified findings. — Failure scenario: an unreadable findings file with otherwise clean coverage (reachable — the findings file and the plan/transcripts are independent files, and the remediation line itself anticipates an operator passing the wrong path) posts Review incomplete — unverified findings disclosed. directly above ⚠️ The reverse-audit findings file could not be read at compose time… — verified by probe at the reviewed commit. The opener names entities the body never presents; the zh half (审查未完成——未验证的发现已披露。) carries the same overclaim. This is a milder instance of the opener/body self-contradiction pattern this PR exists to remove. Suggested fix: widen the opener to cover both shapes (e.g. Review incomplete — verification not shown. with a matching zh), or branch the wording on findingsFileUnreadable.

中文说明

该开头在"findings 文件不可读"这一形态下言过其实。findingsUnverifiedAtCompose 有两种置位形态:残留的 — [unverified] 标记(约 791 行)与文件不可读的 catch(约 802 行,同时置 findingsFileUnreadable)。第二种形态下正文披露的是"文件读不了",而不是"未验证的发现"。— 失败场景:findings 文件不可读但其余覆盖干净(可达——findings 文件与 plan/transcripts 是相互独立的文件,补救文案本身就预设了操作者传错路径)时,发布正文会在 ⚠️ The reverse-audit findings file could not be read at compose time… 上方直接以 Review incomplete — unverified findings disclosed. 开头——已在被审提交上用探针验证。开头提及的对象正文根本没有给出;中文一半(审查未完成——未验证的发现已披露。)同样言过其实。这是本 PR 要消除的"开头/正文自相矛盾"模式的较轻实例。建议修复:把开头放宽到覆盖两种形态(如 Review incomplete — verification not shown. 及对应中文),或按 findingsFileUnreadable 分支措辞。

— qwen3.8-max via Qwen Code /review (v0.21.8)

Comment thread packages/cli/src/commands/review/compose-review.ts
Comment on lines +1443 to +1445
: findingsUnverifiedAtCompose
? {
en: 'Review incomplete — unverified findings disclosed.',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The precedence between the two new opener branches (hasCoverageGaps vs findingsUnverifiedAtCompose) and the zh half of this incomplete opener are pinned by no test: every tagged-findings test runs on the fully covered base() plan, and none uses a han-stamped plan. — Failure scenario: mutation probes at the reviewed commit — (1) swapping these two branches passes 184/184: on a messy run with both an unreviewed chunk and a surviving [unverified] tag, the opener would silently flip between Partially reviewed — gaps disclosed. and Review incomplete — unverified findings disclosed.; (2) corrupting the zh 审查未完成——未验证的发现已披露。 also passes — no han-stamped plan ever passes findingsPath, while the sibling partial opener has both halves pinned by the bilingual test. This branch is tested at half the depth of its sibling. Suggested fix: add one test combining a coverage gap (e.g. unreviewedDimensions: ['security']) with a tagged findings file on a han: true plan, pinning which opener wins and both zh halves.

中文说明

两个新开头分支(hasCoverageGapsfindingsUnverifiedAtCompose)的优先级,以及该"审查未完成"开头的中文一半,都没有任何测试钉住:所有带标记 findings 的测试都跑在完全覆盖的 base() 计划上,且没有一个使用带 han 标记的计划。— 失败场景:在被审提交上做变异探针——(1) 交换这两个分支,184/184 全过:在一次既有未审 chunk 又残留 [unverified] 标记的混乱运行里,开头会在 Partially reviewed — gaps disclosed.Review incomplete — unverified findings disclosed. 之间静默翻转;(2) 破坏中文 审查未完成——未验证的发现已披露。 同样全过——没有任何 han 计划会传 findingsPath,而相邻的 partial 开头中英文两半都被双语测试钉住。该分支的测试深度只有相邻分支的一半。建议修复:新增一个测试,在 han: true 计划上同时制造覆盖缺口(如 unreviewedDimensions: ['security'])与带标记的 findings 文件,钉住哪个开头胜出以及中文两半。

— qwen3.8-max via Qwen Code /review (v0.21.8)

Comment thread packages/cli/src/commands/review/compose-review.ts

@qwen-code-review-bot qwen-code-review-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. 263 tests pass, CI green.

The core fix is sound — replacing Reviewed. followed by Not reviewed: with Partially reviewed — gaps disclosed. eliminates the self-contradiction pattern from #8811. The hasCoverageGaps variable correctly distinguishes coverage gaps (which justify the partial opener) from budget-gap disclosures (which don't), and the findingsUnverifiedAtCompose branch handles the unverified-findings shape with its own distinct opener.

The remaining suggestions from both rounds of review are legitimate edge cases worth following up on (quoted-label escaping, compressSummary dedup surface, test gaps for new branches), but none regress vs. the pre-PR state and none should block this fix.

@yiliang114

Copy link
Copy Markdown
Collaborator Author

Closeout: kept full normalized labels as internal dedup keys, safely rendered compressed public labels with embedded quotes, and made unreadable findings report unavailable instead of unverified. Verified 184 focused tests. Intentionally skipped redundant per-surface coverage and chunk-label presentation changes in this late review round. All active threads are closed; exact-head CI/review is pending.

中文摘要已修复内部标签碰撞、引号转义和 findings 不可读提示;184 个测试通过,未增加重复覆盖或改动 chunk 展示,线程已清零,等待 CI/评审。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed. Suggestions are inline.

Not explored to full depth (tool budget reached): Context: PR #8857 (QwenLM/qwen-code) rewords the /review ...: full repo npm run typecheck /build not run (requires building all workspace packages first); I relied on the passing vitest suite + eslint + the diff's simple …; Context: PR #8857 (QwenLM/qwen-code) rewords the /review ...: none — I completed every check I planned. Tool calls used: about 30. Within budget..

Not reviewed: reverse audit — stopped before round 4 by the review time budget.

中文说明

已审查。 建议见行内评论。

未探索到全部深度(达到工具调用预算):Context: PR #8857 (QwenLM/qwen-code) rewords the /review ...:full repo npm run typecheck /build not run (requires building all workspace packages first); I relied on the passing vitest suite + eslint + the diff's simple …;Context: PR #8857 (QwenLM/qwen-code) rewords the /review ...:none — I completed every check I planned. Tool calls used: about 30. Within budget.

未审查:反向审计——评审时间预算不足,未能开始第 4 轮。

— qwen3.8-max via Qwen Code /review (v0.21.9)

Comment thread packages/cli/src/commands/review/compose-review.ts
Comment thread packages/cli/src/commands/review/compose-review.ts Outdated
function publicAgentSubject(label: string): string | undefined {
return /^chunk \d+$/.test(label)
? undefined
: mdField(JSON.stringify(compressSummary(label)));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] R3-3: the internal dedup key and the public display form have diverged — seenSubjects dedups on the full internal label, while this line truncates the display form independently via compressSummary. — Failure scenario: probe-reproduced — two idle whole-diff agents whose launch-prompt first lines share a ~59-char prefix and differ only past it both survive the dedup, then both get cut at the same word boundary, so one Not reviewed: sentence renders the byte-identical quoted label twice (presenting one apparent agent twice). Pre-diff, the internal label WAS the 60-char display form, so subject-dedup doubled as display-dedup. Cosmetic only — both entries are still posted, nothing is dropped — but it defeats the 1-subject-per-agent correspondence this sentence relies on. Suggested fix: also dedup on the printed subject — skip in the byReason render loop when the resolved publicSubject ?? subject was already emitted for that reason group.

中文说明

内部去重键与公开展示形式已经分离——seenSubjects 用完整的内部标签去重,而这一行通过 compressSummary 独立截断展示形式。— 失败场景(已用探针复现):两个 whole-diff agent 的启动 prompt 首行共享约 59 字符前缀、仅在其后不同,两者都会通过 seenSubjects 去重,又在同一词边界被截断,于是同一条 Not reviewed: 句子里出现两次字节完全相同的带引号标签(看起来像同一个 agent 出现了两次)。PR 前内部标签就是 60 字符的展示形式,subject 去重同时起到了展示去重的作用。仅影响展示——两条记录仍会全部发布、不会丢数据——但破坏了这句话依赖的“一个 agent 一个主语”的对应关系。建议修复:在 byReason 渲染循环中对打印主语也做去重(同一 reason 分组内,若解析出的 publicSubject ?? subject 已输出过则跳过)。

— qwen3.8-max via Qwen Code /review (v0.21.9)

Comment thread packages/cli/src/commands/review/compose-review.ts
Comment thread packages/cli/src/commands/review/compose-review.ts Outdated
function publicAgentSubject(label: string): string | undefined {
return /^chunk \d+$/.test(label)
? undefined
: mdField(JSON.stringify(compressSummary(label)));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] R3-5: mdField strips backticks AFTER compressSummary's whitespace collapse has run, so backtick-bearing labels render with doubled spaces inside the quoted code span — and every CLI-built role-agent identity line contains backticks around the role (the "You are review agent <role> — …" line, agent-prompt.ts:1395/:642). — Failure scenario: probe-reproduced through the real composeReview — an idle role agent renders with doubled spaces around the role name where GitHub preserves whitespace, on both render sites this diff rewrites (chunk agents escape via the chunk N arm; role agents don't). This is the most common production case for the exact sentence this PR rewrites to render agent names cleanly, and no test exercises a backtick-bearing label through this path. The fix sanitizes before the collapse so the existing collapse absorbs the replacement space; do not move space-collapsing into mdField itself — its other callers render diff file paths, where spacing is meaningful.

Suggested change
: mdField(JSON.stringify(compressSummary(label)));
: mdField(JSON.stringify(compressSummary(label.replace(/[`\r\n]+/g, ' '))));
中文说明

mdFieldcompressSummary 的空白折叠之后才去除反引号,因此带反引号的标签渲染成带引号的代码片段时会出现双空格——而每个 CLI 构建的 role agent 身份行都在角色名两侧带反引号("You are review agent <role> — …" 行,agent-prompt.ts:1395/:642)。— 失败场景(已通过真实 composeReview 探针复现):一个 idle 的 role agent 渲染后角色名两侧出现双空格(GitHub 在代码片段内保留空白),本 diff 重写的两处展示面都会出现(chunk agent 走 chunk N 分支幸免,role agent 不会)。这正是本 PR 要“把 agent 名字渲染干净”的那句话在生産中最常见的情况,且没有测试让带反引号的标签走过这条路径。修复方式:在折叠之前先清理反引号,让既有的折叠吸收替换产生的空格(见上方 suggestion);不要把空白折叠移进 mdField 本身——它的其他调用方渲染 diff 文件路径,那里空格是有意义的。

— qwen3.8-max via Qwen Code /review (v0.21.9)

@yiliang114

Copy link
Copy Markdown
Collaborator Author

Changed: normalize removed backticks before summary whitespace compression so public agent labels no longer contain doubled spaces.

Verified: 185 focused tests, ESLint, and Prettier passed. CLI package typecheck was blocked by unrelated unbuilt workspace artifacts.

Intentionally not changed: additional abstraction, documentation, and mutation-only coverage requests.

Pending: exact-head CI and automatic review.

中文摘要

已修复公开 agent label 去反引号后的双空格;聚焦测试与 lint 通过,等待 CI。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: I could not trace which attribution path feeds the quoted label in the prose-label test — label() 's first line is 58 chars (≤60, so compressSummary would no….

Test Plan (not a blocker): 261 tests pass — this review observed 18872 passed.

中文说明

未探索到全部深度(达到工具调用预算):You are review agent reverse-audit — Reverse audit agen...:I could not trace which attribution path feeds the quoted label in the prose-label test — label() 's first line is 58 chars (≤60, so compressSummary would no…

Test Plan(非阻断):261 tests pass — this review observed 18872 passed

— qwen3.8-max via Qwen Code /review (v0.21.9)

Comment thread packages/cli/src/commands/review/compose-review.ts
Comment thread packages/cli/src/commands/review/compose-review.ts Outdated
Comment thread packages/cli/src/commands/review/lib/coverage.ts
Comment thread packages/cli/src/commands/review/compose-review.ts
@yiliang114

Copy link
Copy Markdown
Collaborator Author

Refreshed this branch onto latest main in 839b2d2 after the exact-head Ubuntu test failure; the merge was clean and no product files were edited outside the normal base merge. Verified git diff --check; new exact-head CI is pending.

@yiliang114

Copy link
Copy Markdown
Collaborator Author

Resolved the current review round on 664d10c8a55: fixed coverage-gap ordering for context-unavailable verdicts, reused that coverage state for certification, and disambiguated truncated public labels with counts. The operator-only stderr label-length suggestion was left unchanged as out of scope. Prettier and git diff --check pass; the focused test is blocked during collection by the existing local dependency gap (ajv/dist/2020.js). Exact-head CI is now running.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed. Suggestions are inline.

Not explored to full depth (tool budget reached): You are review agent verify — Verification agent.: claim holds. Reverting the list back to bare it.agent still leaves all 211 tests passing (probe); a whole-diff agent that passed the credit guard can reach bu….

Not reviewed: reverse audit — stopped before round 5 by the review time budget.

中文说明

已审查。 建议见行内评论。

未探索到全部深度(达到工具调用预算):You are review agent verify — Verification agent.:claim holds. Reverting the list back to bare it.agent still leaves all 211 tests passing (probe); a whole-diff agent that passed the credit guard can reach bu…

未审查:反向审计——评审时间预算不足,未能开始第 5 轮。

— qwen3.8-max via Qwen Code /review (v0.21.9)

Comment on lines +981 to +985
it('discloses coverage gaps before the diff-only warning', () => {
const r = composeReview(
base({ contextUnavailable: true, unreviewedDimensions: ['security'] }),
);
expect(r.body.indexOf('Partially reviewed')).toBeLessThan(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The order-only assertion passes vacuously when the partial opener is absent — indexOf returns -1, which is less than any present index. Mutation-proven at this commit: deleting either coverageOpener push (the context-unavailable COMMENT branch at ~line 1597, or the REQUEST_CHANGES parts prepend at ~line 1527) keeps all 211 tests green. The same shape exists in the RC variant below (lines 1191-1201). — Failure scenario: a future edit drops if (coverageOpener) clauses.push(coverageOpener); → posted bodies open Reviewed diff-only — … directly above Not reviewed: security — the exact #8811 self-contradiction this PR fixes — yet both guard tests stay green (-1 < indexOf('Reviewed diff-only')). The code is correct today; only the guard is ineffective. Fix — add a presence assertion before each ordering check:

expect(r.body).toContain('Partially reviewed — gaps disclosed.');
中文说明

[Suggestion] 仅断言顺序的断言在 partial opener 缺失时会空通过——indexOf 返回 -1,比任何存在的下标都小。已在被审提交上用变异验证:删除任意一处 coverageOpener 推送(约 1597 行的 context-unavailable COMMENT 分支,或约 1527 行的 REQUEST_CHANGES parts 前置)后,全部 211 个测试仍然全绿。同样的形态也存在于下方 1191-1201 行的 RC 变体测试中。— 失败场景:未来某次修改删掉 if (coverageOpener) clauses.push(coverageOpener); → 发布正文将以 Reviewed diff-only — … 开头、直接位于 Not reviewed: security 之上——正是本 PR 要修复的 #8811 自相矛盾——而两个守卫测试仍为绿色(-1 < indexOf('Reviewed diff-only'))。代码目前是正确的,只是守卫无效。修复——在每个顺序断言前加一条存在性断言(见上方英文代码)。

— qwen3.8-max via Qwen Code /review (v0.21.9)

Comment on lines +1403 to +1405
...[...named].map(([subject, { count }]) =>
count > 1 ? `${subject} (×${count})` : subject,
),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The new (×N) counting collapses only entries that survive the pre-existing seenSubjects dedup (~line 1352, keyed on the INTERNAL subject) — byte-identical duplicate labels merge into a single uncounted entry, and the new counting idiom makes "no ×N" read as "exactly one". Probe-proven at this commit: two byte-identical idle twins render as one uncounted Not reviewed: "…" line, while check-coverage's stderr prints both. — Failure scenario: a whole-diff agent fails idle; the body's own prescribed remediation ("relaunch each with the same printed prompt") produces a byte-identical record that also fails idle (keySatisfied can never converge on zero-tool-call records). TWO agents read nothing, but the posted body discloses one uncounted name — the operator register and the author register disagree about the count. The dedup predates this PR; what is new is that (×N) establishes an explicit counting register for one collision class while identical internals get no count at all. Fix — count the dropped twins instead of dropping them (move the dedup to after grouping, or feed the seenSubjects-dropped twins into the same-reason group's count).

中文说明

[Suggestion] 新的 (×N) 计数只合并通过了既有 seenSubjects 去重(约 1352 行,以内部 subject 为键)的条目——字节级相同的重复 label 会被合并成一条无计数的条目,而新的计数习惯让"没有 ×N"被读作"恰好一个"。已在被审提交上用探针验证:两个字节级相同的 idle 孪生 agent 只渲染出一条无计数的 Not reviewed: "…",而 check-coverage 的 stderr 两条都打印。— 失败场景:一个 whole-diff agent 以 idle 失败;正文自身给出的补救指引("用相同启动 prompt 重新启动")产生一条字节级相同的记录且同样 idle(keySatisfied 在零工具调用记录上永远无法收敛)。两个 agent 什么都没读,发布正文却只披露一个无计数的名字——操作者记录与作者记录在数量上不一致。去重本身早于本 PR;新之处在于 (×N) 为其中一类碰撞建立了显式计数语义,而完全相同的内部 label 却没有任何计数。修复——把被丢弃的孪生条目计入数量而不是丢弃(把去重移到分组之后,或把被 seenSubjects 丢弃的孪生条目计入同原因分组的计数)。

— qwen3.8-max via Qwen Code /review (v0.21.9)

Comment on lines 1526 to 1528
const parts = [
...(coverageOpener ? [coverageOpener] : []),
...(contextUnavailable ? [contextUnavailableClause] : []),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The REQUEST_CHANGES parts list reproduces the ⚠️-zero-certified / "Reviewed diff-only" adjacency at a second site. Probe-verified reachable at this commit: a REQUEST_CHANGES run with zero certified coverage (no plan, or unreadable transcripts → the coverage subject) plus a failed PR-discussion fetch posts ⚠️ This run could not certify that any of this diff was reviewed. immediately followed by Reviewed diff-only — the PR's existing discussion could not be fetched… — a sentence beginning with "Reviewed" directly under the sentence certifying nothing was reviewed; in one probe scenario it sat between the ⚠️ opener and Not reviewed: coverage — …. This diff introduces the pairing — pre-PR the RC path had no coverage opener. — Failure scenario: the exact #8811 self-contradiction shape this PR removes elsewhere, reached on the RC path (wording only — disclosures stay complete). Fix — reword the shared contextUnavailableClause (~line 1437) so one edit covers both this RC parts list and the COMMENT clause builder, e.g. drop the certifying prefix:

en: 'Diff-only — the PR’s existing discussion could not be fetched, so this is not an approval and not a no-blockers claim.',
中文说明

[Suggestion] REQUEST_CHANGES 的 parts 列表在第二个位置重现了 ⚠️ 零认证 opener 与 "Reviewed diff-only" 的相邻冲突。已在被审提交上用探针验证可达:一个零认证覆盖(无 plan,或 transcript 不可读 → coverage subject)且 PR 讨论抓取失败的 REQUEST_CHANGES 运行,发布正文先出现 ⚠️ This run could not certify that any of this diff was reviewed.,紧随其后是 Reviewed diff-only — the PR's existing discussion could not be fetched…——一句以 "Reviewed" 开头的话直接位于"无法证明任何部分经过审查"的句子之下;其中一个探针场景中它夹在 ⚠️ opener 与 Not reviewed: coverage — … 之间。这个组合由本 diff 引入——PR 之前 RC 路径没有 coverage opener。— 失败场景:本 PR 在别处消除的 #8811 自相矛盾形态在 RC 路径上再次出现(仅措辞问题——披露信息仍完整)。修复——改写共享的 contextUnavailableClause(约 1437 行),一处修改同时覆盖 RC parts 列表与 COMMENT 子句构造器,例如去掉认证式前缀(见上方英文代码)。

— qwen3.8-max via Qwen Code /review (v0.21.9)

@yiliang114

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

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: 586 passed · 0 failed · 586 total

中文 — 判定:✅ 通过 · 可合入(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:586 通过 · 0 失败 · 586 总计

Verification report

Verification report — PR 8857

Verdict: merge-ready — 586/586 scripted assertions passed, 0 unexpected failures. Verified head: 664d10c8a55460d4e09875c71dbfd509a4aeb47e (base tip 7ebd137edb7e48af1ffd447f1ed7a9bb1ed53799).

中文摘要
  • 结论merge-ready。586 条脚本断言全部通过,0 个意外失败。
  • A/B 结论:同一 harness 在 head 与 base 两个构建上各 9/9 通过(各自编码本臂期望)。5 个翻转场景中,base 输出旧的 Reviewed. 开头 + 裸截断标签(truthy tes...),head 输出 Partially reviewed — gaps disclosed. / Review incomplete — … + 带引号、按词边界截断的标签;4 个控制场景(零认证 ⚠️Reviewed — no blockers.、预算缺口下的 Reviewed.、仅 context-unavailable)两臂逐字节一致。中心改动是承重件。
  • 空转检验:把 HEAD 的测试文件原样跑在 base 代码上,211 条中恰好 11 条失败,全部是本次新增/更新的回归测试,且失败信息均为行为不匹配(开头文案),非编译错误。变异矩阵 7 个单点变异全部被套件捕获(6/2/5/1/1/3/1 条红),无存活变异。
  • Findings(均非阻断):(1) 预算缺口披露仍坐在裸 Reviewed.(COMMENT)与 LGTM! ✅(APPROVE)开头之下——与本 PR 所修矛盾同类,但系作者有意钉住的选择(新测试显式断言),APPROVE 变体为预存行为;(2) compressSummary 对跨截断边界的 astral 字符会产出孤立代理项(预存 helper 局限,现从 agent 标签可达,仅显示问题)。
  • 更正:PR 正文称 261 个测试,实测 head 为 290(211+79);正文所述 main 上的 stale-bundle 预存失败在本容器两臂均不复现(28/28 通过)。
  • 未覆盖:未跑真实 PR 的端到端 /review;未对全部兄弟披露面(deferred lint、test-plan 注记)做开头组合穷举(均为预存且标注 "not a blocker" 的通道)。

Central claim and A/B proof

Central claim: a review body that carries coverage-gap disclosures opens with the non-directional Partially reviewed — gaps disclosed. (zh 仅完成部分审查,审查缺口已披露。) instead of Reviewed., and surviving-unverified-findings / unreadable-findings bodies open with explicit Review incomplete — … openers — so the opener never certifies what the disclosures below it take back (#8811).

Secondary claims: (S1) agent-derived disclosure subjects render quoted, word-boundary-truncated, inert-Markdown, with (×N) counting on public-subject collisions while the internal subject stays unquoted; (S2) the untouched openers (Reviewed — no blockers., zero-certified ⚠️, budget-gap Reviewed.) keep their exact wording.

Method: one self-contained vitest harness (ab-harness.test.ts, kept in this artifact dir) drives the real composeReview through nine filesystem-fixture scenarios copied from the suite's own fixture recipes; expectations are encoded per arm via AB_ARM=head|base, so the same file asserts the base shape on base and the head shape on head, plus the absence of the other arm's marker (each arm's run therefore proves which code it executed). Head arm ran in the CI tree; base arm ran in a scratch worktree at HEAD^1 (7ebd137edb) with nested node_modules symlinked from the head tree. Realpath check: the cli vitest config aliases @qwen-code/qwen-code-core to each tree's own packages/core source, and git diff HEAD^1..HEAD -- packages/core package.json package-lock.json is empty — both arms execute identical core code; the only differing code is the PR's three files.

# scenario oracle (first line / markers) base head
S1 partial gap, #8811 shape, bilingual opener; label rendering; zh halves Reviewed. + bare truthy tes... + 它被指向 diff 的行却从未打开 Partially reviewed — gaps disclosed. + `"This PR narrows … truthy test…"` + 启动 prompt 为它指定了 diff 中的行…
S5 surviving — [unverified] tag opener Reviewed. Review incomplete — unverified findings disclosed. (+ zh)
S6 unreadable findingsPath opener Reviewed. Review incomplete — findings unavailable.
S7 context-unavailable + gap opener order warning only, no partial opener partial opener before Reviewed diff-only
S9 REQUEST_CHANGES + gap RC body first line Not reviewed: … Partially reviewed — gaps disclosed.
S2 zero certified (idle) opener ⚠️ zero-certified ⚠️ zero-certified (identical)
S3 fully covered + suggestions opener Reviewed — no blockers. Suggestions are inline. identical
S4 budget gap + suggestion opener Reviewed. + Not explored to full depth identical
S8 context-unavailable alone opener diff-only warning, no Reviewed. identical

Flip count: 5/5 flip scenarios change base→head exactly as the PR claims; 4/4 control cells byte-identical. Witnesses: evidence/01-ab-head-arm.png, evidence/02-ab-base-arm.png (both arms 9/9 as printed). The S1 base cell reproduces the exact #8811 before-string from the PR description, and the head cell the exact after-string, byte for byte.

Reviewer Test Plan walk

  • Step 1 (run the two named suites, "261 tests pass"): ran at head — compose-review.test.ts 211 + check-coverage.test.ts 79 = 290 passed. All pass, but the stated count is stale (see Corrections).
  • Step 2 (unchanged openers still pin): verified by control cells S2/S3/S4/S8 and by the suite's untouched tests passing at head; the zero-certified ⚠️ wording also survives mutation M1a's sibling (M1b) only via its own two tests.
  • Before/After evidence: reproduced verbatim on both arms (S1 row above).

Vacuity and mutation matrix

Vacuity: the HEAD test file executed against base code fails 11/211 — exactly the tests this PR added or updated (partial opener ×4 contexts, quoted label, word-boundary, (×N), inert-Markdown, backtick-collapse, bilingual halves, two findings-opener tests), each failing on the expected-vs-actual opener text, not on imports. The other 200 pass on base, confirming they pin pre-existing behavior.

Mutation matrix at head (compose-review.test.ts, 211 tests; witness evidence/03-mutation-matrix.png):

mutation result pinned by
control (unmutated) 211 passed
M1b coverageOpener removed entirely 6 failed partial + zero-certified opener tests
M1a zero-certified branch removed (ternary first arm) 2 failed the two zero-certified tests
M2 publicAgentSubject → always undefined 5 failed quoting/inert/backtick/word-boundary tests
M3 (×N) counting removed 1 failed the (×2) test
M4 contextUnavailable clause pushed before opener 1 failed the ordering test
M5 label() 60-char mid-word truncation restored 3 failed word-boundary + distinct-label tests
M6 zh unopened reason reverted 1 failed the zh-reason assertion

No surviving mutations; every guard the PR introduces is load-bearing. (Two early malformed variants of M1/M2 were discarded after inspection — the matrix above uses the corrected single-point mutations.)

Corrections

  • Test count: the PR body (both languages) says "261 tests"; measured at the verified head is 290 (211 + 79). The wording fix is otherwise accurate; the number was presumably written at an earlier commit.
  • stale-bundle pre-existing failure: the body says a pre-existing stale-bundle test failure exists on main. In this container lib/stale-bundle.test.ts passes 28/28 on both base and head — the author's failure does not reproduce here (likely environment-dependent). Nothing to attribute; not a regression either way.

Findings (non-blocking)

  1. Budget-gap disclosures keep a certifying-adjacent opener — deliberate, pinned. With a disclosed tool-budget gap and no coverage gap, the COMMENT body opens Reviewed. two lines above Not explored to full depth (tool budget reached): … (S4 cell), and an Approve-capped run opens No issues found. LGTM! ✅ above the same line (probe: {"event":"APPROVE","firstLine":"No issues found. LGTM! ✅","hasBudget":true,"hasPartial":false}). This is the sibling of the exact contradiction class the PR fixes. It is a deliberate, tested choice: the PR's new test asserts toContain('Reviewed.') + not.toContain('Partially reviewed') for the budget case, and the code comment explains budget gaps are "a channel, never a cap" (the certifying Reviewed — no blockers. is denied above them). The APPROVE variant is pre-existing and untouched by this PR. Reported so a future wording round considers whether the PR's own rationale ("any opener starting with 'Reviewed' reads as contradicting the disclosures below it") should extend to the budget channel; not a merge condition.
  2. Astral labels can yield a lone surrogate + (nit, pre-existing helper). compressSummary cuts at UTF-16 unit 59; a label whose 59th unit is a high surrogate renders …\ud83c… (probe: 'a'.repeat(58)+'🎉 bb cc'has lone surrogate: true; BMP-only zh input is safe). The helper is pre-existing (findings summaries); this PR newly routes agent labels through it. Cosmetic (GitHub shows U+FFFD); the base's own slice(0,57) had the same class of issue.

Not covered

  • No end-to-end /review run against a live PR (matches the PR's own stated scope).
  • Opener interplay with the deferred-lint / test-plan / repo-context disclosure blocks was not exhaustively combinated; those blocks all self-label "not a blocker" and are pre-existing, untouched surfaces.
  • The harness drives the same fixture recipes as the suite; it reproduces the fix(cli): require exact daemon marker before env scrub #8811 body shape deterministically, not the model-side degradation that produced the original unopened agent in production.
  • check-coverage.test.ts (79) ran at head only; lib/coverage.ts's changed label() is exercised on base too via the A/B harness and the 200-passing base suite run.
  • Windows/macOS TUI behavior — N/A (pure string composition).

Methodology

Environment: CI verify container (node:22-bookworm), merge-ref checkout 501da10cee (HEAD^1 base 7ebd137edb, HEAD^2 head 664d10c8a5), npm ci + npm run build pre-run. Harnesses are mock-free vitest files driving the real composeReview over real filesystem fixtures (plans, transcripts, prompt records, findings files) — no stub of the code under test; gh is mocked only where the suite itself mocks it (never called by these scenarios). Base arm ran in a scratch git worktree at HEAD^1 (removed after); its packages/{cli,core}/node_modules were symlinks to the head tree's (lockfile untouched by the PR; core aliased to each tree's own source, which is PR-identical). Raw logs in logs/ (head-arm, base-arm, head-tests-on-base, mutation-matrix); harness sources ab-harness.test.ts, mutate.mjs in this dir. Assertion tally (586): named suites at head 290 + stale-bundle head 28 + stale-bundle base 28 + harness head 9 + harness base 9 + vacuity run 211 (200 green + 11 expected-red counted as passing control assertions, names verified to be exactly the PR's new/updated tests) + mutation matrix 8 (7 mutations observed red as predicted + 1 green control) + sibling probe 1 + eslint 1 + repo typecheck 1. fail: 0 — no unexpected outcome in any run.

Evidence images

01-ab-head-arm

02-ab-base-arm

03-mutation-matrix

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed current head 664d10c. One user-visible wording defect still prevents approval: the REQUEST_CHANGES path can emit the zero-certified warning immediately followed by “Reviewed diff-only” when PR discussion fetch also fails, recreating the exact opener/body contradiction this PR is meant to remove. I independently confirmed the current inline report: #8857 (comment). The focused exact-head suites otherwise pass (290 tests), and I found no additional blocking defect in the changed call paths.

@yiliang114
yiliang114 added this pull request to the merge queue Aug 12, 2026
Merged via the queue into QwenLM:main with commit a649f31 Aug 12, 2026
134 checks passed
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.21.11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants