diff --git a/TESTING.md b/TESTING.md index 00a35b4..028f81d 100644 --- a/TESTING.md +++ b/TESTING.md @@ -549,8 +549,9 @@ Recorded shell deviations: `a saved view row matches the concept's` saves one view in both documents and compares the rows; `tests/browser/workspace-saved-views.spec.mjs` covers saving, reopening, the defaults that replace rejected values, and failed reads and saves. -- **D50**: no "Needs review" row in SAVED VIEWS until the tag-review slice (maintainer - ruling, 2026-10-21). The comparison maps only that row out. +- **D50** (retired by #249 PR F, ruling R5): SAVED VIEWS now lists the concept's + "Needs review" row after Favorites, with its count; the comparison maps nothing out + for it (only the concept's sample count "3" reads as the fixture's "0"). - **D51** (maintainer ruling, 2026-10-21, beyond the concept): SAVED VIEWS starts with the concept's Favorites row (star icon, name, count). Its count is the Favorites listing's own `limit=1` total (`/images/output?mode=workspace&favorites=1`), follows @@ -1585,6 +1586,57 @@ npm run --prefix tools/vendor test:workflows -- inspect-concept.spec.mjs workspa npm run --prefix tools/vendor test:comfyui -- inspect-sender.spec.mjs ``` +### Needs review page and recorded deviations (#249 PR F) + +`#/review` is the concept's Metadata review page (app.js `renderReview` a749–760) over +the review routes (`web/js/workspace/review.mjs`, `review-state.mjs`): breadcrumb and +document title "Review" (the concept's), heading "Metadata review" with its line and +**Open activity** (`#/activity`), the `.notice` "N images need a decision" with the +concept's sentence, then `.gallery-layout`: the queue's concept media cards +(`image-card.mjs`, shared with Images; the image opens the shared viewer, Info selects +the item, Tag opens the shared Tag image dialog with the review scope note) and +`section.surface` "Resolve the association" (filename, Tag image, Sampler context, +the chosen path's prompt blocks, Leave for later and primary Apply association). The +first item is selected on load, as the concept's first image is. Sampler context lists +"Sampler · / " per candidate (the server's label), disables +unusable ones, preselects the first workable one and ends with "Leave unresolved" +(rulings R2/R3); "Leave unresolved" shows the ambiguous status in both blocks. Apply +takes a fresh capture (`observe`) and posts the decision; Leave unresolved on a file +under no current root posts without a capture. Both actions return to Images (R7); a +recorded decision re-reads the row's count, as does every metadata scan that ends. A +refused decision keeps the selection and shows the server's sentence (the +association conflict's explanation included) as `p.warning-note` in the panel; a +restored storage or a changed queue reads the queue again; a request with no answer +keeps its capture and body, and the next Apply of the same choice resends it once. +The SAVED VIEWS row "Needs review" (review icon) follows Favorites with the exact +pending count (`GET /prompt_manager/review/items?limit=1`, its `total`). + +- **D86** (plan D63, ruling R6): an empty queue shows only the notice, "No images need a + decision" and "Ambiguous images are added when a metadata scan or a new generation + finds more than one sampler path." +- **D87** (plan D64): an ended metadata scan whose `scope.review_items` is above 0 + carries the concept's **Review metadata** (app.js a738 `go-review`): in its Activity + item after the item's line, and in its job view's heading after Download report. The + item's chip still names its state (D54), where the concept's says "Needs attention". +- **D-NEW-F1** (number to be assigned): the queue cards have no selection checkbox and + no star (the page has no selection bar and the queue carries no favorite state); a + file under no current root has an empty, `aria-disabled` image button and Tag; more + than 40 items add the Images page's `.infinite-continuation` ("40 of 45 images", + Load more images) after the layout; loading and a failed read are the page's status + line (with Retry); a limited or unavailable file and the decision states are text in + the panel's `p.warning-note`; the toasts "Association accepted." and "Left + unresolved." replace the concept's prototype toast. The concept's "Sample review + queue" chip and "sample" wording are prototype copy (D29). +- **D3** and **D20** apply to the queue grid (its cards stay in the grid's flow; it is + not virtualized) and to the panel's prompt blocks (their mount point is transparent). + +Tests: `tests/js/workspace-review-state.test.js` (controller), +`tests/js/workspace-state.test.js` (route and count), +`tests/browser/workspace-review.spec.mjs` (queue, count, apply, leave, Tag, empty +queue, Load more, Activity's Review metadata; items recorded by the real recorder in +`tests/browser/workspace_review_fixture.py`), and the SAVED VIEWS rows in +`shell-concept.spec.mjs` and `workspace-saved-views.spec.mjs`. + ## Workspace data fields for the concept pages (#249) These fields are additive. The legacy `/prompt_manager/images/output` listing (no @@ -1813,7 +1865,9 @@ Needs review decisions and queue reads (#249, `py/api/review.py`, `database/review.py`, `database/review_decisions.py`, `tests/test_api_review.py`): `GET /prompt_manager/review/items?limit=1..40&cursor=` answers `{success, library_epoch, total, items, next_cursor}` with pending items -oldest first; `total` is the exact pending count. The cursor is an HMAC keyset +oldest first (each item adds the card's `url`, the Images listing's +`/prompt_manager/images/serve/?root=&root_scope=` formatted without a +stat, null when the file is under no current root, and `is_video` from its extension); `total` is the exact pending count. The cursor is an HMAC keyset on `(created_at, id)` bound to the storage epoch only (ruling M3, so "load more" keeps working while the monitor adds an item per generation): a malformed one is 400, a well-formed one this process cannot verify (tampered, or opened diff --git a/py/api/review.py b/py/api/review.py index 414bac0..0e3fc62 100644 --- a/py/api/review.py +++ b/py/api/review.py @@ -24,10 +24,12 @@ import hmac import json import os import re +import urllib.parse from aiohttp import web from .annotations import _HEX64, _TOKEN, _UUID, _capture_error, _error +from .inspection_service import VIDEO_EXTENSIONS from .prompts import bad_request try: @@ -253,7 +255,7 @@ class ReviewRoutesMixin: def _review_view(self, row: dict, roots, real_roots) -> dict: """The public ReviewItem of one stored row (no path, no locator).""" found = format_review_target(row["spelled_path"], row["locator"], real_roots) - target = None + target = url = None if found is not None: index, relative = found target = { @@ -261,16 +263,25 @@ class ReviewRoutesMixin: "root_scope": self._gallery_root_scope(roots[index], index), "path": relative, } + # The Images listing's media URL for the same file (formatting only). + url = ( + "/prompt_manager/images/serve/" + f"{urllib.parse.quote(relative, safe='/')}?root={index}" + f"&root_scope={target['root_scope']}" + ) + filename = os.path.basename(row["spelled_path"]) return { "id": row["id"], "state": row["state"], "revision": row["revision"], "created_at": row["created_at"], - "filename": os.path.basename(row["spelled_path"]), + "filename": filename, "target": target, "unavailable": target is None, "candidate_count": row["candidate_count"], "limited": row["limited"], + "url": url, + "is_video": os.path.splitext(filename)[1].lower() in VIDEO_EXTENSIONS, } @staticmethod diff --git a/tests/browser/concept-inventory.mjs b/tests/browser/concept-inventory.mjs index 862ad30..e435338 100644 --- a/tests/browser/concept-inventory.mjs +++ b/tests/browser/concept-inventory.mjs @@ -116,6 +116,8 @@ export async function openProduction(page,route,theme,motion='no-preference') { await expect(page.locator('#primary-nav .nav-count')).toHaveCount(3); // SAVED VIEWS' Favorites count (D51), the concept's starred images in the concept fixture. await expect(page.locator('#saved-nav a[data-route="favorites"] .nav-count')).toHaveCount(1); + // SAVED VIEWS' Needs review count (ruling R5): the exact pending count. + await expect(page.locator('#saved-nav a[data-route="review"] .nav-count')).toHaveCount(1); } /** Computed outline of the focused element, after keyboard focus so `:focus-visible` applies. */ diff --git a/tests/browser/server.py b/tests/browser/server.py index e820a5c..2fe0602 100644 --- a/tests/browser/server.py +++ b/tests/browser/server.py @@ -36,6 +36,7 @@ from tests.browser.workspace_library_editor_fixture import ( ) from tests.browser.workspace_vocabulary_fixture import register_vocabulary_commands from tests.browser.workspace_tagging_fixture import register_tagging_commands +from tests.browser.workspace_review_fixture import register_review_commands CANONICAL_ROOT = Path(__file__).resolve().parents[2] CANONICAL_ASSETS = { @@ -193,6 +194,7 @@ def create_app(root: Path) -> web.Application: library_editor_commands = register_library_editor_commands(routes, api, root) register_vocabulary_commands(routes, api, root) register_tagging_commands(routes, api, root, model) + register_review_commands(routes, api, root) reveal_stub = register_reveal_stub(routes, api, root) register_activity_commands(app, routes, api) diff --git a/tests/browser/shell-concept.spec.mjs b/tests/browser/shell-concept.spec.mjs index a4076d7..ce4d17c 100644 --- a/tests/browser/shell-concept.spec.mjs +++ b/tests/browser/shell-concept.spec.mjs @@ -68,12 +68,12 @@ function remainingFooter(footer,spans) { const OVERVIEW_GLYPH='M3 11 12 4l9 7M5 10v10h14V10M10 20v-6h4v6'; /** - * The concept's SAVED VIEWS rows (app.js a331–335) as production lists them: Favorites with its count (D51), then the - * saved views. D50: no "Needs review" row until the tag-review slice (maintainer ruling 2026-10-21). + * The concept's SAVED VIEWS rows (app.js a331–335) as production lists them: Favorites with its count (D51), Needs + * review with the exact pending count (ruling R5), then the saved views. The concept's "3" is sample data: the concept + * fixture records no review items, so production's count reads "0". */ function savedNav(nav) { - const rows=nav.rows.filter(row=>row.text!=='Needs review'); - return {...nav,rows:rows.map((row,index)=>index===rows.length-1?{...row,gapAfter:null}:row)}; + return {...nav,rows:nav.rows.map(row=>row.text==='Needs review'?{...row,count:{...row.count,text:'0'}}:row)}; } /** Recorded shell deviations only (spec §8). Everything else in the concept inventory must equal production. */ @@ -242,8 +242,9 @@ for(const [width,height] of SIZES)test(`a saved view row matches the concept's $ await openProduction(page,'images','dark'); const actual=await save('Save','#workspace-main'); expect(diffInventory(mapShellConcept(reference).chrome.navs,actual.chrome.navs)).toEqual([]); + const referenceRows=reference.chrome.navs.find(nav=>nav.id==='saved-nav').rows; expect(actual.chrome.navs.find(nav=>nav.id==='saved-nav').rows.map(row=>[row.text,row.count])) - .toEqual([['Favorites',reference.chrome.navs.find(nav=>nav.id==='saved-nav').rows[0].count],['Golden hour',null]]); + .toEqual([['Favorites',referenceRows[0].count],['Needs review',{...referenceRows[1].count,text:'0'}],['Golden hour',null]]); }); test('the inventory reports a missing shell element by its path instead of failing',async({page})=>{ diff --git a/tests/browser/workspace-review.spec.mjs b/tests/browser/workspace-review.spec.mjs new file mode 100644 index 0000000..b5e4394 --- /dev/null +++ b/tests/browser/workspace-review.spec.mjs @@ -0,0 +1,209 @@ +// Needs review (#249 PR F, plan Task F1, Mock inventory M4): the SAVED VIEWS row and its count, the Metadata review +// page over the real review routes, and Activity's Review metadata. Items are recorded by the real recorder +// (tests/browser/workspace_review_fixture.py), as the live monitor records a new output. +import { startCoverage, saveCoverage } from '../../tools/vendor/browser-coverage.mjs'; +import { test, expect } from './fixtures.mjs'; + +const review = '/prompt_manager/workspace#/review'; +const main = page => page.locator('#workspace-main'); +const savedNav = page => page.getByRole('navigation', { name: 'Saved views', exact: true }); +const reviewRow = page => savedNav(page).locator('a[data-route="review"]'); +const panel = page => main(page).getByRole('region', { name: 'Resolve the association', exact: true }); +const context = page => panel(page).getByRole('combobox', { name: 'Sampler context', exact: true }); +const cards = page => main(page).locator('.review-queue article.media-card'); +const toast = page => page.locator('#workspace-toast'); +const NOTICE = 'Multiple sampler paths were found. The existing image links were preserved. Choose the correct context or leave them unresolved.'; +const EMPTY = 'Ambiguous images are added when a metadata scan or a new generation finds more than one sampler path.'; +const OPTIONS = ['Sampler 11 · Golden dunes at dusk / Soft negative, blur', 'Sampler 13 · Harbor lights at night / Soft negative, blur', + 'Leave unresolved']; + +async function seed(request, items = 3) { + const response = await request.post('/__test__/review/reset', { data: { items } }); + await expect(response).toBeOK(); + return response.json(); +} +async function pending(request) { + return (await (await request.get('/prompt_manager/review/items?limit=1')).json()).total; +} +/** The page with its first item's sampler paths loaded. */ +async function open(page) { + await page.goto(review); + await expect(context(page)).toBeEnabled(); +} + +test.beforeEach(async ({ page, browserName }) => { + await startCoverage(page, browserName); + page.reviewErrors = []; + page.on('pageerror', error => page.reviewErrors.push(error.message)); +}); +test.afterEach(async ({ page, browserName, request }) => { + await saveCoverage(page, browserName); + expect(page.reviewErrors).toEqual([]); + await request.post('/__test__/jobs/restart'); +}); + +test('queue shows persisted items and the nav count', async ({ page, request }) => { + await seed(request, 3); + await open(page); + // SAVED VIEWS: Favorites, then Needs review with the exact pending count (ruling R5). + await expect(savedNav(page).locator('a').nth(1)).toHaveAttribute('data-route', 'review'); + await expect(reviewRow(page)).toHaveText('Needs review3'); + await expect(reviewRow(page)).toHaveAttribute('aria-current', 'page'); + await expect(page).toHaveTitle('Review · Prompt Manager'); + await expect(page.locator('#workspace-breadcrumb')).toHaveText('Review'); + await expect(main(page).getByRole('heading', { level: 1 })).toHaveText('Metadata review'); + await expect(main(page).locator('.heading p.muted')).toHaveText('Uncertain evidence stays visible until you decide.'); + await expect(main(page).locator('.notice h2')).toHaveText('3 images need a decision'); + await expect(main(page).locator('.notice p')).toHaveText(NOTICE); + await expect(cards(page)).toHaveCount(3); + await expect(cards(page).first()).toHaveClass(/selected/); + await expect(panel(page).locator('p.small')).toHaveText('review_0000.png'); + await expect(context(page).locator('option')).toHaveText(OPTIONS); + await expect(context(page)).toHaveValue(/^[0-9a-f]{64}$/); + await expect(panel(page).locator('.prompt-section.positive .prompt-text')).toHaveText('Golden dunes at dusk'); + await expect(panel(page).locator('.prompt-section.negative .prompt-text')).toHaveText('Soft negative, blur'); + await context(page).selectOption({ label: OPTIONS[1] }); + await expect(panel(page).locator('.prompt-section.positive .prompt-text')).toHaveText('Harbor lights at night'); + await context(page).selectOption({ label: 'Leave unresolved' }); + await expect(panel(page).locator('.prompt-section.positive .prompt-text')).toHaveText('Multiple possible prompts. Source could not be determined.'); + // Info selects another item. + await cards(page).nth(1).getByRole('button', { name: 'Inspect review_0001.png', exact: true }).click(); + await expect(panel(page).locator('p.small')).toHaveText('review_0001.png'); + await expect(cards(page).nth(1)).toHaveClass(/selected/); + await expect(context(page)).toBeEnabled(); + // The card's image opens the shared viewer. + await cards(page).nth(1).getByRole('button', { name: 'Open review_0001.png', exact: true }).click(); + await expect(page.locator('#viewer-dialog')).toBeVisible(); + await page.keyboard.press('Escape'); + await expect(page.locator('#viewer-dialog')).toHaveCount(0); + // Open activity. + await main(page).getByRole('button', { name: 'Open activity', exact: true }).click(); + await expect(page).toHaveURL(/#\/activity$/); +}); + +test('apply candidate links the pair and the item leaves the queue and the count drops', async ({ page, request }) => { + await seed(request, 2); + await open(page); + await expect(reviewRow(page)).toHaveText('Needs review2'); + const decided = page.waitForResponse(item => /\/review\/items\/[^/]+\/decision$/.test(item.url())); + await panel(page).getByRole('button', { name: 'Apply association', exact: true }).click(); + expect((await decided).status()).toBe(200); + await expect(toast(page)).toHaveText('Association accepted.'); + await expect(page).toHaveURL(/#\/images$/); + await expect(reviewRow(page)).toHaveText('Needs review1'); + expect(await pending(request)).toBe(1); +}); + +test('leave unresolved removes the item from pending', async ({ page, request }) => { + await seed(request, 2); + await open(page); + await context(page).selectOption({ label: 'Leave unresolved' }); + await panel(page).getByRole('button', { name: 'Apply association', exact: true }).click(); + await expect(toast(page)).toHaveText('Left unresolved.'); + await expect(page).toHaveURL(/#\/images$/); + expect(await pending(request)).toBe(1); +}); + +test('leave for later writes nothing and goes to Images', async ({ page, request }) => { + await seed(request, 2); + await open(page); + const writes = []; + page.on('request', item => { if (item.method() !== 'GET') writes.push(item.url()); }); + await panel(page).getByRole('button', { name: 'Leave for later', exact: true }).click(); + await expect(page).toHaveURL(/#\/images$/); + expect(writes).toEqual([]); + expect(await pending(request)).toBe(2); +}); + +test('tag image from review opens the shared dialog with the scope note', async ({ page, request }) => { + await seed(request, 1); + await open(page); + await panel(page).getByRole('button', { name: 'Tag image', exact: true }).click(); + const dialog = page.getByRole('dialog', { name: 'Tag image', exact: true }); + await expect(dialog).toBeVisible(); + await expect(dialog.locator('p.tag-scope-note')).toHaveText('Tags belong to this image. Resolve its prompt association separately in Needs review.'); + await dialog.getByRole('textbox', { name: 'Image tags · edit manually', exact: true }).fill('dunes'); + await dialog.getByRole('button', { name: 'Save manual tags', exact: true }).click(); + await expect(toast(page)).toHaveText('1 tags saved for review_0000.png. Prompt association still awaits review.'); + await expect(dialog).toHaveCount(0); + expect(await pending(request)).toBe(1); + await expect(page).toHaveURL(/#\/review$/); +}); + +test('an empty queue shows the empty notice only', async ({ page, request }) => { + await seed(request, 0); + await page.goto(review); + await expect(main(page).locator('.notice h2')).toHaveText('No images need a decision'); + await expect(main(page).locator('.notice p')).toHaveText(EMPTY); + await expect(main(page).locator('.gallery-layout')).toHaveCount(0); + await expect(reviewRow(page)).toHaveText('Needs review0'); +}); + +test('load more appends the next page', async ({ page, request }) => { + await seed(request, 45); + await open(page); + await expect(cards(page)).toHaveCount(40); + const continuation = main(page).locator('.infinite-continuation'); + await expect(continuation.locator('p')).toHaveText('40 of 45 images'); + await continuation.getByRole('button', { name: 'Load more images', exact: true }).click(); + await expect(cards(page)).toHaveCount(45); + await expect(continuation).toHaveCount(0); +}); + +test('a scan job with review items offers Review metadata in Activity', async ({ page, request }) => { + await seed(request, 1); + const started = await (await request.post('/__test__/jobs/start', { data: { total: 10, review_items: 2 } })).json(); + await expect(await request.post(`/__test__/jobs/${started.job.id}/finish`, { data: {} })).toBeOK(); + const plain = await (await request.post('/__test__/jobs/start', { data: { total: 10 } })).json(); + await expect(await request.post(`/__test__/jobs/${plain.job.id}/finish`, { data: {} })).toBeOK(); + await page.goto('/prompt_manager/workspace#/activity'); + const item = main(page).locator(`.activity-item[data-job-id="${started.job.id}"]`); + await expect(item.getByRole('button', { name: 'Review metadata', exact: true })).toBeVisible(); + await expect(main(page).locator(`.activity-item[data-job-id="${plain.job.id}"] button`)).toHaveCount(0); + await page.goto(`/prompt_manager/workspace#/activity/${started.job.id}`); + const actions = main(page).locator('.heading-actions button'); + await expect(actions).toHaveText(['Download report', 'Review metadata']); + await actions.nth(1).click(); + await expect(page).toHaveURL(/#\/review$/); + await expect(main(page).getByRole('heading', { level: 1 })).toHaveText('Metadata review'); +}); + +test('a failed item read says so and Retry reads it again', async ({ page, request }) => { + await seed(request, 1); + await page.route(url => /\/prompt_manager\/review\/items\/[0-9a-f-]{36}$/.test(url.pathname), + route => route.fulfill({ status: 500, contentType: 'application/json', body: JSON.stringify({ success: false, code: 'review-failed', error: 'x' }) }), + { times: 1 }); + await page.goto(review); + await expect(panel(page).locator('.warning-note')).toHaveText('This item could not be read. Select it again to retry.'); + await expect(panel(page).getByText('Could not read prompts from this file.', { exact: true })).toBeVisible(); + await expect(panel(page).getByRole('button', { name: 'Apply association', exact: true })).toHaveAttribute('aria-disabled', 'true'); + await panel(page).getByRole('button', { name: 'Retry prompt details', exact: true }).click(); + await expect(context(page)).toBeEnabled(); + await expect(context(page).locator('option')).toHaveText(OPTIONS); + await expect(panel(page).locator('.warning-note')).toBeHidden(); +}); + +test('a failed queue read offers Retry', async ({ page, request }) => { + await seed(request, 1); + await page.route(url => url.pathname.endsWith('/prompt_manager/review/items') && url.searchParams.get('limit') === '40', + route => route.fulfill({ status: 500, contentType: 'application/json', body: JSON.stringify({ success: false, code: 'review-failed', error: 'x' }) }), + { times: 1 }); + await page.goto(review); + await expect(main(page).getByRole('alert')).toHaveText('The review queue could not be read.'); + await main(page).getByRole('button', { name: 'Retry', exact: true }).click(); + await expect(context(page)).toBeEnabled(); + await expect(main(page).locator('.notice h2')).toHaveText('1 image needs a decision'); +}); + +test('an association conflict keeps the selection and explains', async ({ page, request }) => { + await seed(request, 1); + await open(page); + await page.route(url => /\/decision$/.test(url.pathname), route => route.fulfill({ status: 409, contentType: 'application/json', + body: JSON.stringify({ success: false, code: 'association-conflict', error: 'This image is already linked to another prompt or negative, so this sampler path cannot be attached safely. The existing links were preserved.' }) }), + { times: 1 }); + await panel(page).getByRole('button', { name: 'Apply association', exact: true }).click(); + await expect(panel(page).locator('.warning-note')).toHaveText(/already linked to another prompt/); + await expect(page).toHaveURL(/#\/review$/); + await expect(context(page)).toBeEnabled(); + expect(await pending(request)).toBe(1); +}); diff --git a/tests/browser/workspace-saved-views.spec.mjs b/tests/browser/workspace-saved-views.spec.mjs index ce8f09f..85a6eca 100644 --- a/tests/browser/workspace-saved-views.spec.mjs +++ b/tests/browser/workspace-saved-views.spec.mjs @@ -372,7 +372,7 @@ test('Favorites is the first SAVED VIEWS row, with a real count that follows eve expect(await favoritesRow(page).evaluate(link => [...link.childNodes].map(node => node.nodeType === Node.TEXT_NODE ? `text:${node.textContent}` : `${node.localName}.${node.getAttribute('class')}`))) .toEqual(['svg.icon', 'text:Favorites', 'span.nav-count']); - expect(await savedNav(page).locator('a').evaluateAll(links => links.map(link => link.dataset.route))).toEqual(['favorites']); + expect(await savedNav(page).locator('a').evaluateAll(links => links.map(link => link.dataset.route))).toEqual(['favorites', 'review']); await star(page, '1:shared/same.png'); await expect(toast(page)).toHaveText('Added to favorites.'); diff --git a/tests/browser/workspace.spec.mjs b/tests/browser/workspace.spec.mjs index fff42a6..5bd0677 100644 --- a/tests/browser/workspace.spec.mjs +++ b/tests/browser/workspace.spec.mjs @@ -21,6 +21,8 @@ const routes = [ ['system', 'System', '/prompt_manager/admin'], // #249 D51: SAVED VIEWS' Favorites, Images filtered to favorites. ['favorites', 'Favorites', null], + // #249 PR F (ruling R5): SAVED VIEWS' Needs review, the concept's Metadata review page (its own heading). + ['review', 'Needs review', null, 'Metadata review'], // #249 Activity-1: the concept's Activity page in the tools-and-system group. ['activity', 'Activity', null], ]; @@ -76,10 +78,10 @@ test('workspace starts with real persisted statistics without mutating data', as }); test('all shell destinations deep-link, reload and expose working existing applications', async ({ page, request }) => { - for (const [route, label, legacy] of routes) { + for (const [route, label, legacy, title = label] of routes) { await page.goto(`${workspace}#/${route}`); const main = page.locator('#workspace-main'); - await expect(main.getByRole('heading', { level: 1, name: label, exact: true })).toBeVisible(); + await expect(main.getByRole('heading', { level: 1, name: title, exact: true })).toBeVisible(); await expect(sidebarLink(page, label)).toHaveAttribute('aria-current', 'page'); if (route === 'tags' || route === 'inspect') await expect(main.locator('.handoff')).toHaveCount(0); if (route === 'inspect') await expect(main.locator('#local-file')).toBeAttached(); @@ -88,7 +90,7 @@ test('all shell destinations deep-link, reload and expose working existing appli await expect(await request.get(legacy.split('#')[0])).toBeOK(); } await page.reload(); - await expect(main.getByRole('heading', { level: 1, name: label, exact: true })).toBeVisible(); + await expect(main.getByRole('heading', { level: 1, name: title, exact: true })).toBeVisible(); } // D29 and #249 E3: the legacy gallery and Library are reached from Overview's tool cards, not from their pages. await page.goto(`${workspace}#/overview`); @@ -308,9 +310,9 @@ test('overview reports loading, offers retry after failure and renders real empt test('small screens keep navigation and the theme accessible without horizontal page overflow', async ({ page }) => { await page.setViewportSize({ width: 375, height: 812 }); await page.goto(`${workspace}#/overview`); - for (const [, label] of routes) { + for (const [, label, , title = label] of routes) { await sidebarLink(page, label).click(); - await expect(page.locator('#workspace-main h1')).toHaveText(label); + await expect(page.locator('#workspace-main h1')).toHaveText(title); } await expect(page.getByRole('combobox', { name: 'Theme', exact: true })).toBeVisible(); expect(await page.evaluate(() => document.documentElement.scrollWidth <= window.innerWidth)).toBe(true); diff --git a/tests/browser/workspace_activity_fixture.py b/tests/browser/workspace_activity_fixture.py index 77f102a..3f5fec8 100644 --- a/tests/browser/workspace_activity_fixture.py +++ b/tests/browser/workspace_activity_fixture.py @@ -131,6 +131,12 @@ def register_activity_commands(app, routes, api): scope={ "files": str(data.get("files", "new")), "output_folders": count(data.get("output_folders"), 2), + # A real scan's count of files it added to Needs review. + **( + {"review_items": count(data["review_items"])} + if "review_items" in data + else {} + ), }, runner=fake_runner(ending), groups=SCAN_GROUP_NAMES, diff --git a/tests/browser/workspace_review_fixture.py b/tests/browser/workspace_review_fixture.py new file mode 100644 index 0000000..c17ca92 --- /dev/null +++ b/tests/browser/workspace_review_fixture.py @@ -0,0 +1,116 @@ +"""Owned, deterministic Needs review items for the workspace page (#249 PR F). + +Each file is a PNG whose workflow has two samplers with their own positive +encoders and one shared negative, so the resolver finds it ``ambiguous``; it +is recorded through the real recorder (``record_review_file``), exactly as the +live monitor records a new output. No path or SQL comes from a request. +""" + +from __future__ import annotations + +import json +import os +from pathlib import Path +import shutil +import time +from typing import TYPE_CHECKING + +from aiohttp import web +from PIL import Image +from PIL.PngImagePlugin import PngInfo + +if TYPE_CHECKING: + from py.api import PromptManagerAPI + +POSITIVES = ("Golden dunes at dusk", "Harbor lights at night") +NEGATIVE = "Soft negative, blur" +# More than one queue page (40), so Load more has a second page. +MAX_ITEMS = 45 +_TABLES = ( + "saved_views", + "image_favorites", + "media_review_decisions", + "media_review_items", + "media_image_tags", + "media_annotation_bytes", + "media_annotation_files", + "image_prompt_pairs", + "prompt_pair_tags", + "prompt_pairs", + "generated_images", + "prompt_tags", + "tags", + "prompt_library_state", + "prompts", +) + + +def _graph() -> dict: + graph = { + "1": {"class_type": "CheckpointLoaderSimple", "inputs": {"ckpt_name": "m"}}, + "2": {"class_type": "CLIPTextEncode", "inputs": {"text": NEGATIVE}}, + } + for index, text in enumerate(POSITIVES): + encoder, sampler = str(10 + 2 * index), str(11 + 2 * index) + graph[encoder] = {"class_type": "CLIPTextEncode", "inputs": {"text": text}} + graph[sampler] = { + "class_type": "KSampler", + "inputs": { + "model": ["1", 0], + "positive": [encoder, 0], + "negative": ["2", 0], + }, + } + return graph + + +def _png(path: Path, colour: tuple[int, int, int]) -> None: + info = PngInfo() + info.add_text("prompt", json.dumps(_graph())) + path.parent.mkdir(parents=True, exist_ok=True) + with Image.new("RGB", (96, 64), colour) as image: + image.save(path, pnginfo=info) + + +def seed_review(api: PromptManagerAPI, root: Path, items: int) -> dict: + """Replace this server's disposable rows and files with ``items`` queued files.""" + with api.db.model.get_connection() as conn: + for table in _TABLES: + conn.execute(f"DELETE FROM {table}") + api.close_annotation_captures() + for name in ("output", "input"): + shutil.rmtree(root / name) + (root / name).mkdir() + output = root / "output" + now = time.time() + files = [] + for number in range(items): + relative = f"review/review_{number:04}.png" + _png(output / relative, (40 + number * 4, 90, 140)) + stamp = now - 60 - number + os.utime(output / relative, (stamp, stamp)) + outcome = api.record_review_file(str(output / relative)) + if outcome != "recorded": + raise RuntimeError(f"{relative} was not recorded: {outcome}") + files.append({"path": relative, "filename": Path(relative).name}) + api.invalidate_gallery_cache() + return {"files": files, "positives": list(POSITIVES), "negative": NEGATIVE} + + +def register_review_commands( + routes: web.RouteTableDef, api: PromptManagerAPI, root: Path +) -> None: + """``POST /__test__/review/reset {items}``: the fixed queue, 0 to MAX_ITEMS.""" + + @routes.post("/__test__/review/reset") + async def reset(request: web.Request) -> web.Response: + data = await request.json() if request.can_read_body else {} + if request.query or not isinstance(data, dict) or set(data) - {"items"}: + raise web.HTTPBadRequest() + items = data.get("items", 3) + if isinstance(items, bool) or not isinstance(items, int): + raise web.HTTPBadRequest() + if not 0 <= items <= MAX_ITEMS: + raise web.HTTPBadRequest() + manifest = await api._run_in_executor(seed_review, api, root, items) + return web.json_response({"success": True, **manifest}) diff --git a/tests/js/workspace-review-state.test.js b/tests/js/workspace-review-state.test.js new file mode 100644 index 0000000..0139af7 --- /dev/null +++ b/tests/js/workspace-review-state.test.js @@ -0,0 +1,343 @@ +// Run with: node --test tests/js/workspace-review-state.test.js +const test = require('node:test'); +const assert = require('node:assert/strict'); +const { createApiClient } = require('../../web/js/api-client.js'); + +// Only an absent module is tolerated, so RED is an assertion failure, not a crash. +const source = import('../../web/js/workspace/review-state.mjs').catch(error => { + if (error.code !== 'ERR_MODULE_NOT_FOUND') throw error; + return {}; +}); +async function load() { + const module = await source; + assert.equal(typeof module.createReviewController, 'function', 'review-state.mjs is not implemented'); + return module; +} + +const EPOCH = '11111111-1111-4111-8111-111111111111'; +const SCOPE = 'c'.repeat(64); +const SHA = 'd'.repeat(64); +const PRINT = 'e'.repeat(64); +const HASH_A = 'a'.repeat(64); +const HASH_B = 'b'.repeat(64); +const HASH_C = 'f'.repeat(64); +const ITEMS = '/prompt_manager/review/items'; +const OBSERVE = '/prompt_manager/image-annotations/observe'; +const CAPTURE = '/prompt_manager/image-annotations/capture/'; + +const flush = () => new Promise(resolve => setImmediate(resolve)); +async function settle(times = 8) { for (let i = 0; i < times; i++) await flush(); } +function response(status, body) { + return { ok: status >= 200 && status < 300, status, statusText: `Status ${status}`, json: async () => body }; +} +const fail = (status, code, error = `failure ${code}`, extra = {}) => response(status, { success: false, code, error, ...extra }); + +function item(id, overrides = {}) { + const path = `dunes/${id}.png`; + return { + id, state: 'pending', revision: 0, created_at: `2026-10-22T10:00:0${id.length}+00:00`, filename: `${id}.png`, + target: { root_index: 0, root_scope: SCOPE, path }, unavailable: false, candidate_count: 3, limited: false, + url: `/prompt_manager/images/serve/${path}?root=0&root_scope=${SCOPE}`, is_video: false, ...overrides, + }; +} +const candidate = (hash, sampler, label, positive, negative, state = 'resolved') => ({ + hash, sampler_id: sampler, state, reason: '', label, + positive: positive === null ? null : { text: positive }, negative: negative === null ? null : { text: negative }, +}); +function detailOf(entry, overrides = {}) { + return { + success: true, library_epoch: EPOCH, ...entry, total: 2, sha256: SHA, dependency_fingerprint: PRINT, + candidates: [ + candidate(HASH_C, '9', 'No positive / Soft negative', null, 'blur', 'unresolved'), + candidate(HASH_A, '12', 'Dunes / Soft negative', 'golden dunes', 'blur'), + candidate(HASH_B, '28', 'Dunes / Harsh negative', 'golden dunes', 'noise'), + ], + ...overrides, + }; +} +const page = (items, overrides = {}) => ({ success: true, library_epoch: EPOCH, total: items.length, items, next_cursor: null, ...overrides }); +const observed = (token = 'capture-1') => ({ + success: true, library_epoch: EPOCH, target: item('one').target, filename: 'one.png', title: 'one.png', tags: [], tag_revision: 0, + capture: { token, sha256: SHA, preview_url: CAPTURE + token, size: 10, width: 8, height: 8, media_type: 'image', + expires_at: '2026-10-22T12:00:00+00:00', generate: { supported: true, reason: null } }, +}); + +/** Real createApiClient over a fetch whose answers come from routes['METHOD path'](call). */ +function harness(routes = {}) { + const calls = []; + const fetch = (url, init = {}) => { + const call = { url, method: init.method, body: init.body === undefined ? undefined : JSON.parse(init.body) }; + calls.push(call); + const path = url.startsWith(CAPTURE) ? CAPTURE : url.split('?')[0]; + const route = routes[`${call.method} ${path}`]; + return new Promise((resolve, reject) => { + init.signal?.addEventListener('abort', () => reject(new DOMException('Aborted', 'AbortError'))); + Promise.resolve().then(() => (route ? route(call) : response(200, { success: true }))).then(resolve, reject); + }); + }; + return { + api: createApiClient(fetch), calls, + of: (method, prefix) => calls.filter(call => call.method === method && call.url.startsWith(prefix)), + }; +} +const QUEUE = [item('one'), item('two')]; +function routes(extra = {}) { + return { + [`GET ${ITEMS}`]: () => response(200, page(QUEUE)), + [`GET ${ITEMS}/one`]: () => response(200, detailOf(QUEUE[0])), + [`GET ${ITEMS}/two`]: () => response(200, detailOf(QUEUE[1], { candidates: [], limited: true, candidate_count: 0 })), + [`POST ${OBSERVE}`]: () => response(200, observed()), + [`POST ${ITEMS}/one/decision`]: call => response(200, { success: true, library_epoch: EPOCH, total: 1, + item: { ...QUEUE[0], state: call.body.choice === 'unresolved' ? 'unresolved' : 'selected', revision: 1 } }), + ...extra, + }; +} +async function loaded(extra) { + const { createReviewController } = await load(); + const h = harness(routes(extra)); + const c = createReviewController({ api: h.api }); + await c.load(); + await settle(); + return { c, h }; +} + +test('notice titles for 0, 1 and 3', async () => { + const { noticeTitle } = await load(); + assert.equal(noticeTitle(0), 'No images need a decision'); + assert.equal(noticeTitle(1), '1 image needs a decision'); + assert.equal(noticeTitle(3), '3 images need a decision'); + assert.equal(noticeTitle(1234), '1,234 images need a decision'); +}); + +test('loads pending items and the exact count', async () => { + const { c, h } = await loaded(); + assert.equal(c.state.status, 'ready'); + assert.equal(c.state.total, 2); + assert.deepEqual(c.state.items.map(entry => entry.id), ['one', 'two']); + const [list] = h.of('GET', ITEMS); + assert.equal(list.url, `${ITEMS}?limit=40`); + // The first item is selected, as the concept selects its first queue image. + assert.equal(c.state.selectedId, 'one'); + assert.ok(Object.isFrozen(c.state)); +}); + +test('selecting a card loads detail and option labels, first workable preselected, unusable disabled', async () => { + const { c } = await loaded(); + assert.equal(c.state.detailStatus, 'ready'); + assert.deepEqual(c.state.options.map(option => [option.label, option.disabled]), [ + ['Sampler 9 · No positive / Soft negative', true], + ['Sampler 12 · Dunes / Soft negative', false], + ['Sampler 28 · Dunes / Harsh negative', false], + ['Leave unresolved', false], + ]); + assert.equal(c.state.options.at(-1).value, 'unresolved'); + assert.equal(c.state.choice, HASH_A); + assert.deepEqual(c.state.prompts, { positive: { status: 'resolved', text: 'golden dunes' }, negative: { status: 'resolved', text: 'blur' } }); + c.choose(HASH_C); + assert.equal(c.state.choice, HASH_A, 'a disabled option cannot be chosen'); + c.choose(HASH_B); + assert.equal(c.state.prompts.negative.text, 'noise'); + c.choose('unresolved'); + assert.equal(c.state.prompts.positive.status, 'ambiguous'); + await c.select('two'); + await settle(); + assert.equal(c.state.selectedId, 'two'); + assert.deepEqual(c.state.options.map(option => option.label), ['Leave unresolved']); + assert.equal(c.state.choice, 'unresolved'); + assert.match(c.state.warning, /too many sampler paths/i); +}); + +test('apply sends the selected candidate hash or unresolved with a fresh capture', async () => { + const { c, h } = await loaded(); + c.choose(HASH_B); + const done = await c.apply(); + assert.equal(done, true); + const [observe] = h.of('POST', OBSERVE); + assert.deepEqual(observe.body, { target: QUEUE[0].target }); + const [decision] = h.of('POST', `${ITEMS}/one/decision`); + assert.deepEqual(decision.body, { expected_epoch: EPOCH, expected_revision: 0, capture_token: 'capture-1', sha256: SHA, + dependency_fingerprint: PRINT, choice: HASH_B }); + assert.equal(c.state.phase, 'done'); + assert.equal(c.state.total, 1); + assert.equal(c.state.toast, 'Association accepted.'); + // The server releases a used capture itself. + assert.equal(h.of('DELETE', CAPTURE).length, 0); + + const again = await loaded(); + again.c.choose('unresolved'); + assert.equal(await again.c.apply(), true); + const [unresolved] = again.h.of('POST', `${ITEMS}/one/decision`); + assert.equal(unresolved.body.choice, 'unresolved'); + assert.equal(unresolved.body.capture_token, 'capture-1'); + assert.equal(again.c.state.toast, 'Left unresolved.'); +}); + +test('an unavailable file can only be left unresolved, without a capture', async () => { + const gone = item('one', { target: null, unavailable: true, url: null }); + const { c, h } = await loaded({ + [`GET ${ITEMS}`]: () => response(200, page([gone])), + [`GET ${ITEMS}/one`]: () => response(200, detailOf(gone)), + }); + assert.ok(c.state.options.slice(0, -1).every(option => option.disabled)); + assert.equal(c.state.choice, 'unresolved'); + assert.match(c.state.warning, /not under a current image source/i); + assert.equal(await c.apply(), true); + assert.equal(h.of('POST', OBSERVE).length, 0); + const [decision] = h.of('POST', `${ITEMS}/one/decision`); + assert.equal(decision.body.capture_token, null); + assert.equal(decision.body.sha256, SHA); +}); + +test('association conflict keeps the selection and shows the inline warning', async () => { + const message = 'This image is already linked to another prompt or negative, so this sampler path cannot be attached safely.'; + const { c, h } = await loaded({ [`POST ${ITEMS}/one/decision`]: () => fail(409, 'association-conflict', message) }); + assert.equal(await c.apply(), false); + assert.equal(c.state.selectedId, 'one'); + assert.equal(c.state.choice, HASH_A); + assert.equal(c.state.phase, 'idle'); + assert.equal(c.state.warning, message); + // The refused decision's capture is released. + assert.equal(h.of('DELETE', CAPTURE).length, 1); + assert.equal(h.of('GET', `${ITEMS}?`).length, 1, 'no reload'); +}); + +test('storage-changed requires reload', async () => { + let lists = 0; + const { c, h } = await loaded({ + [`GET ${ITEMS}`]: () => { lists += 1; return response(200, page(QUEUE)); }, + [`POST ${ITEMS}/one/decision`]: () => fail(409, 'storage-changed', 'Prompt storage was restored; reload the review queue.'), + }); + assert.equal(await c.apply(), false); + await settle(); + assert.equal(lists, 2, 'the queue is read again under the new epoch'); + assert.equal(c.state.warning, 'Prompt storage was restored; reload the review queue.'); + assert.equal(c.state.canRetry, false); + assert.equal(h.of('DELETE', CAPTURE).length, 1); +}); + +test('lost response keeps the selection and retry resends once', async () => { + let attempts = 0; + const { c, h } = await loaded({ + [`POST ${ITEMS}/one/decision`]: call => { + attempts += 1; + if (attempts === 1) return Promise.reject(new TypeError('Failed to fetch')); + return response(200, { success: true, library_epoch: EPOCH, total: 1, item: { ...QUEUE[0], state: 'selected', revision: 1 } }); + }, + }); + assert.equal(await c.apply(), false); + assert.equal(c.state.selectedId, 'one'); + assert.equal(c.state.canRetry, true); + assert.match(c.state.warning, /may not have been saved/i); + assert.equal(h.of('DELETE', CAPTURE).length, 0, 'the capture is kept for the retry'); + const [first] = h.of('POST', `${ITEMS}/one/decision`); + assert.equal(await c.retry(), true); + const sent = h.of('POST', `${ITEMS}/one/decision`); + assert.equal(sent.length, 2); + assert.deepEqual(sent[1].body, first.body, 'the identical decision is resent'); + assert.equal(h.of('POST', OBSERVE).length, 1, 'no second capture'); + assert.equal(await c.retry(), false, 'nothing left to resend'); + assert.equal(h.of('POST', `${ITEMS}/one/decision`).length, 2); +}); + +test('a retry answered by review-conflict reloads the queue', async () => { + let attempts = 0, lists = 0; + const { c } = await loaded({ + [`GET ${ITEMS}`]: () => { lists += 1; return response(200, page(lists === 1 ? QUEUE : [QUEUE[1]])); }, + [`POST ${ITEMS}/one/decision`]: () => { + attempts += 1; + if (attempts === 1) return Promise.reject(new TypeError('Failed to fetch')); + return fail(409, 'review-conflict', 'This item was decided or changed since it was opened. Reload the queue.'); + }, + }); + await c.apply(); + assert.equal(await c.retry(), false); + await settle(); + assert.equal(lists, 2); + assert.equal(c.state.selectedId, 'two'); +}); + +test('a review-conflict on a still pending item reads its detail again', async () => { + let details = 0, decisions = 0; + const { c, h } = await loaded({ + [`GET ${ITEMS}/one`]: () => { details += 1; return response(200, detailOf({ ...QUEUE[0], revision: details - 1 })); }, + [`POST ${ITEMS}/one/decision`]: () => { + decisions += 1; + return decisions === 1 ? fail(409, 'review-conflict', 'This item was decided or changed since it was opened. Reload the queue.') + : response(200, { success: true, library_epoch: EPOCH, total: 1, item: { ...QUEUE[0], state: 'selected', revision: 2 } }); + }, + }); + assert.equal(await c.apply(), false); + await settle(); + assert.equal(details, 2, 'the item is read again'); + assert.equal(c.state.detail.revision, 1); + assert.equal(c.state.warning, 'This item was decided or changed since it was opened. Reload the queue.'); + assert.equal(await c.apply(), true); + assert.equal(h.of('POST', `${ITEMS}/one/decision`).at(-1).body.expected_revision, 1); +}); + +test('leave for later writes nothing', async () => { + const { c, h } = await loaded(); + c.leave(); + assert.equal(c.state.phase, 'done'); + assert.equal(c.state.toast, null); + assert.equal(h.of('POST', '/').length, 0); + assert.equal(h.of('DELETE', '/').length, 0); +}); + +test('load more pages with the cursor and keeps the selection', async () => { + const more = [item('three')]; + const { c, h } = await loaded({ + [`GET ${ITEMS}`]: call => response(200, call.url.includes('cursor=') + ? page(more, { total: 3 }) : page(QUEUE, { total: 3, next_cursor: 'abc_-1' })), + }); + assert.equal(c.state.nextCursor, 'abc_-1'); + await c.loadMore(); + assert.equal(h.of('GET', ITEMS).at(-1).url, `${ITEMS}?limit=40&cursor=abc_-1`); + assert.deepEqual(c.state.items.map(entry => entry.id), ['one', 'two', 'three']); + assert.equal(c.state.nextCursor, null); + assert.equal(c.state.selectedId, 'one'); +}); + +test('a failed queue read is an error state and load retries', async () => { + let lists = 0; + const { c } = await loaded({ [`GET ${ITEMS}`]: () => { lists += 1; return lists === 1 ? fail(500, 'review-failed') : response(200, page(QUEUE)); } }); + assert.equal(c.state.status, 'error'); + await c.load(); + await settle(); + assert.equal(c.state.status, 'ready'); +}); + +test('tag save toast carries the review suffix and the item stays pending', async () => { + const { c, h } = await loaded({ + ['GET /prompt_manager/autotag/models']: () => response(200, { success: true, models: {}, default_prompt: 'x', + wd14_general_threshold: 0.35, wd14_character_threshold: 0.85, suggestion_limit: 10 }), + ['PUT /prompt_manager/image-annotations/tags']: call => response(200, { success: true, library_epoch: EPOCH, sha256: SHA, + tags: call.body.tags, tag_revision: 1, replayed: false }), + }); + const { createImageTaggingController } = await import('../../web/js/workspace/image-tagging-state.mjs'); + const before = h.calls.filter(call => call.url.startsWith(ITEMS)).length; + // The Tag dialog opened from Needs review is the shared controller with the review source. + const tagging = createImageTaggingController({ api: h.api, target: QUEUE[0].target, source: 'review' }); + assert.equal(tagging.state.showScopeNote, true); + await tagging.open(); + await settle(); + tagging.setManual('dunes'); + await tagging.save(); + await settle(); + assert.equal(tagging.state.toast, '1 tags saved for one.png. Prompt association still awaits review.'); + tagging.dispose(); + assert.equal(h.calls.filter(call => call.url.startsWith(ITEMS)).length, before, 'a tag save neither decides nor re-reads the queue'); + assert.deepEqual(c.state.items.map(entry => [entry.id, entry.state]), [['one', 'pending'], ['two', 'pending']]); + assert.equal(c.state.phase, 'idle'); +}); + +test('dispose releases a kept capture and stops reads', async () => { + const { c, h } = await loaded({ [`POST ${ITEMS}/one/decision`]: () => Promise.reject(new TypeError('Failed to fetch')) }); + await c.apply(); + c.dispose(); + await settle(); + const released = h.of('DELETE', CAPTURE); + assert.equal(released.length, 1); + assert.equal(released[0].url, CAPTURE + 'capture-1'); + assert.equal(await c.retry(), false); +}); diff --git a/tests/js/workspace-state.test.js b/tests/js/workspace-state.test.js index a02129a..e79f0c5 100644 --- a/tests/js/workspace-state.test.js +++ b/tests/js/workspace-state.test.js @@ -4,7 +4,7 @@ const source = import('../../web/js/workspace/state.mjs'); test('workspace routes resolve only supported deep links', async () => { const { resolveRoute } = await source; - for (const route of ['overview', 'images', 'library', 'tags', 'inspect', 'system', 'activity']) { + for (const route of ['overview', 'images', 'library', 'tags', 'inspect', 'system', 'activity', 'review']) { assert.equal(resolveRoute(`#/${route}`).id, route); } for (const hash of ['', '#', '#/unknown', '#/images/other', '#/