From 7f83ea60a674ffe3daa0598fb44cefad10844242 Mon Sep 17 00:00:00 2001 From: Sparky <1609870+sparkyfen@users.noreply.github.com> Date: Wed, 16 Sep 2026 11:42:59 -0700 Subject: [PATCH 1/7] fix(admin): five leftovers from the SONA-220 review The markup test's function-body slicer counted braces written inside comments and strings, and scanned to the end of the file when the depth never came back to zero. It now steps over line comments, block comments and string literals, and throws a message naming the function when the closing brace never arrives. Four cases cover it. The FuzzySearch key spec left the saved key behind for the test after it: a failure part way through the saving test handed the serial retry a connected section, which fails the unconnected-state test for a reason that has nothing to do with it. Each test now removes the key before it starts, the way artist-lookup.spec.ts already does for the same row, and the afterAll still puts the row back for the other spec files. pickParent read the tile's lookup kind through a groupMode test that had no second answer to give: both callers run in the new-set mode only. The branch is gone and the kind comes off the tile. The lookup panel's idle arm drew nothing after a parent move onto a tile that was never looked up, so the two shared fields went blank with only the announcement saying why. It now draws the sentence the searching arm draws, aria-hidden so the panel's status region does not repeat what pickParent just announced, and the panel keeps its card while that line is up. The latch comment said each latch rises on input to either field. Each one rises on input to its own field. --- src/lib/artist-lookup-markup.test.ts | 179 +++++++++++++++++--- src/lib/components/ArtistLookupPanel.svelte | 17 +- src/routes/admin/upload/+page.svelte | 17 +- tests/e2e/fuzzysearch-key.spec.ts | 52 ++++-- 4 files changed, 217 insertions(+), 48 deletions(-) diff --git a/src/lib/artist-lookup-markup.test.ts b/src/lib/artist-lookup-markup.test.ts index ba29189a..0ba6b7ef 100644 --- a/src/lib/artist-lookup-markup.test.ts +++ b/src/lib/artist-lookup-markup.test.ts @@ -21,34 +21,152 @@ const ANNOUNCER = read('src/lib/components/LiveAnnouncer.svelte'); // tag-suggestion note it also holds (SONA-220). const APP_CSS = read('src/app.css'); +// The end of the string literal that opens at `i`, one past its closing quote. +// Throws rather than running to the end of the file: an unterminated literal +// means the scanner has lost its place, and a silent slice from there would +// hand every assertion under it the wrong text. +function endOfString(source: string, i: number): number { + const quote = source[i]; + for (let j = i + 1; j < source.length; j++) { + if (source[j] === '\\') { + j++; + continue; + } + if (source[j] === quote) return j + 1; + // A plain quote cannot span a line; a template literal can. + if (quote !== '`' && source[j] === '\n') break; + } + throw new Error(`unterminated ${quote} literal at index ${i} in source`); +} + +// The index of the `close` that matches the first `open` at or after `from`, +// stepping over `//` line comments, `/* */` block comments and string literals +// so a delimiter written inside one never moves the depth. Throws when the +// closer never arrives, so a scan that walks off the end says so instead of +// returning a slice that stops wherever the source did. +function matchDelim( + source: string, + from: number, + open: string, + close: string, + what: string +): number { + let depth = 0; + let i = from; + while (i < source.length) { + if (source.startsWith('//', i)) { + const eol = source.indexOf('\n', i); + i = eol < 0 ? source.length : eol + 1; + continue; + } + if (source.startsWith('/*', i)) { + const end = source.indexOf('*/', i + 2); + if (end < 0) throw new Error(`unterminated block comment at index ${i} in source`); + i = end + 2; + continue; + } + if (source[i] === '"' || source[i] === "'" || source[i] === '`') { + i = endOfString(source, i); + continue; + } + if (source[i] === open) depth++; + else if (source[i] === close && --depth === 0) return i; + i++; + } + throw new Error( + depth === 0 + ? `no ${open} for ${what} in source` + : `no closing ${close} for ${what} in source (depth ${depth} at the end of the file)` + ); +} + // A function's body, sliced from its declaration by matching braces. A // `[\s\S]*?\n\t}` span stops at the first one-tab closing brace, which on a // multi-line return type is the end of the annotation rather than the end of // the body, and every assertion under it then reads the signature alone // (SONA-220). Throws when the function is gone, so a rename can never leave a -// negative assertion asserting nothing. +// negative assertion asserting nothing, and throws again when the braces never +// balance rather than scanning to the end of the file and slicing whatever it +// reached. function fnBody(source: string, name: string): string { const start = source.indexOf(`function ${name}(`); if (start < 0) throw new Error(`no function named ${name} in source`); - let i = start; - for (let depth = 0; ; i++) { - if (source[i] === '(') depth++; - else if (source[i] === ')' && --depth === 0) break; - } + let i = matchDelim(source, start, '(', ')', `${name}'s parameter list`) + 1; // Past the parameter list, a brace-balanced span with another `{` after it is // a return-type annotation; the one nothing follows is the body. for (;;) { - let close = source.indexOf('{', i); - for (let depth = 0; ; close++) { - if (source[close] === '{') depth++; - else if (source[close] === '}' && --depth === 0) break; - } + const close = matchDelim(source, i, '{', '}', `${name}'s body`); const rest = source.slice(close + 1); if (rest[rest.search(/\S/)] !== '{') return source.slice(start, close + 1); i = close + 1; } } +// Every assertion in this file that reads one function rather than the whole +// source rests on the slicer above, so the slicer is pinned too: a brace it +// miscounts hands the assertion under it a body that stops in the middle, and +// a negative assertion then passes on text that was never searched. +describe('the function-body slicer', () => { + it('ignores a brace written inside a comment', () => { + const line = [ + 'function sample() {', + '\t// a stray } in a line comment', + '\tconst kept = 1;', + '}', + 'const after = 2;' + ].join('\n'); + expect(fnBody(line, 'sample')).toContain('const kept = 1;'); + expect(fnBody(line, 'sample')).not.toContain('const after'); + + const block = [ + 'function sample() {', + '\t/* a stray } and a { in a block comment', + '\t that runs over two lines */', + '\tconst kept = 1;', + '}', + 'const after = 2;' + ].join('\n'); + expect(fnBody(block, 'sample')).toContain('const kept = 1;'); + expect(fnBody(block, 'sample')).not.toContain('const after'); + }); + + it('ignores a brace written inside a string', () => { + const strings = [ + 'function sample() {', + "\tconst quoted = '}';", + '\tconst template = `${quoted} {`;', + '\tconst kept = 1;', + '}', + 'const after = 2;' + ].join('\n'); + expect(fnBody(strings, 'sample')).toContain('const kept = 1;'); + expect(fnBody(strings, 'sample')).not.toContain('const after'); + }); + + // The reason the scan is bounded: run off the end and it used to read past + // the string entirely, comparing undefined forever. + it('says so when the closing brace never comes', () => { + expect(() => fnBody('function sample() {\n\tconst unclosed = 1;\n', 'sample')).toThrow( + /no closing \} for sample's body/ + ); + expect(() => fnBody('const x = 1;\n', 'sample')).toThrow(/no function named sample/); + }); + + // The case the slicer exists for: a multi-line return type annotation is a + // balanced brace span that another `{` follows, and the body is the one + // nothing follows. + it('takes the body rather than a multi-line return type', () => { + const annotated = [ + 'function sample(): {', + '\tfield: string;', + '} {', + '\tconst kept = 1;', + '}' + ].join('\n'); + expect(fnBody(annotated, 'sample')).toContain('const kept = 1;'); + }); +}); + describe('lookup button and its disclosure hint', () => { it('offers the button only when a key is configured, on both pages', () => { for (const source of [UPLOAD, EDIT]) { @@ -254,7 +372,9 @@ describe('the panel', () => { // and that first content is the "lookup has started" message. The region is // in the DOM from the first render; the state branch is inside it. it('keeps the live region mounted while idle, collapsed to nothing', () => { - expect(PANEL).toMatch(/class:idle=\{lookup\.kind === 'idle'\}/); + // Collapsed only while the idle arm has nothing to draw: with a cleared + // sentence in it, the padless card would put that line against the edge. + expect(PANEL).toMatch(/class:idle=\{lookup\.kind === 'idle' && !movedEmptied\}/); expect(PANEL).toMatch( /
\s*\{#if lookup\.kind !== 'idle'\}/ ); @@ -263,6 +383,23 @@ describe('the panel', () => { expect(PANEL).not.toMatch(/\.lookup-panel\.idle \{[^}]*display: none/); }); + // A parent move onto a tile that was never looked up empties the two shared + // fields, and the idle arm drew nothing: the announcement was the whole + // telling, so a sighted operator watched the fields go blank with no reason + // anywhere on screen (4.1.3). + it('draws the cleared sentence in the idle arm too, without re-speaking it', () => { + expect(PANEL).toMatch( + /\{:else if movedEmptied\}(?:\s|)*
/); + }); + // "Fu", "We", "e6", "Tw" read as truncated text next to the site's own name. it('marks each result row with a brand icon, not two letters of the name', () => { expect(PANEL).not.toContain('match.site.slice(0, 2)'); @@ -1172,18 +1309,20 @@ describe('what the lookup copy names', () => { expect(PANEL).toMatch( /const statusText = \$derived\(\s+statusSentence\(statusKind, prefill\?\.site \?\? null, \{ title: clash\?\.title \?\? '', editMode \}\)\s+\);/ ); - // And the radio path stays quiet wherever the panel carries the sentence, + // And the radio path stays quiet wherever the panel SPEAKS the sentence, // which is every arm but idle: the searching arm renders it too, and the // region is atomic, so a say() alongside it would be the second telling. + // The idle arm draws it aria-hidden, which the region does not read, so + // the announcement there is still the only telling a screen reader gets. const pickBody = fnBody(UPLOAD, 'pickParent'); expect(pickBody).toMatch(/if \(kind !== 'idle'\) return;/); - // Gated the way sharedLookup is, not read straight off the tile. The panel - // is mounted in the new-set mode only, so outside it no arm carries the - // sentence however that tile's own lookup ended — read from the tile - // there, the guard would fall silent with nothing on screen in its place - // (4.1.3). - expect(pickBody).toContain( - "const kind = (groupMode === 'new' ? tiles[index]?.lookup.kind : undefined) ?? 'idle';" + // Read straight off the tile: both callers run in the new-set mode only, + // so a mode test here had no second answer to give. + expect(pickBody).toContain("const kind = tiles[index]?.lookup.kind ?? 'idle';"); + // The radio is the one caller that could reach another mode, and it is + // rendered inside the new-set branch. + expect(UPLOAD).toMatch( + /\{#if groupMode === 'new'\}(?:[\s\S]{0,600}?)onchange=\{\(\) => pickParent\(i\)\}/ ); expect(UPLOAD).toMatch( /const parentTile = \$derived\(groupMode === 'new' \? \(tiles\[parentIndex\] \?\? null\) : null\);/ diff --git a/src/lib/components/ArtistLookupPanel.svelte b/src/lib/components/ArtistLookupPanel.svelte index d65454dd..32a61fd0 100644 --- a/src/lib/components/ArtistLookupPanel.svelte +++ b/src/lib/components/ArtistLookupPanel.svelte @@ -237,7 +237,7 @@ hears the panel once for it, not once per keystroke. -->
@@ -487,6 +487,17 @@ {#if privateNotice}

{m.admin_lookup_private_notice()}

{/if} + {:else if movedEmptied} + + {/if}
@@ -673,7 +684,9 @@ /* Idle: no card, no space, but the live region above still exists so the first message written into it is announced. Not display:none — a hidden - region is not a region a screen reader watches. */ + region is not a region a screen reader watches. Dropped when the idle arm + has a cleared sentence to draw, so that line sits in the card every other + arm's status line sits in rather than against a padless edge. */ .lookup-panel.idle { border: 0; padding: 0; diff --git a/src/routes/admin/upload/+page.svelte b/src/routes/admin/upload/+page.svelte index dbd45df1..c6a2dd7b 100644 --- a/src/routes/admin/upload/+page.svelte +++ b/src/routes/admin/upload/+page.svelte @@ -536,7 +536,7 @@ // emptied puts "Sona cleared the source post URL" back over a field THEY just // emptied, and the panel re-attributes their own deletion to Sona (SONA-220). // Each latch lives exactly as long as the cleared record it speaks for: it - // rises on input to either field, and applyShared recomputes it against the + // rises on input to its own field, and applyShared recomputes it against the // fields as each result lands. See the recompute there for why. const sharedEdited = $derived({ sourcePostUrl: !sourceTagged && (sharedFilled.sourcePostUrl !== undefined || sourceTypedIn), @@ -1123,16 +1123,17 @@ * region carries the sentence for every settled result, and the searching arm * carries it too, so announcing here on either would say the same thing twice * — the region is atomic, so the panel re-speaks whole when the sentence - * appears in it. Only a lookup that never ran draws no sentence at all, and - * then the two fields empty with nothing on screen saying why (4.1.3). + * appears in it. Only a lookup that never ran carries no sentence of its own, + * so the announcement is this move's telling; the panel's idle arm draws the + * same line for sighted operators, aria-hidden so the region does not repeat + * it (4.1.3). * - * Gated the way `sharedLookup` is, rather than read straight off the tile: - * the panel is mounted in the new-set mode only, so outside it no arm carries - * the sentence however that tile's own lookup ended, and the announcement is - * the only telling there is. */ + * Read straight off the tile: both callers run in the new-set mode only — the + * radio renders under `{#if groupMode === 'new'}`, and the removal path checks + * the mode before calling — so there is no other mode to answer for. */ function pickParent(index: number) { const { cleared } = onParentChanged(index); - const kind = (groupMode === 'new' ? tiles[index]?.lookup.kind : undefined) ?? 'idle'; + const kind = tiles[index]?.lookup.kind ?? 'idle'; if (kind !== 'idle') return; const line = clearedLine(cleared, {}); if (line) announcer.say(line); diff --git a/tests/e2e/fuzzysearch-key.spec.ts b/tests/e2e/fuzzysearch-key.spec.ts index 3c70ead7..10d17459 100644 --- a/tests/e2e/fuzzysearch-key.spec.ts +++ b/tests/e2e/fuzzysearch-key.spec.ts @@ -127,15 +127,37 @@ async function openConnectionsTab(page: Page) { }).toPass(); } -// Serial: the two tests below share one settings row, and the second one's -// starting point is the state the seed leaves behind. +// Take the key away if one is saved, leaving the section unconnected. The +// Connections tab has to be open already. Returns whether it removed anything, +// so a caller can say where the leftover came from. +async function removeSavedKey(page: Page): Promise { + if ((await removeButton(page).count()) === 0) return false; + await removeButton(page).click(); + await confirmRemoval(page); + await expect(keyInput(page)).toBeVisible({ timeout: 15_000 }); + return true; +} + +// Serial: the three tests below share one settings row, and each one's starting +// point is the state the seed leaves behind. test.describe.configure({ mode: 'serial' }); test.describe('admin settings artist lookup key', () => { + // Every test here starts from the unconnected state, whoever left the row + // otherwise. The saving test writes the shared row, so a failure part way + // through it used to hand the next run a connected section: under serial + // retries the block restarts at the unconnected-state test, which then fails + // for a reason that has nothing to do with it. Removing here rather than + // only in afterAll makes each test's starting point its own (the shape + // artist-lookup.spec.ts uses for the same row). test.beforeEach(async ({ page }) => { await adminLogin(page, PASSWORD); await page.goto('/admin/settings'); await openConnectionsTab(page); + if (await removeSavedKey(page)) { + console.warn('fuzzysearch-key: a key was saved before this test; removed it'); + await openConnectionsTab(page); + } }); test('the unconnected state discloses what leaves the site and takes a key', async ({ @@ -179,14 +201,9 @@ test.describe('admin settings artist lookup key', () => { // button (it lives in the panel's else branch) and spin to the timeout. await page.goto('/admin/settings'); await openConnectionsTab(page); - if ((await removeButton(page).count()) > 0) { - // The aborted attempt saved the key: put the section back to - // unconnected before trying again. - await removeButton(page).click(); - await confirmRemoval(page); - await expect(keyInput(page)).toBeVisible(); - await openConnectionsTab(page); - } + // The aborted attempt saved the key: put the section back to + // unconnected before trying again. + if (await removeSavedKey(page)) await openConnectionsTab(page); await page.evaluate(() => { (window as unknown as Record).__sonaSaveMarker = true; }); @@ -264,20 +281,19 @@ test.describe('admin settings artist lookup key', () => { }); // The save above writes to the SHARED seeded DB. If anything between it and the -// final Remove fails, the key stays saved: the CI retry restarts this serial -// block at the unconnected-state test, which then fails for the wrong reason, -// and every later spec sees a connected section. Put the row back the way this -// file found it (legal.spec.ts carries the same guard for privacyPolicy). +// final Remove fails, the key stays saved and every later spec sees a connected +// section — the beforeEach only covers the tests in this file. Put the row back +// the way this file found it (legal.spec.ts carries the same guard for +// privacyPolicy). test.afterAll(async ({ browser }) => { const page = await browser.newPage(); try { await adminLogin(page, PASSWORD); await page.goto('/admin/settings'); await openConnectionsTab(page); - if ((await removeButton(page).count()) === 0) return; - await removeButton(page).click(); - await confirmRemoval(page); - await expect(keyRecord(page)).toHaveCount(0); + if (await removeSavedKey(page)) { + console.warn('fuzzysearch-key: the serial chain left the key behind; removed it here'); + } } finally { await page.close(); } From d2cdd4bf2f23c8a5576a36521926fb0678629669 Mon Sep 17 00:00:00 2001 From: Sparky <1609870+sparkyfen@users.noreply.github.com> Date: Wed, 16 Sep 2026 11:44:21 -0700 Subject: [PATCH 2/7] docs(admin): say each typed-in latch rises on its own field (SONA-220) --- src/routes/admin/upload/+page.svelte | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/routes/admin/upload/+page.svelte b/src/routes/admin/upload/+page.svelte index c6a2dd7b..f1e91bcc 100644 --- a/src/routes/admin/upload/+page.svelte +++ b/src/routes/admin/upload/+page.svelte @@ -519,8 +519,8 @@ // put back (SONA-220). Without it the status line says a field was left as it // was while the operator watched it go blank (4.1.3). let sharedCleared = $state({}); - // Whether the operator has text of their own in each field. Raised on input - // to either field, recomputed against the fields in applyShared when a result + // Whether the operator has text of their own in each field. Each one rises on + // input to its own field, is recomputed against the fields in applyShared when a result // lands, and lowered in resetSharedPrefill. See sharedEdited. let sourceTypedIn = $state(false); let dateTypedIn = $state(false); From 8bd5742b784ffa258048bd6a732125cf7b6007c5 Mon Sep 17 00:00:00 2001 From: Sparky <1609870+sparkyfen@users.noreply.github.com> Date: Wed, 16 Sep 2026 13:00:04 -0700 Subject: [PATCH 3/7] fix(admin): the lookup panel speaks the emptied line once and Close collapses it (SONA-220) The idle arm's sentence is a plain paragraph in the panel's status region, which speaks it the way it speaks the searching arm, so the parent move no longer announces it separately and the text stays in the accessibility tree. Close and the edit page's add-as-variant drop the cleared record, so the panel collapses instead of keeping a card with the dismissed sentence. The e2e that asserted the old silence now asserts the visible line and that the page live region does not repeat it. --- src/lib/artist-lookup-markup.test.ts | 126 +++++++++++------- src/lib/components/ArtistLookupPanel.svelte | 15 ++- .../admin/images/[id]/edit/+page.svelte | 7 + src/routes/admin/upload/+page.svelte | 41 +++--- tests/e2e/artist-lookup.spec.ts | 90 +++++++++---- 5 files changed, 174 insertions(+), 105 deletions(-) diff --git a/src/lib/artist-lookup-markup.test.ts b/src/lib/artist-lookup-markup.test.ts index 0ba6b7ef..1d63b42f 100644 --- a/src/lib/artist-lookup-markup.test.ts +++ b/src/lib/artist-lookup-markup.test.ts @@ -44,6 +44,9 @@ function endOfString(source: string, i: number): number { // so a delimiter written inside one never moves the depth. Throws when the // closer never arrives, so a scan that walks off the end says so instead of // returning a slice that stops wherever the source did. +// Regex literals are NOT tracked: a `/[{}]/` inside a scanned body would move +// the depth and a `/won't/` would open a string that never closes. No function +// this file slices has one. function matchDelim( source: string, from: number, @@ -73,11 +76,7 @@ function matchDelim( else if (source[i] === close && --depth === 0) return i; i++; } - throw new Error( - depth === 0 - ? `no ${open} for ${what} in source` - : `no closing ${close} for ${what} in source (depth ${depth} at the end of the file)` - ); + throw new Error(`no closing ${close} for ${what} in source (depth ${depth} at the end)`); } // A function's body, sliced from its declaration by matching braces. A @@ -387,17 +386,21 @@ describe('the panel', () => { // fields, and the idle arm drew nothing: the announcement was the whole // telling, so a sighted operator watched the fields go blank with no reason // anywhere on screen (4.1.3). - it('draws the cleared sentence in the idle arm too, without re-speaking it', () => { + it('draws the cleared sentence in the idle arm too, spoken by the panel', () => { expect(PANEL).toMatch( - /\{:else if movedEmptied\}(?:\s|)*

