fix(parser): accept literal thresholds in infix power/toughness comparisons (Wasp, Shrinking Savior)

[galuis116]

Closes #5874.

The bug

Wasp, Shrinking Savior — "…draw a card for each creature with power less than 0 on the battlefield." — parsed to a flat Draw { count: Fixed(1) }. The power less than 0 filter was silently dropped, so Wasp always draws 1 card instead of one per creature with power < 0. The parser's swallow-audit flags a DynamicQty SwallowedClause.

Root cause

An infix P/T comparison with a literal threshold doesn't parse. Isolation:

Phrase	PtComparison parsed
power 3 or less (postfix, literal)	✅
power less than the number of cards in your hand (infix, dynamic)	✅
power less than 3 (infix, literal)	❌
toughness greater than 4 (infix, literal)	❌

parse_pt_infix_tail parsed the threshold with parse_quantity_ref (dynamic game-quantity refs only), while the postfix tail (3 or less) uses parse_quantity_expr_number (literals + X). So an infix comparison against a bare number couldn't parse its threshold and the whole comparison failed.

Fix

The infix tail now tries the dynamic ref first (unchanged behavior) and falls back to the literal/X parser — the same value grammar the postfix tail already uses. General across targets and counts, power and toughness, less than/greater than.

Tests

• test_parse_pt_comparison_infix_less_than_literal — "power less than 3" → LE of Offset(Fixed(3), -1).
• test_parse_pt_comparison_infix_greater_than_literal — "toughness greater than 4" → GE of Offset(Fixed(4), +1).

Verification

cargo fmt --all ✓ · engine lib suite 16,612 pass / 0 fail + 2 new ✓ · clippy -p engine --all-targets -D warnings clean ✓

CR 208.1 + CR 107.1.

[gemini-code-assist]

Code Review

This pull request updates the infix power/toughness comparison parser in crates/engine/src/parser/oracle_nom/filter.rs to support literal thresholds (e.g., "power less than 3") in addition to dynamic quantities, and adds corresponding unit tests. The review feedback points out a missing CR 208.1 annotation in the newly modified parser code, which is required by the repository's style guide for rules-touching engine code.

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.

[gemini-code-assist]

[MEDIUM] Missing CR annotation in rules-touching parser code. Evidence: crates/engine/src/parser/oracle_nom/filter.rs:351. Why it matters: Every rules-touching line of engine code must carry a comment of the form CR <number>: <description> to ensure strict fidelity to the MTG Comprehensive Rules. Suggested fix: Add a CR 208.1 annotation to the comment block.

Suggested change

	// The threshold may be a dynamic quantity ("less than the number of …") OR a
	// literal number / X ("power less than 3", Wasp, Shrinking Savior). The
	// postfix form ("3 or less") already accepts literals via
	// `parse_quantity_expr_number`; the infix form must too, so try the dynamic
	// ref first (unchanged behavior) and fall back to the literal/X parser.
	let (rest, value) = alt((
	map(parse_quantity_ref, |qty| QuantityExpr::Ref { qty }),
	parse_quantity_expr_number,
	))
	.parse(rest)?;
	// CR 208.1: Power and toughness are printed values that can be compared against literal thresholds.
	// The threshold may be a dynamic quantity ("less than the number of …") OR a
	// literal number / X ("power less than 3", Wasp, Shrinking Savior). The
	// postfix form ("3 or less") already accepts literals via
	// parse_quantity_expr_number; the infix form must too, so try the dynamic
	// ref first (unchanged behavior) and fall back to the literal/X parser.
	let (rest, value) = alt((
	map(parse_quantity_ref, |qty| QuantityExpr::Ref { qty }),
	parse_quantity_expr_number,
	))
	.parse(rest)?;

References

1. Every rules-touching line of engine code must carry a comment of the form CR : (regex CR \d{3}(.\d+[a-z]?)?). ^(link)

[github-actions]

Parse changes introduced by this PR · 1 card(s), 1 signature(s) (baseline: main 674af5650599)

1 card(s) · ability/Draw · field count: ∅ → # of power ≤0+-1 in battlefield creature

Examples: Wasp, Shrinking Savior

[matthewevans]

Verdict: Changes requested

🔴 Blocker

Please add one discriminating integration test that exercises Wasp, Shrinking Savior through the production card/engine path and asserts its draw count uses the power < 0 battlefield filter. The two new parser-unit cases prove the grammar branch, but do not prove that the card conversion and resolution pipeline retains the parsed filter; this regression must fail when the production parser change is reverted.

🟡 Non-blocking

Gemini raised a missing-CR concern, but the current head already documents the relevant comparison handling with CR 208.1 and CR 107.3a/107.1, so no annotation change is needed.

✅ Clean

The literal threshold is expressed by reusing QuantityExpr and the existing infix comparison path; the parse-diff is current and limited to Wasp, Shrinking Savior, and required CI is green.

Recommendation: add the end-to-end regression test, then request re-review.

[matthewevans]

Verdict: Changes requested — the implementation seam is correct, but the proof must cover the full grammar axis.

🔴 Blocker

crates/engine/src/parser/oracle_nom/filter.rs:351-359 correctly extends the shared infix P/T-tail authority rather than adding a caller-specific arm, but the new tests cover only strict fixed-number examples. Please add:

1. A production-path Wasp, Shrinking Savior regression with creatures at negative, zero, and positive power, proving only the negative-power creature contributes to the resolved draw count.
2. Parser cases for an inclusive literal (less than or equal to 0) and the newly admitted X threshold, with the full QuantityExpr shape asserted.

