Skip to content

fix(deck): stop corrupting card names that literally contain "//" (#4790)#5124

Merged
matthewevans merged 3 commits into
phase-rs:mainfrom
JacobWoodson:fix/4790-split-slash-card-name
Jul 5, 2026
Merged

fix(deck): stop corrupting card names that literally contain "//" (#4790)#5124
matthewevans merged 3 commits into
phase-rs:mainfrom
JacobWoodson:fix/4790-split-slash-card-name

Conversation

@JacobWoodson

Copy link
Copy Markdown
Contributor

Summary

Fixes #4790 — cards whose printed name literally contains // (e.g. SP//dr, Piloted by Peni) were unrecognized when added to a deck, both on import and manual add.

The client deck parser's normalizeCardName treated every / as a split-card separator, so "SP//dr".split("/").filter(Boolean) collapsed to two parts and the name was rewritten to "SP // dr, Piloted by Peni". The engine keys the card under its real //-glued name, so the spaced form matched nothing. This bit both entry points because repairParsedDecknormalizeCardName runs on import and on every deck load from storage.

Fix

Treat whitespace adjacent to // as the separator signal:

  • // with adjacent whitespace is a separator → normalize spacing to canonical " // " ("Fire// Ice", "Wear //Tear""… // …").
  • // glued with no adjacent whitespace is part of a printed name → left verbatim ("SP//dr, Piloted by Peni").
  • Single-slash exporter forms still upgrade to canonical ("Revival/Revenge", "Who / What / When / Where / Why").

Genuine glued split names ("Fire//Ice") are rare and still resolve through the engine's front-face // split, so preserving the real card name is the correct trade-off.

Verification

  • Frontend deckParser (vitest): 73 passed; tsc -b --noEmit clean. New tests cover the SP//dr name, irregular // spacing, multi-part single-slash, already-canonical names, and real DFCs (Peter Parker // The Amazing Spider-Man, Witch Enchanter // Witch-blessed Meadow) in spaced and glued forms.
  • Engine database::card_db (cargo test -p engine): 19 passed. Added two regression tests proving SP//dr, Piloted by Peni resolves to itself (not mis-split) and a glued combined DFC name resolves to its front face.

Note

A card entered as a fully glued combined name by hand ("Peter Parker//The Amazing Spider-Man") still loads (engine resolves it to the front face) but its deck-builder thumbnail falls back to a text tile, since card-data keys multi-face cards by their spaced display name + front-face name. Real importers emit the spaced form and manual add uses the front-face name, so this only affects the rare hand-typed-glued case. Happy to add a front-face fallback to the display lookup (scryfall.ts) if desired.

🤖 Generated with Claude Code

JacobWoodson and others added 3 commits July 4, 2026 23:00
…ase-rs#4790)

The deck parser's normalizeCardName rewrote any name with two slash-parts
into the canonical split form "A // B". For "SP//dr, Piloted by Peni" — a
single-faced card whose printed name literally contains "//" — split("/")
plus filter(Boolean) collapsed it to two parts, so the name became
"SP // dr, Piloted by Peni". The engine keys the card under its real
"//"-glued name, so the spaced form matched nothing and the card was
unrecognized on both import and manual add (repairParsedDeck runs on every
deck load from storage).

Fix: treat whitespace adjacent to "//" as the separator signal. A "//" with
adjacent whitespace ("Fire // Ice", "Fire// Ice", "Wear //Tear") is
canonicalized to " // "; a fully glued "//" with no adjacent whitespace is a
printed name ("SP//dr, ...") and is left verbatim. Single-slash exporter
forms ("Revival/Revenge", "Who / What / When / Where / Why") still upgrade to
canonical " // " via a uniform split/join.

Genuine glued split names ("Fire//Ice") are rare and still resolve through
the engine's front-face "//" split; preserving the real card name is the
correct trade-off.

Tests: added regression coverage for the "SP//dr" name, irregular "//"
spacing, multi-part single-slash, and an already-canonical name.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…fix

Peter Parker // The Amazing Spider-Man and Witch Enchanter // Witch-blessed
Meadow are genuine DFCs (// separates two faces). Verify the parser keeps the
canonical spaced form intact, repairs one-sided spacing, and leaves a glued
combined name verbatim (the engine's lookup_key resolves it via the front
face, mirroring the existing card_db test for a spaced combined name).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add two regression tests for the behavior the frontend phase-rs#4790 fix relies on:
- a single-faced card whose printed name contains "//" ("SP//dr, Piloted
  by Peni") resolves to itself via the exact-match path, not the "//" split;
- a glued combined DFC name ("Front//Back") resolves to the front face,
  identically to the canonical spaced form.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown

Parse changes introduced by this PR

✓ No card-parse changes detected.

@matthewevans matthewevans self-assigned this Jul 5, 2026
@matthewevans matthewevans added the defer-fe Frontend/client/UI PR deferred to Matt's direct review label Jul 5, 2026
@matthewevans

Copy link
Copy Markdown
Member

Reviewed under the maintainer automation sweep. This PR changes the client deck parser, so I’m marking it defer-fe for Matt’s frontend pass rather than approving or enqueuing it from automation. The engine-side file is regression-test coverage around the existing CardDatabase::lookup_key behavior; parse-diff reports no card-parse changes, and the current live checks are green.

@matthewevans

Copy link
Copy Markdown
Member

Thanks for your first contribution to phase.rs. This is a focused fix with the right regression coverage: it preserves the real SP//dr, Piloted by Peni name while keeping normal split/DFC separator cleanup intact.

@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.

Maintainer review: ready. The deck parser normalization fix is scoped, covered, and CI is green.

@matthewevans matthewevans added this pull request to the merge queue Jul 5, 2026
@matthewevans matthewevans removed their assignment Jul 5, 2026
Merged via the queue into phase-rs:main with commit 424cc80 Jul 5, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Bug fix defer-fe Frontend/client/UI PR deferred to Matt's direct review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

can't add [[sp//dr, piloted by peni]] — trying to add mutliple decks with this card, and everytime I do its unrecognize…

2 participants