fix(client): distinguish a permanent's mana abilities and render the unbounded-counter mark in the hover panel

[lgray]

🤖 AI text below 🤖

Summary

Fixes two display-layer defects observed on a real 4-player board (Kilo, Apogee Mind / Freed from the Real / Relic of Legends / Pentad Prism): Relic of Legends' click prompt offered two byte-identical "Add one mana of any color" rows, and the hover status box kept showing a finite counter count after an accepted counter-growth shortcut while the battlefield pill and art-crop badge correctly showed ∞. Both are frontend-only — the engine already ships every value needed, so zero Rust changes.

BUG 1. abilityChoiceLabel synthesised the option label from the produced mana alone and dropped the engine's per-ability description, so any permanent with two mana abilities of the same produced shape collapsed to one indistinguishable option. The engine already ships the discriminator: the CR 602.1a activation-cost text. It is now attached as the option's description, gated on the engine's own CR 605.1a verdict (is_mana_ability === true) and the presence of engine-authored text.

Both conjuncts are load-bearing and measured. Gating on effect.type === "Mana" instead would regress loyalty mana abilities (Chandra, Torch of Defiance +1: Add {R}{R}) by rendering a cost line next to the +1 LoyaltyBadge — exactly what stripLoyaltyCostPrefix exists to suppress. Dropping && ability.description would let formatCost's fallback leak the literal word "Activate" as secondary text (20 of 32 AbilityCost variants have no explicit arm), and would put an identical uninformative {T} under every option of a dual/shock/triome land, whose labels (Add {W} / Add {U}) already discriminate.

BUG 2. CardInfoPanel was the one counter render site with no subscription to DerivedViews::unbounded_counters. The finite count is deliberate — materialize_object_growth_shortcut is documented display-only and is already covered end-to-end from this same dump by kilo_accept_marks_pentad_charge_as_unbounded_display_target. The twice-duplicated selector is extracted into useUnboundedCounterTypes and all three sites subscribe through it (a third copy-paste was the thing to avoid).

Also substitutes the engine's ~ self-reference token (CR 201.5) on the paths this change makes user-visible: abilityChoiceLabel's non-mana tail, the hover activate list, and both blocked-ability read-outs. Without this, the same cost string would render substituted in the hover preview and raw in the choice modal — an inconsistency this PR would otherwise have introduced. Census over the reported dump: 22 of 293 kind: "Activated" abilities leak ~ into cost text, 20 into effect text.

Files changed

• client/src/viewmodel/costLabel.ts — BUG 1 gate + cost line on all three produced sub-cases; ~ substitution on the non-mana tail
• client/src/hooks/useUnboundedCounterTypes.ts (new) — the extracted ∞-mark selector
• client/src/components/card/CardPreview.tsx — BUG 2 in CardInfoPanel; ~ in the activate list and the blocked-ability read-out
• client/src/components/board/PermanentCard.tsx — migrated onto the hook; ~ in the blocked-ability tooltip
• client/src/components/card/ArtCropCard.tsx — migrated onto the hook
• client/src/utils/description.ts — corrected CR cite (201.4b → 201.5) and a doc block that claimed a word-boundary guard the code does not implement
• client/src/viewmodel/__tests__/costLabel.test.ts, client/src/viewmodel/__tests__/manaAbilityOptionDiscrimination.test.ts (new), client/src/components/card/__tests__/CardPreview.test.tsx, client/src/components/board/__tests__/PermanentCard.test.tsx — tests

Track

Developer

LLM

Model: claude-opus-5
Tier: Frontier
Thinking: high

Implementation method (required)

Method: /engine-implementer

CR references

No engine code changed; these are the rules cited in the frontend comments this PR adds or corrects. All eight verified by grep against docs/MagicCompRules.txt.

• CR 201.5 (:1322) — ~ means the object it is on; sub-cases CR 201.5a (:1324, granted) / CR 201.5b (:1327, gained)
• CR 602.1a (:2516) — the activation cost is everything before the colon; the discriminator this PR surfaces
• CR 605.1a (:2683) — mana-ability classification, incl. the "not a loyalty ability" exclusion the gate relies on
• CR 606.1 (:2711) — loyalty abilities
• CR 732.2a (:6372) — shortcut proposal procedure, the origin of the ∞ mark
• CR 701.34a (:3594) — proliferate
• CR 201.4b (:1310) — removed: it is split-card name choice and never described this helper

