feat(phase-ai): add devotion deck-feature axis + DevotionPolicy#6602
feat(phase-ai): add devotion deck-feature axis + DevotionPolicy#6602minion1227 wants to merge 1 commit into
Conversation
CR 700.5: devotion to a color is the number of that color's mana symbols
among the mana costs of permanents you control. It is the payoff currency for
the Theros gods (not creatures below their DevotionGE threshold), Gray Merchant
drains, and X = devotion scalers — 43 cards in the corpus read it. The AI
models mana value and board presence but not pip density, so between two
comparable permanents it could not prefer the double-pip one, and it could not
see that casting one more colored permanent flips a dormant god into a body.
Feature (features/devotion.rs) — structural, no card names:
* threshold payoffs: StaticCondition::DevotionGE { colors, threshold },
walked through the Not/And/Or tree (Erebos gates its "isn't a creature" on
Not{DevotionGE}). Every DISTINCT threshold in the primary color is retained
(Vec<u32>), since each god turns on independently.
* scaling payoffs: QuantityRef::Devotion anywhere in an effect's magnitude,
reached via the engine's own Effect::count_expr authority (drains, damage,
token counts, draw counts, dynamic P/T) or a continuous modification
* pip density: ManaCost::count_colored_pips, the single CR 700.5 counting
authority (hybrid {G/W}{G/W} = 2), over permanent faces only
(CR 110.4 permanent-type test via CoreType::is_permanent_type)
primary_color is the deck's most-devoted color among the colors its payoffs
read; a ChosenColor payoff (Nykthos) makes every color eligible so the deck's
own densest color wins.
Commitment is a geometric mean over (payoff, pip): both mandatory — pips with
no payoff is just a mono deck, a payoff with no pips never turns on. Calibrated
to Mono-Black Devotion > 0.85, with a lone-splashed-payoff anti-calibration
below the floor.
Policy scores CastSpell of a permanent by primary-color pips added, with a god-
activation spike for every DevotionGE threshold the cast newly crosses (a cast
can flip several gods at once). The card-local pip check runs first; only a
confirmed primary-color permanent pays for the count_devotion battlefield scan.
No affordability sweep, no find_legal_targets.
Both penalty fields carry #[serde(default = ...)] backed by shared default
functions the Default impl also calls, so older policy_penalties artifacts that
omit them still deserialize; both are registered UNTUNED pending a paired-seed
calibration.
Known safe undercounts (documented): mana-production ramp (Nykthos lives in a
mana-ability production shape, not an Effect magnitude) and a spell's own
"costs {X} less where X is devotion" self-discount (already applied by the
engine). Both only lower commitment.
Boundary with tribal/mana_ramp: devotion counts colored pips, not creatures of
a type or mana sources.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds devotion deck feature detection, commitment scoring, and a registered tactical policy for permanent casts. The policy rewards primary-color pip progress and newly crossed god thresholds, with configurable penalties and backward-compatible tuning defaults. ChangesDevotion AI support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DeckFeaturesAnalyze
participant DevotionDetect
participant DevotionPolicy
participant PolicyRegistry
DeckFeaturesAnalyze->>DevotionDetect: scan deck card faces and permanent pips
DevotionDetect-->>DeckFeaturesAnalyze: return DevotionFeature
PolicyRegistry->>DevotionPolicy: route CastSpell decision
DevotionPolicy->>DevotionPolicy: count devotion and crossed thresholds
DevotionPolicy-->>PolicyRegistry: return PolicyVerdict
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/phase-ai/src/features/devotion.rs`:
- Around line 284-298: Update continuous_modification_quantity to remove the
wildcard _ => None arm and make its match exhaustive over
ContinuousModification. Preserve the existing QuantityExpr-returning variant
mappings so any newly added quantity-carrying variant causes a compile-time
match failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ca579350-d297-4305-a342-a3ff10e766d9
📒 Files selected for processing (10)
crates/phase-ai/src/config.rscrates/phase-ai/src/features/devotion.rscrates/phase-ai/src/features/mod.rscrates/phase-ai/src/features/tests/devotion.rscrates/phase-ai/src/features/tests/mod.rscrates/phase-ai/src/policies/devotion.rscrates/phase-ai/src/policies/mod.rscrates/phase-ai/src/policies/registry.rscrates/phase-ai/src/policies/tests/devotion.rscrates/phase-ai/src/policies/tests/mod.rs
| fn continuous_modification_quantity( | ||
| m: &engine::types::ability::ContinuousModification, | ||
| ) -> Option<&QuantityExpr> { | ||
| use engine::types::ability::ContinuousModification as CM; | ||
| match m { | ||
| CM::SetDynamicPower { value } | ||
| | CM::SetDynamicToughness { value } | ||
| | CM::SetPowerDynamic { value } | ||
| | CM::SetToughnessDynamic { value } | ||
| | CM::AddDynamicPower { value } | ||
| | CM::AddDynamicToughness { value } | ||
| | CM::AddDynamicKeyword { value, .. } => Some(value), | ||
| _ => None, | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Count ContinuousModification variants to judge exhaustive-match effort.
ast-grep run --pattern 'enum ContinuousModification { $$$ }' --lang rust crates/engine/src/types/ability.rs
rg -n 'game::quantity::continuous_modification_dynamic_quantity' crates/phase-ai crates/engineRepository: phase-rs/phase
Length of output: 512
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- ContinuousModification enum ---'
ast-grep outline crates/engine/src/types/ability.rs --match 'enum ContinuousModification' --view expanded || true
echo
echo '--- around enum definition ---'
rg -n -A80 -B20 'enum ContinuousModification' crates/engine/src/types/ability.rs
echo
echo '--- dynamic quantity mirror ---'
rg -n -A40 -B20 'continuous_modification_dynamic_quantity' crates/engine/srcRepository: phase-rs/phase
Length of output: 50370
Make continuous_modification_quantity exhaustive over ContinuousModification
This helper mirrors crates/engine/src/game/quantity.rs::continuous_modification_dynamic_quantity; drop the _ => None arm so any future QuantityExpr-carrying variant fails to compile instead of being ignored here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/phase-ai/src/features/devotion.rs` around lines 284 - 298, Update
continuous_modification_quantity to remove the wildcard _ => None arm and make
its match exhaustive over ContinuousModification. Preserve the existing
QuantityExpr-returning variant mappings so any newly added quantity-carrying
variant causes a compile-time match failure.
Source: Path instructions
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Changes requested — the detector omits part of its declared devotion-payoff class.
🔴 Blocker
[HIGH] Detect every production QuantityRef::Devotion payoff path, or narrow the feature and its claims. Evidence: crates/phase-ai/src/features/devotion.rs:10-11,21-24 explicitly includes Nykthos ramp and cost reductions, but crates/phase-ai/src/features/devotion.rs:248-253 limits detection to Effect::count_expr() and states that exactly those mana-production and cost-reduction reads are not reached. Why it matters: a deck whose devotion payoff is Nykthos (or a devotion-based self-discount) is classified as having no payoff, so its feature commitment stays at zero and the policy never activates. Suggested fix: extend the existing AST quantity authority/visitor to cover every production QuantityExpr carrier used by this feature, including the mana-production and cost-reduction carriers, and add regressions using the parsed card shapes.
[MED] Make continuous_modification_quantity exhaustive. Evidence: crates/phase-ai/src/features/devotion.rs:284-298 uses _ => None despite mirroring the engine quantity authority. Why it matters: a future dynamic ContinuousModification can silently be omitted from devotion detection. Suggested fix: enumerate the non-quantity variants so the compiler forces this scan to be reconsidered with every enum change.
Recommendation: request changes.
2 participants
Tier: Frontier
Model: claude-opus-4-8
Fresh submission of the devotion axis from current
main. The prior PR (#6590) was closed only for a missing canonicalModel:line (the #6584 gate landed shortly before it opened); this body carries both declarations. The three review blockers matthewevans raised on #6590 — serde defaults for persisted-artifact compatibility, retaining every distinct devotion threshold, and reusing the engine permanent-type authority — are already incorporated here.Summary
Adds a devotion deck-feature axis (
DevotionFeature) and its companionDevotionPolicy— CR 700.5 mono-color pip density.CR 700.5: devotion to a color is the number of that color's mana symbols among the mana costs of permanents you control. It is the payoff currency for the Theros gods (not creatures below their
DevotionGEthreshold), Gray Merchant drains, and X = devotion scalers — 43 cards in the corpus read it. The AI models mana value and board presence but not pip density, so between two comparable permanents it could not prefer the double-pip one, and it could not see that casting one more colored permanent flips a dormant god into a body.Three structural detection axes, no card-name matching:
StaticCondition::DevotionGE { colors, threshold }, walked through theNot/And/Ortree. Every distinct threshold in the primary color is retained (Vec<u32>), since each god turns on independentlyQuantityRef::Devotionin an effect magnitude, reached via the engine's ownEffect::count_exprauthority — so drains, damage, token/draw counts and dynamic P/T are all covered without hand-listing effect variantsManaCost::count_colored_pips, the single CR 700.5 counting authority (hybrid{G/W}{G/W}= 2), over permanent faces only (CoreType::is_permanent_type, CR 110.4)primary_coloris the deck's most-devoted color among the colors its payoffs read; aChosenColorpayoff (Nykthos) makes every color eligible, so the deck's densest color wins.Commitment is a geometric mean over (payoff, pip) — both mandatory: pips with no payoff is just a mono deck, a payoff with no pips never turns on. Calibrated to Mono-Black Devotion > 0.85, with a lone-splashed-payoff anti-calibration below the floor.
Policy scores
CastSpellof a permanent by primary-color pips added, with a god-activation spike for everyDevotionGEthreshold the cast newly crosses — a cast can flip several gods at once (surfaced as thegods_activatedfact).Files changed
crates/phase-ai/src/features/devotion.rs(new) ·features/tests/devotion.rs(new)crates/phase-ai/src/policies/devotion.rs(new) ·policies/tests/devotion.rs(new)crates/phase-ai/src/features/mod.rs,policies/mod.rs,policies/registry.rs,policies/tests/mod.rs,features/tests/mod.rs,config.rsCR references
CR 700.5— devotion is the count of a color's mana symbols among your permanentsCR 110.4— the six permanent types (reusedCoreType::is_permanent_type)CR 702.26b— a phased-out permanent's symbols drop out of devotion (honored by the reusedcount_devotion)CR 109.3— conjunction rule for theAndarm of the gate walkEvery number grep-verified against
docs/MagicCompRules.txt.Implementation method (required)
Method: /add-ai-feature-policy
Track
Developer
LLM
Model: claude-opus-4-8
Thinking: high
Performance
verdict()runs per candidate per search node. The card-local check — pips in the candidate's own mana cost plus a permanent-type check — runs first and rejects every non-permanent and off-color cast. Only a confirmed primary-color permanent pays forcount_devotion, one pass over the AI's battlefield permanents. No affordability sweep, nofind_legal_targets.activation()opts the whole policy out below the commitment floor.Verification
cargo test -p phase-ai --lib— 1494 passed; 0 failed (11 feature + 8 policy tests, including a pre-devotion persisted-artifact test and everyverdictbranch — pip progress, single- and double-god-activation crossings, below-threshold, off-color, and the not-a-permanent path — against a realPolicyContext)cargo clippy -p phase-ai --all-targets -- -D warnings— cleancargo fmt --all— cleancargo ai-gate— on the prior submission this was 0 FAIL, and a paired control on the same base produced a byte-identical report (none of the gate matchups is a devotion deck, soDevotionPolicy::activationreturnsNoneand the policy never fires). CI's paired-seed AI gate will re-confirm on this head.Notes
devotion_pip_progressanddevotion_god_activationcarry#[serde(default = "...")]backed by shared default functions theDefaultimpl also calls, and are registeredUNTUNEDpending a paired-seed calibration.features/tests/andpolicies/tests/per the house convention.tribal/mana_ramp: devotion counts colored pips, not creatures of a type or mana sources.Effectmagnitude) and a spell's own "costs {X} less where X is devotion" self-discount (already applied by the engine). Both only lowercommitment.Summary by CodeRabbit
New Features
Bug Fixes
Tests