refactor: update Document and Connector selection components #456
+293
−87
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Replaced the use of search source connectors with document types for improved clarity and functionality. - Enhanced UI elements for document type selection, including better styling and loading states. - Updated selection logic to handle document types and their counts effectively. - Improved accessibility and user experience in the document and connector selection dialogs.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe pull request refactors the chat input component from a connector-based selection model to a document-type driven UI. A new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant ChatInputGroup
participant useDocumentTypes
participant API as API:/api/v1/documents/type-counts/
User->>ChatInputGroup: Mount/Initialize
ChatInputGroup->>useDocumentTypes: Call useDocumentTypes(searchSpaceId)
useDocumentTypes->>useDocumentTypes: Retrieve token from localStorage
useDocumentTypes->>API: GET with optional search_space_id param
API-->>useDocumentTypes: Return {type: count}
useDocumentTypes->>useDocumentTypes: Convert to DocumentTypeCount[]
useDocumentTypes-->>ChatInputGroup: Return documentTypes, isLoading, isLoaded, error
ChatInputGroup->>ChatInputGroup: Render DocumentSelector with documentTypes
User->>ChatInputGroup: Select/Toggle document types
ChatInputGroup->>ChatInputGroup: Update selectedDocTypes state
User->>ChatInputGroup: Trigger refresh
ChatInputGroup->>useDocumentTypes: Call refreshDocumentTypes(spaceId)
useDocumentTypes->>API: Re-fetch type counts
API-->>useDocumentTypes: Updated counts
useDocumentTypes-->>ChatInputGroup: Updated documentTypes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![recurseml[bot]](https://avatars.githubusercontent.com/in/1054195?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by RecurseML
🔍 Review performed on a09309a..4302493
✨ No bugs found, your code is sparkling clean
✅ Files analyzed, no issues (1)
• surfsense_web/components/chat/ChatInputGroup.tsx
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
surfsense_web/components/chat/ChatInputGroup.tsx(6 hunks)surfsense_web/hooks/use-document-types.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.rules/require_unique_id_props.mdc)
**/*.{jsx,tsx}: When mapping arrays to React elements in JSX/TSX, each rendered element must include a unique key prop
Keys used for React list items should be stable, predictable, and unique among siblings
Files:
surfsense_web/components/chat/ChatInputGroup.tsx
🧬 Code graph analysis (1)
surfsense_web/components/chat/ChatInputGroup.tsx (2)
surfsense_web/hooks/use-document-types.ts (1)
useDocumentTypes(13-97)surfsense_web/contracts/enums/connectorIcons.tsx (1)
getConnectorIcon(22-80)
Comment on lines
+19
to
+66
| const fetchDocumentTypes = useCallback( | ||
| async (spaceId?: number) => { | ||
| if (isLoaded && lazy) return; | ||
|
|
||
| try { | ||
| setIsLoading(true); | ||
| setError(null); | ||
| const token = localStorage.getItem("surfsense_bearer_token"); | ||
|
|
||
| if (!token) { | ||
| throw new Error("No authentication token found"); | ||
| } | ||
|
|
||
| // Build URL with optional search_space_id query parameter | ||
| const url = new URL( | ||
| `${process.env.NEXT_PUBLIC_FASTAPI_BACKEND_URL}/api/v1/documents/type-counts/` | ||
| ); | ||
| if (spaceId !== undefined) { | ||
| url.searchParams.append("search_space_id", spaceId.toString()); | ||
| } | ||
|
|
||
| const response = await fetch(url.toString(), { | ||
| method: "GET", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| Authorization: `Bearer ${token}`, | ||
| }, | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error(`Failed to fetch document types: ${response.statusText}`); | ||
| } | ||
|
|
||
| const data = await response.json(); | ||
|
|
||
| // Convert the object to an array of DocumentTypeCount | ||
| const typeCounts: DocumentTypeCount[] = Object.entries(data).map(([type, count]) => ({ | ||
| type, | ||
| count: count as number, | ||
| })); | ||
|
|
||
| setDocumentTypes(typeCounts); | ||
| setIsLoaded(true); | ||
|
|
||
| return typeCounts; | ||
| } catch (err) { | ||
| setError(err instanceof Error ? err : new Error("An unknown error occurred")); | ||
| console.error("Error fetching document types:", err); |
There was a problem hiding this comment.
Don't skip refetch when the search space changes.
With lazy=true, isLoaded stays true after the first fetch. When the user switches to a different search_space_id, we still call fetchDocumentTypes(newId), but the guard if (isLoaded && lazy) return; fires before we compare the IDs, so the hook keeps returning the stale counts for the previous space. Users opening the selector for a new space will never see the correct document types until a full reload. Please gate the early return on the same space (or reset isLoaded when the space changes). One way:
- const fetchDocumentTypes = useCallback(
- async (spaceId?: number) => {
- if (isLoaded && lazy) return;
+ const [lastFetchedSpaceId, setLastFetchedSpaceId] = useState<number | undefined>(undefined);
+
+ const fetchDocumentTypes = useCallback(
+ async (spaceId?: number) => {
+ const resolvedSpaceId = spaceId ?? searchSpaceId;
+ if (isLoaded && lazy && resolvedSpaceId === lastFetchedSpaceId) return;
@@
- if (spaceId !== undefined) {
- url.searchParams.append("search_space_id", spaceId.toString());
+ if (resolvedSpaceId !== undefined) {
+ url.searchParams.append("search_space_id", resolvedSpaceId.toString());
@@
- setDocumentTypes(typeCounts);
- setIsLoaded(true);
+ setDocumentTypes(typeCounts);
+ setIsLoaded(true);
+ setLastFetchedSpaceId(resolvedSpaceId);
} catch (err) {
@@
- },
- [isLoaded, lazy]
+ },
+ [isLoaded, lazy, searchSpaceId, lastFetchedSpaceId]
);
@@
const refreshDocumentTypes = useCallback(
async (spaceId?: number) => {
setIsLoaded(false);
- await fetchDocumentTypes(spaceId !== undefined ? spaceId : searchSpaceId);
+ setLastFetchedSpaceId(undefined);
+ await fetchDocumentTypes(spaceId !== undefined ? spaceId : searchSpaceId);
},
[fetchDocumentTypes, searchSpaceId]
);This keeps the cached behavior for the same space but correctly refetches when the user changes spaces.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In surfsense_web/hooks/use-document-types.ts around lines 19–66, the
early-return guard `if (isLoaded && lazy) return;` prevents refetch when the
search_space_id changes; modify the logic to track the last fetched space and
only skip when the requested space matches the cached one: add a ref or state
(e.g., lastFetchedSpaceId) and change the guard to `if (isLoaded && lazy &&
lastFetchedSpaceId === spaceId) return;` and update lastFetchedSpaceId when a
successful fetch completes (or alternatively reset isLoaded/lastFetchedSpaceId
in a useEffect when spaceId changes) so switching spaces triggers a new fetch
while preserving lazy caching for the same space.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Motivation and Context
FIX #
Screenshots
API Changes
Change Type
Testing Performed
Checklist
High-level PR Summary
This PR refactors document and connector selection components by replacing search source connectors with document types, improving the clarity of the selection logic. A new
useDocumentTypeshook fetches document type counts from the API, while the UI components inChatInputGroup.tsxhave been redesigned with enhanced styling, better loading states, empty states, and improved accessibility. The connector selector now displays document types with their respective counts in a cleaner, more intuitive grid layout with visual feedback for selected items.⏱️ Estimated Review Time: 15-30 minutes
💡 Review Order Suggestion
surfsense_web/hooks/use-document-types.tssurfsense_web/components/chat/ChatInputGroup.tsxSummary by CodeRabbit
New Features
Improvements