Verification

• Required checks ran clean, or the exact CI-owned alternative is stated below.
• Gate A output below is for the current committed head.
• Final review-impl below is clean for the current committed head.
• Both anchors cite existing analogous code at the same seam.

All commands run in the PR's own worktree on the final rebased head a0a6e24c4, after pnpm install --frozen-lockfile (exit 0).

• pnpm exec tsc --noEmit — exit 0, no output
• pnpm lint — exit 0, ✖ 23 problems (0 errors, 23 warnings); identical to the pre-existing baseline, and zero warnings in any of the 10 touched files
• pnpm vitest run — exit 0, Test Files 267 passed | 4 skipped (271), Tests 2312 passed | 18 todo (2330), 0 failed
• ./scripts/check-parser-combinators.sh <base> — Gate A PASS (see below); vacuous by construction, no file under crates/engine/src/parser/ changed
• Cargo checks — not run, and not applicable: this PR changes zero Rust files (git diff --name-only | grep -c '^crates/' → 0). Per repo policy, cargo is not run from a worktree while Tilt owns the main checkout's target lock.

Test-count reconciliation. Base 438cad092 measured 2318 tests. This PR adds 11 (8 planned + 3 from the review round). Upstream added 1 between 438cad092 and 2c6596de9. 2318 + 11 + 1 = 2330. ✅

Non-vacuity (the reason to trust the above). The two acceptance tests were written first and both bugs were reproduced at tip before any fix: 3 failing assertions with their negative controls passing — the controls passing is what makes them discriminating. Every behavioural claim in this PR was independently revert-probed by a reviewer who did not write the code: reverting each edit alone turns its own test red, and each failure was checked to be attributable to that edit rather than to an upstream conjunct. Measured examples:

Reverted edit	Measured failure
description: costLine	expected 'Add one mana of any color' not to be 'Add one mana of any color'
is_mana_ability === true conjunct	loyalty option's description becomes '+1'
&& ability.description conjunct	description becomes the literal 'Activate'
∞ ternary in CardInfoPanel	expected 'Artifactcharge: 2' to contain 'charge: ∞'
isUnbounded prop only	the ∞ badge still renders, and the tooltip test alone fails — proving the prop, not the glyph, is that test's discriminator
~ substitution on the non-mana tail	expected '{T}, Sacrifice ~' to be '{T}, Sacrifice Ghost Quarter'

Fixtures are engine-serialized values copied verbatim from the reported dump (objects["402"] Relic of Legends, ["409"] Pentad Prism, ["110"] Ghost Quarter, ["7"] Hawkeye), not hand-authored approximations.

Gate A

Gate A PASS head=a0a6e24c4ce6521892747da658a21e3b71c0d712 base=2c6596de9360ef3e8d5df1c20e863631201253aa

Anchored on

• client/src/viewmodel/costLabel.ts:356,373,393,403 — four sibling branches of this same function already return { label, description } with the CR-annotated secondary line; BUG 1 adds a fifth case to that established shape rather than a new convention.
• client/src/components/board/PermanentCard.tsx:941 and client/src/components/card/ArtCropCard.tsx:216 — the two render sites that already read the engine's unbounded_counters channel and pass isUnbounded to CounterTooltip; BUG 2 makes CardInfoPanel the third consumer of that exact pattern, via the selector extracted from these two.

Final review-impl

Final review-impl PASS head=a0a6e24c4ce6521892747da658a21e3b71c0d712

