ship/featui show storm copy counts - #7041
Merged
Merged
Conversation
matthewevans
enabled auto-merge
August 5, 2026 19:20
Contributor
📝 WalkthroughWalkthroughStorm triggers now carry typed copy-count provenance through the engine and API. Derived views calculate prospective counts for the viewing player’s hand. Desktop, mobile, and stack components display localized Storm badges. Parser and integration tests cover source scope and trigger-count behavior. ChangesStorm provenance flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CastResolution
participant TriggerGeneration
participant DerivedViews
participant Client
CastResolution->>TriggerGeneration: preserve cast-time Storm instances
TriggerGeneration->>DerivedViews: expose Storm provenance and copy count
DerivedViews->>Client: provide hand counts and stack provenance
Client->>Client: render localized Storm badges
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
Generated for head Parse changes introduced by this PR · 23 card(s), 9 signature(s) (baseline: main
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
client/src/components/hand/MobileHandDrawer.tsx (1)
307-314: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated Storm copy-count badge into one shared component. All three sites render the identical
bg-violet-700 rounded-fullcount badge with the samet("storm.copies", { count })title, differing only in position and text size. Consolidate into a single presentational component that accepts acountand a position/size variant.
client/src/components/hand/MobileHandDrawer.tsx#L307-L314: replace this<span>block with the shared badge component, passingstormCopyCountand the drawer-card position/size variant.client/src/components/hand/MobileHeldHandCard.tsx#L136-L143: replace this<span>block with the shared badge component, passingstormCopyCountand the held-card position/size variant.client/src/components/hand/PlayerHand.tsx#L1013-L1020: replace this<span>block with the shared badge component, passingstormCopyCountand the fanned hand-card position/size variant.🤖 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 `@client/src/components/hand/MobileHandDrawer.tsx` around lines 307 - 314, Extract the repeated Storm copy-count badge into a shared presentational component accepting count and position/size variants, preserving the shared violet styling and translated title. Replace the badge in client/src/components/hand/MobileHandDrawer.tsx#L307-L314 with the drawer-card variant, client/src/components/hand/MobileHeldHandCard.tsx#L136-L143 with the held-card variant, and client/src/components/hand/PlayerHand.tsx#L1013-L1020 with the fanned hand-card variant.
🤖 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/engine/src/game/triggers.rs`:
- Around line 4224-4237: Update the comment immediately above the
storm_instances calculation to describe Storm as a triggered ability that
functions on the stack, matching CR 702.40a/b. Keep the existing explanation
that Storm instances are frozen at cast time and must not be re-evaluated from
live spell keywords; change only the inaccurate “spell ability”
characterization.
In `@crates/engine/src/parser/oracle_static/evasion.rs`:
- Around line 198-209: Update the runtime matching logic associated with doubler
source filters and TypeFilter::Permanent so it matches only battlefield
permanents, excluding permanent spells on the stack. For leave-the-battlefield
sources, resolve the source through last-known-information or zone-change
history before applying the permanent check, and add a runtime test covering
spell sources.
In `@crates/engine/src/parser/oracle_static/tests.rs`:
- Around line 12086-12087: Update the explanatory comment near the Storm trigger
behavior to include a Verified Comprehensive Rules citation in the format “CR
<number>: <description>”. Use the appropriate rule number and describe that
Storm is preserved because its synthesized trigger consumes every cast-time
instance.
In `@crates/engine/tests/integration/granted_storm_snapshot.rs`:
- Around line 18-20: Correct the rules citation in the Storm grant test comment
by replacing CR 601.2f with CR 601.2a, while preserving the existing CR 611.2f
and CR 702.40a references and explanatory text.
In `@crates/engine/tests/integration/veyran_storm_source_scope.rs`:
- Around line 42-45: Add a positive reach guard before the existing
storm_triggers assertion by using runner.cast(...).commit() to verify Veyran
doubles a cast-triggered ability from a controlled permanent. Keep the
storm_triggers == 1 assertion afterward to confirm Storm remains excluded
because its source is the spell, and ensure both checks exercise the production
pipeline.
---
Nitpick comments:
In `@client/src/components/hand/MobileHandDrawer.tsx`:
- Around line 307-314: Extract the repeated Storm copy-count badge into a shared
presentational component accepting count and position/size variants, preserving
the shared violet styling and translated title. Replace the badge in
client/src/components/hand/MobileHandDrawer.tsx#L307-L314 with the drawer-card
variant, client/src/components/hand/MobileHeldHandCard.tsx#L136-L143 with the
held-card variant, and client/src/components/hand/PlayerHand.tsx#L1013-L1020
with the fanned hand-card variant.
🪄 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: 99a7fba9-55c4-4731-89e5-6f5d93372e67
📒 Files selected for processing (64)
client/src/adapter/types.tsclient/src/components/hand/MobileHandDrawer.tsxclient/src/components/hand/MobileHeldHandCard.tsxclient/src/components/hand/PlayerHand.tsxclient/src/components/stack/StackEntry.tsxclient/src/i18n/locales/de/game.jsonclient/src/i18n/locales/en/game.jsonclient/src/i18n/locales/es/game.jsonclient/src/i18n/locales/fr/game.jsonclient/src/i18n/locales/it/game.jsonclient/src/i18n/locales/pl/game.jsonclient/src/i18n/locales/pt/game.jsoncrates/engine/src/ai_support/mod.rscrates/engine/src/analysis/loop_check.rscrates/engine/src/analysis/resource.rscrates/engine/src/game/ability_utils.rscrates/engine/src/game/archenemy_tests.rscrates/engine/src/game/casting.rscrates/engine/src/game/cipher.rscrates/engine/src/game/derived_views.rscrates/engine/src/game/effects/bounce.rscrates/engine/src/game/effects/change_zone.rscrates/engine/src/game/effects/copy_spell.rscrates/engine/src/game/effects/counter.rscrates/engine/src/game/effects/mod.rscrates/engine/src/game/effects/transform_effect.rscrates/engine/src/game/effects/venture.rscrates/engine/src/game/elimination.rscrates/engine/src/game/engine.rscrates/engine/src/game/engine_exile_return_tests.rscrates/engine/src/game/engine_keyword_action_stack_tests.rscrates/engine/src/game/engine_tests.rscrates/engine/src/game/engine_trigger_target_tests.rscrates/engine/src/game/filter.rscrates/engine/src/game/planechase.rscrates/engine/src/game/planechase_tests.rscrates/engine/src/game/priority.rscrates/engine/src/game/sba.rscrates/engine/src/game/stack.rscrates/engine/src/game/static_abilities.rscrates/engine/src/game/trigger_matchers.rscrates/engine/src/game/triggers.rscrates/engine/src/game/triggers_dedup_regression_tests.rscrates/engine/src/game/triggers_ordering_parity_tests.rscrates/engine/src/game/triggers_pr7_order_template_tests.rscrates/engine/src/game/triggers_push_first_contract_tests.rscrates/engine/src/parser/oracle_static/evasion.rscrates/engine/src/parser/oracle_static/snapshot_tests.rscrates/engine/src/parser/oracle_static/tests.rscrates/engine/src/types/game_state.rscrates/engine/src/types/keywords.rscrates/engine/tests/integration/game_state_boxed_ability_serde.rscrates/engine/tests/integration/granted_storm_snapshot.rscrates/engine/tests/integration/integration_bending.rscrates/engine/tests/integration/issue_3282_consign_to_memory_counter.rscrates/engine/tests/integration/issue_5983_sothera_dies_edict.rscrates/engine/tests/integration/kamigawa_flip_cards.rscrates/engine/tests/integration/louisoix_sacrifice_counter.rscrates/engine/tests/integration/main.rscrates/engine/tests/integration/veyran_storm_source_scope.rscrates/engine/tests/integration/vivien_invocation_reflexive_power.rscrates/phase-ai/src/policies/anti_self_harm.rscrates/phase-ai/src/policies/evasion_removal_priority.rscrates/server-core/src/filter.rs
Comment on lines
+4224
to
+4237
| // CR 702.40a/b: Storm is a spell ability, so its instances are | ||
| // frozen when the spell is cast. Do not re-evaluate live spell | ||
| // keywords after the cast event: a conditional grant may no longer | ||
| // match once the spell itself has entered the cast ledger. | ||
| let storm_instances = state | ||
| .objects | ||
| .get(cast_obj_id) | ||
| .map(|obj| { | ||
| obj.cast_spell_keywords | ||
| .iter() | ||
| .filter(|keyword| matches!(keyword, Keyword::Storm)) | ||
| .count() | ||
| }) | ||
| .unwrap_or_default(); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the CR 702.40a citation: it does not describe Storm as "a spell ability."
The comment states: "Storm is a spell ability, so its instances are frozen when the spell is cast." CR 702.40a actually reads: "Storm is a triggered ability that functions on the stack. "Storm" means "When you cast this spell, copy it for each other spell that was cast before it this turn." "Spell ability" is a distinct CR 113.3a classification, and Storm is not one. Update the comment to accurately describe Storm as a triggered ability that functions on the stack, so the CR annotation matches the rule it cites.
The underlying engineering rationale (snapshot the keyword count at cast time rather than re-querying live keywords) still holds; only the rule characterization is wrong.
As per path instructions for crates/engine/**: "rules-touching code with no verified CR <number>: <description> annotation, or a CR citation whose rule body does not describe the code."
📝 Proposed comment fix
- // CR 702.40a/b: Storm is a spell ability, so its instances are
- // frozen when the spell is cast. Do not re-evaluate live spell
- // keywords after the cast event: a conditional grant may no longer
- // match once the spell itself has entered the cast ledger.
+ // CR 702.40a/b: Storm is a triggered ability that functions on the
+ // stack, and its instance count is fixed when the spell is cast.
+ // Do not re-evaluate live spell keywords after the cast event: a
+ // conditional grant may no longer match once the spell itself has
+ // entered the cast ledger.📝 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.
Suggested change
| // CR 702.40a/b: Storm is a spell ability, so its instances are | |
| // frozen when the spell is cast. Do not re-evaluate live spell | |
| // keywords after the cast event: a conditional grant may no longer | |
| // match once the spell itself has entered the cast ledger. | |
| let storm_instances = state | |
| .objects | |
| .get(cast_obj_id) | |
| .map(|obj| { | |
| obj.cast_spell_keywords | |
| .iter() | |
| .filter(|keyword| matches!(keyword, Keyword::Storm)) | |
| .count() | |
| }) | |
| .unwrap_or_default(); | |
| // CR 702.40a/b: Storm is a triggered ability that functions on the | |
| // stack, and its instance count is fixed when the spell is cast. | |
| // Do not re-evaluate live spell keywords after the cast event: a | |
| // conditional grant may no longer match once the spell itself has | |
| // entered the cast ledger. | |
| let storm_instances = state | |
| .objects | |
| .get(cast_obj_id) | |
| .map(|obj| { | |
| obj.cast_spell_keywords | |
| .iter() | |
| .filter(|keyword| matches!(keyword, Keyword::Storm)) | |
| .count() | |
| }) | |
| .unwrap_or_default(); |
🤖 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/engine/src/game/triggers.rs` around lines 4224 - 4237, Update the
comment immediately above the storm_instances calculation to describe Storm as a
triggered ability that functions on the stack, matching CR 702.40a/b. Keep the
existing explanation that Storm instances are frozen at cast time and must not
be re-evaluated from live spell keywords; change only the inaccurate “spell
ability” characterization.
Source: Path instructions
Comment on lines
+198
to
+209
| /// CR 603.2d: A doubler `affected` filter must name a source-domain constraint. | ||
| /// `Permanent` is a real constraint because spells are not permanents; `Card` | ||
| /// and `Any` alone do not constrain the source. A clause is therefore valid | ||
| /// when it carries a permanent or concrete type/subtype restriction, or a | ||
| /// property such as "another" / "of the chosen type". | ||
| fn doubler_source_is_restrictive(filter: &TargetFilter) -> bool { | ||
| match filter { | ||
| TargetFilter::Typed(tf) => { | ||
| tf.type_filters.iter().any(|t| { | ||
| !matches!( | ||
| t, | ||
| TypeFilter::Permanent | TypeFilter::Card | TypeFilter::Any | ||
| ) | ||
| }) || !tf.properties.is_empty() | ||
| tf.type_filters | ||
| .iter() | ||
| .any(|t| !matches!(t, TypeFilter::Card | TypeFilter::Any)) | ||
| || !tf.properties.is_empty() |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 10 \
'apply_trigger_doubling|matches_target_filter|type_filter_matches|TypeFilter::Permanent|lki_cache|Zone::Battlefield' \
crates/engine/src --glob '*.rs'Repository: phase-rs/phase
Length of output: 50371
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== relevant files =="
git ls-files 'crates/engine/src/game/*.rs' 'crates/engine/src/types/game_state.rs' 'crates/engine/src/parser/oracle_static/evasion.rs' | sed -n '1,120p'
echo
echo "== evasion parser section =="
cat -n crates/engine/src/parser/oracle_static/evasion.rs | sed -n '160,230p'
echo
echo "== TypeFilter definition =="
rg -n -C 6 'enum TypeFilter|TypeFilter::Permanent|CoreType::' crates/engine/src/types crates/engine/src/game --glob '*.rs' | sed -n '1,240p'
echo
echo "== trigger and source matcher section locations =="
rg -n -C 8 'struct Trigger|trigger.source_id|TypeFilter::Permanent|type_filter_matches|matches_target_filter|match_target|target_filter' crates/engine/src/game --glob '*.rs' | sed -n '1,260p'Repository: phase-rs/phase
Length of output: 49949
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== parser type phrase Permanent references =="
rg -n -C 8 'TypeFilter::Permanent|Permanent|parse_type_phrase|typed_filter' cratesngineengine crates/engine/src/parser crates/engine/src/types --glob '*.rs' | sed -n '1,260p'
echo
echo "== trigger doublers and affected filter usages =="
rg -n -C 10 'DoubleTriggers|affected|apply_trigger_doubling|trigger_source_context|triggers_dedup|source_id|matches_target_filter|type_filter_matches_object' crates/engine/src/game crates/engine/src/types --glob '*.rs' | sed -n '1,420p'Repository: phase-rs/phase
Length of output: 24883
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== exact TypeFilter enum =="
rg -n -C 20 '^pub enum TypeFilter|enum TypeFilter' crates/engine/src --glob '*.rs'
echo
echo "== exact TargetFilter and Typed definition =="
rg -n -C 20 '^pub enum TargetFilter|pub struct TypedFilter|struct TypedFilter' crates/engine/src --glob '*.rs'
echo
echo "== trigger_index relevant =="
cat -n crates/engine/src/game/trigger_index.rs | sed -n '1,140p'
echo
echo "== statics DoubleTriggers relevant =="
rg -n -C 20 'DoubleTriggers|affected|cause' crates/engine/src/types/statics.rs | sed -n '3240,3305p'Repository: phase-rs/phase
Length of output: 18018
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== filter module list =="
git ls-files 'crates/engine/src/game/filter*.rs' | sed -n '1,40p'
echo
echo "== filter::matches_target_filter source =="
rg -n -C 25 'pub.*fn matches_target_filter|matches_target_filter|type_filter_matches_object|TypeFilter::Permanent|InZone|filter_props|properties' crates/engine/src/game/filter.rs crates/engine/src/game/*.rs --glob '*.rs' | sed -n '1,520p'
echo
echo "== trigger collector and doubler source locations =="
rg -n -C 12 'DoubleTriggers|apply_trigger_doubling|collect_pending_triggers|trigger_source_context|trigger_source_context_for_latch|TriggerCause|cause|affected' crates/engine/src/game crates/engine/src/types --glob '*.rs' | sed -n '1,520p'
echo
echo "== parser static evasion relevant lines =="
cat -n crates/engine/src/parser/oracle_static/evasion.rs | sed -n '220,360p'Repository: phase-rs/phase
Length of output: 50370
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== exact parser static evasion lines =="
sed -n '220,340p' crates/engine/src/parser/oracle_static/evasion.rs
echo
echo "== exact TypeFilter enum =="
sed -n '3809,3835p' crates/engine/src/types/ability.rs
echo
echo "== exact TargetFilter enum start =="
sed -n '5095,5175p' crates/engine/src/types/ability.rs
echo
echo "== filter matches_target_filter definitions =="
rg -n -C 30 'pub fn matches_target_filter|fn matches_target_filter|pub fn object_matches_target|fn object_matches_target|type_filter_matches_object|TypeFilter::Permanent|InZone|properties' crates/engine/src/game/filter.rs crates/engine/src/game --glob '*.rs' | sed -n '1,800p'
echo
echo "== trigger doublers exact lines =="
rg -n -C 20 'DoubleTriggers|DoubleTrigger|apply_trigger_doubling|affected|valid_card|DoubleTriggers' crates/engine/src/game crates/engine/src/types --glob '*.rs' | sed -n '1,700p'Repository: phase-rs/phase
Length of output: 50371
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== file size/outline for filter =="
wc -l crates/engine/src/game/filter.rs
ast-grep outline crates/engine/src/game/filter.rs --match matches_target_filter --view expanded || true
echo
echo "== matches_target_filter implementation =="
rg -n -C 40 'pub fn matches_target_filter|fn matches_target_filter|matches_target_filter\(' crates/engine/src/game/filter.rs | sed -n '1,600p'
echo
echo "== trigger matcher implementation =="
rg -n -C 40 'pub fn .*matches|fn .*matches|target_filter_matches|matches_target_filter|TypeFilter::Permanent|zone' crates/engine/src/game/triggers.rs crates/engine/src/game --glob 'triggers*.rs' | sed -n '1,900p'
echo
echo "== exact trigger collector and doubler implementation =="
rg -n -C 35 'DoubleTriggers|TriggerCause::DoubleTriggers|DoubleTrigger|collect_pending_triggers|trigger_source_context|trigger_source_context_for_latch|apply_trigger_doubling|affected' crates/engine/src/game crates/engine/src/types --glob '*.rs' | sed -n '1,700p'Repository: phase-rs/phase
Length of output: 50371
Make TypeFilter::Permanent restrict doubles to battlefield permanents.
CR 403.3 defines permanents as objects on the battlefield. A permanent spell has a permanent card type, but it is not a permanent. “Permanent” is accepted as a trigger-doubler source filter, so the runtime matcher must skip stack objects and use LKI zone/zone-change history for leave-the-battlefield sources. Add a runtime test for spell sources to cover the current parser assertion gap.
🤖 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/engine/src/parser/oracle_static/evasion.rs` around lines 198 - 209,
Update the runtime matching logic associated with doubler source filters and
TypeFilter::Permanent so it matches only battlefield permanents, excluding
permanent spells on the stack. For leave-the-battlefield sources, resolve the
source through last-known-information or zone-change history before applying the
permanent check, and add a runtime test covering spell sources.
Source: Path instructions
Comment on lines
+12086
to
+12087
| // remains excluded because its cast-grant count is not consumed, while Storm is | ||
| // preserved because its synthesized trigger consumes every cast-time instance. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate file and relevant lines =="
wc -l crates/engine/src/parser/oracle_static/tests.rs
sed -n '12050,12110p' crates/engine/src/parser/oracle_static/tests.rs | cat -n
echo
echo "== Search for nearby CR citations in parser/tests context =="
rg -n "702\.40[ab]|Storm|cast time|triggers? separately" crates/engine/src/parser crates/engine/src -S
echo
echo "== Check for existing CR style in parser/tests =="
rg -n "CR [0-9]+(\.[0-9]+)*:" crates/engine/src/parser -S | head -40Repository: phase-rs/phase
Length of output: 50370
🌐 Web query:
Magic Comprehensive Rules 2026 CR 702.40a Storm copy count CR 702.40b Storm triggers separately
💡 Result:
According to the Magic: The Gathering Comprehensive Rules, the storm ability is defined under rule 702.40 [1][2]. As of August 2026, the relevant subsections are: 702.40a: Storm is a triggered ability that functions on the stack. "Storm" means "When you cast this spell, copy it for each other spell that was cast before it this turn. If the spell has any targets, you may choose new targets for any of the copies." [1][2] 702.40b: If a spell has multiple instances of storm, each triggers separately [1][2]. Key mechanics of these rules include: - The copies created by the storm ability are placed directly onto the stack and are not considered cast [1]. - Because they are not cast, these copies do not generate their own storm triggers and are not counted by other storm spells cast later in the turn [1]. - If a spell with storm has any targets, you are permitted to choose new targets for each of the individual copies [1].
Citations:
- 1: https://mtg.fandom.com/wiki/Storm
- 2: https://ancestral.vision/additional-rules/keyword-abilities.html
Add a Verified Comprehensive Rules annotation.
Add a CR <number>: <description> citation for the Storm trigger behavior documented by this comment.
🤖 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/engine/src/parser/oracle_static/tests.rs` around lines 12086 - 12087,
Update the explanatory comment near the Storm trigger behavior to include a
Verified Comprehensive Rules citation in the format “CR <number>:
<description>”. Use the appropriate rule number and describe that Storm is
preserved because its synthesized trigger consumes every cast-time instance.
Sources: Path instructions, MCP tools
Comment on lines
+18
to
+20
| /// CR 601.2f + CR 611.2f + CR 702.40a: a Storm grant that only applies before | ||
| /// the caster has cast a spell this turn is latched before the spell enters the | ||
| /// cast ledger, then produces its trigger from that snapshot. |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Correct the CR citation.
Replace CR 601.2f with CR 601.2a. CR 601.2f defines total-cost determination. The cast-time characteristic behavior in this test is covered by CR 601.2a and CR 611.2f. (media.wizards.com)
As per path instructions, rules-touching engine code requires a verified CR citation whose rule body describes the code.
🤖 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/engine/tests/integration/granted_storm_snapshot.rs` around lines 18 -
20, Correct the rules citation in the Storm grant test comment by replacing CR
601.2f with CR 601.2a, while preserving the existing CR 611.2f and CR 702.40a
references and explanatory text.
Source: Path instructions
Comment on lines
+42
to
+45
| assert_eq!( | ||
| storm_triggers, 1, | ||
| "Veyran must not double Storm because Storm belongs to the spell, not a permanent" | ||
| ); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add a positive reach guard for Veyran.
storm_triggers == 1 is only a negative assertion. If VEYRAN_DOUBLER_ORACLE is unimplemented, or its static effect is inactive, this assertion still passes.
First verify through runner.cast(...).commit() that Veyran doubles a cast-triggered ability of a controlled permanent. Then retain this assertion to verify that Storm is excluded because its source is the spell on the stack.
As per path instructions, a negative assertion requires a paired positive reach guard through the production pipeline.
🤖 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/engine/tests/integration/veyran_storm_source_scope.rs` around lines 42
- 45, Add a positive reach guard before the existing storm_triggers assertion by
using runner.cast(...).commit() to verify Veyran doubles a cast-triggered
ability from a controlled permanent. Keep the storm_triggers == 1 assertion
afterward to confirm Storm remains excluded because its source is the spell, and
ensure both checks exercise the production pipeline.
Source: Path instructions
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes