From 0baaaa3932480b33575009987894bc9c049dd9c2 Mon Sep 17 00:00:00 2001 From: Felipe Orlando Date: Mon, 7 Sep 2026 23:31:26 -0300 Subject: [PATCH] feat: choose the base ref used to compare the current branch Co-Authored-By: Claude Opus 5 --- src-tauri/src/fs.rs | 380 +++++++++++++++++++++++---- src/chrome/CompareBasePicker.test.ts | 54 ++++ src/chrome/CompareBasePicker.tsx | 284 ++++++++++++++++++++ src/chrome/GitChangesPanel.tsx | 101 ++++++- src/lib/compareBase.test.ts | 77 ++++++ src/lib/compareBase.ts | 70 +++++ src/lib/fs.ts | 31 ++- src/lib/harness/claudeGit.ts | 3 +- src/lib/harness/codexGit.ts | 3 +- src/lib/harness/cursorGit.ts | 3 +- src/lib/harness/grokGit.ts | 3 +- src/lib/harness/opencodeGit.ts | 3 +- src/lib/harness/registry.ts | 7 +- src/lib/harness/textHarness.ts | 5 +- 14 files changed, 945 insertions(+), 79 deletions(-) create mode 100644 src/chrome/CompareBasePicker.test.ts create mode 100644 src/chrome/CompareBasePicker.tsx create mode 100644 src/lib/compareBase.test.ts create mode 100644 src/lib/compareBase.ts diff --git a/src-tauri/src/fs.rs b/src-tauri/src/fs.rs index 3453f429..f7bd275a 100644 --- a/src-tauri/src/fs.rs +++ b/src-tauri/src/fs.rs @@ -223,15 +223,33 @@ pub struct GitDiffIndex { pub default_branch: Option, pub ahead: i64, pub behind: i64, - pub ahead_of_default: i64, + /// Branch name the diff is compared against. The repository default unless + /// the caller picked another base. + pub base: Option, + /// Ref `base` resolved to, e.g. `origin/main`. Shown so the UI can never + /// imply a comparison it is not actually running. + pub base_ref: Option, + /// Set when the requested base could not be resolved and we fell back to the + /// repository default. The UI surfaces this instead of quietly diffing + /// against a different branch. + pub base_error: Option, + /// Commits on HEAD since it forked from `base_ref`. + pub ahead_of_base: i64, + /// Files and lines HEAD introduced since the fork point (`base...HEAD`). + pub branch_files: i64, + pub branch_additions: i64, + pub branch_deletions: i64, } /// Changed files in the opened folder, with per-file line counts and status. +/// `base` picks the branch-level comparison base; `None` uses the repo default. #[tauri::command] -pub async fn git_diff_index(cwd: String) -> Result { - tauri::async_runtime::spawn_blocking(move || git_diff_index_for(&expand_home(&cwd))) - .await - .map_err(|e| e.to_string()) +pub async fn git_diff_index(cwd: String, base: Option) -> Result { + tauri::async_runtime::spawn_blocking(move || { + git_diff_index_for(&expand_home(&cwd), base.as_deref()) + }) + .await + .map_err(|e| e.to_string()) } /// Changed files and counts without branch/upstream synchronization metadata. @@ -447,18 +465,26 @@ pub async fn git_sync(cwd: String) -> Result<(), String> { #[serde(rename_all = "camelCase")] pub struct GitRangeContext { pub base: String, + /// Ref `base` resolved to, e.g. `origin/main`. + pub base_ref: String, pub head: String, pub commit_summary: String, pub diff_summary: String, pub diff_patch: String, } -/// Commits and diff between the default branch and HEAD, for PR text generation. +/// Commits and diff between the compare base and HEAD, for PR text generation. +/// `base` defaults to the repository default branch when omitted. #[tauri::command] -pub async fn git_range_context(cwd: String) -> Result { - tauri::async_runtime::spawn_blocking(move || git_range_context_for(&expand_home(&cwd))) - .await - .map_err(|e| e.to_string())? +pub async fn git_range_context( + cwd: String, + base: Option, +) -> Result { + tauri::async_runtime::spawn_blocking(move || { + git_range_context_for(&expand_home(&cwd), base.as_deref()) + }) + .await + .map_err(|e| e.to_string())? } #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)] @@ -782,16 +808,16 @@ struct FileAcc { unstaged: bool, } -pub(crate) fn git_diff_index_for(root: &Path) -> GitDiffIndex { - git_diff_index_with(root, true) +pub(crate) fn git_diff_index_for(root: &Path, base: Option<&str>) -> GitDiffIndex { + git_diff_index_with(root, true, base) } /// File list + counts only. Skips ahead/behind/remote lookups used by Git chrome. pub(crate) fn git_diff_files_for(root: &Path) -> GitDiffIndex { - git_diff_index_with(root, false) + git_diff_index_with(root, false, None) } -fn git_diff_index_with(root: &Path, include_sync: bool) -> GitDiffIndex { +fn git_diff_index_with(root: &Path, include_sync: bool, base: Option<&str>) -> GitDiffIndex { let mut files: HashMap = HashMap::new(); let mut statuses: HashMap = HashMap::new(); @@ -882,10 +908,10 @@ fn git_diff_index_with(root: &Path, include_sync: bool) -> GitDiffIndex { }); } out.sort_by(|a, b| a.relative.cmp(&b.relative)); - let sync = if include_sync { - git_sync_for(root) + let (sync, compare) = if include_sync { + (git_sync_for(root), git_base_compare_for(root, base)) } else { - GitSync::default() + (GitSync::default(), GitBaseCompare::default()) }; GitDiffIndex { branch: git_branch(root), @@ -897,7 +923,13 @@ fn git_diff_index_with(root: &Path, include_sync: bool) -> GitDiffIndex { default_branch: sync.default_branch, ahead: sync.ahead, behind: sync.behind, - ahead_of_default: sync.ahead_of_default, + base: compare.base, + base_ref: compare.base_ref, + base_error: compare.base_error, + ahead_of_base: compare.ahead_of_base, + branch_files: compare.branch_files, + branch_additions: compare.branch_additions, + branch_deletions: compare.branch_deletions, } } @@ -1455,7 +1487,7 @@ fn git_discard_file_for(root: &Path, relative: &str) -> Result<(), String> { } fn git_discard_all_for(root: &Path) -> Result<(), String> { - let files: Vec = git_diff_index_for(root) + let files: Vec = git_diff_files_for(root) .files .into_iter() .filter(|file| file.unstaged) @@ -1522,29 +1554,25 @@ fn git_sync_changes_for(root: &Path) -> Result<(), String> { git_push_for(root) } -fn git_range_context_for(root: &Path) -> Result { +fn git_range_context_for(root: &Path, base: Option<&str>) -> Result { let head = git_branch(root).ok_or_else(|| "Not on a branch".to_string())?; - let remote = git_remote_name(root); - let default_branch = git_default_branch(root, remote.as_deref()) - .ok_or_else(|| "Could not resolve the default branch".to_string())?; - let base_ref = match &remote { - Some(remote) - if git_ref_exists(root, &format!("refs/remotes/{remote}/{default_branch}")) => - { - format!("{remote}/{default_branch}") - } - _ => default_branch.clone(), - }; - let spec = format!("{base_ref}...HEAD"); - let commit_summary = - git_run(root, &["log", "--format=%s", &format!("{base_ref}..HEAD")]).unwrap_or_default(); + // No silent fallback here: a pull request opened against the wrong base is + // worse than one that fails to open, so an unresolvable base aborts. + let compare = git_compare_base(root, base)?; + let spec = format!("{}...HEAD", compare.spec); + let commit_summary = git_run( + root, + &["log", "--format=%s", &format!("{}..HEAD", compare.spec)], + ) + .unwrap_or_default(); let diff_summary = git_run(root, &["diff", "--stat", &spec]).unwrap_or_default(); let diff_patch = git_run(root, &["diff", "--no-ext-diff", &spec]).unwrap_or_default(); if commit_summary.trim().is_empty() && diff_patch.trim().is_empty() { return Err("No commits to include in a pull request".into()); } Ok(GitRangeContext { - base: default_branch, + base: compare.name, + base_ref: compare.spec, head, commit_summary, diff_summary, @@ -2822,7 +2850,6 @@ struct GitSync { default_branch: Option, ahead: i64, behind: i64, - ahead_of_default: i64, } fn git_sync_for(root: &Path) -> GitSync { @@ -2840,21 +2867,128 @@ fn git_sync_for(root: &Path) -> GitSync { } else { (0, 0) }; - let ahead_of_default = if let Some(base) = default_ref.as_deref() { - git_ahead_behind(root, base).0 - } else { - ahead - }; GitSync { remote, upstream, default_branch, ahead, behind, - ahead_of_default, } } +/// A compare base resolved to something git can actually diff against. +struct CompareBase { + /// Branch name as the user picked it, without a remote prefix. + name: String, + /// Ref handed to git. Prefers the remote copy, because that is the commit a + /// pull request would really be merged into. + spec: String, +} + +/// Map a user-visible base name onto a ref that exists in this checkout. +/// +/// Accepts a plain branch (`main`, `feature/x`) or a remote-qualified one +/// (`origin/main`). Returns `None` when nothing matches, so callers can report +/// a deleted base instead of falling through to some other branch. +fn git_resolve_base(root: &Path, requested: &str, remote: Option<&str>) -> Option { + let requested = requested.trim(); + if requested.is_empty() { + return None; + } + let tracked = remote + .map(|remote| format!("{remote}/{requested}")) + .filter(|spec| git_ref_exists(root, &format!("refs/remotes/{spec}"))); + if tracked.is_some() || git_ref_exists(root, &format!("refs/heads/{requested}")) { + return Some(CompareBase { + name: requested.to_string(), + spec: tracked.unwrap_or_else(|| requested.to_string()), + }); + } + // Branch on a remote other than the primary one, spelled out in full. + if git_ref_exists(root, &format!("refs/remotes/{requested}")) { + let (_, name) = requested.split_once('/')?; + return Some(CompareBase { + name: name.to_string(), + spec: requested.to_string(), + }); + } + None +} + +/// Resolve the base for `...HEAD`. `requested` is the user's pick; `None` +/// means "use the repository default". +fn git_compare_base(root: &Path, requested: Option<&str>) -> Result { + let remote = git_remote_name(root); + if let Some(requested) = requested.map(str::trim).filter(|value| !value.is_empty()) { + return git_resolve_base(root, requested, remote.as_deref()) + .ok_or_else(|| format!("Compare base \"{requested}\" no longer exists")); + } + let default_branch = git_default_branch(root, remote.as_deref()) + .ok_or_else(|| "Could not resolve the default branch".to_string())?; + git_resolve_base(root, &default_branch, remote.as_deref()) + .ok_or_else(|| format!("Compare base \"{default_branch}\" no longer exists")) +} + +#[derive(Default)] +struct GitBaseCompare { + base: Option, + base_ref: Option, + base_error: Option, + ahead_of_base: i64, + branch_files: i64, + branch_additions: i64, + branch_deletions: i64, +} + +/// Branch-level totals for the status view. A base the user picked that no +/// longer resolves reports the failure next to the repository default it fell +/// back to, so the panel can say which ref it is really diffing. +fn git_base_compare_for(root: &Path, requested: Option<&str>) -> GitBaseCompare { + let (compare, base_error) = match git_compare_base(root, requested) { + Ok(compare) => (Some(compare), None), + Err(error) if requested.is_some() => match git_compare_base(root, None) { + Ok(compare) => (Some(compare), Some(error)), + Err(_) => (None, Some(error)), + }, + Err(_) => (None, None), + }; + let Some(compare) = compare else { + return GitBaseCompare { + base_error, + ..GitBaseCompare::default() + }; + }; + // Three-dot: only what HEAD added since the fork point, so commits that + // landed on the base afterwards do not show up as this branch's work. + let (branch_files, branch_additions, branch_deletions) = + git_numstat_totals(root, &format!("{}...HEAD", compare.spec)); + GitBaseCompare { + base: Some(compare.name), + ahead_of_base: git_ahead_behind(root, &compare.spec).0, + base_ref: Some(compare.spec), + base_error, + branch_files, + branch_additions, + branch_deletions, + } +} + +/// (files, additions, deletions) for a diff spec, from `--numstat`. +fn git_numstat_totals(root: &Path, spec: &str) -> (i64, i64, i64) { + let Some(text) = git_run(root, &["diff", "--no-ext-diff", "--numstat", spec]) else { + return (0, 0, 0); + }; + let mut files: HashMap = HashMap::new(); + add_numstat_map(&text, &mut files); + let mut additions = 0i64; + let mut deletions = 0i64; + for acc in files.values() { + additions += acc.additions; + deletions += acc.deletions; + } + (files.len() as i64, additions, deletions) +} + fn git_remote_name(root: &Path) -> Option { let remotes = git_stdout(root, &["remote"])?; let mut names = remotes @@ -4278,7 +4412,7 @@ mod tests { std::fs::write(dir.0.join("a.txt"), "alpha\ngamma\ndelta\n").unwrap(); std::fs::write(dir.0.join("new.txt"), "hello\nworld\n").unwrap(); - let index = git_diff_index_for(&dir.0); + let index = git_diff_index_for(&dir.0, None); assert_eq!(index.branch.as_deref(), Some("main")); assert_eq!(index.files.len(), 2); @@ -4551,7 +4685,7 @@ mod tests { } std::fs::write(dir.0.join("a.txt"), "beta\n").unwrap(); git_stage_file_for(&dir.0, "a.txt").unwrap(); - let staged = git_diff_index_for(&dir.0) + let staged = git_diff_index_for(&dir.0, None) .files .into_iter() .find(|file| file.relative == "a.txt") @@ -4560,7 +4694,7 @@ mod tests { assert!(!staged.unstaged); git_unstage_file_for(&dir.0, "a.txt").unwrap(); - let unstaged = git_diff_index_for(&dir.0) + let unstaged = git_diff_index_for(&dir.0, None) .files .into_iter() .find(|file| file.relative == "a.txt") @@ -4578,7 +4712,7 @@ mod tests { std::fs::write(dir.0.join("a.txt"), "alpha\nBETA\ngamma\nDELTA\n").unwrap(); git_stage_contents_for(&dir.0, "a.txt", b"alpha\nBETA\ngamma\ndelta\n").unwrap(); - let file = git_diff_index_for(&dir.0) + let file = git_diff_index_for(&dir.0, None) .files .into_iter() .find(|file| file.relative == "a.txt") @@ -4608,7 +4742,7 @@ mod tests { "alpha\n" ); assert!(!dir.0.join("new.txt").exists()); - assert!(git_diff_index_for(&dir.0).files.is_empty()); + assert!(git_diff_index_for(&dir.0, None).files.is_empty()); } #[test] @@ -4634,14 +4768,14 @@ mod tests { "two\n" ); assert!(!dir.0.join("new.txt").exists()); - let file = git_diff_index_for(&dir.0) + let file = git_diff_index_for(&dir.0, None) .files .into_iter() .find(|file| file.relative == "b.txt") .unwrap(); assert!(file.staged); assert!(!file.unstaged); - assert!(git_diff_index_for(&dir.0) + assert!(git_diff_index_for(&dir.0, None) .files .iter() .all(|file| !file.unstaged)); @@ -4661,7 +4795,7 @@ mod tests { std::fs::read_to_string(dir.0.join("a.txt")).unwrap(), "beta\n" ); - let file = git_diff_index_for(&dir.0) + let file = git_diff_index_for(&dir.0, None) .files .into_iter() .find(|file| file.relative == "a.txt") @@ -4679,7 +4813,7 @@ mod tests { std::fs::write(dir.0.join("a.txt"), "beta\n").unwrap(); git_stage_file_for(&dir.0, "a.txt").unwrap(); git_commit_for(&dir.0, "update a").unwrap(); - assert!(git_diff_index_for(&dir.0).files.is_empty()); + assert!(git_diff_index_for(&dir.0, None).files.is_empty()); assert_eq!( git_stdout(&dir.0, &["log", "-1", "--pretty=%s"]).as_deref(), Some("update a") @@ -4742,13 +4876,13 @@ mod tests { std::fs::write(repo.0.join("a.txt"), "beta\n").unwrap(); git_stage_file_for(&repo.0, "a.txt").unwrap(); git_commit_for(&repo.0, "second").unwrap(); - let index = git_diff_index_for(&repo.0); + let index = git_diff_index_for(&repo.0, None); assert_eq!(index.remote.as_deref(), Some("origin")); assert_eq!(index.upstream.as_deref(), Some("origin/main")); assert_eq!(index.default_branch.as_deref(), Some("main")); assert_eq!(index.ahead, 1); assert_eq!(index.behind, 0); - assert_eq!(index.ahead_of_default, 1); + assert_eq!(index.ahead_of_base, 1); } #[test] @@ -4777,17 +4911,147 @@ mod tests { std::fs::write(repo.0.join("a.txt"), "beta\n").unwrap(); git_stage_file_for(&repo.0, "a.txt").unwrap(); git_commit_for(&repo.0, "feature work").unwrap(); - let index = git_diff_index_for(&repo.0); + let index = git_diff_index_for(&repo.0, None); assert_eq!(index.branch.as_deref(), Some("feature")); assert_eq!(index.upstream, None); assert_eq!(index.ahead, 1); - assert_eq!(index.ahead_of_default, 1); - let range = git_range_context_for(&repo.0).unwrap(); + assert_eq!(index.ahead_of_base, 1); + assert_eq!(index.base.as_deref(), Some("main")); + // The remote copy is preferred: it is what a PR would merge into. + assert_eq!(index.base_ref.as_deref(), Some("origin/main")); + let range = git_range_context_for(&repo.0, None).unwrap(); assert_eq!(range.base, "main"); + assert_eq!(range.base_ref, "origin/main"); assert_eq!(range.head, "feature"); assert!(range.commit_summary.contains("feature work")); } + /// `main` -> `feature-a` -> `feature-b`, no remote, so every base resolves + /// to a local ref. + fn init_stacked_repo(dir: &Path) -> bool { + if !init_git_commit(dir, &[("a.txt", "alpha\n")]) + || !git(dir, &["checkout", "-b", "feature-a"]) + { + return false; + } + if std::fs::write(dir.join("a.txt"), "alpha\nfrom-a\n").is_err() { + return false; + } + if !git(dir, &["add", "."]) + || !git(dir, &["commit", "-m", "work on a"]) + || !git(dir, &["checkout", "-b", "feature-b"]) + { + return false; + } + if std::fs::write(dir.join("b.txt"), "from-b\n").is_err() { + return false; + } + git(dir, &["add", "."]) && git(dir, &["commit", "-m", "work on b"]) + } + + #[test] + fn git_range_context_compares_against_the_selected_base() { + let repo = tmp("git-base-stacked"); + if !init_stacked_repo(&repo.0) { + return; + } + let default = git_range_context_for(&repo.0, None).unwrap(); + assert_eq!(default.base, "main"); + assert_eq!(default.base_ref, "main"); + assert!(default.commit_summary.contains("work on a")); + assert!(default.commit_summary.contains("work on b")); + + // Stacked review: only what feature-b added on top of feature-a. + let stacked = git_range_context_for(&repo.0, Some("feature-a")).unwrap(); + assert_eq!(stacked.base, "feature-a"); + assert_eq!(stacked.head, "feature-b"); + assert!(!stacked.commit_summary.contains("work on a")); + assert!(stacked.commit_summary.contains("work on b")); + assert!(stacked.diff_patch.contains("b.txt")); + assert!(!stacked.diff_patch.contains("from-a")); + } + + #[test] + fn git_range_context_rejects_a_deleted_base() { + let repo = tmp("git-base-missing"); + if !init_stacked_repo(&repo.0) { + return; + } + let error = git_range_context_for(&repo.0, Some("gone")).unwrap_err(); + assert!(error.contains("gone"), "{error}"); + } + + #[test] + fn git_diff_index_counts_against_the_selected_base() { + let repo = tmp("git-base-counts"); + if !init_stacked_repo(&repo.0) { + return; + } + let default = git_diff_index_for(&repo.0, None); + assert_eq!(default.base.as_deref(), Some("main")); + assert_eq!(default.ahead_of_base, 2); + assert_eq!(default.branch_files, 2); + assert_eq!(default.branch_additions, 2); + + let stacked = git_diff_index_for(&repo.0, Some("feature-a")); + assert_eq!(stacked.base.as_deref(), Some("feature-a")); + assert_eq!(stacked.base_ref.as_deref(), Some("feature-a")); + assert_eq!(stacked.base_error, None); + assert_eq!(stacked.ahead_of_base, 1); + assert_eq!(stacked.branch_files, 1); + assert_eq!(stacked.branch_additions, 1); + assert_eq!(stacked.branch_deletions, 0); + // Picking a base must not touch the checkout. + assert_eq!(stacked.branch.as_deref(), Some("feature-b")); + } + + #[test] + fn git_diff_index_falls_back_from_a_deleted_base() { + let repo = tmp("git-base-fallback"); + if !init_stacked_repo(&repo.0) { + return; + } + let index = git_diff_index_for(&repo.0, Some("gone")); + assert_eq!(index.base.as_deref(), Some("main")); + assert_eq!(index.ahead_of_base, 2); + let error = index.base_error.unwrap_or_default(); + assert!(error.contains("gone"), "{error}"); + } + + #[test] + fn git_range_context_ignores_base_commits_made_after_the_fork() { + let repo = tmp("git-base-three-dot"); + if !init_git_commit(&repo.0, &[("a.txt", "alpha\n")]) + || !git(&repo.0, &["checkout", "-b", "feature"]) + { + return; + } + std::fs::write(repo.0.join("feature.txt"), "feature\n").unwrap(); + if !git(&repo.0, &["add", "."]) + || !git(&repo.0, &["commit", "-m", "feature work"]) + || !git(&repo.0, &["checkout", "main"]) + { + return; + } + std::fs::write(repo.0.join("main.txt"), "main\n").unwrap(); + if !git(&repo.0, &["add", "."]) + || !git(&repo.0, &["commit", "-m", "main moved on"]) + || !git(&repo.0, &["checkout", "feature"]) + { + return; + } + let range = git_range_context_for(&repo.0, None).unwrap(); + assert!(range.diff_patch.contains("feature.txt")); + // Two-dot would report main.txt as a deletion this branch introduced. + assert!( + !range.diff_patch.contains("main.txt"), + "{}", + range.diff_patch + ); + assert!(!range.commit_summary.contains("main moved on")); + assert_eq!(git_diff_index_for(&repo.0, None).branch_files, 1); + } + #[test] fn git_sync_pulls_then_pushes() { let origin = tmp("git-sync-origin"); @@ -4834,8 +5098,8 @@ mod tests { std::fs::read_to_string(b.0.join("a.txt")).unwrap(), "beta\n" ); - assert_eq!(git_diff_index_for(&b.0).ahead, 0); - assert_eq!(git_diff_index_for(&b.0).behind, 0); + assert_eq!(git_diff_index_for(&b.0, None).ahead, 0); + assert_eq!(git_diff_index_for(&b.0, None).behind, 0); std::fs::write(b.0.join("b.txt"), "from-b\n").unwrap(); git_stage_file_for(&b.0, "b.txt").unwrap(); diff --git a/src/chrome/CompareBasePicker.test.ts b/src/chrome/CompareBasePicker.test.ts new file mode 100644 index 00000000..72676553 --- /dev/null +++ b/src/chrome/CompareBasePicker.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from "vitest"; +import { compareBaseRows, compareBaseSelection } from "./CompareBasePicker"; +import type { GitBranchInfo } from "../lib/fs"; + +const branches: GitBranchInfo[] = [ + { name: "feature-b", current: true, remote: null }, + { name: "feature-a", current: false, remote: null }, + { name: "main", current: false, remote: null }, + { name: "release-2", current: false, remote: "origin" }, +]; + +describe("compareBaseRows", () => { + it("offers every local and remote branch as a base", () => { + expect(compareBaseRows(branches, "", "main").map((row) => row.ref)).toEqual( + ["feature-b", "feature-a", "main", "origin/release-2"], + ); + }); + + it("marks the repository default", () => { + const rows = compareBaseRows(branches, "", "main"); + expect( + rows.filter((row) => row.isDefault).map((row) => row.branch.name), + ).toEqual(["main"]); + }); + + it("matches on the remote as well as the name", () => { + expect( + compareBaseRows(branches, "origin", "main").map((row) => row.branch.name), + ).toEqual(["release-2"]); + expect( + compareBaseRows(branches, "feature-", "main").map((row) => row.ref), + ).toEqual(["feature-b", "feature-a"]); + }); + + it("treats a repo with no resolvable default as having no default row", () => { + expect( + compareBaseRows(branches, "", null).some((row) => row.isDefault), + ).toBe(false); + }); +}); + +describe("compareBaseSelection", () => { + it("stores the default as 'no pick' so a renamed default still applies", () => { + const [main] = compareBaseRows(branches, "main", "main"); + expect(main && compareBaseSelection(main)).toBeNull(); + }); + + it("stores another base by the ref git should compare against", () => { + const [stacked] = compareBaseRows(branches, "feature-a", "main"); + expect(stacked && compareBaseSelection(stacked)).toBe("feature-a"); + const [release] = compareBaseRows(branches, "release-2", "main"); + expect(release && compareBaseSelection(release)).toBe("origin/release-2"); + }); +}); diff --git a/src/chrome/CompareBasePicker.tsx b/src/chrome/CompareBasePicker.tsx new file mode 100644 index 00000000..afd86004 --- /dev/null +++ b/src/chrome/CompareBasePicker.tsx @@ -0,0 +1,284 @@ +import { Check, ChevronDown, GitBranch, Search } from "./icons"; +import { + useEffect, + useMemo, + useRef, + useState, + type KeyboardEvent as ReactKeyboardEvent, +} from "react"; +import { compareBaseRefName } from "../lib/compareBase"; +import type { GitBranchInfo } from "../lib/fs"; +import { useLockOverscroll } from "../hooks/useLockOverscroll"; +import { useProjectBranchesState } from "../hooks/useProjectBranches"; +import { Popover } from "./Popover"; + +type Props = { + cwd: string; + /** Base the panel is actually diffing against, as resolved by git. */ + base: string | null; + /** Repository default, offered as the way back to stock behaviour. */ + defaultBranch: string | null; + enabled?: boolean; + /** `null` clears the pick and returns the folder to the repository default. */ + onPick: (base: string | null) => void; +}; + +const MENU_WIDTH = 260; +const MENU_MIN_HEIGHT = 180; +const MENU_MAX_HEIGHT = 280; + +export type CompareBaseRow = { + branch: GitBranchInfo; + /** What the backend is asked to compare against for this row. */ + ref: string; + isDefault: boolean; +}; + +/** Filtered branch rows, each tagged with the ref the backend should receive. */ +export function compareBaseRows( + branches: GitBranchInfo[], + query: string, + defaultBranch: string | null, +): CompareBaseRow[] { + const needle = query.trim().toLowerCase(); + const matches = needle + ? branches.filter((entry) => { + const hay = entry.remote ? `${entry.name} ${entry.remote}` : entry.name; + return hay.toLowerCase().includes(needle); + }) + : branches; + return matches.map((branch) => ({ + branch, + ref: compareBaseRefName(branch), + isDefault: !!defaultBranch && branch.name === defaultBranch, + })); +} + +/** + * The value to persist for a row. The repository default is stored as `null` + * so a repo that later renames its default branch follows along instead of + * staying pinned to the old name. + */ +export function compareBaseSelection(row: CompareBaseRow): string | null { + return row.isDefault ? null : row.ref; +} + +/** + * Picks the ref the current branch is compared against. Unlike `BranchPicker` + * this never touches the checkout — it only changes what HEAD is diffed with. + */ +export function CompareBasePicker({ + cwd, + base, + defaultBranch, + enabled = true, + onPick, +}: Props) { + const [open, setOpen] = useState(false); + const [query, setQuery] = useState(""); + const [active, setActive] = useState(0); + const root = useRef(null); + const search = useRef(null); + + const inProject = Boolean(cwd) && cwd !== "~"; + const { branches } = useProjectBranchesState(cwd, inProject && open); + + useEffect(() => { + if (!open) return; + setQuery(""); + setActive(0); + search.current?.focus(); + }, [open]); + + useEffect(() => { + if (!enabled) setOpen(false); + }, [enabled]); + + const rows = useMemo( + () => compareBaseRows(branches?.branches ?? [], query, defaultBranch), + [branches, defaultBranch, query], + ); + + useEffect(() => { + setActive((i) => (rows.length === 0 ? 0 : Math.min(i, rows.length - 1))); + }, [rows.length]); + + const pick = (row: CompareBaseRow) => { + onPick(compareBaseSelection(row)); + setOpen(false); + }; + + const onSearchKey = (e: ReactKeyboardEvent) => { + if (e.key === "ArrowDown") { + e.preventDefault(); + if (rows.length > 0) setActive((i) => Math.min(rows.length - 1, i + 1)); + return; + } + if (e.key === "ArrowUp") { + e.preventDefault(); + if (rows.length > 0) setActive((i) => Math.max(0, i - 1)); + return; + } + if (e.key === "Enter") { + e.preventDefault(); + const row = rows[active]; + if (row) pick(row); + } + }; + + const label = base ?? "no base"; + const interactive = enabled && inProject; + + return ( +
+ + {open ? ( + setOpen(false)} + role="dialog" + aria-label="Compare base picker" + data-compare-base-picker + className="flex flex-col overflow-hidden" + > + + + + ) : null} +
+ ); +} + +function BaseList({ + rows, + base, + active, + emptyLabel, + onActive, + onPick, +}: { + rows: CompareBaseRow[]; + base: string | null; + active: number; + emptyLabel: string; + onActive: (index: number) => void; + onPick: (row: CompareBaseRow) => void; +}) { + const lockOverscroll = useLockOverscroll(); + const activeRef = useRef(null); + + useEffect(() => { + activeRef.current?.scrollIntoView({ block: "nearest" }); + }, [active]); + + if (rows.length === 0) { + return ( +
{emptyLabel}
+ ); + } + + return ( +
+ {rows.map((row, index) => { + const highlighted = index === active; + // `base` comes back from git already stripped of its remote prefix, so + // a remote-only branch matches on name. + const selected = row.branch.name === base; + return ( + + ); + })} +
+ ); +} diff --git a/src/chrome/GitChangesPanel.tsx b/src/chrome/GitChangesPanel.tsx index b31c8434..8f137f11 100644 --- a/src/chrome/GitChangesPanel.tsx +++ b/src/chrome/GitChangesPanel.tsx @@ -23,6 +23,7 @@ import { useState, type ReactNode, } from "react"; +import { CompareBasePicker } from "./CompareBasePicker"; import { FileTypeIcon } from "./FileTypeIcon"; import { GitHistoryGraph, @@ -54,6 +55,7 @@ import { type GitPr, } from "../lib/fs"; import type { HarnessId } from "../lib/session"; +import { loadCompareBase, saveCompareBase } from "../lib/compareBase"; import { generateCommitMessage, generatePrContent } from "../lib/harness"; import { invalidateWatchedFiles } from "../lib/fileWatch"; import { MOD } from "../lib/platform"; @@ -95,7 +97,11 @@ export function GitChangesPanel({ onOpenFile, onOpenCommit, }: Props) { - const { index, reload } = useDiffIndex(cwd, enabled); + // The pick is per folder, so a worktree and its parent checkout can review + // against different bases at the same time. + const [base, setBase] = useState(() => loadCompareBase(cwd)); + useEffect(() => setBase(loadCompareBase(cwd)), [cwd]); + const { index, reload } = useDiffIndex(cwd, enabled, base); const files = index?.files ?? []; const paneRef = useRef(null); const [graphHeight, setGraphHeight] = useState(loadGraphPanelHeight); @@ -150,6 +156,19 @@ export function GitChangesPanel({ )} + { + saveCompareBase(cwd, next); + setBase(next); + // The cached index was measured against the old base; drop it so the + // panel never shows counts attributed to the wrong comparison. + indexByCwd.delete(cwd); + reload(); + }} + /> void; +}) { + if (!index?.branch) return null; + const files = index.branchFiles; + return ( +
+
+ + + {index.branch} + + → + + + {files > 0 ? ( + + {files} file{files === 1 ? "" : "s"} + + ) : ( + no branch changes + )} + + +
+ {index.baseError ? ( +

+ {index.baseError}. Comparing against{" "} + {index.base ?? "nothing"} instead. +

+ ) : null} +
+ ); +} + function ChangedFiles({ cwd, textHarness, @@ -253,7 +329,7 @@ function ChangedFiles({ !onDefault && !diverged && files.length === 0 && - (index?.aheadOfDefault ?? 0) > 0 && + (index?.aheadOfBase ?? 0) > 0 && (index?.behind ?? 0) === 0; const canViewPr = hasOpenPr && !!pr?.url; const canPublish = hasRemote && !index?.upstream; @@ -412,7 +488,9 @@ function ChangedFiles({ }; const openCreatedPr = async () => { - const content = await generatePrContent(cwd, textHarness); + // The already-resolved base from the index, not the raw preference, so the + // PR targets exactly the branch the panel says it is comparing against. + const content = await generatePrContent(cwd, textHarness, index?.base); if (!content) throw new Error("Could not prepare pull request content"); const url = await gitPrCreate( cwd, @@ -737,8 +815,8 @@ function GitSyncActions({ : behind > 0 ? `Pull ${behind} commit${behind === 1 ? "" : "s"} from ${dest}` : `Push ${ahead} commit${ahead === 1 ? "" : "s"} to ${dest}`; - const createTitle = index.defaultBranch - ? `Create a pull request into ${index.defaultBranch}` + const createTitle = index.base + ? `Create a pull request into ${index.base}` : "Create pull request"; const viewTitle = pr?.title ? `View PR #${pr.number}: ${pr.title}` @@ -1045,6 +1123,7 @@ function statusColor(status: string): string { function useDiffIndex( cwd: string, enabled: boolean, + base: string | null, ): { index: GitDiffIndex | null; reload: () => void; @@ -1078,7 +1157,7 @@ function useDiffIndex( if (document.hidden && nonce === 0) return; inFlight = true; try { - const next = await gitDiffIndex(cwd); + const next = await gitDiffIndex(cwd, base); if (cancelled) return; const prev = indexRef.current; if (sameIndex(prev, next)) return; @@ -1124,7 +1203,7 @@ function useDiffIndex( document.removeEventListener("visibilitychange", onResume); unsubGit(); }; - }, [cwd, enabled, nonce]); + }, [base, cwd, enabled, nonce]); return { index, reload }; } @@ -1173,7 +1252,13 @@ function sameIndex(prev: GitDiffIndex | null, next: GitDiffIndex): boolean { prev.defaultBranch !== next.defaultBranch || prev.ahead !== next.ahead || prev.behind !== next.behind || - prev.aheadOfDefault !== next.aheadOfDefault + prev.base !== next.base || + prev.baseRef !== next.baseRef || + prev.baseError !== next.baseError || + prev.aheadOfBase !== next.aheadOfBase || + prev.branchFiles !== next.branchFiles || + prev.branchAdditions !== next.branchAdditions || + prev.branchDeletions !== next.branchDeletions ) { return false; } diff --git a/src/lib/compareBase.test.ts b/src/lib/compareBase.test.ts new file mode 100644 index 00000000..ecb186bb --- /dev/null +++ b/src/lib/compareBase.test.ts @@ -0,0 +1,77 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import { + compareBaseRefName, + loadCompareBase, + saveCompareBase, +} from "./compareBase"; + +const KEY = "monocode.compareBase.v1"; + +function mockLocalStorage() { + const data = new Map(); + const storage = { + getItem: (key: string) => data.get(key) ?? null, + setItem: (key: string, value: string) => { + data.set(key, value); + }, + removeItem: (key: string) => { + data.delete(key); + }, + clear: () => { + data.clear(); + }, + key: (index: number) => [...data.keys()][index] ?? null, + get length() { + return data.size; + }, + }; + Object.defineProperty(globalThis, "localStorage", { + value: storage, + configurable: true, + }); +} + +describe("compare base", () => { + beforeEach(mockLocalStorage); + + it("defaults to the repository default before anything is picked", () => { + expect(loadCompareBase("/repos/web")).toBeNull(); + }); + + it("remembers a pick per folder", () => { + saveCompareBase("/repos/web", "release-2"); + saveCompareBase("/repos/web-worktree", "feature-a"); + expect(loadCompareBase("/repos/web")).toBe("release-2"); + expect(loadCompareBase("/repos/web-worktree")).toBe("feature-a"); + }); + + it("keeps folders that share a name apart", () => { + saveCompareBase("/acme/web", "release-2"); + expect(loadCompareBase("/other/web")).toBeNull(); + }); + + it("clears the pick back to the default", () => { + saveCompareBase("/repos/web", "release-2"); + saveCompareBase("/repos/web", null); + expect(loadCompareBase("/repos/web")).toBeNull(); + }); + + it("ignores a corrupt store instead of throwing", () => { + localStorage.setItem(KEY, "not json"); + expect(loadCompareBase("/repos/web")).toBeNull(); + saveCompareBase("/repos/web", "main"); + expect(loadCompareBase("/repos/web")).toBe("main"); + }); + + it("has no base for a folder that is not a project", () => { + saveCompareBase("~", "main"); + expect(loadCompareBase("~")).toBeNull(); + }); + + it("qualifies remote-only branches so the remote cannot be guessed wrong", () => { + expect(compareBaseRefName({ name: "main", remote: null })).toBe("main"); + expect(compareBaseRefName({ name: "main", remote: "upstream" })).toBe( + "upstream/main", + ); + }); +}); diff --git a/src/lib/compareBase.ts b/src/lib/compareBase.ts new file mode 100644 index 00000000..b611f0df --- /dev/null +++ b/src/lib/compareBase.ts @@ -0,0 +1,70 @@ +import { projectKey } from "./paths"; + +const KEY = "monocode.compareBase.v1"; + +/** + * The base ref each folder compares its branch against. Keyed by the full path + * (see `projectKey`), so a worktree keeps its own base instead of inheriting + * the one picked in the main checkout it was branched from. + */ +type Stored = Record; + +function read(): Stored { + try { + const raw = localStorage.getItem(KEY); + if (!raw) return {}; + const parsed: unknown = JSON.parse(raw); + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + return {}; + } + const out: Stored = {}; + for (const [key, value] of Object.entries(parsed as Stored)) { + if (typeof value === "string" && value.trim()) out[key] = value.trim(); + } + return out; + } catch { + return {}; + } +} + +function write(stored: Stored) { + try { + localStorage.setItem(KEY, JSON.stringify(stored)); + } catch { + // private mode / quota + } +} + +/** The saved base for a folder, or `null` to mean "repository default". */ +export function loadCompareBase(cwd: string): string | null { + if (!cwd || cwd === "~") return null; + return read()[projectKey(cwd)] ?? null; +} + +/** Saving `null` clears the pick, putting the folder back on the default. */ +export function saveCompareBase(cwd: string, base: string | null): void { + if (!cwd || cwd === "~") return; + const key = projectKey(cwd); + const stored = read(); + const next = base?.trim(); + if (!next) { + if (!(key in stored)) return; + delete stored[key]; + } else { + if (stored[key] === next) return; + stored[key] = next; + } + write(stored); +} + +/** + * The ref name sent to the backend for a branch row. Remote-only branches are + * qualified so a name that exists on several remotes cannot resolve to the + * wrong one. + */ +export function compareBaseRefName(branch: { + name: string; + remote: string | null; +}): string { + return branch.remote ? `${branch.remote}/${branch.name}` : branch.name; +} diff --git a/src/lib/fs.ts b/src/lib/fs.ts index 9bb1cd83..b80ca81e 100644 --- a/src/lib/fs.ts +++ b/src/lib/fs.ts @@ -76,11 +76,24 @@ export type GitDiffIndex = { defaultBranch: string | null; ahead: number; behind: number; - aheadOfDefault: number; + /** Branch the branch-level diff is compared against, after resolution. */ + base: string | null; + /** Ref `base` resolved to, e.g. `origin/main`. */ + baseRef: string | null; + /** Why a requested base was dropped in favour of the repository default. */ + baseError: string | null; + aheadOfBase: number; + branchFiles: number; + branchAdditions: number; + branchDeletions: number; }; -export function gitDiffIndex(cwd: string): Promise { - return invoke("git_diff_index", { cwd }); +/** `base` picks the branch-level compare base; omit it for the repo default. */ +export function gitDiffIndex( + cwd: string, + base?: string | null, +): Promise { + return invoke("git_diff_index", { cwd, base: base ?? null }); } /** File list and counts only, for diff content views that do not need sync data. */ @@ -202,14 +215,22 @@ export function gitSync(cwd: string): Promise { export type GitRangeContext = { base: string; + baseRef: string; head: string; commitSummary: string; diffSummary: string; diffPatch: string; }; -export function gitRangeContext(cwd: string): Promise { - return invoke("git_range_context", { cwd }); +/** Rejects rather than silently retargeting when `base` cannot be resolved. */ +export function gitRangeContext( + cwd: string, + base?: string | null, +): Promise { + return invoke("git_range_context", { + cwd, + base: base ?? null, + }); } export type GitPr = { diff --git a/src/lib/harness/claudeGit.ts b/src/lib/harness/claudeGit.ts index 93c5033c..50945b8c 100644 --- a/src/lib/harness/claudeGit.ts +++ b/src/lib/harness/claudeGit.ts @@ -36,8 +36,9 @@ export async function generateClaudeCommitMessage(cwd: string): Promise export async function generateClaudePrContent( cwd: string, + base?: string | null, ): Promise<(PrContent & { base: string; head: string }) | null> { - const range = await gitRangeContext(cwd); + const range = await gitRangeContext(cwd, base); let parsed: PrContent | null = null; try { const output = await runClaudeTextPrompt({ diff --git a/src/lib/harness/codexGit.ts b/src/lib/harness/codexGit.ts index e04fd062..7c3c5e2e 100644 --- a/src/lib/harness/codexGit.ts +++ b/src/lib/harness/codexGit.ts @@ -36,8 +36,9 @@ export async function generateCodexCommitMessage(cwd: string): Promise { export async function generateCodexPrContent( cwd: string, + base?: string | null, ): Promise<(PrContent & { base: string; head: string }) | null> { - const range = await gitRangeContext(cwd); + const range = await gitRangeContext(cwd, base); let parsed: PrContent | null = null; try { const output = await runCodexTextPrompt({ diff --git a/src/lib/harness/cursorGit.ts b/src/lib/harness/cursorGit.ts index e5f5415e..05a81aa5 100644 --- a/src/lib/harness/cursorGit.ts +++ b/src/lib/harness/cursorGit.ts @@ -40,8 +40,9 @@ export async function generateCursorCommitMessage(cwd: string): Promise export async function generateCursorPrContent( cwd: string, + base?: string | null, ): Promise<(PrContent & { base: string; head: string }) | null> { - const range = await gitRangeContext(cwd); + const range = await gitRangeContext(cwd, base); let parsed: PrContent | null = null; try { const output = await runCursorTextPrompt({ diff --git a/src/lib/harness/grokGit.ts b/src/lib/harness/grokGit.ts index 7a658bce..5b14d45b 100644 --- a/src/lib/harness/grokGit.ts +++ b/src/lib/harness/grokGit.ts @@ -36,8 +36,9 @@ export async function generateGrokCommitMessage(cwd: string): Promise { export async function generateGrokPrContent( cwd: string, + base?: string | null, ): Promise<(PrContent & { base: string; head: string }) | null> { - const range = await gitRangeContext(cwd); + const range = await gitRangeContext(cwd, base); let parsed: PrContent | null = null; try { const output = await runGrokTextPrompt({ diff --git a/src/lib/harness/opencodeGit.ts b/src/lib/harness/opencodeGit.ts index f3f3a1c5..625b9c49 100644 --- a/src/lib/harness/opencodeGit.ts +++ b/src/lib/harness/opencodeGit.ts @@ -36,8 +36,9 @@ export async function generateOpenCodeCommitMessage(cwd: string): Promise { - const range = await gitRangeContext(cwd); + const range = await gitRangeContext(cwd, base); let parsed: PrContent | null = null; try { const output = await runOpenCodeTextPrompt({ diff --git a/src/lib/harness/registry.ts b/src/lib/harness/registry.ts index 9ea8c417..76d249d5 100644 --- a/src/lib/harness/registry.ts +++ b/src/lib/harness/registry.ts @@ -54,9 +54,11 @@ export type HarnessAdapter = { generateTitle?(input: TitleInput): Promise; /** Optional LLM commit message from staged changes. */ generateCommitMessage?(cwd: string): Promise; - /** Optional LLM pull request title/body from branch diff context. */ + /** Optional LLM pull request title/body from branch diff context. + * `base` overrides the repository default as the comparison base. */ generatePrContent?( cwd: string, + base?: string | null, ): Promise<(PrContent & { base: string; head: string }) | null>; /** Optional LLM branch name from a user message. */ generateBranchName?(cwd: string, message: string): Promise; @@ -278,10 +280,11 @@ export async function generateHarnessCommitMessage( export async function generateHarnessPrContent( harness: HarnessId, cwd: string, + base?: string | null, ): Promise<(PrContent & { base: string; head: string }) | null> { const adapter = getHarness(harness); if (!adapter?.generatePrContent) return null; - return adapter.generatePrContent(cwd); + return adapter.generatePrContent(cwd, base); } export async function generateHarnessBranchName( diff --git a/src/lib/harness/textHarness.ts b/src/lib/harness/textHarness.ts index a50e9107..585de697 100644 --- a/src/lib/harness/textHarness.ts +++ b/src/lib/harness/textHarness.ts @@ -38,9 +38,12 @@ export function generateCommitMessage( return generateHarnessCommitMessage(pickTextHarness(preferred), cwd); } +/** `base` is the ref the branch is being reviewed against, defaulting to the + * repository default when omitted. */ export function generatePrContent( cwd: string, preferred?: HarnessId, + base?: string | null, ): Promise<(PrContent & { base: string; head: string }) | null> { - return generateHarnessPrContent(pickTextHarness(preferred), cwd); + return generateHarnessPrContent(pickTextHarness(preferred), cwd, base); }