Skip to content

feat(parser): parameterize superlative-property aggregate direction (least/lowest/smallest → Min)#5698

Merged
matthewevans merged 1 commit into
phase-rs:mainfrom
ntindle:fix/superlative-aggregate-direction
Jul 12, 2026
Merged

feat(parser): parameterize superlative-property aggregate direction (least/lowest/smallest → Min)#5698
matthewevans merged 1 commit into
phase-rs:mainfrom
ntindle:fix/superlative-aggregate-direction

Conversation

@ntindle

@ntindle ntindle commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

Parameterizes the superlative-property suffix combinator in parse_superlative_property_suffix so the aggregate direction is a single alt: greatest/highest/mostAggregateFunction::Max (existing), and least/lowest/smallestAggregateFunction::Min (new). Both directions feed the same generic superlative_property_filter_prop, so this closes the entire least/lowest/smallest superlative-selection class — over power, toughness, and mana value — across every card that uses the "with the <superlative> <property> among <set>" shape.

No new engine variant. The runtime already ships AggregateFunction::Min (used by values.min() in game/quantity.rs); previously only the Max head was reachable from the parser, so Min selection was a pure parser gap. The Sacrifice/Destroy/return resolvers consume the resulting PtComparison/Cmc filter prop unchanged.

Concretely, The Dining Car's upkeep clause "sacrifice a creature with the least toughness among creatures you control" now parses to a Sacrifice whose target carries a tie-inclusive (Comparator::EQ) PtComparison on toughness bound to Aggregate { function: Min, property: Toughness, filter: creatures you control } — i.e. it selects the least-toughness creature, as the Oracle text specifies.

This was split out of #5651 at matt's request ("GAP A is clean; if you split GAP A out, I'd take it as-is"). GAP B from that branch never touched oracle_target.rs and is not included here — this PR is superlative-direction-only.

Tier: Standard

Files changed

  • crates/engine/src/parser/oracle_target.rs

CR references

  • CR 208.1 — power and toughness (superlative selection over power/toughness)
  • CR 202.3 — mana value (superlative selection over mana value)
  • CR 701.21 — Sacrifice (The Dining Car's upkeep least-toughness sacrifice)

Implementation method (required)

Method: not-applicable — single-combinator parameterization split verbatim out of the already-reviewed #5651 branch (matt approved GAP A as-is); reuses an existing runtime AggregateFunction with no new engine variant.

Track

Developer

LLM

Model: claude-opus-4-8
Thinking: high

Verification

  • Required checks ran clean, or the exact CI-owned alternative is stated below.

  • Gate A output below is for the current committed head.

  • cargo fmt --all — clean

  • ./scripts/check-parser-combinators.sh <merge-base> — Gate A PASS (see below)

  • cargo clippy -p engine -p phase-ai -p engine-wasm --all-targets -- -D warnings — clean (exit 0, no warnings)

  • cargo test -p engine -p phase-ai — all pass; new tests superlative_direction_maps_word_to_aggregate_function and superlative_least_toughness_suffix_emits_min_aggregate_pt_comparison green

  • cargo run --profile tool --features cli --bin oracle-gen -- data --filter "the dining car|goblin ringleader" — The Dining Car upkeep clause emits Sacrifice + Aggregate { function: Min, property: Toughness, filter: creatures you control }, Comparator::EQ; 0 Unimplemented/Unknown on the fixed clause; Goblin Ringleader fully clean; no regression

Gate A

Gate A PASS head=de39411921ca8a62a5daddec8274100bffced85a base=302fab2fee76251c4ed835dfd8bc20fd3e02ee8a

Anchored on

  • crates/engine/src/parser/oracle_target.rs:4658 — the pre-existing value(AggregateFunction::Max, alt((tag("greatest "), tag("highest ")))) head this change lifts into an alt of two value(function, alt(...)) branches (PATTERNS.md §8b cross-product factoring)
  • crates/engine/src/parser/oracle_target.rs:4670 — the sibling property alt (Power/Toughness/ManaValue) that already parameterizes the second axis of this combinator, the pattern the direction axis now mirrors

Claimed parse impact

  • The Dining Car (upkeep least-toughness sacrifice clause)
  • Class-level: every "with the least/lowest/smallest <power|toughness|mana value> among <set>" card

Validation Failures

None.

CI Failures

None.

…(greatest→Max, least/lowest/smallest→Min)
@ntindle
ntindle requested a review from matthewevans as a code owner July 12, 2026 22:46

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request extends the superlative property suffix parser to support minimum superlatives ("least", "lowest", "smallest") mapping to AggregateFunction::Min, alongside the existing maximum superlatives, and adds comprehensive unit tests. The review feedback correctly identifies a violation of Rule R6 regarding a hallucinated Comprehensive Rules (CR) citation in the test comments, where CR 701.21 (drawing a card) was cited instead of CR 701.16 (sacrifice).

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.

Comment thread crates/engine/src/parser/oracle_target.rs
@github-actions

Copy link
Copy Markdown

Parse changes introduced by this PR · 3 card(s), 3 signature(s) (baseline: main 00320e03e3f5)

1 card(s) · ability/Bounce · field target: opponent controls creaturetoughness =min toughness of opponent controls creature opponent controls creature

Examples: Bounce Chamber

1 card(s) · ability/Sacrifice · field target: creaturepower =min power of you control creature creature

Examples: Witch-king, Bringer of Ruin

1 card(s) · ability/Sacrifice · field target: creaturetoughness =min toughness of you control creature creature

Examples: The Dining Car

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

@matthewevans matthewevans self-assigned this Jul 12, 2026

@matthewevans matthewevans left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified the parameterized Min/Max superlative parser, current three-card parse diff, and current CI; Gemini’s CR finding is refuted by docs/MagicCompRules.txt (701.21 is Sacrifice).

@matthewevans matthewevans added the enhancement New feature or request label Jul 12, 2026
@matthewevans
matthewevans added this pull request to the merge queue Jul 12, 2026
@matthewevans matthewevans removed their assignment Jul 12, 2026
Merged via the queue into phase-rs:main with commit 749a41f Jul 12, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants