Refactor: Split thread components and add chat runtime ACL - #406
Conversation
… add chat runtime ACL
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 36 minutes and 33 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughIntroduces a chat runtime abstraction at Changes
Sequence Diagram(s)sequenceDiagram
%% colors use rgba(..., 0.5)
participant User as "User (PromptInput UI)"
participant PromptInput as "PromptInput\n(usePromptInput)"
participant Processor as "processPdfAttachmentsInBackground"
participant Uploader as "uploadSelectedFiles"
participant Ops as "WorkspaceOperations.createItems"
participant OCR as "startAssetProcessing"
participant UI as "Toasts / UI"
User->>PromptInput: Submit with PDF attachments
PromptInput->>Processor: call processPdfAttachmentsInBackground(pdfAttachments, workspaceId, operations)
Processor->>Uploader: uploadSelectedFiles(files)
Uploader-->>Processor: uploaded assets (success/fail)
Processor->>Ops: createItems(converted assets)
Ops-->>Processor: created item IDs
Processor->>OCR: startAssetProcessing(workspaceId, assets, itemIds)
OCR-->>Processor: processing started
alt all uploads succeeded
Processor->>UI: show success toast (assets processed)
else partial failures
Processor->>UI: show warning toast
else no uploads
Processor->>UI: show error toast
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 29 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/lib/chat/runtime/types.ts">
<violation number="1" location="src/lib/chat/runtime/types.ts:11">
P2: The catch-all `type: string` variant defeats discriminated-union narrowing for `ChatMessagePart`, so type guards like `part.type === "text"` no longer safely expose variant-specific fields.</violation>
</file>
<file name="src/components/assistant-ui/thread/EditPromptInput.tsx">
<violation number="1" location="src/components/assistant-ui/thread/EditPromptInput.tsx:35">
P2: Guard unchanged text in `onSubmit`; currently keyboard form submit can bypass the disabled Update button and still send an unchanged edit.</violation>
</file>
<file name="src/components/assistant-ui/thread/UserActionBar.tsx">
<violation number="1" location="src/components/assistant-ui/thread/UserActionBar.tsx:24">
P2: Only set `copied` after `clipboard.writeText` succeeds; currently failures still show a successful copy state.</violation>
</file>
<file name="src/components/assistant-ui/thread/PromptInputToolbar.tsx">
<violation number="1" location="src/components/assistant-ui/thread/PromptInputToolbar.tsx:125">
P2: Avoid nesting `Button` inside `Link`; render the link as the button element via `asChild` to prevent invalid nested interactive elements.</violation>
</file>
<file name="src/components/assistant-ui/thread/hooks/use-prompt-input-paste.ts">
<violation number="1" location="src/components/assistant-ui/thread/hooks/use-prompt-input-paste.ts:17">
P2: Guard `promptInput` before handling paste attachments; otherwise paste is prevented but nothing is uploaded when `promptInput` is null.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| return; | ||
| } | ||
|
|
||
| promptInput?.send(); |
There was a problem hiding this comment.
P2: Guard unchanged text in onSubmit; currently keyboard form submit can bypass the disabled Update button and still send an unchanged edit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/assistant-ui/thread/EditPromptInput.tsx, line 35:
<comment>Guard unchanged text in `onSubmit`; currently keyboard form submit can bypass the disabled Update button and still send an unchanged edit.</comment>
<file context>
@@ -0,0 +1,81 @@
+ return;
+ }
+
+ promptInput?.send();
+ }}
+ >
</file context>
| promptInput?.send(); | |
| if (currentText === originalText) { | |
| return; | |
| } | |
| promptInput?.send(); |
|
|
||
| const handleCopy = useCallback(() => { | ||
| if (!textContent) return; | ||
| navigator.clipboard.writeText(textContent); |
There was a problem hiding this comment.
P2: Only set copied after clipboard.writeText succeeds; currently failures still show a successful copy state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/assistant-ui/thread/UserActionBar.tsx, line 24:
<comment>Only set `copied` after `clipboard.writeText` succeeds; currently failures still show a successful copy state.</comment>
<file context>
@@ -0,0 +1,46 @@
+
+ const handleCopy = useCallback(() => {
+ if (!textContent) return;
+ navigator.clipboard.writeText(textContent);
+ setCopied(true);
+ if (copyTimeoutRef.current) clearTimeout(copyTimeoutRef.current);
</file context>
| Your AI chats won't save unless you are logged in. | ||
| </p> | ||
| <div className="flex items-center gap-2"> | ||
| <Link href="/auth/sign-in" className="flex-1"> |
There was a problem hiding this comment.
P2: Avoid nesting Button inside Link; render the link as the button element via asChild to prevent invalid nested interactive elements.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/assistant-ui/thread/PromptInputToolbar.tsx, line 125:
<comment>Avoid nesting `Button` inside `Link`; render the link as the button element via `asChild` to prevent invalid nested interactive elements.</comment>
<file context>
@@ -0,0 +1,187 @@
+ Your AI chats won't save unless you are logged in.
+ </p>
+ <div className="flex items-center gap-2">
+ <Link href="/auth/sign-in" className="flex-1">
+ <Button
+ variant="outline"
</file context>
Greptile SummarySplits the 1510-line Confidence Score: 5/5Safe to merge — all findings are P2 style/quality suggestions with no present behavioural defects. The PR is a pure structural refactor with zero logic changes. Old consumers of Thread continue to work via the re-export barrel. Renamed attachment exports have no remaining callers of the old names. The three inline comments are all P2. src/lib/chat/runtime/hooks.ts (any-typed selectors) and src/components/assistant-ui/thread/hooks/use-prompt-input-paste.ts (missing useCallback) Important Files Changed
Reviews (1): Last reviewed commit: "Split thread.tsx into components, rename..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return async function handlePaste(e: ClipboardEvent<HTMLTextAreaElement>) { | ||
| const clipboardData = e.clipboardData; | ||
| if (!clipboardData || !workspaceId) return; | ||
|
|
||
| const files = Array.from(clipboardData.files) as File[]; | ||
|
|
||
| if (files.length > 0) { | ||
| e.preventDefault(); | ||
| const imageFile = files.find((file: File) => file.type.startsWith("image/")); | ||
| const fileToUpload = imageFile || files[0]; | ||
|
|
||
| if (fileToUpload) { | ||
| try { | ||
| await promptInput?.addAttachment(fileToUpload); | ||
| } catch (error) { | ||
| console.error("Failed to add file attachment:", error); | ||
| } | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| const clipboardItems = Array.from(clipboardData.items) as DataTransferItem[]; | ||
| const imageItem = clipboardItems.find((item: DataTransferItem) => | ||
| item.type.startsWith("image/"), | ||
| ); | ||
|
|
||
| if (imageItem) { | ||
| e.preventDefault(); | ||
| const file = imageItem.getAsFile(); | ||
| if (file) { | ||
| try { | ||
| await promptInput?.addAttachment(file); | ||
| } catch (error) { | ||
| console.error("Failed to add image attachment:", error); | ||
| } | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
usePromptInputPaste returns a plain async function directly, not wrapped in useCallback. This means handlePaste is a new reference on every render of PromptInput, causing the onPaste prop of ChatPromptInput.Input to change unnecessarily. Wrap the returned function in useCallback with [promptInput, workspaceId] as dependencies to stabilise the reference.
| return useAuiState((s: any) => ({ | ||
| messageCount: s.thread?.messages?.length ?? 0, | ||
| isLoading: !!s.thread?.isLoading, | ||
| isEmpty: s.thread?.isEmpty ?? true, | ||
| isRunning: !!s.thread?.isRunning, | ||
| })); | ||
| } | ||
|
|
||
| export function useIsThreadLoading(): boolean { | ||
| return useAuiState((s: any) => !!s.thread?.isLoading); | ||
| } | ||
|
|
||
| export function useIsThreadEmpty(): boolean { | ||
| return useAuiState((s: any) => s.thread?.isEmpty ?? true); | ||
| } | ||
|
|
||
| export function useIsThreadRunning(): boolean { | ||
| return useAuiState((s: any) => !!s.thread?.isRunning); | ||
| } | ||
|
|
||
| export function useThreadMessageCount(): number { | ||
| return useAuiState((s: any) => s.thread?.messages?.length ?? 0); | ||
| } | ||
|
|
||
| export function useMainThreadId(): string | null { | ||
| return useAuiState((s: any) => s.threads?.mainThreadId ?? null); | ||
| } |
There was a problem hiding this comment.
any type in state selectors
All useAuiState selectors annotate the state argument as (s: any), which discards the type-safety the ACL layer is meant to provide. If @assistant-ui/react exports the state shape, prefer that. At minimum, a local interface describing the accessed fields would prevent silent typos from reaching production.
| const [originalText] = useState<string>(() => promptInput?.getState()?.text ?? ""); | ||
| const [currentText, setCurrentText] = useState<string>( | ||
| () => promptInput?.getState()?.text ?? "", | ||
| ); |
There was a problem hiding this comment.
useState initialiser reads from potentially-null promptInput
Both originalText and currentText call promptInput?.getState()?.text ?? "" inside their lazy initialisers. If usePromptInput() returns null at first render, both values will be "", and the "Update" button will remain permanently disabled because currentText === originalText. Consider using a useEffect to set originalText after mount, or asserting the context is non-null before rendering.
…l-group, assistant-loader, file, image, sources, tool-fallback) to chat-runtime ACL
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (11)
src/lib/chat/runtime/primitives.ts (1)
10-20: Remove redundant type import and use safer type derivation.
BranchPickerPrimitiveis already imported as a value (line 4), so the type-only import on line 10 is redundant. Additionally, the.Root.Propsnamespace type path is not directly exposed in the@assistant-ui/reactAPI—props are defined inline on the Root component. UseComponentPropsfor safer, version-agnostic type derivation:♻️ Suggested fix
import { ActionBarPrimitive, AuiIf, BranchPickerPrimitive, ComposerPrimitive, ErrorPrimitive, MessagePrimitive, ThreadPrimitive, } from "@assistant-ui/react"; -import type { BranchPickerPrimitive as _BPP } from "@assistant-ui/react"; export const ChatThread = ThreadPrimitive; export const ChatMessage = MessagePrimitive; export const ChatPromptInput = ComposerPrimitive; export const ChatActionBar = ActionBarPrimitive; export const ChatBranchPicker = BranchPickerPrimitive; export const ChatError = ErrorPrimitive; export const ChatIf = AuiIf; -export type ChatBranchPickerRootProps = _BPP.Root.Props; +export type ChatBranchPickerRootProps = React.ComponentProps<typeof BranchPickerPrimitive.Root>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/chat/runtime/primitives.ts` around lines 10 - 20, Remove the redundant type-only import of BranchPickerPrimitive and change ChatBranchPickerRootProps to derive props via React.ComponentProps from the value import: stop using the _BPP.Root.Props namespace and instead import BranchPickerPrimitive as a value (already present), then set ChatBranchPickerRootProps to the ComponentProps of BranchPickerPrimitive.Root (i.e., derive props from the actual Root component) so the type is version-agnostic and safe; update any import statements to remove the unused type import.src/components/assistant-ui/thread.tsx (1)
1-1: Siblingthread.tsxandthread/can trip some tooling; consider consolidating post-migration.Having both
src/components/assistant-ui/thread.tsxand asrc/components/assistant-ui/thread/directory as siblings is legal under modern TS/bundler module resolution, but it is an ambiguous pattern for some linters, IDEs, and legacy tooling (e.g., they may resolve"@/components/assistant-ui/thread"to the file vs. the directory inconsistently). Once external import sites are confirmed, consider deletingthread.tsxand lettingthread/index.tsown the module path, or keep only the file and collapse the directory. Fine to defer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/thread.tsx` at line 1, There are two sibling modules exposing the same module name (the exported symbol "Thread" and a sibling directory named "thread"), which can confuse tooling; pick one canonical module and remove the other: either delete the redundant thread.tsx file and let the thread/index entry export "Thread" (update any imports to the directory entry if needed) or collapse the thread/ directory into a single thread.tsx and update its export accordingly; confirm and update external import sites to reference the chosen module path consistently and run linters/build to ensure no remaining ambiguous imports.src/components/assistant-ui/thread/suggestion-actions.ts (1)
5-47: Use a discriminated union type withsatisfiesto strengthen type safety and catch potential typos.The array contains two distinct shapes:
- Dialog-driven entries with
action+useDialog: true- File-input entry with
triggerFileInput: trueWithout an explicit discriminated union, TypeScript infers a widened shape, and the
"search" as PromptBuilderActioncasts silently coerce values. Usingsatisfiesagainst a discriminated union would validate each entry at compile time without theasassertion hiding potential errors. Additionally, this would catch thatcomposerFillis checked by consumers but never provided by any entry here.🔧 Sketch
import type { ComponentType } from "react"; type IconComponent = ComponentType<{ className?: string }>; type DialogSuggestion = { title: string; icon: IconComponent; iconClassName: string; action: PromptBuilderAction; useDialog: true; }; type FileInputSuggestion = { title: string; icon: IconComponent; iconClassName: string; triggerFileInput: true; }; export type SuggestionAction = DialogSuggestion | FileInputSuggestion; export const SUGGESTION_ACTIONS = [ { title: "Search", icon: Search, iconClassName: "size-4 shrink-0 text-sky-500", action: "search", useDialog: true }, // ... { title: "Upload", icon: Upload, iconClassName: "size-4 shrink-0 text-red-400", triggerFileInput: true }, ] as const satisfies readonly SuggestionAction[];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/thread/suggestion-actions.ts` around lines 5 - 47, SUGGESTION_ACTIONS is currently a loosely typed array using "as PromptBuilderAction" casts which hides typos and misses the expected discriminator (e.g., composerFill is referenced by consumers but not provided); define a discriminated union type (e.g., DialogSuggestion with action: PromptBuilderAction and useDialog: true, and FileInputSuggestion with triggerFileInput: true), export a SuggestionAction = DialogSuggestion | FileInputSuggestion, then type SUGGESTION_ACTIONS using "as const satisfies readonly SuggestionAction[]" so each entry (like the ones with action/search, youtube, flashcards, quiz, document and the Upload entry with triggerFileInput) is validated at compile time and the casted "as PromptBuilderAction" can be removed.src/components/assistant-ui/thread/PromptInputShell.tsx (1)
156-163: Minor: conditional mount skips dialog exit animation.
{dialogAction && <PromptBuilderDialog ... />}unmounts the dialog the instantdialogActionflips tonull, so any close/exit transition inPromptBuilderDialogwon't get a chance to play. Also,open={!!dialogAction}is redundant under the outer guard. If you want smooth close behavior, keep the dialog mounted and drive visibility viaopen:♻️ Proposed tweak
- {dialogAction && ( - <PromptBuilderDialog - open={!!dialogAction} - onOpenChange={(open) => !open && setDialogAction(null)} - action={dialogAction} - items={items} - /> - )} + <PromptBuilderDialog + open={dialogAction !== null} + onOpenChange={(open) => !open && setDialogAction(null)} + action={dialogAction ?? undefined} + items={items} + />This assumes
PromptBuilderDialogtolerates a nullishactionwhile closed; otherwise, leaving the current form is fine.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/thread/PromptInputShell.tsx` around lines 156 - 163, The dialog is being unmounted immediately by the outer conditional which prevents exit animations; remove the conditional mount and always render <PromptBuilderDialog ... /> inside PromptInputShell.tsx, drive visibility solely via the open prop (open={!!dialogAction}), keep onOpenChange={(open) => !open && setDialogAction(null)} and pass action={dialogAction} (allowing null when closed), and keep items as-is so the dialog can remain mounted and play its close transition.src/components/assistant-ui/thread/PromptInputToolbar.tsx (2)
53-68: Nit: hardcodedlocalhost:4983for AI Debug.The URL is only reachable in
NODE_ENV === "development", so it's safe, but consider pulling the port into an env var (e.g.NEXT_PUBLIC_AI_DEBUG_URL) so dev setups running on a different port don't have to patch source. Low priority.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/thread/PromptInputToolbar.tsx` around lines 53 - 68, The AI Debug button currently hardcodes "http://localhost:4983" inside the onClick handler; update the onClick in the showAiDebugButton block so it reads the debug URL from an environment variable (e.g. NEXT_PUBLIC_AI_DEBUG_URL) with a sensible fallback (http://localhost:4983), and continue to gate opening in development mode while preserving the existing else branch that calls setIsFeedbackDialogOpen(true); modify the handler around the Bug button/component and the onClick logic to use process.env.NEXT_PUBLIC_AI_DEBUG_URL || "http://localhost:4983" instead of the literal string.
73-147: Optional: de-duplicate the hover handlers on trigger and content.The four hover callbacks (lines 86-96 and 107-117) are identical pairs. Extracting
openImmediately/scheduleClosehandlers (or wrapping the trigger+content in a parent withonMouseEnter/onMouseLeave) would shrink this block and eliminate drift if the 100ms delay is ever tuned. Not blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/thread/PromptInputToolbar.tsx` around lines 73 - 147, The hover enter/leave logic is duplicated between the PopoverTrigger and PopoverContent; extract shared handlers (e.g., openImmediately and scheduleClose) that call clearTimeout(hoverTimeoutRef.current), setIsWarningPopoverOpen(true) and schedule a setTimeout to setIsWarningPopoverOpen(false) with the 100ms delay, and reuse them on the trigger and content onMouseEnter/onMouseLeave props (also ensure scheduleClose stores the timer back into hoverTimeoutRef.current); update references in the component where isWarningPopoverOpen, setIsWarningPopoverOpen, hoverTimeoutRef, PopoverTrigger and PopoverContent are used so focusComposerInput behavior remains unchanged when the popover closes.src/lib/chat/runtime/hooks.ts (1)
59-71:usePromptInputreturns a new object every render — unstable as a dep.The returned
ComposerActionsobject is freshly constructed on every call, so every consumer that listspromptInputin a hook dependency array (e.g.clearMentionQueryinuse-mention-menu.tsandusePromptInputPaste) will invalidate its memo/callback on every render. Sincecomposeritself is accessed imperatively (no subscription), you can eitheruseMemothe wrapper keyed onaui/composeridentity, or return the rawcomposercast toComposerActions. The former preserves the ACL boundary:♻️ Suggested tweak
export function usePromptInput(): ComposerActions | null { const aui = useAui(); - if (!aui) return null; - const composer = aui.composer?.(); - if (!composer) return null; - return { - setText: (t) => composer.setText(t), - send: () => composer.send(), - addAttachment: (f) => composer.addAttachment(f), - setRunConfig: (cfg) => composer.setRunConfig(cfg as any), - getState: () => composer.getState() as unknown as ComposerStateSnapshot | undefined, - }; + const composer = aui?.composer?.(); + return useMemo<ComposerActions | null>(() => { + if (!composer) return null; + return { + setText: (t) => composer.setText(t), + send: () => composer.send(), + addAttachment: (f) => composer.addAttachment(f), + setRunConfig: (cfg) => composer.setRunConfig(cfg as any), + getState: () => + composer.getState() as unknown as ComposerStateSnapshot | undefined, + }; + }, [composer]); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/chat/runtime/hooks.ts` around lines 59 - 71, usePromptInput currently constructs and returns a new ComposerActions object on every call causing unstable dependencies; change it to memoize the wrapper (using useMemo) keyed on aui and the composer identity (aui.composer()) or simply return the raw composer cast to ComposerActions; locate usePromptInput in src/lib/chat/runtime/hooks.ts and update the return to either wrap the methods in useMemo (dependencies: aui, composer) or return composer as ComposerActions, keeping method names setText, send, addAttachment, setRunConfig and getState intact and preserving the ComposerStateSnapshot cast.src/lib/chat/runtime/types.ts (2)
46-54: Nit: move imports to the top of the file.The
import type { ... } from "@assistant-ui/react"statement is declared after the interface/type exports. It's hoisted so this works, but it hurts scanability and several ESLint presets will flag it. Consider moving it above Line 1.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/chat/runtime/types.ts` around lines 46 - 54, Move the type-only import statement (the one importing TextMessagePartProps, FileMessagePartComponent, ImageMessagePartComponent, SourceMessagePartComponent, ToolCallMessagePartComponent, ReasoningMessagePartComponent, ReasoningGroupComponent from "@assistant-ui/react") to the very top of the file, above the exported interfaces/types declarations so the import appears before any exports; this improves scanability and satisfies ESLint rules that expect imports before exports.
3-18:ChatMessagePartdiscriminated union collapses due to catch-all member.The final union member
{ type: string; [k: string]: unknown }is a supertype of every other branch, so TypeScript reduces the whole union to that one shape. As a consequence, narrowing likeif (part.type === "text") { part.text }won't give youChatTextPart—textis typed asunknown. Either drop the catch-all (and accept that unknown part types become a compile error), or drop the specific branches since they're dead weight. If you do want an "unknown-but-valid" fallback, use a branded type ortype: Exclude<string, "text" | "file" | ...>so the narrowing is preserved.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/chat/runtime/types.ts` around lines 3 - 18, The union ChatMessagePart collapses because the final member { type: string; [k: string]: unknown } is a supertype; fix by removing that catch-all member or replace it with a branded fallback such as { type: Exclude<string, "text" | "file" | "image" | "source" | "reasoning" | "tool-call" | "tool-result" | "tool">; [k: string]: unknown } so the discriminated union (ChatTextPart, etc.) can be properly narrowed; update the ChatMessage.content typing accordingly and ensure code using part.type === "text" can access part.text as a string.src/components/assistant-ui/thread/hooks/use-prompt-input-paste.ts (1)
11-52: Optional: memoize the returned handler.
usePromptInputPastereturns a fresh function on every render. It's fine as-is (React reattaches the listener cheaply), but wrapping the handler inuseCallbackkeyed on[promptInput, workspaceId]would match the idiom used byuseMentionMenuin the sibling hook and keep the returned callback stable for consumers who mightuseMemo/useEffecton it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/thread/hooks/use-prompt-input-paste.ts` around lines 11 - 52, The returned paste handler from usePromptInputPaste should be memoized so it doesn't change every render; wrap the inner async function (handlePaste) in React's useCallback and return that memoized function, with dependencies [promptInput, workspaceId] (mirroring useMentionMenu) so consumers can rely on callback stability; update the import to include useCallback if missing and keep all existing logic inside the memoized function.src/components/assistant-ui/thread/hooks/use-mention-menu.ts (1)
68-76: Optional: consider broader whitespace boundary for mention activation.The boundary check only recognizes
" "and"\n". A@typed after a tab or other whitespace (e.g. after an indent, or after\r) won't open the mention menu — andhandleInput's exit conditions on Lines 48-52 similarly miss tab/\r. Using/\s/would make the two checks consistent and more forgiving:♻️ Suggested tweak
- const charBefore = cursorPos > 0 ? textarea.value[cursorPos - 1] : " "; - if (charBefore === " " || charBefore === "\n" || cursorPos === 0) { + const charBefore = cursorPos > 0 ? textarea.value[cursorPos - 1] : " "; + if (cursorPos === 0 || (charBefore !== undefined && /\s/.test(charBefore))) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/thread/hooks/use-mention-menu.ts` around lines 68 - 76, In use-mention-menu, the activation boundary only checks for " " and "\n" so tabs and other whitespace won't open the menu; update the checks in the keypress handler (where it inspects charBefore and cursorPos) to use a whitespace test such as /\s/.test(charBefore) (keep the existing cursorPos === 0 logic) and make the same change inside handleInput's early-exit conditions so both mention activation and input handling consistently accept any whitespace character (reference symbols: use-mention-menu, mentionMenuOpen, setMentionStartIndex, setMentionQuery, handleInput).
🤖 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/thread/AssistantActionBar.tsx`:
- Around line 4-27: The handleCopy function should be made async and await
navigator.clipboard.writeText, wrapping the call in try/catch to handle
rejections and only call setCopied(true) on success (log or surface the error in
the catch); keep the timeout logic but ensure you clear any existing timeout
before creating a new one. Also add a cleanup effect (useEffect with a return)
that clears copyTimeoutRef.current on unmount to avoid state updates after
unmount; reference AssistantActionBar, handleCopy, and copyTimeoutRef when
making these changes.
In `@src/components/assistant-ui/thread/BranchPicker.tsx`:
- Around line 16-24: The prop spread {...rest} currently appears after the
hard-coded hideWhenSingleBranch on ChatBranchPicker.Root so callers can silently
override that default; change the component to either destructure
hideWhenSingleBranch out of props (so external callers cannot override it) or
spread {...rest} before setting hideWhenSingleBranch last on
ChatBranchPicker.Root (ensuring hideWhenSingleBranch is forced). Update the
BranchPicker component where ChatBranchPicker.Root is rendered to implement one
of these two fixes (reference ChatBranchPicker.Root, hideWhenSingleBranch, and
{...rest}).
In `@src/components/assistant-ui/thread/EditPromptInput.tsx`:
- Around line 27-36: The form onSubmit currently only checks uploads before
calling promptInput?.send(), but the Send button also guards against unchanged
edits; mirror that disabled-state guard in the onSubmit handler so
keyboard/programmatic submits behave identically. Concretely: in the onSubmit
function add the same condition used by the Send button (the same variable or
predicate that computes the button's disabled state—e.g., the sendDisabled /
isUnchanged / canSend check used in the JSX) and return early if it indicates
no-op, before calling promptInput?.send(); keep the existing upload check
(useAttachmentUploadStore.getState().uploadingIds) as well so both guards run.
In `@src/components/assistant-ui/thread/PromptInput.tsx`:
- Around line 87-121: The empty-text fallback is inconsistent: when sending
attachments-only the code sets modifiedText to "" which can cause downstream
rejections. Update the logic around modifiedText (and/or the early guard) so
that when attachments.length > 0 you use the same placeholder as reply context;
e.g. compute modifiedText from currentText.trim() || (hasReplyContext ||
attachments.length > 0 ? "Empty message" : ""), then call
promptInput?.setText(modifiedText) and promptInput?.send() as before (symbols to
edit: currentText, attachments, hasReplyContext, modifiedText,
promptInput.setText, promptInput.send).
In `@src/components/assistant-ui/thread/UserActionBar.tsx`:
- Around line 4-28: The copy handler currently sets copied immediately and
doesn't await failures; update the handleCopy function in UserActionBar to await
navigator.clipboard.writeText(textContent) inside a try/catch, only call
setCopied(true) after a successful await, and handle/log errors in the catch
block (do not set the success state on failure). Also add a useEffect cleanup
that clears the copyTimeoutRef timeout on unmount
(clearTimeout(copyTimeoutRef.current) and null it) to avoid stale state updates;
keep references to copyTimeoutRef and the existing timeout-reset logic.
In `@src/components/assistant-ui/thread/UserMessageTruncateContext.tsx`:
- Around line 20-27: The current truncation uses text.slice(0,
truncateCtx.maxChars) which can split surrogate pairs/grapheme clusters (emojis,
ZWJ sequences); change the truncation in UserMessageTruncateContext to perform
grapheme-aware slicing: iterate or segment the string by grapheme clusters (use
Intl.Segmenter('grapheme') when available, or fallback to Array.from(text) or a
small dependency like grapheme-splitter) and then join the first
truncateCtx.maxChars clusters before appending "..." only when truncated; keep
the existing checks on truncateCtx.expanded and truncateCtx.maxChars.
In `@src/lib/chat/runtime/hooks.ts`:
- Around line 17-24: The exported function useThreadState creates a new object
on every call which forces unnecessary re-renders and is unused; remove the
useThreadState function declaration entirely from src/lib/chat/runtime/hooks.ts
(or replace its body by composing and returning values from the existing
selectors useIsThreadLoading, useIsThreadEmpty, useIsThreadRunning, and
useThreadMessageCount so callers receive stable primitive values) and ensure no
other modules import useThreadState after removal.
In `@src/lib/uploads/process-pdf-attachments-in-background.ts`:
- Around line 40-47: The fire-and-forget call to startAssetProcessing(...) must
attach a .catch to handle rejections from the function itself (in addition to
onOcrError which handles internal OCR candidate errors); update the invocation
of startAssetProcessing (the call that passes workspaceId, assets: uploads,
itemIds: createdIds, onOcrError) to append a .catch handler that logs or reports
the error (e.g., using console.error or processLogger) so any rejection during
setup is not left unhandled.
---
Nitpick comments:
In `@src/components/assistant-ui/thread.tsx`:
- Line 1: There are two sibling modules exposing the same module name (the
exported symbol "Thread" and a sibling directory named "thread"), which can
confuse tooling; pick one canonical module and remove the other: either delete
the redundant thread.tsx file and let the thread/index entry export "Thread"
(update any imports to the directory entry if needed) or collapse the thread/
directory into a single thread.tsx and update its export accordingly; confirm
and update external import sites to reference the chosen module path
consistently and run linters/build to ensure no remaining ambiguous imports.
In `@src/components/assistant-ui/thread/hooks/use-mention-menu.ts`:
- Around line 68-76: In use-mention-menu, the activation boundary only checks
for " " and "\n" so tabs and other whitespace won't open the menu; update the
checks in the keypress handler (where it inspects charBefore and cursorPos) to
use a whitespace test such as /\s/.test(charBefore) (keep the existing cursorPos
=== 0 logic) and make the same change inside handleInput's early-exit conditions
so both mention activation and input handling consistently accept any whitespace
character (reference symbols: use-mention-menu, mentionMenuOpen,
setMentionStartIndex, setMentionQuery, handleInput).
In `@src/components/assistant-ui/thread/hooks/use-prompt-input-paste.ts`:
- Around line 11-52: The returned paste handler from usePromptInputPaste should
be memoized so it doesn't change every render; wrap the inner async function
(handlePaste) in React's useCallback and return that memoized function, with
dependencies [promptInput, workspaceId] (mirroring useMentionMenu) so consumers
can rely on callback stability; update the import to include useCallback if
missing and keep all existing logic inside the memoized function.
In `@src/components/assistant-ui/thread/PromptInputShell.tsx`:
- Around line 156-163: The dialog is being unmounted immediately by the outer
conditional which prevents exit animations; remove the conditional mount and
always render <PromptBuilderDialog ... /> inside PromptInputShell.tsx, drive
visibility solely via the open prop (open={!!dialogAction}), keep
onOpenChange={(open) => !open && setDialogAction(null)} and pass
action={dialogAction} (allowing null when closed), and keep items as-is so the
dialog can remain mounted and play its close transition.
In `@src/components/assistant-ui/thread/PromptInputToolbar.tsx`:
- Around line 53-68: The AI Debug button currently hardcodes
"http://localhost:4983" inside the onClick handler; update the onClick in the
showAiDebugButton block so it reads the debug URL from an environment variable
(e.g. NEXT_PUBLIC_AI_DEBUG_URL) with a sensible fallback
(http://localhost:4983), and continue to gate opening in development mode while
preserving the existing else branch that calls setIsFeedbackDialogOpen(true);
modify the handler around the Bug button/component and the onClick logic to use
process.env.NEXT_PUBLIC_AI_DEBUG_URL || "http://localhost:4983" instead of the
literal string.
- Around line 73-147: The hover enter/leave logic is duplicated between the
PopoverTrigger and PopoverContent; extract shared handlers (e.g.,
openImmediately and scheduleClose) that call
clearTimeout(hoverTimeoutRef.current), setIsWarningPopoverOpen(true) and
schedule a setTimeout to setIsWarningPopoverOpen(false) with the 100ms delay,
and reuse them on the trigger and content onMouseEnter/onMouseLeave props (also
ensure scheduleClose stores the timer back into hoverTimeoutRef.current); update
references in the component where isWarningPopoverOpen, setIsWarningPopoverOpen,
hoverTimeoutRef, PopoverTrigger and PopoverContent are used so
focusComposerInput behavior remains unchanged when the popover closes.
In `@src/components/assistant-ui/thread/suggestion-actions.ts`:
- Around line 5-47: SUGGESTION_ACTIONS is currently a loosely typed array using
"as PromptBuilderAction" casts which hides typos and misses the expected
discriminator (e.g., composerFill is referenced by consumers but not provided);
define a discriminated union type (e.g., DialogSuggestion with action:
PromptBuilderAction and useDialog: true, and FileInputSuggestion with
triggerFileInput: true), export a SuggestionAction = DialogSuggestion |
FileInputSuggestion, then type SUGGESTION_ACTIONS using "as const satisfies
readonly SuggestionAction[]" so each entry (like the ones with action/search,
youtube, flashcards, quiz, document and the Upload entry with triggerFileInput)
is validated at compile time and the casted "as PromptBuilderAction" can be
removed.
In `@src/lib/chat/runtime/hooks.ts`:
- Around line 59-71: usePromptInput currently constructs and returns a new
ComposerActions object on every call causing unstable dependencies; change it to
memoize the wrapper (using useMemo) keyed on aui and the composer identity
(aui.composer()) or simply return the raw composer cast to ComposerActions;
locate usePromptInput in src/lib/chat/runtime/hooks.ts and update the return to
either wrap the methods in useMemo (dependencies: aui, composer) or return
composer as ComposerActions, keeping method names setText, send, addAttachment,
setRunConfig and getState intact and preserving the ComposerStateSnapshot cast.
In `@src/lib/chat/runtime/primitives.ts`:
- Around line 10-20: Remove the redundant type-only import of
BranchPickerPrimitive and change ChatBranchPickerRootProps to derive props via
React.ComponentProps from the value import: stop using the _BPP.Root.Props
namespace and instead import BranchPickerPrimitive as a value (already present),
then set ChatBranchPickerRootProps to the ComponentProps of
BranchPickerPrimitive.Root (i.e., derive props from the actual Root component)
so the type is version-agnostic and safe; update any import statements to remove
the unused type import.
In `@src/lib/chat/runtime/types.ts`:
- Around line 46-54: Move the type-only import statement (the one importing
TextMessagePartProps, FileMessagePartComponent, ImageMessagePartComponent,
SourceMessagePartComponent, ToolCallMessagePartComponent,
ReasoningMessagePartComponent, ReasoningGroupComponent from
"@assistant-ui/react") to the very top of the file, above the exported
interfaces/types declarations so the import appears before any exports; this
improves scanability and satisfies ESLint rules that expect imports before
exports.
- Around line 3-18: The union ChatMessagePart collapses because the final member
{ type: string; [k: string]: unknown } is a supertype; fix by removing that
catch-all member or replace it with a branded fallback such as { type:
Exclude<string, "text" | "file" | "image" | "source" | "reasoning" | "tool-call"
| "tool-result" | "tool">; [k: string]: unknown } so the discriminated union
(ChatTextPart, etc.) can be properly narrowed; update the ChatMessage.content
typing accordingly and ensure code using part.type === "text" can access
part.text as a string.
🪄 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: 417bdc0f-c678-4b97-9997-0ed95dc49863
📒 Files selected for processing (37)
src/components/assistant-ui/assistant-loader.tsxsrc/components/assistant-ui/attachment.tsxsrc/components/assistant-ui/file.tsxsrc/components/assistant-ui/image.tsxsrc/components/assistant-ui/markdown-text.tsxsrc/components/assistant-ui/reasoning.tsxsrc/components/assistant-ui/sources.tsxsrc/components/assistant-ui/thread.tsxsrc/components/assistant-ui/thread/AssistantActionBar.tsxsrc/components/assistant-ui/thread/AssistantMessage.tsxsrc/components/assistant-ui/thread/BranchPicker.tsxsrc/components/assistant-ui/thread/EditPromptInput.tsxsrc/components/assistant-ui/thread/MessageError.tsxsrc/components/assistant-ui/thread/PromptInput.tsxsrc/components/assistant-ui/thread/PromptInputShell.tsxsrc/components/assistant-ui/thread/PromptInputToolbar.tsxsrc/components/assistant-ui/thread/Thread.tsxsrc/components/assistant-ui/thread/ThreadLoadingSkeleton.tsxsrc/components/assistant-ui/thread/ThreadSuggestions.tsxsrc/components/assistant-ui/thread/ThreadWelcome.tsxsrc/components/assistant-ui/thread/UserActionBar.tsxsrc/components/assistant-ui/thread/UserMessage.tsxsrc/components/assistant-ui/thread/UserMessageTruncateContext.tsxsrc/components/assistant-ui/thread/VirtualizedMessages.tsxsrc/components/assistant-ui/thread/hooks/use-mention-menu.tssrc/components/assistant-ui/thread/hooks/use-prompt-input-paste.tssrc/components/assistant-ui/thread/index.tssrc/components/assistant-ui/thread/message-components.tssrc/components/assistant-ui/thread/prompt-input-floating-actions.tssrc/components/assistant-ui/thread/suggestion-actions.tssrc/components/assistant-ui/tool-fallback.tsxsrc/components/assistant-ui/tool-group.tsxsrc/lib/chat/runtime/hooks.tssrc/lib/chat/runtime/index.tssrc/lib/chat/runtime/primitives.tssrc/lib/chat/runtime/types.tssrc/lib/uploads/process-pdf-attachments-in-background.ts
| import { useCallback, useMemo, useRef, useState, type FC } from "react"; | ||
| import { ChatActionBar, useChatMessage } from "@/lib/chat/runtime"; | ||
| import { TooltipIconButton } from "@/components/assistant-ui/tooltip-icon-button"; | ||
|
|
||
| export const AssistantActionBar: FC = () => { | ||
| const { content } = useChatMessage(); | ||
|
|
||
| const textContent = useMemo(() => { | ||
| const textParts = content.filter( | ||
| (part): part is { type: "text"; text: string } => part.type === "text", | ||
| ); | ||
| return textParts.map((part) => part.text ?? "").join("\n\n"); | ||
| }, [content]); | ||
|
|
||
| const [copied, setCopied] = useState(false); | ||
| const copyTimeoutRef = useRef<NodeJS.Timeout | null>(null); | ||
|
|
||
| const handleCopy = useCallback(() => { | ||
| if (!textContent) return; | ||
| navigator.clipboard.writeText(textContent); | ||
| setCopied(true); | ||
| if (copyTimeoutRef.current) clearTimeout(copyTimeoutRef.current); | ||
| copyTimeoutRef.current = setTimeout(() => setCopied(false), 2000); | ||
| }, [textContent]); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
MDN Clipboard writeText return value Promise NotAllowedError
💡 Result:
The MDN documentation for the Clipboard.writeText method states that it returns a Promise that resolves once the clipboard contents have been updated successfully. If writing to the clipboard is not allowed due to security restrictions (such as non-secure context, lack of user activation, missing permissions policy allowance, or document not focused), the Promise rejects with a NotAllowedError DOMException. Key details: - Return value on success: Promise (resolves with undefined). - On failure (not allowed): Rejects with DOMException named "NotAllowedError". - Usage example from MDN: async function writeClipboardText(text) { try { await navigator.clipboard.writeText(text); // Success } catch (error) { if (error.name === 'NotAllowedError') { console.error('Clipboard write not allowed'); } else { console.error(error.message); } } } This requires a secure context (HTTPS) and typically user gesture (transient activation) or clipboard-write permission. Clipboard API security: Writing generally requires either transient user activation (e.g., click handler) or the 'clipboard-write' permission. Browsers may enforce document focus and other policies.
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/Clipboard/writeText
- 2: https://developer.mozilla.org/docs/Web/API/Clipboard/writeText
- 3: https://dev.mozilla.org/en-US/docs/Web/API/Clipboard/writeText
- 4: https://developer.cdn.mozilla.net/en-US/docs/Web/API/Clipboard/writeText
- 5: https://developer.mozilla.org/Web/API/Clipboard_API
Handle clipboard write errors and clear timeout on unmount.
navigator.clipboard.writeText() returns a Promise that must be awaited and can reject due to security restrictions (missing permissions, non-HTTPS context, etc.). The current code treats it as fire-and-forget, marking success immediately and leaving errors unhandled. Additionally, the timeout has no cleanup function, risking late state updates after unmount.
Proposed fix
-import { useCallback, useMemo, useRef, useState, type FC } from "react";
+import { useCallback, useEffect, useMemo, useRef, useState, type FC } from "react";
@@
- const copyTimeoutRef = useRef<NodeJS.Timeout | null>(null);
+ const copyTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
+
+ useEffect(() => {
+ return () => {
+ if (copyTimeoutRef.current) clearTimeout(copyTimeoutRef.current);
+ };
+ }, []);
- const handleCopy = useCallback(() => {
+ const handleCopy = useCallback(async () => {
if (!textContent) return;
- navigator.clipboard.writeText(textContent);
- setCopied(true);
- if (copyTimeoutRef.current) clearTimeout(copyTimeoutRef.current);
- copyTimeoutRef.current = setTimeout(() => setCopied(false), 2000);
+ try {
+ await navigator.clipboard.writeText(textContent);
+ setCopied(true);
+ if (copyTimeoutRef.current) clearTimeout(copyTimeoutRef.current);
+ copyTimeoutRef.current = setTimeout(() => {
+ setCopied(false);
+ copyTimeoutRef.current = null;
+ }, 2000);
+ } catch {
+ setCopied(false);
+ }
}, [textContent]);📝 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.
| import { useCallback, useMemo, useRef, useState, type FC } from "react"; | |
| import { ChatActionBar, useChatMessage } from "@/lib/chat/runtime"; | |
| import { TooltipIconButton } from "@/components/assistant-ui/tooltip-icon-button"; | |
| export const AssistantActionBar: FC = () => { | |
| const { content } = useChatMessage(); | |
| const textContent = useMemo(() => { | |
| const textParts = content.filter( | |
| (part): part is { type: "text"; text: string } => part.type === "text", | |
| ); | |
| return textParts.map((part) => part.text ?? "").join("\n\n"); | |
| }, [content]); | |
| const [copied, setCopied] = useState(false); | |
| const copyTimeoutRef = useRef<NodeJS.Timeout | null>(null); | |
| const handleCopy = useCallback(() => { | |
| if (!textContent) return; | |
| navigator.clipboard.writeText(textContent); | |
| setCopied(true); | |
| if (copyTimeoutRef.current) clearTimeout(copyTimeoutRef.current); | |
| copyTimeoutRef.current = setTimeout(() => setCopied(false), 2000); | |
| }, [textContent]); | |
| import { useCallback, useEffect, useMemo, useRef, useState, type FC } from "react"; | |
| import { ChatActionBar, useChatMessage } from "@/lib/chat/runtime"; | |
| import { TooltipIconButton } from "@/components/assistant-ui/tooltip-icon-button"; | |
| export const AssistantActionBar: FC = () => { | |
| const { content } = useChatMessage(); | |
| const textContent = useMemo(() => { | |
| const textParts = content.filter( | |
| (part): part is { type: "text"; text: string } => part.type === "text", | |
| ); | |
| return textParts.map((part) => part.text ?? "").join("\n\n"); | |
| }, [content]); | |
| const [copied, setCopied] = useState(false); | |
| const copyTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); | |
| useEffect(() => { | |
| return () => { | |
| if (copyTimeoutRef.current) clearTimeout(copyTimeoutRef.current); | |
| }; | |
| }, []); | |
| const handleCopy = useCallback(async () => { | |
| if (!textContent) return; | |
| try { | |
| await navigator.clipboard.writeText(textContent); | |
| setCopied(true); | |
| if (copyTimeoutRef.current) clearTimeout(copyTimeoutRef.current); | |
| copyTimeoutRef.current = setTimeout(() => { | |
| setCopied(false); | |
| copyTimeoutRef.current = null; | |
| }, 2000); | |
| } catch { | |
| setCopied(false); | |
| } | |
| }, [textContent]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/assistant-ui/thread/AssistantActionBar.tsx` around lines 4 -
27, The handleCopy function should be made async and await
navigator.clipboard.writeText, wrapping the call in try/catch to handle
rejections and only call setCopied(true) on success (log or surface the error in
the catch); keep the timeout logic but ensure you clear any existing timeout
before creating a new one. Also add a cleanup effect (useEffect with a return)
that clears copyTimeoutRef.current on unmount to avoid state updates after
unmount; reference AssistantActionBar, handleCopy, and copyTimeoutRef when
making these changes.
| onSubmit={(e) => { | ||
| e.preventDefault(); | ||
|
|
||
| if (useAttachmentUploadStore.getState().uploadingIds.size > 0) { | ||
| toast.info("Please wait for uploads to finish before sending"); | ||
| return; | ||
| } | ||
|
|
||
| promptInput?.send(); | ||
| }} |
There was a problem hiding this comment.
Mirror the disabled-state guard in onSubmit.
The button disables unchanged edits, but form-level submit can still call send() via keyboard/programmatic submission. Gate the submit handler with the same condition.
Proposed fix
onSubmit={(e) => {
e.preventDefault();
+ const latestText = promptInput?.getState()?.text ?? currentText;
+ if (latestText === originalText) {
+ return;
+ }
+
if (useAttachmentUploadStore.getState().uploadingIds.size > 0) {
toast.info("Please wait for uploads to finish before sending");
return;
}Also applies to: 57-65
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/assistant-ui/thread/EditPromptInput.tsx` around lines 27 - 36,
The form onSubmit currently only checks uploads before calling
promptInput?.send(), but the Send button also guards against unchanged edits;
mirror that disabled-state guard in the onSubmit handler so
keyboard/programmatic submits behave identically. Concretely: in the onSubmit
function add the same condition used by the Send button (the same variable or
predicate that computes the button's disabled state—e.g., the sendDisabled /
isUnchanged / canSend check used in the JSX) and return early if it indicates
no-op, before calling promptInput?.send(); keep the existing upload check
(useAttachmentUploadStore.getState().uploadingIds) as well so both guards run.
| if (!currentText.trim() && attachments.length === 0 && !hasReplyContext) { | ||
| return; | ||
| } | ||
|
|
||
| const pdfAttachments = attachments.filter((att) => { | ||
| const file = att.file; | ||
| return ( | ||
| file && | ||
| (file.type === "application/pdf" || | ||
| file.name.toLowerCase().endsWith(".pdf") || | ||
| isOfficeDocument(file)) | ||
| ); | ||
| }); | ||
|
|
||
| if (pdfAttachments.length > 0 && currentWorkspaceId) { | ||
| void processPdfAttachmentsInBackground( | ||
| pdfAttachments, | ||
| currentWorkspaceId, | ||
| operations, | ||
| ); | ||
| } | ||
|
|
||
| const modifiedText = | ||
| currentText.trim() || (hasReplyContext ? "Empty message" : ""); | ||
|
|
||
| const customMetadata: Record<string, unknown> = {}; | ||
| if (replySelections.length > 0) { | ||
| customMetadata.replySelections = replySelections; | ||
| } | ||
| promptInput?.setRunConfig( | ||
| Object.keys(customMetadata).length > 0 ? { custom: customMetadata } : {}, | ||
| ); | ||
|
|
||
| promptInput?.setText(modifiedText); | ||
| promptInput?.send(); |
There was a problem hiding this comment.
Minor: empty-text fallback is inconsistent between reply and attachment paths.
When there are attachments only (no text, no reply context), the guard on Line 87 passes, modifiedText resolves to "" on Line 110, and setText("") + send() is called. For the reply-context path there's an explicit "Empty message" placeholder, but for the attachment-only path the user ends up submitting an empty text. If the downstream runtime rejects empty text, attachment-only sends silently break. Consider either removing the attachments-only branch from the empty guard, or applying the same placeholder when any attachment is present.
♻️ Suggested tweak
- const modifiedText =
- currentText.trim() || (hasReplyContext ? "Empty message" : "");
+ const modifiedText =
+ currentText.trim() ||
+ (hasReplyContext || attachments.length > 0 ? "Empty message" : "");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/assistant-ui/thread/PromptInput.tsx` around lines 87 - 121,
The empty-text fallback is inconsistent: when sending attachments-only the code
sets modifiedText to "" which can cause downstream rejections. Update the logic
around modifiedText (and/or the early guard) so that when attachments.length > 0
you use the same placeholder as reply context; e.g. compute modifiedText from
currentText.trim() || (hasReplyContext || attachments.length > 0 ? "Empty
message" : ""), then call promptInput?.setText(modifiedText) and
promptInput?.send() as before (symbols to edit: currentText, attachments,
hasReplyContext, modifiedText, promptInput.setText, promptInput.send).
| import { useCallback, useMemo, useRef, useState, type FC } from "react"; | ||
| import { ChatActionBar, useChatMessage } from "@/lib/chat/runtime"; | ||
| import { TooltipIconButton } from "@/components/assistant-ui/tooltip-icon-button"; | ||
|
|
||
| export const UserActionBar: FC = () => { | ||
| const message = useChatMessage(); | ||
| const [copied, setCopied] = useState(false); | ||
| const copyTimeoutRef = useRef<NodeJS.Timeout | null>(null); | ||
|
|
||
| const textContent = useMemo(() => { | ||
| return message.content | ||
| .filter( | ||
| (part): part is { type: "text"; text: string } => part.type === "text", | ||
| ) | ||
| .map((part) => part.text ?? "") | ||
| .join("\n\n"); | ||
| }, [message.content]); | ||
|
|
||
| const handleCopy = useCallback(() => { | ||
| if (!textContent) return; | ||
| navigator.clipboard.writeText(textContent); | ||
| setCopied(true); | ||
| if (copyTimeoutRef.current) clearTimeout(copyTimeoutRef.current); | ||
| copyTimeoutRef.current = setTimeout(() => setCopied(false), 2000); | ||
| }, [textContent]); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
MDN Clipboard writeText return value Promise NotAllowedError
💡 Result:
The MDN Clipboard.writeText method returns a Promise that resolves once the clipboard contents have been successfully updated with the provided text string. The Promise rejects with a DOMException (such as NotAllowedError) if writing to the clipboard is not permitted, typically due to security restrictions like requiring a secure context (HTTPS), user gesture (e.g., click event), or browser-specific policies (stricter in Safari/iOS). Key details from MDN: - Return value: "A Promise that is resolved once the clipboard's contents have been updated." - Exceptions: "Thrown if writing to the clipboard is not allowed." (Explicitly NotAllowedError DOMException in related write docs and error reports.) Usage example: async function writeClipboardText(text) { try { await navigator.clipboard.writeText(text); console.log('Clipboard updated'); } catch (error) { if (error.name === 'NotAllowedError') { console.error('Clipboard write not allowed:', error.message); } else { console.error('Clipboard error:', error); } } } NotAllowedError commonly occurs outside user gestures, in non-secure contexts, or after async operations expire the gesture context (especially Safari).
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/Clipboard/writeText
- 2: https://developer.mozilla.org/docs/Web/API/Clipboard/writeText
- 3: https://dev.mozilla.org/en-US/docs/Web/API/Clipboard/writeText
- 4: https://developer.mozilla.org/docs/Web/API/Clipboard/write
- 5: https://developer.mozilla.org/en-US/docs/Web/API/Clipboard/write
- 6: https://medium.com/@hailieejkim/safari-clipboard-errors-notallowederror-and-typeerror-fixes-e9c23becd856
- 7: https://stackoverflow.com/questions/62327358/javascript-clipboard-api-safari-ios-notallowederror-message
🏁 Script executed:
# First, find and read the UserActionBar.tsx file
fd -t f UserActionBar.tsxRepository: ThinkEx-OSS/thinkex
Length of output: 115
🏁 Script executed:
# Read the UserActionBar.tsx file to verify the code snippet
cat -n src/components/assistant-ui/thread/UserActionBar.tsxRepository: ThinkEx-OSS/thinkex
Length of output: 2066
🏁 Script executed:
# Check if AssistantActionBar component exists (mentioned in comment for context)
fd -t f AssistantActionBar.tsxRepository: ThinkEx-OSS/thinkex
Length of output: 120
🏁 Script executed:
# Read the AssistantActionBar.tsx to compare implementation
cat -n src/components/assistant-ui/thread/AssistantActionBar.tsx | head -60Repository: ThinkEx-OSS/thinkex
Length of output: 2108
Handle clipboard failures before toggling the success icon.
writeText returns a Promise that can reject (e.g., NotAllowedError for non-secure contexts or outside user gestures). The current code sets the checkmark immediately without awaiting the result, creating a false-success UX. Additionally, the reset timer should be cleared on unmount to prevent stale state updates.
Proposed fix
-import { useCallback, useMemo, useRef, useState, type FC } from "react";
+import { useCallback, useEffect, useMemo, useRef, useState, type FC } from "react";
@@
- const copyTimeoutRef = useRef<NodeJS.Timeout | null>(null);
+ const copyTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
+
+ useEffect(() => {
+ return () => {
+ if (copyTimeoutRef.current) clearTimeout(copyTimeoutRef.current);
+ };
+ }, []);
@@
- const handleCopy = useCallback(() => {
+ const handleCopy = useCallback(async () => {
if (!textContent) return;
- navigator.clipboard.writeText(textContent);
- setCopied(true);
- if (copyTimeoutRef.current) clearTimeout(copyTimeoutRef.current);
- copyTimeoutRef.current = setTimeout(() => setCopied(false), 2000);
+ try {
+ await navigator.clipboard.writeText(textContent);
+ setCopied(true);
+ if (copyTimeoutRef.current) clearTimeout(copyTimeoutRef.current);
+ copyTimeoutRef.current = setTimeout(() => {
+ setCopied(false);
+ copyTimeoutRef.current = null;
+ }, 2000);
+ } catch {
+ setCopied(false);
+ }
}, [textContent]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/assistant-ui/thread/UserActionBar.tsx` around lines 4 - 28,
The copy handler currently sets copied immediately and doesn't await failures;
update the handleCopy function in UserActionBar to await
navigator.clipboard.writeText(textContent) inside a try/catch, only call
setCopied(true) after a successful await, and handle/log errors in the catch
block (do not set the success state on failure). Also add a useEffect cleanup
that clears the copyTimeoutRef timeout on unmount
(clearTimeout(copyTimeoutRef.current) and null it) to avoid stale state updates;
keep references to copyTimeoutRef and the existing timeout-reset logic.
| if ( | ||
| truncateCtx && | ||
| !truncateCtx.expanded && | ||
| truncateCtx.maxChars < Infinity && | ||
| text.length > truncateCtx.maxChars | ||
| ) { | ||
| text = text.slice(0, truncateCtx.maxChars).trim() + "..."; | ||
| } |
There was a problem hiding this comment.
Minor: slice can split surrogate pairs / grapheme clusters.
text.slice(0, maxChars) counts UTF-16 code units, so messages ending the truncation window inside an emoji (surrogate pair, ZWJ sequence, or flag) can produce a lone surrogate or a visibly broken emoji before the "...". Consider grapheme-aware truncation:
♻️ Grapheme-safe alternative
- text = text.slice(0, truncateCtx.maxChars).trim() + "...";
+ // Prefer grapheme-safe truncation to avoid splitting emoji/ZWJ sequences.
+ const segmenter =
+ typeof Intl !== "undefined" && "Segmenter" in Intl
+ ? new Intl.Segmenter(undefined, { granularity: "grapheme" })
+ : null;
+ if (segmenter) {
+ const graphemes = Array.from(segmenter.segment(text), (s) => s.segment);
+ text = graphemes.slice(0, truncateCtx.maxChars).join("").trim() + "...";
+ } else {
+ text = Array.from(text).slice(0, truncateCtx.maxChars).join("").trim() + "...";
+ }Low priority; only visible with emoji/CJK-extension content right at the 250-char boundary.
📝 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.
| if ( | |
| truncateCtx && | |
| !truncateCtx.expanded && | |
| truncateCtx.maxChars < Infinity && | |
| text.length > truncateCtx.maxChars | |
| ) { | |
| text = text.slice(0, truncateCtx.maxChars).trim() + "..."; | |
| } | |
| if ( | |
| truncateCtx && | |
| !truncateCtx.expanded && | |
| truncateCtx.maxChars < Infinity && | |
| text.length > truncateCtx.maxChars | |
| ) { | |
| // Prefer grapheme-safe truncation to avoid splitting emoji/ZWJ sequences. | |
| const segmenter = | |
| typeof Intl !== "undefined" && "Segmenter" in Intl | |
| ? new Intl.Segmenter(undefined, { granularity: "grapheme" }) | |
| : null; | |
| if (segmenter) { | |
| const graphemes = Array.from(segmenter.segment(text), (s) => s.segment); | |
| text = graphemes.slice(0, truncateCtx.maxChars).join("").trim() + "..."; | |
| } else { | |
| text = Array.from(text).slice(0, truncateCtx.maxChars).join("").trim() + "..."; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/assistant-ui/thread/UserMessageTruncateContext.tsx` around
lines 20 - 27, The current truncation uses text.slice(0, truncateCtx.maxChars)
which can split surrogate pairs/grapheme clusters (emojis, ZWJ sequences);
change the truncation in UserMessageTruncateContext to perform grapheme-aware
slicing: iterate or segment the string by grapheme clusters (use
Intl.Segmenter('grapheme') when available, or fallback to Array.from(text) or a
small dependency like grapheme-splitter) and then join the first
truncateCtx.maxChars clusters before appending "..." only when truncated; keep
the existing checks on truncateCtx.expanded and truncateCtx.maxChars.
| void startAssetProcessing({ | ||
| workspaceId, | ||
| assets: uploads, | ||
| itemIds: createdIds, | ||
| onOcrError: (error) => { | ||
| console.error("Error starting assistant file processing:", error); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Attach a .catch to the fire-and-forget startAssetProcessing promise.
onOcrError only fires for OCR candidate failures inside startAssetProcessing. If the function itself rejects (e.g., during setup before the OCR callback is reached), the void prefix suppresses the type warning but still produces an unhandled promise rejection.
🛡️ Proposed fix
- void startAssetProcessing({
+ startAssetProcessing({
workspaceId,
assets: uploads,
itemIds: createdIds,
onOcrError: (error) => {
console.error("Error starting assistant file processing:", error);
},
- });
+ }).catch((error) => {
+ console.error("Error starting assistant file processing:", error);
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/uploads/process-pdf-attachments-in-background.ts` around lines 40 -
47, The fire-and-forget call to startAssetProcessing(...) must attach a .catch
to handle rejections from the function itself (in addition to onOcrError which
handles internal OCR candidate errors); update the invocation of
startAssetProcessing (the call that passes workspaceId, assets: uploads,
itemIds: createdIds, onOcrError) to append a .catch handler that logs or reports
the error (e.g., using console.error or processLogger) so any rejection during
setup is not left unhandled.
…entSnapshot, useAttachmentScope, useAttachmentId, useIsAttachmentImage, ChatAttachment primitive)
There was a problem hiding this comment.
2 issues found across 4 files (changes from recent commits).
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/components/assistant-ui/attachment.tsx">
<violation number="1">
P2: Changing the upload input id breaks external triggers that still query the old id (`prompt-input-file-upload`). Keep the id stable or update all callers together.</violation>
</file>
<file name="src/lib/chat/runtime/hooks.ts">
<violation number="1" location="src/lib/chat/runtime/hooks.ts:165">
P2: `useAttachmentScope` is not subscribed to attachment state, so it can return stale scope values after context/state changes. Read `attachment.source` via `useAuiState` instead of directly from `useAui()`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const aui = useAui(); | ||
| const source = (aui as unknown as { attachment?: { source?: string } } | null)?.attachment?.source; |
There was a problem hiding this comment.
P2: useAttachmentScope is not subscribed to attachment state, so it can return stale scope values after context/state changes. Read attachment.source via useAuiState instead of directly from useAui().
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/chat/runtime/hooks.ts, line 165:
<comment>`useAttachmentScope` is not subscribed to attachment state, so it can return stale scope values after context/state changes. Read `attachment.source` via `useAuiState` instead of directly from `useAui()`.</comment>
<file context>
@@ -153,3 +156,44 @@ export function useMessagePartTextLengthSnapshot(
+ * Reads `aui.attachment.source` from the AttachmentPrimitive context.
+ */
+export function useAttachmentScope(): AttachmentScope {
+ const aui = useAui();
+ const source = (aui as unknown as { attachment?: { source?: string } } | null)?.attachment?.source;
+ return source === "composer" ? "composer" : "message";
</file context>
| const aui = useAui(); | |
| const source = (aui as unknown as { attachment?: { source?: string } } | null)?.attachment?.source; | |
| const source = useAuiState((s: any) => (s.attachment as { source?: string } | undefined)?.source); |
![capy-ai[bot]](https://avatars.githubusercontent.com/in/1915919?s=60&v=4){kind=small}
| ); | ||
|
|
||
| const handleTriggerFileInput = useCallback(() => { | ||
| document.getElementById("prompt-input-file-upload")?.click(); |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
The split changed the welcome suggestions component to click prompt-input-file-upload, but the only hidden input rendered by PromptInputAddAttachment is still composer-file-upload, so the Upload suggestion now does nothing on an empty thread. This is a user-visible regression in a path the PR description explicitly says should be behavior-preserving. Restore the old id or share the input id constant between the trigger and attachment component.
// src/components/assistant-ui/thread/ThreadSuggestions.tsx
document.getElementById("prompt-input-file-upload")?.click();
focusComposerInput(true);Verified against the pre-split implementation in @src/components/assistant-ui/thread.tsx, which clicked composer-file-upload, and against @src/components/assistant-ui/attachment.tsx, which still renders const uploadInputId = "composer-file-upload".
…pload button id drift
- AddYoutubeVideoToolUI, CreateDocumentToolUI, CreateFlashcardToolUI, CreateQuizToolUI,
EditItemToolUI, ExecuteCodeToolUI, ReadWorkspaceToolUI, SearchWorkspaceToolUI,
URLContextToolUI, WebSearchToolUI, YouTubeSearchToolUI now use ChatToolUIProps
and useChatScrollLock from @/lib/chat/runtime instead of importing from
@assistant-ui/react directly.
- Added ChatToolUIProps<Args, Result> type alias to the ACL.
- Fixed bug: Welcome-screen Upload suggestion clicked
getElementById("prompt-input-file-upload") but attachment.tsx rendered
id="composer-file-upload" — IDs now aligned.
- Dropped unused useAui import from YouTubeSearchToolUI.
- AssistantDropzone, PromptBuilderDialog, SpeechToTextButton,
AssistantTextSelectionManager, thread-list-dropdown, AppChatHeader,
MessageContextBadges, PdfPanelHeader, QuizContent, and
use-workspace-context-provider now import from @/lib/chat/runtime
instead of @assistant-ui/react.
- Added to the ACL:
- Primitives: ChatThreadList, ChatThreadListItem
- Hooks: useCurrentChatMessage, useThreadListItemId,
useChatThreadListItem, usePromptInputThreadActions,
useChatAssistantContext
- Types: ChatThreadListItem, CurrentChatMessage,
ChatAssistantContextOptions, PromptInputThreadActions
After this, only the runtime boundary files still import directly from
@assistant-ui/react: WorkspaceRuntimeProvider, chat-toolkit,
custom-thread-history-adapter, custom-thread-list-adapter,
supabase-attachment-adapter, and toCreateMessageWithContext.
There was a problem hiding this comment.
2 issues found across 13 files (changes from recent commits).
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/components/pdf/PdfPanelHeader.tsx">
<violation number="1" location="src/components/pdf/PdfPanelHeader.tsx:105">
P2: Optional chaining here can silently skip adding the screenshot attachment, but the code still shows a success toast. Guard against a missing prompt input and fail into the existing `catch` path instead of reporting success.</violation>
</file>
<file name="src/lib/chat/runtime/hooks.ts">
<violation number="1" location="src/lib/chat/runtime/hooks.ts:222">
P1: `useChatThreadListItem` is not actually safe: it directly uses a context-bound hook and can throw when called outside `ThreadListItem` context (e.g. `AppChatHeader`).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| * Used by AppChatHeader and thread-list-dropdown to read the current thread title and initialization state. | ||
| */ | ||
| export function useChatThreadListItem(): ChatThreadListItem | undefined { | ||
| return useThreadListItem() as unknown as ChatThreadListItem | undefined; |
There was a problem hiding this comment.
P1: useChatThreadListItem is not actually safe: it directly uses a context-bound hook and can throw when called outside ThreadListItem context (e.g. AppChatHeader).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/chat/runtime/hooks.ts, line 222:
<comment>`useChatThreadListItem` is not actually safe: it directly uses a context-bound hook and can throw when called outside `ThreadListItem` context (e.g. `AppChatHeader`).</comment>
<file context>
@@ -197,3 +203,46 @@ export function useAttachmentSnapshot(): ChatAttachmentSnapshot | undefined {
+ * Used by AppChatHeader and thread-list-dropdown to read the current thread title and initialization state.
+ */
+export function useChatThreadListItem(): ChatThreadListItem | undefined {
+ return useThreadListItem() as unknown as ChatThreadListItem | undefined;
+}
+
</file context>
| return useThreadListItem() as unknown as ChatThreadListItem | undefined; | |
| return useAuiState((s: any) => s.threadListItem as ChatThreadListItem | undefined); |
- types.ts: drop catch-all `type: string` variant from ChatMessagePart so discriminated-union narrowing works on part.type === "text" - use-prompt-input-paste: wrap handler in useCallback and early-return when promptInput is null (don't preventDefault if we can't handle it) - BranchPicker: destructure hideWhenSingleBranch out of rest so callers can't override the forced default via prop spread - PdfPanelHeader: throw explicit "Chat composer not ready" error when promptInputRef.current is null so the catch fires with the proper error toast instead of silently reporting success - hooks.ts: delete dead useThreadState hook (zero consumers; narrow hooks like useIsThreadLoading handle all callers) - hooks.ts: replace (s: any) casts with a local AuiStateShape interface describing the state fields the ACL actually reads Pre-existing issues on main that this PR preserves intentionally (clipboard.writeText not awaited, edit-onSubmit guard, nested Button-in- Link, surrogate-pair slicing, process-pdf unhandled rejection) are explicitly out of scope and not touched here.
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 (3)
src/components/chat/AppChatHeader.tsx (1)
154-168:⚠️ Potential issue | 🟡 MinorMinor: false success toast if
threadActionsis unavailable.With
threadActions?.rename(...), if the hook returnedundefined(e.g. provider missing), the optional chain resolves toundefinedsynchronously and the code proceeds totoast.success("Title updated")without any rename actually occurring. TheisThreadInitializedgate on Line 147 guards onremoteId, not on the presence ofthreadActions. Consider either assertingthreadActions(and surfacing a real error) or branching explicitly when it's undefined.Suggested change
- if (trimmedValue && trimmedValue !== currentThreadTitle) { - try { - await threadActions?.rename(trimmedValue); - toast.success("Title updated"); - } catch (error) { + if (trimmedValue && trimmedValue !== currentThreadTitle) { + if (!threadActions) { + toast.error("Rename is unavailable right now"); + setCurrentTitleEditValue(currentThreadTitle); + setIsEditingCurrentTitle(false); + return; + } + try { + await threadActions.rename(trimmedValue); + toast.success("Title updated"); + } catch (error) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/chat/AppChatHeader.tsx` around lines 154 - 168, The success toast is shown even when threadActions is undefined because threadActions?.rename(...) does nothing; update the title-save logic in AppChatHeader to explicitly handle missing threadActions: check if threadActions exists before calling threadActions.rename (or throw/return an error), show toast.error and revert via setCurrentTitleEditValue(currentThreadTitle) when absent, and only show toast.success after a confirmed await threadActions.rename(...) succeeds; ensure setIsEditingCurrentTitle(false) still runs in finally or after the proper branch.src/components/assistant-ui/PromptBuilderDialog.tsx (1)
389-416:⚠️ Potential issue | 🟡 MinorSilent failure when
promptInputis unavailable.If
usePromptInput()returnsnull(chat runtime not mounted / no active composer),promptInput?.setText(builtPrompt)becomes a no-op, and the code still proceeds to callfocusComposerInput()andonOpenChange(false). The user clicks Send, the dialog closes, and the built prompt is discarded with no error surfaced — unlikeQuizContent, which shows"Chat not available. Please try again."in the same scenario.Recommend gating on
promptInputand surfacing a toast before closing:🛡️ Suggested fix
} else { // Select the chosen items in the workspace (like mention menu does) if (action !== "search" && selectedContextIds.size > 0) { selectMultipleCards(Array.from(selectedContextIds)); } - promptInput?.setText(builtPrompt); - focusComposerInput(); + if (!promptInput) { + toast.error("Chat not available. Please try again."); + return; + } + promptInput.setText(builtPrompt); + focusComposerInput(); } onOpenChange(false);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/PromptBuilderDialog.tsx` around lines 389 - 416, handleSubmit silently drops the built prompt when promptInput is null; update handleSubmit to detect when promptInput is unavailable (and onBuild is not provided) and surface an error toast instead of closing the dialog: if onBuild is present continue as before, otherwise if promptInput is null call toast.error("Chat not available. Please try again.") and return early (do not call focusComposerInput(), promptInput?.setText(), selectMultipleCards(), or onOpenChange(false)); reference the handleSubmit function, promptInput, onBuild, promptInput.setText, focusComposerInput, selectMultipleCards, and onOpenChange when making the change.src/components/assistant-ui/SpeechToTextButton.tsx (1)
21-28:⚠️ Potential issue | 🟡 Minor
promptInputhas unstable identity in the dependency array, causing unnecessary effect re-runs.
usePromptInput()returns a new object literal on every render (the object{ setText, send, addAttachment, setRunConfig, getState }is created fresh each time with no memoization). Including it in the effect's dependency array here means the effect will re-run on every parent render wheneverlistening && transcriptevaluates to true, repeatedly callingsetText()with the same values. While not a correctness issue (the call is idempotent), it causes wasteful composer writes and re-renders during an active listening session.Remove
promptInputfrom the dependency array and use a ref to access it instead:Suggested fix
const promptInput = usePromptInput(); + const promptInputRef = useRef(promptInput); + promptInputRef.current = promptInput; ... useEffect(() => { if (listening && transcript) { const separator = originalText && !originalText.endsWith(' ') ? ' ' : ''; - promptInput?.setText(originalText + separator + transcript); + promptInputRef.current?.setText(originalText + separator + transcript); } - }, [transcript, listening, originalText, promptInput]); + }, [transcript, listening, originalText]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/SpeechToTextButton.tsx` around lines 21 - 28, The effect uses promptInput in its dependency array but usePromptInput() returns a new object each render, causing unnecessary re-runs; remove promptInput from the useEffect deps and access it via a stable ref instead: create a ref (e.g., promptInputRef) and update it whenever the promptInput object changes, then inside the useEffect read promptInputRef.current.setText(...) rather than promptInput.setText; keep transcript, listening, and originalText in the dependency array and ensure the ref is updated in a separate useEffect so setText calls remain correct.
🧹 Nitpick comments (3)
src/components/assistant-ui/thread-list-dropdown.tsx (1)
62-67: Nit: prefer a single conditional over two mirroredChatIfblocks.The two
ChatIfbranches use inverse predicates on the samethreads.isLoadingvalue, so both run the state subscription. A single conditional (ternary or if/else inside one branch) would be slightly cheaper and easier to read, though functionally equivalent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/assistant-ui/thread-list-dropdown.tsx` around lines 62 - 67, Replace the two mirrored ChatIf blocks that both subscribe to threads.isLoading with a single ChatIf to avoid duplicate subscriptions: use ChatIf with one condition (accessing ({ threads }) => threads) and inside its child render choose between ThreadListSkeleton and ChatThreadList.Items based on threads.isLoading (e.g., a ternary or if/else), keeping the ThreadListItem onSelect={() => setOpen(false)} component passed into ChatThreadList.Items.src/components/workspace-canvas/QuizContent.tsx (1)
186-254: Optional: drop the redundantcomposer = promptInputalias.Each of
handleUpdateQuiz,handleAskHint, andhandleAskExplainreassignspromptInputinto a localcomposervariable solely to preserve the old naming. SincepromptInputis itself the composer-actions handle, the extra indirection adds noise without clarifying intent and slightly obscures the nullability contract. You can usepromptInputdirectly (or do a single early-return null guard at the top of each handler).♻️ Example refactor for `handleUpdateQuiz`
- // Then send the message via composer - const composer = promptInput; - if (composer) { - try { - composer.setText("Add 5 more questions to this quiz"); - composer.send(); - toast.success("Requesting more questions..."); - } catch (error) { - toast.error("Failed to send request. Please try again."); - } - } else { - toast.error("Chat not available. Please try again."); - } + if (!promptInput) { + toast.error("Chat not available. Please try again."); + return; + } + try { + promptInput.setText("Add 5 more questions to this quiz"); + promptInput.send(); + toast.success("Requesting more questions..."); + } catch { + toast.error("Failed to send request. Please try again."); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/workspace-canvas/QuizContent.tsx` around lines 186 - 254, The three handlers (handleUpdateQuiz, handleAskHint, handleAskExplain) create a redundant local alias "composer" for promptInput; remove that alias and use promptInput directly (or perform a single early-return null guard at the top of each handler) to simplify the nullability check and reduce noise. Specifically, in handleUpdateQuiz/handleAskHint/handleAskExplain, replace checks like "const composer = promptInput; if (composer) { composer.setText(...); composer.send(); }" with either "if (!promptInput) { toast.error(...); return }" followed by direct calls to promptInput.setText(...)/promptInput.send(), preserving the existing calls to selectedCardIds.has, toggleCardSelection, useUIStore.getState().setIsChatExpanded, and focusComposerInput. Ensure toast error branches remain the same behavior.src/lib/chat/runtime/types.ts (1)
16-22: Non-textChatMessagePartvariants are effectively untyped.Every non-text branch is declared as
{ type: "..."; [k: string]: unknown }, so discriminating ontypenarrowstypebut leaves every other property asunknown. Any code that reads.name,.image,.source,.toolName, etc. off these parts will have to cast, which defeats much of the purpose of the discriminated union and is easy to drift from the upstream@assistant-ui/reactpart shapes.If the intent is to fully own message-part identity inside the ACL, consider modeling the fields you actually consume (name/mime/url for file, image/mime for image, sourceType/url/title for source, text/reasoning for reasoning, toolName/args/result/state for tool-*). That will also make downstream narrowing (mentioned in the PR as a follow-up) meaningful.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/chat/runtime/types.ts` around lines 16 - 22, The union ChatMessagePart currently leaves every non-text variant as `{ type: "..."; [k: string]: unknown }`, making narrowing useless; update the ChatMessagePart definition (and related ChatTextPart) to provide concrete typed shapes for each discriminant (e.g., for "file" include name:string, mime?:string, url?:string; for "image" include url:string, mime?:string, alt?:string; for "source" include sourceType?:string, url?:string, title?:string; for "reasoning" include text:string (or reasoning:string) and any confidence/references you consume; for "tool-call"/"tool-result"/"tool" include toolName:string, args?:unknown, result?:unknown, state?:string) so consumers can narrow by `type` and access properties without casting; modify the ChatMessagePart union in src/lib/chat/runtime/types.ts to replace the generic index signatures with these concrete fields.
🤖 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/PromptBuilderDialog.tsx`:
- Around line 389-416: handleSubmit silently drops the built prompt when
promptInput is null; update handleSubmit to detect when promptInput is
unavailable (and onBuild is not provided) and surface an error toast instead of
closing the dialog: if onBuild is present continue as before, otherwise if
promptInput is null call toast.error("Chat not available. Please try again.")
and return early (do not call focusComposerInput(), promptInput?.setText(),
selectMultipleCards(), or onOpenChange(false)); reference the handleSubmit
function, promptInput, onBuild, promptInput.setText, focusComposerInput,
selectMultipleCards, and onOpenChange when making the change.
In `@src/components/assistant-ui/SpeechToTextButton.tsx`:
- Around line 21-28: The effect uses promptInput in its dependency array but
usePromptInput() returns a new object each render, causing unnecessary re-runs;
remove promptInput from the useEffect deps and access it via a stable ref
instead: create a ref (e.g., promptInputRef) and update it whenever the
promptInput object changes, then inside the useEffect read
promptInputRef.current.setText(...) rather than promptInput.setText; keep
transcript, listening, and originalText in the dependency array and ensure the
ref is updated in a separate useEffect so setText calls remain correct.
In `@src/components/chat/AppChatHeader.tsx`:
- Around line 154-168: The success toast is shown even when threadActions is
undefined because threadActions?.rename(...) does nothing; update the title-save
logic in AppChatHeader to explicitly handle missing threadActions: check if
threadActions exists before calling threadActions.rename (or throw/return an
error), show toast.error and revert via
setCurrentTitleEditValue(currentThreadTitle) when absent, and only show
toast.success after a confirmed await threadActions.rename(...) succeeds; ensure
setIsEditingCurrentTitle(false) still runs in finally or after the proper
branch.
---
Nitpick comments:
In `@src/components/assistant-ui/thread-list-dropdown.tsx`:
- Around line 62-67: Replace the two mirrored ChatIf blocks that both subscribe
to threads.isLoading with a single ChatIf to avoid duplicate subscriptions: use
ChatIf with one condition (accessing ({ threads }) => threads) and inside its
child render choose between ThreadListSkeleton and ChatThreadList.Items based on
threads.isLoading (e.g., a ternary or if/else), keeping the ThreadListItem
onSelect={() => setOpen(false)} component passed into ChatThreadList.Items.
In `@src/components/workspace-canvas/QuizContent.tsx`:
- Around line 186-254: The three handlers (handleUpdateQuiz, handleAskHint,
handleAskExplain) create a redundant local alias "composer" for promptInput;
remove that alias and use promptInput directly (or perform a single early-return
null guard at the top of each handler) to simplify the nullability check and
reduce noise. Specifically, in handleUpdateQuiz/handleAskHint/handleAskExplain,
replace checks like "const composer = promptInput; if (composer) {
composer.setText(...); composer.send(); }" with either "if (!promptInput) {
toast.error(...); return }" followed by direct calls to
promptInput.setText(...)/promptInput.send(), preserving the existing calls to
selectedCardIds.has, toggleCardSelection,
useUIStore.getState().setIsChatExpanded, and focusComposerInput. Ensure toast
error branches remain the same behavior.
In `@src/lib/chat/runtime/types.ts`:
- Around line 16-22: The union ChatMessagePart currently leaves every non-text
variant as `{ type: "..."; [k: string]: unknown }`, making narrowing useless;
update the ChatMessagePart definition (and related ChatTextPart) to provide
concrete typed shapes for each discriminant (e.g., for "file" include
name:string, mime?:string, url?:string; for "image" include url:string,
mime?:string, alt?:string; for "source" include sourceType?:string, url?:string,
title?:string; for "reasoning" include text:string (or reasoning:string) and any
confidence/references you consume; for "tool-call"/"tool-result"/"tool" include
toolName:string, args?:unknown, result?:unknown, state?:string) so consumers can
narrow by `type` and access properties without casting; modify the
ChatMessagePart union in src/lib/chat/runtime/types.ts to replace the generic
index signatures with these concrete fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a2095c71-ec20-47ab-9fdc-bfea2d3b13be
📒 Files selected for processing (28)
src/components/assistant-ui/AddYoutubeVideoToolUI.tsxsrc/components/assistant-ui/AssistantDropzone.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/ExecuteCodeToolUI.tsxsrc/components/assistant-ui/PromptBuilderDialog.tsxsrc/components/assistant-ui/ReadWorkspaceToolUI.tsxsrc/components/assistant-ui/SearchWorkspaceToolUI.tsxsrc/components/assistant-ui/SpeechToTextButton.tsxsrc/components/assistant-ui/URLContextToolUI.tsxsrc/components/assistant-ui/WebSearchToolUI.tsxsrc/components/assistant-ui/YouTubeSearchToolUI.tsxsrc/components/assistant-ui/attachment.tsxsrc/components/assistant-ui/thread-list-dropdown.tsxsrc/components/assistant-ui/thread/BranchPicker.tsxsrc/components/assistant-ui/thread/hooks/use-prompt-input-paste.tssrc/components/chat/AppChatHeader.tsxsrc/components/chat/MessageContextBadges.tsxsrc/components/pdf/PdfPanelHeader.tsxsrc/components/workspace-canvas/QuizContent.tsxsrc/hooks/ai/use-workspace-context-provider.tssrc/lib/chat/runtime/hooks.tssrc/lib/chat/runtime/index.tssrc/lib/chat/runtime/primitives.tssrc/lib/chat/runtime/types.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- src/components/assistant-ui/thread/BranchPicker.tsx
- src/components/assistant-ui/thread/hooks/use-prompt-input-paste.ts
- src/lib/chat/runtime/primitives.ts
- src/components/assistant-ui/attachment.tsx
- src/lib/chat/runtime/index.ts
- src/lib/chat/runtime/hooks.ts
…vider
PromptBuilderDialog is mounted from WorkspaceHeader and WorkspaceSection,
which render outside WorkspaceRuntimeProvider when no workspace is
active. useAui() there returns a DefaultAssistantClient proxy whose
.composer and .threadListItem getters return functions that throw when
invoked ("You are using a component or hook that requires an
AuiProvider..."). The ACL's usePromptInput() and
usePromptInputThreadActions() called those getters during render,
crashing the component via ErrorBoundary.
Defer proxy resolution to call time, wrapped in try/catch. Matches the
pre-refactor behavior where `aui?.composer()?.setText(...)` only
resolved at event time (e.g. inside a submit handler, by which point
the component is definitely inside a provider).
1 participant
Splits the monolithic thread.tsx (1510 LOC) into a thread/ directory with one component per file and introduces a chat-runtime ACL to decouple components from @assistant-ui/react. All thread files now import UI primitives and hooks from @/lib/chat/runtime instead of directly from the library. Renames Composer family components to PromptInput* (PromptInput, PromptInputShell, PromptInputToolbar, EditPromptInput) and renames attachment exports (PromptInputAttachments, PromptInputAddAttachment). Extracts the PDF background upload helper to @/lib/uploads/process-pdf-attachments-in-background.ts and moves the mention state machine to hooks/use-mention-menu.ts. Zero behavior changes—existing UI, @-mentions, PDF uploads, and keyboard shortcuts remain intact.
Summary by CodeRabbit
New Features
Refactor