Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughShifts workspace event propagation from client-driven optimistic broadcasts to server-side confirmed broadcasts, removes client optimistic-update hooks and two assistant tool UIs, rebrands "Note" → "Document" in multiple UIs, introduces server broadcast helper, and applies widespread formatting, typing, and minor UI/behavior tweaks across realtime, workspace hooks, editor, and assistant code. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant API as Events API
participant DB as Database
participant Realtime as Supabase Realtime
participant Other as Other Clients
Client->>API: POST /api/workspaces/[id]/events
API->>DB: append_workspace_event(...)
DB-->>API: success + version
activate API
API->>Realtime: broadcastWorkspaceEventFromServer(workspaceId, event{...,version})
Realtime->>Other: workspace_event broadcast
deactivate API
API-->>Client: response
sequenceDiagram
participant Client as Client
participant Hook as useWorkspaceRealtime
participant Channel as Supabase Channel
participant Cache as React Query Cache
Client->>Hook: subscribe(workspaceId)
Hook->>Channel: subscribe workspace:{id}:events
Channel->>Hook: incoming workspace_event
Hook->>Cache: mergeRealtimeEvent(cached, event) -- dedupe & version merge
alt gap detected
Hook->>Cache: invalidate query -> refetch
end
Cache-->>Client: updated events via queries
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+465
to
+472
| JSON.stringify({ | ||
| error: "Internal server error", | ||
| message: | ||
| "An unexpected error occurred while processing your request. Please try again.", | ||
| details: | ||
| process.env.NODE_ENV === "development" ? errorMessage : undefined, | ||
| code: "INTERNAL_ERROR", | ||
| }), |
Check warning
Code scanning / CodeQL
Information exposure through a stack trace Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 29 days ago
In general, the fix is to ensure that information derived from errors (messages, stacks, etc.) is not returned directly to clients. Instead, return generic, user-safe messages and log the detailed error information server-side for debugging and monitoring.
For this specific route, the problematic field is details in the JSON error response (lines 465–472). It currently sets:
details:
process.env.NODE_ENV === "development" ? errorMessage : undefined,The best fix without changing existing behavior too much is:
- Stop including
errorMessage(derived from the thrown error) in the response body. - Optionally replace it with a static, generic string that does not depend on the error content, or omit
detailsentirely. - Keep or enhance server-side logging (
logger.error(...)) so that developers still have access to the actual error message and stack trace.
Concretely:
- In
src/app/api/chat/route.ts, in thecatch (error)block at the bottom, modify thedetailsproperty of the JSON response:- Either remove the
detailsfield or set it to a constant generic string whenNODE_ENV === "development", rather thanerrorMessage.
- Either remove the
- No new imports are required; we already have a
loggerandprocess.env.NODE_ENVavailable.
This will break the taint flow from convertError/error into the response while preserving overall functionality (clients still get a 500 with a helpful but generic message; detailed data remain in logs).
Suggested changeset
1
src/app/api/chat/route.ts
| @@ -466,8 +466,11 @@ | ||
| error: "Internal server error", | ||
| message: | ||
| "An unexpected error occurred while processing your request. Please try again.", | ||
| // Do not expose internal error details or stack traces to the client. | ||
| details: | ||
| process.env.NODE_ENV === "development" ? errorMessage : undefined, | ||
| process.env.NODE_ENV === "development" | ||
| ? "Additional error details are available in the server logs." | ||
| : undefined, | ||
| code: "INTERNAL_ERROR", | ||
| }), | ||
| { |
Copilot is powered by AI and may make mistakes. Always verify output.
Contributor
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Comment on lines
+2
to
+10
| import { | ||
| streamText, | ||
| smoothStream, | ||
| convertToModelMessages, | ||
| pruneMessages, | ||
| stepCountIs, | ||
| wrapLanguageModel, | ||
| tool, | ||
| } from "ai"; |
Comment on lines
+3
to
+9
| import React, { | ||
| useState, | ||
| useEffect, | ||
| useRef, | ||
| useCallback, | ||
| useMemo, | ||
| } from "react"; |
| const createMarkerElementAtRange = useCallback( | ||
| (range: Range): HTMLElement | null => { | ||
| try { | ||
| const originalRect = range.getBoundingClientRect(); |
Comment on lines
+48
to
+66
| import { | ||
| Brain, | ||
| Play, | ||
| ChevronUp, | ||
| ChevronDown, | ||
| ChevronRight, | ||
| X, | ||
| Circle, | ||
| CircleDot, | ||
| ArrowUpIcon, | ||
| ArrowLeft, | ||
| CheckCircle2, | ||
| FileText, | ||
| Folder as FolderIcon, | ||
| Search, | ||
| FolderSearch, | ||
| Globe, | ||
| Sparkles, | ||
| } from "lucide-react"; |
Comment on lines
+77
to
+88
| import { | ||
| ArrowLeft, | ||
| Highlighter, | ||
| AlignLeft, | ||
| AlignCenter, | ||
| Code2, | ||
| Type, | ||
| Sigma, | ||
| Trash2, | ||
| Table as TableIcon, | ||
| Settings, | ||
| } from "lucide-react"; |
Comment on lines
+15
to
+18
| import { | ||
| plainTextToBlocks, | ||
| type Block, | ||
| } from "@/components/editor/BlockNoteEditor"; |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
5 issues found across 32 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/hooks/workspace/use-workspace-presence.ts">
<violation number="1" location="src/hooks/workspace/use-workspace-presence.ts:61">
P2: Clear collaborator state when presence is disabled to avoid showing stale users until the next sync.</violation>
<violation number="2" location="src/hooks/workspace/use-workspace-presence.ts:109">
P2: Avoid depending on the entire `currentUser` object in the channel setup effect; it forces unnecessary channel re-subscriptions.</violation>
</file>
<file name="src/contexts/RealtimeContext.tsx">
<violation number="1" location="src/contexts/RealtimeContext.tsx:46">
P1: Removing `useMemo` for `currentUser` causes a new object reference on every render. Since `useWorkspacePresence` uses `currentUser` in two `useEffect` dependency arrays, this will tear down and recreate the Supabase presence channel on every render — a significant performance regression. Restore `useMemo` to maintain referential stability.</violation>
</file>
<file name="src/workflows/ocr-dispatch/steps/persist-results.ts">
<violation number="1" location="src/workflows/ocr-dispatch/steps/persist-results.ts:54">
P1: Don’t fail the workflow on realtime broadcast errors after the event is already persisted; this can trigger retries that duplicate OCR patch events.</violation>
</file>
<file name="src/app/api/workspaces/[id]/events/route.ts">
<violation number="1" location="src/app/api/workspaces/[id]/events/route.ts:470">
P2: The broadcast is `await`ed, adding Supabase round-trip latency to every successful POST response even though the event is already persisted. Additionally, an unhandled error from `removeChannel` in the `finally` block of `broadcastWorkspaceEventFromServer` could cause a 500. Consider making this fire-and-forget like `checkAndCreateSnapshot` above.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| }; | ||
| }, [session?.user]); | ||
| // Current user info for presence | ||
| const currentUser = session?.user |
Contributor
There was a problem hiding this comment.
P1: Removing useMemo for currentUser causes a new object reference on every render. Since useWorkspacePresence uses currentUser in two useEffect dependency arrays, this will tear down and recreate the Supabase presence channel on every render — a significant performance regression. Restore useMemo to maintain referential stability.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/contexts/RealtimeContext.tsx, line 46:
<comment>Removing `useMemo` for `currentUser` causes a new object reference on every render. Since `useWorkspacePresence` uses `currentUser` in two `useEffect` dependency arrays, this will tear down and recreate the Supabase presence channel on every render — a significant performance regression. Restore `useMemo` to maintain referential stability.</comment>
<file context>
@@ -9,88 +9,88 @@
- };
- }, [session?.user]);
+ // Current user info for presence
+ const currentUser = session?.user
+ ? {
+ id: session.user.id,
</file context>
Comment on lines
+54
to
+57
| await broadcastWorkspaceEventFromServer(workspaceId, { | ||
| ...event, | ||
| version: Number(match[1]), | ||
| }); |
Contributor
There was a problem hiding this comment.
P1: Don’t fail the workflow on realtime broadcast errors after the event is already persisted; this can trigger retries that duplicate OCR patch events.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/workflows/ocr-dispatch/steps/persist-results.ts, line 54:
<comment>Don’t fail the workflow on realtime broadcast errors after the event is already persisted; this can trigger retries that duplicate OCR patch events.</comment>
<file context>
@@ -31,30 +32,35 @@ async function appendWorkspaceEvent(
);
}
+
+ await broadcastWorkspaceEventFromServer(workspaceId, {
+ ...event,
+ version: Number(match[1]),
</file context>
Suggested change
| await broadcastWorkspaceEventFromServer(workspaceId, { | |
| ...event, | |
| version: Number(match[1]), | |
| }); | |
| void broadcastWorkspaceEventFromServer(workspaceId, { | |
| ...event, | |
| version: Number(match[1]), | |
| }).catch(() => {}); |
| }); | ||
|
|
||
| return cleanup; | ||
| }, [workspaceId, currentUser, cleanup, clientKey]); |
Contributor
There was a problem hiding this comment.
P2: Avoid depending on the entire currentUser object in the channel setup effect; it forces unnecessary channel re-subscriptions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/hooks/workspace/use-workspace-presence.ts, line 109:
<comment>Avoid depending on the entire `currentUser` object in the channel setup effect; it forces unnecessary channel re-subscriptions.</comment>
<file context>
@@ -1,123 +1,127 @@
+ });
+
+ return cleanup;
+ }, [workspaceId, currentUser, cleanup, clientKey]);
+
+ useEffect(() => {
</file context>
Suggested change
| }, [workspaceId, currentUser, cleanup, clientKey]); | |
| }, [workspaceId, currentUserId, cleanup, clientKey]); |
| useEffect(() => { | ||
| if (!workspaceId || !currentUser || !clientKey) { | ||
| cleanup(); | ||
| return; |
Contributor
There was a problem hiding this comment.
P2: Clear collaborator state when presence is disabled to avoid showing stale users until the next sync.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/hooks/workspace/use-workspace-presence.ts, line 61:
<comment>Clear collaborator state when presence is disabled to avoid showing stale users until the next sync.</comment>
<file context>
@@ -1,123 +1,127 @@
+ useEffect(() => {
+ if (!workspaceId || !currentUser || !clientKey) {
+ cleanup();
+ return;
+ }
+
</file context>
Comment on lines
+470
to
+473
| await broadcastWorkspaceEventFromServer(id, { | ||
| ...event, | ||
| version: appendResult.version, | ||
| }); |
Contributor
There was a problem hiding this comment.
P2: The broadcast is awaited, adding Supabase round-trip latency to every successful POST response even though the event is already persisted. Additionally, an unhandled error from removeChannel in the finally block of broadcastWorkspaceEventFromServer could cause a 500. Consider making this fire-and-forget like checkAndCreateSnapshot above.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/app/api/workspaces/[id]/events/route.ts, line 470:
<comment>The broadcast is `await`ed, adding Supabase round-trip latency to every successful POST response even though the event is already persisted. Additionally, an unhandled error from `removeChannel` in the `finally` block of `broadcastWorkspaceEventFromServer` could cause a 500. Consider making this fire-and-forget like `checkAndCreateSnapshot` above.</comment>
<file context>
@@ -386,10 +460,18 @@ async function handlePOST(
// Don't fail the request if snapshot creation fails
});
+ await broadcastWorkspaceEventFromServer(id, {
+ ...event,
+ version: appendResult.version,
</file context>
Suggested change
| await broadcastWorkspaceEventFromServer(id, { | |
| ...event, | |
| version: appendResult.version, | |
| }); | |
| broadcastWorkspaceEventFromServer(id, { | |
| ...event, | |
| version: appendResult.version, | |
| }).catch((err) => { | |
| console.error( | |
| `[POST /api/workspaces/${id}/events] Failed to broadcast event:`, | |
| err, | |
| ); | |
| }); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
src/components/assistant-ui/ModelPicker.tsx (1)
1-322: 🛠️ Refactor suggestion | 🟠 MajorSplit this component file to meet the 300-line limit.
src/components/assistant-ui/ModelPicker.tsxis 322 lines, which violates the repository file-size rule. Please extract parts (e.g., model config data and/or dropdown item rendering) into separate modules/components.As per coding guidelines,
**/*.{js,jsx,ts,tsx}: Enforce a maximum of 300 lines per file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/ModelPicker.tsx` around lines 1 - 322, This file exceeds the 300-line limit; split it by moving static data and smaller components into new modules: extract MODEL_PROVIDERS, ALL_MODELS and getModelDisplayName into a new module (e.g., model-data) and export them, and extract ModelProviderIcon and ModelDropdownItem into a new component file (e.g., ModelDropdownItem.tsx) that exports these components; then update ModelPicker to import { MODEL_PROVIDERS, ALL_MODELS, getModelDisplayName } and ModelProviderIcon/ModelDropdownItem, keeping ModelPicker, its useState/useMemo logic, and the DropdownMenu markup in this file and ensuring setSelectedModelId, hoveredModelId state and event handlers remain unchanged. Ensure all exported types (ModelConfig, ModelProvider) used by the new files are exported/imported so types align and rebuild succeeds.src/components/assistant-ui/AssistantTextSelectionManager.tsx (1)
1-1274:⚠️ Potential issue | 🟠 MajorFile exceeds the repository max-size rule (1274 > 300 lines).
This size makes the component difficult to reason about and directly violates the project constraint. Please split this into focused units (e.g., selection tracking hook, marker lifecycle hook, reply/document action handlers, and dialog subcomponents).
As per coding guidelines:
**/*.{js,jsx,ts,tsx}: Enforce a maximum of 300 lines per file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/AssistantTextSelectionManager.tsx` around lines 1 - 1274, The component AssistantTextSelectionManager is over the 300-line limit; split it into focused units: extract selection tracking logic (handleSelectionChange, handleHighlightsChange, SelectableText wiring) into a useSelectionTracking hook, move marker lifecycle (createMarkerElementAtRange, removeMarkerElement, related refs and scroll handling) into a useMarkerLifecycle hook, move action handlers (handleMultiSelect, handleReply, handleCreateDocument, handleSubmitMultiSelections, clearAllHighlights, handleShitAction/handleSubmitNegativeFeedback) into a useSelectionActions hook that accepts refs/state and exposes handlers, and factor the two dialogs and HighlightTooltip usage into small subcomponents (MultiSelectDialog, NegativeFeedbackDialog, HighlightTooltipContainer) that receive props/state; update AssistantTextSelectionManager to import and compose these hooks and subcomponents, keeping only high-level state and wiring (tooltipVisible, inMultiMode, highlights refs) so the top-level file stays under 300 lines.src/hooks/workspace/use-workspace-mutation.ts (1)
1-481:⚠️ Potential issue | 🟡 MinorFile exceeds 300-line limit (481 lines).
As per coding guidelines, files should have a maximum of 300 lines. This file is 60% over the limit. Consider extracting:
appendWorkspaceEventfunction (lines 22-58) → separate module- Version calculation logic (lines 84-119) →
calculateBaseVersionhelper- Retry logic in
onSuccess(lines 257-425) →handleConflictRetryhelper- OCR-specific logging (lines 222-255) → debug utility
Example extraction for retry logic
// src/hooks/workspace/helpers/conflict-retry.ts export async function handleConflictRetry( queryClient: QueryClient, workspaceId: string, event: WorkspaceEvent, retryAttemptsRef: MutableRefObject<Map<string, number>>, appendWorkspaceEvent: typeof appendWorkspaceEvent, ): Promise<void> { // ... retry logic extracted here }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/workspace/use-workspace-mutation.ts` around lines 1 - 481, This file is over the 300-line limit; extract the large logical pieces into helpers to reduce size: move appendWorkspaceEvent (the HTTP POST logic) into its own module (keep the same function name and return shape so existing callers work); extract the version calculation block (the code that computes currentVersion, maxEventVersion, optimisticEventsCount and adjustedBaseVersion) into a calculateBaseVersion(workspaceEvents, cacheVersion, ownEventId?) helper and call it from the mutationFn; extract the long retry flow inside onSuccess into handleConflictRetry(queryClient, workspaceId, event, retryAttemptsRef, appendWorkspaceEvent) that encapsulates the invalidate/refetch/remove-reapply/retry/update/delete logic; and pull the OCR-specific logging into a small utility (e.g., logOcrUpdates(event, version)) to keep onSuccess concise; after extracting, update imports/exports and ensure useWorkspaceMutation calls the new helpers with the same symbols so behavior and types are preserved.src/app/api/workspaces/[id]/events/route.ts (1)
1-488:⚠️ Potential issue | 🟡 MinorFile exceeds 300-line limit (488 lines).
As per coding guidelines, files should have a maximum of 300 lines. Consider extracting:
- OCR stripping helpers (lines 6-58) →
@/lib/workspace/ocr-strip.ts- Event transformation logic →
@/lib/workspace/event-transform.ts- Pagination logic in GET → separate helper function
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/workspaces/`[id]/events/route.ts around lines 1 - 488, The file is too large; extract the OCR helpers (stripOcrPagesFromItem, stripOcrPagesFromState, stripPdfOcrFromEventPayload) into a new module (e.g., export functions from "@/lib/workspace/ocr-strip") and replace their definitions in this file with imports; similarly move the event transformation logic (the mapping that builds WorkspaceEvent objects and computes maxVersion — the code around eventsData.map(...) and maxVersion calculation) into "@/lib/workspace/event-transform" with exported helpers (e.g., transformDbEventsToWorkspaceEvents, getMaxEventVersion) and import them in handleGET/handlePOST; finally extract the pagination/db-fetch loop used when eventCount > PAGE_SIZE into a helper (e.g., fetchWorkspaceEventsPaginated) and call it from handleGET. Update imports and ensure all moved functions keep the same names used here (stripOcrPagesFromItem, stripOcrPagesFromState, stripPdfOcrFromEventPayload, transformDbEventsToWorkspaceEvents, getMaxEventVersion, fetchWorkspaceEventsPaginated) so callers require minimal changes.src/hooks/workspace/use-workspace-operations.ts (2)
1-1188: 🛠️ Refactor suggestion | 🟠 MajorSplit this hook before adding more behavior.
At 1,188 lines, this module is now mixing creation flows, folder operations, debounce state, cache replay, and export helpers in one place. Please extract a few focused hooks/modules here before this gets harder to reason about. As per coding guidelines,
**/*.{js,jsx,ts,tsx}: Enforce a maximum of 300 lines per file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/workspace/use-workspace-operations.ts` around lines 1 - 1188, The file is too large and conflates multiple responsibilities; split useWorkspaceOperations into focused modules: 1) extract item creation logic (functions createItem, createItems, plus helpers like defaultDataFor/getRandomCardColor/findNextAvailablePosition) into a useItemCreation hook; 2) extract update/debounce logic (updateItem, updateItemData, flushPendingChanges, pending refs) into a useItemUpdates hook; 3) extract bulk and layout operations (updateAllItems, getLatestItemFromState, getLatestItemWithPendingChanges, and cache replay usage) into a useBulkLayout hook; 4) extract folder behavior (createFolder, createFolderWithItems, updateFolder, deleteFolder, deleteFolderWithContents, getAllDescendantIds, moveItemToFolder, moveItemsToFolder, filterItemIdsForFolderCreation) into a useFolderOperations hook; ensure each new hook accepts the shared dependencies (mutation from useWorkspaceMutation, queryClient, workspaceId, currentState, userId/userName) and re-export only the original API from a slim useWorkspaceOperations that composes these hooks. Update imports/exports accordingly and remove duplicated logic so each file stays under 300 lines.
841-906:⚠️ Potential issue | 🟠 MajorInclude
currentState.itemsin this callback’s dependencies.
safeItemIdsis derived fromcurrentState.itemson Lines 847-851, but this memoized callback is only recreated formutation,userId, anduserName. After the workspace tree changes,createFolderWithItemscan keep validating against an old graph and miss the cycle filter.🔧 Minimal fix
- [mutation, userId, userName], + [mutation, userId, userName, currentState.items],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/workspace/use-workspace-operations.ts` around lines 841 - 906, The createFolderWithItems useCallback currently captures currentState.items when computing safeItemIds via filterItemIdsForFolderCreation but does not include currentState.items in its dependency array, causing stale cycle checks; update the dependencies of createFolderWithItems to include the array (or a stable reference) representing currentState.items so the callback is recreated when the workspace tree changes (i.e., add currentState.items or a derived stable key to the dependency list alongside mutation, userId, userName) to ensure filterItemIdsForFolderCreation runs against the latest graph.src/components/assistant-ui/CreateFlashcardToolUI.tsx (1)
1-304: 🛠️ Refactor suggestion | 🟠 MajorSplit this file to satisfy the repository max-file-length rule.
This file is 304 lines, exceeding the 300-line limit.
A practical split is to move
CreateFlashcardReceipt(and/or debug logging helpers) into a sibling module.As per coding guidelines
**/*.{js,jsx,ts,tsx}: Enforce a maximum of 300 lines per file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/CreateFlashcardToolUI.tsx` around lines 1 - 304, The file exceeds the 300-line limit—extract the CreateFlashcardReceipt component (and any tightly-coupled debug helpers) into a new sibling module and import it back into this file; specifically move the CreateFlashcardReceipt function, its CreateFlashcardReceiptProps type and related useEffect logging into a new CreateFlashcardReceipt.tsx, export the component, then update the current file to import CreateFlashcardReceipt and keep the remaining CreateFlashcardToolUI render function (which still uses parseFlashcardResult, operations.moveItemToFolder, and CreateFlashcardReceipt) untouched; ensure all referenced symbols (CreateFlashcardReceipt, CreateFlashcardReceiptProps, CreateFlashcardToolUI, parseFlashcardResult) are exported/imported correctly and run typechecks to fix any missing imports or prop typing issues.src/components/onboarding/WorkspaceInstructionModal.tsx (2)
323-330:⚠️ Potential issue | 🟠 MajorKeep the Close button consistent with the generating guard.
Lines 323-330 block Escape while
isGenerating, but Lines 533-541 still render a live Close button whenevercanCloseis true. If that prop combination occurs, the modal can still be closed mid-generation.🔒 Suggested guard
- {canClose && ( + {canClose && !isGenerating && ( <button type="button" - onClick={onRequestClose} + onClick={() => { + if (isGenerating) return; + onRequestClose?.(); + }} className="absolute right-5 top-1/2 -translate-y-1/2 inline-flex items-center gap-1.5 rounded-full px-4 py-1.5 text-sm font-medium bg-white/25 dark:bg-white/10 backdrop-blur-md border border-white/20 dark:border-white/[0.08] text-sidebar-foreground hover:bg-white/35 dark:hover:bg-white/15 shadow-[0_2px_8px_rgba(0,0,0,0.06),inset_0_1px_0_rgba(255,255,255,0.3)] dark:shadow-[0_2px_8px_rgba(0,0,0,0.2),inset_0_1px_0_rgba(255,255,255,0.06)] transition-all duration-200" >Also applies to: 533-541
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/onboarding/WorkspaceInstructionModal.tsx` around lines 323 - 330, The Escape key handler (handleKeyDown) prevents closing while isGenerating, but the Close button rendered in the modal still allows closing when canClose is true; update the Close button’s render/handler to honor the same guard: either disable/hide the Close button when isGenerating or make its click handler check isGenerating and early-return before calling onRequestClose, and keep using canClose as an additional condition; ensure the Close button’s JSX and its onClick reference the same flags (isGenerating, canClose, onRequestClose) so closing cannot occur mid-generation.
117-548: 🛠️ Refactor suggestion | 🟠 MajorSplit this component to get back under the 300-line file limit.
At 548 lines, this file is well past the repo cap.
useCarousel, the banner sections, and/or the step config are good extraction points.As per coding guidelines,
**/*.{js,jsx,ts,tsx}: Enforce a maximum of 300 lines per file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/onboarding/WorkspaceInstructionModal.tsx` around lines 117 - 548, The file exceeds the 300-line limit; split out logical parts to reduce file size by extracting useCarousel, the generation/generation-complete banner UI, and the STEPS config into separate modules. Create a new hook file exporting useCarousel (keeping all refs, state, and returned handlers: activeIndex, step, videoSrc, fading, videoLoaded, goTo, goPrev, goNext, handleVideoEnded, handleVideoCanPlay, pause) and import it into WorkspaceInstructionModal; move the generation banner JSX (the isGenerating block and the generationComplete block) into a small Presentation component (e.g., GenerationBanner) that accepts props (isGenerating, progressText, completedSteps, totalSteps, generationComplete, workspaceSlug, onOpenWorkspace) and replace the inline markup with that component; extract STEPS (and any related constants like FADE_MS, ICON_SLIDE_MS) into a separate steps/config module and import it where needed. Ensure all exported/imported names match (useCarousel, GenerationBanner, STEPS, FADE_MS, ICON_SLIDE_MS) and update imports in WorkspaceInstructionModal so behavior and prop contracts remain identical.
🧹 Nitpick comments (12)
src/components/editor/document-editor.scss (1)
1-1: Consider removing the unused DM Sans font import.The
font-family: "DM Sans"rule was removed from.tiptap.ProseMirror, but the Google Fonts import on line 1 still loads DM Sans. If the font is no longer used for the editor, this import adds unnecessary network overhead (~15-50KB depending on weights loaded).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/editor/document-editor.scss` at line 1, The Google Fonts import includes DM Sans which is no longer used by the editor (.tiptap.ProseMirror), so remove the unused font to avoid extra network weight: either delete the entire `@import` if no other code needs Google Fonts, or replace the URL with one that only loads the Inter family (remove "DM+Sans" from the import string) so that .tiptap.ProseMirror and other components continue using Inter without loading DM Sans.src/app/api/chat/route.ts (1)
408-415: Consider adding type annotation for providerMetadata.The
anytype in the Promise callback reduces type safety.🔧 Suggested type annotation
- void Promise.resolve((result as any).providerMetadata).then((meta: any) => { - const provider = - meta?.gateway?.routing?.resolvedProvider ?? - meta?.gateway?.routing?.finalProvider; + void Promise.resolve((result as any).providerMetadata).then((meta: { gateway?: { routing?: { resolvedProvider?: string; finalProvider?: string } } } | undefined) => { + const provider = meta?.gateway?.routing?.resolvedProvider ?? meta?.gateway?.routing?.finalProvider;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/chat/route.ts` around lines 408 - 415, The callback is using (result as any).providerMetadata which loses type safety; add a proper type for providerMetadata (e.g., an interface/type like ProviderMetadata with gateway.routing.resolvedProvider and finalProvider fields) and annotate the Promise.resolve cast or the result variable accordingly (for example use result as { providerMetadata?: ProviderMetadata } or narrow result to unknown and assert/validate its shape) so the callback signature becomes (meta: ProviderMetadata | undefined) and accesses provider safely when present; update references to provider, providerMetadata, and the Promise.resolve call in the same block to use the new type.src/hooks/workspace/use-workspace-realtime.ts (1)
197-205: Effect dependency array includescleanupbut doesn't use it.The
cleanupfunction is in the dependency array but not called within the effect. The effect uses its own return cleanup function instead. Consider removingcleanupfrom dependencies or using it consistently:Suggested cleanup
}, [ workspaceId, queryClient, - cleanup, onStatusChange, onRemoteEvent, reconnectNonce, removeChannel, ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/workspace/use-workspace-realtime.ts` around lines 197 - 205, The effect dependency array for the useWorkspaceRealtime hook incorrectly includes the cleanup identifier even though the effect body does not call cleanup; remove cleanup from the dependency list (leaving workspaceId, queryClient, onStatusChange, onRemoteEvent, reconnectNonce, removeChannel) so the effect dependencies reflect actual usage, or alternatively, if you intended to reuse the external cleanup function, call cleanup inside the effect (and ensure its behavior/identity is stable) and keep it in the array; update the dependency array accordingly to match whichever approach you choose.src/app/api/workspaces/[id]/events/route.ts (2)
262-268: Replaceanywith proper typing for snapshot state.ESLint flags
state: latestSnapshot.state as any. Consider using a typed state interface:Suggested fix
+import type { AgentState } from "@/lib/workspace/events"; snapshot: latestSnapshot && typeof latestSnapshot.snapshotVersion === "number" ? { version: latestSnapshot.snapshotVersion, - state: latestSnapshot.state as any, + state: latestSnapshot.state as AgentState, } : undefined,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/workspaces/`[id]/events/route.ts around lines 262 - 268, The snapshot object is casting latestSnapshot.state to any; replace this with a concrete type by defining or importing the proper SnapshotState interface and use it instead of any when building the snapshot object (e.g., type SnapshotState = /* shape */ and then set state: latestSnapshot.state as SnapshotState). Locate the code that constructs the snapshot (references: latestSnapshot, snapshotVersion, state) in route.ts and update the type for the snapshot state, or adjust the latestSnapshot type so latestSnapshot.state already matches SnapshotState to remove the cast.
112-120: Replaceanywith proper typing.ESLint flags the
anytype. Define a proper interface for the snapshot result:Suggested fix
+interface LatestSnapshotResult { + id?: string; + snapshotVersion?: number; + state?: Record<string, unknown>; + eventCount?: number; + createdAt?: string; +} - const latestSnapshot = latestSnapshotData[0] as - | { - id?: string; - snapshotVersion?: number; - state?: any; - eventCount?: number; - createdAt?: string; - } - | undefined; + const latestSnapshot = latestSnapshotData[0] as LatestSnapshotResult | undefined;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/workspaces/`[id]/events/route.ts around lines 112 - 120, The current inline union uses `any` for `state` which ESLint disallows; define a proper interface (e.g., SnapshotRow or LatestSnapshotResult) with typed properties for id?: string, snapshotVersion?: number, state: Record<string, unknown> | SpecificStateType, eventCount?: number, createdAt?: string, declare it near the top of the file or next to related types, then replace the inline union used for `latestSnapshot` (and any other uses of `latestSnapshotData`) to use this new interface instead of `any`; if `state` can be multiple shapes, use a discriminated union or Record<string, unknown> to keep typing strict.src/contexts/RealtimeContext.tsx (1)
46-52: Minor: Consider keepinguseMemoforcurrentUser.While the inline conditional works, wrapping
currentUserinuseMemo(keyed onsession?.user) would prevent unnecessary re-renders ofuseWorkspacePresencewhen other session properties change. This is a minor optimization concern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/contexts/RealtimeContext.tsx` around lines 46 - 52, Wrap the currentUser object creation in a React useMemo keyed on session?.user to avoid recreating the object when unrelated session fields change; replace the inline conditional assignment for currentUser with a memoized value (useMemo(() => session?.user ? { id: session.user.id, name: session.user.name || "Anonymous", image: session.user.image ?? undefined } : null, [session?.user])) so useWorkspacePresence receives a stable reference and unnecessary re-renders are prevented.src/lib/realtime/server-broadcast.ts (2)
47-64: Channel created per-broadcast may impact performance at scale.Each
broadcastWorkspaceEventFromServercall creates a new channel, sends one message, and immediately removes it. For high-frequency workspace updates, consider pooling/reusing channels per workspace or batching broadcasts.That said, this is likely acceptable for current usage patterns and can be optimized later if needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/realtime/server-broadcast.ts` around lines 47 - 64, The current broadcastWorkspaceEventFromServer implementation creates and removes a Supabase channel for every call (creating channel via supabase.channel(...) then send and removeChannel), which can degrade performance; modify the logic to pool/reuse channels per workspaceId (e.g., maintain a Map keyed by workspaceId storing active channels), have broadcastWorkspaceEventFromServer look up or create-and-cache the channel, send on the cached channel, and only removeChannel and delete from the Map on explicit teardown (process exit or when no longer needed), ensuring error handling/logging around send remains in place.
16-18: Service role key fallback may have unintended security implications.Falling back from
SUPABASE_SERVICE_ROLE_KEYtoNEXT_PUBLIC_SUPABASE_ANON_KEYseems intentional for development flexibility, but the service role key bypasses RLS. Consider logging which key type is in use or making the fallback behavior explicit in documentation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/realtime/server-broadcast.ts` around lines 16 - 18, The current supabaseKey assignment silently falls back from SUPABASE_SERVICE_ROLE_KEY to NEXT_PUBLIC_SUPABASE_ANON_KEY; update the logic around the supabaseKey variable so the code explicitly logs which key type is being used (service role vs anon) and only allow the anon fallback in non-production (e.g., NODE_ENV !== 'production'), or alternatively throw/require configuration in production; reference the supabaseKey variable and the env names SUPABASE_SERVICE_ROLE_KEY and NEXT_PUBLIC_SUPABASE_ANON_KEY and add a concise log or warning (using the existing logger or console) explaining the chosen key so the behavior is explicit.src/workflows/ocr-dispatch/steps/persist-results.ts (1)
11-58: Good extraction ofappendWorkspaceEventhelper.This helper consolidates version checking, event appending, and broadcasting. Consider moving this to a shared location (e.g.,
@/lib/workspace/event-appender.ts) sosrc/workflows/audio-transcribe/steps/persist-result.tscan reuse it and reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/ocr-dispatch/steps/persist-results.ts` around lines 11 - 58, The appendWorkspaceEvent helper is duplicated and should be extracted to a shared module so other workflows can reuse it; create a new module (e.g., event-appender) exporting the async function appendWorkspaceEvent(workspaceId: string, event: WorkspaceEvent) that contains the existing logic (get_workspace_version, append_workspace_event call, result parsing, conflict handling, and broadcastWorkspaceEventFromServer call), update callers such as the persist-result step in the audio-transcribe workflow to import and use this exported appendWorkspaceEvent, and ensure any helper constants (APPEND_RESULT_REGEX) and types (WorkspaceEvent) used by appendWorkspaceEvent are moved or re-exported so imports remain correct.src/workflows/audio-transcribe/steps/persist-result.ts (1)
38-76: Consider extracting shared append-and-broadcast logic.Both
persistAudioResultandpersistAudioFailurecontain nearly identical logic for:
- Getting workspace version
- Appending event via SQL
- Parsing result
- Broadcasting on success
This duplicates similar patterns in
src/workflows/ocr-dispatch/steps/persist-results.ts. Consider extracting a shared helper (e.g., in@/lib/workspace/) to reduce duplication and ensure consistent behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/audio-transcribe/steps/persist-result.ts` around lines 38 - 76, Both persistAudioResult and persistAudioFailure duplicate the workspace append-and-broadcast flow (get_workspace_version, append_workspace_event SQL call, parse the "(version,modified)" result, throw on unexpected format or version conflict, call broadcastWorkspaceEventFromServer, then trigger checkAndCreateSnapshot); extract this into a shared helper (e.g., appendAndBroadcastWorkspaceEvent in a new module under `@/lib/workspace/`) that accepts (workspaceId, event, db, processLogger) and returns the new version or throws on conflict/unexpected format, encapsulates the SQL calls and parsing logic (the same regex and error messages), calls broadcastWorkspaceEventFromServer with the enriched event, and invokes checkAndCreateSnapshot; update persistAudioResult and persistAudioFailure to call this helper instead of duplicating the logic so behavior is consistent with src/workflows/ocr-dispatch/steps/persist-results.ts.src/hooks/workspace/use-workspace-mutation.ts (1)
290-425: Deeply nested.then()chains reduce readability.The conflict retry logic uses multiple levels of
.then()nesting. Consider refactoring toasync/awaitpattern for cleaner control flow:Suggested refactor to async/await
- queryClient - .invalidateQueries({ - queryKey: ["workspace", workspaceId, "events"], - }) - .then(() => { - // ... nested logic - appendWorkspaceEvent({...}) - .then((retryResult) => { - // ... more nesting - }) - .catch((err) => { - // ... error handling - }); - }); + try { + await queryClient.invalidateQueries({ + queryKey: ["workspace", workspaceId, "events"], + }); + + // Re-apply optimistic update + queryClient.setQueryData<EventResponse>(/*...*/); + + const retryResult = await appendWorkspaceEvent({ + workspaceId, + event, + baseVersion: currentVersion, + }); + + if (retryResult.conflict) { + // Handle still-conflicting case + } else { + // Handle success + } + } catch (err) { + // Consolidated error handling + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/workspace/use-workspace-mutation.ts` around lines 290 - 425, The nested .then() chains in the conflict retry block make the flow hard to follow; refactor the logic around queryClient.invalidateQueries(...) and the subsequent appendWorkspaceEvent(...) call into an async/await helper (e.g., create an async function like retryAppendEvent or handleConflictRetry) that awaits queryClient.invalidateQueries, re-applies the optimistic update via queryClient.setQueryData, computes currentVersion from queryClient.getQueryData, then awaits appendWorkspaceEvent({ workspaceId, event, baseVersion: currentVersion }) and handles success, conflict, and catch paths (updating events, removing optimistic event, cleaning retryAttemptsRef.current.delete(event.id), and forcing invalidateQueries) using sequential try/catch rather than nested .then() chains; update callers to await the helper or call it without nesting so control flow around appendWorkspaceEvent, queryClient.setQueryData, and retryAttemptsRef is linear and readable.src/components/assistant-ui/CreateQuizToolUI.tsx (1)
74-80: SimplifyfolderNamederivation to avoid React compiler memoization skip.The current
useMemodependency pattern is being rejected by the React hooks rule; this is a good candidate for direct derivation (or adjusted dependencies) to restore compiler optimization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/CreateQuizToolUI.tsx` around lines 74 - 80, The folderName derivation uses useMemo with unstable dependency expressions causing the React hooks rule to reject it; replace the useMemo block with a plain derived value computed on render (or, if memoizing is required, move to useMemo with stable dependencies like currentItem?.folderId and workspaceState?.items without optional chaining in the deps). Specifically, remove useMemo around folderName and compute it directly from currentItem and workspaceState (use workspaceState.items.find(item => item.id === currentItem.folderId)?.name ?? null) so the value is stable and the hooks linter won't complain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/assistant-ui/AssistantTextSelectionManager.tsx`:
- Around line 360-397: handleSelectionChange and handleKeyDown both read
multiModeBasePosition but don’t list it in their dependency arrays, causing
stale closures; update the dependency arrays for the useCallback hooks for
handleSelectionChange and handleKeyDown to include multiModeBasePosition (and
any related refs/values like lastTooltipPositionRef if used as dependencies) so
the callbacks capture the current multiModeBasePosition when calling
setTooltipVisible, setTooltipPosition, window.getSelection()?.removeAllRanges(),
and when checking useUIStore.getState().inMultiSelectMode.
- Around line 142-147: The file AssistantTextSelectionManager.tsx is too large
and should be split into smaller modules (e.g., selection logic, UI hooks, and
utilities) to meet the 300-line limit; refactor by extracting related functions
and components into new files and importing them back into the main component.
While splitting, remove the unsafe casts in the useAuiState selectors: replace
(threadListItem as any)?.id and (threads as any)?.mainThreadId by referencing
the correctly typed slice of state (define/consume the proper interface or typed
selector for the AUI state) so useAuiState(({ threadListItem }: AuiState) =>
threadListItem?.id) and useAuiState(({ threads }: AuiState) =>
threads?.mainThreadId) (or equivalent typed helper selectors) to restore type
safety for threadListItemId and mainThreadId.
In `@src/components/assistant-ui/CreateDocumentToolUI.tsx`:
- Around line 221-230: The CreateDocumentToolUI currently doesn't handle the
case where status.type === "complete" but the run returned parsed.success ===
false, leaving content null and showing a blank UI; update the render logic in
CreateDocumentToolUI to add an explicit branch for status.type === "complete" &&
parsed && parsed.success === false that sets content to a ToolUIErrorShell
(e.g., label "Failed to create document" and message from parsed.message),
mirroring the existing incomplete/error handling so users see an error UI
instead of a blank screen.
In `@src/components/assistant-ui/CreateFlashcardToolUI.tsx`:
- Around line 178-189: The Move button's onClick allows the click to bubble and
trigger the parent row navigation; update the onClick handler for the Button
(the one checking currentItem, calling toast.error and setShowMoveDialog) to
accept the click event (e.g., (e) or (event)) and immediately call
e.stopPropagation() (and optionally e.preventDefault()) before the existing
logic so the parent row click isn't fired when clicking Move; ensure the event
parameter type matches React.MouseEvent if needed.
- Around line 42-51: The CreateFlashcardReceiptProps interface uses broad any
types and the Move button handler lets clicks bubble up; replace status:any with
a specific union or a shared RequestStatus type (e.g., 'idle' | 'loading' |
'success' | 'error' or your app's RequestStatus), replace allItems?: any[] with
a typed array (e.g., allItems?: Item[] or Array<FlashcardItem> with
Item/FlashcardItem imported or declared), and change moveItemToFolder signature
to accept concrete types (moveItemToFolder?: (itemId: string, folderId: string |
null) => void is fine but ensure Item types align). Also edit the
CreateFlashcardReceipt component's Move button onClick handler (where move click
is invoked) to accept the event and call e.stopPropagation() before invoking
moveItemToFolder to prevent triggering the parent navigation handler. Ensure you
add or import the Item/RequestStatus types and adjust the interface accordingly
to remove any usages.
In `@src/components/assistant-ui/CreateQuizToolUI.tsx`:
- Around line 153-164: The Move button's onClick handler in CreateQuizToolUI is
missing event propagation control so the parent onClick (the card/container
click) also fires; update the Button's onClick to accept the click event (e:
React.MouseEvent) and call e.stopPropagation() (and e.preventDefault() if
desired) before the existing logic that checks currentItem and calls
setShowMoveDialog(true) to prevent parent navigation when Move is clicked.
- Around line 26-33: The props and callback in CreateQuizToolUI use `any` and
should be strongly typed: import Item from "@/lib/workspace-state/types" and
change the props signature so `status: { type: "complete" | "running" |
"incomplete"; reason?: string }`, `allItems: Item[]`, and update any
iterator/handler like the `(item: any)` callback (e.g., in map/filter or
moveItemToFolder usage) to `(item: Item)`; ensure function signatures such as
`moveItemToFolder?: (itemId: string, folderId: string | null) => void` remain
unchanged and update any local references to use the Item type instead of any.
In `@src/components/onboarding/WorkspaceInstructionModal.tsx`:
- Around line 362-365: The click handlers on the chevrons/dot buttons call
onUserInteracted in addition to the wrapper, causing duplicate firings; update
the onClick handlers (the ones currently written as onClick={() => { pause();
onUserInteracted?.(); }} in the WorkspaceInstructionModal component, including
the handlers that reference pause and onUserInteracted) to only call pause()
(remove the onUserInteracted?.() invocation) so the wrapper’s single
onUserInteracted call handles interaction notification; alternatively, if you
prefer stopping event bubbling, call event.stopPropagation() at the start of
those inner onClick handlers and then only call pause(), but do not call
onUserInteracted there.
In `@src/hooks/workspace/use-workspace-operations.ts`:
- Around line 453-492: The timeout handler is using the render-captured
currentState.items which can be stale; change it to resolve
pendingItemChangesRef.current.get(id) against the latest cached/queried items
state the same way updateItemData/updateAllItems do (e.g., read the up-to-date
items from the cache or shared ref used by updateItemData/updateAllItems) before
running hasDuplicateName, computing newName/newType/folderId, and before
creating the event via createEvent and calling mutation.mutate; ensure you
replace uses of currentState.items with that live cached state so duplicate
checks and the name attached to ITEM_UPDATED reflect the most recent merged
state and still clean up pendingItemChangesRef and updateItemDebounceRef on
early returns.
- Around line 364-399: The placeholder push uses an any escape hatch (data:
baseData as any); instead, construct the fully-typed Item object (use the same
shape as newItem: id, type, name, subtitle, data: baseData typed as ItemData,
color/folderId/layout set as needed) before the layout calculation branch and
then push that typed Item into itemsForLayout when initialLayout is present;
remove the "as any" cast and ensure the pushed object matches the Item interface
so TypeScript/ESLint no-explicit-any errors are resolved (reference:
itemsForLayout, findNextAvailablePosition, newItem, baseData, Item, ItemData,
layout).
- Around line 940-955: The file is too large and the recursive useCallback
getAllDescendantIds creates a recursive closure over an uninitialized binding;
extract the recursion into a named function (e.g., function
getAllDescendantIdsImpl(folderId, items) or a named function expression) and
have the useCallback (getAllDescendantIds) simply call that named function to
avoid the self-reference problem, then move this helper into a new utilities
module (or a smaller helper file) and import it from use-workspace-operations to
shrink the file under 300 lines; update all callers to use the exported
getAllDescendantIds helper or the thin useCallback wrapper which delegates to
the named implementation.
In `@src/hooks/workspace/use-workspace-presence.ts`:
- Around line 111-122: The second useEffect calling channelRef.current.track
when currentUser/clientKey change can race with the channel recreation cleanup
in the first effect; modify the code so tracking happens only after the channel
is fully subscribed — either move the track call into the first effect’s channel
subscription callback (where the channel is created/confirmed) or add a guard
that verifies the channel is active (e.g., channelRef.current exists and is in a
"subscribed" state or await channel.subscribe()) before calling track; update
references to channelRef, joinedAtRef, currentUser, clientKey and the existing
subscription/callback in the first effect accordingly so track never runs
against a stale/null channel.
In `@src/lib/ai/tools/index.ts`:
- Around line 27-40: The ChatToolsConfig.clientTools and createChatTools return
type use `any`; change `clientTools?: Record<string, any>` to `clientTools?:
Record<string, unknown>` and update the function signature return type from
`Record<string, any>` to `Record<string, unknown>` so callers must perform type
guards; update references to `ChatToolsConfig` and `createChatTools` accordingly
(no other logic changes).
---
Outside diff comments:
In `@src/app/api/workspaces/`[id]/events/route.ts:
- Around line 1-488: The file is too large; extract the OCR helpers
(stripOcrPagesFromItem, stripOcrPagesFromState, stripPdfOcrFromEventPayload)
into a new module (e.g., export functions from "@/lib/workspace/ocr-strip") and
replace their definitions in this file with imports; similarly move the event
transformation logic (the mapping that builds WorkspaceEvent objects and
computes maxVersion — the code around eventsData.map(...) and maxVersion
calculation) into "@/lib/workspace/event-transform" with exported helpers (e.g.,
transformDbEventsToWorkspaceEvents, getMaxEventVersion) and import them in
handleGET/handlePOST; finally extract the pagination/db-fetch loop used when
eventCount > PAGE_SIZE into a helper (e.g., fetchWorkspaceEventsPaginated) and
call it from handleGET. Update imports and ensure all moved functions keep the
same names used here (stripOcrPagesFromItem, stripOcrPagesFromState,
stripPdfOcrFromEventPayload, transformDbEventsToWorkspaceEvents,
getMaxEventVersion, fetchWorkspaceEventsPaginated) so callers require minimal
changes.
In `@src/components/assistant-ui/AssistantTextSelectionManager.tsx`:
- Around line 1-1274: The component AssistantTextSelectionManager is over the
300-line limit; split it into focused units: extract selection tracking logic
(handleSelectionChange, handleHighlightsChange, SelectableText wiring) into a
useSelectionTracking hook, move marker lifecycle (createMarkerElementAtRange,
removeMarkerElement, related refs and scroll handling) into a useMarkerLifecycle
hook, move action handlers (handleMultiSelect, handleReply,
handleCreateDocument, handleSubmitMultiSelections, clearAllHighlights,
handleShitAction/handleSubmitNegativeFeedback) into a useSelectionActions hook
that accepts refs/state and exposes handlers, and factor the two dialogs and
HighlightTooltip usage into small subcomponents (MultiSelectDialog,
NegativeFeedbackDialog, HighlightTooltipContainer) that receive props/state;
update AssistantTextSelectionManager to import and compose these hooks and
subcomponents, keeping only high-level state and wiring (tooltipVisible,
inMultiMode, highlights refs) so the top-level file stays under 300 lines.
In `@src/components/assistant-ui/CreateFlashcardToolUI.tsx`:
- Around line 1-304: The file exceeds the 300-line limit—extract the
CreateFlashcardReceipt component (and any tightly-coupled debug helpers) into a
new sibling module and import it back into this file; specifically move the
CreateFlashcardReceipt function, its CreateFlashcardReceiptProps type and
related useEffect logging into a new CreateFlashcardReceipt.tsx, export the
component, then update the current file to import CreateFlashcardReceipt and
keep the remaining CreateFlashcardToolUI render function (which still uses
parseFlashcardResult, operations.moveItemToFolder, and CreateFlashcardReceipt)
untouched; ensure all referenced symbols (CreateFlashcardReceipt,
CreateFlashcardReceiptProps, CreateFlashcardToolUI, parseFlashcardResult) are
exported/imported correctly and run typechecks to fix any missing imports or
prop typing issues.
In `@src/components/assistant-ui/ModelPicker.tsx`:
- Around line 1-322: This file exceeds the 300-line limit; split it by moving
static data and smaller components into new modules: extract MODEL_PROVIDERS,
ALL_MODELS and getModelDisplayName into a new module (e.g., model-data) and
export them, and extract ModelProviderIcon and ModelDropdownItem into a new
component file (e.g., ModelDropdownItem.tsx) that exports these components; then
update ModelPicker to import { MODEL_PROVIDERS, ALL_MODELS, getModelDisplayName
} and ModelProviderIcon/ModelDropdownItem, keeping ModelPicker, its
useState/useMemo logic, and the DropdownMenu markup in this file and ensuring
setSelectedModelId, hoveredModelId state and event handlers remain unchanged.
Ensure all exported types (ModelConfig, ModelProvider) used by the new files are
exported/imported so types align and rebuild succeeds.
In `@src/components/onboarding/WorkspaceInstructionModal.tsx`:
- Around line 323-330: The Escape key handler (handleKeyDown) prevents closing
while isGenerating, but the Close button rendered in the modal still allows
closing when canClose is true; update the Close button’s render/handler to honor
the same guard: either disable/hide the Close button when isGenerating or make
its click handler check isGenerating and early-return before calling
onRequestClose, and keep using canClose as an additional condition; ensure the
Close button’s JSX and its onClick reference the same flags (isGenerating,
canClose, onRequestClose) so closing cannot occur mid-generation.
- Around line 117-548: The file exceeds the 300-line limit; split out logical
parts to reduce file size by extracting useCarousel, the
generation/generation-complete banner UI, and the STEPS config into separate
modules. Create a new hook file exporting useCarousel (keeping all refs, state,
and returned handlers: activeIndex, step, videoSrc, fading, videoLoaded, goTo,
goPrev, goNext, handleVideoEnded, handleVideoCanPlay, pause) and import it into
WorkspaceInstructionModal; move the generation banner JSX (the isGenerating
block and the generationComplete block) into a small Presentation component
(e.g., GenerationBanner) that accepts props (isGenerating, progressText,
completedSteps, totalSteps, generationComplete, workspaceSlug, onOpenWorkspace)
and replace the inline markup with that component; extract STEPS (and any
related constants like FADE_MS, ICON_SLIDE_MS) into a separate steps/config
module and import it where needed. Ensure all exported/imported names match
(useCarousel, GenerationBanner, STEPS, FADE_MS, ICON_SLIDE_MS) and update
imports in WorkspaceInstructionModal so behavior and prop contracts remain
identical.
In `@src/hooks/workspace/use-workspace-mutation.ts`:
- Around line 1-481: This file is over the 300-line limit; extract the large
logical pieces into helpers to reduce size: move appendWorkspaceEvent (the HTTP
POST logic) into its own module (keep the same function name and return shape so
existing callers work); extract the version calculation block (the code that
computes currentVersion, maxEventVersion, optimisticEventsCount and
adjustedBaseVersion) into a calculateBaseVersion(workspaceEvents, cacheVersion,
ownEventId?) helper and call it from the mutationFn; extract the long retry flow
inside onSuccess into handleConflictRetry(queryClient, workspaceId, event,
retryAttemptsRef, appendWorkspaceEvent) that encapsulates the
invalidate/refetch/remove-reapply/retry/update/delete logic; and pull the
OCR-specific logging into a small utility (e.g., logOcrUpdates(event, version))
to keep onSuccess concise; after extracting, update imports/exports and ensure
useWorkspaceMutation calls the new helpers with the same symbols so behavior and
types are preserved.
In `@src/hooks/workspace/use-workspace-operations.ts`:
- Around line 1-1188: The file is too large and conflates multiple
responsibilities; split useWorkspaceOperations into focused modules: 1) extract
item creation logic (functions createItem, createItems, plus helpers like
defaultDataFor/getRandomCardColor/findNextAvailablePosition) into a
useItemCreation hook; 2) extract update/debounce logic (updateItem,
updateItemData, flushPendingChanges, pending refs) into a useItemUpdates hook;
3) extract bulk and layout operations (updateAllItems, getLatestItemFromState,
getLatestItemWithPendingChanges, and cache replay usage) into a useBulkLayout
hook; 4) extract folder behavior (createFolder, createFolderWithItems,
updateFolder, deleteFolder, deleteFolderWithContents, getAllDescendantIds,
moveItemToFolder, moveItemsToFolder, filterItemIdsForFolderCreation) into a
useFolderOperations hook; ensure each new hook accepts the shared dependencies
(mutation from useWorkspaceMutation, queryClient, workspaceId, currentState,
userId/userName) and re-export only the original API from a slim
useWorkspaceOperations that composes these hooks. Update imports/exports
accordingly and remove duplicated logic so each file stays under 300 lines.
- Around line 841-906: The createFolderWithItems useCallback currently captures
currentState.items when computing safeItemIds via filterItemIdsForFolderCreation
but does not include currentState.items in its dependency array, causing stale
cycle checks; update the dependencies of createFolderWithItems to include the
array (or a stable reference) representing currentState.items so the callback is
recreated when the workspace tree changes (i.e., add currentState.items or a
derived stable key to the dependency list alongside mutation, userId, userName)
to ensure filterItemIdsForFolderCreation runs against the latest graph.
---
Nitpick comments:
In `@src/app/api/chat/route.ts`:
- Around line 408-415: The callback is using (result as any).providerMetadata
which loses type safety; add a proper type for providerMetadata (e.g., an
interface/type like ProviderMetadata with gateway.routing.resolvedProvider and
finalProvider fields) and annotate the Promise.resolve cast or the result
variable accordingly (for example use result as { providerMetadata?:
ProviderMetadata } or narrow result to unknown and assert/validate its shape) so
the callback signature becomes (meta: ProviderMetadata | undefined) and accesses
provider safely when present; update references to provider, providerMetadata,
and the Promise.resolve call in the same block to use the new type.
In `@src/app/api/workspaces/`[id]/events/route.ts:
- Around line 262-268: The snapshot object is casting latestSnapshot.state to
any; replace this with a concrete type by defining or importing the proper
SnapshotState interface and use it instead of any when building the snapshot
object (e.g., type SnapshotState = /* shape */ and then set state:
latestSnapshot.state as SnapshotState). Locate the code that constructs the
snapshot (references: latestSnapshot, snapshotVersion, state) in route.ts and
update the type for the snapshot state, or adjust the latestSnapshot type so
latestSnapshot.state already matches SnapshotState to remove the cast.
- Around line 112-120: The current inline union uses `any` for `state` which
ESLint disallows; define a proper interface (e.g., SnapshotRow or
LatestSnapshotResult) with typed properties for id?: string, snapshotVersion?:
number, state: Record<string, unknown> | SpecificStateType, eventCount?: number,
createdAt?: string, declare it near the top of the file or next to related
types, then replace the inline union used for `latestSnapshot` (and any other
uses of `latestSnapshotData`) to use this new interface instead of `any`; if
`state` can be multiple shapes, use a discriminated union or Record<string,
unknown> to keep typing strict.
In `@src/components/assistant-ui/CreateQuizToolUI.tsx`:
- Around line 74-80: The folderName derivation uses useMemo with unstable
dependency expressions causing the React hooks rule to reject it; replace the
useMemo block with a plain derived value computed on render (or, if memoizing is
required, move to useMemo with stable dependencies like currentItem?.folderId
and workspaceState?.items without optional chaining in the deps). Specifically,
remove useMemo around folderName and compute it directly from currentItem and
workspaceState (use workspaceState.items.find(item => item.id ===
currentItem.folderId)?.name ?? null) so the value is stable and the hooks linter
won't complain.
In `@src/components/editor/document-editor.scss`:
- Line 1: The Google Fonts import includes DM Sans which is no longer used by
the editor (.tiptap.ProseMirror), so remove the unused font to avoid extra
network weight: either delete the entire `@import` if no other code needs Google
Fonts, or replace the URL with one that only loads the Inter family (remove
"DM+Sans" from the import string) so that .tiptap.ProseMirror and other
components continue using Inter without loading DM Sans.
In `@src/contexts/RealtimeContext.tsx`:
- Around line 46-52: Wrap the currentUser object creation in a React useMemo
keyed on session?.user to avoid recreating the object when unrelated session
fields change; replace the inline conditional assignment for currentUser with a
memoized value (useMemo(() => session?.user ? { id: session.user.id, name:
session.user.name || "Anonymous", image: session.user.image ?? undefined } :
null, [session?.user])) so useWorkspacePresence receives a stable reference and
unnecessary re-renders are prevented.
In `@src/hooks/workspace/use-workspace-mutation.ts`:
- Around line 290-425: The nested .then() chains in the conflict retry block
make the flow hard to follow; refactor the logic around
queryClient.invalidateQueries(...) and the subsequent appendWorkspaceEvent(...)
call into an async/await helper (e.g., create an async function like
retryAppendEvent or handleConflictRetry) that awaits
queryClient.invalidateQueries, re-applies the optimistic update via
queryClient.setQueryData, computes currentVersion from queryClient.getQueryData,
then awaits appendWorkspaceEvent({ workspaceId, event, baseVersion:
currentVersion }) and handles success, conflict, and catch paths (updating
events, removing optimistic event, cleaning
retryAttemptsRef.current.delete(event.id), and forcing invalidateQueries) using
sequential try/catch rather than nested .then() chains; update callers to await
the helper or call it without nesting so control flow around
appendWorkspaceEvent, queryClient.setQueryData, and retryAttemptsRef is linear
and readable.
In `@src/hooks/workspace/use-workspace-realtime.ts`:
- Around line 197-205: The effect dependency array for the useWorkspaceRealtime
hook incorrectly includes the cleanup identifier even though the effect body
does not call cleanup; remove cleanup from the dependency list (leaving
workspaceId, queryClient, onStatusChange, onRemoteEvent, reconnectNonce,
removeChannel) so the effect dependencies reflect actual usage, or
alternatively, if you intended to reuse the external cleanup function, call
cleanup inside the effect (and ensure its behavior/identity is stable) and keep
it in the array; update the dependency array accordingly to match whichever
approach you choose.
In `@src/lib/realtime/server-broadcast.ts`:
- Around line 47-64: The current broadcastWorkspaceEventFromServer
implementation creates and removes a Supabase channel for every call (creating
channel via supabase.channel(...) then send and removeChannel), which can
degrade performance; modify the logic to pool/reuse channels per workspaceId
(e.g., maintain a Map keyed by workspaceId storing active channels), have
broadcastWorkspaceEventFromServer look up or create-and-cache the channel, send
on the cached channel, and only removeChannel and delete from the Map on
explicit teardown (process exit or when no longer needed), ensuring error
handling/logging around send remains in place.
- Around line 16-18: The current supabaseKey assignment silently falls back from
SUPABASE_SERVICE_ROLE_KEY to NEXT_PUBLIC_SUPABASE_ANON_KEY; update the logic
around the supabaseKey variable so the code explicitly logs which key type is
being used (service role vs anon) and only allow the anon fallback in
non-production (e.g., NODE_ENV !== 'production'), or alternatively throw/require
configuration in production; reference the supabaseKey variable and the env
names SUPABASE_SERVICE_ROLE_KEY and NEXT_PUBLIC_SUPABASE_ANON_KEY and add a
concise log or warning (using the existing logger or console) explaining the
chosen key so the behavior is explicit.
In `@src/workflows/audio-transcribe/steps/persist-result.ts`:
- Around line 38-76: Both persistAudioResult and persistAudioFailure duplicate
the workspace append-and-broadcast flow (get_workspace_version,
append_workspace_event SQL call, parse the "(version,modified)" result, throw on
unexpected format or version conflict, call broadcastWorkspaceEventFromServer,
then trigger checkAndCreateSnapshot); extract this into a shared helper (e.g.,
appendAndBroadcastWorkspaceEvent in a new module under `@/lib/workspace/`) that
accepts (workspaceId, event, db, processLogger) and returns the new version or
throws on conflict/unexpected format, encapsulates the SQL calls and parsing
logic (the same regex and error messages), calls
broadcastWorkspaceEventFromServer with the enriched event, and invokes
checkAndCreateSnapshot; update persistAudioResult and persistAudioFailure to
call this helper instead of duplicating the logic so behavior is consistent with
src/workflows/ocr-dispatch/steps/persist-results.ts.
In `@src/workflows/ocr-dispatch/steps/persist-results.ts`:
- Around line 11-58: The appendWorkspaceEvent helper is duplicated and should be
extracted to a shared module so other workflows can reuse it; create a new
module (e.g., event-appender) exporting the async function
appendWorkspaceEvent(workspaceId: string, event: WorkspaceEvent) that contains
the existing logic (get_workspace_version, append_workspace_event call, result
parsing, conflict handling, and broadcastWorkspaceEventFromServer call), update
callers such as the persist-result step in the audio-transcribe workflow to
import and use this exported appendWorkspaceEvent, and ensure any helper
constants (APPEND_RESULT_REGEX) and types (WorkspaceEvent) used by
appendWorkspaceEvent are moved or re-exported so imports remain correct.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1e538e63-196e-4533-846c-b534da5bc35a
📒 Files selected for processing (32)
src/app/api/chat/route.tssrc/app/api/workspaces/[id]/events/route.tssrc/components/assistant-ui/AddImageToolUI.tsxsrc/components/assistant-ui/AddYoutubeVideoToolUI.tsxsrc/components/assistant-ui/AssistantTextSelectionManager.tsxsrc/components/assistant-ui/CreateDocumentToolUI.tsxsrc/components/assistant-ui/CreateFlashcardToolUI.tsxsrc/components/assistant-ui/CreateQuizToolUI.tsxsrc/components/assistant-ui/EditItemToolUI.tsxsrc/components/assistant-ui/ImageSearchToolUI.tsxsrc/components/assistant-ui/ModelPicker.tsxsrc/components/assistant-ui/PromptBuilderDialog.tsxsrc/components/assistant-ui/WorkspaceRuntimeProvider.tsxsrc/components/assistant-ui/attachment.tsxsrc/components/assistant-ui/thread.tsxsrc/components/editor/DocumentEditor.tsxsrc/components/editor/document-editor.scsssrc/components/onboarding/WorkspaceInstructionModal.tsxsrc/components/tiptap-node/horizontal-rule-node/horizontal-rule-node.scsssrc/components/tiptap-node/list-node/list-node.scsssrc/components/workspace-canvas/CardRenderer.tsxsrc/contexts/RealtimeContext.tsxsrc/hooks/ai/use-optimistic-tool-update.tssrc/hooks/workspace/use-workspace-mutation.tssrc/hooks/workspace/use-workspace-operations.tssrc/hooks/workspace/use-workspace-presence.tssrc/hooks/workspace/use-workspace-realtime.tssrc/lib/ai/tools/index.tssrc/lib/ai/workers/workspace-worker.tssrc/lib/realtime/server-broadcast.tssrc/workflows/audio-transcribe/steps/persist-result.tssrc/workflows/ocr-dispatch/steps/persist-results.ts
💤 Files with no reviewable changes (3)
- src/components/assistant-ui/AddImageToolUI.tsx
- src/hooks/ai/use-optimistic-tool-update.ts
- src/components/assistant-ui/ImageSearchToolUI.tsx
Comment on lines
+142
to
+147
| const threadListItemId = useAuiState( | ||
| ({ threadListItem }) => (threadListItem as any)?.id, | ||
| ); | ||
| const mainThreadId = useAuiState( | ||
| ({ threads }) => (threads as any)?.mainThreadId, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --type=tsx -C2 'useAuiState\(' src
rg -n --type=tsx -C2 'as any' src/components/assistant-ui/AssistantTextSelectionManager.tsxRepository: ThinkEx-OSS/thinkex
Length of output: 90
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the file first
fd 'AssistantTextSelectionManager' src
# Get file size
wc -l src/components/assistant-ui/AssistantTextSelectionManager.tsx
# Read lines around 142-147
sed -n '135,155p' src/components/assistant-ui/AssistantTextSelectionManager.tsx
# Search for all useAuiState calls in the file
echo "=== All useAuiState patterns ==="
rg -n 'useAuiState' src/components/assistant-ui/AssistantTextSelectionManager.tsx
# Search for all "as any" patterns in the file
echo "=== All 'as any' patterns ==="
rg -n 'as any' src/components/assistant-ui/AssistantTextSelectionManager.tsxRepository: ThinkEx-OSS/thinkex
Length of output: 1432
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for useAuiState usage patterns across the codebase
echo "=== All useAuiState patterns in codebase ==="
rg -n 'useAuiState' src --max-count=20
# Look for type definitions related to assistant-ui
echo "=== Search for AssistantUi or related type definitions ==="
rg -n 'AssistantUi' src --max-count=20
# Check for tsconfig or eslint config that might reveal typing rules
echo "=== Config files ==="
fd -e tsconfig.json -e eslintrc -e eslint.config
</script>Repository: ThinkEx-OSS/thinkex
Length of output: 3275
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find linting config files
echo "=== Linting configuration files ==="
find . -maxdepth 3 -type f \( -name '.eslintrc*' -o -name 'eslint.config.*' -o -name '.eslintignore' \) 2>/dev/null | head -20
# Check for TypeScript configuration
echo "=== TypeScript configuration ==="
cat tsconfig.json 2>/dev/null | head -30
# Look at markdown-text.tsx to see the "as any" patterns there
echo "=== Other useAuiState with 'as any' in markdown-text.tsx ==="
sed -n '175,185p' src/components/assistant-ui/markdown-text.tsx
# Check if there's a no-explicit-any eslint rule
echo "=== Search for eslint or linting rules config ==="
rg -n 'no-explicit-any\|@typescript-eslint' . --max-count=10Repository: ThinkEx-OSS/thinkex
Length of output: 1246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the eslint config
echo "=== ESLint configuration ==="
head -100 eslint.config.mjs
# Also check if there's a package.json with relevant linting info
echo "=== Package.json scripts ==="
grep -A 5 '"scripts"' package.json | head -20Repository: ThinkEx-OSS/thinkex
Length of output: 793
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for any references to `@typescript-eslint` in the config
echo "=== Checking for `@typescript-eslint` rules in the config ==="
rg -n '@typescript-eslint' eslint.config.mjs
# Check package.json to see what version of ESLint and dependencies are used
echo "=== Relevant dependencies ==="
grep -E '"eslint|typescript|@typescript-eslint"' package.json
# Let's also verify the exact content of lines 142-147 again for clarity
echo "=== Exact content of lines 142-150 ==="
sed -n '142,150p' src/components/assistant-ui/AssistantTextSelectionManager.tsx
# Check the file size to validate against the 300-line guideline
echo "=== File statistics ==="
wc -l src/components/assistant-ui/AssistantTextSelectionManager.tsxRepository: ThinkEx-OSS/thinkex
Length of output: 122
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get the complete package.json dependencies
echo "=== ESLint and TypeScript dependencies ==="
grep -E 'eslint|typescript' package.json | head -20
# Let's also check the file line count again
echo "=== File line counts ==="
wc -l src/components/assistant-ui/AssistantTextSelectionManager.tsx
echo "Maximum allowed: 300 lines per coding guidelines"
# Verify the exact any casts one more time
echo "=== Verify exact 'as any' occurrences ==="
rg -n 'as any' src/components/assistant-ui/AssistantTextSelectionManager.tsx -A 1 -B 1Repository: ThinkEx-OSS/thinkex
Length of output: 696
Split AssistantTextSelectionManager.tsx — file exceeds 300-line maximum by 4x (1,274 lines).
This file violates the coding guidelines' 300-line limit and requires refactoring into smaller modules. While splitting, also remove the explicit as any casts on lines 143 and 146 ((threadListItem as any)?.id and (threads as any)?.mainThreadId), which weaken type safety in the selector functions passed to useAuiState().
🧰 Tools
🪛 ESLint
[error] 143-143: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 146-146: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/assistant-ui/AssistantTextSelectionManager.tsx` around lines
142 - 147, The file AssistantTextSelectionManager.tsx is too large and should be
split into smaller modules (e.g., selection logic, UI hooks, and utilities) to
meet the 300-line limit; refactor by extracting related functions and components
into new files and importing them back into the main component. While splitting,
remove the unsafe casts in the useAuiState selectors: replace (threadListItem as
any)?.id and (threads as any)?.mainThreadId by referencing the correctly typed
slice of state (define/consume the proper interface or typed selector for the
AUI state) so useAuiState(({ threadListItem }: AuiState) => threadListItem?.id)
and useAuiState(({ threads }: AuiState) => threads?.mainThreadId) (or equivalent
typed helper selectors) to restore type safety for threadListItemId and
mainThreadId.
Comment on lines
940
to
+955
| const getAllDescendantIds = useCallback( | ||
| (folderId: string, items: Item[]): string[] => { | ||
| const directChildren = items.filter(item => item.folderId === folderId); | ||
| const directChildren = items.filter((item) => item.folderId === folderId); | ||
| const descendantIds: string[] = []; | ||
|
|
||
| for (const child of directChildren) { | ||
| descendantIds.push(child.id); | ||
| // Recursively get descendants of nested folders | ||
| if (child.type === 'folder') { | ||
| if (child.type === "folder") { | ||
| descendantIds.push(...getAllDescendantIds(child.id, items)); | ||
| } | ||
| } | ||
|
|
||
| return descendantIds; | ||
| }, | ||
| [] | ||
| [], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l src/hooks/workspace/use-workspace-operations.tsRepository: ThinkEx-OSS/thinkex
Length of output: 115
🏁 Script executed:
cat -n src/hooks/workspace/use-workspace-operations.ts | sed -n '930,960p'Repository: ThinkEx-OSS/thinkex
Length of output: 1321
🏁 Script executed:
# Check for ESLint configuration
find . -maxdepth 3 -type f \( -name ".eslintrc*" -o -name "eslint.config.*" \) | head -20Repository: ThinkEx-OSS/thinkex
Length of output: 82
🏁 Script executed:
# Check if the file violates the 300-line limit
wc -l src/hooks/workspace/use-workspace-operations.tsRepository: ThinkEx-OSS/thinkex
Length of output: 115
🏁 Script executed:
cat -n eslint.config.mjs | head -100Repository: ThinkEx-OSS/thinkex
Length of output: 683
🏁 Script executed:
# Look for any rules related to function expressions, closures, or uninitialized bindings
rg "no-" eslint.config.mjs | head -20Repository: ThinkEx-OSS/thinkex
Length of output: 45
🏁 Script executed:
# Check if there's a specific ESLint rule in Next.js config that would flag this
# Let's search for rules that might catch the "uninitialized binding" pattern
rg "no-func-assign|no-use-before-define|prefer-const" eslint.config.mjsRepository: ThinkEx-OSS/thinkex
Length of output: 45
🏁 Script executed:
# Let's check if there are any actual ESLint errors/warnings logged anywhere
fd -e log -e txt -e md | xargs grep -l "949\|getAllDescendantIds" 2>/dev/null | head -5Repository: ThinkEx-OSS/thinkex
Length of output: 45
🏁 Script executed:
# Look for package.json to understand the project setup better
cat package.json | head -50Repository: ThinkEx-OSS/thinkex
Length of output: 1870
File exceeds the 300-line limit (currently 1188 lines) and requires refactoring.
In addition, the recursive pattern at line 949 should use a named function expression to avoid closing over an uninitialized binding:
Suggested fix for the recursive closure pattern
- const getAllDescendantIds = useCallback(
- (folderId: string, items: Item[]): string[] => {
+ const getAllDescendantIds = useCallback(
+ function collectDescendantIds(
+ folderId: string,
+ items: Item[],
+ ): string[] {
const directChildren = items.filter((item) => item.folderId === folderId);
const descendantIds: string[] = [];
for (const child of directChildren) {
descendantIds.push(child.id);
// Recursively get descendants of nested folders
if (child.type === "folder") {
- descendantIds.push(...getAllDescendantIds(child.id, items));
+ descendantIds.push(...collectDescendantIds(child.id, items));
}
}
return descendantIds;
},
[],
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const getAllDescendantIds = useCallback( | |
| (folderId: string, items: Item[]): string[] => { | |
| const directChildren = items.filter(item => item.folderId === folderId); | |
| const directChildren = items.filter((item) => item.folderId === folderId); | |
| const descendantIds: string[] = []; | |
| for (const child of directChildren) { | |
| descendantIds.push(child.id); | |
| // Recursively get descendants of nested folders | |
| if (child.type === 'folder') { | |
| if (child.type === "folder") { | |
| descendantIds.push(...getAllDescendantIds(child.id, items)); | |
| } | |
| } | |
| return descendantIds; | |
| }, | |
| [] | |
| [], | |
| const getAllDescendantIds = useCallback( | |
| function collectDescendantIds( | |
| folderId: string, | |
| items: Item[], | |
| ): string[] { | |
| const directChildren = items.filter((item) => item.folderId === folderId); | |
| const descendantIds: string[] = []; | |
| for (const child of directChildren) { | |
| descendantIds.push(child.id); | |
| // Recursively get descendants of nested folders | |
| if (child.type === "folder") { | |
| descendantIds.push(...collectDescendantIds(child.id, items)); | |
| } | |
| } | |
| return descendantIds; | |
| }, | |
| [], | |
| ); |
🧰 Tools
🪛 ESLint
[error] 949-949: Error: Cannot access variable before it is declared
getAllDescendantIds is accessed before it is declared, which prevents the earlier access from updating when this value changes over time.
/home/jailuser/git/src/hooks/workspace/use-workspace-operations.ts:949:33
947 | // Recursively get descendants of nested folders
948 | if (child.type === "folder") {
949 | descendantIds.push(...getAllDescendantIds(child.id, items));
| ^^^^^^^^^^^^^^^^^^^getAllDescendantIdsaccessed before it is declared
950 | }
951 | }
952 |
/home/jailuser/git/src/hooks/workspace/use-workspace-operations.ts:940:3
938 |
939 | // Helper to recursively find all descendant IDs (items in folder and nested subfolders)
940 | const getAllDescendantIds = useCallback(
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
941 | (folderId: string, items: Item[]): string[] => {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
942 | const directChildren = items.filter((item) => item.folderId === folderId);
…
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
955 | [],
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
956 | );
| ^^^^^getAllDescendantIdsis declared here
957 |
958 | // deleteFolderWithContents deletes the folder and all items inside it (including nested)
959 | // Uses atomic bulk update pattern (same as handleBulkDelete in WorkspaceSection)
(react-hooks/immutability)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/workspace/use-workspace-operations.ts` around lines 940 - 955, The
file is too large and the recursive useCallback getAllDescendantIds creates a
recursive closure over an uninitialized binding; extract the recursion into a
named function (e.g., function getAllDescendantIdsImpl(folderId, items) or a
named function expression) and have the useCallback (getAllDescendantIds) simply
call that named function to avoid the self-reference problem, then move this
helper into a new utilities module (or a smaller helper file) and import it from
use-workspace-operations to shrink the file under 300 lines; update all callers
to use the exported getAllDescendantIds helper or the thin useCallback wrapper
which delegates to the named implementation.
Comment on lines
+111
to
+122
| useEffect(() => { | ||
| const channel = channelRef.current; | ||
| if (!channel || !currentUser || !clientKey) return; | ||
|
|
||
| channel.track({ | ||
| clientKey, | ||
| userId: currentUser.id, | ||
| userName: currentUser.name, | ||
| userImage: currentUser.image, | ||
| joinedAt: joinedAtRef.current, | ||
| }); | ||
| }, [currentUser, clientKey]); |
There was a problem hiding this comment.
Potential race condition between effects on currentUser change.
When currentUser changes, both effects fire. The first effect (lines 58-109) runs cleanup and recreates the channel, while this second effect immediately calls track(). There's a risk that channelRef.current points to a stale/removed channel or is null during the transition.
Consider consolidating tracking into the first effect's subscription callback, or adding a guard to ensure the channel is fully subscribed before tracking:
Suggested guard
useEffect(() => {
const channel = channelRef.current;
- if (!channel || !currentUser || !clientKey) return;
+ if (!channel || !currentUser || !clientKey) return;
+
+ // Only track if channel is subscribed
+ const state = channel.state;
+ if (state !== 'joined') return;
channel.track({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/workspace/use-workspace-presence.ts` around lines 111 - 122, The
second useEffect calling channelRef.current.track when currentUser/clientKey
change can race with the channel recreation cleanup in the first effect; modify
the code so tracking happens only after the channel is fully subscribed — either
move the track call into the first effect’s channel subscription callback (where
the channel is created/confirmed) or add a guard that verifies the channel is
active (e.g., channelRef.current exists and is in a "subscribed" state or await
channel.subscribe()) before calling track; update references to channelRef,
joinedAtRef, currentUser, clientKey and the existing subscription/callback in
the first effect accordingly so track never runs against a stale/null channel.
Comment on lines
27
to
40
| export interface ChatToolsConfig { | ||
| workspaceId: string | null; | ||
| userId: string | null; | ||
| activeFolderId?: string; | ||
| threadId?: string | null; | ||
| clientTools?: Record<string, any>; | ||
| /** Experiment: enable magic_fetch tool (logs AI data requests to PostHog) */ | ||
| enableMagicFetch?: boolean; | ||
| workspaceId: string | null; | ||
| userId: string | null; | ||
| activeFolderId?: string; | ||
| threadId?: string | null; | ||
| clientTools?: Record<string, any>; | ||
| /** Experiment: enable magic_fetch tool (logs AI data requests to PostHog) */ | ||
| enableMagicFetch?: boolean; | ||
| } | ||
|
|
||
| /** | ||
| * Create all chat tools with the given context | ||
| */ | ||
| export function createChatTools(config: ChatToolsConfig): Record<string, any> { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/lib/ai/tools/index.ts | head -50Repository: ThinkEx-OSS/thinkex
Length of output: 2006
🏁 Script executed:
wc -l src/lib/ai/tools/index.tsRepository: ThinkEx-OSS/thinkex
Length of output: 91
🏁 Script executed:
# Check for ESLint configuration
find . -name ".eslintrc*" -o -name "eslint.config.*" | head -5Repository: ThinkEx-OSS/thinkex
Length of output: 458
🏁 Script executed:
# Search for usages of clientTools and createChatTools
rg "clientTools|createChatTools" -A 3 -B 3 --max-count 20Repository: ThinkEx-OSS/thinkex
Length of output: 2472
🏁 Script executed:
cat eslint.config.mjs | grep -A 5 -B 5 "no-explicit-any\|@typescript-eslint/no-explicit-any" | head -30Repository: ThinkEx-OSS/thinkex
Length of output: 45
🏁 Script executed:
# See what createChatTools actually returns
sed -n '40,93p' src/lib/ai/tools/index.tsRepository: ThinkEx-OSS/thinkex
Length of output: 1535
🏁 Script executed:
# Check the WorkspaceToolContext type to understand the tool structure better
rg "type WorkspaceToolContext|interface WorkspaceToolContext" -A 10Repository: ThinkEx-OSS/thinkex
Length of output: 1438
🏁 Script executed:
# Check if eslint.config.mjs exists and what rules are configured
cat eslint.config.mjs | head -100Repository: ThinkEx-OSS/thinkex
Length of output: 543
🏁 Script executed:
# Try to understand the actual tool object structure better
# Look at one of the tool factory functions to see what they return
head -50 src/lib/ai/tools/process-urls.tsRepository: ThinkEx-OSS/thinkex
Length of output: 1977
🏁 Script executed:
# Check if there are any TypeScript tool types defined
find src/lib/ai/tools -name "*.ts" -type f | xargs grep -l "Tool\|tool" | head -5Repository: ThinkEx-OSS/thinkex
Length of output: 264
🏁 Script executed:
# Check what the frontend tools return type is
rg "frontendTools" src/lib/ai/tools -A 2 -B 2Repository: ThinkEx-OSS/thinkex
Length of output: 842
🏁 Script executed:
# Let's check if there's a tsconfig that might affect type checking
cat tsconfig.json | head -50Repository: ThinkEx-OSS/thinkex
Length of output: 800
🏁 Script executed:
# Check what package provides the tool type
grep -r "from \"ai\"" src/lib/ai/tools/ | head -5Repository: ThinkEx-OSS/thinkex
Length of output: 433
🏁 Script executed:
# Check the package.json to see what ai packages are used
grep -A 5 '"ai"' package.jsonRepository: ThinkEx-OSS/thinkex
Length of output: 232
🏁 Script executed:
# See if there's a Tool type exported or used
rg "Tool<|type Tool" src/lib/ai/tools/ -B 2 -A 2Repository: ThinkEx-OSS/thinkex
Length of output: 45
🏁 Script executed:
# Check what the actual Tool type should be from the ai package
rg "export.*tool|Tool" node_modules/.pnpm/ai@*/node_modules/ai/dist/ | head -20Repository: ThinkEx-OSS/thinkex
Length of output: 4293
🏁 Script executed:
# See how the tools are actually typed in the codebase
grep -r "Record<string" src/lib/ai/tools/ -B 2 -A 2Repository: ThinkEx-OSS/thinkex
Length of output: 1924
🏁 Script executed:
# Check the actual usage in the route to understand what body.tools contains
grep -A 10 "body.tools" src/app/api/chat/route.tsRepository: ThinkEx-OSS/thinkex
Length of output: 501
🏁 Script executed:
# Check if Next.js ESLint configs enable no-explicit-any
npm ls eslint-config-next 2>/dev/null | head -3Repository: ThinkEx-OSS/thinkex
Length of output: 303
🏁 Script executed:
# Look at what Next.js TypeScript ESLint config includes
find node_modules/.pnpm -name "typescript.js" -path "*eslint-config-next*" -exec cat {} \; | grep -A 10 "no-explicit-any" | head -20Repository: ThinkEx-OSS/thinkex
Length of output: 45
🏁 Script executed:
# Check actual Tool type from ai package to understand proper typing
cat node_modules/.pnpm/ai@*/node_modules/ai/dist/index.d.mts | grep -A 5 "^export type Tool"Repository: ThinkEx-OSS/thinkex
Length of output: 45
🏁 Script executed:
# Find the actual Tool type definition
find node_modules/.pnpm/ai@*/node_modules -name "*.d.ts" -o -name "*.d.mts" | xargs grep "^export type Tool" | head -5Repository: ThinkEx-OSS/thinkex
Length of output: 45
🏁 Script executed:
# Check if there's a specific ToolSet type that should be used
rg "ToolSet|Tool<" node_modules/.pnpm/ai@*/node_modules/ai/dist/ | head -15Repository: ThinkEx-OSS/thinkex
Length of output: 2340
🏁 Script executed:
# See what type annotation is actually used in similar tool definitions
grep -r "function create.*Tool" src/lib/ai/tools/ | head -3Repository: ThinkEx-OSS/thinkex
Length of output: 369
🏁 Script executed:
# Check if these any types are actually causing linting issues by searching for lint comments
grep -r "@ts-ignore\|@eslint-disable\|// eslint" src/lib/ai/tools/index.tsRepository: ThinkEx-OSS/thinkex
Length of output: 45
🏁 Script executed:
# Check if there's a way to see what the actual type should be for the return
cat node_modules/.pnpm/ai@*/node_modules/ai/dist/index.d.ts | grep -B 5 -A 10 "^export type Tool<"Repository: ThinkEx-OSS/thinkex
Length of output: 45
🏁 Script executed:
# Check if the return type should actually be ToolSet
rg "ToolSet" node_modules/.pnpm/ai@*/node_modules/ai/ | grep "export" | head -5Repository: ThinkEx-OSS/thinkex
Length of output: 50376
🏁 Script executed:
# See if there's a proper way tools are typed elsewhere in the codebase
rg "Tool\[|Tool<" src/ --type ts --type tsx | head -10Repository: ThinkEx-OSS/thinkex
Length of output: 90
🏁 Script executed:
# Check if there are any actual lint errors/warnings about these lines
npm run lint 2>&1 | grep -A 5 -B 5 "any" | head -50Repository: ThinkEx-OSS/thinkex
Length of output: 3400
🏁 Script executed:
# Check what the actual ToolSet type signature is and if it should be used
grep -r "type ToolSet\|interface ToolSet" node_modules/.pnpm/ai@*/node_modules/ai/dist/index.d.ts | head -3Repository: ThinkEx-OSS/thinkex
Length of output: 277
🏁 Script executed:
# Verify if frontendTools returns a specific typed object or untyped
grep -r "frontendTools" node_modules/.pnpm/assistant-ui*/node_modules/*/dist/ 2>/dev/null | head -5Repository: ThinkEx-OSS/thinkex
Length of output: 45
🏁 Script executed:
# Check if src/lib/ai/tools/index.ts is actually subject to linting
npm run lint 2>&1 | grep "src/lib/ai/tools/index.ts"Repository: ThinkEx-OSS/thinkex
Length of output: 107
🏁 Script executed:
# Check the actual tsconfig to see if strict mode is enabled
grep -A 10 '"strict"' tsconfig.jsonRepository: ThinkEx-OSS/thinkex
Length of output: 324
🏁 Script executed:
# See what the actual import signature is for frontendTools
find node_modules -name "*.d.ts" -path "*assistant-ui*" | xargs grep -l "frontendTools" | head -3Repository: ThinkEx-OSS/thinkex
Length of output: 45
🏁 Script executed:
# Get specific lint errors for the index.ts file
npm run lint 2>&1 | grep -A 20 "src/lib/ai/tools/index.ts"Repository: ThinkEx-OSS/thinkex
Length of output: 1455
🏁 Script executed:
# Check the actual return type more carefully by looking at the full function
sed -n '40,80p' src/lib/ai/tools/index.tsRepository: ThinkEx-OSS/thinkex
Length of output: 1219
🏁 Script executed:
# See what the `@assistant-ui/react-ai-sdk` frontendTools function signature is
find node_modules -name "*.d.ts" -path "*assistant-ui*react*" -exec grep -l "frontendTools" {} \; | head -1Repository: ThinkEx-OSS/thinkex
Length of output: 45
🏁 Script executed:
# Check what tool-utils.ts is doing with the any types
cat src/lib/ai/tools/tool-utils.ts | head -50Repository: ThinkEx-OSS/thinkex
Length of output: 2163
🏁 Script executed:
# Look for proper type definitions that might be better than any or unknown
rg "export.*Tool.*=" src/lib/ai/tools/ -A 2 | head -30Repository: ThinkEx-OSS/thinkex
Length of output: 45
🏁 Script executed:
# Check if there's a way to properly type the return without any or unknown
grep -r "Record<string" src/lib/ai/tools/ | grep -v node_modulesRepository: ThinkEx-OSS/thinkex
Length of output: 550
Replace any with unknown in ChatToolsConfig.clientTools and createChatTools return type.
ESLint flags both as @typescript-eslint/no-explicit-any errors on lines 32 and 40. While unknown doesn't fully resolve the typing challenge with frontendTools() from @assistant-ui/react-ai-sdk, it improves type safety by requiring type guards at consumption points.
💡 Suggested changes
export interface ChatToolsConfig {
workspaceId: string | null;
userId: string | null;
activeFolderId?: string;
threadId?: string | null;
- clientTools?: Record<string, any>;
+ clientTools?: Record<string, unknown>;
/** Experiment: enable magic_fetch tool (logs AI data requests to PostHog) */
enableMagicFetch?: boolean;
}
/**
* Create all chat tools with the given context
*/
-export function createChatTools(config: ChatToolsConfig): Record<string, any> {
+export function createChatTools(
+ config: ChatToolsConfig,
+): Record<string, unknown> {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export interface ChatToolsConfig { | |
| workspaceId: string | null; | |
| userId: string | null; | |
| activeFolderId?: string; | |
| threadId?: string | null; | |
| clientTools?: Record<string, any>; | |
| /** Experiment: enable magic_fetch tool (logs AI data requests to PostHog) */ | |
| enableMagicFetch?: boolean; | |
| workspaceId: string | null; | |
| userId: string | null; | |
| activeFolderId?: string; | |
| threadId?: string | null; | |
| clientTools?: Record<string, any>; | |
| /** Experiment: enable magic_fetch tool (logs AI data requests to PostHog) */ | |
| enableMagicFetch?: boolean; | |
| } | |
| /** | |
| * Create all chat tools with the given context | |
| */ | |
| export function createChatTools(config: ChatToolsConfig): Record<string, any> { | |
| export interface ChatToolsConfig { | |
| workspaceId: string | null; | |
| userId: string | null; | |
| activeFolderId?: string; | |
| threadId?: string | null; | |
| clientTools?: Record<string, unknown>; | |
| /** Experiment: enable magic_fetch tool (logs AI data requests to PostHog) */ | |
| enableMagicFetch?: boolean; | |
| } | |
| /** | |
| * Create all chat tools with the given context | |
| */ | |
| export function createChatTools( | |
| config: ChatToolsConfig, | |
| ): Record<string, unknown> { |
🧰 Tools
🪛 ESLint
[error] 32-32: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 40-40: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/ai/tools/index.ts` around lines 27 - 40, The
ChatToolsConfig.clientTools and createChatTools return type use `any`; change
`clientTools?: Record<string, any>` to `clientTools?: Record<string, unknown>`
and update the function signature return type from `Record<string, any>` to
`Record<string, unknown>` so callers must perform type guards; update references
to `ChatToolsConfig` and `createChatTools` accordingly (no other logic changes).
Contributor
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Member
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/hooks/workspace/use-workspace-operations.ts (1)
862-928:⚠️ Potential issue | 🟡 MinorMissing
currentState.itemsin dependency array may cause stale reads.
createFolderWithItemsaccessescurrentState.itemsat line 871 but the dependency array at line 927 omits it. While this function runs synchronously (unlike debounced callbacks), the closure could still use stale items ifcurrentStateupdates between renders.For consistency with the pattern established by
getLatestItemsFromState, consider either:
- Adding
currentState.itemsto the dependency array, or- Using
getLatestItemsFromState()to fetch fresh items🛠️ Option 1: Add to dependency array
}, - [mutation, userId, userName], + [mutation, userId, userName, currentState.items], );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/workspace/use-workspace-operations.ts` around lines 862 - 928, createFolderWithItems closes over currentState.items which can become stale; update the hook so it reads fresh items instead of relying on the captured array: either add currentState.items to the dependency array of createFolderWithItems or replace the direct access to currentState.items with a call to getLatestItemsFromState() inside createFolderWithItems; reference the function name createFolderWithItems and the symbol currentState.items (or getLatestItemsFromState) and ensure the dependency array includes the chosen dependency (or remove the dependency if you switch to the getter) so the callback always uses up-to-date items.src/components/assistant-ui/CreateFlashcardToolUI.tsx (1)
1-306:⚠️ Potential issue | 🟡 MinorFile exceeds the 300-line limit (currently 306 lines).
As per coding guidelines, this file should be refactored to stay within 300 lines. Consider extracting
CreateFlashcardReceiptinto a separate file (e.g.,CreateFlashcardReceipt.tsx) and importing it here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/CreateFlashcardToolUI.tsx` around lines 1 - 306, This file is 306 lines (over the 300-line limit); extract the CreateFlashcardReceipt component into its own file (e.g., CreateFlashcardReceipt.tsx) and import it into CreateFlashcardToolUI.tsx to reduce file length. Move the CreateFlashcardReceipt function and its CreateFlashcardReceiptProps interface into the new file, ensure necessary hooks and utilities used inside it (useWorkspaceStore, useWorkspaceState, useNavigateToItem, MoveToDialog, toast, Button, Eye, FolderInput, cn, logger, Item type) are imported there, export the component (default or named) and update the original file to import CreateFlashcardReceipt, keeping the CreateFlashcardArgs type, parseFlashcardResult logic, and CreateFlashcardToolUI export unchanged. Ensure prop names and types match exactly (args, result, status, moveItemToFolder, allItems, workspaceName, workspaceIcon, workspaceColor) so callers (CreateFlashcardToolUI) compile without changes.
♻️ Duplicate comments (1)
src/components/assistant-ui/AssistantTextSelectionManager.tsx (1)
1-1274: 🛠️ Refactor suggestion | 🟠 MajorFile exceeds 300-line limit by 4x (1,274 lines).
This file severely violates the coding guideline requiring a maximum of 300 lines. Consider splitting into smaller modules:
- Selection handling logic (
use-text-selection.ts)- Marker element utilities (
marker-utils.ts)- Dialog components (
MultiSelectDialog.tsx,NegativeFeedbackDialog.tsx)- Tooltip action definitions
As per coding guidelines:
**/*.{js,jsx,ts,tsx}: Enforce a maximum of 300 lines per file🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/AssistantTextSelectionManager.tsx` around lines 1 - 1274, This file is too large; split responsibilities into smaller modules: extract selection logic (handleSelectionChange, handleHighlightAdded, handleHighlightsChange, extractMessageContext, extractUserPrompt, handleKeyDown, and related state/hooks) into a new use-text-selection.ts hook used by AssistantTextSelectionManager; move marker DOM helpers (createMarkerElementAtRange, removeMarkerElement, multiModeMarkerElementRef, markerToMouseOffsetRef and related refs/cleanup) into marker-utils.ts with exported functions createMarkerAtRange and removeMarker; move multi-select and negative-feedback UI into MultiSelectDialog.tsx and NegativeFeedbackDialog.tsx components (use props for multiSelectionsForDialog, handlers like handleSubmitMultiSelections, handleCancelMultiDialog, show state setters); and factor tooltip actions (tooltipActions, highlightAction) into a small helper file (tooltip-actions.tsx) that exports actions builders; update AssistantTextSelectionManager to import these modules and keep only composition/UI glue and refs (selectableTextRef, tooltip visibility, and rendering). Ensure to preserve symbol names used above (e.g., handleMultiSelect, handleReply, clearAllHighlights, handleCreateDocument, setTriggerAddHighlight, pendingContexts) so the refactor is minimal for callers.
🧹 Nitpick comments (10)
src/hooks/workspace/use-workspace-presence.ts (2)
96-106: Consider adding error handling for subscription failures.The subscribe callback only handles
"SUBSCRIBED"status. If subscription fails or thetrack()promise rejects, there's no error handling, which could leave presence in an inconsistent state or cause silent failures.🛡️ Suggested error handling
channel.subscribe(async (status) => { if (status === "SUBSCRIBED") { - await channel.track({ - clientKey, - userId: currentUser.id, - userName: currentUser.name, - userImage: currentUser.image, - joinedAt: joinedAtRef.current, - }); + try { + await channel.track({ + clientKey, + userId: currentUser.id, + userName: currentUser.name, + userImage: currentUser.image, + joinedAt: joinedAtRef.current, + }); + } catch (error) { + console.error("Failed to track presence:", error); + } + } else if (status === "CHANNEL_ERROR") { + console.error("Presence channel error"); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/workspace/use-workspace-presence.ts` around lines 96 - 106, The subscription callback for channel.subscribe currently only handles the "SUBSCRIBED" branch and does not handle other statuses or promise rejections from channel.track; update the channel.subscribe handler to (1) handle non-"SUBSCRIBED" statuses (e.g., "FAILED", "UNSUBSCRIBED") and log or set a failure state, and (2) wrap the await channel.track({...}) call in try/catch to handle rejections from track()—use clientKey, currentUser (id/name/image) and joinedAtRef to provide context in error logs and optionally trigger a retry or mark presence as failed so the app doesn't silently ignore subscription/track failures.
109-109: Object reference in dependency array may cause excessive channel recreation.
currentUseris an object that gets recreated on every parent render (seeRealtimeContext.tsxlines 46-52). This causes the effect to re-run and recreate the channel unnecessarily, potentially causing presence flicker for other collaborators.Consider extracting the primitive values used in the effect:
♻️ Proposed fix using primitive dependencies
export function useWorkspacePresence( workspaceId: string | null, options: UseWorkspacePresenceOptions, ): UseWorkspacePresenceReturn { const { currentUser } = options; const currentUserId = currentUser?.id ?? null; + const currentUserName = currentUser?.name ?? null; + const currentUserImage = currentUser?.image; const [collaborators, setCollaborators] = useState<CollaboratorPresence[]>( [], ); // ... useEffect(() => { - if (!workspaceId || !currentUser || !clientKey) { + if (!workspaceId || !currentUserId || !currentUserName || !clientKey) { cleanup(); return; } // ... channel.on("presence", { event: "sync" }, () => { const state = channel.presenceState<PresencePayload>(); const dedupedUsers = new Map<string, CollaboratorPresence>(); for (const presences of Object.values(state)) { const presence = presences[0]; - if (!presence || presence.userId === currentUser.id) continue; + if (!presence || presence.userId === currentUserId) continue; // ... } }); channel.subscribe(async (status) => { if (status === "SUBSCRIBED") { await channel.track({ clientKey, - userId: currentUser.id, - userName: currentUser.name, - userImage: currentUser.image, + userId: currentUserId, + userName: currentUserName, + userImage: currentUserImage, joinedAt: joinedAtRef.current, }); } }); return cleanup; - }, [workspaceId, currentUser, cleanup, clientKey]); + }, [workspaceId, currentUserId, currentUserName, currentUserImage, cleanup, clientKey]);Alternatively, memoize
currentUserin the parentRealtimeProvider.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/workspace/use-workspace-presence.ts` at line 109, The effect in useWorkspacePresence that depends on currentUser causes unnecessary re-runs because currentUser is an unstable object from RealtimeContext; change the dependency to stable primitive values (e.g., destructure and depend on currentUser.id and any specific metadata like currentUser.name/email or a serialized string) instead of the whole currentUser object, so the channel is only recreated when those actual values change; update the dependency array in the effect inside useWorkspacePresence to use those primitives (keeping workspaceId, clientKey, cleanup) or alternatively memoize currentUser in RealtimeProvider to make the object stable.src/components/onboarding/WorkspaceInstructionModal.tsx (1)
1-545: File exceeds 300-line limit (545 lines).This file violates the coding guideline requiring a maximum of 300 lines per file. Consider extracting the
useCarouselhook (lines 117-270) into a separate file likeuse-carousel.tsand theSTEPSconfiguration (lines 55-112) into a constants file.As per coding guidelines:
**/*.{js,jsx,ts,tsx}: Enforce a maximum of 300 lines per file🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/onboarding/WorkspaceInstructionModal.tsx` around lines 1 - 545, The file is over the 300-line limit; extract the carousel logic and large constants to shrink WorkspaceInstructionModal: move the useCarousel hook (function useCarousel) into a new module (e.g., export default or named export from use-carousel.ts) and move the STEPS array (const STEPS and VIDEO_BASE) into a constants module (e.g., onboarding-steps.ts) that exports the array and any related types, then import them into WorkspaceInstructionModal and update references (useCarousel, STEPS, VIDEO_BASE) accordingly so WorkspaceInstructionModal only contains the component UI and wiring.src/components/assistant-ui/AssistantTextSelectionManager.tsx (2)
400-434: Consider removingtooltipPositionfrom dependency array.Line 433 includes
tooltipPositionin the dependency array, but the callback doesn't actually use this value (it only callssetTooltipVisible). This causes unnecessary callback recreation when tooltip position changes.- [setTooltipVisible, multiModeBasePosition, tooltipPosition], + [setTooltipVisible, multiModeBasePosition],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/AssistantTextSelectionManager.tsx` around lines 400 - 434, The handleHighlightAdded callback includes an unused dependency (tooltipPosition) causing unnecessary re-creations; update the dependency array for the handleHighlightAdded useCallback (the function defined as handleHighlightAdded) to remove tooltipPosition and retain only the actual dependencies used (setTooltipVisible and multiModeBasePosition), ensuring the array matches the referenced symbols inside the callback and satisfies hook lint rules.
1163-1163: Consider removing or explaining the empty div.Line 1163 contains an empty
<div>with styling classes that appears to serve as a spacer. If intentional, consider adding a comment or using a semantic element. If unintentional, remove it.- <div className="pt-4 border-t space-y-3"></div> + {/* Visual divider before footer */} + <div className="pt-4 border-t" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/AssistantTextSelectionManager.tsx` at line 1163, In the AssistantTextSelectionManager component's JSX (look for the return/render of AssistantTextSelectionManager), there is an empty <div> with classes "pt-4 border-t space-y-3"; either remove this redundant spacer div or replace it with a semantic element (e.g., an <hr> or a <div aria-hidden="true"> with a short inline comment) and if you keep it, add a concise comment above explaining its purpose. Update the JSX where that empty div occurs to eliminate unused markup or clarify intent so reviewers understand why it remains.src/hooks/workspace/use-workspace-operations.ts (1)
398-422: Consider reusingnewItemfor layout tracking instead of a separate placeholder.Currently,
layoutPlaceholderItemis built at lines 398-407 (callinggetRandomCardColor()once), thennewItemis built at lines 413-422 (callinggetRandomCardColor()again). Since onlylayoutproperties matter forfindNextAvailablePosition, you could buildnewItemfirst and conditionally push it toitemsForLayout:♻️ Suggested simplification
- let layout = undefined; - let layoutPlaceholderItem: Item | null = null; + let layout: Item["layout"] = undefined; if (initialLayout) { const position = findNextAvailablePosition( itemsForLayout, validType, 4, // Default cols name, "", initialLayout.w, initialLayout.h, ); - layout = { lg: position }; - layoutPlaceholderItem = { - id, - type: validType, - name: finalName, - subtitle: "", - data: baseData as ItemData, - color: getRandomCardColor(), - folderId: activeFolderId ?? undefined, - layout, - }; - - // Add placeholder item to layout tracking array so next item doesn't overlap - itemsForLayout.push(layoutPlaceholderItem); } const newItem: Item = { id, type: validType, name: finalName, subtitle: "", data: mergedData as ItemData, color: getRandomCardColor(), folderId: activeFolderId ?? undefined, layout, }; + + // Track for layout calculation before returning + if (layout) { + itemsForLayout.push(newItem); + } + itemsSoFar.push(newItem); return newItem;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/workspace/use-workspace-operations.ts` around lines 398 - 422, The code creates both layoutPlaceholderItem and newItem and calls getRandomCardColor() twice; instead construct newItem first (with id, validType, finalName, mergedData, color via getRandomCardColor(), folderId and layout) and then, when computing positions for findNextAvailablePosition, push newItem into itemsForLayout instead of creating layoutPlaceholderItem so the color and other fields are consistent and you don't call getRandomCardColor() twice; update references to layoutPlaceholderItem to use newItem and remove the unused placeholder creation.src/components/assistant-ui/CreateQuizToolUI.tsx (2)
78-84: useMemo dependency array prevents React Compiler optimization.Same issue as
CreateFlashcardToolUI— optional chaining in dependencies prevents the compiler from preserving memoization.♻️ Proposed fix
const folderName = useMemo(() => { if (!currentItem?.folderId || !workspaceState?.items) return null; const folder = workspaceState.items.find( (item: Item) => item.id === currentItem.folderId, ); return folder?.name || null; - }, [currentItem?.folderId, workspaceState?.items]); + }, [currentItem, workspaceState]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/CreateQuizToolUI.tsx` around lines 78 - 84, The useMemo for folderName in CreateQuizToolUI uses optional chaining in its dependency array which prevents React's compiler optimizations; change the dependency array to reference the stable objects/values directly (e.g., currentItem and workspaceState or currentItem.folderId and workspaceState.items without optional chaining) so React can preserve memoization, ensuring the memo checks run against concrete dependencies used inside the callback (update the useMemo dependencies for folderName accordingly).
58-75: Redundant and overly broad dependencies incurrentItemuseMemo.The dependency array includes both
resultandresult.itemId/result.quizId. Sinceresultalready encompasses its properties, including both is redundant and may cause unnecessary recalculations.♻️ Proposed simplification
const currentItem = useMemo(() => { const targetId = result.itemId || result.quizId; if (!targetId) return undefined; const fromWorkspace = workspaceState?.items?.find( (item: Item) => item.id === targetId, ); if (fromWorkspace) return fromWorkspace; const fromAll = allItems.find((item: Item) => item.id === targetId); if (fromAll) return fromAll; return { id: targetId, name: (result as { title?: string }).title ?? "Quiz", type: "quiz" as const, subtitle: "", data: {}, folderId: undefined, }; - }, [result.itemId, result.quizId, result, workspaceState?.items, allItems]); + }, [result, workspaceState, allItems]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/CreateQuizToolUI.tsx` around lines 58 - 75, The useMemo for currentItem has redundant dependencies (both result and result.itemId/result.quizId); keep a single stable dependency for result to avoid unnecessary recalcs. Update the dependency array of the currentItem useMemo to remove result.itemId and result.quizId and only include result, workspaceState?.items, and allItems (i.e., useMemo(..., [result, workspaceState?.items, allItems])). This targets the currentItem computation that reads result.itemId/result.quizId and ensures correct re-evaluation when result changes without duplicating properties.src/components/assistant-ui/CreateFlashcardToolUI.tsx (2)
43-52: Consider aligningstatustype withCreateQuizToolUIfor consistency.
CreateQuizToolUIdefines a properToolStatusunion type, but this file uses a looser{ type?: string }. Extracting a shared type would improve type safety and consistency.♻️ Suggested shared type
+type ToolStatus = { + type: "complete" | "running" | "incomplete" | "requires-action"; + reason?: string; +}; + interface CreateFlashcardReceiptProps { args: CreateFlashcardArgs; result: FlashcardResult; - status: { type?: string }; + status: ToolStatus; moveItemToFolder?: (itemId: string, folderId: string | null) => void; allItems?: Item[];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/CreateFlashcardToolUI.tsx` around lines 43 - 52, The status field in CreateFlashcardReceiptProps is currently typed loosely as { type?: string }; change it to use the same ToolStatus union used by CreateQuizToolUI (or extract a new shared ToolStatus type and import it) so CreateFlashcardReceiptProps.status: ToolStatus; update references to status in CreateFlashcardReceiptProps usage and the CreateFlashcardToolUI component to match the stricter union type; ensure the shared type is exported from where CreateQuizToolUI defines it (or a common types file) and imported into CreateFlashcardToolUI to keep types consistent.
72-84: useMemo dependency arrays prevent React Compiler optimization.The React Compiler cannot preserve memoization when dependencies use optional chaining (
workspaceState?.items,currentItem?.folderId). Use stable references instead:♻️ Proposed fix
const currentItem = useMemo(() => { if (!result.itemId || !workspaceState?.items) return undefined; return workspaceState.items.find((item: Item) => item.id === result.itemId); - }, [result.itemId, workspaceState?.items]); + }, [result.itemId, workspaceState]); // Get folder name if item is in a folder const folderName = useMemo(() => { if (!currentItem?.folderId || !workspaceState?.items) return null; const folder = workspaceState.items.find( (item: Item) => item.id === currentItem.folderId, ); return folder?.name || null; - }, [currentItem?.folderId, workspaceState?.items]); + }, [currentItem, workspaceState]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/CreateFlashcardToolUI.tsx` around lines 72 - 84, The useMemo hooks for currentItem and folderName use optional-chained dependencies (workspaceState?.items, currentItem?.folderId) which breaks React Compiler optimizations; fix by introducing stable refs and using them in the dependency arrays—e.g. extract const items = workspaceState?.items || [] and use items in the currentItem useMemo deps, and use currentItem and items (not optional-chained expressions) in the folderName useMemo deps; update references to workspaceState?.items and currentItem?.folderId in the dependency arrays to these stable variables (currentItem, items) so the compiler can preserve memoization.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/components/assistant-ui/CreateFlashcardToolUI.tsx`:
- Around line 1-306: This file is 306 lines (over the 300-line limit); extract
the CreateFlashcardReceipt component into its own file (e.g.,
CreateFlashcardReceipt.tsx) and import it into CreateFlashcardToolUI.tsx to
reduce file length. Move the CreateFlashcardReceipt function and its
CreateFlashcardReceiptProps interface into the new file, ensure necessary hooks
and utilities used inside it (useWorkspaceStore, useWorkspaceState,
useNavigateToItem, MoveToDialog, toast, Button, Eye, FolderInput, cn, logger,
Item type) are imported there, export the component (default or named) and
update the original file to import CreateFlashcardReceipt, keeping the
CreateFlashcardArgs type, parseFlashcardResult logic, and CreateFlashcardToolUI
export unchanged. Ensure prop names and types match exactly (args, result,
status, moveItemToFolder, allItems, workspaceName, workspaceIcon,
workspaceColor) so callers (CreateFlashcardToolUI) compile without changes.
In `@src/hooks/workspace/use-workspace-operations.ts`:
- Around line 862-928: createFolderWithItems closes over currentState.items
which can become stale; update the hook so it reads fresh items instead of
relying on the captured array: either add currentState.items to the dependency
array of createFolderWithItems or replace the direct access to
currentState.items with a call to getLatestItemsFromState() inside
createFolderWithItems; reference the function name createFolderWithItems and the
symbol currentState.items (or getLatestItemsFromState) and ensure the dependency
array includes the chosen dependency (or remove the dependency if you switch to
the getter) so the callback always uses up-to-date items.
---
Duplicate comments:
In `@src/components/assistant-ui/AssistantTextSelectionManager.tsx`:
- Around line 1-1274: This file is too large; split responsibilities into
smaller modules: extract selection logic (handleSelectionChange,
handleHighlightAdded, handleHighlightsChange, extractMessageContext,
extractUserPrompt, handleKeyDown, and related state/hooks) into a new
use-text-selection.ts hook used by AssistantTextSelectionManager; move marker
DOM helpers (createMarkerElementAtRange, removeMarkerElement,
multiModeMarkerElementRef, markerToMouseOffsetRef and related refs/cleanup) into
marker-utils.ts with exported functions createMarkerAtRange and removeMarker;
move multi-select and negative-feedback UI into MultiSelectDialog.tsx and
NegativeFeedbackDialog.tsx components (use props for multiSelectionsForDialog,
handlers like handleSubmitMultiSelections, handleCancelMultiDialog, show state
setters); and factor tooltip actions (tooltipActions, highlightAction) into a
small helper file (tooltip-actions.tsx) that exports actions builders; update
AssistantTextSelectionManager to import these modules and keep only
composition/UI glue and refs (selectableTextRef, tooltip visibility, and
rendering). Ensure to preserve symbol names used above (e.g., handleMultiSelect,
handleReply, clearAllHighlights, handleCreateDocument, setTriggerAddHighlight,
pendingContexts) so the refactor is minimal for callers.
---
Nitpick comments:
In `@src/components/assistant-ui/AssistantTextSelectionManager.tsx`:
- Around line 400-434: The handleHighlightAdded callback includes an unused
dependency (tooltipPosition) causing unnecessary re-creations; update the
dependency array for the handleHighlightAdded useCallback (the function defined
as handleHighlightAdded) to remove tooltipPosition and retain only the actual
dependencies used (setTooltipVisible and multiModeBasePosition), ensuring the
array matches the referenced symbols inside the callback and satisfies hook lint
rules.
- Line 1163: In the AssistantTextSelectionManager component's JSX (look for the
return/render of AssistantTextSelectionManager), there is an empty <div> with
classes "pt-4 border-t space-y-3"; either remove this redundant spacer div or
replace it with a semantic element (e.g., an <hr> or a <div aria-hidden="true">
with a short inline comment) and if you keep it, add a concise comment above
explaining its purpose. Update the JSX where that empty div occurs to eliminate
unused markup or clarify intent so reviewers understand why it remains.
In `@src/components/assistant-ui/CreateFlashcardToolUI.tsx`:
- Around line 43-52: The status field in CreateFlashcardReceiptProps is
currently typed loosely as { type?: string }; change it to use the same
ToolStatus union used by CreateQuizToolUI (or extract a new shared ToolStatus
type and import it) so CreateFlashcardReceiptProps.status: ToolStatus; update
references to status in CreateFlashcardReceiptProps usage and the
CreateFlashcardToolUI component to match the stricter union type; ensure the
shared type is exported from where CreateQuizToolUI defines it (or a common
types file) and imported into CreateFlashcardToolUI to keep types consistent.
- Around line 72-84: The useMemo hooks for currentItem and folderName use
optional-chained dependencies (workspaceState?.items, currentItem?.folderId)
which breaks React Compiler optimizations; fix by introducing stable refs and
using them in the dependency arrays—e.g. extract const items =
workspaceState?.items || [] and use items in the currentItem useMemo deps, and
use currentItem and items (not optional-chained expressions) in the folderName
useMemo deps; update references to workspaceState?.items and
currentItem?.folderId in the dependency arrays to these stable variables
(currentItem, items) so the compiler can preserve memoization.
In `@src/components/assistant-ui/CreateQuizToolUI.tsx`:
- Around line 78-84: The useMemo for folderName in CreateQuizToolUI uses
optional chaining in its dependency array which prevents React's compiler
optimizations; change the dependency array to reference the stable
objects/values directly (e.g., currentItem and workspaceState or
currentItem.folderId and workspaceState.items without optional chaining) so
React can preserve memoization, ensuring the memo checks run against concrete
dependencies used inside the callback (update the useMemo dependencies for
folderName accordingly).
- Around line 58-75: The useMemo for currentItem has redundant dependencies
(both result and result.itemId/result.quizId); keep a single stable dependency
for result to avoid unnecessary recalcs. Update the dependency array of the
currentItem useMemo to remove result.itemId and result.quizId and only include
result, workspaceState?.items, and allItems (i.e., useMemo(..., [result,
workspaceState?.items, allItems])). This targets the currentItem computation
that reads result.itemId/result.quizId and ensures correct re-evaluation when
result changes without duplicating properties.
In `@src/components/onboarding/WorkspaceInstructionModal.tsx`:
- Around line 1-545: The file is over the 300-line limit; extract the carousel
logic and large constants to shrink WorkspaceInstructionModal: move the
useCarousel hook (function useCarousel) into a new module (e.g., export default
or named export from use-carousel.ts) and move the STEPS array (const STEPS and
VIDEO_BASE) into a constants module (e.g., onboarding-steps.ts) that exports the
array and any related types, then import them into WorkspaceInstructionModal and
update references (useCarousel, STEPS, VIDEO_BASE) accordingly so
WorkspaceInstructionModal only contains the component UI and wiring.
In `@src/hooks/workspace/use-workspace-operations.ts`:
- Around line 398-422: The code creates both layoutPlaceholderItem and newItem
and calls getRandomCardColor() twice; instead construct newItem first (with id,
validType, finalName, mergedData, color via getRandomCardColor(), folderId and
layout) and then, when computing positions for findNextAvailablePosition, push
newItem into itemsForLayout instead of creating layoutPlaceholderItem so the
color and other fields are consistent and you don't call getRandomCardColor()
twice; update references to layoutPlaceholderItem to use newItem and remove the
unused placeholder creation.
In `@src/hooks/workspace/use-workspace-presence.ts`:
- Around line 96-106: The subscription callback for channel.subscribe currently
only handles the "SUBSCRIBED" branch and does not handle other statuses or
promise rejections from channel.track; update the channel.subscribe handler to
(1) handle non-"SUBSCRIBED" statuses (e.g., "FAILED", "UNSUBSCRIBED") and log or
set a failure state, and (2) wrap the await channel.track({...}) call in
try/catch to handle rejections from track()—use clientKey, currentUser
(id/name/image) and joinedAtRef to provide context in error logs and optionally
trigger a retry or mark presence as failed so the app doesn't silently ignore
subscription/track failures.
- Line 109: The effect in useWorkspacePresence that depends on currentUser
causes unnecessary re-runs because currentUser is an unstable object from
RealtimeContext; change the dependency to stable primitive values (e.g.,
destructure and depend on currentUser.id and any specific metadata like
currentUser.name/email or a serialized string) instead of the whole currentUser
object, so the channel is only recreated when those actual values change; update
the dependency array in the effect inside useWorkspacePresence to use those
primitives (keeping workspaceId, clientKey, cleanup) or alternatively memoize
currentUser in RealtimeProvider to make the object stable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 18ffdbb1-a3d7-4fe8-9f0d-a9f263efd150
📒 Files selected for processing (7)
src/components/assistant-ui/AssistantTextSelectionManager.tsxsrc/components/assistant-ui/CreateDocumentToolUI.tsxsrc/components/assistant-ui/CreateFlashcardToolUI.tsxsrc/components/assistant-ui/CreateQuizToolUI.tsxsrc/components/onboarding/WorkspaceInstructionModal.tsxsrc/hooks/workspace/use-workspace-operations.tssrc/hooks/workspace/use-workspace-presence.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/assistant-ui/CreateDocumentToolUI.tsx
Contributor
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Contributor
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
This was referenced Apr 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary by CodeRabbit
New Features
Bug Fixes
UI/UX Updates
Removed