fix: don't lock completely during catalog import

Author: cfbender

.backlog/tasks/task-21 - Fix-maybeboard-card-add-500.md

@@ -0,0 +1,44 @@
+---
+id: TASK-21
+title: Allow writes during catalog imports
+status: Done
+assignee:
+  - '@cfb'
+created_date: '2026-07-28 18:00'
+updated_date: '2026-07-28 18:14'
+labels: []
+dependencies: []
+type: bug
+ordinal: 21000
+---
+
+## Description
+
+<!-- SECTION:DESCRIPTION:BEGIN -->
+A production deck-card write returned HTTP 500 with SQLite database busy while the Scryfall catalog import held the database-wide write lock. Keep catalog batches atomic while releasing the lock between batches so interactive writes can proceed.
+<!-- SECTION:DESCRIPTION:END -->
+
+## Acceptance Criteria
+<!-- AC:BEGIN -->
+- [x] #1 Interactive deck-card writes can complete while a multi-batch catalog import is running
+- [x] #2 Each catalog batch stores its cards, printings, and search rows atomically
+- [x] #3 A completed catalog import reports the same counts and searchable card data as before
+<!-- AC:END -->
+
+## Implementation Plan
+
+<!-- SECTION:PLAN:BEGIN -->
+1. Replace the catalog-wide SQLite transaction with one immediate transaction per 200-card source batch, keeping each batch's card, printing, and search-row writes atomic while releasing the write lock between batches. 2. Preserve aggregate counts, logging, cache invalidation, and error propagation across the batch loop. 3. Add focused multi-batch regression coverage and exercise a concurrent deck-card write against a real SQLite repository with a catalog import in progress.
+<!-- SECTION:PLAN:END -->
+
+## Implementation Notes
+
+<!-- SECTION:NOTES:BEGIN -->
+Reporter clarified the failure coincided with a Scryfall catalog import and the production exception was SQLite database busy. The import now commits each 200-card source batch independently instead of holding one multi-minute write transaction. A failed later batch can leave earlier, internally consistent upserts committed; imports are idempotent and a failed sync remains stale for retry. Validation: 32 focused import/sync/deck GraphQL tests passed; strict compilation passed; a real temporary SQLite repo imported 20,000 cards while a maybeboard write completed in 7ms and the importer was still active.
+<!-- SECTION:NOTES:END -->
+
+## Final Summary
+
+<!-- SECTION:FINAL_SUMMARY:BEGIN -->
+Released SQLite's write lock between atomic Scryfall catalog batches so interactive writes no longer wait behind the full import. Preserved import counts and searchable rows, added transaction-boundary and failed-batch atomicity tests, and verified a concurrent maybeboard add during a 20,000-card import.
+<!-- SECTION:FINAL_SUMMARY:END -->

lib/manavault/catalog/scryfall/import.ex

@@ -25,19 +25,18 @@ defmodule Manavault.Catalog.Scryfall.Import do
     log_import_started(log_progress?, source_count)
 
     result =
-      Repo.transact(
-        fn ->
-          counts = import_card_batches(cards, now, oracle_tag_index, source_count, log_progress?)
-
+      case import_card_batches(cards, now, oracle_tag_index, source_count, log_progress?) do
+        {:ok, counts} ->
           {:ok,
            %{
              cards_count: counts.cards_count,
              printings_count: counts.printings_count,
              bulk_uri: bulk_uri
            }}
-        end,
-        timeout: :infinity
-      )
+
+        {:error, reason} ->
+          {:error, reason}
+      end
 
     case result do
       {:ok, counts} ->
@@ -54,19 +53,36 @@ defmodule Manavault.Catalog.Scryfall.Import do
   defp import_card_batches(cards, now, oracle_tag_index, source_count, log_progress?) do
     cards
     |> Enum.chunk_every(@batch_size)
-    |> Enum.reduce(initial_import_counts(), fn batch, counts ->
+    |> Enum.reduce_while({:ok, initial_import_counts()}, fn batch, {:ok, counts} ->
       rows = ImportRows.rows(batch, now, oracle_tag_index)
 
-      insert_card_rows(rows.cards)
-      insert_printing_rows(rows.printings)
-      refresh_printing_search_rows(rows.search_rows)
+      case import_batch(rows) do
+        {:ok, :imported} ->
+          counts =
+            counts
+            |> advance_import_counts(length(batch), rows)
+            |> maybe_log_import_progress(log_progress?, source_count)
 
-      counts
-      |> advance_import_counts(length(batch), rows)
-      |> maybe_log_import_progress(log_progress?, source_count)
+          {:cont, {:ok, counts}}
+
+        {:error, reason} ->
+          {:halt, {:error, reason}}
+      end
     end)
   end
 
+  defp import_batch(rows) do
+    Repo.transact(
+      fn ->
+        insert_card_rows(rows.cards)
+        insert_printing_rows(rows.printings)
+        refresh_printing_search_rows(rows.search_rows)
+        {:ok, :imported}
+      end,
+      timeout: :infinity
+    )
+  end
+
   defp initial_import_counts do
     %{
       source_count: 0,

test/manavault/catalog/import_test.exs

@@ -55,7 +55,26 @@ defmodule Manavault.Catalog.ImportTest do
     assert Jason.decode!(prices) == %{"usd" => "1.00"}
   end
 
-  test "import_cards processes batches without dropping search rows" do
+  test "import_cards releases the write lock between batches without dropping search rows" do
+    test_pid = self()
+    handler_id = {__MODULE__, make_ref()}
+
+    :ok =
+      :telemetry.attach(
+        handler_id,
+        [:manavault, :repo, :query],
+        fn _event, _measurements, metadata, pid ->
+          query = metadata |> Map.get(:query, "") |> to_string() |> String.downcase()
+
+          if query == "commit" or String.starts_with?(query, "release savepoint") do
+            send(pid, :catalog_import_batch_committed)
+          end
+        end,
+        test_pid
+      )
+
+    on_exit(fn -> :telemetry.detach(handler_id) end)
+
     cards =
       Enum.map(1..205, fn index ->
         %{
@@ -68,6 +87,8 @@ defmodule Manavault.Catalog.ImportTest do
       end)
 
     assert {:ok, %{cards_count: 205, printings_count: 205}} = Catalog.import_cards(cards)
+    assert_receive :catalog_import_batch_committed
+    assert_receive :catalog_import_batch_committed
     assert Repo.aggregate(Card, :count) == 205
     assert Repo.aggregate(Printing, :count) == 205
 
@@ -79,6 +100,31 @@ defmodule Manavault.Catalog.ImportTest do
              )
   end
 
+  test "import_cards rolls back every write in a failed batch" do
+    Repo.query!("""
+    CREATE TEMP TRIGGER fail_catalog_batch
+    BEFORE INSERT ON scryfall_printings
+    WHEN NEW.scryfall_id = 'scryfall-atomic-batch'
+    BEGIN
+      SELECT RAISE(ABORT, 'catalog batch test failure');
+    END
+    """)
+
+    card = %{
+      @time_walk
+      | "id" => "scryfall-atomic-batch",
+        "oracle_id" => "oracle-atomic-batch",
+        "name" => "Atomic Batch Card"
+    }
+
+    assert_raise Exqlite.Error, ~r/catalog batch test failure/, fn ->
+      Catalog.import_cards([card])
+    end
+
+    refute Repo.get(Card, "oracle-atomic-batch")
+    refute Repo.get(Printing, "scryfall-atomic-batch")
+  end
+
   test "import_cards stores selected oracle tags and derives deck grouping fields" do
     oracle_tags = [
       scryfall_tag(%{