Conversation
- Introduced `isFetchingGraph` state to manage loading state in the graph component. - Updated `CodeGraph` to display a loading spinner while fetching the graph. - Enhanced `Combobox` to show a loading state when fetching options.
- Updated the Combobox component to disable the select input when there are no options and not fetching options, improving user experience.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe changes introduce a new fetching state tracking system for graph operations in the App component, propagating it to CodeGraph to display loading indicators. Additionally, the Combobox component implements throttled repository option fetching with loading visuals and improved error handling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
|
Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here. PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
|
Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here. PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||
|
Static Code Review 📊 ✅ All quality checks passed! |
![typo-app[bot]](https://avatars.githubusercontent.com/in/360763?s=60&v=4){kind=small}
Resolve merge conflicts: - app/components/graphView.tsx: accept deletion (debugger fix already in staging) - app/page.tsx: accept deletion, apply isFetchingGraph to app/src/App.tsx - app/src/components/code-graph.tsx: merge Loader2+Button imports, add loading spinner - app/src/components/combobox.tsx: merge loading state with staging API/headers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
…tion or class' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/src/components/combobox.tsx (1)
61-66: Throttle timestamp set before async fetch completes.
setLastFetch(now)is called beforefetchOptions()starts. If the fetch fails, users must still wait 30 seconds before retrying. Consider movingsetLastFetchinto thetryblock after a successful fetch, or intofinally, so failed fetches don't block immediate retries.♻️ Proposed fix
//check if last fetch was less than 30 seconds ago if (lastFetch && now - lastFetch < 30000) return; - - setLastFetch(now); - - fetchOptions() + + fetchOptions().then(() => setLastFetch(now)) }, [open])Or update the timestamp in
fetchOptionsafter success.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/combobox.tsx` around lines 61 - 66, The throttle timestamp is set before the async fetch completes, which blocks retries if fetchOptions() fails; move the setLastFetch(now) call out of the pre-fetch path and instead update lastFetch only after a successful fetch (e.g. inside the try after await fetchOptions()) or alternatively in a finally block if you want to always record attempts; adjust the code around lastFetch, setLastFetch, fetchOptions and now so errors don't prevent immediate retry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/components/combobox.tsx`:
- Line 70: The Select is allowing placeholder strings to be chosen (causing
onSelectedValue to receive "Fetching options..." or "No options found"); update
the Select usage (component named Select with props
open/onOpenChange/setOpen/disabled/value/onValueChange) to prevent selection of
placeholders by disabling the control while options are loading or empty (i.e.,
include isFetchingOptions in the disabled expression), and ensure the value
passed is undefined/null when showing a placeholder instead of using the
placeholder text as the selected value; alternatively remove placeholder items
from the option list so only real options can be selected and onSelectedValue is
only called for actual option values.
- Around line 27-48: The fetch call in the async block that populates options
can throw network errors which are currently swallowed; wrap the await
fetch(...) and response handling in a try/catch (or add a catch after the
existing try/finally) to catch exceptions, call toast with a descriptive error
message (using the caught error.message) and ensure setIsFetchingOptions(false)
remains in the finally; update the logic around setOptions(...) so it only runs
on success and reference the existing symbols fetch(`/api/list_repos`),
setOptions, setIsFetchingOptions, and toast when implementing the catch and
toast error handling.
---
Nitpick comments:
In `@app/src/components/combobox.tsx`:
- Around line 61-66: The throttle timestamp is set before the async fetch
completes, which blocks retries if fetchOptions() fails; move the
setLastFetch(now) call out of the pre-fetch path and instead update lastFetch
only after a successful fetch (e.g. inside the try after await fetchOptions())
or alternatively in a finally block if you want to always record attempts;
adjust the code around lastFetch, setLastFetch, fetchOptions and now so errors
don't prevent immediate retry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 21575903-fc11-444c-b409-d821e572aeac
📒 Files selected for processing (3)
app/src/App.tsxapp/src/components/code-graph.tsxapp/src/components/combobox.tsx
| try { | ||
| const result = await fetch(`/api/list_repos`, { | ||
| method: 'GET', | ||
| headers: { | ||
| ...AUTH_HEADERS, | ||
| }, | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| const json = await result.json() | ||
| setOptions(json.repositories) | ||
| if (!result.ok) { | ||
| toast({ | ||
| variant: "destructive", | ||
| title: "Uh oh! Something went wrong.", | ||
| description: await result.text(), | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| const json = await result.json() | ||
| setOptions(json.repositories) | ||
| } finally { | ||
| setIsFetchingOptions(false) | ||
| } |
There was a problem hiding this comment.
Network errors are silently swallowed.
If fetch() throws (e.g., network failure), the finally block clears the loading state, but the user receives no feedback. Consider adding a catch block to display an error toast.
🛠️ Proposed fix
try {
const result = await fetch(`/api/list_repos`, {
method: 'GET',
headers: {
...AUTH_HEADERS,
},
})
if (!result.ok) {
toast({
variant: "destructive",
title: "Uh oh! Something went wrong.",
description: await result.text(),
})
return
}
const json = await result.json()
setOptions(json.repositories)
+ } catch (error) {
+ toast({
+ variant: "destructive",
+ title: "Uh oh! Something went wrong.",
+ description: "Failed to fetch repositories. Please try again.",
+ })
} finally {
setIsFetchingOptions(false)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/combobox.tsx` around lines 27 - 48, The fetch call in the
async block that populates options can throw network errors which are currently
swallowed; wrap the await fetch(...) and response handling in a try/catch (or
add a catch after the existing try/finally) to catch exceptions, call toast with
a descriptive error message (using the caught error.message) and ensure
setIsFetchingOptions(false) remains in the finally; update the logic around
setOptions(...) so it only runs on success and reference the existing symbols
fetch(`/api/list_repos`), setOptions, setIsFetchingOptions, and toast when
implementing the catch and toast error handling.
|
|
||
| return ( | ||
| <Select open={open} onOpenChange={setOpen} value={selectedValue} onValueChange={onSelectedValue}> | ||
| <Select open={open} onOpenChange={setOpen} disabled={options.length === 0 && !isFetchingOptions} value={isFetchingOptions ? "Fetching options..." : options.length !== 0 ? selectedValue : "No options found"} onValueChange={onSelectedValue}> |
There was a problem hiding this comment.
Placeholder items are selectable, which may trigger unintended behavior.
During loading, the Select is enabled and shows a "Fetching options..." item. If the user clicks it, onSelectedValue("Fetching options...") will be called, potentially causing errors or unexpected state in the parent. Similarly for "No options found".
Consider disabling the Select during loading, or making placeholder items non-interactive:
🛠️ Proposed fix - disable Select during loading
- <Select open={open} onOpenChange={setOpen} disabled={options.length === 0 && !isFetchingOptions} value={isFetchingOptions ? "Fetching options..." : options.length !== 0 ? selectedValue : "No options found"} onValueChange={onSelectedValue}>
+ <Select open={open} onOpenChange={setOpen} disabled={isFetchingOptions || options.length === 0} value={isFetchingOptions ? undefined : options.length !== 0 ? selectedValue : undefined} onValueChange={onSelectedValue}>This disables the Select while fetching, preventing accidental selection of placeholder items.
Also applies to: 76-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/combobox.tsx` at line 70, The Select is allowing
placeholder strings to be chosen (causing onSelectedValue to receive "Fetching
options..." or "No options found"); update the Select usage (component named
Select with props open/onOpenChange/setOpen/disabled/value/onValueChange) to
prevent selection of placeholders by disabling the control while options are
loading or empty (i.e., include isFetchingOptions in the disabled expression),
and ensure the value passed is undefined/null when showing a placeholder instead
of using the placeholder text as the selected value; alternatively remove
placeholder items from the option list so only real options can be selected and
onSelectedValue is only called for actual option values.
3 participants
PR Type
Enhancement, Bug fix
Description
Added loading indicators for graph fetching and options loading.
isFetchingGraphstate for graph loading.CodeGraphto display a spinner during graph fetching.Comboboxto show a loading state when fetching options.Improved
Comboboxbehavior:Fixed minor issues and improved user experience:
debuggerstatement inGraphView.Changes walkthrough 📝
code-graph.tsx
Added loading state and spinner for graph fetchingapp/components/code-graph.tsx
isFetchingGraphstate to manage graph loading.combobox.tsx
Enhanced combobox with loading state and error handlingapp/components/combobox.tsx
isFetchingOptionsstate for options loading.page.tsx
Added graph loading state and improved error handlingapp/page.tsx
isFetchingGraphstate to manage graph loading.CodeGraphcomponent to use the new loading state.graphView.tsx
Minor cleanup and zoom behavior improvementapp/components/graphView.tsx
debuggerstatement.PR Summary by Typo
Overview
This PR addresses issue #426 by implementing loading states across the application, enhancing user experience during data fetching for graph visualizations and repository selection.
Key Changes
isFetchingGraphstate to display a loading spinner and message while fetching graph data inCodeGraph.tsx.isFetchingOptionsstate and a loading spinner to the repositoryCombobox.tsxwhen fetching available repositories, along with a 30-second caching mechanism.isFetchingGraphstate intopage.tsxto manage the loading status for graph data.debuggerstatement fromgraphView.tsx.Work Breakdown
To turn off PR summary, please visit Notification settings.
Summary by CodeRabbit
New Features
Improvements