From f924d7195dfd0b99b032c822a2d394d42b158687 Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Fri, 30 Oct 2026 16:52:48 -0700 Subject: [PATCH] fix(workspace): Overview cards show the sidebar's counts (#549) Closes #547 --- TESTING.md | 7 +++ tests/browser/workspace-images.spec.mjs | 3 +- tests/browser/workspace-library.spec.mjs | 2 +- tests/browser/workspace.spec.mjs | 75 +++++++++++++++++++----- tests/comfyui/workspace.spec.mjs | 4 +- tests/js/workspace-state.test.js | 34 +++++++---- web/js/workspace/index.mjs | 58 +++++++++++------- web/js/workspace/state.mjs | 21 ++++--- 8 files changed, 143 insertions(+), 61 deletions(-) diff --git a/TESTING.md b/TESTING.md index 3e6853ec..fcf6a83b 100644 --- a/TESTING.md +++ b/TESTING.md @@ -1402,6 +1402,13 @@ seeds two seedless files (`seedless_images` on `/__test__/concept/reset`, with Tag) and asserts that every row reserves the tallest footer and that their own footer is shorter than the row it reserves. +### Overview counts (#547) + +Each Overview card is the count of the page it names: the shell's Library, Images and Tags sidebar counts, one source, +so a card and its badge cannot differ. `/prompt_manager/stats` is for the legacy page and counts raw rows. A visit re-reads +the three unless a read is in flight. An unread or failed card shows a dash; Retry re-reads only the failed counts. +Tests: `tests/js/workspace-state.test.js`, `tests/browser/workspace.spec.mjs`, `tests/comfyui/workspace.spec.mjs`. + ### Images page controls and recorded deviations (#249 PR C) The Images page renders the concept's DOM with real data. Its controls, top to bottom: diff --git a/tests/browser/workspace-images.spec.mjs b/tests/browser/workspace-images.spec.mjs index b6cb2630..002b00c2 100644 --- a/tests/browser/workspace-images.spec.mjs +++ b/tests/browser/workspace-images.spec.mjs @@ -602,7 +602,8 @@ async function staleSourceRecovery(page, request, roots) { const pages = [], inspections = [], options = [], mutations = []; page.on('request', request => { const url = new URL(request.url()); - if (url.pathname.endsWith('/images/output')) pages.push(url); + // limit=1 is a sidebar count read (Overview re-reads it on a visit, #547), not a page of images. + if (url.pathname.endsWith('/images/output') && url.searchParams.get('limit') !== '1') pages.push(url); if (url.pathname.includes('/images/inspect/')) inspections.push(url); if (request.method() !== 'GET') mutations.push(url.pathname); }); diff --git a/tests/browser/workspace-library.spec.mjs b/tests/browser/workspace-library.spec.mjs index 146a8cca..703f3568 100644 --- a/tests/browser/workspace-library.spec.mjs +++ b/tests/browser/workspace-library.spec.mjs @@ -176,7 +176,7 @@ test('failed library reads retry while late search responses cannot overwrite ne // D153 (#447): a Browse visit that still names the retired "Highest rated" sort opens the shelf on Recently added. test('a Library visit with the retired rating sort opens on Recently added',async({page})=>{ await page.goto(route);await expect(cards(page)).toHaveCount(3); - const sorts=[];page.on('request',item=>{const url=new URL(item.url());if(url.pathname==='/prompt_manager/library')sorts.push(url.searchParams.get('sort'));}); + const sorts=[];page.on('request',item=>{const url=new URL(item.url());if(url.pathname==='/prompt_manager/library'&&url.searchParams.get('limit')!=='1')sorts.push(url.searchParams.get('sort'));});// limit=1 is a sidebar count read, not the shelf await page.goto('/prompt_manager/workspace#/overview');await expect(page.locator('#workspace-main h1')).toHaveText('Overview'); await page.evaluate(async()=>{const {browseLibrary}=await import('/prompt_manager/js/workspace/library.mjs');browseLibrary({sort:'rating'});}); await expect(cards(page)).toHaveCount(3); diff --git a/tests/browser/workspace.spec.mjs b/tests/browser/workspace.spec.mjs index 9fbab6d4..db06e7ec 100644 --- a/tests/browser/workspace.spec.mjs +++ b/tests/browser/workspace.spec.mjs @@ -60,6 +60,10 @@ test.afterEach(async ({ page, browserName }) => { test('workspace starts with real persisted statistics without mutating data', async ({ page, request }) => { const before = await (await request.get('/prompt_manager/stats')).json(); + const total = async path => (await (await request.get(`/prompt_manager/${path}`)).json()).total; + const counts = [['Prompts', await total('library?limit=1')], + ['Images', await total('images/output?mode=workspace&limit=1')], + ['Tags', await total('vocabulary/tags?limit=1')]]; const mutations = []; page.on('request', request => { if (!['GET', 'HEAD', 'OPTIONS'].includes(request.method())) mutations.push(request.url()); @@ -69,10 +73,11 @@ test('workspace starts with real persisted statistics without mutating data', as await expect(page.getByRole('navigation', { name: 'Workspace', exact: true })).toBeVisible(); const main = page.locator('#workspace-main'); await expect(main.getByRole('heading', { level: 1, name: 'Overview', exact: true })).toBeVisible(); - for (const [label, key] of [['Prompts', 'total_prompts'], ['Images', 'total_images'], ['Tags', 'total_tags']]) { - await expect(statistic(page, label)).toHaveText(String(before.stats[key])); + for (const [label, count] of counts) await expect(statistic(page, label)).toHaveText(String(count)); + // #547: a card and the sidebar badge of the page it names show one number. + for (const [route, count] of [['library', counts[0][1]], ['images', counts[1][1]], ['tags', counts[2][1]]]) { + await expect(page.locator(`aside.sidebar nav [data-route="${route}"] .nav-count`)).toHaveText(String(count)); } - expect(before.stats.total_prompts).toBe(2); expect((await (await request.get('/prompt_manager/stats')).json()).stats).toEqual(before.stats); expect(mutations).toEqual([]); await expect(page.locator('iframe')).toHaveCount(0); @@ -373,25 +378,65 @@ test('unavailable host palette falls back to the system theme and tracks system }); test('overview reports loading, offers retry after failure and renders real empty statistics', async ({ page }) => { + const sources = ['**/prompt_manager/library?limit=1', '**/prompt_manager/images/output?mode=workspace&limit=1', + '**/prompt_manager/vocabulary/tags?limit=1']; let release; const pending = new Promise(resolve => { release = resolve; }); - await page.route('**/prompt_manager/stats', async route => { - await pending; - await route.fulfill({ status: 503, json: { success: false, error: 'Unavailable' } }); - }); + for (const source of sources) { + await page.route(source, async route => { + await pending; + await route.fulfill({ status: 503, json: { success: false, error: 'Unavailable' } }); + }); + } await page.goto(`${workspace}#/overview`); await expect(page.locator('#workspace-main').getByRole('status')).toBeVisible(); release(); await expect(page.locator('#workspace-main').getByRole('alert')).toBeVisible(); - await page.unroute('**/prompt_manager/stats'); - await page.route('**/prompt_manager/stats', route => route.fulfill({ - json: { success: true, stats: { total_prompts: 0, total_images: 0, total_tags: 0 } }, - })); + for (const source of sources) { + await page.unroute(source); + await page.route(source, route => route.fulfill({ json: { success: true, total: 0, items: [], images: [], tags: [] } })); + } await page.getByRole('button', { name: 'Retry', exact: true }).click(); await expect(page.locator('#workspace-main').getByRole('alert')).toHaveCount(0); for (const label of ['Prompts', 'Images', 'Tags']) { await expect(statistic(page, label)).toHaveText('0'); } + await expect(page.locator('#workspace-main .empty-state')).toBeVisible(); +}); + +test('a slow or failed Images read leaves the other Overview cards and retries only itself', async ({ page }) => { + const images = '**/prompt_manager/images/output?mode=workspace&limit=1'; + let release; + const pending = new Promise(resolve => { release = resolve; }); + await page.route(images, async route => { + await pending; + await route.fulfill({ status: 503, json: { success: false, error: 'Unavailable' } }); + }); + await page.goto(`${workspace}#/overview`); + await expect(statistic(page, 'Prompts')).toHaveText('2'); + await expect(statistic(page, 'Images')).toHaveText('–'); + await expect(page.locator('#workspace-main').getByRole('status')).toBeVisible(); + release(); + await expect(page.locator('#workspace-main').getByRole('alert')).toBeVisible(); + await expect(statistic(page, 'Prompts')).toHaveText('2'); + await page.unroute(images); + let libraryReads = 0; + page.on('request', request => { if (new URL(request.url()).pathname === '/prompt_manager/library') libraryReads++; }); + await page.getByRole('button', { name: 'Retry', exact: true }).click(); + await expect(page.locator('#workspace-main').getByRole('alert')).toHaveCount(0); + await expect(statistic(page, 'Images')).toHaveText(/^\d+$/); + expect(libraryReads).toBe(0); +}); + +test('opening Overview reads each count once beside the sidebar', async ({ page }) => { + let reads = 0; + page.on('request', request => { + const url = new URL(request.url()); + if (url.pathname === '/prompt_manager/images/output' && url.searchParams.get('favorites') === null) reads++; + }); + await page.goto(`${workspace}#/overview`); + await expect(statistic(page, 'Images')).toHaveText(/^\d+$/); + expect(reads).toBe(1); }); test('small screens keep navigation and the theme accessible without horizontal page overflow', async ({ page }) => { @@ -410,11 +455,9 @@ test('leaving a loading overview prevents its delayed result from replacing the let completed; const pending = new Promise(resolve => { release = resolve; }); const finished = new Promise(resolve => { completed = resolve; }); - await page.route('**/prompt_manager/stats', async route => { + await page.route('**/prompt_manager/library?limit=1', async route => { await pending; - await route.fulfill({ json: { - success: true, stats: { total_prompts: 999, total_images: 888, total_tags: 777 }, - } }).catch(() => {}); + await route.fulfill({ json: { success: true, total: 999, items: [] } }).catch(() => {}); completed(); }); await page.goto(`${workspace}#/overview`); @@ -437,7 +480,7 @@ test('leaving a loading overview prevents its delayed result from replacing the test('cached-page restoration remounts one working shell and refreshes its data', async ({ page }) => { let reads = 0; page.on('request', request => { - if (new URL(request.url()).pathname === '/prompt_manager/stats') reads++; + if (new URL(request.url()).pathname === '/prompt_manager/vocabulary/tags') reads++; }); await page.goto(`${workspace}#/overview`); await expect(statistic(page, 'Prompts')).toHaveText('2'); diff --git a/tests/comfyui/workspace.spec.mjs b/tests/comfyui/workspace.spec.mjs index 00d2c8d4..bdf44f34 100644 --- a/tests/comfyui/workspace.spec.mjs +++ b/tests/comfyui/workspace.spec.mjs @@ -8,7 +8,7 @@ test('workspace serves real library data beside either ComfyUI renderer', async text: `Workspace host smoke ${testInfo.project.name}`, tags: ['workspace-host'], } }); expect(saved.ok()).toBe(true); - const stats = (await (await request.get('/prompt_manager/stats')).json()).stats; + const library = await (await request.get('/prompt_manager/library?limit=1')).json(); const workspace = await context.newPage(); const errors = []; workspace.on('pageerror', error => errors.push(error.message)); @@ -16,7 +16,7 @@ test('workspace serves real library data beside either ComfyUI renderer', async await workspace.goto('/prompt_manager/workspace#/overview'); await expect(workspace.locator('#workspace-main h1')).toHaveText('Overview'); await expect(workspace.locator('dt').filter({ hasText: /^Prompts$/ }).locator('+ dd')) - .toHaveText(stats.total_prompts.toLocaleString()); + .toHaveText(library.total.toLocaleString()); await workspace.getByRole('combobox', { name: 'Theme' }).selectOption('light'); await expect(workspace.locator('body')).toHaveAttribute('data-ca-theme', 'light'); const imageResponse = workspace.waitForResponse(response => { diff --git a/tests/js/workspace-state.test.js b/tests/js/workspace-state.test.js index 6b818430..c4711f12 100644 --- a/tests/js/workspace-state.test.js +++ b/tests/js/workspace-state.test.js @@ -123,17 +123,29 @@ test('a job of the Activity page is its own deep link', async () => { dispose(); }); -test('statistics preserve zero and reject incomplete or invalid counts', async () => { - const { summarizeStats } = await source; - assert.deepEqual(summarizeStats({total_prompts:0, total_images:12, total_tags:3}), [ - ['Prompts', 0], ['Images', 12], ['Tags', 3], - ]); - for (const stats of [undefined, null, {}, {total_prompts:0, total_images:-1, total_tags:0}, - {total_prompts:'2', total_images:0, total_tags:0}, - {total_prompts:Infinity, total_images:0, total_tags:0}, - {total_prompts:0, total_images:1.5, total_tags:0}]) { - assert.throws(() => summarizeStats(stats), /statistics/i); - } +test('Overview cards show the count of the page they name', async () => { + const { summarizeOverviewCounts, OVERVIEW_COUNT_ROUTES } = await source; + assert.deepEqual(OVERVIEW_COUNT_ROUTES, ['library', 'images', 'tags']); + const counts = new Map([['library', 64], ['images', 122], ['tags', 306], ['favorites', 8]]); + assert.deepEqual(summarizeOverviewCounts(counts), { + cards: [['Prompts', 64], ['Images', 122], ['Tags', 306]], pending: false, failed: false, empty: false, + }); +}); + +test('Overview tells loading, failed and really empty counts apart', async () => { + const { summarizeOverviewCounts } = await source; + const nothing = summarizeOverviewCounts(new Map()); + assert.deepEqual([nothing.pending, nothing.failed, nothing.empty], [true, false, false]); + assert.deepEqual(nothing.cards, [['Prompts', null], ['Images', null], ['Tags', null]]); + // A slow Images read does not hold back the cards already read. + const partial = summarizeOverviewCounts(new Map([['library', 0], ['tags', 0]])); + assert.deepEqual(partial.cards, [['Prompts', 0], ['Images', null], ['Tags', 0]]); + assert.deepEqual([partial.pending, partial.failed, partial.empty], [true, false, false]); + const failed = summarizeOverviewCounts(new Map([['library', 2], ['images', null], ['tags', 3]])); + assert.deepEqual([failed.pending, failed.failed, failed.empty], [false, true, false]); + assert.deepEqual(failed.cards[1], ['Images', null]); + const empty = summarizeOverviewCounts(new Map([['library', 0], ['images', 0], ['tags', 0]])); + assert.deepEqual([empty.pending, empty.failed, empty.empty], [false, false, true]); }); test('router normalizes unknown routes, avoids duplicate renders, and disposes its listener', async () => { diff --git a/web/js/workspace/index.mjs b/web/js/workspace/index.mjs index 43ca8dcc..3022285d 100644 --- a/web/js/workspace/index.mjs +++ b/web/js/workspace/index.mjs @@ -1,6 +1,6 @@ import '../api-client.js'; import { createButton, createScope } from '../shared/toolkit/core.js'; -import { routes, mountRouter, summarizeStats, routeTitle, loadNavCounts, navigateTo, COLLECTION_ROUTE, +import { routes, mountRouter, summarizeOverviewCounts, OVERVIEW_COUNT_ROUTES, routeTitle, loadNavCounts, navigateTo, COLLECTION_ROUTE, LIBRARY_WRITE_COUNTS, NAV_COUNT_ROUTES, FAVORITE_WRITE_COUNTS, FAVORITE_COUNT_DELAY_MS, REVIEW_WRITE_COUNTS, JOB_END_COUNTS } from './state.mjs'; import { mountWorkspaceTheme } from './theme.mjs'; @@ -153,9 +153,13 @@ function mountWorkspace() { countRequests.get(id)?.abort(); const request = new AbortController(); countRequests.set(id, request); + // A failed count reads as loading again while it is retried; a known one stays until replaced. + if (knownCounts.get(id) === null) { knownCounts.delete(id); notifyCounts(); } // Each badge lands as its own read settles; a count that failed to refresh is removed, never left stale. loadNavCounts(api, request.signal, (_, count) => { - if (!request.signal.aborted) showCount(id, count); + if (request.signal.aborted) return; + countRequests.delete(id); + showCount(id, count); }, [id]); } } @@ -163,7 +167,15 @@ function mountWorkspace() { const link = sidebar.querySelector(`nav [data-route="${id}"]`); link?.querySelector('.nav-count')?.remove(); if (link && Number.isSafeInteger(count)) link.append(element('span', count.toLocaleString(), 'nav-count')); + knownCounts.set(id, count); + notifyCounts(); } + /** #547: Overview shows these same counts, so a card and its badge cannot differ, and no read runs twice. */ + const knownCounts = new Map(); + const countListeners = new Set(); + function notifyCounts() { for (const listener of countListeners) listener(); } + /** Re-read the given counts unless a read is already in flight. */ + function freshenCounts(ids) { refreshCounts(ids.filter(id => !countRequests.has(id))); } /** A count a page has just read itself: it replaces the badge, and an older read in flight no longer lands. */ function setCount(id, count) { countRequests.get(id)?.abort(); @@ -499,34 +511,38 @@ function mountWorkspace() { } tools.append(cards); main.append(tools); - async function loadStats() { - const status = element('p', 'Loading library statistics…'); - status.setAttribute('role', 'status'); - stats.replaceChildren(status); - try { - const result = await api.get('/prompt_manager/stats', { signal: view.signal }); - if (view.signal.aborted) return; - const values = summarizeStats(result.stats); + function showStats() { + const { cards, pending, failed, empty } = summarizeOverviewCounts(knownCounts); + const parts = []; + if (cards.some(([, count]) => count !== null)) { const list = element('dl', undefined, 'stat-grid'); - for (const [label, count] of values) { + for (const [label, count] of cards) { const stat = element('div', undefined, 'stat'); - stat.append(element('dt', label), element('dd', count.toLocaleString())); + stat.append(element('dt', label), element('dd', count === null ? '–' : count.toLocaleString())); list.append(stat); } - stats.replaceChildren(list); - if (values.every(([, count]) => count === 0)) { - stats.append(element('p', 'Your library is empty. Add a prompt in the prompt library or configure sources in System.', 'empty-state')); - } - } catch (error) { - if (view.signal.aborted) return; + parts.push(list); + } + if (pending) { + const status = element('p', 'Loading library statistics…'); + status.setAttribute('role', 'status'); + parts.push(status); + } else if (failed) { const alert = element('p', 'Library statistics could not be loaded. Your saved data has not changed.'); alert.setAttribute('role', 'alert'); const retry = createButton('Retry'); - view.listen(retry, 'click', loadStats); - stats.replaceChildren(alert, retry); + view.listen(retry, 'click', () => refreshCounts(OVERVIEW_COUNT_ROUTES.filter(id => knownCounts.get(id) === null))); + parts.push(alert, retry); + } else if (empty) { + parts.push(element('p', 'Your library is empty. Add a prompt in the prompt library or configure sources in System.', 'empty-state')); } + stats.replaceChildren(...parts); } - loadStats(); + countListeners.add(showStats); + view.cleanup(() => countListeners.delete(showStats)); + showStats(); + // A visit reads them again, as a generation since the last read changes the sidebar's badges too. + freshenCounts(OVERVIEW_COUNT_ROUTES); } scope.cleanup(mountRouter(window, render)); scope.listen(window, 'pagehide', () => scope.dispose()); diff --git a/web/js/workspace/state.mjs b/web/js/workspace/state.mjs index e18e0514..7fe28a6d 100644 --- a/web/js/workspace/state.mjs +++ b/web/js/workspace/state.mjs @@ -95,15 +95,6 @@ export function resolveRoute(hash) { return routes.find(route => hash === `#/${route.id}`) || routes[0]; } -export function summarizeStats(stats) { - return [['Prompts', 'total_prompts'], ['Images', 'total_images'], ['Tags', 'total_tags']] - .map(([label, key]) => { - const count = stats?.[key]; - if (!Number.isSafeInteger(count) || count < 0) throw new Error('Invalid library statistics'); - return [label, count]; - }); -} - // The sidebar's real counts (D27): one bounded `limit=1` read each, whose `total` is the count. Favorites (D51) is the // Favorites page's own listing, so its count is what that page lists. const NAV_COUNT_SOURCES = Object.freeze([ @@ -158,6 +149,18 @@ export async function loadNavCounts(api, signal, onCount = () => {}, ids = NAV_C return new Map([...summarizeNavCounts(pages)].filter(([id]) => ids.includes(id))); } +// #547: each Overview card is the count of the page it names, the same number as that page's sidebar badge. +const OVERVIEW_CARDS = Object.freeze([['Prompts', 'library'], ['Images', 'images'], ['Tags', 'tags']]); +export const OVERVIEW_COUNT_ROUTES = Object.freeze(OVERVIEW_CARDS.map(([, id]) => id)); + +/** Overview's state from the shell's counts: no entry is a read not settled yet, null a read that failed. */ +export function summarizeOverviewCounts(counts) { + const cards = OVERVIEW_CARDS.map(([label, id]) => [label, counts.get(id) ?? null]); + const pending = OVERVIEW_COUNT_ROUTES.some(id => !counts.has(id)); + const failed = OVERVIEW_COUNT_ROUTES.some(id => counts.get(id) === null); + return { cards, pending, failed, empty: !pending && cards.every(([, count]) => count === 0) }; +} + /** Fired on the window to render the current route again, where setting the same hash fires no `hashchange`. */ const ROUTE_REFRESH_EVENT = 'prompt-manager:route-refresh';