🤖 AI text below 🤖
Summary
The add-engine-variant skill checklist can APPROVE a fail-open variant addition. It gates on whether a new enum variant is discoverable and non-duplicative, but it does not ask the one question that determines whether the compiler will help you: are the existing decision sites exhaustive matches, or equality comparisons?
For an enum whose call sites all use ==/!=/matches!, adding a variant compiles clean and every existing site silently takes the "not this variant" branch. The checklist currently passes that case.
Measurement
Measured at 77e686cae, predicate stated so it can be re-run:
| Measurement |
Predicate |
Count |
sub_link decision sites |
lines matching sub_link and one of ==, !=, matches!, in crates/**/*.rs excluding crates/engine/tests/ |
40 |
match statements on sub_link |
match .*sub_link in the same scope |
0 |
Top files by site count: game/effects/mod.rs (16), parser/oracle_effect/lower.rs (8), parser/oracle_effect/assembly.rs (4), game/stack.rs (4).
Because there are zero exhaustive matches, a new SubAbilityLink variant is fail-open at all 40 sites: nothing fails to compile, and each site treats the new variant as "not the variant I care about". Whether that default is correct has to be established by hand, 40 times.
(A narrower predicate used during earlier design work counted 36; the difference is predicate scope, not a changed tree. The structural conclusion — zero exhaustive matches — is identical under both.)
Why this matters concretely
In the #6857 / #7484 lane we considered adding a SubAbilityLink::ModalSibling variant and rejected it for exactly this reason, choosing a separate Option<usize> marker field instead. The deciding factor was that the field's misuse is forced (8 destructure sites the compiler flags) while the variant's misuse is silent (fail-open at every comparison site).
That reasoning was reconstructed by hand during design review. The checklist did not prompt for it, and would have approved the variant.
Proposed remedy
Add a gate to the add-engine-variant checklist, before approval:
- Measure the decision sites for the target enum. Count equality/
matches! comparisons vs match statements, and state the predicate used.
- If exhaustive matches == 0, the addition is fail-open. Do not approve on discoverability grounds alone. Either:
- enumerate every comparison site and justify the "not this variant" default at each, or
- prefer an alternative encoding whose misuse is compiler-forced (a new field on the struct, a wrapper type), and record why.
- Record the counts in the proposal, so a reviewer can re-run the predicate rather than re-deriving the argument.
This is deliberately a measurement gate, not a prohibition — fail-open variants are sometimes right. The gap is that the checklist currently never surfaces the question.
Scope
Process/tooling issue, no runtime behavior change. Filed from the #6857 / #7484 lane alongside #7500 (Grub's Command dropped filter); the two are independent.
🤖 AI text below 🤖
Summary
The
add-engine-variantskill checklist can APPROVE a fail-open variant addition. It gates on whether a new enum variant is discoverable and non-duplicative, but it does not ask the one question that determines whether the compiler will help you: are the existing decision sites exhaustive matches, or equality comparisons?For an enum whose call sites all use
==/!=/matches!, adding a variant compiles clean and every existing site silently takes the "not this variant" branch. The checklist currently passes that case.Measurement
Measured at
77e686cae, predicate stated so it can be re-run:sub_linkdecision sitessub_linkand one of==,!=,matches!, incrates/**/*.rsexcludingcrates/engine/tests/matchstatements onsub_linkmatch .*sub_linkin the same scopeTop files by site count:
game/effects/mod.rs(16),parser/oracle_effect/lower.rs(8),parser/oracle_effect/assembly.rs(4),game/stack.rs(4).Because there are zero exhaustive matches, a new
SubAbilityLinkvariant is fail-open at all 40 sites: nothing fails to compile, and each site treats the new variant as "not the variant I care about". Whether that default is correct has to be established by hand, 40 times.(A narrower predicate used during earlier design work counted 36; the difference is predicate scope, not a changed tree. The structural conclusion — zero exhaustive matches — is identical under both.)
Why this matters concretely
In the #6857 / #7484 lane we considered adding a
SubAbilityLink::ModalSiblingvariant and rejected it for exactly this reason, choosing a separateOption<usize>marker field instead. The deciding factor was that the field's misuse is forced (8 destructure sites the compiler flags) while the variant's misuse is silent (fail-open at every comparison site).That reasoning was reconstructed by hand during design review. The checklist did not prompt for it, and would have approved the variant.
Proposed remedy
Add a gate to the
add-engine-variantchecklist, before approval:matches!comparisons vsmatchstatements, and state the predicate used.This is deliberately a measurement gate, not a prohibition — fail-open variants are sometimes right. The gap is that the checklist currently never surfaces the question.
Scope
Process/tooling issue, no runtime behavior change. Filed from the #6857 / #7484 lane alongside #7500 (Grub's Command dropped filter); the two are independent.