From 024ccd3acffab513f77359e6c3eb249cb1205e7d Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Thu, 22 Oct 2026 13:39:39 -0700 Subject: [PATCH] fix(annotations): re-read a tagged file in place after a save MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After a tag save the selected file's inspection is re-read quietly: the panel and the viewer keep the current details until the new answer lands, with no "Loading file details…" in between. While a Tags filter or a search is active, Images re-reads its loaded range in place (same query, from the first page up to the loaded count, published once), keeping the cards, the scroll position and the selection while the selected file still matches; when it no longer matches, the panel follows the collection. --- tests/browser/workspace-tagging.spec.mjs | 106 ++++++++++++++++++++ tests/js/workspace-images-state.test.js | 46 +++++++++ tests/js/workspace-inspection-state.test.js | 28 ++++++ web/js/workspace/images-state.mjs | 50 ++++++++- web/js/workspace/images.mjs | 28 ++++-- web/js/workspace/inspection-state.mjs | 14 ++- 6 files changed, 260 insertions(+), 12 deletions(-) diff --git a/tests/browser/workspace-tagging.spec.mjs b/tests/browser/workspace-tagging.spec.mjs index 5c20e2f..090f7bb 100644 --- a/tests/browser/workspace-tagging.spec.mjs +++ b/tests/browser/workspace-tagging.spec.mjs @@ -530,3 +530,109 @@ test('search finds an image by its image tag', async ({ page, request }) => { await expect(page.locator('.gallery-body article[data-image-key]')).toHaveCount(1); await expect(card(page, UNLINKED)).toBeVisible(); }); + +// Task C4 rulings: a save re-reads the selected file's details without a loading flash, and under a Tags filter or a +// search the loaded listing is re-read in place (scroll and selection kept while the file still matches). +const cards = page => page.locator('.gallery-body article[data-image-key]'); +const listingOf = (page, check) => page.waitForResponse(item => item.url().includes('/images/output?') && check(new URL(item.url()).searchParams)); + +test('the post-save re-read keeps the panel content until the new read lands', async ({ page }) => { + await page.goto(images); + await expect(card(page, FIRST)).toHaveClass(/\bselected\b/); + await expect(tagRow(page).locator('.chip')).toHaveText(manifest.pair_tags); + await page.evaluate(() => { + const node = document.querySelector('aside[aria-label="Selected image details"]'); + window.__loadingSeen = false; + window.__loadingWatch = new MutationObserver(() => { if (node.textContent.includes('Loading file details')) window.__loadingSeen = true; }); + window.__loadingWatch.observe(node, { childList: true, subtree: true, characterData: true }); + }); + let release; + const held = new Promise(resolve => { release = resolve; }); + const matcher = url => url.pathname === `/prompt_manager/images/inspect/${FIRST}`; + await page.route(matcher, async route => { await held; await route.continue(); }); + await openTag(page, FIRST, { goto: false }); + await manual(page).fill('held_tag'); + const reread = page.waitForRequest(item => new URL(item.url()).pathname === `/prompt_manager/images/inspect/${FIRST}`); + await dialog(page).getByRole('button', { name: 'Save manual tags', exact: true }).click(); + await reread; + await expect(toast(page)).toHaveText('1 tags saved for dunes_00001.png.'); + // While the re-read is held, the panel still shows the file's details. + await expect(panel(page).locator('.panel-heading h2')).toHaveText(manifest.title); + await expect(tagRow(page).locator('.chip')).toHaveText(manifest.pair_tags); + expect(await page.evaluate(() => window.__loadingSeen)).toBe(false); + const landed = inspected(page, FIRST); + release(); + await landed; + await expect(tagRow(page).locator('.chip')).toHaveText(['held_tag', ...manifest.pair_tags]); + expect(await page.evaluate(() => { window.__loadingWatch.disconnect(); return window.__loadingSeen; })).toBe(false); + await page.unroute(matcher); +}); + +test('a save under a Tags filter re-reads the listing: the selection stays while it matches and follows when it leaves', async ({ page, request }) => { + await saveImageTags(request, UNLINKED, ['shared']); + await saveImageTags(request, SECOND, ['shared']); + await page.goto(images); + const read = inspected(page, SECOND); + await infoButton(page, SECOND).click(); + await read; + const filtered = listingOf(page, query => query.get('tags') === 'shared'); + await tagRow(page).getByRole('button', { name: 'shared', exact: true }).click(); + await filtered; + await expect(cards(page)).toHaveCount(2); + // The newest match (SECOND) is selected once the filter applies. + await expect(card(page, SECOND)).toHaveClass(/\bselected\b/); + // Tagging the other match keeps both cards and SECOND selected; the listing is re-read. + await openTag(page, UNLINKED, { goto: false }); + await manual(page).fill('shared, extra'); + let reread = listingOf(page, query => query.get('tags') === 'shared' && !query.has('cursor')); + await dialog(page).getByRole('button', { name: 'Save manual tags', exact: true }).click(); + await reread; + await expect(cards(page)).toHaveCount(2); + await expect(card(page, SECOND)).toHaveClass(/\bselected\b/); + await expect(tagButton(page, UNLINKED)).toBeFocused(); + // SECOND loses the filter tag: it leaves the result and the panel follows the remaining match. + await openTag(page, SECOND, { goto: false }); + await manual(page).fill(''); + reread = listingOf(page, query => query.get('tags') === 'shared' && !query.has('cursor')); + const next = inspected(page, UNLINKED); + await dialog(page).getByRole('button', { name: 'Save manual tags', exact: true }).click(); + await reread; + await next; + await expect(cards(page)).toHaveCount(1); + await expect(card(page, SECOND)).toHaveCount(0); + await expect(card(page, UNLINKED)).toHaveClass(/\bselected\b/); + await expect(panel(page).locator('.panel-heading h2')).toHaveText('unlinked.png'); + await expect(page.getByText('1 of 1 image', { exact: false }).first()).toBeVisible(); +}); + +test('a save under a search keeps the scroll position and the loaded range', async ({ page, request }) => { + await expect(await request.post('/__test__/tagging/reset', { data: { extra_images: 140 } })).toBeOK(); + await page.goto(images); + const searched = listingOf(page, query => query.get('q') === 'bulk'); + await page.getByRole('searchbox', { name: 'Search workspace', exact: true }).fill('bulk'); + await searched; + const continuation = page.locator('#gallery-continuation'); + // Load at least two pages (the search lists 40 a page), then rest a third of the way down. + const loadedCount = async () => { + const text = await continuation.textContent(); + return /caught up/.test(text) ? 140 : Number(/^(\d+) of 140/.exec(text)?.[1] ?? 0); + }; + await expect.poll(async () => { await continuation.scrollIntoViewIfNeeded(); return loadedCount(); }).toBeGreaterThanOrEqual(80); + await page.evaluate(() => window.scrollTo(0, Math.round(document.documentElement.scrollHeight / 3))); + await expect(continuation).not.toBeInViewport(); + const loaded = await continuation.textContent(); + await expect.poll(() => continuation.textContent()).toBe(loaded); + const y = await page.evaluate(() => window.scrollY); + expect(y).toBeGreaterThan(0); + const visible = await cards(page).evaluateAll(nodes => nodes.find(node => { const box = node.getBoundingClientRect(); return box.top > 80 && box.bottom < innerHeight; })?.dataset.imageKey); + expect(visible).toBeTruthy(); + const path = visible.slice(2); + await openTag(page, path, { goto: false }); + await manual(page).fill('kept_in_place'); + const reread = listingOf(page, query => query.get('q') === 'bulk' && !query.has('cursor')); + await dialog(page).getByRole('button', { name: 'Save manual tags', exact: true }).click(); + await reread; + await expect(tagButton(page, path)).toBeFocused(); + await expect(continuation).toHaveText(loaded); + expect(Math.abs(await page.evaluate(() => window.scrollY) - y)).toBeLessThanOrEqual(1); +}); diff --git a/tests/js/workspace-images-state.test.js b/tests/js/workspace-images-state.test.js index f9f33c0..e47ffad 100644 --- a/tests/js/workspace-images-state.test.js +++ b/tests/js/workspace-images-state.test.js @@ -352,3 +352,49 @@ test('selectionLabel counts the selected images as the concept bar does', async assert.equal(selectionLabel(2), '2 selected'); assert.equal(selectionLabel(1234), '1,234 selected'); }); + +test('refresh re-reads the loaded range in place and publishes once, replacing the items', async () => { + const {createImageStream,imageKey}=await source;const requests=[];const states=[]; + const stream=createImageStream({api:{get:(url,{signal})=>{const d=deferred();requests.push({...d,url,signal});return d.promise;}},onChange:(s,hint)=>states.push([s,hint]),batchSize:2}); + assert.equal(typeof stream.refresh,'function','refresh is part of the stream'); + const first=stream.reset({q:'tag'});requests[0].resolve(page([image('a'),image('b')],5,'c1'));await first; + const second=stream.loadMore();requests[1].resolve(page([image('c'),image('d')],5,'c2'));await second; + const before=stream.state;const published=states.length; + const refresh=stream.refresh(); + // The current collection stays shown while the re-read runs; Load more waits for it. + assert.equal(stream.state,before);assert.equal(states.length,published); + assert.equal(await stream.loadMore(),0);assert.equal(requests.length,3); + const query=new URL(requests[2].url,'http://local').searchParams; + assert.equal(query.get('q'),'tag');assert.equal(query.has('cursor'),false); + requests[2].resolve(page([image('a'),image('new')],6,'n1')); + await new Promise(resolve=>setImmediate(resolve)); + assert.equal(new URL(requests[3].url,'http://local').searchParams.get('cursor'),'n1'); + assert.equal(stream.state,before); + requests[3].resolve(page([image('c'),image('d')],6,'n2')); + assert.equal(await refresh,4); + assert.deepEqual(stream.state.items.map(imageKey),['0:a','0:new','0:c','0:d']); + assert.deepEqual([stream.state.total,stream.state.cursor,stream.state.hasMore,stream.state.loaded,stream.state.loading],[6,'n2',true,4,false]); + assert.equal(states.length,published+1);assert.deepEqual(states.at(-1)[1],{replaced:true}); + // Load more continues from the refreshed cursor. + const more=stream.loadMore();assert.equal(new URL(requests[4].url,'http://local').searchParams.get('cursor'),'n2'); + requests[4].resolve(page([image('e')],6));await more; + assert.deepEqual(stream.state.items.map(imageKey),['0:a','0:new','0:c','0:d','0:e']); +}); + +test('refresh stops early when the result shrank, and a reset or failure leaves the collection as it was', async () => { + const {createImageStream,imageKey}=await source;const requests=[]; + const stream=createImageStream({api:{get:(url,{signal})=>{const d=deferred();requests.push({...d,url,signal});return d.promise;}},batchSize:2}); + assert.equal(typeof stream.refresh,'function','refresh is part of the stream'); + const first=stream.reset({tags:['x']});requests[0].resolve(page([image('a'),image('b')],4,'c1'));await first; + const shrink=stream.refresh();requests[1].resolve(page([image('b')],1)); + assert.equal(await shrink,1);assert.deepEqual(stream.state.items.map(imageKey),['0:b']);assert.equal(stream.state.hasMore,false); + const before=stream.state; + const failing=stream.refresh();requests[2].reject(new Error('offline')); + assert.equal(await failing,0);assert.equal(stream.state,before); + const stale=stream.refresh();const fresh=stream.reset({tags:['y']}); + assert.equal(requests[3].signal.aborted,true); + requests[4].resolve(page([image('y')]));await fresh; + requests[3].resolve(page([image('stale')]));assert.equal(await stale,0); + assert.deepEqual(stream.state.items.map(imageKey),['0:y']); + stream.dispose();assert.equal(await stream.refresh(),0); +}); diff --git a/tests/js/workspace-inspection-state.test.js b/tests/js/workspace-inspection-state.test.js index ef65b55..ac33dae 100644 --- a/tests/js/workspace-inspection-state.test.js +++ b/tests/js/workspace-inspection-state.test.js @@ -728,3 +728,31 @@ test('rejects a malformed image_tags facet', () => { } assert.equal(module.toInspectionRecord({ ...envelope(entry), image_tags: { state: 'current', tags: ['é'.repeat(50)] } }, entry, extractGenerationFacts).imageTags.tags[0].length, 50); }); + +test('refresh re-reads the selected file and keeps the shown record until the new read lands', async () => { + const { api, requests } = pendingApi(), updates = []; + const controller = contract('createInspectionController')({ api, extractFacts: extractGenerationFacts }); + assert.equal(typeof controller.refresh, 'function', 'refresh is part of the controller'); + controller.subscribe(state => updates.push(state)); + const load = controller.select(image()); requests[0].resolve(envelope()); await load; + const shown = controller.state, count = updates.length; + const refresh = controller.refresh(); + assert.equal(requests.length, 2); + // No loading state is published: the panel keeps its content. + assert.equal(controller.state, shown); assert.equal(updates.length, count); + requests[1].resolve({ ...envelope(), image_tags: { state: 'current', tags: ['fresh'] } }); await refresh; + assert.deepEqual(controller.state.record.imageTags, { state: 'current', tags: ['fresh'] }); + assert.equal(controller.state.loading, false); assert.equal(updates.length, count + 1); + // A failed quiet re-read reports the failure as a read would. + const failing = controller.refresh(); requests[2].reject(new Error('offline')); await failing; + assert.equal(controller.state.error, true); + // Without a shown record a refresh is an ordinary retry (loading published). + const again = controller.refresh(); assert.equal(controller.state.loading, true); + requests[3].resolve(envelope()); await again; assert.equal(controller.state.error, false); + // A newer selection supersedes a refresh in flight. + const stale = controller.refresh(); const other = controller.select({ ...image(), path: 'folder/b.png', filename: 'b.png', url: image().url.replace('a%20%23%25', 'b') }); + requests[5].resolve(envelope({ ...image(), path: 'folder/b.png', filename: 'b.png', url: image().url.replace('a%20%23%25', 'b') })); await other; + requests[4].resolve(envelope()); await stale; + assert.equal(controller.state.record.identity.path, 'folder/b.png'); + controller.dispose(); assert.equal(await controller.refresh(), undefined); +}); diff --git a/web/js/workspace/images-state.mjs b/web/js/workspace/images-state.mjs index 4c5b84b..211dc70 100644 --- a/web/js/workspace/images-state.mjs +++ b/web/js/workspace/images-state.mjs @@ -110,8 +110,11 @@ export function createImageStream({ api, onChange = () => {}, batchSize = 40, re onChange(state); } + // A refresh in flight (its abort controller): Load more waits for it, a reset or dispose cancels it. + let refreshing = null; + async function loadMore() { - if (disposed || state.loading || !state.hasMore) return 0; + if (disposed || state.loading || !state.hasMore || refreshing) return 0; const request = new AbortController(); controller = request; const cursor = state.cursor; @@ -148,11 +151,53 @@ export function createImageStream({ api, onChange = () => {}, batchSize = 40, re } } + /** + * Re-read the loaded range of the same query (a write may have changed which files match) and publish the new + * collection once, with the hint `{replaced: true}`; the current items stay shown until then. A failure, a reset + * or a dispose leaves the collection as it was. Resolves to the number of items now loaded (0 when abandoned). + */ + async function refresh() { + if (disposed || state.loading) return 0; + refreshing?.abort(); + const request = refreshing = new AbortController(); + const target = Math.max(state.loaded, 1); + const items = [], keys = new Set(); + let cursor = null, total = 0, hasMore = true, positiveCount = null; + try { + while (items.length < target && hasMore) { + const parameters = new URLSearchParams(query); + if (cursor !== null) parameters.set('cursor', cursor); + const page = await readPage({parameters, signal:request.signal}); + if (disposed || request !== refreshing) return 0; + validatePage(page, cursor); + for (const item of page.images) { + const key = imageKey(item); + if (keys.has(key)) continue; + keys.add(key); + items.push(Object.freeze({ ...item })); + } + if (cursor === null) positiveCount = isCount(page.positive_count) ? page.positive_count : null; + ({total, next_cursor: cursor, has_more: hasMore} = page); + } + } catch { + if (request === refreshing) refreshing = null; + return 0; + } + refreshing = null; + seen = keys; + state = Object.freeze({ ...state, items: Object.freeze(items), total, cursor, hasMore, loading: false, error: null, + loaded: items.length, positiveCount }); + onChange(state, {replaced: true}); + return items.length; + } + async function reset(filters = {}, {load = true} = {}) { if (disposed) return 0; const nextQuery = queryFor(filters, batchSize); controller?.abort(); controller = null; + refreshing?.abort(); + refreshing = null; query = nextQuery; seen = new Set(); publish({ items: Object.freeze([]), total: 0, cursor: null, hasMore: true, @@ -164,6 +209,7 @@ export function createImageStream({ api, onChange = () => {}, batchSize = 40, re get state() { return state; }, reset, loadMore, + refresh, /** Patch one loaded item (a favorite toggle); false when it is not loaded or the stream is gone. */ updateItem(key, patch) { const next = withItem(state, key, patch); @@ -178,6 +224,8 @@ export function createImageStream({ api, onChange = () => {}, batchSize = 40, re disposed = true; controller?.abort(); controller = null; + refreshing?.abort(); + refreshing = null; }, }; } diff --git a/web/js/workspace/images.mjs b/web/js/workspace/images.mjs index de1b862..d56ef9d 100644 --- a/web/js/workspace/images.mjs +++ b/web/js/workspace/images.mjs @@ -410,9 +410,13 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage else if (isTagButton(action)) openImageTagging({api, scope, image, source: embedded ? 'library' : 'images', trigger: action}); else if (action) inspect(image); }); - // A saved tag set re-reads the selected file's details (its inspector envelope carries the image tags). - scope.cleanup(onImageTagsSaved(({target}) => { - if (selection === `${target.root_index}:${target.path}`) inspection.retry(); + // A saved tag set re-reads the selected file's details (its inspector envelope carries the image tags) without a + // loading flash. Under a Tags filter or a search the saved file may now match or stop matching: the loaded range + // is re-read in place first, keeping the scroll and the selection while it still matches. + scope.cleanup(onImageTagsSaved(async ({target}) => { + const key = `${target.root_index}:${target.path}`; + if (!embedded && !recovering && (filters.tags.length || filters.q)) await stream.refresh(); + if (!scope.signal.aborted && selection === key) inspection.refresh(); })); function renderVisible() { @@ -548,14 +552,17 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage // The concept's end of the collection (app.js updateImageContinuation). return `${next.loaded ? 'You’re all caught up' : 'No matching images'} · ${next.total.toLocaleString('en-US')} ${noun(next.total, 'image')}`; } - function renderState(next, {patch} = {}) { + function renderState(next, {patch, replaced = false} = {}) { const previous = state; // A patched item (a favorite) keeps every card's size and place; setFavorite repaints its card. const changed = !patch && state?.items !== next.items; if (changed) { - if (!next.items.length) itemIndexes.clear(); - for (let index = state?.items.length || 0; index < next.items.length; index++) itemIndexes.set(imageKey(next.items[index]), index); + // A refreshed collection (`replaced`) is indexed again from its start; a loaded page only appends. + if (!next.items.length || replaced) itemIndexes.clear(); + for (let index = replaced ? 0 : state?.items.length || 0; index < next.items.length; index++) itemIndexes.set(imageKey(next.items[index]), index); } + // The selected file left the refreshed result: the panel follows the collection, as after a reload. + if (replaced && selection !== null && !itemIndexes.has(selection)) dropSelection(); state = next; if (!embedded) { setText(count, countLine(next)); @@ -564,7 +571,7 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage } if (changed) updateLayout(); // The concept's Load more toast (app.js loadMoreImages) for every batch appended to a loaded collection. - if (!embedded && changed && previous?.loaded && next.loaded > previous.loaded) { + if (!embedded && changed && !replaced && previous?.loaded && next.loaded > previous.loaded) { const added = next.loaded - previous.loaded; showWorkspaceToast(`${added.toLocaleString('en-US')} more ${noun(added, 'image')} loaded. ${next.loaded.toLocaleString('en-US')} of ${next.total.toLocaleString('en-US')}.`); } @@ -624,6 +631,13 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage scopeText.textContent = ` ${scopeLabel(filters, roots)}`; toolbar?.setActiveFilters(activeRefinements(filters)); } + function dropSelection() { + inspection.clear(); + detailInspector?.dispose(); detailInspector = null; + for (const video of details.querySelectorAll('video')) releaseVideo(video); + details.replaceChildren(noSelection()); + selection = null; + } function reset() { mediaViewer.stopSlideshow(); main.querySelector('#viewer-dialog[open]')?.close(); diff --git a/web/js/workspace/inspection-state.mjs b/web/js/workspace/inspection-state.mjs index 5f792a4..9b27a42 100644 --- a/web/js/workspace/inspection-state.mjs +++ b/web/js/workspace/inspection-state.mjs @@ -317,11 +317,12 @@ export const rootedFileAdapter = Object.freeze({ export function createInspectionLoader(api, onChange, { extractFacts, adapter = rootedFileAdapter } = {}) { let request = null, disposed = false; return { - async load(image) { + /** `quiet`: publish only the answer, so the shown record stays until it lands (a re-read after a write). */ + async load(image, { quiet = false } = {}) { if (disposed) return; request?.abort(); const current = request = new AbortController(); - onChange({ loading: true }); + if (!quiet) onChange({ loading: true }); try { const result = await api.get(adapter.inspectionPath(image), { signal: current.signal }); if (disposed || request !== current) return; @@ -354,8 +355,8 @@ export function createInspectionController({ api, extractFacts, adapter = rooted } const makeLoader = () => createInspectionLoader(api, publish, { extractFacts, adapter }); let loader = makeLoader(); - function read(image) { - pending = loader.load(image); + function read(image, options) { + pending = loader.load(image, options); return pending; } return { @@ -379,6 +380,11 @@ export function createInspectionController({ api, extractFacts, adapter = rooted if (disposed || !hasAttempt || state.refreshRequired) return; return read(attempted); }, + /** Re-read the selected file keeping the shown record until the answer (a retry when none is shown). */ + refresh() { + if (disposed || !hasAttempt || state.refreshRequired) return; + return read(attempted, { quiet: Boolean(state.record) && !state.loading && !state.error }); + }, clear() { if (disposed) return; loader.dispose(); loader = makeLoader(); current = null; pending = null; attempted = undefined; hasAttempt = false;