feat(review): fold the findings list into the verify/reverse-audit prompt - #6994
Merged
Conversation
…t can't be paraphrased Dogfooding the merged Step 4/5 coverage gate (#6965) on a real 3A review turned up the weakness it was built to expose. The orchestrator delivered every Step 3 prompt verbatim — but PARAPHRASED the Step 4 (verify) and Step 5 (reverse-audit) ones: it added a round number, inserted its own one-line summary, and truncated the recorded line telling the agent the brief is the whole of its instructions. The gate caught it (the launch prompt was not the recorded one) and capped the verdict, even though the agents had opened their briefs and done real work. The root cause is the hand-assembly step. Step 3 is a clean paste; Step 4/5 told the orchestrator to "paste verbatim AND prepend the findings list", and constructing that prepend is where the wording drifted. So this removes the assembly step: `agent-prompt --role verify|reverse-audit --findings <file>` folds the list in and prints one block to paste. There is nothing left to hand-build, so nothing to reword. The findings are printed, not recorded. The record stays the findings-free launch block, so the per-shard (verify) / per-round (reverse-audit) key keeps matching by the add-only delivery rule — a printed `<findings>\n\n<block>` still contains the recorded `<block>` in order — without baking a different findings list into each record. The folded section restates that the brief is authoritative, which is the exact sentence the hand-assembly used to truncate. `--findings` is declared on the brief (`acceptsFindings`, like `acceptsChunk`) and rejected on any other role, on a bare chunk agent, and on `--whole-diff`. SKILL Step 4/5 now write the findings to a file, pass `--findings`, and paste the whole block verbatim — no round number, no rewording. Tests: the verifier/auditor get the findings folded above a findings-free record that still passes the delivery check; an empty findings file tells a first-round auditor nothing is confirmed yet; the record is byte-identical with or without `--findings` (the shared key is unaffected); and the guard rejects the flag everywhere it does not belong.
Collaborator
|
Thanks for the PR! (Re-run — refreshing all stages.) Template looks good ✓ Problem: Observed, with concrete evidence. Transcript forensics from a real 3A review showed Step 4/5 launch prompts matched the CLI-recorded block 0/5 times, while all 13 Step 3 agents matched verbatim. The root cause is identified precisely: the one place the skill still had the model assemble a prompt by hand (prepending the findings list) is exactly where the paraphrase drift crept in — a round number added, a summary inserted, the "nothing replaces the brief" line truncated. Before/after: 0/5 → 3/3 verbatim delivery. Direction: Aligned. This is internal infrastructure for the Size: 142 production lines ( Approach: Minimal and well-scoped. One new Moving on to code review. 🔍 中文说明感谢贡献！（重新运行 — 刷新所有阶段。） 模板完整 ✓ 问题：已观测到，有具体证据。 真实 3A review 的 transcript 取证显示 Step 4/5 的 launch prompt 与 CLI 录制块匹配 0/5 次，而 13 个 Step 3 agent 全部逐字匹配。根因定位精确：skill 里唯一还让模型手工拼接 prompt 的地方（在上方前置 findings 列表）正是改写漂移的入口——加了 round number、插了摘要、截断了「nothing replaces the brief」那句。前后对比：0/5 → 3/3 逐字交付。 方向：对齐。这是 规模：142 行生产代码（ 方案：最小且范围明确。 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Collaborator
|
Code review: No issues found. The implementation is clean and correct. Key design decisions are sound: Tests: 620 passing across 29 files in Real-scenario testing (bundled CLI): All three guard combinations produce the correct error messages. The 中文说明代码审查： 未发现问题。实现干净且正确。 关键设计决策合理： 测试： 真实场景测试（打包后的 CLI）： 三种 guard 组合均产生正确错误消息。 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Confidence: 5/5 — Clean across every stage; a focused, well-tested fix for an observed prompt-delivery failure. This PR solves a real, measured problem: the last hand-assembly step in the review skill's Step 4/5 prompt delivery was exactly where the orchestrator paraphrased the prompt — 0/5 verbatim on a real dogfood run, while Step 3 (which had already been moved into the command by #6892) matched 13/13. The fix follows the same pattern that worked for Step 3: move the variable content into the command, print one block, leave no assembly step to drift. The implementation is tight — 142 production lines, all in the review command. If I had to maintain this in six months, I'd thank the author — the comments explain the why at the decision points, the code follows the exact same patterns as the Step 3 fix, and the tests document the contract. 中文说明置信度：5/5 — 各阶段均干净；针对已观测到的 prompt 交付失败的一个聚焦、测试充分的修复。 这个 PR 解决了一个真实的、可度量的问题：review skill Step 4/5 prompt 交付中最后的手工拼接步骤正是 orchestrator 改写 prompt 的入口——真实 dogfood 运行中 0/5 逐字匹配，而 Step 3（#6892 已搬进命令）13/13 匹配。修复遵循了 Step 3 成功的同一模式：把可变内容搬进命令、打印一块、不留手工拼接的步骤。 实现紧凑——142 行生产代码，全在 review 命令里。 如果六个月后需要维护这段代码，我会感谢作者——注释在决策点解释了 为什么，代码遵循与 Step 3 修复完全相同的模式，测试记录了契约。 — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
previously approved these changes
Jul 16, 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.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
…s, test verify-empty
Three findings from the /review pass over this PR.
The "--findings with no role" error hardcoded `--role verify / --role reverse-audit`,
while the wrong-role branch two above it derives the list from `acceptsFindings`. A
future role with that flag would leave this one telling a stale story. It now derives
the same way (the `--chunk` guard already set this precedent).
The `--findings` test block created temp dirs and never removed them — the one block
in the file without try/finally cleanup. It now tracks each dir and sweeps them in an
`afterEach`.
`findingsSection('verify', '')` — a verifier handed an empty findings file — had no
test; only the reverse auditor's empty branch did. Added the symmetric case: it prints
"nothing to verify", not the auditor's "nothing is confirmed yet".
Collaborator
Author
|
Thanks — the bot approved, and all three inline Suggestions are addressed in 9b51145.
618 review tests pass (617→618), review |
wenshao
commented
Jul 16, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
Author
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
— qwen-latest-series-invite-beta-v77 via Qwen Code /review
Collaborator
Author
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: chunk 1, chunk 2 — no agent reported covering these; nobody read them.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
previously approved these changes
Jul 16, 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. ✅
…3B path The one valid flag triple was untested: reverse-audit declares both acceptsChunk and acceptsFindings, and Step 5 3B launches `--role reverse-audit --chunk N --findings <cumulative>` per chunk per round — the real production path. Added a case: the findings fold above the chunk-scoped prompt, the printed prompt reads that chunk's range (not another's), and the record is that chunk's block, findings-free, keyed `reverse-audit--chunk-N`, and still passes the delivery check. The code was already correct; nothing failed. Now it is pinned.
Collaborator
Author
|
@qwen-code /triage |
qwen-code-ci-bot
previously approved these changes
Jul 16, 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.
Reviewed. Suggestions are inline. Not reviewed: chunk 1, chunk 2 — no agent reported covering these; nobody read them.
— qwen3.7-max via Qwen Code /review
…'s framing The verify and reverse-audit non-empty tests each asserted their own framing heading but not the absence of the other's, so a swap of the two branches in findingsSection would have passed both. Added the mirror assertions: the verifier's prompt must not say "Already confirmed", and the auditor's must not say "The findings you are ruling on". The swap is now caught.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: chunk 1, chunk 2 — no agent reported covering these; nobody read them.
— qwen3.7-max via Qwen Code /review
findingsSection matched `verify` and fell through to the reverse auditor's framing for everything else. A future role that sets acceptsFindings would pass every guard, reach here, and silently receive "Already confirmed — do not re-report" — wrong for any role not hunting gaps. The branches are now explicit (`verify`, `reverse-audit`) with a throwing default, the same reasoning the no-role guard message already follows. Exported and tested: a role it does not frame throws by name.
yiliang114
approved these changes
Jul 16, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. The --findings option, guard chain, and findingsSection branching all look correct. The wasDeliveredVerbatim subsequence check properly handles the prepended findings. Test coverage is thorough.
Collaborator
Author
|
@qwen-code /triage |
qwen-code-ci-bot
approved these changes
Jul 16, 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. ✅
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
Moves the Step 4 (verify) and Step 5 (reverse audit) findings list into the command that builds their prompt, so the orchestrator pastes one block instead of hand-assembling it.
qwen review agent-prompt --role verify|reverse-audit --findings <file>folds the list into what it prints — the verifier gets the shard it rules on, the reverse auditor gets the cumulative confirmed list — and the caller pastes the whole thing verbatim, with nothing to add. The findings are printed but not recorded: the record stays the findings-free launch block, so the per-shard verify key and per-round reverse-audit key still match by the add-only delivery rule (two verify shards record the same block).SKILL.mdStep 4/5 now write the findings to a file, pass--findings, and paste the block verbatim — no round number, no summary, no rewording.Why it's needed
Follow-up to #6892 and #6965. #6965 built a gate that proves Step 4/5 agents ran and read their briefs, from the harness transcripts. Dogfooding the merged chain on a real 3A review surfaced the reason that gate keeps firing: the orchestrator delivered Step 3 prompts verbatim but paraphrased the Step 4/5 ones — it added a round number, inserted its own summary of the diff, and truncated the line that says the brief is the whole of the instructions. Transcript forensics: verify + reverse-audit launch prompts matched the CLI-recorded block 0 of 5 times, while the 13 Step 3 agents matched. The root cause is the one place the skill still had the model assemble a prompt by hand — "paste what the command prints, and add the findings list above it." That hand-assembly is where the drift got in, exactly the failure moving Step 3's prompt into code fixed. So the findings move into the command too, and there is no assembly step left to drift.
Reviewer Test Plan
How to verify
cd packages/cli && npx vitest run src/commands/review→ 617 passing (29 files);npx tsc --noEmit -p packages/cli/tsconfig.jsonfiltered tocommands/review→ clean. The new behavior is fixture-driven: the verifier and auditor get the list folded above; the record is findings-free and byte-identical with or without--findings; the folded prompt still passeswasDeliveredVerbatimagainst the record; an empty file tells the auditor nothing is confirmed yet; an unreadable file is named, not swallowed; and--findingsis rejected on a dimension role, with no role, and with--whole-diff.Evidence (Before & After)
Non-UI change (a CLI subcommand and skill text). Validated end-to-end by re-running
/review 6981with the built CLI and reading the harness transcripts:--findings)--findings)--findingsThe orchestrator adopted the flag, stopped paraphrasing, and the Step 4/5 gate correctly stopped firing.
Tested on
/reviewdogfood)Environment (optional)
Linux; repo-built CLI (
npm run build && npm run bundle) driven under tmux against PR #6981 with the defaultqwen3.7-max. Unit suite vianpx vitest.Risk & Scope
/reviewskill — one new option onagent-promptplus a print/record split, andSKILL.mdStep 4/5. The delivery record and the Step 4/5 coverage floor are unchanged: the record is still the findings-free launch block, so#6965's gate keeps working without modification.--findingsis additive and optional; a call that omits it behaves exactly as before.0/5 → 3/3verbatim metric is the deterministic property it fixes.Linked Issues
Follow-up to #6892 and #6965 (no issue to close).
中文说明
这个 PR 做了什么
把 Step 4(verify)和 Step 5(reverse audit)的 findings 列表移进构建其 prompt 的命令里,让 orchestrator 只粘贴一份、而非手工拼接。
qwen review agent-prompt --role verify|reverse-audit --findings <file>把列表折进打印内容——verifier 拿到本分片要裁决的 findings,reverse auditor 拿到累积确认列表——调用方逐字粘贴整块、无需添加任何东西。findings 打印但不记录:record 保持 findings-free 的 launch block,所以 verify 的 per-shard key 和 reverse-audit 的 per-round key 仍靠 add-only 交付规则匹配(两个 verify 分片记录同一块)。SKILL.mdStep 4/5 改为:把 findings 写到文件、传--findings、逐字粘贴整块——不加 round number、不加摘要、不改写。为什么需要
#6892、#6965 的后续。#6965 建了一个 gate,从 harness transcript 证明 Step 4/5 agent 跑了、读了 brief。在真实 3A review 上 dogfood 合入链时,发现了这个 gate 反复触发的根因:orchestrator 逐字交付了 Step 3 的 prompt,却改写了 Step 4/5 的——加了 round number、插入自己对 diff 的摘要、截断了「brief 是全部指令」那句。transcript 取证:verify + reverse-audit 的 launch prompt 与 CLI 录制块匹配 0/5 次,而 13 个 Step 3 agent 都匹配。根因是 skill 里唯一还让模型手工拼接 prompt 的地方——「粘贴命令输出,并在上方加 findings 列表」。这个手工拼接就是漂移的入口,正是把 Step 3 prompt 搬进代码修掉的那类失败。所以 findings 也搬进命令,不再留手工拼接的步骤。
Reviewer 测试计划
如何验证
cd packages/cli && npx vitest run src/commands/review→ 617 通过(29 文件);npx tsc --noEmit -p packages/cli/tsconfig.json过滤到commands/review→ 干净。新行为由 fixture 驱动:verifier/auditor 拿到折在上方的列表;record 是 findings-free 且传不传--findings都字节相同;折叠后的 prompt 仍能通过wasDeliveredVerbatim对 record 的校验;空文件告诉 auditor 尚无确认;不可读文件按名报错、不吞掉;--findings在维度角色、无角色、--whole-diff上都被拒。证据(前后对比)
非 UI 改动(CLI 子命令 + skill 文本)。用构建的 CLI 重跑
/review 6981、读 harness transcript 做端到端验证:--findings)--findings)--findingsorchestrator 采用了该 flag、停止改写,Step 4/5 gate 正确地不再触发。
测试平台
/reviewdogfood)环境(可选)
Linux;仓库构建的 CLI(
npm run build && npm run bundle)在 tmux 下对 PR #6981 运行,默认qwen3.7-max。单测npx vitest。风险与范围
/reviewskill——agent-prompt一个新选项 + 打印/记录拆分,以及SKILL.mdStep 4/5。交付 record 与 Step 4/5 覆盖 floor 不变:record 仍是 findings-free 的 launch block,所以 feat(review): prove Step 4 (verify) and Step 5 (reverse audit) actually ran #6965 的 gate 无需改动照常工作。--findings是增量、可选;不传它的调用行为与之前完全一致。0/5 → 3/3的 verbatim 指标才是它修复的确定性属性。关联 Issue
#6892、#6965 的后续(无需关闭的 issue)。