From 3cff8a9128e8c251db36c689a0cd1ceef6fb6102 Mon Sep 17 00:00:00 2001 From: Peter Ringelmann Date: Fri, 7 Aug 2026 08:19:00 +0200 Subject: [PATCH] feat(core): reduce unifieid search reveal interval Signed-off-by: Peter Ringelmann --- core/src/services/UnifiedSearchController.ts | 58 ++++++-------- .../services/UnifiedSearchController.spec.ts | 75 ++++++++----------- 2 files changed, 51 insertions(+), 82 deletions(-) diff --git a/core/src/services/UnifiedSearchController.ts b/core/src/services/UnifiedSearchController.ts index 9c1db7b735e..05e9b28d864 100644 --- a/core/src/services/UnifiedSearchController.ts +++ b/core/src/services/UnifiedSearchController.ts @@ -23,7 +23,7 @@ export interface CategorySearchParams { extraQueries?: object } -export const REVEAL_INTERVAL_MS = 1500 +export const REVEAL_INTERVAL_MS = 1000 /** * Results fetched per category per page. Sized for the detail view (which shows the @@ -32,9 +32,10 @@ export const REVEAL_INTERVAL_MS = 1500 export const PAGE_SIZE = 10 /** - * Whether a category has anything for the user to look at. Blocked is deliberately - * withheld, failed carries no entries, and a loading category keeps its previous page up - * (stale-while-revalidate) so it stays visible through a refetch. + * Whether a category has anything for the user to look at. Blocked is deliberately withheld + * and failed carries no entries. Loading counts because paging keeps the pages already + * fetched on screen while the next one is in flight; a new query has no entries to show, so + * it reads as not visible until results actually land. * * Exported so the one definition also serves the Vue-side test doubles; the controller is * the only place that decides category-level visibility. @@ -74,17 +75,12 @@ export class UnifiedSearchController { */ async search(query: string, categories: string[], params?: Record): Promise { this.cancelPendingRequests() - // Stale-while-revalidate: keep the previous page on screen while the new search is in - // flight, so refining a query swaps results in place instead of flashing an empty panel. - // Each recurring category is reseeded with its prior entries below; dropped ones vanish. - const previous = this.searchStates + // A new query hides everything the last one produced. Carrying results over would only + // let them shift under the user once the real ones land, and the results are about to + // differ anyway. So each search is a clean slate: empty screen, then a fresh ordered + // reveal from priority order. Nothing is on screen, so nothing can be displaced. this.searchStates = {} - // Prune rather than clear: survivors keep the slots they already hold, so refining a query - // never re-sorts rendered results back to priority order. A category the new search - // dropped is reseeded invisible if it ever returns, so it re-enters at the bottom. This - // cannot cover the window while the states below are still being reseeded one at a time; - // getRevealOrder() does that. - this.revealOrder = this.revealOrder.filter((category) => categories.includes(category)) + this.revealOrder = [] this.searchGeneration++ const generation = this.searchGeneration this.query = query @@ -92,14 +88,7 @@ export class UnifiedSearchController { this.startRevealTimer() - await Promise.allSettled(categories.map((category) => { - const prev = previous[category] - // Only entries that were actually on screen seed the stale view. A blocked or failed - // category's entries were fetched but never rendered, so they must not carry over - // (and must not let the category skip the ordered reveal). - const staleEntries = prev && isCategoryVisible(prev) ? prev.entries : [] - return this.searchCategory(category, generation, categories, staleEntries) - })) + await Promise.allSettled(categories.map((category) => this.searchCategory(category, generation, categories))) } /** @@ -165,17 +154,18 @@ export class UnifiedSearchController { /** * The ids of the categories currently on screen, in display order. * - * Append-only, so a category never moves up into a slot another one already occupies: a - * result that arrives late renders below what the user is already reading, however high - * its priority. Read this rather than the snapshot's key order, which is the priority - * order and an input to blocking, not a rendering order. + * Append-only within a search, so a category never moves up into a slot another one already + * occupies: a result that arrives late renders below what the user is already reading, + * however high its priority. A new query starts over from priority order, since it clears + * the screen first and so has nothing to displace. Read this rather than the snapshot's key + * order, which is the priority order and an input to blocking, not a rendering order. * - * Every id is indexable in the same snapshot, so a caller can map without guarding. + * Only ever names categories the current snapshot holds, so a caller can map without guarding. * * @return visible category ids, top to bottom */ getRevealOrder(): string[] { - return this.revealOrder.filter((category) => category in this.searchStates) + return [...this.revealOrder] } dispose(): void { @@ -196,13 +186,10 @@ export class UnifiedSearchController { category: string, generation: number, categories: string[], - staleEntries: unknown[] = [], ): Promise { - // Seed with the prior page (stale-while-revalidate) so it stays visible under the - // spinner until the fresh page replaces it. Empty on a first search. this.patchStates({ [category]: { status: 'loading', - entries: staleEntries, + entries: [], cursor: null, hasMore: false, loadMoreFailed: false, @@ -227,12 +214,9 @@ export class UnifiedSearchController { const { entries, cursor, isPaginated } = response.data.ocs.data // Decide blocked vs loaded once, here at settle. Reconcile only promotes after this - // (never re-blocks), so this is the only place a category becomes blocked. A category - // that carried stale results skips blocking: it is already on screen, so blocking it - // would blink it off until its predecessors clear. Ordered reveal is only for the - // first paint, when nothing is shown yet. + // (never re-blocks), so this is the only place a category becomes blocked. this.patchStates({ [category]: { - status: (staleEntries.length === 0 && this.shouldBlockCategory(category, categories)) ? 'blocked' : 'loaded', + status: this.shouldBlockCategory(category, categories) ? 'blocked' : 'loaded', entries, cursor, hasMore: this.hasMorePages(isPaginated, cursor), diff --git a/core/src/tests/services/UnifiedSearchController.spec.ts b/core/src/tests/services/UnifiedSearchController.spec.ts index 856097dff64..2c4af7361f4 100644 --- a/core/src/tests/services/UnifiedSearchController.spec.ts +++ b/core/src/tests/services/UnifiedSearchController.spec.ts @@ -315,30 +315,29 @@ describe('UnifiedSearchController', () => { expect(searchController.getRevealOrder()).toEqual(['talk', 'deck']) }) - it('keeps reveal positions across a refined query', async () => { + it('restarts the reveal order from priority on a new query', async () => { const first = mockProviders(['files', 'talk']) const searchController = new UnifiedSearchController() searchController.search('old', ['files', 'talk']) - // talk gets on screen first, so the session order is talk before files. + // talk got on screen first, so this query renders talk above files. first.talk.resolve(['Old talk']) await vi.advanceTimersByTimeAsync(REVEAL_INTERVAL_MS) first.files.resolve(['Old files']) await vi.advanceTimersByTimeAsync(0) expect(searchController.getRevealOrder()).toEqual(['talk', 'files']) - // Refining must not re-sort what is already on screen back to priority order. - // Both categories stay rendered throughout (stale-while-revalidate), so moving - // them would be a displacement with identical content. + // A new query hides everything: the results are about to be different, so there is + // nothing on screen to protect and the next paint starts from priority order again. const second = mockProviders(['files', 'talk']) searchController.search('new', ['files', 'talk']) - expect(searchController.getRevealOrder()).toEqual(['talk', 'files']) + expect(searchController.getRevealOrder()).toEqual([]) second.files.resolve(['New files']) second.talk.resolve(['New talk']) await vi.advanceTimersByTimeAsync(0) - expect(searchController.getRevealOrder()).toEqual(['talk', 'files']) + expect(searchController.getRevealOrder()).toEqual(['files', 'talk']) }) it('releases a slot when a category loses its results, and appends it again if it returns', async () => { @@ -360,17 +359,17 @@ describe('UnifiedSearchController', () => { await vi.advanceTimersByTimeAsync(0) expect(searchController.getRevealOrder()).toEqual(['talk']) - // It has results again on the next query, so it comes back as a fresh reveal: - // at the end, not back at its old priority slot. + // The next query is a clean slate, so it comes back in preferred order rather than + // staying demoted for the rest of the session. const third = mockProviders(['files', 'talk']) searchController.search('c', ['files', 'talk']) third.files.resolve(['Files c']) third.talk.resolve(['Talk c']) await vi.advanceTimersByTimeAsync(0) - expect(searchController.getRevealOrder()).toEqual(['talk', 'files']) + expect(searchController.getRevealOrder()).toEqual(['files', 'talk']) }) - it('re-appends a category that left the search entirely instead of reclaiming its old slot', async () => { + it('recovers preferred order after a provider filter round trip', async () => { const first = mockProviders(['files', 'talk']) const searchController = new UnifiedSearchController() @@ -380,24 +379,22 @@ describe('UnifiedSearchController', () => { await vi.advanceTimersByTimeAsync(0) expect(searchController.getRevealOrder()).toEqual(['files', 'talk']) - // A provider filter narrows the search: files leaves the category list altogether, - // which is a different exit from losing its results (that one goes through - // syncRevealOrder; this one goes through the prune in search()). + // A provider filter narrows the search: files leaves the category list altogether. const second = mockProviders(['talk']) searchController.search('foo', ['talk']) second.talk.resolve(['Talk result']) await vi.advanceTimersByTimeAsync(0) expect(searchController.getRevealOrder()).toEqual(['talk']) - // The filter comes off. talk never left the screen, so files has to come back below - // it: reclaiming slot 0 would shove a rendered group down. + // The filter comes off. Each search stands on its own, so files is back on top + // instead of being stuck below talk until the popover closes. const third = mockProviders(['files', 'talk']) searchController.search('foo', ['files', 'talk']) third.files.resolve(['Files result']) third.talk.resolve(['Talk result']) await vi.advanceTimersByTimeAsync(0) - expect(searchController.getRevealOrder()).toEqual(['talk', 'files']) + expect(searchController.getRevealOrder()).toEqual(['files', 'talk']) }) it('never hands out a category the snapshot cannot index, part-way through a search', async () => { @@ -443,21 +440,20 @@ describe('UnifiedSearchController', () => { first.talk.resolve(['Talk result']) await vi.advanceTimersByTimeAsync(0) - // A narrower search replaces the first. The dropped categories must - // not linger in the snapshot, nor in the display order: the view maps the - // order straight onto the snapshot and would hit a missing category. + // A narrower search replaces the first. The dropped categories must not linger in + // the snapshot, and nothing from the previous query stays on screen. mockProviders(['files']) searchController.search('second', ['files']) expect(searchController.getSnapshot()).toEqual({ - files: { status: 'loading', entries: ['Files result'], cursor: null, hasMore: false, loadMoreFailed: false }, + files: loading, }) - expect(searchController.getRevealOrder()).toEqual(['files']) + expect(searchController.getRevealOrder()).toEqual([]) }) }) - describe('stale-while-revalidate', () => { - it('keeps the previous results visible while a refetch is in flight', async () => { + describe('changing the query', () => { + it('drops the previous results as soon as the query changes', async () => { const first = mockProviders(['files']) const searchController = new UnifiedSearchController() @@ -465,19 +461,18 @@ describe('UnifiedSearchController', () => { first.files.resolve(['Old result']) await vi.advanceTimersByTimeAsync(0) - // A refined query starts a new search. The prior entries must stay on screen - // (status loading, entries kept) so the panel does not flash empty mid-request. + // The new query is about to return different results, so keeping the old ones up + // would only let them shift under the user once the real ones land. Hide, then show. const second = mockProviders(['files']) searchController.search('new', ['files']) expect(searchController.getSnapshot().files).toEqual({ status: 'loading', - entries: ['Old result'], + entries: [], cursor: null, hasMore: false, loadMoreFailed: false, }) - // The fresh page replaces them once it lands. second.files.resolve(['New result']) await vi.advanceTimersByTimeAsync(0) expect(searchController.getSnapshot().files).toEqual({ @@ -489,7 +484,7 @@ describe('UnifiedSearchController', () => { }) }) - it('settles a refetched category that carried results straight to loaded, never blocked', async () => { + it('puts every category back through the ordered reveal on a new query', async () => { const first = mockProviders(['files', 'talk']) const searchController = new UnifiedSearchController() @@ -499,23 +494,15 @@ describe('UnifiedSearchController', () => { first.talk.resolve(['Old talk']) await vi.advanceTimersByTimeAsync(0) - // Refine. talk (lower priority) comes back before files this time. It already had - // results, so it must not drop into blocked (which excludes it from the rendered - // set and blinks it off screen); it stays visible by settling straight to loaded. + // Refine. talk comes back first this time. Nothing is on screen to protect any more, + // so it takes its turn in the queue again instead of skipping the reveal. const second = mockProviders(['files', 'talk']) searchController.search('new', ['files', 'talk']) second.talk.resolve(['New talk']) await vi.advanceTimersByTimeAsync(0) - expect(searchController.getSnapshot().talk.status).toBe('loaded') - // files is still fetching; its stale page stays up meanwhile. - expect(searchController.getSnapshot().files).toEqual({ - status: 'loading', - entries: ['Old files'], - cursor: null, - hasMore: false, - loadMoreFailed: false, - }) + expect(searchController.getSnapshot().talk.status).toBe('blocked') + expect(searchController.getRevealOrder()).toEqual([]) }) }) @@ -1070,10 +1057,8 @@ describe('UnifiedSearchController', () => { const searchController = new UnifiedSearchController() searchController.search('first', ['files', 'talk']) - // The first search spends its window on talk, then stands the timer down. talk - // comes back empty so it carries no stale results into the second search, which - // would otherwise settle it straight to loaded and never block it. - first.talk.resolve([]) + // The first search spends its window on talk, then stands the timer down. + first.talk.resolve(['First talk']) await vi.advanceTimersByTimeAsync(REVEAL_INTERVAL_MS) expect(searchController.getSnapshot().talk.status).toBe('loaded') expect(vi.getTimerCount()).toBe(0)