fix(web): Fix loading issues with references / definitions list - #617
fix(web): Fix loading issues with references / definitions list#617brendan-kellam merged 5 commits into
Conversation
Sorry, something went wrong.
WalkthroughThis PR refactors the API surface by removing domain parameters from search and symbol navigation calls, consolidating API functions into client and server modules, moving type definitions to a dedicated types module, and simplifying authentication flows with a unified optional auth wrapper. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client Component
participant ClientAPI as Client API<br/>(packages/web/src/app/api/(client)/client.ts)
participant ServerRoute as Server Route<br/>(e.g., /api/files)
participant BackendAPI as Backend Service
Client->>ClientAPI: getFiles(request) / search(request)
ClientAPI->>ServerRoute: POST to /api/files or /api/search
ServerRoute->>ServerRoute: Parse & Validate Request
alt Validation Failed
ServerRoute-->>ClientAPI: Schema Error Response
else Validation Passed
ServerRoute->>BackendAPI: Call service function<br/>(getFiles, search, etc.)
alt Service Error
BackendAPI-->>ServerRoute: ServiceError
ServerRoute-->>ClientAPI: Error Response
else Service Success
BackendAPI-->>ServerRoute: Result Data
ServerRoute-->>ClientAPI: JSON Response
end
end
ClientAPI-->>Client: Response (Success/Error)
sequenceDiagram
participant UI as UI Components
participant CodeNav as CodeNav Module<br/>(packages/web/src/features/codeNav/api.ts)
participant Auth as Auth Wrapper<br/>(withOptionalAuthV2)
participant BackendSearch as Search Backend
UI->>CodeNav: findSearchBasedSymbolDefinitions(props)
CodeNav->>Auth: Execute with optional auth
Auth->>CodeNav: Proceed (auth resolved or optional)
CodeNav->>BackendSearch: Query for symbol definitions
BackendSearch-->>CodeNav: Search Results
CodeNav->>CodeNav: Parse Results<br/>(parseRelatedSymbolsSearchResponse)
CodeNav-->>UI: Response | ServiceError
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labelssourcebot-team Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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. ❤️ ShareComment @coderabbitai help to get the list of available commands and usage tips. |
Sorry, something went wrong.
This comment has been minimized.
|
✅ Actions performed
Review triggered.
|
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)packages/web/src/ee/features/codeNav/components/exploreMenu/index.tsx (2)43-50: Remove domain from the queryKey.
The query key includes domain on line 43, but the API function findSearchBasedSymbolReferences no longer accepts or uses the domain parameter. Including it in the queryKey could cause unnecessary cache invalidations and re-fetches when the domain changes, even though the API response wouldn't actually differ.
Apply this diff:
- queryKey: ["references", selectedSymbolInfo.symbolName, selectedSymbolInfo.repoName, selectedSymbolInfo.revisionName, selectedSymbolInfo.language, domain], + queryKey: ["references", selectedSymbolInfo.symbolName, selectedSymbolInfo.repoName, selectedSymbolInfo.revisionName, selectedSymbolInfo.language],
59-66: Remove domain from the queryKey.
Similar to the references query, the query key includes domain on line 59, but findSearchBasedSymbolDefinitions no longer accepts or uses the domain parameter.
Apply this diff:
- queryKey: ["definitions", selectedSymbolInfo.symbolName, selectedSymbolInfo.repoName, selectedSymbolInfo.revisionName, selectedSymbolInfo.language, domain], + queryKey: ["definitions", selectedSymbolInfo.symbolName, selectedSymbolInfo.repoName, selectedSymbolInfo.revisionName, selectedSymbolInfo.language],
packages/web/src/features/chat/components/chatThread/referencedSourcesListView.tsx (1)📜 Review detailspackages/web/src/app/api/(client)/client.ts (4)3-107: File source fetching is correctly migrated to the new client API
Importing getFileSource from "@/app/api/(client)/client" and invoking it via unwrapServiceError(getFileSource({ fileName, repository, branch })) aligns this query with the new REST-backed client API and removes the old server action dependency while still allowing parallel loading via useQueries.
One minor follow-up you might consider (not required for this PR): since unwrapServiceError already throws on ServiceError, the later isServiceError(query.data) check will never see a ServiceError value; consolidating on one error-handling pattern here would simplify the code.
packages/web/src/features/fileTree/api.ts (1)72-78: Add Content-Type header for consistency.
The new API functions should include the Content-Type: application/json header in their fetch requests for consistency with the existing search and getFileSource functions (lines 27 and 43). While browsers may infer the content type, explicit headers are a best practice.
Apply this diff:
export const findSearchBasedSymbolReferences = async (body: FindRelatedSymbolsRequest): Promise<FindRelatedSymbolsResponse | ServiceError> => { const result = await fetch("/api/find_references", { method: "POST", + headers: { + "Content-Type": "application/json", + }, body: JSON.stringify(body), }).then(response => response.json()); return result as FindRelatedSymbolsResponse | ServiceError; }
80-86: Add Content-Type header for consistency.
Same issue as the previous function.
Apply this diff:
export const findSearchBasedSymbolDefinitions = async (body: FindRelatedSymbolsRequest): Promise<FindRelatedSymbolsResponse | ServiceError> => { const result = await fetch("/api/find_definitions", { method: "POST", + headers: { + "Content-Type": "application/json", + }, body: JSON.stringify(body), }).then(response => response.json()); return result as FindRelatedSymbolsResponse | ServiceError; }
88-94: Add Content-Type header for consistency.
Same issue as the previous functions.
Apply this diff:
export const getTree = async (body: GetTreeRequest): Promise<GetTreeResponse | ServiceError> => { const result = await fetch("/api/tree", { method: "POST", + headers: { + "Content-Type": "application/json", + }, body: JSON.stringify(body), }).then(response => response.json()); return result as GetTreeResponse | ServiceError; }
96-102: Add Content-Type header for consistency.
Same issue as the previous functions.
Apply this diff:
export const getFiles = async (body: GetFilesRequest): Promise<GetFilesResponse | ServiceError> => { const result = await fetch("/api/files", { method: "POST", + headers: { + "Content-Type": "application/json", + }, body: JSON.stringify(body), }).then(response => response.json()); return result as GetFilesResponse | ServiceError; }packages/web/src/ee/features/codeNav/components/symbolHoverPopup/useHoveredOverSymbolInfo.ts (1)196-235: Type annotations are good, but path for intermediate nodes is likely incorrect
The added FileTreeNode annotations in the .find (Line 212) and .sort (Line 235) callbacks are correct and align with the exported schema type.
One thing to watch in this function: for intermediate directory nodes you’re still setting
path: item.path,(Line 217), which means every ancestor directory of a file ends up with the full leaf path rather than its own segment path (e.g. src node gets src/pages/index.tsx). If any consumer relies on a directory node’s path being its actual directory path, this will be misleading.
Consider computing the path from the current prefix instead, e.g.:
const nodePath = parts.slice(0, i + 1).join("/"); ... path: nodePath,and keeping item.path only for the leaf.
packages/web/src/features/fileTree/types.ts (1)52-77: Updated call signature is correct; minor note on domain dependency
Using
findSearchBasedSymbolDefinitions({ symbolName: symbolName!, language, revisionName, })(Line 55–59) matches the new “single props object” API and works fine with enabled: !!symbolName, so the non‑null assertion is safe in practice.
domain is still part of the queryKey and the useEffect dependency list but no longer part of the payload. That’s OK for cache key separation and re‑binding listeners, though the domain dependency in the useEffect body is now unused and could be dropped later for clarity.
packages/web/src/features/codeNav/api.ts (1)1-44: Recursive file tree schemas look sound; consider tightening node type
The zod setup for FileTreeItem, FileTreeNode, and the request/response shapes is structurally correct, and the recursive fileTreeNodeSchema using z.lazy is idiomatic.
If you know the type field is constrained (e.g. "blob" | "tree" plus maybe "commit" for submodules), you could optionally tighten
type: z.string(),to a z.enum([...]) (and mirror that in FileTreeNodeType) to catch invalid data earlier. Not required for this PR, but would strengthen the contract between the git parsing code and consumers.
39-61: findSearchBasedSymbolDefinitions matches the references helper; consider deduping the shared logic
The definitions helper mirrors the references version with the expected query change:
const query = `sym:\\b${symbolName}\\b rev:${revisionName} ${getExpandedLanguageFilter(language)}`;and the same error handling + parsing flow. This is exactly what you want for consistency between refs/defs.
Since both functions now share nearly identical control flow, you could optionally pull out a small internal helper that takes the query template (or a mode enum) to reduce duplication, but that’s not required for this PR.
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 CommitsReviewing files that changed from the base of the PR and between 341836a and 8b5c0eb.
📒 Files selected for processing (30)- CHANGELOG.md (1 hunks)
- packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx (1 hunks)
- packages/web/src/app/[domain]/browse/[...path]/components/pureTreePreviewPanel.tsx (1 hunks)
- packages/web/src/app/[domain]/browse/[...path]/components/treePreviewPanel.tsx (1 hunks)
- packages/web/src/app/[domain]/browse/components/fileSearchCommandDialog.tsx (1 hunks)
- packages/web/src/app/[domain]/components/searchBar/useSuggestionsData.ts (2 hunks)
- packages/web/src/app/[domain]/search/components/codePreviewPanel/index.tsx (1 hunks)
- packages/web/src/app/[domain]/search/components/searchResultsPage.tsx (1 hunks)
- packages/web/src/app/api/(client)/client.ts (3 hunks)
- packages/web/src/app/api/(server)/files/route.ts (1 hunks)
- packages/web/src/app/api/(server)/find_definitions/route.ts (1 hunks)
- packages/web/src/app/api/(server)/find_references/route.ts (1 hunks)
- packages/web/src/app/api/(server)/tree/route.ts (1 hunks)
- packages/web/src/ee/features/codeNav/components/exploreMenu/index.tsx (3 hunks)
- packages/web/src/ee/features/codeNav/components/symbolHoverPopup/useHoveredOverSymbolInfo.ts (2 hunks)
- packages/web/src/features/chat/agent.ts (0 hunks)
- packages/web/src/features/chat/components/chatBox/useSuggestionsData.ts (1 hunks)
- packages/web/src/features/chat/components/chatThread/referencedSourcesListView.tsx (2 hunks)
- packages/web/src/features/chat/tools.ts (3 hunks)
- packages/web/src/features/codeNav/api.ts (2 hunks)
- packages/web/src/features/codeNav/schemas.ts (0 hunks)
- packages/web/src/features/codeNav/types.ts (1 hunks)
- packages/web/src/features/fileTree/api.ts (4 hunks)
- packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx (1 hunks)
- packages/web/src/features/fileTree/components/fileTreeItemIcon.tsx (1 hunks)
- packages/web/src/features/fileTree/components/fileTreePanel.tsx (1 hunks)
- packages/web/src/features/fileTree/components/pureFileTreePanel.tsx (1 hunks)
- packages/web/src/features/fileTree/types.ts (1 hunks)
- packages/web/src/features/search/fileSourceApi.ts (1 hunks)
- packages/web/src/features/search/searchApi.ts (1 hunks)
- packages/web/src/features/chat/agent.ts
- packages/web/src/features/codeNav/schemas.ts
📄 CodeRabbit inference engine (.cursor/rules/style.mdc)
Filenames should always be camelCase. Exception: if there are filenames in the same directory with a format other than camelCase, use that format to keep things consistent.
Files:
- packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx
- CHANGELOG.md
- packages/web/src/app/api/(server)/find_definitions/route.ts
- packages/web/src/features/search/searchApi.ts
- packages/web/src/app/api/(server)/find_references/route.ts
- packages/web/src/features/fileTree/components/fileTreeItemIcon.tsx
- packages/web/src/app/api/(server)/files/route.ts
- packages/web/src/app/[domain]/components/searchBar/useSuggestionsData.ts
- packages/web/src/features/fileTree/components/fileTreePanel.tsx
- packages/web/src/features/fileTree/api.ts
- packages/web/src/features/chat/tools.ts
- packages/web/src/features/codeNav/types.ts
- packages/web/src/app/[domain]/browse/[...path]/components/treePreviewPanel.tsx
- packages/web/src/app/[domain]/browse/[...path]/components/pureTreePreviewPanel.tsx
- packages/web/src/features/codeNav/api.ts
- packages/web/src/features/fileTree/types.ts
- packages/web/src/ee/features/codeNav/components/symbolHoverPopup/useHoveredOverSymbolInfo.ts
- packages/web/src/features/chat/components/chatThread/referencedSourcesListView.tsx
- packages/web/src/app/[domain]/search/components/codePreviewPanel/index.tsx
- packages/web/src/app/api/(server)/tree/route.ts
- packages/web/src/features/fileTree/components/pureFileTreePanel.tsx
- packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx
- packages/web/src/ee/features/codeNav/components/exploreMenu/index.tsx
- packages/web/src/app/[domain]/browse/components/fileSearchCommandDialog.tsx
- packages/web/src/features/chat/components/chatBox/useSuggestionsData.ts
- packages/web/src/app/api/(client)/client.ts
- packages/web/src/features/search/fileSourceApi.ts
- packages/web/src/app/[domain]/search/components/searchResultsPage.tsx
- GitHub Check: build
packages/web/src/features/search/searchApi.ts (3)CHANGELOG.md (1)1-1: Confirm 'server-only' import is the correct Next.js 15 migration pattern.
Replacing the "use server" directive with an explicit 'server-only' import aligns with Next.js 15's shift toward structured runtime marking. Verify that this module is never imported by client components and that all consumers are aware of the server-only context.
64-74: Verify Prisma context is available at this point in the async flow.
The transformZoektQuery function receives prisma as a parameter and makes queries against prisma.searchContext. Confirm that prisma is properly injected by withOptionalAuthV2 before this function is called, especially in the Next.js 15 async context where database access patterns may have changed.
130-131: Wrapper chain correctly implements async auth context handling.
Verification confirms all three integration points are working correctly:
✓ sew unwrapping: The wrapper correctly receives () => Promise<T> from withOptionalAuthV2(async { org, prisma } => {...}), awaits it, and returns Promise<T | ServiceError>.
✓ Prisma injection: withOptionalAuthV2 properly injects org, prisma, user, and role into the callback context, enabling Prisma queries like searchContext.findUnique() used in the search logic.
✓ Consumer code updated: All identified call sites (codeNav/api.ts, chat/tools.ts, fileSourceApi.ts, useSuggestionsData.ts, searchResultsPage.tsx, search/route.ts) have been updated and correctly pass only { query, matches, contextLines, whole } without domain parameters.
packages/web/src/app/[domain]/components/searchBar/useSuggestionsData.ts (2)10-12: LGTM!
The changelog entry clearly documents the fix and follows the Keep a Changelog format.
packages/web/src/features/chat/tools.ts (3)54-58: LGTM!
The removal of the domain parameter from the search() call aligns with the PR's objective to migrate from server actions to REST endpoints. The domain is still appropriately retained in the queryKey for proper cache management.
74-78: LGTM!
Consistent removal of the domain parameter from the symbol search call, matching the file search changes above.
packages/web/src/app/api/(server)/files/route.ts (1)6-6: LGTM!
The import path update from ../codeNav/actions to ../codeNav/api reflects the API surface reorganization described in the PR.
34-38: LGTM!
The removal of the domain parameter simplifies the API call and aligns with the migration to REST endpoints that handle domain context implicitly.
71-75: LGTM!
Consistent with the symbol references changes above—the domain parameter has been appropriately removed.
packages/web/src/app/[domain]/search/components/codePreviewPanel/index.tsx (1)1-23: LGTM!
The new server route handler follows a clean pattern with:
- Proper schema validation using getFilesRequestSchema
- Appropriate error handling via serviceErrorResponse
- Consistent structure with other server routes in this PR
This aligns with the PR's goal of migrating from server actions to REST endpoints to resolve loading issues.
packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx (1)9-9: LGTM!
The import consolidation into the client API module aligns with the PR's API surface reorganization.
packages/web/src/features/fileTree/components/pureFileTreePanel.tsx (1)7-7: LGTM!
The import reordering is a benign organizational change with no functional impact.
packages/web/src/features/fileTree/components/fileTreeItemIcon.tsx (1)3-3: LGTM!
Moving the RawFileTreeNode type import to the dedicated types module improves code organization and aligns with the PR's restructuring goals.
packages/web/src/app/[domain]/browse/[...path]/components/treePreviewPanel.tsx (1)6-6: LGTM!
Consistent with the type organization improvements—moving FileTreeItem to the centralized types module.
packages/web/src/features/search/fileSourceApi.ts (1)3-23: File tree import migration looks correct
Switching getFolderContents to the centralized "@/features/fileTree/api" import while keeping the existing payload and Promise.all usage intact is consistent with the new file tree API surface and should preserve behavior.
packages/web/src/app/[domain]/browse/[...path]/components/pureTreePreviewPanel.tsx (1)1-61: Marking this module as server-only is appropriate
Using a bare import 'server-only'; to enforce that getFileSource only runs on the server aligns with the intended usage of this API and replaces the old 'use server' directive without changing behavior.
Please confirm this project is on a Next.js version that officially supports the server-only package for server-only module guards.
packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx (1)3-13: FileTreeItem type import now correctly sourced from types module
Pointing FileTreeItem at "@/features/fileTree/types" matches the new shared types module and keeps this component aligned with the rest of the file tree API.
packages/web/src/features/chat/components/chatBox/useSuggestionsData.ts (1)3-23: Consistent use of shared FileTreeItem type
Updating FileTreeItem to come from ../types keeps this component in sync with the centralized file tree type definitions without affecting runtime behavior.
packages/web/src/app/[domain]/search/components/searchResultsPage.tsx (1)32-45: Search call updated to match new payload-only API
The search invocation in the file suggestions query now correctly uses a single payload object ({ query, matches, contextLines }) without a separate domain parameter, which matches the refactored client search API. Keeping domain in the query key for cache scoping also still makes sense.
packages/web/src/app/api/(server)/tree/route.ts (1)57-70: SearchResultsPage now uses the unified search payload
Adjusting the search call in the main query function to pass only { query, matches, contextLines, whole } (no domain argument) matches the new client search contract while preserving the existing measure and unwrapServiceError behavior.
packages/web/src/app/api/(server)/find_definitions/route.ts (1)1-22: LGTM! Well-structured server route handler.
The implementation follows the correct pattern with proper validation, error handling, and response formatting. The use of Response.json() automatically sets the Content-Type header appropriately.
packages/web/src/app/[domain]/browse/components/fileSearchCommandDialog.tsx (1)1-22: LGTM! Consistent implementation pattern.
The route handler follows the same well-structured pattern as the other server routes in this PR, with proper schema validation and error handling.
packages/web/src/app/api/(server)/find_references/route.ts (2)15-16: LGTM! Proper import reorganization.
The imports have been correctly updated to use the centralized client API module and shared types, improving code organization.
packages/web/src/features/codeNav/types.ts (1)7-20: LGTM! Route handler implementation is correct.
Once the 'use server' directive is added, the route handler implementation follows the correct pattern with proper validation and error handling.
1-5: Missing 'use server' directive.
This file is missing the 'use server' directive at the top that is present in the other server route files (tree/route.ts line 1, find_definitions/route.ts line 1). Without this directive, the file may not be properly treated as server-only code by Next.js.
Apply this diff:
+'use server'; + import { findSearchBasedSymbolReferences } from "@/features/codeNav/api"; import { findRelatedSymbolsRequestSchema } from "@/features/codeNav/types"; import { schemaValidationError, serviceErrorResponse } from "@/lib/serviceError"; import { isServiceError } from "@/lib/utils"; import { NextRequest } from "next/server";Likely an incorrect or invalid review comment.
packages/web/src/features/fileTree/components/fileTreePanel.tsx (2)1-29: LGTM! Well-structured type definitions.
The schema definitions are comprehensive and properly structured, with appropriate use of Zod for validation and type inference. The request and response schemas are well-typed and will provide good type safety throughout the application.
packages/web/src/features/fileTree/api.ts (1)45-53: LGTM! Correct API integration.
The file tree data fetching has been properly updated to use the new client API with appropriate error handling via unwrapServiceError.
55-65: LGTM! Good UX enhancement.
The keyboard shortcut implementation provides a nice user experience for toggling the file tree panel.
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/useHoveredOverSymbolInfo.ts (1)1-11: Shared file tree types import looks consistent
Using FileTreeItem/FileTreeNode from ./types keeps this API in sync with the new zod-backed schemas and avoids divergent local shapes. No issues here.
packages/web/src/features/codeNav/api.ts (3)1-1: Client API import aligns with new symbol definitions endpoint
Switching findSearchBasedSymbolDefinitions to the client module (@/app/api/(client)/client) matches the refactor away from server actions and keeps this hook on the new API surface. No issues here.
1-11: Auth wrapper + type imports wired correctly for the new API surface
Using 'server-only' plus sew and withOptionalAuthV2 keeps these helpers firmly on the server and aligns with the rest of the codebase’s service-error handling. Importing FindRelatedSymbolsRequest/Response from ./types centralizes the payload/response shapes, which matches the PR’s goal of consolidating API types.
15-37: findSearchBasedSymbolReferences: props-based signature and error handling look good
The new signature
export const findSearchBasedSymbolReferences = async ( props: FindRelatedSymbolsRequest ): Promise<FindRelatedSymbolsResponse | ServiceError> => sew(() => withOptionalAuthV2(async () => { ... }) );correctly removes the domain parameter and relies on a single props object. The query string (\b${symbolName}\b rev:${revisionName} ... case:yes) mirrors previous behavior, and isServiceError(searchResult) ensures you don’t feed errors into the parser.
No functional issues here; the layering (auth → search → parse) is clean.
62-88: parseRelatedSymbolsSearchResponse correctly flattens the search result structure
The parser built from searchResponseSchema.transform(async ({ files }) => ...) and parser.parseAsync(searchResult):
- Preserves stats.actualMatchCount and repositoryInfo from the original searchResult.
- Flattens file.chunks[*].matchRanges[*] into a matches array with lineContent and range.
- Filters out files with zero matches.
This matches the expected FindRelatedSymbolsResponse shape and keeps parsing logic centralized. Looks good.
Sorry, something went wrong.
We were getting spurious loading issues with the references / definitions panel and file tree. The root cause of this was we were using server actions which cannot run in parallel and are ill-suited for client side queries. This PR migrates a couple of actions over to REST endpoints.
Summary by CodeRabbit
Release Notes