One graph as data: CommitGraph as the shared substrate for display and rebase#14623
One graph as data: CommitGraph as the shared substrate for display and rebase#14623mtsgrd wants to merge 2 commits into
Conversation
|
Thanks for spiking this! To my mind it's super valuable to show it can be done, and if I'd start a greenfield project, I'd also aim for keeping the data structure unified, and I'd also not have a segmented graph because overall it's not worth it (or there is not enough evidence that it is). Assuming this PR is good as it in every regard, I ran my final most gruesome test to check performance. The baseline for my nefarious gitlab repository in On this branch, the same run takes ~15s, most of which be accounted to the commit traversal. So once this PR is a candidate for merging, this would certainly be something to look into. Detailed runs
Baseline~700ms This branch~15s ❯ cargo run --release --bin but-debug -- -ttt -C ~/dev-gb/gitlab graph
Finished [`release` profile [optimized]](https://doc.rust-lang.org/cargo/reference/profiles.html#default-profiles) target(s) in 0.31s
Running `target/release/but-debug -ttt -C /Users/byron/dev-gb/gitlab graph`
Workspace(📕🏘️:0:gitbutler/workspace[🌳] <> ✓refs/remotes/origin/master⇣31609 on e9f20c7) {
id: 0,
kind: Managed {
ref_info: RefInfo {
ref_name: FullName(
"refs/heads/gitbutler/workspace",
),
commit_id: Some(
Sha1(bedfe4ffce8c3c224d9ee64891958866c68873c6),
),
worktree: Some(
Worktree {
kind: Main,
owned_by_repo: true,
},
),
},
},
stacks: [],
metadata: Some(
Workspace {
ref_info: RefInfo { created_at: "2023-01-31 14:55:57 +0000", updated_at: None },
stacks: [],
target_ref: "refs/remotes/origin/master",
target_commit_id: Sha1(e9f20c7c9e73ed04592e317f9021b39808f839d3),
push_remote: "origin",
},
),
target_ref: Some(
TargetRef {
ref_name: FullName(
"refs/remotes/origin/master",
),
segment_index: 47653,
commits_ahead: 31609,
},
),
target_commit: Some(
TargetCommit {
commit_id: Sha1(e9f20c7c9e73ed04592e317f9021b39808f839d3),
segment_index: 1,
},
),
}
INFO run [ 15.3s | 0.19% / 100.00% ]
INFO ┝━ ThreadSafeRepository::discover() [ 7.04ms | 0.01% / 0.05% ]
INFO │ ┕━ open_from_paths() [ 5.33ms | 0.03% / 0.03% ]
INFO │ ┕━ gix_odb::Store::at() [ 643µs | 0.00% ]
INFO ┝━ ThreadSafeRepository::open() [ 266µs | 0.00% / 0.00% ]
INFO │ ┕━ open_from_paths() [ 228µs | 0.00% / 0.00% ]
INFO │ ┕━ gix_odb::Store::at() [ 118µs | 0.00% ]
TRACE ┝━ new_ondemand_git2_repo [ 208ns | 0.00% ] gitdir: "/Users/byron/dev-gb/gitlab/.git"
TRACE ┝━ new_ondemand_db [ 84.0ns | 0.00% ] project_data_dir: "/Users/byron/dev-gb/gitlab/.git/gitbutler"
TRACE ┝━ new_ondemand_app_cache [ 42.0ns | 0.00% ] cache_dir: Some("/Users/byron/Library/Caches/com.gitbutler.app.dev") | cache_mode: Disk
DEBUG ┕━ Context::workspace_from_head [ 15.2s | 0.00% / 99.76% ]
DEBUG ┕━ Context::workspace_and_db_with_perm [ 15.2s | 0.02% / 99.76% ]
TRACE ┝━ DbHandle::new_at_path [ 2.78ms | 0.02% / 0.02% ]
TRACE │ ┕━ run_migration [ 198µs | 0.00% ]
TRACE ┝━ DbHandle::new_at_path [ 278µs | 0.00% / 0.00% ]
TRACE │ ┕━ run_migration [ 10.5µs | 0.00% ]
TRACE ┝━ Graph::from_commit_traversal [ 15.2s | 99.49% ] tip: Sha1(bedfe4ffce8c3c224d9ee64891958866c68873c6)
TRACE ┝━ Graph::into_workspace [ 36.0ms | 0.24% ]
WARN │ ┕━ 🚧 [warn]: Ignoring stack Some(RefInfo { ref_name: FullName("refs/heads/gitbutler/target"), commit_id: Some(Sha1(e9f20c7c9e73ed04592e317f9021b39808f839d3)), worktree: None }) (1) as it is pruned
TRACE ┕━ DbHandle::new_at_path [ 423µs | 0.00% / 0.00% ]
TRACE ┕━ run_migration [ 19.5µs | 0.00% ] |
|
@Byron thanks for flagging so early, I'll replicate and see if I can figure out what the material difference is(!) I'd also love to go over some of the details with you once I've had a chance to do another pass or two, I only just managed to tie it together end to end last night. |
bf8558e to
8b661d4
Compare
|
@Byron would you mind running that perf test again? It should have come down by quite a bit, albeit not fully matching the 0.6s. |
5799aaa to
20e94b5
Compare
c15b0d4 to
25c8616
Compare
06c1c78 to
d8d9ae7
Compare
25c8616 to
f80ff8c
Compare
a0b8f35 to
a5f91cf
Compare
|
I think it will be best if you had my test-setup, which is usually a local target branch that is far behind its remote counterpart. With the latest version from here, the issue is similar to what it was before, albeit improved at ~10s. In theory, traversing the entire commit-graph once (in my repo) can't take more than 1.5s even without commit-graph cache. So reaching 10s means it's another class of performance issue entirely. What makes the current implementation fast is:
UpdateIt turns out (and was already visible in the performance trace) that the time is spent in the segment graph conversion. Everything else is fast as it was before. And knowing that the segment graph is only used for visualization in SVGs and tests, it's completely non-critical. Also I am pretty sure the performance of this can be improved as well. So for me the only question left is why there are about 8k added lines of code - it should be about the same or less, ideally. Details
❯ cargo run --release --bin but-debug -- -ttt -C ~/dev-gb/gitlab graph
Finished [`release` profile [optimized]](https://doc.rust-lang.org/cargo/reference/profiles.html#default-profiles) target(s) in 0.36s
Running `target/release/but-debug -ttt -C /Users/byron/dev-gb/gitlab graph`
Workspace(📕🏘️:0:gitbutler/workspace[🌳] <> ✓refs/remotes/origin/master⇣31609 on e9f20c7) {
id: 0,
kind: Managed {
ref_info: RefInfo {
ref_name: FullName(
"refs/heads/gitbutler/workspace",
),
commit_id: Some(
Sha1(bedfe4ffce8c3c224d9ee64891958866c68873c6),
),
worktree: Some(
Worktree {
kind: Main,
owned_by_repo: true,
},
),
},
},
stacks: [],
metadata: Some(
Workspace {
ref_info: RefInfo { created_at: "2023-01-31 14:55:57 +0000", updated_at: None },
stacks: [],
target_ref: "refs/remotes/origin/master",
target_commit_id: Sha1(e9f20c7c9e73ed04592e317f9021b39808f839d3),
push_remote: "origin",
},
),
target_ref: Some(
TargetRef {
ref_name: FullName(
"refs/remotes/origin/master",
),
segment_index: 47653,
commits_ahead: 31609,
},
),
target_commit: Some(
TargetCommit {
commit_id: Sha1(e9f20c7c9e73ed04592e317f9021b39808f839d3),
segment_index: 1,
},
),
}
INFO run [ 10.4s | 0.28% / 100.00% ]
INFO ┝━ ThreadSafeRepository::discover() [ 7.48ms | 0.02% / 0.07% ]
INFO │ ┕━ open_from_paths() [ 5.64ms | 0.05% / 0.05% ]
INFO │ ┕━ gix_odb::Store::at() [ 784µs | 0.01% ]
INFO ┝━ ThreadSafeRepository::open() [ 270µs | 0.00% / 0.00% ]
INFO │ ┕━ open_from_paths() [ 221µs | 0.00% / 0.00% ]
INFO │ ┕━ gix_odb::Store::at() [ 114µs | 0.00% ]
TRACE ┝━ new_ondemand_git2_repo [ 208ns | 0.00% ] gitdir: "/Users/byron/dev-gb/gitlab/.git"
TRACE ┝━ new_ondemand_db [ 125ns | 0.00% ] project_data_dir: "/Users/byron/dev-gb/gitlab/.git/gitbutler"
TRACE ┝━ new_ondemand_app_cache [ 84.0ns | 0.00% ] cache_dir: Some("/Users/byron/Library/Caches/com.gitbutler.app.dev") | cache_mode: Disk
DEBUG ┕━ Context::workspace_from_head [ 10.4s | 0.00% / 99.65% ]
DEBUG ┕━ Context::workspace_and_db_with_perm [ 10.4s | 0.03% / 99.65% ]
TRACE ┝━ DbHandle::new_at_path [ 2.52ms | 0.02% / 0.02% ]
TRACE │ ┕━ run_migration [ 51.6µs | 0.00% ]
TRACE ┝━ DbHandle::open_existing_read_only_at_path [ 26.4µs | 0.00% ]
TRACE ┝━ DbHandle::new_at_path [ 217µs | 0.00% / 0.00% ]
TRACE │ ┕━ run_migration [ 11.1µs | 0.00% ]
TRACE ┝━ Graph::from_tip [ 10.4s | 0.16% / 99.39% ] seed: Sha1(bedfe4ffce8c3c224d9ee64891958866c68873c6)
TRACE │ ┝━ walker::traverse [ 640ms | 6.13% ]
TRACE │ ┝━ CommitGraph::from_walk_outcome [ 14.0ms | 0.13% ]
TRACE │ ┝━ enrichment_inputs [ 257ms | 2.46% ]
TRACE │ ┝━ gather_and_plan [ 30.9ms | 0.14% / 0.30% ]
TRACE │ │ ┝━ facts [ 9.14ms | 0.09% ]
TRACE │ │ ┕━ chain_plan [ 7.56ms | 0.07% ]
TRACE │ ┝━ graph_from_commit_graph [ 9.40s | 0.00% / 90.02% ] commits: 80086
TRACE │ │ ┕━ segment_data::build [ 9.40s | 90.02% ]
TRACE │ ┝━ apply_ad_hoc_orders [ 83.0ns | 0.00% ]
TRACE │ ┝━ derive_ref_positions [ 14.7ms | 0.14% ]
TRACE │ ┕━ render_segment_graph [ 3.50ms | 0.03% ]
TRACE ┝━ Graph::into_workspace [ 20.8ms | 0.20% ]
WARN │ ┕━ 🚧 [warn]: Ignoring stack Some(RefInfo { ref_name: FullName("refs/heads/gitbutler/target"), commit_id: Some(Sha1(e9f20c7c9e73ed04592e317f9021b39808f839d3)), worktree: None }) (1) as it is pruned
TRACE ┕━ DbHandle::new_at_path [ 431µs | 0.00% / 0.00% ]
TRACE ┕━ run_migration [ 19.4µs | 0.00% ]
gitbutler.commit-graph-experiment on commit-graph-experiment [$≡] took 10s
❯ but-debug -ttt -C ~/dev-gb/gitlab graph
Workspace(📕🏘️:0:gitbutler/workspace[🌳] <> ✓refs/remotes/origin/master⇣31609 on e9f20c7) {
id: 0,
kind: Managed {
ref_info: RefInfo {
ref_name: FullName(
"refs/heads/gitbutler/workspace",
),
commit_id: Some(
Sha1(bedfe4ffce8c3c224d9ee64891958866c68873c6),
),
worktree: Some(
Worktree {
kind: Main,
owned_by_repo: true,
},
),
},
},
stacks: [],
metadata: Some(
Workspace {
ref_info: RefInfo { created_at: "2023-01-31 14:55:57 +0000", updated_at: None },
stacks: [],
target_ref: "refs/remotes/origin/master",
target_commit_id: Sha1(e9f20c7c9e73ed04592e317f9021b39808f839d3),
push_remote: "origin",
},
),
target_ref: Some(
TargetRef {
ref_name: FullName(
"refs/remotes/origin/master",
),
segment_index: NodeIndex(1),
commits_ahead: 31609,
},
),
target_commit: Some(
TargetCommit {
commit_id: Sha1(e9f20c7c9e73ed04592e317f9021b39808f839d3),
segment_index: NodeIndex(3),
},
),
}
INFO run [ 686ms | 3.22% / 100.00% ]
TRACE ┝━ new_ondemand_git2_repo [ 375ns | 0.00% ] gitdir: "/Users/byron/dev-gb/gitlab/.git"
TRACE ┝━ new_ondemand_db [ 83.0ns | 0.00% ] project_data_dir: "/Users/byron/dev-gb/gitlab/.git/gitbutler"
TRACE ┝━ new_ondemand_app_cache [ 125ns | 0.00% ] cache_dir: Some("/Users/byron/Library/Caches/com.gitbutler.app.dev") | cache_mode: Disk
DEBUG ┕━ Context::workspace_from_head [ 664ms | 0.01% / 96.78% ]
DEBUG ┕━ Context::workspace_and_db_with_perm [ 664ms | 0.59% / 96.78% ]
TRACE ┝━ DbHandle::new_at_path [ 3.16ms | 0.45% / 0.46% ]
TRACE │ ┕━ run_migration [ 63.9µs | 0.01% ]
TRACE ┝━ DbHandle::open_existing_read_only_at_path [ 40.0µs | 0.01% ]
TRACE ┝━ DbHandle::new_at_path [ 374µs | 0.05% / 0.05% ]
TRACE │ ┕━ run_migration [ 14.4µs | 0.00% ]
TRACE ┝━ Graph::from_commit_traversal [ 651ms | 91.50% / 94.79% ] tip: Sha1(bedfe4ffce8c3c224d9ee64891958866c68873c6) | ref_name: "refs/heads/gitbutler/workspace"
TRACE │ ┕━ post_processed [ 22.6ms | 3.29% ]
WARN │ ┕━ 🚧 [warn]: Ignoring stack Some(RefInfo { ref_name: FullName("refs/heads/gitbutler/target"), commit_id: Some(Sha1(e9f20c7c9e73ed04592e317f9021b39808f839d3)), worktree: None }) (NodeIndex(3)) as it is pruned
TRACE ┝━ Graph::into_workspace [ 5.62ms | 0.82% ]
WARN │ ┕━ 🚧 [warn]: Ignoring stack Some(RefInfo { ref_name: FullName("refs/heads/gitbutler/target"), commit_id: Some(Sha1(e9f20c7c9e73ed04592e317f9021b39808f839d3)), worktree: None }) (NodeIndex(3)) as it is pruned
TRACE ┕━ DbHandle::new_at_path [ 369µs | 0.05% / 0.05% ]
TRACE ┕━ run_migration [ 17.8µs | 0.00% ] |
9f52d9a to
f599912
Compare
…isplay and rebase GitButler's core data structure is a git graph, and we had three languages for talking about it: but-graph's segment graph (minted mid-traversal, then ~10 repair passes fixing what earlier passes broke), but-rebase's petgraph StepGraph (rebuilt from the segment graph on every edit, with references as connectivity-bearing nodes), and the projection with hand-maintained cross-pointers. The same physical fact was encoded differently in each, joined by translation layers that drifted. This replaces them with one. The new model: a CommitGraph — an arena of commits where handles are the structure and ObjectIds are payload — walked once, then shared by display and rebase. Three design moves carry everything: - Stack order is the parent array. The workspace commit's parent order IS the stack order; the order-stacks post-pass, the parent_order metadata field, and the tied-order bug class are gone. - References are positions, not nodes. Refs live in a stored layout — per pick, ordered groups whose list structure IS the order — so excluding a commit excludes everything positioned on it. The connectivity-leak bug class (the stack-split collapse) dies here. - Rebase rewrites in place. A rebase overwrites commit-id payload while node ids, parent arrays, and positions survive; selectors stay valid across edits and the revision remapping machinery is deleted. The build is gather-then-build: facts (pure reads), then the plan (every boundary, chain, name, and placement decision as data), then segments authored final and installed as the graph's storage — nothing is repaired because nothing is built broken. The rebase editor adopts the arena wholesale and is created from the CommitGraph plus its stored ref layout alone; it never reads segments. After a rebase the mutated arena already IS the next workspace: previews and materialized refreshes project it directly instead of re-walking the repository. Deleted along the way, each behind an oracle or census: petgraph in both crates, every builder repair pass, the revision machinery, ref rank and below-pointers, reference nodes and their edge rules. The migration was verified throughout — env-gated tripwires and a 560-graph corpus census gated each repair-pass deletion, and mutate-then-project was required to equal rewalk-then-project on a field-exact projection fingerprint, suite-wide with zero snapshot churn. Costs, stated honestly: ~0.9s vs ~0.32s on the 371k-commit stress bench (hotspots understood, paths forward known); display still speaks segments (the projection's output shape is unchanged); and a handful of small, per-class reviewed behavior changes around upstream counts. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
f599912 to
2ca5695
Compare
The committed snapshots were captured against stale cached fixtures that predated the shared.sh workspace-commit rebuild fix; CI generates fixtures fresh and saw the stacks in workspace-parent order (A, B). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
I had Codex run an analysis on where the SLOC additions are located. I had it check for SegmentData is used as well, and it seems to still be in the critical path, i.e. is used for the workspace projection. Maybe that changes the next time this is pushed. Compared with local
The net growth mostly comes from:
Main source: Big buckets:
Largest individual net additions:
So the short answer: the ~8k net increase is mostly new |
|
Thanks @Byron! As a last piece I'll see if there's a way we can rely solely on CommiGraph and fold in the SegmentGraph type and related machinery. That should in theory get the line counts closer to parity. |
GitButler's core data structure is a git graph, and we had three different languages for talking about it. This branch replaces them with one. The write-up below is structured as: the new model → what it buys us → the world it replaces → the honest costs → where to start reading.
1. The new model: one graph as data
A
CommitGraph— an arena of commits where handles are the structure andObjectIds are payload, each node carrying an ordered parent array (a parent's slot IS its parent order), and references as positioned data, not nodes. One substrate, walked once, then shared by display and rebase:flowchart LR R[("repo")] -- walk --> CG["CommitGraph<br/>ordered parent arrays · refs as positioned data<br/>· the stored ref layout"] CG -- "facts → pure plan →<br/>segments authored final" --> SG["segment graph<br/>(a derived view)"] SG -- project --> WS["Workspace"] CG == "adopted as the editor's arena;<br/>refs ingested from the stored layout" ==> ED["rebase Editor"] ED == "mutate picks/refs;<br/>rebase rewrites commit ids IN PLACE" ==> CG CG -. "previews AND materialized refreshes<br/>project the mutated arena = the next workspace" .-> WSThe same object round-trips: walk → display → edit → project → display. The module tree speaks the pipeline's names:
walk→build→projection.Concretely:
CommitGraphisnodes: Vec<CommitNode>addressed by plainusizehandles; the structure isparent_slots— per node, an ordered slot array where the slot index IS the parent order — whileObjectIds are payload looked up through a rebuildableby_idindex,childrenis derived reverse adjacency, and deletion tombstones instead of removing, so handles are stable by construction. The build stores its ref layout here too — the plan's placement decisions (RefLayout) and the derived editor-grade positions (RefPositions) — so the decisions survive the build as data. The rebase editor adopts that arena wholesale (EditorGraphIndex::Node(i)IS arena indexi) and adds side tables:settings(per-pick rebase options),refs(names + flags), and its own layout — the editor-space mirror of the stored one.Three design moves carry everything:
parent_idsorder IS the stack order. The order-stacks post-pass, theparent_ordermetadata field, and the tied-order bug class are gone — the order is read straight off the merge commit. (Empty stacks — which have no parent-array presence yet — surface at their metadata index, the one place their position exists, and which the parent array mirrors once they gain commits.)RefGroups, each an ordered bottom→top run of references sharing oneCarry(which of the pick's incoming edges enter through the group). A ref'sbelowIS the previous group member and its rank IS its list index — order as list structure, never a stored number. Excluding a commit excludes everything positioned on it, because refs are not part of the reachability structure. Corpus-validated: groups are ≤3 deep, ~1.5k dup-parent cases need the parent-slot key.The build is gather-then-build: the builder first gathers facts from the CommitGraph (pure reads), then decides the entire plan as data — every boundary, chain, name, and remote claim — and only then authors the graph's
Segments directly, final at creation: names, extents, connections, endpoints. The finished graph installs them as its storage in one consuming step (position IS segment id), and the stored ref layout derives from those same authored segments — so display and rebase consume the same decisions instead of each re-deriving them. Nothing is repaired because nothing is built broken.Everything else is derived, not stored:
Factsand the plan are transient values, the segment graph is a view, and "stacks" and "branches" first exist as concepts in theWorkspaceprojection — the substrate, builder, and editor never store them. And the graph survives mutation: after a rebase the editor's arena — commit ids rewritten in place, refs repositioned — already IS the next workspace. Both halves of the mutation flow project it directly: the preview (overlayed_workspace) projects the not-yet-materialized arena with the pending ref edits served from an in-memory overlay, and the materialized refresh (Workspace::refresh_from_commit_graph) projects the same arena after the ref edits land on disk. No repository re-traversal in either case — a rewalk remains only as the fallback when the arena has nothing to project (unborn HEAD, or HEAD outside the editor's graph).2. What this buys us
but_graph::CommitGraph; positions, chains, and edges are the one ref model shared by mutation and (eventually) display. No translation layers, no id remapping.Segmentthe graph stores and the projection reads — the former build-side row table and its render-time translation are folded away.place_ref,transfer_stack,split_chain, …) rather than authoring node/edge surgery by hand.3. The world it replaces
To work on workspace or rebase code you had to hold three parallel models of the same git graph in your head. A commit was a segment member in but-graph, a petgraph node in but-rebase, and a projection entry with cached cross-pointers in the output — each with its own types, identity, and lifetime, joined by hand-maintained translation layers. The same physical fact ("X is the second parent of Y", "branch B sits on commit C") was encoded differently in each. That is more schema than a developer can keep in working memory: every fix had to be reasoned about in three vocabularies at once, and the layers drifted.
The three languages:
#1, with cached cross-pointers (sibling_segment_id,base_segment_id, storedgeneration) kept consistent by hand.flowchart LR R[("repo")] -- "walk mints segments<br/>mid-traversal" --> SG["segment graph"] SG -- "~10 ordered repair passes<br/>splits · reconciliation · normalization ·<br/>each fixing what earlier ones broke" --> SG SG -- project --> WS["Workspace"] SG -- "rebuilt from scratch<br/>on EVERY edit" --> SP["petgraph StepGraph<br/>refs as connectivity-bearing nodes"] SP -- rebase --> R R -. "full re-walk of the repository<br/>after every rebase" .-> SGThe load-bearing flaw: a ref node sits interposed on the path into its commit, so it bears connectivity. You cannot exclude a commit from a reachability computation without its ref nodes leaking connectivity around the hole. That one property was the common root of three bug classes, discovered independently:
parent_ordermetadata field plus an order-stacks post-pass — and ties broke differently depending on walk order.Each is the same failure: a fact encoded in structure — a post-pass order, a connectivity-bearing ref node, per-rebuild graph indices — instead of carried directly as data.
4. The honest costs
A bit of performance. On the 371k-commit stress bench (homebrew-cask,
clone --filter=blob:none+ commit-graph), master's architecture does ~0.32s; the rewrite does ~0.90s. Remaining hotspots: walker object decode (~308ms), mint-time commit clones (~77ms), facts (~57ms) — all with understood paths forward. This figure is after a scale pass triggered by a real-repo report (a ~700k-commit GitLab clone went 691ms → 15.2s, ~22×, inGraph::from_commit_traversal), which found two failure modes that fixture-sized tests can never surface, both fixed here:ObjectIdlookups through std SipHash maps plus commit-payload clones were ~77% of the build. Single-writerObjectId-keyed maps now usegix::hashtable(prehashed) and the clones are gone — 2.56s → 1.43s at the time of the fix.SegmentGraph::edges_directed(Incoming)scanned all segments per call; a maintained incoming-edge index makes it O(degree). The build path never hit it, but statistics/projection/ad-hoc queries at scale went from ~12min to ~1.5s.The durable lesson: graph changes get benched against 10⁵–10⁶-commit repos (
but-debug graph -C <repo> --no-debug-workspace --limit); unit fixtures cannot catch either class.Display still speaks segments. The row/segment duality is folded away (the build authors what the graph stores), but the projection's output shape is unchanged; a stacks-native projection remains possible follow-up work, as a restructure — never a parallel implementation.
A handful of intentional behavior changes — small and per-class reviewed: disjoint targets contribute no upstream counts; one limit-clipped fixture now counts all visible upstream commits; two tests deleted with the orphaned legacy queries they exercised; the legacy vb.toml reconcile now moves a branch head between stacks instead of duplicating it when the projection folds stacks (the duplicate could survive its own removal, stranding a phantom stack after pull).
A known-inherited walk-limit overrun (a stale local branch can drag a managed-workspace walk thousands of commits below the common base). Proven to exist on master too; deferred, not a regression.
5. Where to start reading
crates/but-graph/src/lib.rs— the pipeline overview; start here.crates/but-graph/src/commit_graph.rs— the substrate and the write-through seam.crates/but-graph/src/build/— the builder, one file per phase: facts / plan / materialize / segment_data / ref_positions (+ chains, remotes, ad_hoc);mod.rsholds the assemblers, and gather-then-build reads top to bottom.crates/but-graph/src/build/segment_data.rs— the authored segments and the final install.crates/but-graph/src/ref_layout.rs— the stored ref layout: the plan's placement decisions and the editor-grade positions (the display↔rebase handoff).crates/but-rebase/src/graph_rebase/mod.rs,creation.rs,positions.rs,materialize.rs— the editor, its creation from the CommitGraph + stored layout (no segment read), the reference-position model, and THE FLIP.crates/but-graph/tests/graph/walk/with_workspace.rs— what the output means.Supersedes #14452, whose work is folded into this branch.
Vocabulary
The same physical graph is spoken about in a few domains, and each term has one home — the code holds this line, so a word tells you which layer you're reading:
but-graph)ObjectIdis payloadbut-rebase)(child, slot)RefGroup)Carry)None(a root branching off the pick),All, or an explicit edge listbut-graph)Appendix: snapshot change review
Systematic review of all 251 changed snapshots (click to expand)
Snapshot review: a6b2916 → 2ca5695
Systematic review of every snapshot change on the branch, focused on ordering
changes rather than cosmetic churn.
Method
snapbox::str![...]body from both revisions and paired themby (enclosing test fn, occurrence index): 1640 paired snapshots.
:N:,stack-id braces
{N}, object hashes, trailing whitespace) so fixture regensdon't drown the signal. The diffs below display the real snapshot lines
(real hashes); only the diff alignment uses the normalized form.
below with its diff inline.
The ordering rule
The single invariant behind all ordering changes:
It manifests differently per renderer:
but status/ projection debug (≡📙:)git log --graph* | |)StackEntryJSON listsMutations now also write ws-commit parents in stack order, so parent order,
metadata order, and display order agree after every operation. Empty stacks
with no parent presence slot in at their metadata index.
The old rule was metadata order plus an order-stacks post-pass with
traversal-dependent tie-breaks — hence the many diffs where stacks swap places.
Regression coverage: the renamed tests
empty_move_display_order_follows_workspace_parentsandnon_empty_move_display_order_follows_workspace_parentspin the ruleexplicitly.
Evidence spot-checks:
three-stacks(squash.rs): octopus ws commit with parents (A, B, C) —git-log snapshot shows C, B, A top-to-bottom;
but statusshows A, B, C.two-stacks-one-empty(move2.rs): empty B survives as a real ws-commitparent —
git mergeused to silently drop it as an already-merged ancestor,so the shared fixture helper now rebuilds the ws commit with every argument
as a parent, matching how
butitself writes workspace commits.move_bottom_branch_to_top_of_another_stack(move_branch.rs): parents(C, A) → projection lists the C-stack first.
Fixtures build the same repositories as on the base revision — needless
argument flips were reverted, so every diff below reflects the new code, not a
changed fixture. The only fixture-side change is the helper fix above (empty
stacks kept as real ws-commit parents) plus a handful of new scenarios added
for new tests.
Goal bits: why
(⌂|1)became(⌂)The
|1suffix on commit lines is not the segment generation (that is the[N]bracket on segment lines, still displayed). It renders the walk'sgoal bits: spare
CommitFlagsbits the traversal assigns to "find thiscommit" targets — the entrypoint, the target tip, and remote/local
convergence seeds — and checks to decide when a walk side may stop.
propagated them down through ancestry, so after the walk nearly every
commit still carried the entrypoint's goal bit — hence
(⌂|1)everywhere.Limitpayloads;only commits where a goal flag is genuinely retained end up storing it.
Nothing reads goal bits from the finished graph.
The renderer is unchanged (
CommitFlags::debug_stringstill prints|bitswhen present) and current snapshots still show goal bits where commits carry
them — see
walk/with_workspace.rs,remote_name.rs,editor_creation.rs.The 41 diffs in this class are the same tests built with the same options;
they simply reflect that goal state is now transient walk bookkeeping instead
of leaking into the persistent graph.
Class verdicts
NodeIndex(N)→ ids/Sha1)(⌂|1)→(⌂))but statusno longer bails on a workspace without a managed commit; same fix as the row above, seen from the CLIplace_ref), not standalone steps, so merge commits render their own row; indent follows git's merge-row conventionNo anomalies found: every diff traces to one of the documented intentional
changes. Every one of the 251 diffs was reviewed individually against these
classes; the per-class explanations follow.
Per-class explanations
Stack display order = workspace-commit parent array (49)
What you see: stacks swap position in projection/debug output,
but status, andStackEntryJSON lists — the lines themselves are unchanged.Why: the old display order was workspace metadata order plus an order-stacks
post-pass whose ties were traversal-dependent: the walker's queue pops tips by
generation-then-committer-time, so a stack with younger commits could surface first,
and fixtures without workspace metadata got whatever the traversal produced (e.g.
disjoint_stacks_stay_separatelistedstack-bfirst purely because the B commitswere created later). The new projection reads the workspace-commit parent array,
first parent first — deterministic, recorded in git itself, and now also the order
mutations write. Empty stacks with no parent presence slot in at their metadata index.
Regression-pinned by the
*_display_order_follows_workspace_parentstests.Mutations write ws-commit parents in stack order (git-log lane swaps) (38)
What you see: in
git log --graphsnapshots, commits trade rails (* |↔| *)and branch blocks change vertical order; the commit set is identical.
Why: mutations now write workspace merge-commit parents in stack order. In git's
graph rendering the first parent keeps the leftmost rail while the last parent's
lane prints topmost, so parents (A, B, C) read C, B, A top-to-bottom. All fixtures
build the same repos as before — only the parent order recorded in newly written
workspace commits changed.
petgraph removal: NodeIndex(N) → plain ids / Sha1(hash) (71)
What you see:
NodeIndex(N)becomes a plain number, andlower_bound: NodeIndex(n)becomes the actual commitSha1(hash).Why: the petgraph-backed segment graph was deleted; debug output no longer leaks
petgraph's index type, and lower bounds are expressed as the commit they point to
rather than a segment index. Pure representation change — no ordering or content
semantics involved.
Walk goal-bits no longer stored on commits: (⌂|1) → (⌂) (41)
What you see: commit flag suffixes like
(⌂|1)lose their|1part.Why: the
|Nsuffix renders walk goal bits — spareCommitFlagsbits thetraversal assigns to "find this commit" targets (see the "Goal bits" section at the
top). The old walker stored these bits on commits and propagated them through
ancestry, so nearly every commit kept the entrypoint's goal bit after the walk. The
new walker keeps goal bookkeeping in queue
Limitpayloads; only commits thatgenuinely retain a goal flag store it, and nothing reads goal bits from the finished
graph. The renderer is unchanged and still prints goal bits where present
(
walk/with_workspace.rs,remote_name.rs,editor_creation.rs). Same tests, sameoptions — the snapshots just reflect the new engine's state.
Stack-split collapse fixed (shared excluded base no longer merges stacks) (5)
What you see: one
# Stackblock splits into two, and the target ref (e.g.main) disappears from inside stack listings.Why: the old projection merged two stacks into one when they only shared an
excluded base segment, and rendered the target as a trailing reference inside a
stack. Fixed: stacks that share nothing but excluded history stay separate, and the
target bounds stacks instead of appearing in them. Documented by the rename
divergent_stacks_sharing_base_merge_with_target→divergent_stacks_sharing_excluded_base_stay_separate_with_target.Managed workspace commit always present (3)
What you see: initial states that used to be a bare workspace ref (and a failing
but status, exit 1) now show a managedGitButler Workspace Commit, exit 0, andvalid JSON with
"stacks": [].Why: every branch-creation/workspace-entry path now persists the managed
workspace commit (the empty-stack-persistence constraint), so the workspace is
well-formed from the start instead of becoming valid only after the first commit.
Commands that failed now succeed (exit 1 → exit 0) (2)
What you see:
but statussnapshots flip fromstatus=exit status: 1withError: GitButler mode exit requiredtostatus=exit status: 0with valid JSON(
"stacks": []).Why: the flip side of "Managed workspace commit always present" above. These
tests capture status before the first commit; previously the workspace ref existed
without a managed workspace commit, which the CLI treated as a broken mode and bailed
out of. Now every workspace-entry path persists the managed commit, so the workspace
is well-formed from the start and status succeeds. A behavior fix, not a masked
failure — the same tests' later assertions are unchanged.
Editor debug renderer tweaks (11)
What you see:
◎reference lines in the rebase-editor ASCII graph changeindentation, and some merge commits gain a row (e.g.
asymmetric_merge_long_first_branch: old output went◎ refs/heads/mainstraight into
├─╮; new output shows◎ refs/heads/mainand then● 9999999before the split).
Why: the editor rework changed the model for refs. The old
StepGraphmade aref a standalone step node — merge edges hung directly off the ref, so the merge
commit it pointed at didn't exist as a node and the ref line absorbed the merge
row (and its wide indent). The new
EditorGraphplaces refs as attachments oncommits (
place_ref), so the commit is a real node rendered on its own row withthe ref line above it at plain width. Indentation follows git's convention: a
merge/fork row pads its hash to the post-split lane width (like git's
* hashabove
|\), while post-join rows use the collapsed width — consistent across allmerge snapshots in the file.
Unclassified — review individually (31)
These didn't auto-bucket because two effects overlap in one snapshot. Reviewed
individually, they decompose into the classes above: mostly stack-order swaps where
the CLI short-ids (
g0,h0, …) travel with the swap (ids are assigned by displayposition), plus a few diffs combining a reorder with an adjacent content change
(e.g. target exclusion or the managed ws commit). Nothing here introduces a class
of its own.