feat(review): custom base branch for code review (#599)#602
Open
backnotprop wants to merge 4 commits intomainfrom
Open
feat(review): custom base branch for code review (#599)#602backnotprop wants to merge 4 commits intomainfrom
backnotprop wants to merge 4 commits intomainfrom
Conversation
The "Branch diff" and "PR Diff" modes were locked to whatever the repo
auto-detects as the default branch. Users on feature branches cut from
`develop`, `release/*`, or another feature branch had no way to re-base
the review. Adds a searchable branch picker that appears next to the
diff-type dropdown when the active mode uses a base.
- Shared: listBranches + resolveBaseBranch in review-core; GitContext
gains availableBranches. Both Bun (review.ts) and Pi (serverReview.ts)
consume the same helpers; Pi picks them up via vendor.sh.
- Server: /api/diff/switch and /api/file-content accept optional `base`,
fall back to detected default when unknown.
- UI: BaseBranchPicker (Radix Popover + search + grouped local/remote).
Mounts only for branch/merge-base modes, hidden in PR mode. Diff-type
labels become generic ("Branch diff", "PR Diff") when the picker is
wired up — branch name lives in the picker.
- Session-only state (no cookie) to avoid the random-port cookie-jar
quirk that would make local persistence look broken.
For provenance purposes, this commit was AI assisted.
…isible Addresses review feedback on #602 — three related issues where my dedup + detected-default logic could force diffs against stale or non-existent refs. - getDefaultBranch returns the full remote ref (origin/main) instead of stripping to bare main. Diffs now run against the upstream tip by default, which stays fresh; local main falls out of the picture when it's stale. Falls back to local main/master only when no remote is configured. - listBranches no longer dedupes origin/foo when local foo exists. Both refs stay selectable — they can point to different commits, and the user needs to be able to pick either explicitly. - resolveBaseBranch gains a reverse fallback: bare `main` resolves to `origin/main` when only the remote is tracked (fresh clones). - getGitContext treats `origin/X` and `X` as equivalent when deciding whether to show branch/merge-base options — otherwise users on local main would suddenly see Branch diff / PR Diff options after the detected-default change, which wasn't the intent. For provenance purposes, this commit was AI assisted.
…fault The Codex/Claude/Tour prompt builders were reading gitContext.defaultBranch, which is frozen at server startup. After a user switched to a different base via /api/diff/switch, the agent was told to run `git diff <stale-default>..HEAD` and analyzed a different diff than the one in the UI. Tracks the active base as server-side state (`currentBase`), initialized from the detected default and updated in /api/diff/switch alongside currentDiffType. buildCommand reads currentBase instead. Mirrored on Bun and Pi. For provenance purposes, this commit was AI assisted.
Replaces the three native <select> dropdowns with Radix primitives that match the base-branch picker visually and behaviorally. - DiffTypePicker (Radix DropdownMenu): per-option info icon with a plain-English tooltip explaining what each mode shows. References "picked below" to make the base picker's relationship explicit. - WorktreePicker (Radix Popover + search): matches BaseBranchPicker shape. Search input appears only when there are >3 worktrees. - Tooltip gains an opt-in `wide` prop so multi-line hints wrap cleanly. Existing nowrap callsites unaffected. - TooltipProvider mounted at the review-editor App root. - ReviewDiffPanel keys on `reviewBase` to remount DiffViewer on base change — fixes a transient "trailing context mismatch" warning from @pierre/diffs caused by a race between the new patch and the new file-content fetch. For provenance purposes, this commit was AI assisted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
develop,release/*, or another feature branch can actually review the right delta instead of being locked to the auto-detected default.packages/shared/review-core.ts(listBranches,resolveBaseBranch,GitContext.availableBranches) so both the Bun runtime (packages/server/review.ts) and the Pi node runtime (apps/pi-extension/server/serverReview.ts) pick it up — Pi viavendor.sh.How it works
/api/diff/switchand/api/file-contentaccept an optionalbaseparam;resolveBaseBranchvalidates against the known branch list and falls back to the detected default for unknown values.BaseBranchPicker) with search, grouped Local/Remote, "detected" badge, and a "Reset to detected" action. Session-only state (no cookie) to avoid the random-port cookie-jar quirk that would make local persistence look broken.Closes
Closes #599
Test plan
main: picker defaults tomain, switching to any other branch refetches the diff.develop: pickdevelop, confirm the diff now shows only the feature's delta.plannotator review <PR-URL>) → picker does not appear.bash apps/pi-extension/vendor.sh, verify/api/diffreturnsavailableBranchesand acceptsbase.For provenance purposes, this PR was AI assisted.