Repository navigation
skills: redundant-guard deletes a guard keeping two readers on one predicate #1692
Description
Problem
morphic-reviewing-prs' references/testing.md#redundant-guard says: where a guard's revert or mutant survives and removing it changes no outcome, "a second reader already makes the check: delete the guard". A guard can survive every test and every input while still being what keeps two readers on one predicate. Deleting it then splits one reading into two, which morphic-verifying-changes' references/derive-never-maintain.md#parallel-implementations-agree forbids. Neither rule names the other, so a review following the first undoes the second.
The case that showed it, on PR #1689. In compilers/openapi/internal/schema, readAllOf marks which allOf branches fold only where allOfDecides(c, s) holds, and preserveUnmergedBranch consumes a branch's folded keywords by those marks. positionVoices, which delivers the same keywords to the node and carriers, gates on the same allOfDecides. Once that PR made allOfDecides true at both composers, forcing the guards open changed nothing:
# readAllOf without `if allOfDecides`, branchSpeaks and preserveUnmergedBranch without their nil checks
go test ./compilers/openapi/... ./internal/harness -> all ok
corpus of 2,071 specs, guards forced open vs head -> specs 2071 changed 0 exit2/timeout 0
golden spec -> IR and diagnostics byte-identical
By redundant-guard the guards go. Doing so makes consumption read voicingOf alone while delivery still reads allOfDecides and voicingOf. Should the two ever disagree again, a branch's keyword is consumed and lands nowhere, silently, where the shared gate keeps the branch whole. A disagreement of that kind is what the PR fixed: allOfDecides had excluded an allOf beside a union that the composers still composed.
Direction
A draft: add an exception to redundant-guard. A guard that gives a consumer the predicate another reader delivers by stands, with a comment at its line naming that reader, as references/rounds.md#oddity-needs-a-comment asks; its Probe checks for such a reader before the deletion. parallel-implementations-agree could name redundant-guard as the rule it outranks here.
Priority: P4, rule 1 (housekeeping: a skills rule changes no lowering, IR or contract behaviour). The commands above, on fix/openapi-fold-distributed-branches @1ae30ac9c (morphic main @0610ee63f).
Activity
OmarAlJarrah commentedon Nov 10, 2026
A second case the same clause gets wrong, from #1688. There,
hasUnionSiblingsstopped askingbranchesSayof a schema that writes no union, so no lowering reachedbranchesSay's guard for a schema with no branches any more. Byredundant-guardthe guard goes, since forcing it open changes no output. But the guard keeps the function's own contract, not a second reader's. Over no branches, "every branch says this keyword" holds of every keyword the schema writes, so a later caller asking of a union-less schema would have each of those keywords consumed and kept nowhere:The branch keeps the guard and holds it with a direct call (
TestBranchesSay_NoBranchesSayNothing), asTestAllNullUnion_NoBranchesFalsealready holdsallNullUnion's for the same reason. So an exception here might name both cases: a guard another reader delivers by, and a guard that keeps a function total over an input class no current caller sends, which a test can reach directly.