diff --git a/TESTING.md b/TESTING.md index 89aa2e40..1fb6a93b 100644 --- a/TESTING.md +++ b/TESTING.md @@ -1830,8 +1830,9 @@ Inspect plan keeps D60–D66 (plan D60 → D83, D61 → D84, D65 → D88, D68 blocks' context ("Current file image tags" in History). An empty source draws no section. Repeats are removed within a source only, so a linked tag that is also an image tag shows in both sections and each count is true. D21's filter rules are - unchanged. History's current-file read carries no `image_tags` facet (D88), so there - only the linked section shows. `tests/browser/inspector-concept.spec.mjs` wraps the + unchanged. History's current-file read carries the same `image_tags` facet as the Images + read (#457), so History's inline current file and its viewer show the image tags section + too (`test_current_inspection_carries_the_files_image_tags`). `tests/browser/inspector-concept.spec.mjs` wraps the concept's row in the same sections at runtime (`withTagSections`, the D154 rule injected) and compares the measured sections (frame, heading, label, row); a mutation of the label color, the row margin or the top rule fails it. Browser tests: @@ -6332,7 +6333,7 @@ The versioned public reads are: | `GET /prompt_manager/library/{positive_id}/history/pairs/{pair_id}/newest` | The pair's newest recorded run for the history run dot (#249 D26): `{run: {run_id, anchor_id, context_id}}` or `{run: null}` | | `GET /prompt_manager/history/runs/{run_id}` | Frozen fields, per-field provenance, capture and outcome for a known UUID | | `GET /prompt_manager/history/runs/{run_id}/outputs` | Ordered host-reported occurrences, including repeated locations | -| `GET /prompt_manager/history/runs/{run_id}/output/current/{media,thumbnail,inspection}` | Authorized current file at that reported location, using the returned epoch/scope URL | +| `GET /prompt_manager/history/runs/{run_id}/output/current/{media,thumbnail,inspection}` | Authorized current file at that reported location, using the returned epoch/scope URL; `inspection` carries the shared inspector envelope including the file's `image_tags` facet (#457) | The newest-pair read takes no query. It answers 404 when the positive is not an eligible saved prompt or the pair is not one of its pairs. It walks the positive's exact diff --git a/py/api/history.py b/py/api/history.py index 9426ee0c..a62c9821 100644 --- a/py/api/history.py +++ b/py/api/history.py @@ -27,7 +27,11 @@ from .history_current import ( resolve_current_candidate, snapshot_current_output_scope, ) -from .inspection_service import CurrentFileChanged, inspect_current_file +from .inspection_service import ( + CurrentFileChanged, + image_tags_reader, + inspect_current_file, +) from .images import image_category_key try: @@ -778,6 +782,14 @@ class HistoryRoutesMixin: ), own, ), + # The file's own image tags, keyed as the Tag dialog + # keys them: the normcased realpath (#457). + image_tags=image_tags_reader( + db, + str(resolved.canonical_target), + self.logger, + close=True, + ), recorded=recorded_output_lookup( db, close=True, reference=receipt["reference"] ), diff --git a/py/api/images.py b/py/api/images.py index 4726d995..01b27e3f 100644 --- a/py/api/images.py +++ b/py/api/images.py @@ -55,6 +55,7 @@ from .inspection_service import ( IMAGE_EXTENSIONS as _IMAGE_EXTENSIONS, SERVABLE_EXTENSIONS, VIDEO_EXTENSIONS, + image_tags_reader, inspect_current_file, ) @@ -3962,28 +3963,9 @@ class ImageRoutesMixin: ) def _inspection_image_tags(self, root: Path, parts: tuple) -> Callable: - """The inspector's ``image_tags`` reader for one rooted file (S1). - - The returned function takes the inspection's own stable stat and - answers ``current`` only for a byte record verified at exactly that - ``(size, mtime_ns)``; a failed read is ``unavailable``, never an error. - """ + """The inspector's ``image_tags`` reader for one rooted file (S1).""" locator = os.path.normcase(os.path.join(os.path.realpath(root), *parts)) - - def read(size: int, mtime_ns: int) -> dict: - try: - found = self.db.current_image_tags({locator: (size, mtime_ns)}) - except Exception: - self.logger.warning( - "Inspection image tags could not be read", exc_info=True - ) - return {"state": "unavailable", "tags": []} - tags = found.get(locator) - if tags is None: - return {"state": "none", "tags": []} - return {"state": "current", "tags": list(tags)} - - return read + return image_tags_reader(self.db, locator, self.logger) def _read_own_category(self, sources, path): """One file's own category, None, or OWN_CATEGORY_UNREAD (blocking). diff --git a/py/api/inspection_service.py b/py/api/inspection_service.py index 3a99af86..b635d889 100644 --- a/py/api/inspection_service.py +++ b/py/api/inspection_service.py @@ -93,6 +93,35 @@ def _in_host_temp(path: Path) -> bool: return False +def image_tags_reader( + db: Any, locator: str, logger: Any, *, close: bool = False +) -> Callable[[int, int], dict]: + """The inspector's ``image_tags`` reader for one file *locator* (S1). + + The returned function takes the inspection's own stable stat and answers + ``current`` only for a byte record verified at exactly that + ``(size, mtime_ns)``; a failed read is ``unavailable``, never an error. + ``close`` closes the calling thread's connection afterwards, for routes + that close theirs after reading (History). + """ + + def read(size: int, mtime_ns: int) -> dict: + try: + found = db.current_image_tags({locator: (size, mtime_ns)}) + except Exception: + logger.warning("Inspection image tags could not be read", exc_info=True) + return {"state": "unavailable", "tags": []} + finally: + if close: + db.model.close() + tags = found.get(locator) + if tags is None: + return {"state": "none", "tags": []} + return {"state": "current", "tags": list(tags)} + + return read + + def inspect_current_file( path: Path, *, diff --git a/tests/browser/workspace-history.spec.mjs b/tests/browser/workspace-history.spec.mjs index b8ae2588..d24e391b 100644 --- a/tests/browser/workspace-history.spec.mjs +++ b/tests/browser/workspace-history.spec.mjs @@ -1,6 +1,7 @@ import {startCoverage,saveCoverage} from '../../tools/vendor/browser-coverage.mjs'; import {test,expect} from './fixtures.mjs'; import {armUnseenRestores,unseenRestore} from './unseen-restore.mjs'; +import {saveImageTags} from './concept-inventory.mjs'; import {readFile,writeFile} from 'node:fs/promises'; import {createHash} from 'node:crypto'; @@ -495,23 +496,38 @@ test('the inline current file names its prompts apart from the frozen run fields expect(await exact(dialog(page).getByRole('region',{name:'Positive prompt',exact:true}).locator('p.prompt-text'))).toBe(before); }); -// #449 (D154): History's inline current file draws tags in the shared labelled sections, named with its context so they -// stay apart from the frozen run's fields. The server sends History no image_tags facet today (D88), so the read is -// given one here to prove the shared render in History. -test('the inline current file labels its image tags after its negative prompt',async({page})=>{ +// #449 (D154), #457: History's inline current file draws the file's own image tags in the shared labelled section, named +// with its context so they stay apart from the frozen run's fields. The tags are saved on the real file through the Tag +// routes and come from History's own current-file read (its `image_tags` facet), as Images reads them. +test('the inline current file labels its image tags after its negative prompt',async({page,request})=>{ + const reads=[]; await page.route(url=>url.pathname.endsWith('/inspection'),async route=>{const response=await route.fetch();const body=await response.json(); + reads.push(body.image_tags); body.prompts={positive:{status:'resolved',text:'current file positive',source:'graph-inferred'},negative:{status:'resolved',text:'current file negative',source:'graph-inferred'}}; - body.image_tags={state:'current',tags:['history_tag']}; await route.fulfill({response,json:body});}); - await open(page,page.fixture.detail_id);await marker(page,page.fixture.runs.shared).click();await expect(dialog(page).locator('[data-receipt]')).toHaveCount(5); - await dialog(page).getByRole('button',{name:'Raw',exact:true}).click(); - await dialog(page).locator('[data-receipt]').first().getByRole('button',{name:'View current file',exact:true}).click(); - await expect(page.locator('dialog#viewer-dialog')).toBeVisible();await page.keyboard.press('Escape');await expect(dialog(page)).toBeVisible(); const current=dialog(page).getByRole('region',{name:'Current reported file',exact:true}); const tags=current.getByRole('region',{name:'Current file image tags',exact:true}); - await expect(tags.locator('.role-label')).toHaveText('Image tags · 1');await expect(tags.locator('.tags > .chip')).toHaveText(['history_tag']); + const showCurrent=async()=>{ + await open(page,page.fixture.detail_id);await marker(page,page.fixture.runs.shared).click();await expect(dialog(page).locator('[data-receipt]')).toHaveCount(5); + await dialog(page).getByRole('button',{name:'Raw',exact:true}).click(); + await dialog(page).locator('[data-receipt]').first().getByRole('button',{name:'View current file',exact:true}).click(); + await expect(page.locator('dialog#viewer-dialog')).toBeVisible();await page.keyboard.press('Escape');await expect(dialog(page)).toBeVisible(); + await expect(current.getByRole('region',{name:'Current file negative prompt',exact:true})).toBeVisible(); + }; + // An untagged file draws no image tags section. + await showCurrent();await expect(tags).toHaveCount(0);expect(reads.at(-1)).toEqual({state:'none',tags:[]}); + await dialog(page).getByRole('button',{name:'Close recorded run',exact:true}).click(); + expect(await saveImageTags(request,'history/current.png',['history_tag','dusk'])).toEqual(['history_tag','dusk']); + await showCurrent();expect(reads.at(-1)).toEqual({state:'current',tags:['history_tag','dusk']}); + await expect(tags.locator('.role-label')).toHaveText('Image tags · 2');await expect(tags.locator('.tags > .chip')).toHaveText(['history_tag','dusk']); const negative=current.getByRole('region',{name:'Current file negative prompt',exact:true}); expect(await negative.evaluate((node,section)=>({after:Boolean(node.compareDocumentPosition(section)&Node.DOCUMENT_POSITION_FOLLOWING),inside:node.contains(section)}),await tags.elementHandle())).toEqual({after:true,inside:false}); + // The viewer opened from History shares the inspection: the same section, plain chips (no Tags filter there, D88). + await dialog(page).locator('[data-receipt]').first().getByRole('button',{name:'View current file',exact:true}).click(); + const viewerTags=page.locator('dialog#viewer-dialog #viewer-details section.tag-section.image-tags'); + await expect(viewerTags.locator('.role-label')).toHaveText('Image tags · 2'); + await expect(viewerTags.locator('span.chip')).toHaveText(['history_tag','dusk']);await expect(viewerTags.locator('button')).toHaveCount(0); + await page.keyboard.press('Escape');await expect(dialog(page)).toBeVisible(); }); // #332 CUJ 5 (MC8, §5.3): History's current-file inspector reads the file's own category with the Images key rule, so diff --git a/tests/test_api_history_outputs.py b/tests/test_api_history_outputs.py index c43f43f1..b3832030 100644 --- a/tests/test_api_history_outputs.py +++ b/tests/test_api_history_outputs.py @@ -1015,6 +1015,122 @@ class HistoryOutputAPITests(ImageAPITestCase): response = await self.current(descriptor["inspection_url"]) self.assert_error(response, await response.json(), 422, "response-limit") + async def tag_image(self, relative, tags): + """Save *tags* on Images root 0's *relative* file through the Tag routes. + + The real observe and save routes store the locator the Tag dialog + stores (``media_capture._resolve``), so History must find it (#457). + """ + self.addCleanup(self.api.close_annotation_captures) + roots = self.api._get_all_output_dirs() + target = { + "root_index": 0, + "root_scope": self.api._gallery_root_scope(roots[0], 0), + "path": relative, + } + response = await self.client.post( + "/prompt_manager/image-annotations/observe", json={"target": target} + ) + observed = await response.json() + self.assertEqual(response.status, 200, observed) + token = observed["capture"]["token"] + try: + response = await self.client.put( + "/prompt_manager/image-annotations/tags", + json={ + "target": target, + "capture_token": token, + "sha256": observed["capture"]["sha256"], + "expected_epoch": observed["library_epoch"], + "expected_tag_revision": observed["tag_revision"], + "tags": list(tags), + }, + ) + saved = await response.json() + self.assertEqual(response.status, 200, saved) + finally: + await self.client.delete( + f"/prompt_manager/image-annotations/capture/{token}" + ) + return saved["tags"] + + async def test_current_inspection_carries_the_files_image_tags(self): + """#457: History reads the file's own image tags, as Images does (S1).""" + sub = self.output_dir / "batch" + sub.mkdir() + tagged = png(sub / "tagged.png", prompt=graph("Tagged file")) + png(self.output_dir / "plain.png", prompt=graph("Plain file")) + self.assertEqual( + await self.tag_image("batch/tagged.png", ["dunes", "warm_light"]), + ["dunes", "warm_light"], + ) + start = self.capture( + [ + self.ref("1", subfolder="batch", filename="tagged.png"), + self.ref("2", filename="plain.png"), + ] + ) + _, page = await self.outputs(start) + urls = [ + item["current_candidate"]["media"]["inspection_url"] + for item in page["items"] + ] + bodies = [] + for url in urls: + response = await self.current(url) + body = await response.json() + self.assertEqual(response.status, 200, body) + bodies.append(body) + self.assertEqual( + bodies[0].get("image_tags"), + {"state": "current", "tags": ["dunes", "warm_light"]}, + ) + # Image tags are the file's own search metadata, never prompt tags. + self.assertEqual(bodies[0]["prompts"]["positive"]["text"], "Tagged file") + self.assertEqual(bodies[1].get("image_tags"), {"state": "none", "tags": []}) + # New bytes at the same path: the tags belong to the old byte record. + stat = tagged.stat() + os.utime(tagged, ns=(stat.st_atime_ns, stat.st_mtime_ns + 10**9)) + response = await self.current(urls[0]) + body = await response.json() + self.assertEqual(response.status, 200, body) + self.assertEqual(body["image_tags"], {"state": "none", "tags": []}) + + async def test_current_inspection_finds_image_tags_under_a_symlinked_output(self): + """An output folder reached through a link (another drive) still matches.""" + link = self.comfy_dir / "linked_output" + self._make_symlink(self.output_dir, link, target_is_directory=True) + self.api._get_all_output_dirs = lambda: [link] + self.host_output = str(link) + png(self.output_dir / "image.png", prompt=graph("Linked folder")) + self.assertEqual(await self.tag_image("image.png", ["dunes"]), ["dunes"]) + start = self.capture([self.ref()]) + descriptor = await self.media(start) + response = await self.current(descriptor["inspection_url"]) + body = await response.json() + self.assertEqual(response.status, 200, body) + self.assertEqual(body["image_tags"], {"state": "current", "tags": ["dunes"]}) + + async def test_current_inspection_image_tag_failure_is_unavailable(self): + png(self.output_dir / "image.png", prompt=graph("File evidence")) + await self.tag_image("image.png", ["dunes"]) + start = self.capture([self.ref()]) + descriptor = await self.media(start) + with ( + self.assertLogs(self.api.logger, "WARNING") as logs, + patch.object( + type(self.api.db), + "current_image_tags", + side_effect=sqlite3.OperationalError("locked"), + ), + ): + response = await self.current(descriptor["inspection_url"]) + body = await response.json() + self.assertEqual(response.status, 200, body) + self.assertEqual(body["image_tags"], {"state": "unavailable", "tags": []}) + self.assertEqual(body["prompts"]["positive"]["text"], "File evidence") + self.assertNotIn(str(self.output_dir), "\n".join(logs.output)) + async def test_optional_candidate_failure_preserves_every_occurrence_without_urls( self, ): diff --git a/tests/test_inspection_response.py b/tests/test_inspection_response.py index 6b8412be..6188ce2e 100644 --- a/tests/test_inspection_response.py +++ b/tests/test_inspection_response.py @@ -887,6 +887,36 @@ class TestImageTagsFacet(unittest.TestCase): self.assertNotIn("image_tags", json.loads(encode(inspection({})))) self.assertNotIn("image_tags", self.encoded(None)) + def history_encoded(self, facet): + from utils.inspection_response import encode_history_inspection_response + + body = encode_history_inspection_response( + current_output={"history_epoch": "epoch"}, + file={"observation": "current-file", "size": 100}, + prompts=PROMPTS, + resolution={"state": "resolved", "sampler_id": "3", "reason": ""}, + inspection=inspection({"prompt": graph_text()}), + library={"status": "none"}, + image_tags=facet, + ) + return json.loads(body) + + def test_history_envelope_validates_image_tags_like_the_rooted_one(self): + """#457: History's current-file read carries the same facet.""" + for facet in ( + {"state": "current", "tags": ["dunes", "warm_light"]}, + {"state": "none", "tags": []}, + {"state": "unavailable", "tags": []}, + {"state": "current", "tags": ["a,b"]}, + {"state": "current", "tags": [], "locator": "/abs/path.png"}, + ): + with self.subTest(facet=facet): + self.assertEqual( + self.history_encoded(facet)["image_tags"], + self.encoded(facet)["image_tags"], + ) + self.assertNotIn("image_tags", self.history_encoded(None)) + class TestVideoProbeBoundsMatchPngBudgets(unittest.TestCase): """ffprobe escapes tag text; the stdout bounds must never gate a record diff --git a/utils/inspection_response.py b/utils/inspection_response.py index ba6a43f0..4aff7885 100644 --- a/utils/inspection_response.py +++ b/utils/inspection_response.py @@ -548,8 +548,12 @@ def encode_history_inspection_response( resolution: dict, inspection: "InspectionRead | None", library: dict, + image_tags: dict | None = None, ) -> bytes: - """Use occurrence identity with the same current-file evidence projection.""" + """Use occurrence identity with the same current-file evidence projection. + + *image_tags*, when given, adds the validated ``image_tags`` facet (#457). + """ return _encode_inspection( identity={ "history_version": 1, @@ -561,4 +565,5 @@ def encode_history_inspection_response( resolution=resolution, inspection=inspection, library=library, + image_tags=image_tags, )