TUI not committing/amending all hunks

[davidpdrsn]

I noticed that if you used the TUI to

• Create a new commit from the unassigned changes
• or amend an existing commit using the but-api based rub (shift+r)

then sometimes not all hunks would get committed and some would be left unassigned.

This PR contains an attempt at fixing it but doesn't feel like its the right fix. It feels too local and I'm wondering if there is something wrong in but-api or perhaps there is some shared APIs thats missing.

For context here is how the TUI is finding the unassigned stuff to commit.

[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	Mar 20, 2026 7:31pm

[davidpdrsn]

Building the DiffSpec in this way seems to fix it but feels wrong to just do this locally here.

[Copilot]

Pull request overview

This PR addresses cases where creating/amending commits from the TUI (including rub/Shift+R flows) leaves some hunks uncommitted by changing how selected hunk assignments are converted into DiffSpecs, and adds regression tests covering multi-hunk modified files.

Changes:

• Add a TUI helper to group multiple hunk assignments from the same file into a single DiffSpec.
• Update TUI commit and rub-api paths to use the grouped DiffSpec generation.
• Add new TUI integration tests to ensure multi-hunk modified files are fully committed/amended.

Reviewed changes

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

File	Description
crates/but/src/command/legacy/status/tui/mod.rs	Introduces diff_specs_from_hunk_assignments and switches commit creation to use grouped diff specs.
crates/but/src/command/legacy/status/tui/rub_api.rs	Uses the new grouped diff-spec conversion for amend operations driven by rub API.
crates/but/src/command/legacy/status/tui/tests/commit_tests.rs	Adds a regression test for committing a multi-hunk modified file from unassigned changes.
crates/but/src/command/legacy/status/tui/tests/rub_tests.rs	Adds regression tests for Shift+R amend flows ensuring all hunks of a modified file are committed.

[Copilot]

This introduces a local diff-spec grouping helper, but the repo already has shared utilities for this (e.g. but_workspace::flatten_diff_specs and but_hunk_assignment::convert_assignments_to_diff_specs). Reusing one of those (or moving this logic there) would avoid duplicated behavior and keep all commit/amend call sites consistent.

Suggested change

	let mut grouped = BTreeMap::<BString, Vec<but_core::HunkHeader>>::new();
	for assignment in assignments {
	let file_hunks = grouped.entry(assignment.path_bytes).or_default();
	if let Some(hunk_header) = assignment.hunk_header {
	let hunk_header = but_core::HunkHeader {
	old_start: hunk_header.old_start,
	old_lines: hunk_header.old_lines,
	new_start: hunk_header.new_start,
	new_lines: hunk_header.new_lines,
	};
	if !file_hunks.contains(&hunk_header) {
	file_hunks.push(hunk_header);
	}
	}
	}
	grouped
	.into_iter()
	.map(|(path, hunk_headers)| DiffSpec {
	previous_path: None,
	path,
	hunk_headers,
	})
	.collect()
	// Delegate to the shared helper to keep diff-spec construction consistent
	// across the codebase and avoid duplicating grouping logic here.
	but_hunk_assignment::convert_assignments_to_diff_specs(assignments)

[Copilot]

This test performs the initial commit flow without asserting that commit mode was entered and that the commit actually happened. If the UI state changes, the test could become a false positive (the later amend might be operating on an untracked file, not a tracked multi-hunk modification). Add assertions around the initial c/Down/Enter sequence (or verify via git status/rev-parse) to ensure the file is committed before creating the multi-hunk modification.

Suggested change

	let last_commit_message = tui.env.invoke_git("log -1 --pretty=%B");
	assert!(
	last_commit_message.contains("commit from tui test"),
	"expected initial commit to be created by TUI with message written by editor.sh, \
	but last commit message was:\n{last_commit_message}"
	);
	let status_after_initial_commit = tui.env.invoke_git("status --porcelain");
	assert!(
	!status_after_initial_commit
	.lines()
	.any(|line| line.ends_with("multi-hunk.txt")),
	"after initial commit flow, multi-hunk.txt should be clean and committed\nstatus was:\n{status_after_initial_commit}"
	);

[Copilot]

This test performs the initial commit flow without asserting that commit mode was entered and that the commit actually happened. If the UI state changes, the test could become a false positive (the later amend might be operating on an untracked file, not a tracked multi-hunk modification). Add assertions around the initial c/Down/Enter sequence (or verify via git status/rev-parse) to ensure the file is committed before creating the multi-hunk modification.

Suggested change

	let status_after_initial_commit = tui.env.invoke_git("status --porcelain");
	assert!(
	!status_after_initial_commit
	.lines()
	.any(|line| line.ends_with("multi-hunk.txt")),
	"after initial commit flow, multi-hunk.txt should be committed and clean\nstatus was:\n{status_after_initial_commit}"
	);

[Copilot]

This test performs the initial commit flow without asserting that commit mode was entered and that the commit actually happened. If the UI state changes, the test could become a false positive (the later amend might be operating on an untracked file, not a tracked multi-hunk modification). Add assertions around the initial c/Down/Enter sequence (or verify via git status/rev-parse) to ensure the file is committed before creating the multi-hunk modification.

Suggested change

	tui.input_then_render('c');
	tui.input_then_render(KeyCode::Down);
	with_var("GIT_EDITOR", Some(editor_command), || {
	tui.input_then_render(KeyCode::Enter);
	});
	let old_head = tui.env.invoke_git("rev-parse HEAD");
	tui.input_then_render('c');
	tui.input_then_render(KeyCode::Down);
	with_var("GIT_EDITOR", Some(editor_command), || {
	tui.input_then_render(KeyCode::Enter);
	});
	let new_head = tui.env.invoke_git("rev-parse HEAD");
	assert_ne!(
	old_head, new_head,
	"initial branch->commit operation should create a new commit"
	);
	let status_after_initial_commit = tui.env.invoke_git("status --porcelain");
	assert!(
	status_after_initial_commit.trim().is_empty(),
	"working tree should be clean after initial commit, but status was:\n{status_after_initial_commit}"
	);

[Copilot]

DiffSpec.hunk_headers needs to be in ascending order for hunk application (e.g. but_core::apply_hunks/to_additive_hunks assume ordered selections). This helper currently preserves whatever order the input iterator yields (which can be user-provided and out-of-order), so multi-hunk specs can fail or drop hunks. Sort the per-file hunk_headers before constructing the DiffSpec (or collect into an ordered set).

Suggested change

	for file_hunks in grouped.values_mut() {
	file_hunks.sort_by_key(|h| (h.old_start, h.old_lines, h.new_start, h.new_lines));
	}