From 460f39298b366c3ae958af1a4d57c407f454732f Mon Sep 17 00:00:00 2001 From: Frederic Cabassut Date: Wed, 15 Jul 2026 09:55:26 +0200 Subject: [PATCH] fix(search): preserve unsynced hits and avoid empty-state flash Address review feedback on the search-results preservation change: - reconcileSearchResults now keeps a result absent from the canonical notes store (a server hit not yet loaded locally, e.g. from another device) instead of dropping it, so valid matches no longer vanish mid-session. Locally deleted/left notes are still pruned at their call sites; trashed notes are still dropped. - SearchBar commits searchQuery only once its results arrive, so the view no longer flashes an empty state over the current notes while a request is in flight, and searchQuery stays consistent with searchResults on a failed request. - Expand reconcile unit tests (keep-absent, trashed-drop, mixed order, empty) and add an e2e test asserting notes stay visible during an in-flight search. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/lib/components/SearchBar.svelte | 8 ++++- src/lib/stores/notes.test.ts | 48 ++++++++++++++++++++++++----- src/lib/stores/notes.ts | 18 +++++++++-- tests/e2e/search.spec.ts | 28 +++++++++++++++++ 4 files changed, 90 insertions(+), 12 deletions(-) diff --git a/src/lib/components/SearchBar.svelte b/src/lib/components/SearchBar.svelte index fdbcf18..9e0d8db 100644 --- a/src/lib/components/SearchBar.svelte +++ b/src/lib/components/SearchBar.svelte @@ -31,9 +31,9 @@ searchController = undefined; const trimmedQuery = query.trim(); - searchQuery.set(trimmedQuery); if (!trimmedQuery) { + searchQuery.set(''); searchResults.set([]); return; } @@ -48,7 +48,13 @@ if (res.ok) { const results: Note[] = await res.json(); if (searchController !== controller || query.trim() !== trimmedQuery) return; + // Commit the query alongside its results so the view switches to the + // (possibly empty) result set only once it's ready — this avoids + // flashing an empty state over the current notes while the request is + // in flight, and keeps searchQuery consistent with searchResults on + // a failed request (neither updates). searchResults.set(results); + searchQuery.set(trimmedQuery); } } catch (error) { if (!(error instanceof DOMException && error.name === 'AbortError')) { diff --git a/src/lib/stores/notes.test.ts b/src/lib/stores/notes.test.ts index f7e26f8..21a75ca 100644 --- a/src/lib/stores/notes.test.ts +++ b/src/lib/stores/notes.test.ts @@ -99,15 +99,47 @@ describe('mergeNotesByVersion', () => { }); describe('reconcileSearchResults', () => { - it('updates existing results and removes notes absent from canonical state', () => { - const updated = makeNote({ id: 'n1', title: 'Updated', version: 2 }); - const removed = makeNote({ id: 'n2', title: 'Removed' }); + it('replaces a stale result with the canonical note', () => { + const canonical = makeNote({ id: 'n1', title: 'Updated', version: 2 }); - const result = reconcileSearchResults([updated], [ - makeNote({ id: 'n1', title: 'Stale', version: 1 }), - removed - ]); + const result = reconcileSearchResults( + [canonical], + [makeNote({ id: 'n1', title: 'Stale', version: 1 })] + ); - expect(result).toEqual([updated]); + expect(result).toEqual([canonical]); + }); + + it('keeps a result absent from the canonical store (hit not yet loaded locally)', () => { + const serverHit = makeNote({ id: 'n2', title: 'From another device' }); + + const result = reconcileSearchResults([makeNote({ id: 'n1' })], [serverHit]); + + expect(result).toEqual([serverHit]); + }); + + it('drops a result that is trashed in the canonical store', () => { + const trashed = makeNote({ id: 'n1', trashed: true }); + + const result = reconcileSearchResults([trashed], [makeNote({ id: 'n1', trashed: false })]); + + expect(result).toEqual([]); + }); + + it('preserves result order while reconciling a mix of cases', () => { + const canonical = makeNote({ id: 'n1', title: 'Canonical', version: 2 }); + const trashed = makeNote({ id: 'n2', trashed: true }); + const absent = makeNote({ id: 'n3', title: 'Absent' }); + + const result = reconcileSearchResults( + [canonical, trashed], + [makeNote({ id: 'n1', title: 'Stale', version: 1 }), trashed, absent] + ); + + expect(result).toEqual([canonical, absent]); + }); + + it('returns an empty array when there are no results', () => { + expect(reconcileSearchResults([makeNote({ id: 'n1' })], [])).toEqual([]); }); }); diff --git a/src/lib/stores/notes.ts b/src/lib/stores/notes.ts index 60807c2..733b233 100644 --- a/src/lib/stores/notes.ts +++ b/src/lib/stores/notes.ts @@ -11,15 +11,27 @@ export const notesLoaded = writable(false); export const searchQuery = writable(''); export const searchResults = writable([]); +/** + * Reconcile search results against the canonical notes store so local edits and + * trashing propagate into the active result set without re-querying the server. + * + * A result absent from the canonical store is kept as-is — it's a server hit not + * yet loaded locally (e.g. created on another device), and dropping it would make + * valid matches disappear mid-session. Notes removed locally (deleted or left) are + * pruned explicitly at their call sites; trashed notes are dropped here. + */ export function reconcileSearchResults(canonicalNotes: Note[], results: Note[]): Note[] { const notesMap = new Map(canonicalNotes.map((note) => [note.id, note])); return results.flatMap((result) => { - const updated = notesMap.get(result.id); - return updated && !updated.trashed ? [updated] : []; + const canonical = notesMap.get(result.id); + if (!canonical) return [result]; + return canonical.trashed ? [] : [canonical]; }); } -// Keep searchResults in sync with note updates/deletions in notes store +// Keep searchResults in sync with edits/trashing in the notes store. These search +// stores (like the others in this module) are client-only state; the subscription +// lives for the app's lifetime and is intentionally never unsubscribed. notes.subscribe(($notes) => { searchResults.update(($results) => { if ($results.length === 0) return $results; diff --git a/tests/e2e/search.spec.ts b/tests/e2e/search.spec.ts index 949999a..741569f 100644 --- a/tests/e2e/search.spec.ts +++ b/tests/e2e/search.spec.ts @@ -83,4 +83,32 @@ test.describe('Search', () => { await expect(page.getByText('Race Fast 3812925')).toBeVisible(); await expect(page.getByText('Race Slow 3812925')).not.toBeVisible(); }); + + test('Scenario: Current notes stay visible while a search request is in flight', async ({ authenticatedPage: page }) => { + // Given two notes exist + await createNote(page, 'Flash Alpha 771903'); + await createNote(page, 'Flash Beta 771903'); + + // And the search request is held open before it can respond + let releaseSearch: () => void = () => {}; + const searchGate = new Promise((resolve) => { + releaseSearch = resolve; + }); + await page.route('**/api/search?*', async (route) => { + await searchGate; + await route.continue(); + }); + + // When the user starts a search + await page.getByTestId('search-input').fill('Flash Alpha 771903'); + + // Then the current notes stay on screen instead of flashing an empty state + await expect(page.getByText('Flash Beta 771903')).toBeVisible(); + await expect(page.getByText('Flash Alpha 771903')).toBeVisible(); + + // And once the results arrive, only the match remains + releaseSearch(); + await expect(page.getByText('Flash Alpha 771903')).toBeVisible(); + await expect(page.getByText('Flash Beta 771903')).not.toBeVisible(); + }); });