fix(review): a mutant whose own test was red is not a survivor either - #8345
Merged
Conversation
Found by running /review against PR #8213. Six hunks in `packages/acp-bridge/src/bridge.ts` were correctly held at `inconclusive` because `bridge.test.ts` never ran green in the unmutated baseline, while eight mutants in the SAME file were scored `survived` and shipped as findings on the strength of other files' tests passing. A mutant runs against `greenProbes` only, so a red collocated test is excluded from the run - and "every affected test still passed" is then computed over a set that omits the one test most likely to catch the deletion. That is exactly the inference the hunk loop refuses, in its own words: the other probes passing shows only that THEY do not cover it, not that nothing does. The comment under the hunk guard shows how the gap survived review: it framed the asymmetry as "mutants guard the killed direction, hunks guard the survived one". True of an inconclusive RUN, and it left a mutant's survived direction unguarded against an absent covering test. Two separate guards, one of which was read as the whole rule. Checked before the budget, so a candidate that cannot yield a verdict does not spend a suite run to say so. The regression test carries both halves - a file whose own test is red, and one whose own test is green - because a guard that swept up the second would be no better.
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
Collaborator
|
Thanks for the PR! Template looks good ✓ — the "why" is folded into "What this PR does" rather than its own heading, but everything substantive is present: motivation, mechanism, test plan, before/after evidence, risk notes, and the Chinese translation. Problem: observed, not theoretical. Dogfooding Direction: aligned — a correctness fix to the repo's own Size: not applicable — Approach: the scope is exactly right. The fix reuses the hunk loop's existing collocated-probe guard ( Risk: no elevated risk signals — no high-risk paths matched. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— "why" 并入了 "What this PR does" 而非独立标题，但实质内容齐全：动机、机制、测试计划、前后证据、风险说明和中文翻译。 问题：已观测到，而非理论性的。对 #8213 实跑 方向：对齐 —— 这是对仓库自身 规模：不适用 —— 方案：范围恰到好处。修复复用了 hunk 循环现有的同位探针守卫（ 风险：无升级风险信号 —— 未命中高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
Code reviewMy independent proposal before reading the diff: mirror the hunk loop's collocated-probe guard into the mutant loop, hold the candidate at
One non-blocking nit: the comment above the positive-control section (~line 1970) says the hunk loop pushes collocated-probe inconclusives before its budget check "so its own figure excludes them and this one did not" — "this one did not" is now stale, since the mutant loop does too after this change. The hunk-loop comment got updated; this one slipped through. Worth a one-line touch-up in passing. Test evidence (PR's own CI)The ubuntu unit suite is still running on the reviewed commit; macOS/Windows and integration checks are skipped by workflow design. Not verified: the suite result itself — quoted below as-is, no polling; the finalize job updates this table when CI settles. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The central claim is pinned by the new integration test itself — it runs the real handler end-to-end and, per the PR, fails with 中文说明代码审查读 diff 之前我独立提出的方案相同：把 hunk 循环的同位探针守卫镜像到变异体循环，在候选花掉一次套件运行之前将其判为
一个非阻塞的小问题：阳性对照段上方的注释（约 1970 行）说 hunk 循环在预算检查前 push 同位探针 inconclusive，"因此它自己的数字排除了它们，而这个循环没有" —— "这个循环没有"现在已经过时，因为本次改动后变异体循环也这样做了。hunk 循环的注释更新了，这条漏掉了。顺手改一行即可。 测试证据（PR 自身 CI）ubuntu 单元套件在被审 commit 上仍在运行；macOS/Windows 与集成检查按工作流设计被跳过。未验证：套件结果本身 —— 如下原样引用，不做轮询；CI 落定后由 finalize 任务更新此表。 核心结论由新增的集成测试本身钉住 —— 它端到端运行真实 handler，据 PR 所述，若删除守卫会以 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
|
Confidence: 4/5 — clean, minimal fix that closes a demonstrated gap; only nit is the one stale comment noted above. Stepping back: the problem was observed live — eight mutant survivors shipped as findings on #8213 while the hunk loop correctly held the same file — and the fix is the smallest possible one. The guard already existed next door; this just applies it. It is strictly silence-biased, so the worst case is an inconclusive where there might have been a false survivor, never a new false finding. The regression test is the good kind: structured to fail against the unpatched code with the exact sentence the live run produced, and it carries a green-own-test file to prove the guard doesn't over-reach. My independent proposal before reading the diff was identical, which is about as strong a match as that exercise gets. The one reservation is cosmetic: the budget comment that now misdescribes the mutant loop. Not worth holding the PR over. Approval deferred until CI lands green on 中文说明置信度：4/5 —— 干净、最小的修复，堵上了一个已被实证的缺口；唯一的小问题是上面提到的那条过时注释。 退一步看：问题是线上观测到的 —— #8213 上 8 个变异体 survivor 作为 finding 发出，而 hunk 循环对同一文件正确拦下 —— 修复也是最小可行的。守卫隔壁就有，这里只是套用。它严格偏静默，最坏情况是把一个虚假的 survivor 变成 inconclusive，绝不会产生新的虚假 finding。回归测试是好的那种：其构造使得对未打补丁的代码会以实跑产生的原句变红，并带了一个自身测试为绿的文件来证明守卫没有扩大化。我在读 diff 之前独立提出的方案与之完全一致。 唯一的保留意见是表面性的：预算注释现在对变异体循环的描述有误。不值得为此扣住 PR。 批准推迟到 CI 在 — Qwen Code · qwen3.8-max-preview 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. |
qwen-code-ci-bot
approved these changes
Aug 1, 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 — CI landed green after the review. ✅
qwen-code-ci-bot
approved these changes
Aug 2, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM！✅
— qwen3.8-max-preview via Qwen Code /review
doudouOUC
approved these changes
Aug 2, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed at 0b18aae. The diagnosis holds and the fix is the minimal shape of it: the guard reuses the existing exported collocatedProbe rather than reimplementing the stem match, so the mutant loop now has exactly the same reach as the hunk loop — including the same limits (file.test.ts/file.spec.ts beside the file; __tests__/ and file.integration.test.ts are outside the helper's contract in both loops). Symmetry, not a second heuristic that can drift.
The correctness question for a guard like this is whether it over-suppresses, and the integration test answers it with a negative control rather than only the positive case: alongside f.ts (red collocated test → mutants held inconclusive, detail naming f.test.ts and "did not run green"), it asserts forG.every((m) => m.verdict !== 'inconclusive') for g.ts, whose collocated test is green. So the guard is file-scoped and a red probe in one file cannot sweep up another's mutants. It drives the real runHandler against a real git worktree add tree, not a stub.
I checked the two integration points the guard could have disturbed. Findings: only verdict === 'survived' becomes a mutant-survived finding, so every candidate this guard moves off survived removes a false finding and adds nothing actionable in its place. Budget accounting: guard-skipped candidates are pushed into mutantResults before the continue, so mutantsSkippedForBudget = candidates.length - mutantResults.length still counts only the genuinely unprocessed tail — they are reported as inconclusive with a reason, not silently dropped, and they do not inflate the budget-skip count.
One consequence worth stating plainly, since the new comment's justification glosses it: because the guard continues before runOneMutant, these candidates can no longer be scored killed either — previously a mutant in such a file could still be killed by some other green probe. The comment says "a candidate that cannot yield a verdict should not spend a suite run to say so", but killed was a verdict it could have yielded. The trade is right — killed is not actionable, and in the measured case it buys back eight suite runs for candidates that can produce a real survivor — I would just not describe it as yielding no verdict.
Ran the touched suites locally at this HEAD: integration 26/26 and the new case passes; unit 133/134. The one failure is safeRmWithin > unlinks a symlink that is itself the target — grep-confirmed zero hits in this diff, and it fails identically on origin/main on this machine (macOS symlink semantics), so it is inherited, not introduced. CI is fully green.
This was referenced Aug 2, 2026
Collaborator
|
Released in v0.21.4. |
pchaganti
pushed a commit
to pchaganti/qx-qwen-code
that referenced
this pull request
Aug 7, 2026
…QwenLM#8390) * feat(review): say so when the bundle is older than the review it runs Every `qwen review …` step runs the BUILT bundle, not the working tree. So editing a review command, or switching to a branch that contains one, changes nothing about the run until someone rebuilds -- and the failure is silent and total: the run behaves like the last build, and every conclusion drawn from it is a conclusion about that build. Measured on 2026-08-02, dogfooding /review against QwenLM#8368 from a checkout whose bundle was fourteen hours old. Three things were invalidated at once and none announced itself: `drive` and `mock-provider` had merged that morning and were absent from the binary, so "the agent never reached for them" measured nothing; and QwenLM#8345's guard against scoring a mutant `survived` when its own collocated test was red had merged too, so the run reproduced the bug it fixed and filed three findings the current code holds as `inconclusive`. The round was discarded and re-run after a rebuild. `parse-args` is the first command of every review, which makes it the only place a notice reaches a reader before they act on a result. It names the file that is ahead, by how much, what actually runs from the bundle, and the command to rebuild -- "rebuild" without evidence is advice nobody can check. mtime, not git: the question is whether this bundle was built from this source, and a git comparison answers a different one. A margin absorbs a checkout, which writes everything at once in no guaranteed order. An installed package has no sources beside it, finds nothing to compare, and stays silent -- a check that cannot see the files must not accuse the build. Also documents `findings --test-delta` for users: it can lower a severity, and therefore change what the verdict is computed from, so it belongs beside `--outcomes` rather than only in the skill. * fix(review): watch the file every subcommand is registered in `packages/cli/src/commands/review.ts` is where all 30-odd subcommands are imported and registered, and it sits beside the directory rather than in it -- so a new command, or a changed dispatch, was exactly the change this check could not see. A root may now be a single file, which is what that one is. Confirmed end to end: with `review.ts` three hours ahead of a fresh bundle, the warning names it. Also two comments that did not match the code: symlinks of every kind are skipped, not only directories (`isFile()` is false for a symlinked file too), and the module now says what `QWEN_CODE_CLI` already covers -- talking to a different program -- so it is clear this guards the other half, the right program built before the change. * fix(review): compare content, because a timestamp check cried wolf The first version compared the bundle's mtime against the newest review source, and it was wrong in the direction that matters most. `git checkout` rewrites every file that differs between two commits, so returning to the branch a bundle was built from re-stamps exactly those files and the check calls a byte-for-byte correct bundle stale. Measured: with the sources untouched and the bundle two minutes older, it warned. A line that fires when nothing is wrong teaches its reader to skip the line, which would have made this worse than absent. The build now stamps a digest of the review sources it bundled into `dist/review-sources.sha256`, and the check re-derives that digest from the tree and compares. No margin to tune, no clock to trust, and no answer but the true one. Verified end to end across all five cases: a clean tree is silent, a source touched but unchanged is silent, and a real change under any of the three roots -- the command directory, the `review.ts` that registers them, the bundled skill -- warns. The digest is now one rule stated twice, since the build script cannot import the package it runs before building. `scripts/tests/review-source-digest.test.ts` holds the two equal, on this repo and on a synthetic tree that exercises the file-shaped root; a package test may not reach into `scripts/`, so it lives on the side of the boundary that may. Paths are folded relative to the repo root with separators normalised, and the file list is sorted -- `readdir` order is a property of the filesystem, so without it a bundle built in CI and a tree cloned locally would hash the same source differently and every run would warn. * fix(review): a diagnostic must not kill the run, and tests are not the bundle Two Criticals and five suggestions from review, all verified before changing anything. `writeStderrLine` throws on EPIPE, so stderr piped to `head` would have killed the review before it parsed a single argument -- a warning that destroys the run it was warning about, and the opposite of this change's own invariant. `writeStderrLineSafe` is the convention for diagnostics in this subsystem and is what it calls now. `reviewSourceRoots` builds paths with the platform `join`, and the test asserted forward-slash literals, so all three elements would have failed on the merge queue's Windows leg -- which the pull_request event never runs, so the green CI here proved nothing about it. Test files left the digest. esbuild follows imports from the CLI entry and no test is reachable that way, so folding them in fired the warning for an edit that cannot change a byte of the bundle -- the false positive this module already rejected once. 112 files became 61, and a test-only edit is now silent while a production one still warns. The handler wiring is tested at last, against a real temp tree rather than a mock of the reads under test: the derivation from `process.argv[1]`, the stamp read, and the warning. All three mutations the review named -- dropping the call, reading the stamp from the wrong directory, collapsing repoRoot to distDir -- now redden it. Also: the stamp's filename is pinned across the boundary it crosses (the build wrote a literal while the check read `DIGEST_FILE`, so a one-sided rename would have silenced the feature with every test green); the digest is computed only when there is a stamp to compare it against, instead of hashing a hundred files for a value the first guard discards; the `rebuildCommand` parameter no caller ever set is gone; and the build script's comment no longer claims a code-sharing relationship that does not exist. * fix(review): fixtures are not in the bundle either The same false positive, a third time and one directory over. Excluding tests from the digest was right and incomplete: `review/__fixtures__` holds four files — three responder modules and a captured comment — that a test loads at runtime, from no import the bundler follows. Measured against `dist`: none of the four appears in it, so editing one changed the digest while the bundle stayed byte-identical and the warning claimed a review command had changed. Both walks skip the directory now, and the parity test's synthetic tree grows a fixture and a `.spec.tsx` so the two implementations are held equal on the whole exclusion, not just the part the first case exercised. Reverting one side reddens the local case AND both parity cases, which is what that guard is for. Verified the other direction too, since an exclusion can overshoot: every review source that reaches `dist` is still covered. `DESIGN.md` and `SKILL.md` both ship and both remain in the digest — checked, not assumed, after two rounds of this exact mistake. Six cases end to end after a rebuild: a clean tree, a test edit and a fixture edit are silent; a production edit, a `review.ts` edit and a `DESIGN.md` edit each warn. * fix(review): allowlist the stamp, and stop guessing what the bundle holds The Critical first: `create-standalone-package.js` fails on any top-level dist entry outside its allowlist, and `review-sources.sha256` was on neither list. The next release would have aborted the standalone archive on all five targets, and no PR-time job runs the packager, which is why this suite is green. Allowlisted -- shipping it is harmless, since a standalone install has no `packages/` to compare against and the check stays silent there. `lib/test-utils.ts` was in the digest: test support with a production-looking name, imported by two test files and nothing else. That is the fourth patch to one rule -- `.test.ts`, then `__fixtures__/`, then this, plus `.DS_Store` -- and each was found by a reviewer after it shipped. So the rule stops being a list somebody remembers to extend: a new test asserts the property the list approximates, that every file the digest folds in is reachable from production code and nothing reachable is left out. Dropping `test-utils.ts` from the exclusion reddens it, which is the fifth instance failing in CI instead of in a review. Three branches that no test reached, each with a mutant the review measured surviving the whole suite: the walk's symlink skip (a directory cycle would send the first command of every review into unbounded recursion), the read-failure path (hashing the survivors of a concurrent checkout would accuse a tree that is merely mid-change), and the build's stamp call site (removing it left the scripts suite green while `npm run bundle` silently stopped writing the stamp). All three now redden. And `unmeasured` had no reader, so the one edge this check cannot measure but can see -- sources present, stamp absent -- passed in silence. That is the state of every existing checkout the moment this ships, and it is exactly the silent failure the change was written to end. It now says so, while an installed package, which has no sources either, still says nothing. * fix(review): the guard was shallower than the property it claimed The guard added last round asserts that every file in the digest is reachable from production code. It did not: a file imported by nothing passed, because the filter also required some test to import it; only `.ts` was inspected, so a test-only `.tsx` or `.mts` helper walked through; and it read static imports only, while this directory has nine `await import('./…')` edges. It asserts the property now — every extension, orphans included, dynamic edges seen — and the tree has no violators, so the strictness cost nothing today and is there for the next file. `__snapshots__` joins the exclusions. `vitest --update` regenerating a snapshot would have moved the digest with the bundle byte-identical; none exists under the review roots today only by chance, and 120 `toMatchSnapshot()` calls live elsewhere in this package. Three couplings that no test held: - the allowlist entry that fixed the release-breaking R2-1 -- reverting those five lines left the whole scripts suite green, and the next failure would have been a release aborting on all five targets. `isAllowedDistEntry` is exported and the stamp's own name is asserted against it, so a one-sided rename fails here instead; - the `.DS_Store` member of `NOT_BUNDLED_FILE`, absent from the repo and so from the parity tree -- one-sided removal stayed green while a macOS checkout would digest differently on the two sides forever; - each `unmeasured` reason. Swapping the two arguments at the single call site kept all 76 tests green while telling a pre-stamp checkout its sources were missing. And two comments that said the opposite of the code beneath them: the digest is computed unconditionally on purpose (the pre-stamp notice needs it), and `NOT_BUNDLED_FILE` helpers are deliberately not importers, since nothing reaches the bundle through a file the bundle does not contain. The two stderr diagnostics are documented for users, beside the sibling paragraph this PR already added. * fix(review): measure only the layout that can carry a stamp `npm start` launches `node <root>/packages/cli`, and node sets `argv[1]` to that directory -- so the derivation found sources under `<root>` with no stamp beside them and printed "could not check" on every review, forever, with advice that could never make it stop. That is the fires-when-nothing-is-wrong failure this change argues against, on the path `start.js` sets `QWEN_CODE_CLI` to precisely so reviews reach that build. Only a `<root>/dist/cli.js` layout is measured now; anything else has no stamp to find and no way to grow one. The build-side digest could kill `npm run bundle` where the check side degrades gracefully: a file vanishing mid-walk threw out of the hash loop, and the stamp is the copier's last step, so the build would fail with every asset already in place. Caught and skipped -- a missing stamp is `unmeasured`, which the runtime already treats as an acceptable answer. The skill now says what to do with the warning, which is the half that makes it reach a human: `parse-args` runs inside an agent's shell tool, the user reads the agent's summary rather than raw stderr, and a line nobody repeats is a line nobody sees -- which is how the 2026-08-02 round went wrong in the first place. It also records that the instruction cannot help the run that needs it, since the skill comes from the same bundle. And the scope is stated where silence could be over-read: the digest covers the review commands, the file that registers them, and the bundled skill -- not the shared helpers those import. A quiet run means the review code matches the bundle, not that the tree does. * fix(review): refuse to certify a bundle the copier may not describe The stamp described the tree as the COPIER saw it, and the copier runs after esbuild -- so a source edited in between, or `copy_bundle_assets.js` run on its own (it self-executes), wrote a digest certifying a `cli.js` built from something else. Silence then means "verified fresh" when it is not, and that is the only direction here where a quiet run is affirmatively wrong rather than merely uninformative: every other gap degrades to `unmeasured`. Timestamps are the wrong tool for judging staleness and the right one for judging whether this stamp can be honest at all, so the build refuses when any source is newer than the bundle it would attest to, and says why. Driven for real: touching a review source and running the copier alone now prints "skipped the source digest rather than certify a bundle it may not describe". `it('counts the same files')` compared nothing -- it asserted `> 50` on the build side while the check side exposes no count, so the title claimed a parity the body never checked, and the margin over the real 56 made it a future false alarm in `scripts/` for an unrelated change. Removed; the digest parity already holds the file set. "Root is a file" was inferred from `readdirSync` raising ENOTDIR, an assumption about every platform's libuv on the one root that is a file -- `review.ts`, where "a new subcommand was registered" lives. `statSync(root).isFile()` says it instead. And the check itself moves out of the handler into `bundleStalenessNotices`, which is where the rest of it already lived. `parse-args` is about parsing arguments again, the wording is testable without the yargs harness, and a second caller -- an agent resuming a review never runs step 1 -- is one line. * fix(review): align the twin walk, and stop a test from passing on nothing The build side still inferred "this root is a file" from `readdirSync` raising ENOTDIR, one commit after the check side stopped doing exactly that and said why. A platform that maps the case differently would drop `commands/review.ts` from one digest and not the other, and a byte-for-byte correct bundle would warn on every review forever, on that platform alone, with rebuilding reproducing the same one-sided walk. Both sides ask `statSync(...).isFile()` now. Fixing one half of a pair and not the other is the mistake this file keeps making. The filename parity test had been passing on nothing since the previous commit: it matched `writeFileSync(join(distDir, '…'))` against the script's source, the literal moved into a `stampPath` variable, and the regex returned `undefined` so the assertion compared against nothing. It runs the build against a fixture now and reads the name off `dist/`, so it measures what the build does instead of what its source looks like. Renaming the stamp on one side reddens it. Also from review: the duplicated comment block in `parse-args`; an unreadable source now says the check could not run rather than passing in the same silence as an installed package, which is what the docstring already promised; the "could not check" line no longer asserts that the checkout predates the feature, since the build has three refusal paths and one of them means the opposite; every refusal removes an existing stamp, because leaving an older attestation beside a newer bundle is a weaker form of the certifying it refuses; and `drive` calls the check, which the module comment argued for and the diff had not done -- a resumed review never runs step 1, and that is where the long work starts. * fix(review): pin the regex group the parity tree missed, and say source, not command * fix(review): allowlist what the bundle holds, and cover the drive notice (QwenLM#8390) * fix(review): treat unreadable review sources as unmeasured (QwenLM#8390) * test(review): pin the stamp guard mutations that survived the suite (QwenLM#8390) * fix(review): close staleness-check gaps and pin the round-4 survivors (QwenLM#8390) * fix(review): close round-5 staleness gaps for parity, refusals, and partial checkouts (QwenLM#8390) * fix(review): close round-6 gaps in the clause classifier, symlink layout, and pin honesty (QwenLM#8390) * fix(review): close round-7 gaps in the closure oracle, parity pin, and refusal pins (QwenLM#8390) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): close round-8 gaps from the maintainer review (QwenLM#8390) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): close round-9 nits from the maintainer review (QwenLM#8390) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): pin the lease root in the synthetic digest parity case (QwenLM#8390) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Autofix <autofix@qwen-code.dev> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
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
3 participants
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
Applies the hunk loop's collocated-test guard to the mutant loop, so a mutant in a file whose own test was red in the unmutated baseline is held at
inconclusiveinstead of being scoredsurvived.Found by dogfooding — running
/reviewagainst #8213 withqwen3.8-max-preview. In one run,packages/acp-bridge/src/bridge.tsproduced:inconclusivebridge.test.tsdid not run green in the baselinesurvived→ 8 findingsSame file, same cause, opposite conclusions — and the eight became findings an author would act on.
Mechanism. A mutant runs against
greenProbesonly, so a red collocated test is excluded from the run. "Every affected test still passed" is then computed over a set that omits the one test most likely to catch the deletion — exactly the inference the hunk loop refuses in its own words: the other probes passing shows only that THEY do not cover it, not that nothing does.How the gap survived review. The comment under the hunk guard framed the asymmetry as "mutants guard the
killeddirection, hunks guard thesurvivedone". That is true of an inconclusive run, and it left a mutant'ssurviveddirection unguarded against an absent covering test. Two separate guards, one of which read as the whole rule. That comment is corrected here too.The check runs before the budget test, so a candidate that cannot yield a verdict does not spend a suite run to say so.
Reviewer Test Plan
How to verify
cd packages/cli && npx vitest run src/commands/review src/commands/review.test.ts; expect 47 files / 1436 tests green.holds a mutant at inconclusive when its OWN test was red in the baselineintest-efficacy.integration.test.ts. Delete the guard intest-efficacy.tsand it fails withexpected 'survived' to be 'inconclusive'— the same sentence feat(serve): establish workspace runtime ownership #8213 produced live.Evidence (Before & After)
Before, on #8213:
mutants -> {killed: 0, survived: 8}onbridge.ts, alongsidehunks -> {inconclusive: 6}on the same file for the stated reason. After: those mutants reportinconclusivenamingbridge.test.ts, and nomutant-survivedfinding is emitted for the file.The regression test carries both halves — a file whose own test is red (
f.ts) and one whose own test is green (g.ts) — because a guard that swept up the second would be no better than the gap it closes. Non-UI: N/A.Tested on
Risk & Scope
survivedinto aninconclusive, never the reverse, so no new finding can be produced by this change.中文说明
本 PR 做了什么
把 hunk 循环里的「同位测试」守卫补到变异体循环上：若某文件自身的测试在未变异基线中就是红的，该文件里的变异体判为
inconclusive，而不是survived。这是 dogfooding 发现的 —— 用
qwen3.8-max-preview对 #8213 实跑/review。同一次运行里，packages/acp-bridge/src/bridge.ts产出了：6 个 hunk 判inconclusive（理由：同位的bridge.test.ts基线未跑绿），而同一文件的 8 个变异体判survived并发出了 8 条 finding。同一文件、同一原因、相反结论 —— 而那 8 条会被作者当作"这些语句没有测试覆盖"去处理。机制：变异体只在
greenProbes上运行，红的同位测试被排除在运行之外；于是"每个受影响的测试删掉该语句后仍然通过"是在一个缺少了最可能抓住它的那个测试的集合上算出来的 —— 正是 hunk 循环用自己的话拒绝过的推理：其他探针通过只说明它们不覆盖它，不说明没有东西覆盖它。这个缺口是怎么通过评审的：hunk 守卫下方的注释把这种不对称表述为"变异体守
killed方向，hunk 守survived方向"。这对"不确定的运行"是成立的，但它让变异体的survived方向对"缺失的覆盖测试"毫无防护。两道独立的守卫，其中一道被当成了整条规则。该注释一并修正。检查放在预算判定之前，因此一个无法得出结论的候选不会为了说这句话而花掉一次套件运行。
Reviewer 测试计划
cd packages/cli && npx vitest run src/commands/review src/commands/review.test.ts；预期 47 文件 / 1436 测试全绿。test-efficacy.integration.test.ts中的holds a mutant at inconclusive when its OWN test was red in the baseline。删掉test-efficacy.ts里的守卫，它会以expected 'survived' to be 'inconclusive'变红 —— 与 feat(serve): establish workspace runtime ownership #8213 实跑时产生的是同一句话。证据（前后对比）
修复前在 #8213 上：
bridge.ts报mutants -> {killed: 0, survived: 8}，同一文件的hunks -> {inconclusive: 6}且理由如上。修复后：这些变异体报inconclusive并点名bridge.test.ts，该文件不再产出mutant-survivedfinding。回归测试同时包含两半 —— 自身测试为红的
f.ts与自身测试为绿的g.ts—— 因为一个会把后者也一并扫掉的守卫，并不比它所修补的缺口更好。非 UI 改动：不适用。风险与影响范围
survived变成inconclusive，不能反向，因此这个改动不会产生任何新 finding。