From 66b3d078db05db0030facc2cc90fad378b2e4a35 Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Wed, 21 Oct 2026 16:38:38 -0700 Subject: [PATCH] fix(activity): re-read known files whose size changed, and review follow-ups The new scope now skips a known, resolved file only while its stored generated_images.file_size equals its size on disk; a NULL stored size is re-read. A file replaced at the same path is read again and its conflict shows under Needs review. The scan stores each file's size when it ingests it, and both sides of the known-set match go through normalize_image_path. A failed known-set read logs and falls back to reading every file, and an empty request body asks for the default scope. The All files switch is described by its hint, a lost job's Download report is disabled, the report toast is skipped once the view is gone, and the scan log opens at its newest line every time. Fixtures use the real scope shape. --- TESTING.md | 65 ++++++--- database/operations.py | 70 ++++++---- py/api/admin.py | 46 +++++- py/api/job_routes.py | 8 +- tests/browser/workspace-activity.spec.mjs | 13 ++ tests/browser/workspace_activity_fixture.py | 2 +- tests/js/workspace-activity-store.test.js | 2 +- tests/test_jobs.py | 11 +- tests/test_scan_scope.py | 147 ++++++++++++++++++-- web/js/workspace/activity-job-view.mjs | 15 +- web/js/workspace/activity-scan.mjs | 22 +-- web/js/workspace/workspace-dialogs.mjs | 5 +- 12 files changed, 321 insertions(+), 85 deletions(-) diff --git a/TESTING.md b/TESTING.md index 8ce50b8..21aaa78 100644 --- a/TESTING.md +++ b/TESTING.md @@ -629,26 +629,44 @@ Recorded shell deviations: 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 "= / · 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. + again." (the switch's `aria-describedby`) 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"}` (no body, an empty body or no + `scope` is "new"; another value is a 400). + - In "new" the scan skips, before reading any metadata, the files the Library already + links with a resolved link and that are unchanged on disk: exactly one + `generated_images` row for the file, exactly one distinct pair link on it + (`image_prompt_pairs`, written only by resolved ingestion), and a stored + `file_size` equal to the file's size in this walk. Both paths go through + `normalize_image_path` (absolute, normalized, `os.path.normcase`); a row without + `file_path` matches by its `image_path`. + - Read again: 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, not linked, a + file whose size changed (one replaced at the same path, e.g. a reused ComfyUI + counter name, is read and its conflict lands under Needs review), and a row with no + stored size (unknown is not known). The scan now stores each file's size when it + ingests it, so a file read once is skipped by the next "new" pass; the first pass + after the upgrade fills the missing sizes, which reports those files as enriched + (Added) instead of unchanged. + - The known set is one streamed query that keeps only the scanned paths. If it cannot + be read, the scan logs "Known files could not be read · reading every file" and + reads every file. + - A skipped file counts as processed and unchanged in the legacy counts (not as + found) and under Already known, with the detail "Skipped · already linked" and the + log line "= / · Skipped · already linked"; the log's first line names + the scope ("Scan started · N output folders · new and unresolved files" or "· all + files"). A legacy-page scan that attaches to a running "new" job therefore shows + the skipped files in its `unchanged` count. + - "All files" reads every file, as the scan did in Activity-1, and the legacy + `/prompt_manager/scan` route always does when it starts the scan (its SSE payload + keys 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, the description and the + request both ways; `tests/test_scan_scope.py` covers the skip, the size rule, the + replaced file, the re-read of unresolved files, "all", the legacy route, the + fallback, the empty body, the 400 and Windows case folding. - **D59** (job report and log, Activity-2 Task 5): the job view's "Download report" is enabled. It fetches `GET /prompt_manager/jobs//report` through the api-client URL (so a proxy mount works) and saves a JSON attachment named @@ -657,8 +675,11 @@ Recorded shell deviations: holds the summary, every counter, each outcome group's exact `count`, its listed `rows` (at most 200) and a `truncated` flag, the scope and the log; rows and log lines carry root-relative paths only, never an absolute path (an unknown id is a 404). - The scan log opens at its newest line and follows new lines only while the reader is - at the bottom; a reader who scrolled up keeps the position. + A job lost with a restart has no report on the server, so its Download report is + disabled; a view left while the report downloads still saves it but shows no toast. + The scan log opens at its newest line every time it is opened and follows new lines + only while the reader is at the bottom; a reader who scrolled up keeps the position + while it stays open (checked on Chromium, Firefox and WebKit). `test_report_is_attachment_without_absolute_paths` and the browser tests `download report saves the job JSON` and `log keeps the reader's scroll position` cover it. - **D29**: no prototype copy. There is no "Interactive concept" badge and no diff --git a/database/operations.py b/database/operations.py index 2694522..26b28d1 100644 --- a/database/operations.py +++ b/database/operations.py @@ -181,14 +181,16 @@ 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, +# The scan's known set (#249 Activity-2): every image row, with its stored file +# size and 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, that row is resolved, and the row's stored +# size equals the file's size now; several rows (one file linked to more than +# one prompt) are ambiguous, and an unknown (NULL) stored size is not known. 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, gi.file_size, (SELECT COUNT(DISTINCT ip.pair_id) FROM image_prompt_pairs ip WHERE ip.image_id = gi.id) = 1 FROM generated_images gi""" @@ -2293,41 +2295,51 @@ 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. + def get_known_resolved_paths(self, files: Dict[str, Optional[int]]) -> Set[str]: + """The paths in ``files`` 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``. + ``files`` maps each scanned path to its size on disk (None when it could + not be read). A path qualifies when exactly one ``generated_images`` row + claims the file, that row has exactly one pair link + (``image_prompt_pairs``, written only by resolved ingestion), and its + stored ``file_size`` is not NULL and equals the given size, so a file + replaced at the same path is read again. 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 qualify. Both sides go through ``normalize_image_path`` (absolute, + normalized, ``os.path.normcase``), so a Windows row stored in another + case or spelling still matches. One query, streamed; returns the + matching keys of ``files``. """ - wanted: Dict[str, List[str]] = {} - for path in paths: - key = os.path.normcase(normalize_image_path(str(path))) - wanted.setdefault(key, []).append(str(path)) + wanted: Dict[str, List[Tuple[str, Optional[int]]]] = {} + for path, size in files.items(): + key = normalize_image_path(str(path)) + wanted.setdefault(key, []).append((str(path), size)) if not wanted: return set() rows_seen: Dict[str, int] = {} - resolved: Set[str] = set() + stored_sizes: Dict[str, int] = {} 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) + for file_path, image_path, file_size, is_resolved in rows: + # image_path is NOT NULL; file_path is NULL only on old rows. + key = normalize_image_path(file_path or image_path) 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]} + if is_resolved and file_size is not None: + stored_sizes[key] = file_size + return { + path + for key, stored_size in stored_sizes.items() + if rows_seen[key] == 1 + for path, size in wanted[key] + if size is not None and size == stored_size + } def get_image_link_summaries( self, paths: List[str], allow_filename_fallback: bool = True diff --git a/py/api/admin.py b/py/api/admin.py index 62925d5..bc900ee 100644 --- a/py/api/admin.py +++ b/py/api/admin.py @@ -207,6 +207,30 @@ def _scan_status_line(done, total, batches, elapsed): return f"{line} · about {minutes} min left" +def _file_size(path): + """``path``'s size on disk, or None when it cannot be read.""" + try: + return os.path.getsize(path) + except OSError: + return None + + +def _with_file_size(canonical, media_file): + """Record the file's size in ``file_info`` unless the metadata carries one. + + Stored as ``generated_images.file_size``; the "new" scan scope skips a known + file only while its size is unchanged. + """ + info = canonical.get("file_info") + info = dict(info) if isinstance(info, dict) else {} + if info.get("size") is None: + size = _file_size(media_file) + if size is not None: + info["size"] = size + if info: + canonical["file_info"] = info + + def _scan_roots(output_dirs): """(absolute root, public label, listing index), the most specific first. @@ -1560,9 +1584,7 @@ class AdminRoutesMixin: roots = _scan_roots(output_dirs) 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] - ) + known = await self._known_scan_files(job, media_files) async for event in self._scan_batches( media_files, counts, job, roots, known ): @@ -1586,6 +1608,23 @@ class AdminRoutesMixin: "message": "An internal error occurred. Check server logs for details.", } + async def _known_scan_files(self, job, media_files): + """The "new" scope's skip set; a failed read falls back to reading all.""" + try: + return await self._run_in_executor(self._known_scan_files_sync, media_files) + except Exception: + self.logger.warning( + "Known files could not be read; the scan reads every file", + exc_info=True, + ) + job.log_line("Known files could not be read · reading every file") + return set() + + def _known_scan_files_sync(self, media_files): + """Stat each scanned file and ask the database which are known (blocking).""" + sizes = {str(media_file): _file_size(media_file) for media_file in media_files} + return self.db.get_known_resolved_paths(sizes) + async def _scan_batches(self, media_files, counts, job, roots=(), known=()): """Process ``media_files`` in executor batches, updating ``counts`` in place. @@ -1740,6 +1779,7 @@ class AdminRoutesMixin: pass if not canonical.get("prompt") and not canonical.get("parameters"): return None + _with_file_size(canonical, media_file) resolution = resolve_prompt_pair(canonical) if resolution["state"] == "resolved": result = ingest_resolved_pair( diff --git a/py/api/job_routes.py b/py/api/job_routes.py index 5086417..71b35cb 100644 --- a/py/api/job_routes.py +++ b/py/api/job_routes.py @@ -24,12 +24,16 @@ def _bad_request(message): 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. + An empty body (also a chunked one with no bytes), or one without + ``scope``, asks for the default. """ if not request.can_read_body: return DEFAULT_SCAN_SCOPE + text = await request.text() + if not text.strip(): # e.g. a chunked POST that sent no bytes + return DEFAULT_SCAN_SCOPE try: - body = await request.json() + body = json.loads(text) except ValueError: return None if not isinstance(body, dict): diff --git a/tests/browser/workspace-activity.spec.mjs b/tests/browser/workspace-activity.spec.mjs index 6aa0b56..374aefe 100644 --- a/tests/browser/workspace-activity.spec.mjs +++ b/tests/browser/workspace-activity.spec.mjs @@ -417,6 +417,7 @@ test('all files checkbox changes the scope line and the request', async ({ page 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.'); + await expect(allFiles).toHaveAccessibleDescription('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'); @@ -625,6 +626,9 @@ test('job view renders 200-row cap note', async ({ page, request }) => { // Review M12: with the list capped and the log closed, more files only update the counters; the detail is read // again only when the log opens with new lines. + // A read still due from opening the view (at most one per DETAIL_INTERVAL_MS, 1 s) lands first; Firefox is slow + // enough that it otherwise fell inside the window below. + await page.waitForTimeout(1200); const reads = []; page.on('request', item => { if (new URL(item.url()).pathname === `/prompt_manager/jobs/${job.id}`) reads.push(Date.now()); }); await step(request, job.id, { processed: 300, bulk: { group: 'added', count: 40 } }); @@ -691,6 +695,8 @@ test('an unknown or restarted job view says so', async ({ page, request }) => { await expect(await request.post('/__test__/jobs/restart')).toBeOK(); await expect(heading(page).locator('p.muted')).toHaveText(LOST); await expect(heading(page).getByRole('button', { name: 'Cancel scan', exact: true })).toHaveCount(0); + // A lost job has no report on the server (review M9): Download report is disabled rather than failing. + await expect(heading(page).getByRole('button', { name: 'Download report', exact: true })).toBeDisabled(); // What was read before the restart stays on the page. await expect(fileRows(page)).toHaveCount(1); await expect(pill(page)).toBeHidden(); @@ -760,6 +766,13 @@ test('log keeps the reader\'s scroll position', async ({ page, request }) => { await step(request, job.id, { processed: 120, records: more(90, 30) }); await expect(pre).toContainText('log/f_119.png'); expect((await metrics()).top).toBe(40); + // Closed and opened again (review M6): every open starts at the newest line, also after lines arrived meanwhile. + await scanLog(page).locator('summary').click(); + await expect(scanLog(page)).not.toHaveAttribute('open', ''); + await step(request, job.id, { processed: 150, records: more(120, 30) }); + await scanLog(page).locator('summary').click(); + await expect(pre).toContainText('log/f_149.png'); + await expect.poll(async () => { const m = await metrics(); return m.max > 0 && m.top >= m.max - 2; }).toBe(true); }); test('a failed report download says so and leaves the button usable', async ({ page, request }) => { diff --git a/tests/browser/workspace_activity_fixture.py b/tests/browser/workspace_activity_fixture.py index 40a9054..77f102a 100644 --- a/tests/browser/workspace_activity_fixture.py +++ b/tests/browser/workspace_activity_fixture.py @@ -129,7 +129,7 @@ def register_activity_commands(app, routes, api): str(data.get("title", "Scan for metadata")), record_changing=True, scope={ - "mode": "all", + "files": str(data.get("files", "new")), "output_folders": count(data.get("output_folders"), 2), }, runner=fake_runner(ending), diff --git a/tests/js/workspace-activity-store.test.js b/tests/js/workspace-activity-store.test.js index 211825c..89eb72d 100644 --- a/tests/js/workspace-activity-store.test.js +++ b/tests/js/workspace-activity-store.test.js @@ -11,7 +11,7 @@ const ID_C = 'c'.repeat(32); function summary(overrides = {}) { return { id: ID_A, kind: 'scan', title: 'Scan for metadata', state: 'running', started_at: 1000, finished_at: null, progress: 0, status_line: '', processed: 0, total: 0, - counters: { added: 0, known: 0, review: 0, no_metadata: 0, errors: 0 }, scope: { mode: 'all', output_folders: 2 }, + counters: { added: 0, known: 0, review: 0, no_metadata: 0, errors: 0 }, scope: { files: 'new', output_folders: 2 }, ...overrides }; } diff --git a/tests/test_jobs.py b/tests/test_jobs.py index dfedf8a..7f87f85 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -157,7 +157,7 @@ class TestJobRegistry(unittest.IsolatedAsyncioTestCase): "scan", "Scan for metadata", record_changing=True, - scope={"mode": "all"}, + scope={"files": "new", "output_folders": 2}, runner=_gated_runner(gate), ) self.assertEqual(job.state, "running") @@ -183,7 +183,7 @@ class TestJobRegistry(unittest.IsolatedAsyncioTestCase): "log_length", }, ) - self.assertEqual(summary["scope"], {"mode": "all"}) + self.assertEqual(summary["scope"], {"files": "new", "output_folders": 2}) # What the detail would list, so a view reads it only when that changed. self.assertEqual((summary["listed"], summary["log_length"]), ({}, 0)) self.assertIsNone(summary["finished_at"]) @@ -373,7 +373,12 @@ class TestJobOutcomesAndLog(unittest.TestCase): job.record("bogus", {"path": "a.png", "root": "output"}, "x") def test_detail_and_progress(self): - job = Job("scan", "Scan", scope={"mode": "all"}, groups=SCAN_GROUP_NAMES) + job = Job( + "scan", + "Scan", + scope={"files": "new", "output_folders": 2}, + groups=SCAN_GROUP_NAMES, + ) job.update(progress=150, status_line="Processing file 3 of 4", processed=3) job.update(total=4) job.record("known", {"path": "a.png", "root": "output"}, "= a.png") diff --git a/tests/test_scan_scope.py b/tests/test_scan_scope.py index d41f19b..6044714 100644 --- a/tests/test_scan_scope.py +++ b/tests/test_scan_scope.py @@ -24,6 +24,7 @@ from py.config import GalleryConfig # noqa: E402 from utils.prompt_pair_resolver import resolve_prompt_pair # noqa: E402 SKIPPED_DETAIL = "Skipped · already linked" +ON_DISK = object() # _link_resolved: store the file's size on disk def _graph(text): @@ -40,9 +41,17 @@ def _graph(text): 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).""" + def _link_resolved(self, path, text, size=ON_DISK): + """``path`` linked with a resolved pair link (what a resolved scan writes). + + The row stores ``size`` as its file size: by default the file's size on + disk (None when the file does not exist). + """ + if size is ON_DISK: + size = os.path.getsize(path) if os.path.exists(path) else None metadata = {"prompt": _graph(text)} + if size is not None: + metadata["file_info"] = {"size": size} result = ingest_resolved_pair( self.api.db, str(path), metadata, resolve_prompt_pair(metadata) ) @@ -182,6 +191,91 @@ class TestScanScope(ScanScopeTestCase): self.assertEqual(payloads[-1]["type"], "complete") self.assertEqual(payloads[-1]["unchanged"], 1) + async def test_file_replaced_at_a_known_path_is_reread_and_needs_review(self): + GalleryConfig.MONITORING_DIRECTORIES = [self._root(self.output_dir)] + path = self.output_dir / "out_00012.png" + _write_png(path, prompt_text="the first prompt") + self._link_resolved(path, "the first prompt") + before = os.path.getsize(path) + # ComfyUI reused the name for a new image with another prompt. + _write_png(path, prompt_text="a much longer second prompt for the same name") + self.assertNotEqual(os.path.getsize(path), before) + reads = self._spy_reads() + + job = await self._scan() + + self.assertEqual(reads, ["out_00012.png"]) + self.assertEqual(job["counters"]["review"], 1) + self.assertEqual(job["counters"]["known"], 0) + self.assertIn("different prompt", job["outcomes"]["review"][0]["detail"]) + + async def test_scanned_files_store_their_size_so_the_next_new_scan_skips(self): + GalleryConfig.MONITORING_DIRECTORIES = [self._root(self.output_dir)] + _write_png(self.output_dir / "fresh.png", prompt_text="a fresh prompt") + first = await self._scan() + self.assertEqual(first["counters"]["added"], 1) + reads = self._spy_reads() + + second = await self._scan() + + self.assertEqual(reads, []) + self.assertEqual(second["outcomes"]["known"][0]["detail"], SKIPPED_DETAIL) + + async def test_stored_null_size_is_reread(self): + GalleryConfig.MONITORING_DIRECTORIES = [self._root(self.output_dir)] + path = self.output_dir / "old.png" + _write_png(path, prompt_text="an old prompt") + self._link_resolved(path, "an old prompt", size=None) + reads = self._spy_reads() + + job = await self._scan() + + self.assertEqual(reads, ["old.png"]) + # The read stores the size it was missing, so the file is known next time. + self.assertEqual(job["counters"]["added"], 1) + self.assertEqual( + self.api.db.get_known_resolved_paths({str(path): os.path.getsize(path)}), + {str(path)}, + ) + + async def test_known_set_failure_falls_back_to_reading_every_file(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() + + def broken(files): + raise sqlite3.OperationalError("database is locked") + + self.api.db.get_known_resolved_paths = broken + with self.assertLogs("prompt_manager.api", level="WARNING"): + job = await self._scan() + + self.assertEqual(job["state"], "finished") + self.assertEqual(reads, ["known.png"]) + self.assertIn( + "Known files could not be read · reading every file", "\n".join(job["log"]) + ) + + async def test_empty_read_body_asks_for_the_default_scope(self): + """A chunked POST with no bytes can read a body: still the default.""" + from py.api.job_routes import _scan_scope + + class Request: + can_read_body = True + + def __init__(self, text): + self._text = text + + async def text(self): + return self._text + + self.assertEqual(await _scan_scope(Request("")), "new") + self.assertEqual(await _scan_scope(Request(" \n")), "new") + self.assertEqual(await _scan_scope(Request('{"scope": "all"}')), "all") + self.assertIsNone(await _scan_scope(Request("{"))) + async def test_invalid_scope_is_refused(self): for body in ({"scope": "everything"}, {"scope": 1}, ["new"]): with self.subTest(body=body): @@ -203,17 +297,50 @@ class TestKnownResolvedPaths(ScanScopeTestCase): 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_resolved(known, "a red square", size=10) self._link_without_pair(fallback, "a fallback") - asked = [str(known), str(fallback), str(self.output_dir / "new.png")] + asked = {str(known): 10, str(fallback): 10, str(self.output_dir / "n.png"): 10} self.assertEqual(self.api.db.get_known_resolved_paths(asked), {str(known)}) - self.assertEqual(self.api.db.get_known_resolved_paths([]), set()) + self.assertEqual(self.api.db.get_known_resolved_paths({}), set()) + + async def test_size_must_match_and_a_stored_null_size_is_not_known(self): + same, other = self.output_dir / "same.png", self.output_dir / "other.png" + unknown = self.output_dir / "unknown.png" + self._link_resolved(same, "same size", size=10) + self._link_resolved(other, "other size", size=10) + self._link_resolved(unknown, "no stored size", size=None) + asked = {str(same): 10, str(other): 11, str(unknown): 10} + + self.assertEqual(self.api.db.get_known_resolved_paths(asked), {str(same)}) + # A file the walk could not stat is not known either. + self.assertEqual(self.api.db.get_known_resolved_paths({str(same): None}), set()) + + async def test_row_without_file_path_matches_through_image_path(self): + known = self.output_dir / "legacy.png" + self._link_resolved(known, "a legacy row", size=10) + with sqlite3.connect(self.api.db.model.db_path) as conn: + conn.execute("UPDATE generated_images SET file_path=NULL") + conn.close() + + found = self.api.db.get_known_resolved_paths({str(known): 10}) + self.assertEqual(found, {str(known)}) + + async def test_stored_spelling_is_normalized_like_the_scanned_path(self): + known = self.output_dir / "sub" / "known.png" + self._link_resolved(known, "a red square", size=10) + spelled = os.path.join(str(self.output_dir), "sub", "..", "sub", "known.png") + with sqlite3.connect(self.api.db.model.db_path) as conn: + conn.execute("UPDATE generated_images SET file_path=?", (spelled,)) + conn.close() + + found = self.api.db.get_known_resolved_paths({str(known): 10}) + self.assertEqual(found, {str(known)}) 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}") + self._link_resolved(path, f"prompt {path.name}", size=10) original = self.api.db.model.get_connection opened = [] @@ -223,13 +350,13 @@ class TestKnownResolvedPaths(ScanScopeTestCase): 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]) + found = self.api.db.get_known_resolved_paths({str(p): 10 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") + self._link_resolved(stored, "a red square", size=10) # 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( @@ -239,9 +366,9 @@ class TestKnownResolvedPaths(ScanScopeTestCase): 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}) + self.assertEqual(self.api.db.get_known_resolved_paths({asked: 10}), {asked}) if os.path.normcase("A") == "A": # a case-sensitive platform - self.assertEqual(self.api.db.get_known_resolved_paths([asked]), set()) + self.assertEqual(self.api.db.get_known_resolved_paths({asked: 10}), set()) class TestSkippedOutcome(unittest.TestCase): diff --git a/web/js/workspace/activity-job-view.mjs b/web/js/workspace/activity-job-view.mjs index cf01e68..c553198 100644 --- a/web/js/workspace/activity-job-view.mjs +++ b/web/js/workspace/activity-job-view.mjs @@ -189,7 +189,6 @@ export function mountActivityJobView(main, { store, scope, api, jobId }) { let readTimer = null; let roots = null; let lastState = null; - let logOpened = false; // Set once this page's own list read answered, or the detail read said the job is unknown. let listedOnce = false; let notFound = false; @@ -272,6 +271,10 @@ export function mountActivityJobView(main, { store, scope, api, jobId }) { document.title = `${job.title} · Prompt Manager`; parts.description.textContent = jobDetail(job); cancel.textContent = scan ? 'Cancel scan' : 'Cancel job'; + // A job lost with the server has no report there (review M9). + const lost = job.state === 'lost'; + if (lost && document.activeElement === download) parts.h1.focus(); + download.disabled = lost; if (job.state === 'running') { if (!cancel.isConnected) parts.actions.append(cancel); } else if (cancel.isConnected) { @@ -350,17 +353,19 @@ export function mountActivityJobView(main, { store, scope, api, jobId }) { } scope.listen(log, 'toggle', () => { - // A log opened for the first time starts at its newest line. - if (log.open && !logOpened) { logOpened = true; pre.scrollTop = pre.scrollHeight; } + // Every open starts at the newest line: while closed the log has no layout box (Firefox, WebKit), so a kept + // position would be lost anyway (review M6). + if (log.open) pre.scrollTop = pre.scrollHeight; const job = store.find(jobId); if (log.open && job && job.state !== 'lost' && detailChanged(job)) scheduleRead(); }); scope.listen(download, 'click', async () => { - if (download.getAttribute('aria-disabled') === 'true') return; + if (download.disabled || download.getAttribute('aria-disabled') === 'true') return; download.setAttribute('aria-disabled', 'true'); try { - await downloadJobReport(jobId); + // A view left while the report downloads shows no toast (review M10). + await downloadJobReport(jobId, { signal: scope.signal }); } finally { download.removeAttribute('aria-disabled'); } diff --git a/web/js/workspace/activity-scan.mjs b/web/js/workspace/activity-scan.mjs index 875e630..8ec2c87 100644 --- a/web/js/workspace/activity-scan.mjs +++ b/web/js/workspace/activity-scan.mjs @@ -18,11 +18,12 @@ export const REPORT_FAILED = 'The report could not be downloaded. Try again.'; const JOBS_REPORT_BASE = '/prompt_manager/jobs'; export const START_FAILED = 'The scan could not start. Try again.'; const FALLBACK_NAME = 'prompts_backup.db'; +const REPORT_FALLBACK_NAME = 'prompt-manager-report.json'; -/** The download name from a Content-Disposition header, or the fallback. */ -export function backupFilename(disposition) { +/** The download name from a Content-Disposition header, or `fallback`. */ +export function backupFilename(disposition, fallback = FALLBACK_NAME) { const match = /filename="?([^";]+)"?/i.exec(disposition || ''); - return match ? match[1] : FALLBACK_NAME; + return match ? match[1] : fallback; } /** Save a fetched response through a temporary link, named by `filename`. */ @@ -52,17 +53,22 @@ export async function downloadDatabaseBackup({ fetchUrl = window.ApiClient.fetch } } -/** Download a job's JSON report (Activity-2, D59) through the api-client URL, so a proxy mount works; toasts the outcome. */ -export async function downloadJobReport(jobId, { fetchUrl = window.ApiClient.fetchUrl, notify = showWorkspaceToast } = {}) { +/** + * Download a job's JSON report (Activity-2, D59) through the api-client URL, so a proxy mount works; toasts the outcome + * unless `signal` (the view's scope) was aborted meanwhile. + */ +export async function downloadJobReport(jobId, { fetchUrl = window.ApiClient.fetchUrl, notify = showWorkspaceToast, + signal = null } = {}) { try { const response = await fetchUrl(`${JOBS_REPORT_BASE}/${encodeURIComponent(jobId)}/report`); if (!response.ok) throw new Error(`Report failed with ${response.status}`); const blob = await response.blob(); - saveResponse(blob, backupFilename(response.headers.get('Content-Disposition')).replace(FALLBACK_NAME, 'prompt-manager-report.json')); - notify(REPORT_DONE); + saveResponse(blob, backupFilename(response.headers.get('Content-Disposition'), REPORT_FALLBACK_NAME)); + // The file is saved either way; a view left meanwhile (`signal` aborted) shows no toast. + if (!signal?.aborted) notify(REPORT_DONE); return true; } catch { - notify(REPORT_FAILED); + if (!signal?.aborted) notify(REPORT_FAILED); return false; } } diff --git a/web/js/workspace/workspace-dialogs.mjs b/web/js/workspace/workspace-dialogs.mjs index b6ac31c..72b3e86 100644 --- a/web/js/workspace/workspace-dialogs.mjs +++ b/web/js/workspace/workspace-dialogs.mjs @@ -373,7 +373,10 @@ export function openScanReviewDialog({ scope, sources, onBackup, onStart, trigge 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)); + const hint = el('p', SCAN_ALL_FILES_HINT); + hint.id = 'scan-all-files-hint'; + allFiles.setAttribute('aria-describedby', hint.id); + optionText.append(optionLabel, hint); option.append(optionText, toggle); const chosenScope = () => (allFiles.checked ? 'all' : 'new'); allFiles.addEventListener('change', () => { values.Scope.textContent = SCAN_SCOPE_LABELS[chosenScope()]; });