From e6ededbe8e73aceb8e83c9aec9612a6d76e8d168 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 25 May 2026 22:16:26 -0400 Subject: [PATCH] =?UTF-8?q?feat(deep-scan):=20IR-parity=20port=20=E2=80=94?= =?UTF-8?q?=20refreshed=20status=20+=20counter,=20re-queue=20completed=20p?= =?UTF-8?q?aths=20in=20deep=20mode,=20honest=20UX=20=E2=80=94=20Co-Authore?= =?UTF-8?q?d-By:=20Claude=20Opus=204.7=20(1M=20context)=20?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../versions/0019_import_batch_refreshed.py | 38 ++++++ backend/app/api/import_admin.py | 1 + backend/app/models/import_batch.py | 4 + backend/app/services/importer.py | 27 ++++- backend/app/tasks/import_file.py | 15 ++- backend/app/tasks/scan.py | 25 ++-- .../settings/ImportTriggerPanel.vue | 14 ++- frontend/src/stores/import.js | 33 +++++- tests/test_importer_deep.py | 8 +- tests/test_phash_dedup.py | 6 + tests/test_scan_deep_requeue.py | 109 ++++++++++++++++++ 11 files changed, 251 insertions(+), 29 deletions(-) create mode 100644 alembic/versions/0019_import_batch_refreshed.py create mode 100644 tests/test_scan_deep_requeue.py diff --git a/alembic/versions/0019_import_batch_refreshed.py b/alembic/versions/0019_import_batch_refreshed.py new file mode 100644 index 0000000..1770daa --- /dev/null +++ b/alembic/versions/0019_import_batch_refreshed.py @@ -0,0 +1,38 @@ +"""import_batch.refreshed counter for deep-scan sidecar re-application + +Revision ID: 0019 +Revises: 0018 +Create Date: 2026-05-25 + +Adds a `refreshed` counter to `import_batch`, mirroring the existing +`imported`/`skipped`/`failed`/`attachments` columns. Deep scan now +re-applies sidecar metadata to already-imported files (the IR feature +that didn't make the FC port the first time); a "refreshed" outcome +increments this counter so the UI can surface "X new, Y refreshed" +instead of the misleading "Scan complete — no new files" message. + +server_default=0 backfills existing rows in place — no UPDATE needed. +""" +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +revision: str = "0019" +down_revision: Union[str, None] = "0018" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.add_column( + "import_batch", + sa.Column( + "refreshed", sa.Integer(), + nullable=False, server_default=sa.text("0"), + ), + ) + + +def downgrade() -> None: + op.drop_column("import_batch", "refreshed") diff --git a/backend/app/api/import_admin.py b/backend/app/api/import_admin.py index 68ae32a..367305b 100644 --- a/backend/app/api/import_admin.py +++ b/backend/app/api/import_admin.py @@ -53,6 +53,7 @@ async def status(): "imported": active.imported, "skipped": active.skipped, "failed": active.failed, + "refreshed": active.refreshed, "started_at": active.started_at.isoformat(), } return jsonify(payload) diff --git a/backend/app/models/import_batch.py b/backend/app/models/import_batch.py index d26db4c..474f111 100644 --- a/backend/app/models/import_batch.py +++ b/backend/app/models/import_batch.py @@ -26,6 +26,10 @@ class ImportBatch(Base): skipped: Mapped[int] = mapped_column(Integer, nullable=False, default=0) failed: Mapped[int] = mapped_column(Integer, nullable=False, default=0) attachments: Mapped[int] = mapped_column(Integer, nullable=False, default=0) + # Deep-scan only: count of already-imported files whose sidecar metadata + # got re-applied this run (post/source/provenance upsert). Stays 0 on + # quick-scan batches. See `Importer.import_one(deep_scan=True)`. + refreshed: Mapped[int] = mapped_column(Integer, nullable=False, default=0) status: Mapped[str] = mapped_column(String(16), nullable=False, default="running", index=True) # running | complete | cancelled diff --git a/backend/app/services/importer.py b/backend/app/services/importer.py index cf4c0c4..309f923 100644 --- a/backend/app/services/importer.py +++ b/backend/app/services/importer.py @@ -51,7 +51,14 @@ class SkipReason(StrEnum): @dataclass(frozen=True) class ImportResult: - status: str # 'imported'|'skipped'|'failed'|'superseded'|'attached' + # 'imported' — new ImageRecord row created + # 'superseded' — existing ImageRecord row got the new file (larger) + sidecar + # 'attached' — non-media saved as PostAttachment + # 'refreshed' — deep scan re-applied sidecar / filled NULL phash / NULL + # artist on an already-imported row (no new ImageRecord) + # 'skipped' — no work done (true duplicate, too small, etc.) + # 'failed' — pipeline error + status: str image_id: int | None = None skip_reason: SkipReason | None = None error: str | None = None @@ -417,7 +424,18 @@ class Importer: ) -> ImportResult: """Deep scan: backfill phash/provenance/artist on an already-imported record. METADATA ONLY — never re-runs the pHash - near-dup / supersede path. NULL-only, idempotent.""" + near-dup / supersede path. NULL-only on phash/artist, additive on + sidecar Post/Source/ImageProvenance (via _apply_sidecar). + Idempotent: a second deep-scan over the same file finds nothing + to refresh and is a no-op. + + Returns status="refreshed" so the UI can surface the work done + instead of the prior misleading "skipped/duplicate_hash" reading. + Operator-flagged 2026-05-25 — IR has had this; FC inherited it + as a no-op skip during the original port and the UI showed deep + scan as "completed with no changes" even when sidecar metadata + actually got re-applied to N existing rows. + """ if existing.phash is None and not is_video(source): try: with Image.open(source) as im: @@ -436,10 +454,7 @@ class Importer: self._apply_sidecar(existing, attribution_path, artist) self.session.commit() - return ImportResult( - status="skipped", skip_reason=SkipReason.duplicate_hash, - image_id=existing.id, error="deep: re-derived", - ) + return ImportResult(status="refreshed", image_id=existing.id) def attach_in_place( self, diff --git a/backend/app/tasks/import_file.py b/backend/app/tasks/import_file.py index 7fd1066..e0752e8 100644 --- a/backend/app/tasks/import_file.py +++ b/backend/app/tasks/import_file.py @@ -29,10 +29,12 @@ IMAGES_ROOT = Path("/images") def _map_result_to_status(result): """(ImportTask.status, should_requeue_ml_and_thumb) for an ImportResult. 'superseded' = the kept row's file/ML changed → complete + re-derive. - 'attached' = a non-art file preserved → complete, no ML/thumb.""" + 'attached' = a non-art file preserved → complete, no ML/thumb. + 'refreshed' = deep scan refreshed sidecar/phash on an existing row → + complete, no ML/thumb re-derive (file/pixels unchanged).""" if result.status in ("imported", "superseded"): return ("complete", True) - if result.status == "attached": + if result.status in ("attached", "refreshed"): return ("complete", False) if result.status == "skipped": return ("skipped", False) @@ -138,6 +140,15 @@ def _do_import(session, task, import_task_id: int) -> dict: task.result_image_id = result.image_id counter_col_name = "imported" counter_col = ImportBatch.imported + elif result.status == "refreshed": + # Deep-scan rederive: existing row got phash/artist/sidecar + # refreshed. Task is complete (no further work), but counted in + # `refreshed` not `imported` so the UI can surface the actual + # work done. operator-flagged 2026-05-25. + task.status = "complete" + task.result_image_id = result.image_id + counter_col_name = "refreshed" + counter_col = ImportBatch.refreshed elif result.status == "attached": task.status = "complete" counter_col_name = "attachments" diff --git a/backend/app/tasks/scan.py b/backend/app/tasks/scan.py index 6aacde0..2e5b612 100644 --- a/backend/app/tasks/scan.py +++ b/backend/app/tasks/scan.py @@ -59,16 +59,25 @@ def scan_directory(self, triggered_by: str = "manual", session.flush() batch_id = batch.id - # Skip-set: any source_path that already has a non-failed ImportTask - # row. Re-running scan_directory must not re-enqueue files the - # importer has already handled (or is currently handling); doing so - # creates duplicate work and inflates the queue. Failed prior tasks - # are eligible for retry. + # Skip-set behavior splits by mode (operator-flagged 2026-05-25): + # + # quick: any non-failed prior ImportTask (active OR finished) is + # skipped — quick scan only does new-file enqueue, so re-touching + # already-imported files is wasted work. + # + # deep: ONLY currently-in-flight tasks (pending/queued/processing) + # are skipped. Completed and skipped tasks ARE re-queued because + # deep scan exists precisely to re-touch already-imported files + # (refresh sidecar metadata, fill NULL phash, fill NULL artist + # via Importer._deep_rederive). Matches IR's deep-scan behavior. + active_statuses = ["pending", "queued", "processing"] + if mode == "deep": + skip_statuses = active_statuses + else: + skip_statuses = active_statuses + ["complete", "skipped"] non_failed_existing = set(session.execute( select(ImportTask.source_path).where( - ImportTask.status.in_( - ["pending", "queued", "processing", "complete", "skipped"] - ), + ImportTask.status.in_(skip_statuses), ) ).scalars().all()) diff --git a/frontend/src/components/settings/ImportTriggerPanel.vue b/frontend/src/components/settings/ImportTriggerPanel.vue index 963d456..8873d92 100644 --- a/frontend/src/components/settings/ImportTriggerPanel.vue +++ b/frontend/src/components/settings/ImportTriggerPanel.vue @@ -10,6 +10,9 @@ {{ store.activeBatch.scan_mode === 'deep' ? 'Deep scanning' : 'Scanning' }} {{ store.activeBatch.source_path || '/import' }} — imported {{ store.activeBatch.imported }}, + skipped {{ store.activeBatch.skipped }}, failed {{ store.activeBatch.failed }} / {{ store.activeBatch.total_files }} files @@ -26,11 +29,12 @@

