Skip to content

Parsing: Commas and Apostrophes in Bare Name Searches - #895

Open
daveycodez wants to merge 5 commits into
jbylund:mainfrom
daveycodez:comma-name-search
Open

Parsing: Commas and Apostrophes in Bare Name Searches#895
daveycodez wants to merge 5 commits into
jbylund:mainfrom
daveycodez:comma-name-search

Conversation

@daveycodez

Copy link
Copy Markdown
Contributor

rograkh, son of rograkh was a parse error; so was urza's bauble. Both work on Scryfall, and card names are full of exactly this punctuation. Based on main; touches only the two parser files + a new test file. (Note: #872/#879/#893 also touch the parser files — the hunks here are in the tokenizer/regex layer neither PR modifies, so merges stay clean in either order.)

Measured semantics (api.scryfall.com, 2026-08-08)

Input Scryfall This PR
rograkh, son of rograkh finds Rograkh, Son of Rohgahh same — bare words shed attached commas (son, returns the identical 275 results as son)
rograkh , son works standalone commas skip like whitespace
t:goblin, matches nothing (comma stays in the value) same — mirrored rather than second-guessed
"son," quoted = verbatim same
urza's bauble finds Urza's Bauble apostrophe is word-internal when a word character follows
name:'power' unchanged: a leading apostrophe still opens a quoted string, and a dangling urza' is still an unclosed-quote error

Implementation

