Code navigation #315
Code navigation #315
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces a comprehensive code navigation feature set, including symbol references and definitions, hover popups, and a bottom explore panel. It refactors code preview and search result panels, enhances editor theming, and restructures the filter panel logic for bidirectional filtering. New hooks, UI components, and backend APIs support these features, with significant improvements to code highlighting, navigation, and user interface consistency. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant CodeEditor
participant API
participant CodeNavActions
participant SearchAPI
User->>UI: Open file or search for symbol
UI->>CodeEditor: Render code preview panel
CodeEditor->>API: Request symbol references/definitions (if code-nav enabled)
API->>CodeNavActions: findSearchBasedSymbolReferences/Definitions
CodeNavActions->>SearchAPI: search (with symbol query)
SearchAPI-->>CodeNavActions: Search results (files, matches)
CodeNavActions-->>API: Parsed symbol references/definitions
API-->>CodeEditor: Symbol data
CodeEditor->>UI: Show hover popup, explore menu, highlight ranges
User->>UI: Click reference/definition or navigate
UI->>CodeEditor: Highlight and scroll to symbol location
Possibly related PRs
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
brendan-kellam
force-pushed
the
bkellam/code_nav
branch
from
838791f to
0ca6ba6
Compare
|
@coderabbitai 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: 14
🧹 Nitpick comments (26)
packages/web/src/app/components/keyboardShortcutHint.tsx (1)
11-17: Consider maintaining consistency with Tailwind utility classes.The change from Tailwind's
text-xsclass to inline styles reduces consistency with the rest of the codebase. Inline styles have higher specificity and can be harder to maintain.Consider using Tailwind's arbitrary value syntax or custom utility classes instead:
- <kbd - className="px-2 py-1 font-semibold font-sans border rounded-md" - style={{ - fontSize: "0.65rem", - lineHeight: "0.875rem", - }} - > + <kbd className="px-2 py-1 font-semibold font-sans border rounded-md text-[0.65rem] leading-[0.875rem]">This maintains consistency with Tailwind's approach while achieving the same visual result.
packages/web/src/app/[domain]/search/components/filterPanel/entry.tsx (1)
11-11: Consider documenting the usage of isHidden property.The
isHiddenproperty is added to theEntrytype but not used within this component. If this property is intended for parent components to conditionally render entries, consider adding a comment to clarify its purpose.packages/web/src/ee/features/codeNav/components/symbolHoverPopup/symbolHoverTargetsExtension.ts (3)
13-48: Remove duplicate node types from the array.The
NODE_TYPESarray contains duplicate entries:
"Identifier"appears on lines 22 and 32"PropertyName"appears on lines 19 and 26This could lead to unnecessary iterations during the tree traversal.
const NODE_TYPES = [ // Typescript + Python "VariableName", "VariableDefinition", "TypeDefinition", "TypeName", "PropertyName", - "PropertyDefinition", "JSXIdentifier", "Identifier", // C# "VarName", "TypeIdentifier", - "PropertyName", "MethodName", "Ident", "ParamName", "AttrsNamedArg", // C/C++ - "Identifier", "NamespaceIdentifier", "FieldIdentifier", // Objective-C "variableName", "variableName.definition", // Java "Definition", // Rust "BoundIdentifier", // Go "DefName", "FieldName", // PHP "ClassMemberName", "Name" ]
58-62: Consider removing commented debug code.The commented debug helper function should be removed for production code or moved to a development-only conditional block.
- // @note: useful for debugging - // const getTextAt = (from: number, to: number) => { - // const doc = state.doc; - // return doc.sliceString(from, to); - // }
64-71: Performance consideration for large files.The tree iteration processes the entire document synchronously, which could cause UI blocking for very large files. Consider implementing a threshold or debouncing mechanism for files above a certain size.
tree?.iterate({ enter: (node) => { - // console.log(node.type.name, getTextAt(node.from, node.to)); if (NODE_TYPES.includes(node.type.name)) { decorations.push(decoration.range(node.from, node.to)); } }, });packages/web/src/hooks/useCodeMirrorHighlighter.ts (1)
13-13: Consider grouping related tags for better maintainability.The tag mappings could be better organized by grouping related functionality. For example, all variable-related tags could be grouped together.
// Names, identifiers, properties - { tag: [t.name, t.deleted, t.character, t.propertyName, t.macroName, t.variableName], class: 'text-editor-tag-name' }, + { tag: [t.name, t.propertyName, t.macroName, t.variableName], class: 'text-editor-tag-name' }, + { tag: [t.deleted, t.character], class: 'text-editor-tag-name' },packages/web/src/app/[domain]/browse/[...path]/components/rangeHighlightingExtension.ts (1)
28-30: Consider adding bounds checking for line numbers.The loop assumes
start.lineNumberandend.lineNumberare valid. Consider adding bounds checking to prevent errors if invalid line numbers are provided.const decorations: Range<Decoration>[] = []; + const maxLine = state.doc.lines; for (let line = start.lineNumber; line <= end.lineNumber; line++) { + if (line > 0 && line <= maxLine) { decorations.push(lineDecoration.range(state.doc.line(line).from)); + } }packages/web/src/app/[domain]/browse/components/bottomPanel.tsx (1)
36-43: Consider removingenableOnFormTagsandenableOnContentEditableoptions.These options allow the keyboard shortcut to trigger while users are typing in forms or content-editable areas, which could be disruptive. Also, the description "Open Explore Panel" is slightly misleading since the shortcut toggles the panel state.
useHotkeys("shift+mod+e", (event) => { event.preventDefault(); updateBrowseState({ isBottomPanelCollapsed: !isBottomPanelCollapsed }); }, { - enableOnFormTags: true, - enableOnContentEditable: true, - description: "Open Explore Panel", + description: "Toggle Explore Panel", });packages/web/src/app/[domain]/search/components/filterPanel/useFilterMatches.ts (2)
10-10: Remove unnecessary empty line.- export const useFilteredMatches = (
24-31: Consider adding a default case to the switch statement.While TypeScript ensures all cases are covered, adding a default case improves robustness and satisfies exhaustiveness checking.
switch (filterBy) { case 'repository': return matches.filter((match) => isInRepoSet(match.repository)); case 'language': return matches.filter((match) => isInLanguageSet(match.language)); case 'all': return matches.filter((match) => isInRepoSet(match.repository) && isInLanguageSet(match.language)); + default: + return matches; }packages/web/src/app/[domain]/browse/browseStateProvider.tsx (1)
47-61: Consider adding validation for parsed state structure.While the JSON parsing has error handling, there's no validation that the parsed state conforms to the BrowseState interface structure. Malformed state could lead to runtime issues.
Consider adding state validation:
useEffect(() => { if (hydratedBrowseState) { try { const parsedState = JSON.parse(hydratedBrowseState) as Partial<BrowseState>; + // Validate parsed state structure + if (typeof parsedState === 'object' && parsedState !== null) { onUpdateState(parsedState); + } else { + console.warn("Invalid browse state structure in URL parameter"); + } } catch (error) { console.error("Error parsing hydratedBrowseState", error); }packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx (2)
50-89: Consider extracting highlight range parsing into a separate utility function.The highlight range parsing logic is complex and could benefit from being extracted into a reusable utility function for better testability and maintainability.
Consider extracting this logic:
// utils/parseHighlightRange.ts export const parseHighlightRange = (highlightRangeQuery: string): BrowseHighlightRange | undefined => { const rangeRegex = /^(\d+:\d+,\d+:\d+|\d+,\d+)$/; if (!rangeRegex.test(highlightRangeQuery)) { return; } const [start, end] = highlightRangeQuery.split(',').map((range) => { if (range.includes(':')) { return range.split(':').map((val) => parseInt(val, 10)); } const line = parseInt(range, 10); return [line]; }); if (start.length === 1 || end.length === 1) { return { start: { lineNumber: start[0] }, end: { lineNumber: end[0] } }; } else { return { start: { lineNumber: start[0], column: start[1] }, end: { lineNumber: end[0], column: end[1] } }; } };
196-196: Apply optional chaining suggestion from static analysis.The static analysis tool correctly identified that this can be simplified using optional chaining.
- {editorRef && editorRef.view && currentSelection && ( + {editorRef?.view && currentSelection && (🧰 Tools
🪛 Biome (1.9.4)
[error] 196-196: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/useHoveredOverSymbolInfo.ts (4)
33-34: Consider making timeout values configurable.The hardcoded timeout values work well but could benefit from being configurable, especially for accessibility or user preference customization.
Consider extracting to a configuration object:
const HOVER_CONFIG = { MOUSE_OVER_TIMEOUT_MS: 500, MOUSE_OUT_TIMEOUT_MS: 100, } as const;
51-51: Apply optional chaining suggestion from static analysis.The static analysis tool correctly identified that this can be simplified using optional chaining.
- return (symbolElement && symbolElement.textContent) ?? undefined; + return symbolElement?.textContent ?? undefined;🧰 Tools
🪛 Biome (1.9.4)
[error] 51-51: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
54-80: Optimize query configuration for better performance.The query configuration is well-structured, but consider adding error handling and optimizing the select function.
const { data: symbolDefinitions, isLoading: isSymbolDefinitionsLoading } = useQuery({ queryKey: ["definitions", symbolName, repoName, revisionName, language, domain], queryFn: () => unwrapServiceError( findSearchBasedSymbolDefinitions({ symbolName: symbolName!, repoName, language, revisionName, }, domain) ), - select: ((data) => { + select: useCallback((data) => { return data.files.flatMap((file) => { return file.matches.map((match) => { return { lineContent: match.lineContent, language: file.language, fileName: file.fileName, repoName: file.repository, range: match.range, } }) }) - - }), + }, []), enabled: !!symbolName, staleTime: Infinity, + retry: 1, })
98-109: Improve event handling robustness.The mouse event handling could be more robust by checking for null targets and handling edge cases.
const handleMouseOver = (event: MouseEvent) => { + if (!event.target) return; + const target = (event.target as HTMLElement).closest(`[${SYMBOL_HOVER_TARGET_DATA_ATTRIBUTE}="true"]`) as HTMLElement; if (!target) { return; } clearTimers(); setSymbolElement(target); mouseOverTimerRef.current = setTimeout(() => { setIsVisible(true); }, SYMBOL_HOVER_POPUP_MOUSE_OVER_TIMEOUT_MS); };packages/web/src/ee/features/codeNav/components/exploreMenu/index.tsx (2)
84-90: Enhance error messaging for better user experience.The error message could be more descriptive to help users understand what went wrong.
if (isError) { return ( <div className="flex flex-row items-center justify-center h-full"> - <p>Error loading {activeExploreMenuTab}</p> + <p className="text-sm text-muted-foreground">Failed to load {activeExploreMenuTab} for symbol "{selectedSymbolInfo.symbolName}"</p> </div> ) }
105-125: Remove or fix the disabled tooltip.The tooltip is disabled but still has content defined. This adds unnecessary code and confusion.
Either enable the tooltip by removing the
disabledattribute:<TooltipTrigger - disabled={true} className="mr-auto" >Or remove the tooltip entirely if it's not needed:
-<Tooltip - delayDuration={100} -> - <TooltipTrigger - disabled={true} - className="mr-auto" - > - <Badge - variant="outline" - className="w-fit h-fit flex-shrink-0 select-none" - > - Search Based - </Badge> - </TooltipTrigger> - <TooltipContent - side="top" - align="start" - > - Symbol references and definitions found using a best-guess search heuristic. - </TooltipContent> -</Tooltip> +<Badge + variant="outline" + className="w-fit h-fit flex-shrink-0 select-none mr-auto" +> + Search Based +</Badge>packages/web/src/app/[domain]/search/components/codePreviewPanel/index.tsx (1)
60-64: Enhance error message for better user feedback.The error message could be more descriptive to help users understand the issue.
if (isError) { return ( - <p>Failed to load file source</p> + <div className="flex flex-col items-center justify-center h-full text-muted-foreground"> + <p className="text-sm">Failed to load file source</p> + <p className="text-xs mt-1">Please try again or check if the file exists</p> + </div> ) }packages/web/src/ee/features/codeNav/components/symbolHoverPopup/index.tsx (2)
83-85: Address the TODO: Implement modifier key behavior for navigation.The comment indicates that the ctrl/cmd key behavior for navigation needs to be implemented. This is a common UX pattern where users hold a modifier key to enable click-to-navigate functionality.
Would you like me to implement the modifier key detection logic or open an issue to track this enhancement?
86-95: Consider adding element reference check in cleanup.The event listener management is correct, but could be made more robust by storing a reference to the element.
useEffect(() => { if (!symbolInfo || !symbolInfo.symbolDefinitions) { return; } + const element = symbolInfo.element; - symbolInfo.element.addEventListener("click", onGotoDefinition); + element.addEventListener("click", onGotoDefinition); return () => { - symbolInfo.element.removeEventListener("click", onGotoDefinition); + element.removeEventListener("click", onGotoDefinition); } }, [symbolInfo, onGotoDefinition]);packages/web/src/features/codeNav/actions.ts (2)
33-33: Consider the limitations of word boundary regex for symbol matching.The
\bword boundary may not work correctly for symbols containing special characters (e.g., operators like++,->, or symbols starting/ending with underscores). This could lead to missed matches or incorrect results depending on the programming language.Consider using language-specific symbol matching patterns or a more flexible regex approach:
-const query = `\\b${symbolName}\\b repo:^${escapeStringRegexp(repoName)}$ rev:${revisionName} lang:${language}` +// For more accurate symbol matching, consider escaping the symbol and using lookaround assertions +const escapedSymbol = escapeStringRegexp(symbolName); +const query = `(?<![\\w_])${escapedSymbol}(?![\\w_]) repo:^${escapeStringRegexp(repoName)}$ rev:${revisionName} lang:${language}`Alternatively, consider implementing language-specific patterns based on the
languageparameter.
85-109: Remove unnecessary async from the transform function.The transform function is marked as async but doesn't contain any await statements, making it unnecessarily return a Promise.
-const parser = searchResponseSchema.transform(async ({ files }) => ({ +const parser = searchResponseSchema.transform(({ files }) => ({packages/web/src/app/[domain]/components/lightweightCodeHighlighter.tsx (2)
63-69: Address the TODO: Handle multi-line highlight ranges.The current implementation only checks if a range starts or ends on a line, but doesn't handle ranges that span across multiple lines. This could lead to incorrect highlighting in the middle lines of a multi-line range.
Would you like me to implement the logic to handle multi-line ranges correctly?
157-172: Consider simplifying the nested async logic.The nested async IIFE makes the code harder to read. Consider extracting it to a separate function or simplifying the flow.
Here's a cleaner approach:
- const parser = await (async () => { - const found = LanguageDescription.matchLanguageName( - builtinLanguages, - languageName, - true - ); - - if (!found) { - return null; - } - - if (!found.support) { - await found.load(); - } - return found.support ? found.support.language.parser : null; - })(); + const found = LanguageDescription.matchLanguageName( + builtinLanguages, + languageName, + true + ); + + if (!found) { + return plainTextLanguage.parser; + } + + if (!found.support) { + await found.load(); + } + + const parser = found.support?.language.parser;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (61)
.cursor/rules/style.mdc(1 hunks)packages/web/.eslintrc.json(1 hunks)packages/web/package.json(3 hunks)packages/web/src/app/[domain]/browse/README.md(1 hunks)packages/web/src/app/[domain]/browse/[...path]/codePreview.tsx(0 hunks)packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx(1 hunks)packages/web/src/app/[domain]/browse/[...path]/components/rangeHighlightingExtension.ts(1 hunks)packages/web/src/app/[domain]/browse/[...path]/page.tsx(4 hunks)packages/web/src/app/[domain]/browse/browseStateProvider.tsx(1 hunks)packages/web/src/app/[domain]/browse/components/bottomPanel.tsx(1 hunks)packages/web/src/app/[domain]/browse/hooks/useBrowseNavigation.ts(1 hunks)packages/web/src/app/[domain]/browse/hooks/useBrowseState.ts(1 hunks)packages/web/src/app/[domain]/browse/layout.tsx(1 hunks)packages/web/src/app/[domain]/components/connectionCreationForms/secretCombobox.tsx(1 hunks)packages/web/src/app/[domain]/components/editorContextMenu.tsx(2 hunks)packages/web/src/app/[domain]/components/fileHeader.tsx(3 hunks)packages/web/src/app/[domain]/components/keyboardShortcutHint.tsx(0 hunks)packages/web/src/app/[domain]/components/lightweightCodeHighlighter.tsx(1 hunks)packages/web/src/app/[domain]/components/searchBar/searchBar.tsx(3 hunks)packages/web/src/app/[domain]/components/searchBar/searchSuggestionsBox.tsx(1 hunks)packages/web/src/app/[domain]/components/searchBar/useSuggestionsData.ts(4 hunks)packages/web/src/app/[domain]/search/components/codePreviewPanel/codePreview.tsx(7 hunks)packages/web/src/app/[domain]/search/components/codePreviewPanel/index.tsx(1 hunks)packages/web/src/app/[domain]/search/components/filterPanel/entry.tsx(3 hunks)packages/web/src/app/[domain]/search/components/filterPanel/index.tsx(5 hunks)packages/web/src/app/[domain]/search/components/filterPanel/useFilterMatches.ts(1 hunks)packages/web/src/app/[domain]/search/components/filterPanel/useGetSelectedFromQuery.ts(1 hunks)packages/web/src/app/[domain]/search/components/searchResultsPanel/codePreview.tsx(0 hunks)packages/web/src/app/[domain]/search/components/searchResultsPanel/fileMatch.tsx(3 hunks)packages/web/src/app/[domain]/search/components/searchResultsPanel/fileMatchContainer.tsx(6 hunks)packages/web/src/app/[domain]/search/components/searchResultsPanel/index.tsx(4 hunks)packages/web/src/app/[domain]/search/components/searchResultsPanel/lightweightCodeMirror.tsx(0 hunks)packages/web/src/app/[domain]/search/page.tsx(10 hunks)packages/web/src/app/api/(server)/references/route.ts(1 hunks)packages/web/src/app/components/keyboardShortcutHint.tsx(1 hunks)packages/web/src/app/globals.css(4 hunks)packages/web/src/components/ui/animatedResizableHandle.tsx(1 hunks)packages/web/src/components/ui/loading-button.tsx(1 hunks)packages/web/src/ee/features/codeNav/components/exploreMenu/index.tsx(1 hunks)packages/web/src/ee/features/codeNav/components/exploreMenu/referenceList.tsx(1 hunks)packages/web/src/ee/features/codeNav/components/symbolHoverPopup/index.tsx(1 hunks)packages/web/src/ee/features/codeNav/components/symbolHoverPopup/symbolDefinitionPreview.tsx(1 hunks)packages/web/src/ee/features/codeNav/components/symbolHoverPopup/symbolHoverTargetsExtension.ts(1 hunks)packages/web/src/ee/features/codeNav/components/symbolHoverPopup/useHoveredOverSymbolInfo.ts(1 hunks)packages/web/src/features/codeNav/actions.ts(1 hunks)packages/web/src/features/codeNav/schemas.ts(1 hunks)packages/web/src/features/codeNav/types.ts(1 hunks)packages/web/src/features/entitlements/constants.ts(2 hunks)packages/web/src/features/search/fileSourceApi.ts(2 hunks)packages/web/src/features/search/schemas.ts(2 hunks)packages/web/src/features/search/searchApi.ts(4 hunks)packages/web/src/features/search/types.ts(1 hunks)packages/web/src/hooks/useCodeMirrorHighlighter.ts(1 hunks)packages/web/src/hooks/useCodeMirrorLanguageExtension.ts(1 hunks)packages/web/src/hooks/useCodeMirrorTheme.ts(1 hunks)packages/web/src/hooks/useTailwind.ts(0 hunks)packages/web/src/lib/errorCodes.ts(1 hunks)packages/web/src/lib/extensions/searchResultHighlightExtension.ts(2 hunks)packages/web/src/lib/utils.ts(6 hunks)packages/web/src/tailwind.ts(1 hunks)packages/web/tailwind.config.ts(1 hunks)
💤 Files with no reviewable changes (5)
- packages/web/src/hooks/useTailwind.ts
- packages/web/src/app/[domain]/components/keyboardShortcutHint.tsx
- packages/web/src/app/[domain]/search/components/searchResultsPanel/codePreview.tsx
- packages/web/src/app/[domain]/browse/[...path]/codePreview.tsx
- packages/web/src/app/[domain]/search/components/searchResultsPanel/lightweightCodeMirror.tsx
🧰 Additional context used
🧬 Code Graph Analysis (20)
packages/web/src/app/[domain]/components/editorContextMenu.tsx (1)
packages/web/src/app/[domain]/browse/hooks/useBrowseNavigation.ts (1)
HIGHLIGHT_RANGE_QUERY_PARAM(14-14)
packages/web/src/features/codeNav/types.ts (1)
packages/web/src/features/codeNav/schemas.ts (1)
findRelatedSymbolsResponseSchema(4-20)
packages/web/src/components/ui/loading-button.tsx (1)
packages/web/src/components/ui/button.tsx (2)
ButtonProps(36-40)Button(56-56)
packages/web/src/features/search/types.ts (1)
packages/web/src/features/search/schemas.ts (2)
repositoryInfoSchema(34-40)rangeSchema(13-16)
packages/web/src/app/[domain]/components/searchBar/searchBar.tsx (2)
packages/web/src/components/ui/tooltip.tsx (3)
Tooltip(30-30)TooltipTrigger(30-30)TooltipContent(30-30)packages/web/src/app/components/keyboardShortcutHint.tsx (1)
KeyboardShortcutHint(8-22)
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/symbolHoverTargetsExtension.ts (1)
packages/web/src/lib/utils.ts (1)
measureSync(241-259)
packages/web/src/app/[domain]/components/fileHeader.tsx (1)
packages/web/src/app/[domain]/browse/hooks/useBrowseNavigation.ts (1)
useBrowseNavigation(25-59)
packages/web/src/ee/features/codeNav/components/exploreMenu/referenceList.tsx (7)
packages/web/src/features/codeNav/types.ts (1)
FindRelatedSymbolsResponse(4-4)packages/web/src/features/search/types.ts (2)
RepositoryInfo(28-28)SourceRange(29-29)packages/web/src/app/[domain]/browse/hooks/useBrowseNavigation.ts (1)
useBrowseNavigation(25-59)packages/web/src/components/ui/scroll-area.tsx (1)
ScrollArea(48-48)packages/web/src/app/[domain]/components/fileHeader.tsx (1)
FileHeader(26-103)packages/web/src/lib/utils.ts (1)
base64Decode(202-205)packages/web/src/app/[domain]/components/lightweightCodeHighlighter.tsx (1)
LightweightCodeHighlighter(38-149)
packages/web/src/app/[domain]/browse/[...path]/components/rangeHighlightingExtension.ts (1)
packages/web/src/app/[domain]/browse/hooks/useBrowseNavigation.ts (1)
BrowseHighlightRange(6-12)
packages/web/src/lib/extensions/searchResultHighlightExtension.ts (1)
packages/web/src/features/search/types.ts (1)
SourceRange(29-29)
packages/web/src/ee/features/codeNav/components/exploreMenu/index.tsx (7)
packages/web/src/app/[domain]/browse/hooks/useBrowseState.ts (1)
useBrowseState(6-12)packages/web/src/lib/utils.ts (1)
unwrapServiceError(289-296)packages/web/src/features/codeNav/actions.ts (2)
findSearchBasedSymbolReferences(15-47)findSearchBasedSymbolDefinitions(50-82)packages/web/src/components/ui/resizable.tsx (2)
ResizablePanelGroup(48-48)ResizablePanel(48-48)packages/web/src/components/ui/tooltip.tsx (3)
Tooltip(30-30)TooltipTrigger(30-30)TooltipContent(30-30)packages/web/src/components/ui/animatedResizableHandle.tsx (1)
AnimatedResizableHandle(5-11)packages/web/src/ee/features/codeNav/components/exploreMenu/referenceList.tsx (1)
ReferenceList(17-75)
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/index.tsx (5)
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/useHoveredOverSymbolInfo.ts (2)
SymbolDefinition(18-24)useHoveredOverSymbolInfo(36-142)packages/web/src/ee/features/codeNav/components/symbolHoverPopup/symbolDefinitionPreview.tsx (1)
SymbolDefinitionPreview(18-62)packages/web/src/components/ui/separator.tsx (1)
Separator(31-31)packages/web/src/components/ui/loading-button.tsx (1)
LoadingButton(30-30)packages/web/src/components/ui/button.tsx (1)
Button(56-56)
packages/web/src/app/[domain]/search/components/searchResultsPanel/fileMatchContainer.tsx (2)
packages/web/src/features/search/types.ts (1)
SearchResultFile(18-18)packages/web/src/components/ui/button.tsx (1)
Button(56-56)
packages/web/src/app/[domain]/browse/browseStateProvider.tsx (2)
packages/web/src/app/[domain]/browse/components/bottomPanel.tsx (1)
BOTTOM_PANEL_MIN_SIZE(16-16)packages/web/src/hooks/useNonEmptyQueryParam.ts (1)
useNonEmptyQueryParam(18-25)
packages/web/src/app/[domain]/browse/hooks/useBrowseNavigation.ts (1)
packages/web/src/app/[domain]/browse/browseStateProvider.tsx (2)
BrowseState(7-17)SET_BROWSE_STATE_QUERY_PARAM(26-26)
packages/web/src/features/search/searchApi.ts (1)
packages/web/src/features/search/types.ts (1)
SourceRange(29-29)
packages/web/src/app/[domain]/search/components/filterPanel/useFilterMatches.ts (1)
packages/web/src/app/[domain]/search/components/filterPanel/useGetSelectedFromQuery.ts (1)
useGetSelectedFromQuery(7-17)
packages/web/src/app/[domain]/components/lightweightCodeHighlighter.tsx (3)
packages/web/src/features/search/types.ts (1)
SourceRange(29-29)packages/web/src/hooks/useCodeMirrorHighlighter.ts (1)
useCodeMirrorHighlighter(6-86)packages/web/src/lib/utils.ts (1)
measure(261-279)
packages/web/src/app/[domain]/search/components/searchResultsPanel/index.tsx (1)
packages/web/src/features/search/types.ts (1)
SearchResultFile(18-18)
packages/web/src/app/[domain]/search/page.tsx (10)
packages/web/src/hooks/useNonEmptyQueryParam.ts (1)
useNonEmptyQueryParam(18-25)packages/web/src/lib/utils.ts (2)
unwrapServiceError(289-296)createPathWithQueryParams(25-36)packages/web/src/app/api/(client)/client.ts (1)
search(20-35)packages/web/src/features/search/types.ts (1)
SearchResultFile(18-18)packages/web/src/app/[domain]/search/components/filterPanel/useFilterMatches.ts (1)
useFilteredMatches(11-36)packages/web/src/components/ui/tooltip.tsx (3)
Tooltip(30-30)TooltipTrigger(30-30)TooltipContent(30-30)packages/web/src/components/ui/button.tsx (1)
Button(56-56)packages/web/src/app/components/keyboardShortcutHint.tsx (1)
KeyboardShortcutHint(8-22)packages/web/src/components/ui/animatedResizableHandle.tsx (1)
AnimatedResizableHandle(5-11)packages/web/src/app/[domain]/search/components/codePreviewPanel/index.tsx (1)
CodePreviewPanel(18-73)
🪛 Biome (1.9.4)
packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx
[error] 196-196: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/useHoveredOverSymbolInfo.ts
[error] 51-51: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (87)
.cursor/rules/style.mdc (1)
6-7: Style rules are clear.
The bullets correctly mandate 4-space indentation and camelCase filenames (with an exception for existing formats), which aligns with the project’s consistency goals.packages/web/src/app/[domain]/components/searchBar/searchSuggestionsBox.tsx (1)
18-18: Use of absolute import aligns with project conventions
The import path forKeyboardShortcutHintnow uses the alias@/app/..., improving consistency and readability across the codebase.packages/web/.eslintrc.json (1)
10-11: Ensure ESLint plugin is correctly applied
Extendingplugin:@tanstack/query/recommendedaligns linting with new React Query usage. Confirm that@tanstack/eslint-plugin-queryis installed and that plugin rules are activated.packages/web/src/app/[domain]/components/editorContextMenu.tsx (2)
12-12: Appropriate use of named constant for query parameter
ImportingHIGHLIGHT_RANGE_QUERY_PARAMcentralizes the query key and prevents hardcoded strings. This improves maintainability and consistency across navigation logic.
110-113: Use constant instead of hardcoded string in URL builder
Replacing the literal'highlightRange'withHIGHLIGHT_RANGE_QUERY_PARAMincreatePathWithQueryParamsreduces risk of typos and keeps URL param logic in sync withuseBrowseNavigation.packages/web/package.json (2)
149-149: Add ESLint plugin for React Query
Adding@tanstack/eslint-plugin-queryin devDependencies supports the newly extended query linting rules.
37-37:✅ Verification successful
Verify usage of
@codemirror/language-data
A new dependency has been added—ensure it’s actually imported and utilized (for example, in language data extensions) to avoid unnecessary bundle size increase.
🏁 Script executed:
#!/bin/bash # Verify that @codemirror/language-data is imported in the codebase rg "@codemirror/language-data"Length of output: 413
🏁 Script executed:
#!/bin/bash # Locate lightweightCodeHighlighter.tsx and verify usage of the imported `builtinLanguages` fd lightweightCodeHighlighter.tsx -t f -x rg -n "builtinLanguages" -A2 {}Length of output: 390
Dependency usage confirmed for
@codemirror/language-data
Thelanguagesexport is imported asbuiltinLanguagesin
packages/web/src/app/[domain]/components/lightweightCodeHighlighter.tsx
and is actively passed (lines 159–161) into theuseCodeMirrorHighlighterhook. No unused dependency.packages/web/src/hooks/useCodeMirrorLanguageExtension.ts (1)
7-7: LGTM! Function rename improves semantic clarity.The rename from
useSyntaxHighlightingExtensiontouseCodeMirrorLanguageExtensionbetter reflects the function's broader purpose of providing language extension support, not just syntax highlighting.packages/web/src/features/search/types.ts (1)
28-29: LGTM! Type additions follow established patterns.The new
RepositoryInfoandSourceRangetypes are properly inferred from their corresponding zod schemas. TheSourceRangetype provides better naming consistency across the codebase compared to the previousSearchResultRange.packages/web/src/features/codeNav/types.ts (1)
1-4: LGTM! Clean type definition following established patterns.The file follows the standard pattern of inferring types from zod schemas. The
FindRelatedSymbolsResponsetype name is descriptive and aligns with the code navigation feature's purpose.packages/web/src/app/[domain]/browse/README.md (1)
1-12: Excellent documentation for file browser URL structure.The README provides clear documentation of the URL path template with a practical example. This will help developers understand the routing structure for the file browser feature.
packages/web/src/app/[domain]/components/connectionCreationForms/secretCombobox.tsx (1)
43-43: LGTM! Improved caching strategy with domain scoping.Adding the
domainto the query key ensures secrets are cached separately per domain, preventing cross-domain data issues and enabling proper cache invalidation per domain context.packages/web/src/app/[domain]/browse/hooks/useBrowseState.ts (1)
1-12: LGTM! Clean React hook implementation.This hook follows React best practices perfectly:
- Proper context usage with error boundary checking
- Clear error message for developer guidance
- Clean, minimal implementation that serves its purpose well
The pattern ensures components can safely access the browse state context while providing helpful debugging information if used incorrectly.
packages/web/src/features/search/fileSourceApi.ts (2)
1-1: Good addition of server directive.The
'use server'directive correctly indicates this code runs in a server-side context, which is appropriate for this API function.
46-46: LGTM! webUrl addition enhances navigation capabilities.Adding the
webUrlproperty to the response enables direct linking to external code hosts, which aligns perfectly with the code navigation features being introduced. The property is sourced from the existing file metadata, maintaining consistency with the search API.packages/web/src/features/search/schemas.ts (2)
66-68: Good addition of stats object with match count.Adding the
statsobject withmatchCountprovides valuable aggregate information about search results. This enhances the search functionality by giving users insights into the total number of matches found.
117-117: LGTM! webUrl property supports external navigation.Adding the optional
webUrlproperty to the file source response schema enables direct linking to external code hosts, which is essential for the code navigation features. The optional nature maintains backward compatibility.packages/web/src/tailwind.ts (1)
1-5: Excellent refactoring to centralize Tailwind config resolution.This approach is much better than the previous hook-based solution:
- Eliminates redundant config resolution across components
- Provides a single source of truth for the resolved Tailwind configuration
- Improves performance by resolving once rather than memoizing in each component
- Simplifies imports for components that need Tailwind config access
Clean and effective architectural improvement!
packages/web/src/app/api/(server)/references/route.ts (1)
1-37: Well-structured API route with proper error handling.The implementation correctly validates required headers and query parameters, uses appropriate error codes, and follows consistent error handling patterns.
Minor suggestions for enhanced robustness:
- Consider adding input sanitization for the query parameters
- Add rate limiting if this endpoint will be heavily used
- Consider adding request logging for debugging purposes
The current implementation is solid and follows good API design practices.
packages/web/src/app/[domain]/search/components/filterPanel/useGetSelectedFromQuery.ts (1)
6-17: Clean and well-structured custom hook implementation.The hook properly uses
useCallbackfor memoization and handles missing query parameters gracefully by returning an empty Set.Consider adding JSDoc documentation to improve developer experience:
+/** + * Custom hook to parse comma-separated query parameter values into Sets + * @returns Object containing getSelectedFromQuery function + */ export const useGetSelectedFromQuery = () => { const searchParams = useSearchParams(); + /** + * Parses a query parameter into a Set of strings + * @param param - The query parameter name to parse + * @returns Set of comma-separated values or empty Set if parameter doesn't exist + */ const getSelectedFromQuery = useCallback((param: string): Set<string> => {packages/web/src/components/ui/animatedResizableHandle.tsx (1)
5-11: Excellent implementation of animated resize handle.The component effectively uses data attribute selectors for state-based styling and provides smooth transitions. The use of
data-[resize-handle-state=drag]anddata-[resize-handle-state=hover]is a clean approach for handling different interaction states.The implementation follows good practices:
- Proper separation of concerns
- Consistent theming with accent colors
- Smooth transition effects with appropriate delay
- Clean and maintainable code structure
packages/web/src/app/[domain]/search/components/filterPanel/entry.tsx (1)
27-27: Well implemented disabled state functionality.The
isDisabledproperty is properly destructured and correctly applied to the component styling with theopacity-50class. This provides clear visual feedback for disabled entries.Also applies to: 42-42
packages/web/src/components/ui/loading-button.tsx (2)
9-26: Excellent implementation of LoadingButton component.This component follows React best practices:
- Properly extends
ButtonPropswith additionalloadingprop- Correctly forwards refs using
React.forwardRef- Appropriately merges the
disabledstate (loading || props.disabled)- Clean conditional rendering of the loading spinner
- Proper TypeScript typing throughout
The implementation is clean, reusable, and maintains accessibility by disabling the button during loading state.
28-28: Good practice setting displayName for debugging.Setting the
displayNamehelps with React DevTools debugging and is a good practice for forwardRef components.packages/web/src/app/[domain]/components/searchBar/searchBar.tsx (4)
46-46: Improved import strategy for Tailwind configuration.The change from using the
useTailwindhook to directly importing the resolved Tailwind configuration is cleaner and more efficient. This aligns with the broader refactoring mentioned in the AI summary.
163-163: Correctly updated useMemo dependency array.Good catch removing the
tailwinddependency from theuseMemodependency array sincetailwindis now a static import rather than a hook value that could change.
269-280: Enhanced user experience with tooltip for keyboard shortcut.The addition of a
Tooltipwrapper around theKeyboardShortcutHintwith a 100ms delay provides helpful context for users about the "/" keyboard shortcut functionality. The tooltip positioning and content are appropriate.
45-45: Import path change from relative to absolute.The import path for
KeyboardShortcutHintchanged from a relative import to an absolute import. This is consistent with the broader component reorganization mentioned in the AI summary.packages/web/src/app/[domain]/components/searchBar/useSuggestionsData.ts (1)
40-40: LGTM! Good practice for multi-tenant caching.The addition of
domainto all query keys ensures proper data isolation and cache scoping per domain context. This is excellent for preventing cross-domain data leakage and maintaining correct caching behavior in a multi-tenant application.Also applies to: 53-53, 73-73, 103-103
packages/web/src/lib/extensions/searchResultHighlightExtension.ts (1)
3-3: LGTM! Consistent with broader type and styling refactoring.The changes correctly update:
- Type imports from
SearchResultRangetoSourceRange- aligns with the type definition inpackages/web/src/features/search/types.ts- CSS class names from
cm-searchMatchtosearchMatch- removes CodeMirror-specific prefixes for more generic stylingThese updates maintain consistency with the broader codebase refactoring while preserving the extension's functionality.
Also applies to: 7-7, 10-10, 49-49, 52-52, 55-55
packages/web/src/features/entitlements/constants.ts (1)
20-20: LGTM! Proper entitlement setup for code navigation feature.The addition of the
"code-nav"entitlement is correctly implemented:
- Added to the main entitlements array for type safety
- Appropriately granted to enterprise self-hosted plans only
- Follows existing patterns and maintains consistency with the entitlement system
This properly gates the new code navigation features behind enterprise plans.
Also applies to: 31-32
packages/web/src/app/[domain]/components/fileHeader.tsx (3)
1-1: LGTM! Proper setup for client-side navigation.The addition of
'use client'directive anduseBrowseNavigationhook correctly enables client-side navigation functionality for the file header component.Also applies to: 8-8, 40-40
65-69: Minor styling improvements applied.The spacing adjustments using inline styles and className-based margins improve the visual layout of the branch information display.
77-87:❌ Incorrect review comment
Verify revision name usage in navigation.
The clickable file name implementation looks good, but using
branchDisplayNameasrevisionNamemight not always be correct. Branch display names could be formatted for UI purposes and may not match the actual revision identifiers expected by the navigation system.Consider verifying that
branchDisplayNamealways contains the correct revision identifier, or if a separate revision prop should be passed to the component.
🏁 Script executed:
#!/bin/bash # Description: Check how branchDisplayName is used in other navigation contexts # Expected: Find other usages to verify if branchDisplayName is appropriate for navigation rg -A 5 -B 5 "branchDisplayName.*revision|revision.*branchDisplayName"Length of output: 3790
🏁 Script executed:
#!/bin/bash # Find the exact path of fileHeader.tsx fd fileHeader.tsx -t f # Display the top of the file to inspect prop definitions sed -n '1,200p' packages/web/src/app/*/components/fileHeader.tsxLength of output: 3565
🏁 Script executed:
#!/bin/bash # Locate the useBrowseNavigation hook fd useBrowseNavigation -t f # Inspect its implementation to see what revisionName it provides sed -n '1,200p' packages/web/src/app/[domain]/browse/hooks/useBrowseNavigation.tsLength of output: 1980
No action needed:
branchDisplayNameis the actual revision identifier
ThebranchDisplayNameprop is sourced directly from the actualrevisionName(e.g. in your browse page you passbranchDisplayName={revisionName}), andbranchDisplayTitleexists for any UI‐specific formatting or tooltip. This means usingbranchDisplayNamefor navigation is correct, and no additional prop is required.Likely an incorrect or invalid review comment.
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/symbolHoverTargetsExtension.ts (1)
50-78: Excellent use of performance measurement and proper StateField patterns.The implementation correctly uses
ensureSyntaxTreeto parse the entire document and follows CodeMirror StateField best practices. The performance measurement withmeasureSyncis a good practice for monitoring expensive operations.packages/web/src/hooks/useCodeMirrorHighlighter.ts (1)
6-86: Well-structured syntax highlighting configuration.The hook provides comprehensive coverage of syntax highlighting tags with a consistent CSS class naming convention. The use of
useMemoensures the highlighter is created only once, which is good for performance.packages/web/src/features/codeNav/schemas.ts (2)
4-20: Clean and well-structured schema definition.The schema properly reuses existing schemas through imports and provides a clear structure for the API response. The use of
z.array()and nested objects follows Zod best practices.
12-12:✅ Verification successful
Consider making webUrl required if it's always expected.
The
webUrlfield is marked as optional with.optional(). Verify if this field should always be present or if there are legitimate cases where it might be missing.
🏁 Script executed:
#!/bin/bash # Search for usage of webUrl field to understand if it's always expected rg -A 3 -B 3 "webUrl" --type tsLength of output: 30721
🏁 Script executed:
#!/bin/bash echo "----- packages/web/src/features/codeNav/schemas.ts (first 50 lines) -----" sed -n '1,50p' packages/web/src/features/codeNav/schemas.ts echo echo "----- packages/web/src/features/codeNav/actions.ts (first 50 lines) -----" sed -n '1,50p' packages/web/src/features/codeNav/actions.tsLength of output: 2693
🏁 Script executed:
#!/bin/bash echo "----- packages/web/src/features/codeNav/actions.ts (lines 50-150) -----" sed -n '50,150p' packages/web/src/features/codeNav/actions.tsLength of output: 2249
No change needed:
webUrlcan be legitimately absent
The search API returnswebUrlonly when a repository URL template is available (otherwise it’sundefined), and downstream components guard on its presence before rendering links. KeepingwebUrloptional infindRelatedSymbolsResponseSchemaaligns with those cases.packages/web/src/app/[domain]/browse/[...path]/components/rangeHighlightingExtension.ts (1)
15-39: Solid implementation of range highlighting extension.The extension correctly handles both column-specific and line-only highlighting scenarios. The column position calculations properly convert from 1-indexed to 0-indexed positions, and the StateField implementation follows CodeMirror patterns.
packages/web/src/app/[domain]/search/components/searchResultsPanel/fileMatch.tsx (1)
62-70: LGTM! Clean transition to LightweightCodeHighlighter.The replacement of
CodePreviewwithLightweightCodeHighlighteris well-implemented with appropriate props for syntax highlighting, line numbers, and match highlighting.packages/web/src/ee/features/codeNav/components/symbolHoverPopup/symbolDefinitionPreview.tsx (1)
18-62: LGTM! Well-structured component with proper optimization.The component follows React best practices with:
- Proper memoization of computed values (
highlightRangesanddecodedLineContent)- Clean integration with
LightweightCodeHighlighter- Informative tooltip explaining the "Search Based" nature of the definition
- Appropriate prop destructuring and usage
packages/web/src/ee/features/codeNav/components/exploreMenu/referenceList.tsx (4)
21-26: LGTM! Efficient repository lookup optimization.The memoized
repoInfoMapprovides O(1) repository lookups instead of O(n) searches, which will improve performance when rendering multiple files with references.
50-51: LGTM! Good UX with sorted matches.Sorting matches by line number ensures a logical reading order for users navigating through references in a file.
58-66: LGTM! Clean navigation integration.The navigation logic properly uses the
useBrowseNavigationhook with all required parameters including the highlight range for precise positioning.
91-95: LGTM! Proper memoization in nested component.Both
decodedLineContentandhighlightRangesare correctly memoized to prevent unnecessary re-computations and re-renders.packages/web/src/app/[domain]/search/components/searchResultsPanel/fileMatchContainer.tsx (3)
106-114: LGTM! Clean Preview button implementation.The Preview button provides clear user interaction for file preview functionality, with appropriate styling and click handling.
122-125: LGTM! Consistent with FileMatch refactoring.Removing the
onOpenprop aligns with the FileMatch component's new internal navigation handling using theuseBrowseNavigationhook.
136-136: LGTM! Improved accessibility with focus styles.The addition of focus ring styles enhances keyboard navigation accessibility for the "Show more matches" functionality.
Also applies to: 146-146
packages/web/src/app/[domain]/browse/components/bottomPanel.tsx (1)
19-114: Well-structured component implementation!The component demonstrates good React practices with proper use of refs, effects, and state management. The conditional rendering logic is clear and the UI states are well-handled.
packages/web/tailwind.config.ts (2)
79-108: Excellent editor theme configuration!The comprehensive syntax tag color definitions provide great flexibility for editor theming. The consistent naming convention and CSS variable usage make it easy to customize themes.
4-150: Great improvements to the Tailwind configuration!The migration from HSL function calls to direct CSS variable references simplifies the configuration and improves maintainability. The addition of editor and sidebar themes provides excellent customization capabilities.
packages/web/src/app/[domain]/search/components/filterPanel/useFilterMatches.ts (1)
26-30:✅ Verification successful
Verify that SearchResultFile type guarantees repository and language fields.
The filtering logic directly accesses
match.repositoryandmatch.languagewithout null checks. Please ensure the type definition guarantees these fields are always present.
🏁 Script executed:
#!/bin/bash # Description: Check SearchResultFile type definition to ensure repository and language fields are required # Search for SearchResultFile type definition ast-grep --pattern 'type SearchResultFile = { $$$ }' # Also check with interface syntax ast-grep --pattern 'interface SearchResultFile { $$$ }' # Use ripgrep as fallback to find the type definition rg -A 10 "type SearchResultFile|interface SearchResultFile" --type tsLength of output: 2054
🏁 Script executed:
#!/bin/bash # Inspect SearchResponse definition and its Zod schema to verify required fields rg -n "type SearchResponse" -A 30 --type ts packages/web/src/features/search/types.ts rg -n "interface SearchResponse" -A 30 --type ts packages/web/src/features/search/types.ts rg -n "searchResponseSchema" -A 30 --type ts packages/web/src/features/search/types.tsLength of output: 2104
🏁 Script executed:
#!/bin/bash # Display the definition of searchResponseSchema to verify required fields rg -n "export const searchResponseSchema" -A 200 --type ts packages/web/src/features/search/schemas.tsLength of output: 2754
Filtering logic is safe—no null checks needed
ThesearchResponseSchema(packages/web/src/features/search/schemas.ts:69–79) defines bothrepositoryandlanguageas requiredz.string()fields on each file entry, somatch.repositoryandmatch.languageare guaranteed to be present at runtime. You can leave the filter calls as-is.packages/web/src/app/[domain]/browse/browseStateProvider.tsx (1)
1-24: LGTM! Well-structured state interface and imports.The BrowseState interface is well-defined with clear types, and the default state configuration is appropriate. The import structure follows good practices.
packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx (1)
150-178:❓ Verification inconclusive
Verify navigation behavior for edge cases.
The
onGotoDefinitionfunction handles single vs multiple definitions well, but consider edge cases where symbolDefinitions might contain invalid data.
🏁 Script executed:
#!/bin/bash # Description: Check if SymbolDefinition type validation is handled in the backend API # Expected: Backend should validate symbol definition structure ast-grep --pattern 'interface SymbolDefinition { $$$ }' # Also check if there are any validation utilities for symbol definitions rg -A 5 "SymbolDefinition.*validate|validate.*SymbolDefinition"Length of output: 125
Verify SymbolDefinition edge cases in onGotoDefinition
Ensure that
onGotoDefinitioncan safely handle unexpected or malformedSymbolDefinitionentries before navigating:
- Location:
packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx
Function:onGotoDefinition- Action items:
- Confirm that the backend always returns valid
fileName,repoName, andrangefor eachSymbolDefinition.- Add runtime guards in the single-definition branch, for example:
if (symbolDefinitions.length === 1) { const symbolDefinition = symbolDefinitions[0]; - const { fileName, repoName } = symbolDefinition; + const { fileName, repoName, range } = symbolDefinition; + if (!fileName || !repoName || !range) { + return; + } navigateToPath({ repoName, revisionName, path: fileName, pathType: 'blob', highlightRange: range, }) }- Optionally, add logging or error reporting for any skipped definitions to aid debugging.
packages/web/src/app/[domain]/browse/hooks/useBrowseNavigation.ts (1)
6-12: Well-designed union type for flexible highlight ranges.The BrowseHighlightRange type elegantly handles both line-only and line-column highlight formats using discriminated unions. This provides good type safety while maintaining flexibility.
packages/web/src/ee/features/codeNav/components/exploreMenu/index.tsx (1)
172-206: Well-implemented Entry component.The component properly handles edge cases with count display and uses appropriate memoization.
packages/web/src/app/[domain]/browse/[...path]/page.tsx (3)
53-66: Good error handling for repository not found.The explicit handling of repository not found errors with a user-friendly UI is a great improvement over throwing exceptions.
77-81: Good default value for revision.Defaulting to 'HEAD' when no revision is specified ensures consistent behavior.
98-103: Well-implemented external link feature.The external link to the code host is properly implemented with:
- Security attributes (
noopener noreferrer)- Conditional rendering
- Proper icon with alt text
- Good UX with code host name display
Also applies to: 124-138
packages/web/src/app/[domain]/search/components/codePreviewPanel/index.tsx (2)
24-28: Good branch selection logic with clear documentation.The memoized branch selection with a clear explanatory comment makes the code easy to understand.
53-58: Comprehensive loading state handling.Checking both
isLoadingandisPendingensures all loading scenarios are covered.packages/web/src/hooks/useCodeMirrorTheme.ts (2)
14-55: Excellent theme generation with Tailwind integration.The dynamic theme generation using Tailwind tokens is well-implemented with:
- Proper memoization
- Clean destructuring of theme properties
- Correct dependency array
- Integration with syntax highlighting
59-133: Well-structured theme extension creator.The
createThemeExtensionfunction is excellently implemented with:
- Conditional application of styles to avoid unnecessary properties
- Proper handling of all theme settings
- Good reference to the source pattern
- Clean code structure
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/index.tsx (1)
40-73: LGTM! Well-implemented positioning logic.The floating-ui integration is properly configured with appropriate middleware for offset, flipping, and shifting. The boundary constraint using the editor's DOM element ensures the popup stays within the editor viewport.
packages/web/src/lib/utils.ts (3)
137-137: LGTM! Improved naming consistency.The change from "Generic Git Host" to "Git Host" is more concise and consistent with other code host names.
241-279: LGTM! Useful enhancement to timing utilities.The addition of the optional
outputLogparameter provides flexibility to suppress console output when needed, while maintaining backwards compatibility with the default value oftrue.
298-308: LGTM! Well-implemented parameter validation utility.The
requiredQueryParamGuardfunction provides a clean and consistent way to validate required query parameters in API routes, with proper error handling and clear error messages.packages/web/src/app/[domain]/search/components/codePreviewPanel/codePreview.tsx (3)
111-164: LGTM! Well-designed navigation logic.The navigation callbacks handle different scenarios appropriately:
- Single definition navigates directly for better UX
- Multiple definitions and references open the explore panel
- Browse state is properly configured for each scenario
260-269: LGTM! Proper feature gating with entitlements.The symbol hover popup is correctly gated behind the "code-nav" entitlement check, ensuring the feature is only available to authorized users.
35-36:✅ Verification successful
Verify all component usages have been updated.
Making
fileandrepoNamerequired props is a breaking change. Ensure all usages of this component have been updated to provide these props.
🏁 Script executed:
#!/bin/bash # Description: Find all usages of CodePreview component to verify they provide required props # Search for CodePreview component usage rg -A 5 '<CodePreview' --type tsx --type jsxLength of output: 74
I’ll search without relying on file-type filters to locate all
<CodePreviewusages:
🏁 Script executed:
#!/bin/bash # Find all usages of CodePreview in .tsx and .jsx files rg --glob '*.tsx' --glob '*.jsx' -n '<CodePreview'Length of output: 333
I’ll pull the surrounding lines for each usage to confirm both
fileandrepoNameprops are provided:
🏁 Script executed:
#!/bin/bash echo "Context in search/page.tsx:" sed -n '330,360p' 'packages/web/src/app/[domain]/search/page.tsx' echo -e "\nContext in codePreviewPanel/index.tsx:" sed -n '60,80p' 'packages/web/src/app/[domain]/search/components/codePreviewPanel/index.tsx' echo -e "\nContext in browse/page.tsx:" sed -n '130,160p' 'packages/web/src/app/[domain]/browse/[...path]/page.tsx'Length of output: 2320
All
<CodePreview>usages updatedA search across the repo shows the only direct
<CodePreview>invocation (incodePreviewPanel/index.tsx) now supplies both required props:
- packages/web/src/app/[domain]/search/components/codePreviewPanel/index.tsx:
<CodePreview file={file} repoName={previewedFile.repository} onClose={onClose} />No other direct
<CodePreview>usages exist. No further changes are needed.packages/web/src/features/search/searchApi.ts (3)
9-9: Type rename looks good.The change from
SearchResultRangetoSourceRangealigns with the updated type definitions and improves naming consistency across the codebase.
216-301: Good refactoring of the files array construction.Extracting the files mapping logic into a separate constant improves code readability and follows the Single Responsibility Principle. The logic remains unchanged while making the code structure cleaner.
334-343: Useful addition of aggregated match statistics.The new
statsproperty withmatchCountprovides valuable aggregate information that can be displayed in the UI, eliminating the need for manual summation in consuming components.packages/web/src/app/[domain]/search/components/filterPanel/index.tsx (3)
20-34: Excellent documentation for the bidirectional filtering feature.The comprehensive JSDoc clearly explains the component's behavior and provides helpful examples. This will greatly aid future maintainability.
42-91: Well-implemented bidirectional filtering logic.The implementation correctly handles the interdependency between repository and language filters, preventing users from selecting combinations that yield no results. The use of custom hooks (
useGetSelectedFromQueryanduseFilteredMatches) promotes code reusability.
125-129: Consider performance optimization for large datasets.For repositories with many files, filtering visible entries on every render might impact performance. Consider memoizing these computations only when the relevant dependencies change.
- const visibleRepos = useMemo(() => Object.values(repos).filter((entry) => !entry.isHidden), [repos]); - const visibleLanguages = useMemo(() => Object.values(languages).filter((entry) => !entry.isHidden), [languages]); + const visibleRepos = useMemo(() => { + return Object.values(repos).filter((entry) => !entry.isHidden); + }, [repos]); + + const visibleLanguages = useMemo(() => { + return Object.values(languages).filter((entry) => !entry.isHidden); + }, [languages]);Note: The current implementation is already using
useMemo, so this is just a formatting suggestion. The performance consideration is already addressed.packages/web/src/app/[domain]/search/components/searchResultsPanel/index.tsx (3)
22-46: Excellent implementation of scroll state restoration.The use of browser history state to preserve scroll position, measurements cache, and UI state provides a seamless experience when users navigate back to search results. The implementation correctly handles all necessary state elements.
90-101: Smart use of debouncing for history state updates.Debouncing the scroll offset updates (100ms) prevents excessive history replacements while still capturing the user's scroll position effectively. This is a good performance optimization.
103-116: Thoughtful UX improvement for the show/hide matches toggle.Scrolling to the top of the file match when collapsing ensures the "show fewer matches" button remains visible. This attention to detail improves the user experience.
packages/web/src/app/globals.css (3)
7-163: Excellent improvements to CSS variable organization and theming!The changes enhance maintainability by:
- Using consistent
hsl()function syntax for all color variables- Adding comprehensive editor-specific variables for better theme customization
- Maintaining clear separation between light and dark themes
242-254: Well-designed hover styles for improved code navigation UX!The hover underline effect with smooth transitions provides excellent visual feedback for interactive code elements.
187-199:✅ Verification successful
Verify the search match class name changes are applied consistently.
The class names have been changed from
.cm-searchMatchto.searchMatch. Ensure all components using these classes have been updated.
🏁 Script executed:
#!/bin/bash # Description: Verify that old class names are not used anywhere in the codebase # Search for any remaining usage of the old class names echo "Searching for old .cm-searchMatch class usage..." rg "cm-searchMatch" --type ts --type tsx --type js --type jsx # Search for usage of new class names to ensure they're being applied echo -e "\nSearching for new .searchMatch class usage..." rg "searchMatch(?!-selected)" --type ts --type tsx --type js --type jsx -A 2Length of output: 621
🏁 Script executed:
#!/bin/bash # Verify no residual usage of the old .cm-searchMatch class echo "🔍 Searching for old .cm-searchMatch usage in code and markup files..." rg -F "cm-searchMatch" -g "*.{js,jsx,ts,tsx,html}" -n || echo "✅ No occurrences of .cm-searchMatch found." echo # Verify that the new .searchMatch class is actually referenced in code echo "🔍 Searching for .searchMatch usage in code and markup files..." rg -F "searchMatch" -g "*.{js,jsx,ts,tsx,html}" -n || echo "⚠️ No occurrences of .searchMatch found."Length of output: 996
Consistency Verified for Search Match Classes
- No remaining occurrences of the old
.cm-searchMatchselector.- New
.searchMatch/.searchMatch-selectedclasses are referenced in:
- packages/web/src/app/[domain]/browse/[...path]/components/rangeHighlightingExtension.ts
- packages/web/src/lib/extensions/searchResultHighlightExtension.ts
- packages/web/src/app/[domain]/components/lightweightCodeHighlighter.tsx
All relevant files have been updated—no further action required.
packages/web/src/app/[domain]/search/page.tsx (5)
25-35: Good improvements to imports and naming clarity!The renamed constant
DEFAULT_MAX_MATCH_COUNTbetter describes its purpose, and the new imports support the enhanced UI functionality.
56-58: Robust handling of query parameter parsing!The improved parsing logic with NaN checking ensures the application handles invalid input gracefully.
228-238: Consider potential keyboard shortcut conflicts.The
Cmd+B(Mac) /Ctrl+B(Windows/Linux) shortcut might conflict with the browser's bookmark shortcut, which could confuse users.Consider using a different shortcut combination or documenting this override clearly in the UI. Alternative suggestions:
Cmd/Ctrl+Shift+Ffor filter panelCmd/Ctrl+\(backslash)Cmd/Ctrl+[(left bracket)
222-222: Excellent simplification of state management!The refactoring from separate file/match indices to a single
previewedFilestate makes the code more intuitive and maintainable. The conditional rendering of the preview panel is clean and efficient.Also applies to: 316-318, 331-348
226-226:❓ Verification inconclusive
Add error handling for localStorage operations.
The
useLocalStoragehook might throw exceptions in private browsing mode or when storage quota is exceeded.Consider wrapping localStorage operations in a try-catch or ensuring the
useLocalStoragehook handles errors gracefully:// If the hook doesn't handle errors internally, consider creating a wrapper: const [isFilterPanelCollapsed, setIsFilterPanelCollapsed] = useSafeLocalStorage('isFilterPanelCollapsed', false); // Where useSafeLocalStorage wraps the operations in try-catch
🏁 Script executed:
#!/bin/bash # Description: Check if useLocalStorage hook has error handling # Search for the useLocalStorage implementation echo "Searching for useLocalStorage implementation..." fd -e ts -e tsx -e js -e jsx | xargs rg -l "useLocalStorage" | xargs rg -A 10 "export.*useLocalStorage"Length of output: 208
🏁 Script executed:
#!/bin/bash # Enhanced search for useLocalStorage implementation echo "Searching for export patterns..." rg -nE 'export (function|const) useLocalStorage' -t js -t jsx -t ts -t tsx echo "Searching for function definitions..." rg -n 'function useLocalStorage' -t js -t jsx -t ts -t tsx echo "Searching for any usage of useLocalStorage..." rg -n 'useLocalStorage' -t js -t jsx -t ts -t tsx echo "Searching for files with 'localStorage' in their names..." fd -e ts -e tsx -e js -e jsx -i localstorageLength of output: 1079
#!/bin/bash echo "1. All occurrences of useLocalStorage:" rg "useLocalStorage" -n . echo "2. Import statements referencing useLocalStorage:" rg "import.*useLocalStorage" -n . echo "3. Definitions of useLocalStorage in the repo:" rg "function useLocalStorage" -n . rg "const useLocalStorage" -n .
Verify error handling for localStorage usage
The call on line 226 persists UI state viauseLocalStorage('isFilterPanelCollapsed', false), which can throw (e.g. in private mode or when storage is full). Confirm whether youruseLocalStoragehook already wraps alllocalStorageoperations in a try/catch. If it does not, consider one of the following:
- Add error handling directly inside the existing hook
- Introduce a
useSafeLocalStoragewrapper that catches and silences/storage-quota errorsLocation:
- packages/web/src/app/[domain]/search/page.tsx, line 226
packages/web/src/app/[domain]/browse/[...path]/components/rangeHighlightingExtension.ts
Show resolved
Hide resolved
packages/web/src/app/[domain]/search/components/searchResultsPanel/index.tsx
Show resolved
Hide resolved
brendan-kellam
force-pushed
the
bkellam/code_nav
branch
from
8655246 to
e7e5287
Compare
…rs cause performance problems when rendering thousands of code cells. The workaround found is to use tagHighlighter and rely on tailwind to drive theming in CSS.
…'t need to re-render when doing a goto def
…xpand language filter to resolve related langs like TypeScript, JavaScript, TSX, and JSX.
brendan-kellam
force-pushed
the
bkellam/code_nav
branch
from
9f95da8 to
8417242
Compare
Sourcebot V4 introduces authentication, performance improvements and code navigation. Checkout the [migration guide](https://docs.sourcebot.dev/self-hosting/upgrade/v3-to-v4-guide) for information on upgrading your instance to v4. ### Changed - [**Breaking Change**] Authentication is now required by default. Notes: - When setting up your instance, email / password login will be the default authentication provider. - The first user that logs into the instance is given the `owner` role. ([docs](https://docs.sourcebot.dev/docs/more/roles-and-permissions)). - Subsequent users can request to join the instance. The `owner` can approve / deny requests to join the instance via `Settings` > `Members` > `Pending Requests`. - If a user is approved to join the instance, they are given the `member` role. - Additional login providers, including email links and SSO, can be configured with additional environment variables. ([docs](https://docs.sourcebot.dev/self-hosting/configuration/authentication)). - Clicking on a search result now takes you to the `/browse` view. Files can still be previewed by clicking the "Preview" button or holding `Cmd` / `Ctrl` when clicking on a search result. [#315](#315) ### Added - [Sourcebot EE] Added search-based code navigation, allowing you to jump between symbol definition and references when viewing source files. [Read the documentation](https://docs.sourcebot.dev/docs/search/code-navigation). [#315](#315) - Added collapsible filter panel. [#315](#315) ### Fixed - Improved scroll performance for large numbers of search results. [#315](#315)
This PR introduces code navigation, along with a host of performance improvements to how we render search results.
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Documentation
Chores
Refactor
Style
Tests