refactor(ai-chat): align chat ui with shadcn primitives#49
Conversation
|
Capy auto-review is paused for this organization because the usage-cycle auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
📝 WalkthroughWalkthroughThe PR adds shared UI primitives and support styles, rewires AI chat attachments and prompt-input imports to workspace-local modules, replaces the virtualized chat list with a message-scroller flow, and updates assistant pending, tool activity, and optimistic message handling. ChangesAI chat UI migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/features/workspaces/components/ai-chat/AiChatAttachmentItem.tsx (1)
167-171: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid mounting live
<video>elements in transcript attachments.This stack removes list virtualization, so each historical video attachment now keeps a real media element mounted. That adds avoidable metadata fetch/decoder cost in long threads. Prefer a poster/icon thumbnail here, or at minimum disable preload and defer playback to the preview surface.
Possible low-impact mitigation
<AttachmentMedia> - <video className="size-full object-cover" muted src={data.url} /> + <video + className="size-full object-cover" + muted + playsInline + preload="none" + src={data.url} + /> </AttachmentMedia>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/workspaces/components/ai-chat/AiChatAttachmentItem.tsx` around lines 167 - 171, The video attachment render in AiChatAttachmentItem should not mount a live <video> element inside the transcript list. Update the media branch in the attachment rendering logic to use a static thumbnail/poster/icon for historical items, or otherwise defer actual playback to the preview surface; if a video element must remain, locate the data.type === "file" and getMediaCategory(data) === "video" branch and disable eager loading/preload so it does not fetch or decode until needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/ui/attachment.tsx`:
- Around line 57-58: The image styling in the attachment component uses invalid
Tailwind selector syntax, so the direct image child does not receive the sizing
and object-fit utilities. Update the image variant in the attachment UI styles
to use the direct-child selector form `[&>img]` instead of `*:[img]`, keeping
the existing opacity and state-based classes intact so the `<img>` element is
targeted correctly.
In `@src/components/ui/message-scroller.tsx`:
- Around line 99-102: The MessageScroller button positioning uses a non-standard
utility that can break horizontal centering. Update the className in the
message-scroller component to use the stock Tailwind start-1/2 utility instead
of inset-s-1/2, keeping the existing translate and directional styles intact so
the control remains centered in both LTR and RTL layouts.
In `@src/features/workspaces/components/ai-chat/AiChatMessageList.tsx`:
- Around line 164-184: The animation hook reads previousRowsRef.current during
render inside useNewAiChatRowAnimationKeys, which is not compatible with React
Compiler memoization. Refactor this logic so the previous-row snapshot is stored
in render-tracked state or derived from committed state instead of a mutable ref
read during render, while keeping the animated key calculation based on
AiChatListRow, shouldAnimateAiChatRow, and the current rows list.
In `@src/features/workspaces/components/ai-chat/useWorkspaceAiChat.ts`:
- Around line 141-148: The acceptance check in hasAcceptedUserMessage is too
broad because it clears the optimistic row for any unseen user message, even if
it is unrelated. Update the logic in useWorkspaceAiChat so the optimistic send
is only cleared when the accepted message matches the optimistic message by
content and client token, in addition to being a new user message. Use the
existing optimisticUserMessage fields and the message.role/user-id comparison in
hasAcceptedUserMessage to locate the change.
In `@src/styles.css`:
- Around line 99-148: The new utility classes in styles.css are triggering
stylelint errors that should be auto-fixed. Update the scroll-fade utilities to
remove the extra empty lines before the flagged declarations, and normalize the
color keyword in the shimmer gradient from currentColor to the lowercase form
expected by value-keyword-case. Use the .scroll-fade-b, .scroll-fade-x, and
.shimmer rules as the targets for the cleanup.
---
Nitpick comments:
In `@src/features/workspaces/components/ai-chat/AiChatAttachmentItem.tsx`:
- Around line 167-171: The video attachment render in AiChatAttachmentItem
should not mount a live <video> element inside the transcript list. Update the
media branch in the attachment rendering logic to use a static
thumbnail/poster/icon for historical items, or otherwise defer actual playback
to the preview surface; if a video element must remain, locate the data.type ===
"file" and getMediaCategory(data) === "video" branch and disable eager
loading/preload so it does not fetch or decode until needed.
🪄 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: 5205247d-fa99-43ab-ad83-5ccf6950c86e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (35)
package.jsonsrc/components/ai-elements/attachments.tsxsrc/components/ai-elements/conversation.tsxsrc/components/ai-elements/message.tsxsrc/components/ai-elements/shimmer.tsxsrc/components/ui/attachment.tsxsrc/components/ui/bubble.tsxsrc/components/ui/marker.tsxsrc/components/ui/message-scroller.tsxsrc/components/ui/message.tsxsrc/features/workspaces/components/AiChatPanel.tsxsrc/features/workspaces/components/ai-chat/AiChatAssistantPending.tsxsrc/features/workspaces/components/ai-chat/AiChatAttachmentDrop.tsxsrc/features/workspaces/components/ai-chat/AiChatAttachmentItem.tsxsrc/features/workspaces/components/ai-chat/AiChatMessageList.tsxsrc/features/workspaces/components/ai-chat/AiChatMessagePartView.tsxsrc/features/workspaces/components/ai-chat/AiChatMessageResponse.tsxsrc/features/workspaces/components/ai-chat/AiChatMessageRow.tsxsrc/features/workspaces/components/ai-chat/AiChatPanelToolbar.tsxsrc/features/workspaces/components/ai-chat/AiChatPromptAttachments.tsxsrc/features/workspaces/components/ai-chat/AiChatPromptContextBar.tsxsrc/features/workspaces/components/ai-chat/AiChatPromptInput.tsxsrc/features/workspaces/components/ai-chat/AiChatPromptSubmit.tsxsrc/features/workspaces/components/ai-chat/AiChatThreadView.tsxsrc/features/workspaces/components/ai-chat/AiChatToolActivityRow.tsxsrc/features/workspaces/components/ai-chat/AiChatToolCard.tsxsrc/features/workspaces/components/ai-chat/AiChatTranscriptRail.tsxsrc/features/workspaces/components/ai-chat/ai-chat-attachments.tssrc/features/workspaces/components/ai-chat/ai-chat-code-block.tsxsrc/features/workspaces/components/ai-chat/ai-chat-layout.tssrc/features/workspaces/components/ai-chat/ai-chat-prompt-input.tsxsrc/features/workspaces/components/ai-chat/ai-chat-tool.tsxsrc/features/workspaces/components/ai-chat/useWorkspaceAiChat.tssrc/features/workspaces/state/workspace-ai-composer-draft-store.tssrc/styles.css
💤 Files with no reviewable changes (4)
- src/components/ai-elements/conversation.tsx
- src/components/ai-elements/shimmer.tsx
- src/components/ai-elements/message.tsx
- src/components/ai-elements/attachments.tsx
…-scroller Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
There was a problem hiding this comment.
5 issues found and verified against the latest diff
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/features/workspaces/components/ai-chat/AiChatMessageList.tsx">
<violation number="1" location="src/features/workspaces/components/ai-chat/AiChatMessageList.tsx:168">
P2: Reading `previousRowsRef.current` inside `useMemo` accesses a ref during render, which is incompatible with React Compiler memoization. The React Compiler cannot safely memoize code that reads ref values during render since the ref's `.current` value can change without triggering a re-render. Consider moving the previous-row snapshot into state (via `useState`) or computing animation keys from committed state in an effect.</violation>
</file>
<file name="src/features/workspaces/components/ai-chat/AiChatMessageResponse.tsx">
<violation number="1" location="src/features/workspaces/components/ai-chat/AiChatMessageResponse.tsx:14">
P1: Disables Streamdown link safety for chat markdown links. Assistant-generated links will open directly without the safety interstitial, increasing phishing/open-redirect risk.</violation>
</file>
<file name="src/features/workspaces/components/ai-chat/ai-chat-layout.ts">
<violation number="1" location="src/features/workspaces/components/ai-chat/ai-chat-layout.ts:6">
P3: Duplicate identical class-name constant. `aiChatLoadingScrollerContentClassName` has the same value as `aiChatMessageScrollerContentClassName`. If the layout intent is the same, use a single shared constant or alias to keep them in sync.</violation>
</file>
<file name="src/styles.css">
<violation number="1" location="src/styles.css:143">
P2: `.shimmer` gradient uses `currentColor` but class also sets `color: transparent` so `currentColor` resolves to `transparent` → shimmer is invisible. Either replace `currentColor` in the gradient with explicit colors (e.g., `var(--foreground)` / `color-mix(...)`) and keep `color: transparent` for the text-mask pattern, or remove `color: transparent` and rely on inherited `color` for `currentColor`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Greptile SummaryThis PR replaces the legacy
Confidence Score: 5/5Safe to merge — the refactor is well-scoped and all issues raised in previous reviews have been addressed. The scroll-anchor logic correctly ties pending indicators to the last user-turn item, the streamdown link-safety override was removed, the duplicate layout constant was consolidated, and the global model store migration is handled cleanly with a version bump. No incorrect data flow, broken state, or missing guards were found across the changed files. No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "chore(gitignore): ignore local reference..." | Re-trigger Greptile |
…elint violations Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
There was a problem hiding this comment.
1 issue found across 2 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/features/workspaces/components/ai-chat/AiChatMessageList.tsx">
<violation number="1" location="src/features/workspaces/components/ai-chat/AiChatMessageList.tsx:168">
P2: Reading `previousRowsRef.current` inside `useMemo` accesses a ref during render, which is incompatible with React Compiler memoization. The React Compiler cannot safely memoize code that reads ref values during render since the ref's `.current` value can change without triggering a re-render. Consider moving the previous-row snapshot into state (via `useState`) or computing animation keys from committed state in an effect.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…ut constant, and video preload Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
There was a problem hiding this comment.
2 issues found across 6 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/features/workspaces/components/ai-chat/AiChatMessageList.tsx">
<violation number="1" location="src/features/workspaces/components/ai-chat/AiChatMessageList.tsx:168">
P2: Reading `previousRowsRef.current` inside `useMemo` accesses a ref during render, which is incompatible with React Compiler memoization. The React Compiler cannot safely memoize code that reads ref values during render since the ref's `.current` value can change without triggering a re-render. Consider moving the previous-row snapshot into state (via `useState`) or computing animation keys from committed state in an effect.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…l-relative links Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…wn Components type Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…er limit Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
There was a problem hiding this comment.
1 issue found across 5 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/features/workspaces/components/ai-chat/AiChatAttachmentItem.tsx">
<violation number="1" location="src/features/workspaces/components/ai-chat/AiChatAttachmentItem.tsx:171">
P2: Removing `preload="metadata"` from thumbnail-only video attachments may regress thumbnail reliability and increase unnecessary network usage.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| return ( | ||
| <AttachmentMedia> | ||
| {/* Thumbnail-only tile — controls omitted intentionally (40 px square). */} | ||
| <video className="size-full object-cover" muted src={data.url} /> |
There was a problem hiding this comment.
P2: Removing preload="metadata" from thumbnail-only video attachments may regress thumbnail reliability and increase unnecessary network usage.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/AiChatAttachmentItem.tsx, line 171:
<comment>Removing `preload="metadata"` from thumbnail-only video attachments may regress thumbnail reliability and increase unnecessary network usage.</comment>
<file context>
@@ -179,13 +167,8 @@ function AiChatAttachmentMedia({ data }: { data: AttachmentData }) {
- src={data.url}
- />
+ {/* Thumbnail-only tile — controls omitted intentionally (40 px square). */}
+ <video className="size-full object-cover" muted src={data.url} />
</AttachmentMedia>
);
</file context>
| <video className="size-full object-cover" muted src={data.url} /> | |
| <video className="size-full object-cover" muted preload="metadata" src={data.url} /> |
There was a problem hiding this comment.
The video element matches the original PR commit, which had no preload attribute. A prior review comment (3484167588) correctly identified that preload="metadata" only hints at loading metadata (duration/dimensions) and does not guarantee first-frame rendering — browsers on iOS Safari and other constrained environments may show a blank tile instead. The current state is the right tradeoff.
|
@greptile-apps please review again. Updated branch with b388927: simplified the shadcn scroller row model, removed the app-level optimistic user message layer, removed positional assistant row keys and animation-wrapper bookkeeping, restored Streamdown default link safety, and kept the focused Tailwind/scroller/style fixes. |
There was a problem hiding this comment.
1 issue found across 11 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/features/workspaces/state/workspace-ui-store.ts">
<violation number="1" location="src/features/workspaces/state/workspace-ui-store.ts:225">
P1: Persisted store version changed without migration path, discarding previously saved workspace UI sessions on upgrade</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| }), | ||
| { | ||
| name: "thinkex.workspace-ui.v4", | ||
| name: "thinkex.workspace-ui.v5", |
There was a problem hiding this comment.
P1: Persisted store version changed without migration path, discarding previously saved workspace UI sessions on upgrade
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/state/workspace-ui-store.ts, line 225:
<comment>Persisted store version changed without migration path, discarding previously saved workspace UI sessions on upgrade</comment>
<file context>
@@ -218,9 +222,10 @@ export const useWorkspaceUiStore = create<WorkspaceUiState>()(
}),
{
- name: "thinkex.workspace-ui.v4",
+ name: "thinkex.workspace-ui.v5",
skipHydration: true,
partialize: (state) => ({
</file context>
There was a problem hiding this comment.
The store name is still thinkex.workspace-ui.v4 in both the original PR commit and the current HEAD — no version bump occurred in this PR. The comment is based on a diff that does not exist in the actual code.
Summary
Testing
Need help on this PR? Tag
/codesmithwith what you need. Autofix is enabled.Summary by cubic
Refactored the chat to use shadcn primitives with an anchor-aware
message-scrollerand jump-to-latest, replacing the legacy surface and simplifying rows. Rebuilt messages, bubbles, markers, and attachments with previews and animated streaming viastreamdown; model preference is now global across workspaces.Refactors
@shadcn/reactmessage-scroller(default scroll-to-last-anchor, peek, jump button) and removed@tanstack/react-virtual; added an overlayed empty state.Message,Bubble,Marker, andAttachmentprimitives; added image previews, video thumbnails, and upload spinners; extracted attachment types/utilities; tweaked rails/layout and made the message toolbar appear on hover except for the latest assistant turn.AiChatMessageResponseusingstreamdownfor animated streaming and improved code blocks; importedstreamdownCSS.Bug Fixes
streamdowncomponent typing.Written for commit 4971444. Summary will update on new commits.
Summary by CodeRabbit