Identical rules in both parsers, asserted per-case by parity tests:

  • Hand tokenizer: the word scan accepts , and word-internal '; the whitespace skip set gains ,; bare-word name nodes rstrip(",") (quoted names and field values untouched).
  • pyparsing: the word and value regexes accept the same shapes; default whitespace chars gain ,; the implicit-AND pre-tokenizer (whose quote-balance check was what actually rejected urza's) gets the same word shape; , joins _FP_UNSAFE_CHARS so such queries take the full grammar.

Deliberately out of scope (each a different defect)

  • will-o-the-wisp lexes fine (hyphenated names reassemble) but misses because the real name has an apostrophe the query lacks — that's punctuation-insensitive name matching, a data/matching feature akin to the existing accent folding (Eowyn should match Éowyn #649), not tokenization.
  • circle of protection: red (colon inside a name) is still a parse error.

Happy to file/take either as follow-ups.

Validation

  • New api/parsing/tests/test_special_char_names.py: equivalence cases, per-case parser parity, verbatim quoted/field values, apostrophe-in-value, error cases still error.
  • Suites: api + parsing + scripts + client 2,318 passed; ruff + format clean.
  • Live on the dev stack: rograkh, son of rograkh → Rograkh, Son of Rohgahh; urza's bauble → Urza's Bauble; pow=2, tou=3 → 952 cards — each matching Scryfall.

@github-actions github-actions Bot added parser All things parser related api Changes to the HTTP API / request handling in api_resource.py tests Test suite additions or changes python size/M 100-316 changed lines labels Aug 8, 2026
@github-actions github-actions Bot added frontend Changes to the static JS/HTML/CSS client UI javascript size/L 317-999 changed lines and removed size/M 100-316 changed lines labels Aug 11, 2026
`rograkh, son of rograkh` was a parse error, and so was `urza's bauble`
— both work on Scryfall, and card names are full of exactly this
punctuation. Both tokenizers rejected the characters outright: ',' had
no lexer rule at all, and ' always opened a quoted string, so a
possessive mid-word meant an unclosed quote.

The rules, measured against api.scryfall.com (2026-08-08):

- A comma attached to a bare word is shed from the name filter
  ("son," returns the identical 275 results as "son").
- A comma standing alone is skipped like whitespace ("rograkh , son"
  filters exactly as "rograkh son").
- A comma in a FIELD value stays verbatim — "t:goblin," matches
  nothing on Scryfall either, so mirroring beats guessing.
- Quoted names keep their commas.
- An apostrophe is part of a word when a word character follows
  ("urza's"); a leading apostrophe still opens a quoted string, so
  name:'power' lexes exactly as before, and a dangling "urza' " is
  still an unclosed-quote error.

Implemented identically in both parsers — the hand tokenizer's word
scan and skip set, and the pyparsing grammar's word/value regexes,
default whitespace characters, and implicit-AND pre-tokenizer (whose
quote-balance check was what actually rejected "urza's"). The comma
also joins the fast-path unsafe characters so those queries take the
full grammar. Parser parity asserted per case in the new test file.

Not covered, deliberately: "will-o-the-wisp" already lexes (hyphenated
names reassemble) but misses because the real card name carries an
apostrophe the query lacks — punctuation-insensitive name MATCHING is
a different feature than tokenization. "circle of protection: red"
(colon inside a name) is likewise still a parse error. Both are noted
in the PR rather than crammed in here.

Suites: api + parsing + scripts + client 2,318 passed; ruff + format
clean. Live on the dev stack: "rograkh, son of rograkh" -> Rograkh,
Son of Rohgahh; "urza's bauble" -> Urza's Bauble; "pow=2, tou=3" ->
952 cards, all matching Scryfall.
This branch made a word-internal apostrophe part of the word, but left the quote balancer treating
every apostrophe as an opening quote. So the frontend turned `urza's` into `urza's'` before sending
it, and the search failed with `Failed to parse query: "urza's'"`.

That is worse than the behavior it replaced. Before this branch, `urza's` at least parsed — as
`name:urza AND name:s`, a coincidental match, but a result. The fix as shipped made the UI's most
obvious apostrophe query an error, and only through the balancer: the same query typed straight into
the API worked fine, which is why nothing caught it.

Both balancers get the same rule the tokenizer uses — a word character either side of an apostrophe
makes it part of the word:

  api/parsing/parsing_f.py   balance_partial_query
  api/static/app.js          balanceSuffix

An apostrophe with no word character before it is still an opening quote, so `name:'power` still
balances to `name:'power'`, and `urza's'` still gets its closing quote because nothing follows it.

Eight cases added to static/fixtures/balance_queries.json — the fixture BOTH balancers are tested
against, so the two cannot drift. That shared fixture is why this is a five-line change rather than
two independent ones.

Gates: pytest api/ 2,367 passed, 19 xfailed; jest 1,749 passed; ruff clean.
Typing toward "urza's" broke at the apostrophe. "urza'" was a lex error, so the balancer did the
only thing it could and appended a second one. "urza''" parses — but as `urza` AND an empty quoted
string, so the search silently widened to every card containing "urza" and the count line read

    35 cards where the name contains Urza and

with nothing after the "and". The apostrophe the user typed was gone from the search, the result
count was wrong, and the request on the wire carried two apostrophes for the one that was typed.

_scan_word_end took an apostrophe into the word only when a word character followed it. At end of
input nothing follows, so the word ended and the apostrophe opened a string. But end of input is
not evidence of a quote — it is evidence of a half-typed word, which is the normal state of every
keystroke a typeahead sees. It now counts as word-continuation.

Only end of input. "urza' bauble" is unchanged and still an error: an apostrophe followed by a
space really is an unclosed quote, and test_dangling_apostrophe_is_still_an_error pins it. The
pyparsing side needs (?:'(?=\Z))? rather than a bare '? for exactly this reason -- an unanchored
optional apostrophe swallows the one in "urza' bauble" and the error disappears.

FOUR places encode the word shape and all four had to move together, which is the story worth
keeping: hand_parser._scan_word_end, and in pyparsing_based the `word` production, the
`string_value_word` used for field values, and the `string_value_tok` in the implicit-AND
pre-tokenizer -- whose comment already said "same word shape as the main grammar". The first three
agreeing was not enough: the pre-tokenizer ran first and rejected the query before the grammar was
consulted. Balancers mirror the same rule in parsing_f and app.js; a balancer that disagrees with
the lexer emits something the lexer rejects, which is how this started.

Separately, in nodes.py: an operand that explains to "" is not a constraint and has no clause to
contribute, but it was still joined, leaving the dangling connector above. A TrueNode is the usual
source -- an empty quoted string is one. Filtering on the empty string rather than on TrueNode
covers anything else that explains to nothing, and "''" now explains as "" instead of the same
dangling shape.

The shared balance fixture flips one pinned case: "urza's'" now balances to itself rather than
gaining a third apostrophe, since the trailing one is part of the word. Added "urza'", "o'",
"urza''" and "urza' " (the last still takes a closing quote -- a space follows, so it is a quote).

Tests: 1720 parser tests pass, 19 xfailed; jest 1753 pass; ruff check, ruff format and prettier
clean. Explanation cases pin "urza'", "urza's", "urza''" and "''"; the balancer's end-to-end parse
check gains "urza'" and "o'".
@github-actions github-actions Bot added size/M 100-316 changed lines and removed size/L 317-999 changed lines labels Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api Changes to the HTTP API / request handling in api_resource.py frontend Changes to the static JS/HTML/CSS client UI javascript parser All things parser related python size/M 100-316 changed lines tests Test suite additions or changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant