fix(review): one disclosure per subject — dedupe Not-reviewed, collapse an all-built-none-launched roster - #7190
Merged
Merged
Conversation
…t, collapse an all-built-none-launched roster A public review body on #7188 was a wall of twenty-two "Not reviewed" clauses for eleven roles: the run built every roster prompt, launched not one agent, and every disclosure appeared twice. Two defects, one wall: - compose-review's `unreviewed` list fills from two sides -- the caller's `unreviewedDimensions` and the coverage recomputation -- and a run that pastes the gate's own gap lines into its input posts every one of them twice. The chunk list has deduped by its `chunk <id>` prefix since that exact bug shipped there; the role list now dedupes by label too (everything before the entry's last em-dash segment, because an invariant agent's label legitimately carries one). When both sides name the same subject, the coverage-derived text wins -- it is the evidence-bounded register the body is written in. - coverage collapses "nobody BUILT anything" to one line, but the launched-side twin -- every prompt built, none launched -- was still one line per dimension, burying the single fact that explains all of them: the run stopped at the builder. Collapsed the same way, one line for the run; the per-role selectors survive for the repair.
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with public exhibits — the CI review bot posted 22 duplicate disclosure clauses for 11 roles on #7188, and 99 clauses burying 6 real findings on #7166. Both are linked and verifiable. Direction: well-aligned. The review bot's disclosure composition is a real quality problem that makes the actual review unreadable by the PR author it's meant to serve. CHANGELOG has no direct reference but the area is squarely within the project's mission. Size: not applicable — changes are in Approach: the scope feels right. A structural Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，有公开展品——CI review bot 在 #7188 上为 11 个角色发布了 22 条重复披露；在 #7166 上 99 条披露淹没了 6 条真实 findings。两者均已关联且可验证。 方向：对齐。review bot 的披露作文问题使 PR 作者无法读到真正的 review 内容。CHANGELOG 无直接参考但该领域在项目使命范围内。 规模：不适用——改动在 方案：范围合理。coverage 站点（边界已知）的结构化 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Code review: clean — no issues found. The diff introduces a Tests are well-constructed — each named for its failure mode (caller echo dedup, em-dash-carrying reason, all-built-none-launched shape, #7188 end-to-end). Mutation testing across 12 variants reported all killed. Test run: No real-scenario tmux test needed — the bug is in review-body composition (not interactive TUI), and the unit tests exercise the exact shapes observed in the public exhibits (#7188, #7166). 中文说明代码审查： 干净——未发现问题。 diff 在 测试构造良好——每个以其失败模式命名。报告 12 个突变全部击杀。 测试结果： 130 测试全绿（check-coverage 57 + compose-review 73）。 无需 tmux 真实场景测试——bug 在 review 正文拼装（非交互式 TUI），单元测试覆盖了公开展品中观测到的形状。 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Confidence: 5/5 — real bug with public exhibits, minimal structural fix that mirrors existing patterns, 130 tests green, no concerns at any stage. This is a clean, focused fix. The bug is real and publicly observable — review bodies with dozens of duplicate disclosure clauses burying actual findings. The solution is correct: move the subject/reason boundary from parsed prose (fragile, since reasons are free-form) to structural pairs at the source. The dedup logic is simple and mirrors the existing chunk-prefix pattern. Every change in the diff serves the stated goal — no scope creep, no drive-by refactors. The Independent proposal would have been: add structured entries at the coverage site, dedup caller echoes, group same-reason subjects. That's exactly what this PR does — no simpler path missed. LGTM. ✅ 中文说明信心度：5/5 —— 有公开展品的真实 bug，最小化结构修复且与既有模式一致，130 测试全绿，各阶段均无顾虑。 干净、聚焦的修复。bug 真实且可公开观测——review 正文中数十条重复披露淹没了真实 findings。方案正确：将 subject/reason 边界从解析 prose（脆弱，因 reason 是自由文本）移至源头的结构化对。去重逻辑简单且与既有 chunk 前缀模式一致。diff 中的每一处改动都服务于 stated 目标——无范围蔓延、无顺手重构。
独立方案会是：在 coverage 站点加结构化条目、caller 回抄去重、同因 subject 分组——与此 PR 完全一致，未发现更简路径。 可以合并。✅ — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
previously approved these changes
Jul 18, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
…ise text on collision Second exhibit, same wall: the review posted on #7166 was ninety-nine disclosure paragraphs over four causes -- forty-three chunks all launched with rewritten prompts, fifty-five roles built and never launched -- with the six real findings buried beneath. Neither #7190 mechanism touched it: nothing was duplicated, and the roster was not all-unlaunched (the chunk agents ran). - Explained disclosures now group by their reason text, the same last em-dash segment the dedup keys on: same cause, one sentence, every subject on it -- exactly as the bare names already grouped under the shared whiff sentence. A reason embedding per-subject detail (an unread brief's path) differs per entry and keeps its own line. - The dedup now prefers coverage-derived text over the caller's AND the earliest coverage category on a within-coverage collision: a rewritten chunk is also, to the roster, a requirement with no verbatim launch, and the later roster text told the author "no agent was launched" about an agent that demonstrably ran.
Collaborator
Author
|
Pushed a second commit ( Two additions:
Suite green (127 in the two touched files, full review dir green); mutations: precision-order flip and grouping removal each turn exactly their test red (8/8 total on this PR now). |
…the posted body "see the terminal output" pointed the PR author at a terminal only the operator has -- a dead reference that eight hours of real bot reviews carried onto five different pull requests. Say what happened and that nothing is owed, in the body's own register.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
wenshao
commented
Jul 19, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
Author
There was a problem hiding this comment.
— Codex GPT-5 via Qwen Code /review
…coverage-side collapse
Review follow-through on three confirmed Criticals, which converge on
one design:
- The all-built-none-launched collapse misfired on the exhibit it was
built for: candidatesOf is also all-empty when every agent ran on a
REWRITTEN prompt, so the aggregate claimed "the run stopped at the
prompt builder" beside forty-three rewritten-launch disclosures that
said otherwise. Removed -- and it also discarded the per-role subjects
before the caller's echoes could dedupe against them.
- The last-em-dash subject/reason split reparsed rendered prose, and a
reason is free-form: labels carry a dash for an invariant's file,
error interpolations carry anything. Coverage now hands compose the
entries as {subject, reason} pairs (a new `disclosures` field beside
the prose twins the stderr formatting keeps); the caller's entries are
never parsed at all -- they dedupe by prefix against known coverage
subjects, exactly as the chunk list already did, and render verbatim
otherwise.
- With subjects structural and per-role entries surviving to the dedup,
the same-reason grouping does the collapsing for every shape at once:
the #7188 wall, the #7166 wall, and the caller-echo variant all render
as one sentence per cause -- with each subject under its most precise
cause only.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
Collaborator
Author
|
@qwen-code /triage |
qwen-code-ci-bot
previously approved these changes
Jul 19, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Collaborator
|
Independent review — LGTM, one doc fix before merge. Traced the full One thing to fix before merge, plus a couple optional: 1. The PR description is out of sync with the shipped code. "Evidence (Before & After)" and Reviewer Test Plan item 2 still describe the coverage-side collapse that 65c9d20 removed — 2. (Optional) Each reason is now maintained in two places — the prose form (a 3. (Nit) Not blocking: the |
…ctural entry Review follow-through: every reason lived twice -- the prose push that feeds check-coverage's stderr and the disclose() that feeds the posted body -- and an edit to one and not the other would silently diverge what the operator reads from what the author is told. disclose() now returns the joined prose and the stderr arrays consume it, so there is one statement of each sentence. Plus the two nits: the needless covEntries copy, and a -1 guard on the verification-gap split (safe today by construction, latent tomorrow).
Collaborator
Author
|
All three taken, in
Thanks for tracing the caps/events/remediation preservation independently — and for the macOS |
wenshao
force-pushed
the
fix/compose-disclosure-wall
branch
from
July 19, 2026 03:35
84a4118 to
4b62bb6
Compare
Contributor
|
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）为单个提交。 |
doudouOUC
approved these changes
Jul 19, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Re-reviewed at 4b62bb6. Everything I flagged is addressed, and I confirmed the follow-up is behavior-preserving rather than merely plausible:
- Prose single-sourcing (the duplication I raised):
disclose()now returns${subject} — ${reason}and the stderr arrays consume its return value, so each sentence is stated once. I checked all six converted sites — rewritten (both variants), every-dimension, missing-role (no-record / not-launched / reached-only-credited), and unread-brief — and the joined prose is byte-identical to what check-coverage printed before. The pinned suites agree:compose-review.test.ts73 +check-coverage.test.ts57 green, and since this commit touches no test files, that green is the proof the output didn't move. - Nits:
covEntriesis now an alias (coverageEntriesis never mutated past that point, so it's safe), and the verification-gap split has both acut === -1guard and a matching empty-reason render path (Not reviewed: <subject>., no dangling— .). - Description: the Evidence / Test-Plan sections now describe the shipped grouped-by-cause output (
chunk 1, …, chunk 43 — … / <roles> — its prompt was built, but no agent … launched with it), not the removed coverage-side collapse. - The three Codex Criticals remain resolved; no unresolved threads.
The only red locally is test-efficacy (git-worktree-in-worktree + a macOS rmSync-on-symlink case) — environmental, reproduced identically before and after this commit; CI is green on Linux.
LGTM — approving.
Collaborator
Author
|
@qwen-code /triage |
qwen-code-ci-bot
approved these changes
Jul 19, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
This was referenced Jul 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
One disclosure per subject, one sentence per cause, in the register the reader can use — taken from two shipped review bodies (#7188: 22 duplicate clauses for 11 roles; #7166: 99 clauses over four causes, six real findings buried beneath).
compose-reviewits disclosures structurally (disclosures: {subject, reason}[], beside the prose twins the stderr formatting keeps; the prose is derived from the structural entry, so the two registers cannot diverge). Reparsing rendered prose for a subject/reason boundary was refused by review — reasons are free-form (invariant labels carry—for their file; error interpolations carry anything).unreviewedDimensionsentries are never parsed; they match known coverage subjects by prefix (exactly how the chunk list already dedupes) and render verbatim otherwise. On a collision the coverage text wins, and within coverage the earliest category wins — a chunk flaggedrewrittenmust not also read "no agent was launched" about an agent that demonstrably ran.Not reviewed: chunk 1, …, chunk 43 — launched with a prompt that is not the one the CLI built.— which collapses every wall shape at once (all-unlaunched, all-rewritten, caller-echo variants). An earlier coverage-side collapse was removed in review: it misfired on all-rewritten rosters and discarded the per-role subjects the dedup needs.Why it's needed
The two linked bodies are what an external contributor actually received when a review run failed: walls of operator-register process text burying the review itself. The delivery gates were right to disclose; the composition of the disclosure was the defect.
Reviewer Test Plan
How to verify
cd packages/cli && npx vitest run src/commands/review— 31 files, 746 tests green.terminal outputis banned from the body.Evidence (Before & After)
Before (#7166, verbatim): 99 paragraphs of
Not reviewed: chunk N — launched with a prompt that is not the one the CLI built./… — its prompt was built, but no agent on record was launched with it.After, same state: one sentence per cause —
Not reviewed: chunk 1, chunk 2, …, chunk 43 — launched with a prompt that is not the one the CLI built. Not reviewed: <55 roles> — its prompt was built, but no agent on record was launched with it.— and a caller echoing any of it adds nothing.Tested on
Risk & Scope
missingRoleSelectorsare unchanged (the cap andcanCertifysum both inlets, pinned by the suites).test-efficacylocal failures observed by a reviewer are pre-existingsafeRmWithinsymlink behavior, unrelated (Linux CI green).Linked Issues
Exhibits: the review bodies posted on #7188 and #7166.
中文说明
本 PR 做了什么
一个主题一条披露、一个原因一句话、且用读者能用的语域——取材自两条已发布的 review 正文(#7188:11 角色重复成 22 条;#7166:四种原因 99 条,6 条真 findings 被埋)。
disclosures: {subject, reason}[],prose 双胞胎保留给 stderr 且由结构化条目派生,两个语域不可能分叉)。从渲染文本反解析 subject/reason 边界被 review 否决——reason 是自由文本(invariant label 自带—文件段、错误插值什么都可能有)。unreviewedDimensions条目从不解析,按已知 coverage subject 前缀匹配(与 chunk 列表既有做法一致),否则原样渲染。碰撞时 coverage 文本胜出,coverage 内部最早类别胜出——被判rewritten的 chunk 不能同时被说成"无 agent 启动"。Not reviewed: chunk 1, …, chunk 43 — launched with a prompt that is not the one the CLI built.——一次性坍缩所有墙形(全未启动、全改写、回抄变体)。早先 coverage 侧的坍缩被 review 否决后移除:它在全改写 roster 上误触发,且丢弃了去重所需的 per-role subject。为什么需要
两条展品正文就是外部贡献者在 run 失败时真实收到的东西:操作语域的过程文本墙压过 review 本身。交付门披露是对的;披露的作文是缺陷。
验证方式
cd packages/cli && npx vitest run src/commands/review—— 31 文件 746 测试全绿。证据(前后对照)
修复前(#7166 原样):99 段
Not reviewed: chunk N — launched with a prompt that is not the one the CLI built./… — its prompt was built, but no agent on record was launched with it.修复后,同一状态:每原因一句——
Not reviewed: chunk 1, …, chunk 43 — <rewritten 原因>. Not reviewed: <55 角色> — its prompt was built, but no agent on record was launched with it.——caller 再回抄也不增行。风险与范围
missingRoleSelectors不变(cap 与canCertify对两个进水口求和,套件钉住)。test-efficacy失败是既有safeRmWithinsymlink 行为,与本 PR 无关(Linux CI 绿)。关联
展品:#7188 与 #7166 上发布的 review 正文。