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 }); }