-
Notifications
You must be signed in to change notification settings - Fork 214
feat: Added the ability to browser different branches and tag from th… #822
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: Added the ability to browser different branches and tag from th… #822
Conversation
…e /browse page. - Added a branch/tag selector dropdown component.
WalkthroughThis PR introduces branch and tag selection functionality for repositories. It adds a new React component for selecting Git references, creates API endpoints to fetch refs filtered by indexed revisions, integrates the selector into the file tree panel, and updates search logic to normalize and query multiple ref variants. Changes
Sequence DiagramsequenceDiagram
actor User
participant UI as BranchTagSelector Component
participant Router as Next.js Router
participant Client as API Client
participant Server as /api/refs Handler
participant Git as Git Repository
participant Search as Search System
User->>UI: Select branch/tag
UI->>UI: onRefChange callback triggered
UI->>Router: Navigate with new ref parameter
Router->>Search: New path loaded with updated ref
Search->>Client: Query fileSource with new ref
Client->>Server: POST /api/refs (validate ref)
Server->>Git: Fetch branches and tags
Git-->>Server: Return filtered refs
Server-->>Client: Return refs response
Client->>Search: Parse and normalize refs
Search-->>Search: Execute parallel searches across ref variants
Search-->>UI: Update file tree with new ref context
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@brendan-kellam lmk what your thoughts are or if you have any questions/concerns/suggestions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/web/src/app/`[domain]/components/branchTagSelector.tsx:
- Around line 30-40: Destructure isError from the useQuery call that fetches
refs (the one using queryKey ['refs', repoName] and queryFn that calls getRefs
and isServiceError) and add a conditional render branch that shows an error
UI/message when isError is true instead of falling back to "No branches
available"; do the same change for the other identical rendering block around
the later lines (the same pattern that lists branches/tags), ensuring the UI
surfaces the failure and does not silently render empty lists when getRefs
throws.
In `@packages/web/src/app/api/`(server)/refs/route.ts:
- Around line 9-21: The POST handler should catch JSON parse errors from
request.json() so malformed JSON results in schema validation errors instead of
an unhandled 500; wrap the call to request.json() in a try/catch inside the
exported POST function, on parse error set body to null (or undefined) before
calling getRefsRequestSchema.safeParseAsync(body) and let the existing
validation/error paths handle the response. Update the block around
request.json(), parsed/getRefs, and serviceErrorResponse to use this guarded
body, and apply the same pattern to other POST/PUT route handlers that call
request.json().
In `@packages/web/src/features/search/parser.ts`:
- Around line 263-278: RevisionExpr currently maps short revision names only to
refs/heads/* via normalizedPattern, so update the RevisionExpr handling to emit
both branch and tag forms for short refs: when value is neither '*' nor 'HEAD'
and does not already start with 'refs/', produce both `refs/heads/${value}` and
`refs/tags/${value}` (instead of a single `refs/heads/${value}`) so callers can
try both; adjust the returned shape from the RevisionExpr case (refer to
normalizedPattern and the branch object) to include both patterns (e.g., an
array of patterns or a dual-pattern field) consistent with how fileSourceApi.ts
resolves short refs.
🧹 Nitpick comments (2)
packages/web/src/app/[domain]/components/pathHeader.tsx (1)
60-60: Unused propbranchDisplayTitleshould be removed from the interface.The prop is destructured with an underscore prefix indicating it's intentionally unused, but it's still defined in the
FileHeaderPropsinterface at line 37. This creates a confusing API where consumers might pass this prop expecting it to have an effect.Consider either:
- Removing
branchDisplayTitlefrom the interface if the tooltip functionality is no longer needed, or- Restoring the title attribute if it was removed accidentally.
♻️ Option 1: Remove the unused prop from the interface
interface FileHeaderProps { path: string; pathHighlightRange?: { from: number; to: number; } pathType?: 'blob' | 'tree'; repo: { name: string; codeHostType: CodeHostType; displayName?: string; externalWebUrl?: string; }, isBranchDisplayNameVisible?: boolean; branchDisplayName?: string; revisionName?: string; - branchDisplayTitle?: string; isCodeHostIconVisible?: boolean; isFileIconVisible?: boolean; repoNameClassName?: string; }And update the destructuring:
export const PathHeader = ({ repo, path, pathHighlightRange, revisionName, branchDisplayName = revisionName, isBranchDisplayNameVisible = !!branchDisplayName, - branchDisplayTitle: _branchDisplayTitle, pathType = 'blob',packages/web/src/app/[domain]/components/branchTagSelector.tsx (1)
89-157: Addtype="button"to avoid accidental form submits.If this component is ever placed inside a
<form>, the buttons could submit it by default.♻️ Proposed tweak
<button + type="button" className="flex items-center gap-1.5 px-2 py-1 text-xs font-semibold text-gray-700 hover:bg-gray-100 rounded-md transition-colors" aria-label="Switch branches or tags" > @@ <button + type="button" onClick={() => setActiveTab('branches')} className={cn( "flex-1 px-4 py-2 text-sm font-medium transition-colors border-b-2", @@ <button + type="button" onClick={() => setActiveTab('tags')} className={cn( "flex-1 px-4 py-2 text-sm font-medium transition-colors border-b-2",
| const { data: refsData, isLoading } = useQuery({ | ||
| queryKey: ['refs', repoName], | ||
| queryFn: async () => { | ||
| const result = await getRefs({ repoName }); | ||
| if (isServiceError(result)) { | ||
| throw new Error('Failed to fetch refs'); | ||
| } | ||
| return result; | ||
| }, | ||
| enabled: open || currentRef === 'HEAD', | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the file exists and read it
FILE="packages/web/src/app/[domain]/components/branchTagSelector.tsx"
if [ -f "$FILE" ]; then
wc -l "$FILE"
else
echo "File not found: $FILE"
exit 1
fiRepository: sourcebot-dev/sourcebot
Length of output: 133
🏁 Script executed:
#!/bin/bash
# Read the relevant sections of the file
FILE="packages/web/src/app/[domain]/components/branchTagSelector.tsx"
# Read lines 25-45 to see context around line 30-40
sed -n '25,45p' "$FILE"
echo "---"
# Read lines 155-170 to see context around line 160-165
sed -n '155,170p' "$FILE"Repository: sourcebot-dev/sourcebot
Length of output: 1649
🏁 Script executed:
#!/bin/bash
# Search for the useQuery call to see all destructured variables
FILE="packages/web/src/app/[domain]/components/branchTagSelector.tsx"
rg "useQuery" "$FILE" -A 10 -B 2Repository: sourcebot-dev/sourcebot
Length of output: 990
🏁 Script executed:
#!/bin/bash
FILE="packages/web/src/app/[domain]/components/branchTagSelector.tsx"
# Read the complete conditional rendering block
sed -n '157,180p' "$FILE"Repository: sourcebot-dev/sourcebot
Length of output: 1501
🏁 Script executed:
#!/bin/bash
FILE="packages/web/src/app/[domain]/components/branchTagSelector.tsx"
# Check the entire file for any error handling or isError references
rg "isError|error" "$FILE" -iRepository: sourcebot-dev/sourcebot
Length of output: 211
Add error state handling to surface fetch failures.
The useQuery hook only destructures isLoading, leaving isError unchecked. When getRefs fails and throws an error, React Query sets isError internally, but the UI silently falls back to empty lists ("No branches available") rather than showing a failure message. This masks failures from the user.
Destructure isError from useQuery and add a conditional branch to render an error state.
🔧 Proposed fix
- const { data: refsData, isLoading } = useQuery({
+ const { data: refsData, isLoading, isError } = useQuery({
queryKey: ['refs', repoName],
queryFn: async () => {
const result = await getRefs({ repoName });
if (isServiceError(result)) {
throw new Error('Failed to fetch refs');
}
return result;
},
enabled: open || currentRef === 'HEAD',
});
@@
- {isLoading ? (
+ {isLoading ? (
<div className="p-4 text-sm text-gray-500 text-center">
Loading...
</div>
- ) : (
+ ) : isError ? (
+ <div className="p-4 text-sm text-red-600 text-center">
+ Failed to load branches/tags
+ </div>
+ ) : (
<div className="p-1">Also applies to the same rendering pattern at lines 160-165.
🤖 Prompt for AI Agents
In `@packages/web/src/app/`[domain]/components/branchTagSelector.tsx around lines
30 - 40, Destructure isError from the useQuery call that fetches refs (the one
using queryKey ['refs', repoName] and queryFn that calls getRefs and
isServiceError) and add a conditional render branch that shows an error
UI/message when isError is true instead of falling back to "No branches
available"; do the same change for the other identical rendering block around
the later lines (the same pattern that lists branches/tags), ensuring the UI
surfaces the failure and does not silently render empty lists when getRefs
throws.
| export const POST = async (request: NextRequest) => { | ||
| const body = await request.json(); | ||
| const parsed = await getRefsRequestSchema.safeParseAsync(body); | ||
| if (!parsed.success) { | ||
| return serviceErrorResponse(requestBodySchemaValidationError(parsed.error)); | ||
| } | ||
|
|
||
| const response = await getRefs(parsed.data); | ||
| if (isServiceError(response)) { | ||
| return serviceErrorResponse(response); | ||
| } | ||
|
|
||
| return Response.json(response); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's understand the codebase structure and look for error handling patterns
find packages/web -name "*.ts" -o -name "*.tsx" | head -20Repository: sourcebot-dev/sourcebot
Length of output: 1265
🏁 Script executed:
# Search for middleware or error handling related to API routes
rg -t ts "middleware|error.*handler|catch.*json\(\)" packages/web/src --max-count 20Repository: sourcebot-dev/sourcebot
Length of output: 631
🏁 Script executed:
# Check the specific file and its surrounding context
cat -n packages/web/src/app/api/\(server\)/refs/route.tsRepository: sourcebot-dev/sourcebot
Length of output: 966
🏁 Script executed:
# Look for other API routes to understand error handling patterns
fd -t f "route.ts" packages/web/src/app/api --exec cat -n {} \;Repository: sourcebot-dev/sourcebot
Length of output: 50044
🏁 Script executed:
# Check if there's a global error handler or middleware setup
rg -t ts "error.*Middleware|Error.*Handler|catch.*Error" packages/web/src/app --max-count 15Repository: sourcebot-dev/sourcebot
Length of output: 145
Wrap request.json() in try-catch to prevent unhandled errors on malformed JSON.
request.json() throws on invalid JSON, returning a 500 instead of a validation error. Catching the error and passing null to schema validation provides consistent error handling.
🔧 Proposed fix
export const POST = async (request: NextRequest) => {
- const body = await request.json();
+ let body: unknown = null;
+ try {
+ body = await request.json();
+ } catch {
+ // fall through to schema validation error
+ }
const parsed = await getRefsRequestSchema.safeParseAsync(body);
if (!parsed.success) {
return serviceErrorResponse(requestBodySchemaValidationError(parsed.error));
}Note: This pattern should be applied to all POST/PUT endpoints that call request.json().
🤖 Prompt for AI Agents
In `@packages/web/src/app/api/`(server)/refs/route.ts around lines 9 - 21, The
POST handler should catch JSON parse errors from request.json() so malformed
JSON results in schema validation errors instead of an unhandled 500; wrap the
call to request.json() in a try/catch inside the exported POST function, on
parse error set body to null (or undefined) before calling
getRefsRequestSchema.safeParseAsync(body) and let the existing validation/error
paths handle the response. Update the block around request.json(),
parsed/getRefs, and serviceErrorResponse to use this guarded body, and apply the
same pattern to other POST/PUT route handlers that call request.json().
| case RevisionExpr: { | ||
| // normalize the ref to match zoekt's indexed format | ||
| // zoekt stores branches as 'refs/heads/branch-name' and tags as 'refs/tags/tag-name' | ||
| const normalizedPattern = value === '*' ? "" : | ||
| value === 'HEAD' ? 'HEAD' : | ||
| value.startsWith('refs/') ? value : | ||
| `refs/heads/${value}`; | ||
|
|
||
| return { | ||
| branch: { | ||
| // Special case - "*" means search all branches. Passing in a | ||
| // blank string will match all branches. | ||
| pattern: value === '*' ? "" : value, | ||
| pattern: normalizedPattern, | ||
| exact: false | ||
| }, | ||
| query: "branch" | ||
| }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Explore repository structure to understand the codebase
find . -type f -name "*.ts" | grep -E "(parser|fileSourceApi)" | head -20Repository: sourcebot-dev/sourcebot
Length of output: 247
🏁 Script executed:
#!/bin/bash
# Check the parser.ts file content around lines 263-278
if [ -f "packages/web/src/features/search/parser.ts" ]; then
sed -n '260,280p' packages/web/src/features/search/parser.ts
fiRepository: sourcebot-dev/sourcebot
Length of output: 802
🏁 Script executed:
#!/bin/bash
# Find fileSourceApi.ts and look for getRefsToTry implementation
find . -type f -name "*fileSourceApi*"Repository: sourcebot-dev/sourcebot
Length of output: 118
🏁 Script executed:
#!/bin/bash
# Search for references to zoekt and how it handles branches/tags
rg -n "refs/heads|refs/tags" --type=ts | head -30Repository: sourcebot-dev/sourcebot
Length of output: 1647
🏁 Script executed:
#!/bin/bash
# Examine fileSourceApi.ts to see getRefsToTry implementation and context
cat -n packages/web/src/features/search/fileSourceApi.ts | head -50Repository: sourcebot-dev/sourcebot
Length of output: 2585
🏁 Script executed:
#!/bin/bash
# Look for how parser.ts RevisionExpr is used and if there's any other handling for tags
rg -n -B5 -A15 "case RevisionExpr" packages/web/src/features/search/parser.tsRepository: sourcebot-dev/sourcebot
Length of output: 916
🏁 Script executed:
#!/bin/bash
# Check if there are tests for RevisionExpr that might clarify intended behavior
rg -n -l "RevisionExpr" --type=ts | xargs grep -l "test\|spec\|describe" 2>/dev/null | head -10Repository: sourcebot-dev/sourcebot
Length of output: 109
🏁 Script executed:
#!/bin/bash
# Search for any documentation or comments about tag handling in revision searches
rg -n "tag.*rev|rev.*tag" --type=ts -iRepository: sourcebot-dev/sourcebot
Length of output: 12493
🏁 Script executed:
#!/bin/bash
# Check how parser output (with branch: query) is used in search
rg -n "branch.*pattern|query.*branch" packages/web/src/features/search/ --type=ts -A3 | head -40Repository: sourcebot-dev/sourcebot
Length of output: 2193
🏁 Script executed:
#!/bin/bash
# Look for search implementation to understand how branch query is processed
cat -n packages/web/src/features/search/searchApi.ts | head -80Repository: sourcebot-dev/sourcebot
Length of output: 3499
🏁 Script executed:
#!/bin/bash
# Check if there are tests or examples showing rev: usage with tags
rg -n "rev:" --type=ts --type=tsx | grep -E "test|tag|v\d" | head -20Repository: sourcebot-dev/sourcebot
Length of output: 94
🏁 Script executed:
#!/bin/bash
# Look for any logic that transforms the parser output for zoekt queries
rg -n -B3 -A10 "query.*branch|branch.*query" packages/web/src/features/search/searchApi.tsRepository: sourcebot-dev/sourcebot
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Look for zoektSearcher to see how branch queries are converted
cat -n packages/web/src/features/search/zoektSearcher.ts | head -150Repository: sourcebot-dev/sourcebot
Length of output: 8429
🏁 Script executed:
#!/bin/bash
# Search for how branch pattern is used in zoekt
rg -n "branch" packages/web/src/features/search/zoektSearcher.ts -B2 -A5Repository: sourcebot-dev/sourcebot
Length of output: 1759
🏁 Script executed:
#!/bin/bash
# Check ir.ts to understand the QueryIR structure
cat -n packages/web/src/features/search/ir.tsRepository: sourcebot-dev/sourcebot
Length of output: 11110
🏁 Script executed:
#!/bin/bash
# Check if there are tests for rev: syntax with tags
find . -type f \( -name "*.test.ts" -o -name "*.spec.ts" \) | xargs grep -l "rev:" 2>/dev/null | head -5Repository: sourcebot-dev/sourcebot
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Search for any documentation or comments about zoekt branch pattern matching
rg -n "pattern.*regex|exact.*false" packages/web/src/features/search/ --type=ts -B2 -A2Repository: sourcebot-dev/sourcebot
Length of output: 479
🏁 Script executed:
#!/antml/bash
# Check if there's any logic to handle tags differently in parser
rg -n "tag\|branch" packages/web/src/features/search/parser.ts -i -B2 -A2Repository: sourcebot-dev/sourcebot
Length of output: 49
🏁 Script executed:
#!/an/bash
# Check how zoekt's Branch query type works - look at proto definitions
find . -path "*/proto/*" -name "*.proto" -o -name "*webserver*" | grep -i zoekt | head -10Repository: sourcebot-dev/sourcebot
Length of output: 107
🏁 Script executed:
#!/bin/bash
# Search for any comments or issues about tag handling in revision searches
rg -n -i "todo|fixme|bug|issue" packages/web/src/features/search/parser.ts | head -20Repository: sourcebot-dev/sourcebot
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Check the search landing page to understand what users should expect
cat -n packages/web/src/app/[domain]/search/components/searchLandingPage.tsx | grep -A5 -B5 "by branch or tag"Repository: sourcebot-dev/sourcebot
Length of output: 855
🏁 Script executed:
#!/bin/bash
# Check if there are any comments in fileSourceApi comparing behavior with parser
cat -n packages/web/src/features/search/fileSourceApi.ts | sed -n '15,45p'Repository: sourcebot-dev/sourcebot
Length of output: 1526
RevisionExpr normalization only supports short branch names, not tags.
The parser normalizes short revision names to refs/heads/${value} only, which means queries like rev:v1.0.0 will not match tags stored as refs/tags/v1.0.0.
This is inconsistent with fileSourceApi.ts, which explicitly tries both refs/heads/${ref} and refs/tags/${ref} patterns when resolving short ref names (see lines 28-29). The UI also suggests both should work (searchLandingPage.tsx displays "rev:HEAD (by branch or tag)").
To align behavior: normalize short names to support both branch and tag patterns, similar to the approach in fileSourceApi.ts.
🤖 Prompt for AI Agents
In `@packages/web/src/features/search/parser.ts` around lines 263 - 278,
RevisionExpr currently maps short revision names only to refs/heads/* via
normalizedPattern, so update the RevisionExpr handling to emit both branch and
tag forms for short refs: when value is neither '*' nor 'HEAD' and does not
already start with 'refs/', produce both `refs/heads/${value}` and
`refs/tags/${value}` (instead of a single `refs/heads/${value}`) so callers can
try both; adjust the returned shape from the RevisionExpr case (refer to
normalizedPattern and the branch object) to include both patterns (e.g., an
array of patterns or a dual-pattern field) consistent with how fileSourceApi.ts
resolves short refs.
|
thanks @CyberT17 , I'm out atm but will take a look later |
Added the ability to browser different branches and tag from the /browse page.
implements [FR] Branch selector dropdown in the
/browseview #810Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.