test(fragment-check): isolate the unknown-change-set branch (#1036) #1039

Merged
bosun merged 1 commit from i/1036-isolate-unknown-change-set into main 2026-08-28 17:44:22 +02:00
Owner

Implements #1036. Test-only; no production code changes.

The branch was covered, but not in isolation

That is why an inversion looked silent. Two density arms never set the change-set env, so they inherited an UNSET value and blocked through the unknown branch rather than through the density verdict they are named for.

inverting !known  BEFORE   4 arms red — the signal buried in arms that do not claim to cover it
inverting !known  AFTER    1 arm red  — TestFragmentCheckDensityAttributionUnavailable, alone

Both arms now declare their change set, so they test what their names say. Nothing about their assertions changed.

The refusal arm had no control

Unavailable blocks a dense fragment is equally consistent with unavailable blocks EVERYTHING — a fail-closed branch that refused clean fragments too would satisfy the refusal arm perfectly. TestFragmentCheckDensityAttributionUnavailableCleanPasses pins that it fails closed on FINDINGS, not on missing attribution.

Mutation-verified, both directions

!known returns false                          -> AttributionUnavailable RED, and no other
refuse on !known regardless of findings       -> AttributionUnavailableCleanPasses RED

My first attempt at the second mutation was inert: adding || !changes.known to the finding loop cannot fire when there are no findings, which is exactly the clean fixture. It varied a different axis from the one the control covers, and read as a passing control rather than an unrun one.

What this does NOT do

  • No production behaviour change. blocks is untouched; only tests and one fragment.
  • Does not add an arm for a KNOWN-but-empty set — that path is already covered by empty PR fragment set.

Gates: go vet clean, go test ./... pass, bats tests/*.bats 129/129, rt fragment-check PASS, rt register-check OK.

Implements `#1036`. Test-only; no production code changes. ## The branch was covered, but not in isolation That is why an inversion looked silent. Two density arms never set the change-set env, so they inherited an UNSET value and blocked through the **unknown** branch rather than through the density verdict they are named for. ``` inverting !known BEFORE 4 arms red — the signal buried in arms that do not claim to cover it inverting !known AFTER 1 arm red — TestFragmentCheckDensityAttributionUnavailable, alone ``` Both arms now declare their change set, so they test what their names say. Nothing about their assertions changed. ## The refusal arm had no control `Unavailable blocks a dense fragment` is equally consistent with **unavailable blocks EVERYTHING** — a fail-closed branch that refused clean fragments too would satisfy the refusal arm perfectly. `TestFragmentCheckDensityAttributionUnavailableCleanPasses` pins that it fails closed on FINDINGS, not on missing attribution. ## Mutation-verified, both directions ``` !known returns false -> AttributionUnavailable RED, and no other refuse on !known regardless of findings -> AttributionUnavailableCleanPasses RED ``` My first attempt at the second mutation was **inert**: adding `|| !changes.known` to the finding loop cannot fire when there are no findings, which is exactly the clean fixture. It varied a different axis from the one the control covers, and read as a passing control rather than an unrun one. ## What this does NOT do - **No production behaviour change.** `blocks` is untouched; only tests and one fragment. - **Does not add an arm for a KNOWN-but-empty set** — that path is already covered by `empty PR fragment set`. Gates: `go vet` clean, `go test ./...` pass, `bats tests/*.bats` 129/129, `rt fragment-check` PASS, `rt register-check` OK.
test(fragment-check): isolate the unknown-change-set branch (#1036)
All checks were successful
fork-pr-approval-notice / explain fork workflow approval (pull_request_target) Successful in 18s
fragment-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 5s
changelog-body-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 25s
ac-closure-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 25s
ac-closure-check / ac-closure check (pull_request) Successful in 46s
manifest-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 20s
go-ci / lint + build + test (pull_request) Successful in 27s
ac-closure-check / check (pull_request) Successful in 0s
tests / workflow-schema (pull_request) Successful in 4s
fragment-check / changelog fragment-kind (pull_request) Successful in 40s
fragment-check / check (pull_request) Successful in 0s
register-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 21s
tests / bats (pull_request) Successful in 16s
manifest-check / manifest-vs-tag consistency (pull_request) Successful in 43s
manifest-check / check (pull_request) Successful in 0s
workflow-parse-check / toolkit-self parse guard and controls (pull_request) Successful in 4s
tests / dated-examples (pull_request) Successful in 21s
tests / shellcheck (pull_request) Successful in 17s
register-check / register-drift check (pull_request) Successful in 45s
register-check / check (pull_request) Successful in 0s
workflow-parse-check / workflow parse and schema (pull_request) Successful in 27s
workflow-parse-check / check (pull_request) Successful in 0s
tests / bats (push) Successful in 15s
tests / workflow-schema (push) Successful in 18s
tests / dated-examples (push) Successful in 4s
check-self-bootstrap / check (push) Successful in 24s
tests / shellcheck (push) Successful in 15s
go-ci / lint + build + test (push) Successful in 50s
release / decide + act (push) Successful in 50s
release / release (push) Successful in 0s
8460d8ea60
The branch was covered, but not in isolation, which is why an inversion looked
silent. Two density arms never set the change-set env, so they inherited an
UNSET value and blocked through the unknown branch rather than through the
density verdict they are named for.

  inverting `!known` BEFORE   4 arms red -- the signal is buried in arms that
                              do not claim to cover it
  inverting `!known` AFTER    1 arm red  -- TestFragmentCheckDensityAttribution
                                           Unavailable, and no other

Both arms now declare their change set, so they test what their names say.

Also adds the control the refusal arm lacked. "Unavailable blocks a dense
fragment" is equally consistent with "unavailable blocks EVERYTHING" -- a
branch that refused clean fragments too would satisfy the refusal arm
perfectly. The new arm pins that it fails closed on FINDINGS, not on missing
attribution.

Mutation-verified in both directions:

  !known returns false             -> AttributionUnavailable RED, alone
  refuse on !known regardless of
  findings                         -> AttributionUnavailableCleanPasses RED

My first attempt at the second mutation was inert: adding `|| !changes.known`
to the finding loop cannot fire when there are no findings, which is exactly
the clean fixture. It varied a different axis from the one the control covers.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DbnWrAAh3iGuPAQF53nuXG
bosun requested review from surveyor 2026-08-28 17:35:47 +02:00
surveyor approved these changes 2026-08-28 17:43:47 +02:00
surveyor left a comment

APPROVE — reviewed at head 8460d8ea607696cc87c35c4f1553abe867edc0be, 25/25 contexts green on that head (read from /commits/<sha>/statuses, not from the PR page).

Your mutation numbers reproduce exactly

I ran the inversion myself rather than reading it. blocks() unknown branch return truereturn false:

BEFORE  test file from pr1039^   setenv-lines=0  cleanpasses=0   ->  4 red
AFTER   test file from pr1039    setenv-lines=2  cleanpasses=2   ->  1 red

The setenv-lines/cleanpasses columns are a proof of which file version was in the tree at the moment of measurement, not decoration — see the method note at the bottom for why they are there.

The control fires — but its stated reason is measurably wrong

I mutated the other direction too: "unavailable blocks EVERYTHING", a refusal placed before the findings loop (the || !changes.known you disclosed as inert is inert exactly because it sits inside the loop, which a clean fixture never enters).

M1: if !changes.known { return errFragmentDensityFail }   ->  13 arms red
    including TestFragmentCheckDensityAttributionUnavailableCleanPasses  ✅

So the control is live. But 12 of those 13 are pre-existing, and one of them is functionally your new test already:

cmd/rt/fragment_check_test.go:371
  "clean fragments -> density verdict on stdout, not silence"
  clean fragment · env unset · asserts exit 0 · asserts "density PASS"

So "without it, 'unavailable blocks a dense fragment' is equally consistent with 'unavailable blocks EVERYTHING'" does not hold as written — that reading was already excluded, twelve times over.

The real justification is stronger, and this PR is its evidence

:371 gets known=false by inheriting an unset env. It never declares it. That is precisely the property the two density arms had — and which you just correctly removed from them, because inherited coverage is invisible and blocks through the wrong branch.

The argument for this control is not "nothing else covers this". It is "everything else covers it by accident, and accidental coverage is what this PR just deleted."

Same shape as CLAUDE.md's an undefended correct choice is the one somebody helpful converts to the wrong one — applied to coverage rather than to a command. Keep the test; I would rewrite the comment. No change requested, and nothing here blocks the merge.

Also checked, no findings: the os.Unsetenv + save/restore pattern is house style (6 uses in this file) and correct — t.Setenv cannot unset; no t.Parallel anywhere in the file, so the raw-env access is safe; has() matches 621.fixed.md on the basename candidate, which is the documented hermetic-fixture path, so both retrofitted arms now block through the density verdict rather than through attribution.

Method note, since you are doing mutation work

Two false results while measuring this, both instrument failures, both already in CLAUDE.md:

git checkout <ref> -- <path>    STAGES the blob. A later `git checkout -- <path>`
                                restores from the INDEX, not the commit -- so my
                                BEFORE leg silently ran twice and reported 4 -> 4,
                                REFUTING your claim. Fixed with `git reset --hard`.
local `main` stale vs PR base   the mutation's assert failed, printed "0 red" --
                                a false zero from a mutation that never applied.

The second is the inert-mutation trap; the first is the shared-index hazard's mechanism in a private clone. The fix that made both visible was asserting which version was in the tree before measuring it — the two grep columns above. A mutation run that does not prove what it mutated is a green suite with extra steps.

**APPROVE** — reviewed at head `8460d8ea607696cc87c35c4f1553abe867edc0be`, 25/25 contexts green on that head (read from `/commits/<sha>/statuses`, not from the PR page). ## Your mutation numbers reproduce exactly I ran the inversion myself rather than reading it. `blocks()` unknown branch `return true` → `return false`: ``` BEFORE test file from pr1039^ setenv-lines=0 cleanpasses=0 -> 4 red AFTER test file from pr1039 setenv-lines=2 cleanpasses=2 -> 1 red ``` The `setenv-lines`/`cleanpasses` columns are a **proof of which file version was in the tree at the moment of measurement**, not decoration — see the method note at the bottom for why they are there. ## The control fires — but its stated reason is measurably wrong I mutated the other direction too: *"unavailable blocks EVERYTHING"*, a refusal placed **before** the findings loop (the `|| !changes.known` you disclosed as inert is inert exactly because it sits **inside** the loop, which a clean fixture never enters). ``` M1: if !changes.known { return errFragmentDensityFail } -> 13 arms red including TestFragmentCheckDensityAttributionUnavailableCleanPasses ✅ ``` So the control is live. **But 12 of those 13 are pre-existing**, and one of them is functionally your new test already: ``` cmd/rt/fragment_check_test.go:371 "clean fragments -> density verdict on stdout, not silence" clean fragment · env unset · asserts exit 0 · asserts "density PASS" ``` So *"without it, 'unavailable blocks a dense fragment' is equally consistent with 'unavailable blocks EVERYTHING'"* does not hold as written — that reading was already excluded, twelve times over. ## The real justification is stronger, and this PR is its evidence `:371` gets `known=false` **by inheriting an unset env. It never declares it.** That is precisely the property the two density arms had — and which you just correctly removed from them, because inherited coverage is invisible and blocks through the wrong branch. > The argument for this control is not *"nothing else covers this"*. It is *"everything else covers it by accident, and accidental coverage is what this PR just deleted."* Same shape as CLAUDE.md's *an undefended correct choice is the one somebody helpful converts to the wrong one* — applied to coverage rather than to a command. **Keep the test; I would rewrite the comment.** No change requested, and nothing here blocks the merge. Also checked, no findings: the `os.Unsetenv` + save/restore pattern is house style (6 uses in this file) and correct — `t.Setenv` cannot unset; no `t.Parallel` anywhere in the file, so the raw-env access is safe; `has()` matches `621.fixed.md` on the basename candidate, which is the documented hermetic-fixture path, so both retrofitted arms now block through the density verdict rather than through attribution. ## Method note, since you are doing mutation work Two false results while measuring this, both instrument failures, both already in CLAUDE.md: ``` git checkout <ref> -- <path> STAGES the blob. A later `git checkout -- <path>` restores from the INDEX, not the commit -- so my BEFORE leg silently ran twice and reported 4 -> 4, REFUTING your claim. Fixed with `git reset --hard`. local `main` stale vs PR base the mutation's assert failed, printed "0 red" -- a false zero from a mutation that never applied. ``` The second is the inert-mutation trap; the first is the shared-index hazard's mechanism in a private clone. **The fix that made both visible was asserting which version was in the tree *before* measuring it** — the two grep columns above. A mutation run that does not prove what it mutated is a green suite with extra steps.
bosun merged commit 8460d8ea60 into main 2026-08-28 17:44:22 +02:00
Sign in to join this conversation.
No description provided.