diff --git a/.claude/skills/oracle-parser/SKILL.md b/.claude/skills/oracle-parser/SKILL.md index 6fd4c0e56b..198fbe0bd4 100644 --- a/.claude/skills/oracle-parser/SKILL.md +++ b/.claude/skills/oracle-parser/SKILL.md @@ -416,7 +416,7 @@ Unlabeled handlers interleaved between labeled slots are shown as `—` rows. | `0` | Semicolon-separated keyword line ("Defender; reach"); colon guard excludes activated abilities | per-part keyword extraction | `oracle.rs` | | `1` | Modal block: "Choose one —" header + mode lines, or Spree + `+` lines (consumes multiple lines) | `parse_oracle_block()` + `lower_oracle_block()` | `oracle_modal.rs` | | — | "Equip {cost}" / "Equip — {cost}" (not "Equipped …"); "Crew N" with trailing cadence sentence | `try_parse_equip()`, `parse_crew_keyword()` | `oracle.rs` | -| `1b` | Keyword-only line (guard: "{kw} abilities you activate cost {N} less" is a static, not a keyword line) | `extract_granted_keyword_list()` (permissive — see §3a) | `oracle_keyword.rs` | +| `1b` | Keyword-only line (guard: "{kw} abilities you activate cost {N} less" is a static, not a keyword line) | `parse_router_keyword_list()` (strict — see §3a) | `oracle_keyword.rs` | | `2` | "Enchant {filter}" | skip (handled externally) | — | | — | Commander-permission / deck-construction copy-limit sentences (skip); named equip " — Equip {cost}" | `try_parse_equip()` | `oracle.rs` | | `11` | Planeswalker loyalty `+N:` / `−N:` / `0:` / `[+N]:` (runs here despite the label) | `try_parse_loyalty_line()` | `oracle.rs` | @@ -466,6 +466,8 @@ the wrong one at a router boundary is the silent-swallow bug class. |---|---|---| | `parse_keyword_line_core()` | Remainder-**preserving** core: `Option<(Keyword, &str /* unconsumed */)>`. The single authority both wrappers are built on. | Internal — call a wrapper, not this. | | `parse_router_keyword_line()` | **STRICT / all-consuming.** Candidate recognizer → reminder strip → core → `all_consuming` permitted tail (P/R/M modifiers). Returns a typed `RoutedKeywordLine` only when the line parses *completely*. | The **only** surface that may license a router to CONSUME a whole line. | +| `parse_router_keyword_fragment()` | **STRICT.** One keyword phrase: core + all-consuming permitted tail + modifiers. The primitive the other strict surfaces are built on. | Any router intercept parsing a single keyword phrase (flashback, suspend, specialize, buyback, escalate, commander ninjutsu, …). | +| `parse_router_keyword_list()` | **STRICT.** The keyword-LIST sibling: comma parts, MTGJSON validation, protection expansion — with every part all-consuming. | Router slots and routing classifiers facing a keyword *list* (priorities 0 and 1b, `is_semicolon_keyword_line`, `is_spell_resolution_instruction_line`). `parse_router_keyword_line` cannot serve here: it parses ONE keyword and takes no MTGJSON names, so it cannot see a bare-keyword line ("Flying, vigilance"). | | `parse_granted_keyword_fragment()` / `extract_granted_keyword_list()` | **PERMISSIVE.** Takes the leading keyword and **discards the remainder** — by design. | **Embedded grant contexts only**: static/token/vote/class-level/effect-payload payloads ("…gains vanishing 3 if …"), where a trailing clause belongs to the *enclosing* sentence. | **Consume-on-success (the rule).** A candidate recognizer (`is_keyword_cost_line`) @@ -485,16 +487,15 @@ strict parser fails the build. `KNOWN_NOUN_PARAM_LEAKS` is a **ratchet** listing families whose noun/filter parameter still absorbs a trailing clause — entries are deleted as each is fixed, never added to. -**Migration status (accurate as of Plan 02 step 5).** The strict router is wired at -priority `9` (spell) and priority `13` (permanent). The remaining keyword-cost router -entries — priority `0`, `1b`, `8f`, the flashback/suspend/buyback/escalate/commander-ninjutsu -intercepts, and the two line classifiers — **still call the permissive surface** and are -therefore still capable of swallowing a semantic tail. That surviving set is enumerated -and frozen by **Gate G** in `scripts/check-parser-combinators.sh`: it counts the permissive -calls inside `parse_oracle_ir`, `is_semicolon_keyword_line`, and -`is_spell_resolution_instruction_line` and fails if the count rises (a new permissive -router call) *or* falls without lowering the gate's expected floor. Gate G is a ratchet, -not a blessing — when every count reaches 0, the boundary is fully enforced. +**The boundary is fully enforced.** Every router slot and routing classifier now parses +through a strict surface; the permissive symbols are not even imported into `oracle.rs`. +**Gate G** in `scripts/check-parser-combinators.sh` is the plain whole-file invariant: a +permissive keyword-parser symbol appearing anywhere inside `parse_oracle_ir`, +`is_semicolon_keyword_line`, or `is_spell_resolution_instruction_line` fails the build, at +any count. Note the ordering consequence this closed: priority `1b` runs long before the +strict routers at `9`/`13`, so while it was permissive it claimed keyword lines first +whenever MTGJSON named the keyword — which silently shadowed the strict wiring downstream +for exactly the cards MTGJSON knows about. ### `is_static_pattern()` — `oracle_classifier.rs` Gates Priority `7`. Returns false for `target`-leading lines, then matches diff --git a/crates/engine/src/parser/oracle.rs b/crates/engine/src/parser/oracle.rs index b1e22bac7f..fd461d8e76 100644 --- a/crates/engine/src/parser/oracle.rs +++ b/crates/engine/src/parser/oracle.rs @@ -68,8 +68,8 @@ use super::oracle_ir::feature::ItemIdTracks; use super::oracle_ir::relation::{DocumentRelationIr, LinkedChoiceKind}; pub use super::oracle_keyword::keyword_display_name; use super::oracle_keyword::{ - extract_granted_keyword_list, is_keyword_cost_line, parse_granted_keyword_fragment, - parse_kicker_additional_cost_line, parse_router_keyword_line, + is_keyword_cost_line, is_kicker_family_line, parse_kicker_additional_cost_line, + parse_router_keyword_fragment, parse_router_keyword_line, parse_router_keyword_list, }; use super::oracle_level::parse_level_blocks; use super::oracle_modal::{ @@ -2148,6 +2148,11 @@ fn is_standalone_spell_keyword_action_line(line: &str) -> bool { parsed } +/// A classifier MAY probe with the STRICT parser; it may NOT probe with a helper +/// that discards the remainder. This one gates a routing decision (priority 0 and +/// the spell-resolution guard), so a permissive probe would report "this whole line +/// is keywords" about a line carrying an unparsed semantic clause — and the router +/// would then consume it. fn is_semicolon_keyword_line(line: &str, mtgjson_keyword_names: &[String]) -> bool { let mut saw_multiple_parts = false; let mut parts = line @@ -2158,13 +2163,13 @@ fn is_semicolon_keyword_line(line: &str, mtgjson_keyword_names: &[String]) -> bo return false; }; - if extract_granted_keyword_list(first, mtgjson_keyword_names).is_none() { + if parse_router_keyword_list(first, mtgjson_keyword_names).is_none() { return false; } for part in parts { saw_multiple_parts = true; - if extract_granted_keyword_list(part, mtgjson_keyword_names).is_none() { + if parse_router_keyword_list(part, mtgjson_keyword_names).is_none() { return false; } } @@ -2201,8 +2206,12 @@ fn is_spell_resolution_instruction_line( return false; } + // Strict probe: a line is only "not a spell resolution instruction, it's a + // keyword line" when it parses COMPLETELY as keywords. A permissive probe here + // reports true for "Cycling {2} if you control an artifact" and the conditional + // tail is never parsed by anything. if !is_ability_activate_cost_static(&lower) - && extract_granted_keyword_list(line, mtgjson_keyword_names).is_some() + && parse_router_keyword_list(line, mtgjson_keyword_names).is_some() { return false; } @@ -3705,19 +3714,18 @@ pub(crate) fn parse_oracle_ir( .map(|s| s.trim()) .filter(|s| !s.is_empty()) .collect(); + // Consume-on-success: EVERY part must parse completely as keywords. The + // permissive form accepted a part carrying a semantic clause ("cycling + // {2} if you control an artifact"), consumed the whole line, and dropped + // that clause with no keyword and no diagnostic. if parts.len() > 1 { - let all_keywords = parts.iter().all(|part| { - extract_granted_keyword_list(part, mtgjson_keyword_names).is_some() - }); - if all_keywords { - for part in &parts { - if let Some(extracted) = - extract_granted_keyword_list(part, mtgjson_keyword_names) - { - for __item in extracted { - emitter.keyword_at(item_line, __item); - } - } + let routed: Option>> = parts + .iter() + .map(|part| parse_router_keyword_list(part, mtgjson_keyword_names)) + .collect(); + if let Some(routed) = routed { + for keyword in routed.into_iter().flatten() { + emitter.keyword_at(item_line, keyword); } i += 1; continue; @@ -3761,8 +3769,10 @@ pub(crate) fn parse_oracle_ir( // CR 702.122 + CR 602.5b: Crew with a trailing "Activate only once each // turn." cadence sentence. Must run before the generic keyword-only - // extraction below — that path parses "Crew N" via `parse_granted_keyword_fragment` - // and would consume the line, dropping the cadence sentence. + // extraction below: that path would emit a bare `Crew N` and leave the cadence + // sentence to be re-parsed as its own unit. This intercept models both in one + // keyword. (Both surfaces are strict about the tail — `parse_crew_keyword` ends + // in `all_consuming`, and priority 1b now routes through the strict list.) if lower_starts_with(&lower, "crew ") { if let Some(crew_kw) = parse_crew_keyword(&lower) { emitter.keyword_at(item_line, crew_kw); @@ -3774,9 +3784,17 @@ pub(crate) fn parse_oracle_ir( // Priority 1b: keyword-only line — extract any keywords for the union set // Guard: "{Keyword} abilities you activate cost {N} less" is a static ability, // not a keyword line. Don't let keyword extraction consume it. + // Consume-on-success. This slot runs LONG before the strict routers at + // priority 9 / 13, so whenever MTGJSON names the keyword — the common case — + // it is THIS slot, not those, that decides the line. On the permissive + // surface it consumed "Cycling {2} if you control an artifact" as a bare + // Cycling and dropped the condition, which made the strict wiring downstream + // unreachable for exactly the cards MTGJSON knows about. Only a completely + // parsed keyword list may consume the line now; anything else falls through + // and becomes an honest, exact-unit `Effect::Unimplemented`. let is_ability_cost_static = is_ability_activate_cost_static(&lower); if !is_ability_cost_static { - if let Some(extracted) = extract_granted_keyword_list(&line, mtgjson_keyword_names) { + if let Some(extracted) = parse_router_keyword_list(&line, mtgjson_keyword_names) { if let Some(cost) = parse_kicker_additional_cost_line(&line, &lower) { merge_kicker_additional_cost(&mut result.additional_cost, cost); additional_cost_line.get_or_insert(item_line); @@ -4660,7 +4678,7 @@ pub(crate) fn parse_oracle_ir( if lower_starts_with(&lower, "flashback") { if line.contains('\u{2014}') { let lower_clean = lower.trim_end_matches('.').trim(); - if let Some(kw) = parse_granted_keyword_fragment(lower_clean) { + if let Some(kw) = parse_router_keyword_fragment(lower_clean) { emitter.keyword_at(item_line, kw); i += 1; continue; @@ -4668,17 +4686,21 @@ pub(crate) fn parse_oracle_ir( } else if let Some((flashback_part, reduction_part)) = split_flashback_trailing_self_spell_cost_reduction(&line, &lower) { + // ATOMIC + consume-on-success. The split PROMISES two semantic halves. + // The previous form advanced `i` unconditionally, so a line whose + // keyword half parsed but whose cost-reduction half did not (or vice + // versa) was consumed with the other half silently dropped. Both must + // parse, or the line falls through and stays honestly red. let flashback_lower = flashback_part.to_lowercase(); - if let Some(kw) = parse_granted_keyword_fragment(&flashback_lower) { + if let (Some(kw), Some(def)) = ( + parse_router_keyword_fragment(&flashback_lower), + parse_flashback_trailing_self_spell_cost_reduction(reduction_part), + ) { emitter.keyword_at(item_line, kw); - } - if let Some(def) = - parse_flashback_trailing_self_spell_cost_reduction(reduction_part) - { emitter.static_at(item_line, def); + i += 1; + continue; } - i += 1; - continue; } } @@ -5079,7 +5101,7 @@ pub(crate) fn parse_oracle_ir( // Spells (instants/sorceries) with Suspend would otherwise be caught by // the is_spell branch and produce an Unimplemented effect. if lower_starts_with(&lower, "suspend ") { - if let Some(kw) = parse_granted_keyword_fragment(&lower) { + if let Some(kw) = parse_router_keyword_fragment(&lower) { emitter.keyword_at(item_line, kw); i += 1; continue; @@ -5089,7 +5111,7 @@ pub(crate) fn parse_oracle_ir( // Digital-only Specialize: "specialize {cost}" — MTGJSON may omit the keyword // when it appears as a standalone rules line; intercept before dispatch fallback. if lower_starts_with(&lower, "specialize ") { - if let Some(kw) = parse_granted_keyword_fragment(&lower) { + if let Some(kw) = parse_router_keyword_fragment(&lower) { emitter.keyword_at(item_line, kw); i += 1; continue; @@ -5101,7 +5123,7 @@ pub(crate) fn parse_oracle_ir( // is intercepted as a keyword, not parsed as an effect. // MTGJSON keywords array only says "Harmonize" (no cost), so we extract cost here. // Format: "Harmonize {cost} (reminder text)" — space-separated. - // Note: When MTGJSON provides "Harmonize" in keywords, extract_granted_keyword_list at + // Note: When MTGJSON provides "Harmonize" in keywords, the strict keyword list at // priority 1b already handles this. This is a fallback for test/edge cases. if lower_starts_with(&lower, "harmonize ") { if let Some(harmonize_kw) = parse_harmonize_keyword(&line) { @@ -5123,41 +5145,53 @@ pub(crate) fn parse_oracle_ir( } } - // Priority 8f: Kicker / Multikicker / Replicate cost lines — must run BEFORE Priority 9 - // (spell catch-all) so these keyword declarations on spell cards don't become Unimplemented. + // Priority 8f: CR 702.33 Kicker / CR 702.33c Multikicker / CR 702.56 Replicate / + // CR 702.187 Mayhem cost lines — must run BEFORE Priority 9 (spell catch-all) so + // these keyword declarations on spell cards don't become Unimplemented. // We cannot use is_keyword_cost_line here because it would also catch "flashback" // etc. whose specific em-dash parsers run between Priority 9 and Priority 13. // Note: "mayhem" IS in is_keyword_cost_line and is handled at Priority 1b via MTGJSON // keywords when present; this guard catches it when keywords[] is empty. - if alt(( - tag::<_, _, OracleError<'_>>("kicker"), - tag("multikicker"), - tag("replicate"), - tag("mayhem"), - )) - .parse(lower.as_str()) - .is_ok() - { + // + // Two defects fixed here (task #123), both of which made this the worst site: + // (a) CLASS-A SILENT SWALLOW. `i += 1; continue;` used to sit OUTSIDE both + // `if let Some` blocks, so a candidate line that NEITHER parser could + // parse was consumed with no keyword, no additional cost, and no + // diagnostic — it vanished and the card rendered as fully supported. + // The line is now consumed only if something was actually recorded. + // (b) NO WORD BOUNDARY. The dispatch was a bare `alt((tag("kicker"), …))`, + // so it matched any line merely STARTING with those letters — + // "Kickerfoo {2}" was accepted and then vanished via (a). + // `is_kicker_family_line` shares `is_keyword_cost_line`'s boundary rule. + if is_kicker_family_line(&lower) { + let mut recorded = false; if let Some(cost) = parse_kicker_additional_cost_line(&line, &lower) { merge_kicker_additional_cost(&mut result.additional_cost, cost); + additional_cost_line.get_or_insert(item_line); + recorded = true; } - if let Some(kw) = parse_granted_keyword_fragment(&lower) { + if let Some(kw) = parse_router_keyword_fragment(&lower) { emitter.keyword_at(item_line, kw); + recorded = true; } - i += 1; - continue; + if recorded { + i += 1; + continue; + } + // Nothing parsed: fall through to the spell catch-all / priority 15 so the + // line becomes an honest, exact-unit `Effect::Unimplemented`. } // CR 702.27a: Buyback em-dash form — "Buyback—Sacrifice a land." (Constant // Mists) etc. MTGJSON omits the Buyback keyword when the cost is non-mana, - // so `extract_granted_keyword_list` bails and the line would otherwise fall through + // so the priority-1b keyword list bails and the line would otherwise fall through // to the spell-effect catch-all and produce `Unimplemented`. Intercept here // before the spell catch-all, mirroring the Flashback em-dash intercept above. // structural: not dispatch — em-dash char presence gates the cost sub-parser, // which uses nom combinators in `parse_buyback_cost` / `parse_oracle_cost`. if lower_starts_with(&lower, "buyback") && line.contains('\u{2014}') { let lower_clean = lower.trim_end_matches('.').trim(); - if let Some(kw) = parse_granted_keyword_fragment(lower_clean) { + if let Some(kw) = parse_router_keyword_fragment(lower_clean) { emitter.keyword_at(item_line, kw); i += 1; continue; @@ -5173,7 +5207,7 @@ pub(crate) fn parse_oracle_ir( .is_ok() { let lower_clean = lower.trim_end_matches('.').trim(); - if let Some(kw) = parse_granted_keyword_fragment(lower_clean) { + if let Some(kw) = parse_router_keyword_fragment(lower_clean) { emitter.keyword_at(item_line, kw); i += 1; continue; @@ -5508,7 +5542,7 @@ pub(crate) fn parse_oracle_ir( // CR 702.49d: Commander ninjutsu is not in MTGJSON keywords — extract explicitly. if lower_starts_with(&lower, "commander ninjutsu ") { - if let Some(kw) = parse_granted_keyword_fragment(&lower) { + if let Some(kw) = parse_router_keyword_fragment(&lower) { emitter.keyword_at(item_line, kw); i += 1; continue; @@ -5518,7 +5552,7 @@ pub(crate) fn parse_oracle_ir( // CR 702.138a: Escape is extracted by the generic keyword-cost guards — // the `is_spell` guard above (Priority 9) for instants/sorceries and the // `is_keyword_cost_line` guard below (Priority 13) for permanents — via the - // `escape—` branch registered in `parse_granted_keyword_fragment`, alongside its + // `escape—` branch registered in `parse_keyword_line_core`, alongside its // evoke/embalm/eternalize/escalate em-dash siblings. No dedicated intercept // is needed here. @@ -5540,17 +5574,18 @@ pub(crate) fn parse_oracle_ir( if let Some((flashback_part, reduction_part)) = split_flashback_trailing_self_spell_cost_reduction(&line, &lower) { + // ATOMIC + consume-on-success — see the identical split above (priority 7 + // guard). Both halves must parse or the line is not consumed. let flashback_lower = flashback_part.to_lowercase(); - if let Some(kw) = parse_granted_keyword_fragment(&flashback_lower) { + if let (Some(kw), Some(def)) = ( + parse_router_keyword_fragment(&flashback_lower), + parse_flashback_trailing_self_spell_cost_reduction(reduction_part), + ) { emitter.keyword_at(item_line, kw); - } - if let Some(def) = - parse_flashback_trailing_self_spell_cost_reduction(reduction_part) - { emitter.static_at(item_line, def); + i += 1; + continue; } - i += 1; - continue; } } // Consume-on-success. The previous form advanced `i` OUTSIDE the @@ -5632,8 +5667,11 @@ pub(crate) fn parse_oracle_ir( } // Try as keyword — the ability-word prefix ("Void Shields —") was // stripped, so the remainder may be a keyword line that Priority 1b - // missed because it ran on the unprefixed original line. - if let Some(kw) = parse_granted_keyword_fragment(&effect_lower) { + // missed because it ran on the unprefixed original line. Strict: the + // stripped remainder must be a COMPLETE keyword declaration, or the line + // continues to the static/effect paths and ultimately an honest + // `Effect::Unimplemented`. + if let Some(kw) = parse_router_keyword_fragment(&effect_lower) { if !matches!(kw, Keyword::Unknown(_)) { emitter.keyword_at(item_line, kw); i += 1; diff --git a/crates/engine/src/parser/oracle_keyword.rs b/crates/engine/src/parser/oracle_keyword.rs index 2794861342..20ee7dc6ff 100644 --- a/crates/engine/src/parser/oracle_keyword.rs +++ b/crates/engine/src/parser/oracle_keyword.rs @@ -295,13 +295,90 @@ pub(crate) fn parse_kicker_additional_cost_line(raw: &str, lower: &str) -> Optio }) } +/// `parse_oracle_cost` NEVER fails — it hands back `AbilityCost::Unimplemented` +/// for text it cannot type. So "the cost parser returned" is not evidence the cost +/// parsed. Without the `ability_cost_is_fully_typed` guard, a kicker line with a +/// semantic tail ("Kicker {2} if you control an artifact") yields a `Some(_)` cost +/// built from the untyped remainder, and the priority-8f router then consumes the +/// line AND fabricates a cost — strictly worse than declining, because the card +/// renders as supported. Declining lets the line fall through to an honest, +/// exact-unit `Effect::Unimplemented`. fn parse_kicker_cost_payload(input: &str) -> Option { let stripped = strip_reminder_text(input); let cost_text = stripped.trim().trim_end_matches('.').trim(); if cost_text.is_empty() { return None; } - Some(parse_oracle_cost(cost_text)) + let cost = parse_oracle_cost(cost_text); + ability_cost_is_fully_typed(&cost).then_some(cost) +} + +/// Which keyword-parse contract a keyword-LIST consumer needs for each part. +/// +/// This is the list-level counterpart of the `parse_granted_keyword_fragment` / +/// `parse_router_keyword_line` split, expressed as a typed axis rather than two +/// near-duplicate list walks. The list semantics (comma parts, MTGJSON validation, +/// protection expansion, `instances_function_separately`) are identical in both +/// modes; only the per-part remainder contract differs, so THAT is the parameter. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum KeywordRemainderPolicy { + /// EMBEDDED GRANT. The trailing clause belongs to the ENCLOSING sentence + /// ("… gains vanishing 3 if that creature doesn't have vanishing"), so the + /// part parser must take the leading keyword and leave the rest alone. + /// Discarding the remainder is CORRECT here. + DiscardRemainder, + /// ROUTER. The line IS the unit. Any unconsumed semantic prose means the + /// router may not consume the line — it must fall through to ordinary + /// parsing and become an honest, exact-unit `Effect::Unimplemented` rather + /// than vanishing with no keyword and no diagnostic. + RequireAllConsuming, +} + +impl KeywordRemainderPolicy { + /// The single place the policy turns into an actual per-part parse. + fn parse_part(self, lower: &str) -> Option { + match self { + Self::DiscardRemainder => parse_granted_keyword_fragment(lower), + Self::RequireAllConsuming => parse_router_keyword_fragment(lower), + } + } +} + +/// PERMISSIVE keyword-list extraction — see [`KeywordRemainderPolicy::DiscardRemainder`]. +/// +/// Valid ONLY in embedded grant contexts (static/token/vote/class-level payloads). +/// A router that consumes a line on this result silently swallows whatever the +/// keyword did not explain. Routers must call [`parse_router_keyword_list`]. +pub(crate) fn extract_granted_keyword_list( + line: &str, + mtgjson_keyword_names: &[String], +) -> Option> { + parse_keyword_list_with_policy( + line, + mtgjson_keyword_names, + KeywordRemainderPolicy::DiscardRemainder, + ) +} + +/// STRICT keyword-list extraction — see [`KeywordRemainderPolicy::RequireAllConsuming`]. +/// +/// The list-level sibling of [`parse_router_keyword_line`], and the ONLY keyword-list +/// surface a router or a routing classifier may use. Every comma part must parse to a +/// typed keyword with an all-consuming permitted (`P`/`R`/`M`) tail; one part carrying +/// a semantic clause declines the WHOLE line. +/// +/// `parse_router_keyword_line` cannot serve this role: it parses a single keyword and +/// takes no MTGJSON names, so it cannot see a bare-keyword line ("Flying, vigilance") +/// or the MTGJSON-authoritative parts that carry no Oracle parameter. +pub(crate) fn parse_router_keyword_list( + line: &str, + mtgjson_keyword_names: &[String], +) -> Option> { + parse_keyword_list_with_policy( + line, + mtgjson_keyword_names, + KeywordRemainderPolicy::RequireAllConsuming, + ) } /// Try to extract keywords from a keyword-only line (comma-separated). @@ -312,15 +389,19 @@ fn parse_kicker_cost_payload(input: &str) -> Option { /// Returns only keywords not already covered by MTGJSON names — these are typically /// parameterized keywords where MTGJSON lists the name (e.g. "Protection") but /// Oracle text has the full form (e.g. "Protection from multicolored"). -pub(crate) fn extract_granted_keyword_list( +/// +/// `policy` decides the per-part remainder contract — the ONE axis on which the +/// grant and router surfaces differ. +fn parse_keyword_list_with_policy( line: &str, mtgjson_keyword_names: &[String], + policy: KeywordRemainderPolicy, ) -> Option> { let line_without_reminder = strip_reminder_text(line); let line = strip_keyword_activation_cost_prefix(line_without_reminder.trim()); if mtgjson_keyword_names.is_empty() { - return parse_mtgjson_missing_standalone_keyword_line(line); + return parse_mtgjson_missing_standalone_keyword_line(line, policy); } if mtgjson_keyword_names.iter().any(|n| n == "mobilize") { @@ -393,7 +474,7 @@ pub(crate) fn extract_granted_keyword_list( // place printed multiplicity survives — emit one Keyword per occurrence // so the runtime's per-instance trigger loop fires correctly. Synthesis // reconciles the deduped MTGJSON copy against these. - if let Some(kw) = parse_granted_keyword_fragment(&lower) { + if let Some(kw) = policy.parse_part(&lower) { if kw.instances_function_separately() { new_keywords.push(kw); } @@ -403,14 +484,14 @@ pub(crate) fn extract_granted_keyword_list( // Prefix match: Oracle text has more detail (e.g. "protection from red"). // Extract the full parameterized keyword. - if let Some(kw) = parse_granted_keyword_fragment(&lower) { + if let Some(kw) = policy.parse_part(&lower) { new_keywords.push(kw); continue; } } // Not an MTGJSON match — try parsing as any keyword (for keyword-only line validation) - if let Some(kw) = parse_granted_keyword_fragment(&lower) { + if let Some(kw) = policy.parse_part(&lower) { if !matches!(kw, Keyword::Unknown(_)) { // Keywords not in MTGJSON (e.g., firebending) must be extracted here. // They also validate the line as a keyword line. @@ -476,9 +557,12 @@ fn strip_activation_cost_dash(rest: &str) -> Option<&str> { .map(|(keyword_text, _)| keyword_text.trim_start()) } -fn parse_mtgjson_missing_standalone_keyword_line(line: &str) -> Option> { +fn parse_mtgjson_missing_standalone_keyword_line( + line: &str, + policy: KeywordRemainderPolicy, +) -> Option> { let lower = line.to_lowercase(); - let keyword = parse_granted_keyword_fragment(&lower)?; + let keyword = policy.parse_part(&lower)?; match keyword { Keyword::ForMirrodin => Some(vec![keyword]), // CR 702.89a: Umbra armor (printed as "umbra armor"/"totem armor") is a @@ -2006,6 +2090,62 @@ pub(crate) fn parse_router_keyword_line(line: &str) -> Option }) } +/// The remainder-CHECKING sibling of [`parse_granted_keyword_fragment`], which is +/// literally `parse_keyword_line_core(t).map(|(kw, _discarded_remainder)| kw)`. +/// +/// Same core, same typed keyword — but any unconsumed SEMANTIC prose rejects the +/// fragment instead of being thrown away. Only a permitted (`P`/`M`) tail may +/// remain. This is the primitive every router-context keyword parse is built on: +/// the whole-line router ([`parse_router_keyword_line`]) adds candidate recognition +/// and reminder-text handling on top; the list router +/// ([`parse_router_keyword_list`]) applies it per comma part. +/// +/// Takes ALREADY-LOWERCASED text, matching `parse_granted_keyword_fragment`'s +/// contract, so it is a drop-in at every site that used the permissive surface. +pub(crate) fn parse_router_keyword_fragment(lower: &str) -> Option { + let (keyword, unconsumed) = parse_keyword_line_core(lower)?; + let (modifiers, _had_terminal_punctuation) = parse_permitted_keyword_tail(unconsumed)?; + apply_keyword_line_modifiers(keyword, &modifiers) +} + +/// A candidate prefix matches only at a WORD BOUNDARY: the prefix must be followed +/// by end-of-line, whitespace, or an em-dash. +/// +/// Extracted from [`is_keyword_cost_line`]'s guard so that every candidate +/// recognizer shares one boundary rule. Without it, a bare `tag("kicker")` accepts +/// "Kickerfoo {2}" — the router claims a line it cannot parse and (before this +/// unit) consumed it with no keyword and no diagnostic. +fn matches_keyword_prefix_at_word_boundary(lower: &str, prefix: &str) -> bool { + tag::<_, _, OracleError<'_>>(prefix) + .parse(lower) + .is_ok_and(|(rest, _)| { + rest.is_empty() + || rest.as_bytes().first() == Some(&b' ') + || rest.as_bytes().first() == Some(&b'\t') + || tag::<_, _, OracleError<'_>>("\u{2014}").parse(rest).is_ok() + }) +} + +/// CR 702.33 Kicker / CR 702.33c Multikicker (a kicker variant, not a sibling rule) +/// / CR 702.56 Replicate / CR 702.187 Mayhem — the four keyword ADDITIONAL COSTS +/// that declare themselves on their own Oracle line. +/// +/// The priority-8f candidate set. Deliberately NOT merged into +/// [`KEYWORD_COST_PREFIXES`]: these are keyword ADDITIONAL COSTS whose lines also +/// feed `parse_kicker_additional_cost_line`, and `is_keyword_cost_line` additionally +/// gates `is_spell_resolution_instruction_line` — widening it there would change +/// which lines count as spell-resolution text. +pub(crate) const KICKER_FAMILY_PREFIXES: [&str; 4] = + ["kicker", "multikicker", "replicate", "mayhem"]; + +/// Whether a line is a priority-8f kicker-family candidate, guarded at a word +/// boundary. Candidate recognition ONLY — never evidence that the line parses. +pub(crate) fn is_kicker_family_line(lower: &str) -> bool { + KICKER_FAMILY_PREFIXES + .iter() + .any(|prefix| matches_keyword_prefix_at_word_boundary(lower, prefix)) +} + /// Bare-integer-count keywords whose `FromStr` arm does `p.parse().unwrap_or(N)` /// (or wraps the integer in `QuantityExpr::Fixed`) over the parameter string — /// see the arms in `types/keywords.rs`. For these the generic normalizer must @@ -2588,18 +2728,9 @@ pub(crate) const KEYWORD_COST_PREFIXES: [&str; 95] = [ /// precisely the bug this unit removes. Only `parse_router_keyword_line` returning /// `Some` licenses a router to consume the line. pub(crate) fn is_keyword_cost_line(lower: &str) -> bool { - KEYWORD_COST_PREFIXES.iter().any(|kw| { - tag::<_, _, OracleError<'_>>(*kw) - .parse(lower) - .is_ok_and(|(rest, _)| { - rest.is_empty() - || rest.as_bytes().first() == Some(&b' ') - || rest.as_bytes().first() == Some(&b'\t') - || tag::<_, _, OracleError<'_>>("\u{2014}") - .parse(rest) - .is_ok() - }) - }) + KEYWORD_COST_PREFIXES + .iter() + .any(|kw| matches_keyword_prefix_at_word_boundary(lower, kw)) // CR 702.29e: Typecycling — first word ends in "cycling" but isn't "cycling" itself || lower .split_whitespace() diff --git a/crates/engine/src/parser/oracle_tests.rs b/crates/engine/src/parser/oracle_tests.rs index 653fd20248..3654b6d008 100644 --- a/crates/engine/src/parser/oracle_tests.rs +++ b/crates/engine/src/parser/oracle_tests.rs @@ -21328,3 +21328,308 @@ fn ill_gotten_gains_return_clause_binds_bounded_multi_target() { return_clause.multi_target ); } +// =========================================================================== +// Task #123 — router wiring: every router slot and routing classifier parses +// keyword lines through a STRICT (remainder-checking) surface. +// +// The invariant under test is CONSUME-ON-SUCCESS: a router may advance past a +// line only when the line parsed COMPLETELY. A keyword-cost line carrying a +// semantic clause it cannot model must fall through and become an honest, +// exact-unit `Effect::Unimplemented` — never vanish with no keyword and no +// diagnostic, which renders the card as fully supported. +// +// Each swallow witness below is paired with a POSITIVE control on the same +// family, because "everything is now Unimplemented" would satisfy the swallow +// assertions vacuously. The controls prove the valid line still routes. +// =========================================================================== + +/// The hostile suffix: semantic prose no permitted (`P`/`R`/`M`) tail admits. +const T123_SEMANTIC_TAIL: &str = " if you control an artifact"; + +fn t123_parse(text: &str, kws: &[&str], types: &[&str]) -> ParsedAbilities { + let kwv: Vec = kws.iter().map(|s| s.to_string()).collect(); + let tyv: Vec = types.iter().map(|s| s.to_string()).collect(); + parse_oracle_text(text, "Witness", &kwv, &tyv, &[]) +} + +fn t123_unimplemented_count(parsed: &ParsedAbilities) -> usize { + parsed + .abilities + .iter() + .filter(|a| matches!(&*a.effect, Effect::Unimplemented { .. })) + .count() +} + +/// A swallowed line: consumed, nothing recorded, nothing flagged. The exact +/// failure this unit exists to remove. +fn t123_assert_not_swallowed(label: &str, parsed: &ParsedAbilities) { + assert!( + t123_unimplemented_count(parsed) > 0, + "[{label}] the line was CONSUMED with no Effect::Unimplemented — a silent \ + swallow. keywords={:?} abilities={} statics={} triggers={}", + parsed.extracted_keywords, + parsed.abilities.len(), + parsed.statics.len(), + parsed.triggers.len(), + ); +} + +/// THE 8f witness. `Kickerfoo {2}` is a fabricated keyword line: no such +/// keyword exists, so nothing can parse it. +/// +/// Before this unit, priority 8f dispatched on a bare `tag("kicker")` with NO +/// word-boundary guard, so "kickerfoo" matched the "kicker" prefix; then +/// `i += 1; continue;` sat OUTSIDE both `if let Some` blocks, so the line was +/// consumed with no keyword, no additional cost, and no diagnostic. It vanished +/// entirely, and the card rendered as fully supported. +/// +/// Red-first: on pre-fix code this test FAILS with 0 abilities / 0 keywords / +/// 0 Unimplemented — the line is simply gone. +#[test] +fn t123_kickerfoo_synthetic_line_is_honestly_unimplemented_not_swallowed() { + let parsed = t123_parse("Kickerfoo {2}", &[], &["Instant"]); + + t123_assert_not_swallowed("kickerfoo", &parsed); + assert_eq!( + t123_unimplemented_count(&parsed), + 1, + "expected exactly ONE exact-unit Unimplemented for the one bogus line" + ); + assert!( + parsed.extracted_keywords.is_empty(), + "a fabricated keyword line must not manufacture a keyword, got {:?}", + parsed.extracted_keywords + ); +} + +/// Priority 8f, defect (a): a REAL kicker prefix at a word boundary, with a +/// semantic tail neither the additional-cost parser nor the keyword parser can +/// model. Pre-fix this produced `Kicker(generic: 0)` — the tail eaten AND the +/// cost fabricated as 0 (`parse_oracle_cost` never fails, so the cost slot was +/// built out of untyped prose). +#[test] +fn t123_kicker_with_semantic_tail_is_not_consumed() { + let text = format!("Kicker {{2}}{T123_SEMANTIC_TAIL}"); + let parsed = t123_parse(&text, &[], &["Instant"]); + + t123_assert_not_swallowed("kicker+tail", &parsed); + assert!( + !parsed + .extracted_keywords + .iter() + .any(|k| matches!(k, Keyword::Kicker(_))), + "must not commit a Kicker built from an unmodelled tail, got {:?}", + parsed.extracted_keywords + ); +} + +/// Positive control for 8f: the valid lines still route to typed keywords with +/// their real costs, and raise no Unimplemented. Without this, the two tests +/// above would pass even if 8f had simply been deleted. +#[test] +fn t123_kicker_family_valid_lines_still_route() { + for (text, label) in [ + ("Kicker {2}", "kicker"), + ("Multikicker {1}{R}", "multikicker"), + ("Replicate {2}", "replicate"), + ] { + let parsed = t123_parse(text, &[], &["Instant"]); + assert_eq!( + t123_unimplemented_count(&parsed), + 0, + "[{label}] a VALID keyword line must not become Unimplemented: {parsed:#?}" + ); + } + + // The cost must be the printed one, not a fabricated empty cost. + let replicate = t123_parse("Replicate {2}", &[], &["Instant"]); + assert!( + replicate + .extracted_keywords + .iter() + .any(|k| matches!(k, Keyword::Replicate(_))), + "Replicate {{2}} must yield a Replicate keyword, got {:?}", + replicate.extracted_keywords + ); +} + +/// THE SHADOW witness (priority 1b). This is the finding the census did not +/// record: priority 1b runs LONG before the strict routers at priority 9/13, so +/// whenever MTGJSON names the keyword — the common case — it is 1b, not those, +/// that decides the line. +/// +/// Pre-fix, 1b's permissive list parser consumed this whole line as a bare +/// `Cycling`, dropped the conditional, and raised nothing. The strict wiring +/// downstream never even saw it. +/// +/// The `mtgjson EMPTY` half is the discriminator: it proves the difference is +/// caused by 1b's MTGJSON-gated path and not by some unrelated slot. +#[test] +fn t123_priority_1b_does_not_shadow_the_strict_router() { + let text = format!("Cycling {{2}}{T123_SEMANTIC_TAIL}"); + + // MTGJSON names the keyword -> pre-fix, priority 1b claimed and swallowed it. + let named = t123_parse(&text, &["cycling"], &["Creature"]); + t123_assert_not_swallowed("1b cycling+tail (mtgjson named)", &named); + assert!( + !named + .extracted_keywords + .iter() + .any(|k| matches!(k, Keyword::Cycling(_))), + "must not commit a Cycling built from an unmodelled tail, got {:?}", + named.extracted_keywords + ); + + // MTGJSON silent -> 1b declines, and the line was ALREADY honest pre-fix. + // Both paths must now agree; that agreement is the point of the migration. + let unnamed = t123_parse(&text, &[], &["Creature"]); + t123_assert_not_swallowed("1b cycling+tail (mtgjson silent)", &unnamed); +} + +/// Positive control for 1b: bare keyword lines and correctly-parameterized +/// keyword lines still route. This is the path a naive "just make it strict" +/// change would destroy, since `parse_router_keyword_line` alone cannot see a +/// bare-keyword list. +#[test] +fn t123_priority_1b_valid_keyword_lines_still_route() { + let bare = t123_parse("Flying, vigilance", &["flying", "vigilance"], &["Creature"]); + assert_eq!( + t123_unimplemented_count(&bare), + 0, + "a bare keyword list must still route: {bare:#?}" + ); + + let cycling = t123_parse("Cycling {2}", &["cycling"], &["Creature"]); + assert_eq!( + t123_unimplemented_count(&cycling), + 0, + "a valid parameterized keyword line must still route: {cycling:#?}" + ); + assert!( + cycling + .extracted_keywords + .iter() + .any(|k| matches!(k, Keyword::Cycling(_))), + "Cycling {{2}} must yield a Cycling keyword, got {:?}", + cycling.extracted_keywords + ); +} + +/// THE OVERREACH CONTROL, and the reason this unit does NOT refuse a line merely +/// because its parsed keyword list came back empty. +/// +/// A bare, MTGJSON-named keyword line parses to an EMPTY `extracted_keywords`: the +/// keyword rides the card's MTGJSON metadata, and the router's job on this line is +/// simply to consume it without raising anything. So "the strict parser produced no +/// keywords" is NOT evidence of a swallow here — it is the normal, correct outcome +/// for the single most common keyword line in the corpus. A refusal keyed on that +/// emptiness would satisfy every swallow assertion above and false-red every vanilla +/// evergreen creature in the pool. +/// +/// Green in BOTH worlds, pre- and post-fix. That is the point: it is the line the +/// migration must NOT move. +#[test] +fn t123_metadata_named_bare_keyword_line_still_routes() { + let parsed = t123_parse("Flying", &["flying"], &["Creature"]); + + assert_eq!( + t123_unimplemented_count(&parsed), + 0, + "a bare metadata-named keyword line must not become Unimplemented: {parsed:#?}" + ); + assert!( + parsed.abilities.is_empty() && parsed.statics.is_empty() && parsed.triggers.is_empty(), + "a bare metadata-named keyword line must be consumed cleanly, got {parsed:#?}" + ); +} + +/// Priority 1b, Crew. The strict `parse_crew_keyword` intercept correctly +/// DECLINES "Crew 2 " (its cadence tail is `all_consuming`), and +/// the line then fell into 1b's permissive list — which ate the tail. The leak +/// was never in the Crew parser; it was downstream, exactly as the charter says. +#[test] +fn t123_crew_with_semantic_tail_falls_through_to_unimplemented() { + let text = format!("Crew 2{T123_SEMANTIC_TAIL}"); + let parsed = t123_parse(&text, &["crew"], &["Artifact"]); + + t123_assert_not_swallowed("crew+tail", &parsed); + assert!( + !parsed + .extracted_keywords + .iter() + .any(|k| matches!(k, Keyword::Crew { .. })), + "must not commit a Crew built from an unmodelled tail, got {:?}", + parsed.extracted_keywords + ); + + // Positive control: the plain Crew line, and the modeled cadence sentence, + // both still route (this is the NOT-A-DEFECT path — do not regress it). + let plain = t123_parse("Crew 2", &["crew"], &["Artifact"]); + assert!( + plain + .extracted_keywords + .iter() + .any(|k| matches!(k, Keyword::Crew { .. })), + "plain Crew 2 must still route, got {:?}", + plain.extracted_keywords + ); +} + +/// Priority 0, the semicolon keyword line. Every part must parse completely; +/// one part carrying a semantic clause declines the WHOLE line. +#[test] +fn t123_semicolon_keyword_line_with_semantic_tail_is_not_consumed() { + let text = format!("Defender; cycling {{2}}{T123_SEMANTIC_TAIL}"); + let parsed = t123_parse(&text, &["defender", "cycling"], &["Creature"]); + + t123_assert_not_swallowed("semicolon+tail", &parsed); + + // Positive control: a clean semicolon keyword line still routes. + let clean = t123_parse("Defender; reach", &["defender", "reach"], &["Creature"]); + assert_eq!( + t123_unimplemented_count(&clean), + 0, + "a clean semicolon keyword line must still route: {clean:#?}" + ); +} + +/// The advance-on-partial family: each of these router intercepts used to call +/// the permissive fragment parser and advance `i` on a parse that had discarded +/// the remainder. Every one of them ate its tail AND fabricated its cost. +/// +/// Driven as a table because the defect — and the fix — are one shape, not six. +#[test] +fn t123_advance_on_partial_intercepts_no_longer_eat_their_tails() { + let cases: [(&str, &str, &[&str]); 5] = [ + ("suspend", "Suspend 3—{1}{R}", &["Sorcery"]), + ("specialize", "Specialize {2}", &["Creature"]), + ("escalate", "Escalate {2}", &["Instant"]), + ( + "commander ninjutsu", + "Commander ninjutsu {1}{U}", + &["Creature"], + ), + ("buyback", "Buyback {3}", &["Instant"]), + ]; + + for (label, valid_line, types) in cases { + // Swallow witness: valid keyword + a clause nothing can model. + let hostile = format!("{valid_line}{T123_SEMANTIC_TAIL}"); + let parsed = t123_parse(&hostile, &[], types); + t123_assert_not_swallowed(label, &parsed); + + // Positive control: the same line WITHOUT the tail must still route, + // or the assertion above is passing for the wrong reason. + let clean = t123_parse(valid_line, &[], types); + assert_eq!( + t123_unimplemented_count(&clean), + 0, + "[{label}] the valid line must still route to a keyword: {clean:#?}" + ); + assert!( + !clean.extracted_keywords.is_empty(), + "[{label}] the valid line must yield a keyword, got {:?}", + clean.extracted_keywords + ); + } +} diff --git a/scripts/check-parser-combinators.sh b/scripts/check-parser-combinators.sh index 104615cc33..65e9759e4d 100755 --- a/scripts/check-parser-combinators.sh +++ b/scripts/check-parser-combinators.sh @@ -67,14 +67,16 @@ fi # THE BOUNDARY. There are two keyword-parsing surfaces and they are NOT # interchangeable: # -# parse_router_keyword_line() STRICT. All-consuming: it returns a typed -# keyword only when the candidate line parses -# completely (keyword + permitted P/R/M tail -# to eof). This is the ONLY surface that may -# license a router to CONSUME a whole line. +# parse_router_keyword_line() STRICT, whole line. All-consuming: returns a +# parse_router_keyword_list() STRICT, keyword list (comma parts + MTGJSON). +# parse_router_keyword_fragment() STRICT, one keyword phrase. +# These return a typed keyword ONLY when the text +# parses completely (keyword + permitted P/R/M +# tail). They are the ONLY surfaces that may +# license a router to CONSUME a line. # -# parse_granted_keyword_fragment() PERMISSIVE. By design it takes the leading -# extract_granted_keyword_list() keyword and DISCARDS the remainder — correct +# parse_granted_keyword_fragment() PERMISSIVE. By design they take the leading +# extract_granted_keyword_list() keyword and DISCARD the remainder — correct # for an EMBEDDED grant ("...gains vanishing 3 # if ..." inside a static/token/vote payload), # and invalid at a whole-line router boundary, @@ -84,76 +86,33 @@ fi # A router that advances a line on a permissive parse is a SILENT SWALLOW: no # keyword recorded, no diagnostic, and the card renders as fully supported. # -# THIS GATE IS A RATCHET, NOT A BLESSING. Plan 02 step 5 wired the strict router -# into priorities 9 and 13 but left the remaining router entries on the permissive -# surface. Those survivors are enumerated below as an EXACT expected count per -# router context. The gate fails if the count goes UP (a new permissive router call -# — the regression this exists to stop) and equally if it goes DOWN without updating -# the number (so it can never quietly drift out of truth). +# STATUS: MIGRATION COMPLETE (task #123). Plan 02 step 5 wired the strict router into +# priorities 9 and 13; task #123 migrated the remaining 16 permissive calls (priorities +# 0, 1b, 8f, the flashback/suspend/specialize/buyback/escalate/commander-ninjutsu/d20 +# intercepts, and the two routing classifiers) onto the strict surfaces. The permissive +# symbols are now ABSENT from oracle.rs entirely — not merely unused, but not imported. # -# DEFINITION OF DONE: entries are DELETED from this allowlist as each router entry -# converts to parse_router_keyword_line. When every EXPECTED_* reaches 0, this -# becomes the plain "no permissive symbol in a router context" gate that Plan 02 -# step 7 actually asks for, and the migration-status paragraph in SKILL.md §3a goes -# with it. Same instrument as KNOWN_NOUN_PARAM_LEAKS in oracle_keyword.rs. -# -# OWNER OF THE RESIDUAL: task #123 (P02-U5 router wiring). Every surviving call -# below is that task's work-list. Do not "fix" one in isolation without lowering the -# corresponding floor here in the same commit. -# -# THE ALLOWLIST — 16 permissive calls, each with why it is still here (task #123): -# -# parse_oracle_ir (13): -# :3720,:3725 priority 0, semicolon keyword line — commits on Some(vec![]), -# which extract_granted_keyword_list returns on MTGJSON metadata -# alone, with the parameterized Oracle line never parsed. -# :3789 priority 1b, keyword-only line — same Some(vec![]) leak; this -# is also where a "Crew N " loses its tail after the -# strict Crew intercept correctly declines it. -# :4673,:4682 flashback em-dash / cost-reduction split — non-atomic: advances -# even when only one half of the split parsed. -# :5092,:5102 suspend / specialize — advance-on-partial. -# :5154 priority 8f kicker/multikicker/replicate/mayhem — HIGHEST SEVERITY. -# Class-A: `i += 1; continue;` sits OUTSIDE both `if let Some` blocks, -# so a line neither parser can handle is consumed with NO keyword and -# NO Unimplemented. Also dispatches on a bare alt(( tag("kicker"), … )) -# with no word-boundary guard, unlike is_keyword_cost_line. -# :5170,:5186 buyback em-dash / escalate — advance-on-partial. -# :5521 commander ninjutsu — advance-on-partial. -# :5554 priority 13 residual — advance-on-partial. -# :5646 d20 table / ability-word retry — advance-on-partial. -# -# is_semicolon_keyword_line (2) :2161,:2167 -# is_spell_resolution_instruction_line (1) :2205 -# Classifiers, not routers, but they gate routing decisions on a permissive parse. -# Step 5 item 11: a classifier MAY probe with the strict parser; it may NOT call a -# helper that discards the remainder. +# This gate is therefore no longer a ratchet with an allowlist. It is the plain +# invariant Plan 02 step 7 asks for: NO permissive keyword-parser symbol may appear in +# a router context, at any count, ever. A reintroduction fails the build. # # SPAN EXTRACTION: from the `fn NAME(` signature at column 0 to the next `}` at # column 0. Brace COUNTING would be wrong here — oracle.rs is saturated with mana # symbols ("{T}", "{2}{B}") inside string literals, and a naive counter reads # those as scope. rustfmt guarantees a top-level fn closes with `}` at column 0, # so the anchor is exact. Full-line comments are excluded so that prose NAMING a -# permissive symbol (there are several) is not miscounted as a call. +# permissive symbol is not miscounted as a call. # --------------------------------------------------------------------------- ORACLE_RS='crates/engine/src/parser/oracle.rs' PERMISSIVE_SYMS='parse_granted_keyword_fragment|extract_granted_keyword_list' # Pre-rename spellings. These must not come back under any name, in any context. LEGACY_SYMS='parse_keyword_from_oracle|extract_keyword_line' -# NOT listed above: `parse_crew_keyword`. Plan 02 step 5 item 11 groups it with the -# remainder-discarding helpers, but that describes the PRE-step-5 code. As it stands -# it is strict: its cadence tail is `all_consuming(tag("activate only once each -# turn"))`, so "Crew 2 if you control an artifact" returns None rather than eating -# the suffix, and its call site advances only inside `if let Some`. The line then -# falls through to priority 1b — and it is 1b's `extract_granted_keyword_list` that -# would swallow the tail, which is exactly the call this gate is counting. Listing -# parse_crew_keyword here would encode a false claim about the code. - -# Expected surviving permissive calls per router context (see ratchet note above). -EXPECTED_parse_oracle_ir=13 -EXPECTED_is_semicolon_keyword_line=2 -EXPECTED_is_spell_resolution_instruction_line=1 +# NOT a router-context symbol: `parse_crew_keyword`. Plan 02 step 5 item 11 groups it +# with the remainder-discarding helpers, but that describes the PRE-step-5 code. As it +# stands it is strict: its cadence tail is `all_consuming(tag("activate only once each +# turn"))`, so "Crew 2 if you control an artifact" returns None rather than eating the +# suffix, and its call site advances only inside `if let Some`. arch_fail=0 @@ -174,19 +133,13 @@ for ctx in parse_oracle_ir is_semicolon_keyword_line is_spell_resolution_instruc continue fi actual="$(printf '%s\n' "$span" | grep -cE "$PERMISSIVE_SYMS" || true)" - eval "expected=\$EXPECTED_$ctx" - if [ "$actual" -gt "$expected" ]; then - echo "✗ (G) $ctx: $actual permissive keyword-parser calls, expected $expected." >&2 - echo " A NEW permissive call entered a router context. Routers must consume a" >&2 - echo " line only via all-consuming parse_router_keyword_line(); the permissive" >&2 - echo " surface discards the remainder and silently swallows semantics — no" >&2 - echo " keyword, no diagnostic, and the card renders as fully supported." >&2 - arch_fail=1 - elif [ "$actual" -lt "$expected" ]; then - echo "✗ (G) $ctx: $actual permissive calls, expected $expected — a router entry was" >&2 - echo " migrated to the strict parser. That is the GOAL (task #123): lower" >&2 - echo " EXPECTED_$ctx to $actual here, in the same commit, and delete that" >&2 - echo " entry from the allowlist comment above. The floor must never drift." >&2 + if [ "$actual" -ne 0 ]; then + echo "✗ (G) $ctx: $actual permissive keyword-parser call(s), expected 0." >&2 + echo " Routers must consume a line only via the STRICT surfaces" >&2 + echo " (parse_router_keyword_line / _list / _fragment). The permissive" >&2 + echo " surface discards the remainder and silently swallows semantics —" >&2 + echo " no keyword, no diagnostic, and the card renders as fully supported." >&2 + echo " This migration is COMPLETE (task #123); do not reintroduce it." >&2 arch_fail=1 fi done