From 86fe80da254cbae8e92786e222be4c74a2270270 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Tue, 8 Sep 2026 19:39:27 -0700 Subject: [PATCH] perf: preserve store state on unchanged document titles (#19459) * perf: preserve store state on unchanged document titles * fix(browser): compare every doc-history field before skipping a title refresh A hand-listed title check would silently swallow any field added to WorkspaceDocHistoryEntry later. Cover the over-cap trim path too. --------- Co-authored-by: m4air Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> --- .../slices/browser/browser-history-actions.ts | 32 ++++--- .../workspace-document-title-refresh.test.ts | 87 +++++++++++++++++++ src/shared/workspace-doc-history.ts | 15 ++++ 3 files changed, 123 insertions(+), 11 deletions(-) create mode 100644 src/renderer/src/store/slices/workspace-document-title-refresh.test.ts diff --git a/src/renderer/src/store/slices/browser/browser-history-actions.ts b/src/renderer/src/store/slices/browser/browser-history-actions.ts index 85c5b4f5479..fce9d58cae1 100644 --- a/src/renderer/src/store/slices/browser/browser-history-actions.ts +++ b/src/renderer/src/store/slices/browser/browser-history-actions.ts @@ -9,7 +9,8 @@ import { import { MAX_WORKSPACE_DOC_HISTORY_ENTRIES, normalizeWorkspaceDocHistoryEntries, - normalizeWorkspaceDocHistoryTitle + normalizeWorkspaceDocHistoryTitle, + workspaceDocHistoryEntriesEqual } from '../../../../../shared/workspace-doc-history' import { browserPageDocLocationsEqual } from '../../../../../shared/browser-page-doc-location' import { ORCA_BROWSER_BLANK_URL } from '../../../../../shared/constants' @@ -37,16 +38,25 @@ export function createBrowserHistoryActions( title ?? existing?.title, docLocation ) - const next: WorkspaceDocHistoryEntry[] = existing - ? s.workspaceDocHistory.map((entry) => - entry === existing - ? { - ...entry, - title: normalizedTitle, - ...(bump ? { lastVisitedAt: now, visitCount: entry.visitCount + 1 } : {}) - } - : entry - ) + const updated: WorkspaceDocHistoryEntry | null = existing + ? { + ...existing, + title: normalizedTitle, + ...(bump ? { lastVisitedAt: now, visitCount: existing.visitCount + 1 } : {}) + } + : null + // A repeated title-updated event from a live preview changes nothing; hand back the same + // state so no subscriber re-renders. Over the cap the old path still had trimming to do. + if ( + existing && + updated && + workspaceDocHistoryEntriesEqual(updated, existing) && + s.workspaceDocHistory.length <= MAX_WORKSPACE_DOC_HISTORY_ENTRIES + ) { + return s + } + const next: WorkspaceDocHistoryEntry[] = updated + ? s.workspaceDocHistory.map((entry) => (entry === existing ? updated : entry)) : [ { docLocation, title: normalizedTitle, lastVisitedAt: now, visitCount: 1 }, ...s.workspaceDocHistory diff --git a/src/renderer/src/store/slices/workspace-document-title-refresh.test.ts b/src/renderer/src/store/slices/workspace-document-title-refresh.test.ts new file mode 100644 index 00000000000..21e2cf77751 --- /dev/null +++ b/src/renderer/src/store/slices/workspace-document-title-refresh.test.ts @@ -0,0 +1,87 @@ +import { describe, expect, it, vi } from 'vitest' +import { createTestStore } from './store-test-helpers' +import { createStoreCascadesMockApi } from './store-cascades-test-harness' +import { + MAX_WORKSPACE_DOC_HISTORY_ENTRIES, + workspaceDocHistoryEntriesEqual, + type WorkspaceDocHistoryEntry +} from '../../../../shared/workspace-doc-history' + +createStoreCascadesMockApi() +const WT = 'repo1::/path/wt1' + +describe('workspace document history title refresh', () => { + it('does not publish unchanged titles but preserves real visits and title changes', () => { + const store = createTestStore() + const location = { + kind: 'workspace-doc' as const, + worktreeId: WT, + filePath: '/path/wt1/doc.html' + } + store.getState().recordWorkspaceDocVisit(location, 'Document') + const history = store.getState().workspaceDocHistory + const listener = vi.fn() + const unsubscribe = store.subscribe(listener) + for (let i = 0; i < 200; i++) { + store.getState().recordWorkspaceDocVisit(location, 'Document', { bump: false }) + } + expect(listener).not.toHaveBeenCalled() + expect(store.getState().workspaceDocHistory).toBe(history) + store.getState().recordWorkspaceDocVisit(location, 'Renamed', { bump: false }) + expect(store.getState().workspaceDocHistory[0]).toEqual({ ...history[0], title: 'Renamed' }) + store.getState().recordWorkspaceDocVisit(location, 'Renamed') + expect(store.getState().workspaceDocHistory[0].visitCount).toBe(2) + unsubscribe() + }) + + it('still trims an over-cap history that a persisted state carried in', () => { + const store = createTestStore() + const location = { + kind: 'workspace-doc' as const, + worktreeId: WT, + filePath: '/path/wt1/doc-0.html' + } + const overCap: WorkspaceDocHistoryEntry[] = Array.from( + { length: MAX_WORKSPACE_DOC_HISTORY_ENTRIES + 5 }, + (_unused, index) => ({ + docLocation: { + kind: 'workspace-doc' as const, + worktreeId: WT, + filePath: `/path/wt1/doc-${index}.html` + }, + title: `Doc ${index}`, + lastVisitedAt: 1_000 - index, + visitCount: 1 + }) + ) + store.setState({ workspaceDocHistory: overCap }) + + // The title is already current, but skipping here would strand the list above its cap forever. + store.getState().recordWorkspaceDocVisit(location, 'Doc 0', { bump: false }) + + expect(store.getState().workspaceDocHistory).toHaveLength(MAX_WORKSPACE_DOC_HISTORY_ENTRIES) + expect(store.getState().workspaceDocHistory[0].title).toBe('Doc 0') + }) + + it('compares every entry field, so an added field cannot be silently dropped', () => { + const base: WorkspaceDocHistoryEntry = { + docLocation: { kind: 'workspace-doc', worktreeId: WT, filePath: '/path/wt1/doc.html' }, + title: 'Document', + lastVisitedAt: 10, + visitCount: 1 + } + expect(workspaceDocHistoryEntriesEqual(base, { ...base })).toBe(true) + expect(workspaceDocHistoryEntriesEqual(base, { ...base, title: 'Other' })).toBe(false) + expect(workspaceDocHistoryEntriesEqual(base, { ...base, visitCount: 2 })).toBe(false) + expect(workspaceDocHistoryEntriesEqual(base, { ...base, lastVisitedAt: 11 })).toBe(false) + expect( + workspaceDocHistoryEntriesEqual(base, { ...base, docLocation: { ...base.docLocation } }) + ).toBe(false) + expect( + workspaceDocHistoryEntriesEqual(base, { + ...base, + ...({ faviconUrl: 'x' } as Partial) + }) + ).toBe(false) + }) +}) diff --git a/src/shared/workspace-doc-history.ts b/src/shared/workspace-doc-history.ts index f5965cdadfb..bf9e111dc8f 100644 --- a/src/shared/workspace-doc-history.ts +++ b/src/shared/workspace-doc-history.ts @@ -16,6 +16,21 @@ export type WorkspaceDocHistoryEntry = { export const MAX_WORKSPACE_DOC_HISTORY_ENTRIES = 100 +/** + * Shallow-equal over whatever keys an entry actually carries, rather than a hand-listed subset, so + * a field added to `WorkspaceDocHistoryEntry` later cannot slip past a skip-if-unchanged check. + */ +export function workspaceDocHistoryEntriesEqual( + left: WorkspaceDocHistoryEntry, + right: WorkspaceDocHistoryEntry +): boolean { + const leftKeys = Object.keys(left) as (keyof WorkspaceDocHistoryEntry)[] + return ( + leftKeys.length === Object.keys(right).length && + leftKeys.every((key) => Object.is(left[key], right[key])) + ) +} + /** The title fence the page store applies, for history rows: a url-as-title falls back to the file. */ export function normalizeWorkspaceDocHistoryTitle( title: string | null | undefined,