Use when adding a new deck-aware AI feature (`DeckFeatures` axis) and its companion policies (`TacticalPolicy` + optional `MulliganPolicy`) to `phase-ai`. Codifies the structural-detection pattern, AST-type locations, parts-based predicate convention, registry wiring sites, and recurring review traps so a new feature ships in a single commit without re-research.
Install
npx skillscat add phase-rs/phase/add-ai-feature-policy Install via the SkillsCat registry.
Adding a Deck-Aware AI Feature + Policy
This is the authoritative checklist for adding a new feature to the three-layer AI architecture (Features → Plan → Policies). Nine features exist as reference: landfall, mana_ramp, tribal, control, aristocrats, aggro_pressure, tokens_wide, plus_one_counters, spellslinger_prowess. Each follows the same pattern; mirror it.
Before you start: Read landfall.rs (simplest), then aristocrats.rs (geometric-mean commitment with identity lookup), then one feature similar in shape to yours. Trace from features/<name>.rs::detect() through DeckFeatures::analyze and AiSession::from_game through policies/<name>.rs::verdict so the data flow is concrete.
CR Verification Rule. Every CR number in annotations MUST be verified by grepping
docs/MagicCompRules.txtBEFORE you write it. Do NOT rely on memory or prior prompts. The 701.x and 702.x sections are arbitrary sequential assignments that hallucinate consistently. Rungrep -n "^701.34" docs/MagicCompRules.txtfor every number. CR 122.1d is Stun (NOT +1/+1 — that's CR 122.1a). CR 701.27 is Transform (NOT Proliferate — that's CR 701.34). Two real catches from the existing batch — you will have similar misses unless you grep.
1. Architecture (memorize this)
AiSession (per-game cache; built once in AiSession::from_game)
├─ features: HashMap<PlayerId, DeckFeatures> ← Layer 1: structural detection
├─ plan: HashMap<PlayerId, PlanSnapshot> ← Layer 2: bottoming + curve schedule
├─ deck_profile / strategy: per-player strategic profiles
├─ synergy: HashMap<PlayerId, SynergyGraph>
└─ memory: PolicyMemory
DeckFeatures
├─ landfall: LandfallFeature
├─ mana_ramp: ManaRampFeature
├─ tribal: TribalFeature
├─ control: ControlFeature
├─ aristocrats: AristocratsFeature
├─ aggro_pressure: AggroPressureFeature
├─ tokens_wide: TokensWideFeature
├─ plus_one_counters: PlusOneCountersFeature
├─ spellslinger_prowess: SpellslingerProwessFeature
└─ <your new feature>
Policies (Layer 3)
├─ TacticalPolicy — in-game decisions (CastSpell, ActivateAbility, DeclareAttackers, ...)
└─ MulliganPolicy — pre-game hand evaluation (sibling trait)
Both run inside their respective registries (`PolicyRegistry::default`,
`MulliganRegistry::default`). Activation is the single multiplicative knob.AiSession::from_game analyzes current_main plus current_commander; commander entries are
weighted as build-around cards before feature/profile/synergy detection. One-off analysis helpers
that only receive a deck slice stay pure over that slice.
Single-knob rule. A TacticalPolicy exposes activation() returning Option<f32>. The registry multiplies verdict.delta * activation exactly once. There is no archetype_scale, no turn_mult, no second pass. If a policy needs archetype/turn-sensitive weight, it computes it inside activation() from the inputs.
Score contract. Tactical policy deltas are card-equivalent units: delta = 1.0 means one card of expected value. Use the band helpers on PolicyVerdict so the chosen strength is visible at the call site:
| Band | Helper | Range | Meaning |
|---|---|---|---|
| Nudge | PolicyVerdict::nudge |
(0.0, 0.3] |
tie-breaker |
| Preference | PolicyVerdict::preference |
(0.3, 1.5] |
half-card to 1.5-card preference |
| Strong | PolicyVerdict::strong |
(1.5, 5.0] |
multi-card swing |
| Critical | PolicyVerdict::critical |
(5.0, 15.0] |
game-deciding |
| Veto | PolicyVerdict::reject |
n/a | never take this action |
All scoring scalars come from AiConfig::policy_penalties / PolicyPenalties with rationale comments. Thresholds that only gate activation stay as pub const in the policy/feature module. Never encode vetoes as sentinel scores.
2. Hard constraints (non-negotiable)
| Rule | Why | How to apply |
|---|---|---|
| Structural detection only. No card-name matching for classification. | Name lists rot; structural patterns generalize across sets. | Walk CardFace.{abilities,triggers,static_abilities,replacements,keywords,card_type}. Use crate::ability_chain::collect_chain_effects for ability chains. |
Identity lookup is exempt. Vec<String>::iter().any(|n| n == &obj.name) is allowed when the policy needs to re-find a structurally-classified card on the battlefield or in hand. |
Mulligan policies see GameObject shells whose abilities aren't resolved yet. |
Populate payoff_names: Vec<String> once per UNIQUE face in detect(), NOT per playset copy. The no_name_matching lint at features/tests/no_name_matching.rs enforces the forbidden-pattern list and exempts identity lookup. |
CR annotations grep-verified. Every CR comment must match docs/MagicCompRules.txt. |
Hallucinated CR numbers create false confidence. | Run grep -n "^XXX.Y" docs/MagicCompRules.txt for every annotation BEFORE writing. Don't annotate plumbing/utility code. |
Single-authority cost rule. Always ability.cost_categories(), never destructure AbilityCost. |
Scattered cost handling means new cost types break in a dozen places. | When you need "this ability sacrifices a permanent", use cost_categories().contains(&CostCategory::SacrificesPermanent). |
No bool fields. Use existing typed enums (ControllerRef, Comparator, TurnOrder, etc.). |
Booleans aren't composable; an enum is self-documenting. | If you need a "your-vs-opponent" distinction, use ControllerRef::You/Opponent. If you need on-play vs on-draw, use the existing TurnOrder. |
Use crate::ability_chain::collect_chain_effects. |
Single source of truth for ability-chain walking. | Never re-implement collect_chain_effects. If you need to walk a ResolvedAbility chain in a policy, use policies/context.rs::collect_ability_effects (handles sub_ability chain). |
No git add -A or git add .. |
Pre-existing untracked files at the repo root (e.g., parser-batch-plan.md) get accidentally committed. |
Use explicit git add path1 path2 .... |
No unguarded board-wide / affordability engine calls in activation()/verdict(). |
Policies run per-candidate at every AI search node; a board-wide sweep there is a wall-clock landmine that cargo ai-gate cannot see. |
Order predicates cheap-to-expensive: card-local AST checks first, max_x_value / find_legal_targets / mana sweeps last and only once the class is confirmed. See the Performance section. |
Performance — verdict() runs in the AI search inner loop
PolicyRegistry::verdicts() (registry.rs:375-433) runs every policy whose decision_kinds() matches the candidate's DecisionKind, calling activation() then verdict(), for every candidate action, at every node the AI search expands. On a large late-game board that is thousands of verdict() calls per decision. Treat verdict() and everything it transitively calls as an inner-loop function — the same discipline you would apply to code inside the engine's search.
The trap that shipped (x_cast_gate, commit 3de827350d). XCastGatePolicy::verdict() calls engine::game::max_x_value per candidate. max_x_value (casting_costs.rs:7592) walks the whole battlefield calling feasible_mana_capacity per permanent, and for a spell adds a per-X concrete_cost_for_x cost-orchestration loop — a board-wide affordability sweep. Run per candidate per search node on a turn-41 board, it regressed the Court of Grace decision from ~2.1s to ~5.7s (isolated A/B). The policy was rules-correct and passed cargo ai-gate with 0 win-rate flips; the entire cost was wall-clock.
Rules:
Order predicates cheap-to-expensive; short-circuit on card-local AST before anything board-wide. Check the candidate's own
ManaCost/AbilityCost/ effect chain (a handful ofmatches!over one card's AST) first, and reach for a board-wide or affordability engine call only after the card-local checks prove the candidate is even a member of the class you gate.x_cast_gatecorrectly gatesmax_x_valuebehind an{X}-shard check — but any predicate that ANDs a board-wide call with a card-local one must run the card-local one FIRST. AND is commutative, so the ordering is free latency. (Forx_cast_gatespecifically, the card-localno_op_at_x_zeropayoff walk should precede the board-widemax_x_value— it spares the sweep for every{X}card whose payoff has a fixed residual or does not scale with X.)These engine calls are board-wide or state-cloning — never call them unguarded in
activation()/verdict():max_x_value/feasible_mana_capacity(affordability sweep),find_legal_targets(self-heals to a full scan), anySimulationFilter-cloning path, and mana-availability sweeps (the pass-priority mana clone storm and declare-attackers mana sweep are documented quadratic hot paths). Prefer a*_for_simulationvariant where one exists. If you must call one, gate it behind the cheapest structural discriminator and push it as late in the predicate as the logic allows.activation()is the free opt-out; use it. It receives only(features, state, player)— not the candidate — so it cannot inspect the specific card, but it CAN switch the whole policy off for decks/states the policy never fires on (if features.<feat>.commitment < FLOOR { None }). A policy that returnsSome(1.0)unconditionally (anactivation-constantbackstop, likex_cast_gate) pays fullverdict()cost on every matching candidate — acceptable only whenverdict()itself short-circuits cheaply per rule 1.cargo ai-gateis blind to latency, andcargo ai-perf-gateis blind to un-instrumented calls.ai-gatemeasures win-rate only.ai-perf-gate(crates/phase-ai/src/bin/ai_perf_gate.rs) field-wise sums enginePerfCounterSnapshotfields (crates/engine/src/game/perf_counters.rs) againstcrates/phase-ai/baselines/perf-baseline.json— but it only sees paths that bump a counter.max_x_value/feasible_mana_capacitybump none, so BOTH gates missed the regression above; a manual wall-clock A/B caught it. Therefore, if your policy adds a board-wide/affordability engine call that is not already counter-instrumented, EITHER add aPerfCounterSnapshotfield for it (and refresh the perf baseline) soai-perf-gatecan see it, OR run and attach a wall-clock A/B on a large late-game board showing no material regression. A 0-flipai-gateis necessary but NOT sufficient.Wall-clock / deadline guards are inert under
ai-gate— a 0-diff is expected, not a no-op signal. If any code you add gates onself.deadline.expired(), measurement mode nulls the deadline (Deadline::none()), so the guard is a dead branch duringai-gateand the run is byte-identical by construction. Never refresh baselines to absorb a divergence from a deadline-only change — a divergence there is a bug (a guard fired on a live path), not a baseline event. Seeai-duel's Measurement And Gates section for the mechanism and thewith_deadline-on-a-non-measurement-config test requirement.
3. AST type table — where things live
This table is the single biggest research-saver. Every column has been verified against engine source during the 9-feature batch.
| Concept | AST type | File:line |
|---|---|---|
| Card face (the deck-time entity) | CardFace { name, card_type, mana_cost, abilities, triggers, static_abilities, replacements, keywords, power, toughness, ... } |
crates/engine/src/types/card.rs:30+ |
| Core type set | CardFace.card_type.core_types: Vec<CoreType>; CoreType::{Creature, Instant, Sorcery, Land, Artifact, Enchantment, Planeswalker, Battle, Tribal, Kindred} |
card_type.rs:74-77 |
| Mana cost / value | CardFace.mana_cost: ManaCost; ManaCost::mana_value() -> u32 |
card.rs:51; mana.rs:468 |
| Face-level keywords | CardFace.keywords: Vec<Keyword> (including Keyword::Prowess, Keyword::Haste, Keyword::EtbCounter { counter_type, count }, etc.) |
card.rs:60; keywords.rs:24+ |
| Ability definition | CardFace.abilities: Vec<AbilityDefinition>; AbilityDefinition.kind: AbilityKind::{Spell, Activated, Triggered, Static} |
card.rs:62; ability.rs:3749 |
| Ability cost | AbilityDefinition.cost: Option<AbilityCost>. Use ability.cost_categories() (returns Vec<CostCategory>) — never destructure variants. |
ability.rs:1748+ |
| Effect chain | AbilityDefinition.effect: Effect + sub_ability: Option<Box<AbilityDefinition>>. Use crate::ability_chain::collect_chain_effects(ability) to walk. |
ability.rs:2080+ |
| Triggered ability | CardFace.triggers: Vec<TriggerDefinition>; key fields: mode: TriggerMode, valid_card: Option<TargetFilter>, valid_target: Option<TargetFilter>, counter_filter: Option<CounterTriggerFilter>, constraint: Option<TriggerConstraint>, execute: Option<Box<AbilityDefinition>> |
card.rs:61; ability.rs:4515-4600 |
| Trigger modes | TriggerMode::{ChangesZone, SpellCast, SpellCastOrCopy, Attacks, AttackersDeclared, YouAttack, TokenCreated, TokenCreatedOnce, CounterAdded, CounterAddedOnce, CounterAddedAll, CounterTypeAddedAll, ...} |
triggers.rs:24+ |
| Trigger constraints | TriggerConstraint::{NthSpellThisTurn { n, filter }, MaxTimesPerTurn { count }, ...} |
ability.rs:4484+ |
| Static ability | CardFace.static_abilities: Vec<StaticDefinition>. Note: on GameObject the field is static_definitions (different name!). |
card.rs:63; ability.rs:4677-4694 |
| Continuous mods (anthems) | StaticDefinition.modifications: Vec<ContinuousModification>; key variants: AddPower { value }, AddToughness { value }, AddKeyword { keyword }, PowerToughnessAdd, GrantTrigger, AddStaticMode, AddAllCreatureTypes, ... |
ability.rs:5005-5100 |
| Static mode | StaticDefinition.mode: StaticMode::{Continuous, AdditionalLandDrop { count }, MayPlayAdditionalLand, ReduceCost { spell_filter, ... }, ...} |
statics.rs:115+ |
| Replacement effect | CardFace.replacements: Vec<ReplacementDefinition>; event: ReplacementEvent::{ETB, AddCounter, ...}; quantity_modification: Option<QuantityModification::{Double, Plus { value }, Minus { value }}> |
card.rs:64; ability.rs:4818-4874; replacements at replacements.rs:39+ |
| Target filter | TargetFilter::{Any, Player, Opponent, Controller, Typed(TypedFilter), Or { filters }, And { filters }, SelfRef, ...} |
ability.rs:3083+ |
| Typed filter | TypedFilter { type_filters: Vec<TypeFilter>, controller: Option<ControllerRef>, properties: Vec<FilterProp>, ... }; controller-scoping via ControllerRef::{You, Opponent, ...} |
ability.rs:1011-1019, :813-818 |
| Type filter variants | TypeFilter::{Creature, Instant, Sorcery, Land, ..., Subtype(String), AnyOf(Vec<TypeFilter>)} |
ability.rs:794+ |
| Filter properties | FilterProp::{Attacking, Tapped, CountersGE { counter_type, count }, ...} |
ability.rs:834-859 |
| Counter type | CounterType::Plus1Plus1 (typed enum, used in filters/triggers); serialized as string "P1P1" (used in Effect::AddCounter/PutCounter/Token.enter_with_counters/Keyword::EtbCounter). Both sides must be checked when classifying counter cards. |
counter.rs:5-52 |
| Counter trigger filter | CounterTriggerFilter { counter_type: CounterType, threshold: Option<u32> } (no Default impl — construct fields explicitly) |
ability.rs:4501-4513 |
| Game object (runtime) | GameObject { name, controller, zone, abilities, static_definitions, replacement_definitions, card_types, mana_cost, counters: HashMap<CounterType, u32>, keywords, ... }. Note: GameObject has NO triggers field — triggers are registered in the trigger-watcher subsystem. |
game/game_object.rs:25+ |
| Game state | GameState { battlefield, stack, players, objects, current_starting_player, ... }. Stack entries via state.stack.iter(); resolved abilities via entry.ability() -> Option<&ResolvedAbility>. |
game_state.rs:30+ |
| Game action | GameAction::{CastSpell { object_id, ... }, ActivateAbility { source_id, ability_index }, DeclareAttackers { attacks }, PlayLand { ... }, ...} |
actions.rs:20+ |
| WaitingFor (decision context) | WaitingFor::{Priority { player }, DeclareAttackers { valid_attacker_ids, ... }, ChooseTarget { ... }, BetweenGamesSideboard, ...}. Engine pre-computes valid_attacker_ids (CR 508.1a) — use that, don't re-scan. |
game_state.rs:430+ |
GameObject vs CardFace API drift. This bites every feature. CardFace has static_abilities/replacements; GameObject has static_definitions/replacement_definitions. Detection (CardFace) and policy (GameObject) need parallel access — use parts-based predicates (Section 5).
4. Required artifacts per feature
Every feature must produce ALL of these:
- AST verification table in module docstring — one line per axis citing the exact
crates/engine/src/types/...:linewhere the type lives + a CR number where applicable. Mirrorlandfall.rs:1-12andaggro_pressure.rs:1-25. This is your proof that the AST supports detection without parser changes. <Name>Featurestruct — counters asu32, ratios asf32, identity-lookup lists asVec<String>, singlecommitment: f32(0.0..=1.0). NO bool fields. Document each field with a CR if it implements a rule.- Commitment formula with calibration anchor in a doc comment. Commitments are density-normalized per 60 nonland cards so 60-card and 99-card decks with the same density score the same. Use only the shared helpers in
features/commitment.rs:commitment::density_per_60(count, total_nonland),commitment::weighted_sum(&[(weight, density)]), andcommitment::geometric_mean(&[pillar_density]). Show the math AND a real-deck calibration ("Mono-Red Burn: ~16 one-drops + 4 burn → commitment ≈ 0.85"). Anti-calibration ("UW control → commitment ≈ 0.0") is also required. Two formula shapes are established:- Weighted sum, clamped (when missing pillars are tolerable). Pattern: control, aggro_pressure, spellslinger.
- Geometric mean with zero-pillar collapse (when missing a pillar = not this archetype). Pattern: aristocrats, tokens_wide.
- Parts-based predicate exports — one
pub(crate) fn is_<X>_parts(<minimal CardFace slices>) -> boolper axis. Each takes the slice the policy actually has access to onGameObject(e.g.,is_burn_spell_parts(core_types: &[CoreType], abilities: &[AbilityDefinition])). Add a publicis_X(face: &CardFace)wrapper if useful for tests; remove it if dead per clippy. The_partspredicates are the contract — they let the policy classify liveGameObjects without reconstructing aCardFace. - Threshold constants —
pub const. Mirrortribal.rs:47-67:LORD_PRIORITY_FLOOR,MULLIGAN_FLOOR,AGGRO_TEMPO_FLOOR. Each has a docstring explaining the semantic level it gates. - CR annotation table — one CR per rule-bearing comment, every number grep-verified. Don't annotate utility/plumbing code.
- Policy specs — for each tactical/mulligan policy:
id(),decision_kinds(),activation()formula,verdict()reason categories with stablekindstrings. Reason kinds are a frozen identifier set. - Wiring sites — registry/session/plan edits (Section 7).
- Test plan — feature-level (one per axis + calibration anchor + opt-out + clamp), tactical policy (activation gate + each verdict branch), mulligan (each verdict tier).
5. The parts-based predicate convention
This is the architectural heart. Every classifier exists in two shapes:
// Public-facing convenience (used in detect() and tests):
pub fn is_burn_spell(face: &CardFace) -> bool {
is_burn_spell_parts(&face.card_type.core_types, &face.abilities)
}
// Reusable contract (used in policies against GameObject fields):
pub(crate) fn is_burn_spell_parts(
core_types: &[CoreType],
abilities: &[AbilityDefinition],
) -> bool {
abilities.iter().any(|a| {
a.kind == AbilityKind::Spell
&& collect_chain_effects(a)
.iter()
.any(|e| matches!(e, Effect::DealDamage { target, .. }
if filter_can_target_player(target)))
})
}Why parts-based:
- Detection runs against
CardFace(the deck list). - Policies run against
GameObject(the live battlefield/hand/stack). - The fields between them have different names (
static_abilitiesvsstatic_definitions,replacementsvsreplacement_definitions) but the same shape. - Parts predicates take the slice — both call sites pass their respective field. No conversion, no duplication.
Rule of thumb: if your detect() uses face.X, the corresponding parts predicate takes X: &[T] and the policy passes &obj.X (or the equivalently-named runtime field).
6. The TacticalPolicy / MulliganPolicy contract
// crates/phase-ai/src/policies/registry.rs
pub trait TacticalPolicy: Send + Sync {
fn id(&self) -> PolicyId;
fn decision_kinds(&self) -> &'static [DecisionKind];
fn activation(&self, features: &DeckFeatures, state: &GameState, player: PlayerId) -> Option<f32>;
fn verdict(&self, ctx: &PolicyContext<'_>) -> PolicyVerdict;
}
// crates/phase-ai/src/policies/mulligan/mod.rs
pub trait MulliganPolicy: Send + Sync {
fn id(&self) -> PolicyId;
fn evaluate(
&self,
hand: &[ObjectId],
state: &GameState,
features: &DeckFeatures,
plan: &PlanSnapshot,
turn_order: TurnOrder,
mulligans_taken: u8,
) -> MulliganScore;
}activation() patterns:
if features.<feat>.commitment < FLOOR { None } else { Some(features.<feat>.commitment) }— opt-out gate + linear scaling. Standard.- Constant
Some(1.0)requires an// activation-constant: <reason>marker (lint atpolicies/tests/activation_marker_lint.rs).
verdict() patterns:
- Return
PolicyVerdict::{nudge,preference,strong,critical}(ctx.config.policy_penalties.<named_field>, reason)for scores. ReturnPolicyVerdict::reject(reason)only for genuine vetoes. PolicyReason::new("stable_kind_string").with_fact("metric_name", value as i64)— kind is&'static str, frozen per policy.- Mirror
free_outlet_activation.rs:80-94: re-classify the live ability structurally using parts predicates againstobj.abilities/obj.keywords/etc. — don't trust deck-time classification at decision time.
MulliganPolicy::evaluate patterns:
- Opt-out below
MULLIGAN_FLOORreturnsScore { delta: 0.0, reason: <name>_keepables_na }. - Walk
hand(Vec<ObjectId>); look up each viastate.objects.get(&oid). Classify by core type, mana value, identity lookup against featurepayoff_names. - Verdict tiers in priority order (first match returns):
_keepable_ideal(+1.5..2.0),_workable(+0.5..1.0), penalty (-1.0..-1.5), default (0.0). turn_orderandmulligans_takenare real inputs. New policies must consume them or carry an// input-unused: <reason>marker next to the binding.
7. Wiring sites (file:line — read each before editing)
For a feature named <feat> with <Feat>Feature struct and policies <Feat>Policy + <Feat>Mulligan:
| File | Edit |
|---|---|
crates/phase-ai/src/features/mod.rs |
Add pub mod <feat>; + pub use <feat>::<Feat>Feature; + pub <feat>: <Feat>Feature field on DeckFeatures. |
crates/phase-ai/src/features/<feat>.rs |
NEW. The feature module + tests. |
crates/phase-ai/src/features/mod.rs |
Call <feat>::detect(deck) in DeckFeatures::analyze. |
crates/phase-ai/src/policies/mod.rs |
Add pub mod <feat>; (and any new tactical policy module). |
crates/phase-ai/src/policies/<feat>.rs |
NEW. Tactical policy + tests. |
crates/phase-ai/src/policies/registry.rs |
Add PolicyId::<Feat> (and <Feat>Mulligan) variants. Import super::<feat>::<Feat>Policy. Add Box::new(<Feat>Policy) to PolicyRegistry::default's vec. |
crates/phase-ai/src/policies/mulligan/<feat>_keepables.rs |
NEW (if a mulligan policy is in scope). |
crates/phase-ai/src/policies/mulligan/mod.rs |
Add module + pub use + Box::new(<Feat>Mulligan) to MulliganRegistry::default's vec. |
crates/phase-ai/src/config.rs |
Add the policy's scoring scalar(s) to PolicyPenalties, and register each new field in ACTIVE_POLICY_PENALTY_FIELDS or UNTUNED_POLICY_PENALTY_FIELDS. Not optional — a test enforces it. See Section 7a. |
crates/phase-ai/src/plan/curves.rs |
OPTIONAL: add tempo-class branch in tempo_class_for (carefully ordered — see Section 8). Add expected-curve adjustments in expected_lands_for/expected_mana_for/expected_threats_for if archetype shifts the curve. Plan data is consumed by mulligan bottoming and curve expectations; do not add zombie plan fields. |
The no_name_matching lint at features/tests/no_name_matching.rs automatically scans both src/features/ and src/policies/ — no manual exemption needed unless your file names appear in the lint's allow-list (unlikely).
7a. PolicyPenalties registration — ACTIVE vs UNTUNED
Every scoring scalar you add to PolicyPenalties must be listed in exactly one of two sets in crates/phase-ai/src/config.rs. This is enforced, not advisory:
every_policy_penalty_is_tuning_registered_or_explicitly_untuned(config.rs:1342) serializesPolicyPenalties::default()and asserts set equality between its field names andACTIVE_POLICY_PENALTY_FIELDS ∪ UNTUNED_POLICY_PENALTY_FIELDS. A new field in neither set — or a stale name in either set — turnscargo nextest run -p phase-aired (Tilttest-ai; CI runs it workspace-wide via thecargo nextest run --profile ci … --workspacestep in.github/workflows/ci.yml). The same test requires everyUNTUNED_entry's reason string to be non-empty.
Which set? ACTIVE_POLICY_PENALTY_FIELDS (config.rs:617) is the CMA-ES parameter vector: ai_tune builds the --group penalties parameter names from it (bin/ai_tune.rs:102) and policy_penalties_from_params deserializes the optimizer's output back over exactly those fields, clamped to [-15.0, 15.0] (bin/ai_tune.rs:222). Anything listed there is a free variable the optimizer will overwrite.
ACTIVE_— soft heuristics whose magnitude is a preference (nudge/preference/strong band). Only after a paired-seedcargo ai-gatecalibration; that's what every currentUNTUNED_reason is waiting for.UNTUNED_(config.rs:658,(field, reason)pairs) — the default, and the mandatory home for any game-deciding scalar whose value is load-bearing for correctness rather than taste (acritical-band win-detector weight, e.g. a CR 104.2a "last player standing" crown term). Listing such a scalar inACTIVE_licenses CMA-ES to tune a win detector into noise.
When in doubt, ship in UNTUNED_ with a reason. Promotion is a one-line move backed by an ai-gate report; demotion after the optimizer has already smeared the value is not.
8. Plan/curves placement order (critical)
tempo_class_for in plan/curves.rs returns the FIRST matching TempoClass. Order matters because hybrid decks should resolve to a sensible class.
Established order (DON'T break this):
- Ramp first (landfall.commitment > 0.5 OR mana_ramp.commitment > 0.5) →
Ramp. Ramp+anything reads as Ramp. - Tribal (tribal.commitment > AGGRO_TEMPO_FLOOR) →
Aggro. Tribal+aggro reads as Aggro (tribal wins position). - Aggro pressure (aggro_pressure.commitment >= AGGRO_TEMPO_FLOOR) →
Aggro. - Tokens wide (tokens_wide.commitment >= TOKENS_WIDE_TEMPO_FLOOR) →
Aggro. - Control (control.commitment > 0.55 AND control.reactive_tempo > 0.35) →
Control. - Aristocrats (aristocrats.commitment > 0.5) →
Midrange. - Plus-one counters (plus_one_counters.commitment > 0.5) →
Midrange. - Default to archetype.
When inserting a new branch: ramp-style features go FIRST (they dominate), aggro/midrange/control go in the middle, "fallback" archetypes last. Use >= for consistency; the existing > in some branches is being migrated.
9. Recurring review traps (avoid these)
These are real bugs caught in the 9-feature batch's reviews. Each cost a follow-up commit.
| Trap | Caught in | Fix pattern |
|---|---|---|
| CR drift — citing a rule number from memory that's actually a different rule | Counters review caught CR 122.1d (Stun, not +1/+1) and CR 701.27 (Transform, not Proliferate). Aggro caught CR 702.2 (Deathtouch, used for keyword field) and CR 800.4 (multiplayer, used for min-life helper). | Grep docs/MagicCompRules.txt for every CR before writing. Don't annotate plumbing. |
Per-copy name push — payoff_names.push(name) inside for _ in 0..entry.count loop inflates the list to playset size |
Tokens review caught (4× pushes per Bitterblossom). | Hoist the push outside the loop. Counts use entry.count; identity-lookup lists use one push per unique face. |
Combo card double-push — if is_X { names.push() } if is_Y { names.push() } pushes the same name twice when both flags fire |
Tokens review caught (Glorious Anthem + Overrun-shape). | Use if is_X || is_Y { names.push() } once per face. |
Opponent-scoped trigger counted as your payoff — failing to reject valid_target = ControllerRef::Opponent makes "punisher" cards register as your payoffs |
Counters review caught. | Mirror aristocrats::typed_filter_is_creature_you_control's if matches!(typed.controller, Some(ControllerRef::Opponent)) { return false; }. |
Stack inspection only walks head effect — state.stack.iter().for_each(|e| check(&resolved.effect)) misses counter effects in sub_ability |
Counters review caught. | Use super::context::collect_ability_effects(resolved) to walk the full ResolvedAbility chain. |
| Doubler check on activated source — passive replacements (Hardened Scales, Doubling Season) live on permanents, not on activated abilities | Counters review caught the dead branch. | Scan state.battlefield for permanents with replacement_definitions matching the predicate. |
is_X wrapper unused — clippy flags wrappers without callers |
Spellslinger reviewer noted; agent removed in implementation. | Keep _parts predicates always; add public wrappers only if a real caller exists, otherwise remove. |
Loose calibration assertion — assert!(commitment > FLOOR) doesn't enforce the docstring's calibration anchor |
Spellslinger review caught (burn floor was 0.30 vs doc claim 0.40). | Tighten test bound to match the calibration claim. |
Misnamed test — opponent_scope_payoff_ignored actually tested the no-counter-filter case |
Counters review caught. | Verify test names match what they assert. |
git add -A picks up untracked planning files at repo root |
Aggro fix-up commit accidentally included parser-batch-plan.md/parser-fallback-plan.md. |
ALWAYS use explicit git add path1 path2 .... The repo root has untracked files unrelated to AI work. |
10. Cross-feature interaction patterns
When your feature should influence an existing policy, prefer surgical amplification over a new policy:
// Inside an existing policy's verdict() or score():
let amp = 1.0 + (features.<your_feat>.commitment as f64).clamp(0.0, 1.0) * 0.5;
penalty *= amp;Examples shipped:
tokens_wideamplifiesBoardWipeTelegraphPolicy(wide-board decks fear sweepers more).mana_ramp::is_mana_dork_partsis consumed byHoldManaUpForInteractionPolicy(mana sources count untapped Sol Ring shapes).landfall::ability_searches_library_for_landis consumed byaristocrats(anti-fetchland gate prevents fetchlands counting as outlets).
When a helper needs to be shared across two features, promote it to pub(crate) in the owning module with a doc note ("Shared with X for Y"), don't recreate it.
11. Test plan template
Every feature MUST have:
Feature tests:
empty_deck_produces_defaultsvanilla_creature_not_registereddetects_<each_axis>(one per detection axis)<exclusion>_does_not_count(negative cases — e.g.,treasure_token_does_not_count,reach_does_not_count_as_evasion,loyalty_counter_not_counted)opponent_scope_<X>_ignored(controller rejection where applicable)<calibration_anchor_name>_hits_floor(positive calibration:assert!(commitment > 0.85))<negative_archetype>_below_floor(negative calibration)commitment_clamps_to_one(overflow saturation)<identity_list>_dedup(one push per unique face — see Section 9 trap)
Tactical policy tests:
activation_opts_out_below_flooractivation_opts_in_above_floor- One test per
verdictbranch (positive scoring case) - One test per
verdictbranch (negative/penalty case) non_<X>_action_returns_zero(<X>_nareason kind)
Mulligan policy tests:
opts_out_when_commitment_lowideal_hand_<descriptors>→ positive delta with<feat>_keepable_idealreason- Each verdict tier (workable, penalty, default)
- A
turn_order/mulligans_takentest, or an explicit// input-unused: <reason>marker in the implementation.
Measurement evidence:
- New-policy PRs attach a
rtk cargo ai-gatepaired-seed report. No flips is acceptable for narrow policies, but the run must exist. - If the policy calls any board-wide or affordability engine function (
max_x_value,find_legal_targets, mana-availability /SimulationFiltersweeps), ALSO attachcargo ai-perf-gate(or a wall-clock A/B on a large late-game board).ai-gatemeasures win-rate only and is structurally blind to per-decision latency;ai-perf-gateis blind to any call that bumps noPerfCounterSnapshotfield. See the Performance section.
12. Implementation order (each step compiles before the next)
- Read
landfall.rs+ the most-similar reference feature end to end. - Verify each CR number you'll cite by grepping
docs/MagicCompRules.txt. - STEP 0 (if needed): refactor an existing predicate from another feature to
pub(crate)parts shape (e.g.,control::is_card_draw_partswas promoted for spellslinger). Run the existing tests; behavior must be preserved. - Optionally start from
scripts/new-ai-policy.sh <feat>; it writes the three scaffold files and prints the wiring checklist. - Create
features/<feat>.rs: struct + thresholds +detect()+ parts predicates + tests. - Wire
features/mod.rs. - Create
policies/<feat>.rs(tactical) + tests. AddPolicyIdvariant +Box::new()registration. - Create
policies/mulligan/<feat>_keepables.rs+ tests. AddPolicyIdvariant + register inMulliganRegistry::default. - (Optional) Add
plan/curves.rsbranches with tests. - (Optional) Cross-feature amplification (Section 10).
- Run
cargo fmt --all, then use Tilt verification:./scripts/tilt-wait.sh clippy test-ai. If Tilt is down, fall back to the direct phase-ai clippy/test commands. - Run
rtk cargo ai-gatefor new policy work. - Commit with explicit
git add path1 path2 .... NEVERgit add -A. - Spawn an opus advisory review (see
git log --grep="address review"for prior fix-up commit messages as templates). Address MUST FIX in a follow-up commit; CONSIDER items at your discretion.
13. Sample input contract (when invoking this skill)
When calling this skill or asking an agent to implement a new feature, provide:
- Feature name (
<feat>, snake_case). - Detection axes — one bullet per axis with the AST type to inspect (use the table in Section 3).
- Calibration anchor — a real archetype's component counts so the formula can be sanity-checked.
- Policies in scope — tactical only, or also mulligan, and any cross-feature amplification.
- Boundary discussion — overlap with existing features (e.g., "tokens_wide overlaps with aristocrats on token generators; both axes are independent — a deck can score high in both").
The skill provides everything else (struct shape, formula choice, predicate signatures, CR table, wiring, tests, traps).