Those cases prove the actual 0 boundary, the optional comparison clause, and the expanded literal/X axis. They must fail if the new alternative is removed.

🟡 Non-blocking

I compared this with #4866. That PR required a separate static-restriction parse because its source-first wording had been routed to the inverse CantBlock behavior. This PR does not repeat that mistake: parse_pt_comparison is already the shared target/static filter authority, so extending its comparison tail is the right location.

✅ Clean

The change reuses QuantityExpr and preserves dynamic thresholds by trying parse_quantity_ref before the existing literal/X parser. Runtime filter evaluation already resolves PtComparison values through the shared quantity evaluator.

Recommendation: add the three discriminating cases above, then request re-review.

[galuis116]

Thanks for the detailed review - all three points are addressed in cee8fd2.

1. Production-path resolution regression (negative / zero / positive power).
game::filter::tests::wasp_draw_filter_counts_only_negative_power_creatures parses the full Wasp, Shrinking Savior card through parse_oracle_text, extracts the draw count's QuantityRef::ObjectCount { filter }, and drives that exact parsed filter through matches_target_filter against three battlefield creatures at power -1, 0, and +1. It asserts only the -1 creature satisfies power < 0, so exactly one contributes to the resolved draw count. This is tied to the fix: removing the infix-literal alternative collapses the draw count to Fixed(1), the ObjectCount extraction returns None, and the .expect(...) panics -- the test fails.

2. Inclusive-literal + X threshold parser cases, full QuantityExpr asserted.

• test_parse_pt_comparison_infix_less_than_or_equal_literal: "power less than or equal to 0" -> PtComparison { Power, Current, LE, QuantityExpr::Fixed { value: 0 } } (non-strict, no offset -- pins the 0 boundary and the optional "or equal to" clause on the literal axis).
• test_parse_pt_comparison_infix_less_than_x: "power less than x" -> PtComparison { Power, Current, LE, QuantityExpr::Offset { inner: Variable("X"), offset: -1 } } (the newly admitted X threshold).

Both assert the complete QuantityExpr shape and fail if the new alt((parse_quantity_ref, parse_quantity_expr_number)) branch is reverted, since the literal/X value would no longer parse.

Rebased onto latest main; cargo fmt, full engine lib suite (16679 passed), and clippy -p engine --tests are green.

[matthewevans]

Verdict: changes requested — the grammar coverage is now clean, but the requested production resolution proof is still missing.

🔴 Blocker

crates/engine/src/game/filter.rs:7212-7268 parses Wasp and calls matches_target_filter for three synthetic creatures, but it never resolves the parsed QuantityExpr::Ref { qty: ObjectCount { .. } } through the quantity authority or executes the Draw effect. That proves the parser retained a filter and the leaf matcher understands it; it does not prove Wasp's draw count is one in the production resolution pipeline. A defect in game/quantity.rs's ObjectCount resolution (or in the Draw path's use of the count) would still pass this test.

Please extend the same fixture to invoke the parsed count through the shared quantity resolver and assert the concrete result is 1, or add an integration scenario that resolves the Wasp trigger and asserts exactly one card drawn. Keep the -1/0/+1 inputs; that is the discriminating boundary that proves the actual power < 0 behavior.

🟡 Non-blocking

The new parser cases now cover strict literal, inclusive literal, and X thresholds with complete QuantityExpr assertions, which closes the grammar-axis feedback from the prior review.

✅ Clean

oracle_nom/filter.rs:351-365 remains the shared infix P/T comparison seam, and the current parse-diff is limited to Wasp, Shrinking Savior. CI is green on this head.

Recommendation: request changes — add the one resolver or end-to-end draw assertion, then re-request review.

[galuis116]

Addressed the resolution-pipeline blocker in 66eb7f7.

wasp_draw_filter_counts_only_negative_power_creatures now resolves the FULL parsed draw count -- QuantityExpr::Ref { qty: ObjectCount { .. } } -- through game::quantity::resolve_quantity, the same shared authority the Draw effect consults in production, and asserts the concrete result is 1:

let resolved = resolve_quantity(&state, &count_expr, PlayerId(0), neg);
assert_eq!(resolved, 1, "the production quantity resolver must count exactly the one power<0 creature");

The -1 / 0 / +1 inputs are unchanged, so the discriminating boundary is preserved. A defect in game/quantity.rs's ObjectCount resolution (or in the count's use) now fails the test -- not merely a broken leaf matcher. The per-creature matches_target_filter checks remain as the boundary pin. Removing the infix-literal alternative still collapses the count to Fixed(1), the ObjectCount extraction returns None, and the .expect(...) fails -- so the regression stays tied to the parser change.

Rebased onto latest main; cargo fmt and the targeted test are green.

[matthewevans]

Verdict: Approved

🔴 Blocker

None.

🟡 Non-blocking

None.

✅ Clean

• The shared infix P/T-comparison parser now accepts the complete threshold grammar: dynamic quantities remain first, while literal values and X use the existing typed quantity parser.
• Strict versus inclusive boundaries are covered, and the Wasp regression verifies the parsed ObjectCount through the same target-aware quantity resolver used by Effect::Draw.
• The real parse diff is limited to Wasp, Shrinking Savior; the current branch is rebased onto main and includes the requested CR 208.1 annotation.

Recommendation: approved and queued for merge.