docs(skills): add walker-classification step to add-engine-variant checklist (combo-detector skill implementation update) - #4905
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the SKILL.md documentation for adding an engine variant. It introduces a new step (Step 3) detailing how to classify new variants or fields on existing variants in the fail-closed ability-scan walker (crates/engine/src/game/ability_scan.rs), and increments the numbering of the subsequent steps. There are no review comments, and I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
matthewevans
left a comment
There was a problem hiding this comment.
[HIGH] This PR modifies repository agent/skill instructions, which is a sweep hard-stop.
Evidence: the diff includes .claude/skills/add-engine-variant/SKILL.md. Instruction and skill files steer future automated review/implementation behavior, so external contributor PRs cannot change them through the normal PR review loop.
Please remove the .claude/skills/add-engine-variant/SKILL.md change from this PR. If that skill update is intentional, it needs separate direct maintainer handling outside the contributor sweep.
|
🤖 AI text below 🤖 Extended the step-3 checklist text (a71b6f0) to cover the second compiler-exhaustive classification surface that #4904's review round added to the walker: the resolution-time choice classifier. It documents the SHIP criteria for claiming an |
Parse changes introduced by this PR · 13 card(s), 18 signature(s) (baseline: main
|
|
@matthewevans you might need to take a look at this one with human eyes since the bot doesn't like its skills being touched! |
…ecklist PR phase-rs#4904 (growing-cascade detector) introduced a fail-closed ability-scan walker (crates/engine/src/game/ability_scan.rs) whose projected-resource axis the detector's soundness rests on. New enum VARIANTS fail to compile there (exhaustive matches, no wildcards), but a new FIELD on an existing variant whose arm keeps a struct-rest `..` (the CONSERVATIVE arms) is silently dropped — a fail-open that can only be caught by contributor process, not the compiler. This adds the classification step to the add-engine-variant checklist so contributors adding fields to walker-classified variants promote the arm to an explicit destructure and classify the field on every axis. Split out of phase-rs#4904 per repository sweep policy: instruction/skill files require separate direct maintainer handling and cannot ride contributor engine PRs. Assisted-by: ClaudeCode:claude-fable-5
…hecklist step The choice-free soundness gate (phase-rs#4904) adds a second compiler-exhaustive classification surface to ability_scan.rs. Extend step 3 with the SHIP criteria for claiming an Effect variant choice-free: resolver trace in the arm comment, no-`..` destructure, and the pinned guard test update. Assisted-by: ClaudeCode:claude-fable-5
a71b6f0 to
c9fd23a
Compare
This is split off from #4904 as requested, jargon here refers to features implemented in that PR.
🤖 AI text below 🤖
What
Adds one step to the
add-engine-variantskill checklist: classify new variants/fields in the fail-closed ability-scan walker (crates/engine/src/game/ability_scan.rs), introduced by #4904.Why
The walker's projected-resource axis is what the growing-cascade detector's soundness rests on (a silently-dropped projected-resource read = potential false combo-win). The compiler enforces classification for new variants (exhaustive matches, no wildcards) and for new fields on arms destructured without
..— but the 42 CONSERVATIVE arms deliberately keep..(a new field can't make "reads everything" wronger), so a read-bearing field added to a CONSERVATIVE-classified variant is the one residual only contributor process can catch. This checklist step is that process: promote the arm to an explicit destructure and classify the field on every axis.Provenance
This change originally rode in #4904 (
1d9161eec) and was removed from that PR at maintainer request — instruction/skill files require separate direct maintainer handling and cannot ride contributor engine PRs (sweep hard-stop). This PR is that separate handling: docs-only, one file, no code.Related: #4904 (the walker + detector this checklist step protects).
🤖 Generated with Claude Code
https://claude.ai/code/session_01CwQE5oyMqZ9T4BPMsih3Kj