\{movedEmptied\}<\/p>/ ); - // The same sentence the searching arm draws and pickParent announces, off - // the one chooser: a second source could name a different field. + // The same sentence the searching arm draws, off the one chooser: a second + // source could name a different field. expect(PANEL).toMatch(/const movedEmptied = \$derived\(clearedLine\(cleared, edited\)\);/); - // aria-hidden is the whole reason the announcement can stay: the paragraph - // lands inside the panel's own status region, which would otherwise say it - // a second time. + // And NOT aria-hidden: the paragraph lands inside the panel's own status + // region, which speaks it the way it speaks every other arm, so the move + // needs no announcement of its own and the sentence stays in the + // accessibility tree rather than being spoken and gone. + expect(PANEL).not.toMatch(/class="lookup-status lookup-emptied" aria-hidden/); expect(PANEL).toMatch(/

/); + // Which is why the radio path says nothing at all. + expect(fnBody(UPLOAD, 'pickParent')).not.toContain('announcer.say('); }); // "Fu", "We", "e6", "Tw" read as truncated text next to the site's own name. @@ -720,15 +723,23 @@ describe('the "From lookup" tag', () => { // lines unnoticed (SONA-220). const clearedRecord = source === UPLOAD ? 'sharedCleared' : 'lookupCleared'; // Every site that resets it beside what a result wrote, which is every - // one but startLookup's own drop: that one clears a sentence the page - // has already spoken, touches nothing the result filled, and must NOT - // lower the latches — they carry text the operator typed before the - // search, which the result still has to be told about. Cut out by - // slicing that function away rather than by a lookbehind on the line: + // one but startLookup's own drop and Close's: those two clear a + // sentence the page has already spoken, touch nothing the result + // filled, and must NOT lower the latches — they carry text the operator + // typed before the search, which the result still has to be told about, + // and Close leaves every field exactly as it found it. Cut out by + // slicing those functions away rather than by a lookbehind on the line: // the drop is a bare statement, so nothing on the line itself tells it // from the resets that do count. The edit page's two reset sites both // sit outside its startLookup, so the slice costs that file nothing. - const counted = source.replace(fnBody(source, 'startLookup'), ''); + // The edit page's addAsVariant goes idle on its own and drops the record + // the way Close does, leaving the fields as it found them, so it is + // sliced away for the same reason. + const closer = source === UPLOAD ? 'closeSharedLookup' : 'closeLookup'; + const counted = source + .replace(fnBody(source, 'startLookup'), '') + .replace(fnBody(source, closer), '') + .replace(fnBody(source, 'addAsVariant'), ''); const resets = counted.match(new RegExp(`${clearedRecord} = \\{\\};`, 'g')) ?? []; const withLatches = counted.match( @@ -1214,8 +1225,8 @@ describe('what the lookup copy names', () => { // And nothing else writes sharedCleared. The catch that synthesises a // failure used to assign the held record straight, which skipped the check // above and had the panel report a URL the operator had typed back in. - // Five assignments in the file and no more: the declaration, two blankings - // that hand over no record — startLookup's and the reset's — and + // Six assignments in the file and no more: the declaration, three blankings + // that hand over no record — startLookup's, the reset's and Close's — and // applyShared's two. Everything that DESCRIBES a clearing goes through // applyShared. expect(UPLOAD.match(/sharedCleared = [^;]*/g) ?? []).toEqual([ @@ -1223,7 +1234,8 @@ describe('what the lookup copy names', () => { 'sharedCleared = {}', 'sharedCleared = {}', 'sharedCleared = { ...emptied }', - 'sharedCleared = cleared' + 'sharedCleared = cleared', + 'sharedCleared = {}' ]); expect(UPLOAD).toMatch(/if \(isParent\(key\)\) applyShared\(failed, emptied\);/); // The record dies with the request it was waiting for, in BOTH arms of @@ -1236,12 +1248,13 @@ describe('what the lookup copy names', () => { const startBody = fnBody(UPLOAD, 'startLookup'); // And a lookup started on the parent tile drops the move's record on the // way in, where nothing else would, whatever that tile's last lookup came - // to. Whatever the record describes has been told already — by pickParent - // on an idle tile, by the settled arm on any other — and this search has - // changed nothing yet, so the searching arm about to render must not put - // the same sentence in the panel's atomic status region (4.1.3). Gated on - // the tile being idle, a plain repeat lookup after a no-match carried the - // no-match's record into the searching arm and said it a second time. + // to. Whatever the record describes has been told already — by the panel's + // idle arm on an idle tile, by the settled arm on any other — and this + // search has changed nothing yet, so the searching arm about to render must + // not put the same sentence in the panel's atomic status region (4.1.3). + // Gated on the tile being idle, a plain repeat lookup after a no-match + // carried the no-match's record into the searching arm and said it a + // second time. expect(startBody).toMatch( /if \(isParent\(key\)\) \{[\s\S]{0,900}?\n\t\t\tsharedCleared = \{\};\s+resetSharedResult\(\);\s+\}/ ); @@ -1276,18 +1289,18 @@ describe('what the lookup copy names', () => { // The move itself says nothing: one tick holds one line, so the caller // picks it. Each of the three callers says at most one. expect(UPLOAD).not.toMatch(/function onParentChanged\([\s\S]{0,800}?announcer\.say/); - for (const name of ['pickParent', 'returnToNewSet']) { - const body = fnBody(UPLOAD, name); - // One chain, so however many sentences it can choose between, a tick - // reaches exactly one of them: two say() calls in a tick leave the region - // holding the second, which is how the clearing went unspoken. pickParent - // ends on one line held in a variable, returnToNewSet on an else-if chain - // that ends the same way — either shape says at most one thing. - expect(body).toMatch(/announcer\.say/); - expect(body).toMatch( - /const line = clearedLine\(cleared, \{\}\);\s+if \(line\) announcer\.say\(line\);/ - ); - } + // returnToNewSet is the one caller that speaks: it runs on the flip back + // into the new-set mode, where the panel was unmounted a moment ago and has + // nothing to say for the fields it left behind. One chain, so however many + // sentences it can choose between, a tick reaches exactly one of them: two + // say() calls in a tick leave the region holding the second, which is how + // the clearing went unspoken. The else-if chain ends on one line held in a + // variable. + const returnBody = fnBody(UPLOAD, 'returnToNewSet'); + expect(returnBody).toMatch(/announcer\.say/); + expect(returnBody).toMatch( + /const line = clearedLine\(cleared, \{\}\);\s+if \(line\) announcer\.say\(line\);/ + ); // One chain for what a move emptied, in the shared module rather than on // the page: the panel's searching arm renders the same three sentences // while the result is still out, and a copy on the page could disagree @@ -1309,16 +1322,12 @@ describe('what the lookup copy names', () => { expect(PANEL).toMatch( /const statusText = \$derived\(\s+statusSentence\(statusKind, prefill\?\.site \?\? null, \{ title: clash\?\.title \?\? '', editMode \}\)\s+\);/ ); - // And the radio path stays quiet wherever the panel SPEAKS the sentence, - // which is every arm but idle: the searching arm renders it too, and the - // region is atomic, so a say() alongside it would be the second telling. - // The idle arm draws it aria-hidden, which the region does not read, so - // the announcement there is still the only telling a screen reader gets. + // And the radio path stays quiet: the panel SPEAKS the sentence in every + // arm — the settled ones, the searching one, and the idle one — and the + // region is atomic, so a say() alongside any of them would be the second + // telling. const pickBody = fnBody(UPLOAD, 'pickParent'); - expect(pickBody).toMatch(/if \(kind !== 'idle'\) return;/); - // Read straight off the tile: both callers run in the new-set mode only, - // so a mode test here had no second answer to give. - expect(pickBody).toContain("const kind = tiles[index]?.lookup.kind ?? 'idle';"); + expect(pickBody).not.toContain('announcer.say('); // The radio is the one caller that could reach another mode, and it is // rendered inside the new-set branch. expect(UPLOAD).toMatch( @@ -1496,15 +1505,32 @@ describe('focus after the panel goes away', () => { expect(UPLOAD).toMatch(/function focusLookupOrigin\(\)/); expect(UPLOAD).toMatch(/bind:this=\{lookupPill\}/); expect(UPLOAD).toMatch(/bind:this=\{tileLookupButtons\[tile\.key\]\}/); - expect(UPLOAD).toMatch(/function closeSharedLookup[\s\S]{0,300}?focusLookupOrigin\(\)/); + expect(UPLOAD).toMatch(/function closeSharedLookup[\s\S]{0,600}?focusLookupOrigin\(\)/); // The pill renders above one file only, so in a group it is null: without a // third link the chain would focus nothing if the parent's button ever went // away. The select is where moveFocusOffTileButton ends up too. expect(UPLOAD).toMatch(/\(button \?\? lookupPill \?\? artistSelect\)\?\.focus\(\);/); - expect(EDIT).toMatch(/function closeLookup\(\)[\s\S]{0,200}?lookupPill\?\.focus\(\)/); + expect(EDIT).toMatch(/function closeLookup\(\)[\s\S]{0,300}?lookupPill\?\.focus\(\)/); expect(EDIT).toMatch(/onclose=\{closeLookup\}/); }); + // Close puts the lookup back to idle, and the idle arm draws whatever the + // move or the result emptied. The record left standing, the panel the + // operator just closed stayed on screen as a bordered card holding that + // sentence (SONA-220). + it('drops the cleared record when the panel is closed', () => { + expect(fnBody(UPLOAD, 'closeSharedLookup')).toContain('sharedCleared = {};'); + expect(fnBody(EDIT, 'closeLookup')).toContain('lookupCleared = {};'); + // The edit page's "Add as a variant" goes idle on its own rather than + // through closeLookup, so it drops the record itself; the upload page's + // goes through closeSharedLookup. + expect(fnBody(EDIT, 'addAsVariant')).toContain('lookupCleared = {};'); + expect(fnBody(UPLOAD, 'addAsVariant')).toContain('closeSharedLookup('); + // Which is what collapses the panel: the idle class is off while a cleared + // sentence stands. + expect(PANEL).toMatch(/class:idle=\{lookup\.kind === 'idle' && !movedEmptied\}/); + }); + it('lands on the select that "Add as a variant" just populated', () => { expect(UPLOAD).toMatch( /function addAsVariant[\s\S]{0,1000}?existingParentSelect\?\.focus\(\)/ diff --git a/src/lib/components/ArtistLookupPanel.svelte b/src/lib/components/ArtistLookupPanel.svelte index 32a61fd0..054db5e3 100644 --- a/src/lib/components/ArtistLookupPanel.svelte +++ b/src/lib/components/ArtistLookupPanel.svelte @@ -491,13 +491,14 @@ - + sighted operator was told nothing (4.1.3). This arm draws the + searching arm's sentence and its class, both off the same chooser, + so the two states name the fields the same way. + Not aria-hidden: the paragraph lands inside the panel's own status + region, which speaks it the way it speaks the searching arm, so the + move needs no announcement of its own and the sentence stays in the + accessibility tree instead of being spoken and gone. --> +

