Backend- file ingestion API Updates

tests/test_batch_catalog_ingest.py

@@ -0,0 +1,640 @@
+"""Tests for multi-file batch ingestion.
+
+Follows test_store_catalog_pipeline.py: monkeypatch the storage and embedding
+boundary *on the pipeline module object*, build real spreadsheets in memory,
+and point the batch directory at tmp_path so nothing is written into the repo.
+
+Nothing here loads sentence-transformers or torch, and nothing reaches the
+network: every batch runs with use_llm=False and fetch_images=False, which are
+also the defaults the endpoint ships.
+"""
+from __future__ import annotations
+
+import io
+import json
+import queue
+import threading
+import time
+
+import pytest
+
+from app.core import batch_ingest
+from app.core import store_catalog_pipeline as pipeline
+
+openpyxl = pytest.importorskip("openpyxl")
+
+HEADERS = ["Product Name", "Category", "Brand"]
+ROWS = [
+    ["Amul Butter 100g", "Butter", "Amul"],
+    ["Amul Cheese Slices 200g", "Cheese", "Amul"],
+]
+
+
+def _sheet(headers=HEADERS, rows=ROWS) -> bytes:
+    wb = openpyxl.Workbook()
+    ws = wb.active
+    ws.append(headers)
+    for row in rows:
+        ws.append(row)
+    buf = io.BytesIO()
+    wb.save(buf)
+    return buf.getvalue()
+
+
+def _csv(headers=HEADERS, rows=ROWS) -> bytes:
+    lines = [",".join(headers)] + [",".join(r) for r in rows]
+    return ("\n".join(lines)).encode("utf-8")
+
+
+@pytest.fixture(autouse=True)
+def _isolate_sku_counter(tmp_path, monkeypatch):
+    """Same reason as test_store_catalog_pipeline: keep the SKU sequence file
+    out of the working tree and make numbering deterministic per test."""
+    from app.services import sku_service
+    monkeypatch.setattr(sku_service, "_data_dir", tmp_path / "sku_sequences")
+
+
+@pytest.fixture(autouse=True)
+def batch_root(tmp_path, monkeypatch):
+    """Point BATCH_UPLOAD_DIR at tmp_path.
+
+    `batch_ingest.batch_root()` reads the setting at call time precisely so
+    this is possible; patching it at import time would not reach the module.
+    """
+    root = tmp_path / "batch_uploads"
+    monkeypatch.setattr(batch_ingest, "BATCH_UPLOAD_DIR", root)
+    return root
+
+
+@pytest.fixture(autouse=True)
+def no_background_worker(monkeypatch):
+    """Keep the real worker thread out of every test that does not want it.
+
+    THIS FIXTURE EXISTS BECAUSE ITS ABSENCE CORRUPTED THE WORKING TREE.
+
+    `batch_root()` resolves BATCH_UPLOAD_DIR at call time. That is correct in
+    production, where the setting never moves. In a test it moves twice: the
+    fixture points it at tmp_path, and monkeypatch puts it back when the test
+    ends. An endpoint test POSTs to /ingest, the worker starts on a background
+    thread, the test returns - and the worker, waking up after teardown, writes
+    its manifest into the repository's real `data/batch_uploads/`. Two such
+    directories were committed-adjacent before this was noticed.
+
+    So `submit` is stubbed by default and endpoint tests assert on what they
+    are actually about. The two tests that exercise the worker itself take
+    `real_submit` and wait for it to drain before returning.
+    """
+    from app.core import batch_worker
+
+    real = batch_worker.submit
+    submitted: list = []
+    monkeypatch.setattr(batch_worker, "submit", submitted.append)
+
+    class Handle:
+        pass
+
+    handle = Handle()
+    handle.submitted = submitted
+    handle.real_submit = real
+    return handle
+
+
+@pytest.fixture
+def store(monkeypatch):
+    """A fake brand table shared by every file in a batch."""
+    table: dict = {}
+
+    def fake_upsert(brand, rows, cleanup=False):
+        assert cleanup is False, "cleanup=True would delete the brand's existing catalog"
+        for row in rows:
+            table[row["image_id"]] = dict(row)
+        return len(rows)
+
+    monkeypatch.setattr(pipeline, "upsert_brand_products", fake_upsert)
+    monkeypatch.setattr(pipeline, "get_products_by_brand", lambda b, **kw: list(table.values()))
+    monkeypatch.setattr(pipeline, "embed_texts", lambda texts: [[0.0] * 384 for _ in texts])
+    return table
+
+
+# ---------------------------------------------------------------------------
+# Staging
+# ---------------------------------------------------------------------------
+def test_staging_writes_files_and_a_manifest(batch_root):
+    manifest = batch_ingest.stage_batch([("a.csv", _csv()), ("b.xlsx", _sheet())])
+
+    directory = batch_root / manifest.batch_id
+    assert (directory / "manifest.json").exists()
+    assert sorted(p.name for p in directory.iterdir()) == [
+        "00_a.csv", "01_b.xlsx", "manifest.json",
+    ]
+    assert [f.status for f in manifest.files] == ["queued", "queued"]
+    assert manifest.files_total == 2
+
+
+def test_stored_names_cannot_escape_the_batch_directory(batch_root):
+    """UploadFile.filename is attacker-controlled and is used to build a path."""
+    manifest = batch_ingest.stage_batch([
+        ("../../evil.csv", _csv()),
+        ("..\\windows\\evil2.csv", _csv()),
+        ("/etc/passwd", _csv()),
+    ])
+
+    directory = batch_root / manifest.batch_id
+    for entry in manifest.files:
+        written = directory / entry.stored_name
+        assert written.resolve().parent == directory.resolve()
+        assert ".." not in entry.stored_name
+        assert "/" not in entry.stored_name and "\\" not in entry.stored_name
+    # And nothing landed outside.
+    assert not (batch_root.parent / "evil.csv").exists()
+
+
+def test_invalid_files_are_recorded_not_dropped(batch_root):
+    """An operator who selected three files and sees two must be told why."""
+    manifest = batch_ingest.stage_batch(
+        [("good.csv", _csv())],
+        invalid=[("broken.pdf", "Unsupported file type")],
+    )
+    assert manifest.files_total == 2
+    bad = [f for f in manifest.files if f.filename == "broken.pdf"][0]
+    assert bad.status == "failed"
+    assert bad.detail == "Unsupported file type"
+    assert bad.stored_name == ""
+
+
+def test_manifest_round_trips_through_disk(batch_root):
+    manifest = batch_ingest.stage_batch([("a.csv", _csv())], fetch_images=True)
+    reloaded = batch_ingest.read_manifest(manifest.batch_id)
+    assert reloaded.batch_id == manifest.batch_id
+    assert reloaded.fetch_images is True
+    assert [f.filename for f in reloaded.files] == ["a.csv"]
+
+
+def test_unreadable_manifest_is_skipped_not_raised(batch_root):
+    manifest = batch_ingest.stage_batch([("a.csv", _csv())])
+    batch_ingest.manifest_path(manifest.batch_id).write_text("{ not json", encoding="utf-8")
+    assert batch_ingest.read_manifest(manifest.batch_id) is None
+    assert batch_ingest.list_manifests() == []
+
+
+def test_batch_dir_rejects_a_traversing_id(batch_root):
+    with pytest.raises(ValueError):
+        batch_ingest.batch_dir("../escape")
+
+
+# ---------------------------------------------------------------------------
+# Running a batch
+# ---------------------------------------------------------------------------
+def test_every_file_runs_and_totals_are_summed(store, batch_root):
+    manifest = batch_ingest.stage_batch([("a.csv", _csv()), ("b.xlsx", _sheet())])
+    result = batch_ingest.run_batch(manifest.batch_id)
+
+    assert result.status == "done"
+    assert result.files_done == 2
+    assert result.files_failed == 0
+    assert [f.status for f in result.files] == ["done", "done"]
+    # Both files carry the same two products, so the second file backfills or
+    # skips rather than inserting again - but every row was seen.
+    assert result.totals()["rows_total"] == 4
+    assert result.totals()["products_built"] >= 4
+    assert "Amul" in result.brands()
+
+
+def test_one_unparseable_file_does_not_stop_the_others(store, batch_root):
+    """The whole point of a batch: file 2 failing must not lose files 1 and 3."""
+    manifest = batch_ingest.stage_batch([
+        ("good1.csv", _csv()),
+        ("truncated.xlsx", _sheet()),
+    ])
+    # Corrupt the second staged file after staging, so run_batch meets it cold.
+    # It has to be the .xlsx: pandas' CSV reader is tolerant enough that binary
+    # noise in a .csv still parses into *something*, which is not the failure
+    # this test is about.
+    directory = batch_ingest.batch_dir(manifest.batch_id)
+    (directory / manifest.files[1].stored_name).write_bytes(b"\x00\x01\x02not a sheet")
+
+    result = batch_ingest.run_batch(manifest.batch_id)
+
+    assert result.files_done == 1
+    assert result.files_failed == 1
+    assert result.status == "partial", "a partial batch must not read as success"
+    assert result.files[1].detail
+
+
+def test_a_batch_with_a_pre_failed_file_is_partial(store, batch_root):
+    manifest = batch_ingest.stage_batch(
+        [("good.csv", _csv())],
+        invalid=[("broken.pdf", "Unsupported file type")],
+    )
+    result = batch_ingest.run_batch(manifest.batch_id)
+    assert result.status == "partial"
+    assert result.files_done == 1
+    assert result.files_failed == 1
+
+
+def test_storage_error_marks_the_file_failed_not_done(monkeypatch, store, batch_root):
+    """Rows built but nothing written is a failure, exactly as in the single-file path."""
+    def exploding_upsert(brand, rows, cleanup=False):
+        raise RuntimeError("connection refused")
+
+    monkeypatch.setattr(pipeline, "upsert_brand_products", exploding_upsert)
+
+    manifest = batch_ingest.stage_batch([("a.csv", _csv())])
+    result = batch_ingest.run_batch(manifest.batch_id)
+
+    assert result.files[0].status == "failed"
+    assert "connection refused" in result.files[0].detail
+    assert result.status == "failed"
+
+
+def test_progress_reaches_the_callback(store, batch_root):
+    manifest = batch_ingest.stage_batch([("a.csv", _csv())])
+    seen = []
+
+    def on_change(current):
+        entry = current.files[0]
+        if entry.stage_index:
+            seen.append((entry.stage_index, entry.stage_name))
+
+    batch_ingest.run_batch(manifest.batch_id, on_change=on_change)
+
+    assert seen, "expected stage progress"
+    assert max(s for s, _ in seen) == pipeline.TOTAL_STAGES
+    assert seen[-1][1] == pipeline.STAGE_NAMES[-1]
+
+
+def test_cancel_stops_before_the_next_file(store, batch_root):
+    manifest = batch_ingest.stage_batch([
+        ("a.csv", _csv()), ("b.csv", _csv()), ("c.csv", _csv()),
+    ])
+    calls = {"n": 0}
+
+    def should_cancel():
+        # Let the first file through, then cancel.
+        calls["n"] += 1
+        return calls["n"] > 1
+
+    result = batch_ingest.run_batch(manifest.batch_id, should_cancel=should_cancel)
+
+    assert result.files[0].status == "done"
+    assert [f.status for f in result.files[1:]] == ["cancelled", "cancelled"]
+
+
+def test_rerunning_a_finished_batch_does_nothing(store, batch_root):
+    manifest = batch_ingest.stage_batch([("a.csv", _csv())])
+    batch_ingest.run_batch(manifest.batch_id)
+    before = batch_ingest.read_manifest(manifest.batch_id).files[0].finished_at
+
+    again = batch_ingest.run_batch(manifest.batch_id)
+    assert again.files[0].finished_at == before, "a done file must not run twice"
+
+
+# ---------------------------------------------------------------------------
+# Recovery and retention
+# ---------------------------------------------------------------------------
+def test_scan_interrupted_marks_but_does_not_run(monkeypatch, batch_root):
+    """A crash loop must not re-enter the heaviest work in the app on every boot."""
+    monkeypatch.setattr(batch_ingest, "BATCH_AUTO_RESUME", False)
+
+    manifest = batch_ingest.stage_batch([("a.csv", _csv()), ("b.csv", _csv())])
+    manifest.status = "running"
+    manifest.files[0].status = "done"
+    manifest.files[1].status = "running"
+    batch_ingest.write_manifest(manifest)
+
+    resumable = batch_ingest.scan_interrupted()
+
+    assert resumable == [manifest.batch_id]
+    reloaded = batch_ingest.read_manifest(manifest.batch_id)
+    assert reloaded.status == "interrupted"
+    assert reloaded.files[0].status == "done", "a finished file is not re-run"
+    assert reloaded.files[1].status == "queued", "the in-flight file is queued again"
+
+
+def test_scan_interrupted_leaves_finished_batches_alone(store, batch_root):
+    manifest = batch_ingest.stage_batch([("a.csv", _csv())])
+    batch_ingest.run_batch(manifest.batch_id)
+
+    assert batch_ingest.scan_interrupted() == []
+    assert batch_ingest.read_manifest(manifest.batch_id).status == "done"
+
+
+def test_purge_removes_old_finished_batches_only(store, batch_root, monkeypatch):
+    monkeypatch.setattr(batch_ingest, "BATCH_RETENTION_DAYS", 7)
+
+    old = batch_ingest.stage_batch([("a.csv", _csv())])
+    batch_ingest.run_batch(old.batch_id)
+    stale = batch_ingest.read_manifest(old.batch_id)
+    stale.created_at = time.time() - 30 * 86400
+    batch_ingest.write_manifest(stale)
+
+    recent = batch_ingest.stage_batch([("b.csv", _csv())])
+    batch_ingest.run_batch(recent.batch_id)
+
+    removed = batch_ingest.purge_expired()
+
+    assert removed == [old.batch_id]
+    assert not batch_ingest.batch_dir(old.batch_id).exists()
+    assert batch_ingest.batch_dir(recent.batch_id).exists()
+
+
+def test_purge_spares_an_old_batch_that_never_ran(batch_root, monkeypatch):
+    """Deleting the only copy of an unrun batch's input is not how to debug it."""
+    monkeypatch.setattr(batch_ingest, "BATCH_RETENTION_DAYS", 7)
+
+    manifest = batch_ingest.stage_batch([("a.csv", _csv())])
+    manifest.created_at = time.time() - 30 * 86400
+    batch_ingest.write_manifest(manifest)
+
+    assert batch_ingest.purge_expired() == []
+    assert batch_ingest.batch_dir(manifest.batch_id).exists()
+
+
+def test_purge_is_disabled_when_retention_is_zero(batch_root, monkeypatch):
+    monkeypatch.setattr(batch_ingest, "BATCH_RETENTION_DAYS", 0)
+    manifest = batch_ingest.stage_batch([("a.csv", _csv())])
+    manifest.created_at = time.time() - 999 * 86400
+    manifest.status = "done"
+    batch_ingest.write_manifest(manifest)
+    assert batch_ingest.purge_expired() == []
+
+
+# ---------------------------------------------------------------------------
+# The worker
+# ---------------------------------------------------------------------------
+def test_worker_runs_one_batch_at_a_time(store, batch_root, monkeypatch,
+                                         no_background_worker):
+    """Two batches must serialise, not compete for the single vCPU."""
+    from app.core import batch_worker
+
+    # Idle housekeeping deletes directories. It runs after the queue drains,
+    # which can be after teardown has moved BATCH_UPLOAD_DIR back to the real
+    # one - so it is disabled here rather than raced with.
+    monkeypatch.setattr(batch_worker.batch_ingest, "purge_expired", lambda *a, **k: [])
+
+    overlapping = []
+    active = {"n": 0}
+    guard = threading.Lock()
+    finished = threading.Event()
+    done_count = {"n": 0}
+
+    real_run = batch_ingest.run_batch
+
+    def instrumented(batch_id, **kwargs):
+        with guard:
+            active["n"] += 1
+            if active["n"] > 1:
+                overlapping.append(batch_id)
+        try:
+            time.sleep(0.05)
+            return real_run(batch_id, **kwargs)
+        finally:
+            with guard:
+                active["n"] -= 1
+                done_count["n"] += 1
+                if done_count["n"] == 2:
+                    finished.set()
+
+    monkeypatch.setattr(batch_worker.batch_ingest, "run_batch", instrumented)
+
+    first = batch_ingest.stage_batch([("a.csv", _csv())])
+    second = batch_ingest.stage_batch([("b.csv", _csv())])
+    no_background_worker.real_submit(first.batch_id)
+    no_background_worker.real_submit(second.batch_id)
+
+    assert finished.wait(timeout=60), "batches did not finish"
+    # Drain before teardown moves BATCH_UPLOAD_DIR back.
+    batch_worker._queue.join()
+    assert overlapping == [], "two batches ran concurrently"
+
+
+def test_worker_survives_a_batch_that_raises(store, batch_root, monkeypatch,
+                                             no_background_worker):
+    """A worker that dies would leave every later batch queued behind nothing."""
+    from app.core import batch_worker
+
+    monkeypatch.setattr(batch_worker.batch_ingest, "purge_expired", lambda *a, **k: [])
+
+    done = threading.Event()
+    real_run = batch_ingest.run_batch
+    seen = []
+
+    def flaky(batch_id, **kwargs):
+        seen.append(batch_id)
+        if len(seen) == 1:
+            raise RuntimeError("boom")
+        try:
+            return real_run(batch_id, **kwargs)
+        finally:
+            done.set()
+
+    monkeypatch.setattr(batch_worker.batch_ingest, "run_batch", flaky)
+
+    bad = batch_ingest.stage_batch([("a.csv", _csv())])
+    good = batch_ingest.stage_batch([("b.csv", _csv())])
+    no_background_worker.real_submit(bad.batch_id)
+    no_background_worker.real_submit(good.batch_id)
+
+    assert done.wait(timeout=60), "the worker died on the first batch"
+    batch_worker._queue.join()
+    assert seen == [bad.batch_id, good.batch_id]
+
+
+# ---------------------------------------------------------------------------
+# Endpoints
+# ---------------------------------------------------------------------------
+INGEST = "/api/admin/catalog-batch/ingest"
+PREVIEW = "/api/admin/catalog-batch/preview"
+
+
+def _files(*pairs):
+    return [("files", (name, io.BytesIO(content), "application/octet-stream"))
+            for name, content in pairs]
+
+
+def test_endpoints_require_an_admin(client, user_headers):
+    assert client.post(PREVIEW, files=_files(("a.csv", _csv()))).status_code == 401
+    assert client.post(
+        PREVIEW, files=_files(("a.csv", _csv())), headers=user_headers
+    ).status_code == 403
+    assert client.get("/api/admin/catalog-batch/batches").status_code == 401
+
+
+def test_preview_reports_each_file_separately(client, admin_headers):
+    response = client.post(
+        PREVIEW,
+        files=_files(("a.csv", _csv()), ("broken.pdf", b"%PDF-1.4")),
+        headers=admin_headers,
+    )
+    assert response.status_code == 200
+    body = response.json()
+    assert body["files_total"] == 2
+    assert body["files_ok"] == 1
+    good = [f for f in body["files"] if f["filename"] == "a.csv"][0]
+    assert good["ok"] is True
+    assert good["rows_total"] == 2
+    assert good["brand_column_present"] is True
+    bad = [f for f in body["files"] if f["filename"] == "broken.pdf"][0]
+    assert bad["ok"] is False and bad["error"]
+    assert body["stages"] == list(pipeline.STAGE_NAMES)
+
+
+def test_ingest_returns_202_and_a_batch_id(client, admin_headers, store, batch_root):
+    response = client.post(
+        INGEST, files=_files(("a.csv", _csv()), ("b.xlsx", _sheet())), headers=admin_headers
+    )
+    assert response.status_code == 202
+    body = response.json()
+    assert body["files_total"] == 2
+    assert body["use_llm"] is False, "the LLM stage must be opt-in for a batch"
+    assert body["fetch_images"] is False, "image search must be opt-in for a batch"
+    assert len(body["files"]) == 2
+    assert body["files"][0]["total_stages"] == pipeline.TOTAL_STAGES
+
+    polled = client.get(
+        f"/api/admin/catalog-batch/batches/{body['batch_id']}", headers=admin_headers
+    )
+    assert polled.status_code == 200
+    assert polled.json()["batch_id"] == body["batch_id"]
+
+
+def test_ingest_rejects_a_batch_where_nothing_is_usable(client, admin_headers):
+    response = client.post(
+        INGEST, files=_files(("broken.pdf", b"%PDF-1.4")), headers=admin_headers
+    )
+    assert response.status_code == 400
+    assert "broken.pdf" in response.json()["detail"]
+
+
+def test_ingest_keeps_the_good_files_when_one_is_bad(client, admin_headers, store, batch_root):
+    response = client.post(
+        INGEST,
+        files=_files(("a.csv", _csv()), ("broken.pdf", b"%PDF-1.4")),
+        headers=admin_headers,
+    )
+    assert response.status_code == 202
+    body = response.json()
+    assert body["files_total"] == 2
+    statuses = {f["filename"]: f["status"] for f in body["files"]}
+    assert statuses["broken.pdf"] == "failed"
+
+
+def test_too_many_files_is_413(client, admin_headers, monkeypatch):
+    from app.api.routers import batch_catalog
+
+    monkeypatch.setattr(batch_catalog, "BATCH_MAX_FILES", 2)
+    response = client.post(
+        INGEST,
+        files=_files(("a.csv", _csv()), ("b.csv", _csv()), ("c.csv", _csv())),
+        headers=admin_headers,
+    )
+    assert response.status_code == 413
+    assert "2-file limit" in response.json()["detail"]
+
+
+def test_a_file_over_the_size_limit_is_413(client, admin_headers, monkeypatch):
+    from app.api.routers import batch_catalog
+
+    monkeypatch.setattr(batch_catalog, "MAX_UPLOAD_BYTES", 64)
+    response = client.post(
+        INGEST, files=_files(("big.csv", b"x" * 500)), headers=admin_headers
+    )
+    assert response.status_code == 413
+
+
+def test_too_many_rows_across_the_batch_is_413(client, admin_headers, monkeypatch):
+    from app.api.routers import batch_catalog
+
+    monkeypatch.setattr(batch_catalog, "BATCH_MAX_TOTAL_ROWS", 3)
+    response = client.post(
+        INGEST,
+        files=_files(("a.csv", _csv()), ("b.csv", _csv())),
+        headers=admin_headers,
+    )
+    assert response.status_code == 413
+    assert "3 rows" in response.json()["detail"]
+
+
+def test_a_full_queue_is_429_and_the_batch_is_kept(client, admin_headers, monkeypatch, batch_root):
+    """Refusing work is fine; losing the upload that was refused is not."""
+    from app.api.routers import batch_catalog
+
+    def full(_batch_id):
+        raise queue.Full()
+
+    monkeypatch.setattr(batch_catalog.batch_worker, "submit", full)
+
+    response = client.post(INGEST, files=_files(("a.csv", _csv())), headers=admin_headers)
+    assert response.status_code == 429
+
+    staged = batch_ingest.list_manifests()
+    assert len(staged) == 1, "the refused batch must still be on disk"
+    assert staged[0].status == "queued"
+
+
+def test_unknown_batch_is_404(client, admin_headers):
+    response = client.get(
+        "/api/admin/catalog-batch/batches/deadbeef", headers=admin_headers
+    )
+    assert response.status_code == 404
+
+
+def test_resume_requeues_an_interrupted_batch(client, admin_headers, monkeypatch, batch_root):
+    from app.api.routers import batch_catalog
+
+    submitted = []
+    monkeypatch.setattr(batch_catalog.batch_worker, "submit", submitted.append)
+
+    manifest = batch_ingest.stage_batch([("a.csv", _csv())])
+    manifest.status = "interrupted"
+    batch_ingest.write_manifest(manifest)
+
+    response = client.post(
+        f"/api/admin/catalog-batch/batches/{manifest.batch_id}/resume", headers=admin_headers
+    )
+    assert response.status_code == 200
+    assert submitted == [manifest.batch_id]
+
+
+def test_resume_on_a_finished_batch_is_409(client, admin_headers, store, batch_root):
+    manifest = batch_ingest.stage_batch([("a.csv", _csv())])
+    batch_ingest.run_batch(manifest.batch_id)
+
+    response = client.post(
+        f"/api/admin/catalog-batch/batches/{manifest.batch_id}/resume", headers=admin_headers
+    )
+    assert response.status_code == 409
+
+
+def test_cancel_on_a_queued_batch_cancels_its_files(client, admin_headers, batch_root):
+    manifest = batch_ingest.stage_batch([("a.csv", _csv()), ("b.csv", _csv())])
+
+    response = client.post(
+        f"/api/admin/catalog-batch/batches/{manifest.batch_id}/cancel", headers=admin_headers
+    )
+    assert response.status_code == 200
+    body = response.json()
+    assert body["status"] == "cancelled"
+    assert {f["status"] for f in body["files"]} == {"cancelled"}
+
+
+def test_listing_shows_batches_from_disk(client, admin_headers, batch_root):
+    first = batch_ingest.stage_batch([("a.csv", _csv())])
+    second = batch_ingest.stage_batch([("b.csv", _csv())])
+
+    response = client.get("/api/admin/catalog-batch/batches", headers=admin_headers)
+    assert response.status_code == 200
+    ids = {b["batch_id"] for b in response.json()["batches"]}
+    assert {first.batch_id, second.batch_id} <= ids
+
+
+def test_manifest_on_disk_is_valid_json_after_a_run(store, batch_root):
+    """The recovery path reads every manifest it finds; a torn write breaks boot."""
+    manifest = batch_ingest.stage_batch([("a.csv", _csv())])
+    batch_ingest.run_batch(manifest.batch_id)
+
+    raw = json.loads(batch_ingest.manifest_path(manifest.batch_id).read_text(encoding="utf-8"))
+    assert raw["status"] == "done"
+    assert raw["files"][0]["status"] == "done"
+    # The temp file must not survive the write.
+    assert not (batch_ingest.batch_dir(manifest.batch_id) / "manifest.json.tmp").exists()

