fix(parser): parse subtype-gated token substitution replacements (Fisher's Talent #5636)#5883
Conversation
Fisher's Talent's levels 2 and 3 ("If you would create a Fish token,
create a 3/3 blue Shark creature token instead"; Shark -> Octopus) lowered
to inert Spell-kind Token abilities with replacements: 0 -- a Spell-kind
ability on a permanent never fires, so the levels silently did nothing.
parse_replacement_line had no arm for the "if you would create a <subtype>
token, create <token> instead" one-for-one substitution shape (the token
dispatch only matched "a token" / "one or more tokens"; Xorn and Manufactor
handle the plus / one-of-each shapes). Add parse_subtype_token_substitution,
composing the Xorn subtype gate (ReplacementCondition::TokenSubtypeMatches)
with the Divine Visitation substitution payload (the substitute Effect::Token
in `execute`). The applier swaps only the token spec and keeps the proposed
event's count, so N <subtype> tokens become N substitute tokens. The arm
requires the first-person "you would create" antecedent so token_owner_scope
(You) is provably correct (CR 109.5); oracle_class wraps the result with the
class-level gate.
Verified: Fisher's Talent now emits two Mandatory CreateToken substitution
replacements (Fish->Shark, Shark->Octopus), and the L2->L3 cascade sequences
correctly through the replacement loop.
Closes phase-rs#5636
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements support for parsing subtype-gated mandatory token substitutions (such as Fisher's Talent: 'If you would create a token, create instead') by introducing the parse_subtype_token_substitution helper and adding corresponding unit tests. The review feedback correctly identifies a violation of the L2 Sibling coverage rule, pointing out that the implementation lacks support for vowel-starting token subtypes (e.g., 'an Elf token' or 'an Octopus token') in both the fast-path check and the parser combinator logic.
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.
| if nom_primitives::scan_contains(&lower, "you would create a ") | ||
| && nom_primitives::scan_contains(&lower, "instead") | ||
| { |
There was a problem hiding this comment.
[HIGH] Missing support for vowel-starting token subtypes (e.g., 'an Elf token', 'an Octopus token').
Why it matters: The current fast-path check only matches 'you would create a ', which means any replacement effects for token subtypes starting with a vowel will be silently ignored and fail to parse.
This violates the L2 Sibling coverage rule in the Repository Style Guide because grammatical variants like 'an' (for vowel-starting subtypes) are not covered.
if (nom_primitives::scan_contains(&lower, "you would create a ")
|| nom_primitives::scan_contains(&lower, "you would create an "))
&& nom_primitives::scan_contains(&lower, "instead")
{References
- L2 Sibling coverage: Ensure grammatical variants like 'an' are covered when extending parser arms. (link)
| let ((subtype, descriptor), _) = nom_on_lower(lower, lower, |i| { | ||
| let (i, _) = take_until::<_, _, OracleError<'_>>("you would create a ").parse(i)?; | ||
| let (i, _) = tag("you would create a ").parse(i)?; | ||
| let (i, subtype) = take_until::<_, _, OracleError<'_>>(" token, ").parse(i)?; | ||
| let (i, _) = tag(" token, create ").parse(i)?; | ||
| let (i, descriptor) = take_until::<_, _, OracleError<'_>>(" instead").parse(i)?; | ||
| Ok(( | ||
| i, | ||
| (subtype.trim().to_string(), descriptor.trim().to_string()), | ||
| )) | ||
| })?; |
There was a problem hiding this comment.
[HIGH] Missing support for vowel-starting token subtypes in the parser combinator logic.
Why it matters: The parser strictly expects 'you would create a ' and will fail to parse 'you would create an ' for vowel-starting subtypes like Elf, Insect, or Octopus.
This violates the L2 Sibling coverage rule in the Repository Style Guide because grammatical variants like 'an' (for vowel-starting subtypes) are not covered.
| let ((subtype, descriptor), _) = nom_on_lower(lower, lower, |i| { | |
| let (i, _) = take_until::<_, _, OracleError<'_>>("you would create a ").parse(i)?; | |
| let (i, _) = tag("you would create a ").parse(i)?; | |
| let (i, subtype) = take_until::<_, _, OracleError<'_>>(" token, ").parse(i)?; | |
| let (i, _) = tag(" token, create ").parse(i)?; | |
| let (i, descriptor) = take_until::<_, _, OracleError<'_>>(" instead").parse(i)?; | |
| Ok(( | |
| i, | |
| (subtype.trim().to_string(), descriptor.trim().to_string()), | |
| )) | |
| })?; | |
| let ((subtype, descriptor), _) = nom_on_lower(lower, lower, |i| { | |
| let (i, _) = take_until::<_, _, OracleError<'_>>("you would create ").parse(i)?; | |
| let (i, _) = tag("you would create ").parse(i)?; | |
| let (i, _) = nom::branch::alt((tag("a "), tag("an "))).parse(i)?; | |
| let (i, subtype) = take_until::<_, _, OracleError<'_>>(" token, ").parse(i)?; | |
| let (i, _) = tag(" token, create ").parse(i)?; | |
| let (i, descriptor) = take_until::<_, _, OracleError<'_>>(" instead").parse(i)?; | |
| Ok(( | |
| i, | |
| (subtype.trim().to_string(), descriptor.trim().to_string()), | |
| )) | |
| })?; |
References
- L2 Sibling coverage: Ensure grammatical variants like 'an' are covered when extending parser arms. (link)
Parse changes introduced by this PR · 1 card(s), 4 signature(s) (baseline: main
|
…ubtypes
The subtype-gated token-substitution frame matched only "you would create a
<subtype> token", so a vowel-starting subtype ("an Elf token", "an Octopus
token") was silently dropped -- an L2 sibling-coverage gap. Consume
"you would create " then alt(("a ", "an ")), and widen the fast-path guard to
the "you would create a" prefix (which covers both articles; the exact frame
still rejects "another ..."). No shipped card uses the "an" antecedent today,
so this is class completeness (no card-data change); a regression test covers
the "an <vowel-subtype>" form.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed the vowel-starting-subtype finding in 4dc26b4: the frame now consumes |
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Verdict: Approved
🔴 Blocker
- None.
🟡 Non-blocking
- None.
✅ Clean
- The parser lowers to the existing
CreateToken/TokenSubtypeMatchesreplacement authority rather than adding new replacement machinery. - The current head resolves the
a/ansibling feedback, and the four Fisher's Talent parse changes exactly match the intended two replacement levels replacing the former bare token effects.
Recommendation: Merge via the queue.
3 participants
Closes #5636.
Problem
Fisher's Talent's levels 2/3 — "If you would create a Fish token, create a 3/3 blue Shark creature token instead" (and Shark→Octopus) — parsed to
replacements: 0plus two inertSpell-kind Token abilities. A Spell-kind ability on a permanent never fires, so levels 2 and 3 silently did nothing in shipped data (the card still counted as supported).Fix
parse_replacement_linehad no arm for the subtype-gated one-for-one token substitution shape "if you would create a<subtype>token, create<token>instead". The existing token-creation arms cover doubling/additional shapes (Doubling Season "a token" / "one or more tokens"; Xorn "instead create those tokens plus"; Manufactor "instead create one of each") but not substitution.Add
parse_subtype_token_substitution, composing two existing, runtime-supported mechanisms:ReplacementCondition::TokenSubtypeMatches) so it fires only for the named subtype, andEffect::Tokencarried inexecute; the applier (game/replacement.rs) swaps only the token's characteristics and keeps the proposed event's count, so N<subtype>tokens become N substitute tokens.The arm requires the first-person "you would create" antecedent so the
token_owner_scope(You)is provably correct (an "if an effect / an opponent would create … instead" line is a different scope and is not captured).oracle_class.rswraps the result with the class-level gate, so the swap is active only at the level that grants it.No new engine types or variants — a pure parser addition.
Verification
subtype_gated_token_substitution_fishers_talentdrivesparse_replacement_lineand asserts theCreateTokenevent,TokenSubtypeMatches([Fish])condition,token_owner_scope(You), exactMandatorymode, and the 3/3 blue Shark substitute — plus negative cases proving it does not steal the Manufactor comma-list shape or a non-substitution "instead" effect.cargo test -p engine --lib(new test) — green.CreateTokensubstitution replacements (Fish→Shark, Shark→Octopus); the L2→L3 cascade sequences correctly (create-Fish → Shark → Octopus) through the replacement loop.cargo clippy -p engine --lib -- -D warnings— clean.Model: claude-opus-4-8[1m]