{movedEmptied}

{/if}
diff --git a/src/routes/admin/images/[id]/edit/+page.svelte b/src/routes/admin/images/[id]/edit/+page.svelte index aa401a28..03625278 100644 --- a/src/routes/admin/images/[id]/edit/+page.svelte +++ b/src/routes/admin/images/[id]/edit/+page.svelte @@ -474,7 +474,10 @@ extraParents = [...extraParents, { id: clash.imageId, title: clash.title }]; } selectedParentId = String(clash.imageId); + // Idle, and without the cleared record, for the same reason closeLookup + // drops it: the idle arm draws whatever stands there. lookup = { kind: 'idle' }; + lookupCleared = {}; // The click unmounted its own button; land on the select it just set. await tick(); parentSelect?.focus(); @@ -484,6 +487,10 @@ * back to the control the lookup started from. */ function closeLookup() { lookup = { kind: 'idle' }; + // And the record of what that lookup emptied: the panel's idle arm draws + // it, so a record left standing kept the panel open as a bordered card + // holding the sentence after the operator asked for it to go (SONA-220). + lookupCleared = {}; lookupPill?.focus(); } diff --git a/src/routes/admin/upload/+page.svelte b/src/routes/admin/upload/+page.svelte index f1e91bcc..03e1bb3d 100644 --- a/src/routes/admin/upload/+page.svelte +++ b/src/routes/admin/upload/+page.svelte @@ -520,8 +520,8 @@ // was while the operator watched it go blank (4.1.3). let sharedCleared = $state({}); // Whether the operator has text of their own in each field. Each one rises on - // input to its own field, is recomputed against the fields in applyShared when a result - // lands, and lowered in resetSharedPrefill. See sharedEdited. + // input to its own field, is recomputed against the fields in applyShared when + // a result lands, and is lowered in resetSharedPrefill. See sharedEdited. let sourceTypedIn = $state(false); let dateTypedIn = $state(false); // Not $state: nothing renders it. See takePendingCleared. @@ -608,10 +608,10 @@ // LAST result does go now: it is about a search that is over. if (isParent(key)) { // The move's own record goes too. Whatever it describes has already been - // told — by pickParent's announcement when the move landed on an idle - // tile, by the settled arm otherwise — and this search has changed - // nothing yet, so the searching arm about to render must not say it - // again into the panel's atomic status region (4.1.3). The one record + // told — by the panel's idle arm when the move landed on an idle tile, + // by the settled arm otherwise — and this search has changed nothing + // yet, so the searching arm about to render must not say it again into + // the panel's atomic status region (4.1.3). The one record // the searching arm does speak for is a move that landed on a search // still in flight, and no lookup can start on that tile: the guard above // returns on a searching tile rather than restarting it, which is also @@ -1119,24 +1119,17 @@ } /** The Parent radio moved, or the parent tile was removed and the radio - * landed on another one. The panel under it is already mounted and its status - * region carries the sentence for every settled result, and the searching arm - * carries it too, so announcing here on either would say the same thing twice - * — the region is atomic, so the panel re-speaks whole when the sentence - * appears in it. Only a lookup that never ran carries no sentence of its own, - * so the announcement is this move's telling; the panel's idle arm draws the - * same line for sighted operators, aria-hidden so the region does not repeat - * it (4.1.3). + * landed on another one. Says nothing: the panel under it is already mounted + * and its status region carries the sentence in every arm — the settled ones, + * the searching one, and the idle one, which draws what the move emptied — so + * the region is what tells the operator, once, and an announcement here would + * be the same thing twice (4.1.3). * - * Read straight off the tile: both callers run in the new-set mode only — the - * radio renders under `{#if groupMode === 'new'}`, and the removal path checks - * the mode before calling — so there is no other mode to answer for. */ + * Both callers run in the new-set mode only — the radio renders under + * `{#if groupMode === 'new'}`, and the removal path checks the mode before + * calling — so there is no other mode to answer for. */ function pickParent(index: number) { - const { cleared } = onParentChanged(index); - const kind = tiles[index]?.lookup.kind ?? 'idle'; - if (kind !== 'idle') return; - const line = clearedLine(cleared, {}); - if (line) announcer.say(line); + onParentChanged(index); } function useLookupArtist(artist: { id: number; name: string }) { @@ -1208,6 +1201,10 @@ function closeSharedLookup(options: { focus?: boolean } = {}) { const tile = parentTile; if (tile) tile.lookup = { kind: 'idle' }; + // And the record of what that lookup emptied: the idle arm draws it, so a + // record left standing kept the panel open as a bordered card holding the + // sentence after the operator asked for it to go (SONA-220). + sharedCleared = {}; if (options.focus !== false) focusLookupOrigin(); } diff --git a/tests/e2e/artist-lookup.spec.ts b/tests/e2e/artist-lookup.spec.ts index d579a69e..b5b8586a 100644 --- a/tests/e2e/artist-lookup.spec.ts +++ b/tests/e2e/artist-lookup.spec.ts @@ -258,6 +258,25 @@ const THREE_TAG_SUGGESTION = { const panel = (page: Page) => page.getByRole('region', { name: 'Artist lookup' }); +/** The sentence a parent move onto a tile with no lookup of its own leaves on + * screen. The panel's status region draws it and speaks it, so it is one node: + * visible, drawn as a report of what the two fields just did rather than in the + * muted colour the advice lines use, and left in the accessibility tree instead + * of spoken and gone (4.1.3). */ +async function expectMovedEmptiedSentence(page: Page) { + const emptied = panel(page) + .locator('p.lookup-status') + .filter({ + hasText: 'cleared the source post URL and commissioned date the last lookup filled' + }); + await expect(emptied).toBeVisible(); + await expect(emptied).toHaveClass(/lookup-emptied/); + await expect(emptied).not.toHaveAttribute('aria-hidden', /.*/); + // And the panel is the open card, not the collapsed idle one it would be + // with nothing to draw. + await expect(panel(page)).toBeVisible(); +} + test('without a key there is no button, only a pointer at Settings', async ({ page }) => { test.setTimeout(60_000); await adminLogin(page, PASSWORD); @@ -928,11 +947,10 @@ test.describe('with a key saved', () => { await expect(dateInput(page)).toHaveValue(''); await expect(page.locator('#source-lookup-tag')).toHaveCount(0); await expect(page.locator('#commissioned-lookup-tag')).toHaveCount(0); - // And it says so. The new parent has no result, so the panel shows nothing - // about the two fields that just emptied under the operator (4.1.3). - await expect(page.locator(LIVE_REGION)).toContainText( - "Sona cleared the source post URL and commissioned date the last lookup filled." - ); + // And it says so. The new parent has no result, so the panel's idle arm is + // what carries the sentence for the two fields that just emptied under the + // operator (4.1.3). + await expectMovedEmptiedSentence(page); }); test('a clash after a lookup says it emptied the field, not that it was empty', async ({ @@ -2583,9 +2601,10 @@ test.describe('with a key saved', () => { ); }); - // A parent with no lookup of its own draws no sentence at all: the panel under - // it is idle, and the two fields go blank with nothing on screen saying why - // (4.1.3). So the move says it, and says it once. + // A parent with no lookup of its own used to draw no sentence at all: the + // panel under it is idle, and the two fields went blank with nothing on + // screen saying why (4.1.3). The idle arm draws it now, inside the panel's + // own status region, so one node both shows the sentence and speaks it. test('a parent move onto a tile with no lookup says what it emptied', async ({ page }) => { await stubLookup(page, matchedBody()); await twoDoneTiles(page); @@ -2598,9 +2617,10 @@ test.describe('with a key saved', () => { await expect(sourceInput(page)).toHaveValue(''); await expect(dateInput(page)).toHaveValue(''); - await expect(page.locator(LIVE_REGION)).toHaveText( - "Sona cleared the source post URL and commissioned date the last lookup filled." - ); + await expectMovedEmptiedSentence(page); + // And nothing says it a second time: the page's own live region is the + // other place this sentence could come from. + await expect(page.locator(LIVE_REGION)).not.toContainText('the last lookup filled'); }); // And starting that tile's own lookup must not say it again. The searching @@ -2616,9 +2636,7 @@ test.describe('with a key saved', () => { await expect(dateInput(page)).toHaveValue('2026-03-04'); await page.locator('input[name="parentPick"]').nth(1).check(); - const spoken = - "Sona cleared the source post URL and commissioned date the last lookup filled."; - await expect(page.locator(LIVE_REGION)).toHaveText(spoken); + await expectMovedEmptiedSentence(page); // The second tile is the parent now and has never been looked up. Held // open, so the searching arm is what is on screen. @@ -2626,9 +2644,9 @@ test.describe('with a key saved', () => { await tileLookup(page).nth(1).click(); await expect(panel(page)).toContainText('Looking up'); await expect(panel(page)).not.toContainText('the last lookup filled'); - // And the announcer holds the one telling, unchanged: nothing was queued - // behind it, so nothing was said twice. - await expect(page.locator(LIVE_REGION)).toHaveText(spoken); + // And nothing was queued into the page's live region behind it either, so + // the move was never told twice. + await expect(page.locator(LIVE_REGION)).not.toContainText('the last lookup filled'); release(); @@ -3396,6 +3414,29 @@ test.describe('with a key saved', () => { await expect(page.locator('#source-lookup-tag')).toBeVisible(); }); + // Close puts the lookup back to idle, and the idle arm draws whatever that + // lookup emptied. The record left standing, Close left the panel on screen as + // a bordered card holding that sentence, with no way to dismiss it (SONA-220). + test('closing a lookup that emptied the fields collapses the panel', async ({ page }) => { + await stubLookup(page, matchedBody()); + await oneDoneTile(page); + await pill(page).click(); + await expect(sourceInput(page)).toHaveValue(POST_URL); + await expect(dateInput(page)).toHaveValue('2026-03-04'); + + // A no-match is a result with nothing to prefill, so it empties both fields + // and the panel says so. + await stubLookup(page, { enabled: true, matches: [] }); + await pill(page).click(); + await expect(sourceInput(page)).toHaveValue(''); + await expect(panel(page)).toContainText( + 'Sona cleared the source post URL and commissioned date the last lookup filled, because this lookup filled neither one.' + ); + + await panel(page).getByRole('button', { name: 'Close' }).click(); + await expect(panel(page)).toBeHidden(); + }); + // parentIndex is submitted as the hidden field the server picks the parent // with, so a tile removed ahead of the parent must move it along: otherwise // the piece that saves is a different file than the shared artist, date and @@ -3435,9 +3476,8 @@ test.describe('with a key saved', () => { // Removing the parent tile itself routes through the same pickParent the // radio does, so the fields re-derive from the tile the parent pick lands on. - // The tile that lands there has no lookup of its own, so its panel draws no - // sentence and nothing on screen says why the two fields just went blank - // (4.1.3) — the removal says it, once. + // The tile that lands there has no lookup of its own, and the panel's idle arm + // is what says why the two fields just went blank (4.1.3). test('removing the parent tile re-derives the fields and says what it emptied', async ({ page }) => { @@ -3471,12 +3511,10 @@ test.describe('with a key saved', () => { await expect(dateInput(page)).toHaveValue(''); await expect(page.locator('#source-lookup-tag')).toHaveCount(0); await expect(page.locator('#commissioned-lookup-tag')).toHaveCount(0); - // Said once, and by the removal: the panel under an idle lookup carries no - // sentence to read it from. - await expect(page.locator(LIVE_REGION)).toHaveText( - "Sona cleared the source post URL and commissioned date the last lookup filled." - ); - await expect(panel(page)).not.toContainText('cleared the source post URL'); + // Said once, and by the panel: its idle arm draws the sentence and its + // status region speaks it. + await expectMovedEmptiedSentence(page); + await expect(page.locator(LIVE_REGION)).not.toContainText('the last lookup filled'); }); // In the existing-piece mode no tile is the parent: the panel is not rendered From 4bd006d1c43d805004194400cd0f57198354ec57 Mon Sep 17 00:00:00 2001 From: Sparky <1609870+sparkyfen@users.noreply.github.com> Date: Wed, 16 Sep 2026 13:24:48 -0700 Subject: [PATCH 4/7] fix(admin): Cancel drops the lookup's cleared record too, and the edit page's collapse paths get e2e coverage (SONA-220) --- src/lib/artist-lookup-markup.test.ts | 32 +++++++++++--- src/routes/admin/upload/+page.svelte | 7 ++++ tests/e2e/artist-lookup.spec.ts | 62 ++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 5 deletions(-) diff --git a/src/lib/artist-lookup-markup.test.ts b/src/lib/artist-lookup-markup.test.ts index 1d63b42f..175b898b 100644 --- a/src/lib/artist-lookup-markup.test.ts +++ b/src/lib/artist-lookup-markup.test.ts @@ -76,7 +76,11 @@ function matchDelim( else if (source[i] === close && --depth === 0) return i; i++; } - throw new Error(`no closing ${close} for ${what} in source (depth ${depth} at the end)`); + throw new Error( + depth === 0 + ? `no ${open} for ${what} in source` + : `no closing ${close} for ${what} in source (depth ${depth} at the end)` + ); } // A function's body, sliced from its declaration by matching braces. A @@ -149,6 +153,11 @@ describe('the function-body slicer', () => { /no closing \} for sample's body/ ); expect(() => fnBody('const x = 1;\n', 'sample')).toThrow(/no function named sample/); + // And the other way the scan can walk off the end: the opening delimiter + // never arrives at all, which is not a missing closer. + expect(() => fnBody('function sample()\n', 'sample')).toThrow( + /no \{ for sample's body in source/ + ); }); // The case the slicer exists for: a multi-line return type annotation is a @@ -736,9 +745,13 @@ describe('the "From lookup" tag', () => { // the way Close does, leaving the fields as it found them, so it is // sliced away for the same reason. const closer = source === UPLOAD ? 'closeSharedLookup' : 'closeLookup'; + // Cancel is sliced away for the same reason as Close: it leaves both + // fields exactly as it found them and only drops a sentence the page + // has already spoken. const counted = source .replace(fnBody(source, 'startLookup'), '') .replace(fnBody(source, closer), '') + .replace(fnBody(source, 'cancelLookup'), '') .replace(fnBody(source, 'addAsVariant'), ''); const resets = counted.match(new RegExp(`${clearedRecord} = \\{\\};`, 'g')) ?? []; const withLatches = @@ -1225,14 +1238,15 @@ describe('what the lookup copy names', () => { // And nothing else writes sharedCleared. The catch that synthesises a // failure used to assign the held record straight, which skipped the check // above and had the panel report a URL the operator had typed back in. - // Six assignments in the file and no more: the declaration, three blankings - // that hand over no record — startLookup's, the reset's and Close's — and - // applyShared's two. Everything that DESCRIBES a clearing goes through - // applyShared. + // Seven assignments in the file and no more: the declaration, four + // blankings that hand over no record — startLookup's, Cancel's, the + // reset's and Close's — and applyShared's two. Everything that DESCRIBES a + // clearing goes through applyShared. expect(UPLOAD.match(/sharedCleared = [^;]*/g) ?? []).toEqual([ 'sharedCleared = $state({})', 'sharedCleared = {}', 'sharedCleared = {}', + 'sharedCleared = {}', 'sharedCleared = { ...emptied }', 'sharedCleared = cleared', 'sharedCleared = {}' @@ -1521,6 +1535,14 @@ describe('focus after the panel goes away', () => { it('drops the cleared record when the panel is closed', () => { expect(fnBody(UPLOAD, 'closeSharedLookup')).toContain('sharedCleared = {};'); expect(fnBody(EDIT, 'closeLookup')).toContain('lookupCleared = {};'); + // Cancel ends the same way — idle, with the panel gone — so it drops the + // record too. The upload page's cancelLookup goes idle on its own rather + // than through Close, and left the record standing: a parent move that + // emptied the fields mid-search then kept the panel on screen as a + // bordered card with no Close button after Cancel (SONA-220). The edit + // page's cancelLookup routes through closeLookup, covered above. + expect(fnBody(UPLOAD, 'cancelLookup')).toContain('sharedCleared = {};'); + expect(fnBody(EDIT, 'cancelLookup')).toContain('closeLookup();'); // The edit page's "Add as a variant" goes idle on its own rather than // through closeLookup, so it drops the record itself; the upload page's // goes through closeSharedLookup. diff --git a/src/routes/admin/upload/+page.svelte b/src/routes/admin/upload/+page.svelte index 03e1bb3d..b24f8ecd 100644 --- a/src/routes/admin/upload/+page.svelte +++ b/src/routes/admin/upload/+page.svelte @@ -862,6 +862,13 @@ if (pendingCleared?.key === key) pendingCleared = null; const tile = tiles.find((t) => t.key === key); if (tile) tile.lookup = { kind: 'idle' }; + // And the record a parent move parked for the search being cancelled, the + // way closeSharedLookup drops it: the idle arm draws that record too, so a + // record left standing kept the panel open as a bordered card holding the + // sentence after the operator cancelled, and the atomic status region + // spoke it again as the arm changed (SONA-220). The fields are left + // exactly as they are found, so the typed-in latches stay where they are. + sharedCleared = {}; } /** Undo what a previous shared prefill wrote, but only where the operator has diff --git a/tests/e2e/artist-lookup.spec.ts b/tests/e2e/artist-lookup.spec.ts index b5b8586a..fd80d280 100644 --- a/tests/e2e/artist-lookup.spec.ts +++ b/tests/e2e/artist-lookup.spec.ts @@ -3437,6 +3437,68 @@ test.describe('with a key saved', () => { await expect(panel(page)).toBeHidden(); }); + // Cancel ends the same way, and the searching arm it ends is the one arm that + // draws a parent move's record. Left standing, Cancel dropped the panel to + // idle with that sentence still in it — a bordered card with no Close button + // on it, and the atomic status region spoke the sentence again as the arm + // changed (SONA-220). + test('cancelling a lookup that emptied the fields collapses the panel', async ({ page }) => { + await stubLookup(page, matchedBody()); + await twoDoneTiles(page); + + await tileLookup(page).nth(0).click(); + await expect(sourceInput(page)).toHaveValue(POST_URL); + await expect(dateInput(page)).toHaveValue('2026-03-04'); + + // The parent moves onto the second tile while its search is still out, so + // the move empties both fields and the searching arm says so. + const release = await deferredLookup(page, matchedBody()); + await tileLookup(page).nth(1).click(); + await expect(tileLookup(page).nth(1)).toHaveAttribute('aria-busy', 'true'); + + await page.locator('input[name="parentPick"]').nth(1).check(); + await expect(sourceInput(page)).toHaveValue(''); + await expect(dateInput(page)).toHaveValue(''); + const moved = 'Sona cleared the source post URL and commissioned date the last lookup filled.'; + await expect(panel(page)).toContainText(moved); + + await panel(page).getByRole('button', { name: 'Cancel lookup' }).click(); + release(); + await expect(panel(page)).toBeHidden(); + await expect(page.locator('body')).not.toContainText(moved); + }); + + // The edit page's two drops, which only source assertions covered: Close and + // "Add as a variant" both go idle with a record standing, and the idle arm + // draws it there too (SONA-220). + test('the edit page collapses the panel on Close too', async ({ page }) => { + await stubLookup(page, xMatchBody()); + await gotoEditHydrated(page); + await aNoMatchEmptiesWhatTheLastLookupFilled(page); + + await panel(page).getByRole('button', { name: 'Close' }).click(); + await expect(panel(page)).toBeHidden(); + }); + + test('the edit page collapses the panel when the clash becomes the parent', async ({ page }) => { + await stubLookup(page, xMatchBody()); + await gotoEditHydrated(page); + + await pill(page).click(); + await expect(sourceInput(page)).toHaveValue(X_POST); + + // The same post, now claimed by another piece: the prefill puts no URL + // back, so the field the first lookup filled empties and the panel says so. + await stubLookup(page, { ...xMatchBody(), sourceClash: sourceClash(9001, 'Clash Piece') }); + await pill(page).click(); + await expect(sourceInput(page)).toHaveValue(''); + await expect(panel(page)).toContainText('cleared the source post URL the last lookup filled'); + + await panel(page).getByRole('button', { name: 'Add as a variant' }).click(); + await expect(page.locator('select[name="parentImageId"]')).toHaveValue('9001'); + await expect(panel(page)).toBeHidden(); + }); + // parentIndex is submitted as the hidden field the server picks the parent // with, so a tile removed ahead of the parent must move it along: otherwise // the piece that saves is a different file than the shared artist, date and From 50e8b649e1c08e93efb7d619de34d941a1f4d7cc Mon Sep 17 00:00:00 2001 From: Sparky <1609870+sparkyfen@users.noreply.github.com> Date: Wed, 16 Sep 2026 13:33:11 -0700 Subject: [PATCH 5/7] docs(admin): tighten two comments on the cancel record drop (SONA-220) --- src/lib/artist-lookup-markup.test.ts | 4 +--- src/routes/admin/upload/+page.svelte | 8 ++++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/lib/artist-lookup-markup.test.ts b/src/lib/artist-lookup-markup.test.ts index 175b898b..fb9ad5d6 100644 --- a/src/lib/artist-lookup-markup.test.ts +++ b/src/lib/artist-lookup-markup.test.ts @@ -745,9 +745,7 @@ describe('the "From lookup" tag', () => { // the way Close does, leaving the fields as it found them, so it is // sliced away for the same reason. const closer = source === UPLOAD ? 'closeSharedLookup' : 'closeLookup'; - // Cancel is sliced away for the same reason as Close: it leaves both - // fields exactly as it found them and only drops a sentence the page - // has already spoken. + // Cancel too: it only drops a sentence the page has already spoken. const counted = source .replace(fnBody(source, 'startLookup'), '') .replace(fnBody(source, closer), '') diff --git a/src/routes/admin/upload/+page.svelte b/src/routes/admin/upload/+page.svelte index b24f8ecd..f42302f0 100644 --- a/src/routes/admin/upload/+page.svelte +++ b/src/routes/admin/upload/+page.svelte @@ -863,11 +863,11 @@ const tile = tiles.find((t) => t.key === key); if (tile) tile.lookup = { kind: 'idle' }; // And the record a parent move parked for the search being cancelled, the - // way closeSharedLookup drops it: the idle arm draws that record too, so a + // way closeSharedLookup drops it. The idle arm draws that record too, so a // record left standing kept the panel open as a bordered card holding the - // sentence after the operator cancelled, and the atomic status region - // spoke it again as the arm changed (SONA-220). The fields are left - // exactly as they are found, so the typed-in latches stay where they are. + // sentence after the operator cancelled, and the atomic status region spoke + // it again as the arm changed (SONA-220). This leaves the fields exactly as + // it found them, so the typed-in latches stay where they are. sharedCleared = {}; } From bc30dea17eebe32cb64b6b362a0e2143cd1c6af8 Mon Sep 17 00:00:00 2001 From: Sparky <1609870+sparkyfen@users.noreply.github.com> Date: Wed, 16 Sep 2026 13:38:02 -0700 Subject: [PATCH 6/7] fix(admin): only the parent's cancel drops the cleared record, and note nested templates in the scanner caveat (SONA-220) --- src/lib/artist-lookup-markup.test.ts | 5 +++-- src/routes/admin/upload/+page.svelte | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/lib/artist-lookup-markup.test.ts b/src/lib/artist-lookup-markup.test.ts index fb9ad5d6..3ae42bb1 100644 --- a/src/lib/artist-lookup-markup.test.ts +++ b/src/lib/artist-lookup-markup.test.ts @@ -45,8 +45,9 @@ function endOfString(source: string, i: number): number { // closer never arrives, so a scan that walks off the end says so instead of // returning a slice that stops wherever the source did. // Regex literals are NOT tracked: a `/[{}]/` inside a scanned body would move -// the depth and a `/won't/` would open a string that never closes. No function -// this file slices has one. +// the depth and a `/won't/` would open a string that never closes. Nor are +// template literals nested inside a `${}` expression: the scan ends at the +// next backtick. No function this file slices has either. function matchDelim( source: string, from: number, diff --git a/src/routes/admin/upload/+page.svelte b/src/routes/admin/upload/+page.svelte index f42302f0..42a2ce0a 100644 --- a/src/routes/admin/upload/+page.svelte +++ b/src/routes/admin/upload/+page.svelte @@ -867,8 +867,9 @@ // record left standing kept the panel open as a bordered card holding the // sentence after the operator cancelled, and the atomic status region spoke // it again as the arm changed (SONA-220). This leaves the fields exactly as - // it found them, so the typed-in latches stay where they are. - sharedCleared = {}; + // it found them, so the typed-in latches stay where they are. The record + // only ever describes the parent, so only the parent's cancel drops it. + if (isParent(key)) sharedCleared = {}; } /** Undo what a previous shared prefill wrote, but only where the operator has From 692847d21f6f6aa7a2b839bc6bb4d80dbc3fe01f Mon Sep 17 00:00:00 2001 From: Sparky <1609870+sparkyfen@users.noreply.github.com> Date: Wed, 16 Sep 2026 13:57:21 -0700 Subject: [PATCH 7/7] fix(admin): the idle cleared card gets a Close button, and Cancel keeps the record that explains the blank fields (SONA-220) Cancel ends the search but the two shared fields stay blank, and the cleared record is the only account of why, so it stays and the panel falls to its idle arm. That arm now renders Close alone, on the same handler the settled arms use, which drops the record and collapses the card. The parent-move card had no dismissal before; now it does, and an e2e drives both paths. --- src/lib/artist-lookup-markup.test.ts | 45 ++++++++++++------ src/lib/components/ArtistLookupPanel.svelte | 20 ++++++-- src/routes/admin/upload/+page.svelte | 15 +++--- tests/e2e/artist-lookup.spec.ts | 52 ++++++++++++++++++--- 4 files changed, 101 insertions(+), 31 deletions(-) diff --git a/src/lib/artist-lookup-markup.test.ts b/src/lib/artist-lookup-markup.test.ts index 3ae42bb1..0d56dbc5 100644 --- a/src/lib/artist-lookup-markup.test.ts +++ b/src/lib/artist-lookup-markup.test.ts @@ -413,6 +413,20 @@ describe('the panel', () => { expect(fnBody(UPLOAD, 'pickParent')).not.toContain('announcer.say('); }); + // The same card with no way out of it: the actions row was gated on a lookup + // that was not idle, so this arm drew a bordered card holding the sentence + // and nothing to dismiss it with, while every other arm that draws that + // sentence offers Close or Cancel (SONA-220). + it('gives the idle cleared card a Close button', () => { + expect(PANEL).toMatch(/\{#if lookup\.kind !== 'idle' \|\| movedEmptied\}\s*
/); + // Close alone — nothing is running and nothing has settled, so there is + // nothing to retry, apply or cancel — and on the same onclose the settled + // arms use, which drops the record and collapses the panel. + expect(PANEL).toMatch( + /\{#if lookup\.kind === 'idle'\}(?:\s|)*
- {#if lookup.kind !== 'idle'} + {#if lookup.kind !== 'idle' || movedEmptied}
+ + {:else if lookup.kind === 'searching'} diff --git a/src/routes/admin/upload/+page.svelte b/src/routes/admin/upload/+page.svelte index 42a2ce0a..e89ef8af 100644 --- a/src/routes/admin/upload/+page.svelte +++ b/src/routes/admin/upload/+page.svelte @@ -862,14 +862,13 @@ if (pendingCleared?.key === key) pendingCleared = null; const tile = tiles.find((t) => t.key === key); if (tile) tile.lookup = { kind: 'idle' }; - // And the record a parent move parked for the search being cancelled, the - // way closeSharedLookup drops it. The idle arm draws that record too, so a - // record left standing kept the panel open as a bordered card holding the - // sentence after the operator cancelled, and the atomic status region spoke - // it again as the arm changed (SONA-220). This leaves the fields exactly as - // it found them, so the typed-in latches stay where they are. The record - // only ever describes the parent, so only the parent's cancel drops it. - if (isParent(key)) sharedCleared = {}; + // `sharedCleared` is NOT dropped here. Cancelling ends the search, but the + // two shared fields stay blank, and that record is the only account of why + // they are (4.1.3): dropped, the panel collapsed and the sentence went with + // it, leaving the operator with two empty fields and no reason anywhere on + // screen. The panel goes to its idle arm instead, which draws the sentence + // and a Close button — and Close is what drops the record and collapses the + // card (SONA-220). } /** Undo what a previous shared prefill wrote, but only where the operator has diff --git a/tests/e2e/artist-lookup.spec.ts b/tests/e2e/artist-lookup.spec.ts index fd80d280..5d7d7dc3 100644 --- a/tests/e2e/artist-lookup.spec.ts +++ b/tests/e2e/artist-lookup.spec.ts @@ -3437,12 +3437,15 @@ test.describe('with a key saved', () => { await expect(panel(page)).toBeHidden(); }); - // Cancel ends the same way, and the searching arm it ends is the one arm that - // draws a parent move's record. Left standing, Cancel dropped the panel to - // idle with that sentence still in it — a bordered card with no Close button - // on it, and the atomic status region spoke the sentence again as the arm - // changed (SONA-220). - test('cancelling a lookup that emptied the fields collapses the panel', async ({ page }) => { + // Cancel does NOT end that way. The searching arm it ends is the one arm that + // draws a parent move's record, and the two fields the move emptied are still + // blank after the cancel: dropping the record collapsed the panel and took + // the only reason for those blank fields with it (4.1.3). The panel falls to + // its idle arm holding the sentence, and Close is the way out of it + // (SONA-220). + test('cancelling a lookup that emptied the fields keeps the sentence until Close', async ({ + page + }) => { await stubLookup(page, matchedBody()); await twoDoneTiles(page); @@ -3464,10 +3467,47 @@ test.describe('with a key saved', () => { await panel(page).getByRole('button', { name: 'Cancel lookup' }).click(); release(); + + // The idle arm: the sentence still on screen over the two fields it + // describes, with Close as its one action and nothing left to cancel. + await expectMovedEmptiedSentence(page); + await expect(panel(page).getByRole('button', { name: 'Cancel lookup' })).toHaveCount(0); + const close = panel(page).getByRole('button', { name: 'Close' }); + await expect(close).toBeVisible(); + // And the page's own live region does not say it a second time: the panel's + // status region is where this sentence lives. + await expect(page.locator(LIVE_REGION)).not.toContainText('the last lookup filled'); + + await close.click(); await expect(panel(page)).toBeHidden(); await expect(page.locator('body')).not.toContainText(moved); }); + // The other way into that idle card, and the one with no test on it: a parent + // move onto a tile that was never looked up. No lookup runs, no result lands, + // so the card the sentence sits in has only its own Close to go by — without + // one the operator's ways out were typing into a field or starting another + // lookup (SONA-220). + test('the idle card a parent move leaves closes on its own Close', async ({ page }) => { + await stubLookup(page, matchedBody()); + await twoDoneTiles(page); + + await tileLookup(page).nth(0).click(); + await expect(sourceInput(page)).toHaveValue(POST_URL); + await expect(dateInput(page)).toHaveValue('2026-03-04'); + + // The second tile has never been looked up, so the move empties both fields + // with no result on its way and the idle arm is what says so. + await page.locator('input[name="parentPick"]').nth(1).check(); + await expect(sourceInput(page)).toHaveValue(''); + await expect(dateInput(page)).toHaveValue(''); + await expectMovedEmptiedSentence(page); + + await panel(page).getByRole('button', { name: 'Close' }).click(); + await expect(panel(page)).toBeHidden(); + await expect(page.locator('body')).not.toContainText('the last lookup filled'); + }); + // The edit page's two drops, which only source assertions covered: Close and // "Add as a variant" both go idle with a record standing, and the idle arm // draws it there too (SONA-220).