perf: preserve matching General settings sections during search (#23177)

This commit is contained in:
Neil
2026-09-27 13:21:54 -07:00
committed by GitHub
parent bdaf9a3ed3
commit 3f1bf68c84
4 changed files with 329 additions and 9 deletions
+90
View File
@@ -426,6 +426,96 @@
],
"demotionRule": "Remain experimental until repeated independent/CI evidence; investigate ordering or unbounded-wait regressions without weakening completion or deadline assertions."
},
{
"id": "settings.general-section-lifetime",
"title": "General settings search preserves matching section state",
"maturity": "experimental",
"protection": "partial",
"owner": "renderer-settings",
"layer": "renderer-unit",
"surfaces": [
"General settings search",
"app version and remote update display",
"autosave settings draft"
],
"platforms": [
"macos",
"linux",
"windows"
],
"providers": [
"local",
"remote-runtime",
"ssh",
"wsl"
],
"coveredPlatforms": [
"macos"
],
"coveredProviders": [],
"coverageNotes": "Actual GeneralPane, update/version and remote-status components with mocked store/API; actual autosave form. Windows runtime on/off simulated. No physical updater, paired host, settings write or native input.",
"motivatingLinks": [
"https://github.com/stablyai/orca/blob/main/src/renderer/src/components/settings/GeneralPane.tsx"
],
"invariant": "A matching General settings section retains its owner and state as preceding search results disappear; an actually removed section releases its owner and reloads when shown again.",
"oracle": "Across 14 matching searches the Updates section keeps the same DOM node and the version-read and remote-refresh counts do not grow, instead of six each. Store-published remote update state still reaches the retained section. Explicit refresh and genuine hide/reopen still read; drafts yield to externally changed saved settings; a removed owner cannot publish its late version.",
"commands": [
"ORCA_BACKGROUND_LAUNCH=1 pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/settings/GeneralPane.section-lifetime.test.tsx"
],
"testFiles": [
"src/renderer/src/components/settings/GeneralPane.section-lifetime.test.tsx"
],
"assertionRefs": [
{
"file": "src/renderer/src/components/settings/GeneralPane.section-lifetime.test.tsx",
"assertions": [
"keeps showing store-published remote update state without a remount",
"keeps explicit remote refresh and true hide/reopen reads",
"preserves an autosave draft until external settings change or the section hides",
"does not publish a version result from a genuinely unmounted section"
]
}
],
"evidenceRuns": [
{
"date": "2026-09-26",
"runner": "local",
"platform": "macos",
"command": "ORCA_BACKGROUND_LAUNCH=1 pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/settings/GeneralPane.section-lifetime.test.tsx",
"result": "passed",
"durationSeconds": 1.89,
"summary": "7 tests/1 suite pass after the shared SettingsSectionStack adoption. Read-count oracles restated as node identity plus non-growing counts; original main fails both retained-section cases, the draft, the release-picker reveal and the late-version publish."
}
],
"runtimeBudget": {
"p95Seconds": 10,
"scope": "Focused renderer-unit suites, p95 not established."
},
"flakeHistory": {
"status": "not-started",
"evidence": "Author and independent local evidence only; no CI soak."
},
"redGreenEvidence": {
"status": "complete",
"evidence": "Candidate permanent6pass versus original4fail2controls. Additional independent pending-version/error-context/store-refresh3pass versus original2fail1control."
},
"performanceBudget": {
"required": true,
"evidence": "Actual version and remote-refresh admissions 6 to 1 across 14 matching queries, which also stops one paired-host update probe per keystroke. Section map now lives in the shared SettingsSectionStack; no added poll, cache, listener or subprocess. No packaged CPU/frame/heap claim."
},
"knownGaps": [
"CI soak pending.",
"Physical updater/install, local/daemon PTYs and mobile are unaffected; no execution-host routing, process authority or wire change.",
"Native Linux/Windows, SSH/WSL and paired remote-runtime behavior not exercised; synthetic runtime flags cover renderer branches only.",
"Matching search no longer incidentally refreshes version/remote-status/CLI discovery; explicit checks and real hide/reopen remain. The installed version cannot change in-process and remote update entries are store-published, so neither goes stale without a signal.",
"Retained drafts, release reveal and pending/error context last until a real hide or their own transition; programmatic-query draft test does not reproduce ordinary mouse-search blur/commit.",
"Hidden Electron actual-form proof is separate; no packaged latency, heap or native focus evidence."
],
"promotionCriteria": [
"Retain count, draft, owner cleanup and refresh controls; collect CI soak and relevant full-app/provider evidence before promotion."
],
"demotionRule": "Keep experimental until soak; investigate state or cleanup regressions without weakening owner, read-count or draft assertions."
},
{
"id": "agent-session.journal-streaming-replay",
"title": "Journal replay bounds obsolete revision memory without changing recovery",
@@ -0,0 +1,206 @@
// @vitest-environment happy-dom
import { act, cleanup, fireEvent, render, screen } from '@testing-library/react'
import { afterEach, beforeEach, expect, it, vi } from 'vitest'
import { getDefaultSettings } from '../../../../shared/constants'
import { GeneralPane } from './GeneralPane'
const fake = vi.hoisted(() => ({
query: '',
getVersion: vi.fn(async () => '1.4.100'),
refresh: vi.fn(async () => {}),
action: vi.fn(),
updates: new Map([
['synthetic-host', { environmentId: 'synthetic-host', phase: 'current', name: 'Synthetic' }]
])
}))
vi.mock('@/i18n/i18n', () => ({
i18n: { language: 'en' },
translate: (_key: string, fallback: string, values?: Record<string, string | number>) =>
Object.entries(values ?? {}).reduce(
(text, [key, value]) => text.replaceAll(`{{${key}}}`, String(value)),
fallback
)
}))
vi.mock('@/store', () => ({
useAppStore: (selector: (state: Record<string, unknown>) => unknown) =>
selector({
settingsSearchQuery: fake.query,
updateStatus: { state: 'idle' },
remoteServerUpdates: fake.updates,
remoteServerUpdatesChecking: false,
remoteServerUpdatesRunning: false,
refreshRemoteServerUpdates: fake.refresh,
setRemoteServerUpdateDialogOpen: fake.action
})
}))
vi.mock('@/components/settings/GeneralWorkspaceSettingsSection', () => ({
GeneralWorkspaceSettingsSection: () => null
}))
vi.mock('@/components/settings/CliSection', () => ({ CliSection: () => null }))
vi.mock('@/components/settings/GeneralSupportSection', () => ({
GeneralSupportSection: () => null
}))
vi.mock('@/components/settings/ReleaseChannelSection', () => ({
ReleaseChannelSection: () => <div>Release picker open</div>
}))
vi.mock('@/components/settings/DefaultWindowsProjectRuntimeSetting', () => ({
DefaultWindowsProjectRuntimeSetting: () => null
}))
beforeEach(() => {
vi.clearAllMocks()
fake.query = ''
fake.updates = new Map([
['synthetic-host', { environmentId: 'synthetic-host', phase: 'current', name: 'Synthetic' }]
])
Object.assign(window, { api: { updater: { getVersion: fake.getVersion } } })
})
afterEach(() => {
cleanup()
Reflect.deleteProperty(window, 'api')
})
it.each([false, true])(
'retains update sections with Windows runtime support %s',
async (wslSupportedPlatform) => {
const props = {
settings: getDefaultSettings('/synthetic'),
updateSettings: vi.fn(),
fontSuggestions: [],
wslSupportedPlatform
}
const view = render(<GeneralPane {...props} />)
await act(async () => {})
// The invariant is identity, not a magic number: the matched section keeps the same DOM
// node across every query edit, so editing the query cannot cost an extra read.
const mountedHeading = screen.getByText('Updates', { exact: true })
const versionReadsAfterMount = fake.getVersion.mock.calls.length
const remoteRefreshesAfterMount = fake.refresh.mock.calls.length
for (const query of [
'u',
'up',
'upd',
'upda',
'updat',
'update',
'',
'v',
've',
'ver',
'vers',
'versi',
'versio',
'version'
]) {
fake.query = query
await act(async () => view.rerender(<GeneralPane {...props} />))
expect(screen.getByText('Updates', { exact: true })).toBe(mountedHeading)
}
expect(fake.getVersion.mock.calls.length).toBe(versionReadsAfterMount)
expect(fake.refresh.mock.calls.length).toBe(remoteRefreshesAfterMount)
}
)
it('keeps showing store-published remote update state without a remount', async () => {
const props = {
settings: getDefaultSettings('/synthetic'),
updateSettings: vi.fn(),
fontSuggestions: []
}
fake.query = 'update'
const view = render(<GeneralPane {...props} />)
await act(async () => {})
const mountedHeading = screen.getByText('Updates', { exact: true })
expect(screen.getByText('1 paired server · 1 up to date')).toBeTruthy()
// Remote update entries live in the store, so a retained section stays fresh through the
// subscription rather than through the remount a query edit used to force.
fake.updates = new Map([
['synthetic-host', { environmentId: 'synthetic-host', phase: 'available', name: 'Synthetic' }]
])
fake.query = 'updat'
await act(async () => view.rerender(<GeneralPane {...props} />))
expect(screen.getByText('Updates', { exact: true })).toBe(mountedHeading)
expect(screen.getByText('1 paired server · 1 ready to update')).toBeTruthy()
expect(fake.refresh.mock.calls.length).toBe(1)
})
it('keeps explicit remote refresh and true hide/reopen reads', async () => {
const props = {
settings: getDefaultSettings('/synthetic'),
updateSettings: vi.fn(),
fontSuggestions: []
}
const view = render(<GeneralPane {...props} />)
await act(async () => {})
fireEvent.click(screen.getByRole('button', { name: 'Check for Server Updates' }))
expect(fake.refresh).toHaveBeenCalledTimes(2)
fake.query = 'Tab Order'
await act(async () => view.rerender(<GeneralPane {...props} />))
expect(screen.queryByText('Updates', { exact: true })).toBeNull()
fake.query = 'update'
await act(async () => view.rerender(<GeneralPane {...props} />))
expect(fake.getVersion).toHaveBeenCalledTimes(2)
expect(fake.refresh).toHaveBeenCalledTimes(3)
})
it('preserves an autosave draft until external settings change or the section hides', async () => {
const settings = { ...getDefaultSettings('/synthetic'), editorAutoSaveDelayMs: 1000 }
const props = { settings, updateSettings: vi.fn(), fontSuggestions: [] }
const view = render(<GeneralPane {...props} />)
await act(async () => {})
fireEvent.change(screen.getByRole('spinbutton'), { target: { value: '2500' } })
fake.query = 'Auto Save'
await act(async () => view.rerender(<GeneralPane {...props} />))
expect(screen.getByRole('spinbutton').getAttribute('value')).toBe('2500')
expect(props.updateSettings).not.toHaveBeenCalled()
const changedProps = { ...props, settings: { ...settings, editorAutoSaveDelayMs: 3000 } }
await act(async () => view.rerender(<GeneralPane {...changedProps} />))
expect(screen.getByRole('spinbutton').getAttribute('value')).toBe('3000')
fireEvent.change(screen.getByRole('spinbutton'), { target: { value: '3500' } })
fake.query = 'Tab Order'
await act(async () => view.rerender(<GeneralPane {...changedProps} />))
expect(screen.queryByRole('spinbutton')).toBeNull()
fake.query = 'Auto Save'
await act(async () => view.rerender(<GeneralPane {...changedProps} />))
expect(screen.getByRole('spinbutton').getAttribute('value')).toBe('3000')
expect(props.updateSettings).not.toHaveBeenCalled()
})
it('retains the revealed release picker only while Updates remains visible', async () => {
const props = {
settings: getDefaultSettings('/synthetic'),
updateSettings: vi.fn(),
fontSuggestions: []
}
const view = render(<GeneralPane {...props} />)
await act(async () => {})
fireEvent.click(screen.getByText('Updates', { exact: true }), { altKey: true })
expect(screen.getByText('Release picker open')).toBeTruthy()
fake.query = 'update'
await act(async () => view.rerender(<GeneralPane {...props} />))
expect(screen.getByText('Release picker open')).toBeTruthy()
fake.query = 'Tab Order'
await act(async () => view.rerender(<GeneralPane {...props} />))
fake.query = 'update'
await act(async () => view.rerender(<GeneralPane {...props} />))
expect(screen.queryByText('Release picker open')).toBeNull()
})
it('does not publish a version result from a genuinely unmounted section', async () => {
let resolveOld: ((value: string) => void) | undefined
fake.getVersion.mockImplementationOnce(
() =>
new Promise<string>((resolve) => {
resolveOld = resolve
})
)
const props = {
settings: getDefaultSettings('/synthetic'),
updateSettings: vi.fn(),
fontSuggestions: []
}
const view = render(<GeneralPane {...props} />)
await act(async () => {})
fake.query = 'Tab Order'
await act(async () => view.rerender(<GeneralPane {...props} />))
fake.query = 'update'
await act(async () => view.rerender(<GeneralPane {...props} />))
expect(screen.getByText('Current version: 1.4.100')).toBeTruthy()
await act(async () => resolveOld?.('1.0.0'))
expect(screen.getByText('Current version: 1.4.100')).toBeTruthy()
expect(screen.queryByText('Current version: 1.0.0')).toBeNull()
})
@@ -1,7 +1,6 @@
import type React from 'react'
import type { GlobalSettings } from '../../../../shared/global-settings-types'
import { useAppStore } from '../../store'
import { Separator } from '../ui/separator'
import { CliSection } from './CliSection'
import { GeneralEditorSettingsSection } from './GeneralEditorSettingsSection'
import { GeneralSupportSection } from './GeneralSupportSection'
@@ -18,6 +17,7 @@ import {
} from './general-search'
import { getGeneralProjectRuntimeSearchEntries } from './general-project-runtime-search'
import { RecentTabOrderControl } from './RecentTabOrderControl'
import { SettingsSectionStack } from './SettingsSectionStack'
import { matchesSettingsSearch, type SettingsSearchEntry } from './settings-search'
import { SearchableSetting } from './SearchableSetting'
import { SettingsSubsectionHeader, SettingsSwitchRow } from './SettingsFormControls'
@@ -279,18 +279,15 @@ export function GeneralPane({
// its own loading placeholder and its own collapsing Separator. Without
// that separation, a dangling divider would remain above the collapsed
// section.
].filter(Boolean)
]
return (
<div className="space-y-6">
{visibleSections.map((section, index) => (
<div key={index} className="space-y-6">
{index > 0 ? <Separator /> : null}
{section}
</div>
))}
<SettingsSectionStack sections={visibleSections} spacing="section" />
{matchesSettingsSearch(searchQuery, getGeneralSupportSearchEntries()) ? (
<GeneralSupportSection hasPrecedingSections={visibleSections.length > 0} />
<GeneralSupportSection
hasPrecedingSections={visibleSections.some((section) => section !== null)}
/>
) : null}
</div>
)
@@ -0,0 +1,27 @@
import type { ReactElement } from 'react'
import { Separator } from '../ui/separator'
// Why: settings panes rebuild their section list from the search query, so a pane-local
// `key={index}` re-binds every key when an earlier section drops out. React then tears down
// and remounts each surviving section, discarding its unsaved drafts and re-issuing its
// loads once per keystroke. Keying by the section's own element key keeps a section that
// stays matched mounted for the whole search.
export function SettingsSectionStack({
sections,
spacing
}: {
sections: readonly (ReactElement | null)[]
spacing: 'section' | 'group'
}): ReactElement {
const visibleSections = sections.filter((section): section is ReactElement => section !== null)
return (
<>
{visibleSections.map((section, index) => (
<div key={section.key} className={spacing === 'group' ? 'space-y-8' : 'space-y-6'}>
{index > 0 ? <Separator /> : null}
{section}
</div>
))}
</>
)
}