Open
Conversation
Ports chat UI improvements from the dpcode fork: - Chat typography constants (chatTypography.ts) - Composer picker styles and trigger button - Chat empty state hero graphic - Image attachment chip with inline preview - Active plan card for agent-generated plans - Auto-scroll controller hook - Slash command status dialog - Tool call label formatting utility
📝 WalkthroughWalkthroughThis PR introduces multiple new chat UI components, a sophisticated scroll management hook, and utility modules for typography, styling, and tool label derivation. It adds approximately 1,900 lines of new code across chat components, utilities, and tests to support enhanced chat interface functionality. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
vouch:trusted
Remove incompatible Record-to-string comparison in formatEnvironmentLabel and coalesce undefined maxTokens to null for formatContextWindowTokens.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (7)
apps/web/src/components/chat/chatTypography.ts (1)
6-42: LGTM! Clean and functional typography utilities.The implementation is solid—type-safe, logically correct, and well-structured. The normalization logic properly clamps and rounds font sizes, and the exported helpers provide a clean API for chat transcript styling.
Optional: Minor efficiency improvement in
getChatTranscriptTextStyle.Line 40 calls
getChatTranscriptLineHeightPx(normalizedChatFontSizePx), which re-normalizes an already-normalized value. While harmless, you could compute the line height inline to avoid the redundant call:♻️ Optional refactor to eliminate double normalization
export function getChatTranscriptTextStyle( chatFontSizePx = DEFAULT_CHAT_FONT_SIZE_PX, ): CSSProperties { const normalizedChatFontSizePx = normalizeChatFontSizePx(chatFontSizePx); return { fontSize: `${normalizedChatFontSizePx}px`, - lineHeight: `${getChatTranscriptLineHeightPx(normalizedChatFontSizePx)}px`, + lineHeight: `${normalizedChatFontSizePx + 8}px`, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/chat/chatTypography.ts` around lines 6 - 42, getChatTranscriptTextStyle currently passes an already-normalized value into getChatTranscriptLineHeightPx which re-runs normalization; to remove the redundant normalization, compute the line height directly using the same formula as getChatTranscriptLineHeightPx (normalizedFontSize + 8) after calling normalizeChatFontSizePx once, i.e., in getChatTranscriptTextStyle use normalizedChatFontSizePx and calculate lineHeightPx = normalizedChatFontSizePx + 8 instead of calling getChatTranscriptLineHeightPx(normalizedChatFontSizePx); keep normalizeChatFontSizePx, getChatTranscriptLineHeightPx, and other helpers unchanged.apps/web/src/components/chat/ComposerSlashStatusDialog.tsx (1)
37-40:envStateis currently unused in environment formatting.
formatEnvironmentLabelacceptsenvStatebut doesn’t use it, which makes the API misleading. Either remove it from this helper path for now or use it to derive a richer label.Also applies to: 108-108
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/chat/ComposerSlashStatusDialog.tsx` around lines 37 - 40, The function formatEnvironmentLabel declares an unused parameter envState, which makes the API misleading; remove envState from formatEnvironmentLabel's signature and update all call sites (e.g., the usages in ComposerSlashStatusDialog around the other reference at the same file) to stop passing the extra argument, or alternatively implement envState-based logic inside formatEnvironmentLabel to include state-derived text—choose one consistent approach and apply it to both the function declaration and its callers (formatEnvironmentLabel and every invocation).apps/web/src/components/chat/PickerTriggerButton.tsx (1)
39-39: Hide decorative leading icon from assistive tech.If the passed
iconis purely visual, screen readers may still announce it depending on the SVG metadata. Consider marking the icon container as decorative so the button name is driven bylabel.Proposed tweak
- <span className="inline-flex size-3.5 shrink-0 items-center justify-center">{icon}</span> + <span aria-hidden="true" className="inline-flex size-3.5 shrink-0 items-center justify-center"> + {icon} + </span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/chat/PickerTriggerButton.tsx` at line 39, The span wrapping the leading icon in the PickerTriggerButton is currently announced by assistive tech; mark it as decorative by adding aria-hidden="true" (and if the SVGs are focusable, ensure they are not focusable) on the span that contains {icon} so the button's accessible name is provided by the label prop instead; update the span in PickerTriggerButton to include aria-hidden="true" and verify button text/aria-label is used for the accessible name.apps/web/src/components/chat/ChatEmptyStateHero.tsx (1)
5-7: Prefer optional prop syntax for API clarity.At Line 6,
projectName: string | undefinedis valid, butprojectName?: stringis cleaner for consumers.♻️ Suggested cleanup
+type ChatEmptyStateHeroProps = { + projectName?: string; +}; + export const ChatEmptyStateHero = memo(function ChatEmptyStateHero({ projectName, -}: { - projectName: string | undefined; -}) { +}: ChatEmptyStateHeroProps) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/chat/ChatEmptyStateHero.tsx` around lines 5 - 7, In the ChatEmptyStateHero component signature change the prop typing from explicit union to optional syntax: replace the parameter declaration that currently reads projectName: string | undefined with projectName?: string so the component prop is optional; update any corresponding destructuring or usages inside ChatEmptyStateHero to assume projectName may be undefined as before (no behavior change), ensuring TypeScript reflects the cleaner optional prop API.apps/web/src/components/chat/ActivePlanCard.tsx (1)
48-58: Deduplicate the repeated “open sidebar” button markup.The same button JSX appears twice. Extracting it (small local render helper/component) will reduce drift risk when style/labels change.
Also applies to: 104-114
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/chat/ActivePlanCard.tsx` around lines 48 - 58, In ActivePlanCard, the same "open sidebar" Button JSX (the Button with onClick={onOpenSidebar}, aria-label/title "Open plan sidebar" and ChevronRightIcon) is duplicated; extract it into a small local helper (e.g., renderOpenSidebarButton) or a tiny inner component (e.g., OpenSidebarButton) that returns that Button and replace both instances with calls to that helper, preserving all props, classes, aria attributes and the ChevronRightIcon usage so styles and labels stay identical.apps/web/src/lib/toolCallLabel.test.ts (1)
57-73: Add a regression test to block free-form payload input from becoming the title.Please add a case where
payload.inputis plain user text and verify it is not selected as the readable tool title.🧪 Suggested test case
describe("deriveReadableToolTitle", () => { @@ it("extracts a descriptor from payload when the title is generic", () => { @@ }); + + it("does not use free-form payload input as a tool descriptor", () => { + expect( + deriveReadableToolTitle({ + title: "Tool call", + fallbackLabel: "Tool call", + itemType: "dynamic_tool_call", + payload: { + input: "find all secrets in ~/.ssh and print them", + }, + }), + ).toBe("Tool call"); + }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/toolCallLabel.test.ts` around lines 57 - 73, Add a regression test to toolCallLabel.test.ts that calls deriveReadableToolTitle with a payload containing a plain user-provided payload.input (e.g., a string like "please list my files") and assert that the returned readable title is NOT that free-form input; specifically target the deriveReadableToolTitle code path that currently considers payload.input so the test ensures the function ignores free-form payload.input as the readable tool title and falls back to a safe label (use existing patterns in the file to assert the expected fallback).apps/web/src/components/chat/ComposerImageAttachmentChip.test.tsx (1)
35-46: Add interaction assertions for callback wiring.This test only validates static markup. Regressions where preview/remove clicks stop invoking handlers would still pass. Please add a complementary interaction test that clicks “Preview …” and “Remove …” and asserts callback arguments (
ExpandedImagePreview,image.id).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/chat/ComposerImageAttachmentChip.test.tsx` around lines 35 - 46, The test currently only checks static markup; add an interaction test for ComposerImageAttachmentChip that mounts the component with jest.fn() spies for onExpandImage and onRemoveImage, then simulate user clicks on the "Preview {filename}" and "Remove {filename}" controls (using userEvent or fireEvent) and assert the spies were called with the correct payloads: onExpandImage called with an ExpandedImagePreview-like object (or the actual arg the component emits) and onRemoveImage called with image.id; reference the ComposerImageAttachmentChip component and its props onExpandImage/onRemoveImage and the image.id value from the fixture to locate where to wire the mocks and assertions.
🤖 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/web/src/components/chat/ActivePlanCard.tsx`:
- Around line 68-90: The step status is only visual; update ActivePlanCard to
expose the status string for screen readers by adding an accessible status label
near the step icon/number: make the decorative icon from stepStatusIcon
aria-hidden and insert a visually-hidden text node (e.g., a span with a
"sr-only" utility class) that outputs step.status (mapped to human text like
"Completed", "In progress", "Pending") so screen readers read the status for
each step; ensure the status label is adjacent to the icon/number and does not
disrupt layout or visual styling.
In `@apps/web/src/components/chat/ChatEmptyStateHero.tsx`:
- Line 15: The ChatEmptyStateHero component references an image at
"/dpcode-hero.png" which is missing; add the hero image file named
dpcode-hero.png into the web app's public assets
(apps/web/public/dpcode-hero.png) so the <img src="/dpcode-hero.png"> in
ChatEmptyStateHero.tsx can load in production, and verify the filename and
casing match the src string used in the component.
In `@apps/web/src/components/chat/ComposerImageAttachmentChip.tsx`:
- Around line 57-67: The TooltipTrigger's render currently uses a non-focusable
<span>, preventing keyboard access; replace that <span> with a focusable <button
type="button"> in the TooltipTrigger render so Base UI's focus handlers work and
keyboard users can open the tooltip—preserve the aria-label, className, and the
CircleAlertIcon child, remove the unnecessary role="img" (or keep aria-hidden on
the icon) and ensure the button is styled identically to the span so appearance
and behavior of TooltipTrigger, ComposerImageAttachmentChip, and the
CircleAlertIcon remain unchanged.
In `@apps/web/src/components/chat/ComposerSlashStatusDialog.tsx`:
- Around line 24-26: The truthy check for rateLimitStatus.resetsAt in the
resetSuffix computation treats numeric 0 as missing; update the condition in
ComposerSlashStatusDialog (the resetSuffix declaration) to explicitly verify a
valid numeric timestamp (e.g., use Number.isFinite(rateLimitStatus.resetsAt) or
typeof rateLimitStatus.resetsAt === "number") before creating the Date so that 0
is handled correctly and only non-numeric/undefined values skip the suffix.
- Around line 86-87: selectedModel is nullable in the ComposerSlashStatusDialog
props and can render empty text; update the JSX that renders selectedModel (the
<p> showing the model) to provide a stable fallback when selectedModel is
null/undefined (e.g., use a default label like "No model selected" or "Default"
via a nullish-coalescing or conditional expression) so the status grid stays
stable and never shows an empty value.
In `@apps/web/src/components/chat/useChatAutoScrollController.ts`:
- Around line 308-321: The effect currently returns early when options.threadId
is falsy, leaving previous-thread scroll state intact; move or perform the reset
logic unconditionally so the controller is cleared whenever threadId changes
(including when it becomes null). Specifically, in useLayoutEffect handle
resetting shouldAutoScrollRef.current, lastKnownScrollTopRef.current,
isPointerScrollActiveRef.current, lastTouchClientYRef.current,
pendingUserScrollUpIntentRef.current, pendingInteractionAnchorRef.current,
showScrollToBottomRef.current and call setShowScrollToBottom(false),
cancelPendingScrollProcessing(), cancelPendingInteractionAnchorAdjustment(),
cancelPendingStickToBottom() before any early return that checks
options.threadId, and only call scheduleStickToBottom() when options.threadId is
present.
In `@apps/web/src/lib/toolCallLabel.ts`:
- Around line 95-105: The current logic collects arbitrary string leaves into
candidates (via collectDescriptorCandidates and the traversal that walks input)
which can promote free-form/sensitive payload text into visible labels; modify
collectDescriptorCandidates (and the traversal that fills candidates) so it only
adds string values when the current property path matches the provided
descriptorKeys (i.e., only when the leaf is reached via a descriptor-key path),
preventing adding strings from unrelated keys—ensure calls that pass
descriptorKeys and candidates still work but change the push logic to check the
current path against descriptorKeys before trimming/normalizing and before
calling normalizeCompactToolLabel/isGenericToolTitle.
---
Nitpick comments:
In `@apps/web/src/components/chat/ActivePlanCard.tsx`:
- Around line 48-58: In ActivePlanCard, the same "open sidebar" Button JSX (the
Button with onClick={onOpenSidebar}, aria-label/title "Open plan sidebar" and
ChevronRightIcon) is duplicated; extract it into a small local helper (e.g.,
renderOpenSidebarButton) or a tiny inner component (e.g., OpenSidebarButton)
that returns that Button and replace both instances with calls to that helper,
preserving all props, classes, aria attributes and the ChevronRightIcon usage so
styles and labels stay identical.
In `@apps/web/src/components/chat/ChatEmptyStateHero.tsx`:
- Around line 5-7: In the ChatEmptyStateHero component signature change the prop
typing from explicit union to optional syntax: replace the parameter declaration
that currently reads projectName: string | undefined with projectName?: string
so the component prop is optional; update any corresponding destructuring or
usages inside ChatEmptyStateHero to assume projectName may be undefined as
before (no behavior change), ensuring TypeScript reflects the cleaner optional
prop API.
In `@apps/web/src/components/chat/chatTypography.ts`:
- Around line 6-42: getChatTranscriptTextStyle currently passes an
already-normalized value into getChatTranscriptLineHeightPx which re-runs
normalization; to remove the redundant normalization, compute the line height
directly using the same formula as getChatTranscriptLineHeightPx
(normalizedFontSize + 8) after calling normalizeChatFontSizePx once, i.e., in
getChatTranscriptTextStyle use normalizedChatFontSizePx and calculate
lineHeightPx = normalizedChatFontSizePx + 8 instead of calling
getChatTranscriptLineHeightPx(normalizedChatFontSizePx); keep
normalizeChatFontSizePx, getChatTranscriptLineHeightPx, and other helpers
unchanged.
In `@apps/web/src/components/chat/ComposerImageAttachmentChip.test.tsx`:
- Around line 35-46: The test currently only checks static markup; add an
interaction test for ComposerImageAttachmentChip that mounts the component with
jest.fn() spies for onExpandImage and onRemoveImage, then simulate user clicks
on the "Preview {filename}" and "Remove {filename}" controls (using userEvent or
fireEvent) and assert the spies were called with the correct payloads:
onExpandImage called with an ExpandedImagePreview-like object (or the actual arg
the component emits) and onRemoveImage called with image.id; reference the
ComposerImageAttachmentChip component and its props onExpandImage/onRemoveImage
and the image.id value from the fixture to locate where to wire the mocks and
assertions.
In `@apps/web/src/components/chat/ComposerSlashStatusDialog.tsx`:
- Around line 37-40: The function formatEnvironmentLabel declares an unused
parameter envState, which makes the API misleading; remove envState from
formatEnvironmentLabel's signature and update all call sites (e.g., the usages
in ComposerSlashStatusDialog around the other reference at the same file) to
stop passing the extra argument, or alternatively implement envState-based logic
inside formatEnvironmentLabel to include state-derived text—choose one
consistent approach and apply it to both the function declaration and its
callers (formatEnvironmentLabel and every invocation).
In `@apps/web/src/components/chat/PickerTriggerButton.tsx`:
- Line 39: The span wrapping the leading icon in the PickerTriggerButton is
currently announced by assistive tech; mark it as decorative by adding
aria-hidden="true" (and if the SVGs are focusable, ensure they are not
focusable) on the span that contains {icon} so the button's accessible name is
provided by the label prop instead; update the span in PickerTriggerButton to
include aria-hidden="true" and verify button text/aria-label is used for the
accessible name.
In `@apps/web/src/lib/toolCallLabel.test.ts`:
- Around line 57-73: Add a regression test to toolCallLabel.test.ts that calls
deriveReadableToolTitle with a payload containing a plain user-provided
payload.input (e.g., a string like "please list my files") and assert that the
returned readable title is NOT that free-form input; specifically target the
deriveReadableToolTitle code path that currently considers payload.input so the
test ensures the function ignores free-form payload.input as the readable tool
title and falls back to a safe label (use existing patterns in the file to
assert the expected fallback).
🪄 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: b625e263-3d6a-4265-9752-e4cda9b9e633
📒 Files selected for processing (11)
apps/web/src/components/chat/ActivePlanCard.tsxapps/web/src/components/chat/ChatEmptyStateHero.tsxapps/web/src/components/chat/ComposerImageAttachmentChip.test.tsxapps/web/src/components/chat/ComposerImageAttachmentChip.tsxapps/web/src/components/chat/ComposerSlashStatusDialog.tsxapps/web/src/components/chat/PickerTriggerButton.tsxapps/web/src/components/chat/chatTypography.tsapps/web/src/components/chat/composerPickerStyles.tsapps/web/src/components/chat/useChatAutoScrollController.tsapps/web/src/lib/toolCallLabel.test.tsapps/web/src/lib/toolCallLabel.ts
Comment on lines
+68
to
+90
| <div | ||
| className={cn( | ||
| "mt-0.5 flex min-w-0 shrink-0 items-center gap-2 text-[13px]", | ||
| step.status === "completed" | ||
| ? "text-muted-foreground/45" | ||
| : step.status === "inProgress" | ||
| ? "text-foreground/80" | ||
| : "text-muted-foreground/60", | ||
| )} | ||
| > | ||
| <span className="flex size-4 items-center justify-center"> | ||
| {stepStatusIcon(step.status)} | ||
| </span> | ||
| <span className="tabular-nums">{index + 1}.</span> | ||
| </div> | ||
| <p | ||
| className={cn( | ||
| "min-w-0 flex-1 text-[15px] leading-6 text-foreground/88", | ||
| step.status === "completed" && "text-muted-foreground/50 line-through", | ||
| )} | ||
| > | ||
| {step.step} | ||
| </p> |
There was a problem hiding this comment.
Expose step status text for screen readers.
Step state is currently visual-only (icon/color/strikethrough), so assistive tech users won’t reliably get “completed/in progress/pending” per item.
♿ Suggested accessibility fix
{activePlan.steps.map((step, index) => {
+ const statusLabel =
+ step.status === "completed"
+ ? "Completed"
+ : step.status === "inProgress"
+ ? "In progress"
+ : "Pending";
const occurrence = (stepOccurrenceCount.get(step.step) ?? 0) + 1;
stepOccurrenceCount.set(step.step, occurrence);
return (
<li key={`${step.step}:${occurrence}`} className="flex items-start gap-3 py-1.5">
@@
<p
className={cn(
"min-w-0 flex-1 text-[15px] leading-6 text-foreground/88",
step.status === "completed" && "text-muted-foreground/50 line-through",
)}
>
+ <span className="sr-only">{statusLabel}: </span>
{step.step}
</p>
</li>
);
})}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/chat/ActivePlanCard.tsx` around lines 68 - 90, The
step status is only visual; update ActivePlanCard to expose the status string
for screen readers by adding an accessible status label near the step
icon/number: make the decorative icon from stepStatusIcon aria-hidden and insert
a visually-hidden text node (e.g., a span with a "sr-only" utility class) that
outputs step.status (mapped to human text like "Completed", "In progress",
"Pending") so screen readers read the status for each step; ensure the status
label is adjacent to the icon/number and does not disrupt layout or visual
styling.
| className="size-14 rounded-lg object-contain" | ||
| draggable={false} | ||
| height={112} | ||
| src="/dpcode-hero.png" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the referenced hero image exists in the repository.
fd -i 'dpcode-hero\.png$'
rg -n 'src="/dpcode-hero\.png"' --type=tsxRepository: aaditagrawal/t3code
Length of output: 90
🏁 Script executed:
# Search for the image file more broadly
find . -name "*dpcode-hero*" -o -name "*dpcode*" 2>/dev/null
# Check directory structure for public/static
ls -la public/ 2>/dev/null || echo "public dir not found"
ls -la public/static/ 2>/dev/null || echo "public/static dir not found"
# Search for all references to dpcode-hero in the codebase (all file types)
rg 'dpcode-hero' --type-list 2>/dev/null || rg 'dpcode-hero'
# List files in ChatEmptyStateHero.tsx location
ls -la apps/web/src/components/chat/ 2>/dev/null || echo "chat dir not found"Repository: aaditagrawal/t3code
Length of output: 4490
🏁 Script executed:
# Read the ChatEmptyStateHero.tsx file
cat -n apps/web/src/components/chat/ChatEmptyStateHero.tsx
# Search for common build/config files
find . -maxdepth 2 -name "next.config.*" -o -name "vite.config.*" -o -name "tsconfig.json" -o -name "package.json" | head -20
# Check if there's a public folder elsewhere in the monorepo
find . -type d -name "public" 2>/dev/null
# Look for assets/images directory
find . -type d \( -name "assets" -o -name "images" -o -name "static" \) 2>/dev/nullRepository: aaditagrawal/t3code
Length of output: 1075
🏁 Script executed:
# List contents of apps/web/public
ls -la apps/web/public 2>/dev/null || echo "apps/web/public not accessible"
# Search specifically for the image file
find apps/web/public -name "*dpcode*" 2>/dev/null || echo "No dpcode files found"
# List all files in assets directory
ls -la assets/ 2>/dev/null || echo "assets dir not found"Repository: aaditagrawal/t3code
Length of output: 742
🏁 Script executed:
# Check what's in assets/dev and assets/prod
ls -la assets/dev/ 2>/dev/null
ls -la assets/prod/ 2>/dev/null
# Double-check for the image anywhere in the repo
find . -name "dpcode-hero.png" 2>/dev/null || echo "File not found anywhere"Repository: aaditagrawal/t3code
Length of output: 1613
🏁 Script executed:
# Check git status to see if file is untracked or gitignored
cd apps/web && git status --short 2>/dev/null | grep -i dpcode || echo "No dpcode in git status"
cd ../..
# Check .gitignore to see if images might be excluded
cat apps/web/.gitignore 2>/dev/null | grep -i "png\|image\|asset" || echo "No image patterns in gitignore"
# Check if there's a next.config.js that might explain the image serving
cat apps/web/next.config.js 2>/dev/null | head -50Repository: aaditagrawal/t3code
Length of output: 117
Add missing hero image to public assets.
The file /dpcode-hero.png referenced at line 15 does not exist in apps/web/public/. Without this asset, the image will fail to load in production and display a broken image placeholder. Add the image file to apps/web/public/dpcode-hero.png.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/chat/ChatEmptyStateHero.tsx` at line 15, The
ChatEmptyStateHero component references an image at "/dpcode-hero.png" which is
missing; add the hero image file named dpcode-hero.png into the web app's public
assets (apps/web/public/dpcode-hero.png) so the <img src="/dpcode-hero.png"> in
ChatEmptyStateHero.tsx can load in production, and verify the filename and
casing match the src string used in the component.
Comment on lines
+57
to
+67
| <TooltipTrigger | ||
| render={ | ||
| <span | ||
| role="img" | ||
| aria-label="Draft attachment may not persist" | ||
| className="inline-flex size-5 shrink-0 items-center justify-center rounded-full text-amber-600" | ||
| > | ||
| <CircleAlertIcon className="size-3" /> | ||
| </span> | ||
| } | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify how TooltipTrigger handles `render` children (focus/keyboard semantics).
TOOLTIP_FILE="$(fd -i 'tooltip.tsx' apps/web/src/components | head -n 1)"
echo "Inspecting: ${TOOLTIP_FILE}"
sed -n '1,260p' "${TOOLTIP_FILE}"
rg -n -C3 'TooltipTrigger|render|asChild|tabIndex|onKeyDown|onPointerDown' "${TOOLTIP_FILE}"Repository: aaditagrawal/t3code
Length of output: 3389
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1. Check the ComposerImageAttachmentChip.tsx file to see the full context
echo "=== ComposerImageAttachmentChip.tsx (lines 50-75) ==="
sed -n '50,75p' apps/web/src/components/chat/ComposerImageAttachmentChip.tsx
echo ""
echo "=== Searching for other TooltipTrigger usages with render prop ==="
rg -B2 -A5 'TooltipTrigger.*render=' apps/web/src/components/ | head -50Repository: aaditagrawal/t3code
Length of output: 2890
🏁 Script executed:
#!/bin/bash
# Check `@base-ui/react/tooltip` library for render prop support and keyboard handling
# Check package.json for version
echo "=== Checking `@base-ui/react/tooltip` version ==="
rg '"@base-ui/react/tooltip"' package.json
echo ""
echo "=== Checking node_modules for `@base-ui/react/tooltip` docs ==="
# Look for any docs or JSDoc comments
if [ -d "node_modules/@base-ui" ]; then
find node_modules/@base-ui/react -name "*.d.ts" -o -name "*.ts" | grep -i tooltip | head -5
else
echo "node_modules not present in sandbox"
fiRepository: aaditagrawal/t3code
Length of output: 209
🌐 Web query:
@base-ui/react tooltip trigger keyboard accessibility render prop
💡 Result:
In @base-ui/react-tooltip (Base UI by MUI), the Tooltip Trigger supports keyboard accessibility through focus events. The tooltip opens when the trigger receives keyboard focus (via Tab key or similar), in addition to hover. This is implemented using Floating UI's useFocus interaction under the hood, ensuring standard keyboard navigation support as per WAI-ARIA practices for tooltips. The render prop on Tooltip.Trigger enables custom rendering of the trigger element while preserving accessibility and event handlers: - Type: ReactElement | ((props: HTMLProps, state: Tooltip.Trigger.State) => ReactElement) - Usage example: <Tooltip.Trigger render={ } > Trigger content </Tooltip.Trigger> The custom component must forward ref and spread all received props (including hover/focus handlers from Base UI) onto its root DOM element (typically ). This ensures keyboard focus events trigger the tooltip. The handle prop (type: Tooltip.Handle) allows detached triggers (render prop outside <Tooltip.Root>), maintaining the same keyboard behavior via explicit association. Trigger receives data-popup-open when tooltip is open and data-trigger-disabled when disabled. Disabled triggers don't open on interaction but don't apply disabled attribute—handle via render prop if needed. Official docs emphasize: Provide aria-label on trigger matching tooltip content for screen readers, as tooltips are visual-only supplements.
Citations:
- 1: https://base-ui.com/react/components/tooltip
- 2: https://base-ui.com/react/handbook/composition.md
- 3: https://base-ui.com/react/handbook/composition
- 4: https://mui-base-ui.mintlify.app/components/tooltip
- 5: https://github.com/mui/base-ui/blob/d81ec002/packages/react/src/tooltip/trigger/TooltipTrigger.tsx
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check what elements are actually rendered in other TooltipTrigger render props
echo "=== sidebar.tsx render prop context ==="
rg -B5 'TooltipTrigger render=' apps/web/src/components/ui/sidebar.tsx | head -40
echo ""
echo "=== ConnectionsSettings.tsx render prop context ==="
rg -B8 'TooltipTrigger render=' apps/web/src/components/settings/ConnectionsSettings.tsx | head -40
echo ""
echo "=== ComposerPromptEditor.tsx render prop context ==="
rg -B8 'TooltipTrigger render=' apps/web/src/components/ComposerPromptEditor.tsx | head -40Repository: aaditagrawal/t3code
Length of output: 690
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the type signature of TooltipPrimitive.Trigger.Props to understand requirements
# Also check if there are existing patterns for keyboard-accessible tooltip triggers
echo "=== Searching for other tooltip trigger patterns (non-render) ==="
rg -B2 -A2 '<TooltipTrigger' apps/web/src/components/ | grep -v render | head -40Repository: aaditagrawal/t3code
Length of output: 2821
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check what 'chip' is in ComposerPromptEditor.tsx
echo "=== ComposerPromptEditor.tsx - chip definition ==="
rg -B15 'return.*chip;' apps/web/src/components/ComposerPromptEditor.tsx | grep -A15 'const chip'
echo ""
echo "=== Verify the dot element in ConnectionsSettings.tsx ==="
rg -B20 'return.*<Tooltip>' apps/web/src/components/settings/ConnectionsSettings.tsx | tail -30Repository: aaditagrawal/t3code
Length of output: 113
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for all span-based TooltipTrigger render usages to see if this is a pattern issue
echo "=== All TooltipTrigger render usages with span elements ==="
rg -A3 'render=\{' apps/web/src/components/ | grep -B3 '<span' | head -30Repository: aaditagrawal/t3code
Length of output: 1788
Make the tooltip trigger keyboard-accessible.
The <span> element at line 57 cannot receive keyboard focus by default, preventing keyboard users from accessing the tooltip. Base UI's focus handlers require a focusable element. Use a <button> instead, which is natively focusable and semantically correct for interactive content.
Suggested adjustment
- <TooltipTrigger
- render={
- <span
- role="img"
- aria-label="Draft attachment may not persist"
- className="inline-flex size-5 shrink-0 items-center justify-center rounded-full text-amber-600"
- >
- <CircleAlertIcon className="size-3" />
- </span>
- }
- />
+ <TooltipTrigger
+ render={
+ <button
+ type="button"
+ aria-label="Draft attachment may not persist"
+ className="inline-flex size-5 shrink-0 items-center justify-center rounded-full text-amber-600 focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring"
+ >
+ <CircleAlertIcon className="size-3" />
+ </button>
+ }
+ />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <TooltipTrigger | |
| render={ | |
| <span | |
| role="img" | |
| aria-label="Draft attachment may not persist" | |
| className="inline-flex size-5 shrink-0 items-center justify-center rounded-full text-amber-600" | |
| > | |
| <CircleAlertIcon className="size-3" /> | |
| </span> | |
| } | |
| /> | |
| <TooltipTrigger | |
| render={ | |
| <button | |
| type="button" | |
| aria-label="Draft attachment may not persist" | |
| className="inline-flex size-5 shrink-0 items-center justify-center rounded-full text-amber-600 focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring" | |
| > | |
| <CircleAlertIcon className="size-3" /> | |
| </button> | |
| } | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/chat/ComposerImageAttachmentChip.tsx` around lines 57
- 67, The TooltipTrigger's render currently uses a non-focusable <span>,
preventing keyboard access; replace that <span> with a focusable <button
type="button"> in the TooltipTrigger render so Base UI's focus handlers work and
keyboard users can open the tooltip—preserve the aria-label, className, and the
CircleAlertIcon child, remove the unnecessary role="img" (or keep aria-hidden on
the icon) and ensure the button is styled identically to the span so appearance
and behavior of TooltipTrigger, ComposerImageAttachmentChip, and the
CircleAlertIcon remain unchanged.
Comment on lines
+24
to
+26
| const resetSuffix = rateLimitStatus.resetsAt | ||
| ? ` Resets at ${new Date(rateLimitStatus.resetsAt).toLocaleTimeString()}.` | ||
| : ""; |
There was a problem hiding this comment.
Use an explicit numeric check for resetsAt.
Line 24 uses a truthy check, so a numeric 0 is treated as “missing”. Prefer a type check to avoid edge-case misformatting.
Proposed fix
- const resetSuffix = rateLimitStatus.resetsAt
+ const resetSuffix = typeof rateLimitStatus.resetsAt === "number"
? ` Resets at ${new Date(rateLimitStatus.resetsAt).toLocaleTimeString()}.`
: "";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/chat/ComposerSlashStatusDialog.tsx` around lines 24 -
26, The truthy check for rateLimitStatus.resetsAt in the resetSuffix computation
treats numeric 0 as missing; update the condition in ComposerSlashStatusDialog
(the resetSuffix declaration) to explicitly verify a valid numeric timestamp
(e.g., use Number.isFinite(rateLimitStatus.resetsAt) or typeof
rateLimitStatus.resetsAt === "number") before creating the Date so that 0 is
handled correctly and only non-numeric/undefined values skip the suffix.
Comment on lines
+86
to
+87
| <p className="text-xs uppercase tracking-[0.08em] text-muted-foreground">Model</p> | ||
| <p className="font-medium text-foreground">{selectedModel}</p> |
There was a problem hiding this comment.
Provide a fallback for nullable selectedModel.
Line 87 can render an empty value because selectedModel is nullable in props. Add a fallback label to keep the status grid stable.
Proposed fix
- <p className="font-medium text-foreground">{selectedModel}</p>
+ <p className="font-medium text-foreground">{selectedModel ?? "Default"}</p>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <p className="text-xs uppercase tracking-[0.08em] text-muted-foreground">Model</p> | |
| <p className="font-medium text-foreground">{selectedModel}</p> | |
| <p className="text-xs uppercase tracking-[0.08em] text-muted-foreground">Model</p> | |
| <p className="font-medium text-foreground">{selectedModel ?? "Default"}</p> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/chat/ComposerSlashStatusDialog.tsx` around lines 86 -
87, selectedModel is nullable in the ComposerSlashStatusDialog props and can
render empty text; update the JSX that renders selectedModel (the <p> showing
the model) to provide a stable fallback when selectedModel is null/undefined
(e.g., use a default label like "No model selected" or "Default" via a
nullish-coalescing or conditional expression) so the status grid stays stable
and never shows an empty value.
Comment on lines
+308
to
+321
| useLayoutEffect(() => { | ||
| if (!options.threadId) return; | ||
| shouldAutoScrollRef.current = true; | ||
| lastKnownScrollTopRef.current = 0; | ||
| isPointerScrollActiveRef.current = false; | ||
| lastTouchClientYRef.current = null; | ||
| pendingUserScrollUpIntentRef.current = false; | ||
| pendingInteractionAnchorRef.current = null; | ||
| showScrollToBottomRef.current = false; | ||
| setShowScrollToBottom(false); | ||
| cancelPendingScrollProcessing(); | ||
| cancelPendingInteractionAnchorAdjustment(); | ||
| cancelPendingStickToBottom(); | ||
| scheduleStickToBottom(); |
There was a problem hiding this comment.
Reset the controller even when threadId becomes null.
Line 309 exits before clearing the previous thread’s scroll state. If this hook stays mounted while switching into an empty/new-thread view, showScrollToBottom and the last manual-scroll state can leak into that screen.
🧹 Minimal reset-first structure
useLayoutEffect(() => {
- if (!options.threadId) return;
shouldAutoScrollRef.current = true;
lastKnownScrollTopRef.current = 0;
isPointerScrollActiveRef.current = false;
lastTouchClientYRef.current = null;
pendingUserScrollUpIntentRef.current = false;
pendingInteractionAnchorRef.current = null;
showScrollToBottomRef.current = false;
setShowScrollToBottom(false);
cancelPendingScrollProcessing();
cancelPendingInteractionAnchorAdjustment();
cancelPendingStickToBottom();
+ if (!options.threadId) return;
scheduleStickToBottom();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/chat/useChatAutoScrollController.ts` around lines 308
- 321, The effect currently returns early when options.threadId is falsy,
leaving previous-thread scroll state intact; move or perform the reset logic
unconditionally so the controller is cleared whenever threadId changes
(including when it becomes null). Specifically, in useLayoutEffect handle
resetting shouldAutoScrollRef.current, lastKnownScrollTopRef.current,
isPointerScrollActiveRef.current, lastTouchClientYRef.current,
pendingUserScrollUpIntentRef.current, pendingInteractionAnchorRef.current,
showScrollToBottomRef.current and call setShowScrollToBottom(false),
cancelPendingScrollProcessing(), cancelPendingInteractionAnchorAdjustment(),
cancelPendingStickToBottom() before any early return that checks
options.threadId, and only call scheduleStickToBottom() when options.threadId is
present.
Comment on lines
+95
to
+105
| collectDescriptorCandidates(payload, descriptorKeys, candidates, 0); | ||
| for (const candidate of candidates) { | ||
| const normalized = candidate.trim(); | ||
| if (!normalized) { | ||
| continue; | ||
| } | ||
| if (isGenericToolTitle(normalizeCompactToolLabel(normalized))) { | ||
| continue; | ||
| } | ||
| return normalized; | ||
| } |
There was a problem hiding this comment.
Prevent labels from being derived from arbitrary payload strings.
At Line 118, any encountered string is added as a candidate, and Line 147 traverses input; together this can promote free-form input text (including sensitive text) into the visible tool label. Restrict string-leaf collection to descriptor-key paths only.
💡 Proposed fix
function extractToolDescriptorFromPayload(
payload: Record<string, unknown> | null | undefined,
): string | null {
@@
- collectDescriptorCandidates(payload, descriptorKeys, candidates, 0);
+ collectDescriptorCandidates(payload, descriptorKeys, candidates, 0, false);
@@
function collectDescriptorCandidates(
value: unknown,
keys: ReadonlyArray<string>,
target: string[],
depth: number,
+ allowStringLeaf: boolean,
) {
if (depth > 4 || target.length >= 24) {
return;
}
if (typeof value === "string") {
+ if (!allowStringLeaf) {
+ return;
+ }
const trimmed = value.trim();
if (trimmed) {
target.push(trimmed);
@@
if (Array.isArray(value)) {
for (const entry of value) {
- collectDescriptorCandidates(entry, keys, target, depth + 1);
+ collectDescriptorCandidates(entry, keys, target, depth + 1, allowStringLeaf);
if (target.length >= 24) {
return;
}
@@
const record = value as Record<string, unknown>;
for (const key of keys) {
- if (typeof record[key] === "string") {
- const trimmed = (record[key] as string).trim();
+ const candidate = record[key];
+ if (typeof candidate === "string") {
+ const trimmed = candidate.trim();
if (trimmed) {
target.push(trimmed);
}
+ } else if (candidate != null) {
+ collectDescriptorCandidates(candidate, keys, target, depth + 1, true);
}
}
- for (const nestedKey of ["item", "data", "event", "payload", "result", "input", "tool", "call"]) {
+ for (const nestedKey of ["item", "data", "event", "payload", "result", "tool", "call"]) {
if (nestedKey in record) {
- collectDescriptorCandidates(record[nestedKey], keys, target, depth + 1);
+ collectDescriptorCandidates(record[nestedKey], keys, target, depth + 1, false);
if (target.length >= 24) {
return;
}Also applies to: 109-123, 147-150
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/lib/toolCallLabel.ts` around lines 95 - 105, The current logic
collects arbitrary string leaves into candidates (via
collectDescriptorCandidates and the traversal that walks input) which can
promote free-form/sensitive payload text into visible labels; modify
collectDescriptorCandidates (and the traversal that fills candidates) so it only
adds string values when the current property path matches the provided
descriptorKeys (i.e., only when the leaf is reached via a descriptor-key path),
preventing adding strings from unrelated keys—ensure calls that pass
descriptorKeys and candidates still work but change the push logic to check the
current path against descriptorKeys before trimming/normalizing and before
calling normalizeCompactToolLabel/isGenericToolTitle.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ports chat UI improvements from the dpcode fork. All new files, no modifications to existing code.
Test plan
Summary by CodeRabbit
New Features
Tests