diff --git a/src/renderer/src/components/editor/CombinedDiffViewer.tsx b/src/renderer/src/components/editor/CombinedDiffViewer.tsx index 00fdc450b..f140c5b39 100644 --- a/src/renderer/src/components/editor/CombinedDiffViewer.tsx +++ b/src/renderer/src/components/editor/CombinedDiffViewer.tsx @@ -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).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(() => { return getCombinedBranchEntries(file.branchEntriesSnapshot, liveBranchEntries) }, [file.branchEntriesSnapshot, liveBranchEntries]) diff --git a/src/renderer/src/components/editor/combined-diff-entries.test.ts b/src/renderer/src/components/editor/combined-diff-entries.test.ts index 859cd92d8..c3ff00eca 100644 --- a/src/renderer/src/components/editor/combined-diff-entries.test.ts +++ b/src/renderer/src/components/editor/combined-diff-entries.test.ts @@ -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' }] diff --git a/src/renderer/src/components/editor/combined-diff-entries.ts b/src/renderer/src/components/editor/combined-diff-entries.ts index e27e50376..73d2fe44a 100644 --- a/src/renderer/src/components/editor/combined-diff-entries.ts +++ b/src/renderer/src/components/editor/combined-diff-entries.ts @@ -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() + 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 { + const entriesByPath = new Map() + 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): 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 } diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 4aa4d8ad8..36f73ea9a 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -1103,7 +1103,7 @@ "notePasteTooLarge": "笔记字段的粘贴内容过大。", "reuseExistingBranch": "复用分支", "reuseExistingBranchHint": "检出已有分支，而不是基于它创建新分支。", - "createMultiple": "Create more" + "createMultiple": "创建更多" }, "NewWorkspaceComposerModal": { "fa90f739a5": "创建工作区之前选择项目、工作区名称和 Agent。" diff --git a/tests/e2e/large-diff-freeze-repro.spec.ts b/tests/e2e/large-diff-freeze-repro.spec.ts index 2696e45e7..dc8292d54 100644 --- a/tests/e2e/large-diff-freeze-repro.spec.ts +++ b/tests/e2e/large-diff-freeze-repro.spec.ts @@ -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 { const repoId = await orcaPage.evaluate(async (pathToRepo: string) => { @@ -92,14 +67,6 @@ async function addAndActivateRepo(orcaPage: Page, repoPath: string): Promise { 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 }) + } + }) }) diff --git a/tests/e2e/large-diff-repro-fixtures.ts b/tests/e2e/large-diff-repro-fixtures.ts new file mode 100644 index 000000000..fe1f803a4 --- /dev/null +++ b/tests/e2e/large-diff-repro-fixtures.ts @@ -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 } +}