update-graph-overlay

[Caleb-T-Owens]

As I’ve worked on using the graph overlay for the rebase engine, I’ve ended up encountering some situations that haven’t been covered before.

Namly:

• What happens if a reference is specified multiple times in the overlays list.
• How can I express dropping a commit.

For the former, there are a couple approaches we can take.

• Define some precidence for duplicate references
• bail in some way if we see duplicate references defined
• Define some type that can’t have multiple references added

Of these, having some form of precidence seemed like the least invasive approach, so that is what I went with. Now, we take dropped references, then overrides from idx 0 to n, git references, and non-overrides from idx 0 to n.

[vercel]

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 7, 2026 10:31am

[Copilot]

Pull request overview

Adds more robust overlay behavior in but-graph to support rebase-engine scenarios where references can be duplicated in overlays and where references can be explicitly “dropped” from traversal results.

Changes:

• Add dropped_references support to Overlay and enforce it in reference lookup + ref collection.
• Define deterministic precedence for duplicate overlay references (first occurrence wins) and deduplicate refs across overlay/git/non-overriding sources.
• Add integration tests covering dropping refs (including HEAD ref) and duplicate override precedence.

Reviewed changes

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

File	Description
crates/but-graph/src/init/overlay.rs	Implements dropped-reference filtering, duplicate-reference precedence, and adjusts ref collection logic accordingly.
crates/but-graph/src/init/mod.rs	Extends Overlay struct with a dropped_references field and related docs.
crates/but-graph/tests/graph/init/overlay.rs	New tests validating dropped refs + duplicate override precedence behavior via snapshots.
crates/but-graph/tests/graph/init/mod.rs	Registers the new overlay test module.

[Copilot]

Pull request overview

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

[Copilot]

move_commit calls editor.rebase() twice (once up-front to build a Workspace from overlayed_graph(), and again at the end). Editor::rebase() performs a full step traversal + cherry-pick loop (even for identity outcomes), so this duplicates potentially expensive work. Consider avoiding the initial rebase by using the already-available workspace/projection (e.g., pass &Workspace into this function, or add an accessor on Editor/SuccessfulRebase that doesn’t require running the rebase engine).

Suggested change

	editor: Editor<'ws, 'meta, M>,
	subject_commit: impl ToCommitSelector,
	anchor: impl ToSelector,
	side: InsertSide,
	) -> anyhow::Result<SuccessfulRebase<'ws, 'meta, M>> {
	let sucessful_rebase = editor.rebase()?;
	let workspace = sucessful_rebase.overlayed_graph()?.into_workspace()?;
	let mut editor = sucessful_rebase.into_editor();
	mut editor: Editor<'ws, 'meta, M>,
	subject_commit: impl ToCommitSelector,
	anchor: impl ToSelector,
	side: InsertSide,
	) -> anyhow::Result<SuccessfulRebase<'ws, 'meta, M>> {
	let workspace = editor.overlayed_graph()?.into_workspace()?;

[Caleb-T-Owens]

In practice, it's always going to be an identity operation, and the graph overlay should be a no-op. As such, the performance impact should be minimal. This is however gaining us the assurance that we're operating on the right graph data no matter what was called before the operation, which is quite valuable.

[Copilot]

tear_off_branch calls editor.rebase() up-front solely to compute overlayed_graph()?.into_workspace(), and later calls editor.rebase() again to produce the returned SuccessfulRebase. Since Editor::rebase() runs the full rebase/cherry-pick loop, this doubles the amount of work for the operation. Consider restructuring so workspace context can be derived without running rebase() twice (e.g., pass &Workspace in, or expose a cheap accessor for the underlying workspace/projection).

[Byron]

I think being able to turn an edit back into a graph is fantastic, and even more so because it's all in memory.

It's a bit sad that we have to hack around the lack of in-memory reference edits in gix::Repository, but so be it.
It's also great to have a couple of tests for the overlay, which I think could stay even once gix::Repository can do in-memory reference edits for good measure.

(This was a very quick armchair review for me and I only looked at but-graph, knowing that Overlay is just a means to an end)

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

[Copilot]

In SuccessfulRebase::overlayed_graph, the Step::Reference arm binds id from Step::Pick(Pick { id, .. }) by reference (due to match ergonomics on &Step), but then returns it without dereferencing. This should be gix::ObjectId, not &gix::ObjectId, and will fail to compile. Dereference/copy the id (as done in the Step::Pick arm) before returning it.

Suggested change

	Some((id, Some(refname.clone())))
	Some((*id, Some(refname.clone())))