Quick scan walks /import and enqueues - new files only (skips paths already on a non-failed ImportTask). - Deep scan additionally chains a phash backfill - across the existing library — use after bulk-imports to catch - near-duplicates that slipped through. Both modes route non-media - + sidecar pairs through PostAttachment capture. + new files only. + Deep scan additionally re-walks already-imported + files so updated sidecar metadata (post title/date/attribution) and + previously-NULL phashes / artist links get refreshed. Use after + bulk-downloading fresh sidecars for existing content. Both modes + route non-media + sidecar pairs through PostAttachment capture. An active batch is in progress. Wait for it to finish, or click diff --git a/frontend/src/stores/import.js b/frontend/src/stores/import.js index 6b4dd04..64e5146 100644 --- a/frontend/src/stores/import.js +++ b/frontend/src/stores/import.js @@ -66,21 +66,42 @@ export const useImportStore = defineStore('import', () => { // batch flashes 'running' for <100ms then 'complete' before the // first refreshStatus() lands; UI never sees the active state). const label = mode === 'deep' - ? 'Deep scan triggered (pHash backfill chained)' + ? 'Deep scan triggered (re-applying sidecar metadata + filling NULL phash/artist on existing rows)' : mode === 'verify' ? 'Library verify triggered' : 'Quick scan triggered' window.__fcToast?.({ text: label, type: 'success' }) await refreshStatus() - // Re-poll twice over ~5s to catch quick-finalize transitions and - // surface a result toast either way. Skip the "no new files" hint - // for 'verify' since it doesn't walk the import_root. + // Re-poll twice over ~5s and produce an HONEST follow-up toast. + // Operator-flagged 2026-05-25: the prior "no new files" message was + // misleading because deep scan IS doing work (refresh) even when + // there are no new files to import. Surface the real workload count + // (imported + refreshed + queued) instead. For quick scan + zero + // queued work, fall back to "up to date" instead of the old + // implementation-detail-leaking message. setTimeout(async () => { await refreshStatus() await loadTasks(true) - if (!activeBatch.value && mode !== 'verify') { + if (activeBatch.value || mode === 'verify') return + // Batch finalized quickly; figure out what actually happened. + // The task list was just refreshed; the freshest row(s) carry + // the batch outcome. + const batchId = tasks.value[0]?.batch_id + const sameBatch = batchId + ? tasks.value.filter(t => t.batch_id === batchId) + : [] + const refreshedCount = sameBatch.filter(t => t.status === 'complete' && t.result_image_id).length + const newImported = sameBatch.filter(t => t.status === 'complete' && t.result_image_id && !t.error).length + if (mode === 'deep' && sameBatch.length > 0) { window.__fcToast?.({ - text: 'Scan complete — no new files (everything already on an ImportTask row)', + text: `Deep scan finished — ${sameBatch.length} file(s) processed`, type: 'info', }) + } else if (mode === 'quick' && sameBatch.length > 0) { + window.__fcToast?.({ + text: `Quick scan finished — ${newImported} new file(s) queued`, + type: 'info', + }) + } else { + window.__fcToast?.({ text: 'Library is up to date', type: 'info' }) } }, 2000) } catch (e) { diff --git a/tests/test_importer_deep.py b/tests/test_importer_deep.py index b94f722..c54e73d 100644 --- a/tests/test_importer_deep.py +++ b/tests/test_importer_deep.py @@ -64,9 +64,13 @@ def test_deep_rederives_phash_and_provenance(db_sync, import_layout): deep = _mk(db_sync, import_layout, deep=True) r2 = deep.import_one(src) - assert r2.status == "skipped" - assert "deep" in (r2.error or "") + # Outcome flipped from "skipped+duplicate_hash" to "refreshed" 2026-05-25 + # so the UI can surface deep scan's actual work instead of showing it as + # a no-op. See ImportResult.status comment + _deep_rederive docstring. + assert r2.status == "refreshed" assert r2.image_id == rec.id + assert r2.skip_reason is None + assert r2.error is None db_sync.expire_all() rec2 = db_sync.get(ImageRecord, rec.id) diff --git a/tests/test_phash_dedup.py b/tests/test_phash_dedup.py index 55fde03..83b5725 100644 --- a/tests/test_phash_dedup.py +++ b/tests/test_phash_dedup.py @@ -241,3 +241,9 @@ def test_import_task_maps_superseded_to_complete_and_requeues(): assert _map_result_to_status( ImportResult(status="failed", error="boom") ) == ("failed", False) + # Refreshed (deep scan): complete + no ML/thumb re-derive (pixels + # unchanged). Added 2026-05-25 alongside ImportBatch.refreshed + # counter so deep scan reports "X refreshed" instead of "no work". + assert _map_result_to_status( + ImportResult(status="refreshed", image_id=5) + ) == ("complete", False) diff --git a/tests/test_scan_deep_requeue.py b/tests/test_scan_deep_requeue.py new file mode 100644 index 0000000..ff7d2e5 --- /dev/null +++ b/tests/test_scan_deep_requeue.py @@ -0,0 +1,109 @@ +"""Deep scan re-queues already-completed ImportTasks. + +Operator-flagged 2026-05-25: deep scan used to skip everything that +already had a non-failed ImportTask row, making a deep re-scan a no-op +when no new files were added. That defeated the entire point of deep +scan (re-apply sidecar metadata to existing rows). The skip-set now +splits by mode — quick keeps the old "any non-failed" semantics; deep +skips ONLY actively-in-flight statuses. +""" + +import pytest +from PIL import Image +from sqlalchemy import func, select + +from backend.app.models import ImportBatch, ImportSettings, ImportTask +from backend.app.tasks.scan import scan_directory + +pytestmark = pytest.mark.integration + + +def _img(path): + path.parent.mkdir(parents=True, exist_ok=True) + Image.new("RGB", (40, 40), (10, 200, 80)).save(path, "JPEG") + + +def test_deep_scan_requeues_completed_task(db_sync, tmp_path, monkeypatch): + """Quick scan then deep scan of the same /import: the file completed + in the first run should be re-enqueued by the deep run (different + ImportTask id, same source_path).""" + import_root = tmp_path / "import" + settings = db_sync.execute( + select(ImportSettings).where(ImportSettings.id == 1) + ).scalar_one() + settings.import_scan_path = str(import_root) + db_sync.commit() + + src = import_root / "Mae" / "p.jpg" + _img(src) + + from backend.app import celery_app + + celery_app.celery.conf.task_always_eager = False # explicit + try: + first_batch_id = scan_directory.run(triggered_by="manual", mode="quick") + first_task = db_sync.execute( + select(ImportTask).where(ImportTask.batch_id == first_batch_id) + ).scalar_one() + # Simulate the worker having finished it. + first_task.status = "complete" + db_sync.commit() + + # Now deep scan: the SAME file should get a NEW ImportTask row. + second_batch_id = scan_directory.run(triggered_by="manual", mode="deep") + assert second_batch_id != first_batch_id + + new_tasks_in_second_batch = db_sync.execute( + select(func.count()) + .select_from(ImportTask) + .where(ImportTask.batch_id == second_batch_id) + ).scalar_one() + assert new_tasks_in_second_batch == 1, ( + "deep scan did not re-queue the completed file" + ) + + # And the second batch's task should be for the same source_path + # as the first (proves it's a re-queue, not a different file). + sp = db_sync.execute( + select(ImportTask.source_path) + .where(ImportTask.batch_id == second_batch_id) + ).scalar_one() + assert sp == str(src) + finally: + celery_app.celery.conf.task_always_eager = False + + +def test_quick_scan_does_not_requeue_completed_task(db_sync, tmp_path): + """The flip side: quick scan still keeps the old skip semantics — + a file with a completed ImportTask row from a prior batch is NOT + re-enqueued on a fresh quick scan.""" + import_root = tmp_path / "import" + settings = db_sync.execute( + select(ImportSettings).where(ImportSettings.id == 1) + ).scalar_one() + settings.import_scan_path = str(import_root) + db_sync.commit() + + src = import_root / "Mae" / "q.jpg" + _img(src) + + first_batch_id = scan_directory.run(triggered_by="manual", mode="quick") + first_task = db_sync.execute( + select(ImportTask).where(ImportTask.batch_id == first_batch_id) + ).scalar_one() + first_task.status = "complete" + db_sync.commit() + + second_batch_id = scan_directory.run(triggered_by="manual", mode="quick") + second_batch_count = db_sync.execute( + select(func.count()) + .select_from(ImportTask) + .where(ImportTask.batch_id == second_batch_id) + ).scalar_one() + assert second_batch_count == 0, ( + "quick scan re-queued an already-completed task — should have skipped" + ) + + # And the second batch should self-finalize (files_seen=0). + second_batch = db_sync.get(ImportBatch, second_batch_id) + assert second_batch.status == "complete"