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.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 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)
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
| ext = node.properties['ext'] | ||
| path = node.properties['path'] | ||
| name = node.properties['name'] | ||
| file = File(path, name, ext) |
| self.assertIsNotNone(f) | ||
| self.assertEqual(f.properties['name'], 'src.c') | ||
| self.assertEqual(f.properties['ext'], '.c') | ||
| self.assertEqual(File('', 'src.c', '.c'), f) |
|
|
||
| def test_add_file(self): | ||
| file = File(Path('/path/to/file.txt'), None) | ||
| file = File('/path/to/file', 'file', 'txt') |
| def test_file_add_function(self): | ||
| file = File(Path('/path/to/file.txt'), None) | ||
| self.graph.add_file(file) | ||
| file = File('/path/to/file', 'file', 'txt') |
| self.assertIsNotNone(f) | ||
| self.assertEqual(f.properties['name'], 'src.py') | ||
| self.assertEqual(f.properties['ext'], '.py') | ||
| self.assertEqual(File('', 'src.py', '.py'), f) |
| from api import Graph | ||
| from api.entities import File | ||
| from typing import List, Optional | ||
| from api import * |
| from falkordb import FalkorDB | ||
| from api import Graph | ||
| from api.entities import File | ||
| from typing import List, Optional |
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