feat(review): give the verifier a probe capability — run a runnable claim, don't just read it - #7756
Conversation
…laim, don't just read it Measured on this repo: read-only verification is where the review misses its hardest bugs. Handed the exact low-confidence finding for a `!` command that executes twice, the strongest model's Step-3 agents traced the mechanism and called it correct (0/3 across the full roster). Validated the fix directly — when the same models (3.7-max and 3.8-max-preview) were allowed to WRITE AND RUN a probe, both confirmed the double-execute from observed behaviour (`sendShellCommand called twice with ["git push"]`), both did the probe-validity self-check unprompted, and a plausible-but-false negative control was correctly refuted with no fabrication. So the verifier's brief now says: when a finding's failure scenario is a runnable claim about a named unit and the repo has a fast unit harness (vitest/jest/pytest), write a minimal probe, run it, and let the observed behaviour settle the verdict. Two rules keep it evidence rather than theatre — a mandatory self-check that the probe flips between buggy and correct, and leaving the tree exactly as found. A finding a probe confirmed carries `Source: [probe]`, which compose-review treats as deterministic (a run produced it) like `[build]`/`[test]`. This is Phase 1: the agent-driven loop the validation exercised, no new command. The deterministic runner + artifact and the finding-generation side (emitting a probeable low-confidence finding rather than concluding "correct") are follow-ups.
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Review: feat(review): give the verifier a probe capability
This is a well-designed feature that addresses a measured gap in read-only verification. The approach is sound and the implementation is clean.
✅ Strengths
-
Evidence-based design: The PR validates the problem (strongest model traced a real double-execute bug and called it correct 0/3 times) and validates the solution (probes confirmed the bug). This is excellent engineering.
-
Two-rule guardrails: The mandatory self-check (probe must flip) and tree restoration rules keep probes as evidence rather than "theatre". This prevents false positives from probes that don't actually distinguish buggy from correct.
-
Proper integration: Adding
[probe]to deterministic sources alongside[build]/[test]is the right abstraction. A probe-run finding has the same "run produced it" nature. -
Good test coverage: Tests verify both the capability presence and the isolation (doesn't bleed into dimension briefs). The compose-review test confirms deterministic handling.
-
Clear documentation: SKILL.md and the brief text explain the rules clearly.
🔶 Suggestions (non-blocking)
-
Security documentation: The brief explicitly says "run it in the worktree". While this matches Build/Test's risk tier, it would be worth documenting in SKILL.md that probes run with the same trust assumptions as existing build/test execution (i.e., PR code is untrusted and executed). Future phases with sandbox hardening will improve this.
-
Probe hygiene edge case: What happens if a probe crashes or times out? The brief says "fall back to the reading-based verdict" for un-runnable probes, which is correct, but explicitly mentioning cleanup on crash/timeouts could prevent stale probe files.
-
Minor: regex readability: The regex
/\[(?:build|test|probe)\]/iworks, but a comment or extracted constant likeDETERMINISTIC_SOURCES_PATTERNmight improve maintainability as this list grows.
✅ Checklist
- Code correctness: Logic is sound
- Tests: Adequate coverage for the new capability
- Documentation: SKILL.md and inline comments explain well
- Security: Risk tier matches existing Build/Test execution
- No breaking changes: Additive feature
Verdict
Approve — This is a thoughtful, measured addition that solves a real problem with appropriate guardrails. The measured validation gives confidence the approach works.
|
Thanks for the PR! Template: the headings differ from the template ( Problem: observed and measured, not theoretical. The PR describes a concrete case where read-only verification traced a real double-execute bug ( Direction: aligned. This improves the review command's verifier — the component whose job is exactly to catch what reading alone misses. The probe capability is scoped to the verifier's existing worktree+shell risk tier (same as Build & Test), and the PR is explicit about what's deferred (deterministic runner, finding-generation side). CHANGELOG: no direct reference, but the review command is actively developed and this is a natural capability extension. Size: core path touched ( Approach: the scope feels right — this is the minimal instruction + plumbing change. The probe instructions go into the verifier brief (where the verifier already runs with a shell), the 中文说明感谢贡献！ 模板：标题与模板不同（ 问题：已观测且有实测数据，非理论性加固。PR 描述了一个具体案例：只读验证追踪了一个真实的双执行 bug（ 方向：对齐。这改进了 review 命令的 verifier——其职责恰恰是捕捉仅靠阅读会遗漏的问题。probe 能力限定在 verifier 已有的 worktree+shell 风险层级（与 Build & Test 相同），PR 明确说明了延后的部分（确定性运行器、finding 生成侧）。 规模：触及核心路径（ 方案：范围恰当——这是最小的指令+管道变更。无无关改动，无范围蔓延。进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: given the problem (verifier misses bugs that running code would catch), I would (1) add probe instructions to the verifier brief — when a finding is a runnable claim about a named unit, the repo has a fast test harness, and reading hasn't settled it, write and run a minimal probe; (2) require a self-check that the probe flips between buggy and correct, so it's evidence not noise; (3) add Findings: no blockers. The diff is clean and every change earns its place:
Reuse check: the change extends the existing TestingThis is a CI run — PR code is not executed here. Evidence comes from the PR's own CI checks. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The main unit suite ( Real-scenario testing: N/A — this is a review-infrastructure change (agent prompt text + compose-review regex), not user-visible TUI behavior. 中文说明代码审查：无阻塞项。diff 干净，每处改动都有必要：
测试：CI 运行中，不执行 PR 代码。主要单测套件（ubuntu）仍在运行。作者报告 918 项测试通过、ESLint 和 tsc 干净——这是作者的声明，CI 将确认。 真实场景测试：不适用——这是 review 基础设施变更（agent 提示词 + compose-review 正则），非用户可见 TUI 行为。 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean across every stage; the change is minimal, well-motivated by measured evidence, and does exactly one thing. This is a tight, well-evidenced capability addition. The problem is real and measured — the strongest model traced a genuine double-execute bug and called it correct, and the same model caught it immediately when allowed to run a probe. The implementation is the minimum viable version: one paragraph of verifier instructions, one regex alternation, one doc line, two focused tests. No new abstractions, no scope creep, no unrelated edits. My independent proposal matched the PR's approach exactly — I didn't find a simpler path it missed. The probe instructions are well-specified (the self-check/flip rule is what keeps it evidence rather than theatre), the If I had to maintain this in six months, I'd thank the author — the code is self-documenting, the tests explain why they exist, and the change is easy to revert if the probe approach doesn't pan out. Approval deferred until CI lands green on 中文说明这是一个紧凑、有实测证据支撑的能力增强。问题真实且有度量——最强模型追踪了一个真实的双执行 bug 却判为正确，同一模型在允许运行 probe 后立即发现了它。实现是最小可行版本：一段 verifier 指令、一个正则交替项、一行文档、两个聚焦测试。无新抽象、无范围蔓延、无无关编辑。 独立提案与 PR 方案完全一致——没有找到更简路径。probe 指令规格清晰（自校验/翻转规则使其成为证据而非表演）， 审批推迟至 CI 在 — Qwen Code · qwen3.8-max-preview Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
Reconcile compose-review with QwenLM#7756 (probe-verify): the deterministic regex keeps `probe` and my provenance count-tracking (build/test/probe are trusted tags; `[lint]` is not — the gate's own findings are deterministic by provenance). Keep both the '[lint] is not deterministic' and '[probe] is deterministic' tests.
|
Released in v0.21.1. |
|
|
3 participants
What & why
Read-only verification is where this review misses its hardest bugs, and that isn't a guess — it's measured. Handed the exact low-confidence finding for a
!command that executes twice in a new task, the strongest model's full Step-3 roster (with worktree access) traced the mechanism and called it correct — 0 of 3, one attacker persona walking right into it.Then I validated the fix directly. When the same models were allowed to WRITE AND RUN a probe instead of only reasoning:
!double-execute)sendShellCommand called twice with ["git push"]; self-check: guard→1, revert→2createSession called once, no fabricationBoth models confirmed the bug they had missed read-only, both did the probe-validity self-check unprompted, and the plausible-but-false control was correctly refuted. The ceiling is real and bidirectional.
How it works
This is the verifier's brief gaining one capability. When a finding's failure scenario is a runnable claim about a named unit (a function, a component, a route) and the repo has a fast unit harness (
vitest/jest/pytest) and tracing-by-reading hasn't settled it, the verifier may write a probe — a minimal test that reproduces the scenario and records what actually happens — and run it. Two rules keep it evidence and not theatre:A finding a probe confirmed carries
Source: [probe], whichcompose-reviewnow treats as deterministic (a run produced it) like[build]/[test]— so it is never softened for a verification it never owed.Scope
Phase 1: the agent-driven loop the validation exercised — no new command. The verifier already runs in the worktree with a shell (same risk tier as Build & Test, which builds and tests PR code there today); this gives it the instruction to probe. Deliberately deferred: a deterministic
qwen review proberunner + artifact (sandbox hardening, proof-of-execution for compose-review), and the finding-generation side — getting an agent to emit a probeable low-confidence finding rather than concluding "correct", the one gap the validation did not cover.Independent of the
script-lintPRs (#7749/#7751); branches off currentmain.Validation
/reviewcommand suite green (918 tests); ESLint clean at--max-warnings 0;tsc --noEmitclean over the review tree.[probe]body Critical is deterministic incompose-review(stands without a separate verifier).中文说明
背景与动机
只读式验证正是本评审漏掉最难那类 bug 的地方——这不是猜测,是实测出来的。把"新任务里
!命令执行两次"这条低置信发现原样交给最强模型的全 Step-3 roster(带 worktree),它把机制走了一遍却判成"correct"——3 个里 0 个,攻击者视角还径直走到 bug 面前。随后我直接验证了这个补法:给同一批模型"能写+跑 probe"的能力(而非只推理):
!双执行)sendShellCommand 被调用两次,参数 ["git push"];自校验:加守卫→1、revert→2createSession 只调 1 次,无编造两个模型都确认了它们只读下漏掉的 bug,都未经提示就做了 probe 有效性自校验,似真实假的负控被正确推翻。天花板真实、且双向。
实现方式
这是给 verifier 的 brief 增加一项能力。当一条发现的 failure scenario 是关于具名单元的可运行命题(函数/组件/路由),且仓库有快速单测 harness(
vitest/jest/pytest),且只读追踪未能定论时,verifier 可以写一个 probe(最小复现、记录真实发生了什么)并运行。两条硬规则让它是证据而非表演:被 probe 确认的发现带
Source: [probe],compose-review现在把它当作确定性(由一次运行产生),与[build]/[test]同级——因此不会因"没跑本不该跑的验证"而被降级。范围
Phase 1:验证所跑的那个 agent 驱动闭环——不新增命令。 verifier 本就在 worktree 里带 shell 运行(与 Build & Test 同一风险级别,后者今天就在那里构建并测试 PR 代码);本 PR 给它"去 probe"的指令。刻意延后:确定性
qwen review probe运行器 + 产物(沙箱加固、给 compose-review 的执行证明),以及 finding-generation 侧——让 agent产出一条可 probe 的低置信发现,而不是直接判"correct"(验证唯一没覆盖的那一环)。与
script-lint两个 PR(#7749/#7751)相互独立;基于当前main。验证
/review命令全量测试通过(918 项);ESLint 在--max-warnings 0下无告警;review 目录tsc --noEmit干净。compose-review对[probe]body Critical 的确定性处理(无独立 verifier 也成立)。