fix(parser): parse "play up to <n> additional lands this turn" (Summer Bloom #5979)

[xfodev]

Fixes #5979.

Summary

Summer Bloom ({1}{G} sorcery — "You may play up to three additional lands this turn.") never granted its extra land plays. The turn-scoped additional-land grant parser recognized play an additional land and play <n> additional lands but not the "up to" hedge form.

Root cause is a two-site interaction:

1. parse_additional_land_head (used by is_specialized_duration_carrier) did not match play up to <n> additional lands, so the shell treated the clause as a non-duration-carrier and stripped the trailing this turn into the clause duration.
2. try_parse_additional_land_this_turn then required a literal this turn tag that was already gone, so the clause fell through instead of becoming an AdditionalLandDrop grant.

Both parse sites now accept an optional up to before the count.

Rules

CR 305.2: a "play up to <n> additional lands this turn" grant is functionally identical to a bare "<n> additional lands" grant — playing lands is already optional, so the "up to" hedge grants the same +n land-play allowance. It therefore maps to AdditionalLandDrop { count: n }, matching the existing count-n branch.

Class, not card

The fix is grammar-level: any "play up to <n> additional lands this turn" card is now covered, not just Summer Bloom. The opt(tag("up to ")) composes into the existing count alt() — no new AST shape, no per-card branch.

Anchored on

• crates/engine/src/parser/oracle_effect/mod.rs:11818 — existing alt() count branch ((parse_number, tag(" additional lands"))) that I extended with opt(tag("up to ")).
• crates/engine/src/parser/clause_shell.rs:459 — existing parse_additional_land_head alt() (("play ", parse_number, " additional lands")) extended the same way, keeping the duration-carrier detector in sync with the effect parser.

Same nom combinator family (alt/tag/opt/value), same naming, same modules.

Test

Added summer_bloom_up_to_three_additional_lands_parses_count in oracle_effect/tests.rs, mirroring the existing play_two_additional_lands_this_turn_parses_count: asserts the clause parses to a GenericEffect carrying AdditionalLandDrop { count: 3 } affecting the controller, UntilEndOfTurn, non-optional.

Verification

• Gate A (parser combinator purity): PASS — see below. New code uses only nom combinators (alt/tag/opt/value/map).
• Track: Non-developer. This sandbox has no C toolchain (cc/ld absent) and no root to install one, so a local cargo/clippy/test build is not possible here. CI will run the full suite. The change is a two-line grammar extension using combinators already in scope, and the added test is a near-exact copy of the existing, passing play_two_additional_lands_this_turn_parses_count (same asserts, count 3 instead of 2, up to in the input).
• cargo fmt --all run clean (no diff).

Gate A

Gate A PASS head=04d731fa398abc659dc974463e01fb2aa0cb2320 base=51cff6adbb783c64559f2c004a206fc4f0968f40

Model: claude-opus-4-8
Thinking: High
Tier: Frontier

[gemini-code-assist]

Code Review

This pull request updates the parse_additional_land_head parser in crates/engine/src/parser/clause_shell.rs to support parsing the phrase "play up to additional lands" by making the "up to " segment optional using the opt combinator. It also adds a comment referencing CR 305.2 to document this rule. There are no review comments, and I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[github-actions]

Parse changes introduced by this PR · 2 card(s), 3 signature(s) (baseline: main 51cff6adbb78)

🟢 Added (2 signatures)

• 1 card · ➕ ability/AdditionalLandDrop(2) · added: AdditionalLandDrop(2) (affects=controller, duration=until end of turn, grants=AdditionalLandDrop(2))
  • Affected (first 3): Journey of Discovery
• 1 card · ➕ ability/AdditionalLandDrop(3) · added: AdditionalLandDrop(3) (affects=controller, duration=until end of turn, grants=AdditionalLandDrop(3))
  • Affected (first 3): Summer Bloom

🔴 Removed (1 signature)

• 2 cards · ➖ ability/CastFromZone · removed: CastFromZone (duration=until end of turn, target=any target)
  • Affected (first 3): Journey of Discovery, Summer Bloom

1 card(s) had Oracle-text changes (errata/reprint) — excluded as non-parser.

[matthewevans]

Blocked: the regression does not cover the second affected parser context.

🔴 Blocker

[MED] The new regression only parses Summer Bloom's standalone sentence. Evidence: crates/engine/src/parser/oracle_effect/tests.rs:22147; the fresh coverage artifact also identifies Journey of Discovery, whose Choose one modal bullet reaches this grammar through a distinct full-card parsing context. Why it matters: both cards changed from CastFromZone to AdditionalLandDrop, so a standalone sentence assertion cannot keep the modal-card path from regressing. Suggested fix: add a registered regression that parses Journey of Discovery's full Oracle text and asserts its second mode produces AdditionalLandDrop { count: 2 } through the production card parser.

✅ Clean

• The optional up to segment is modeled with the existing nom::combinator::opt parser at both grammar seams; no new engine surface or one-off string dispatch was introduced.
• Required CI is green and the current coverage artifact supports the intended two-card grammar class.

[xfodev]

Thanks @matthewevans — addressed in 176194f.

Added a registered regression through the production card parser (crates/engine/src/parser/oracle_tests.rs, journey_of_discovery_modal_up_to_two_additional_lands) that parses Journey of Discovery's full Oracle text — Choose one — • Search your library for up to two basic land cards… • You may play up to two additional lands this turn. Entwine {2}{G} — and asserts:

• modal.mode_count == 2 (Entwine registers as a keyword line, not a third mode),
• r.abilities.len() == 2 (one ability per mode),
• the second mode parses to Effect::GenericEffect carrying StaticMode::AdditionalLandDrop { count: 2 } — i.e. the modal-mode path no longer misroutes to CastFromZone.

This exercises the distinct modal-bullet parsing context alongside the existing standalone-sentence regression, so neither the Summer Bloom path nor the Journey of Discovery path can silently regress.

Gate A PASS head=176194fc94a48b8530a475a5b2b3b6e9c1a1cfaf base=51cff6adbb783c64559f2c004a206fc4f0968f40

[matthewevans]

Blocked: the modal parser regression fixes the requested context gap, but the supported card path still lacks a runtime assertion.

🔴 Blocker

[MED] journey_of_discovery_modal_up_to_two_additional_lands stops after inspecting the parsed StaticMode at crates/engine/src/parser/oracle_tests.rs:3777-3812. It never casts the spell or observes the transient land-drop effect that the production resolver must register. Why it matters: this parser change changes coverage from CastFromZone to supported AdditionalLandDrop; an AST shape assertion cannot prove the parser-produced GenericEffect reaches additional_land_drops through apply()/stack resolution. Suggested fix: add a registered integration regression using GameScenario + GameRunner::cast(...).resolve() with Summer Bloom's exact Oracle text and assert additional_land_drops(outcome.state(), P0) == 3; follow crates/engine/tests/integration/explore_spell_3315.rs:38-68 for the existing runtime pattern.

✅ Clean

• The new Journey of Discovery full-card regression covers the previously missing modal context and would fail on the pre-fix parser.
• The only new commit since the prior head changes that regression file; the existing two-card parse-diff remains explained by the already-reviewed grammar change.

Recommendation: add the single runtime regression, then request re-review. The contributor test gate is documented in docs/AI-CONTRIBUTOR.md.

[xfodev]

Thanks @matthewevans — runtime assertion added in bffd99d.

New registered integration regression crates/engine/tests/integration/summer_bloom_5979.rs (wired into tests/integration/main.rs) casts Summer Bloom's exact Oracle text through the production resolver and asserts the transient land-drop effect is actually registered:

let spell = scenario
    .add_spell_to_hand_from_oracle(P0, "Summer Bloom", false,
        "You may play up to three additional lands this turn.")
    .with_mana_cost(ManaCost::zero())
    .id();
let outcome = scenario.build().cast(spell).resolve();
assert_eq!(additional_land_drops(outcome.state(), P0), 3);

Follows the explore_spell_3315.rs pattern you referenced. This proves the parser-produced GenericEffect reaches additional_land_drops via cast(...).resolve() (the transient-continuous-effect path in static_abilities::additional_land_drops), not just that the AST shape is AdditionalLandDrop.

Coverage now: parser structure (Summer Bloom standalone + Journey of Discovery modal context) and runtime resolution. Gate A PASS head=bffd99dc23cf31902a05315359e208160f8ba336.

Note on local verification: my sandbox has no C linker + no root, so I bootstrapped a zig cc Rust linker — dependency compilation succeeds, but this 7.7 GB host OOMs (SIGKILL) linking the multi-thousand-test engine binaries, so I'm relying on CI for the full test run. The runtime test mirrors explore_spell_3315.rs exactly (differing only in Oracle text and the expected count of 3).

[xfodev]

Local verification update: I got a memory-fitting harness working (standalone binary linked against the prebuilt test-support engine rlib), and ran the exact runtime scenario from the new test:

additional_land_drops = 3
PASS

So the cast(...).resolve() path is confirmed locally — Summer Bloom's grant reaches additional_land_drops and sums to 3 — not just relied on CI. The committed integration test (summer_bloom_5979.rs) encodes this same assertion.

[matthewevans]

Reviewed current head 211ade160ae54d37446cffdb403b59358a477175: the parser change uses the existing nom seams, the two-card parse delta is explained, and the registered Summer Bloom scenario proves the resolver grants three additional land drops at runtime.