-
Notifications
You must be signed in to change notification settings - Fork 134
feat(ask_sb): Add search context into ask sourcebot toolbar #397
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@msukkari your pull request is missing a changelog! |
""" WalkthroughThis update refactors repository selection across the chat application to support both repositories and search contexts through a unified "context item" model. It introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI (ChatBox/Toolbar)
participant ContextSelector
participant API (chat/route)
participant DB
User->>UI (ChatBox/Toolbar): Selects repos or contexts via ContextSelector
UI (ChatBox/Toolbar)->>ContextSelector: Opens selector, user picks items
ContextSelector-->>UI (ChatBox/Toolbar): Returns selected ContextItems
UI (ChatBox/Toolbar)->>API (chat/route): Sends message with selectedRepos & selectedContexts
API (chat/route)->>DB: Fetch repos for selectedContexts
DB-->>API (chat/route): Returns repos for contexts
API (chat/route)->>API (chat/route): Merge & dedupe repos
API (chat/route)-->>UI (ChatBox/Toolbar): Streams agent response
UI (ChatBox/Toolbar)-->>User: Displays chat response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/web/src/app/[domain]/chat/[id]/components/chatThreadPanel.tsx (1)
35-51
: State initialization logic is comprehensive and correct.The initialization properly handles both repositories and contexts from the last user message metadata. The fallback logic for display names and missing repo info is well implemented.
Consider extracting the mapping logic to helper functions for improved readability:
+const mapRepoToContextItem = (repoName: string, repos: RepositoryQuery[]): ContextItem => { + const repoInfo = repos.find(r => r.repoName === repoName); + return { + type: 'repo' as const, + value: repoName, + name: repoInfo?.repoDisplayName || repoName.split('/').pop() || repoName, + codeHostType: repoInfo?.codeHostType + }; +}; + +const mapContextToContextItem = (context: string): ContextItem => ({ + type: 'context' as const, + value: context, + name: context +}); const [selectedItems, setSelectedItems] = useState<ContextItem[]>([ - ...defaultSelectedRepos.map(repoName => { - const repoInfo = repos.find(r => r.repoName === repoName); - return { - type: 'repo' as const, - value: repoName, - name: repoInfo?.repoDisplayName || repoName.split('/').pop() || repoName, - codeHostType: repoInfo?.codeHostType - }; - }), - ...defaultSelectedContexts.map(context => ({ type: 'context' as const, value: context, name: context })) + ...defaultSelectedRepos.map(repoName => mapRepoToContextItem(repoName, repos)), + ...defaultSelectedContexts.map(mapContextToContextItem) ]);packages/web/src/features/chat/components/chatBox/contextSelector.tsx (1)
219-233
: Icon rendering logic is functional but could be cleaner.The inline IIFE for code host icon rendering works but could be more readable.
Consider extracting the icon rendering to a helper function:
+const renderRepoIcon = (item: ContextItem) => { + const codeHostIcon = item.codeHostType ? getCodeHostIcon(item.codeHostType) : null; + return codeHostIcon ? ( + <Image + src={codeHostIcon.src} + alt={`${item.codeHostType} icon`} + width={16} + height={16} + className={cn("h-4 w-4", codeHostIcon.className)} + /> + ) : ( + <FolderIcon className="h-4 w-4 text-muted-foreground" /> + ); +}; {item.type === 'context' ? ( <LibraryBigIcon className="h-4 w-4 text-muted-foreground" /> ) : ( - // Render code host icon for repos - (() => { - const codeHostIcon = item.codeHostType ? getCodeHostIcon(item.codeHostType) : null; - return codeHostIcon ? ( - <Image - src={codeHostIcon.src} - alt={`${item.codeHostType} icon`} - width={16} - height={16} - className={cn("h-4 w-4", codeHostIcon.className)} - /> - ) : ( - <FolderIcon className="h-4 w-4 text-muted-foreground" /> - ); - })() + renderRepoIcon(item) )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
packages/web/src/actions.ts
(1 hunks)packages/web/src/app/[domain]/chat/[id]/components/chatThreadPanel.tsx
(5 hunks)packages/web/src/app/[domain]/chat/components/newChatPanel.tsx
(3 hunks)packages/web/src/app/[domain]/components/homepage/agenticSearch.tsx
(4 hunks)packages/web/src/app/api/(server)/chat/route.ts
(3 hunks)packages/web/src/features/chat/components/chatBox/chatBox.tsx
(7 hunks)packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx
(2 hunks)packages/web/src/features/chat/components/chatBox/contextSelector.tsx
(1 hunks)packages/web/src/features/chat/components/chatBox/repoSelector.tsx
(0 hunks)packages/web/src/features/chat/components/chatThread/chatThread.tsx
(7 hunks)packages/web/src/features/chat/types.ts
(3 hunks)packages/web/src/features/chat/useCreateNewChatThread.ts
(2 hunks)packages/web/src/features/chat/utils.ts
(2 hunks)
💤 Files with no reviewable changes (1)
- packages/web/src/features/chat/components/chatBox/repoSelector.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*
📄 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/features/chat/utils.ts
packages/web/src/app/[domain]/chat/[id]/components/chatThreadPanel.tsx
packages/web/src/actions.ts
packages/web/src/features/chat/types.ts
packages/web/src/app/[domain]/chat/components/newChatPanel.tsx
packages/web/src/app/[domain]/components/homepage/agenticSearch.tsx
packages/web/src/features/chat/components/chatBox/contextSelector.tsx
packages/web/src/features/chat/components/chatBox/chatBox.tsx
packages/web/src/features/chat/components/chatThread/chatThread.tsx
packages/web/src/features/chat/useCreateNewChatThread.ts
packages/web/src/app/api/(server)/chat/route.ts
packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (41)
packages/web/src/features/chat/utils.ts (1)
175-175
: LGTM! Clean addition of optional selectedContexts parameter.The function signature correctly adds the optional
selectedContexts
parameter and properly includes it in the metadata object. The implementation maintains backward compatibility and follows the same pattern asselectedRepos
.Also applies to: 209-209
packages/web/src/actions.ts (1)
1865-1867
: LGTM! Proper enhancement to include repository data.The addition of
include: { repos: true }
and therepoCount
calculation correctly enriches the search context data to support the new context selector functionality. The implementation is clean and efficient.Also applies to: 1873-1873
packages/web/src/features/chat/useCreateNewChatThread.ts (1)
22-22
: LGTM! Well-structured refactor to support unified context selection.The changes properly adapt the function to work with
ContextItem[]
instead of repository strings. The separation logic for repos and contexts is clean, and all usage points have been consistently updated.Also applies to: 26-30, 46-46
packages/web/src/features/chat/types.ts (1)
54-54
: LGTM! Consistent type definitions for the new selectedContexts feature.The type definitions properly extend the existing schemas and types to include
selectedContexts
. The optional nature and Zod validation patterns are consistent with the existingselectedRepos
implementation.Also applies to: 143-143, 161-161
packages/web/src/app/[domain]/components/homepage/agenticSearch.tsx (1)
130-132
: LGTM! Comprehensive and consistent refactor to context-based selection.The component has been properly updated to use the unified context selection model. All state variables, prop names, and handlers have been consistently renamed from repo-specific to context-specific terminology. The functionality is preserved while adapting to the new
ContextItem
interface.Also applies to: 162-169, 176-179, 205-205
packages/web/src/app/[domain]/chat/components/newChatPanel.tsx (6)
13-13
: LGTM - Import addition is consistent with the refactor.The import of
ContextItem
correctly supports the transition from repository-only selection to unified context item selection.
26-26
: State refactoring looks correct.The change from
selectedRepos
toselectedItems
withContextItem[]
type properly supports the unified selection of both repositories and search contexts. The localStorage key is also appropriately updated.
28-28
: Consistent naming update for selector state.The rename from
isRepoSelectorOpen
toisContextSelectorOpen
aligns with the component's expanded functionality.
31-32
: Callback properly updated for new data structure.The
onSubmit
callback correctly usesselectedItems
and the dependency array is properly maintained.
51-52
: Props correctly updated for ChatBox component.The props
selectedItems
andonContextSelectorOpenChanged
properly reflect the new unified context selection model.
58-62
: ChatBoxToolbar props properly aligned with new interface.All props related to context selection have been consistently updated to support both repositories and search contexts.
packages/web/src/app/[domain]/chat/[id]/components/chatThreadPanel.tsx (2)
82-82
: Dependency array correctly updated.Adding
repos
to the dependency array is necessary since the effect now depends on repository metadata for proper ContextItem creation.
97-98
: Props correctly updated to use unified context selection.The transition from repository-specific props to context item props is properly implemented.
packages/web/src/features/chat/components/chatBox/contextSelector.tsx (8)
32-39
: Well-designed discriminated union type.The
ContextItem
type properly supports both repositories and search contexts with appropriate type-specific properties. The discriminated union pattern enables type-safe handling throughout the codebase.
41-49
: Comprehensive and well-typed props interface.The interface properly defines all necessary props for a multi-select context selector, including proper typing for repositories, search contexts, and state management callbacks.
71-81
: Keyboard handling implements good UX patterns.The Enter key to open and Backspace to remove last item (when input is empty) follows common multi-select UI conventions and enhances usability.
83-99
: Toggle logic is well-implemented with good UX considerations.The scroll position preservation prevents jarring UI jumps during selection changes, and the item comparison logic correctly handles the discriminated union type.
109-126
: Items processing logic is well-structured.The mapping from repositories and search contexts to the unified
ContextItem
format is clean and handles edge cases like missing display names appropriately.
128-145
: Sorting logic enhances user experience.Prioritizing selected items and grouping contexts before repositories creates an intuitive interface. The memoization ensures good performance.
147-152
: Scroll position restoration is properly implemented.The effect correctly restores scroll position after re-renders, improving user experience during selection changes.
239-246
: Badge display for context repo counts is informative.The badge correctly shows repository counts for search contexts with proper pluralization logic.
packages/web/src/app/api/(server)/chat/route.ts (4)
67-67
: Request parsing correctly updated for new parameter.The extraction of
selectedContexts
alongsideselectedRepos
properly supports the unified context selection model.
97-97
: Interface properly extended for new parameter.The optional
selectedContexts
parameter correctly reflects the API enhancement.
218-218
: Expanded repository list correctly used in agent stream.The
expandedRepos
variable properly replaces the originalselectedRepos
to include repositories from selected contexts.
191-210
: Ensure performance tuning for the searchContext queryThe repository-expansion logic in
packages/web/src/app/api/(server)/chat/route.ts
(lines 191–210) correctly fetches contexts, flattens their repos, and deduplicates the list. However, I didn’t find any migrations or schema definitions for indexes onsearchContext(orgId, name)
or on the join table linking contexts to repos.• Please verify that your database has appropriate indexes (e.g., on
(orgId, name)
insearchContext
and on the context–repo junction) to support large datasets.
• Consider adding performance monitoring or tracing around theprisma.searchContext.findMany
call (e.g., Prisma query logging or explain plans) to catch potential slowdowns early.packages/web/src/features/chat/components/chatBox/chatBox.tsx (5)
31-32
: Props correctly updated for unified context selection.The transition from repository-specific props to context item props maintains functionality while supporting the expanded selection model.
53-53
: Filtering logic properly extracts repositories for suggestions.The filter operation correctly extracts repository values from the unified
ContextItem[]
for use with the existing suggestions system.
117-117
: Submit validation correctly updated for context items.Using
selectedItems.length
properly validates that either repositories or search contexts (or both) are selected.
149-149
: User messaging appropriately updated.The toast message correctly mentions both "repositories or search contexts" to reflect the expanded functionality.
326-326
: Tooltip messaging is consistent with expanded functionality.The tooltip correctly informs users about the requirement to select repositories or search contexts.
packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx (5)
15-19
: LGTM: Import additions support the new context selector functionality.The new imports are well-organized and necessary for the component's enhanced functionality to handle both repositories and search contexts.
24-27
: LGTM: Props interface properly updated for unified context selection.The interface changes from repo-specific to context-aware naming and types are consistent with the broader refactoring to support both repositories and search contexts through a unified
ContextItem
model.
33-36
: LGTM: Component signature consistently updated.The parameter names properly match the updated interface, maintaining consistency throughout the component.
39-50
: LGTM: Well-implemented async data fetching with proper error handling.The
useQuery
implementation correctly:
- Uses domain-based cache keys for proper invalidation
- Filters out service errors in the
select
function- Provides safe fallback to empty array
95-107
: LGTM: ContextSelector properly integrated with appropriate props and fallbacks.The implementation correctly:
- Passes all necessary props to the ContextSelector
- Provides safe fallback for
searchContexts
with|| []
- Updates tooltip text to reflect the new unified functionality
- Maintains consistent styling and behavior
packages/web/src/features/chat/components/chatThread/chatThread.tsx (6)
26-26
: LGTM: Import addition supports the unified context model.The import of
ContextItem
is necessary for the new type definitions in the props interface.
38-39
: LGTM: Props interface consistently updated for unified context selection.The interface changes from repo-specific to context-aware naming and the corresponding parameter updates maintain consistency with the broader refactoring.
Also applies to: 49-50
61-61
: LGTM: State variable appropriately renamed for context selector.The renaming from
isRepoSelectorOpen
toisContextSelectorOpen
is consistent with the UI component changes.
115-117
: LGTM: Context extraction logic properly implemented.The filtering and mapping logic correctly separates repositories and contexts from the unified
selectedItems
array. The dependency array is properly updated to includeselectedItems
.Also applies to: 122-122, 126-126
242-245
: LGTM: Consistent context extraction in onSubmit callback.The same extraction pattern is consistently applied in the
onSubmit
callback, maintaining coherent behavior throughout the component.Also applies to: 251-251
328-329
: LGTM: Child component props consistently updated.The props passed to
ChatBox
andChatBoxToolbar
are properly updated to match their new interfaces, maintaining consistency throughout the component hierarchy.Also applies to: 335-338
packages/web/src/app/[domain]/chat/[id]/components/chatThreadPanel.tsx
Outdated
Show resolved
Hide resolved
packages/web/src/features/chat/components/chatBox/contextSelector.tsx
Outdated
Show resolved
Hide resolved
packages/web/src/features/chat/components/chatThread/chatThread.tsx
Outdated
Show resolved
Hide resolved
packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/web/src/app/[domain]/chat/[id]/components/chatThreadPanel.tsx (1)
62-75
: Address duplicated mapping logic as noted in previous review.The mapping logic here duplicates the state initialization code (lines 43-53), creating maintenance burden and potential inconsistency as mentioned in the previous review comment.
🧹 Nitpick comments (1)
packages/web/src/app/[domain]/chat/[id]/components/chatThreadPanel.tsx (1)
52-52
: Enhance context mapping to utilize available search context data.The current mapping only sets
name: context
for search contexts, butSearchContextQuery
likely contains additional properties likedescription
andrepoCount
that could enhance the UI.Consider utilizing the full search context data:
-...defaultSelectedContexts.map(context => ({ type: 'context' as const, value: context, name: context })) +...defaultSelectedContexts.map(contextName => { + const contextInfo = searchContexts.find(ctx => ctx.name === contextName); + return { + type: 'context' as const, + value: contextName, + name: contextName, + description: contextInfo?.description, + repoCount: contextInfo?.repoCount + }; +})Apply similar changes to the URL parameter mapping logic.
Also applies to: 74-74
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/web/src/app/[domain]/chat/[id]/components/chatThreadPanel.tsx
(6 hunks)packages/web/src/app/[domain]/chat/[id]/page.tsx
(4 hunks)packages/web/src/app/[domain]/chat/components/newChatPanel.tsx
(2 hunks)packages/web/src/app/[domain]/chat/page.tsx
(4 hunks)packages/web/src/app/[domain]/components/homepage/agenticSearch.tsx
(5 hunks)packages/web/src/app/[domain]/components/homepage/index.tsx
(4 hunks)packages/web/src/app/[domain]/page.tsx
(3 hunks)packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx
(2 hunks)packages/web/src/features/chat/components/chatThread/chatThread.tsx
(7 hunks)packages/web/src/lib/schemas.ts
(1 hunks)packages/web/src/lib/types.ts
(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/web/src/lib/types.ts
- packages/web/src/lib/schemas.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/web/src/app/[domain]/chat/components/newChatPanel.tsx
- packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx
- packages/web/src/app/[domain]/components/homepage/agenticSearch.tsx
- packages/web/src/features/chat/components/chatThread/chatThread.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*
📄 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]/chat/page.tsx
packages/web/src/app/[domain]/components/homepage/index.tsx
packages/web/src/app/[domain]/chat/[id]/page.tsx
packages/web/src/app/[domain]/page.tsx
packages/web/src/app/[domain]/chat/[id]/components/chatThreadPanel.tsx
🧬 Code Graph Analysis (4)
packages/web/src/app/[domain]/chat/page.tsx (2)
packages/web/src/actions.ts (1)
getSearchContexts
(1858-1876)packages/web/src/lib/serviceError.ts (1)
ServiceErrorException
(16-20)
packages/web/src/app/[domain]/components/homepage/index.tsx (1)
packages/web/src/lib/types.ts (1)
SearchContextQuery
(29-29)
packages/web/src/app/[domain]/chat/[id]/page.tsx (2)
packages/web/src/actions.ts (1)
getSearchContexts
(1858-1876)packages/web/src/lib/serviceError.ts (1)
ServiceErrorException
(16-20)
packages/web/src/app/[domain]/chat/[id]/components/chatThreadPanel.tsx (3)
packages/web/src/features/chat/types.ts (1)
SetChatStatePayload
(140-144)packages/web/src/lib/types.ts (2)
RepositoryQuery
(28-28)SearchContextQuery
(29-29)packages/web/src/features/chat/components/chatBox/contextSelector.tsx (1)
ContextItem
(32-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
packages/web/src/app/[domain]/page.tsx (1)
1-1
: LGTM! Consistent implementation following established patterns.The integration of
getSearchContexts
follows the exact same pattern as existing data fetching calls (getRepos
,getUserChatHistory
) with proper error handling and prop passing. The implementation is consistent and well-structured.Also applies to: 25-25, 32-35, 60-60
packages/web/src/app/[domain]/chat/[id]/page.tsx (1)
1-1
: LGTM! Consistent data fetching and error handling pattern.The implementation follows the established pattern for data fetching with proper error handling using
ServiceErrorException
and correctly passessearchContexts
to theChatThreadPanel
component.Also applies to: 25-25, 38-41, 82-82
packages/web/src/app/[domain]/components/homepage/index.tsx (1)
5-5
: LGTM! Clean prop threading implementation.The addition of
searchContexts
prop follows proper TypeScript patterns with correct type import, interface extension, and component prop passing. The implementation maintains consistency with existing code structure.Also applies to: 16-16, 29-29, 87-87
packages/web/src/app/[domain]/chat/page.tsx (1)
1-1
: LGTM! Follows established data fetching pattern.The implementation is consistent with other page components, properly fetching
searchContexts
, handling service errors, and passing the data as props to child components.Also applies to: 21-21, 33-36, 56-56
packages/web/src/app/[domain]/chat/[id]/components/chatThreadPanel.tsx (1)
6-6
: LGTM! Proper type imports and prop additions.The import of new types (
SearchContextQuery
,ContextItem
) and the addition ofsearchContexts
prop to the component interface and signature are implemented correctly.Also applies to: 11-11, 16-16, 25-25, 99-99
packages/web/src/app/[domain]/chat/[id]/components/chatThreadPanel.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/web/src/features/chat/components/chatBox/contextSelector.tsx (1)
32-46
: Current discriminated union implementation is adequateThe type definitions already implement a discriminated union pattern with the
type
field as the discriminant. The TypeScript compiler can properly narrow types based on thetype
field, making the current implementation type-safe and meeting the intent of the previous review comment.
🧹 Nitpick comments (3)
packages/web/src/features/chat/components/chatBox/contextSelector.tsx (3)
90-106
: Consider optimizing item comparison logicThe item comparison logic correctly handles selection state, but the comparison is repeated in multiple places. Consider extracting this into a helper function for better maintainability.
+const itemsEqual = (a: ContextItem, b: ContextItem) => + a.type === b.type && a.value === b.value; const toggleItem = (item: ContextItem) => { // Store current scroll position before state update if (scrollContainerRef.current) { scrollPosition.current = scrollContainerRef.current.scrollTop; } - const isSelected = selectedItems.some( - (selected) => selected.type === item.type && selected.value === item.value - ); + const isSelected = selectedItems.some(selected => itemsEqual(selected, item)); const newSelectedItems = isSelected - ? selectedItems.filter( - (selected) => !(selected.type === item.type && selected.value === item.value) - ) + ? selectedItems.filter(selected => !itemsEqual(selected, item)) : [...selectedItems, item]; onSelectedItemsChange(newSelectedItems); };
134-151
: Consider simplifying the sorting logicThe sorting logic correctly prioritizes selected items and contexts, but the nested ternary operators make it less readable. Consider using a more explicit approach.
const sortedItems = React.useMemo(() => { + const getSortPriority = (item: ContextItem, isSelected: boolean) => { + if (isSelected) return 0; // Selected items first + if (item.type === 'context') return 1; // Then contexts + return 2; // Finally repos + }; + return allItems .map((item) => ({ item, isSelected: selectedItems.some( (selected) => selected.type === item.type && selected.value === item.value ) })) - .sort((a, b) => { - // Selected items first - if (a.isSelected && !b.isSelected) return -1; - if (!a.isSelected && b.isSelected) return 1; - // Then contexts before repos - if (a.item.type === 'context' && b.item.type === 'repo') return -1; - if (a.item.type === 'repo' && b.item.type === 'context') return 1; - return 0; - }) + .sort((a, b) => { + const priorityA = getSortPriority(a.item, a.isSelected); + const priorityB = getSortPriority(b.item, b.isSelected); + return priorityA - priorityB; + }); }, [allItems, selectedItems]);
225-238
: Complex inline rendering logic should be extractedThe inline immediately invoked function expression (IIFE) for rendering code host icons makes the JSX harder to read. Consider extracting this logic into a separate function or component.
+ const renderItemIcon = (item: ContextItem) => { + if (item.type === 'context') { + return <LibraryBigIcon className="h-4 w-4 text-muted-foreground" />; + } + + const codeHostIcon = item.codeHostType ? getCodeHostIcon(item.codeHostType) : null; + return codeHostIcon ? ( + <Image + src={codeHostIcon.src} + alt={`${item.codeHostType} icon`} + width={16} + height={16} + className={cn("h-4 w-4", codeHostIcon.className)} + /> + ) : ( + <FolderIcon className="h-4 w-4 text-muted-foreground" /> + ); + }; <div className="flex items-center gap-2 flex-1"> - {item.type === 'context' ? ( - <LibraryBigIcon className="h-4 w-4 text-muted-foreground" /> - ) : ( - // Render code host icon for repos - (() => { - const codeHostIcon = item.codeHostType ? getCodeHostIcon(item.codeHostType) : null; - return codeHostIcon ? ( - <Image - src={codeHostIcon.src} - alt={`${item.codeHostType} icon`} - width={16} - height={16} - className={cn("h-4 w-4", codeHostIcon.className)} - /> - ) : ( - <FolderIcon className="h-4 w-4 text-muted-foreground" /> - ); - })() - )} + {renderItemIcon(item)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/web/src/app/[domain]/chat/[id]/components/chatThreadPanel.tsx
(6 hunks)packages/web/src/features/chat/components/chatBox/contextSelector.tsx
(1 hunks)packages/web/src/features/chat/components/chatThread/chatThread.tsx
(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/web/src/features/chat/components/chatThread/chatThread.tsx
- packages/web/src/app/[domain]/chat/[id]/components/chatThreadPanel.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*
📄 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/features/chat/components/chatBox/contextSelector.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (6)
packages/web/src/features/chat/components/chatBox/contextSelector.tsx (6)
3-30
: LGTM - Clean imports and dependenciesThe imports are well-organized and include all necessary dependencies for the component functionality.
48-56
: Well-designed component interfaceThe props interface properly extends
React.ButtonHTMLAttributes<HTMLButtonElement>
and includes all necessary props for the component's functionality. Good separation of concerns with explicit handlers for state management.
75-88
: Keyboard handling logic is well-implementedThe keyboard event handling properly manages Enter to open the popover and Backspace to remove the last selected item when input is empty. The logic is intuitive and follows common UX patterns.
116-132
: Efficient memoization of item transformationGood use of
useMemo
to prevent unnecessary recalculation of the combined items array. The transformation logic correctly maps both contexts and repositories to the unifiedContextItem
type.
153-158
: Smart scroll position preservationExcellent implementation of scroll position preservation using refs. This maintains user experience by preventing the list from jumping back to the top when items are selected/deselected.
278-279
: Proper displayName assignment for debuggingGood practice to set the displayName for forwardRef components to aid in debugging and React DevTools identification.
Summary by CodeRabbit
New Features
Improvements
Refactor
Chores