Skip to content

fix(deck): don't wipe deck on unparseable bulk-replace input#95

Merged
jcserv merged 1 commit into
mainfrom
fm/bulk-replace-guard-k9
Jul 16, 2026
Merged

fix(deck): don't wipe deck on unparseable bulk-replace input#95
jcserv merged 1 commit into
mainfrom
fm/bulk-replace-guard-k9

Conversation

@jcserv

@jcserv jcserv commented Jul 16, 2026

Copy link
Copy Markdown
Owner

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

  • Guarded bulk-edit replace mode in lib/deck/io/intake.ts so a parse failure no longer clears the deck: when intake yields zero cards and non-empty warnings (e.g. malformed JSON), intakeDecklist now 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 per DeckCard and committing a mass delete under a misleading "Saved — removed N" message.
  • Preserved the distinct case of a legitimately empty/whitespace paste (zero cards, no warnings), which may still intentionally clear the deck.
  • Added a regression test in intake.test.ts covering issue Bulk-edit replace mode wipes deck on unparseable input #87: on an unparseable replace-mode paste, applyChanges and deckCard.findMany are 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.

  • Evidence: Dialog Alert before/after (visual) (local file: /var/folders/b2/rft4yv4177d6mpkbhbwj7svc0000gn/T/no-mistakes-evidence/01KXMHR2Z1EBJEZT3Y8PA2AGQ3/dialog-alert.png)
Evidence: Rendered dialog HTML
<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8" />
<title>Issue #87 — bulk-replace with malformed JSON</title>
<script src="https://cdn.tailwindcss.com"></script>
<style>
  body { background:#0b0b0f; color:#e7e7ea; font-family: ui-sans-serif, system-ui, sans-serif; }
  .card { background:#151519; border:1px solid #2a2a32; }
  .muted { color:#a1a1aa; }
  .destructive { color:#f87171; }
  .ok { color:#4ade80; }
  .mono { font-family: ui-monospace, SFMono-Regular, Menlo, monospace; }
</style>
</head>
<body class="p-8">
  <div class="max-w-5xl mx-auto">
    <h1 class="text-xl font-semibold mb-1">Bulk edit → Replace mode: pasting malformed JSON into a 5-card deck</h1>
    <p class="muted text-sm mb-6">The dialog Alert the user sees after clicking <span class="mono">Save</span>. Same result payload as the differential CLI evidence.</p>

    <div class="grid grid-cols-2 gap-6">
      <!-- BEFORE -->
      <div>
        <div class="text-sm mb-2"><span class="destructive font-semibold">✗ BEFORE (base bc3f1f9 — bug #87)</span></div>
        <div class="card rounded-xl p-4">
          <div class="text-xs muted mb-2">Textarea (unparseable JSON):</div>
          <div class="mono text-xs bg-[#0b0b0f] border border-[#2a2a32] rounded p-2 mb-3">{ not valid json</div>
          <div class="text-xs muted mb-3">0 cards parsed</div>
          <div class="card rounded-lg border px-3 py-2 flex gap-2 items-start">
            <span class="destructive mt-0.5">⚠</span>
            <div>
              <div class="font-medium destructive">Saved — added 0, removed 5, updated 0</div>
              <ul class="list-disc pl-4 mt-1 text-xs muted">
                <li>Decklist looks like JSON but could not be parsed</li>
              </ul>
            </div>
          </div>
          <div class="text-xs destructive mt-3">→ All 5 DeckCards deleted under a "success" message. Deck wiped.</div>
        </div>
      </div>

      <!-- AFTER -->
      <div>
        <div class="text-sm mb-2"><span class="ok font-semibold">✓ AFTER (fix 3f17a0e)</span></div>
        <div class="card rounded-xl p-4">
          <div class="text-xs muted mb-2">Textarea (unparseable JSON):</div>
          <div class="mono text-xs bg-[#0b0b0f] border border-[#2a2a32] rounded p-2 mb-3">{ not valid json</div>
          <div class="text-xs muted mb-3">0 cards parsed</div>
          <div class="card rounded-lg border px-3 py-2 flex gap-2 items-start">
            <span class="destructive mt-0.5">⚠</span>
            <div>
              <div class="font-medium">Saved — added 0, removed 0, updated 0</div>
              <ul class="list-disc pl-4 mt-1 text-xs muted">
                <li>Decklist looks like JSON but could not be parsed</li>
              </ul>
            </div>
          </div>
          <div class="text-xs ok mt-3">→ Guard short-circuits before any DB read/mutation. Deck intact; warning surfaced.</div>
        </div>
      </div>
    </div>

    <div class="mt-8 text-xs muted">
      Empty/whitespace paste (0 cards, <span class="mono">no</span> warnings) is unaffected — it may still intentionally clear the deck.
    </div>
  </div>
</body>
</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"]

================ ISSUE #87 EVIDENCE ================
Existing deck size ............ 5 cards
Pasted (replace mode) ......... '{ not valid json'
---------------------------------------------------
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"]
===================================================


 Test Files  1 passed (1)
      Tests  1 passed (1)
   Start at  21:12:37
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"]

================ ISSUE #87 EVIDENCE ================
Existing deck size ............ 5 cards
Pasted (replace mode) ......... '{ not valid json'
---------------------------------------------------
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"]
===================================================


 Test Files  1 passed (1)
      Tests  1 passed (1)
   Start at  21:12:16

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. new aborts 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 json in replace mode) run on base bc3f1f9 intake.ts → removed=5, applyChanges called (deck wiped); restored fixed 3f17a0e intake.ts → removed=0, deckCard.findMany & applyChanges never called, warning surfaced
  • Rendered 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.

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

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@jcserv, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 41 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9096b5b9-8d4e-4c01-a606-6b688d0920e8

📥 Commits

Reviewing files that changed from the base of the PR and between bc3f1f9 and 3f17a0e.

📒 Files selected for processing (2)
  • lib/deck/io/__tests__/intake.test.ts
  • lib/deck/io/intake.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fm/bulk-replace-guard-k9

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@jcserv
jcserv merged commit 3f09357 into main Jul 16, 2026
5 checks passed
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.

1 participant