mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
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 <m4air@m4airs-MacBook-Air.local> Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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<WorkspaceDocHistoryEntry>)
|
||||
})
|
||||
).toBe(false)
|
||||
})
|
||||
})
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user