Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions .claude/skills/add-engine-variant/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ If you find yourself about to type `pub enum Foo { ... NewVariant ... }` in `cra

### Stage 1: Existence verification (5-grep protocol)

The variant might already exist under a different engine-native name. The mtgish AST and the engine vocabulary are not 1:1; the engine often has the concept under a different name.
The variant might already exist under a different engine-native name. External data formats and the engine vocabulary are not 1:1; the engine often has the concept under a different name.

> The canonical engine surface is `data/engine-inventory.json` (gitignored). Run `cargo engine-inventory` to (re)generate it locally before grepping for existing variants.

Expand All @@ -46,7 +46,7 @@ rg -n "<concept_keyword>" crates/engine/src/database/synthesis.rs
**Stage 1 verdicts:**

- **EXISTS_SAME_NAME**: variant exists. Stop. Wire to it.
- **EXISTS_DIFFERENT_NAME**: concept exists under engine-native name (e.g., mtgish `CreateTriggerUntil` engine `Effect::CreateDelayedTrigger`). Stop. Map mtgish AST to the existing slot.
- **EXISTS_DIFFERENT_NAME**: concept exists under an engine-native name (e.g., an external `CreateTriggerUntil` maps to engine `Effect::CreateDelayedTrigger`). Stop. Map the source data to the existing slot.
- **EXISTS_AS_PARAMETER**: concept exists as a parameter value of a more general variant (e.g., "untapped" exists as `Tap { negated: true }`). Stop. Use the parameter form.
- **DOES_NOT_EXIST**: proceed to Stage 2.

Expand Down Expand Up @@ -131,7 +131,7 @@ If you've made it through all three stages with EXTEND_OK / WITHIN_SECTION verdi
## Anti-patterns this skill prevents

- "Audit said add variant X" → adding sibling without verification (the audit is wrong about a third of the time per session metrics)
- "Engine doesn't have it" → without grepping for engine-native names (CreateTriggerUntil mtgish ↔ CreateDelayedTrigger engine)
- "Engine doesn't have it" → without grepping for engine-native names (external CreateTriggerUntil ↔ CreateDelayedTrigger engine)
- "Just one more sibling" → ignoring the sibling-cluster smell and compounding parameterization debt
- "Unify under one type" → crossing CR rule sections and conflating runtime resolvers
- "Type-only stub returning a placeholder true/false" → masks runtime correctness; pair with real evaluation logic before shipping. The historical example: `ReplacementCondition::CastViaKicker` initially shipped with a `=> true` stub that silently over-applied to non-kicked spells. Resolved by adding `Option<KickerVariant>` to the variant and tracking `SpellContext.kickers_paid` at cast resolution — the runtime now actually evaluates the gate. Always verify the resolver does what the variant doc-comment claims.
Expand All @@ -150,7 +150,6 @@ Refusing is the correct outcome when any stage fails. Coverage waits, architectu
## Related artifacts

- Workspace `CLAUDE.md` — "Parameterize, don't proliferate" principle (the policy this skill enforces)
- Crate `mtgish-import/CLAUDE.md` Rule §8 — audit-verdict filter discipline
- Memory note `feedback_parameterize_dont_proliferate.md` — user directive recording this as a hard rule

## Inputs / outputs
Expand Down
2 changes: 1 addition & 1 deletion .claude/skills/bug-triage/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ Do NOT start implementation — do not run `$engine-implementer` — until a ful

Adding a new enum variant or a new field to a struct-variant has compile-time blast radius across the WHOLE workspace. This was the single most common gap class across multi-round plan reviews — make it an explicit plan section and a mandatory reviewer check.

- **New field on a struct-variant** (e.g. `split` on `Effect::SearchLibrary`): `#[serde(default)]` does NOT make it optional for Rust *literals*. Every `Effect::Variant { … }` construction across ALL crates (engine, phase-ai, mtgish-import, tests) must set it; every non-`..` destructure must bind it or add `..`. Enumerate with `grep -rn "Effect::Variant {" crates/` — treat the grep, not a hand-list, as authoritative.
- **New field on a struct-variant** (e.g. `split` on `Effect::SearchLibrary`): `#[serde(default)]` does NOT make it optional for Rust *literals*. Every `Effect::Variant { … }` construction across ALL crates and tests must set it; every non-`..` destructure must bind it or add `..`. Enumerate with `grep -rn "Effect::Variant {" crates/` — treat the grep, not a hand-list, as authoritative.
- **New `WaitingFor` / enum variant**: breaks every EXHAUSTIVE `match` (no wildcard). Enumerate ALL sites by grepping a recently-added sibling variant (`rg "WaitingFor::SomeRecentVariant"`), then classify each match as exhaustive (needs an arm) vs `_`/`..`/`if let`/`matches!` (doesn't). Span engine + phase-ai + server-core. One `WaitingFor` variant typically needs ~5 arms (e.g. `acting_player`, `ai_support/candidates`, `phase-ai/decision_kind::classify`, `phase-ai/search::fallback_action`, `game/scenario::waiting_for_kind`).
- **THE SILENT KILLER — `matches!` dispatch gates**: a handler reachable only if its variant is listed in a `matches!` guard (e.g. `engine_resolution_choices::handles`) is NOT compile-checked. Omit it and the handler is silent dead code — the action falls through to InvalidAction; clippy and the type system say nothing. Grep for `matches!`/membership predicates that gate dispatch and add the variant; add a test asserting the action is *handled* (not InvalidAction).

Expand Down
2 changes: 0 additions & 2 deletions .claude/skills/engine-implementer/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ description: "End-to-end phase.rs implementation pipeline: plan, review-plan, im

This is the orchestrator for the phase.rs implementation pipeline. It runs as a **skill in the main thread** so it can spawn agents for every step that benefits from fresh context (plan review, surgical implementation, implementation review). Do not turn this into an agent — agents cannot spawn sub-agents, which is what made earlier versions silently degrade.

> **⚠️ `mtgish` is dormant — DO NOT route implementation work through it.** `mtgish/`, `crates/mtgish-import/`, and `data/mtgish-*` are NOT live consumers of the engine, parser, or card data. Reject any plan section, executor edit, or review fix that touches mtgish files; surface it to the user instead of silently shipping it. PRs that only modify mtgish are rejected on sight.

## Roles

| Step | Where it runs | Why |
Expand Down
2 changes: 0 additions & 2 deletions .claude/skills/engine-planner/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ Produce an implementation plan for the phase.rs engine. Design for the class, no

This skill produces the plan only. The plan-review loop belongs to the caller — when invoked from `/engine-implementer`, the orchestrator owns the loop. When invoked standalone, run `/review-engine-plan` against the plan yourself and iterate until clean.

> **⚠️ `mtgish` is dormant — out of scope for ALL plans.** Never plan changes to `mtgish/`, `crates/mtgish-import/`, or `data/mtgish-*`. The import pipeline is not a live consumer of the engine or parser; new variants, parser patterns, and effects do NOT need to be mirrored there. If a task description references mtgish, surface the contradiction and stop — do not silently include mtgish in the plan.

## Input

A task description: parser enhancement/fix, or engine mechanic enhancement/fix. May reference cards, Oracle text patterns, CR rules, or coverage gaps.
Expand Down
22 changes: 7 additions & 15 deletions .claude/wf/msh-wave3-plan.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
# MSH Wave 3 — Replacement Effects (Cluster B): Implementation Plan

Generated by Wave3Planner (via /engine-planner). Decision LOCKED: **Approach (A)** —
no new `ReplacementDefinition` fields (mtgish is out of bounds). Reuse the existing
`execute` field + a new `TokenCoreTypeMatches` condition variant + a new
`Effect::RemoveAllDamage` primitive. Zero mtgish edits.
no new `ReplacementDefinition` fields. Reuse the existing `execute` field +
a new `TokenCoreTypeMatches` condition variant + a new
`Effect::RemoveAllDamage` primitive.

> REVISION (round 2): All sections passed review EXCEPT the Wolverine combat-batch
> simultaneity case (previously deferred as a follow-up — that was rules-incorrect).
Expand Down Expand Up @@ -80,7 +80,7 @@ parser limits in `parse_damage_modification_static` (oracle_replacement.rs:4014;
value(AttachedTo, tag("enchanted creature"))))`, consulted by `parse_damage_source_subject_filter` (:4401)
before `parse_type_phrase`.
Result: `ReplacementDefinition::new(DamageDone).damage_modification(Double).damage_source_filter(AttachedTo)`.
NO type-surface changes, NO mtgish impact. CR: 614.1a, 301.5/702.6 (Equipment/equip → AttachedTo host link).
NO type-surface changes. CR: 614.1a, 301.5/702.6 (Equipment/equip → AttachedTo host link).

**⚠ CLASSIFICATION GUARD (reviewer nit #4 — BLOCKING for the FIRST ordering).** The "parser-only / lowest-risk /
implement FIRST" classification rests entirely on `damage_source_filter = AttachedTo` resolving as a *damage-source*
Expand Down Expand Up @@ -114,8 +114,8 @@ Approach (A) — no new ReplacementDefinition field:
(fn head replacement.rs:3283; match :3291; add the new arm in the condition-match body — the brief's earlier
":3624" pointer is one arm region inside that same match, both are the same match block) against
`spec.characteristics.core_types`. Parser sets `condition(TokenCoreTypeMatches{core_types:[Creature]})`.
Run `/add-engine-variant` gate. mtgish-SAFE: mtgish-import only CONSTRUCTS ReplacementCondition variants,
never matches them exhaustively; engine's `evaluate_replacement_condition` is the only exhaustive match (add one arm).
Run `/add-engine-variant` gate. Engine's `evaluate_replacement_condition`
is the only exhaustive match (add one arm).
AUDIT all other `ReplacementCondition::` match sites for exhaustiveness before finalizing.
- Parameterize check: only 2 token-characteristic gates (subtype, core-type) → below 3-variant threshold, distinct
fields → NOT a sibling-cluster smell.
Expand Down Expand Up @@ -157,19 +157,11 @@ Approach (A) implementation:
the Phase-B-before-Phase-C invariant (combat_damage.rs:869-944 / deal_damage.rs:446) that makes the heal
combat-batch-correct. Standard `/add-engine-effect` (resolver in effects/mod.rs, EffectKind, targeting, AI/FE
serialization, coverage).
- **mtgish-safety (CITATION FIX #1, CONFIRMED):** adding `Effect::RemoveAllDamage` cannot break mtgish. mtgish-import
only ever CONSTRUCTS engine `Effect` / `ReplacementCondition` values — it never exhaustively matches the engine
`Effect` enum. Where it does match over `Effect`, every site has a catch-all: `rewrite_bound_x_in_effect`
(convert/action.rs:451), `rewrite_any_target_filter_in_effect` (:597), `apply_player_target` (:5458, a named
`other =>` strict-fail arm), and `apply_token_flags` (:6459) — all fall through on unrecognized variants. (The
previously-cited `convert/token.rs:607` was WRONG: that line is a `match effect { TokenCopyEffect::AddAbility… }`
over `TokenCopyEffect`, an mtgish AST type, NOT engine `Effect` — removed.) Conclusion unchanged: mtgish-safe.
Re-confirm these catch-alls compile during impl (engine-only build), but no mtgish edits are needed or permitted.
CR: 614.1a, 510.2 (combat damage simultaneous — same-batch instances must not heal each other),
120.6 (marked damage remains until cleanup — heal removes early), 120.1/120.3, 614.5.

## Sequential implementation order (all share oracle_replacement.rs → strictly sequential)
1. Mjölnir — parser-only, no type surface, no mtgish, lowest risk. FIRST **after the classification-guard
1. Mjölnir — parser-only, no type surface, lowest risk. FIRST **after the classification-guard
confirmation test passes**; if AttachedTo-as-source-filter needs runtime wiring, reclassify and reorder
(do not block 2/3).
2. Divine Visitation — `TokenCoreTypeMatches` condition + token-substitution shape/applier (execute-reuse). MEDIUM.
Expand Down
3 changes: 1 addition & 2 deletions .claude/wf/msh-wave4-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

**Scope:** 3 cards on the `main` checkout, one coherent wave.
**Cards:** Dragon Man, Reformed Robot (Cluster C, CDA) · Wolverine, Claws Out (Cluster C, combat-damage assignment) · Ms. Marvel, Elastic Ally (Cluster H, new FilterProp).
**mtgish:** OUT OF BOUNDS. No step in this plan touches `mtgish/`, `crates/mtgish-import/`, or `data/mtgish-*`. No mtgish mirroring is required for any of these changes.
**Skills applied:** `/add-static-ability`, `/add-trigger`, `/add-engine-variant`, `/oracle-parser`.

---
Expand Down Expand Up @@ -260,4 +259,4 @@ Only `oracle_static/` is touched by two cards, in **different files/functions**
3. **Dedicated vs parameterized self-comparison (Ms. Marvel).** Plan recommends the dedicated `PowerExceedsBase` (2 siblings < threshold; mirrors `ToughnessGTPower`; avoids a multi-site refactor under concurrent edits). Reviewer may mandate `PtSelfComparison` instead — explicit decision point; categorical boundary (CR 208) holds either way.
4. **`Aggregate` doc-comment** says "battlefield objects" but the resolver is zone-general — correct it as part of Dragon Man (prevents the next agent repeating the recon error).
5. **Inert-until-regen.** All three are parser changes; `from_oracle_text` unit tests pass even with stale deployed data. Tests MUST use `add_real_card` + rehydrate to exercise the real card-data path; regenerate card-data before claiming "supported".
6. **No stop-and-return blockers.** No mtgish dependency, no missing infrastructure — all three implementable on `main` now.
6. **No stop-and-return blockers.** No missing infrastructure — all three are implementable on `main` now.
1 change: 0 additions & 1 deletion .claude/wf/msh-wave6-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,6 @@ Runtime tests use `add_real_card` + rehydrate and drive the REAL declare-attacke

## Constraints honored
- Do NOT touch `StaticMode` (#4010/#3958) — scope rides `attack_defended`; ✓.
- mtgish dormant — not referenced; ✓.
- Surgical/additive; no whole-file rewrites; commit by pathspec; Tilt-first (no raw cargo); ✓.
- All parser dispatch via nom combinators; ✓ (Nom Compliance section).
- No bool flags — `AttackTargetFilter` + `RestrictionPlayerScope` + `RestrictionExpiry` typed enums; ✓.
Expand Down
2 changes: 1 addition & 1 deletion .claude/wf/powerup-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ The **parser is the detector** throughout.

## 3. Step-by-Step Implementation Plan

> Verification cadence per CLAUDE.md: `cargo fmt --all` directly; everything else via Tilt (`tilt logs clippy/test-engine`, `./scripts/tilt-wait.sh`). Do NOT run `cargo build/clippy/test` directly. mtgish is out of scope — touch nothing under `mtgish/`, `crates/mtgish-import/`, `data/mtgish-*`.
> Verification cadence per CLAUDE.md: `cargo fmt --all` directly; everything else via Tilt (`tilt logs clippy/test-engine`, `./scripts/tilt-wait.sh`). Do NOT run `cargo build/clippy/test` directly.

### Step 1 — Add `AbilityTag::PowerUp` + `keyword_str()` (gate (a))
- **`types/ability.rs:11645`** — add to `AbilityTag` (after `Backup`): `/// CR 602.5b + CR 602.1: This ability originated from a Power-up keyword definition.\nPowerUp,`. (Re-grep `602.5b` and `602.1` before writing — verified `MagicCompRules.txt:2541`, `:2510`.)
Expand Down
3 changes: 1 addition & 2 deletions .claude/wf/teamwork-conditional-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,8 @@ predicate (the existing honest-refusal path, oracle_casting.rs:244-255); the Swa
`Condition_If` detector continues to flag it. We add a guard test asserting **no** unconditional
flash grant leaks. We do **not** ship a green-but-wrong flash grant.

