fix(deck): don't wipe deck on unparseable bulk-replace input#95
Conversation
Replace-mode intake diffed an empty parse against the whole deck, so a parse failure (e.g. malformed JSON) emitted a remove op per card and committed a mass delete under a "Saved — removed N" success message. Short-circuit replace mode with a warning-only no-op when the parse yields zero cards but non-empty warnings — a parse failure, distinct from a legitimately empty paste (zero cards, no warnings) that may intentionally clear the deck. Closes #87
|
Warning Review limit reached
Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Intent
Fix issue #87: bulk-edit replace mode wiped the entire deck when the pasted decklist failed to parse (e.g. malformed JSON). The json adapter catches a parse failure and returns {cards:[], warnings:[...]} instead of throwing; in replace mode intakeDecklist diffed that empty desired list against every existing DeckCard, so diffDeck emitted a remove op per card and applyChanges committed a mass delete under a misleading 'Saved — removed N' success message. Fix (lib/deck/io/intake.ts): short-circuit replace mode with a warning-only no-op result when the parse yielded zero cards AND non-empty warnings (a parse failure), distinct from a legitimately empty/whitespace paste (zero cards, no warnings) which may intentionally clear the deck and is preserved. Guard runs before any DB read or mutation. Added a regression test in intake.test.ts asserting applyChanges and deckCard.findMany are never called and removed===0 with the parse warning surfaced. Pre-existing bug on main, orthogonal to the #30 multi-category branch; referenced #87 in the commit.
What Changed
lib/deck/io/intake.tsso a parse failure no longer clears the deck: when intake yields zero cards and non-empty warnings (e.g. malformed JSON),intakeDecklistnow short-circuits to a warning-only no-op before any DB read or mutation, instead of diffing an empty desired list into a remove op perDeckCardand committing a mass delete under a misleading "Saved — removed N" message.intake.test.tscovering issue Bulk-edit replace mode wipes deck on unparseable input #87: on an unparseable replace-mode paste,applyChangesanddeckCard.findManyare never called,removed === 0, and the parse warning is surfaced.Risk Assessment
✅ Low: Nine-line additive guard with a matching regression test that precisely implements the required parse-failure short-circuit before any DB access, distinguishing parse failure from intentional empty paste, with no behavior regressions.
Testing
Installed deps and generated the Prisma client (postinstall had failed on a missing DATABASE_URL; supplied a dummy URL for codegen only, no DB connection). Ran the intake unit suite (16 pass, including the #87 regression asserting applyChanges and deckCard.findMany are never called and removed===0 with the parse warning surfaced). Built a differential evidence harness that drives the real intakeDecklist end-to-end (real parse + diffDeck + tally, DB mocked with a 5-card deck) pasting malformed JSON in replace mode: on the base commit it commits 5 remove ops and reports "Saved — removed 5" (deck wiped), while the fixed code short-circuits before any DB read/mutation and reports removed=0 with the warning "Decklist looks like JSON but could not be parsed". Captured a rendered before/after screenshot of the bulk-edit dialog Alert the user actually sees. Removed the temporary evidence test from the repo; worktree is clean and node_modules/lib/generated are gitignored. Overall: the fix satisfies the intent — parse-failure no longer wipes the deck, while a legitimately empty paste is preserved.
/var/folders/b2/rft4yv4177d6mpkbhbwj7svc0000gn/T/no-mistakes-evidence/01KXMHR2Z1EBJEZT3Y8PA2AGQ3/dialog-alert.png)Evidence: Rendered dialog HTML
Evidence: Fixed-code behavior transcript
DB read (deckCard.findMany) ... false DB mutate (applyChanges) ...... false Remove ops committed .......... 0 Dialog Alert title ............ Saved — added 0, removed 0, updated 0 Warnings surfaced ............. ["Decklist looks like JSON but could not be parsed"]Evidence: Base-code (buggy) behavior transcript
DB read (deckCard.findMany) ... true DB mutate (applyChanges) ...... true Remove ops committed .......... 5 Dialog Alert title ............ Saved — added 0, removed 5, updated 0 Warnings surfaced ............. ["Decklist looks like JSON but could not be parsed"]Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
pnpm exec vitest run lib/deck/io/__tests__/intake.test.ts— 16 pass incl. newaborts without wiping the deck when the paste fails to parse (issue #87)Differential harness (real intakeDecklist + real parse/diffDeck/tally, mocked 5-card deck,{ not valid jsonin replace mode) run on basebc3f1f9intake.ts → removed=5, applyChanges called (deck wiped); restored fixed3f17a0eintake.ts → removed=0, deckCard.findMany & applyChanges never called, warning surfacedRendered before/after dialog Alert to HTML and screenshotted via chrome-devtools-axi to show the user-visible message✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.