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
This commit is contained in:
2026-09-11 16:58:52 +02:00
parent 40b871f888
commit 007dc99447
5 changed files with 141 additions and 46 deletions
+51
View File
@@ -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`);
+35 -11
View File
@@ -5756,6 +5756,7 @@ fn repository_files_with_status(
repo: &Path, repo: &Path,
status: &GitStatus, status: &GitStatus,
) -> Result<Vec<GitRepositoryFile>, String> { ) -> Result<Vec<GitRepositoryFile>, String> {
let status_index = file_status_index(&status.files);
let mut files = BTreeMap::<String, GitRepositoryFile>::new(); let mut files = BTreeMap::<String, GitRepositoryFile>::new();
let tracked_output = run_git(repo, ["ls-files", "-z", "-t", "--cached", "--deleted"])?; let tracked_output = run_git(repo, ["ls-files", "-z", "-t", "--cached", "--deleted"])?;
@@ -5772,7 +5773,7 @@ fn repository_files_with_status(
continue; continue;
} }
let path = String::from_utf8_lossy(path_bytes).into_owned(); 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( files.insert(
path.clone(), path.clone(),
GitRepositoryFile { GitRepositoryFile {
@@ -5785,7 +5786,7 @@ fn repository_files_with_status(
let untracked_output = run_git(repo, ["ls-files", "-z", "--others", "--exclude-standard"])?; let untracked_output = run_git(repo, ["ls-files", "-z", "--others", "--exclude-standard"])?;
for path in parse_nul_paths(&untracked_output) { 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( files.insert(
path.clone(), path.clone(),
GitRepositoryFile { GitRepositoryFile {
@@ -6151,16 +6152,23 @@ fn parse_nul_paths(output: &[u8]) -> Vec<String> {
.collect() .collect()
} }
fn status_for_file(statuses: &[GitFileStatus], path: &str) -> Option<FileStatusKind> { fn file_status_index(statuses: &[GitFileStatus]) -> std::collections::HashMap<&str, Option<FileStatusKind>> {
let status = find_status(statuses, path)?; let mut index = std::collections::HashMap::with_capacity(statuses.len());
for status in statuses {
if matches!(status.staged, Some(FileStatusKind::Conflicted)) let kind = if matches!(status.staged, Some(FileStatusKind::Conflicted))
|| matches!(status.unstaged, Some(FileStatusKind::Conflicted)) || matches!(status.unstaged, Some(FileStatusKind::Conflicted))
{ {
return 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);
}
} }
index
status.unstaged.or(status.staged)
} }
fn has_unresolved_conflicts(status: &GitStatus) -> bool { fn has_unresolved_conflicts(status: &GitStatus) -> bool {
@@ -10115,6 +10123,22 @@ mod tests {
assert_eq!(docs.replace("\r\n", "\n"), "docs2\n"); 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] #[test]
fn repository_files_include_tracked_deleted_and_untracked_entries() { fn repository_files_include_tracked_deleted_and_untracked_entries() {
let repo = init_temp_repo("repository_files"); let repo = init_temp_repo("repository_files");
+17 -14
View File
@@ -912,18 +912,17 @@
const path = activeRepoPath; const path = activeRepoPath;
backgroundFetchInFlight = true; backgroundFetchInFlight = true;
try { try {
let refsFetched = false; let fetchedStatus: GitStatus | null = null;
try { try {
await fetchRemote(path); fetchedStatus = await fetchRemote(path);
refsFetched = true;
} catch { } catch {
// Manual Fetch/Pull surfaces remote errors; background work stays silent. // Manual Fetch/Pull surfaces remote errors; background work stays silent.
} }
const notesFetched = await backgroundFetchCommitNotes(path); const notesFetched = await backgroundFetchCommitNotes(path);
if (sameRepoPath(path, activeRepoPath)) { if (sameRepoPath(path, activeRepoPath)) {
if (refsFetched) { if (fetchedStatus) {
applyStatus(await getStatus(path)); applyStatus(fetchedStatus);
await refreshRefsAndCommitGraph(path); await refreshRefsAndCommitGraph(path);
} else if (notesFetched) { } else if (notesFetched) {
await refreshCommitHistory(path); await refreshCommitHistory(path);
@@ -944,17 +943,16 @@
backgroundFetchInFlight = true; backgroundFetchInFlight = true;
try { try {
let refsFetched = false; let fetchedStatus: GitStatus | null = null;
try { try {
await fetchRemote(path); fetchedStatus = await fetchRemote(path);
refsFetched = true;
} catch { } catch {
// Manual Fetch/Pull surfaces remote errors; background work stays silent. // Manual Fetch/Pull surfaces remote errors; background work stays silent.
} }
const notesFetched = await backgroundFetchCommitNotes(path); const notesFetched = await backgroundFetchCommitNotes(path);
if (refsFetched) { if (fetchedStatus) {
const nextStatus = await getStatus(path); const nextStatus = fetchedStatus;
if (sameRepoPath(path, activeRepoPath)) { if (sameRepoPath(path, activeRepoPath)) {
applyStatus(nextStatus); applyStatus(nextStatus);
await refreshRefsAndCommitGraph(path); await refreshRefsAndCommitGraph(path);
@@ -995,6 +993,14 @@
lastOpened: openTab?.lastOpened ?? cached?.lastOpened ?? 0, 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) { if (openTab) {
repoTabs = repoTabs.map((tab) => sameRepoPath(tab.path, path) ? row : tab); repoTabs = repoTabs.map((tab) => sameRepoPath(tab.path, path) ? row : tab);
} }
@@ -1019,10 +1025,7 @@
backgroundRepoStatusIndex += 1; backgroundRepoStatusIndex += 1;
try { try {
if (fetchFirst) { const nextStatus = fetchFirst ? await fetchRemote(path) : await getStatus(path);
await fetchRemote(path);
}
const nextStatus = await getStatus(path);
updateRepoManagementStatus(path, nextStatus); updateRepoManagementStatus(path, nextStatus);
} catch { } catch {
// ignore this repo — same rationale as the active-repo background fetch above // ignore this repo — same rationale as the active-repo background fetch above
+3 -21
View File
@@ -1,5 +1,6 @@
<script lang="ts"> <script lang="ts">
import { Cherry, ChevronDown, ChevronRight, CloudOff, EllipsisVertical, GitBranch, GitMerge, LoaderCircle, RotateCcw, StickyNote, Tag, X } from "@lucide/svelte"; import { Cherry, ChevronDown, ChevronRight, CloudOff, EllipsisVertical, GitBranch, GitMerge, LoaderCircle, RotateCcw, StickyNote, Tag, X } from "@lucide/svelte";
import { visibleParentResolver } from "../graphParents";
import type { FileStatusKind, GitCommit, GitCommitFile } from "../types"; import type { FileStatusKind, GitCommit, GitCommitFile } from "../types";
interface GraphSegment { interface GraphSegment {
@@ -338,25 +339,6 @@
return branchesAreVisible(row?.branchLabels ?? []); return branchesAreVisible(row?.branchLabels ?? []);
} }
function nearestVisibleGraphParents(
hash: string,
visibleHashes: Set<string>,
commitByHash: Map<string, GitCommit>,
seen: Set<string>,
): string[] {
if (visibleHashes.has(hash)) return [hash];
if (seen.has(hash)) return [];
seen.add(hash);
const commit = commitByHash.get(hash);
if (!commit) return [];
return uniqueStrings(
commit.parents.flatMap((parentHash) => (
nearestVisibleGraphParents(parentHash, visibleHashes, commitByHash, new Set(seen))
)),
);
}
function branchMembershipByHash(items: GitCommit[]): Map<string, string[]> { function branchMembershipByHash(items: GitCommit[]): Map<string, string[]> {
const commitByHash = new Map(items.map((commit) => [commit.hash, commit])); const commitByHash = new Map(items.map((commit) => [commit.hash, commit]));
const membership = new Map<string, Set<string>>(); const membership = new Map<string, Set<string>>();
@@ -395,12 +377,12 @@
function visibleCommitEntriesForGraph(items: GitCommit[], branchMembership: Map<string, string[]>): VisibleCommitEntry[] { function visibleCommitEntriesForGraph(items: GitCommit[], branchMembership: Map<string, string[]>): VisibleCommitEntry[] {
const visibleItems = items.filter((commit) => branchesAreVisible(branchMembership.get(commit.hash) ?? [])); const visibleItems = items.filter((commit) => branchesAreVisible(branchMembership.get(commit.hash) ?? []));
const visibleHashes = new Set(visibleItems.map((commit) => commit.hash)); const visibleHashes = new Set(visibleItems.map((commit) => commit.hash));
const commitByHash = new Map(items.map((commit) => [commit.hash, commit])); const resolveParents = visibleParentResolver(items, visibleHashes);
return visibleItems.map((commit) => { return visibleItems.map((commit) => {
const parents = uniqueStrings( const parents = uniqueStrings(
commit.parents.flatMap((parentHash) => ( commit.parents.flatMap((parentHash) => (
nearestVisibleGraphParents(parentHash, visibleHashes, commitByHash, new Set()) resolveParents(parentHash)
)), )),
); );
return { commit, graphCommit: { ...commit, parents } }; return { commit, graphCommit: { ...commit, parents } };
+35
View File
@@ -0,0 +1,35 @@
interface ParentCommit { hash: string; parents: string[] }
// Resolve hidden ancestry once per graph, without recursion or copying a visited
// set for every path through a merge. Preserve Git's first-parent ordering.
export function visibleParentResolver(items: ParentCommit[], visibleHashes: Set<string>) {
const commits = new Map(items.map(commit => [commit.hash, commit]));
const cache = new Map<string, string[]>();
return (hash: string): string[] => {
const visiting = new Set<string>();
const stack: { hash: string; expanded: boolean }[] = [{ hash, expanded: false }];
while (stack.length) {
const frame = stack.pop()!;
if (cache.has(frame.hash)) continue;
if (visibleHashes.has(frame.hash)) {
cache.set(frame.hash, [frame.hash]);
continue;
}
const commit = commits.get(frame.hash);
if (!commit) { cache.set(frame.hash, []); continue; }
if (frame.expanded) {
cache.set(frame.hash, [...new Set(commit.parents.flatMap(parent => cache.get(parent) ?? []))]);
visiting.delete(frame.hash);
} else {
if (visiting.has(frame.hash)) continue;
visiting.add(frame.hash);
stack.push({ hash: frame.hash, expanded: true });
for (let index = commit.parents.length - 1; index >= 0; index--) {
const parent = commit.parents[index];
if (!visiting.has(parent)) stack.push({ hash: parent, expanded: false });
}
}
}
return cache.get(hash) ?? [];
};
}