fix(changes-tab): prevent freeze and stale highlights when staging/unstaging (#6229)

* fix(changes-tab): prevent freeze and stale highlights when staging/unstaging

When files are staged/unstaged while the Changes tab is open, the
diff viewer was re-rendering all sections, causing UI freezes and
highlight flickering.

- Add resolveCombinedUncommittedSnapshotEntries() to reconcile
  snapshot entries with live git status without destroying existing
  loaded diff content.
- Track retainedResolvedSnapshotEntries from current sections to
  preserve area state when live git status temporarily loses entries.
- Use Map-based O(snapshot + live) algorithm instead of nested loops.
- Add unit tests for the resolver logic.
- Add e2e test with performance measurement for stale unstaged diffs.

Co-authored-by: Orca <help@stably.ai>

* Prevent duplicate entries in uncommitted snapshot resolution

Track resolved snapshot entries using composite area and path keys to
ensure staged and unstaged sections remain distinct and do not duplicate
when live Git status changes or disappears.

Additionally, switch the retained entries from a Map to a list to support
multiple areas per path, and update CombinedDiffViewer to avoid rebuilding
the snapshot list on row load state changes.

* test: add test coverage for duplicate-path snapshot remapping

- Add unit test verifying that duplicate-path snapshots do not remap
  to a retained fallback area.
- Robustify E2E large diff freeze repro test by ensuring interval
  timers are always cleared in a finally block.
- Standardize path utilities in large diff fixtures to ensure cross-platform
  compatibility.

---------

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinjing 2026-06-23 22:16:20 -07:00 committed by GitHub
parent c3a089ff44
commit 5106e33947
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 455 additions and 46 deletions

View File

@ -66,6 +66,7 @@ import {
import {
getCombinedBranchEntries,
getCombinedUncommittedEntries,
resolveCombinedUncommittedSnapshotEntries,
shouldAutoReloadCombinedDiffFromGitStatus
} from './combined-diff-entries'
import { getCombinedDiffCommitMessageBody } from './combined-diff-commit-message'
@ -127,6 +128,23 @@ function invalidateCombinedDiffCachesForRelativePath(relativePath: string): void
}
}
function getRetainedResolvedSnapshotEntries(sections: readonly DiffSection[]): GitStatusEntry[] {
return sections.flatMap((section) =>
section.area === undefined
? []
: [
{
path: section.path,
status: section.status as GitStatusEntry['status'],
area: section.area,
oldPath: section.oldPath,
added: section.added,
removed: section.removed
}
]
)
}
if (typeof window !== 'undefined') {
window.addEventListener(ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT, (event) => {
const detail = (event as CustomEvent<EditorPathMutationTarget>).detail
@ -423,11 +441,18 @@ export default function CombinedDiffViewer({
() => file.uncommittedEntriesSnapshot?.filter((e) => e.conflictStatus !== 'unresolved'),
[file.uncommittedEntriesSnapshot]
)
const uncommittedEntries = React.useMemo(
() =>
snapshotEntries ?? getCombinedUncommittedEntries(gitStatusEntries, file.combinedAreaFilter),
[snapshotEntries, gitStatusEntries, file.combinedAreaFilter]
)
const uncommittedEntries = React.useMemo(() => {
if (!snapshotEntries) {
return getCombinedUncommittedEntries(gitStatusEntries, file.combinedAreaFilter)
}
// Why: row load state changes must not rebuild the snapshot entry list;
// the ref is only consulted when live Git status changes.
return resolveCombinedUncommittedSnapshotEntries(
snapshotEntries,
gitStatusEntries,
getRetainedResolvedSnapshotEntries(sectionsRef.current)
)
}, [snapshotEntries, gitStatusEntries, file.combinedAreaFilter])
const branchEntries = React.useMemo<GitBranchChangeEntry[]>(() => {
return getCombinedBranchEntries(file.branchEntriesSnapshot, liveBranchEntries)
}, [file.branchEntriesSnapshot, liveBranchEntries])

View File

@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'
import {
getCombinedBranchEntries,
getCombinedUncommittedEntries,
resolveCombinedUncommittedSnapshotEntries,
shouldAutoReloadCombinedDiffFromGitStatus
} from './combined-diff-entries'
import type { GitBranchChangeEntry, GitStatusEntry } from '../../../../shared/types'
@ -50,6 +51,153 @@ describe('getCombinedUncommittedEntries', () => {
})
})
describe('resolveCombinedUncommittedSnapshotEntries', () => {
it('uses the live staged area when a snapshot unstaged file has been staged', () => {
const snapshotEntries: GitStatusEntry[] = [
{ path: 'src/file.ts', status: 'modified', area: 'unstaged', added: 2, removed: 1 }
]
const liveEntries: GitStatusEntry[] = [
{ path: 'src/file.ts', status: 'modified', area: 'staged', added: 2, removed: 1 }
]
expect(resolveCombinedUncommittedSnapshotEntries(snapshotEntries, liveEntries)).toEqual([
{ path: 'src/file.ts', status: 'modified', area: 'staged', added: 2, removed: 1 }
])
})
it('uses the live unstaged area when a snapshot staged file has been unstaged', () => {
const snapshotEntries: GitStatusEntry[] = [
{ path: 'src/file.ts', status: 'modified', area: 'staged' }
]
const liveEntries: GitStatusEntry[] = [
{ path: 'src/file.ts', status: 'modified', area: 'unstaged' }
]
expect(resolveCombinedUncommittedSnapshotEntries(snapshotEntries, liveEntries)).toEqual([
{ path: 'src/file.ts', status: 'modified', area: 'unstaged' }
])
})
it('keeps the snapshot area when that area is still present for a path', () => {
const snapshotEntries: GitStatusEntry[] = [
{ path: 'src/file.ts', status: 'modified', area: 'unstaged', added: 2 }
]
const liveEntries: GitStatusEntry[] = [
{ path: 'src/file.ts', status: 'modified', area: 'staged' },
{ path: 'src/file.ts', status: 'modified', area: 'unstaged', added: 9 }
]
expect(resolveCombinedUncommittedSnapshotEntries(snapshotEntries, liveEntries)).toEqual(
snapshotEntries
)
})
it('keeps a retained resolved area when live status no longer includes the path', () => {
const snapshotEntries: GitStatusEntry[] = [
{ path: 'src/file.ts', status: 'modified', area: 'unstaged' }
]
const retained: GitStatusEntry[] = [{ path: 'src/file.ts', status: 'modified', area: 'staged' }]
expect(resolveCombinedUncommittedSnapshotEntries(snapshotEntries, [], retained)).toEqual([
{ path: 'src/file.ts', status: 'modified', area: 'staged' }
])
})
it('uses live metadata when a stale snapshot area moves to a rename entry', () => {
const snapshotEntries: GitStatusEntry[] = [
{ path: 'src/new.ts', status: 'modified', area: 'unstaged', oldPath: 'src/old-copy.ts' }
]
const liveEntries: GitStatusEntry[] = [
{
path: 'src/new.ts',
status: 'renamed',
area: 'staged',
oldPath: 'src/old.ts',
added: 3,
removed: 1
}
]
expect(resolveCombinedUncommittedSnapshotEntries(snapshotEntries, liveEntries)).toEqual([
{
path: 'src/new.ts',
status: 'renamed',
area: 'staged',
oldPath: 'src/old.ts',
added: 3,
removed: 1
}
])
})
it('keeps retained staged and unstaged sections distinct when live status disappears', () => {
const snapshotEntries: GitStatusEntry[] = [
{ path: 'src/file.ts', status: 'modified', area: 'staged', added: 4 },
{ path: 'src/file.ts', status: 'modified', area: 'unstaged', added: 2 }
]
const retained: GitStatusEntry[] = [
{ path: 'src/file.ts', status: 'modified', area: 'staged' },
{ path: 'src/file.ts', status: 'modified', area: 'unstaged' }
]
expect(resolveCombinedUncommittedSnapshotEntries(snapshotEntries, [], retained)).toEqual(
snapshotEntries
)
})
it('does not remap duplicate-path snapshots to a retained fallback area', () => {
const snapshotEntries: GitStatusEntry[] = [
{ path: 'src/file.ts', status: 'modified', area: 'staged', added: 4 },
{ path: 'src/file.ts', status: 'modified', area: 'unstaged', added: 2 }
]
const retained: GitStatusEntry[] = [{ path: 'src/file.ts', status: 'modified', area: 'staged' }]
expect(resolveCombinedUncommittedSnapshotEntries(snapshotEntries, [], retained)).toEqual([
{ path: 'src/file.ts', status: 'modified', area: 'staged', added: 4 }
])
})
it('does not let a retained stale area duplicate an original target area', () => {
const snapshotEntries: GitStatusEntry[] = [
{ path: 'src/file.ts', status: 'modified', area: 'unstaged', added: 2 },
{ path: 'src/file.ts', status: 'modified', area: 'staged', added: 4 }
]
const retained: GitStatusEntry[] = [{ path: 'src/file.ts', status: 'modified', area: 'staged' }]
expect(resolveCombinedUncommittedSnapshotEntries(snapshotEntries, [], retained)).toEqual([
{ path: 'src/file.ts', status: 'modified', area: 'staged', added: 4 }
])
})
it('drops a stale snapshot area when resolving it would duplicate an existing area', () => {
const snapshotEntries: GitStatusEntry[] = [
{ path: 'src/file.ts', status: 'modified', area: 'unstaged', added: 2 },
{ path: 'src/file.ts', status: 'modified', area: 'staged', added: 4 }
]
const liveEntries: GitStatusEntry[] = [
{ path: 'src/file.ts', status: 'modified', area: 'staged', added: 6 }
]
expect(resolveCombinedUncommittedSnapshotEntries(snapshotEntries, liveEntries)).toEqual([
{ path: 'src/file.ts', status: 'modified', area: 'staged', added: 4 }
])
})
it('keeps the original target area when it appears before a stale area', () => {
const snapshotEntries: GitStatusEntry[] = [
{ path: 'src/file.ts', status: 'modified', area: 'staged', added: 4 },
{ path: 'src/file.ts', status: 'modified', area: 'unstaged', added: 2 }
]
const liveEntries: GitStatusEntry[] = [
{ path: 'src/file.ts', status: 'modified', area: 'staged', added: 6 }
]
expect(resolveCombinedUncommittedSnapshotEntries(snapshotEntries, liveEntries)).toEqual([
{ path: 'src/file.ts', status: 'modified', area: 'staged', added: 4 }
])
})
})
describe('getCombinedBranchEntries', () => {
it('uses an explicitly empty snapshot instead of falling back to live entries', () => {
const liveEntries: GitBranchChangeEntry[] = [{ path: 'src/live.ts', status: 'modified' }]

View File

@ -19,6 +19,94 @@ export function getCombinedUncommittedEntries(
})
}
export function resolveCombinedUncommittedSnapshotEntries(
snapshotEntries: readonly GitStatusEntry[],
liveEntries: readonly GitStatusEntry[],
retainedResolvedEntries: readonly GitStatusEntry[] = []
): GitStatusEntry[] {
const liveEntriesByPath = getGitStatusEntriesByPath(liveEntries)
const retainedEntriesByPath = getGitStatusEntriesByPath(retainedResolvedEntries)
const snapshotAreaKeys = new Set(snapshotEntries.map(getUncommittedAreaPathKey))
const resolvedEntries: GitStatusEntry[] = []
const resolvedAreaKeys = new Set<string>()
const pushResolvedEntry = (entry: GitStatusEntry): void => {
const areaKey = getUncommittedAreaPathKey(entry)
if (resolvedAreaKeys.has(areaKey)) {
return
}
resolvedAreaKeys.add(areaKey)
resolvedEntries.push(entry)
}
for (const snapshotEntry of snapshotEntries) {
const livePathEntries = liveEntriesByPath.get(snapshotEntry.path) ?? []
if (livePathEntries.some((liveEntry) => liveEntry.area === snapshotEntry.area)) {
pushResolvedEntry(snapshotEntry)
continue
}
const retainedPathEntries = retainedEntriesByPath.get(snapshotEntry.path) ?? []
if (
livePathEntries.length === 0 &&
retainedPathEntries.some((retainedEntry) => retainedEntry.area === snapshotEntry.area)
) {
pushResolvedEntry(snapshotEntry)
continue
}
const movedEntry =
livePathEntries[0] ?? (retainedPathEntries.length === 1 ? retainedPathEntries[0] : undefined)
if (!movedEntry || movedEntry.area === snapshotEntry.area) {
pushResolvedEntry(snapshotEntry)
continue
}
const movedAreaKey = getUncommittedAreaPathKey({
path: snapshotEntry.path,
area: movedEntry.area
})
if (snapshotAreaKeys.has(movedAreaKey)) {
continue
}
if (resolvedAreaKeys.has(movedAreaKey)) {
continue
}
// Why: a snapshot-backed Changes tab can outlive stage/unstage actions.
// Load the area Git now reports so Monaco doesn't diff identical files.
pushResolvedEntry({
...snapshotEntry,
area: movedEntry.area,
status: movedEntry.status,
oldPath: movedEntry.oldPath,
added: movedEntry.added,
removed: movedEntry.removed,
submodule: movedEntry.submodule
})
}
return resolvedEntries
}
function getGitStatusEntriesByPath(
entries: readonly GitStatusEntry[]
): Map<string, GitStatusEntry[]> {
const entriesByPath = new Map<string, GitStatusEntry[]>()
for (const entry of entries) {
const pathEntries = entriesByPath.get(entry.path)
if (pathEntries) {
pathEntries.push(entry)
} else {
entriesByPath.set(entry.path, [entry])
}
}
return entriesByPath
}
function getUncommittedAreaPathKey(entry: Pick<GitStatusEntry, 'area' | 'path'>): string {
return `${entry.area}\0${entry.path}`
}
export function getCombinedBranchEntries(
snapshotEntries: readonly GitBranchChangeEntry[] | undefined,
liveEntries: readonly GitBranchChangeEntry[]
@ -35,7 +123,7 @@ export function shouldAutoReloadCombinedDiffFromGitStatus({
mode: CombinedDiffFileTreeMode
hasUncommittedEntriesSnapshot: boolean
}): boolean {
// Why: snapshot-backed tabs intentionally preserve the tab-open diff while
// Why: snapshot-backed tabs preserve the tab-open file list while
// staging/commit status churns; targeted editor-write reloads still refresh.
return mode === 'uncommitted' && !hasUncommittedEntriesSnapshot
}

View File

@ -1103,7 +1103,7 @@
"notePasteTooLarge": "笔记字段的粘贴内容过大。",
"reuseExistingBranch": "复用分支",
"reuseExistingBranchHint": "检出已有分支,而不是基于它创建新分支。",
"createMultiple": "Create more"
"createMultiple": "创建更多"
},
"NewWorkspaceComposerModal": {
"fa90f739a5": "创建工作区之前选择项目、工作区名称和 Agent。"

View File

@ -1,38 +1,13 @@
import { execFileSync } from 'child_process'
import { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'fs'
import os from 'os'
import path from 'path'
import { randomUUID } from 'crypto'
import { rmSync, writeFileSync } from 'fs'
import type { Page } from '@stablyai/playwright-test'
import { test, expect } from './helpers/orca-app'
import { waitForSessionReady } from './helpers/store'
import { getLargeDiffRenderLimit } from '../../src/shared/large-diff-render-limit'
type IsolatedLargeDiffRepo = {
repoPath: string
relativePath: string
absolutePath: string
}
function runGit(repoPath: string, args: string[]): void {
execFileSync('git', args, { cwd: repoPath, stdio: 'pipe' })
}
function createIsolatedLargeDiffRepo(): IsolatedLargeDiffRepo {
const repoPath = realpathSync(mkdtempSync(path.join(os.tmpdir(), 'orca-large-diff-repro-')))
runGit(repoPath, ['init'])
runGit(repoPath, ['config', 'user.email', 'e2e@test.local'])
runGit(repoPath, ['config', 'user.name', 'E2E Test'])
mkdirSync(path.join(repoPath, 'src'), { recursive: true })
const relativePath = path.join('src', `large-diff-${randomUUID()}.ts`)
const absolutePath = path.join(repoPath, relativePath)
writeFileSync(absolutePath, 'export const seed = 1\n')
runGit(repoPath, ['add', '-A'])
runGit(repoPath, ['commit', '-m', 'Initial large diff repro fixture'])
return { repoPath, relativePath, absolutePath }
}
import {
buildLargeTypeScriptFile,
createIsolatedLargeDiffRepo,
createIsolatedStagedLocaleDiffRepo
} from './large-diff-repro-fixtures'
async function addAndActivateRepo(orcaPage: Page, repoPath: string): Promise<string> {
const repoId = await orcaPage.evaluate(async (pathToRepo: string) => {
@ -92,14 +67,6 @@ async function addAndActivateRepo(orcaPage: Page, repoPath: string): Promise<str
return worktreeId
}
function buildLargeTypeScriptFile(lineCount: number): string {
const lines: string[] = []
for (let i = 0; i < lineCount; i += 1) {
lines.push(`export const largeDiffValue${i} = ${i}`)
}
return `${lines.join('\n')}\n`
}
test.describe('Large diff freeze repro', () => {
test.describe.configure({ mode: 'serial' })
test.use({ seedTestRepo: false })
@ -192,4 +159,92 @@ test.describe('Large diff freeze repro', () => {
rmSync(fixture.repoPath, { recursive: true, force: true })
}
})
test('opening stale unstaged combined diffs after staging keeps the renderer responsive', async ({
orcaPage
}) => {
await waitForSessionReady(orcaPage)
const fixture = createIsolatedStagedLocaleDiffRepo()
try {
const worktreeId = await addAndActivateRepo(orcaPage, fixture.repoPath)
const measurement = await orcaPage.evaluate(
async ({ wId, repoPath, expectedPaths }) => {
const store = window.__store
if (!store) {
throw new Error('window.__store is not available')
}
const status = await window.api.git.status({ worktreePath: repoPath })
store.getState().setGitStatus(wId, status)
const entries = status.entries.filter((entry) => entry.area === 'staged')
const entryPaths = entries.map((entry) => entry.path)
const missing = expectedPaths.filter((path) => !entryPaths.includes(path))
if (missing.length > 0) {
throw new Error(`staged locale fixture missing entries: ${missing.join(', ')}`)
}
// Why: reproduce stale snapshot behavior by opening combined diffs
// as "unstaged" using entries captured from the staged status snapshot.
const staleUnstagedEntries = entries.map((entry) => ({ ...entry, area: 'unstaged' }))
const intervalMs = 50
const samples: number[] = []
let last = performance.now()
let maxLagMs = 0
const timer = window.setInterval(() => {
const now = performance.now()
const lag = Math.max(0, now - last - intervalMs)
maxLagMs = Math.max(maxLagMs, lag)
samples.push(lag)
last = now
}, intervalMs)
const startedAt = performance.now()
store.getState().openAllDiffs(wId, repoPath, undefined, 'unstaged', staleUnstagedEntries)
let editorCount = 0
let fallbackCount = 0
try {
while (performance.now() - startedAt < 30_000) {
await new Promise((resolve) => window.setTimeout(resolve, 50))
editorCount = document.querySelectorAll('.monaco-diff-editor').length
fallbackCount = document.querySelectorAll(
'[data-testid="large-diff-fallback"]'
).length
if (editorCount + fallbackCount >= Math.min(entries.length, 5)) {
await new Promise((resolve) => window.setTimeout(resolve, 1_000))
break
}
}
} finally {
window.clearInterval(timer)
}
const classHits = Array.from(
document.querySelectorAll(
'.monaco-diff-editor .line-insert, .monaco-diff-editor .line-delete, .monaco-diff-editor .char-insert, .monaco-diff-editor .char-delete'
)
).length
return {
editorCount,
fallbackCount,
classHits,
maxLagMs,
sampleCount: samples.length,
p95LagMs: samples.length
? [...samples].sort((a, b) => a - b)[Math.floor(samples.length * 0.95)]
: 0
}
},
{ wId: worktreeId, repoPath: fixture.repoPath, expectedPaths: fixture.relativePaths }
)
console.log(`stale unstaged combined diff measurement ${JSON.stringify(measurement)}`)
expect(measurement.editorCount + measurement.fallbackCount).toBeGreaterThanOrEqual(5)
expect(measurement.classHits).toBeGreaterThan(0)
expect(measurement.maxLagMs).toBeLessThan(1_000)
} finally {
rmSync(fixture.repoPath, { recursive: true, force: true })
}
})
})

View File

@ -0,0 +1,93 @@
import { execFileSync } from 'child_process'
import { mkdirSync, mkdtempSync, realpathSync, writeFileSync } from 'fs'
import os from 'os'
import path from 'path'
import { randomUUID } from 'crypto'
export type IsolatedLargeDiffRepo = {
repoPath: string
relativePath: string
absolutePath: string
}
export type IsolatedStagedLocaleDiffRepo = {
repoPath: string
relativePaths: string[]
}
function runGit(repoPath: string, args: string[]): void {
execFileSync('git', args, { cwd: repoPath, stdio: 'pipe' })
}
export function createIsolatedLargeDiffRepo(): IsolatedLargeDiffRepo {
const repoPath = realpathSync(mkdtempSync(path.join(os.tmpdir(), 'orca-large-diff-repro-')))
runGit(repoPath, ['init'])
runGit(repoPath, ['config', 'user.email', 'e2e@test.local'])
runGit(repoPath, ['config', 'user.name', 'E2E Test'])
mkdirSync(path.join(repoPath, 'src'), { recursive: true })
const relativePath = path.join('src', `large-diff-${randomUUID()}.ts`)
const absolutePath = path.join(repoPath, relativePath)
writeFileSync(absolutePath, 'export const seed = 1\n')
runGit(repoPath, ['add', '-A'])
runGit(repoPath, ['commit', '-m', 'Initial large diff repro fixture'])
return { repoPath, relativePath, absolutePath }
}
export function buildLargeTypeScriptFile(lineCount: number): string {
const lines: string[] = []
for (let i = 0; i < lineCount; i += 1) {
lines.push(`export const largeDiffValue${i} = ${i}`)
}
return `${lines.join('\n')}\n`
}
function buildLocaleLikeJson(fileIndex: number, entryCount: number): string {
const lines = ['{']
for (let i = 0; i < entryCount; i += 1) {
const value = `locale ${fileIndex} original ${i} `.repeat(5).trim()
const comma = i + 1 === entryCount ? '' : ','
lines.push(` "entry_${String(i).padStart(5, '0')}": "${value}"${comma}`)
}
lines.push('}')
return `${lines.join('\n')}\n`
}
function modifyLocaleLikeJson(content: string, fileIndex: number): string {
const lines = content.split('\n')
const changedLineIndex = 3200 + fileIndex
lines[changedLineIndex] = lines[changedLineIndex].replace('original', 'updated')
lines.splice(changedLineIndex + 4, 1)
return lines.join('\n')
}
export function createIsolatedStagedLocaleDiffRepo(): IsolatedStagedLocaleDiffRepo {
const repoPath = realpathSync(mkdtempSync(path.join(os.tmpdir(), 'orca-staged-locale-repro-')))
runGit(repoPath, ['init'])
runGit(repoPath, ['config', 'user.email', 'e2e@test.local'])
runGit(repoPath, ['config', 'user.name', 'E2E Test'])
mkdirSync(path.join(repoPath, 'src', 'locales'), { recursive: true })
const toAbsoluteFsPath = (relativePosixPath: string): string =>
path.join(repoPath, ...relativePosixPath.split(path.posix.sep))
const relativePaths: string[] = []
for (let fileIndex = 0; fileIndex < 5; fileIndex += 1) {
const relativePath = path.posix.join('src', 'locales', `locale-${fileIndex}.json`)
const absolutePath = toAbsoluteFsPath(relativePath)
const original = buildLocaleLikeJson(fileIndex, 3600)
writeFileSync(absolutePath, original)
relativePaths.push(relativePath)
}
runGit(repoPath, ['add', '-A'])
runGit(repoPath, ['commit', '-m', 'Initial locale fixture'])
for (let fileIndex = 0; fileIndex < relativePaths.length; fileIndex += 1) {
const absolutePath = toAbsoluteFsPath(relativePaths[fileIndex])
const original = buildLocaleLikeJson(fileIndex, 3600)
writeFileSync(absolutePath, modifyLocaleLikeJson(original, fileIndex))
}
runGit(repoPath, ['add', '-A'])
return { repoPath, relativePaths }
}