Skip to content

Fix duplicate fallback cards during catalogue sync - #311

Merged
Git-Romer merged 3 commits into
Git-Romer:mainfrom
engelben:fix/catalogue-fallback-duplicates
Jul 27, 2026
Merged

Fix duplicate fallback cards during catalogue sync#311
Git-Romer merged 3 commits into
Git-Romer:mainfrom
engelben:fix/catalogue-fallback-duplicates

Conversation

@engelben

@engelben engelben commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Fix duplicate fallback cards during catalogue sync

Problem

Database sessions use autoflush=False.

When a localized set contains fewer native cards than its declared total, _sync_set_card_catalogue() queues the native cards and immediately calculates missing-language fallbacks. Without a flush, that query cannot see the queued native rows and may generate fallback rows with the same primary keys.

The German smp set exposes this consistently. TCGdex reports an official total of 248 cards, while the German card list contains 240 native cards. The remaining 8 cards need English fallback rows.

Fix

  • Flush native card rows before calculating missing-language fallbacks.
  • Count unique native card IDs when deciding whether fallbacks are needed.
  • Skip duplicate native rows returned by the upstream API.
  • Skip fallback rows that overlap native cards.
  • Deduplicate fallback rows in both the normal and recovery paths.
  • Roll back failed transactions before reading ORM-backed values for logging.
  • Run the catalogue tests with autoflush=False.
  • Add regression coverage for native and fallback overlap, duplicate native rows, duplicate fallback rows, and rollback recovery.
  • Bring the branch up to date with current main.
  • Update the version to 1.27.2.

Expected result

The German smp sync keeps all 240 native cards, creates only the 8 missing English fallback cards, and completes without duplicate-key or pending-rollback errors. Repeating the sync keeps the same 248 rows.

Validation

  • Fresh independent reviewer pass completed, with all findings addressed.
  • 284 backend tests passed, with 1 existing skip.
  • 79 frontend tests passed.
  • Backend and frontend production Docker builds passed.
  • PostgreSQL 18 validation passed using the live TCGdex smp data.
  • The real import produced 240 native German cards and 8 English fallback cards on two consecutive syncs.
  • Python compilation, version consistency, and git diff --check passed.

@hiddenbanana

Copy link
Copy Markdown
Contributor

Nice catch, and a more thorough fix than what we'd done. We hit this exact crash on our fork a while back (smp-style set where native card count < declared total, same cards_pkey collision) and patched around it locally on 2026-07-18, but never got around to opening an issue or upstreaming it — sorry for the overlap.

Our fix was narrower: just add a db.flush() after queuing native cards and before calling build_missing_language_cards_for_set(), so the fallback's existence check actually sees rows added earlier in the same autoflush=False session instead of re-cloning them. That was enough to stop the crash for us, but it only fixes the visibility timing — it doesn't defend against duplicate IDs slipping through some other way.

This PR is more elegant because it doesn't just close the timing gap, it also fails safe if the assumption breaks: tracking native_card_ids and skipping any fallback (or duplicate native) row that collides with one already queued means a future edge case can't reintroduce the same cards_pkey violation even if flush ordering changes again. The rollback-before-logging reordering is also a nice catch we didn't have — cheap insurance against a PendingRollbackError masking the real error in the log.

@Git-Romer

Copy link
Copy Markdown
Owner

@engelben, this was a good catch. The German smp example made it easy to reproduce how native cards and fallback cards could collide when autoflush was disabled.

Your flush and rollback changes fixed the original failure. While testing some related edge cases, I added a few more safeguards:

• Fallback decisions now count unique native cards instead of the raw API list.
• Duplicate native and fallback rows are skipped safely.
• The recovery path also handles duplicate fallback data.
• Additional tests cover the overlap, duplicate upstream rows, and rollback recovery.

I also brought the branch up to date with main and updated the version to 1.27.2.

The final result was tested with PostgreSQL 18 and the real TCGdex data. Both consecutive syncs produced the correct 240 native German cards and 8 English fallbacks. All 284 backend tests, 79 frontend tests, and both production builds passed.

The PR is ready now. The concrete smp example was especially helpful because it gave us a real dataset to verify the complete fix against. Thank you!

@Git-Romer
Git-Romer merged commit 4862908 into Git-Romer:main Jul 27, 2026
2 checks passed
@engelben
engelben deleted the fix/catalogue-fallback-duplicates branch July 28, 2026 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants