Import+Frontend: Merge Card Faces Into One Searchable Row, and Flip to the Back - #894
Open
daveycodez wants to merge 12 commits into
Open
Import+Frontend: Merge Card Faces Into One Searchable Row, and Flip to the Back#894daveycodez wants to merge 12 commits into
daveycodez wants to merge 12 commits into
Conversation
daveycodez
force-pushed
the
dfc-face-merge
branch
from
August 9, 2026 19:16
12e3014 to
d3d6247
Compare
daveycodez
force-pushed
the
dfc-face-merge
branch
2 times, most recently
from
August 9, 2026 23:29
5aa7a3a to
fb7c3a6
Compare
daveycodez
marked this pull request as draft
August 10, 2026 20:37
daveycodez
marked this pull request as ready for review
August 15, 2026 22:52
This was referenced Aug 16, 2026
…o the Back Every multi-face printing used to fan out to one row per face sharing one scryfall_id; the upsert's ON CONFLICT then kept whichever face came last — the back. That is how every battle vanished (t:battle matched zero rows corpus-wide), t:sorcery missed every MDFC spell side, front-face oracle text was unsearchable (jbylund#873), and ~1,343 printings carried the wrong types, colors, and stats (jbylund#400's audit). The fix is the "single conceptual card" jbylund#400 discussion pointed at, and it turns out to be the semantically correct shape, not a compromise: Scryfall ANDs search predicates at the CARD level, each satisfiable by any face — measured against api.scryfall.com (2026-08-08): `t:sorcery t:land` returns the MDFC lands, `o:` conjunctions match across faces (Ral, Monsoon Mage), and `c:b` matches Westvale Abbey's back-face-only color. One row per printing carrying any-face unions reproduces that directly; one row per FACE would instead break every cross-face conjunction (no single face satisfies both terms). - preprocess_card still runs each face through the full pipeline, then _merge_processed_faces collapses the rows: the front supplies identity and display (cmc, mana cost, illustration, image, prices, raw blob — matching Scryfall's own top-level fields); card_types/card_subtypes/ card_colors/card_keywords/produced_mana union across faces; oracle_text and flavor_text join with a newline face separator; type_line joins with " // " exactly as Scryfall renders it. The P/T and loyalty stat groups come from the first face that has them, as a group, so numeric and _text columns never describe different faces. - "Battle" joins CARD_TYPES in the parser: it was absent, so t:battle fell through to the subtype arm on both the SQL and engine paths — a guaranteed zero-match the moment battles became visible. No engine change: the store already had TYPE_BATTLE and reproduced the miss in isolation before the vocabulary fix. - The raw blob is the front face's dict (every existing top-level consumer keeps meaning "the front") with the raw card_faces re-attached; copy_images_to_s3 now uploads card_faces[1]'s image under the face-2 key — and stops writing every face to the face-1 key the upload path had hardcoded. - The site gets a Scryfall-style flip button, as progressive enhancement so the no-JS render (parity fixture) is untouched: a card named "A // B" is probed for a face-2 CDN image, and only cards that have one — transform/MDFC, never split/adventure — grow the button, in the results grid, the modal, and the card page. Measured on the rebuilt dev stack after reimport (97,802 cards): t:battle 0 -> 36 cards / 53 printings, exactly Scryfall's 36; t:sorcery t:land 20, exactly Scryfall's 20; the jbylund#873 commander query 21 -> 34 of Scryfall's 43, and all 9 misses are never-legal draft_innovation cards the corpus excludes by policy; Fire // Ice findable by both halves' text; Brutal Cathar matches t:human again. Known residual, tested and documented: when several faces carry a stat group (2/2 // 3/3), only the front's values are searchable — Scryfall also matches the back's pow=3. Per-face numerics are a follow-up if measurement warrants. Suites: api + parsing + scripts 2,193 passed; jest 1,747 passed (6 new flip tests); ruff + format + prettier clean; app.min.js rebuilt.
The merge sets the merged row's raw_card_blob to the FRONT FACE's dict with card_faces re-attached, so that existing top-level reads keep meaning "the front". Going through those reads, exactly one needs that: image_uris, which this PR already gives a card_faces->0 fallback in copy_images_to_s3.py. Everything else read from the blob (lang, set_type, games, finishes, frame_effects, image_status, reserved, game_changer) is card-level and identical either way. With that covered, the blob can hold the card verbatim, which is worth more than the promotion. Every searchable field is merged onto the row's own columns, so the blob has no derivation left to do, and keeping it as Scryfall sent it is what makes the card answerable later: a card object cannot be rebuilt from a face. card_faces is gone, name and type_line are the front's, and which fields a real card carries at top level varies by LAYOUT -- a split card has mana_cost and image_uris there, a transform card does not -- so no rule strips the promoted keys correctly for both. Measured on a transform and a split card: the blob now differs from the object Scryfall sent by the lifted card_name and nothing else, where before it had 11 extra top-level keys and three that disagreed (object=card_face, the front's name, the front's type_line). prefer_weights.py's seven image reads go through one front_image() helper that coalesces to card_faces->0; both of its SQL templates were run against the 97,808-card corpus. Five new tests in TestMultiFaceRawBlob pin the invariant for both layouts and fail on the previous behavior. Suites: api + parsing + scripts + client 2,514 passed, jest 1,747 passed, ruff clean.
Every transform/MDFC printing on the CDN is wrong today: img/bot/6/1/745.webp --
the FACE-1 key, which the site renders as the front -- is Slicer, High-Speed
Antagonist, the back. Face 2 is a 403. The front image is nowhere.
The cause predates this branch. Until the face merge, a multi-face row WAS its
last face, so raw_card_blob is the back face's dict; Scryfall omits top-level
image_uris on a multi-face card and puts one on each face, so
`raw_card_blob->'image_uris'` resolved to the BACK's images, and process_card
wrote them under a hardcoded face-1 key.
The previous two commits do fix this for rows written from now on: the blob is
the card verbatim, so top-level image_uris is absent on a transform card and the
coalesce falls through to card_faces->0 as intended. What they cannot fix is the
corpus that already exists. Every row imported before them still holds the old
blob, where the first coalesce branch is non-NULL and wrong, so a sync run today
keeps uploading the back face as the front until a full reimport has rewritten
every row.
Reading columns instead removes the dependency entirely:
- the image URL is a pure function of scryfall_id, which is NOT NULL and
uniquely indexed;
- whether a back face exists is card_layout, which is indexed and
lowercase-checked, and correctly excludes split/adventure/flip -- they share
a "//" name and have no second physical face.
That is right on the existing corpus with no reimport, and stays right for rows
whose blob never captured the faces at all. Verified against Scryfall: both
derived URLs for bot/6 return 200, and the front one is Slicer, Hired Muscle.
ONE-TIME REPAIR REQUIRED, and this part no reimport fixes either. The S3 diff is
`db_cards - s3_cards`, so it only uploads what is ABSENT -- and the wrong face-1
objects are present. They will never be replaced by a normal run. Run once with
--no-skip-existing to overwrite them; the flag already exists.
That same append-only diff means a card Scryfall re-scans keeps its stale art
forever. image_updated_at is the natural key for it. Left alone here: a real
bug, but a different one.
The new test pins the query text, which is unusual and deliberate: every other
test in the file mocks fetchall, so all of them pass just as happily against a
query reading the wrong column. That is exactly how this shipped.
The button worked and looked borrowed. Measured against the control it is modelled on -- .card-grid-item-transform-button in Scryfall's own stylesheet -- every value was wrong: position top:1.6em right:1.6em -> over the art's right edge, ~26% down size 2.2em (font-relative) -> 44px fill --color-card-background -> #fff border none -> 2px solid #343242 rest opacity .85 -> opacity .6 hover rotate(180deg) -> opacity 1, no transform Position was the substantive one, and it needed more than copying their numbers. Scryfall's container (.card-grid-item) holds the image and nothing else, so top:26% lands on the art. Ours (.card-item) holds the image PLUS the name, mana, type, text and set rows, so the same percentage measures a box roughly twice as tall and drops the button into the card body. Moving the button inside .card-page-link would fix the geometry and nest a <button> in an <a>, which is invalid and breaks keyboard traversal -- so the tile becomes a query container instead and the offsets are expressed against the image: a card's art is a fixed 488x680, so its height is always 1.3934x the tile's content width, and 26% of that is 36.23cqw. The modal keeps plain percentages, its wrapper being image-only. Theme colours were the other real problem. The control sits ON the artwork, which is neither theme: against a black card border in dark mode it nearly vanished, and against bright art in light mode it read as a hole punched in the card. A white disc with a dark ring resolves against both because it depends on neither. The glyph is now an SVG. A text character's ink box is not its em box, so flex centring aligns the em box and leaves the arrow visibly off-centre -- by an amount that changes with whichever fallback font renders it, so nudging it would have been tuning for one machine. The outline strokes with currentColor, which also gives the inverted state its icon colour for free. 22px inside the 44px disc: Scryfall insets theirs 7px, but their artwork carries padding inside its own viewBox where this one strokes to the edge, so matching the inset would make a visibly heavier mark than theirs. Matching the ink is what reads the same. And it carries STATE now, not just an action: while the back face is showing the button inverts to a dark fill with a white ring (Scryfall's `.spooky`). Once the art has changed, the button is the only thing on screen that still says which face you are looking at. Toggled on click rather than after the transition, so the control answers the press, and mirrored into aria-pressed. The 180-degree hover spin is gone. The glyph already says "turn over"; spinning the control implied the control was the thing that rotates. Sizes and centring were compared by rendering the button at its real proportions rather than judged from the markup. prettier clean.
The button's offsets are percentages of the card image, but nothing it was anchored to IS the image. A grid tile (.card-item) is the image plus the name, mana, type, text and set rows, so a percentage measured from it lands well below the art. The modal's .modal-image-wrapper is a flex area far wider than the picture centred inside it, so on a single large card the button sat out in the margin BESIDE the card rather than on it. The frame cannot be the <a> that already wraps the image: a <button> inside an <a> is interactive content nested in interactive content, invalid and a break in keyboard traversal. So the JS that injects the button now also injects a plain <div class="card-image-frame"> around the link, with width: fit-content so the box is the image's box. Both call sites go through one helper. Injected rather than templated, deliberately: the server render and its parity fixture are untouched, and the frame exists only on the cards that actually get a button. An earlier attempt derived the image's height from the fixed 745x1041 aspect and placed the button with container units. That was right for the grid and would have been wrong in the modal, where the image is sized by the available HEIGHT rather than by width. A box that hugs the image needs no arithmetic and cannot disagree with the image. prettier clean.
`card_art_tags` was attached by joining on `illustration_id`, which since the face merge is the FRONT face's -- so an art tag that exists only on a double-faced card's BACK art was unreachable by any query. Scryfall answers otherwise, and it is measurable against api.scryfall.com (2026-08-16): `arttag:snow e:khm` is 75 there and 73 under the front-only reading, missing Birgi // Harnfel and Esika // The Prismatic Bridge, whose snow is on the back art; and `-art:human e:khm t:creature` is 135 there against 136, the surplus being Valki // Tibalt, because Tibalt is the human and Tibalt is the back. A printing SHOWS every face's art, so `illustration_ids` records exactly that -- front first, deduped -- and it is the merge that makes the column necessary: before it, each face was its own row carrying its own illustration, and the tag join reached the back face because the back face WAS a row. `_FACE_LIST_UNIONS` is the whole of the merge-side change, because each face row already computes its own single-element list; a split or adventure card, whose one illustration sits on the card and not on its faces, dedupes back to one entry. The sync is keyed on `scryfall_id` rather than `illustration_id` because an illustration id no longer determines a row's whole value: a double-faced card's tags come from two of them, and a batch of illustration ids can split a card's illustrations across two statements where a batch of cards cannot. `_union_art_tags` resolves the union in Python before `_sync_card_tags` -- which is unchanged, and still shared with the oracle path -- and a single-illustration card references the dump's existing dict rather than copying it, so the corpus-wide allocation is bounded by the 9,368 printings that show more than one illustration. The rule was checked against the corpus rather than assumed. Replaying the 2026-08-16 art-tags dump over the bulk cards, the union predicts Scryfall's answer for `arttag:snow e:khm` set-for-set -- 75 names, no surplus and no shortfall -- where the front-only reading predicts 73. Of those 9,368 multi-illustration printings, 5,491 gain at least one tag from a non-front face. `prefer_score` reads the union too, and deliberately: `art_style` in backfill_prefer_scores.sql asks what art a printing shows, and a printing shows all of it. Splitting the column into two readings would buy three rows -- exactly three printings corpus-wide flip `art_style` under the union: Tribute to Horobi // Echo of Death's Wail neo/356 in English and German, whose back art is `anime`, and Thaumatic Compass // Spires of Orazca pxtc/249, whose back art is `line-art`. The migration backfills from `raw_card_blob->'card_faces'`, which on an already-imported row is the only record of the back's illustration. Without it the column would be empty until the next card import and every card would be cleared of its art tags in the meantime; the backfill is run against real rows in test_upsert_cards.py so it cannot drift from what preprocess_card writes.
daveycodez
force-pushed
the
dfc-face-merge
branch
from
August 21, 2026 23:39
a9bfc56 to
7d84e93
Compare
…ylund#963 Put It Upstream jbylund#963 moved _upsert_cards and the pools off APIResource onto AdminResource / AppContext; the tests this branch adds still called api_resource._upsert_cards and api_resource._conn_pool, which is what the python-test job red on the rebase was. Same accessors upstream's own tests use now: api_resource.admin._upsert_cards, api_resource.app_context.reader_pool.
# Conflicts: # api/parsing/tests/test_pyparsing_parser.py
…tes Was Answering Queries
Merging a card's faces into one row is what this branch is for, and joining their
texts is how it makes every face searchable. `_merge_processed_faces` glues
`oracle_text` and `flavor_text` with `_FACE_TEXT_SEPARATOR` ("\n//\n"). THIS
BRANCH INVENTED THAT STRING. Scryfall stores nothing like it and never joins — it
matches each face separately — so every character of the separator is a position
in our haystack and in nobody else's, and patterns were answering from it. The
defect cannot exist on main, where each face is its own row; it arrives with the
merge, so it is fixed here.
Measured against api.scryfall.com, 2026-08-28 (its answer on the left, what a
joined column answers on the right):
o:/\ndraw/ 381 vs 389 the 8 extras every one a two-face card whose
BACK face opens with "Draw" — the separator
ends in a newline, so "\n//\nDraw…" contains
"\ndraw" and the join answers what no face does
o:/\sdraw/ 3,604 vs 3,611 the same eight, `\s` reaching that newline
o:/\nwhenever/ 3,839 vs 3,885 and 46 more of them
o:/\/\// 1 vs 849 the one real card is SP//dr
o:"//" 1 vs 849 the same query after `lower_literal_regexes`
ft:/\/\// 0 vs 262 flavor text is joined the same way
t:/\/\// 930 and the type line is NOT ours to undo
THE FIX IS TO SPLIT THE STORED VALUE BACK at match time and run the pattern, or
the needle, per segment. No store change, no reimport, no new column: the bytes
were always right and only the reading of them was wrong. A single-face card has
no separator, so the split yields one segment and costs one scan for a needle
that is not there.
THE SUBSTRING PATH IS NOT THE EXOTIC HALF, IT IS THE HALF USERS REACH.
`o:/\/\//` is a plain literal, so `lower_literal_regexes` rewrites it into the
contains-predicate `o:"//"` before the engine sees a pattern at all — a fix
applied only to the regex arm would have left the largest row of that table
exactly where it was. Five sites in card_engine/src/filter.rs, because a text
predicate on a joined column reaches an answer five different ways and all five
had to agree: the two evaluation arms (`TextRegex`, `TextContains`), the two
bind-time `FlavorMatch` rewrites that `ft:` takes instead of evaluation, and the
verify beneath the memoized `OracleMatch` set. The jbylund#734 trigram narrow above that
verify is untouched and stays sound either way — a face is a SUBSTRING of the
join, so every literal factor the narrow requires of a survivor is still in the
joined text it indexed — but the verify is the answer, and a needle straddling
the separator must not survive it.
THE SQL FALLBACK NEEDED THE SAME SEMANTICS, or `_search_sql` and `_search_engine`
would answer one query two ways. `oracle_text`/`flavor_text` leaves now compile to
a per-face EXISTS over `string_to_array(col, separator)`. The `:` form keeps the
whole-value `LIKE` as its leading conjunct so `gin_trgm_ops` still drives the
scan and only survivors pay for the split — sound because a face is a substring
of the join, so that conjunct can only ever be a superset. The regex form gets no
such prefilter: `~*` is not newline-sensitive, so after the split `^`/`$` bind at
each face's ends and a face can match where the join does not. The `CASE WHEN col
IS NULL` guard keeps the leaf three-valued — EXISTS is two-valued, and without it
`-ft:/x/` would start matching every printing with no flavor text at all. Verified
row-for-row against a real PostgreSQL 14 on the fixture below: engine and SQL now
return identical sets for all 18 queries tried, negations and the NULL column
included.
ANCHORS BIND PER FACE NOW, and that is the one place this ADDS matches rather
than removing them. `compile_search_regex` builds with `(?i)` and no `(?m)`, so
`^`/`$` bound at the ends of the whole joined value; splitting binds them at each
face's ends, which is what Scryfall does — it anchors inside the face it is
matching. `o:/^draw/` therefore starts matching a card whose back face opens with
"Draw", which is correct. It still does not bind per LINE the way Scryfall's Ruby
anchors do; that divergence is older than this change and untouched by it.
THREE COLUMNS JOIN FACES AND ONLY TWO SEPARATORS ARE OURS. `type_line` joins with
" // ", which is Scryfall's own top-level field for a split card ("Instant //
Instant") — `t:/\/\//` answers 930 there, so splitting it would be inventing a bug
rather than fixing one. It is also not a search column here at all (`t:` reads
`card_types`). That asymmetry is why the rule is a per-column decision,
`FACE_JOINED_TEXT_COLUMNS`, and it is asserted in both directions.
The separator itself moved from a private constant in api/card_processing.py to
api/parsing/db_info.py, where the SQL generator can reach it, and the engine's
copy is exported to Python so a test can pin the two together. Nothing else would
have noticed them drifting: the importer would keep joining with one string and
the matcher would keep splitting on the other, silently answering from the seam
again, with every other test still green.
Tests: three engine tests over a three-card fixture (the two-face card whose back
opens on "draw", the single-face control whose OWN newline precedes it, and the
two-face card with a real internal "\ndraw" that splitting must not throw out),
covering the eval walk, the `ft:` bind rewrites, and the memoized `o:` verify;
one end-to-end class in api/tests/test_engine_unit.py that goes through
`parse_scryfall_query`, because `o:/\/\//` never reaches the engine as a regex and
a hand-built filter would miss the path users take; the type-line negative control
and the constant-parity check. Suites: cargo test 169 passed / 56 ignored, clippy
1.97.1 --all-targets -D warnings clean, pytest 3,447 passed (testcontainers
excluded).
…ryfall's `mana:` Haystack Is Not Its `mana_cost` Field
`_merge_processed_faces` keeps the FRONT face's `mana_cost_text`. Each face row
is the parent card overlaid with the face and line 332 sets the column from
that dict, so the merged row carries face 0's cost and nothing else — a defect
this branch creates, exactly like the `\n//\n` separator the previous commit
fixed: on `main`, where each face is its own row, every face's cost is stored.
THE RULE IS NOT SCRYFALL'S TOP-LEVEL `mana_cost`. Reading its card objects
gives a layout table — the field is present on the one-image layouts
(split 350, adventure 459, prepare 95, flip 45) and absent on the two-image
ones (art_series 2,650, transform 1,065, modal_dfc 328, double_faced_token 120,
reversible_card 81), over all 5,193 faced printings in the 2026-08-28
default_cards bulk with 0 exceptions — and that table is wrong about SEARCH.
Scryfall's index carries the join for BOTH groups. Probed on api.scryfall.com
2026-08-28, each as the card's own `!"…"` ANDed with the pattern so the corpus
filters cannot confound the count:
| card | layout | probe | Scryfall |
|---|---|---|---|
| `Extus, Oriq Overlord // Awaken the Blood Avatar` | modal_dfc, NO top-level cost | `mana:/\/\//` | **1** |
| | | `mana:/{b}{b} \/\/ /` | **1** — a pattern SPANS the seam, so it is one string |
| | | `mana:/{r}$/` | **1** — ending in the BACK face's last pip |
| `Fire // Ice` | split | `mana:/^{u}$/` | **0** — the back half alone is not a value of its own |
| `Delver of Secrets // Insectile Aberration` | transform | `mana:/^{u}$/` | **1** — an EMPTY back face contributes nothing... |
| | | `mana:/^{u} /` | **0** — ...not even a separator |
| `Westvale Abbey // Ormendahl, Profane Prince` | transform, both costless | `mana:/^$/` | **1** — so an all-costless card is EMPTY... |
| | | `mana:/\/\//` | **0** — ...never `" // "` |
Corpus-wide the same day: `mana:/\/\// is:mdfc` is 40 of 100, and
`mana:/^$/ is:artseries` is 2,243 of 2,243.
So `mana_cost_text` joins the faces' NON-EMPTY costs with `" // "` — Scryfall's
own field where it sends one (949 of 949 reproduced byte for byte) and the same
derivation where it does not. It goes into `_FACE_JOINED_TEXTS` beside
`type_line`, whose separator is Scryfall's too, and NOT into
`FACE_JOINED_TEXT_COLUMNS`: matching must leave both whole, and the asserted
`_JOINED_WITH_SLASHES` tuple is now the line between the two groups rather than
a `key == "type_line"` test.
`mana_cost_jsonb` and `devotion` do not move. Both are computed per face from
that face's own cost and the merge keeps the front's, paired with the card's
own `cmc`; only the printed STRING joins.
**`is:hybrid` had to be pinned to the front face, and it is two cards wide.**
`BOOLEAN_IS_TAGS["hybrid"]` read `cards.mana_cost_text`, and Scryfall's
`is:hybrid` asks only about the front. Replaying both readings over the
2026-08-28 default_cards bulk and diffing against all 603 of
api.scryfall.com's `is:hybrid` (fetched card for card):
| reading | answers | misses of Scryfall's 603 |
|---|---|---|
| front face's cost | 617 | **0** |
| the joined cost | 619 | **0**, but two extras |
| layout-gated (front on two-sided, whole cost otherwise) | 617 | **0** |
The two extras are `Abigale, Poet Laureate // Heroic Stanza` and
`Lluwen, Exchange Student // Pest Friend` — `prepare` printings whose only
hybrid pip is on the back, and Scryfall calls neither hybrid. The layout-gated
reading is the rule stated exactly and answers the same 617, because no split,
adventure or flip card in the corpus carries a hybrid pip only on its back — so
the expression coalesces to `card_faces->0->>'mana_cost'` instead, same set for
less SQL. The 14 cards both readings add over Scryfall's 603 are from sets
outside its default filters.
`phyrexian` is left reading the column: the same replay gives 76 either way and
misses 0 of Scryfall's 73, because its `/P}` pips never sit on a back face
alone and its `oracle_text` half already spans every face.
Tests: three in `TestFaceMerging` — the split join with the not-split assertion
in both directions, the costless-face drop in both its shapes (one empty face,
then both), and `mana_cost_jsonb` staying the front's. All three fail on the
previous behaviour.
`pytest --ignore=api/tests/test_integration_testcontainers.py`: **3,450 passed**.
`cargo test` 169 passed / 56 ignored; `cargo clippy --all-targets -- -D warnings`
on the pinned 1.97.1 clean; `ruff check` clean.
… Bare "Instant" for the Only Five-Faced Card `_FACE_JOINED_TEXTS` joins every face's `type_line` with `" // "`, and that join is Scryfall's own answer nearly everywhere. Measured over the 2026-08-31 default_cards bulk: of the 5,112 faced printings that carry a top-level `type_line`, 5,110 carry exactly `" // ".join(face type lines)` — split (`Bind // Liberate` and `Fire // Ice` are both "Instant // Instant"), adventure (`Champions of Archery // Join the Group` is "Legendary Creature — Human Archer // Sorcery — Adventure"), flip, transform and MDFC alike, each confirmed live on api.scryfall.com the same day. THE TWO THAT DIFFER are the two printings of the only five-faced card in the corpus, `Who // What // When // Where // Why` — und/75 and unh/120. Scryfall's row is the bare "Instant" where this join says it five times. So the join is a derivation of the field, not the field: the CARD's own line now outranks it wherever the card has one. THE FALLBACK IS THE LIVE HALF, and this is where the shape earns its `if`. A reversible printing carries no top-level `type_line` at all — 81 of 81 in the same bulk — and three of them survive every filter `preprocess_card` applies: tdm/378, tdm/379 and tdm/381, the Tarkir omen dragons, whose doubled `X // X` siblings the name filter drops. Reading `card["type_line"]` unconditionally would null the type line on exactly those three; the join is their only one, so it stands. NOTHING MOVES IN TODAY'S ROWS, and that is stated rather than assumed: 2,049 faced printings reach the merge, 0 of them diverge, and both five-faced printings are `not_legal` in every format (and `funny`), so the legality filter drops them before this line. The rule is written where the join is made so that the next five-faced card, or the next reversible one that is not `X // X`, lands correctly instead of being found in production. `card_types`/`card_subtypes` are NOT recomputed from the new string, deliberately. They stay the per-face union `_FACE_LIST_UNIONS` builds, which is the only reading that survives a joined line: `parse_type_line` splits on the FIRST em dash, so parsing "Legendary Creature — Human Archer // Sorcery — Adventure" back would file "Sorcery" under SUBTYPES. The test helper's top-level `type_line` now defaults to the faces' join when `card_faces` is given, because that is what Scryfall sends on 5,110 of 5,112 and the old fixed "Creature — Test" default made every faced fixture model a card that does not exist. Three cases are pinned: the two-half card where join and card agree, the five-part card where they do not, and the reversible shape with no line of its own. Removing the re-assertion fails the second; making it unconditional fails the third.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the core of #400 and #873: multi-face cards become one searchable row carrying every face's data, the "single conceptual card" direction from the #873 discussion. Based directly on
main; no overlap with the other open PRs (#872/#877/#879/#888/#889/#890/#893).The bug being fixed
preprocess_card()fans each multi-face card out to one row per face — all sharing onescryfall_id— and the upsert'sON CONFLICT (scryfall_id)keeps whichever face came last: the back. Hence the #400 audit's findings:t:battlematched zero rows corpus-wide,t:sorcerymissed every MDFC spell side, front-face oracle text was invisible (#873), and ~1,343 printings carried the wrong types/colors/stats. Split cards were hit too (Fire // Ice was findable by Ice's text but not Fire's).Why one merged row, not one row per face
Measured against api.scryfall.com (2026-08-08): Scryfall ANDs search predicates at the card level, each satisfiable by any face.
t:sorcery t:landreturns the MDFC lands — no single face is botho:"flip a coin" o:"instant and sorcery"matches Ral, Monsoon Mage — the texts live on different facesc:bmatches Westvale Abbey, whose only black face is the backpow=3matches Brutal Cathar via its back faceOne row per printing with any-face unions reproduces those semantics directly. One row per face — the original #400 plan — would instead break every cross-face conjunction (no face-row satisfies both terms) on top of colliding on the primary key. The merged row isn't a compromise; it's the correct shape.
What the merge does (
_merge_processed_faces)cmc,mana_cost,illustration_id, image, prices, raw blobcard_types,card_subtypes,card_colors,card_keywords,produced_mana,illustration_idsoracle_text,flavor_text\n//\n— both texts substring-searchable. The newline keeps.from crossing a face, and that turned out to be all it keeps out; see the regex update belowtype_line"Battle — Siege // Creature — Moonfolk Ninja", exactly Scryfall's rendering_textcolumns never describe different facesEach face still runs through the full existing pipeline first; the merge is ~40 lines over the processed rows. No schema migration, no engine changes, no SQL-gen changes — the engine inherits everything on reload.
Plus one vocabulary fix this exposed:
"Battle"was missing from the parser'sCARD_TYPES, sot:battlerouted to the subtype arm on both paths — a guaranteed zero-match the moment battles became visible. The engine was innocent (itsTYPE_BATTLEbit worked in isolation once the parser sentcard_types).Frontend: back-face flip button
Scryfall-style flip, implemented as progressive enhancement so the no-JS server render (parity fixture) is byte-identical: cards named
A // Bare probed for a face-2 CDN image, and only cards that have one — transform/MDFC, never split/adventure — grow the button (results grid, modal, and card page).copy_images_to_s3.pynow uploadscard_faces[1]'s image under the face-2 key (the raw blob keeps the front'simage_urisat top level for every existing consumer, with the rawcard_facesre-attached), and the upload path stops hardcoding face 1 in the S3 key. Ops note: backs appear on the site after a reimport + image sync run.Validation (rebuilt dev stack, full reimport, 97,802 cards, 2026-08-08)
t:battlet:sorcery t:landdraft_innovationcards the corpus excludes by policy — 34/34 of the achievable setis:permanenton battles!"Nissa, Vastwood Seer" t:creature!"Westvale Abbey" c:b!"Brutal Cathar" t:humanKnown residual (tested + documented): when several faces carry a stat group (Brutal Cathar's 2/2 // 3/3), only the front's values are searchable; Scryfall also matches the back's
pow=3. Per-face numerics (and back-facemana:) are a measured follow-up if wanted.Tests
test_card_processing.py: rewritten DFC tests + aTestFaceMergingclass — battle/MDFC unions, front-priority and back-fallback stat groups,*-power group consistency, text joining, all-faces-filtered, one-row-per-printing (the PK-fight regression).test_pyparsing_parser.py:t:battleroutes tocard_types,t:siegestays a subtype.test_copy_images_to_s3.py: face-2 emission with its own PNG URL; single-faced cards stay front-only.app.test.js: 6 new flip tests (face URLs, probe gating + caching, flip toggling, single-injection). The no-JS parity suite passes untouched.Update:
raw_card_blobis now the card Scryfall sent (956f8a6)The merged row's blob was the front face's dict with
card_facesre-attached, so existing top-level reads keep meaning "the front". Going through those reads, exactly one needs that —image_uris— and thecopy_images_to_s3.pychange already in this PR gives it acard_faces->0fallback. Everything else read from the blob (lang,set_type,games,finishes,frame_effects,image_status,reserved,game_changer) is card-level and identical either way;backfill_prefer_scores.sqlnever touchesimage_uris.With that covered, the blob can hold the card verbatim, which buys something the promotion cannot. Every searchable field is merged onto the row's own columns, so the blob has no derivation left to do, and keeping it as Scryfall sent it is what makes the card answerable later:
Why a reader could not have undone the promotion instead. A card object cannot be rebuilt from a face:
card_facesis gone,nameandtype_lineare the front's, and which fields a real card carries at top level varies by layout — a split card hasmana_costandimage_uristhere, a transform card does not. No rule strips the promoted keys correctly for both.Measured against the object Scryfall sent, before and after:
object,name,type_lineall disagreecard_nameonlycard_nameonlycard_nameis the combined name the importer lifts before splitting faces — one key, stripped by any reader that wants the original.Also in this commit:
prefer_weights.py's seven image reads go through onefront_image()helper that coalesces tocard_faces->0, so a transform printing no longer silently drops out of theart_crop IS NOT NULLtuning queries. Both of its SQL templates were run against the 97,808-card dev corpus.Tests: five new in
TestMultiFaceRawBlob, covering both layouts, verified to fail on the previous behavior. Suites: api + parsing + scripts + client 2,514 passed, jest 1,747 passed, ruff clean.Why now: #912 needs the card object to serve Scryfall-identical
/cards/*payloads, and was carrying acards.scryfall_jsoncolumn to hold a second copy for multi-face rows. With this change that column is gone — the blob is already the answer. #912 is now stacked on this PR.Update: art tags follow every face the row shows (a9bfc56)
Folded in from #930, which is now closed. I opened it as a separate stacked PR first and that was the wrong call: this repairs damage this PR does, so merging the two separately would mean merging a regression and its fix as two units, neither of which has landed. One PR that was always correct is strictly better.
What the merge broke
card_art_tagsis attached by joining onmagic.cards.illustration_id:On
mainthat is right, and it is right because faces are separate rows: a double-faced card's back face is its own row carrying its ownillustration_id, so a tag on the back art lands on a row andarttag:finds the card.This PR gives the merged row the front's
illustration_id— correctly, matching Scryfall's own top-level field. But then the back's illustration exists nowhere on the row, and a tag that lives only on the back art becomes unreachable by any query. Measured against api.scryfall.com, 2026-08-16:arttag:snow e:khm-art:human e:khm t:creatureThe two missing from the first are
Birgi, God of Storytelling // Harnfel, Horn of BountyandEsika, God of the Tree // The Prismatic Bridge— their snow is on the back face's art. The surplus in the second isValki, God of Lies // Tibalt, Cosmic Impostor: Tibalt is the human, and Tibalt is the back.The rule, checked against the corpus rather than assumed
Replaying the 2026-08-16 art-tags dump over the bulk cards, the union predicts Scryfall's
arttag:snow e:khmanswer set-for-set — 75 names, no surplus and no shortfall — where the front-only reading predicts 73. On the same bulk, 9,368 printings show more than one illustration and 5,491 of them gain at least one tag from a non-front face.Mechanism
A row that merges its faces has to carry every illustration it shows, so
illustration_ids jsonbdoes — front first, deduped.illustration_idstays the front's for display.The merge-side change is one tuple entry, because each face row already computes its own single-element list and
_merge_processed_facesalready unions front-first with dedupe:A split or adventure card — one
illustration_idon the card, none on its faces — has both face rows inherit the same id and collapse back to one entry, which is the right answer: there is one piece of art to tag.jsonband notuuid[], for a mechanical reason as much as a stylistic one:api/db/bulk_upsert.pyresolves a column through_PG_TYPE, wherejsonbextracts withobj->and anARRAYcolumn falls through totextand fails the insert. Matchingcard_types/card_subtypeskeepsbulk_upsertuntouched.The sync is keyed on
scryfall_id, notillustration_id— the part I'd most want a second opinion on. An illustration id no longer determines a row's whole value, and it cannot be batched that way either:_sync_card_tagsbatches at 5,000, and a batch of illustration ids can split one card's illustrations across two statements where a batch of cards cannot. So_union_art_tagsresolves the union in Python and_sync_card_tagsis unchanged — still shared byte-for-byte with the oracle path, still diffing, batching andIS DISTINCT FROM-guarded. The costs, stated plainly:scryfall_idjoin on the unique index instead ofidx_cards_illustration_id, and a clear step that is exact per row.Doing the union in SQL over the array instead needs either a whole-table
UPDATEwith a per-row aggregate — the shape #910 was about — or a temp table plus an unbatched statement. Neither seemed worth giving up the batching for.The migration backfills from
raw_card_blob->'card_faces', which on an already-imported row is the only record of the back's illustration. Without it the column is empty until the next card import and_sync_card_tagsclears every card's art tags in the meantime.prefer_scorereads the union too, deliberatelyart_styleinbackfill_prefer_scores.sqlreadscard_art_tags, so this moves representative selection. Measured rather than guessed: exactly three printings corpus-wide flipart_styleunder the union.Tribute to Horobi // Echo of Death's Wailneo/356, English and German — back art isanimeThaumatic Compass // Spires of Orazcapxtc/249 — back art isline-artI fed the union to
prefer_scorerather than keeping a front-only reading for it, and that is a choice, not a consequence. For it:art_styleasks what art a printing shows, and a printing shows all of it — a card whose back is anime is partly anime. Against: two readings would protect three rows from a demotion #720's evidence didn't rule on. Three rows is small enough that I'd take either answer; say the word and it's a one-line split.artwork_printings— still right given the merge, and one thing it quietly fixesThe other
card_art_tags-adjacent consumer in that file isartwork_printings, which groups on(illustration_id, card_name)and feedsillustration_count. It is untouched here, and I checked that it is still correct rather than merely unchanged:illustration_idand so its own group. Post-merge the group keyed on the front illustration has exactly one member per printing — the same membership the front-face row had before — so no surviving row'sillustration_countmoves.illustration_idand the samecard_name, so they landed in the same group and each printing was counted twice. Post-merge each printing counts once. That is ~836 English printings (Scryfall: 345 split, 449 adventure, 42 flip), and the correction is uniform across every printing of such a card, so it cannot flip a representative —prefer_scoreranks printings within a card, and a monotonic shift applied to all of them preserves the order.So grouping on the front id remains the right key: it measures how many printings show this artwork, and the row shows the front. The residual, unchanged by this PR and a different question, is that two printings sharing a front illustration but differing on the back count as one artwork here while Scryfall's
unique=arttreats them as two.Tests
Against real Postgres, not mocks (
test_tagging_integration.py,test_upsert_cards.py):illustration_idturns it red, which I ran as a negative control rather than assumingpreprocess_cardwould, on rows written by the real import pathAnd as units: front-first ordering, a split card's faces collapsing to one illustration, a face with no art contributing nothing, an untagged illustration omitted rather than mapped to
{}, and the no-copy property of the single-illustration path.python -m pytest --ignore=api/tests/test_integration_testcontainers.py: 2,551 passed, 19 xfailed. That file errors 22 times for me identically with and without this commit — its class-scoped container binds a fixed host port and collides with the session container locally — so I've left it to CI.ruff checkandruff format --checkclean.Update: a joined column is matched per face now — the separator was answering queries (9298416)
Joining the face texts is what makes every face searchable, and it is also the one thing in this PR that creates a query defect that cannot exist on
main._FACE_TEXT_SEPARATOR("\n//\n") is a string this branch invented. Scryfall stores nothing like it and never joins — it matches each face separately — so every character of that separator is a position in our haystack and in nobody else's, and patterns were answering from it. Onmain, where each face is its own row, there is nothing to match at.The table cell above claimed the newline kept regexes from spanning faces. It keeps
.from spanning faces. It keeps nothing else out, and a pattern that spells the separator's own characters was matching it happily. Measured against api.scryfall.com, 2026-08-28 (its answer, against what a joined column answers):o:/\ndraw/"\n//\nDraw…"contains"\ndraw"o:/\sdraw/\sreaching that newlineo:/\nwhenever/o:/\/\//o:"//"lower_literal_regexeslowers the literalft:/\/\//t:/\/\//The fix is to split the stored value back at match time and run the pattern, or the needle, per segment. No store change, no reimport, no schema change, no
ARCHIVE_FORMAT_VERSION-style bump: the bytes were always right and only the reading of them was wrong. A single-face card has no separator, so the split yields one segment and costs one scan for a needle that is not there.The substring path is not the exotic half, it is the half users reach
o:/\/\//is a plain literal, solower_literal_regexesrewrites it into the contains-predicateo:"//"before the engine sees a pattern at all. A fix applied only to the regex arm would have left the largest row of that table exactly where it was.Five sites in
card_engine/src/filter.rs, because a text predicate on a joined column reaches an answer five different ways and all five had to agree:TextRegexandTextContainsFlavorMatchrewrites —ft:never reaches evaluation at all, so a fix applied only there would have left the 262 where it wasOracleMatchsetThe #734 trigram narrow above that verify is untouched and stays sound either way — a face is a substring of the join, so every literal factor the narrow requires of a survivor is still present in the joined text it indexed — but the verify is the answer, and a needle straddling the separator must not survive it.
The SQL fallback needed the same semantics
Otherwise
_search_sqland_search_engineanswer one query two ways, which is its own bug.oracle_text/flavor_textleaves now compile to a per-faceEXISTSoverstring_to_array(col, separator)::form keeps the whole-valueLIKEas its leading conjunct, sogin_trgm_opsstill drives the scan and only survivors pay for the split. Sound because a face is a substring of the join, so that conjunct can only ever be a superset.~*is not newline-sensitive, so after the split^/$bind at each face's ends and a face can match where the join does not; ANDing the join's answer in would drop real matches.CASE WHEN col IS NULL THEN NULL ELSE EXISTS (…) ENDkeeps the leaf three-valued.EXISTSis two-valued, andflavor_textis nullable: without the guard,-flavor:/x/would start matching every printing that has no flavor text at all.Verified row-for-row against a real PostgreSQL on the fixture below: engine and SQL return identical sets for all 18 queries tried, negations and the NULL column included.
Anchors bind per face now, and that is the one place this adds matches
compile_search_regexbuilds with(?i)and no(?m), so^/$bound at the ends of the whole joined value. Splitting binds them at each face's ends, which is what Scryfall does — it anchors inside the face it is matching.o:/^draw/therefore starts matching a card whose back face opens with "Draw", which is correct. It still does not bind per line the way Scryfall's Ruby anchors do; that divergence is older than this PR and untouched by it.Three columns join faces and only two separators are ours
type_linejoins with" // ", which is Scryfall's own top-level field for a split card ("Instant // Instant") —t:/\/\//answers 930 there, so splitting it would be inventing a bug rather than fixing one. It is also not a search column here at all (t:readscard_types). That asymmetry is why the rule is a per-column decision,FACE_JOINED_TEXT_COLUMNS, asserted in both directions.The separator itself moved from a private constant in
api/card_processing.pytoapi/parsing/db_info.py, where the SQL generator can reach it, and the engine's copy is exported to Python so a test can pin the two together. Nothing else would have noticed them drifting: the importer would keep joining with one string and the matcher would keep splitting on the other, silently answering from the seam again, with every other test still green.Tests
A three-card fixture separates the bug from everything that must not move: the two-face card whose back opens on "draw", the single-face control whose own newline precedes it, and the two-face card with a real internal
"\ndraw"that splitting must not throw out.ft:bind rewrites (asserting the rewrite actually fired, so the test cannot silently measure the wrong path), and the memoizedo:verifyapi/tests/test_engine_unit.pythat goes throughparse_scryfall_query, becauseo:/\/\//never reaches the engine as a regex and a hand-built filter would miss the path users taketype_linenegative control, in both directions, and the Rust/Python constant-parity checktest_sql_gen.py/test_regex_patterns.pyupdated to the new shape, with a namedface_split_like()helper so the intent reads rather than the stringAll three engine tests were run against the unsplit code as a negative control and fail there.
Suites:
cargo test169 passed / 56 ignored;cargo clippy --all-targets -- -D warningson the pinned 1.97.1 clean;pytest3,447 passed (testcontainers excluded);ruff checkclean.Also in this push:
upstream/mainmerged in (through #1011), so the branch is current.Update: the merged row's printed cost is every face's, and the card object is not the evidence (9a83786)
The table at the top of this PR says the merge takes
mana_costfrom the front face because that "matches Scryfall's own top-level fields (verified on its card objects)". The card objects are right and the conclusion is wrong: Scryfall'smana:haystack is not its top-levelmana_costfield.Reading the objects gives a clean layout table, and it holds over all 5,193 faced printings in the 2026-08-28 default_cards bulk with 0 exceptions — the field is PRESENT on the one-image layouts (split 350, adventure 459, prepare 95, flip 45) and ABSENT on the two-image ones (art_series 2,650, transform 1,065, modal_dfc 328, double_faced_token 120, reversible_card 81). Search does not follow it. Probed against api.scryfall.com, 2026-08-28, each as the card's own
!"…"ANDed with the pattern so the corpus filters cannot confound the count:Extus, Oriq Overlord // Awaken the Blood Avatarmana_costat allmana:/\/\//mana:/{b}{b} \/\/ /mana:/{r}$/Fire // Icemana:/^{u}$/Delver of Secrets // Insectile Aberrationmana:/^{u}$/mana:/^{u} /Westvale Abbey // Ormendahl, Profane Princemana:/^$/mana:/\/\//" // "Corpus-wide the same day:
mana:/\/\// is:mdfcis 40 of 100, andmana:/^$/ is:artseriesis 2,243 of 2,243.The change
mana_cost_textjoins the faces' non-empty costs with" // ". That reproduces Scryfall's own field byte for byte wherever it sends one (949 of 949) and supplies the same string on the layouts where it sends none.It joins by going into
_FACE_JOINED_TEXTSbesidetype_line— and, liketype_line, not intoFACE_JOINED_TEXT_COLUMNS, because the separator is Scryfall's rather than this branch's and matching must leave it whole. Thekey == "type_line"test in the merge becomes a_JOINED_WITH_SLASHEStuple, so the two-group split is a named constant asserted in both directions rather than a string comparison.mana_cost_jsonbanddevotiondo not move: both are computed per face from that face's own cost and the merge keeps the front's, paired with the card's owncmc. Only the printed STRING joins.is:hybridhad to be pinned to the front face, and it is exactly two cards wideBOOLEAN_IS_TAGS["hybrid"]readscards.mana_cost_text, so this change would have moved it. Scryfall'sis:hybridasks only about the front face. Replaying all three candidate readings over the 2026-08-28 default_cards bulk and diffing against all 603 of api.scryfall.com'sis:hybrid, fetched card for card:The two extras are
Abigale, Poet Laureate // Heroic StanzaandLluwen, Exchange Student // Pest Friend—prepareprintings whose only hybrid pip is on the back, and Scryfall calls neither hybrid. The layout-gated reading is the rule stated exactly and answers the same 617, because no split, adventure or flip card in the corpus carries a hybrid pip only on its back; so the expression coalesces toraw_card_blob->'card_faces'->0->>'mana_cost'instead — the same set for less SQL, and available only because this PR already made the blob verbatim. (The 14 cards both readings add over 603 are from sets outside Scryfall's default filters.)phyrexianis deliberately left reading the column: the same replay gives 76 either way, missing 0 of Scryfall's 73, because its/P}pips never sit on a back face alone and itsoracle_texthalf already spans every face.Tests
Three in
TestFaceMerging, all three failing on the previous behaviour:mana_cost_text in _FACE_JOINED_TEXTSandnot in FACE_JOINED_TEXT_COLUMNSasserted in both directions — the same pair thetype_linetest already pins"{U}", not"{U} // ") and both empty ("", not" // ")mana_cost_jsonbstaying the front's while the string joinsSuites:
pytest --ignore=api/tests/test_integration_testcontainers.py3,450 passed;cargo test169 passed / 56 ignored;cargo clippy --all-targets -- -D warningson the pinned 1.97.1 clean;ruff checkclean.