**mtgish-safety statement.** Zero edits to `mtgish/`, `crates/mtgish-import/`, or `data/mtgish-*`.
No new `Effect`, `AbilityCondition`, `AdditionalCostOrigin`, or `SpecialClause` variant (all
already exist), so no mtgish mirroring is implicated. Changes are confined to parser files
already exist). Changes are confined to parser files
(`conditions.rs`, `lower.rs`, `oracle_modal.rs`, `oracle_nom/condition.rs`) plus tests.

### Blast-radius bounding (mandatory — EMH + We Say Thee Nay! fixes)
Expand Down
2 changes: 1 addition & 1 deletion .claude/wf/teamwork-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ One coherent change set; nothing half-wired:
13. (If Quantum Reduction flash shipped) `types/ability.rs` `ParsedCondition::AdditionalCostPaid` + its flash-timing evaluation.
14. Integration tests per §11; snapshot updates if any card's parsed abilities change (`cargo coverage` to confirm the 17 cards drop their unsupported markers).

**Verification cadence (Tilt-first):** `cargo fmt --all` (direct, always) → if Tilt up, `./scripts/tilt-wait.sh --timeout 240 clippy test-engine card-data`; else direct `cargo clippy --all-targets -- -D warnings` + `cargo test -p engine`. `cargo coverage` (direct one-shot) to confirm the 17 cards. `pnpm type-check`/`pnpm lint` for the FE diff. Do NOT touch `mtgish/`, `crates/mtgish-import/`, or `data/mtgish-*` (dormant — out of scope).
**Verification cadence (Tilt-first):** `cargo fmt --all` (direct, always) → if Tilt up, `./scripts/tilt-wait.sh --timeout 240 clippy test-engine card-data`; else direct `cargo clippy --all-targets -- -D warnings` + `cargo test -p engine`. `cargo coverage` (direct one-shot) to confirm the 17 cards. `pnpm type-check`/`pnpm lint` for the FE diff.

---

Expand Down
2 changes: 1 addition & 1 deletion .claude/wf/token-storm-scaling-gate-plan-r2.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ Add a permanent CI regression gate proving the `StaticModePresence` O(1) index k

## Applicable skills

No new engine surface is introduced (no new effect, keyword, trigger, static, variant, or parser pattern). This is a **test-only** change composing existing pub APIs, so `/add-engine-effect`, `/add-engine-variant`, and the parser skills do not apply. The governing guidance is `/card-test`'s foot-gun list (adapted: these are enumeration/perf tests, not cast-pipeline tests) and the CLAUDE.md verification-matrix contract. No `mtgish` involvement.
No new engine surface is introduced (no new effect, keyword, trigger, static, variant, or parser pattern). This is a **test-only** change composing existing pub APIs, so `/add-engine-effect`, `/add-engine-variant`, and the parser skills do not apply. The governing guidance is `/card-test`'s foot-gun list (adapted: these are enumeration/perf tests, not cast-pipeline tests) and the CLAUDE.md verification-matrix contract.

## Analogous trace (hard gate)

Expand Down
2 changes: 1 addition & 1 deletion .claude/wf/xgate-depth-plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -512,4 +512,4 @@ compile error, so there is no silent gap).

No engine files (`casting_costs.rs` untouched); no `can_afford_x_at_least_one` primitive (deferred,
justified); no changes to `x_reference.rs` / `x_value.rs` semantics; no display-derive revival in sim
mode; no other policy's behavior; `mtgish` untouched.
mode; no other policy's behavior.
1 change: 0 additions & 1 deletion .coderabbit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ reviews:
- "!client/public/**.jpg" # binary assets
- "!client/src/wasm/**" # generated by ./scripts/build-wasm.sh
- "!**/*.d.ts" # committed wasm type artifact
- "!data/mtgish-cards.json" # generated import data
- "!data/semantic-audit.json" # generated by `cargo semantic-audit`
- "!data/parser-gaps.json" # generated by `cargo parser-gaps`
- "!data/engine-inventory.json" # generated by `cargo engine-inventory`
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ jobs:
# caught before it lands on the branch that feeds staging/preview.
env:
PROPTEST_CASES: ${{ github.event_name == 'pull_request' && '32' || '256' }}
run: cargo nextest run --profile ci --partition count:${{ matrix.shard }}/4 --workspace --exclude phase-tauri --exclude mtgish-import --features engine/proptest --status-level fail --final-status-level fail
run: cargo nextest run --profile ci --partition count:${{ matrix.shard }}/4 --workspace --exclude phase-tauri --features engine/proptest --status-level fail --final-status-level fail

card-data-gate:
name: Card data (generate, validate, coverage)
Expand Down
5 changes: 0 additions & 5 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ client/coverage/
data/*
!data/standard-cards.txt
!data/mtgjson/
!data/mtgish-cards.json
data/mtgjson/AtomicCards.json
data/mtgjson/CardTypes.json
data/mtgjson/Meta.json
Expand Down Expand Up @@ -119,10 +118,6 @@ parser-batch-plan.md
parser-fallback-plan.md
tmp/

# External reference / scratch
mtgish/
mtgish*.tgz

# Python bytecode cache
__pycache__/
*.pyc
Expand Down
1 change: 0 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ tilt up # continuous build/test — leave running
## Pull requests

- Target `origin/main` (`phase-rs/phase`).
- Don't modify `mtgish/`, `crates/mtgish-import/`, or `data/mtgish-*` (dormant);
PRs that only touch them are rejected.
- If you used an LLM, use `.github/PULL_REQUEST_TEMPLATE.md` for the PR body,
fill every section, and report the model on its canonical `Model:` line; see
Expand Down
Loading
Loading