From a32e66729bc77e43357235b2764897d91c955d41 Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Wed, 21 Oct 2026 14:55:11 -0700 Subject: [PATCH] feat(activity): scan new and unresolved files by default, all files on request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scan for metadata now skips, before reading any metadata, the files the Library already links with a resolved link (one generated_images row with exactly one pair link), found with one streamed query that keeps only the scanned paths and compares paths with os.path.normcase on both sides. Skipped files count as Already known ("Skipped · already linked"). Unlinked, fallback-linked, ambiguous and unresolved files are read again. POST /prompt_manager/jobs/scan takes {scope: "new" | "all"} (default "new", anything else is a 400). The review dialog shows the concept's "New and unresolved files" and adds an "All files" switch (D58). The legacy /prompt_manager/scan route keeps scanning all files with unchanged SSE payloads. --- TESTING.md | 29 ++- database/operations.py | 49 ++++ py/api/admin.py | 69 ++++-- py/api/job_routes.py | 34 ++- py/api/jobs.py | 7 + tests/browser/activity-concept.spec.mjs | 57 ++++- tests/browser/workspace-activity.spec.mjs | 52 +++++ tests/browser/workspace-images.spec.mjs | 2 +- tests/test_jobs.py | 5 + tests/test_scan_scope.py | 263 ++++++++++++++++++++++ web/js/workspace/activity-scan.mjs | 14 +- web/js/workspace/workspace-dialogs.mjs | 36 ++- 12 files changed, 568 insertions(+), 49 deletions(-) create mode 100644 tests/test_scan_scope.py diff --git a/TESTING.md b/TESTING.md index 8069def..8e44834 100644 --- a/TESTING.md +++ b/TESTING.md @@ -550,8 +550,9 @@ Recorded shell deviations: configured sources.", button "Review scan"). It opens the concept's `scan-review`, "Review metadata scan": no sample-plan `.demo-note` (prototype copy); Sources and Scope in a `dl.meta-grid` (the concept's maintenance plan uses `dl`; `scan-review` - uses a `div`) reading "N output folders" (the discovered sources) and "All files" - (fixed in Activity-1; Activity-2 adds the scope option); the concept's sentence; then + uses a `div`) reading "N output folders" (the discovered sources) and the concept's + "New and unresolved files" (the D58 checkbox switches it to "All files"); the + concept's sentence; then a backup `.notice` (the concept's System notice box): `h3` "Back up the database first (recommended)", "Downloads a copy of the prompts database before the scan changes any records." and "Back up now", which downloads the existing `/prompt_manager/backup` @@ -624,6 +625,30 @@ Recorded shell deviations: is running: ." with the title linking that job's view (the shared toast now accepts link parts). A toast with a link stays 20 s instead of 6.5 s, and any toast stays while the pointer or focus is on it. +- **D58** (scan scope, Activity-2 Task 4): the concept's `scan-review` states a fixed + scope, "New and unresolved files". Production keeps that value as the default and adds + the concept's own System toggle after the `dl.meta-grid`: a `.setting-row` with the + label "All files", the hint "Also read the files already linked to their prompt + again." and the Toolkit switch (`label.ca-switch`, a checkbox with `role="switch"`, + as the concept's `hydrateSwitches` renders it), unchecked. Checking it switches the Scope value to "All files"; Start scan sends + `POST /prompt_manager/jobs/scan {scope: "new" | "all"}` (another value is a 400). In + "new" the scan skips, before reading any metadata, the files the Library already links + with a resolved link: exactly one `generated_images` row for the file (by + `normalize_image_path`, `os.path.normcase` on both sides) and exactly one distinct + pair link on it (`image_prompt_pairs`, written only by resolved ingestion). Files + linked without a pair (the monitor's fallback and FIFO links, the legacy positive-only + import), linked to more than one prompt or pair, or not linked are read again. The + known set is one streamed query that keeps only the scanned paths. A skipped file + counts as processed and unchanged in the legacy counts and under Already known, with + the detail "Skipped · already linked" and the log line "= <root>/<path> · Skipped · + already linked"; the log's first line names the scope ("Scan started · N output + folders · new and unresolved files" or "· all files"). "All files" reads every file, + as the scan did in Activity-1, and the legacy `/prompt_manager/scan` route always does + (its SSE payloads are unchanged). `activity-concept.spec.mjs` compares the switch with + the concept's System → Sources toggle ("Watch for new outputs"), without its + place-dependent top margin; `workspace-activity.spec.mjs` covers the Scope value and + the request both ways; `tests/test_scan_scope.py` covers the skip, the re-read of + unresolved files, "all", the legacy route, the 400 and Windows case folding. - **D29**: no prototype copy. There is no "Interactive concept" badge and no "What changes & why" button. The footer reads "ComfyAssets · Local tools. Your library.", and the title has no " concept" suffix. Save view's dialog action reads diff --git a/database/operations.py b/database/operations.py index 2584498..2694522 100644 --- a/database/operations.py +++ b/database/operations.py @@ -181,6 +181,19 @@ AND p.id NOT IN (SELECT prompt_id FROM live_hint_prompts) """ +# The scan's known set (#249 Activity-2): every image row, with whether its pair +# link is resolved: exactly one distinct pair, the rule _LINK_SUMMARY_SQL and +# get_image_prompt_info use. A file is known and resolved when it has exactly one +# image row and that row is resolved; several rows (one file linked to more than +# one prompt) are ambiguous. The rows are streamed in fetches of +# KNOWN_PATHS_FETCH_SIZE and only the scanned paths are kept, so memory follows +# the scan (at most MAX_SCAN_FILES paths), not the table. +_KNOWN_PATHS_SQL = """SELECT gi.file_path, gi.image_path, + (SELECT COUNT(DISTINCT ip.pair_id) FROM image_prompt_pairs ip + WHERE ip.image_id = gi.id) = 1 +FROM generated_images gi""" +KNOWN_PATHS_FETCH_SIZE = 1000 + # One SELECT reads the link summaries of at most this many paths: three bound # parameters per path stays under SQLite's historical 999-variable limit. LINK_SUMMARY_BATCH = 200 @@ -2280,6 +2293,42 @@ class PromptDatabase( "parameters": row[8] if len(row) > 8 else None, } + def get_known_resolved_paths(self, paths: List[str]) -> Set[str]: + """The ``paths`` the Library already links with a resolved link. + + A path qualifies when exactly one ``generated_images`` row claims the + file and that row has exactly one pair link (``image_prompt_pairs``, + written only by resolved ingestion). Files linked without a pair (the + monitor's fallback and FIFO links, the legacy positive-only import), + linked to several prompts or pairs, or not linked at all do not. + Paths compare by ``normalize_image_path`` and ``os.path.normcase`` on + both sides, so a Windows row stored in another case still matches. + One query, streamed; returns the matching items of ``paths``. + """ + wanted: Dict[str, List[str]] = {} + for path in paths: + key = os.path.normcase(normalize_image_path(str(path))) + wanted.setdefault(key, []).append(str(path)) + if not wanted: + return set() + rows_seen: Dict[str, int] = {} + resolved: Set[str] = set() + with self.model.get_connection() as conn: + cursor = conn.execute(_KNOWN_PATHS_SQL) + while True: + rows = cursor.fetchmany(KNOWN_PATHS_FETCH_SIZE) + if not rows: + break + for file_path, image_path, is_resolved in rows: + stored = file_path or normalize_image_path(image_path) + key = os.path.normcase(stored) + if key not in wanted: + continue + rows_seen[key] = rows_seen.get(key, 0) + 1 + if is_resolved: + resolved.add(key) + return {path for key in resolved if rows_seen[key] == 1 for path in wanted[key]} + def get_image_link_summaries( self, paths: List[str], allow_filename_fallback: bool = True ) -> Dict[str, Dict[str, Any]]: diff --git a/py/api/admin.py b/py/api/admin.py index 8519cd1..62925d5 100644 --- a/py/api/admin.py +++ b/py/api/admin.py @@ -17,7 +17,14 @@ from pathlib import Path from aiohttp import web from .prompts import thumbnail_url_for -from .jobs import SCAN_GROUP_NAMES, SCAN_GROUPS, Job, JobBusy +from .jobs import ( + DEFAULT_SCAN_SCOPE, + SCAN_GROUP_NAMES, + SCAN_GROUPS, + SCAN_SCOPES, + Job, + JobBusy, +) try: from ...utils.parallel import map_parallel @@ -150,6 +157,8 @@ SCAN_FINAL_STATES = { "cancelled": "cancelled", "error": "failed", } +# The scope as the scan log's first line names it (the review dialog's words). +SCAN_SCOPE_LOG = {"new": "new and unresolved files", "all": "all files"} MIN_BATCHES_FOR_ESTIMATE = 2 # "about X min left" waits for this many batches # Log line prefix per outcome group (spec: + added, = known, ? review, ! error, # - no usable metadata). @@ -174,6 +183,7 @@ SCAN_DETAILS = { "linked": "Linked to an existing prompt", "enriched": "Metadata added to its prompt", "unchanged": "Already linked to its prompt", + "skipped": "Skipped · already linked", "ambiguous": "Needs review: more than one prompt could match", "unresolved": "Needs review: the prompt could not be traced in the workflow", "conflict": "Needs review: already linked to a different prompt", @@ -1426,7 +1436,7 @@ class AdminRoutesMixin: job = self._jobs.active(SCAN_JOB_KIND) if job is None: try: - job = self._start_scan_job() + job = self._start_scan_job("all") except JobBusy as busy: return self._long_job_busy_response(busy.running.title) return await self._stream_scan_job(request, job) @@ -1450,16 +1460,19 @@ class AdminRoutesMixin: } ) - def _start_scan_job(self): + def _start_scan_job(self, files=DEFAULT_SCAN_SCOPE): """Register the scan job synchronously; its runner starts on the loop. - Raises JobBusy while another record-changing job runs. + ``files`` is the scope (``SCAN_SCOPES``). Raises JobBusy while another + record-changing job runs. """ + if files not in SCAN_SCOPES: + raise ValueError(f"Unknown scan scope: {files!r}") job = self._jobs.start( SCAN_JOB_KIND, SCAN_JOB_TITLE, record_changing=True, - scope={"mode": "all"}, + scope={"files": files}, runner=self._run_scan_job, groups=SCAN_GROUP_NAMES, ) @@ -1525,7 +1538,8 @@ class AdminRoutesMixin: dir_names = [self._public_path(d) for d in output_dirs] folders = _plural(len(output_dirs), "output folder") job.scope["output_folders"] = len(output_dirs) - job.log_line(f"Scan started · {folders}") + scope = SCAN_SCOPE_LOG[job.scope.get("files", "all")] + job.log_line(f"Scan started · {folders} · {scope}") job.update(status_line=f"Looking for media files in {folders}") yield self._scan_progress( 0, f"Scanning {len(output_dirs)} directory(ies) for media files..." @@ -1544,7 +1558,14 @@ class AdminRoutesMixin: 5, f"Found {len(media_files)} media files to process...", counts ) roots = _scan_roots(output_dirs) - async for event in self._scan_batches(media_files, counts, job, roots): + known = set() + if media_files and job.scope.get("files") != "all": + known = await self._run_in_executor( + self.db.get_known_resolved_paths, [str(f) for f in media_files] + ) + async for event in self._scan_batches( + media_files, counts, job, roots, known + ): yield event self.logger.info( @@ -1565,11 +1586,12 @@ class AdminRoutesMixin: "message": "An internal error occurred. Check server logs for details.", } - async def _scan_batches(self, media_files, counts, job, roots=()): + async def _scan_batches(self, media_files, counts, job, roots=(), known=()): """Process ``media_files`` in executor batches, updating ``counts`` in place. Records each file's outcome in ``job`` and yields one progress event - per batch. + per batch. Files in ``known`` (path strings) are not read: they count + as processed and unchanged and are recorded as skipped. """ total = len(media_files) started = time.monotonic() @@ -1578,18 +1600,23 @@ class AdminRoutesMixin: if job.cancelled: return batch = media_files[batch_start : batch_start + SCAN_BATCH_SIZE] - batch_results = await self._run_in_executor( - self._extract_batch_metadata_sync, batch - ) - outcomes = [] - batch_counts = await self._run_in_executor( - self._ingest_scan_batch_sync, - batch_results, - roots=roots, - outcomes=outcomes, - ) - for key in counts: - counts[key] += batch_counts[key] + skipped = [f for f in batch if str(f) in known] + to_read = [f for f in batch if str(f) not in known] + outcomes = [_scan_outcome(f, roots, "skipped") for f in skipped] + counts["processed"] += len(skipped) + counts["unchanged"] += len(skipped) + if to_read: + batch_results = await self._run_in_executor( + self._extract_batch_metadata_sync, to_read + ) + batch_counts = await self._run_in_executor( + self._ingest_scan_batch_sync, + batch_results, + roots=roots, + outcomes=outcomes, + ) + for key in counts: + counts[key] += batch_counts[key] for group, row, log_line in outcomes: job.record(group, row, log_line) diff --git a/py/api/job_routes.py b/py/api/job_routes.py index 1d63eb4..0b8b936 100644 --- a/py/api/job_routes.py +++ b/py/api/job_routes.py @@ -10,13 +10,34 @@ import json from aiohttp import web -from .jobs import JobBusy +from .jobs import DEFAULT_SCAN_SCOPE, SCAN_SCOPES, JobBusy def _not_found(): return web.json_response({"success": False, "error": "Job not found"}, status=404) +def _bad_request(message): + return web.json_response({"success": False, "error": message}, status=400) + + +async def _scan_scope(request): + """The requested scan scope ("new" or "all"), or None when the body is invalid. + + An empty body, or one without ``scope``, asks for the default. + """ + if not request.can_read_body: + return DEFAULT_SCAN_SCOPE + try: + body = await request.json() + except ValueError: + return None + if not isinstance(body, dict): + return None + scope = body.get("scope", DEFAULT_SCAN_SCOPE) + return scope if scope in SCAN_SCOPES else None + + def _summary_event(job): return f"data: {json.dumps(job.summary())}\n\n".encode("utf-8") @@ -65,9 +86,16 @@ class JobRoutesMixin: return web.json_response({"success": True, "cancellation_requested": requested}) async def start_scan_job(self, request): - """Start the metadata scan as a job (202), or 409 naming the busy job.""" + """Start the metadata scan as a job (202), or 409 naming the busy job. + + The body's ``scope`` is "new" (the default: new and unresolved files) + or "all"; anything else is a 400. + """ + files = await _scan_scope(request) + if files is None: + return _bad_request('The scan scope must be "new" or "all"') try: - job = self._start_scan_job() + job = self._start_scan_job(files) except JobBusy as busy: running = busy.running return web.json_response( diff --git a/py/api/jobs.py b/py/api/jobs.py index 4918e4c..d37dfbd 100644 --- a/py/api/jobs.py +++ b/py/api/jobs.py @@ -35,12 +35,19 @@ MAX_FINISHED_JOBS = 20 # finished jobs this session keeps # The metadata scan's outcome groups, in counter-tile order of the spec's # mapping, and the per-file scan status each group collects. Files without # usable metadata have no status and go to "no_metadata". +# Which files a scan reads (#249 Activity-2): "new" skips the files the Library +# already links with a resolved link (get_known_resolved_paths) before reading +# them, so new, unlinked, unresolved and ambiguous files are read; "all" reads +# every file again. The workspace job defaults to "new"; legacy /scan is "all". +SCAN_SCOPES = ("new", "all") +DEFAULT_SCAN_SCOPE = "new" SCAN_GROUP_NAMES = ("added", "known", "review", "no_metadata", "errors") SCAN_GROUPS = { "added": "added", "linked": "added", "enriched": "added", "unchanged": "known", + "skipped": "known", # "new" scope: known and resolved, not read again "ambiguous": "review", "conflict": "review", "unresolved": "review", diff --git a/tests/browser/activity-concept.spec.mjs b/tests/browser/activity-concept.spec.mjs index 4d6d37b..e3c8f9f 100644 --- a/tests/browser/activity-concept.spec.mjs +++ b/tests/browser/activity-concept.spec.mjs @@ -63,16 +63,35 @@ function measureNotice(selector) { }; } -/** D55: the concept's dialog with the production differences applied. */ +/** A concept `.setting-row` switch: its frame, its label and hint, and its checkbox (not its place-dependent margin). */ +function measureToggle(selector) { + const row = document.querySelector(selector); + const css = (node, ...keys) => node ? Object.fromEntries(keys.map(key => [key, getComputedStyle(node)[key]])) : null; + return { + children: [...row.children].map(node => [node.localName, ...node.classList].join('.')), + frame: css(row, 'display', 'alignItems', 'justifyContent', 'columnGap', 'paddingTop', 'paddingBottom', + 'borderTopWidth', 'borderTopStyle', 'borderTopColor'), + label: css(row.querySelector('label'), 'fontSize', 'fontWeight', 'color'), + hint: css(row.querySelector('p'), 'fontSize', 'color', 'marginTop'), + box: css(row.querySelector('input[type="checkbox"]'), 'accentColor', 'width', 'height', 'appearance', + 'backgroundColor', 'borderTopLeftRadius'), + role: row.querySelector('input').getAttribute('role'), + labelled: row.querySelector('label').htmlFor === row.querySelector('input').id, + }; +} + +/** D55 and D58: the concept's dialog with the production differences applied. */ function mapReviewConcept(reference) { return { ...reference, - // No sample-plan note (prototype copy); the backup box sits before the actions. + // No sample-plan note (prototype copy); the D58 "All files" toggle follows the grid; the backup box sits + // before the actions. body: [...reference.body.filter(item => item !== 'p.demo-note').slice(0, -1), 'div.notice', reference.body.at(-1)] - .map(item => item === 'div.meta-grid' ? 'dl.meta-grid' : item), - // A `dl`, as in the concept's maintenance plan; real sources and the fixed Activity-1 scope. + .flatMap(item => item === 'div.meta-grid' ? ['dl.meta-grid', 'div.setting-row'] : [item]), + // A `dl`, as in the concept's maintenance plan; real sources. The scope keeps the concept's + // "New and unresolved files" until "All files" is checked. grid: { ...reference.grid, tag: 'dl', entries: reference.grid.entries.map(entry => ({ ...entry, - dd: { Sources: '2 output folders', Scope: 'All files' }[entry.dt] })) }, + dd: { Sources: '2 output folders', Scope: 'New and unresolved files' }[entry.dt] })) }, actions: { ...reference.actions, buttons: reference.actions.buttons.map(button => button.text === 'Simulate scan' ? { ...button, text: 'Start scan' } : button) }, }; @@ -92,7 +111,8 @@ async function conceptReview(page, theme) { await page.getByRole('combobox', { name: 'Theme', exact: true }).selectOption(theme); await page.locator('#main').getByRole('button', { name: 'Manage sources', exact: true }).click(); await expect(page.locator('#main .notice')).toBeVisible(); - return { review, notice: await page.evaluate(measureNotice, '#main .notice') }; + return { review, notice: await page.evaluate(measureNotice, '#main .notice'), + toggle: await page.evaluate(measureToggle, '#main .setting-row:has(input[type="checkbox"])') }; } async function productionReview(page, theme) { @@ -103,7 +123,16 @@ async function productionReview(page, theme) { await expect(page.locator('dialog#modal[open] #modal-title')).toHaveText('Review metadata scan'); // The swapped dialog can open under the pointer that pressed Review scan; measure it at rest. await page.mouse.move(0, 0); - return { review: await page.evaluate(measureReview), notice: await page.evaluate(measureNotice, 'dialog#modal[open] .notice') }; + const review = await page.evaluate(measureReview); + const notice = await page.evaluate(measureNotice, 'dialog#modal[open] .notice'); + // The concept's switch is on; compare ours on too, then turn it off again (the default). + const allFiles = page.getByRole('switch', { name: 'All files', exact: true }); + await allFiles.check(); + await page.mouse.move(0, 0); + const toggle = await page.evaluate(measureToggle, 'dialog#modal[open] .setting-row'); + await allFiles.uncheck(); + await page.mouse.move(0, 0); + return { review, notice, toggle }; } for (const theme of THEMES) test(`review dialog matches the concept scan-review ${theme}`, async ({ page, request }) => { @@ -114,19 +143,29 @@ for (const theme of THEMES) test(`review dialog matches the concept scan-review const production = await productionReview(page, theme); expect(diffInventory(mapReviewConcept(concept.review), production.review)).toEqual([]); expect(diffInventory(concept.notice, production.notice)).toEqual([]); + expect(diffInventory(concept.toggle, production.toggle)).toEqual([]); + await expect(page.locator('dialog#modal[open] .setting-row > div > label')).toHaveText('All files'); + await expect(page.locator('dialog#modal[open] .setting-row input')).not.toBeChecked(); + await expect(page.getByRole('switch', { name: 'All files', exact: true })).toBeVisible(); await expect(page.locator('dialog#modal[open] .notice h3')).toHaveText('Back up the database first (recommended)'); await expect(page.locator('dialog#modal[open] .notice .row-actions button')).toHaveText(['Back up now']); // Mutation check: one CSS change and one removed element are both noticed, then undone to an exact match again. - const undoCss = await mutate(page, { css: '#modal .meta-grid { gap: 3px; } #modal .notice { padding: 2px; }' }); + const undoCss = await mutate(page, { css: '#modal .meta-grid { gap: 3px; } #modal .notice { padding: 2px; } #modal .setting-row { gap: 3px; }' }); const cssMutated = await page.evaluate(measureReview); const noticeMutated = await page.evaluate(measureNotice, 'dialog#modal[open] .notice'); + await page.getByRole('switch', { name: 'All files', exact: true }).check(); + await page.mouse.move(0, 0); + const toggleMutated = await page.evaluate(measureToggle, 'dialog#modal[open] .setting-row'); + await page.getByRole('switch', { name: 'All files', exact: true }).uncheck(); + await page.mouse.move(0, 0); + expect(diffInventory(concept.toggle, toggleMutated).map(item => item.path)).toEqual(['frame.columnGap']); expect(diffInventory(mapReviewConcept(concept.review), cssMutated).map(item => item.path)).toEqual(expect.arrayContaining(['grid.columnGap', 'grid.rowGap'])); expect(diffInventory(concept.notice, noticeMutated).map(item => item.path)).toEqual(['frame.paddingTop', 'frame.paddingLeft']); await undoCss(); const undoDom = await mutate(page, { remove: ['#modal-body > p.small'] }); const domMutated = await page.evaluate(() => [...document.querySelectorAll('#modal-body > *')].map(node => node.localName)); - expect(domMutated).toEqual(['dl', 'div', 'div']); + expect(domMutated).toEqual(['dl', 'div', 'div', 'div']); await undoDom(); expect(diffInventory(mapReviewConcept(concept.review), await page.evaluate(measureReview))).toEqual([]); }); diff --git a/tests/browser/workspace-activity.spec.mjs b/tests/browser/workspace-activity.spec.mjs index f61eac5..0edca2e 100644 --- a/tests/browser/workspace-activity.spec.mjs +++ b/tests/browser/workspace-activity.spec.mjs @@ -392,6 +392,58 @@ test('start scan creates a job, toasts, and opens its view', async ({ page, requ await expect(scanLog(page).locator('pre')).toContainText('Scan finished'); }); +// ── Task 4 (Activity-2): scan scope, new and unresolved files by default ───────────────────── + +test('all files checkbox changes the scope line and the request', async ({ page }) => { + await page.setViewportSize({ width: 1600, height: 1100 }); + const bodies = []; + page.on('request', item => { + if (item.method() === 'POST' && new URL(item.url()).pathname === '/prompt_manager/jobs/scan') bodies.push(item.postDataJSON()); + }); + const review = page.getByRole('dialog', { name: 'Review metadata scan', exact: true }); + const scopeValue = review.locator('.meta-grid > div').filter({ has: page.locator('dt', { hasText: 'Scope' }) }).locator('dd'); + // The concept's switch: a checkbox with role=switch. + const allFiles = review.getByRole('switch', { name: 'All files', exact: true }); + const openReview = async () => { + await page.goto(`${workspace}#/images`); + await page.getByRole('button', { name: 'Import', exact: true }).click(); + await page.getByRole('dialog', { name: 'Import & inspect', exact: true }).locator('.operation') + .filter({ hasText: 'Scan for metadata' }).getByRole('button', { name: 'Review scan', exact: true }).click(); + await expect(review).toBeVisible(); + }; + + // The default: the concept's "New and unresolved files", the box unchecked; the request asks for "new". + await openReview(); + await expect(scopeValue).toHaveText('New and unresolved files'); + await expect(allFiles).not.toBeChecked(); + await expect(review.locator('.setting-row p')).toHaveText('Also read the files already linked to their prompt again.'); + // Checking and unchecking switches the Scope value both ways. + await allFiles.check(); + await expect(scopeValue).toHaveText('All files'); + await allFiles.uncheck(); + await expect(scopeValue).toHaveText('New and unresolved files'); + await review.getByRole('button', { name: 'Start scan', exact: true }).click(); + await expect(page).toHaveURL(/#\/activity\/[0-9a-f]{32}$/); + await expect.poll(() => bodies.length).toBe(1); + expect(bodies[0]).toEqual({ scope: 'new' }); + await expect(heading(page).locator('h1')).toHaveText('Scan for metadata'); + // Wait for the first scan to end so the second is not refused as busy. + await scanLog(page).locator('summary').click(); + await expect(scanLog(page).locator('pre')).toContainText('Scan finished', { timeout: 15000 }); + await expect(scanLog(page).locator('pre')).toContainText('new and unresolved files'); + + // Checked: "All files", and the request asks for "all". + await openReview(); + await allFiles.check(); + await expect(scopeValue).toHaveText('All files'); + await review.getByRole('button', { name: 'Start scan', exact: true }).click(); + await expect.poll(() => bodies.length).toBe(2); + expect(bodies[1]).toEqual({ scope: 'all' }); + await scanLog(page).locator('summary').click(); + await expect(scanLog(page).locator('pre')).toContainText('Scan started · ', { timeout: 15000 }); + await expect(scanLog(page).locator('pre')).toContainText('all files'); +}); + test('job view counters follow the mapping and update live', async ({ page, request }) => { await page.setViewportSize({ width: 1600, height: 1100 }); const job = await startJob(request); diff --git a/tests/browser/workspace-images.spec.mjs b/tests/browser/workspace-images.spec.mjs index 0e799ed..99dbbba 100644 --- a/tests/browser/workspace-images.spec.mjs +++ b/tests/browser/workspace-images.spec.mjs @@ -2060,7 +2060,7 @@ test('scan for metadata opens the review dialog from Import', async ({ page, req await openScanReview(page); const dialog = scanReview(page); await expect(dialog.locator('dl.meta-grid > div > dt')).toHaveText(['Sources', 'Scope']); - await expect(dialog.locator('dl.meta-grid > div > dd')).toHaveText(['2 output folders', 'All files']); + await expect(dialog.locator('dl.meta-grid > div > dd')).toHaveText(['2 output folders', 'New and unresolved files']); await expect(dialog.locator('#modal-body > p.small')).toHaveText('Enrich positive / negative associations, reuse canonical text and report unchanged files. Do not inflate run counts or recreate existing thumbnails.'); await expect(dialog.locator('.notice h3')).toHaveText('Back up the database first (recommended)'); await expect(dialog.locator('.dialog-actions button')).toHaveText(['Cancel', 'Start scan']); diff --git a/tests/test_jobs.py b/tests/test_jobs.py index 9dffb42..dfedf8a 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -13,12 +13,14 @@ import unittest sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) from py.api.jobs import ( # noqa: E402 + DEFAULT_SCAN_SCOPE, LOG_CAP_LINE, MAX_FINISHED_JOBS, MAX_LOG_LINES, MAX_OUTCOME_ROWS, SCAN_GROUP_NAMES, SCAN_GROUPS, + SCAN_SCOPES, Job, JobBusy, JobRegistry, @@ -395,12 +397,15 @@ class TestJobOutcomesAndLog(unittest.TestCase): "linked": "added", "enriched": "added", "unchanged": "known", + # Activity-2: a file the "new" scope skipped is Already known. + "skipped": "known", "ambiguous": "review", "conflict": "review", "unresolved": "review", "failed": "errors", }, ) + self.assertEqual((SCAN_SCOPES, DEFAULT_SCAN_SCOPE), (("new", "all"), "new")) self.assertEqual( SCAN_GROUP_NAMES, ("added", "known", "review", "no_metadata", "errors") ) diff --git a/tests/test_scan_scope.py b/tests/test_scan_scope.py new file mode 100644 index 0000000..d41f19b --- /dev/null +++ b/tests/test_scan_scope.py @@ -0,0 +1,263 @@ +"""Scan scope (#249 Activity-2 Task 4): new and unresolved files by default. + +A "Scan for metadata" job started without ``scope`` (or with ``"new"``) skips +the files the Library already links with a resolved link, before their +metadata is read, and counts them as Already known. ``"all"`` reads every file +again, as the legacy ``/prompt_manager/scan`` route always does. +""" + +import asyncio +import json +import os +import sqlite3 +import unittest +from unittest.mock import patch + +try: + from tests.test_api_admin import AdminAPITestCase, _sse_payloads, _write_png +except ImportError: # discovered with tests/ as the top-level directory + from test_api_admin import AdminAPITestCase, _sse_payloads, _write_png + +from database import operations # noqa: E402 +from database.pair_ingestion import ingest_resolved_pair # noqa: E402 +from py.config import GalleryConfig # noqa: E402 +from utils.prompt_pair_resolver import resolve_prompt_pair # noqa: E402 + +SKIPPED_DETAIL = "Skipped · already linked" + + +def _graph(text): + return { + "1": {"class_type": "CLIPTextEncode", "inputs": {"text": text}}, + "2": {"class_type": "CLIPTextEncode", "inputs": {"text": "bad"}}, + "3": { + "class_type": "KSampler", + "inputs": {"positive": ["1", 0], "negative": ["2", 0]}, + }, + } + + +class ScanScopeTestCase(AdminAPITestCase): + """Helpers: link files the way the scan and the monitor do, spy on reads.""" + + def _link_resolved(self, path, text): + """``path`` linked with a resolved pair link (what a resolved scan writes).""" + metadata = {"prompt": _graph(text)} + result = ingest_resolved_pair( + self.api.db, str(path), metadata, resolve_prompt_pair(metadata) + ) + self.assertEqual(result["status"], "added") + + def _link_without_pair(self, path, text): + """``path`` linked to a prompt with no pair link (fallback / FIFO link).""" + prompt_id = self.api.db.save_prompt(text, None, [], None, None) + self.assertTrue(self.api.db.link_image_to_prompt(prompt_id, str(path))) + return prompt_id + + def _spy_reads(self): + reads = [] + original = self.api._extract_comfyui_metadata + + def spy(path): + reads.append(os.path.basename(path)) + return original(path) + + self.api._extract_comfyui_metadata = spy + return reads + + async def _scan(self, body=None): + kwargs = {} if body is None else {"json": body} + resp = await self.client.request("POST", "/prompt_manager/jobs/scan", **kwargs) + data = await resp.json() + self.assertEqual(resp.status, 202, data) + await asyncio.wait_for(self.api._scan_job.task, 10) + detail = await self.client.request( + "GET", f"/prompt_manager/jobs/{data['job']['id']}" + ) + return (await detail.json())["job"] + + +class TestScanScope(ScanScopeTestCase): + + async def test_new_scope_skips_known_resolved_files_without_reading_them(self): + GalleryConfig.MONITORING_DIRECTORIES = [self._root(self.output_dir)] + known = self.output_dir / "known.png" + _write_png(known, prompt_text="a red square") + _write_png(self.output_dir / "new.png", prompt_text="a blue circle") + self._link_resolved(known, "a red square") + reads = self._spy_reads() + + job = await self._scan() + + self.assertEqual(reads, ["new.png"]) + self.assertEqual(job["scope"]["files"], "new") + self.assertEqual(job["counters"]["known"], 1) + self.assertEqual(job["counters"]["added"], 1) + self.assertEqual(job["processed"], 2) + self.assertEqual( + job["outcomes"]["known"], + [ + { + "path": "known.png", + "root": "output", + "root_index": 0, + "detail": SKIPPED_DETAIL, + } + ], + ) + self.assertIn(f"= output/known.png · {SKIPPED_DETAIL}", "\n".join(job["log"])) + self.assertTrue( + job["log"][0].endswith( + "Scan started · 1 output folder · new and unresolved files" + ) + ) + legacy = self.api._scan_job.last_event + self.assertEqual((legacy["processed"], legacy["unchanged"]), (2, 1)) + self.assertTrue(job["status_line"].startswith("Processing file 2 of 2")) + + async def test_whole_known_batch_skips_the_executor_reads(self): + from py.api import admin + + GalleryConfig.MONITORING_DIRECTORIES = [self._root(self.output_dir)] + for name in ("a.png", "b.png"): + _write_png(self.output_dir / name, prompt_text=f"prompt {name}") + self._link_resolved(self.output_dir / name, f"prompt {name}") + _write_png(self.output_dir / "c.png", prompt_text="prompt c") + reads = self._spy_reads() + + with patch.object(admin, "SCAN_BATCH_SIZE", 1): + job = await self._scan({"scope": "new"}) + + self.assertEqual(reads, ["c.png"]) + self.assertEqual(job["counters"]["known"], 2) + self.assertEqual(job["counters"]["added"], 1) + + async def test_unresolved_files_are_rescanned_in_new_scope(self): + GalleryConfig.MONITORING_DIRECTORIES = [self._root(self.output_dir)] + names = ("fallback.png", "conflict.png", "unlinked.png") + for name in names: + _write_png(self.output_dir / name, prompt_text=f"text of {name}") + # Linked without a resolved pair link (the monitor's fallback / FIFO link). + self._link_without_pair(self.output_dir / "fallback.png", "text of fallback") + # Resolved once, then also linked to a second prompt: ambiguous. + conflict = self.output_dir / "conflict.png" + self._link_resolved(conflict, "text of conflict.png") + self._link_without_pair(conflict, "another prompt") + reads = self._spy_reads() + + job = await self._scan() + + self.assertEqual(sorted(reads), sorted(names)) + self.assertNotIn(SKIPPED_DETAIL, json.dumps(job["outcomes"])) + + async def test_all_scope_reads_everything(self): + GalleryConfig.MONITORING_DIRECTORIES = [self._root(self.output_dir)] + known = self.output_dir / "known.png" + _write_png(known, prompt_text="a red square") + self._link_resolved(known, "a red square") + reads = self._spy_reads() + + job = await self._scan({"scope": "all"}) + + self.assertEqual(reads, ["known.png"]) + self.assertEqual(job["scope"]["files"], "all") + self.assertEqual(job["counters"]["known"], 1) + self.assertNotEqual(job["outcomes"]["known"][0]["detail"], SKIPPED_DETAIL) + self.assertTrue( + job["log"][0].endswith("Scan started · 1 output folder · all files") + ) + + async def test_legacy_scan_route_reads_everything(self): + GalleryConfig.MONITORING_DIRECTORIES = [self._root(self.output_dir)] + known = self.output_dir / "known.png" + _write_png(known, prompt_text="a red square") + self._link_resolved(known, "a red square") + reads = self._spy_reads() + + resp = await self.client.request("POST", "/prompt_manager/scan", data="{}") + payloads = _sse_payloads(await resp.text()) + + self.assertEqual(reads, ["known.png"]) + self.assertEqual(self.api._scan_job.scope["files"], "all") + self.assertEqual(payloads[-1]["type"], "complete") + self.assertEqual(payloads[-1]["unchanged"], 1) + + async def test_invalid_scope_is_refused(self): + for body in ({"scope": "everything"}, {"scope": 1}, ["new"]): + with self.subTest(body=body): + resp = await self.client.request( + "POST", "/prompt_manager/jobs/scan", json=body + ) + self.assertEqual(resp.status, 400) + data = await resp.json() + self.assertFalse(data["success"]) + resp = await self.client.request( + "POST", "/prompt_manager/jobs/scan", data="not json" + ) + self.assertEqual(resp.status, 400) + self.assertIsNone(self.api._scan_job) + + +class TestKnownResolvedPaths(ScanScopeTestCase): + """``PromptDatabase.get_known_resolved_paths``: one streamed query.""" + + async def test_returns_the_given_paths_that_are_known_and_resolved(self): + known, fallback = self.output_dir / "known.png", self.output_dir / "fb.png" + self._link_resolved(known, "a red square") + self._link_without_pair(fallback, "a fallback") + asked = [str(known), str(fallback), str(self.output_dir / "new.png")] + + self.assertEqual(self.api.db.get_known_resolved_paths(asked), {str(known)}) + self.assertEqual(self.api.db.get_known_resolved_paths([]), set()) + + async def test_one_query_streamed_in_small_fetches(self): + paths = [self.output_dir / f"{index}.png" for index in range(5)] + for path in paths: + self._link_resolved(path, f"prompt {path.name}") + original = self.api.db.model.get_connection + opened = [] + + def counting(): + opened.append(1) + return original() + + self.api.db.model.get_connection = counting + with patch.object(operations, "KNOWN_PATHS_FETCH_SIZE", 2): + found = self.api.db.get_known_resolved_paths([str(p) for p in paths]) + self.assertEqual(found, {str(p) for p in paths}) + self.assertEqual(len(opened), 1) + + async def test_windows_path_case_matches_known_set(self): + stored = self.output_dir / "Sub" / "Known.PNG" + self._link_resolved(stored, "a red square") + # A row written before paths were case-folded keeps its own spelling. + with sqlite3.connect(self.api.db.model.db_path) as conn: + conn.execute( + "UPDATE generated_images SET file_path=?", (str(stored).upper(),) + ) + conn.close() + asked = str(self.output_dir / "sub" / "known.png") + + with patch("os.path.normcase", lambda path: path.lower()): + self.assertEqual(self.api.db.get_known_resolved_paths([asked]), {asked}) + if os.path.normcase("A") == "A": # a case-sensitive platform + self.assertEqual(self.api.db.get_known_resolved_paths([asked]), set()) + + +class TestSkippedOutcome(unittest.TestCase): + + def test_skipped_file_is_already_known_with_its_own_detail(self): + from py.api.admin import _scan_outcome + + root = os.path.abspath("out") + group, row, line = _scan_outcome( + os.path.join(root, "a", "b.png"), [(root, "output", 0)], "skipped" + ) + self.assertEqual(group, "known") + self.assertEqual(row["detail"], SKIPPED_DETAIL) + self.assertEqual(row["path"], "a/b.png") + self.assertEqual(line, f"= output/a/b.png · {SKIPPED_DETAIL}") + + +if __name__ == "__main__": + unittest.main() diff --git a/web/js/workspace/activity-scan.mjs b/web/js/workspace/activity-scan.mjs index 9fdfd09..47c0760 100644 --- a/web/js/workspace/activity-scan.mjs +++ b/web/js/workspace/activity-scan.mjs @@ -1,7 +1,8 @@ /** * Scan for metadata (#249 Activity-1, D55): Images → Import → "Scan for metadata" → Review scan opens the concept's * "Review metadata scan". Its backup box downloads the existing `/prompt_manager/backup` copy and toasts the outcome; - * Start scan starts the session job through the Activity store, which toasts "Scan started.", and opens the job's view. + * Start scan starts the session job through the Activity store over the chosen scope (new and unresolved files, or + * all files with the D58 checkbox), which toasts "Scan started.", and opens the job's view. * A start refused because another record-changing job runs toasts "Another job is running: <title>." with a link to * that job (D57). */ @@ -49,10 +50,13 @@ export function busyToast(running) { return ['Another job is running: ', { text: running.title, href: `#/activity/${running.id}` }, '.']; } -/** Start the scan job and open its view; a refused or failed start says why and stays on the page. */ -export async function startMetadataScan({ store, scope, notify = showWorkspaceToast, navigate = navigateTo }) { +/** + * Start the scan job over `files` ('new': new and unresolved files, or 'all') and open its view; a refused or failed + * start says why and stays on the page. + */ +export async function startMetadataScan({ store, scope, files = 'new', notify = showWorkspaceToast, navigate = navigateTo }) { try { - const result = await store.start('scan'); + const result = await store.start('scan', { scope: files }); if (scope.signal.aborted) return; if (result.busy !== undefined) { notify(busyToast(result.busy)); @@ -68,5 +72,5 @@ export async function startMetadataScan({ store, scope, notify = showWorkspaceTo export function openMetadataScan({ scope, store, sources, trigger = null }) { openScanReviewDialog({ scope, sources, trigger, onBackup: () => downloadDatabaseBackup(), - onStart: () => startMetadataScan({ store, scope }) }); + onStart: ({ scope: files }) => startMetadataScan({ store, scope, files }) }); } diff --git a/web/js/workspace/workspace-dialogs.mjs b/web/js/workspace/workspace-dialogs.mjs index 6872a68..b6ac31c 100644 --- a/web/js/workspace/workspace-dialogs.mjs +++ b/web/js/workspace/workspace-dialogs.mjs @@ -20,7 +20,7 @@ * * Every opener takes the page `scope`: disposing it closes and removes the shell, so a dialog never outlives its page. */ -import { createButton } from '../shared/toolkit/core.js'; +import { createButton, createSwitch } from '../shared/toolkit/core.js'; const FIELD_LABELS = Object.freeze({ source: 'Source', folder: 'Folder', media: 'Media', tags: 'Tags' }); const FIELD_ORDER = Object.keys(FIELD_LABELS); @@ -342,22 +342,41 @@ export const SCAN_REVIEW_SENTENCE = 'Enrich positive / negative associations, re const BACKUP_LABEL = 'Back up now'; const BACKUP_BUSY_LABEL = 'Backing up…'; +export const SCAN_SCOPE_LABELS = { new: 'New and unresolved files', all: 'All files' }; +export const SCAN_ALL_FILES_HINT = 'Also read the files already linked to their prompt again.'; + /** - * The concept's "Review metadata scan" (`scan-review`, D55). `sources` is the number of output folders the scan reads; - * the scope is fixed to "All files" in Activity-1. `onBackup` runs the database backup (the button is aria-disabled - * and reads "Backing up…" until it settles, and the dialog stays open); Start scan closes the dialog, then `onStart`. + * The concept's "Review metadata scan" (`scan-review`, D55). `sources` is the number of output folders the scan reads. + * The scope is the concept's "New and unresolved files"; the "All files" checkbox (D58, the concept's `.setting-row` + * switch, unchecked) switches the Scope value to "All files". `onBackup` runs the database backup (the button is + * aria-disabled and reads "Backing up…" until it settles, and the dialog stays open); Start scan closes the dialog, then + * calls `onStart({scope: 'new' | 'all'})`. */ export function openScanReviewDialog({ scope, sources, onBackup, onStart, trigger = null }) { requireScope(scope); requireCallbacks({ onBackup, onStart }); if (!Number.isSafeInteger(sources) || sources < 0) throw new TypeError('The scan review needs its source count'); const grid = el('dl', undefined, 'meta-grid'); + const values = {}; for (const [term, value] of [['Sources', `${sources.toLocaleString('en-US')} output ${sources === 1 ? 'folder' : 'folders'}`], - ['Scope', 'All files']]) { + ['Scope', SCAN_SCOPE_LABELS.new]]) { const entry = el('div'); - entry.append(el('dt', term), el('dd', value)); + values[term] = el('dd', value); + entry.append(el('dt', term), values[term]); grid.append(entry); } + // The concept's System toggle: a `.setting-row` whose checkbox is the Toolkit switch (concept `hydrateSwitches`). + const toggle = createSwitch('', '', 'scan-all-files'); + const allFiles = toggle.querySelector('input'); + allFiles.checked = false; + const option = el('div', undefined, 'setting-row'); + const optionText = el('div'); + const optionLabel = el('label', SCAN_SCOPE_LABELS.all); + optionLabel.htmlFor = allFiles.id; + optionText.append(optionLabel, el('p', SCAN_ALL_FILES_HINT)); + option.append(optionText, toggle); + const chosenScope = () => (allFiles.checked ? 'all' : 'new'); + allFiles.addEventListener('change', () => { values.Scope.textContent = SCAN_SCOPE_LABELS[chosenScope()]; }); const notice = el('div', undefined, 'notice'); const copy = el('div'); copy.append(el('h3', 'Back up the database first (recommended)'), @@ -381,10 +400,11 @@ export function openScanReviewDialog({ scope, sources, onBackup, onStart, trigge const start = createButton('Start scan', 'ca-primary'); cancel.addEventListener('click', () => finish()); start.addEventListener('click', () => { + const files = chosenScope(); finish(); - onStart(); + onStart({ scope: files }); }); const actions = el('div', undefined, 'dialog-actions'); actions.append(cancel, start); - show('Review metadata scan', [grid, el('p', SCAN_REVIEW_SENTENCE, 'small'), notice, actions], { scope, trigger }); + show('Review metadata scan', [grid, option, el('p', SCAN_REVIEW_SENTENCE, 'small'), notice, actions], { scope, trigger }); }