Disclosure of exactly what ran when: /review-impl round 2 returned READY TO COMMIT (0 blocker / 0 high / 0 med, 3 low) against the pre-commit tree. Two of the three lows were comment-accuracy defects and were fixed; those fixes were then independently verified by a separate agent that re-derived every census figure itself and confirmed the diff was comment-only. The rebase onto 2c6596de9 touched no line of this PR (upstream's CardPreview.tsx edit is in CardImagePreview, disjoint from all four edit regions), and all three gates were re-run green afterwards on this head.

Claimed parse impact

None.

Scope Expansion

The review found that abilityChoiceLabel's non-mana tail and two blocked-ability read-outs leaked a raw ~. These were folded in rather than deferred: they are the same defect class, live in files already in scope, and GamePage.tsx was verified as the only production caller of abilityChoiceLabel, so a fix in the shared function is the root-cause fix rather than a per-caller guard. Leaving them would have shipped a self-inflicted inconsistency, since this PR is what makes the substituted form visible alongside the raw one.

Deliberately not in scope, each with a measured blocker:

• FU-A components/controls/AttackTargetPicker — a chip stands for N grouped objects while the mark is per object id, so it needs a per-counter-type .every() intersection threaded through groupByName/AttackerStack (mirroring isUnboundedPile). A representative-only lookup would render a false ∞, which is strictly worse than today's missing one — the fail-safe direction battlefieldProps.ts already documents in-code.
• FU-B components/hud/DialogAttachmentCard — no component test file exists, so a change there could not be made discriminating.
• FU-C OptionalEffectModal / TriggerOrderModal / ReplacementModal render engine description raw (51 replacement_definitions + 13 static_definitions in the dump carry ~). Different WaitingFor channel, different files, each needing its own fixture.
• F1 the real fix for the residual below: an engine-authored cost_text, so the frontend stops splitting Oracle text on ":" at all.

Disclosed residual. The && ability.description conjunct means two descriptionless mana abilities sharing a produced shape still collapse. Measured, not assumed: the engine has six non-parser mana-ability constructors, and two pairs qualify — Treasure + Gold and Spawn + Powerstone (game/effects/token.rs), reachable because apply_token_ability_payload extends obj.abilities without dedup. Those shapes collapse identically today, so this is not a regression; dropping the conjunct would fix them at the cost of the dual-land and "Activate" regressions above. The right fix is engine-side (F4: give those four constructors a .description(…); their CR 111.10 doc comments already hold the text), subsumed by F1.

Validation Failures

None.

CI Failures

None.

Summary by CodeRabbit

• Bug Fixes

  • Unbounded counters now display as ∞ consistently in cards, previews, and tooltips.
  • Ability labels and blocked-ability messages now replace ~ with the relevant card or source name.
  • Improved activated-ability labels, including mana ability descriptions, costs, and distinct choices.
  • Prevented unsupported or missing unbounded counters from appearing in the interface.

• Tests

  • Added coverage for counter display, self-reference substitution, ability labeling, and mana ability selection.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 40c8005e-71f0-4794-bc2f-6f545ef725ab

📥 Commits

Reviewing files that changed from the base of the PR and between b056ad6 and a0a6e24.

📒 Files selected for processing (10)

• client/src/components/board/PermanentCard.tsx
• client/src/components/board/__tests__/PermanentCard.test.tsx
• client/src/components/card/ArtCropCard.tsx
• client/src/components/card/CardPreview.tsx
• client/src/components/card/__tests__/CardPreview.test.tsx
• client/src/hooks/useUnboundedCounterTypes.ts
• client/src/utils/description.ts
• client/src/viewmodel/__tests__/costLabel.test.ts
• client/src/viewmodel/__tests__/manaAbilityOptionDiscrimination.test.ts
• client/src/viewmodel/costLabel.ts

---

📝 Walkthrough

Walkthrough

Updates card and ability displays to resolve ~ using the source object name, centralizes unbounded-counter lookup, renders marked counters as ∞, and expands viewmodel and component tests for mana labels, blocked abilities, activation labels, and counter behavior.

Changes

Ability label formatting

Layer / File(s)	Summary
Ability-choice formatting and validation client/src/viewmodel/costLabel.ts, client/src/utils/description.ts, client/src/viewmodel/__tests__/*	abilityChoiceLabel now applies object-aware ~ substitution and conditionally includes engine-provided mana descriptions; tests cover mana, loyalty, activation, and option-discrimination cases.

Unbounded counter lookup

Layer / File(s)	Summary
Shared unbounded-counter hook client/src/hooks/useUnboundedCounterTypes.ts	Adds a hook that reads per-object unbounded counter types and returns a stable empty array when data is unavailable.

Card component rendering

Layer / File(s)	Summary
Board and preview display integration client/src/components/board/PermanentCard.tsx, client/src/components/card/ArtCropCard.tsx, client/src/components/card/CardPreview.tsx, client/src/components/{board,card}/__tests__/*	Card components use the shared counter hook, display unbounded counters as ∞, and resolve ~ in blocked-ability and activation text; regression tests cover these displays.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested labels: bug

Suggested reviewers: matthewevans

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main UI fixes for mana-ability disambiguation and unbounded counter rendering.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[matthewevans]

Approved after current-head review: frontend and Rust CI are green; merge state is clean; CodeRabbit has no actionable findings.