Chat UI rehaul + animation#1693
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ShimmerLabel, replaces many Shimmer usages with it, introduces StreamingMessageText to progressively reveal assistant text, and updates MessagePartsRenderer and MessageList to use per-message streaming animation and the new components. Changes
Sequence Diagram(s)sequenceDiagram
participant MessageList
participant MessagePartsRenderer
participant StreamingMessageText
participant UI
MessageList->>MessagePartsRenderer: render(message, shouldAnimateStreaming)
MessagePartsRenderer->>StreamingMessageText: render text part (isAnimating := shouldAnimateStreaming)
StreamingMessageText-->>StreamingMessageText: progressively reveal characters while animating
StreamingMessageText->>UI: expose displayText for rendering
MessageList->>UI: render assistant thinking indicator using ShimmerLabel when applicable
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
packages/ui/src/components/ai-elements/shimmer-label.tsx (2)
17-33: Consider wrapping withmemofor consistency withShimmer.
Shimmerismemo-ized;ShimmerLabelisn't. In streaming contexts (rapid parent re-renders),ShimmerLabelwill recreate its JSX subtree on every render even though the innerShimmeris protected by its own memo.♻️ Add memo
+import { memo } from "react"; + -export const ShimmerLabel = ({ +const ShimmerLabelComponent = ({ children, className, shimmerClassName, isShimmering = true, ...props }: ShimmerLabelProps) => ( <span className={cn("shrink-0 whitespace-nowrap font-medium", className)}> {isShimmering ? ( <Shimmer className={shimmerClassName} {...props}> {children} </Shimmer> ) : ( children )} </span> ); + +export const ShimmerLabel = memo(ShimmerLabelComponent);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/shimmer-label.tsx` around lines 17 - 33, ShimmerLabel recreates its subtree on every parent render while Shimmer is memoized; wrap ShimmerLabel with React.memo to avoid unnecessary re-renders. Import memo from React (or use React.memo) and export the memoized component (e.g., wrap the ShimmerLabel function with memo and set a displayName like "ShimmerLabel" if desired) so the component benefits from the same memoization as Shimmer.
1-33: Add a co-located test file.As per coding guidelines, implementation files should be accompanied by
FileName.test.tsx. A basic test covering shimmering vs. non-shimmering render paths would be valuable.Based on coding guidelines: "Co-locate tests with implementation files using the naming convention
FileName.test.tsxorFileName.test.ts".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/shimmer-label.tsx` around lines 1 - 33, Add a co-located test file named shimmer-label.test.tsx next to shimmer-label.tsx that imports the ShimmerLabel component and verifies both render paths: when isShimmering=true it renders the Shimmer child (expecting presence of the Shimmer element or class passed via shimmerClassName) and when isShimmering=false it renders the raw children text; use the ShimmerLabel props (children, shimmerClassName, isShimmering) to drive the two assertions and a test renderer like React Testing Library to mount and query for text vs shimmer element.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MessagePartsRenderer/MessagePartsRenderer.tsx (1)
99-105: RedundantisLastAssistantguard —isStreamingalready incorporates it.
MessageListpassesshouldAnimateStreaming(which isisLastAssistant && (isStreaming || submitStatus === "submitted")) as theisStreamingprop. Multiplying byisLastAssistantagain on line 102 is idempotent (A && A === A) but obscures the intent.♻️ Proposed cleanup
- isAnimating={isLastAssistant && isStreaming} + isAnimating={isStreaming}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MessagePartsRenderer/MessagePartsRenderer.tsx` around lines 99 - 105, The isLastAssistant check is redundant because MessageList already computes shouldAnimateStreaming = isLastAssistant && (isStreaming || submitStatus === "submitted") and passes it as the isStreaming prop; update the StreamingMessageText usage to rely on the isStreaming prop directly (remove the extra isLastAssistant && wrapper) so that the isAnimating prop becomes simply isStreaming, keeping mermaidConfig/components unchanged and retaining StreamingMessageText as the component to modify.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MessageList/MessageList.tsx (1)
80-81:shouldAnimateStreamingwithsubmitStatus === "submitted"may unnecessarily activate animation on the last assistant message.When
submitStatus === "submitted"(request sent but no streaming yet), if the last message is an assistant message,shouldAnimateStreamingbecomestrueeven though no new tokens are arriving.StreamingMessageTextwill start an interval, but sincetextis not growing, the interval callback exits immediately each tick. It's harmless but fires unnecessary timers.If the intent is only to animate while tokens are actually flowing, scope the condition to
isStreaming:♻️ Proposed tightening (if the current behavior is unintentional)
- const shouldAnimateStreaming = - isLastAssistant && (isStreaming || submitStatus === "submitted"); + const shouldAnimateStreaming = isLastAssistant && isStreaming;If the
submitStatus === "submitted"inclusion is intentional (e.g., to pre-warm the animation before the first token), the current behavior is safe; consider adding an inline comment explaining the intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MessageList/MessageList.tsx` around lines 80 - 81, The current shouldAnimateStreaming expression in MessageList.tsx (const shouldAnimateStreaming = isLastAssistant && (isStreaming || submitStatus === "submitted")) causes StreamingMessageText to start timers when submitStatus === "submitted" even though no tokens are flowing; either tighten the condition to only use isStreaming (const shouldAnimateStreaming = isLastAssistant && isStreaming) so animation/timers only start when tokens are actually streaming, or if the pre-warm behavior is intentional, keep the expression but add an inline comment near shouldAnimateStreaming explaining why submitStatus === "submitted" must trigger the animation.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MessagePartsRenderer/components/StreamingMessageText/StreamingMessageText.tsx (2)
1-57: Missing co-located test file.
StreamingMessageText.tsxcontains non-trivial stateful logic (progressive text reveal,startsWithreset guard,isAnimatingtransitions). NoStreamingMessageText.test.tsxis added alongside it.As per coding guidelines, tests should be co-located with implementation files using the
FileName.test.tsxnaming convention. Consider adding at least:
- Initial render with
isAnimating=false→ full text shown immediatelyisAnimating=true→ only partial text at first tickisAnimatingtransition fromtruetofalse→ full text snaps immediatelytextreplacement (not an extension) → resets to new text🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MessagePartsRenderer/components/StreamingMessageText/StreamingMessageText.tsx` around lines 1 - 57, Add a co-located test file StreamingMessageText.test.tsx that mounts the StreamingMessageText component and exercises its stateful behavior: test initial render with isAnimating=false shows full text immediately; test isAnimating=true advances displayText over time by mocking timers (use jest.useFakeTimers()) and asserting partial text after one tick (STREAM_TEXT_TICK_MS and STREAM_TEXT_CHARS_PER_TICK constants can be referenced to compute expected length); test toggling isAnimating from true to false snaps displayText to the full text immediately; test replacing text with a new non-extension string resets displayText to the new text (uses startsWith guard path). Use render from testing-library/react and jest timers/act to advance timers and query displayed text.
25-44: Interval torn down and recreated on everytextprop change.With
[text, isAnimating]as the effect dependency array, every incoming streaming chunk fires the cleanup + setup cycle. While correctness is preserved (thestartsWithguard prevents regression), frequent teardown/recreation of the interval wastes work. A ref-based target-text approach avoids this:♻️ Proposed refactor — stable interval, ref-tracked target
+import { useEffect, useRef, useState } from "react"; -import { useEffect, useState } from "react"; export function StreamingMessageText({ text, isAnimating, mermaid, components }: StreamingMessageTextProps) { const [displayText, setDisplayText] = useState(text); + const targetRef = useRef(text); + const isAnimatingRef = useRef(isAnimating); useEffect(() => { + targetRef.current = text; + isAnimatingRef.current = isAnimating; + if (!isAnimating) { setDisplayText(text); - return; + } else { + setDisplayText((previous) => (text.startsWith(previous) ? previous : text)); } + }, [text, isAnimating]); - setDisplayText((previous) => (text.startsWith(previous) ? previous : text)); - - const intervalId = window.setInterval(() => { + useEffect(() => { + const intervalId = window.setInterval(() => { setDisplayText((previous) => { - if (previous.length >= text.length) return previous; + const target = targetRef.current; + if (!isAnimatingRef.current) return target; + if (previous.length >= target.length) return previous; const nextLength = Math.min( - text.length, + target.length, previous.length + STREAM_TEXT_CHARS_PER_TICK, ); - return text.slice(0, nextLength); + return target.slice(0, nextLength); }); }, STREAM_TEXT_TICK_MS); - return () => window.clearInterval(intervalId); - }, [text, isAnimating]); + }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MessagePartsRenderer/components/StreamingMessageText/StreamingMessageText.tsx` around lines 25 - 44, The effect currently recreates the interval on every text change; change it to use a ref-tracked target so the interval can remain stable: add a targetTextRef that you update whenever the text prop changes, then modify the useEffect that currently references text and isAnimating so it only depends on isAnimating (and sets/clears the interval based on isAnimating); inside the interval callback read targetTextRef.current to compute the next slice and call setDisplayText, preserving the startsWith guard on initial sync when isAnimating flips false to true (update logic around setDisplayText, useEffect, isAnimating, text, STREAM_TEXT_CHARS_PER_TICK, and STREAM_TEXT_TICK_MS accordingly).packages/ui/src/components/ai-elements/message.tsx (1)
318-325:mode="streaming"hardcoded as default even for completed (non-animating) messages.The Streamdown usage pattern in the library's own docs conditionally sets
mode={isStreaming ? 'streaming' : 'static'}, ensuring completed messages use static rendering (no streaming-specific overhead). Heremode="streaming"applies to all messages by default. While callers can override it via{...props}(sincemodeis not destructured), consumers that don't explicitly passmodewill get streaming mode even for fully-rendered history messages.♻️ Proposed refactor
export const MessageResponse = memo( - ({ className, animated, isAnimating, ...props }: MessageResponseProps) => ( + ({ className, animated, isAnimating, mode, ...props }: MessageResponseProps) => ( <Streamdown animated={animated ?? defaultMessageAnimation} className={cn( "text-foreground [&>*:first-child]:mt-0 [&>*:last-child]:mb-0 [&_ol]:list-outside [&_ol]:pl-6 [&_ul]:list-outside [&_ul]:pl-6 [&_:not(pre)>code]:break-all", className, )} isAnimating={isAnimating} - mode="streaming" + mode={mode ?? (isAnimating ? "streaming" : "static")} plugins={isAnimating ? undefined : streamdownPlugins} {...props} /> ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/message.tsx` around lines 318 - 325, The JSX currently hardcodes mode="streaming" which forces streaming rendering for all messages; change it to respect a provided mode prop and otherwise pick streaming only when isAnimating is true by replacing mode="streaming" with something like: mode={typeof mode !== "undefined" ? mode : (isAnimating ? "streaming" : "static")}, keeping the other props (animated, className, isAnimating, plugins/streamdownPlugins) unchanged so callers that pass an explicit mode still win and completed messages render in static mode.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MessagePartsRenderer/components/StreamingMessageText/StreamingMessageText.tsx`:
- Around line 47-55: StreamingMessageText currently forces MessageResponse to
think it's not animating (isAnimating={false}), which allows plugins like
mermaid to run on partial markdown; update StreamingMessageText to forward the
real animation state instead: locate the StreamingMessageText component and pass
the actual animation flag (e.g., isAnimating or animationInProgress) into
<MessageResponse isAnimating={isAnimating}> (ensure the component either
receives that prop from its parent or computes it from its internal state used
to drive the character-by-character stream) so MessageResponse can disable
streamdownPlugins during animation.
In `@packages/ui/src/components/ai-elements/message.tsx`:
- Line 40: The user-role classes on the Message container are wrong: remove
"ml-auto" and "justify-end" from the conditional class for from === "user" and
replace them with "items-end" so the flex-col container aligns children to the
right; keep existing group-[.is-user]:ml-auto and group-[.is-user]:w-fit on
MessageContent (which already handle horizontal shrinking and right placement).
Update the class expression in the Message component (the conditional that
currently returns "is-user ml-auto justify-end" vs "is-assistant") to return
"is-user items-end" for the user case.
- Around line 236-239: The border-radius selectors are inverted in the cn(...)
call in Message's className: replace the negated selectors that target inner
edges with selectors that target the outer edges (use :first-child to apply
rounded-l-md and :last-child to apply rounded-r-md) so the first child gets left
rounding and the last child gets right rounding; update the selector string
passed to cn(...) around the className variable in message.tsx accordingly.
- Line 56: The class string "is-user:dark" in the Message component
(packages/ui/src/components/ai-elements/message.tsx) is a no-op and should be
removed; locate the class list containing "is-user:dark flex min-w-0 max-w-full
flex-col gap-2 overflow-hidden text-sm select-text" and delete the
"is-user:dark" token, and if you need conditional styling for user messages
follow the existing pattern used elsewhere (e.g., use group-[.is-user]:
modifiers) to apply variant styles instead.
- Around line 25-30: Remove the dead/ineffective CSS and fix the inverted
border-radius selectors: in the Message container component (the element using
ml-auto justify-end), remove the classes "ml-auto" and "justify-end" since they
are ineffective for a w-full flex-col container; in the MessageContent component
remove the invalid "is-user:dark" variant (it's not a valid Tailwind variant and
should be deleted); and update the grouped button radius selectors that
currently use "[&>*:not(:first-child)]:rounded-l-md" and
"[&>*:not(:last-child)]:rounded-r-md" to the correct
"[&>*:first-child]:rounded-l-md" and "[&>*:last-child]:rounded-r-md" so only the
outer edges of the first and last children are rounded.
In `@packages/ui/src/components/ai-elements/reasoning.tsx`:
- Line 122: The Shimmer speed change is likely unintentional—restore the
original faster shimmer by adding the duration prop back to the Shimmer usage in
the render return (i.e., change the JSX return in the component that currently
returns <Shimmer>Thinking...</Shimmer> to include duration={1}); ensure the
Shimmer import/props match existing API and run the component to verify the
shimmer feels snappy during active reasoning.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MessageList/MessageList.tsx`:
- Around line 80-81: The current shouldAnimateStreaming expression in
MessageList.tsx (const shouldAnimateStreaming = isLastAssistant && (isStreaming
|| submitStatus === "submitted")) causes StreamingMessageText to start timers
when submitStatus === "submitted" even though no tokens are flowing; either
tighten the condition to only use isStreaming (const shouldAnimateStreaming =
isLastAssistant && isStreaming) so animation/timers only start when tokens are
actually streaming, or if the pre-warm behavior is intentional, keep the
expression but add an inline comment near shouldAnimateStreaming explaining why
submitStatus === "submitted" must trigger the animation.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MessagePartsRenderer/components/StreamingMessageText/StreamingMessageText.tsx`:
- Around line 1-57: Add a co-located test file StreamingMessageText.test.tsx
that mounts the StreamingMessageText component and exercises its stateful
behavior: test initial render with isAnimating=false shows full text
immediately; test isAnimating=true advances displayText over time by mocking
timers (use jest.useFakeTimers()) and asserting partial text after one tick
(STREAM_TEXT_TICK_MS and STREAM_TEXT_CHARS_PER_TICK constants can be referenced
to compute expected length); test toggling isAnimating from true to false snaps
displayText to the full text immediately; test replacing text with a new
non-extension string resets displayText to the new text (uses startsWith guard
path). Use render from testing-library/react and jest timers/act to advance
timers and query displayed text.
- Around line 25-44: The effect currently recreates the interval on every text
change; change it to use a ref-tracked target so the interval can remain stable:
add a targetTextRef that you update whenever the text prop changes, then modify
the useEffect that currently references text and isAnimating so it only depends
on isAnimating (and sets/clears the interval based on isAnimating); inside the
interval callback read targetTextRef.current to compute the next slice and call
setDisplayText, preserving the startsWith guard on initial sync when isAnimating
flips false to true (update logic around setDisplayText, useEffect, isAnimating,
text, STREAM_TEXT_CHARS_PER_TICK, and STREAM_TEXT_TICK_MS accordingly).
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MessagePartsRenderer/MessagePartsRenderer.tsx`:
- Around line 99-105: The isLastAssistant check is redundant because MessageList
already computes shouldAnimateStreaming = isLastAssistant && (isStreaming ||
submitStatus === "submitted") and passes it as the isStreaming prop; update the
StreamingMessageText usage to rely on the isStreaming prop directly (remove the
extra isLastAssistant && wrapper) so that the isAnimating prop becomes simply
isStreaming, keeping mermaidConfig/components unchanged and retaining
StreamingMessageText as the component to modify.
In `@packages/ui/src/components/ai-elements/message.tsx`:
- Around line 318-325: The JSX currently hardcodes mode="streaming" which forces
streaming rendering for all messages; change it to respect a provided mode prop
and otherwise pick streaming only when isAnimating is true by replacing
mode="streaming" with something like: mode={typeof mode !== "undefined" ? mode :
(isAnimating ? "streaming" : "static")}, keeping the other props (animated,
className, isAnimating, plugins/streamdownPlugins) unchanged so callers that
pass an explicit mode still win and completed messages render in static mode.
In `@packages/ui/src/components/ai-elements/shimmer-label.tsx`:
- Around line 17-33: ShimmerLabel recreates its subtree on every parent render
while Shimmer is memoized; wrap ShimmerLabel with React.memo to avoid
unnecessary re-renders. Import memo from React (or use React.memo) and export
the memoized component (e.g., wrap the ShimmerLabel function with memo and set a
displayName like "ShimmerLabel" if desired) so the component benefits from the
same memoization as Shimmer.
- Around line 1-33: Add a co-located test file named shimmer-label.test.tsx next
to shimmer-label.tsx that imports the ShimmerLabel component and verifies both
render paths: when isShimmering=true it renders the Shimmer child (expecting
presence of the Shimmer element or class passed via shimmerClassName) and when
isShimmering=false it renders the raw children text; use the ShimmerLabel props
(children, shimmerClassName, isShimmering) to drive the two assertions and a
test renderer like React Testing Library to mount and query for text vs shimmer
element.
| return ( | ||
| <MessageResponse | ||
| animated={false} | ||
| isAnimating={false} | ||
| mermaid={mermaid} | ||
| components={components} | ||
| > | ||
| {displayText} | ||
| </MessageResponse> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the MessageResponse component definition
fd -t f "MessageResponse" --extension tsx --extension tsRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Search for MessageResponse component definition in the codebase
rg "export.*MessageResponse|function MessageResponse|const MessageResponse" -t ts -t tsxRepository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Look for the streamdownPlugins usage pattern
rg "streamdownPlugins" -t ts -t tsx -A 2 -B 2Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Find the StreamingMessageText.tsx file to understand context
fd "StreamingMessageText.tsx" -t fRepository: superset-sh/superset
Length of output: 271
🏁 Script executed:
# Search for MessageResponse without file type
rg "export.*MessageResponse|function MessageResponse|const MessageResponse" -A 3Repository: superset-sh/superset
Length of output: 618
🏁 Script executed:
# Search for streamdownPlugins
rg "streamdownPlugins" -A 3 -B 3Repository: superset-sh/superset
Length of output: 1066
🏁 Script executed:
# Read the StreamingMessageText file to understand context
cat apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MessagePartsRenderer/components/StreamingMessageText/StreamingMessageText.tsxRepository: superset-sh/superset
Length of output: 1348
🏁 Script executed:
# Search for MessageResponse file location
fd "MessageResponse" -t fRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Read the full MessageResponse component to understand the complete behavior
cat packages/ui/src/components/ai-elements/message.tsxRepository: superset-sh/superset
Length of output: 10689
🏁 Script executed:
# Search for Streamdown component to understand how it handles partial markdown
rg "Streamdown" -B 5 -A 10 | head -100Repository: superset-sh/superset
Length of output: 8855
🏁 Script executed:
# Look for any test files that might show expected behavior
fd "StreamingMessageText.test" -o fd "message.test" -o fd "streamdown" -iRepository: superset-sh/superset
Length of output: 187
🏁 Script executed:
# Search for comments or documentation about why plugins are disabled during isAnimating
rg "isAnimating.*plugins|plugins.*isAnimating" -B 3 -A 3Repository: superset-sh/superset
Length of output: 555
🏁 Script executed:
# Look for any issues or errors related to partial markdown rendering
rg "partial.*markdown|incomplete.*markdown|broken.*fence" -iRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check if there are git history comments about this logic
git log --oneline packages/ui/src/components/ai-elements/message.tsx | head -20Repository: superset-sh/superset
Length of output: 79
🏁 Script executed:
# Search for mermaid plugin usage context
rg "@streamdown/mermaid" -B 3 -A 3Repository: superset-sh/superset
Length of output: 3197
🏁 Script executed:
# Search for any other uses of StreamingMessageText to understand the pattern
rg "StreamingMessageText" -B 3 -A 3Repository: superset-sh/superset
Length of output: 7650
🏁 Script executed:
# Check the Streamdown package docs or types to understand mode="streaming"
rg "mode.*streaming|streaming.*mode" -B 2 -A 2Repository: superset-sh/superset
Length of output: 901
🏁 Script executed:
# Look for any error handling or edge cases with markdown plugins
rg "mermaid|Streamdown" apps/desktop/src/renderer/components/MarkdownRenderer -B 2 -A 5Repository: superset-sh/superset
Length of output: 3681
🏁 Script executed:
# Check if there are any related issues or comments about plugin behavior during animation
rg "animated.*plugin|plugin.*animated|isAnimating" -B 2 -A 2Repository: superset-sh/superset
Length of output: 8234
Pass isAnimating prop through to MessageResponse.
MessageResponse disables plugins during animation with plugins={isAnimating ? undefined : streamdownPlugins} to safely handle partial markdown. By hardcoding isAnimating={false}, the mermaid plugin runs on incomplete markdown during the character-by-character animation, potentially causing rendering artifacts. Thread the actual animation state instead.
🔧 Proposed fix
return (
<MessageResponse
animated={false}
- isAnimating={false}
+ isAnimating={isAnimating}
mermaid={mermaid}
components={components}
>
{displayText}
</MessageResponse>
);📝 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.
| return ( | |
| <MessageResponse | |
| animated={false} | |
| isAnimating={false} | |
| mermaid={mermaid} | |
| components={components} | |
| > | |
| {displayText} | |
| </MessageResponse> | |
| return ( | |
| <MessageResponse | |
| animated={false} | |
| isAnimating={isAnimating} | |
| mermaid={mermaid} | |
| components={components} | |
| > | |
| {displayText} | |
| </MessageResponse> | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MessagePartsRenderer/components/StreamingMessageText/StreamingMessageText.tsx`
around lines 47 - 55, StreamingMessageText currently forces MessageResponse to
think it's not animating (isAnimating={false}), which allows plugins like
mermaid to run on partial markdown; update StreamingMessageText to forward the
real animation state instead: locate the StreamingMessageText component and pass
the actual animation flag (e.g., isAnimating or animationInProgress) into
<MessageResponse isAnimating={isAnimating}> (ensure the component either
receives that prop from its parent or computes it from its internal state used
to drive the character-by-character stream) so MessageResponse can disable
streamdownPlugins during animation.
| const defaultMessageAnimation = { | ||
| animation: "blurIn", | ||
| sep: "char", | ||
| duration: 180, | ||
| easing: "cubic-bezier(0.22, 1, 0.36, 1)", | ||
| } as const; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's see the actual file content around the flagged lines
head -100 packages/ui/src/components/ai-elements/message.tsx | tail -80Repository: superset-sh/superset
Length of output: 1910
🏁 Script executed:
# Check if Streamdown is installed and find its type definitions
fd -name "*.json" | grep -E "(package|tsconfig)" | head -5Repository: superset-sh/superset
Length of output: 294
🏁 Script executed:
# Search for Streamdown in package.json to understand the version
rg "streamdown" package.json -A 1Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check for Streamdown in dependencies
cat package.json | grep -A 2 -B 2 "streamdown"Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Get file size and full content around the flagged lines
wc -l packages/ui/src/components/ai-elements/message.tsxRepository: superset-sh/superset
Length of output: 118
🏁 Script executed:
# Read the full file in sections to see all flagged lines
cat -n packages/ui/src/components/ai-elements/message.tsx | sed -n '1,80p'Repository: superset-sh/superset
Length of output: 2702
🏁 Script executed:
# Look for tailwind.config and check for custom variants
fd "tailwind.config" --type fRepository: superset-sh/superset
Length of output: 97
🏁 Script executed:
# Get lines around 236-239 for MessageBranchSelector border-radius context
cat -n packages/ui/src/components/ai-elements/message.tsx | sed -n '220,250p'Repository: superset-sh/superset
Length of output: 928
🏁 Script executed:
# Get lines around 318/324 for MessageResponse animated prop context
cat -n packages/ui/src/components/ai-elements/message.tsx | sed -n '310,340p'Repository: superset-sh/superset
Length of output: 1256
🏁 Script executed:
# Check for tailwind config in packages/ui
fd "tailwind.config" packages/ui --type fRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Search for Streamdown in the workspace root package.json
rg "streamdown" -A 1 packages/ui/package.jsonRepository: superset-sh/superset
Length of output: 195
🏁 Script executed:
# Check if node_modules streamdown types exist and search for animated prop definition
rg "animated" -g "*/streamdown/**" --type ts -A 2 -B 1 | head -40Repository: superset-sh/superset
Length of output: 212
🏁 Script executed:
# Also try finding streamdown package info or tsconfig references
find . -name "package.json" -path "*/streamdown/*" | head -3Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Search for tailwind.config in the entire packages/ui directory tree
find packages/ui -name "*.config.*" -type f | grep -E "(tail|tw)"Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check the root tailwind.config to understand if is-user variant is defined
find . -maxdepth 2 -name "tailwind.config.*" -type fRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Search for other Streamdown component usages in the codebase to see if animated is used elsewhere
rg "Streamdown" --type ts --type tsx -B 2 -A 5 | head -60Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Search for is-user variant definition in the codebase
rg "is-user" --type ts --type tsx --type js -B 2 -A 2 | head -40Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Look for tailwind config or variant definitions anywhere
rg "variants|extend|theme" --type ts -g "*config*" | head -30Repository: superset-sh/superset
Length of output: 2321
🏁 Script executed:
# Check if there's any documentation or comments about the animated prop
rg "animated" packages/ui/src/components/ai-elements/message.tsx -B 3 -A 3Repository: superset-sh/superset
Length of output: 506
🏁 Script executed:
# Try without tsx type specification
rg "Streamdown" -B 2 -A 5 | head -80Repository: superset-sh/superset
Length of output: 5620
🏁 Script executed:
# Search for is-user in any file type
rg "is-user" -B 2 -A 2 | head -40Repository: superset-sh/superset
Length of output: 1156
🌐 Web query:
Streamdown v2.2.0 animated prop type definition TypeScript
💡 Result:
In Streamdown v2.2.x (including v2.2.0), the “animated” flag is the isAnimating prop, typed as a boolean:
isAnimating?: boolean;It’s listed as a core prop in the Streamdown configuration docs [1]. It’s also referenced by the caret feature docs (carets show only when isAnimating={true} in streaming mode) [2].
Sources: [1] [2]
🌐 Web query:
Streamdown GitHub animated prop boolean or object configuration
💡 Result:
In Streamdown, the animated prop is typed as boolean | AnimateOptions. When set to true, it enables the default per-word animation; when set to an object, you can customize it. The animation is only active while isAnimating is true—when isAnimating={false}, the animate plugin is removed from the rehype pipeline (so completed messages render without the extra <span> wrappers). [1][2]
Enable (boolean)
import { Streamdown } from "streamdown";
import "streamdown/styles.css";
<Streamdown animated isAnimating={status === "streaming"}>
{markdown}
</Streamdown>[1][3]
Configure (object)
<Streamdown
animated={{
animation: "blurIn", // "fadeIn" | "blurIn" | "slideUp" | custom string (prefixed with `sd-`)
duration: 200, // ms (default 150)
easing: "ease-out", // CSS timing function (default "ease")
sep: "word", // "word" | "char" (default "word")
}}
isAnimating={status === "streaming"}
>
{markdown}
</Streamdown>[1]
Note: To use animated, you must import the animation CSS: import "streamdown/styles.css"; [1][3]
Remove dead CSS and fix inverted border-radius selectors.
Line 40: The ml-auto justify-end classes on the Message container are ineffective. ml-auto cannot push a w-full element right (already full width), and justify-end on flex-col aligns children to the bottom, not the right. The actual right-alignment comes from group-[.is-user]:ml-auto group-[.is-user]:w-fit applied to MessageContent (line 57). Remove ml-auto justify-end from line 40.
Line 56: The is-user:dark class is invalid—is-user is not a registered Tailwind variant, and dark is a mode modifier, not a utility class. This has no visual effect and should be removed.
Lines 236–239: The border-radius selectors are inverted. Currently, [&>*:not(:first-child)]:rounded-l-md rounds the left side of all children except the first, and [&>*:not(:last-child)]:rounded-r-md rounds the right side of all children except the last. This reverses the intended styling. Use [&>*:first-child]:rounded-l-md [&>*:last-child]:rounded-r-md instead to round only the outer edges of the first and last buttons.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/ai-elements/message.tsx` around lines 25 - 30,
Remove the dead/ineffective CSS and fix the inverted border-radius selectors: in
the Message container component (the element using ml-auto justify-end), remove
the classes "ml-auto" and "justify-end" since they are ineffective for a w-full
flex-col container; in the MessageContent component remove the invalid
"is-user:dark" variant (it's not a valid Tailwind variant and should be
deleted); and update the grouped button radius selectors that currently use
"[&>*:not(:first-child)]:rounded-l-md" and "[&>*:not(:last-child)]:rounded-r-md"
to the correct "[&>*:first-child]:rounded-l-md" and
"[&>*:last-child]:rounded-r-md" so only the outer edges of the first and last
children are rounded.
| className={cn( | ||
| "group flex w-full flex-col gap-2", | ||
| from === "user" ? "is-user" : "is-assistant", | ||
| from === "user" ? "is-user ml-auto justify-end" : "is-assistant", |
There was a problem hiding this comment.
ml-auto is a no-op on w-full, and justify-end aligns vertically not horizontally.
Two issues with the user-role classes appended to a flex-col w-full container:
ml-autoon awidth: 100%element has no effect — the element is already full-width.justify-endin aflex-colcontext aligns children to the bottom of the container (main axis is vertical), not to the right. This could pushMessageContentandMessageActionsto the bottom if theMessagecontainer ever has surplus height.
The actual right-alignment is correctly handled by group-[.is-user]:ml-auto group-[.is-user]:w-fit on MessageContent (line 57). If the intent is also to right-align items within the column, items-end is the appropriate class.
🐛 Proposed fix
- from === "user" ? "is-user ml-auto justify-end" : "is-assistant",
+ from === "user" ? "is-user items-end" : "is-assistant",📝 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.
| from === "user" ? "is-user ml-auto justify-end" : "is-assistant", | |
| from === "user" ? "is-user items-end" : "is-assistant", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/ai-elements/message.tsx` at line 40, The user-role
classes on the Message container are wrong: remove "ml-auto" and "justify-end"
from the conditional class for from === "user" and replace them with "items-end"
so the flex-col container aligns children to the right; keep existing
group-[.is-user]:ml-auto and group-[.is-user]:w-fit on MessageContent (which
already handle horizontal shrinking and right placement). Update the class
expression in the Message component (the conditional that currently returns
"is-user ml-auto justify-end" vs "is-assistant") to return "is-user items-end"
for the user case.
| className={cn( | ||
| "flex w-full min-w-0 flex-col gap-3 text-sm select-text", | ||
| "group-[.is-user]:w-full", | ||
| "is-user:dark flex min-w-0 max-w-full flex-col gap-2 overflow-hidden text-sm select-text", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find tailwind config files
fd -e ts -e js -e cjs -e mjs "tailwind.config" | head -20Repository: superset-sh/superset
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Check for is-user variant definitions in tailwind config and CSS files
rg "is-user" --type=ts --type=js --type=css -nRepository: superset-sh/superset
Length of output: 577
🏁 Script executed:
#!/bin/bash
# Look at the specific file context
head -100 packages/ui/src/components/ai-elements/message.tsx | cat -nRepository: superset-sh/superset
Length of output: 3206
🏁 Script executed:
#!/bin/bash
# Read the tailwind config file for packages/email
cat -n packages/email/tailwind.config.tsRepository: superset-sh/superset
Length of output: 1253
🏁 Script executed:
#!/bin/bash
# Check if there are other tailwind config files in the packages/ui directory
fd -e ts -e js "tailwind.config" packages/ui/Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Look for any custom variant registrations in the entire codebase
rg "addVariant|addUtilities" --type=ts --type=js -A 3 | head -50Repository: superset-sh/superset
Length of output: 46
Remove the is-user:dark class — it produces no CSS.
is-user is not a registered Tailwind variant, and dark is not a utility class. The string is-user:dark is a no-op and generates no CSS. Remove it from line 56. The correct pattern for conditional styling based on the is-user class is already being used elsewhere in this file with group-[.is-user]:, so use that approach if additional styling is needed for user messages.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/ai-elements/message.tsx` at line 56, The class
string "is-user:dark" in the Message component
(packages/ui/src/components/ai-elements/message.tsx) is a no-op and should be
removed; locate the class list containing "is-user:dark flex min-w-0 max-w-full
flex-col gap-2 overflow-hidden text-sm select-text" and delete the
"is-user:dark" token, and if you need conditional styling for user messages
follow the existing pattern used elsewhere (e.g., use group-[.is-user]:
modifiers) to apply variant styles instead.
| className={cn( | ||
| "[&>*:not(:first-child)]:rounded-l-md [&>*:not(:last-child)]:rounded-r-md", | ||
| className, | ||
| )} |
There was a problem hiding this comment.
Inverted :not() selectors apply inner-edge rounding instead of outer-edge rounding.
The current selectors produce the opposite of the intended button-group border-radius:
| Button | Gets from :not(:first-child) |
Gets from :not(:last-child) |
Result |
|---|---|---|---|
| B1 (first) | — | rounded-r-md |
inner-right rounded ❌ |
| B2 (middle) | rounded-l-md |
rounded-r-md |
fully rounded ❌ |
| B3 (last) | rounded-l-md |
— | inner-left rounded ❌ |
For a connected horizontal button group, the outer corners — B1's left and B3's right — should be rounded. Remove the not() negation:
🐛 Proposed fix
<ButtonGroup
className={cn(
- "[&>*:not(:first-child)]:rounded-l-md [&>*:not(:last-child)]:rounded-r-md",
+ "[&>*:first-child]:rounded-l-md [&>*:last-child]:rounded-r-md",
className,
)}
orientation="horizontal"
{...props}
/>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/ai-elements/message.tsx` around lines 236 - 239,
The border-radius selectors are inverted in the cn(...) call in Message's
className: replace the negated selectors that target inner edges with selectors
that target the outer edges (use :first-child to apply rounded-l-md and
:last-child to apply rounded-r-md) so the first child gets left rounding and the
last child gets right rounding; update the selector string passed to cn(...)
around the className variable in message.tsx accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/ui/src/components/ai-elements/bash-tool.tsx (2)
64-64:_hasOutputis dead code — remove it.
_hasOutput(line 64) is declared but never read;hasOutputon line 81 (which also includescommand) is what's actually used throughout the component. The underscore prefix suggests it was already known to be unused, but it should be removed to keep the file clean.🧹 Proposed cleanup
- const _hasOutput = Boolean(stdout || stderr); - const stdoutLimited = useMemo(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/bash-tool.tsx` at line 64, Remove the dead declaration of _hasOutput (const _hasOutput = Boolean(stdout || stderr);) from the BashTool component; rely on the existing hasOutput (and command) logic already used elsewhere, and ensure there are no remaining references to _hasOutput so the file compiles cleanly.
74-74:hasMoreOutputis computed but never consumed — either wire it up or remove it.
hasMoreOutput(line 74) is derived fromstdoutLimited.truncated || stderrLimited.truncatedbut is not referenced anywhere in the JSX or in any conditional logic. If it was meant to drive a visual "expand" affordance (e.g., a chevron in the header when content is truncated), that wiring is missing — users currently have no indication that the collapsed view is truncated and that clicking the header reveals more. Otherwise it should be removed.🧹 Proposed cleanup if no expand indicator is needed
- const hasMoreOutput = stdoutLimited.truncated || stderrLimited.truncated; - const commandSummary = useMemo(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/bash-tool.tsx` at line 74, The local const hasMoreOutput (computed from stdoutLimited.truncated || stderrLimited.truncated) is unused; either remove this unused variable from the BashTool component or wire it into the UI (e.g., render a truncated indicator/chevron in the component header or next to the output block and use hasMoreOutput to toggle its visibility and/or the expand/collapse behavior). Locate the declaration of hasMoreOutput and the JSX for the output/header (references: hasMoreOutput, stdoutLimited, stderrLimited, BashTool) and either delete the unused const and any related dead code, or add a conditional element in the JSX that uses hasMoreOutput to show an affordance and hook it into the existing expand/collapse handler.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MastraToolCallBlock/MastraToolCallBlock.tsx (1)
212-215:toToolDisplayState(part)is called twice — extract to a local variable.Both branches of the ternary call the same pure function on the same argument. A single variable avoids the redundant call and makes the intent clearer.
♻️ Proposed refactor
+const baseDisplayState = toToolDisplayState(part); const displayState = - isOutputError && toToolDisplayState(part) === "output-available" + isOutputError && baseDisplayState === "output-available" ? "output-error" - : toToolDisplayState(part); + : baseDisplayState;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MastraToolCallBlock/MastraToolCallBlock.tsx` around lines 212 - 215, The ternary calls toToolDisplayState(part) twice; extract its result into a local variable (e.g., const toolState = toToolDisplayState(part)) and then compute displayState using that variable (use toolState in place of both toToolDisplayState(part) occurrences), keeping the existing conditional logic that uses isOutputError and part unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MastraToolCallBlock/MastraToolCallBlock.tsx`:
- Around line 206-210: isOutputError currently only detects boolean true,
missing string/object error payloads; update the logic in MastraToolCallBlock to
treat any non-null output.error as an error (e.g., check "error" in output &&
!!(output as any).error) and adjust the errorText extraction used later to
handle string errors and object errors (use (output as any).error if it's a
string, else fallback to (output as any).message or JSON.stringify((output as
any).error) for objects) so tool responses like { error: "msg" } or { error: {
code:1 } } are correctly flagged and displayed.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MastraToolCallBlock/MastraToolCallBlock.tsx`:
- Around line 212-215: The ternary calls toToolDisplayState(part) twice; extract
its result into a local variable (e.g., const toolState =
toToolDisplayState(part)) and then compute displayState using that variable (use
toolState in place of both toToolDisplayState(part) occurrences), keeping the
existing conditional logic that uses isOutputError and part unchanged.
In `@packages/ui/src/components/ai-elements/bash-tool.tsx`:
- Line 64: Remove the dead declaration of _hasOutput (const _hasOutput =
Boolean(stdout || stderr);) from the BashTool component; rely on the existing
hasOutput (and command) logic already used elsewhere, and ensure there are no
remaining references to _hasOutput so the file compiles cleanly.
- Line 74: The local const hasMoreOutput (computed from stdoutLimited.truncated
|| stderrLimited.truncated) is unused; either remove this unused variable from
the BashTool component or wire it into the UI (e.g., render a truncated
indicator/chevron in the component header or next to the output block and use
hasMoreOutput to toggle its visibility and/or the expand/collapse behavior).
Locate the declaration of hasMoreOutput and the JSX for the output/header
(references: hasMoreOutput, stdoutLimited, stderrLimited, BashTool) and either
delete the unused const and any related dead code, or add a conditional element
in the JSX that uses hasMoreOutput to show an affordance and hook it into the
existing expand/collapse handler.
| const isOutputError = | ||
| output != null && | ||
| typeof output === "object" && | ||
| "error" in output && | ||
| (output as { error?: boolean }).error === true; |
There was a problem hiding this comment.
isOutputError only catches error === true; string/object error payloads go undetected.
The guard (output as { error?: boolean }).error === true is satisfied exclusively when error is the boolean true. Common error shapes like { error: "command not found" } (string) or { error: { code: 1 } } (object) all evaluate to false, so these tools silently render as successful output. The companion errorText extraction (line 217) expecting a separate message field reinforces a very specific convention ({ error: true, message: "..." }) that may not be followed uniformly across tools.
Consider broadening the truthiness check:
🛡️ Proposed fix
const isOutputError =
output != null &&
typeof output === "object" &&
"error" in output &&
- (output as { error?: boolean }).error === true;
+ !!(output as { error?: unknown }).error;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MastraToolCallBlock/MastraToolCallBlock.tsx`
around lines 206 - 210, isOutputError currently only detects boolean true,
missing string/object error payloads; update the logic in MastraToolCallBlock to
treat any non-null output.error as an error (e.g., check "error" in output &&
!!(output as any).error) and adjust the errorText extraction used later to
handle string errors and object errors (use (output as any).error if it's a
string, else fallback to (output as any).message or JSON.stringify((output as
any).error) for objects) so tool responses like { error: "msg" } or { error: {
code:1 } } are correctly flagged and displayed.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/ui/src/components/ai-elements/bash-tool.tsx (2)
119-122:fade-in-0missing from the open-state animation — asymmetric withfade-out-0on closeThe closed transition has
data-[state=closed]:fade-out-0but the open transition has no correspondingdata-[state=open]:fade-in-0, so the panel slides in without fading. Typical Radix/Tailwind patterns pair these symmetrically.♻️ Suggested fix
<CollapsibleContent className={cn( - "border-t border-border data-[state=closed]:fade-out-0 data-[state=closed]:slide-out-to-top-2 data-[state=open]:slide-in-from-top-2 outline-none data-[state=closed]:animate-out data-[state=open]:animate-in", + "border-t border-border data-[state=closed]:fade-out-0 data-[state=closed]:slide-out-to-top-2 data-[state=open]:fade-in-0 data-[state=open]:slide-in-from-top-2 outline-none data-[state=closed]:animate-out data-[state=open]:animate-in", )} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/bash-tool.tsx` around lines 119 - 122, CollapsibleContent's animation classes are asymmetric: add the matching open-state fade class so the panel fades in as it slides in. Update the className on CollapsibleContent to include data-[state=open]:fade-in-0 alongside the existing data-[state=closed]:fade-out-0 (preserving other classes like data-[state=open]:slide-in-from-top-2 and data-[state=closed]:slide-out-to-top-2) so open and close animations are symmetric.
60-60:hasOutputname is slightly misleading — considerhasContent
commandis included in the check, so this is true whenever there's any content to render (command, stdout, or stderr), not just when there's shell output. The variable controls both the collapse trigger's enabled state and whether theCollapsibleContentis mounted, so the namehasContentwould better reflect its purpose.♻️ Suggested rename
- const hasOutput = Boolean(command || stdout || stderr); + const hasContent = Boolean(command || stdout || stderr);Then update the three references on lines 68, 69, 74, 78, 118 accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/bash-tool.tsx` at line 60, Rename the boolean hasOutput to hasContent in packages/ui/src/components/ai-elements/bash-tool.tsx because it reflects any renderable content (command, stdout, stderr), not just shell output; update the declaration (const hasOutput = ...) to const hasContent = Boolean(command || stdout || stderr) and replace every use of hasOutput in this component—including where it controls the collapse trigger enabled state and the CollapsibleContent mounting/props (the places currently referencing hasOutput) to use hasContent instead so the variable name accurately describes its purpose.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/ui/src/components/ai-elements/bash-tool.tsx`:
- Around line 119-122: CollapsibleContent's animation classes are asymmetric:
add the matching open-state fade class so the panel fades in as it slides in.
Update the className on CollapsibleContent to include
data-[state=open]:fade-in-0 alongside the existing
data-[state=closed]:fade-out-0 (preserving other classes like
data-[state=open]:slide-in-from-top-2 and
data-[state=closed]:slide-out-to-top-2) so open and close animations are
symmetric.
- Line 60: Rename the boolean hasOutput to hasContent in
packages/ui/src/components/ai-elements/bash-tool.tsx because it reflects any
renderable content (command, stdout, stderr), not just shell output; update the
declaration (const hasOutput = ...) to const hasContent = Boolean(command ||
stdout || stderr) and replace every use of hasOutput in this component—including
where it controls the collapse trigger enabled state and the CollapsibleContent
mounting/props (the places currently referencing hasOutput) to use hasContent
instead so the variable name accurately describes its purpose.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
packages/ui/src/components/ai-elements/tool.tsx (2)
60-74: Consider extracting the static icon map to module-level.All entries in the
iconsRecord are purely static JSX with no runtime dependencies. Moving it outside the function avoids recreating the object and its React element descriptors on every render call.♻️ Proposed refactor
-const getStatusIcon = (status: ToolDisplayState) => { - const icons: Record<ToolDisplayState, ReactNode> = { - "awaiting-input": <CircleIcon className="size-3" />, - "input-streaming": <CircleIcon className="size-3" />, - "input-complete": <ClockIcon className="size-3 animate-pulse" />, - "input-available": <ClockIcon className="size-3 animate-pulse" />, - "approval-requested": <ClockIcon className="size-3 text-yellow-600" />, - "approval-responded": <CheckCircleIcon className="size-3 text-blue-600" />, - "output-available": <CheckIcon className="size-3 text-green-600" />, - "output-error": <XIcon className="size-3 text-red-600" />, - "output-denied": <XCircleIcon className="size-3 text-orange-600" />, - }; - - return icons[status]; -}; +const STATUS_ICONS: Record<ToolDisplayState, ReactNode> = { + "awaiting-input": <CircleIcon className="size-3" />, + "input-streaming": <CircleIcon className="size-3" />, + "input-complete": <ClockIcon className="size-3 animate-pulse" />, + "input-available": <ClockIcon className="size-3 animate-pulse" />, + "approval-requested": <ClockIcon className="size-3 text-yellow-600" />, + "approval-responded": <CheckCircleIcon className="size-3 text-blue-600" />, + "output-available": <CheckIcon className="size-3 text-green-600" />, + "output-error": <XIcon className="size-3 text-red-600" />, + "output-denied": <XCircleIcon className="size-3 text-orange-600" />, +}; + +const getStatusIcon = (status: ToolDisplayState) => STATUS_ICONS[status];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/tool.tsx` around lines 60 - 74, The icon map inside getStatusIcon is recreated on every call; extract the static Record into a module-level constant (e.g., STATUS_ICONS or ICONS) typed as Record<ToolDisplayState, ReactNode> and initialize it with the same JSX entries, keep any required imports, then simplify getStatusIcon to just return STATUS_ICONS[status]; this removes per-render allocation while preserving types and behavior.
66-70: Hard-coded Tailwind color utilities may break dark-mode/theme consistency.The per-state colors (
text-yellow-600,text-blue-600,text-green-600,text-red-600,text-orange-600) are raw palette values rather than semantic design tokens. If the project defines tokens for warning/info/success/error states (e.g.,text-warning,text-destructive) those should be used here instead to ensure colors respond correctly to theme switching.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/tool.tsx` around lines 66 - 70, The status-to-icon mapping uses hard-coded Tailwind palette classes; update the className values for the icons (ClockIcon, CheckCircleIcon, CheckIcon, XIcon, XCircleIcon) to use the project's semantic color tokens instead (e.g., replace text-yellow-600/text-blue-600/text-green-600/text-red-600/text-orange-600 with text-warning, text-info, text-success, text-destructive, text-warning or your project's equivalent tokens) so colors follow theme/dark-mode rules; ensure you use the defined token names in the mapping where the keys are "approval-requested", "approval-responded", "output-available", "output-error", and "output-denied".packages/ui/src/components/ai-elements/bash-tool.tsx (2)
60-60:hasOutputname is misleading — it includescommand(an input field)
hasOutputgates both trigger enablement andCollapsibleContentrendering, butcommandis the tool's input, not its output. A name likehasContentwould more accurately reflect that the flag covers all three fields.♻️ Rename suggestion
- const hasOutput = Boolean(command || stdout || stderr); + const hasContent = Boolean(command || stdout || stderr);Then update all downstream references (
hasOutput→hasContenton lines 68, 69, 75, 79, 121).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/bash-tool.tsx` at line 60, Rename the misleading variable hasOutput to hasContent in packages/ui/src/components/ai-elements/bash-tool.tsx: change const hasOutput = Boolean(command || stdout || stderr) to const hasContent = Boolean(command || stdout || stderr) and update all downstream references (where hasOutput is used to enable triggers and to control CollapsibleContent rendering) to use hasContent instead; ensure usages inside JSX and any prop/handler names referencing hasOutput are updated consistently so the flag reflects that it covers both input (command) and outputs (stdout, stderr).
68-69:hasOutput &&guard inonOpenChangeis redundantThe trigger button is
disabled={!hasOutput}, so the browser suppresses the native click event and Radix's handler won't fire when there's no content. The guard is defensive but adds noise to a controlled component's callback.♻️ Simplification
- onOpenChange={(open) => hasOutput && setIsOutputExpanded(open)} + onOpenChange={setIsOutputExpanded}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/bash-tool.tsx` around lines 68 - 69, The onOpenChange callback uses a redundant guard "hasOutput &&" because the trigger is already disabled when !hasOutput; remove the guard and directly call setIsOutputExpanded(open) in the onOpenChange handler (leave the open prop logic as-is: open={hasOutput ? isOutputExpanded : false}) so onOpenChange becomes a simple setter using isOutputExpanded/setIsOutputExpanded without the extra condition.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ui/src/components/ai-elements/bash-tool.tsx`:
- Around line 96-117: The status area can render empty when state ===
"output-available" but exitCode is undefined; update the BashTool component to
handle that case by either narrowing the prop types so exitCode is required when
state === "output-available" or adding a UI fallback: inside the status render
(where isPending/isSuccess/isError are used) add a branch that detects state ===
"output-available" && exitCode == null and shows a neutral indicator (e.g.,
"Done" or a dash) so the user sees explicit feedback when no success/failure
flags are set.
- Line 3: Update the incorrect lucide-react import and usage: replace the import
symbol TerminalIcon with Terminal in the import list (keep CheckIcon and XIcon)
and then change all references to TerminalIcon in the BashTool component to use
Terminal instead (e.g., the icon usage inside the render/JSX where TerminalIcon
is referenced). This ensures the component imports { CheckIcon, Terminal, XIcon
} from lucide-react and uses Terminal wherever TerminalIcon was used.
---
Nitpick comments:
In `@packages/ui/src/components/ai-elements/bash-tool.tsx`:
- Line 60: Rename the misleading variable hasOutput to hasContent in
packages/ui/src/components/ai-elements/bash-tool.tsx: change const hasOutput =
Boolean(command || stdout || stderr) to const hasContent = Boolean(command ||
stdout || stderr) and update all downstream references (where hasOutput is used
to enable triggers and to control CollapsibleContent rendering) to use
hasContent instead; ensure usages inside JSX and any prop/handler names
referencing hasOutput are updated consistently so the flag reflects that it
covers both input (command) and outputs (stdout, stderr).
- Around line 68-69: The onOpenChange callback uses a redundant guard "hasOutput
&&" because the trigger is already disabled when !hasOutput; remove the guard
and directly call setIsOutputExpanded(open) in the onOpenChange handler (leave
the open prop logic as-is: open={hasOutput ? isOutputExpanded : false}) so
onOpenChange becomes a simple setter using isOutputExpanded/setIsOutputExpanded
without the extra condition.
In `@packages/ui/src/components/ai-elements/tool.tsx`:
- Around line 60-74: The icon map inside getStatusIcon is recreated on every
call; extract the static Record into a module-level constant (e.g., STATUS_ICONS
or ICONS) typed as Record<ToolDisplayState, ReactNode> and initialize it with
the same JSX entries, keep any required imports, then simplify getStatusIcon to
just return STATUS_ICONS[status]; this removes per-render allocation while
preserving types and behavior.
- Around line 66-70: The status-to-icon mapping uses hard-coded Tailwind palette
classes; update the className values for the icons (ClockIcon, CheckCircleIcon,
CheckIcon, XIcon, XCircleIcon) to use the project's semantic color tokens
instead (e.g., replace
text-yellow-600/text-blue-600/text-green-600/text-red-600/text-orange-600 with
text-warning, text-info, text-success, text-destructive, text-warning or your
project's equivalent tokens) so colors follow theme/dark-mode rules; ensure you
use the defined token names in the mapping where the keys are
"approval-requested", "approval-responded", "output-available", "output-error",
and "output-denied".
| "use client"; | ||
|
|
||
| import { CheckIcon, XIcon } from "lucide-react"; | ||
| import { CheckIcon, TerminalIcon, XIcon } from "lucide-react"; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
lucide-react 0.563.0 TerminalIcon available
💡 Result:
In lucide-react@0.563.0, the icon is exported as Terminal (Lucide uses a kebab-case → PascalCase naming convention, not *Icon-suffixed exports), so TerminalIcon is not a named export. [1][2]
Use:
import { Terminal } from "lucide-react";
// or, if you want that local name:
import { Terminal as TerminalIcon } from "lucide-react";Sources: [1] [2]
Fix incorrect lucide-react import: TerminalIcon does not exist at version 0.563.0
In lucide-react 0.563.0, the icon is exported as Terminal, not TerminalIcon. Update the import on line 3:
import { CheckIcon, Terminal, XIcon } from "lucide-react";Then use Terminal wherever TerminalIcon is referenced in the component (line 83).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/ai-elements/bash-tool.tsx` at line 3, Update the
incorrect lucide-react import and usage: replace the import symbol TerminalIcon
with Terminal in the import list (keep CheckIcon and XIcon) and then change all
references to TerminalIcon in the BashTool component to use Terminal instead
(e.g., the icon usage inside the render/JSX where TerminalIcon is referenced).
This ensures the component imports { CheckIcon, Terminal, XIcon } from
lucide-react and uses Terminal wherever TerminalIcon was used.
| {/* Status */} | ||
| <div className="ml-2 flex shrink-0 items-center gap-2"> | ||
| {!isPending && ( | ||
| <div className="flex items-center gap-1 text-xs text-muted-foreground"> | ||
| {isSuccess && ( | ||
| <> | ||
| <CheckIcon className="h-3 w-3" /> | ||
| <span>Success</span> | ||
| </> | ||
| )} | ||
| {isError && ( | ||
| <> | ||
| <XIcon className="h-3 w-3" /> | ||
| <span>Failed</span> | ||
| </> | ||
| )} | ||
| </div> | ||
| )} | ||
| <div className="flex h-6 w-6 items-center justify-center"> | ||
| {isPending && <Loader size={12} />} | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Silent no-op when state === "output-available" with exitCode === undefined
When output is available but exitCode is undefined, isPending is false, isSuccess is false, and isError is false, so the status area renders as an empty row with no feedback for the user. The type system does not prevent this combination.
If an exit code is always expected when output is available, consider narrowing the prop types or adding a fallback indicator (e.g., a neutral "Done" label):
{!isPending && (
<div className="flex items-center gap-1 text-xs text-muted-foreground">
{isSuccess && (
<>
<CheckIcon className="h-3 w-3" />
<span>Success</span>
</>
)}
{isError && (
<>
<XIcon className="h-3 w-3" />
<span>Failed</span>
</>
)}
+ {!isSuccess && !isError && (
+ <span>Done</span>
+ )}
</div>
)}📝 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.
| {/* Status */} | |
| <div className="ml-2 flex shrink-0 items-center gap-2"> | |
| {!isPending && ( | |
| <div className="flex items-center gap-1 text-xs text-muted-foreground"> | |
| {isSuccess && ( | |
| <> | |
| <CheckIcon className="h-3 w-3" /> | |
| <span>Success</span> | |
| </> | |
| )} | |
| {isError && ( | |
| <> | |
| <XIcon className="h-3 w-3" /> | |
| <span>Failed</span> | |
| </> | |
| )} | |
| </div> | |
| )} | |
| <div className="flex h-6 w-6 items-center justify-center"> | |
| {isPending && <Loader size={12} />} | |
| </div> | |
| </div> | |
| {/* Status */} | |
| <div className="ml-2 flex shrink-0 items-center gap-2"> | |
| {!isPending && ( | |
| <div className="flex items-center gap-1 text-xs text-muted-foreground"> | |
| {isSuccess && ( | |
| <> | |
| <CheckIcon className="h-3 w-3" /> | |
| <span>Success</span> | |
| </> | |
| )} | |
| {isError && ( | |
| <> | |
| <XIcon className="h-3 w-3" /> | |
| <span>Failed</span> | |
| </> | |
| )} | |
| {!isSuccess && !isError && ( | |
| <span>Done</span> | |
| )} | |
| </div> | |
| )} | |
| <div className="flex h-6 w-6 items-center justify-center"> | |
| {isPending && <Loader size={12} />} | |
| </div> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/ai-elements/bash-tool.tsx` around lines 96 - 117,
The status area can render empty when state === "output-available" but exitCode
is undefined; update the BashTool component to handle that case by either
narrowing the prop types so exitCode is required when state ===
"output-available" or adding a UI fallback: inside the status render (where
isPending/isSuccess/isError are used) add a branch that detects state ===
"output-available" && exitCode == null and shows a neutral indicator (e.g.,
"Done" or a dash) so the user sees explicit feedback when no success/failure
flags are set.
1 participant
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
New Features
Improvements
Bug Fixes