tests/test_orchestration_defs.py

@@ -54,6 +54,11 @@ def test_every_expected_asset_exists(asset_graph):
         "training_dataset",
         "trained_models",
         "model_evaluation",
+        # batch (uploaded spreadsheets)
+        "batch_manifest",
+        "batch_parsed_files",
+        "batch_catalog_rows",
+        "batch_ingest_report",
     }
 
 
@@ -70,6 +75,9 @@ def test_every_expected_asset_exists(asset_graph):
         ("nutrition_models", {"nutrition_data"}),
         ("trained_models", {"training_dataset"}),
         ("model_evaluation", {"trained_models"}),
+        ("batch_parsed_files", {"batch_manifest"}),
+        ("batch_catalog_rows", {"batch_manifest", "batch_parsed_files"}),
+        ("batch_ingest_report", {"batch_catalog_rows"}),
     ],
 )
 def test_lineage_edges(asset_graph, asset_key, expected_parents):
@@ -113,12 +121,32 @@ def test_cross_brand_assets_are_not_partitioned(asset_graph):
         assert not asset_graph.get(AssetKey(key)).is_partitioned, key
 
 
+def test_batch_assets_are_not_partitioned(asset_graph):
+    """A batch is created by an upload, not enumerated in advance.
+
+    Partitioning these would mean a dynamic partition per upload that is never
+    removed, so the partition set grows without bound and the UI fills with
+    dead keys. The batch id belongs in run config, which is what run config is
+    for.
+    """
+    from dagster import AssetKey
+
+    for key in (
+        "batch_manifest",
+        "batch_parsed_files",
+        "batch_catalog_rows",
+        "batch_ingest_report",
+    ):
+        assert not asset_graph.get(AssetKey(key)).is_partitioned, key
+
+
 def test_all_four_jobs_resolve(defs):
     assert {job.name for job in defs.jobs} == {
         "catalog_ingestion_job",
         "embedding_refresh_job",
         "nutrition_enrichment_job",
         "ml_training_job",
+        "batch_ingestion_job",
     }
 
 
@@ -138,7 +166,10 @@ def test_every_schedule_ships_stopped(defs):
 def test_sensor_ships_stopped_and_is_not_hot(defs):
     from dagster import DefaultSensorStatus
 
-    assert defs.sensors, "expected the seed-catalog sensor"
+    assert {s.name for s in defs.sensors} == {
+        "seed_catalog_sensor",
+        "batch_upload_sensor",
+    }
     for sensor in defs.sensors:
         assert sensor.default_status == DefaultSensorStatus.STOPPED, sensor.name
         assert sensor.minimum_interval_seconds >= 60, sensor.name
@@ -148,7 +179,8 @@ def test_network_assets_retry_and_are_bounded(asset_graph):
     """Retries must exist on the flaky steps and must never be unbounded."""
     from dagster import AssetKey
 
-    for key in ("raw_products", "enriched_products", "product_embeddings"):
+    for key in ("raw_products", "enriched_products", "product_embeddings",
+                "batch_catalog_rows"):
         policy = asset_graph.get(AssetKey(key)).assets_def.op.retry_policy
         assert policy is not None, key
         assert 0 < policy.max_retries <= 3, key