commit ordering by estib-vega · Pull Request #13153 · gitbutlerapp/gitbutler

estib-vega · 2026-04-02T00:45:55Z

Create but-workspace commit ordering function that sorts the commits by
their parentage, according to the workspace appearance.

Cool, but why?

In order to enable graph mutations with multiple commits, I need to sort the commits from parent-most to child-most.
That way I can do one operation at a time in memory, parent to child. And then materialize it at the very end.

Ordering should reduce the probability of running into conflicts.

Disclaimer

This heavily uses AI for the generation of this algorithm

Comprehensive docs

https://github.com/gitbutlerapp/gitbutler/pull/13153/changes#diff-bb49765eeedd900a433bba2fc60faf0d82730392d9053af079c8c1d2407aa905

vercel · 2026-04-02T00:45:58Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
gitbutler-web	Ignored	Preview	Apr 2, 2026 5:58pm

Copilot

Pull request overview

Adds a deterministic commit-ordering utility to but-workspace that sorts selected commits parent-first (topological order) and uses workspace traversal order as the tie-breaker for unrelated commits—intended to support multi-commit in-memory graph mutations with fewer conflicts.

Changes:

Introduce but_workspace::ordering::order_commit_selectors_by_parentage (new ordering module).
Add integration tests covering linear chains, disjoint commits, mixed ancestry, deduplication, and singleton behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
crates/but-workspace/src/lib.rs	Exposes the new `ordering` module from the crate root.
crates/but-workspace/src/ordering/mod.rs	Defines the `ordering` module and re-exports the ordering function.
crates/but-workspace/src/ordering/commit_parentage.rs	Implements parentage-based ordering with workspace-order tie-breaks.
crates/but-workspace/tests/workspace/main.rs	Registers the new `ordering` test module.
crates/but-workspace/tests/workspace/ordering/mod.rs	Wires up commit-parentage ordering tests.
crates/but-workspace/tests/workspace/ordering/commit_parentage.rs	Adds test coverage for the ordering behavior and invariants.

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Caleb-T-Owens · 2026-04-02T18:15:46Z

