From 3dc7d6f26fa60c690e12379916d32510b740b21e Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Thu, 22 Oct 2026 15:04:23 -0700 Subject: [PATCH] fix(annotations): final review round for the Tag image dialog - Option fields show the server defaults that Generate sends: the controller bumps an options revision on open and on a model change, and the panel re-renders only then (I1). - A tag conflict shows the current set at once ("Saved now: ...") and puts it ahead of the draft in the manual field, so the next Save sends what is shown (I2). - The opening sentence is the concept's on-screen copy; "Remove anything you do not want to add." leads the chips only while chips exist; the comparison's opened mapping is gone (I3). - ApiClient gains an additive keepalive option; the capture release and the Save PUT use it, and pagehide closes the dialog (I4). - Closing during a save keeps the capture until the PUT settles, then releases it and still reports the outcome (toast, page refresh) (I5). - A refresh asked during a page load is pending and runs once it settles; re-reads use pages up to 200 files; a failed re-read toasts; Load more waits for a refresh (I6, M2, M3). - The selection bar drops checked files a refreshed result no longer lists (I7). - D116 records Retry; after a failed Generate it takes Generate's own path, and after a changed storage, file or expired capture it observes again keeping the draft (I9, M4, M5). - A save made from Tags marks a filtered Images visit stale (M6). - images.mjs drops under 800 lines: the tagging wiring moves to images-tagging.mjs and the favorites star to images-favorites.mjs (unchanged), and reset() reuses dropSelection() (M1). - Restore the activity-scan.mjs coverage floors; drop the duplicate paired captures test (I8, M9). --- TESTING.md | 81 ++++++--- .../browser/tagging-canonical-comparison.mjs | 19 +- tests/browser/tagging-concept.spec.mjs | 33 ++-- .../workspace-tagging-lifecycle.spec.mjs | 71 ++++++++ tests/browser/workspace-tagging.spec.mjs | 171 +++++++++++++++--- tests/browser/workspace_tagging_fixture.py | 55 ++++++ tests/js/api-client.test.js | 11 ++ .../js/workspace-image-tagging-state.test.js | 157 +++++++++++++++- tests/js/workspace-images-state.test.js | 90 ++++++++- web/js/api-client.js | 10 +- web/js/workspace/image-tagging-state.mjs | 125 +++++++++---- web/js/workspace/image-tagging.mjs | 70 ++++--- web/js/workspace/images-favorites.mjs | 81 +++++++++ web/js/workspace/images-selection.mjs | 9 + web/js/workspace/images-state.mjs | 38 +++- web/js/workspace/images-tagging.mjs | 42 +++++ web/js/workspace/images.mjs | 129 ++++--------- 17 files changed, 941 insertions(+), 251 deletions(-) create mode 100644 tests/browser/workspace-tagging-lifecycle.spec.mjs create mode 100644 web/js/workspace/images-favorites.mjs create mode 100644 web/js/workspace/images-tagging.mjs diff --git a/TESTING.md b/TESTING.md index e8a9f96..e77e01e 100644 --- a/TESTING.md +++ b/TESTING.md @@ -1070,18 +1070,28 @@ concept's Tag (`app.js` mediaCard). Tag opens the concept's **Tag image** dialog scrolls. Focus starts on × and returns to the Tag button. The preview is the capture's own `preview_url` (through `ApiClient.resolveUrl`): an `img`, or a `video preload="metadata" muted`. The capture is released on ×, Escape, Cancel, -Save, a route change (the shell closes its dialog) and `pagehide`. Generate is -explicit and uses the server's real defaults (`GET /prompt_manager/autotag/models`); -an invalid field reports native validity and sends nothing. Save writes the manual +Save, a route change (the shell closes its dialog) and `pagehide`, which closes the +dialog (a page restored from the back/forward cache shows no dead dialog); the +release `DELETE` and the Save `PUT` are sent with `keepalive` (`ApiClient`'s +additive option), so they finish while the page unloads. A save still in flight +when the dialog closes keeps its capture until the `PUT` settles, then releases it +and still reports the outcome: the saved toast and every page's refresh, or the +failure as a toast. Generate is explicit and uses the server's real defaults +(`GET /prompt_manager/autotag/models`), and the fields show exactly those values +(they are re-rendered when the observation lands and on a model change, never while +typing); an invalid field reports native validity and sends nothing. A tag conflict +shows the current set at once: the status line appends "Saved now: ." and the +manual field holds the current set ahead of the draft (repeats dropped), so the next +Save sends what is shown against the current revision. Save writes the manual names, then the remaining chips, deduplicated case-insensitively (first spelling wins), and toasts the mock's copy verbatim, " tags saved for ." even for one tag, plus " Prompt association still awaits review." from Needs review. Tagging one file never changes another file (an equal-byte copy included), a prompt, a pair or a link. States the mock lacks (generating, busy, conflict, changed storage or file, expired capture, unconfirmed save, unavailable model, -video) are text in `#tag-generation-status` only; **Retry** (ruling T5) leads the -actions while the controller allows it (failed observation, failed Generate, -unconfirmed save), where the other workspace dialogs place their Retry. The Tags +video) are text in `#tag-generation-status` only; **Retry** (D116) leads the +actions while the controller allows it, where the other workspace dialogs place +their Retry. The Tags page re-reads its lists after every confirmed save, so the Images counts follow. The image-tagging plan's deviation numbers were renumbered by +23 because the @@ -1093,13 +1103,13 @@ Inspect plan keeps D60–D66 (plan D60 → D83, D61 → D84, D65 → D88, D68 confidence (`98%` for WD14, "Suggested" for language models) and a × "Remove ". A chip has the concept `.chip`'s 4px corner (the later `.tag-suggestion` row rule would give 5px) around the row's own 12px text. At most the requested - count, all preselected. The help copy reads "Remove anything you do not want to - add." and removing the last chip shows "All suggestions removed. Generate again or - enter tags manually." The concept's markup carries that initial sentence ("Uncheck - anything…"), but its `renderTagModelOptions` (a1062–1076) replaces it with "Generate - suggestions with the selected model before approving." as soon as the dialog opens, - so the concept never shows it; production shows ruling T4's sentence on open and - the model-changed sentence after a model change. + count, all preselected. While chips exist, ruling T4's "Remove anything you do not + want to add." (`p.small`) leads them inside `#tag-suggestions`; removing the last + chip shows "All suggestions removed. Generate again or enter tags manually." The + opening sentence is the concept's on-screen copy, "Generate suggestions with the + selected model before approving.": the concept's markup carries another ("Choose a + model… Uncheck anything…"), but its `renderTagModelOptions` (a1062–1076) replaces it + as soon as the dialog opens, so the opened state is compared unmapped. - **D84** (plan D61, rulings T1/T2): the Tagging model options are legacy's four engines in the mock's order with legacy labels and sizes (`WD14 SwinV2 · ~400 MB`, `WD14 ViT · ~400 MB`, `GGUF JoyCaption (Recommended) · ~4.5 GB` in the @@ -1125,6 +1135,18 @@ Inspect plan keeps D60–D66 (plan D60 → D83, D61 → D84, D65 → D88, D68 its "Auto-tag images" always opens the dialog. Production opens it for the image selected in Images when it was last left; with none it is `aria-disabled` and pressing it toasts "Select an image in Images first." (no new chrome). +- **D116** (ruling T5): **Retry** leads `.dialog-actions` (before Cancel and the + primary) while a failed observation, a failed Generate, an unconfirmed save or an + answer that needs a fresh observation (changed storage or file, expired capture) + can be retried; the concept has no failure states, so it has no Retry. After a + failed Generate it takes Generate's own path (an invalid field reports its + validity); an unconfirmed save resends the identical body; after a changed storage, + file or expired capture it observes again and keeps the typed draft (when the saved + set changed meanwhile, the current set leads the draft and the status line says + "The tags of this image changed since the dialog opened. Saved now: ."). + Browser tests: "D116: Retry leads the actions after an unconfirmed save…", "D116: + Retry after an expired capture observes again and keeps the typed draft (M5)" and + the paired `production-unknown-outcome` capture. - **D29** (no prototype copy) in the dialog: no "Prototype · suggestions are simulated…" note, the status reads "Suggestions from . Approve tags to save." (not "Sample suggestions from …"), and the toast has no " in this tab". @@ -1136,11 +1158,23 @@ filter or a search is active the saved file may start or stop matching, so Image re-reads its loaded range in place (`createImageStream().refresh()`: the same query from the first page until the loaded count, published once). The cards, the scroll position and the selection stay while the selected file still matches; when it no -longer does, the panel follows the collection (its first file), as after a reload. -A refresh never runs beside a page load already in flight. Browser tests: "the -post-save re-read keeps the panel content until the new read lands", "a save under a -Tags filter re-reads the listing: …" and "a save under a search keeps the scroll -position and the loaded range". +longer does, the panel follows the collection (its first file), as after a reload, +and the selection bar keeps only the checked files still listed, so its count is what +Set category and Export metadata apply to. A refresh never runs beside a page load +already in flight: asked during one, it is pending and runs once that page settles +(success or failure; a reset drops it), and Load more waits for a running refresh +(it neither loads nor moves focus). The re-read asks for pages as large as the rest of +the loaded range (at most 200 files, at least one batch), one after another (each +cursor comes from the page before); a failed re-read toasts "Images could not be +refreshed after the tag save. Refresh image collection to retry." and keeps the +cards. A save made while Images is left (Tags › Auto-tag images) marks a saved Images +visit with a Tags filter or a search stale: returning lists it again with its +filters instead of restoring the old cards. Browser tests: "the post-save re-read +keeps the panel content until the new read lands", "a save under a Tags filter +re-reads the listing: …", "a save under a search keeps the scroll position and the +loaded range", "a failed listing re-read after a save says so and keeps the cards +(M2)" and "a tag save from Tags reloads a filtered Images visit instead of restoring +it (M6)"; the pending refresh and the page sizes are unit-tested. Concept comparison (`tests/browser/tagging-concept.spec.mjs` with `tagging-canonical-comparison.mjs`, plan Task C4): production runs on the concept @@ -1158,13 +1192,13 @@ production-versus-concept difference of PR C and its record: | Difference | Record | | --- | --- | -| Removable chips with × instead of checked checkbox rows; "Remove anything…"; the all-removed copy; the initial sentence on open | D83 (ruling T4) | +| Removable chips with × instead of checked checkbox rows; "Remove anything…" while chips exist; the all-removed copy | D83 (ruling T4) | | Legacy engine labels and values, GGUF in the Ollama slot, server default instructions | D84 (rulings T1/T2) | | Suggestions count field | D91 (ruling T3-bulk) | | No prototype note, "Suggestions from …", no " in this tab" | D29 | | Image tags first in the inspector and viewer chip row | D88 (ruling T7/C2) | | Auto-tag images `aria-disabled` without a selected image | D115 | -| Retry leads the actions when the controller allows it | ruling T5 (state text only otherwise) | +| Retry leads the actions when the controller allows it | D116 (ruling T5) | | States the concept lacks (busy, conflict, changed storage or file, unconfirmed save, unavailable model, video) | Global Constraints: `#tag-generation-status` text only | Native image tagging (#249 PR B routes, PR C UI): `POST @@ -1193,12 +1227,17 @@ no model files; every test asserts no download request), `tests/js/workspace-inspection-state.test.js` and `tests/browser/inspector-concept.spec.mjs` ("image tags saved on the concept image keep the inspector equal to the concept"), and for the comparison `tests/browser/tagging-concept.spec.mjs` and -`tests/js/workspace-images-state.test.js` (the in-place refresh). +`tests/js/workspace-images-state.test.js` (the in-place refresh), and on all four +projects `tests/browser/workspace-tagging-lifecycle.spec.mjs` (leaving the page with +the dialog open releases the capture with no console error; closing during a save +releases it after the save and still reports it). ```sh node --test tests/js/workspace-image-tagging*.test.js tests/js/workspace-images-lifecycle.test.js npm run --prefix tools/vendor test:workflows -- workspace-tagging.spec.mjs tagging-concept.spec.mjs \ workspace-vocabulary.spec.mjs images-concept.spec.mjs --project chromium --project touch +npm run --prefix tools/vendor test:workflows -- workspace-tagging-lifecycle.spec.mjs tagging-concept.spec.mjs \ + --project firefox --project webkit ``` ### Prompt library cards and recorded deviations (#249 PR E) diff --git a/tests/browser/tagging-canonical-comparison.mjs b/tests/browser/tagging-canonical-comparison.mjs index 9edd3b7..d9ff0fd 100644 --- a/tests/browser/tagging-canonical-comparison.mjs +++ b/tests/browser/tagging-canonical-comparison.mjs @@ -101,17 +101,18 @@ const CONCEPT_LABELS=Object.freeze({'WD14 SwinV2 · fast classifier':'WD14 SwinV /** * The concept inventory with the recorded deviations applied: * - no prototype note (Global Constraints: `Prototype · suggestions are simulated…` is not shipped); - * - D83 (ruling T4): "Remove anything you do not want to add." and every suggestion a chip with a `Remove ` × - * instead of a checked checkbox row: the concept's own `.chip` corner (`chip`: a concept `.chip` inventory, the - * heading's size chip) around the row's own text (12px) in the checked row's colours (accent border, `--soft-blue`); + * - D83 (ruling T4): while suggestions exist, "Remove anything you do not want to add." leads them, and every + * suggestion is a chip with a `Remove ` × instead of a checked checkbox row: the concept's own `.chip` corner + * (`chip`: a concept `.chip` inventory, the heading's size chip) around the row's own text (12px) in the checked + * row's colours (accent border, `--soft-blue`); * - D84 (plan D61): the legacy engine labels and values, and the server's default instructions (`instructions`); * - D91 (plan D68): a Suggestions count field after the model's own fields, on the concept's number field tokens * (`numberField`: the concept's General confidence field) with the configured count (`limit`); - * - D29: the prototype's "Sample suggestions from …" status reads "Suggestions from …"; - * - D83, `opened`: on open the concept's renderTagModelOptions (a1062–1076) replaces its own initial sentence with the - * model-changed one at once; production shows the initial sentence of ruling T4 until a model change. + * - D29: the prototype's "Sample suggestions from …" status reads "Suggestions from …". + * The opened state is compared unmapped: both show the concept's on-screen "Generate suggestions with the selected + * model before approving." (its renderTagModelOptions, a1062–1076, replaces the markup's sentence at once). */ -export function mapConceptTagging(reference,{instructions,limit=10,chip,numberField,opened=false}) { +export function mapConceptTagging(reference,{instructions,limit=10,chip,numberField}) { const [select,...own]=reference.modelPanel.fields; const suggestionsField={...numberField,label:'Suggestions',control:{...numberField.control,id:'image-suggestion-limit', value:String(limit),min:'1',max:'100',step:'1'}}; @@ -119,9 +120,7 @@ export function mapConceptTagging(reference,{instructions,limit=10,chip,numberFi const rows=reference.suggestions.rows.map(row=>({...row, control:row.control?.kind==='checkbox'&&row.control.checked?{kind:'remove',label:`Remove ${row.name}`}:row.control, borderTopLeftRadius:chip.borderTopLeftRadius})); - const initial='Choose a model, then generate tags. Remove anything you do not want to add.'; - const copy=opened&&reference.suggestions.copy==='Generate suggestions with the selected model before approving.'?initial: - reference.suggestions.copy?.replace('Uncheck anything you do not want to add.','Remove anything you do not want to add.')??null; + const copy=rows.length&&reference.suggestions.copy===null?'Remove anything you do not want to add.':reference.suggestions.copy; const status=reference.suggestions.status.text.replace(/^Sample suggestions from (.+)\. Approve tags to save\.$/, (_,label)=>`Suggestions from ${CONCEPT_LABELS[label]??label}. Approve tags to save.`); return {...reference, diff --git a/tests/browser/tagging-concept.spec.mjs b/tests/browser/tagging-concept.spec.mjs index cea982b..ac15236 100644 --- a/tests/browser/tagging-concept.spec.mjs +++ b/tests/browser/tagging-concept.spec.mjs @@ -43,8 +43,8 @@ async function conceptPath(request) { } /** - * Close an open Tag dialog before leaving the page: leaving with it open releases its capture during unload, which - * WebKit reports as a failed fetch. + * Close an open Tag dialog before opening the next page, so each comparison starts from a settled page. Leaving with + * the dialog open is covered on its own (workspace-tagging-lifecycle.spec.mjs: the keepalive release on pagehide). */ async function leave(page) { if (await tagDialog(page).count()) { @@ -84,7 +84,7 @@ const inventory = async page => { }; /** The concept and production inventories of one dialog state, the concept mapped (D29, D83, D84, D91). */ -async function compareState(page, { theme, act = async () => {}, opened = false }) { +async function compareState(page, { theme, act = async () => {} }) { await openTagDialog(page, 'concept', theme); const conceptOpened = await inventory(page); await act(page, 'concept'); @@ -93,7 +93,7 @@ async function compareState(page, { theme, act = async () => {}, opened = false await act(page, 'production'); const actual = await inventory(page); const numberField = conceptOpened.modelPanel.fields.find(field => field.control.id === 'image-general-threshold'); - const reference = mapConceptTagging(concept, { instructions: INSTRUCTIONS, chip: concept.heading.size, numberField, opened }); + const reference = mapConceptTagging(concept, { instructions: INSTRUCTIONS, chip: concept.heading.size, numberField }); return { reference: withoutChipGeometry(reference), actual: withoutChipGeometry(actual) }; } @@ -103,12 +103,12 @@ for (const [width, height] of SIZES) for (const theme of THEMES) { await saveImageTags(request, await conceptPath(request), CONCEPT_TAGS); await page.setViewportSize({ width, height }); // Opened, generated with WD14 (the concept's default), then generated with a language model. - for (const [state, act, opened] of [ - ['opened', async () => {}, true], - ['generated WD14', (page, side) => generateWith(page, side), false], - ['generated GGUF', (page, side) => generateWith(page, side, 'gguf'), false], + for (const [state, act] of [ + ['opened', async () => {}], + ['generated WD14', (page, side) => generateWith(page, side)], + ['generated GGUF', (page, side) => generateWith(page, side, 'gguf')], ]) { - const { reference, actual } = await compareState(page, { theme, act, opened }); + const { reference, actual } = await compareState(page, { theme, act }); expect(diffTaggingInventory(reference, actual), `${state} at ${width}x${height} ${theme}`).toEqual([]); } // Pinned concept geometry (M2), so a mapping mistake on both sides cannot pass unseen. @@ -172,7 +172,7 @@ test('review-scoped dialog shows the scope note', async ({ page, request }) => { expect(actual.scopeNote.text).toBe('Tags belong to this image. Resolve its prompt association separately in Needs review.'); expect(actual.order).toEqual(['tag-image-heading', 'tag-scope-note small', 'field', 'tag-model-panel', 'tag-suggestion-panel', 'dialog-actions']); const numberField = opened.modelPanel.fields.find(field => field.control.id === 'image-general-threshold'); - const reference = mapConceptTagging(opened, { instructions: INSTRUCTIONS, chip: opened.heading.size, numberField, opened: true }); + const reference = mapConceptTagging(opened, { instructions: INSTRUCTIONS, chip: opened.heading.size, numberField }); expect(diffTaggingInventory(reference, actual)).toEqual([]); }); @@ -188,7 +188,7 @@ for (const [width, height] of SIZES) test(`card action group matches M1 at ${wid test('the comparison notices a changed panel border or primary label', async ({ page, request }) => { await saveImageTags(request, await conceptPath(request), CONCEPT_TAGS); - const { reference, actual } = await compareState(page, { theme: 'dark', opened: true }); + const { reference, actual } = await compareState(page, { theme: 'dark' }); expect(diffTaggingInventory(reference, actual)).toEqual([]); const undo = await mutate(page, { css: '.tag-model-panel { border: 0 !important; }' }); const borderless = withoutChipGeometry(await inventory(page)); @@ -274,6 +274,17 @@ test('paired Tag dialog captures', async ({ page, request }, testInfo) => { await page.unroute(writes); await page.keyboard.press('Escape'); + // Unconfirmed save (a 504): D116's Retry leads the actions, before Cancel and the disabled primary. + await page.route(writes, route => route.fulfill({ status: 504, body: 'Gateway Timeout' })); + await openTagDialog(page, 'production', 'dark'); + await save(); + await expect(status()).toHaveText('The save could not be confirmed. Retry sends the same tags again.'); + expect((await inventory(page)).actions.buttons.map(button => [button.text, button.primary, button.disabled])).toEqual( + [['Retry', false, false], ['Cancel', false, false], ['Save manual tags', true, true]]); + await shot('production-unknown-outcome'); + await page.unroute(writes); + await page.keyboard.press('Escape'); + // Review scope: the concept's Needs review dialog and production's review-sourced dialog. await leave(page); await openConcept(page, 'review', 'dark'); diff --git a/tests/browser/workspace-tagging-lifecycle.spec.mjs b/tests/browser/workspace-tagging-lifecycle.spec.mjs new file mode 100644 index 0000000..a255a9b --- /dev/null +++ b/tests/browser/workspace-tagging-lifecycle.spec.mjs @@ -0,0 +1,71 @@ +// The Tag image dialog's capture lifetime when the page or the dialog goes away mid-flight (#249 PR C final review I4, +// I5), run on all four projects: leaving the page with the dialog open releases the capture (a `keepalive` DELETE sent +// on `pagehide`, which also closes the dialog), and closing the dialog while its save is in flight keeps the capture +// until the save settles, then still reports the save. The fixture is tests/browser/workspace_tagging_fixture.py. +import { startCoverage, saveCoverage } from '../../tools/vendor/browser-coverage.mjs'; +import { test, expect } from './fixtures.mjs'; + +const images = '/prompt_manager/workspace#/images'; +const FIRST = 'tagging/dunes_00001.png'; +const TAGS_URL = '**/prompt_manager/image-annotations/tags'; +const dialog = page => page.getByRole('dialog', { name: 'Tag image', exact: true }); +const manual = page => dialog(page).getByRole('textbox', { name: 'Image tags · edit manually', exact: true }); +const tagButton = page => page.locator(`article[data-image-key="0:${FIRST}"]`).getByRole('button', { name: 'Tag or auto-tag dunes_00001.png', exact: true }); +const openCaptures = async request => (await (await request.get('/__test__/tagging/captures')).json()).captures; + +test.beforeEach(async ({ page, request, browserName }) => { + await startCoverage(page, browserName); + await expect(await request.post('/__test__/tagging/reset', { data: {} })).toBeOK(); + page.problems = []; + page.on('pageerror', error => page.problems.push(`pageerror: ${error.message}`)); + page.on('console', message => { if (message.type() === 'error') page.problems.push(`console: ${message.text()}`); }); +}); +test.afterEach(async ({ page, browserName }) => { + await saveCoverage(page, browserName); + expect(page.problems).toEqual([]); +}); + +async function openTag(page) { + await page.goto(images); + const observed = page.waitForResponse(item => item.url().endsWith('/image-annotations/observe')); + await tagButton(page).click(); + await expect(dialog(page)).toBeVisible(); + expect((await observed).status()).toBe(200); + await expect(manual(page)).toBeEditable(); +} + +test('leaving the page with the dialog open releases its capture without a console error (I4)', async ({ page, request }) => { + await openTag(page); + expect(await openCaptures(request)).toBe(1); + // Another document: the old one's pagehide closes the dialog and sends the release with keepalive. + await page.goto('/prompt_manager/workspace?left=1#/tags'); + await expect(page.locator('#workspace-main h1')).toHaveText('Tags & categories'); + await expect.poll(() => openCaptures(request)).toBe(0); + await expect(dialog(page)).toHaveCount(0); +}); + +test('closing the dialog during a save releases the capture after the save and still reports it (I5)', async ({ page, request }) => { + await openTag(page); + await manual(page).fill('saved_after_close'); + const order = []; + page.on('response', item => { if (item.url().endsWith('/image-annotations/tags')) order.push(`PUT ${item.status()}`); }); + page.on('request', item => { if (item.url().includes('/image-annotations/capture/') && item.method() === 'DELETE') order.push('DELETE'); }); + let release; + const held = new Promise(resolve => { release = resolve; }); + const sent = new Promise(resolve => { + page.route(TAGS_URL, async route => { resolve(); await held; await route.continue(); }); + }); + await dialog(page).getByRole('button', { name: 'Save manual tags', exact: true }).click(); + await sent; + await page.keyboard.press('Escape'); + await expect(dialog(page)).toHaveCount(0); + expect(await openCaptures(request)).toBe(1); + expect(order).toEqual([]); + release(); + await expect(page.locator('#workspace-toast')).toHaveText('1 tags saved for dunes_00001.png.'); + await expect.poll(() => order).toEqual(['PUT 200', 'DELETE']); + await expect.poll(() => openCaptures(request)).toBe(0); + // The page followed the late save: the selected file's details show the new image tag. + await expect(page.getByRole('complementary', { name: 'Selected image details', exact: true }).getByText('saved_after_close', { exact: true })).toBeVisible(); + await page.unroute(TAGS_URL); +}); diff --git a/tests/browser/workspace-tagging.spec.mjs b/tests/browser/workspace-tagging.spec.mjs index 090f7bb..06d24a6 100644 --- a/tests/browser/workspace-tagging.spec.mjs +++ b/tests/browser/workspace-tagging.spec.mjs @@ -3,13 +3,15 @@ // downloaded or loaded: the fake answers, and every test checks that no download request was sent. import { startCoverage, saveCoverage } from '../../tools/vendor/browser-coverage.mjs'; import { test, expect } from './fixtures.mjs'; -import { seedConcept, openConcept, openProduction, saveImageTags } from './concept-inventory.mjs'; +import { saveImageTags } from './concept-inventory.mjs'; +import { taggingInventory } from './tagging-canonical-comparison.mjs'; const images = '/prompt_manager/workspace#/images'; const MODULE = '/prompt_manager/js/workspace/image-tagging.mjs'; const MODEL_LABELS = ['WD14 SwinV2 · ~400 MB', 'WD14 ViT · ~400 MB', 'GGUF JoyCaption (Recommended) · ~4.5 GB', 'HuggingFace JoyCaption · ~16 GB']; -const INITIAL_COPY = 'Choose a model, then generate tags. Remove anything you do not want to add.'; +// The concept's on-screen opening sentence (its renderTagModelOptions replaces the markup's at once, D83). const MODEL_CHANGED = 'Generate suggestions with the selected model before approving.'; +const CHIP_HINT = 'Remove anything you do not want to add.'; const ALL_REMOVED = 'All suggestions removed. Generate again or enter tags manually.'; const card = (page, path) => page.locator(`article[data-image-key="0:${path}"]`); const tagButton = (page, path) => card(page, path).getByRole('button', { name: `Tag or auto-tag ${path.split('/').pop()}`, exact: true }); @@ -106,7 +108,7 @@ test('tag dialog control inventory and order match M2', async ({ page }) => { await expect(suggestions).toHaveAttribute('aria-labelledby', 'tag-suggestions-title'); await expect(suggestions.locator('h3#tag-suggestions-title')).toHaveText('Review suggestions'); await expect(suggestions.locator('#suggestion-count')).toHaveText('Not generated'); - await expect(suggestions.locator('#tag-suggestions > p.small')).toHaveText(INITIAL_COPY); + await expect(suggestions.locator('#tag-suggestions > p.small')).toHaveText(MODEL_CHANGED); await expect(status(page)).toHaveAttribute('role', 'status'); await expect(status(page)).toHaveText(''); await expect(body.locator('.dialog-actions button')).toHaveText(['Cancel', 'Save manual tags']); @@ -122,6 +124,9 @@ test('generate shows real fixture percentages for WD14 and Suggested for GGUF, a const answer = await generated(page); expect(answer.request().postDataJSON()).toMatchObject({ model_type: 'wd14-swinv2', options: { general_threshold: 0.35, character_threshold: 0.85, suggestion_limit: 10 } }); await expect(chips(page)).toHaveCount(10); + // D83: ruling T4's instruction leads the chips, only while chips exist. + await expect(dialog(page).locator('#tag-suggestions > *')).toHaveCount(2); + await expect(dialog(page).locator('#tag-suggestions > p.small')).toHaveText(CHIP_HINT); await expect(chips(page).first().locator(':scope > span').first()).toHaveText('sandstone'); await expect(chips(page).locator('.tag-confidence').first()).toHaveText('98%'); await expect(chips(page).nth(1).locator('.tag-confidence')).toHaveText('97%'); @@ -244,7 +249,7 @@ test('unavailable model shows inline status and never downloads', async ({ page, await expect(status(page)).toHaveText('WD14 SwinV2 is not downloaded. Download it from the legacy Auto-tag page.'); await expect(chips(page)).toHaveCount(0); await expect(dialog(page).locator('#suggestion-count')).toHaveText('Not generated'); - // Retry (ruling T5) leads the actions while the controller allows it, and generates again. + // D116: Retry leads the actions while the controller allows it, and generates again. await expect(dialog(page).locator('.dialog-actions button')).toHaveText(['Retry', 'Cancel', 'Save manual tags']); await expect(await request.post('/__test__/tagging/models', { data: { unavailable: [] } })).toBeOK(); const again = page.waitForResponse(item => item.url().endsWith('/image-annotations/suggest')); @@ -436,32 +441,6 @@ test('virtualized cards release listeners on unmount', async ({ page, request }) expect(await count()).toBe(mounted); }); -// Paired captures for review (the exact concept comparison is plan Task C4): the concept's dialog and production's, -// before and after Generate, on the concept-shaped fixture. Only that both open and generate is asserted here. -for (const [width, height] of [[1600, 1100], [390, 844]]) test(`paired Tag dialog captures ${width}x${height}`, async ({ page, request }, testInfo) => { - await seedConcept(request); - await page.setViewportSize({ width, height }); - const shot = async name => { - await page.evaluate(async () => { await document.fonts.ready; await Promise.all([...document.images].map(image => image.decode().catch(() => {}))); }); - const path = testInfo.outputPath(`${name}.png`); - await page.screenshot({ path }); - await testInfo.attach(name, { path, contentType: 'image/png' }); - }; - for (const side of ['concept', 'production']) { - if (side === 'concept') await openConcept(page, 'images', 'dark'); - else await openProduction(page, 'images', 'dark'); - const first = page.locator(side === 'concept' ? '#image-grid > article.media-card' : 'article[data-image-key]').first(); - await first.getByRole('button', { name: /^Tag or auto-tag / }).click(); - const shell = page.getByRole('dialog', { name: 'Tag image', exact: true }); - await expect(shell).toBeVisible(); - if (side === 'production') await expect(shell.locator('img.tag-image-preview')).toHaveAttribute('src', /capture/); - await shot(`${side}-tag-dialog-${width}`); - await shell.getByRole('button', { name: 'Generate suggestions', exact: true }).click(); - await expect(shell.locator('#suggestion-count')).not.toHaveText('Not generated'); - await shot(`${side}-tag-dialog-generated-${width}`); - } -}); - // Task C3 (D88): the side panel and the viewer details draw the file's own current image tags first, then D21's linked // tags not already shown (case-insensitive). Panel image tags are Tags filter buttons; the viewer's chips are plain. const panel = page => page.getByRole('complementary', { name: 'Selected image details', exact: true }); @@ -581,6 +560,10 @@ test('a save under a Tags filter re-reads the listing: the selection stays while await expect(cards(page)).toHaveCount(2); // The newest match (SECOND) is selected once the filter applies. await expect(card(page, SECOND)).toHaveClass(/\bselected\b/); + // Both matches checked: the selection bar keeps only what the refreshed result still lists (I7). + for (const path of [SECOND, UNLINKED]) await card(page, path).getByRole('checkbox', { name: `Select ${path.split('/').pop()}`, exact: true }).check(); + const bulkbar = page.locator('#bulkbar .bulkbar'); + await expect(bulkbar.locator('strong')).toHaveText('2 selected'); // Tagging the other match keeps both cards and SECOND selected; the listing is re-read. await openTag(page, UNLINKED, { goto: false }); await manual(page).fill('shared, extra'); @@ -603,6 +586,15 @@ test('a save under a Tags filter re-reads the listing: the selection stays while await expect(card(page, UNLINKED)).toHaveClass(/\bselected\b/); await expect(panel(page).locator('.panel-heading h2')).toHaveText('unlinked.png'); await expect(page.getByText('1 of 1 image', { exact: false }).first()).toBeVisible(); + await expect(bulkbar.locator('strong')).toHaveText('1 selected'); + await expect(card(page, UNLINKED).getByRole('checkbox')).toBeChecked(); + // Export metadata acts on exactly the one file the bar counts. + const exported = []; + page.on('request', item => { if (item.url().includes('/prompt_manager/images/inspect/')) exported.push(new URL(item.url()).pathname); }); + const download = page.waitForEvent('download'); + await bulkbar.getByRole('button', { name: 'Export metadata', exact: true }).click(); + await download; + expect(exported).toEqual([`/prompt_manager/images/inspect/${UNLINKED}`]); }); test('a save under a search keeps the scroll position and the loaded range', async ({ page, request }) => { @@ -636,3 +628,122 @@ test('a save under a search keeps the scroll position and the loaded range', asy await expect(continuation).toHaveText(loaded); expect(Math.abs(await page.evaluate(() => window.scrollY) - y)).toBeLessThanOrEqual(1); }); + +// Final review round (I1, I2, I9, M5, M6). +test('the option fields show the server defaults that Generate sends (I1)', async ({ page, request }) => { + await expect(await request.post('/__test__/tagging/defaults', { data: { general: 0.5, character: 0.9, limit: 7 } })).toBeOK(); + await openTag(page); + const field = name => dialog(page).getByRole('spinbutton', { name, exact: true }); + await expect(field('General confidence')).toHaveValue('0.5'); + await expect(field('Character confidence')).toHaveValue('0.9'); + await expect(field('Suggestions')).toHaveValue('7'); + const answer = await generated(page); + expect(answer.request().postDataJSON().options).toEqual({ general_threshold: 0.5, character_threshold: 0.9, suggestion_limit: 7 }); + await expect(chips(page)).toHaveCount(7); + // A model change re-renders the new model's fields from the server defaults, and keeps the count (D91). + await field('Suggestions').fill('4'); + await model(page).selectOption('wd14-vit'); + await expect(field('General confidence')).toHaveValue('0.5'); + await expect(field('Suggestions')).toHaveValue('4'); +}); + +test('a tag conflict shows the other writer\'s tags before Save, and a second Save keeps them (I2)', async ({ page, request }) => { + await openTag(page); + await manual(page).fill('mine'); + // Another writer (a second tab, the legacy page) saves while the dialog is open. + expect(await saveImageTags(request, FIRST, ['changed_elsewhere'])).toEqual(['changed_elsewhere']); + const conflict = page.waitForResponse(item => item.url().endsWith('/image-annotations/tags') && item.request().method() === 'PUT'); + await dialog(page).getByRole('button', { name: 'Save manual tags', exact: true }).click(); + expect((await conflict).status()).toBe(409); + await expect(status(page)).toContainText('Saved now: changed_elsewhere.'); + await expect(manual(page)).toHaveValue('changed_elsewhere, mine'); + await expect(dialog(page)).toBeVisible(); + const second = page.waitForResponse(item => item.url().endsWith('/image-annotations/tags') && item.request().method() === 'PUT'); + await dialog(page).getByRole('button', { name: 'Save manual tags', exact: true }).click(); + expect((await second).request().postDataJSON().tags).toEqual(['changed_elsewhere', 'mine']); + await expect(toast(page)).toHaveText('2 tags saved for dunes_00001.png.'); + expect(await savedTags(request, FIRST)).toEqual(['changed_elsewhere', 'mine']); +}); + +test('D116: Retry leads the actions after an unconfirmed save and resends the same tags', async ({ page, request }) => { + await openTag(page); + await manual(page).fill('retried'); + let lost = true; + await page.route('**/prompt_manager/image-annotations/tags', route => (lost ? (lost = false, route.fulfill({ status: 504, body: 'Gateway Timeout' })) : route.continue())); + await dialog(page).getByRole('button', { name: 'Save manual tags', exact: true }).click(); + await expect(status(page)).toHaveText('The save could not be confirmed. Retry sends the same tags again.'); + const inventory = await page.locator('dialog#modal').evaluate(taggingInventory); + expect(inventory.actions.buttons.map(button => [button.text, button.primary, button.disabled])).toEqual( + [['Retry', false, false], ['Cancel', false, false], ['Save manual tags', true, true]]); + expect(inventory.actions.buttons[0].minHeight).toBe(inventory.actions.buttons[1].minHeight); + const resent = page.waitForResponse(item => item.url().endsWith('/image-annotations/tags') && item.request().method() === 'PUT'); + await dialog(page).getByRole('button', { name: 'Retry', exact: true }).click(); + expect((await resent).request().postDataJSON().tags).toEqual(['retried']); + await expect(toast(page)).toHaveText('1 tags saved for dunes_00001.png.'); + expect(await savedTags(request, FIRST)).toEqual(['retried']); + await page.unroute('**/prompt_manager/image-annotations/tags'); +}); + +test('D116: Retry after an expired capture observes again and keeps the typed draft (M5)', async ({ page, request }) => { + await openTag(page); + await manual(page).fill('kept_draft'); + // The store forgets every capture (as its idle sweep would), so Save answers capture-expired. + await expect(await request.post('/__test__/tagging/suggestions', { data: { scored: null } })).toBeOK(); + await dialog(page).getByRole('button', { name: 'Save manual tags', exact: true }).click(); + await expect(dialog(page).locator('.dialog-actions .ca-primary')).toHaveAttribute('aria-disabled', 'true'); + const observed = page.waitForResponse(item => item.url().endsWith('/image-annotations/observe')); + await dialog(page).getByRole('button', { name: 'Retry', exact: true }).click(); + expect((await observed).status()).toBe(200); + await expect(dialog(page).locator('.dialog-actions button')).toHaveText(['Cancel', 'Save manual tags']); + await expect(manual(page)).toHaveValue('kept_draft'); + await dialog(page).getByRole('button', { name: 'Save manual tags', exact: true }).click(); + await expect(toast(page)).toHaveText('1 tags saved for dunes_00001.png.'); + expect(await savedTags(request, FIRST)).toEqual(['kept_draft']); +}); + +test('a tag save from Tags reloads a filtered Images visit instead of restoring it (M6)', async ({ page, request }) => { + await saveImageTags(request, SECOND, ['shared']); + await saveImageTags(request, UNLINKED, ['shared']); + await page.goto(images); + await infoButton(page, SECOND).click(); + const filtered = listingOf(page, query => query.get('tags') === 'shared'); + await tagRow(page).getByRole('button', { name: 'shared', exact: true }).click(); + await filtered; + await expect(cards(page)).toHaveCount(2); + await expect(card(page, SECOND)).toHaveClass(/\bselected\b/); + // Tags › Auto-tag images tags the selected SECOND without the filter tag. + await nav(page, 'Tags & categories').click(); + await page.locator('#workspace-main .heading-actions').getByRole('button', { name: 'Auto-tag images', exact: true }).click(); + await expect(manual(page)).toHaveValue('shared'); + await manual(page).fill('other'); + await dialog(page).getByRole('button', { name: 'Save manual tags', exact: true }).click(); + await expect(toast(page)).toHaveText('1 tags saved for dunes_00002.png.'); + const reloaded = listingOf(page, query => query.get('tags') === 'shared' && !query.has('cursor')); + await nav(page, 'Images').click(); + await reloaded; + await expect(cards(page)).toHaveCount(1); + await expect(card(page, UNLINKED)).toBeVisible(); + await expect(page.getByRole('button', { name: 'Filters · 1', exact: true })).toBeVisible(); +}); + +test('a failed listing re-read after a save says so and keeps the cards (M2)', async ({ page, request }) => { + await saveImageTags(request, SECOND, ['shared']); + await saveImageTags(request, UNLINKED, ['shared']); + await page.goto(images); + await infoButton(page, SECOND).click(); + const filtered = listingOf(page, query => query.get('tags') === 'shared'); + await tagRow(page).getByRole('button', { name: 'shared', exact: true }).click(); + await filtered; + await expect(cards(page)).toHaveCount(2); + await openTag(page, SECOND, { goto: false }); + await manual(page).fill(''); + const failing = url => url.pathname === '/prompt_manager/images/output' && url.searchParams.get('tags') === 'shared'; + await page.route(failing, route => route.fulfill({ status: 500, contentType: 'application/json', body: '{"success":false,"error":"down"}' })); + const reread = page.waitForResponse(item => failing(new URL(item.url()))); + await dialog(page).getByRole('button', { name: 'Save manual tags', exact: true }).click(); + expect((await reread).status()).toBe(500); + await expect(toast(page)).toHaveText('Images could not be refreshed after the tag save. Refresh image collection to retry.'); + await expect(cards(page)).toHaveCount(2); + await expect(page.locator('#gallery-continuation [aria-disabled="true"]')).toHaveCount(0); + await page.unroute(failing); +}); diff --git a/tests/browser/workspace_tagging_fixture.py b/tests/browser/workspace_tagging_fixture.py index 777c0db..31cbd2f 100644 --- a/tests/browser/workspace_tagging_fixture.py +++ b/tests/browser/workspace_tagging_fixture.py @@ -188,6 +188,7 @@ def register_tagging_commands( model.unavailable = set() model.suggestions = None model.loaded = None + _set_defaults(model, DEFAULTS) # A test that ends with the dialog open leaves its capture behind; drop # every capture so the next test starts below the store's limits. await api._run_in_executor(api.close_annotation_captures) @@ -240,6 +241,60 @@ def register_tagging_commands( await api._run_in_executor(api.close_annotation_captures) return web.json_response({"success": True}) + @routes.post("/__test__/tagging/defaults") + async def defaults(request: web.Request) -> web.Response: + """Serve other tagging defaults from ``GET /autotag/models``. + + ``{"general": g, "character": c, "limit": n}``: the WD14 thresholds + (in [0, 1]) and the persisted suggestion count (1..100), so a test can + tell the server's values from the dialog's built-in ones. Reset + restores the standard 0.35 / 0.85 / 10. + """ + data = await request.json() if request.can_read_body else None + if request.query or not _valid_defaults(data): + raise web.HTTPBadRequest() + _set_defaults(model, (data["general"], data["character"], data["limit"])) + return web.json_response({"success": True}) + + @routes.get("/__test__/tagging/captures") + async def captures(request: web.Request) -> web.Response: + """How many captures the store holds (a released capture is gone).""" + if request.query: + raise web.HTTPBadRequest() + store = api._captures + count = store.stats()["captures"] if store is not None else 0 + return web.json_response({"captures": count}) + + +# The standard tagging defaults: WD14 general and character thresholds, and +# the suggestion count (``AutotagConfig``). +DEFAULTS = (0.35, 0.85, 10) + + +def _set_defaults(model, values) -> None: + from py.config import AutotagConfig + + general, character, limit = values + model.wd14_general_threshold = general + model.wd14_character_threshold = character + AutotagConfig.SUGGESTION_LIMIT = limit + + +def _valid_defaults(data: object) -> bool: + unit = ( + lambda value: isinstance(value, (int, float)) + and not isinstance(value, bool) + and 0 <= value <= 1 + ) + return ( + isinstance(data, dict) + and set(data) == {"general", "character", "limit"} + and unit(data["general"]) + and unit(data["character"]) + and type(data["limit"]) is int + and 1 <= data["limit"] <= 100 + ) + def _valid_scored(scored: object) -> bool: return ( diff --git a/tests/js/api-client.test.js b/tests/js/api-client.test.js index 9743ec4..01b45b4 100644 --- a/tests/js/api-client.test.js +++ b/tests/js/api-client.test.js @@ -315,3 +315,14 @@ test("ApiError exposes only a validated current tag set from a tag-conflict body }); } }); + +test("keepalive is forwarded only when asked, so a request can outlive its page", async () => { + const fetch = fakeFetch(); + const api = createApiClient(fetch); + const writes = [api.del("/x", { keepalive: true }), api.put("/x", {}, { keepalive: true }), api.post("/x", {}, { keepalive: true }), + api.del("/x"), api.put("/x", {}), api.get("/x")]; + assert.deepEqual(fetch.calls.map(call => call.init.keepalive), [true, true, true, undefined, undefined, undefined]); + assert.equal("keepalive" in fetch.calls[3].init, false); + for (const call of fetch.calls) call.resolve(jsonResponse(200, { success: true })); + await Promise.all(writes); +}); diff --git a/tests/js/workspace-image-tagging-state.test.js b/tests/js/workspace-image-tagging-state.test.js index 0ace61a..604716f 100644 --- a/tests/js/workspace-image-tagging-state.test.js +++ b/tests/js/workspace-image-tagging-state.test.js @@ -159,7 +159,7 @@ test('opens with saved tags joined by comma-space', async () => { assert.equal(c.state.model, 'wd14-swinv2'); assert.equal(c.state.countText, 'Not generated'); assert.equal(c.state.primaryLabel, 'Save manual tags'); - assert.equal(c.state.suggestionsText, 'Choose a model, then generate tags. Remove anything you do not want to add.'); + assert.equal(c.state.suggestionsText, 'Generate suggestions with the selected model before approving.'); assert.equal(c.state.showScopeNote, false); assert.equal(c.state.canSave, true); assert.equal(c.state.canGenerate, true); @@ -448,9 +448,9 @@ test('conflict keeps draft and exposes current saved tags', async () => { await c.save(); assert.equal(c.state.phase, 'conflict'); assert.deepEqual(c.state.currentTags, ['night', 'rain']); - assert.equal(c.state.manual, 'Minimal, dune'); + assert.equal(c.state.manual, 'night, rain, Minimal, dune'); assert.equal(c.state.suggestions.length, 3); - assert.equal(c.state.statusText, 'The tags of this image changed since the dialog opened; review them first.'); + assert.equal(c.state.statusText, 'The tags of this image changed since the dialog opened; review them first. Saved now: night, rain.'); assert.equal(c.state.canSave, true); answer = call => response(200, receipt(call.body.tags, { tag_revision: 6 })); await c.save(); @@ -565,10 +565,15 @@ test('model-unavailable and model-busy set the status line only', async () => { test('a storage restore during generate requires a reopen', async () => { const { c } = await opened({ [`POST ${SUGGEST}`]: () => fail(409, 'storage-changed', 'Prompt storage was restored; reopen the Tag dialog.') }); + c.setManual('kept'); await c.generate(); assert.equal(c.state.phase, 'storage-changed'); assert.equal(c.state.canSave, false); - assert.equal(await c.retry(), false); + assert.equal(c.state.canGenerate, false); + await c.retry(); + await settle(); + assert.equal(c.state.phase, 'ready', 'Retry observes again (D116)'); + assert.equal(c.state.manual, 'kept'); }); test('dispose aborts requests and releases the capture once', async () => { @@ -618,3 +623,147 @@ test('a failing capture release is not reported as a dialog error', async () => assert.equal(h.of('DELETE', CAPTURE).length, 1); assert.equal(c.state.phase, 'ready'); }); + +test('the opening sentence is the concept on-screen copy and the chip hint shows only with chips (D83)', async () => { + const { createImageTaggingController } = await load(); + const h = harness(defaultRoutes()); + const c = createImageTaggingController({ api: h.api, target: TARGET }); + assert.equal(c.state.suggestionsText, 'Generate suggestions with the selected model before approving.'); + assert.equal(c.state.chipHint, ''); + await c.open(); + await settle(); + assert.equal(c.state.suggestionsText, 'Generate suggestions with the selected model before approving.'); + assert.equal(c.state.chipHint, ''); + await c.generate(); + assert.equal(c.state.chipHint, 'Remove anything you do not want to add.'); + for (const row of [...c.state.suggestions]) c.remove(row.name); + assert.equal(c.state.chipHint, ''); +}); + +test('the option fields follow the server defaults: open and a model change bump the revision, typing does not', async () => { + const { c, h } = await opened({ + [`GET ${MODELS}`]: () => response(200, models({ suggestion_limit: 7, wd14_general_threshold: 0.5, wd14_character_threshold: 0.9 })), + }); + const opening = c.state.optionsRevision; + assert.ok(opening > 0, 'the observation that applies the server defaults bumps the revision'); + assert.deepEqual({ ...c.state.options }, { general_threshold: 0.5, character_threshold: 0.9, instructions: DEFAULT_PROMPT, suggestion_limit: 7 }); + c.setOption('general_threshold', '0.6'); + assert.equal(c.state.optionsRevision, opening, 'typing never re-renders the fields'); + c.setModel('wd14-vit'); + assert.equal(c.state.optionsRevision, opening + 1); + assert.equal(c.state.options.general_threshold, 0.5); + await c.generate(); + assert.deepEqual(h.of('POST', SUGGEST)[0].body.options, { general_threshold: 0.5, character_threshold: 0.9, suggestion_limit: 7 }); +}); + +test('a conflict shows the current set and merges it ahead of the draft, so a second Save never drops it unseen', async () => { + let answer = () => fail(409, 'tag-conflict', 'The tags of this image changed since the dialog opened; review them first.', + { tags: ['changed_elsewhere', 'Minimal'], tag_revision: 5 }); + const { c, h } = await opened({ [`PUT ${TAGS}`]: call => answer(call) }); + const draft = c.state.draftRevision; + c.setManual('minimal, dune'); + await c.save(); + assert.equal(c.state.phase, 'conflict'); + assert.equal(c.state.statusText, + 'The tags of this image changed since the dialog opened; review them first. Saved now: changed_elsewhere, Minimal.'); + assert.equal(c.state.manual, 'changed_elsewhere, Minimal, dune', 'the current set leads, the draft follows, repeats dropped'); + assert.equal(c.state.draftRevision, draft + 1, 'the dialog rewrites its field'); + answer = call => response(200, receipt(call.body.tags, { tag_revision: 6 })); + await c.save(); + const second = h.of('PUT', TAGS)[1].body; + assert.deepEqual(second.tags, ['changed_elsewhere', 'Minimal', 'dune']); + assert.equal(second.expected_tag_revision, 5); + const empty = await opened({ [`PUT ${TAGS}`]: () => fail(409, 'tag-conflict', 'Changed.', { tags: [], tag_revision: 9 }) }); + empty.c.setManual('a'); + await empty.c.save(); + assert.equal(empty.c.state.statusText, 'Changed. Saved now: no tags.'); + assert.equal(empty.c.state.manual, 'a'); +}); + +test('the capture release and the save survive leaving the page (keepalive)', async () => { + const { c, h } = await opened(); + await c.save(); + assert.equal(h.of('PUT', TAGS)[0].init.keepalive, true); + const other = await opened(); + other.c.dispose(); + const [release] = other.h.of('DELETE', CAPTURE); + assert.equal(release.init.keepalive, true); + assert.equal(h.of('POST', OBSERVE)[0].init.keepalive, undefined, 'reads are not kept alive'); +}); + +test('closing during a save releases the capture only after the PUT settles and reports the outcome', async () => { + const outcomes = []; + const gate = deferred(); + const { c, h } = await opened({ [`PUT ${TAGS}`]: call => gate.promise.then(() => response(200, receipt(call.body.tags))) }, + { onLateSave: outcome => outcomes.push(outcome) }); + c.setManual('dune'); + const saving = c.save(); + c.dispose(); + await settle(); + assert.equal(h.of('DELETE', CAPTURE).length, 0, 'the capture stays while its save is in flight'); + gate.resolve(); + assert.equal(await saving, true); + await settle(); + const order = h.calls.filter(call => call.method !== 'GET').map(call => call.method); + assert.deepEqual(order, ['POST', 'PUT', 'DELETE']); + assert.deepEqual(outcomes, [{ saved: true, filename: 'dunes_00001.png', tags: ['dune'], toast: '1 tags saved for dunes_00001.png.' }]); + assert.equal(c.state.closed, true); + + for (const [route, message] of [ + [() => fail(409, 'tag-conflict', 'Changed elsewhere.', { tags: ['x'], tag_revision: 5 }), 'Changed elsewhere.'], + [() => fail(400, 'tag-limit', 'Too many.'), 'Too many.'], + [() => Promise.reject(new TypeError('Failed to fetch')), 'The tag save could not be confirmed. Open Tag image again to check the saved tags.'], + ]) { + const late = []; + const held = deferred(); + const run = await opened({ [`PUT ${TAGS}`]: () => held.promise.then(route) }, { onLateSave: outcome => late.push(outcome) }); + const pending = run.c.save(); + run.c.cancel(); + held.resolve(); + assert.equal(await pending, false); + await settle(); + assert.deepEqual(late, [{ saved: false, message }]); + assert.equal(run.h.of('DELETE', CAPTURE).length, 1); + } + const quiet = await opened(); + quiet.c.dispose(); + assert.equal(quiet.h.of('DELETE', CAPTURE).length, 1, 'with no save in flight the capture is released at once'); + const { createImageTaggingController } = await load(); + assert.throws(() => createImageTaggingController({ api: h.api, target: TARGET, onLateSave: 'x' }), TypeError); +}); + +test('retry after a changed storage, file or expired capture observes again and keeps the draft', async () => { + for (const code of ['storage-changed', 'file-changed', 'capture-expired']) { + let revision = 3; + const { c, h } = await opened({ + [`PUT ${TAGS}`]: () => fail(409, code, `server says ${code}`), + [`POST ${OBSERVE}`]: () => response(200, observed({ tag_revision: revision, tags: revision === 3 ? ['Minimal', 'warm_light'] : ['night'] })), + }); + c.setManual('kept draft'); + await c.save(); + assert.equal(c.state.canRetry, true); + const draft = c.state.draftRevision; + await c.retry(); + await settle(); + assert.equal(c.state.phase, 'ready'); + assert.equal(c.state.manual, 'kept draft', 'an unchanged tag set keeps the draft as typed'); + assert.equal(c.state.draftRevision, draft + 1); + assert.equal(h.of('POST', OBSERVE).length, 2); + revision = 4; + await c.save(); + await c.retry(); + await settle(); + assert.equal(c.state.manual, 'night, kept draft', 'a set changed meanwhile is shown ahead of the draft'); + assert.equal(c.state.statusText, 'The tags of this image changed since the dialog opened. Saved now: night.'); + assert.equal(c.state.tagRevision, 4); + } + // A first observation that failed has no draft to keep: its retry fills the saved tags. + const { createImageTaggingController } = await load(); + let first = true; + const h = harness(defaultRoutes({ [`POST ${OBSERVE}`]: () => (first ? (first = false, fail(500, 'observe-failed')) : response(200, observed())) })); + const c = createImageTaggingController({ api: h.api, target: TARGET }); + await c.open(); + await c.retry(); + await settle(); + assert.equal(c.state.manual, 'Minimal, warm_light'); +}); diff --git a/tests/js/workspace-images-state.test.js b/tests/js/workspace-images-state.test.js index e47ffad..b17aa9b 100644 --- a/tests/js/workspace-images-state.test.js +++ b/tests/js/workspace-images-state.test.js @@ -365,12 +365,14 @@ test('refresh re-reads the loaded range in place and publishes once, replacing t assert.equal(stream.state,before);assert.equal(states.length,published); assert.equal(await stream.loadMore(),0);assert.equal(requests.length,3); const query=new URL(requests[2].url,'http://local').searchParams; - assert.equal(query.get('q'),'tag');assert.equal(query.has('cursor'),false); - requests[2].resolve(page([image('a'),image('new')],6,'n1')); + // The loaded range is re-read in pages as large as it (at most 200), not in the listing's batches (M2). + assert.equal(query.get('q'),'tag');assert.equal(query.has('cursor'),false);assert.equal(query.get('limit'),'4'); + requests[2].resolve(page([image('a'),image('new'),image('c')],6,'n1')); await new Promise(resolve=>setImmediate(resolve)); - assert.equal(new URL(requests[3].url,'http://local').searchParams.get('cursor'),'n1'); + const next=new URL(requests[3].url,'http://local').searchParams; + assert.equal(next.get('cursor'),'n1');assert.equal(next.get('limit'),'2','the rest, at least one batch'); assert.equal(stream.state,before); - requests[3].resolve(page([image('c'),image('d')],6,'n2')); + requests[3].resolve(page([image('d')],6,'n2')); assert.equal(await refresh,4); assert.deepEqual(stream.state.items.map(imageKey),['0:a','0:new','0:c','0:d']); assert.deepEqual([stream.state.total,stream.state.cursor,stream.state.hasMore,stream.state.loaded,stream.state.loading],[6,'n2',true,4,false]); @@ -390,7 +392,7 @@ test('refresh stops early when the result shrank, and a reset or failure leaves assert.equal(await shrink,1);assert.deepEqual(stream.state.items.map(imageKey),['0:b']);assert.equal(stream.state.hasMore,false); const before=stream.state; const failing=stream.refresh();requests[2].reject(new Error('offline')); - assert.equal(await failing,0);assert.equal(stream.state,before); + await assert.rejects(failing,/offline/,'a failed re-read is reported to the page');assert.equal(stream.state,before); const stale=stream.refresh();const fresh=stream.reset({tags:['y']}); assert.equal(requests[3].signal.aborted,true); requests[4].resolve(page([image('y')]));await fresh; @@ -398,3 +400,81 @@ test('refresh stops early when the result shrank, and a reset or failure leaves assert.deepEqual(stream.state.items.map(imageKey),['0:y']); stream.dispose();assert.equal(await stream.refresh(),0); }); + +test('a refresh asked while a page loads runs once that page settles (I6)', async () => { + const {createImageStream,imageKey}=await source;const requests=[];const states=[]; + const stream=createImageStream({api:{get:(url,{signal})=>{const d=deferred();requests.push({...d,url,signal});return d.promise;}},onChange:(s,hint)=>states.push(hint),batchSize:2}); + const first=stream.reset({tags:['x']});requests[0].resolve(page([image('a'),image('b')],4,'c1'));await first; + for (const settle of ['resolve','reject']) { + const more=stream.loadMore();const sent=requests.length; + assert.equal(await stream.refresh(),0,'nothing is read beside the page in flight'); + assert.equal(await stream.refresh(),0); + assert.equal(requests.length,sent); + if (settle==='resolve') requests[sent-1].resolve(page([image('c'),image('d')],4,'c2')); + else requests[sent-1].reject(new Error('offline')); + await more; + await new Promise(resolve=>setImmediate(resolve)); + assert.equal(requests.length,sent+1,'exactly one pending refresh follows'); + assert.equal(new URL(requests[sent].url,'http://local').searchParams.has('cursor'),false); + requests[sent].resolve(page([image('a'),image('b'),image('c'),image('d')],6,'r1')); + await new Promise(resolve=>setImmediate(resolve)); + assert.deepEqual(states.at(-1),{replaced:true}); + assert.equal(states.filter(hint=>hint?.replaced).length,settle==='resolve'?1:2); + } + // A reset drops a pending refresh. + const more=stream.loadMore();const sent=requests.length; + assert.equal(await stream.refresh(),0); + const fresh=stream.reset({tags:['y']}); + requests[sent].resolve(page([image('y')]));await fresh; + requests[sent-1].resolve(page([image('late')],1));await more; + await new Promise(resolve=>setImmediate(resolve)); + assert.equal(requests.length,sent+1);assert.deepEqual(stream.state.items.map(imageKey),['0:y']); + assert.equal(stream.refreshing,false); +}); + +test('refresh reads a long loaded range in at most 200-file pages and exposes refreshing (M2, M3)', async () => { + const {createImageStream}=await source;const requests=[]; + const stream=createImageStream({api:{get:(url,{signal})=>{const d=deferred();requests.push({...d,url,signal});return d.promise;}},batchSize:150}); + const many=(prefix,n)=>Array.from({length:n},(_,i)=>image(`${prefix}${i}`)); + const first=stream.reset({q:'x'});requests[0].resolve(page(many('a',150),1000,'c1'));await first; + const second=stream.loadMore();requests[1].resolve(page(many('b',150),1000,'c2'));await second; + const refresh=stream.refresh(); + assert.equal(stream.refreshing,true); + assert.equal(new URL(requests[2].url,'http://local').searchParams.get('limit'),'200'); + requests[2].resolve(page(many('r',200),1000,'n1')); + await new Promise(resolve=>setImmediate(resolve)); + assert.equal(new URL(requests[3].url,'http://local').searchParams.get('limit'),'150'); + requests[3].resolve(page(many('s',150),1000,'n2')); + assert.equal(await refresh,350);assert.equal(requests.length,4); + assert.equal(stream.refreshing,false); +}); + +test('a second refresh supersedes the first, repeats in a re-read are dropped, and dispose abandons one in flight', async () => { + const {createImageStream,imageKey}=await source;const requests=[];const states=[]; + const stream=createImageStream({api:{get:(url,{signal})=>{const d=deferred();requests.push({...d,url,signal});return d.promise;}},onChange:(s,hint)=>states.push(hint),batchSize:2}); + const first=stream.reset({q:'x'});requests[0].resolve(page([image('a'),image('b')],2));await first; + const older=stream.refresh();const newer=stream.refresh(); + assert.equal(requests[1].signal.aborted,true,'the older re-read is aborted'); + requests[2].resolve(page([image('b'),image('b'),image('a')],2)); + assert.equal(await newer,2); + requests[1].resolve(page([image('old')],1));assert.equal(await older,0); + assert.deepEqual(stream.state.items.map(imageKey),['0:b','0:a']); + assert.equal(states.filter(hint=>hint?.replaced).length,1); + const pending=stream.refresh(); + stream.dispose(); + assert.equal(requests[3].signal.aborted,true); + requests[3].resolve(page([image('late')],1)); + assert.equal(await pending,0);assert.deepEqual(stream.state.items.map(imageKey),['0:b','0:a']); +}); + +test('a refresh deferred behind a page load is dropped when the stream is disposed meanwhile', async () => { + const {createImageStream}=await source;const requests=[]; + const stream=createImageStream({api:{get:(url,{signal})=>{const d=deferred();requests.push({...d,url,signal});return d.promise;}},batchSize:2}); + const first=stream.reset({q:'x'});requests[0].resolve(page([image('a'),image('b')],4,'c1'));await first; + const more=stream.loadMore(); + assert.equal(await stream.refresh(),0); + stream.dispose(); + requests[1].resolve(page([image('c')],4,'c2'));await more; + await new Promise(resolve=>setImmediate(resolve)); + assert.equal(requests.length,2,'no refresh runs after dispose'); +}); diff --git a/web/js/api-client.js b/web/js/api-client.js index c407f05..df1f7a0 100644 --- a/web/js/api-client.js +++ b/web/js/api-client.js @@ -64,9 +64,11 @@ function createApiClient(fetchImpl) { const doFetch = fetchImpl || ((...args) => globalThis.fetch(...args)); - async function request(method, url, { body, signal, cache } = {}) { + async function request(method, url, { body, signal, cache, keepalive } = {}) { const init = { method, headers: {} }; if (cache !== undefined) init.cache = cache; + // keepalive lets a write or a release finish while its page unloads (pagehide). + if (keepalive) init.keepalive = true; if (body !== undefined) { init.headers["Content-Type"] = "application/json"; init.body = JSON.stringify(body); @@ -96,11 +98,11 @@ return { // cache is a fetch RequestCache mode such as "no-store"; omitted keeps the default. get: (url, { signal, cache } = {}) => request("GET", url, { signal, cache }), - post: (url, body, { signal } = {}) => request("POST", url, { body, signal }), - put: (url, body, { signal } = {}) => request("PUT", url, { body, signal }), + post: (url, body, { signal, keepalive } = {}) => request("POST", url, { body, signal, keepalive }), + put: (url, body, { signal, keepalive } = {}) => request("PUT", url, { body, signal, keepalive }), patch: (url, body, { signal } = {}) => request("PATCH", url, { body, signal }), // A DELETE body is optional; the favorites store names its file in one. - del: (url, { signal, body } = {}) => request("DELETE", url, { body, signal }), + del: (url, { signal, body, keepalive } = {}) => request("DELETE", url, { body, signal, keepalive }), }; } diff --git a/web/js/workspace/image-tagging-state.mjs b/web/js/workspace/image-tagging-state.mjs index 4f9fbc0..e37b6b6 100644 --- a/web/js/workspace/image-tagging-state.mjs +++ b/web/js/workspace/image-tagging-state.mjs @@ -8,7 +8,10 @@ * cancel() or dispose() supersedes older requests, so a late answer can never * stage chips or supply a token. Writes are sent once, without an abort signal * (aborting cannot undo a write); a lost answer is resent only by retry(), with - * the identical body (the server answers a duplicate as `replayed`). + * the identical body (the server answers a duplicate as `replayed`). A save in + * flight when the dialog closes still settles: the capture is released after it, + * and `onLateSave(outcome)` reports what happened. Writes and the capture release + * are sent with `keepalive`, so they finish while the page unloads. */ import {noun} from './plural.mjs'; @@ -30,17 +33,20 @@ const DEFAULTS = Object.freeze({general_threshold: 0.35, character_threshold: 0. const MAX_SUGGESTIONS = 100; const MAX_INSTRUCTION_CHARS = 4000; const EDITABLE = new Set(['ready', 'generate-error', 'write-error', 'conflict']); -const RETRYABLE = new Set(['observe-error', 'generate-error', 'unknown-outcome']); -// Answers after which only a fresh observation can save (the capture or epoch is gone). +// Answers after which only a fresh observation can save (the capture or epoch is gone); Retry observes again (D116). const REOPEN = {'storage-changed': 'storage-changed', 'file-changed': 'file-changed', 'source-changed': 'file-changed', 'capture-expired': 'capture-expired'}; +const REOBSERVE = new Set(Object.values(REOPEN)); +const RETRYABLE = new Set(['observe-error', 'generate-error', 'unknown-outcome', ...REOBSERVE]); // Generic failure envelopes whose fixed sentence the dialog words itself. const GENERIC_CODES = new Set(['observe-failed', 'suggest-failed', 'save-failed', 'capture-error']); // A proxy timeout or bad gateway cannot tell whether the server committed the write. const UNCONFIRMED_STATUS = new Set([502, 503, 504]); const COPY = Object.freeze({ - initial: 'Choose a model, then generate tags. Remove anything you do not want to add.', // D83 + // The concept's on-screen opening sentence (renderTagModelOptions a1062 replaces its markup's at once). modelChanged: 'Generate suggestions with the selected model before approving.', + chipHint: 'Remove anything you do not want to add.', // D83 + changedElsewhere: 'The tags of this image changed since the dialog opened.', allRemoved: 'All suggestions removed. Generate again or enter tags manually.', // D83 empty: 'No suggestions met this threshold. Adjust it or enter tags manually.', generating: 'Generating suggestions…', @@ -48,6 +54,7 @@ const COPY = Object.freeze({ generateFailed: 'Auto-tagging failed. Check server logs for details.', saveFailed: 'The tags could not be saved.', unknown: 'The save could not be confirmed. Retry sends the same tags again.', + unknownClosed: 'The tag save could not be confirmed. Open Tag image again to check the saved tags.', reviewSuffix: ' Prompt association still awaits review.', }); @@ -95,6 +102,12 @@ export function parseManualTags(text) { return dedupeTags(typeof text === 'string' ? text.split(',') : []); } +/** The server's current set ahead of the user's draft: what the dialog shows after another writer saved. */ +function mergeCurrent(current, draft) { + return dedupeTags([...current, ...parseManualTags(draft)]).join(', '); +} +const savedNow = tags => `Saved now: ${tags.length ? tags.join(', ') : 'no tags'}.`; + /** `null` when *options* are valid for *model*, else `{field, message}` (nothing is sent). */ export function optionError(model, options) { const option = modelOf(model); @@ -186,7 +199,9 @@ function initialState(source) { phase: 'observing', closed: false, source, showScopeNote: source === 'review', image: null, savedTags: [], tagRevision: 0, currentTags: null, manual: '', model: MODEL_OPTIONS[0].id, options: {...DEFAULTS}, generated: false, suggestions: [], - suggestionsText: COPY.initial, statusText: '', generateSupported: false, generateReason: null, toast: null, + suggestionsText: COPY.modelChanged, statusText: '', generateSupported: false, generateReason: null, toast: null, + // Bumped whenever the controller itself rewrites the option fields or the manual draft, never while typing. + optionsRevision: 0, draftRevision: 0, }; } @@ -197,16 +212,18 @@ function derive(state) { ...state, primaryLabel: state.generated ? 'Approve selected tags' : 'Save manual tags', countText: state.generated ? `${state.suggestions.length} ${noun(state.suggestions.length, 'suggestion')}` : 'Not generated', + chipHint: state.suggestions.length ? COPY.chipHint : '', canSave: editable, canGenerate: editable && state.generateSupported, canRetry: RETRYABLE.has(state.phase), }; } -export function createImageTaggingController({api, target, source = 'images'} = {}) { +export function createImageTaggingController({api, target, source = 'images', onLateSave = () => {}} = {}) { requireValue(object(api) && ['get', 'post', 'put', 'del'].every(name => typeof api[name] === 'function'), 'An api client is required'); const imageTarget = validateTarget(target); requireValue(SOURCES.has(source), 'Unknown tagging source'); + requireValue(typeof onLateSave === 'function', 'onLateSave must be a function'); let state = freeze(derive(initialState(source))); const listeners = new Set(); @@ -220,6 +237,9 @@ export function createImageTaggingController({api, target, source = 'images'} = let readAbort = null; let generateAbort = null; let lastBody = null; + // A save in flight: closing hands the capture release to its completion. + let saving = false; + let observedOnce = false; function set(patch) { if (closed) return; @@ -229,7 +249,7 @@ export function createImageTaggingController({api, target, source = 'images'} = function releaseToken(token) { // Best effort: an unreleased capture expires on the server's idle timeout. - api.del(CAPTURE_URL + encodeURIComponent(token)).catch(() => {}); + api.del(CAPTURE_URL + encodeURIComponent(token), {keepalive: true}).catch(() => {}); } function releaseCapture() { if (capture) releaseToken(capture.token); @@ -244,16 +264,19 @@ export function createImageTaggingController({api, target, source = 'images'} = return state.generateSupported ? '' : (state.generateReason || ''); } - async function open() { + /** Observe the file; `keepDraft` (a Retry after a changed file, storage or capture) keeps the typed draft. */ + async function open({keepDraft = false} = {}) { if (closed) return; const serial = ++openSerial; + const draft = keepDraft ? state.manual : null; + const seenRevision = state.tagRevision; stopGenerating(); releaseCapture(); readAbort?.abort(); readAbort = new AbortController(); const defaultsRead = readDefaults(api, readAbort.signal); lastBody = null; - set({phase: 'observing', statusText: '', toast: null, currentTags: null, generated: false, suggestions: [], suggestionsText: COPY.initial}); + set({phase: 'observing', statusText: '', toast: null, currentTags: null, generated: false, suggestions: [], suggestionsText: COPY.modelChanged}); let value; try { const answer = await api.post(OBSERVE_URL, {target: {...imageTarget}}); @@ -276,14 +299,21 @@ export function createImageTaggingController({api, target, source = 'images'} = epoch = value.library_epoch; defaults = await defaultsRead; if (closed || serial !== openSerial) return; + observedOnce = true; const {width, height, media_type: mediaType, preview_url: previewUrl, generate} = value.capture; + // A kept draft stays as typed unless another writer changed the set meanwhile: then the set leads it (I2). + const changed = draft !== null && value.tag_revision !== seenRevision; + const reason = generate.supported ? '' : (generate.reason || ''); set({ phase: 'ready', image: {filename: value.filename, title: value.title, width, height, mediaType, previewUrl, dimensions: width && height ? `${width} × ${height}` : null}, - savedTags: value.tags.slice(), tagRevision: value.tag_revision, manual: value.tags.join(', '), - options: {...defaults}, generateSupported: generate.supported, generateReason: generate.reason, - statusText: generate.supported ? '' : (generate.reason || ''), + savedTags: value.tags.slice(), tagRevision: value.tag_revision, + manual: draft === null ? value.tags.join(', ') : changed ? mergeCurrent(value.tags, draft) : draft, + draftRevision: state.draftRevision + 1, + options: {...defaults}, optionsRevision: state.optionsRevision + 1, + generateSupported: generate.supported, generateReason: generate.reason, + statusText: changed ? `${COPY.changedElsewhere} ${savedNow(value.tags)}${reason ? ` ${reason}` : ''}` : reason, }); } @@ -298,6 +328,7 @@ export function createImageTaggingController({api, target, source = 'images'} = const phase = state.phase === 'generating' || state.phase === 'generate-error' ? 'ready' : state.phase; set({ phase, model: id, generated: false, suggestions: [], suggestionsText: COPY.modelChanged, statusText: reasonText(), + optionsRevision: state.optionsRevision + 1, // Like the mock, a model change re-renders its fields with the defaults; the count is kept (D91). options: {...state.options, general_threshold: defaults.general_threshold, character_threshold: defaults.character_threshold, instructions: defaults.instructions}, @@ -349,44 +380,61 @@ export function createImageTaggingController({api, target, source = 'images'} = async function send(body) { const serial = ++saveSerial; lastBody = body; + saving = true; set({phase: 'saving', statusText: ''}); - let value; + let value = null, failure = null; try { - value = validateReceipt(await api.put(TAGS_URL, body)); + value = validateReceipt(await api.put(TAGS_URL, body, {keepalive: true})); } catch (error) { - if (closed || serial !== saveSerial) return false; - failSave(error); + failure = error; + } finally { + saving = false; + } + if (serial !== saveSerial) return false; + if (closed) return settleClosed(value, failure); + if (failure) { + set(failedSave(failure)); return false; } - if (closed || serial !== saveSerial) return false; lastBody = null; - const n = value.tags.length; - set({ - phase: 'saved', savedTags: value.tags.slice(), tagRevision: value.tag_revision, currentTags: null, statusText: '', - // The mock's toast verbatim (app.js:1545, without the demo's "in this tab"): "tags" even for one. - toast: `${n} tags saved for ${state.image.filename}.${source === 'review' ? COPY.reviewSuffix : ''}`, - }); + set({phase: 'saved', savedTags: value.tags.slice(), tagRevision: value.tag_revision, currentTags: null, statusText: '', + toast: toastOf(value)}); return true; } + // The mock's toast verbatim (app.js:1545, without the demo's "in this tab"): "tags" even for one. + function toastOf(value) { + return `${value.tags.length} tags saved for ${state.image.filename}.${source === 'review' ? COPY.reviewSuffix : ''}`; + } + /** The dialog closed while its save was in flight: release the capture now, then report the outcome. */ + function settleClosed(value, failure) { + releaseCapture(); + if (!failure) { + onLateSave({saved: true, filename: state.image.filename, tags: value.tags.slice(), toast: toastOf(value)}); + return true; + } + const {phase, statusText} = failedSave(failure); + // A conflict's own sentence, not its merged "Saved now" view: the dialog that would show it is gone. + const message = phase === 'unknown-outcome' ? COPY.unknownClosed : phase === 'conflict' ? failure.message : statusText; + onLateSave({saved: false, message}); + return false; + } - function failSave(error) { + /** The state patch of a failed save. */ + function failedSave(error) { if (!isApiError(error) || error.status < 400 || UNCONFIRMED_STATUS.has(error.status)) { - set({phase: 'unknown-outcome', statusText: COPY.unknown}); - return; + return {phase: 'unknown-outcome', statusText: COPY.unknown}; } lastBody = null; if (error.code === 'tag-conflict') { - // The user reviews the current set; a second Save is against the revision now shown. + // The current set is shown (status and field) ahead of the draft; a second Save is against its revision. const current = error.current; - set({phase: 'conflict', statusText: error.message, currentTags: current ? current.tags.slice() : null, - ...(current ? {tagRevision: current.tag_revision} : {})}); - return; + if (!current) return {phase: 'conflict', statusText: error.message, currentTags: null}; + return {phase: 'conflict', currentTags: current.tags.slice(), tagRevision: current.tag_revision, + statusText: `${error.message} ${savedNow(current.tags)}`, + manual: mergeCurrent(current.tags, state.manual), draftRevision: state.draftRevision + 1}; } - if (REOPEN[error.code]) { - set({phase: REOPEN[error.code], statusText: error.message}); - return; - } - set({phase: 'write-error', statusText: messageOf(error, COPY.saveFailed)}); + if (REOPEN[error.code]) return {phase: REOPEN[error.code], statusText: error.message}; + return {phase: 'write-error', statusText: messageOf(error, COPY.saveFailed)}; } async function save() { @@ -400,7 +448,8 @@ export function createImageTaggingController({api, target, source = 'images'} = async function retry() { if (closed || !state.canRetry) return false; - if (state.phase === 'observe-error') return open(); + if (state.phase === 'observe-error') return open({keepDraft: observedOnce}); + if (REOBSERVE.has(state.phase)) return open({keepDraft: true}); if (state.phase === 'generate-error') return generate(); return send(lastBody); } @@ -410,11 +459,11 @@ export function createImageTaggingController({api, target, source = 'images'} = set({closed: true}); closed = true; openSerial += 1; - saveSerial += 1; stopGenerating(); readAbort?.abort(); readAbort = null; - releaseCapture(); + // A save in flight keeps its capture until it settles (send releases it); aborting cannot undo the write. + if (!saving) releaseCapture(); } function dispose() { diff --git a/web/js/workspace/image-tagging.mjs b/web/js/workspace/image-tagging.mjs index 0cd8187..8993c27 100644 --- a/web/js/workspace/image-tagging.mjs +++ b/web/js/workspace/image-tagging.mjs @@ -5,14 +5,16 @@ * class, whose sticky `.dialog-actions` stay reachable while the content scrolls. * * Recorded deviations: D83 (#249 plan D60) the suggestions are removable chips, every one preselected, at most the - * requested count, with "Remove anything you do not want to add." and the all-removed copy; D84 (plan D61) the - * legacy engines' labels and sizes, GGUF in the concept's Ollama slot, instructions prefilled with the server's - * default; D91 (plan D68) a Suggestions count field for every model. States the concept lacks (busy, conflict, - * changed storage or file, lost answers, unavailable model, video) are only text in `#tag-generation-status`; Retry - * (ruling T5) leads the actions wherever the controller allows it, as the other workspace dialogs place theirs. + * requested count, with "Remove anything you do not want to add." while chips exist and the all-removed copy; D84 + * (plan D61) the legacy engines' labels and sizes, GGUF in the concept's Ollama slot, instructions prefilled with the + * server's default; D91 (plan D68) a Suggestions count field for every model. States the concept lacks (busy, + * conflict, changed storage or file, lost answers, unavailable model, video) are only text in + * `#tag-generation-status`; D116 (ruling T5) Retry leads the actions wherever the controller allows it, as the other + * workspace dialogs place theirs. * * The capture is released when the dialog closes (×, Escape, Cancel, Save), when the route changes (the shell - * closes its dialog) and on `pagehide`. + * closes its dialog) and on `pagehide` (which closes the dialog; the release is sent with `keepalive`). A save in + * flight at close keeps its capture until it settles, and its outcome is still toasted and announced. */ import { createButton, createScope } from '../shared/toolkit/core.js'; import { createImageTaggingController, MODEL_OPTIONS, optionError } from './image-tagging-state.mjs'; @@ -99,7 +101,8 @@ function numberInput(value, {min, max, step}) { /** * The concept's `.tag-model-panel` (a1049): `heading`, the Tagging model select, the model's options and Generate. - * It follows `controller` until `scope` is disposed (reused by the bulk Auto-tag dialog). + * It follows `controller` until `scope` is disposed (reused by the bulk Auto-tag dialog). Returns the `element` and + * `requestGenerate()`, Generate's own path (field validity first), for a Retry after a failed Generate. */ export function renderModelPanel({scope, controller, heading = 'Auto-tag this image'}) { const panel = el('div', undefined, 'tag-model-panel'); @@ -115,7 +118,7 @@ export function renderModelPanel({scope, controller, heading = 'Auto-tag this im generate.append(createWorkspaceIcon('tagImage'), ' Generate suggestions'); panel.append(el('h3', heading), field('Tagging model', select, 'image-tag-model'), options, generate); const inputs = new Map(); - let shownModel = null; + let shownModel = null, shownOptions = null; function renderOptions(state) { inputs.clear(); @@ -148,17 +151,19 @@ export function renderModelPanel({scope, controller, heading = 'Auto-tag this im } } function update(state) { - if (state.model !== shownModel) { + // A model change or the observation's server defaults rewrite the fields; typing never does (I1). + if (state.model !== shownModel || state.optionsRevision !== shownOptions) { shownModel = state.model; + shownOptions = state.optionsRevision; select.value = state.model; renderOptions(state); } setDisabled(generate, !state.canGenerate); } select.addEventListener('change', () => controller.setModel(select.value)); - generate.addEventListener('click', () => { + function requestGenerate() { const {state} = controller; - if (disabled(generate) || !state.canGenerate) return; + if (!state.canGenerate) return; // The concept's native validity first (a1104–1114), then what the controller would refuse (a blank field). for (const input of inputs.values()) input.setCustomValidity(''); const invalid = [...inputs.values()].find(input => !input.checkValidity()); @@ -171,10 +176,11 @@ export function renderModelPanel({scope, controller, heading = 'Auto-tag this im return; } controller.generate(); - }); + } + generate.addEventListener('click', () => { if (!disabled(generate)) requestGenerate(); }); scope.cleanup(controller.subscribe(update)); update(controller.state); - return panel; + return {element: panel, requestGenerate}; } function previewOf(image, state) { @@ -191,7 +197,15 @@ function previewOf(image, state) { export function openImageTagging({api, scope, image, source = 'images', trigger = null, onSaved = null}) { const target = imageTarget(image); if (!target) throw new TypeError('The image has no tagging target'); - const controller = createImageTaggingController({api, target, source}); + const saved = ({filename: name, tags, toast}) => { + const event = {target, filename: name, tags, source}; + announceSaved(event); + onSaved?.(event); + showWorkspaceToast(toast); + }; + // A save that settles after the dialog closed is still reported: its toast, and the pages' refresh (I5). + const onLateSave = outcome => (outcome.saved ? saved(outcome) : showWorkspaceToast(outcome.message)); + const controller = createImageTaggingController({api, target, source, onLateSave}); const local = createScope(); let open = true; let completed = false; @@ -220,7 +234,7 @@ export function openImageTagging({api, scope, image, source = 'images', trigger manual.addEventListener('input', () => controller.setManual(manual.value)); // 5. The model panel; 6. the suggestions. - const panel = renderModelPanel({scope: local, controller}); + const {element: panel, requestGenerate} = renderModelPanel({scope: local, controller}); const suggestions = el('section', undefined, 'tag-suggestion-panel'); suggestions.setAttribute('aria-labelledby', 'tag-suggestions-title'); const panelHeading = el('div', undefined, 'panel-heading'); @@ -247,7 +261,7 @@ export function openImageTagging({api, scope, image, source = 'images', trigger if (controller.state.showScopeNote) nodes.push(el('p', SCOPE_NOTE, 'tag-scope-note small')); nodes.push(manualField, panel, suggestions, actions); - let shownImage = null, shownRows = null, shownText = null, filled = false; + let shownImage = null, shownRows = null, shownText = null, shownDraft = 0; function renderImage(info) { shownImage = info; const next = previewOf(null, {image: info}); @@ -268,7 +282,7 @@ export function openImageTagging({api, scope, image, source = 'images', trigger if (!state.suggestions.length) { list.replaceChildren(el('p', state.suggestionsText, 'small')); } else { - // D83: removable chips instead of the concept's checkbox rows. + // D83: removable chips instead of the concept's checkbox rows, under ruling T4's instruction. const chips = el('div', undefined, 'tags'); for (const row of state.suggestions) { const chip = el('span', undefined, 'chip tag-suggestion'); @@ -278,7 +292,7 @@ export function openImageTagging({api, scope, image, source = 'images', trigger chip.append(el('span', row.name), el('span', row.label, 'tag-confidence'), remove); chips.append(chip); } - list.replaceChildren(chips); + list.replaceChildren(el('p', state.chipHint, 'small'), chips); } if (focused >= 0) { const buttons = [...list.querySelectorAll('button')]; @@ -287,18 +301,16 @@ export function openImageTagging({api, scope, image, source = 'images', trigger } function complete(state) { completed = true; - const event = {target, filename: state.image.filename, tags: state.savedTags.slice(), source}; closeWorkspaceDialog(); - announceSaved(event); - onSaved?.(event); - showWorkspaceToast(state.toast); + saved({filename: state.image.filename, tags: state.savedTags.slice(), toast: state.toast}); } function render(state) { if (!open || completed) return; if (state.image && state.image !== shownImage) renderImage(state.image); - if (!filled && state.phase !== 'observing' && state.image) { - filled = true; - manual.value = state.manual; + // The observation's saved tags, a kept draft, or a conflict's current set ahead of the draft (I2). + if (state.draftRevision !== shownDraft) { + shownDraft = state.draftRevision; + if (manual.value !== state.manual) manual.value = state.manual; } if (state.suggestions !== shownRows || state.suggestionsText !== shownText) renderSuggestions(state); setText(count, state.countText); @@ -306,7 +318,7 @@ export function openImageTagging({api, scope, image, source = 'images', trigger setText(primary, state.primaryLabel); setDisabled(primary, !state.canSave); if (state.canRetry !== retry.isConnected) { - if (state.canRetry) actions.prepend(retry); + if (state.canRetry) actions.prepend(retry); // D116 else { if (document.activeElement === retry) primary.focus(); retry.remove(); @@ -316,12 +328,14 @@ export function openImageTagging({api, scope, image, source = 'images', trigger } cancel.addEventListener('click', () => closeWorkspaceDialog()); - retry.addEventListener('click', () => controller.retry()); + // Retry after a failed Generate takes Generate's own path, so an invalid field reports its validity (M4). + retry.addEventListener('click', () => (controller.state.phase === 'generate-error' ? requestGenerate() : controller.retry())); primary.addEventListener('click', () => { if (disabled(primary) || !controller.state.canSave) return; controller.save(); }); - const pagehide = () => controller.dispose(); + // Leaving the page closes the dialog, so a page restored from the back/forward cache shows no dead dialog (I4). + const pagehide = () => { if (open) closeWorkspaceDialog(); }; window.addEventListener('pagehide', pagehide); function release() { diff --git a/web/js/workspace/images-favorites.mjs b/web/js/workspace/images-favorites.mjs new file mode 100644 index 0000000..7767278 --- /dev/null +++ b/web/js/workspace/images-favorites.mjs @@ -0,0 +1,81 @@ +/** + * The media card's star (#249 D7 = A: the concept's `actions.favorite` against the favorites store), for the Images + * page and every Library-embedded gallery (PR E). Its toasts are the concept's without the demo's "in this tab"; the + * concept has no failure copy, so a failure follows the workspace "could not be …" pattern. + * + * One favorite request per file at a time (TESTING.md): the requests outlive the page that sent them, so the keys in + * flight and the pages that receive each answer live here. Extracted from images.mjs unchanged. + */ +import { imageKey } from './images-state.mjs'; +import { showWorkspaceToast } from './toast.mjs'; + +const FAVORITE_URL = '/prompt_manager/images/favorite'; +const FAVORITE_DONE = Object.freeze({true:'Added to favorites.', false:'Removed from favorites.'}); +const FAVORITE_FAILED = Object.freeze({true:'Favorite could not be added. Try again.', false:'Favorite could not be removed. Try again.'}); + +const favoritePending = new Set(); +let pageTarget = null; +// Library-embedded galleries showing media cards; keyed like the Images page, so each repaints its own copy. +const embeddedTargets = new Set(); +let visitTarget = () => {}; +// Told after every confirmed favorite change, wherever it was made (the shell's Favorites count, #249 D51). +const favoriteListeners = new Set(); + +/** Run `listener()` after each confirmed favorite change; returns the unsubscribe. */ +export function onFavoriteChange(listener) { + favoriteListeners.add(listener); + return () => favoriteListeners.delete(listener); +} +/** Whether the favorite request of the file `key` is still in flight. */ +export function favoriteBusy(key) { + return favoritePending.has(key); +} +/** `target(key, favorite)` patches the saved Images visit while no Images page is mounted. */ +export function setFavoriteVisitTarget(target) { + visitTarget = target; +} +/** Receive favorite states with `setFavorite(key, favorite)` until `scope` ends: the Images page or an embedded gallery. */ +export function receiveFavorites({scope, embedded, setFavorite}) { + if (!embedded) { + pageTarget = setFavorite; + scope.cleanup(() => { if (pageTarget === setFavorite) pageTarget = null; }); + } else { + embeddedTargets.add(setFavorite); + scope.cleanup(() => embeddedTargets.delete(setFavorite)); + } +} +/** Give a favorite state to the mounted Images page, else the saved visit, and to every embedded gallery. */ +function deliver(key, favorite) { + if (pageTarget) pageTarget(key, favorite); + else visitTarget(key, favorite); + for (const target of embeddedTargets) target(key, favorite); +} + +/** A star whose request is still in flight is marked busy: pressing it again does nothing until the answer. */ +export function paintStar(button, favorite, pending) { + button.textContent = favorite ? '★' : '☆'; + button.setAttribute('aria-pressed', String(favorite)); + if (pending) button.setAttribute('aria-disabled', 'true'); else button.removeAttribute('aria-disabled'); +} + +/** Optimistic toggle: the star changes at once and returns to its previous state if the store refuses. */ +export async function toggleFavorite(api, image) { + const key = imageKey(image); + if (favoritePending.has(key)) return; + const favorite = !image.favorite; + favoritePending.add(key); + deliver(key, favorite); + const body = {root_index: image.root_index, path: image.path}; + let saved = false; + try { + await (favorite ? api.put(FAVORITE_URL, body) : api.del(FAVORITE_URL, {body})); + saved = true; + } catch { + // Announced below; the star returns to its previous state. + } + favoritePending.delete(key); + deliver(key, saved ? favorite : !favorite); + showWorkspaceToast(saved ? FAVORITE_DONE[favorite] : FAVORITE_FAILED[favorite]); + // On Favorites an unstarred card stays (with ☆) until the next load, so the star can be undone in place (D51). + if (saved) for (const listener of favoriteListeners) listener(); +} diff --git a/web/js/workspace/images-selection.mjs b/web/js/workspace/images-selection.mjs index ec985c6..6938e52 100644 --- a/web/js/workspace/images-selection.mjs +++ b/web/js/workspace/images-selection.mjs @@ -25,6 +25,7 @@ function el(tag, text, className) { * - `getItems()` is the listing in its order; `checked` restores a saved visit's selection. * - `onCategorySet()` reloads the categories and the listing after a successful Set category (the reload clears the * selection through `clear`); focus then moves to `focusFallback` if it was on the hidden action. + * - `retain(kept)` drops the checked files a refreshed listing no longer holds (a tag save under a filter, I7). */ export function mountSelectionBar({ scope, api, grid, getItems, focusFallback, onCategorySet, checked: initial = Object.freeze(new Set()) }) { let checked = initial; @@ -54,6 +55,13 @@ export function mountSelectionBar({ scope, api, grid, getItems, focusFallback, o for (const box of grid.querySelectorAll(CHECKBOX)) box.checked = false; render(); } + /** Keep only the checked files `kept(key)` still lists (a refreshed result), so the count is what the actions apply to. */ + function retain(kept) { + const next = [...checked].filter(kept); + if (next.length === checked.size) return; + checked = Object.freeze(new Set(next)); + render(); + } function checkedImages() { return getItems().filter(image => checked.has(imageKey(image))); } @@ -142,6 +150,7 @@ export function mountSelectionBar({ scope, api, grid, getItems, focusFallback, o isChecked: key => checked.has(key), checked: () => checked, clear, + retain, /** Whether a Set category write is still in flight. */ writing: () => categorizing, }; diff --git a/web/js/workspace/images-state.mjs b/web/js/workspace/images-state.mjs index 211dc70..d133eba 100644 --- a/web/js/workspace/images-state.mjs +++ b/web/js/workspace/images-state.mjs @@ -9,6 +9,8 @@ export function imageKey(image) { export const IMAGE_SORTS = Object.freeze(['newest', 'oldest', 'name', 'name-desc', 'rating']); const isCount = value => Number.isSafeInteger(value) && value >= 0; +// The largest page a refresh asks for (the listing clamps at 500). +const REFRESH_PAGE_LIMIT = 200; function queryFor(filters, batchSize) { const query = new URLSearchParams({ mode: 'workspace', limit: String(batchSize) }); @@ -110,8 +112,10 @@ export function createImageStream({ api, onChange = () => {}, batchSize = 40, re onChange(state); } - // A refresh in flight (its abort controller): Load more waits for it, a reset or dispose cancels it. + // A refresh in flight (its abort controller): Load more waits for it, a reset or dispose cancels it. A refresh + // asked while a page loads is pending and runs once that page settles (I6). let refreshing = null; + let refreshPending = false; async function loadMore() { if (disposed || state.loading || !state.hasMore || refreshing) return 0; @@ -148,16 +152,29 @@ export function createImageStream({ api, onChange = () => {}, batchSize = 40, re publish({ loading: false, error }); } return 0; + } finally { + runPendingRefresh(request); } } + function runPendingRefresh(request) { + // Only the page this load owned: a reset or dispose already dropped the pending refresh. + if (disposed || !refreshPending || state.loading || request.signal.aborted) return; + refreshPending = false; + // A failure is the page's to report on the refresh it asked for; here it has no caller left. + refresh().catch(() => {}); + } /** * Re-read the loaded range of the same query (a write may have changed which files match) and publish the new - * collection once, with the hint `{replaced: true}`; the current items stay shown until then. A failure, a reset - * or a dispose leaves the collection as it was. Resolves to the number of items now loaded (0 when abandoned). + * collection once, with the hint `{replaced: true}`; the current items stay shown until then. Pages are as large + * as the rest of the range (at most REFRESH_PAGE_LIMIT, at least one batch), read one after another (each page's + * cursor comes from the one before). A read failure rejects and leaves the collection as it was; a reset or a + * dispose abandons it. Resolves to the number of items now loaded (0 when abandoned, or deferred behind a page + * load in flight, which runs it once settled). */ async function refresh() { - if (disposed || state.loading) return 0; + if (disposed) return 0; + if (state.loading) { refreshPending = true; return 0; } refreshing?.abort(); const request = refreshing = new AbortController(); const target = Math.max(state.loaded, 1); @@ -166,6 +183,7 @@ export function createImageStream({ api, onChange = () => {}, batchSize = 40, re try { while (items.length < target && hasMore) { const parameters = new URLSearchParams(query); + parameters.set('limit', String(Math.min(REFRESH_PAGE_LIMIT, Math.max(target - items.length, batchSize)))); if (cursor !== null) parameters.set('cursor', cursor); const page = await readPage({parameters, signal:request.signal}); if (disposed || request !== refreshing) return 0; @@ -179,9 +197,11 @@ export function createImageStream({ api, onChange = () => {}, batchSize = 40, re if (cursor === null) positiveCount = isCount(page.positive_count) ? page.positive_count : null; ({total, next_cursor: cursor, has_more: hasMore} = page); } - } catch { - if (request === refreshing) refreshing = null; - return 0; + } catch (error) { + // A newer refresh, a reset or a dispose replaced this one (and aborted it): nothing to report. + if (request !== refreshing) return 0; + refreshing = null; + throw error; } refreshing = null; seen = keys; @@ -198,6 +218,7 @@ export function createImageStream({ api, onChange = () => {}, batchSize = 40, re controller = null; refreshing?.abort(); refreshing = null; + refreshPending = false; query = nextQuery; seen = new Set(); publish({ items: Object.freeze([]), total: 0, cursor: null, hasMore: true, @@ -210,6 +231,8 @@ export function createImageStream({ api, onChange = () => {}, batchSize = 40, re reset, loadMore, refresh, + /** Whether a refresh is re-reading the loaded range (Load more waits for it). */ + get refreshing() { return refreshing !== null; }, /** Patch one loaded item (a favorite toggle); false when it is not loaded or the stream is gone. */ updateItem(key, patch) { const next = withItem(state, key, patch); @@ -222,6 +245,7 @@ export function createImageStream({ api, onChange = () => {}, batchSize = 40, re dispose() { if (disposed) return; disposed = true; + refreshPending = false; controller?.abort(); controller = null; refreshing?.abort(); diff --git a/web/js/workspace/images-tagging.mjs b/web/js/workspace/images-tagging.mjs new file mode 100644 index 0000000..babfd78 --- /dev/null +++ b/web/js/workspace/images-tagging.mjs @@ -0,0 +1,42 @@ +/** + * The Images page's side of image tagging (#249 PR C): a card's Tag opens the Tag image dialog, and every confirmed + * tag save re-reads what it may have changed on a mounted page. + * + * Under a Tags filter or a search the saved file may start or stop matching, so the loaded range is re-read in place + * (`createImageStream().refresh()`), keeping the scroll and the selection while it still matches; a failed re-read + * toasts and leaves the cards as they were. The selected file's inspection is then re-read quietly (its envelope + * carries the image tags), without a loading flash. + */ +import { isTagButton, onImageTagsSaved, openImageTagging } from './image-tagging.mjs'; +import { showWorkspaceToast } from './toast.mjs'; + +export const REFRESH_FAILED = 'Images could not be refreshed after the tag save. Refresh image collection to retry.'; + +/** Open Tag image for `image` when `action` is its card's Tag button (without selecting the card, as the concept's + * `image-tag` does); false for any other control. */ +export function openCardTagging({api, scope, image, action, embedded}) { + if (!isTagButton(action)) return false; + openImageTagging({api, scope, image, source: embedded ? 'library' : 'images', trigger: action}); + return true; +} + +/** + * Follow confirmed tag saves until `scope` ends. `filtered()` tells whether a Tags filter or a search is active, + * `selection()` is the selected image key, and `onRefreshing(busy)` marks Load more busy while the re-read runs. + */ +export function followImageTagSaves({scope, stream, inspection, filtered, selection, onRefreshing}) { + scope.cleanup(onImageTagsSaved(async ({target}) => { + const key = `${target.root_index}:${target.path}`; + if (filtered()) { + onRefreshing(true); + try { + await stream.refresh(); + } catch { + if (!scope.signal.aborted) showWorkspaceToast(REFRESH_FAILED); + } finally { + if (!scope.signal.aborted) onRefreshing(false); + } + } + if (!scope.signal.aborted && selection() === key) inspection.refresh(); + })); +} diff --git a/web/js/workspace/images.mjs b/web/js/workspace/images.mjs index d56ef9d..19045c0 100644 --- a/web/js/workspace/images.mjs +++ b/web/js/workspace/images.mjs @@ -15,7 +15,10 @@ import { openImportDialog, openRefineDialog } from './workspace-dialogs.mjs'; import { openMetadataScan } from './activity-scan.mjs'; import { showWorkspaceToast } from './toast.mjs'; import { imagesFilters, imagesViewState } from './saved-views.mjs'; -import { createTagButton, isTagButton, openImageTagging, onImageTagsSaved } from './image-tagging.mjs'; +import { createTagButton, onImageTagsSaved } from './image-tagging.mjs'; +import { followImageTagSaves, openCardTagging } from './images-tagging.mjs'; +import { favoriteBusy, paintStar, receiveFavorites, setFavoriteVisitTarget, toggleFavorite } from './images-favorites.mjs'; +export { onFavoriteChange } from './images-favorites.mjs'; /** The concept's panel without an image (app.js updateImages a456–458). */ function noSelection() { @@ -74,38 +77,17 @@ function rescanFinished(result) { if (![total, roots].every(value => Number.isSafeInteger(value) && value >= 0)) return 'Rescan finished.'; return `Rescan finished. ${total.toLocaleString('en-US')} ${noun(total, 'file')} in ${roots.toLocaleString('en-US')} output ${noun(roots, 'folder')}.`; } -// D7 = A: the concept's star (app.js actions.favorite) against the favorites store; its toasts without the demo's -// "in this tab". The concept has no failure copy, so a failure follows the workspace "could not be …" pattern. -const FAVORITE_URL = '/prompt_manager/images/favorite'; -const FAVORITE_DONE = Object.freeze({true:'Added to favorites.', false:'Removed from favorites.'}); -const FAVORITE_FAILED = Object.freeze({true:'Favorite could not be added. Try again.', false:'Favorite could not be removed. Try again.'}); // The concept card's first `.media-info` reservation and list row (image-layout.mjs); real rows are measured. const INITIAL_METRICS = Object.freeze({columns:1, gap:13, footerHeight:54, listHeight:97}); let lastVisit = null; -// One favorite request per file at a time (TESTING.md): the requests outlive the Images page that sent them, so the keys -// in flight and the page that receives each answer live here, beside the saved visit. -const favoritePending = new Set(); -let favoriteTarget = null; -// Library-embedded galleries showing media cards (#249 PR E); keyed like the Images page, so each repaints its own copy. -const embeddedFavoriteTargets = new Set(); -/** - * Give a favorite state to the mounted Images page, or to the saved visit while none is mounted, and to every mounted - * Library-embedded gallery. - */ -function deliverFavorite(key, favorite) { - if (favoriteTarget) favoriteTarget(key, favorite); - else if (lastVisit?.state) lastVisit = {...lastVisit, state: withItem(lastVisit.state, key, {favorite})}; - for (const target of embeddedFavoriteTargets) target(key, favorite); -} -// Told after every confirmed favorite change, wherever it was made (the shell's Favorites count, #249 D51). -const favoriteListeners = new Set(); -/** Run `listener()` after each confirmed favorite change; returns the unsubscribe. */ -export function onFavoriteChange(listener) { - favoriteListeners.add(listener); - return () => favoriteListeners.delete(listener); -} +// The star's answers patch the saved visit while no Images page is mounted (D7). +setFavoriteVisitTarget((key, favorite) => { + if (lastVisit?.state) lastVisit = {...lastVisit, state: withItem(lastVisit.state, key, {favorite})}; +}); +// A tag save made while Images is left (Tags › Auto-tag images) may change what a filtered visit lists: it reloads (M6). +onImageTagsSaved(() => { if (lastVisit?.filters.tags.length || lastVisit?.filters.q) lastVisit = {...lastVisit, stale: true}; }); /** The image selected in Images when it was last left (the concept's `state.selectedImage`, for Tags › Auto-tag images). */ export function selectedImage() { const key = lastVisit?.selection; @@ -134,10 +116,12 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage scope.cleanup(() => inspection.dispose()); scope.cleanup(() => detailInspector?.dispose()); const saved = embedded || !savedState ? null : imagesFilters(savedState); - const visit = embedded || saved || favorites ? null : lastVisit; - let density = visit?.density || 'comfortable'; + const prior = embedded || saved || favorites ? null : lastVisit; + // A stale visit keeps its filters, layout and size, and lists again instead of restoring (M6). + const visit = prior?.stale ? null : prior; + let density = prior?.density || 'comfortable'; let metrics = {...INITIAL_METRICS}; - let layout = visit?.layout || 'grid', selection = visit?.selection || null, frame = 0, loadFrame = 0, restoreFrame = 0, observer; + let layout = prior?.layout || 'grid', selection = visit?.selection || null, frame = 0, loadFrame = 0, restoreFrame = 0, observer; let state, geometry = { positions: [], columns: 1, height: 0, maxHeight: 0 }, galleryWidth = 0, optionsRequest, categoryRequest, recovering = false, rescanning = false; // Discovered sources (each with its own folders) and every folder, for "Refine this view" and the scope chip. let roots = [], folders = []; @@ -145,7 +129,7 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage let refine = null; const rendered = new Map(); const itemIndexes = new Map(); - const filters = { root: 'all', q: '', category: '', tags: [], subfolder: '', media: 'all', sort: 'newest', ...visit?.filters, ...saved, + const filters = { root: 'all', q: '', category: '', tags: [], subfolder: '', media: 'all', sort: 'newest', ...prior?.filters, ...saved, favorites: favorites || saved?.favorites === true }; filters.tags = [...filters.tags]; if (!IMAGE_SORT_OPTIONS.some(([value]) => value === filters.sort)) filters.sort = 'newest'; @@ -339,7 +323,7 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage if (typeof image.favorite === 'boolean') { const star = createButton('', 'star-button'); star.setAttribute('aria-label', `Favorite ${image.filename}`); - paintStar(star, image.favorite, favoritePending.has(key)); + paintStar(star, image.favorite, favoriteBusy(key)); overlays.append(star); } art.append(open, overlays); @@ -357,67 +341,24 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage markSelected(card, key === selection); return card; } - /** A star whose request is still in flight is marked busy: pressing it again does nothing until the answer. */ - function paintStar(button, favorite, pending) { - button.textContent = favorite ? '★' : '☆'; - button.setAttribute('aria-pressed', String(favorite)); - if (pending) button.setAttribute('aria-disabled', 'true'); else button.removeAttribute('aria-disabled'); - } function setFavorite(key, favorite) { if (scope.signal.aborted) return; stream.updateItem(key, {favorite}); const button = rendered.get(key)?.querySelector('.star-button'); - if (button) paintStar(button, favorite, favoritePending.has(key)); + if (button) paintStar(button, favorite, favoriteBusy(key)); } // The Images page and every Library-embedded gallery receive favorite answers (#249 D7, PR E). - if (!embedded) { - favoriteTarget = setFavorite; - scope.cleanup(() => { if (favoriteTarget === setFavorite) favoriteTarget = null; }); - } else { - embeddedFavoriteTargets.add(setFavorite); - scope.cleanup(() => embeddedFavoriteTargets.delete(setFavorite)); - } - /** Optimistic toggle: the star changes at once and returns to its previous state if the store refuses. */ - async function toggleFavorite(image) { - const key = imageKey(image); - if (favoritePending.has(key)) return; - const favorite = !image.favorite; - favoritePending.add(key); - deliverFavorite(key, favorite); - const body = {root_index: image.root_index, path: image.path}; - let saved = false; - try { - await (favorite ? api.put(FAVORITE_URL, body) : api.del(FAVORITE_URL, {body})); - saved = true; - } catch { - // Announced below; the star returns to its previous state. - } - favoritePending.delete(key); - deliverFavorite(key, saved ? favorite : !favorite); - showWorkspaceToast(saved ? FAVORITE_DONE[favorite] : FAVORITE_FAILED[favorite]); - // On Favorites an unstarred card stays (with ☆) until the next load, so the star can be undone in place (D51). - if (saved) for (const listener of favoriteListeners) listener(); - } + receiveFavorites({scope, embedded, setFavorite}); scope.listen(grid, 'click', event => { const card = event.target.closest('article[data-image-key]'); if (!card) return; const image = state.items.find(item => imageKey(item) === card.dataset.imageKey); if (!image) return; const action = event.target.closest('.image-card-actions button'); - if (event.target.closest('.star-button')) toggleFavorite(image); + if (event.target.closest('.star-button')) toggleFavorite(api, image); else if (event.target.closest('.image-open')) previewImage(image); - // Tag opens the dialog without selecting the card, as the concept's `image-tag` does. - else if (isTagButton(action)) openImageTagging({api, scope, image, source: embedded ? 'library' : 'images', trigger: action}); - else if (action) inspect(image); + else if (action && !openCardTagging({api, scope, image, action, embedded})) inspect(image); }); - // A saved tag set re-reads the selected file's details (its inspector envelope carries the image tags) without a - // loading flash. Under a Tags filter or a search the saved file may now match or stop matching: the loaded range - // is re-read in place first, keeping the scroll and the selection while it still matches. - scope.cleanup(onImageTagsSaved(async ({target}) => { - const key = `${target.root_index}:${target.path}`; - if (!embedded && !recovering && (filters.tags.length || filters.q)) await stream.refresh(); - if (!scope.signal.aborted && selection === key) inspection.refresh(); - })); function renderVisible() { if (!state || scope.signal.aborted) return; @@ -561,8 +502,10 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage if (!next.items.length || replaced) itemIndexes.clear(); for (let index = replaced ? 0 : state?.items.length || 0; index < next.items.length; index++) itemIndexes.set(imageKey(next.items[index]), index); } - // The selected file left the refreshed result: the panel follows the collection, as after a reload. + // The selected file left the refreshed result: the panel follows the collection, as after a reload; the + // selection bar keeps only the checked files still listed (I7). if (replaced && selection !== null && !itemIndexes.has(selection)) dropSelection(); + if (replaced) bar?.retain(key => itemIndexes.has(key)); state = next; if (!embedded) { setText(count, countLine(next)); @@ -586,12 +529,15 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage scheduleLoad(); } const stream = createImageStream({ api, fetchPage, onChange: renderState, restore: visit ? {filters, state:visit.state} : undefined }); + followImageTagSaves({scope, stream, inspection, selection:() => selection, filtered:() => !embedded && !recovering && Boolean(filters.tags.length || filters.q), + onRefreshing:busy => more.setAttribute('aria-disabled', String(busy || Boolean(state?.loading) || recovering))}); scope.cleanup(() => { stream.dispose(); for (const video of body.querySelectorAll('video')) releaseVideo(video); }); function pressLoad() { - if (recovering || state?.loading) return; + // A refresh re-reads the loaded range: Load more waits for it (and focus stays where it is, M3). + if (recovering || state?.loading || stream.refreshing) return; focusAfterLoad = state?.loaded ?? 0; stream.loadMore(); } @@ -631,26 +577,23 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage scopeText.textContent = ` ${scopeLabel(filters, roots)}`; toolbar?.setActiveFilters(activeRefinements(filters)); } - function dropSelection() { - inspection.clear(); - detailInspector?.dispose(); detailInspector = null; + /** No selected file; `keepPanel` (recovering sources) leaves the panel's details shown. */ + function dropSelection({keepPanel = false} = {}) { for (const video of details.querySelectorAll('video')) releaseVideo(video); - details.replaceChildren(noSelection()); - selection = null; - } - function reset() { - mediaViewer.stopSlideshow(); - main.querySelector('#viewer-dialog[open]')?.close(); - if (!recovering) { + if (!keepPanel) { inspection.clear(); detailInspector?.dispose(); detailInspector = null; details.replaceChildren(noSelection()); } selection = null; + } + function reset() { + mediaViewer.stopSlideshow(); + main.querySelector('#viewer-dialog[open]')?.close(); + dropSelection({keepPanel:recovering}); focusAfterLoad = null; // The ruled selection lifetime: a replaced collection may no longer hold the checked files. bar?.clear(); - for (const video of details.querySelectorAll('video')) releaseVideo(video); // A toolbar change while discovery is still pending keeps the saved Source and Folder; the settled discovery // then lists the view with every current value (#249 M3). if (savedScopePending && !discoverySettled) { updateScope(); return; }