review-engine-plan

name	review-engine-plan
description	Review phase.rs engine, parser, AI, frontend, or rules implementation plans before code is written. Use when Codex needs an architectural gate for plans involving parser changes, engine mechanics, MTG Comprehensive Rules behavior, new variants, targeting, replacement effects, stack/casting flow, AI policy, or frontend GameAction workflows.

Review Engine Plan

Review the plan as an architectural gate. Reject the plan if any required dimension is missing, superficial, or contradicted by code evidence.

Required Checks

1. Class vs card

   • Identify how many cards or patterns the plan covers.
   • Reject one-card plans unless the card is only the validating consumer of a reusable building block.

2. Building-block reuse

   • Confirm the plan consulted relevant existing modules from the CLAUDE.md building-block table.
   • Reject duplicated logic already covered by parser/oracle_nom/, parser/oracle_util.rs, game/filter.rs, game/quantity.rs, game/ability_utils.rs, game/keywords.rs, or nearby helpers.
   • Require justification for every new helper.

3. Trace verification

   • The plan must name an analogous existing feature and list the file path trace followed end to end.
   • Reject plans that did not trace an existing feature.

4. Abstraction layer correctness

   • Parser logic belongs in parser/.
   • Runtime rules belong in game/ or game/effects/.
   • Types belong in types/.
   • Game logic must not leak into frontend or WASM bridge.
   • Display formatting must not leak into the engine.
   • i18n boundary: if the plan adds frontend UI or log text, it must route frontend-authored strings through t() (react-i18next, keys in client/src/i18n/locales/en/<ns>.json) and leave engine/card pass-through raw. Reject plans that hardcode user-facing chrome strings or that wrap card/Oracle/enum text in t(). See client/src/i18n/README.md.

5. Idiomatic Rust

   • Prefer typed enums such as ControllerRef, Comparator, and Option<T> over bool fields.
   • Prefer exhaustive matches over wildcard catch-alls when the type set is known.
   • Prefer existing strip_prefix/parser helpers over format!() plus matching.

6. Nom compliance for parser plans

   • If any parser file changes, the plan must specify exact nom combinators or existing parser functions for every detection, dispatch, or classification step.
   • Reject plans using contains(), starts_with(), ends_with(), find(), or heuristics for Oracle parsing.
   • The parser is the detector; try the real parser rather than duplicating detection logic.

7. CR verification

   • Every referenced CR number must be verified against docs/MagicCompRules.txt.
   • If CR comments are added or changed, the plan must say how they will be verified.

8. Skill checklist adherence

   • Identify applicable skills, such as $add-engine-effect, $oracle-parser, $add-keyword, $add-trigger, $add-static-ability, $add-replacement-effect, $add-interactive-effect, or $casting-stack-conditions.
   • Reject plans that omit required checklist steps from applicable skills.

9. Verification matrix

   • Reject plans without a claim-to-test map for every behavioral claim.
   • Each map entry must name the changed seam/function, production entry point, runtime test, revert-failing assertion, sibling/negative cases, and coverage status impact.
   • Reject helper-only tests for changes whose production path goes through apply(), WaitingFor/GameAction, casting/stack, combat declaration, replacement handling, or the scenario runner.
   • Parser shape tests do not satisfy runtime semantics or coverage-support claims. Parser-only shape tests are acceptable only when unsupported semantics remain honest via Effect::unimplemented, an equivalent strict-failure marker, or unchanged red coverage.
   • Reject parser plans that accept full Oracle text while dropping semantics unless the plan explicitly preserves an Unimplemented/coverage gap.
   • Reject negative-only test plans without a paired positive reach-guard: a planned assertion like !detector(...) or "X is NOT applied" must also prove the input got past upstream short-circuits (parse succeeded, zero Effect::Unimplemented), or an early-return makes it pass vacuously.
   • Cast-pipeline runtime tests must be planned via the /card-test recipe with the card's verbatim Oracle text, not a paraphrase.
   • If the plan adds a field to an existing enum variant or struct, require an enumeration of every construction/consumption site of that variant and how each threads the new field (resume/continuation paths, single-vs-multi-pick branches, and adapter payload constructors are the recurring drop points).

10. Identity / provenance contract

• For any "this way", "that source", "chosen", "cast using", "from among them", selected target/mode, duration-bound effect, replacement predicate, or controller/owner-relative text, require the plan to name the source phrase/rules concept, selected authority type and id/value, binding time/event, live vs snapshotted/latched semantics, storage location, consumption point, invalidation/expiration behavior, and a multi-authority hostile fixture.
• Reject plans that rely on rescanning matching permissions, sources, costs, replacements, tracked sets, controllers, owners, or choices at consumption time unless they prove the rescan is equivalent for a multi-authority fixture.

11. Scope matrix

• For target, player, combat, controller, owner, protector, or defender changes, require the plan to enumerate the variants/scopes reachable at the touched production boundary.
• Include permissions, costs, choice provenance, tracked sets, duration snapshots, source/controller/owner shifts, and serialization/protocol/card-data boundaries when those are touched.
• Require negative tests for semantically adjacent sibling variants that are plausibly affected, or a concrete explanation for why a sibling is unreachable/out of scope.

Review Loop

Return every gap to the planner. Require a revised full plan, then re-review the entire revised plan with fresh context. Repeat until a full round returns clean or the caller stops the process.

Output

Lead with blockers and material gaps. For each issue, include evidence and the required revision. If the plan is clean, say that no blocking gaps were found and name any residual assumptions.