+        }
+    }
+
+    let merge_base = match editor.repo().merge_base(left.id, right.id) {


Something I’ve been thinking about in some of my graph overlay work is how to reconcile the fact that a pick statement’s OIDs may not imply the same history as the commit graph. For example, with editor.insert_below(target, pick, below), the target will still have the same OID, but the editor graph itself has changed relative to what git would be telling you.

In most cases, that divergence should be fine. However, if more drastic operations are being performed, such as reparenting commits, it could lead to unexpected issues.

In https://github.com/gitbutlerapp/gitbutler/pull/13080, I’ve actually removed public access to the workspace property from the editor, because it always reflects the version of the workspace at the time the editor was created and therefore isn’t representative of the current graph state.

I’ve also modified the functions that were using it so that they call .rebase() and retrieve the latest overlay graph from SuccessfulRebase. That means that when we call into_editor again, we can work with the most up-to-date graph, aligned with the editor’s current state.

I’d love @Byron’s thoughts on this, but I’ve been wondering more and more whether it would be helpful to have some graph-inspection operations that run specifically on the editor’s step graph, so we can find things like merge bases there as well.

It would mean an additional implementation of logic similar to what already exists in but_graph, but it would also let us operate without the same desync concerns.

Perhaps I’m missing some context here, though. Do let me know.

I’d love @Byron’s thoughts on this, but I’ve been wondering more and more whether it would be helpful to have some graph-inspection operations that run specifically on the editor’s step graph, so we can find things like merge bases there as well.

It would mean an additional implementation of logic similar to what already exists in but_graph, but it would also let us operate without the same desync concerns.

@Caleb-T-Owens That's an interesting problem. The 'safest' way seems to be to provide workspace() or graph() so that the editor can ensure the graph is representing the latest edits. While its more expensive, I do fear de-sync a bit, but ultimately would also trust that it's probably fine. The but-graph version of the merge-base implementation is generated based on the implementation in gix, which was hand-implemented based on Git :D.
However, my preference right now is to do the super-simple and explicit thing that makes the data structure clear, i.e. editor.workspace().graph.merge_base…(). @estib-vega I'd also think that a method could be added that returns CommitIDs instead of SegmentIDs.

Byron

I only looked at it from an organizational perspective and think changes have to be made.
My second feeling is that but-rebase should probably have more native support for at least parts of what needs to be accomplished here, but it's nothing more than that (a feeling) either.

Byron · 2026-04-02T23:37:57Z

There is actually a way to use this file as documentation for the Rust module itself by inclusion, tower does that a lot. That's definitely something I'd use to not have a loose file lying around in the source tree.

Byron · 2026-04-02T23:39:14Z

 pub mod ui;

+/// Utilities for deterministic ordering operations.
+pub mod ordering;


I'd definitely prefer this module to be in commit as it's about commit ordering.

Hmmm maybe. The motivation is that this will be used by branch & by commits, and any API that should be able to take multiple commits as input parameters.

If we later need branch ordering (e.g. for moving multiple branches at a time) we could add that method there. Even if we're ordering commits behind the scenes

Ah, I see, maybe it's more about branches then? It's just that when I see but_workspace::ordering, I think of the ordering of workspaces. Is there any place that is more specific but still fitting?

This could be maybe also part of but-graph? Maybe we let the graph sort things since it should have all the information there. Merge bases and topology information.

If you think that would work, I am all for it. My question would be if it's better in the step-graph though as it is related to mutation, and it's technically a different graph which may (or may not) be better suited for this kind of thing.

In any case, it seems like this can move elsewhere then 🎉.

Byron · 2026-04-02T23:41:56Z

+pub(crate) fn find_commit_segment_index(
+    workspace: &Workspace,
+    commit_id: gix::ObjectId,
+) -> Option<SegmentIndex> {


This should certainly be in but-graph, given that it provides a way to find the actual segment owning a commit by ObjectId in a workspace. This is quite under-used right now, so a simple way to do this is missing.

Yep, that's true

Create but-workspace commit ordering function that sorts the commits by their parentage, according to the workspace appearance.

estib-vega · 2026-04-15T09:27:55Z

Update:

The function no longer uses the workspace at all.
The ordering function lives in but-graph and is accessible from the editor.

Addressed other feedback as well regarding the docs and where the find_commit_segment_index (accessible through the workspace).

github-actions bot added the rust Pull requests that update Rust code label Apr 2, 2026

estib-vega requested a review from Copilot April 2, 2026 00:46

Copilot started reviewing on behalf of estib-vega April 2, 2026 00:46 View session

Copilot AI reviewed Apr 2, 2026

View reviewed changes

Comment thread crates/but-workspace/src/ordering/commit_parentage.rs Outdated

Comment thread crates/but-workspace/src/ordering/commit_parentage.rs Outdated

Comment thread crates/but-workspace/src/ordering/commit_parentage.rs Outdated

estib-vega force-pushed the commit-ordering branch 3 times, most recently from 4c46bce to e916590 Compare April 2, 2026 17:24

estib-vega marked this pull request as ready for review April 2, 2026 17:24

estib-vega requested review from Byron, Caleb-T-Owens and krlvi as code owners April 2, 2026 17:24

Copilot AI review requested due to automatic review settings April 2, 2026 17:24

Copilot started reviewing on behalf of estib-vega April 2, 2026 17:24 View session

Copilot AI reviewed Apr 2, 2026

View reviewed changes

Comment thread crates/but-workspace/src/ordering/commit_parentage.rs Outdated

Comment thread crates/but-workspace/src/commit/squash_commits.rs Outdated

Comment thread crates/but-workspace/src/commit/mod.rs

Comment thread crates/but-workspace/src/ordering/commit_parentage.rs Outdated

estib-vega force-pushed the commit-ordering branch from e916590 to b91d1b3 Compare April 2, 2026 17:47

Copilot AI review requested due to automatic review settings April 2, 2026 17:58

estib-vega force-pushed the commit-ordering branch from b91d1b3 to 9aa529c Compare April 2, 2026 17:58

Copilot started reviewing on behalf of estib-vega April 2, 2026 17:59 View session

Copilot AI reviewed Apr 2, 2026

View reviewed changes

Comment thread crates/but-workspace/src/ordering/commit_parentage.rs Outdated

Comment thread crates/but-workspace/src/ordering/commit_parentage.md Outdated

Caleb-T-Owens reviewed Apr 2, 2026

View reviewed changes

Byron reviewed Apr 2, 2026

View reviewed changes

commit ordering

07e88ef

Create but-workspace commit ordering function that sorts the commits by their parentage, according to the workspace appearance.

estib-vega force-pushed the commit-ordering branch from 9aa529c to 07e88ef Compare April 15, 2026 08:49

Conversation

estib-vega commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Cool, but why?

Disclaimer

Comprehensive docs

Uh oh!

vercel bot commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Caleb-T-Owens Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Byron left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

estib-vega commented Apr 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

estib-vega commented Apr 2, 2026 •

edited

Loading

vercel bot commented Apr 2, 2026 •

edited

Loading

Caleb-T-Owens Apr 2, 2026 •

edited

Loading