Fix Endless Ranks of HYDRA - #6991
Conversation
github-actions
Bot
added
the
needs-maintainer
|
Warning Review limit reached
Next review available in: 5 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe semantic audit now recognizes descriptionless delayed-trigger graveyard-to-hand effects and adds regression coverage. The engine also normalizes relative paths to forward slashes after string conversion. ChangesDelayed graveyard recursion audit
Relative path normalization
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/engine/src/game/coverage.rs`:
- Around line 9420-9443: Update the graveyard-to-hand predicate in
ability_tree_any so it only credits the ChangeZone leaf when effective_lower
contains the normalized self-reference marker (such as “return ~”), while
retaining the existing return, graveyard, and hand checks. Add a two-line
regression case covering both a SelfRef recursion effect and a targeted
creature-card return to ensure credit does not cross lines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b6653c3c-3ed6-480a-b18e-46e7c09fc850
📒 Files selected for processing (2)
crates/engine/src/game/coverage.rscrates/engine/src/game/engine.rs
|
Generated for head Parse changes introduced by this PR✓ No card-parse changes detected. |
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Changes requested — this current head can still mark an unrelated return line as covered.
🔴 Blocker
crates/engine/src/game/coverage.rs:9434-9456 searches a descriptionless SelfRef ChangeZone leaf across every ability tree, but the predicate is keyed only to return / graveyard / hand in the candidate line. A card with a self-recursion line plus an unrelated return target creature card from your graveyard to your hand line can therefore cross-credit the unrelated line and recreate the SilentDrop false-green. CodeRabbit identified the core cross-credit issue; the current-head code confirms it.
Please make the self-reference check line-local, accepting the legitimate return this card form as well as normalized card-name/~ references, and add a two-line negative regression containing both the self-recursion and targeted-return lines. Do not restrict this to norm.contains("return ~"): normalize_card_name_refs intentionally does not normalize this card (crates/engine/src/game/coverage.rs:8684-8687; crates/engine/src/parser/oracle_util.rs:888-895).
✅ Clean
The descriptionless delayed-trigger traversal itself is the appropriate mechanism to preserve; the required guard is its line-to-leaf association.
Recommendation: request changes — add the line-local self-reference guard and the discriminating two-line regression, then request re-review.
matthewevans
removed
the
needs-maintainer
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Verified the requested line-local self-reference guard and the two-line cross-credit regression on current head 2b79a3a. All required CI checks are green.
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Approved for exact head 2b79a3aabcdd282751d1f43ba07d0633ca38fa02: current parse evidence is clean and all required checks are green.
2 participants
Summary
Fixes a parse-fidelity defect on Endless Ranks of HYDRA.
Issue: audit-flagged: SilentDrop — part of the Oracle text is silently dropped from the parsed AST; identify the missing clause and restore it.
Files changed
CR references
Track
Developer
LLM
Model: claude-opus-4-8
Thinking: high
Tier: Frontier
Verification
cargo fmt --all— clean./scripts/check-parser-combinators.sh (Gate A)— PASS exit 0 (Gate G + full combinator scan). Family-D skipped: python3/python are permission-denied WindowsApps stubs (anticipated env limitation). Achieved clean skip by running with WindowsApps dir filtered from PATH socommand -v python3is false and the script bypasses the D0 self-test and completes toGate A PASS.cargo clippy-strict— clean (exit 0)cargo test -p phase-engine— clean after in-loop fix. One failure: Windows-only path-separator bug in pinned CR 603.5 census test (crates/engine/src/game/engine.rs:15185) where Path::display() emits backslash vs pinned forward-slash coordinates; census counts passed. Fixed with .replace('','/') when buildingrel(no-op on Unix, pinned line numbers/counts untouched). Re-run: integration 4480 passed/0 failed, lib passed, exit 0.cargo export-cards data --stats --sidecar-dir client/public && cp client/public/card-data.json data/card-data.json— clean (recipe corrected). Literal recipe streams main JSON to stdout (binary writes file only with --output) so client/public/card-data.json stayed stale; added --output client/public/card-data.json to write it fresh from this branch's engine, then copied to data/card-data.json (fresh 04:22, 34868 cards, 92.1% implemented).cargo coverage— clean (exit 0); Endless Ranks of HYDRA supported:true gap_count:0 (Token + CreateDelayedTrigger both supported)cargo semantic-audit— clean (exit 0); Endless Ranks of HYDRA has 0 findings (absent from fresh data/semantic-audit.json)Scope Expansion
The SilentDrop is a verified FALSE POSITIVE (both Oracle lines are fully in the AST); per the approved plan the class-level cause is in the semantic-audit heuristic, not the parser, so the fix adds one structural ChangeZone(GY->Hand, SelfRef) arm to line_matches_effect_type in game/coverage.rs (QA tooling only, zero parser/card/runtime change).
Validation Failures
None.
CI Failures
None.
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests