Make interdiff rebase-aware by recording a base per revision #3

New issue

Closed

opened 2026-08-09 16:27:40 +00:00 by a73x · 2 comments

a73x commented

2026-08-09 16:27:40 +00:00

(Migrated from git.a73x.sh)

Spec: docs/superpowers/specs/2026-08-09-patch-revision-refs-design.md

interdiff (patch.rs:501-525) is a flat tree-to-tree diff between two revision trees with no base awareness. When a rebase separates the revisions, the result includes every upstream commit rebased over, burying the author's actual response to review.

Single-revision diff does not have this problem - resolve_base_tree (patch.rs:454-466) three-dots against the current base branch. Only the interdiff path needs fixing.

Fix:

Move base onto Revision as merge_base(<base_ref tip>, ), computed when the revision is recorded and stored. Merge-base rather than branch tip, so base moves only when the author actually rebases.
Drop PatchState.base_commit (state.rs:209-211) rather than keep a second source of truth.
Bases equal: tree diff, as today. This is also the pure-squash case, which correctly yields an empty interdiff.
Bases differ: cherry-pick the OLDER revision onto the NEWER revision's base via git2::merge_commits, then tree-diff against the newer revision. Direction matters - the newer revision is under review and must be shown as its author recorded it.
Conflicting cherry-pick is reported as a conflict, not rendered as a misleading diff.

Revisions with no stored base fall back to recomputing against the current base branch, which is today's behaviour.

Migrated from git-collab issue 8db8d346fa3e19cdd74593d1fee3ce6e0567299d.

Original author: a73x dev@a73x.sh.

Close reason: merged in patch f02f05c3

Spec: docs/superpowers/specs/2026-08-09-patch-revision-refs-design.md interdiff (patch.rs:501-525) is a flat tree-to-tree diff between two revision trees with no base awareness. When a rebase separates the revisions, the result includes every upstream commit rebased over, burying the author's actual response to review. Single-revision diff does not have this problem - resolve_base_tree (patch.rs:454-466) three-dots against the current base branch. Only the interdiff path needs fixing. Fix: - Move base onto Revision as merge_base(<base_ref tip>, <revision commit>), computed when the revision is recorded and stored. Merge-base rather than branch tip, so base moves only when the author actually rebases. - Drop PatchState.base_commit (state.rs:209-211) rather than keep a second source of truth. - Bases equal: tree diff, as today. This is also the pure-squash case, which correctly yields an empty interdiff. - Bases differ: cherry-pick the OLDER revision onto the NEWER revision's base via git2::merge_commits, then tree-diff against the newer revision. Direction matters - the newer revision is under review and must be shown as its author recorded it. - Conflicting cherry-pick is reported as a conflict, not rendered as a misleading diff. Revisions with no stored base fall back to recomputing against the current base branch, which is today's behaviour. --- Migrated from git-collab issue `8db8d346fa3e19cdd74593d1fee3ce6e0567299d`. Original author: a73x <dev@a73x.sh>. Close reason: merged in patch f02f05c3

a73x commented

2026-08-09 18:45:24 +00:00

(Migrated from git.a73x.sh)

Correction to this issue's scoping premise, found while reviewing patch 659f0350.

This issue says 'Single-revision diff does not have this problem - resolve_base_tree (patch.rs:454-466) three-dots against the current base branch. Only the interdiff path needs fixing.' That is not right. Three-dotting against the current base branch fails in the same way once the base branch has absorbed the patch: merge_base collapses to the revision commit itself and 'patch diff --revision N' renders empty for every merged patch. Verified by demonstration; details and repro in issue 57575b50.

The rest of this issue stands. The per-revision base it asks for has now landed in patch 659f0350 (Revision.base, computed as merge_base at record time), so the enabling data is in place and the remaining work here is the interdiff algorithm itself.

Correction to this issue's scoping premise, found while reviewing patch 659f0350. This issue says 'Single-revision diff does not have this problem - resolve_base_tree (patch.rs:454-466) three-dots against the current base branch. Only the interdiff path needs fixing.' That is not right. Three-dotting against the current base branch fails in the same way once the base branch has absorbed the patch: merge_base collapses to the revision commit itself and 'patch diff --revision N' renders empty for every merged patch. Verified by demonstration; details and repro in issue 57575b50. The rest of this issue stands. The per-revision base it asks for has now landed in patch 659f0350 (Revision.base, computed as merge_base at record time), so the enabling data is in place and the remaining work here is the interdiff algorithm itself.

a73x commented

2026-08-09 19:09:21 +00:00

(Migrated from git.a73x.sh)

Concrete evidence, generated during review of 659f0350 rather than argued from the code.

Setup: a patch with one real edit to feature.txt. Upstream advances three commits touching other files. The author rebases and records revision 2.

'patch diff --between 1 2' emitted FOUR files: one real change, three pure upstream churn. 75% noise from a trivially busy upstream. On a real main the author's actual response to review would be buried entirely.

Also worth noting the reviewer could not tell this had happened from any output - nothing displays a revision's base, so a polluted interdiff is indistinguishable from a clean one. Filed separately as a disclosure issue; it is worth fixing even before this one lands.

Concrete evidence, generated during review of 659f0350 rather than argued from the code. Setup: a patch with one real edit to feature.txt. Upstream advances three commits touching other files. The author rebases and records revision 2. 'patch diff --between 1 2' emitted FOUR files: one real change, three pure upstream churn. 75% noise from a trivially busy upstream. On a real main the author's actual response to review would be buried entirely. Also worth noting the reviewer could not tell this had happened from any output - nothing displays a revision's base, so a polluted interdiff is indistinguishable from a clean one. Filed separately as a disclosure issue; it is worth fixing even before this one lands.