From 2323aa0be5f9ba038d092cd2493066ab10dd9d6b Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Fri, 23 Oct 2026 03:35:34 -0700 Subject: [PATCH] fix(system): thumbnail row read, tracked health job and guarded host save MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The Overview's thumbnail row is its own read of the thumbnail status: "Checking thumbnails…" until it answers, unavailable when it fails. - The health job is tracked in the Activity store (new store.track) and followed through its event stream instead of polling every 150 ms. - A host save before a read returned a revision fails without a request; System reads the host values on mount and after a conflict. - "100+ problems found" when the integrity count is capped. - D-NEW-S2-5 is D123 everywhere; the unused Overview hand-off is gone. - The concept spec builds the health report from the concept's own rows and asserts the full unmapped path list; surface tops pin the 18px offset. - TESTING.md: D96's reason, the read-only (WAL/SHM) standard, the UNC URI and the new behavior. Coverage floors raised for the touched files. --- TESTING.md | 62 ++++++--- tests/browser/system-concept.spec.mjs | 77 +++++++++--- tests/browser/workspace-system.spec.mjs | 29 ++++- tests/browser/workspace.spec.mjs | 2 +- tests/js/workspace-activity-store.test.js | 18 +++ tests/js/workspace-system-overview.test.js | 125 ++++++++++++++----- tests/js/workspace-system-state.test.js | 37 ++++++ tools/vendor/frontend-coverage-baseline.json | 16 +-- web/js/workspace/activity-store.mjs | 10 ++ web/js/workspace/index.mjs | 2 +- web/js/workspace/system-overview.mjs | 121 ++++++++++++------ web/js/workspace/system-state.mjs | 13 +- web/js/workspace/system.mjs | 19 +-- 13 files changed, 411 insertions(+), 120 deletions(-) diff --git a/TESTING.md b/TESTING.md index a2f55e53..09089c83 100644 --- a/TESTING.md +++ b/TESTING.md @@ -1811,21 +1811,40 @@ listeners and dialogs never outlive it. Drafts (`system-state.mjs`) live for the page session (Q10). Save writes the dirty browser fields, then the host fields (`POST /prompt_manager/settings` with `expected_revision`, #220), then the runtime fields. A failed domain keeps its drafts and -highlights their controls; `settings-changed` is a conflict and keeps the draft. No -delivered section edits a setting yet, so Save stays disabled until Appearance (S3). +highlights their controls; `settings-changed` is a conflict and keeps the draft. System +reads the host values on mount and again after a conflict; until a read returned a +revision, a host save fails without a request, so it never sends a blind +`expected_revision: null` (which S1 merges into the file on disk). No delivered section +edits a setting yet, so Save stays disabled until Appearance (S3). Overview (`system-overview.mjs`) reads `GET /prompt_manager/system/overview` once per visit through `latestOnly()` on the section scope; a late answer after the section left paints nothing. Until it answers, the section shows "Loading the overview…"; a failure shows "The overview could not be loaded. Your saved data has not changed." and Retry, -never zeros. The health check (`POST /prompt_manager/system/health`) is a `read` +never zeros. The thumbnail row is a separate read of `/images/thumbnails/status` (a +walk of every source), so the other values never wait for it: "Checking thumbnails…" +until it answers. The health check (`POST /prompt_manager/system/health`) is a `read` registry job of kind `health`: one at a time, beside a scan, not cancellable and with -no file outcomes. A press while one runs (another tab) waits for that one. The last -report lives for this server session (`GET /prompt_manager/system/health`). Statistics +no file outcomes. The page tracks the started job in the Activity store (its event +stream; the store marks it lost if the server stops listing it), never by polling. A press while one runs (another tab) waits +for that one. The last report lives for this server session (`GET +/prompt_manager/system/health`) and describes the database it was taken from: a +restore that replaces the database forgets it, and a check that started before the +swap is not kept. The check's own connection opens a `mode=ro` URI that keeps a Windows +UNC path (`file:////server/share/prompts.db`; pathlib's `file://server/share/...` is a +remote authority SQLite refuses). A check that raises fails only itself, and the +missing-files check reads its rows inside the snapshot but stats the files after it +ended, so a slow source never pins the WAL. Statistics read `GET /prompt_manager/system/statistics` (`by_category`, the legacy `/stats` totals and `rated_images`). -Recorded deviations (plan D60–D64 renumbered D92–D96; D121 and D122 assigned): +Read-only standard for a check or preview (S2's health check; S6 and S8 previews use +the same): the database and `-wal` bytes are unchanged, the `-shm` size is unchanged, +the connection's `total_changes` is 0 at close and the data reads back unchanged. The +`-shm` bytes are not compared: every WAL reader rewrites its read marks there, so no +reader can keep it byte-identical (`test_health_check_never_writes`). + +Recorded deviations (plan D60–D64 renumbered D92–D96; D121, D122 and D123 assigned): - **D92** (shell): no "Sample configuration" chip. The Save status is truthful: "No unsaved changes.", "Unsaved changes.", "Saved in this browser." / "Saved to the server @@ -1841,8 +1860,9 @@ Recorded deviations (plan D60–D64 renumbered D92–D96; D121 and D122 assigned "Ambiguous image links" reads "Not available yet." with its Review metadata action `aria-disabled` (CSS under D93) until the Needs review queue reports a count (Q6). "Backup status" reads "No retained backup record." until verified backups exist (S8). - The thumbnail row is "No missing thumbnails", "N missing thumbnails" (the pending count - of `/images/thumbnails/status`) or "Thumbnail status unavailable"; Review thumbnails + The thumbnail row is "Checking thumbnails…" until its own read answers, then "No + missing thumbnails", "N missing thumbnails" (the pending count of + `/images/thumbnails/status`) or "Thumbnail status unavailable"; Review thumbnails opens Sources until the thumbnail dialog (S5). "One display configuration" says "Grid and density settings are shared across views." (no paging). The sources row is "N output sources" with "Monitoring and enrichment share one source list.", prefixed @@ -1857,7 +1877,8 @@ Recorded deviations (plan D60–D64 renumbered D92–D96; D121 and D122 assigned files, Ambiguous metadata, Consistency (added). Missing files has one row per image source when there are several ("Missing files · output"). Each chip is the real result: "Passed", a count with what was found ("2 broken references", "1 link needs - review", the concept's "N links need review" and "N images need review"), "Not + review", the concept's "N links need review" and "N images need review"; "100+ + problems found" when SQLite's integrity check stopped at its limit), "Not available" (an unavailable source), "Not available yet" (Ambiguous metadata, Q6) or "Could not check". A `p.small` after the list says when it was checked and whether every check passed, some could not run, or issues need review. Close / Review missing @@ -1868,13 +1889,15 @@ Recorded deviations (plan D60–D64 renumbered D92–D96; D121 and D122 assigned then a second table "Library totals" (Total / Count: Prompts, Tags, Images from the legacy statistics, and Rated images: indexed images whose linked prompt is rated). The Overview row says "Runs and ratings by category, with library totals." because - prompt, tag and date usage is not recorded. A failed read toasts "Statistics could not + that is what the dialog shows (the concept's own dialog shows only "Runs by + category"); per-prompt run counts, prompt tags and dated runs are recorded, but no + table of them exists yet. A failed read toasts "Statistics could not be loaded. Your saved data has not changed." - **D121** (production hub): the Overview hub's System card reads the concept's System heading and "Open System"; the concept has no hub card. - **D122** (shell): the section nav's `aria-label` is "System sections"; the concept's literal label is its prototype action name "system-tab sections". -- **D-NEW-S2-5** (Maintenance, pending a number): the concept's Maintenance notice +- **D123** (Maintenance): the concept's Maintenance notice ("Check first. Review changes. Then apply." + Run health checks) is delivered ahead of the rest of Maintenance, above its D94 hand-off, as the native place that runs the health check. A press shows "Checking…" (`aria-disabled`) and opens the report; a @@ -1882,15 +1905,20 @@ Recorded deviations (plan D60–D64 renumbered D92–D96; D121 and D122 assigned `tests/browser/system-concept.spec.mjs` compares the shell and Overview with the concept (dark and light, 1600×1100 and 390×844), mapping only the deviations above; unmapped, -the difference is exactly the D92, D93, D96 and D122 paths. `statistics and health -dialogs match the concept` maps D95 and D96 and compares the Maintenance notice -unchanged. `system comparison detects mutations` removes a row, renames a metric label, -swaps the `--line` token, recolours a chip, removes the totals table and renames a -caption. Paired captures (`{concept,production}-system-{dark,light}-{1600,390}.png`, +the difference is exactly the D92, D93, D96 and D122 paths. Each surface's `top` is +measured, which pins the concept's `.surface + .surface` 18px offset that concept.css +also applies inside `.two-col` (copied as is). `statistics and health dialogs match +the concept` maps D95 from the concept's own five rows (labels verbatim): it inserts +Pair uniqueness after Canonical hashes and Consistency last, splits Missing files per +source when there are several, replaces only the chips with the fixture's expected +results, and asserts the full unmapped path list; it maps D96 and compares the +Maintenance notice unchanged. `system comparison detects mutations` removes a row, +renames a metric label, swaps the `--line` token, recolours a chip, renames a check, +removes the totals table and renames a caption. Paired captures (`{concept,production}-system-{dark,light}-{1600,390}.png`, `{concept,production}-{statistics,health}-{dark,light}.png`) are test artifacts; there are no PNG baselines. `tests/browser/workspace-system.spec.mjs` covers the real counts, the health check and metric 4, a failed check, the thumbnail row against the status -endpoint, the review row (Q6), the section links, the Statistics dialog against the +endpoint (painted after the rest when its read is held, unavailable when it fails), the review row (Q6), the section links, the Statistics dialog against the route, a load failure with Retry and late answers after leaving. Its hooks are `tests/browser/workspace_system_fixture.py` (`/__test__/system/*`: reset, injected failures, an absent source, a deleted linked file, unthumbnailed files) on the diff --git a/tests/browser/system-concept.spec.mjs b/tests/browser/system-concept.spec.mjs index aa4904fd..c32a62e6 100644 --- a/tests/browser/system-concept.spec.mjs +++ b/tests/browser/system-concept.spec.mjs @@ -5,7 +5,7 @@ import { SIZES, THEMES, seedConcept, openConcept, openProduction, diffInventory, // #249 System S2 Task 6: System's shell and its Overview section (app.js renderSystem 658–706), the Statistics dialog // ("Usage statistics", a1600–1604) and the health report (a1439–1456) against the immutable concept, on the // concept-shaped real fixture. Each concept measurement is mapped only by the recorded deviations (D92–D96, D121, -// D122, D-NEW-S2-5), each commented where it applies; after that the difference must be empty, and a mutation of a +// D122, D123), each commented where it applies; after that the difference must be empty, and a mutation of a // row, a label or a token must be noticed. test.use({ locale: 'en-US', timezoneId: 'UTC' }); @@ -33,6 +33,13 @@ function measureSystem(mainSelector) { const rect = node.getBoundingClientRect(), base = origin.getBoundingClientRect(); return { left: Math.round(rect.left - base.left), width: Math.round(rect.width) }; }; + // A surface's top: from `.two-col` beside its neighbour, else from the bottom of the surface above it. This pins the + // concept's `.surface + .surface` 18px offset, which concept.css applies inside `.two-col` too (copied, not ruled). + const top = node => { + const rect = node.getBoundingClientRect(), previous = node.previousElementSibling?.getBoundingClientRect(); + if (previous && rect.top >= previous.bottom) return Math.round(rect.top - previous.bottom); + return Math.round(rect.top - node.parentElement.getBoundingClientRect().top); + }; const text = node => node?.textContent.trim() ?? null; const button = node => ({ text: text(node), className: node.className, ...css(node, 'backgroundColor', 'color', 'fontSize', 'paddingTop', 'paddingLeft', 'borderTopWidth', 'borderTopColor', @@ -69,7 +76,7 @@ function measureSystem(mainSelector) { }, columns: { ...css(columns, 'display', 'gridTemplateColumns', 'columnGap', 'rowGap'), box: box(columns), - surfaces: [...columns.children].map(surface => ({ tree: [...surface.children].map(name), box: box(surface), + surfaces: [...columns.children].map(surface => ({ tree: [...surface.children].map(name), box: box(surface), top: top(surface), heading: text(surface.querySelector(':scope > h2')), rows: [...surface.querySelectorAll(':scope > .operation')].map(row => ({ tree: [...row.children].map(name), title: text(row.querySelector('h3')), description: text(row.querySelector('p')), @@ -200,6 +207,8 @@ async function conceptOverview(page, theme) { async function productionOverview(page, theme) { await openProduction(page, 'system/overview', theme); await expect(page.locator('#workspace-main .summary-metrics')).toBeVisible(); + // The thumbnail row is its own read (I2): compare once it answered. + await expect(page.locator('#workspace-main .operation h3').filter({ hasText: 'thumbnails' })).toHaveText('No missing thumbnails'); await page.mouse.move(0, 0); return page.evaluate(measureSystem, '#workspace-main'); } @@ -209,7 +218,7 @@ for (const theme of THEMES) for (const [width, height] of SIZES) { await page.setViewportSize({ width, height }); const overview = await serverJson(request, '/prompt_manager/system/overview'); // The fixture's thumbnails all exist, so the concept's "No missing thumbnails" is also the real title. - expect(overview.thumbnails.missing).toBe(0); + expect((await serverJson(request, '/prompt_manager/images/thumbnails/status')).pending).toBe(0); const concept = await conceptOverview(page, theme); expect(concept.heading.actions).toEqual(['span.chip']); // Paired captures are artifacts only; there are no PNG baselines. @@ -275,21 +284,50 @@ function mapStatisticsConcept(reference, statistics) { }; } -/** D95: the concept's health report titled "Health report", every real check, the checked line, the same actions. */ -function mapHealthConcept(reference, report) { - const chips = { passed: 'Passed', unavailable: 'Not available yet' }; - const rows = report.checks.flatMap(check => check.id === 'missing_files' && check.sources.length > 1 - ? check.sources.map(source => [`${check.label} · ${source.name}`, source.count === 0 ? 'Passed' : 'Not available']) - : [[check.label, chips[check.status]]]); +// The fixture's image sources and each check's real chip on it: nothing is damaged, Needs review does not exist yet (Q6). +const FIXTURE_SOURCES = ['output', 'input']; +const FIXTURE_CHIPS = Object.freeze({ 'Integrity check': 'Passed', 'Foreign keys & pair references': 'Passed', + 'Canonical hashes': 'Passed', 'Pair uniqueness': 'Passed', 'Missing files': 'Passed', 'Ambiguous metadata': 'Not available yet', + Consistency: 'Passed' }); +const chipOf = title => { + if (!(title in FIXTURE_CHIPS)) throw new Error(`No expected chip for ${title}`); + return FIXTURE_CHIPS[title]; +}; + +/** + * D95: the concept's health report from its own five rows, labels verbatim, titled "Health report". Only the D95 rows + * are inserted where they are recorded (Pair uniqueness after Canonical hashes, Consistency last); Missing files has a + * row per image source when there are several; each chip is the fixture's expected result. + */ +function mapHealthConcept(reference, sources = FIXTURE_SOURCES) { + const [first] = reference.operations.rows; + const added = title => ({ ...first, title, chip: chipOf(title) }); + const rows = reference.operations.rows.flatMap(row => { + const mapped = row.title === 'Missing files' && sources.length > 1 + ? sources.map(source => ({ ...row, title: `Missing files · ${source}`, chip: chipOf(row.title) })) + : [{ ...row, chip: chipOf(row.title) }]; + return row.title === 'Canonical hashes' ? [...mapped, added('Pair uniqueness')] : mapped; + }); return { ...reference, title: 'Health report', // No sample note (prototype copy); D95 adds the checked line after the list. body: ['div.operation-list', 'p.small', 'div.dialog-actions'], - operations: { ...reference.operations, rows: rows.map(([title, chip]) => ({ tree: ['h3', 'span.chip'], title, chip })) }, + operations: { ...reference.operations, rows: [...rows, added('Consistency')] }, }; } +// Unmapped, the health report differs exactly where D95 is recorded: the title, the body sequence (no sample note, the +// checked line), the rows after Canonical hashes (Pair uniqueness inserted, Missing files per source) and the chips of +// the concept's sample findings. +const UNMAPPED_HEALTH_PATHS = [ + 'body.0', 'body.1', + ...[3, 4].flatMap(row => [`operations.rows.${row}.chip`, `operations.rows.${row}.title`]), + // The concept's five rows end at row 4; rows 5–7 exist only with the D95 rows and the second source. + ...[5, 6, 7].flatMap(row => ['chip', 'title', 'tree.0', 'tree.1', 'tree.length'].map(key => `operations.rows.${row}.${key}`)), + 'operations.rows.length', 'title', +]; + async function conceptDialogs(page, theme, testInfo) { await openConcept(page, 'system', theme); await page.locator('#main').getByRole('button', { name: 'Explore', exact: true }).click(); @@ -333,23 +371,30 @@ for (const theme of THEMES) test(`statistics and health dialogs match the concep const production = await productionDialogs(page, theme, testInfo); const statistics = await serverJson(request, '/prompt_manager/system/statistics'); const { report } = await serverJson(request, '/prompt_manager/system/health'); + // The fixture the expected chips describe: its two sources, and the concept's checks plus the D95 ones. + const missing = report.checks.find(check => check.id === 'missing_files'); + expect(missing.sources.map(source => source.name)).toEqual(FIXTURE_SOURCES); expect(diffInventory(mapStatisticsConcept(concept.statistics, statistics), production.statistics)).toEqual([]); - expect(diffInventory(mapHealthConcept(concept.health, report), production.health)).toEqual([]); - // D-NEW-S2-5: Maintenance's notice is the concept's own, delivered ahead of the rest of the section. + expect(diffInventory(mapHealthConcept(concept.health), production.health)).toEqual([]); + // D123: Maintenance's notice is the concept's own, delivered ahead of the rest of the section. expect(diffInventory(concept.notice, production.notice)).toEqual([]); // Unmapped, the dialogs differ only where D95 and D96 are recorded. - expect(diffInventory(concept.health, production.health).map(item => item.path.split('.')[0]) - .filter((part, index, all) => all.indexOf(part) === index).sort()).toEqual(['body', 'operations', 'title']); + expect(diffInventory(concept.health, production.health).map(item => item.path).sort()).toEqual(UNMAPPED_HEALTH_PATHS); expect(diffInventory(concept.statistics, production.statistics).map(item => item.path.split('.')[0]) .filter((part, index, all) => all.indexOf(part) === index).sort()).toEqual(['body', 'tables']); // Mutation check: a changed chip token in the health report is noticed, then undone to an exact match again. const undo = await mutate(page, { css: '#modal .operation .chip { background: rgb(255, 0, 0); }' }); - expect(diffInventory(mapHealthConcept(concept.health, report), await page.evaluate(measureDialog)).map(item => item.path)) + expect(diffInventory(mapHealthConcept(concept.health), await page.evaluate(measureDialog)).map(item => item.path)) .toEqual(['operations.chip.backgroundColor']); await undo(); - expect(diffInventory(mapHealthConcept(concept.health, report), await page.evaluate(measureDialog))).toEqual([]); + expect(diffInventory(mapHealthConcept(concept.health), await page.evaluate(measureDialog))).toEqual([]); + // A renamed check (database/health.py's label) is noticed against the concept's own label. + await page.evaluate(() => { document.querySelector('#modal .operation h3').textContent = 'Integrity checks'; }); + expect(diffInventory(mapHealthConcept(concept.health), await page.evaluate(measureDialog)).map(item => item.path)) + .toEqual(['operations.rows.0.title']); + await page.evaluate(() => { document.querySelector('#modal .operation h3').textContent = 'Integrity check'; }); // And a removed "Library totals" table and a changed caption in the statistics dialog. await page.keyboard.press('Escape'); await page.locator('#workspace-main .section-nav').getByRole('button', { name: 'Overview', exact: true }).click(); diff --git a/tests/browser/workspace-system.spec.mjs b/tests/browser/workspace-system.spec.mjs index f1b4b156..75d2069b 100644 --- a/tests/browser/workspace-system.spec.mjs +++ b/tests/browser/workspace-system.spec.mjs @@ -6,6 +6,7 @@ import { seedConcept } from './concept-inventory.mjs'; // Health report read the server, never sample values (D93, D95, D96). The hooks are workspace_system_fixture.py's. const workspace = '/prompt_manager/workspace'; const OVERVIEW_URL = '/prompt_manager/system/overview'; +const THUMBNAILS_URL = '/prompt_manager/images/thumbnails/status'; const number = value => value.toLocaleString('en-US'); test.beforeEach(async ({ page, request, browserName }) => { @@ -49,8 +50,11 @@ test('overview shows real counts and Not checked', async ({ page, request }) => const writes = []; page.on('request', item => { if (!['GET', 'HEAD', 'OPTIONS'].includes(item.method())) writes.push(item.url()); }); const overview = await serverJson(request, OVERVIEW_URL); + const thumbnails = await serverJson(request, THUMBNAILS_URL); // The concept fixture: six positives, their saved negatives and 24 linked images, all with thumbnails. - expect([overview.library.positive_prompts, overview.images.indexed, overview.thumbnails.missing]).toEqual([6, 24, 0]); + expect([overview.library.positive_prompts, overview.images.indexed, thumbnails.pending]).toEqual([6, 24, 0]); + // I2: the Overview never walks the sources for thumbnails; the row reads the status route itself. + expect(overview).not.toHaveProperty('thumbnails'); const layout = await openOverview(page); expect(await layout.locator('.summary-metrics > .metric').evaluateAll(items => items.map(item => [item.querySelector('strong').textContent, item.querySelector('span').textContent]))).toEqual([ @@ -139,6 +143,29 @@ test('thumbnail row reflects the status endpoint', async ({ page, request }) => await expect(page).toHaveURL(`${workspace}#/system/sources`); }); +test('the thumbnail row paints when its own status read answers', async ({ page, request }) => { + const held = async () => (await serverJson(request, '/__test__/hold')).held[THUMBNAILS_URL] ?? 0; + await expect(await request.post('/__test__/hold', { data: { path: THUMBNAILS_URL } })).toBeOK(); + const layout = await openOverview(page); + const row = layout.locator('.operation').filter({ has: page.getByRole('button', { name: 'Review thumbnails', exact: true }) }); + // The other values do not wait for the walk of every source. + await expect(row.locator('h3')).toHaveText('Checking thumbnails…'); + await expect(layout.locator('.summary-metrics > .metric')).toHaveCount(4); + await expect.poll(held).toBe(1); + await expect(await request.post('/__test__/release')).toBeOK(); + await expect(row.locator('h3')).toHaveText('No missing thumbnails'); +}); + +test('a failed thumbnail read is unavailable, never zero', async ({ page }) => { + await page.route(`**${THUMBNAILS_URL}`, route => route.fulfill({ status: 500, contentType: 'application/json', + body: JSON.stringify({ success: false, error: 'Injected failure' }) })); + const layout = await openOverview(page); + const row = layout.locator('.operation').filter({ has: page.getByRole('button', { name: 'Review thumbnails', exact: true }) }); + await expect(row.locator('h3')).toHaveText('Thumbnail status unavailable'); + await expect(layout.getByRole('alert')).toHaveCount(0); + await expect(layout.locator('.metric').nth(2).locator('strong')).toHaveText('24'); +}); + test('ambiguous row reads Not available yet (Q6)', async ({ page }) => { const layout = await openOverview(page); const row = layout.locator('.operation').filter({ hasText: 'Ambiguous image links' }); diff --git a/tests/browser/workspace.spec.mjs b/tests/browser/workspace.spec.mjs index 292e1f0a..f065b4c5 100644 --- a/tests/browser/workspace.spec.mjs +++ b/tests/browser/workspace.spec.mjs @@ -463,7 +463,7 @@ test('System is native: the concept shell, section navigation, sections, hand-of // The section changes in place: focus stays on the pressed section, not the page title. await expect(button).toBeFocused(); // S2 Task 6: Overview is native (workspace-system.spec.mjs); Maintenance shows its health-check notice above - // the hand-off (D-NEW-S2-5); every other section hands off. + // the hand-off (D123); every other section hands off. if (hash === 'overview') await expect(layout.locator(':scope > .summary-metrics')).toBeVisible(); expect(await layout.evaluate(node => [...node.children].map(child => `${child.localName}.${child.className}`))) .toEqual({ overview: ['div.summary-metrics', 'div.two-col', 'div.save-row'], diff --git a/tests/js/workspace-activity-store.test.js b/tests/js/workspace-activity-store.test.js index 5fce2cda..82b69dac 100644 --- a/tests/js/workspace-activity-store.test.js +++ b/tests/js/workspace-activity-store.test.js @@ -182,6 +182,24 @@ test('start returns busy with the running job', async () => { world.dispose(); }); +test('track follows a job another route started, with no read of its own', async () => { + const { createActivityStore } = await source; + const { world, ...deps } = harness(); + const store = createActivityStore(deps); + const seen = []; + store.subscribe(() => seen.push(store.find(ID_A)?.state)); + const job = store.track(summary({ kind: 'health', title: 'Health check' })); + assert.equal(job.id, ID_A); + assert.equal(world.gets, 0, 'the started job is known without a list read'); + assert.deepEqual(world.open().map(stream => stream.url), [`/prompt_manager/jobs/${ID_A}/events`]); + assert.deepEqual(world.toasts, [], 'the route that started it says so itself'); + send(world.open()[0], summary({ kind: 'health', title: 'Health check', state: 'finished', progress: 100 })); + assert.deepEqual(seen, ['running', 'finished']); + assert.equal(world.open().length, 0, 'an ended job is no longer followed'); + assert.throws(() => store.track({ id: 'nope' }), /Invalid job/); + world.dispose(); +}); + test('a list that is not a job list counts as a failed read', async () => { const { createActivityStore } = await source; const { world, api, ...deps } = harness(); diff --git a/tests/js/workspace-system-overview.test.js b/tests/js/workspace-system-overview.test.js index bccea458..bafcd0f9 100644 --- a/tests/js/workspace-system-overview.test.js +++ b/tests/js/workspace-system-overview.test.js @@ -12,7 +12,6 @@ const OVERVIEW = Object.freeze({ backup: null, needs_review: { pending: null }, sources: { configured: 2, effective: 2, auto_detected: false }, - thumbnails: { missing: 0 }, }); test('metrics are the server counts and Not checked before any health check', async () => { @@ -35,7 +34,7 @@ test('metric 4 is the last health result', async () => { }); test('library health rows never invent a review count, a backup or thumbnails', async () => { - const { ambiguousTitle, backupText, thumbnailTitle } = await overviewModule; + const { ambiguousTitle, backupText } = await overviewModule; assert.equal(ambiguousTitle({ pending: null }), 'Ambiguous image links'); assert.equal(ambiguousTitle(undefined), 'Ambiguous image links'); assert.equal(ambiguousTitle({ pending: 1 }), '1 ambiguous image link'); @@ -43,10 +42,28 @@ test('library health rows never invent a review count, a backup or thumbnails', assert.equal(backupText(null), 'No retained backup record.'); assert.equal(backupText({ verified_at: 'not a time' }), 'No retained backup record.'); assert.match(backupText({ verified_at: '2026-10-22T09:30:00Z' }), /^Last verified backup .+\.$/); - assert.equal(thumbnailTitle({ missing: 0 }), 'No missing thumbnails'); - assert.equal(thumbnailTitle({ missing: 1 }), '1 missing thumbnail'); - assert.equal(thumbnailTitle({ missing: 12 }), '12 missing thumbnails'); - assert.equal(thumbnailTitle({ missing: null }), 'Thumbnail status unavailable'); +}); + +test('the thumbnail row is its own read of the thumbnail status', async () => { + const { thumbnailTitle, THUMBNAILS_LOADING } = await overviewModule; + // Until `/images/thumbnails/status` answers the row says it is checking; a failed read is unavailable, never 0. + assert.equal(THUMBNAILS_LOADING, 'Checking thumbnails…'); + assert.equal(thumbnailTitle(undefined), THUMBNAILS_LOADING); + assert.equal(thumbnailTitle({ pending: 0 }), 'No missing thumbnails'); + assert.equal(thumbnailTitle({ pending: 1 }), '1 missing thumbnail'); + assert.equal(thumbnailTitle({ pending: 12 }), '12 missing thumbnails'); + assert.equal(thumbnailTitle(null), 'Thumbnail status unavailable'); + assert.equal(thumbnailTitle({ pending: 'x' }), 'Thumbnail status unavailable'); + // With no image source there is nothing to count. + assert.equal(thumbnailTitle({ pending: 0 }, { effective: 0 }), 'Thumbnail status unavailable'); + assert.equal(thumbnailTitle({ pending: 2 }, { effective: 1 }), '2 missing thumbnails'); +}); + +test('an integrity count SQLite stopped at reads as a lower bound', async () => { + const { checkChip } = await overviewModule; + assert.equal(checkChip({ id: 'integrity', status: 'issues', count: 100, capped: true }), '100+ problems found'); + assert.equal(checkChip({ id: 'integrity', status: 'issues', count: 37, capped: false }), '37 problems found'); + assert.equal(checkChip({ id: 'integrity', status: 'issues', count: 1 }), '1 problem found'); }); test('the sources row tells configured, available and detected sources apart', async () => { @@ -134,19 +151,57 @@ function fakeApi(answers) { return { calls, get: url => answer('GET', url), post: url => answer('POST', url) }; } -const immediate = { setTimeout: callback => { queueMicrotask(callback); return 0; }, clearTimeout() {} }; const JOB = '0123456789abcdef0123456789abcdef'; +const job = (state, id = JOB, kind = 'health') => ({ id, kind, state }); -test('a health check waits for its read job, then reads the report', async () => { +/** + * A fake Activity store holding `jobs`: `track(job)` lists a started job, `load()` applies the next of `reads` (the + * server's job lists) and `push(job)` is a stream update; each notifies as the real store does. It never polls. + */ +function fakeActivity({ jobs = [], reads = [] } = {}) { + const listeners = new Set(); + const emit = () => { for (const listener of [...listeners]) listener(); }; + const store = { + loads: 0, + find: id => jobs.find(item => item.id === id), + running: () => jobs.filter(item => item.state === 'running'), + subscribe(listener) { listeners.add(listener); return () => listeners.delete(listener); }, + listeners: () => listeners.size, + async load() { + store.loads += 1; + if (reads.length) jobs = reads.shift(); + emit(); + }, + track(item) { + if (!item?.id) throw new TypeError('Invalid job'); + store.push(item); + return item; + }, + push(item) { + jobs = [...jobs.filter(other => other.id !== item.id), item]; + emit(); + }, + }; + return store; +} + +test('a health check tracks its job in the Activity store, then reads the report', async () => { const { runHealthCheck } = await overviewModule; const api = fakeApi({ - 'POST /prompt_manager/system/health': [{ job: { id: JOB } }], - [`GET /prompt_manager/jobs/${JOB}`]: [{ job: { state: 'running' } }, { job: { state: 'finished' } }], + 'POST /prompt_manager/system/health': [{ job: job('running') }], 'GET /prompt_manager/system/health': [{ report: REPORT }], }); - assert.equal(await runHealthCheck(api, { timers: immediate }), REPORT); - assert.deepEqual(api.calls, ['POST /prompt_manager/system/health', `GET /prompt_manager/jobs/${JOB}`, - `GET /prompt_manager/jobs/${JOB}`, 'GET /prompt_manager/system/health']); + const activity = fakeActivity(); + const checking = runHealthCheck(api, { activity }); + await new Promise(resolve => setImmediate(resolve)); + // The store follows the started job's event stream; nothing reads or polls the job here. + assert.equal(activity.loads, 0); + assert.equal(activity.find(JOB).state, 'running'); + assert.deepEqual(api.calls, ['POST /prompt_manager/system/health']); + activity.push(job('finished')); + assert.equal(await checking, REPORT); + assert.deepEqual(api.calls, ['POST /prompt_manager/system/health', 'GET /prompt_manager/system/health']); + assert.equal(activity.listeners(), 0); }); test('a check already running elsewhere is waited for, not refused', async () => { @@ -154,35 +209,47 @@ test('a check already running elsewhere is waited for, not refused', async () => const busy = Object.assign(new Error('busy'), { status: 409, code: 'busy' }); const api = fakeApi({ 'POST /prompt_manager/system/health': [busy], - 'GET /prompt_manager/jobs': [{ jobs: [{ id: 'f'.repeat(32), kind: 'scan', state: 'running' }, { id: JOB, kind: 'health', state: 'running' }] }], - [`GET /prompt_manager/jobs/${JOB}`]: [{ job: { state: 'finished' } }], 'GET /prompt_manager/system/health': [{ report: REPORT }], }); - assert.equal(await runHealthCheck(api, { timers: immediate }), REPORT); - assert.ok(api.calls.includes(`GET /prompt_manager/jobs/${JOB}`)); + const activity = fakeActivity({ reads: [[job('running', 'f'.repeat(32), 'scan'), job('running')]] }); + const checking = runHealthCheck(api, { activity }); + await new Promise(resolve => setImmediate(resolve)); + assert.equal(activity.loads, 1); + activity.push(job('finished')); + assert.equal(await checking, REPORT); + // The other check ended between the refusal and the read: its report is the latest. + const ended = fakeApi({ 'POST /prompt_manager/system/health': [busy], 'GET /prompt_manager/system/health': [{ report: REPORT }] }); + assert.equal(await runHealthCheck(ended, { activity: fakeActivity({ reads: [[]] }) }), REPORT); }); test('a failed, lost or reportless check rejects; other errors pass through', async () => { const { runHealthCheck, waitForJob, HEALTH_FAILED_TEXT } = await overviewModule; - const failed = fakeApi({ 'POST /prompt_manager/system/health': [{ job: { id: JOB } }], - [`GET /prompt_manager/jobs/${JOB}`]: [{ job: { state: 'failed' } }] }); - await assert.rejects(runHealthCheck(failed, { timers: immediate }), { message: HEALTH_FAILED_TEXT }); - const lost = fakeApi({ [`GET /prompt_manager/jobs/${JOB}`]: [Object.assign(new Error('gone'), { status: 404 })] }); - assert.equal(await waitForJob(lost, JOB, { timers: immediate }), 'lost'); - const empty = fakeApi({ 'POST /prompt_manager/system/health': [{ job: { id: JOB } }], - [`GET /prompt_manager/jobs/${JOB}`]: [{ job: { state: 'finished' } }], 'GET /prompt_manager/system/health': [{ report: null }] }); - await assert.rejects(runHealthCheck(empty, { timers: immediate }), { message: HEALTH_FAILED_TEXT }); + const failed = fakeApi({ 'POST /prompt_manager/system/health': [{ job: job('failed') }] }); + await assert.rejects(runHealthCheck(failed, { activity: fakeActivity() }), { message: HEALTH_FAILED_TEXT }); + // The store marks a job lost when the server no longer lists it (ComfyUI restarted). + const restarted = fakeActivity({ jobs: [job('running')] }); + const waiting = waitForJob(restarted, JOB); + restarted.push(job('lost')); + assert.equal(await waiting, 'lost'); + const empty = fakeApi({ 'POST /prompt_manager/system/health': [{ job: job('finished') }], + 'GET /prompt_manager/system/health': [{ report: null }] }); + await assert.rejects(runHealthCheck(empty, { activity: fakeActivity() }), { message: HEALTH_FAILED_TEXT }); const broken = fakeApi({ 'POST /prompt_manager/system/health': [Object.assign(new Error('server'), { status: 500 })] }); - await assert.rejects(runHealthCheck(broken, { timers: immediate }), { message: 'server' }); + await assert.rejects(runHealthCheck(broken, { activity: fakeActivity() }), { message: 'server' }); + const invalid = fakeApi({ 'POST /prompt_manager/system/health': [{ job: null }] }); + await assert.rejects(runHealthCheck(invalid, { activity: fakeActivity() }), { message: 'Invalid job' }); }); test('waiting stops when its signal aborts', async () => { const { waitForJob } = await overviewModule; const controller = new AbortController(); - const api = fakeApi({ [`GET /prompt_manager/jobs/${JOB}`]: [{ job: { state: 'running' } }] }); - const never = { setTimeout: () => 1, clearTimeout() {} }; - const waiting = waitForJob(api, JOB, { signal: controller.signal, timers: never }); + const activity = fakeActivity({ jobs: [job('running')] }); + const waiting = waitForJob(activity, JOB, { signal: controller.signal }); await new Promise(resolve => setImmediate(resolve)); controller.abort(new Error('left')); await assert.rejects(waiting, { message: 'left' }); + assert.equal(activity.listeners(), 0); + const gone = new AbortController(); + gone.abort(new Error('already')); + await assert.rejects(waitForJob(fakeActivity(), JOB, { signal: gone.signal }), { message: 'already' }); }); diff --git a/tests/js/workspace-system-state.test.js b/tests/js/workspace-system-state.test.js index 0cc36efb..a9e09118 100644 --- a/tests/js/workspace-system-state.test.js +++ b/tests/js/workspace-system-state.test.js @@ -123,6 +123,8 @@ test('stale revision reports conflict and keeps the draft', async () => { assert.equal(drafts.dirty('host'), true); assert.equal(drafts.get('host', 'result_timeout'), 45); assert.equal(drafts.statusText(), 'Settings changed elsewhere. Reload this section to see them.'); + // The conflict read the host values again (their new revision), keeping the draft. + assert.deepEqual(domains.calls.slice(-2).map(([name]) => name), ['writeHost', 'readHost']); // Reloading the section reads the new revision and keeps the draft (Q10). domains.outcome.host = null; await drafts.load('host'); @@ -131,6 +133,41 @@ test('stale revision reports conflict and keeps the draft', async () => { assert.deepEqual(domains.calls.at(-1)[2], { expected_revision: 'sha256:one' }); }); +test('host save without a read revision never posts', async () => { + const { createSystemDrafts } = await source; + // Never read: an edit made before the host values (and their revision) arrived must not merge blindly. + const unread = adapters({ host: { result_timeout: 30 } }); + const drafts = createSystemDrafts(unread); + drafts.set('host', 'result_timeout', 45); + assert.deepEqual(await drafts.save(), { browser: 'clean', host: 'failed', runtime: 'clean' }); + assert.deepEqual(unread.calls, []); + assert.equal(drafts.dirty('host'), true); + assert.deepEqual([...drafts.failed('host')], ['result_timeout']); + // A read of an unreadable configuration has no revision either. + const unreadable = adapters({ host: { result_timeout: 30 }, revision: null }); + const blind = createSystemDrafts(unreadable); + await blind.load('host'); + blind.set('host', 'result_timeout', 45); + assert.equal((await blind.save()).host, 'failed'); + assert.equal(unreadable.calls.filter(([name]) => name === 'writeHost').length, 0); + // Once read, the same draft saves with the revision. + await drafts.load('host'); + assert.equal((await drafts.save()).host, 'saved'); + assert.deepEqual(unread.calls.at(-1), ['writeHost', { result_timeout: 45 }, { expected_revision: 'sha256:one' }]); +}); + +test('reloadHost reads the host values and says whether it could', async () => { + const { createSystemDrafts } = await source; + const domains = adapters({ host: { result_timeout: 30 } }); + const drafts = createSystemDrafts(domains); + assert.equal(await drafts.reloadHost(), true); + assert.equal(drafts.get('host', 'result_timeout'), 30); + const broken = createSystemDrafts({ ...domains, readHost: async () => { throw new Error('offline'); } }); + assert.equal(await broken.reloadHost(), false); + broken.set('host', 'result_timeout', 45); + assert.equal((await broken.save()).host, 'failed'); +}); + test('no domain called when clean', async () => { const domains = adapters({ browser: { layout: 'grid' } }); const drafts = await loaded(domains); diff --git a/tools/vendor/frontend-coverage-baseline.json b/tools/vendor/frontend-coverage-baseline.json index 3ac526b5..88dfa16e 100644 --- a/tools/vendor/frontend-coverage-baseline.json +++ b/tools/vendor/frontend-coverage-baseline.json @@ -450,22 +450,22 @@ "statements": 98 }, "workspace/system-overview.mjs": { - "lines": 95, - "branches": 91, + "lines": 96, + "branches": 92, "functions": 98, - "statements": 95 + "statements": 96 }, "workspace/system-state.mjs": { "lines": 98, - "branches": 91, + "branches": 92, "functions": 92, "statements": 98 }, "workspace/system.mjs": { - "lines": 91, - "branches": 84, - "functions": 62, - "statements": 91 + "lines": 93, + "branches": 85, + "functions": 69, + "statements": 93 }, "workspace/theme.mjs": { "lines": 98, diff --git a/web/js/workspace/activity-store.mjs b/web/js/workspace/activity-store.mjs index 72eb6aba..0729d449 100644 --- a/web/js/workspace/activity-store.mjs +++ b/web/js/workspace/activity-store.mjs @@ -336,6 +336,16 @@ export function createActivityStore({ api, scope, connect = defaultConnect, noti return { busy: store.running()[0] ?? null, message: error.message }; } }, + /** + * Follow a job another route started (its summary from that route's answer, e.g. the health check's + * `POST /system/health`): it is listed and its stream opened at once, with no read and no start toast. + */ + track(raw) { + const job = parseJob(raw); + if (!job) throw new TypeError('Invalid job'); + upsert(job); + return job; + }, /** Ask a running job to stop at its next file boundary; false once it has ended. */ async cancel(id) { const result = await api.post(`${JOBS_URL}/${encodeURIComponent(id)}/cancel`, {}, { signal: scope.signal }); diff --git a/web/js/workspace/index.mjs b/web/js/workspace/index.mjs index 4648c3d6..d59e61b1 100644 --- a/web/js/workspace/index.mjs +++ b/web/js/workspace/index.mjs @@ -250,7 +250,7 @@ function mountWorkspace() { if (route.id === 'system') { // The concept's System (app.js renderSystem): its heading description, section nav and Save row. description.textContent = route.description; - system = mountSystem(main, api, view, { section: route.section }); + system = mountSystem(main, api, view, { section: route.section, activity }); return; } if (route.id === 'inspect') { diff --git a/web/js/workspace/system-overview.mjs b/web/js/workspace/system-overview.mjs index b58810a9..ac902064 100644 --- a/web/js/workspace/system-overview.mjs +++ b/web/js/workspace/system-overview.mjs @@ -5,11 +5,12 @@ * - The four `.summary-metrics` and the two `.two-col` surfaces ("Library health", "Workspace preferences") come from * one `GET /prompt_manager/system/overview`. D93: metric 4 is the last health result (Not checked / Passed / * N issues / Incomplete); the review, backup, thumbnail and sources rows say what the server reports, and nothing - * reads 0 before the read answers: a load shows a status line, a failure an alert and Retry. + * reads 0 before the read answers: a load shows a status line, a failure an alert and Retry. The thumbnail row is + * its own read of `/images/thumbnails/status` (a walk of every source): "Checking thumbnails…" until it answers. * - D96: the Statistics dialog lists the real runs and average rating per category, then "Library totals". * - D95: the "Health report" lists every check the server ran, Missing files per image source, with a chip per real - * result and when it was checked. The check runs from Maintenance's notice ("Run health checks", D-NEW-S2-5) as a - * read job of the registry; the report opens when it ends. + * result and when it was checked. The check runs from Maintenance's notice ("Run health checks", D123) as a + * read job of the registry, tracked in the Activity store (its event stream); the report opens when it ends. * * Reads run through `latestOnly()` on the section's scope, so a late answer never repaints a newer section. */ @@ -23,11 +24,11 @@ import { operationRow } from './system-controls.mjs'; export const OVERVIEW_URL = '/prompt_manager/system/overview'; export const STATISTICS_URL = '/prompt_manager/system/statistics'; export const HEALTH_URL = '/prompt_manager/system/health'; -const JOBS_URL = '/prompt_manager/jobs'; +export const THUMBNAILS_URL = '/prompt_manager/images/thumbnails/status'; export const HEALTH_KIND = 'health'; -export const JOB_POLL_MS = 150; export const LOADING_TEXT = 'Loading the overview…'; +export const THUMBNAILS_LOADING = 'Checking thumbnails…'; export const LOAD_FAILED_TEXT = 'The overview could not be loaded. Your saved data has not changed.'; export const STATISTICS_FAILED_TEXT = 'Statistics could not be loaded. Your saved data has not changed.'; export const HEALTH_FAILED_TEXT = 'The health check could not finish. Nothing was changed.'; @@ -76,10 +77,14 @@ export function backupText(backup) { return Number.isNaN(when.getTime()) ? 'No retained backup record.' : `Last verified backup ${when.toLocaleString()}.`; } -/** D93: the thumbnail row's title, the `/images/thumbnails/status` plan's pending count. */ -export function thumbnailTitle(thumbnails) { - const missing = thumbnails?.missing; - if (!Number.isSafeInteger(missing)) return 'Thumbnail status unavailable'; +/** + * D93: the thumbnail row's title from the `/images/thumbnails/status` answer: undefined while it is read, null when + * the read failed; with no image source (`sources.effective` 0) there is nothing to count. + */ +export function thumbnailTitle(status, sources) { + if (status === undefined) return THUMBNAILS_LOADING; + const missing = status?.pending; + if (!Number.isSafeInteger(missing) || sources?.effective === 0) return 'Thumbnail status unavailable'; if (missing === 0) return 'No missing thumbnails'; return `${number(missing)} missing ${noun(missing, 'thumbnail')}`; } @@ -112,7 +117,10 @@ export function checkChip(check) { if (check.status === 'failed') return 'Could not check'; if (check.status === 'unavailable') return check.id === 'ambiguous_metadata' ? NOT_AVAILABLE.slice(0, -1) : 'Not available'; const [one, many] = ISSUE_NOUNS[check.id] ?? ['issue', 'issues']; - return Number.isSafeInteger(check.count) ? `${number(check.count)} ${check.count === 1 ? one : many}` : 'Issues found'; + if (!Number.isSafeInteger(check.count)) return 'Issues found'; + // SQLite stops listing integrity problems at its limit, so a capped count is a lower bound ("100+"). + if (check.capped === true) return `${number(check.count)}+ ${many}`; + return `${number(check.count)} ${check.count === 1 ? one : many}`; } function sourceChip(source) { @@ -207,39 +215,47 @@ export function healthNodes(report, { scope, onReviewMissing }) { return [list, el('p', healthSummary(report), 'small'), actions]; } -/** Wait until the job `id` has ended; resolves its final state ('finished', 'failed', 'cancelled' or 'lost'). */ -export async function waitForJob(api, id, { signal, interval = JOB_POLL_MS, timers = globalThis } = {}) { - for (;;) { - let job; - try { - ({ job } = await api.get(`${JOBS_URL}/${encodeURIComponent(id)}`, { signal, cache: 'no-store' })); - } catch (error) { - if (error?.status === 404) return 'lost'; - throw error; +/** + * Wait until the job `id` has ended, following it in the Activity store (`activity`), which streams the job's events + * and marks it lost when the server no longer lists it; nothing here polls. Resolves its final state ('finished', + * 'failed', 'cancelled' or 'lost'). + */ +export function waitForJob(activity, id, { signal } = {}) { + if (signal?.aborted) return Promise.reject(signal.reason); + return new Promise((resolve, reject) => { + const check = () => { + const state = activity.find(id)?.state; + if (state && state !== 'running') finish(resolve, state); + }; + const onAbort = () => finish(reject, signal.reason); + const unsubscribe = activity.subscribe(check); + function finish(settle, value) { + unsubscribe(); + signal?.removeEventListener('abort', onAbort); + settle(value); } - if (job?.state !== 'running') return job?.state ?? 'lost'; - await new Promise((resolve, reject) => { - const timer = timers.setTimeout(resolve, interval); - signal?.addEventListener('abort', () => { timers.clearTimeout(timer); reject(signal.reason); }, { once: true }); - }); - } + signal?.addEventListener('abort', onAbort, { once: true }); + check(); + }); } /** - * Run the health check and resolve its report. A check already running (409 busy, another tab) is waited for instead - * of refused. Rejects when the check fails or ends without a report. + * Run the health check and resolve its report. The started job is tracked in the Activity store; a check already + * running (409 busy, another tab) is found in the store's job list and waited for instead of refused. Rejects when the + * check fails or ends without a report. */ -export async function runHealthCheck(api, { signal, interval, timers } = {}) { +export async function runHealthCheck(api, { activity, signal } = {}) { let id; try { - ({ job: { id } } = await api.post(HEALTH_URL, {}, { signal })); + const { job } = await api.post(HEALTH_URL, {}, { signal }); + id = activity.track(job).id; } catch (error) { if (error?.status !== 409 || error?.code !== 'busy') throw error; - const { jobs } = await api.get(JOBS_URL, { signal, cache: 'no-store' }); - id = jobs.find(job => job.kind === HEALTH_KIND && job.state === 'running')?.id; + await activity.load(); + id = activity.running().find(job => job.kind === HEALTH_KIND)?.id; // The other check ended between the refusal and this read: its report is the latest. } - const state = id ? await waitForJob(api, id, { signal, interval, timers }) : 'finished'; + const state = id ? await waitForJob(activity, id, { signal }) : 'finished'; if (state !== 'finished') throw new Error(HEALTH_FAILED_TEXT); const { report } = await api.get(HEALTH_URL, { signal, cache: 'no-store' }); if (!report) throw new Error(HEALTH_FAILED_TEXT); @@ -255,9 +271,9 @@ export function openHealthReport(report, { scope, trigger = null, navigate = nav /** * The concept's Maintenance notice ("Check first. Review changes. Then apply." + "Run health checks", a689), which - * S2 delivers ahead of the rest of Maintenance (D-NEW-S2-5): the press runs the read-only check, then opens the report. + * S2 delivers ahead of the rest of Maintenance (D123): the press runs the read-only check, then opens the report. */ -export function healthNotice({ api, scope, navigate = navigateTo, notify = showWorkspaceToast }) { +export function healthNotice({ api, activity, scope, navigate = navigateTo, notify = showWorkspaceToast }) { const notice = el('div', undefined, 'notice'); const text = el('div'); text.append(el('h2', 'Check first. Review changes. Then apply.'), @@ -269,7 +285,7 @@ export function healthNotice({ api, scope, navigate = navigateTo, notify = showW run.setAttribute('aria-disabled', 'true'); run.textContent = 'Checking…'; try { - const report = await runHealthCheck(api, { signal: scope.signal }); + const report = await runHealthCheck(api, { activity, signal: scope.signal }); if (scope.signal.aborted) return; openHealthReport(report, { scope, trigger: run, navigate }); } catch (error) { @@ -300,6 +316,7 @@ function surface(heading, rows) { return section; } +/** The Overview's nodes, and its thumbnail row (painted by its own read). */ function overviewNodes(overview, { scope, navigate, openStatistics }) { const go = section => () => navigate(`#/system/${section}`); const review = operationRow({ scope, title: ambiguousTitle(overview.needs_review), @@ -309,13 +326,14 @@ function overviewNodes(overview, { scope, navigate, openStatistics }) { // D93 (Q6): the review queue (Needs review) is not part of the workspace yet. review.querySelector('button').setAttribute('aria-disabled', 'true'); const [sourcesTitle, sourcesDescription] = sourcesCopy(overview.sources); + const thumbnails = operationRow({ scope, title: THUMBNAILS_LOADING, description: 'Generate only missing previews when necessary.', + label: 'Review thumbnails', onClick: go('sources') }); const health = surface('Library health', [ review, operationRow({ scope, title: 'Backup status', description: backupText(overview.backup), label: 'View backups', onClick: go('backups') }), // Until the thumbnail review dialog (S5), Review thumbnails opens Sources, where thumbnail generation lives. - operationRow({ scope, title: thumbnailTitle(overview.thumbnails), description: 'Generate only missing previews when necessary.', - label: 'Review thumbnails', onClick: go('sources') }), + thumbnails, ]); const preferences = surface('Workspace preferences', [ operationRow({ scope, title: 'One display configuration', description: 'Grid and density settings are shared across views.', @@ -326,7 +344,7 @@ function overviewNodes(overview, { scope, navigate, openStatistics }) { ]); const columns = el('div', undefined, 'two-col'); columns.append(health, preferences); - return [metricsNode(overview), columns]; + return { nodes: [metricsNode(overview), columns], thumbnails }; } /** @@ -338,7 +356,15 @@ export function mountOverview(container, { api, scope, navigate = navigateTo, no latestOnly = window.ApiClient.latestOnly, STALE = window.ApiClient.STALE }) { const overviewRead = latestOnly({ lifetime: scope.signal }); const statisticsRead = latestOnly({ lifetime: scope.signal }); + const thumbnailsRead = latestOnly({ lifetime: scope.signal }); let shown = []; + // The thumbnail status (undefined while read, null when it failed), painted into the row whenever both are there. + let thumbnailStatus; + let thumbnailRow = null; + let sources = null; + const paintThumbnails = () => { + if (thumbnailRow) thumbnailRow.querySelector('h3').textContent = thumbnailTitle(thumbnailStatus, sources); + }; function swap(nodes) { const anchor = shown.find(node => node.isConnected); @@ -368,7 +394,11 @@ export function mountOverview(container, { api, scope, navigate = navigateTo, no try { const overview = await overviewRead(signal => api.get(OVERVIEW_URL, { signal, cache: 'no-store' })); if (overview === STALE || scope.signal.aborted) return; - swap(overviewNodes(overview, { scope, navigate, openStatistics })); + const { nodes, thumbnails } = overviewNodes(overview, { scope, navigate, openStatistics }); + thumbnailRow = thumbnails; + sources = overview.sources; + swap(nodes); + paintThumbnails(); } catch (error) { if (scope.signal.aborted) return; const alert = el('p', LOAD_FAILED_TEXT); @@ -377,13 +407,26 @@ export function mountOverview(container, { api, scope, navigate = navigateTo, no scope.listen(retry, 'click', () => { swap(loading()); load(); + loadThumbnails(); }); const panel = el('section', undefined, 'surface'); panel.append(alert, retry); swap([panel]); } } + async function loadThumbnails() { + let status; + try { + status = await thumbnailsRead(signal => api.get(THUMBNAILS_URL, { signal, cache: 'no-store' })); + } catch (error) { + status = null; + } + if (status === STALE || scope.signal.aborted) return; + thumbnailStatus = status ?? null; + paintThumbnails(); + } const first = swap(loading()); load(); + loadThumbnails(); return first; } diff --git a/web/js/workspace/system-state.mjs b/web/js/workspace/system-state.mjs index fceaeda4..2c24e67e 100644 --- a/web/js/workspace/system-state.mjs +++ b/web/js/workspace/system-state.mjs @@ -7,7 +7,11 @@ * * `save()` writes browser, then host, then runtime, and only the domains with changes. A domain that fails keeps its * drafts dirty and its keys `failed()`, so the section highlights them; the others are saved. A host save refused - * because the configuration changed elsewhere (`settings-changed`) is a `conflict` and keeps the draft. Drafts live for + * because the configuration changed elsewhere (`settings-changed`) is a `conflict` and keeps the draft. A host save before + * `load('host')` returned a revision fails without a request: a blind write would merge over a configuration changed + * elsewhere (S1 reads a null `expected_revision` as "merge into what is on disk"). `reloadHost()` reads the host values + * again (System calls it on mount); a host conflict reloads them itself, so the next save carries the current revision. + * Drafts live for * the page session (Q10a): a section left and opened again shows them, and a reload of its values keeps them. * * Adapters: `readBrowser()` and `readRuntime()` return the values; `readHost()` returns `{values, revision}`. @@ -138,6 +142,7 @@ export function createSystemDrafts(adapters) { return result?.persisted === false ? 'tab-only' : 'saved'; } if (domain === 'host') { + if (typeof revision !== 'string') { fail(domain, written); return 'failed'; } const result = await adapters.writeHost(values, { expected_revision: revision }); if (result?.code === CONFLICT_CODE) { fail(domain, written); return 'conflict'; } if (!result?.success || result.persisted !== true) { fail(domain, written); return 'failed'; } @@ -154,12 +159,17 @@ export function createSystemDrafts(adapters) { } } + /** Read the host values again; a failed read leaves host saves refused until one succeeds. */ + const reloadHost = () => load('host').then(() => true, () => false); + /** Save the dirty domains in order; a Save pressed while one runs joins it. */ function save() { saving ??= (async () => { for (const domain of DRAFT_DOMAINS) failedKeys[domain].clear(); const result = {}; for (const domain of DRAFT_DOMAINS) result[domain] = await saveDomain(domain); + // Settings changed elsewhere: read them (and their revision), keeping the drafts (Q10). + if (result.host === 'conflict') await reloadHost(); return result; })().then(result => { saving = null; @@ -172,6 +182,7 @@ export function createSystemDrafts(adapters) { return { load, + reloadHost, get, set, dirty, diff --git a/web/js/workspace/system.mjs b/web/js/workspace/system.mjs index 8459f8b9..0fa76585 100644 --- a/web/js/workspace/system.mjs +++ b/web/js/workspace/system.mjs @@ -24,7 +24,6 @@ const GALLERY = Object.freeze({ href: '/prompt_manager/gallery', label: 'Open ex * settings, thumbnails, duplicates and cache tools. */ export const SECTION_HANDOFFS = Object.freeze({ - overview: [ADMIN], appearance: [GALLERY, ADMIN], sources: [ADMIN, GALLERY], integrations: [ADMIN], @@ -56,7 +55,8 @@ export function systemDrafts(api) { writeRuntime: undelivered, async readHost() { const result = await api.get(SETTINGS_URL, { cache: 'no-store' }); - return { values: result.settings ?? {}, revision: result.revision ?? null }; + // The store reads a missing value as none and a missing revision as unreadable (no host save). + return { values: result.settings, revision: result.revision }; }, writeHost: (changes, { expected_revision }) => api.post(SETTINGS_URL, { ...changes, expected_revision }), }); @@ -87,20 +87,25 @@ function markFailed(layout, store) { /** * The sections delivered so far: Overview (S2 Task 6), and Maintenance's notice with "Run health checks" above the - * hand-off to the maintenance tools that are not native yet (D-NEW-S2-5). + * hand-off to the maintenance tools that are not native yet (D123). */ export const DELIVERED_SECTIONS = Object.freeze({ overview: mountOverview, - maintenance: (container, { api, scope }) => [healthNotice({ api, scope }), ...handoff('maintenance')], + maintenance: (container, { api, activity, scope }) => [healthNotice({ api, activity, scope }), ...handoff('maintenance')], }); /** * Mount System under the heading `renderHeading` wrote in `main`. Returns `{show(section)}`, which shows another * section in place. `scope` is the page's view scope; `sections` maps a section id to a delivered section's mount - * (`(container, {api, scope, drafts}) => nodes`), and every other section hands off. + * (`(container, {api, activity, scope, drafts}) => nodes`), and every other section hands off. `activity` is the page's + * Activity store, which follows the jobs a section starts. + * + * The host values are read on mount (and by the store again after a conflict), so a save carries the current revision; + * until a read returned one, a host save fails without a request (system-state.mjs). */ -export function mountSystem(main, api, scope, { section, sections = DELIVERED_SECTIONS, navigate = navigateTo }) { +export function mountSystem(main, api, scope, { section, activity, sections = DELIVERED_SECTIONS, navigate = navigateTo }) { const store = systemDrafts(api); + store.reloadHost(); const nav = sectionNav({ label: 'System sections', sections: SYSTEM_SECTIONS, active: section, scope, onSelect: id => navigate(`#/system/${id}`) }); const layout = el('div', undefined, 'settings-layout'); @@ -134,7 +139,7 @@ export function mountSystem(main, api, scope, { section, sections = DELIVERED_SE for (const button of nav.children) button.setAttribute('aria-pressed', String(button.dataset.value === next)); while (layout.firstChild !== saveRow) layout.firstChild.remove(); const mount = sections[next]; - const nodes = mount ? mount(layout, { api, scope: sectionScope, drafts: store }) : handoff(next); + const nodes = mount ? mount(layout, { api, activity, scope: sectionScope, drafts: store }) : handoff(next); layout.prepend(...nodes); paintStatus(); }