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.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughApp now tracks graph-fetching with an Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant CodeGraph
participant Server
Note over User,App: User triggers graph fetch (e.g., create/select project)
User->>App: requestFetchGraph(repo)
App->>CodeGraph: set isFetchingGraph = true
App->>Server: fetch graph data(repo)
Server-->>App: graph data / error
App->>CodeGraph: set isFetchingGraph = false
CodeGraph->>CodeGraph: update local options if needed
CodeGraph-->>User: render graph or placeholder (shows spinner while fetching)
sequenceDiagram
participant User
participant Combobox
participant Server
Note over User,Combobox: User opens/selects combobox
User->>Combobox: open()
Combobox->>Combobox: check lastFetch (throttle 30s)
alt allowed
Combobox->>Server: fetch repositories
Server-->>Combobox: repositories / error
Combobox->>Combobox: setOptions(repos), set isFetchingOptions=false
Combobox-->>User: show options
else throttled
Combobox-->>User: use cached options or show “Fetching options...” if empty
end
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 docstrings
🧪 Generate unit tests (beta)
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
- 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.
…tion or class' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
gkorland
force-pushed
the
add-loading-states
branch
from
17706ca to
1b62f19
Compare
gkorland
left a comment
gkorland
left a comment
There was a problem hiding this comment.
Rebased on staging and addressed review comments:
-
Network error handling: Added catch block in
fetchOptions()to show an error toast on fetch failures (network errors were previously swallowed). -
Placeholder selection prevention: Disabled the Select during loading (
isFetchingOptions) and when no options are available. Changed value toundefinedinstead of placeholder text soonSelectedValuecan never receive placeholder strings. -
Rebase conflicts: Resolved conflicts with staging's restructured paths (
app/components/→app/src/components/), API changes (/api/repo→/api/list_repos), and styling updates (text-gray-400→text-muted-foreground).
Add isFetchingGraph state to App.tsx and pass it to both desktop and mobile CodeGraph components. Set true on fetch start, false in finally block so the loading spinner renders correctly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-graph into add-loading-states
The PR bundled test refactors that assumed entity classes (Function, Struct, Class) and a File(path, name, ext) constructor that don't exist on staging. Also reverts incorrect pygit2 -> gitpython switch. Keeps the frontend loading state changes (the actual purpose of this PR) intact while fixing all File.__init__ wrong-argument issues. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add catch block for network errors with toast notification - Disable Select during loading and when no options available - Use placeholder prop instead of selectable placeholder items - Move lastFetch timestamp update to after successful fetch Addresses CodeRabbit review comments on combobox.tsx. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CodeGraph now manages options locally instead of receiving them from App. Remove the dead props from the interface and stop passing them from App.tsx. The Combobox inside CodeGraph fetches options on its own. Addresses typo-app review comment about local state shadowing props. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Agent Review SummaryComments Fixed (agreed & resolved)
Comments DeclinedNone — all actionable comments were addressed. Tests Added / UpdatedNo new tests required — changes are frontend-only (loading states, error handling, prop cleanup). CI StatusAll 10 checks passing ✅ (Build, Docker, CodeQL ×4, CodeRabbit, Playwright ×2) |
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