refactor: split DashboardHome.tsx (489 -> 222 lines) + upgrade CLAUDE.md#250
Merged
DevanshuNEU merged 17 commits intoFeb 21, 2026
Merged
Conversation
Added sections: - React Patterns: derived state, module-scope components, stable keys, state colocation, cn() utility - Testing: Vitest setup, file naming, test locations, CI integration - Code Review: CodeRabbit workflow, PR scope policy - Architecture: full directory tree with descriptions - Data Flow: React Query caching, WebSocket, Supabase caching - Environment Variables: .env.example as source of truth, VITE_ prefix, startup_checks.py for new vars - Expanded What NOT to Do with lessons learned from recent sprints Updated: - Large File Exceptions: removed old DependencyGraph/index.tsx (now 150 lines), added GraphView/index.tsx and MatrixView/index.tsx - Frontend rules: added React Query mandate, hooks directory, types.ts - Backend rules: singleton pattern, startup validation - Commits: added PR scope rule (one concern per PR) Total: 68 -> 136 lines. Every rule earned from real bugs and review findings.
Before: 489 lines, 5 concerns in one file After: 4 files, each with one concern Extracted: - RepoDetailView.tsx (140 lines): single-repo view with tabs, header, back navigation. Receives repo data + callbacks as props. - RepoListView.tsx (81 lines): repo grid with header, add/import buttons, stats. Pure presentational. - lib/api-errors.ts (32 lines): extractErrorMessage, isUpgradeError, safeStringify. Generic utilities, not dashboard-specific. DashboardHome.tsx (222 lines): now just state management, API handlers (addAndIndex, handleGitHubImport, handleReindex), and modal orchestration. Also: - Deduplicated addRepo and githubImport into shared addAndIndex helper - Moved AddRepoForm outside AnimatePresence (was incorrectly nested) - All modals rendered at the bottom of DashboardHome, not inside views Zero visual/behavioral changes. Build passes. 13 tests green. Closes OPE-16
|
@DevanshuNEU is attempting to deploy a commit to the Dev's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
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. 📝 WalkthroughWalkthroughThe PR refactors the dashboard by extracting RepoListView and RepoDetailView from DashboardHome, centralizes frontend error utilities (safeStringify, extractErrorMessage, isUpgradeError), adds MAX_FREE_REPOS and RepoTab types, and substantially expands CLAUDE.md documentation. No backend or public API surfaces were changed. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 |
Collaborator
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
frontend/src/components/dashboard/RepoDetailView.tsx (2)
23-23:RepoTabtype is duplicated across files.
RepoTabis defined identically here and inDashboardHome.tsx(line 20). Per your CLAUDE.md guideline ("Shared types go infrontend/src/types.ts"), consider exporting it fromtypes.tsand importing in both files to keep a single source of truth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/dashboard/RepoDetailView.tsx` at line 23, The RepoTab type is duplicated; extract the union type alias RepoTab from both locations into the shared frontend/src/types.ts and export it, then replace the local definitions in RepoDetailView (RepoTab) and DashboardHome (where RepoTab is duplicated) with an import of the exported RepoTab; update any references to use the imported type so there is a single source of truth.
76-84:.replace('.git', '')may match mid-string.
String.replace('.git', '')replaces the first occurrence anywhere in the URL, not just a trailing suffix. A regex with an end anchor is more precise:Proposed fix
- href={repo.git_url?.replace('.git', '')} + href={repo.git_url?.replace(/\.git$/, '')}The same pattern appears at line 124 in this file and in
DashboardHome.tsxline 90.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/dashboard/RepoDetailView.tsx` around lines 76 - 84, The current use of repo.git_url?.replace('.git', '') can remove a mid-string occurrence; update both occurrences in RepoDetailView and the one in DashboardHome so you only strip a trailing ".git" — either test endsWith(".git") and remove the last 4 characters, or use a regex that matches ".git" anchored to the end of the string — apply this change where repo.git_url is rendered (the anchor link and the other mention) so only a trailing suffix is removed.frontend/src/components/dashboard/RepoListView.tsx (1)
11-11:MAX_FREE_REPOSis duplicated as a magic number.This constant is
3here, and the same limit appears asmaxSelectable={3}inDashboardHome.tsx(line 210). Consider extracting it to a shared constants file (or at least importing this constant inDashboardHome) so the free-tier limit is defined once.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/dashboard/RepoListView.tsx` at line 11, MAX_FREE_REPOS is duplicated as a magic number; extract this value into a single exported constant and import it where needed. Create or use a shared constants module (e.g., export const MAX_FREE_REPOS = 3) and replace the local declaration in RepoListView (remove const MAX_FREE_REPOS = 3) and the hardcoded maxSelectable={3} in DashboardHome to import and use the shared MAX_FREE_REPOS constant instead. Ensure both files import the same symbol name to keep the free-tier limit defined in one place.frontend/src/components/dashboard/DashboardHome.tsx (1)
133-150:handleReindexdoesn't use centralized error handling.Unlike
addAndIndex, this handler doesn't parse the error response body or checkisUpgradeError. If re-indexing can hit backend limits (e.g., repo grew too large), the user would see a generic error instead of the upgrade modal. Consider reusingextractErrorMessagehere for consistency, even if upgrade errors are unlikely during re-index.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/dashboard/DashboardHome.tsx` around lines 133 - 150, Update handleReindex to use the same centralized error parsing and upgrade check as addAndIndex: when the POST response is not ok, parse the response body and pass it to extractErrorMessage to get a user-friendly message, then call isUpgradeError with that message and trigger the same upgrade-modal flow used in addAndIndex (e.g., the component's upgrade modal state setter) instead of always showing a generic toast; also include the extracted message in the toast/log for non-upgrade errors so failures remain informative.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLAUDE.md`:
- Line 71: Replace the hardcoded fork reference string "PR into
`OpenCodeIntel/opencodeintel:main` from `DevanshuNEU/opencodeintel-fork-new`"
with a generic phrasing so the CLAUDE.md remains accurate for all contributors;
update the line to something like "PR into `OpenCodeIntel/opencodeintel:main`
from your fork" or remove the fork portion entirely to avoid referencing the
specific fork name.
- Around line 76-98: Update the fenced code block in CLAUDE.md that contains the
directory tree to include a language specifier (e.g., "text" or "plaintext") so
markdownlint MD040 is satisfied; locate the opening triple-backtick that
precedes the "backend/ # FastAPI, Python 3.11+" line and
change it to include the specifier (e.g., ```text) while leaving the rest of the
block unchanged.
In `@frontend/src/components/dashboard/DashboardHome.tsx`:
- Around line 108-131: handleGitHubImport currently shows toast.success
unconditionally after calling addAndIndex, so when addAndIndex returns null
(upgrade flow) the success toast still appears; update the loop to only call
toast.success and the indexing modal logic (setIndexingRepoId,
setIndexingRepoName, setShowIndexingModal) when repoId is truthy, and skip the
success toast when repoId is null/undefined (but still handle errors and ensure
setLoading(false) and refreshRepos() run after the loop).
---
Nitpick comments:
In `@frontend/src/components/dashboard/DashboardHome.tsx`:
- Around line 133-150: Update handleReindex to use the same centralized error
parsing and upgrade check as addAndIndex: when the POST response is not ok,
parse the response body and pass it to extractErrorMessage to get a
user-friendly message, then call isUpgradeError with that message and trigger
the same upgrade-modal flow used in addAndIndex (e.g., the component's upgrade
modal state setter) instead of always showing a generic toast; also include the
extracted message in the toast/log for non-upgrade errors so failures remain
informative.
In `@frontend/src/components/dashboard/RepoDetailView.tsx`:
- Line 23: The RepoTab type is duplicated; extract the union type alias RepoTab
from both locations into the shared frontend/src/types.ts and export it, then
replace the local definitions in RepoDetailView (RepoTab) and DashboardHome
(where RepoTab is duplicated) with an import of the exported RepoTab; update any
references to use the imported type so there is a single source of truth.
- Around line 76-84: The current use of repo.git_url?.replace('.git', '') can
remove a mid-string occurrence; update both occurrences in RepoDetailView and
the one in DashboardHome so you only strip a trailing ".git" — either test
endsWith(".git") and remove the last 4 characters, or use a regex that matches
".git" anchored to the end of the string — apply this change where repo.git_url
is rendered (the anchor link and the other mention) so only a trailing suffix is
removed.
In `@frontend/src/components/dashboard/RepoListView.tsx`:
- Line 11: MAX_FREE_REPOS is duplicated as a magic number; extract this value
into a single exported constant and import it where needed. Create or use a
shared constants module (e.g., export const MAX_FREE_REPOS = 3) and replace the
local declaration in RepoListView (remove const MAX_FREE_REPOS = 3) and the
hardcoded maxSelectable={3} in DashboardHome to import and use the shared
MAX_FREE_REPOS constant instead. Ensure both files import the same symbol name
to keep the free-tier limit defined in one place.
addAndIndex returns null when the upgrade modal is shown. The toast was firing unconditionally -- now only shows on successful add.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
frontend/src/components/dashboard/RepoDetailView.tsx (1)
23-23:RepoTabtype is duplicated inDashboardHome.tsx(line 20).Both files define the identical
RepoTabtype. Per CLAUDE.md: "Shared types go infrontend/src/types.ts." Consider exporting it from there (or from this file) and importing in both places.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/dashboard/RepoDetailView.tsx` at line 23, The RepoTab type is duplicated; remove the duplicate in RepoDetailView and instead import the shared type from the central types file (export RepoTab from frontend/src/types.ts or add it there if missing) so both RepoDetailView and DashboardHome use the same exported RepoTab definition; update the import statements in RepoDetailView and DashboardHome to reference the shared RepoTab and delete the local type declaration.frontend/src/components/dashboard/DashboardHome.tsx (1)
196-203:onRetrysilently swallows non-upgrade index failures.The
catchblock on line 202 shows a generic toast but doesn't check whether the error is an upgrade error (unlikeaddAndIndex). If the retry fails due to a limit/size issue, the user sees "Failed to retry indexing" instead of the upgrade modal.Proposed improvement
onRetry={async () => { if (!indexingRepoId) return try { - await fetch(`${API_URL}/repos/${indexingRepoId}/index/async`, { + const res = await fetch(`${API_URL}/repos/${indexingRepoId}/index/async`, { method: 'POST', headers: { Authorization: `Bearer ${session?.access_token}` }, }) - } catch { toast.error('Failed to retry indexing') } + if (!res.ok) { + const err = await res.json().catch(() => ({})) + if (isUpgradeError(err)) { showUpgradeModal(err, indexingRepoName); return } + throw new Error(extractErrorMessage(err, 'Failed to retry indexing')) + } + } catch (e) { + toast.error('Failed to retry indexing', { + description: e instanceof Error ? e.message : undefined, + }) + } }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/dashboard/DashboardHome.tsx` around lines 196 - 203, The onRetry handler currently swallows all errors and only shows a generic toast; update the onRetry async function (the handler referencing indexingRepoId, API_URL and session?.access_token) to inspect the failed response/error like addAndIndex does: catch the error or non-2xx response, parse the response body or check status to detect the "upgrade required" / size limit error and trigger the same upgrade modal flow used by addAndIndex (rather than only showing toast), otherwise show the generic toast; ensure you propagate or log unexpected errors appropriately so upgrade conditions surface to the user.frontend/src/components/dashboard/RepoListView.tsx (1)
11-11: Extract the3limit to a shared constants file.The repo limit appears as a magic number in three places:
MAX_FREE_REPOSin RepoListView.tsx,maxSelectable={3}in DashboardHome.tsx (line 210), and the defaultmaxSelectable = 3in GitHubRepoSelector.tsx. Define it once in a constants file and reuse it everywhere to avoid maintenance issues if the limit changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/dashboard/RepoListView.tsx` at line 11, Extract the magic number 3 into a shared constant (e.g., FREE_REPO_LIMIT) in a common constants module, export it, and replace the literal and local constant uses: remove/replace MAX_FREE_REPOS in RepoListView.tsx, replace maxSelectable={3} in DashboardHome.tsx, and replace the default maxSelectable = 3 in GitHubRepoSelector.tsx to import and use the shared FREE_REPO_LIMIT; update imports where needed and run type checks to ensure the new constant is referenced consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/dashboard/DashboardHome.tsx`:
- Around line 116-121: The current check uses reference equality (repo ===
githubRepos[githubRepos.length - 1]) to decide which repo triggers the indexing
modal, which fails if the last array element errors; instead track the last
successfully added repo and open the modal for that one: after successful add
(where repoId is set and toast.success is called) assign a local
lastSuccessfulRepo and lastSuccessfulRepoName, and after the loop call
setIndexingRepoId(lastSuccessfulRepo),
setIndexingRepoName(lastSuccessfulRepoName) and setShowIndexingModal(true) only
if lastSuccessfulRepo is defined (update the logic around repoId, githubRepos,
setIndexingRepoId, setIndexingRepoName, setShowIndexingModal accordingly).
In `@frontend/src/components/dashboard/RepoDetailView.tsx`:
- Around line 76-84: The code currently uses repo.git_url?.replace('.git', '')
which removes the first occurrence of ".git" anywhere in the string; change both
occurrences to strip only a trailing suffix by using a regex anchored to the end
(e.g. repo.git_url?.replace(/\.git$/, '')) or by checking
repo.git_url?.endsWith('.git') and slicing off the last 4 chars; update the
anchor href and the other place where replace is used so URLs like
"https://github.com/user/.github.git" are handled correctly.
---
Duplicate comments:
In `@CLAUDE.md`:
- Line 76: The fenced code block in CLAUDE.md currently uses bare triple
backticks which triggers markdownlint MD040; change the opening fence from ```
to ```text (i.e., add the "text" language specifier) so the directory tree is
marked as plain text; locate the triple backticks in the file and update the
opening fence accordingly.
- Line 71: Replace the contributor-specific hardcoded fork reference "PR into
OpenCodeIntel/opencodeintel:main from DevanshuNEU/opencodeintel-fork-new" in
CLAUDE.md with a generic instruction such as "PR into
OpenCodeIntel/opencodeintel:main from your fork"; update the sentence that
contains that exact phrase so it no longer names a specific contributor and
instead uses the generic wording.
In `@frontend/src/components/dashboard/DashboardHome.tsx`:
- Around line 108-131: The success toast is shown unconditionally in
handleGitHubImport even when addAndIndex returns null (upgrade path); update the
loop to only call toast.success when repoId is truthy (i.e., addAndIndex
returned an actual id) and handle the null case separately (either no toast or a
different message). Specifically modify the block using addAndIndex, repoId,
setIndexingRepoId, setIndexingRepoName and setShowIndexingModal so toast.success
is executed only when repoId is non-null and the upgrade/modal path does not
trigger a success toast.
---
Nitpick comments:
In `@frontend/src/components/dashboard/DashboardHome.tsx`:
- Around line 196-203: The onRetry handler currently swallows all errors and
only shows a generic toast; update the onRetry async function (the handler
referencing indexingRepoId, API_URL and session?.access_token) to inspect the
failed response/error like addAndIndex does: catch the error or non-2xx
response, parse the response body or check status to detect the "upgrade
required" / size limit error and trigger the same upgrade modal flow used by
addAndIndex (rather than only showing toast), otherwise show the generic toast;
ensure you propagate or log unexpected errors appropriately so upgrade
conditions surface to the user.
In `@frontend/src/components/dashboard/RepoDetailView.tsx`:
- Line 23: The RepoTab type is duplicated; remove the duplicate in
RepoDetailView and instead import the shared type from the central types file
(export RepoTab from frontend/src/types.ts or add it there if missing) so both
RepoDetailView and DashboardHome use the same exported RepoTab definition;
update the import statements in RepoDetailView and DashboardHome to reference
the shared RepoTab and delete the local type declaration.
In `@frontend/src/components/dashboard/RepoListView.tsx`:
- Line 11: Extract the magic number 3 into a shared constant (e.g.,
FREE_REPO_LIMIT) in a common constants module, export it, and replace the
literal and local constant uses: remove/replace MAX_FREE_REPOS in
RepoListView.tsx, replace maxSelectable={3} in DashboardHome.tsx, and replace
the default maxSelectable = 3 in GitHubRepoSelector.tsx to import and use the
shared FREE_REPO_LIMIT; update imports where needed and run type checks to
ensure the new constant is referenced consistently.
RepoTab was defined identically in both DashboardHome.tsx and RepoDetailView.tsx. Moved to types.ts as single source of truth.
Was hardcoded as 3 in RepoListView.tsx and DashboardHome.tsx. Now exported from config/api.ts as single source of truth.
Reference equality check (repo === githubRepos[last]) meant if the last repo errored, the modal never opened for earlier successes. Now tracks lastSuccessId/lastSuccessName and opens modal after the loop.
String.replace('.git', '') removes the first occurrence anywhere.
Regex /\.git$/ only strips the trailing suffix, so URLs like
.github.git are handled correctly.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
CLAUDE.md (1)
84-88: Consider listing the new extracted components underdashboard/.The tree shows
Dashboard-specific: Sidebar, TopNav, DashboardHomebut doesn't mention the newly extractedRepoListViewandRepoDetailView. Since this PR introduces them as key dashboard pieces, a brief mention keeps the tree accurate.Proposed update
- dashboard/ # Dashboard-specific: Sidebar, TopNav, DashboardHome + dashboard/ # Sidebar, TopNav, DashboardHome, RepoListView, RepoDetailView🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLAUDE.md` around lines 84 - 88, Update the components tree in CLAUDE.md so the dashboard section accurately lists the newly extracted components: add "RepoListView" and "RepoDetailView" alongside the existing "Sidebar, TopNav, DashboardHome" under the dashboard/ entry (e.g., "Dashboard-specific: Sidebar, TopNav, DashboardHome, RepoListView, RepoDetailView") so the documentation reflects the PR changes.frontend/src/components/dashboard/DashboardHome.tsx (1)
107-139:setLoading(true)is redundantly called on every loop iteration.It's set inside the
tryon each iteration (line 115) but only cleared once after the loop (line 137). Moving it before theforloop is slightly cleaner.Proposed tweak
if (githubRepos.length === 0) return let lastSuccessId: string | null = null let lastSuccessName = '' + setLoading(true) for (const repo of githubRepos) { try { - setLoading(true) const repoId = await addAndIndex(repo.name, repo.clone_url, repo.default_branch)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/dashboard/DashboardHome.tsx` around lines 107 - 139, In handleGitHubImport, remove the redundant per-iteration setLoading(true) inside the try and call setLoading(true) once before the for loop starts so loading is set once for the whole import operation; keep the existing setLoading(false) after the loop and preserve the error handling/toast logic and the post-loop behavior that uses lastSuccessId/lastSuccessName to call setIndexingRepoId, setIndexingRepoName, setShowIndexingModal and refreshRepos.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/dashboard/DashboardHome.tsx`:
- Around line 86-105: In handleAddRepo, the repo name extraction uses
.replace('.git','') which removes the first occurrence and can mangle names;
change the replacement to use an end-anchored regex (e.g. replace(/\.git$/, ''))
on the value produced by gitUrl.split('/').pop() so only a trailing ".git" is
stripped while preserving names like "dotgit-example.git"; keep the existing
fallback to 'unknown' and the rest of the flow (setIndexingRepoId,
setIndexingRepoName, setShowIndexingModal, setShowAddForm, refreshRepos)
unchanged.
---
Nitpick comments:
In `@CLAUDE.md`:
- Around line 84-88: Update the components tree in CLAUDE.md so the dashboard
section accurately lists the newly extracted components: add "RepoListView" and
"RepoDetailView" alongside the existing "Sidebar, TopNav, DashboardHome" under
the dashboard/ entry (e.g., "Dashboard-specific: Sidebar, TopNav, DashboardHome,
RepoListView, RepoDetailView") so the documentation reflects the PR changes.
In `@frontend/src/components/dashboard/DashboardHome.tsx`:
- Around line 107-139: In handleGitHubImport, remove the redundant per-iteration
setLoading(true) inside the try and call setLoading(true) once before the for
loop starts so loading is set once for the whole import operation; keep the
existing setLoading(false) after the loop and preserve the error handling/toast
logic and the post-loop behavior that uses lastSuccessId/lastSuccessName to call
setIndexingRepoId, setIndexingRepoName, setShowIndexingModal and refreshRepos.
Same mid-string strip bug we fixed in RepoDetailView. This instance
was in the name = gitUrl.split('/').pop()?.replace('.git', '') line.
Was called per-iteration inside try block. Only needs to be set once before the loop starts.
Collaborator
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
frontend/src/components/dashboard/DashboardHome.tsx (5)
43-48: Prefer an immutableURLSearchParamsupdate
searchParams.delete(...)mutates the object returned byuseSearchParams()in-place before passing it tosetSearchParams. Creating a newURLSearchParamsis the idiomatic, side-effect-free approach.♻️ Proposed refactor
- searchParams.delete('openGitHubImport') - setSearchParams(searchParams, { replace: true }) + const next = new URLSearchParams(searchParams) + next.delete('openGitHubImport') + setSearchParams(next, { replace: true })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/dashboard/DashboardHome.tsx` around lines 43 - 48, The current useEffect mutates the URLSearchParams returned by useSearchParams() via searchParams.delete(...); instead create an immutable copy (e.g., new URLSearchParams(searchParams)), call delete on that copy, and pass the new instance to setSearchParams so you avoid in-place mutation; update the block around useEffect/useSearchParams, preserving the setShowGitHubSelector behavior but replacing the direct searchParams.delete call with deletion on the new URLSearchParams copy before calling setSearchParams.
141-158:handleReindexdoesn't read the error body or check for upgrade errorsUnlike
addAndIndex,handleReindexdiscards the response body on failure and never callsisUpgradeError/showUpgradeModal. A repo exceeding the free tier limit would silently show a generic toast instead of theUpgradeLimitModal.♻️ Suggested improvement
- if (!response.ok) throw new Error('Failed to start re-indexing') + if (!response.ok) { + const err = await response.json().catch(() => ({})) + if (isUpgradeError(err)) { showUpgradeModal(err, selectedRepoData.name); return } + throw new Error(extractErrorMessage(err, 'Failed to start re-indexing')) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/dashboard/DashboardHome.tsx` around lines 141 - 158, handleReindex currently throws a generic error on non-OK fetch and never inspects the response body or checks for upgrade limits; update handleReindex to read and parse the response body when response.ok is false (e.g., await response.json() or text), pass that body into isUpgradeError to detect billing/upgrade issues, and if isUpgradeError returns true call showUpgradeModal with the parsed error instead of the generic toast; otherwise keep the existing toast/log path. Ensure you still setLoading(true/false) around the request and only setIndexingRepoId, setIndexingRepoName, and setShowIndexingModal when the response is successful, mirroring the behavior used in addAndIndex.
165-188:AnimatePresencedirect children are missingkeypropsDirect children must each have a unique
keyprop soAnimatePresencecan track their presence in the tree. Without them, framer-motion cannot reliably sequence the exit animation before the enter animation inmode="wait".♻️ Proposed fix
{!isRepoView && ( <RepoListView + key="list" repos={repos} ... /> )} {isRepoView && ( <RepoDetailView + key="detail" repo={selectedRepoData} ... /> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/dashboard/DashboardHome.tsx` around lines 165 - 188, AnimatePresence's direct children (the RepoListView and RepoDetailView elements) need stable unique key props so framer-motion can sequence exit/enter when mode="wait"; add a key to the RepoListView (e.g., key="repo-list" or key={`list`}) and to the RepoDetailView (e.g., key={`repo-detail-${selectedRepo}`}) so the key changes when you switch views, updating the rendered nodes and allowing exit animation before enter. Locate the two components inside AnimatePresence in DashboardHome (RepoListView and RepoDetailView) and add appropriate key attributes that include the selectedRepo id for the detail view.
43-48: Consider immutable URLSearchParams updateMutating the
searchParamsobject directly (searchParams.delete(...)) before passing it tosetSearchParamsis non-idiomatic with react-router-dom and can cause subtle issues if the reference is shared.♻️ Suggested immutable pattern
- searchParams.delete('openGitHubImport') - setSearchParams(searchParams, { replace: true }) + const next = new URLSearchParams(searchParams) + next.delete('openGitHubImport') + setSearchParams(next, { replace: true })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/dashboard/DashboardHome.tsx` around lines 43 - 48, The useEffect currently mutates the shared searchParams by calling searchParams.delete(...); instead, create an immutable copy inside the useEffect (e.g., new URLSearchParams(searchParams)), call delete on that copy and pass the new instance to setSearchParams(..., { replace: true }); update the logic around useEffect / setShowGitHubSelector to use the copied params so searchParams is never modified in-place.
141-158:handleReindexdoesn't handle upgrade errors or extract the response bodyUnlike
addAndIndex,handleReindexignores the response body (indexResponse.json()is never called) and won't surface an upgrade-limit error viaUpgradeLimitModal. A repo that's over the size limit will show a generic "Re-indexing failed" toast instead.♻️ Suggested improvement
if (!response.ok) throw new Error('Failed to start re-indexing') + if (!response.ok) { + const err = await response.json().catch(() => ({})) + if (isUpgradeError(err)) { showUpgradeModal(err, selectedRepoData.name); return } + throw new Error(extractErrorMessage(err, 'Failed to start re-indexing')) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/dashboard/DashboardHome.tsx` around lines 141 - 158, handleReindex currently only checks response.ok and throws a generic error, so it never reads the response body or surfaces upgrade-limit errors; modify handleReindex to call await response.json() when response.ok is false (or always parse the body) and inspect the returned payload for the upgrade/limit error shape (e.g., error.code === 'UPGRADE_LIMIT' or a specific message field), and if detected trigger the UpgradeLimitModal (use the same modal state setter your app uses for that modal) instead of showing the generic toast; otherwise proceed to setIndexingRepoId, setIndexingRepoName and setShowIndexingModal as before, and include the actual error details in the toast/console for non-upgrade failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLAUDE.md`:
- Line 50: Update the "Backend tests" line so it clarifies the working directory
or uses an explicit path: change the text "pytest tests/ -v" to either "cd
backend && pytest tests/ -v" (to indicate running from the backend directory) or
replace the command with "pytest backend/tests/ -v" so it works from the repo
root; ensure the line describing backend tests clearly states which option is
expected.
In `@frontend/src/components/dashboard/DashboardHome.tsx`:
- Around line 107-139: The handleGitHubImport function can leave loading stuck
true if an exception escapes the for-loop; refactor handleGitHubImport to wrap
the main work (setting loading, looping over githubRepos and calling
addAndIndex, setting lastSuccess values and showing the indexing modal) in a try
block and move setLoading(false) and refreshRepos() into a finally block so they
always run; keep existing toast/error handling in the loop and retain setting
setIndexingRepoId/setIndexingRepoName/setShowIndexingModal based on
lastSuccessId inside the try before finally.
- Around line 107-139: handleGitHubImport sets loading true but calls
setLoading(false) and refreshRepos() only on the happy path; move those cleanup
calls into a finally so they always run. Specifically, after setLoading(true),
wrap the import loop and the subsequent "show indexing modal for the last
successfully added repo" logic in a try block and place setLoading(false) and
refreshRepos() in the corresponding finally block; keep
lastSuccessId/lastSuccessName declared before the try so the modal logic can
still run inside the try using those variables. Reference: handleGitHubImport,
setLoading, refreshRepos, setIndexingRepoId, setIndexingRepoName,
setShowIndexingModal.
- Around line 91-96: The AddRepoForm isn't being closed when addAndIndex returns
null because setShowAddForm(false) is nested inside the if (repoId) block; move
or call setShowAddForm(false) so it runs regardless of repoId (e.g., call
setShowAddForm(false) before or after the if, or add an else branch) and ensure
the upgrade path that triggers setShowUpgradeModal still fires when repoId is
falsy; update the code around addAndIndex, setShowIndexingRepoId,
setShowIndexingRepoName, setShowIndexingModal and setShowUpgradeModal
accordingly.
- Around line 91-96: When addAndIndex can return null (e.g., upgrade limit) the
current block only calls setShowAddForm(false) inside if (repoId), leaving
AddRepoForm open; move or duplicate the call so setShowAddForm(false) is
executed unconditionally after the addAndIndex handling. Specifically, in
DashboardHome (the block using repoId, setIndexingRepoId, setIndexingRepoName,
setShowIndexingModal and setShowAddForm), ensure setShowAddForm(false) runs even
when repoId is falsy (addAndIndex returned null) so the AddRepoForm modal always
closes while showUpgradeModal or other fallback modals are shown.
- Around line 165-188: AnimatePresence's direct children (the conditional
RepoListView and RepoDetailView) need unique key props so framer-motion can
track enter/exit when mode="wait"; update the DashboardHome component to add
explicit key attributes to those direct children (e.g., key="repo-list" for
RepoListView and key={`repo-detail-${selectedRepo || 'none'}`} for
RepoDetailView) so keys change when isRepoView or selectedRepo changes, keeping
all other props and handlers (onSelectRepo, onBack, onReindex, etc.) unchanged.
---
Nitpick comments:
In `@frontend/src/components/dashboard/DashboardHome.tsx`:
- Around line 43-48: The current useEffect mutates the URLSearchParams returned
by useSearchParams() via searchParams.delete(...); instead create an immutable
copy (e.g., new URLSearchParams(searchParams)), call delete on that copy, and
pass the new instance to setSearchParams so you avoid in-place mutation; update
the block around useEffect/useSearchParams, preserving the setShowGitHubSelector
behavior but replacing the direct searchParams.delete call with deletion on the
new URLSearchParams copy before calling setSearchParams.
- Around line 141-158: handleReindex currently throws a generic error on non-OK
fetch and never inspects the response body or checks for upgrade limits; update
handleReindex to read and parse the response body when response.ok is false
(e.g., await response.json() or text), pass that body into isUpgradeError to
detect billing/upgrade issues, and if isUpgradeError returns true call
showUpgradeModal with the parsed error instead of the generic toast; otherwise
keep the existing toast/log path. Ensure you still setLoading(true/false) around
the request and only setIndexingRepoId, setIndexingRepoName, and
setShowIndexingModal when the response is successful, mirroring the behavior
used in addAndIndex.
- Around line 165-188: AnimatePresence's direct children (the RepoListView and
RepoDetailView elements) need stable unique key props so framer-motion can
sequence exit/enter when mode="wait"; add a key to the RepoListView (e.g.,
key="repo-list" or key={`list`}) and to the RepoDetailView (e.g.,
key={`repo-detail-${selectedRepo}`}) so the key changes when you switch views,
updating the rendered nodes and allowing exit animation before enter. Locate the
two components inside AnimatePresence in DashboardHome (RepoListView and
RepoDetailView) and add appropriate key attributes that include the selectedRepo
id for the detail view.
- Around line 43-48: The useEffect currently mutates the shared searchParams by
calling searchParams.delete(...); instead, create an immutable copy inside the
useEffect (e.g., new URLSearchParams(searchParams)), call delete on that copy
and pass the new instance to setSearchParams(..., { replace: true }); update the
logic around useEffect / setShowGitHubSelector to use the copied params so
searchParams is never modified in-place.
- Around line 141-158: handleReindex currently only checks response.ok and
throws a generic error, so it never reads the response body or surfaces
upgrade-limit errors; modify handleReindex to call await response.json() when
response.ok is false (or always parse the body) and inspect the returned payload
for the upgrade/limit error shape (e.g., error.code === 'UPGRADE_LIMIT' or a
specific message field), and if detected trigger the UpgradeLimitModal (use the
same modal state setter your app uses for that modal) instead of showing the
generic toast; otherwise proceed to setIndexingRepoId, setIndexingRepoName and
setShowIndexingModal as before, and include the actual error details in the
toast/console for non-upgrade failures.
setShowAddForm(false) was inside the if(repoId) block, so when upgrade modal triggered (repoId null), the form stayed open behind it.
setLoading(false) and refreshRepos() were on the happy path only. If something unexpected threw outside the inner try/catch, loading would stay true forever. Now guaranteed to clean up.
framer-motion needs unique keys on direct children of AnimatePresence to track enter/exit. Without keys, exit animations don't fire when switching between list and detail views.
Created immutable copy before deleting openGitHubImport param. Mutating the object from useSearchParams() directly is incorrect.
Collaborator
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
This was referenced Mar 1, 2026
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.
What
Split the 489-line DashboardHome.tsx into focused components per CLAUDE.md's <200 line rule. Also upgraded CLAUDE.md with comprehensive coding standards.
DashboardHome split
Before: 1 file, 489 lines, 5 concerns
After: 4 files, largest 222 lines
Bonus: deduplicated addRepo/githubImport into shared
addAndIndexhelper.Zero visual/behavioral changes. Build passes. 13 tests green.
CLAUDE.md upgrade (68 -> 136 lines)
New sections: React Patterns, Testing, Code Review, Architecture tree, Data Flow, Environment Variables. Updated large file exceptions. Every rule earned from real bugs.
Linear
Closes OPE-16
Summary by CodeRabbit
Refactor
New Features
Documentation