From 007dc994474f75f1ed418556f9a0bff4a805aef6 Mon Sep 17 00:00:00 2001 From: Christoph Brandau Date: Fri, 11 Sep 2026 16:58:52 +0200 Subject: [PATCH] feat(history): add visible-parent resolver and status optimizations Introduce an iterative visibleParentResolver for commit-graph rendering, replacing a recursive traversal. It preserves first-parent ordering, deduplicates converging paths, and avoids stack overflows; a test suite validates correctness and performance. Additionally, streamline background fetch handling in the UI to reuse fetchRemote's returned status and avoid unnecessary status reads and tab invalidation, and refactor Git status handling in Rust to build a file_status_index that preserves rename and first-match semantics while speeding lookups. - Add visibleParentResolver + comprehensive tests for ancestry handling - Reuse fetchRemote result to apply status and skip unchanged updates - Replace status_for_file with file_status_index to improve performance --- scripts/graph-parents.test.mjs | 51 ++++++++++++++++++++++++++ src-tauri/src/git.rs | 46 +++++++++++++++++------ src/App.svelte | 31 +++++++++------- src/lib/components/HistoryPanel.svelte | 24 ++---------- src/lib/graphParents.ts | 35 ++++++++++++++++++ 5 files changed, 141 insertions(+), 46 deletions(-) create mode 100644 scripts/graph-parents.test.mjs create mode 100644 src/lib/graphParents.ts diff --git a/scripts/graph-parents.test.mjs b/scripts/graph-parents.test.mjs new file mode 100644 index 0000000..012329c --- /dev/null +++ b/scripts/graph-parents.test.mjs @@ -0,0 +1,51 @@ +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import test from 'node:test'; +import ts from 'typescript'; +const source = readFileSync(new URL('../src/lib/graphParents.ts', import.meta.url), 'utf8'); +const compiled = ts.transpileModule(source, { compilerOptions: { module: ts.ModuleKind.ESNext, target: ts.ScriptTarget.ES2022 } }).outputText; +const { visibleParentResolver } = await import(`data:text/javascript;base64,${Buffer.from(compiled).toString('base64')}`); +function oldResolver(hash, visible, commits, seen = new Set()) { + if (visible.has(hash)) return [hash]; + if (seen.has(hash)) return []; + seen.add(hash); + const commit = commits.get(hash); + return commit ? [...new Set(commit.parents.flatMap(parent => oldResolver(parent, visible, commits, new Set(seen))))] : []; +} +test('preserves parent order and deduplicates converging paths', () => { + const items = [{hash:'tip',parents:['a','b']},{hash:'a',parents:['x','y']},{hash:'b',parents:['y','z']}]; + const resolve = visibleParentResolver(items,new Set(['x','y','z'])); + assert.deepEqual(resolve('tip'),['x','y','z']); + assert.deepEqual(resolve('missing'),[]); + assert.deepEqual(resolve('x'),['x']); +}); +test('matches previous traversal across deterministic merge DAGs and visibility filters', () => { + let seed=42; + const random=()=>((seed=(Math.imul(seed,1664525)+1013904223)>>>0)/2**32); + for(let run=0;run<80;run++) { + const items=Array.from({length:40},(_,i)=>({hash:String(i),parents:i===39?[]:[String(i+1),...(random()<.5?[String(i+1+Math.floor(random()*(39-i)))]:[])]})); + const visible=new Set(items.filter(()=>random()<.35).map(x=>x.hash)); + const resolve=visibleParentResolver(items,visible); + const map=new Map(items.map(x=>[x.hash,x])); + for(const item of items) assert.deepEqual(resolve(item.hash),oldResolver(item.hash,visible,map)); + } +}); +test('handles 20000 hidden ancestors without overflowing the call stack', () => { + const items=Array.from({length:20000},(_,i)=>({hash:String(i),parents:[String(i+1)]})); + assert.deepEqual(visibleParentResolver(items,new Set(['20000']))('0'),['20000']); +}); +test('shared merge ancestry is expanded only once', () => { + let reads=0; + const items=Array.from({length:30},(_,i)=>({hash:String(i),get parents(){reads++;return i===29?['root']:[String(i+1),String(Math.min(i+2,29))];}})); + const resolve=visibleParentResolver(items,new Set(['root'])); + assert.deepEqual(resolve('0'),['root']); + const initial=reads; + assert.deepEqual(resolve('1'),['root']); + assert.equal(reads,initial); + assert.ok(reads<300); +}); +const items=Array.from({length:24},(_,i)=>({hash:String(i),parents:i===23?['root']:[String(i+1),String(Math.min(i+2,23))]})); +const visible=new Set(['root']);const map=new Map(items.map(x=>[x.hash,x])); +const before=performance.now();oldResolver('0',visible,map);const oldMs=performance.now()-before; +const after=performance.now();visibleParentResolver(items,visible)('0');const newMs=performance.now()-after; +console.log(`Synthetic shared-ancestry benchmark (24 commits): old ${oldMs.toFixed(2)} ms, new ${newMs.toFixed(2)} ms`); diff --git a/src-tauri/src/git.rs b/src-tauri/src/git.rs index 29f1428..c498063 100644 --- a/src-tauri/src/git.rs +++ b/src-tauri/src/git.rs @@ -5756,6 +5756,7 @@ fn repository_files_with_status( repo: &Path, status: &GitStatus, ) -> Result, String> { + let status_index = file_status_index(&status.files); let mut files = BTreeMap::::new(); let tracked_output = run_git(repo, ["ls-files", "-z", "-t", "--cached", "--deleted"])?; @@ -5772,7 +5773,7 @@ fn repository_files_with_status( continue; } let path = String::from_utf8_lossy(path_bytes).into_owned(); - let status = status_for_file(&status.files, &path); + let status = status_index.get(path.as_str()).copied().flatten(); files.insert( path.clone(), GitRepositoryFile { @@ -5785,7 +5786,7 @@ fn repository_files_with_status( let untracked_output = run_git(repo, ["ls-files", "-z", "--others", "--exclude-standard"])?; for path in parse_nul_paths(&untracked_output) { - let status = status_for_file(&status.files, &path).or(Some(FileStatusKind::Untracked)); + let status = status_index.get(path.as_str()).copied().flatten().or(Some(FileStatusKind::Untracked)); files.insert( path.clone(), GitRepositoryFile { @@ -6151,16 +6152,23 @@ fn parse_nul_paths(output: &[u8]) -> Vec { .collect() } -fn status_for_file(statuses: &[GitFileStatus], path: &str) -> Option { - let status = find_status(statuses, path)?; - - if matches!(status.staged, Some(FileStatusKind::Conflicted)) - || matches!(status.unstaged, Some(FileStatusKind::Conflicted)) - { - return Some(FileStatusKind::Conflicted); +fn file_status_index(statuses: &[GitFileStatus]) -> std::collections::HashMap<&str, Option> { + let mut index = std::collections::HashMap::with_capacity(statuses.len()); + for status in statuses { + let kind = if matches!(status.staged, Some(FileStatusKind::Conflicted)) + || matches!(status.unstaged, Some(FileStatusKind::Conflicted)) + { + Some(FileStatusKind::Conflicted) + } else { + status.unstaged.or(status.staged) + }; + // Match find_status: the first record wins, including rename aliases. + index.entry(status.path.as_str()).or_insert(kind); + if let Some(old_path) = status.old_path.as_deref() { + index.entry(old_path).or_insert(kind); + } } - - status.unstaged.or(status.staged) + index } fn has_unresolved_conflicts(status: &GitStatus) -> bool { @@ -10115,6 +10123,22 @@ mod tests { assert_eq!(docs.replace("\r\n", "\n"), "docs2\n"); } + #[test] + fn file_status_index_preserves_rename_conflict_and_first_match_semantics() { + let statuses = vec![ + GitFileStatus { path: "new".into(), old_path: Some("old".into()), staged: Some(FileStatusKind::Renamed), unstaged: Some(FileStatusKind::Modified) }, + GitFileStatus { path: "old".into(), old_path: None, staged: Some(FileStatusKind::Added), unstaged: None }, + GitFileStatus { path: "conflict".into(), old_path: None, staged: Some(FileStatusKind::Conflicted), unstaged: Some(FileStatusKind::Modified) }, + GitFileStatus { path: "empty".into(), old_path: None, staged: None, unstaged: None }, + ]; + let index = file_status_index(&statuses); + assert_eq!(index.get("new"), Some(&Some(FileStatusKind::Modified))); + assert_eq!(index.get("old"), Some(&Some(FileStatusKind::Modified))); + assert_eq!(index.get("conflict"), Some(&Some(FileStatusKind::Conflicted))); + assert_eq!(index.get("empty"), Some(&None)); + assert_eq!(index.get("missing"), None); + } + #[test] fn repository_files_include_tracked_deleted_and_untracked_entries() { let repo = init_temp_repo("repository_files"); diff --git a/src/App.svelte b/src/App.svelte index 5f8dc94..7b064f2 100644 --- a/src/App.svelte +++ b/src/App.svelte @@ -912,18 +912,17 @@ const path = activeRepoPath; backgroundFetchInFlight = true; try { - let refsFetched = false; + let fetchedStatus: GitStatus | null = null; try { - await fetchRemote(path); - refsFetched = true; + fetchedStatus = await fetchRemote(path); } catch { // Manual Fetch/Pull surfaces remote errors; background work stays silent. } const notesFetched = await backgroundFetchCommitNotes(path); if (sameRepoPath(path, activeRepoPath)) { - if (refsFetched) { - applyStatus(await getStatus(path)); + if (fetchedStatus) { + applyStatus(fetchedStatus); await refreshRefsAndCommitGraph(path); } else if (notesFetched) { await refreshCommitHistory(path); @@ -944,17 +943,16 @@ backgroundFetchInFlight = true; try { - let refsFetched = false; + let fetchedStatus: GitStatus | null = null; try { - await fetchRemote(path); - refsFetched = true; + fetchedStatus = await fetchRemote(path); } catch { // Manual Fetch/Pull surfaces remote errors; background work stays silent. } const notesFetched = await backgroundFetchCommitNotes(path); - if (refsFetched) { - const nextStatus = await getStatus(path); + if (fetchedStatus) { + const nextStatus = fetchedStatus; if (sameRepoPath(path, activeRepoPath)) { applyStatus(nextStatus); await refreshRefsAndCommitGraph(path); @@ -995,6 +993,14 @@ lastOpened: openTab?.lastOpened ?? cached?.lastOpened ?? 0, }; + // Do not invalidate the dashboard or synchronously serialize the entire + // status cache when a background poll found no display changes. + const unchanged = (previous: RepoTab | undefined) => previous + && previous.branch === row.branch && previous.ahead === row.ahead + && previous.behind === row.behind && previous.changed === row.changed + && previous.lastOpened === row.lastOpened; + if (unchanged(cached) && (!openTab || unchanged(openTab))) return; + if (openTab) { repoTabs = repoTabs.map((tab) => sameRepoPath(tab.path, path) ? row : tab); } @@ -1019,10 +1025,7 @@ backgroundRepoStatusIndex += 1; try { - if (fetchFirst) { - await fetchRemote(path); - } - const nextStatus = await getStatus(path); + const nextStatus = fetchFirst ? await fetchRemote(path) : await getStatus(path); updateRepoManagementStatus(path, nextStatus); } catch { // ignore this repo — same rationale as the active-repo background fetch above diff --git a/src/lib/components/HistoryPanel.svelte b/src/lib/components/HistoryPanel.svelte index c863e5b..9f5e7ed 100644 --- a/src/lib/components/HistoryPanel.svelte +++ b/src/lib/components/HistoryPanel.svelte @@ -1,5 +1,6 @@