From 454ac12e1dea33a3c14a3d78c9234239dc6d021d Mon Sep 17 00:00:00 2001 From: Vito Date: Wed, 21 Oct 2026 12:36:38 -0700 Subject: [PATCH 1/2] fix(images): list sources and folders that load after Refine opened (#272) * fix(images): list sources and folders that load after Refine opened "Refine this view" copied the discovered sources and folders when it opened, so opening Filters before /gallery/subfolders answered left Source and Folder at "All sources" / "All output folders" until the dialog was reopened. On a slow CI runner this stalled the touch "collection viewer follows filtered loaded order beyond mounted cards" workflow in selectOption('0') until the 30s timeout. openRefineDialog now returns setOptions(key, options); the Images page calls it when discovery settles, and the open dialog refills Source and its Folder list, showing the value it opened with (or the one chosen since) once offered. It does nothing after the dialog closed or was replaced. * test(library): read filtered shelf titles after the cards' frame The Library concept comparison read card titles as soon as the caption changed and the card count dropped below six. Production empties the shelf when a filter applies, writes the new caption when the page answers and places the cards in the next animation frame, so on a slow CI runner the empty shelf satisfied "fewer than six" and the titles read as []. Settle one frame pair before reading, as the Images and Inspector comparisons already do. --- tests/browser/library-concept.spec.mjs | 4 ++ tests/browser/workspace-images.spec.mjs | 24 ++++++++++++ tests/js/workspace-toolbar.test.js | 50 +++++++++++++++++++++++++ web/js/workspace/images.mjs | 10 ++++- web/js/workspace/workspace-dialogs.mjs | 25 +++++++++++-- 5 files changed, 107 insertions(+), 6 deletions(-) diff --git a/tests/browser/library-concept.spec.mjs b/tests/browser/library-concept.spec.mjs index 1589ad2..afc73ae 100644 --- a/tests/browser/library-concept.spec.mjs +++ b/tests/browser/library-concept.spec.mjs @@ -158,6 +158,10 @@ async function filteredOf(page,apply,undo) { await apply(); await expect.poll(()=>captionOf(page)).not.toBe(before); await expect.poll(()=>page.locator(CARDS).count()).toBeLessThan(CONCEPT_PROMPTS); + // Production empties the shelf when the filter applies, writes the answer's caption at once and places its cards + // in the next animation frame (one paint for a viewer), so the empty shelf already counts as "fewer": read the + // titles only after that frame, as the Images and Inspector comparisons do. + await settle(page); const result={caption:await captionOf(page),titles:await titlesOf(page)}; await undo(); await expect.poll(()=>captionOf(page)).toBe(before); diff --git a/tests/browser/workspace-images.spec.mjs b/tests/browser/workspace-images.spec.mjs index 5a97715..2398862 100644 --- a/tests/browser/workspace-images.spec.mjs +++ b/tests/browser/workspace-images.spec.mjs @@ -1487,6 +1487,30 @@ test('a star press leaves the continuation status, count and caption date untouc expect(await page.evaluate(() => window.__statusMutations)).toEqual([]); }); +// Opened before the source and folder discovery answered (a slow server), Refine lists them once it does. +test('Refine opened before discovery settles lists the discovered sources and folders', async ({ page }) => { + let release, started; + const held = new Promise(resolve => { release = resolve; }), hit = new Promise(resolve => { started = resolve; }); + await page.route('**/prompt_manager/gallery/subfolders?*', async route => { started(); await held; await route.continue(); }, { times: 1 }); + await page.goto(workspace); + await expect(cards(page)).toHaveCount(2); + await hit; + await filtersButton(page).click(); + const source = refineDialog(page).getByLabel('Source', { exact: true }); + const folder = refineDialog(page).getByLabel('Folder', { exact: true }); + const values = select => select.locator('option').evaluateAll(options => options.map(option => option.value)); + expect(await values(source)).toEqual(['all']); + expect(await values(folder)).toEqual(['']); + release(); + await expect.poll(() => values(source)).toEqual(['all', '0', '1']); + expect(await values(folder)).toEqual(['', 'shared']); + await expect(source).toHaveValue('all'); + await source.selectOption('1'); + await refineDialog(page).getByRole('button', { name: 'Apply filters', exact: true }).click(); + await expect(cards(page)).toHaveCount(1); + await expect(cards(page)).toHaveAttribute('data-image-key', '1:shared/same.png'); +}); + // #249 final review m1: as in the old field row, a Source offers only its own folders. test('Refine offers only the chosen source folders', async ({ page, request }) => { await expect(await request.post('/__test__/add-nested-image')).toBeOK(); diff --git a/tests/js/workspace-toolbar.test.js b/tests/js/workspace-toolbar.test.js index bf01384..789569d 100644 --- a/tests/js/workspace-toolbar.test.js +++ b/tests/js/workspace-toolbar.test.js @@ -342,6 +342,56 @@ test('refine dialog round-trips values and clear resets them', async () => { } }); +test('Refine lists options discovered after it opened and keeps the wanted values', async () => { + const { openRefineDialog, openImportDialog, closeWorkspaceDialog } = await dialogsModule; + const dom = fakeDOM(); + const scope = (await coreModule).createScope(); + try { + let bySource = {}; + const optionsFor = source => [['', 'All output folders'], ...(bySource[source] ?? []).map(path => [path, path])]; + const fields = [{ key: 'source', options: [['all', 'All sources']] }, { key: 'folder', options: optionsFor('all'), optionsFor }, + REFINE_FIELDS[2]]; + const applied = []; + const refine = openRefineDialog({ scope, fields, values: { source: '1', folder: 'temp-only', media: 'video' }, + onApply: value => applied.push(value), onClear() {} }); + const [source, folder, media] = byId(dom, 'modal-body').children.filter(child => child.className === 'field') + .map(field => field.children[1]); + // Opened before discovery: only the cleared values. + assert.deepEqual(optionPairs(source), [['all', 'All sources']]); + assert.equal(source.value, 'all'); + assert.equal(folder.value, ''); + bySource = { all: ['portraits', 'temp-only'], 0: ['portraits'], 1: ['temp-only'] }; + refine.setOptions('source', [['all', 'All sources'], ['0', 'output'], ['1', 'archive']]); + // The values it opened with, now offered, and the chosen Source's folders. + assert.deepEqual(optionPairs(source).map(([value]) => value), ['all', '0', '1']); + assert.equal(source.value, '1'); + assert.deepEqual(optionPairs(folder), [['', 'All output folders'], ['temp-only', 'temp-only']]); + assert.equal(folder.value, 'temp-only'); + // A choice made since wins over the opening value; tags and unknown keys are ignored. + source.value = '0'; + source.dispatchEvent(new Event('change')); + refine.setOptions('source', [['all', 'All sources'], ['0', 'output'], ['1', 'archive']]); + refine.setOptions('tags', [['x', 'x']]); + assert.equal(source.value, '0'); + assert.deepEqual(optionPairs(folder).map(([value]) => value), ['', 'portraits']); + assert.equal(media.value, 'video'); + button(byId(dom, 'modal-body'), 'Apply filters').click(); + assert.deepEqual(applied, [{ source: '0', folder: '', media: 'video' }]); + // Closed: a late discovery leaves the shell alone. + refine.setOptions('source', [['all', 'All sources']]); + assert.equal(source.options.length, 3); + // Replaced by another dialog: the same. + const again = openRefineDialog({ scope, fields, values: {}, onApply() {}, onClear() {} }); + openImportDialog({ scope, onInspect() {}, onRescan() {} }); + again.setOptions('source', [['all', 'All sources'], ['9', 'late']]); + assert.equal(byId(dom, 'modal-title').textContent, 'Import & inspect'); + } finally { + closeWorkspaceDialog(); + scope.dispose(); + dom.restore(); + } +}); + test('the Folder list follows the chosen Source', async () => { const { openRefineDialog, closeWorkspaceDialog } = await dialogsModule; const dom = fakeDOM(); diff --git a/web/js/workspace/images.mjs b/web/js/workspace/images.mjs index e3ab44b..ddc1b09 100644 --- a/web/js/workspace/images.mjs +++ b/web/js/workspace/images.mjs @@ -133,6 +133,8 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage 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 = []; + // The open "Refine this view", so a discovery that settles after it opened still lists its sources and folders. + 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, @@ -643,8 +645,8 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage /** "Refine this view": Source, Folder, Media and Tags, the filters the old field row held (D14). */ function openFilters(trigger) { if (recovering) return; - openRefineDialog({scope, trigger, fields:[ - {key:'source', options:[['all', 'All sources'], ...roots.map(root => [String(root.index), root.label])]}, + refine = openRefineDialog({scope, trigger, fields:[ + {key:'source', options:sourceOptions()}, {key:'folder', options:folderOptions('all'), optionsFor:folderOptions}, {key:'media', options:MEDIA_OPTIONS}, {key:'tags'}, @@ -654,6 +656,9 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage applyRefinement(cleared); }}); } + function sourceOptions() { + return [['all', 'All sources'], ...roots.map(root => [String(root.index), root.label])]; + } /** The chosen Source's folders, as the old field row reloaded them; every folder under All sources. */ function folderOptions(source) { const root = source === 'all' ? null : roots.find(item => String(item.index) === source); @@ -750,6 +755,7 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage roots = data.roots.filter(root => Number.isSafeInteger(root?.index) && typeof root.label === 'string') .map(root => ({index:root.index, label:root.label, subfolders:paths(root.subfolders)})); folders = paths(data.subfolders); + refine?.setOptions('source', sourceOptions()); updateScope(); sourcesRetry = null; return true; diff --git a/web/js/workspace/workspace-dialogs.mjs b/web/js/workspace/workspace-dialogs.mjs index 78179d8..1364ec9 100644 --- a/web/js/workspace/workspace-dialogs.mjs +++ b/web/js/workspace/workspace-dialogs.mjs @@ -152,6 +152,9 @@ function choose(select, value) { * Tags; Folder may add `optionsFor(source)`, the options of one Source, so its list follows the chosen Source and a * folder that Source lacks returns to the first option. `values` holds the current value per key. Apply reports the edited values; Clear resets every field and * reports the cleared values plus `category: ''`, so the page clears its toolbar category too. Both close first. + * Returns `{setOptions(key, options)}` for options discovered after the dialog opened (a slow folder discovery): the + * select is refilled and shows the value the field opened with, or the one chosen since, once it is offered; a Folder + * that follows its Source is refilled from `optionsFor` again. It does nothing once this dialog closed or was replaced. */ export function openRefineDialog({ scope, fields, values = {}, onApply, onClear, trigger = null }) { requireScope(scope); @@ -172,16 +175,21 @@ export function openRefineDialog({ scope, fields, values = {}, onApply, onClear, choose(select, values[key]); return [{ key, control: select, cleared: item.options[0][0], optionsFor: item.optionsFor }]; }); + // Each select's wanted value: the one it opened with until the person picks another (see setOptions). + const wanted = new Map(controls.map(({ key }) => [key, values[key]])); + for (const { key, control } of controls) { + if (key !== 'tags') control.addEventListener('change', () => wanted.set(key, control.value)); + } const source = controls.find(item => item.key === 'source'); const folder = controls.find(item => item.key === 'folder' && typeof item.optionsFor === 'function'); - const followSource = () => { - const chosen = folder.control.value; + const followSource = (chosen = folder.control.value) => { fillSelect(folder.control, 'folder', folder.optionsFor(source.control.value)); choose(folder.control, chosen); }; if (source && folder) { - followSource(); - source.control.addEventListener('change', followSource); + followSource(values.folder); + // A Source that lacks the chosen folder clears it, and that cleared folder is the wanted one from then on. + source.control.addEventListener('change', () => { followSource(); wanted.set('folder', folder.control.value); }); } const read = () => Object.fromEntries(controls.map(({ key, control }) => [key, key === 'tags' ? control.value.trim() : control.value])); @@ -207,6 +215,15 @@ export function openRefineDialog({ scope, fields, values = {}, onApply, onClear, ...controls.map(({ key, control }) => field(FIELD_LABELS[key], control, `filter-${key}`)), actions, ], { scope, trigger }); + const opened = session; + function setOptions(key, options) { + const item = controls.find(candidate => candidate.key === key); + if (session !== opened || !item || key === 'tags') return; + fillSelect(item.control, key, options); + choose(item.control, wanted.get(key)); + if (source && folder && (item === source || item === folder)) followSource(wanted.get('folder')); + } + return { setOptions }; } /** One operation row; a press on its busy (aria-disabled) action is ignored and keeps the dialog open. */ From 6e9d8f81fd5facffd3ef7c09c3066c26403b38d7 Mon Sep 17 00:00:00 2001 From: Vito Date: Wed, 21 Oct 2026 13:08:38 -0700 Subject: [PATCH 2/2] ci(coverage): fast check that every frontend file has a coverage baseline (#271) The frontend coverage job only reports a missing baseline entry after its full browser run. coverage:baseline-check compares the files the gate measures (same discoverSources) with frontend-coverage-baseline.json in milliseconds and also flags entries for files that no longer exist. npm test runs the same check. --- TESTING.md | 7 ++++ tests/vendor/coverage-baseline-check.test.mjs | 33 ++++++++++++++++ tools/vendor/coverage-baseline-check.mjs | 38 +++++++++++++++++++ tools/vendor/package.json | 1 + 4 files changed, 79 insertions(+) create mode 100644 tests/vendor/coverage-baseline-check.test.mjs create mode 100644 tools/vendor/coverage-baseline-check.mjs diff --git a/TESTING.md b/TESTING.md index 98a075a..147eb95 100644 --- a/TESTING.md +++ b/TESTING.md @@ -109,6 +109,13 @@ the baseline updater. Raise floors with `npm run --prefix tools/vendor coverage:update`; it reruns the suites and cannot lower an existing floor. Review baseline changes like code changes. +`npm run --prefix tools/vendor coverage:baseline-check` runs in well under a +second without a browser and fails, naming the files, when a measured +`web/js` file has no baseline entry or the baseline lists a file that no longer +exists. It uses the gate's own `discoverSources`, and `npm test` runs the same +check, so run it before pushing a new or deleted frontend file instead of +waiting for the `frontend coverage` job. + The `viewerjs` check runs on every PR, including backend-only changes. It is an aggregate job that passes only when the `frontend coverage (chromium and touch)` job and both `page workflows (firefox)` and `page workflows (webkit)` jobs diff --git a/tests/vendor/coverage-baseline-check.test.mjs b/tests/vendor/coverage-baseline-check.test.mjs new file mode 100644 index 0000000..187d174 --- /dev/null +++ b/tests/vendor/coverage-baseline-check.test.mjs @@ -0,0 +1,33 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import { baselineGaps, measuredSources, readBaseline } from '../../tools/vendor/coverage-baseline-check.mjs'; + +const floor = { lines: 80, branches: 80, functions: 80, statements: 80 }; + +test('baseline gaps name measured files without a baseline entry', () => { + const gaps = baselineGaps(['a.js', 'nested/new.mjs'], { 'a.js': floor }); + assert.deepEqual(gaps.missing, ['nested/new.mjs']); + assert.deepEqual(gaps.stale, []); +}); + +test('baseline gaps name baseline entries whose file no longer exists', () => { + const gaps = baselineGaps(['a.js'], { 'a.js': floor, 'gone.js': floor }); + assert.deepEqual(gaps.missing, []); + assert.deepEqual(gaps.stale, ['gone.js']); +}); + +test('baseline gaps are empty when files and baseline agree', () => { + assert.deepEqual(baselineGaps(['a.js'], { 'a.js': floor }), { missing: [], stale: [] }); +}); + +test('the committed baseline covers exactly the measured frontend sources', () => { + const gaps = baselineGaps(measuredSources(), readBaseline()); + assert.deepEqual(gaps.missing, [], `Missing baseline (run coverage:update): ${gaps.missing.join(', ')}`); + assert.deepEqual(gaps.stale, [], `Stale baseline entries (file removed): ${gaps.stale.join(', ')}`); +}); + +test('the real gate and this check select files through the same function', () => { + const gate = readFileSync(new URL('../../tools/vendor/frontend-coverage.mjs', import.meta.url), 'utf8'); + assert.match(gate, /discoverSources\(sourceRoot\)/); +}); diff --git a/tools/vendor/coverage-baseline-check.mjs b/tools/vendor/coverage-baseline-check.mjs new file mode 100644 index 0000000..795ccaf --- /dev/null +++ b/tools/vendor/coverage-baseline-check.mjs @@ -0,0 +1,38 @@ +// Fast, browser-free check that frontend-coverage-baseline.json lists exactly the +// files the coverage gate measures. CI's `frontend coverage` job reports a missing +// entry only after ~20 minutes; this reports it in milliseconds. +import { readFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath, pathToFileURL } from 'node:url'; +import { repositoryRoot } from './vendor-viewerjs.mjs'; +import { discoverSources } from './frontend-coverage.mjs'; + +const baselinePath = join(dirname(fileURLToPath(import.meta.url)), 'frontend-coverage-baseline.json'); + +export function measuredSources() { + return discoverSources(join(repositoryRoot, 'web/js')); +} + +export function readBaseline() { + return JSON.parse(readFileSync(baselinePath, 'utf8')); +} + +export function baselineGaps(files, baseline) { + const measured = new Set(files); + return { + missing: files.filter(file => !baseline[file]), + stale: Object.keys(baseline).filter(file => !measured.has(file)).sort(), + }; +} + +function main() { + const { missing, stale } = baselineGaps(measuredSources(), readBaseline()); + if (!missing.length && !stale.length) { + console.log('Coverage baseline matches the measured frontend files.'); + return; + } + if (missing.length) console.error(`Missing baseline for:\n ${missing.join('\n ')}\nRun: npm run --prefix tools/vendor coverage:update`); + if (stale.length) console.error(`Baseline lists files that no longer exist:\n ${stale.join('\n ')}\nRemove them from tools/vendor/frontend-coverage-baseline.json`); + process.exitCode = 1; +} +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) main(); diff --git a/tools/vendor/package.json b/tools/vendor/package.json index 0fda145..c64b903 100644 --- a/tools/vendor/package.json +++ b/tools/vendor/package.json @@ -15,6 +15,7 @@ "test:comfyui": "playwright test --config comfyui.config.mjs", "test:executions": "playwright test --config playwright.execution.config.mjs", "test:coverage": "node frontend-coverage.mjs", + "coverage:baseline-check": "node coverage-baseline-check.mjs", "coverage:update": "node frontend-coverage.mjs --update-baseline", "test:compat": "playwright test --config pack-compat.config.mjs" },