fix(ai): block menace-class attackers via the engine minimum-blocker authority (#7183) - #7241
fix(ai): block menace-class attackers via the engine minimum-blocker authority (#7183)#7241mike-theDude wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughThe engine now enumerates bounded gang-block candidates for attackers with multiple-blocker requirements. Combat AI uses generalized blocker floors, selects legal gang blocks under lethal pressure, and preserves single-block behavior for unrestricted attackers. ChangesBlocker floor handling
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: 🟠 High · up to This change updates AI blocking for menace and minimum-blocker attackers, but the current implementation can still choose an insufficient gang block against a lethal non-deathtouch trampler, causing the player to die while losing the blockers, and the required paired-seed AI behavior gate has not been run. Merge should wait for the trample correction and verification. Sequence Diagram(s)sequenceDiagram
participant CombatAI
participant EngineBlockRestrictions
participant GangBlockEvaluation
participant BlockerCandidates
CombatAI->>EngineBlockRestrictions: read minimum blocker requirement
CombatAI->>GangBlockEvaluation: evaluate required blocker set
GangBlockEvaluation->>BlockerCandidates: enumerate legal gang-block candidates
BlockerCandidates->>CombatAI: return bounded blocker assignments
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Resolved — commit message amended. (This comment originally said the commit was left unamended to avoid re-queuing CI; that no longer applies.) The commit body previously read "Same for Emrakul, the Aeons Torn (#6874) and every Menace creature," and closed by calling #6874 "the same bug." The Emrakul half was wrong on both counts. Emrakul carries no minimum-blocker floor — Amended
CI restarted from scratch on the new SHA, including the paired-seed AI gate. |
![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/combat_ai.rs`:
- Around line 828-834: The attacker-processing logic around required_blockers
and the later deathtouch/first-strike cleanup must preserve lethal floor-sized
gang blocks even when blockers die or cannot kill the attacker. Allow legal
blocker pairs for floor-restricted deathtouch and first-strike attackers to
reach the lethal stabilization path, retain the minimum-blocker floor, and avoid
rejecting or removing those blocks solely because the attacker survives or
blockers die in first strike. Add regressions covering both attacker classes.
🪄 Autofix
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: 88148971-914f-4615-b9aa-63a8a2e7c5e6
📒 Files selected for processing (4)
crates/engine/src/ai_support/candidates.rscrates/engine/tests/integration/main.rscrates/engine/tests/integration/min_blocker_floor_block_candidates_7183.rscrates/phase-ai/src/combat_ai.rs
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Blocked — the new minimum-blocker survival path still declines legal blocks against deathtouch and first-strike attackers.
🔴 Blocker
[MED] Floored attackers with deathtouch or first strike still receive no life-saving gang block. Evidence: crates/phase-ai/src/combat_ai.rs:1018-1044 filters ordinary blockers against first strike and then unconditionally continues for every deathtouch attacker before the new needed_blockers top-up and gang_stabilize branch. Why it matters: at 6 life, a nontrampling menace/deathtouch attacker (or a min-3 deathtouch attacker) can be legally blocked by its required number of creatures; it remains blocked and cannot assign damage to the player, even though the blockers die. CR 702.2c says any nonzero deathtouch combat damage is lethal, while CR 510.1c says a blocked attacker assigns damage to its blockers and, if none remain, assigns no combat damage. The same conclusion holds for a nontrampling first-striker: CR 702.7b gives it damage only in the first damage step, so killed blockers leave it blocked with no later player damage. Suggested fix: preserve the existing value-saving skip for nonlethal cases, but let the min-floor Stabilize path form a legal full-floor chump block for these nontrampling cases; add discriminating menace/min-3 tests covering deathtouch and first strike.
✅ Clean
The candidate enumerator reads the engine's block_requirements and sends bounded gang proposals through the existing completion authority; the ordinary MinBlockers route is at the right seam.
Request changes: cover the deathtouch/first-strike minimum-floor survival cases, then re-run the current-head review.
…authority (phase-rs#7183) Pathrazer of Ulamog attacks and the AI never blocks it, at any life total. The same holds for every Menace creature. CR 509.1b lets an attacker carry a floor on how many creatures must block it. Two things create one: the Menace keyword (CR 702.111b, floor 2) and a `CantBeBlockedExceptBy { MinBlockers { min } }` static parsed from "can't be blocked except by N or more creatures" (Pathrazer, floor 3). The engine has a single authority for that floor — `min_blockers_required` — which `validate_blockers` enforces and the UI displays. Neither the AI's candidate enumerator nor its combat heuristic read it; both hardcoded the Menace keyword, so a `MinBlockers` attacker was treated as blockable by one creature and every declaration built against it was illegal. Three defects, all fixed by routing through the existing authority: 1. `blocker_actions` seeded only the empty declaration plus every *single* `(blocker, attacker)` pair. Against a floor above 1 all of them are illegal, so `complete_blocker_proposals` rewrote each to the same tax-free witness and dedup collapsed the candidate set to one entry — "don't block". Measured on an 8-blocker board: Pathrazer offered 1 legal declaration, Menace 1, vanilla 9. Now seeds bounded `count`-sized gang combinations per floored attacker; legality still decided downstream. 2. `choose_blockers_with_profile` keyed all six of its passes on `has_keyword(Menace)`, so a `MinBlockers` attacker fell through the single-blocker passes (producing an illegal lone block) and the gang pass computed a floor of 1 for it. The floor now comes from `min_blockers_required_from_precomputed`, hoisted once. Read through that rather than `block_requirements_for_player` because the latter derives from `state.combat`, which hypothetical-block callers may not populate. The gang top-up also loops: menace needs 2, Pathrazer needs 3, and one top-up could not reach an arbitrary floor. 3. A floored attacker is skipped by both single-blocker chump passes, which are the only places carrying a survival override, so the gang pass is its sole blocking route — and that pass gated purely on trade value. The AI held a legal, life-saving three-creature block and died at 6 life to an 11/11. The gang pass now mirrors the single-blocker override for floored attackers only, guard-for-guard: same `effective_life <= power * 3` threshold with the CR 903.10a commander tightening, same `commander_chump_unsafe` rejection (against the gang's combined toughness, CR 702.19b), same damage-reflection exclusion. Unfloored attackers keep their existing value semantics — they already ran the single-blocker passes with the full guard set. Tests: 4 engine integration tests drive `legal_actions_full` at a real declare-blockers prompt (menace floor 2, MinBlockers floor 3, no declaration below any floor, unfloored attackers keep single blocks); 4 phase-ai tests cover the heuristic (gang-blocks at lethal for both floor sources, never under-fills, does not over-block at 40 life). Both MinBlockers arms fail on pre-fix logic. Note: this does not explain the reported *freeze*. That is pinned to `EffectZoneChoice` by phase-rs#6874's console diagnostic; the annihilator sacrifice path itself was exercised across annihilator 3-8, boards of 4-30 permanents, 2- and 4-player pods, both attack directions, and aura/host batches with every enumerated candidate applied — no empty candidate set, no rejected proposal, no `None` from the AI. phase-rs#6874 reports that same sacrifice softlock and has a saved state attached, which is the artifact most likely to settle it. It is NOT fixed by this change: Emrakul carries no minimum-blocker floor (Flying, Protection from spells that are one or more colors, Annihilator 6, and one CantBeCountered static -- no Menace, no CantBeBlockedExceptBy { MinBlockers }), so the blocking fix here does not touch it. Verification: cargo fmt clean; clippy --workspace --all-targets -D warnings clean; phase-engine 18844 lib + 4815 integration green; phase-ai 2075 lib + all integration green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
7b52aa3 to
084ad38
Compare
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Blocked — the current head still rejects legal, life-saving minimum-floor gang blocks against deathtouch and first-strike attackers.
🔴 Blocker
[MED] Floored deathtouch and first-strike attackers still bypass the new survival path. Evidence: combat_ai.rs lines 1018-1036 removes ordinary blockers from effective candidates when a first striker would kill them before their damage step; lines 1038-1044 unconditionally skip every deathtouch attacker before the needed-blockers top-up and gang-stabilize branch. Why it matters: a legal floor-sized block against a nontrampling attacker still prevents all player combat damage even if every blocker dies or cannot kill the attacker. The checked CR text says: CR 510.1c, "If no creatures are currently blocking it ... it assigns no combat damage"; CR 702.2c makes any nonzero deathtouch combat damage lethal; CR 702.7b puts ordinary blockers after the first-strike step. The current tests cover only keyword-free floor attackers, so reverting neither rejection changes their result. Suggested fix: retain the value-preserving skip outside lethal stabilization, but allow the existing min-floor gang-stabilize route to form a legal complete, nontrampling gang block for deathtouch and first/double-strike attackers; add discriminating Menace and MinBlockers-min-3 regressions for both classes.
✅ Clean
The candidate enumerator uses bounded proposals through complete_blocker_proposals, and current head 084ad38 has the same tree as the previous reviewed head; this refreshes the prior finding after the message-only force-push.
Request changes: cover the deathtouch/first-strike minimum-floor survival cases on a new head, then re-run the current-head review.
|
Generated for head Parse changes introduced by this PR✓ No card-parse changes detected. |
…phase-rs#7183) Review follow-up. A floored attacker (Menace, CR 702.111b, or a `MinBlockers` restriction, CR 509.1b) is skipped by both single-blocker passes, so the gang-blocking pass is its only blocking route. Two value heuristics sat in front of the survival override on that route and fired first: - the CR 702.7b first-strike filter emptied the candidate set whenever no blocker survived to the second damage step, leaving the floor unfillable; - the CR 702.2c deathtouch skip `continue`d out of the pass entirely. Both are correct as value heuristics and wrong as survival gates. CR 510.1c: a blocked creature assigns its combat damage to the creatures blocking it, and "if no creatures are currently blocking it (if, for example, they were destroyed or removed from combat), it assigns no combat damage." So for a nontrampling attacker a legal block prevents *every* point of damage to the player whether or not the blockers survive and whether or not they can kill it — a doomed block is still a full save. A menace/deathtouch or min-3 first-striker therefore walked past a full board for lethal. Trample is the boundary (CR 702.19b): excess damage past lethal to the blockers is assigned to the player, and under deathtouch "lethal" is only 1 per blocker (CR 702.2c), so a trampler's damage still lands. The widened route is gated on `!attacker_has_trample` for exactly that reason. Scoped so the value heuristics are unchanged outside the lifesaving route: `doomed_block_still_saves` requires the CR 509.1b floor above 1, the Stabilize objective, and the same `effective_life <= attacker_power * 3` pressure test the single-blocker passes use. Deathtouch attackers are still never ganged for value (`gang_kills_for_value`), and blockers pulled in from the doomed pool are added with zero power so they pad the floor without inflating the kill claim. Adds five regressions crossing both floor sources with both damage-keyword classes, plus the trample boundary. Each is discriminating: reverting the deathtouch guard fails the two deathtouch tests, neutralizing the doomed top-up pool fails the two first/double-strike tests, and dropping the trample term fails the boundary test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@matthewevans — blocker addressed in Root cause. The two value heuristics on the gang pass ran before the survival override, and a floored attacker has no other blocking route:
Fix. Hoisted the lethal-pressure test above both heuristics and introduced
Trample boundary. Per CR 702.19b, excess damage past lethal-to-blockers is assigned to the player, and under deathtouch lethal is only 1 per blocker (CR 702.2c) — so a deathtouch trampler still connects. The widened route excludes tramplers; the pre-existing non-deathtouch trample chump behavior (which absorbs gang toughness) is unchanged. Scoping — no behavior change outside the lifesaving route. Regressions (5 new, crossing both floor sources with both damage-keyword classes plus the boundary):
Each was verified discriminating by reverting the fix piecewise:
The two gang-block tests also assert the declaration survives Verification (Tilt was down, so direct cargo per CLAUDE.md's fallback):
All CR numbers cited here and in the code were verified against Ready for a current-head review. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/combat_ai.rs`:
- Around line 793-818: Run cargo ai-gate for the AI behavior change introduced
around required_blockers and attach the resulting paired-seed report with the
review evidence; do not defer verification to CI.
- Around line 1150-1175: Update gang_stabilize in the gang-block decision to
require the gang’s combined toughness to absorb lethal trampler damage, matching
the single-blocker damage_prevented behavior, while preserving the existing
non-trample and deathtouch handling. Add a regression covering a floored
non-deathtouch trampler whose gang cannot absorb lethal damage, and surface the
relevant MTG rules restriction gap as required by the project guidance.
🪄 Autofix
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: 14a14e64-8caf-4f07-8bf4-4a1cfc3ab8cd
📒 Files selected for processing (1)
crates/phase-ai/src/combat_ai.rs
| // CR 509.1b + CR 702.111b: the minimum number of creatures that must block a | ||
| // given attacker. Menace is only the most common source of that floor — a | ||
| // `MinBlockers` restriction ("can't be blocked except by three or more | ||
| // creatures", e.g. Pathrazer of Ulamog) imposes an arbitrary one, and every | ||
| // pass below must respect it or the declaration it builds is illegal and gets | ||
| // rewritten to the empty witness by `complete_blocker_proposal` (issue #7183). | ||
| // | ||
| // `min_blockers_required` is the engine's single authority for the floor — the | ||
| // same value `validate_blockers` enforces and `block_requirements_for_player` | ||
| // shows the UI — so the AI can never plan against a rule different from the one | ||
| // enforced. Read through it rather than through `block_requirements_for_player` | ||
| // because that helper derives from `state.combat`, which callers that evaluate | ||
| // a hypothetical block (lookahead, tests) may not have populated; the floor is | ||
| // a property of the attacker's own restrictions, not of the combat record. | ||
| // | ||
| // The block-restriction statics are collected once here and threaded through | ||
| // the precomputed variant: every pass below reads this per attacker, and the | ||
| // non-precomputed entry point re-walks the battlefield on each call. | ||
| let block_restriction_statics = engine::game::combat::collect_block_restriction_statics(state); | ||
| let required_blockers = |attacker_id: &ObjectId| -> usize { | ||
| engine::game::combat::min_blockers_required_from_precomputed( | ||
| state, | ||
| *attacker_id, | ||
| &block_restriction_statics, | ||
| ) as usize | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Run the paired-seed AI gate for this behavior change.
This change alters block declarations for a whole attacker class, so it is an AI behavior change. The PR description states that cargo ai-gate was deferred to CI and not run locally. Local deferral is not equivalent to passing verification. Run cargo ai-gate and attach the paired-seed report with the review evidence.
As per coding guidelines, "For AI policy changes, use PolicyVerdict band helpers rather than raw sentinel scores, run cargo ai-gate, and attach the paired-seed report when refreshing baselines." Based on learnings, the same requirement is recorded for AI behavior changes in CLAUDE.md.
🤖 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/combat_ai.rs` around lines 793 - 818, Run cargo ai-gate
for the AI behavior change introduced around required_blockers and attach the
resulting paired-seed report with the review evidence; do not defer verification
to CI.
Sources: Coding guidelines, Learnings
| let gang_toughness: i32 = gang_set | ||
| .iter() | ||
| .filter_map(|bid| state.objects.get(bid).and_then(|b| b.toughness)) | ||
| .sum(); | ||
| let gang_reflects_damage = gang_set.iter().any(|bid| { | ||
| state | ||
| .objects | ||
| .get(bid) | ||
| .is_some_and(has_damage_reflection_to_controller) | ||
| }); | ||
| let gang_stabilize = floor_stabilize_route | ||
| && !gang_reflects_damage | ||
| && !commander_chump_unsafe(state, player, attacker_id, gang_toughness); | ||
|
|
||
| // CR 702.2c: never gang a deathtouch attacker for *value* — every blocker | ||
| // assigned any damage dies, so the kill is paid for with the whole gang. | ||
| // Preserves the pre-existing skip for deathtouch attackers now that the | ||
| // survival route above no longer short-circuits them out of the pass. | ||
| let gang_kills_for_value = !attacker_has_deathtouch | ||
| && combined_power >= attacker_toughness | ||
| && gang_value <= attacker_value; | ||
|
|
||
| // Only gang-block if combined power can kill AND total value risked <= | ||
| // attacker value — or if declining loses the game outright. Never below the | ||
| // CR 509.1b floor, which would make the declaration illegal. | ||
| if gang_set.len() >= needed_blockers && (gang_kills_for_value || gang_stabilize) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add the trample absorption term to gang_stabilize.
floor_stabilize_route is true for trampling attackers. doomed_block_still_saves excludes trample, but gang_stabilize does not. A non-deathtouch trampler with a floor above 1 therefore reaches this gate with no check that the gang absorbs the damage. Deathtouch tramplers exit earlier at Line 1078, so they are unaffected.
Result: at lethal pressure the AI commits a floor-sized gang whose combined toughness is below attacker_power, the excess tramples to the player under CR 702.19b, the player still dies, and the creatures are lost. The single-blocker pass avoids this because it computes damage_prevented as blocker_toughness for tramplers and requires damage_prevented >= 2. The comment above claims guard-for-guard parity with that pass, but the trample term is missing.
Add a regression for a floored non-deathtouch trampler whose gang cannot absorb lethal. The existing menace_deathtouch_trampler_is_not_chump_ganged_at_lethal test passes through the deathtouch skip, so it does not cover this route.
As per path instructions, surface MTG rules gaps for the restriction class and check trample boundaries.
🐛 Proposed fix: require trample absorption before the survival override
+ // CR 702.19b: a trampler assigns only lethal damage to its blockers and
+ // pushes the excess to the player, so the gang must absorb enough of the
+ // attacker's power for the block to save the player at all.
+ let gang_absorbs_lethal = !attacker_has_trample
+ || effective_life > attacker_power.saturating_sub(gang_toughness);
let gang_stabilize = floor_stabilize_route
+ && gang_absorbs_lethal
&& !gang_reflects_damage
&& !commander_chump_unsafe(state, player, attacker_id, gang_toughness);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let gang_toughness: i32 = gang_set | |
| .iter() | |
| .filter_map(|bid| state.objects.get(bid).and_then(|b| b.toughness)) | |
| .sum(); | |
| let gang_reflects_damage = gang_set.iter().any(|bid| { | |
| state | |
| .objects | |
| .get(bid) | |
| .is_some_and(has_damage_reflection_to_controller) | |
| }); | |
| let gang_stabilize = floor_stabilize_route | |
| && !gang_reflects_damage | |
| && !commander_chump_unsafe(state, player, attacker_id, gang_toughness); | |
| // CR 702.2c: never gang a deathtouch attacker for *value* — every blocker | |
| // assigned any damage dies, so the kill is paid for with the whole gang. | |
| // Preserves the pre-existing skip for deathtouch attackers now that the | |
| // survival route above no longer short-circuits them out of the pass. | |
| let gang_kills_for_value = !attacker_has_deathtouch | |
| && combined_power >= attacker_toughness | |
| && gang_value <= attacker_value; | |
| // Only gang-block if combined power can kill AND total value risked <= | |
| // attacker value — or if declining loses the game outright. Never below the | |
| // CR 509.1b floor, which would make the declaration illegal. | |
| if gang_set.len() >= needed_blockers && (gang_kills_for_value || gang_stabilize) { | |
| let gang_toughness: i32 = gang_set | |
| .iter() | |
| .filter_map(|bid| state.objects.get(bid).and_then(|b| b.toughness)) | |
| .sum(); | |
| let gang_reflects_damage = gang_set.iter().any(|bid| { | |
| state | |
| .objects | |
| .get(bid) | |
| .is_some_and(has_damage_reflection_to_controller) | |
| }); | |
| // CR 702.19b: a trampler assigns only lethal damage to its blockers and | |
| // pushes the excess to the player, so the gang must absorb enough of the | |
| // attacker's power for the block to save the player at all. | |
| let gang_absorbs_lethal = !attacker_has_trample | |
| || effective_life > attacker_power.saturating_sub(gang_toughness); | |
| let gang_stabilize = floor_stabilize_route | |
| && gang_absorbs_lethal | |
| && !gang_reflects_damage | |
| && !commander_chump_unsafe(state, player, attacker_id, gang_toughness); | |
| // CR 702.2c: never gang a deathtouch attacker for *value* — every blocker | |
| // assigned any damage dies, so the kill is paid for with the whole gang. | |
| // Preserves the pre-existing skip for deathtouch attackers now that the | |
| // survival route above no longer short-circuits them out of the pass. | |
| let gang_kills_for_value = !attacker_has_deathtouch | |
| && combined_power >= attacker_toughness | |
| && gang_value <= attacker_value; | |
| // Only gang-block if combined power can kill AND total value risked <= | |
| // attacker value — or if declining loses the game outright. Never below the | |
| // CR 509.1b floor, which would make the declaration illegal. | |
| if gang_set.len() >= needed_blockers && (gang_kills_for_value || gang_stabilize) { |
🤖 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/combat_ai.rs` around lines 1150 - 1175, Update
gang_stabilize in the gang-block decision to require the gang’s combined
toughness to absorb lethal trampler damage, matching the single-blocker
damage_prevented behavior, while preserving the existing non-trample and
deathtouch handling. Add a regression covering a floored non-deathtouch trampler
whose gang cannot absorb lethal damage, and surface the relevant MTG rules
restriction gap as required by the project guidance.
Source: Path instructions
3 participants
Fixes #7183.
Related to #6874, but not a fix for it: Emrakul carries no minimum-blocker floor (
Flying,Protection(spells that are one or more colors),Annihilator 6, and oneCantBeCounteredstatic — no Menace, noCantBeBlockedExceptBy { MinBlockers }), so the blocking fix below does not touch it. The two issues share only the annihilator sacrifice softlock, which this PR does not fix — see the last section.Model: claude-opus-5[1m]
Tier: Frontier
Thinking: High
Problem
Pathrazer of Ulamog attacks and the AI never blocks it, at any life total. The same holds for every Menace creature.
CR 509.1b lets an attacker impose a floor on how many creatures must block it. Two things create one: the Menace keyword (CR 702.111b, floor 2) and a
CantBeBlockedExceptBy { MinBlockers { min } }static parsed from "can't be blocked except by N or more creatures" (Pathrazer, floor 3).The engine already has a single authority for that floor —
min_blockers_required— whichvalidate_blockersenforces andblock_requirements_for_playershows the UI. Neither the AI's candidate enumerator nor its combat heuristic read it; both hardcoded the Menace keyword. Pathrazer carries no Menace keyword, so it was treated as blockable by one creature and every declaration the AI built against it was illegal, then silently rewritten to the empty declaration bycomplete_blocker_proposal.Three defects, all fixed by routing through the existing authority
1.
blocker_actionscould not express a legal gang block. It seeded only the empty declaration plus every single(blocker, attacker)pair. Against a floor above 1 all of them are illegal, socomplete_blocker_proposalsrewrote each to the same tax-free witness and dedup collapsed the whole candidate set to one entry — "don't block". Measured on an 8-blocker board:DeclareBlockers(before → after)Now seeds bounded
count-sized combinations per floored attacker, drawn from that attacker's own eligible blockers. Legality is still decided downstream bycomplete_blocker_proposals, which remains the single authority.2.
choose_blockers_with_profilekeyed all six passes onhas_keyword(Menace). AMinBlockersattacker fell through the single-blocker passes (producing an illegal lone block) and the gang pass computed a floor of 1 for it. The floor now comes frommin_blockers_required_from_precomputed, hoisted once for the whole call. Read through that rather thanblock_requirements_for_playerbecause the latter derives fromstate.combat, which callers evaluating a hypothetical block (lookahead, tests) may not have populated — the floor is a property of the attacker's own restrictions, not of the combat record. The gang top-up also loops now: menace needs 2, Pathrazer needs 3, and a single top-up could never reach an arbitrary floor.3. The gang pass had no survival override. A floored attacker is skipped by both single-blocker chump passes, which are the only places carrying
should_chump_stabilize— so the gang pass is its sole blocking route, and it gated purely on trade value. Concretely: at 6 life facing an 11/11, the AI held a legal three-creature block and declined it becausegang_value=30 > attacker_value=26, then died. The gang pass now mirrors the single-blocker override for floored attackers only, guard-for-guard: the sameeffective_life <= attacker_power * 3threshold with the CR 903.10a commander-damage tightening, the samecommander_chump_unsaferejection (evaluated against the gang's combined toughness per CR 702.19b), and the same damage-reflection exclusion. Unfloored attackers keep their existing value semantics — they already ran the single-blocker passes with the full guard set, so widening the gate for them would double-count their chump decision.Anchored on
crates/engine/src/game/combat.rs:5982—min_blockers_required, the engine's stated single authority for the floor ("validate_blocksenforces it andblock_requirements_for_playersurfaces it to the UI, so the count a player sees can never disagree with the count the engine enforces"). Both fixes read the floor through this rather than re-deriving it.crates/engine/src/game/combat.rs:2105—complete_blocker_proposals, the existing legality/witness authority thatblocker_actionsalready routed through; the new gang seeds go through the same call unchanged.crates/engine/src/ai_support/candidates.rs:4516—bounded_select_card_candidates, the existing bounded-combination pattern (pool_cap+output_capoverbounded_combinations_for_sizes); the gang-block seeding mirrors it, including the siblingGANG_BLOCK_*cap constants placed besideSELECTION_*.crates/phase-ai/src/combat_ai.rs:933— the single-blockershould_chump_stabilizeoverride the gang-pass override is mirrored from, guard-for-guard.Tests
Four engine integration tests (
crates/engine/tests/integration/min_blocker_floor_block_candidates_7183.rs) drive the reallegal_actions_fullat a genuine declare-blockers prompt: menace floor 2 reachable,MinBlockersfloor 3 reachable, no enumerated declaration below any floor (2/3/4), and unfloored attackers keep their single-blocker candidates.Four
combat_aitests cover the heuristic: gang-blocks at lethal for both floor sources, never under-fills a floor it cannot satisfy, and does not over-block at 40 life (guards against the fix over-correcting).Both
MinBlockersarms were verified to fail on pre-fix logic by temporarily reverting each change independently. The two menace tests pass before and after — they are class-sibling regression guards, not bug proofs, and are labelled as such.Gate A
Verification
cargo fmt --all --checkcleancargo clippy --workspace --all-targets -- -D warningscleancargo test -p phase-engine— 18844 lib + 4815 integration + 30 others, 0 failurescargo test -p phase-ai— 2075 lib + all integration, 0 failuresNo AI behavior baselines were refreshed;
cargo ai-gatehas not been run for this change (see below).What this PR does not fix
It does not explain the reported freeze, and #7183/#6874 should stay open until that is settled. This PR makes the AI play the card correctly; the softlock is a separate failure at a different decision point.
#6874 pins it via its console diagnostic —
AI controller halted after 3 failed proposals on EffectZoneChoice— i.e. the annihilator sacrifice prompt, not blocking. I exercised that path across annihilator 3–8, boards of 4–30 permanents, 2- and 4-player pods, both attack directions, and aura/host batches, applying every enumerated candidate rather than just the AI's pick: no empty candidate set, no rejected proposal, noNonefrom the AI. Also ruled out: chooser misrouting,priority_playerdesync, and aDeclareBlockerswedge (complete_blocker_proposalsalways emits a witness, so that legal set is never empty). The remaining candidates are a panic insidegetAiActionor state I could not synthesize — #6874 has a saved game state attached, which would settle it.Two adjacent quality issues observed but deliberately left alone, as out of scope here:
SELECTION_POOL_CAP = 12means sacrifice candidates only ever come from the first 12 eligible permanents, so on a wide board most of the pool is unreachable to both the AI and (via the enumeration gate) a human in a server game.Since this changes AI combat behavior,
cargo ai-gatewith a paired-seed report is worth running before merge — I did not run it.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes