fix(ui): restore green build (monaco 0.55 pin + dead css import) - #43
Conversation
Restore a green @argos/ui build broken by the deps update (#42). style.css removed a dead shadcn/tailwind.css import to a file that never existed (silently skipped on some platforms, failed in CI). monaco-editor pinned to ^0.55.1 because 0.56.0 tightened its exports map and no longer exposes the deep editor.api subpath that a transitive importer needs; 0.55.1 wildcard exports still allow it. bun.lock updated.
|
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:
📝 WalkthroughWalkthroughThe pull request updates ACP turn streaming and steering, adds plan events and a persisted continue-indicator setting, migrates UI primitives from Radix to Base UI, and updates chat, settings, Markdown, and message rendering. ChangesACP execution and steering
Settings and UI
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
Confidence Score: 5/5The PR appears safe to merge, with no concrete changed-code defect identified. The Monaco range remains below 0.56 and satisfies the dependent package’s compatibility range, while the removed CSS import had no resolvable target and its required local styling remains present.
|
| Filename | Overview |
|---|---|
| packages/ui/package.json | Changes Monaco Editor from the incompatible 0.56 release line to ^0.55.1, which also satisfies stream-monaco’s declared peer range. |
| bun.lock | Resolves Monaco Editor to 0.55.1, updates its transitive packages, and deduplicates stream-monaco’s previously nested Monaco installation. |
| packages/ui/src/assets/style.css | Removes an unresolved dead import without removing the stylesheet’s existing Tailwind scanning, shadcn theme tokens, or component styles. |
Reviews (1): Last reviewed commit: "fix(ui): pin monaco 0.55, drop dead css ..." | Re-trigger Greptile
There was a problem hiding this comment.
Pull request overview
Restores a green @argos/ui build after the dependency update by removing a failing CSS import and pinning monaco-editor to a compatible version (with corresponding lockfile updates).
Changes:
- Removed a dead
@importforshadcn/tailwind.cssthat doesn’t exist in the repo and can fail CI builds. - Pinned
monaco-editorto^0.55.1to avoid0.56.0export-map / compatibility issues. - Updated
bun.lockto reflect the pinned dependency and resolved dependency graph.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/ui/src/assets/style.css | Removes a non-existent CSS import that can break builds. |
| packages/ui/package.json | Pins monaco-editor to a compatible version for @argos/ui builds. |
| bun.lock | Updates lockfile entries for the pinned monaco version and related transitive deps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
React Doctor found 42 new issues in 18 files · 42 warnings · score 0 / 100 (Critical) · 40 fixed · vs 42 warnings
Reviewed by React Doctor for commit |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/daemon/test/daemonSessionRoutes.test.ts (1)
890-909: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the spy result for the assertion.
interruptActiveTurnis a privateAcpProviderExecutionPortmethod, soexpect(provider.interruptActiveTurn, ...)can fail TypeScript checking. Store thevi.spyOn(...)result and assert on that result instead.Proposed fix
- vi.spyOn(provider as any, "interruptActiveTurn").mockResolvedValue(undefined); + const interruptActiveTurn = vi.spyFrom(provider as any, "interruptActiveTurn").mockResolvedValue(undefined); - expect(provider.interruptActiveTurn).toHaveBeenCalledWith("session-1"); + expect(interruptActiveTurn).toHaveBeenCalledWith("session-1");🤖 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 `@apps/daemon/test/daemonSessionRoutes.test.ts` around lines 890 - 909, Store the vi.spyOn result for interruptActiveTurn in a local variable before exercising steerActiveTurn, then assert that variable was called with "session-1" instead of accessing provider.interruptActiveTurn directly. Keep the existing mock behavior and call arguments unchanged.packages/ui/src/components/AppBar.tsx (1)
90-119: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRender window controls in browser mode on macOS.
Lines 90, 101, and 119 use
!isBrowser. This renders the controls on native macOS and hides them in browser mode. UseisBrowserin each predicate.Proposed fix
- {(!isMacOS || !isBrowser) && ( + {(!isMacOS || isBrowser) && (🤖 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 `@packages/ui/src/components/AppBar.tsx` around lines 90 - 119, Update the three window-control render predicates in AppBar so they use isBrowser rather than !isBrowser, ensuring the controls render in macOS browser mode while preserving the existing non-macOS behavior.packages/ui/src/components/message/MessageBlockAction.tsx (1)
47-60: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winStop the interval when the block crosses 180 seconds.
The age check runs only when the effect starts or its dependencies change. A block that is 179 seconds old starts
progressTimer, then continues to callsetCurrentTimeafter 180 seconds while its status remainsloadingorpending. This defeats the stale-block guard and can leave an interval running for the block's lifetime.Add a timeout or check the age inside the interval and clear it at the limit.
Suggested fix
useEffect(() => { - if (isRateLimitActive) { - if (Date.now() - block.timestamp > 180_000) return; + if (!isRateLimitActive) return; + const remainingMs = 180_000 - (Date.now() - block.timestamp); + if (remainingMs <= 0) return; + + progressTimer.current = window.setInterval(() => { + setCurrentTime(Date.now()); + }, 1000); + + const stopTimer = window.setTimeout(() => { + if (progressTimer.current !== null) { + window.clearInterval(progressTimer.current); + progressTimer.current = null; + } + }, remainingMs); + + return () => { + window.clearTimeout(stopTimer); + if (progressTimer.current !== null) { + window.clearInterval(progressTimer.current); + progressTimer.current = null; + } - progressTimer.current = window.setInterval(() => { - setCurrentTime(Date.now()); - }, 1000); - } - return () => { - if (progressTimer.current) { - clearInterval(progressTimer.current); - } - }; }, [isRateLimitActive, block.timestamp]);🤖 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 `@packages/ui/src/components/message/MessageBlockAction.tsx` around lines 47 - 60, Update the interval created in the rate-limit branch of the MessageBlockAction effect to check the block age on each tick and clear the interval once it reaches 180 seconds, preventing further setCurrentTime updates. Preserve the existing immediate stale-block guard and cleanup behavior.
🟡 Minor comments (3)
packages/ui/src/components/chat/composables/useChatStatusBarAcpConfig.ts-87-87 (1)
87-87: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winClear session loading after a pushed configuration update.
When
handleAcpConfigOptionsReadyaccepts a session payload, it sets the configuration as loaded but does not clearisAcpSessionConfigLoading. The loading indicator remains visible until the retry request finishes, or indefinitely if that request stalls.Set
setIsAcpSessionConfigLoading(false)in the acceptedconversationIdbranch.🤖 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 `@packages/ui/src/components/chat/composables/useChatStatusBarAcpConfig.ts` at line 87, Update handleAcpConfigOptionsReady so the accepted conversationId branch clears isAcpSessionConfigLoading by calling setIsAcpSessionConfigLoading(false) when the pushed session configuration is applied..commandcode/skills/migrate-radix-to-base/menus.md-67-67 (1)
67-67: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDocument
data-anchor-hiddenonPositioneronly.Base UI applies this hook to the positioning part, so remove
Popupfrom these migration notes and keep the CSS workaround scoped to thePositioner.
.commandcode/skills/migrate-radix-to-base/menus.md#L67.commandcode/skills/migrate-radix-to-base/overlays.md#L231🤖 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 @.commandcode/skills/migrate-radix-to-base/menus.md at line 67, Update the hideWhenDetached migration notes to document data-anchor-hidden only on Positioner, removing Popup references and scoping the CSS workaround selector to Positioner. Apply this change in .commandcode/skills/migrate-radix-to-base/menus.md:67 and .commandcode/skills/migrate-radix-to-base/overlays.md:231.Source: MCP tools
.commandcode/skills/migrate-radix-to-base/menus.md-52-52 (1)
52-52: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd
''to the documented focus callback interaction types.Base UI includes the empty interaction value, so add it to the focus callback types:
menus.md#L52: include''forfinalFocus.overlays.md#L57-L58: include''forinitialFocusandfinalFocus.🤖 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 @.commandcode/skills/migrate-radix-to-base/menus.md at line 52, Add the empty interaction value '' to the documented focus callback types: update finalFocus in menus.md, and both initialFocus and finalFocus in overlays.md. Preserve the existing interaction types and callback signatures while including '' in each relevant InteractionType union.Source: MCP tools
🤖 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 @.agents/skills/migrate-radix-to-base/class-mapping.md:
- Around line 3-4: Restrict the class rewrites in the class-mapping guidance to
verified migrated components by requiring selector-producer verification before
changing className values, cva definitions, or cn calls; do not rewrite custom
application hooks or intentionally untouched third-party components. Apply this
same scope restriction in .agents/skills/migrate-radix-to-base/class-mapping.md
lines 3-4 and .commandcode/skills/migrate-radix-to-base/class-mapping.md lines
3-4.
In @.agents/skills/migrate-radix-to-base/display-misc.md:
- Around line 63-66: Update the nonce migration entry in both
.agents/skills/migrate-radix-to-base/display-misc.md (lines 63-66) and
.commandcode/skills/migrate-radix-to-base/display-misc.md (lines 63-66) to
document Base UI’s CSPProvider, including both nonce and disableStyleElements,
instead of stating that no CSP equivalent exists.
In @.agents/skills/migrate-radix-to-base/form-controls.md:
- Around line 70-80: The Select positioning migration guidance must state that
Base UI ignores most Positioner positioning props while alignItemWithTrigger is
active, except during automatic fallback. Add this clarification to both
.agents/skills/migrate-radix-to-base/form-controls.md lines 70-80 and
.commandcode/skills/migrate-radix-to-base/form-controls.md lines 70-80,
preserving the existing prop mappings.
In @.agents/skills/migrate-radix-to-base/menus.md:
- Around line 9-14: Update the DirectionProvider reference in the migration
table to use the current `@base-ui/react/direction-provider` package path instead
of `@base-ui-components/react/direction-provider`, keeping the skill’s Base UI
package namespace consistent.
In @.agents/skills/migrate-radix-to-base/SKILL.md:
- Around line 122-124: Expand the “Verify and report” validation sequence in
both .agents/skills/migrate-radix-to-base/SKILL.md lines 122-124 and
.commandcode/skills/migrate-radix-to-base/SKILL.md lines 122-124 to include bun
run format, bun run lint, and the repository’s test command alongside the
existing per-file typecheck, batch build, and final full-build checks.
- Around line 57-60: Replace every grep-based search in the migration
documentation with the repository-approved fff MCP search. Update the leftover
sweep in .agents/skills/migrate-radix-to-base/SKILL.md:57-60, report-template
search in .agents/skills/migrate-radix-to-base/SKILL.md:154-156, mapping sweep
in .agents/skills/migrate-radix-to-base/universal-patterns.md:57-60, and
consumer sweep in .agents/skills/migrate-radix-to-base/consumer-props.md:54-55;
apply the corresponding fff replacements at
.commandcode/skills/migrate-radix-to-base/SKILL.md:57-60 and :154-156,
.commandcode/skills/migrate-radix-to-base/universal-patterns.md:57-60, and
.commandcode/skills/migrate-radix-to-base/consumer-props.md:54-55, preserving
each search’s existing patterns and scope.
- Around line 16-21: Pin the shadcn CLI to the exact reviewed version in the
preflight command described by both
.agents/skills/migrate-radix-to-base/SKILL.md lines 16-21 and
.commandcode/skills/migrate-radix-to-base/SKILL.md lines 16-21, replacing the
`@latest` reference while continuing to use the detected package manager for
execution and installs.
In @.agents/skills/migrate-radix-to-base/universal-patterns.md:
- Around line 76-80: The individual package migration guidance must only remove
an `@radix-ui/react-`* dependency after a repository-wide import sweep confirms
zero remaining consumers. Update the corresponding instruction in both
.agents/skills/migrate-radix-to-base/universal-patterns.md (lines 76-80) and
.commandcode/skills/migrate-radix-to-base/universal-patterns.md (lines 76-80),
preserving the named-import migration while making dependency removal
conditional.
In @.commandcode/skills/migrate-radix-to-base/menus.md:
- Around line 7-17: Revise the universal rules in
.commandcode/skills/migrate-radix-to-base/menus.md lines 7-17 so asChild maps to
render only for renderable parts; document that it is dropped for non-rendering
parts such as Menu.Root and overlay Root components. Apply the same narrowed
mapping and dropped-prop guidance in
.commandcode/skills/migrate-radix-to-base/overlays.md lines 7-13, without
changing the other migration mappings.
In `@apps/daemon/src/host/acp-provider-execution.ts`:
- Around line 561-575: Update the steering interruption flow around
active.controller.abort() to resolve every pendingPermissions entry for the
session with "cancelled" before awaiting active.donePromise. Extract or reuse
the existing pending-permission resolution logic from cancelGeneration so both
cancellation paths clear stale permission overlays consistently.
- Around line 563-575: The interruptActiveTurn flow currently starts its
four-second timeout only after getRuntime and session.connection.agent.notify
complete. Restructure interruptActiveTurn so runtime lookup, cancellation
notification, and active.donePromise all race against one shared four-second
deadline, allowing steerActiveTurn to proceed even when ACP access or
cancellation stalls.
In `@build/artifact-demo/nebula-flow.html`:
- Around line 164-173: Update initParticles() so each newly created particle
receives the intended theme color for hue, saturation, and lightness instead of
zero values. Preserve the existing particle allocation and position/velocity
initialization, and ensure resetField() also produces visible theme-colored
particles through this initialization path.
- Around line 41-43: Update the range controls and their labels so each slider
has an accessible name: associate the labels with the `speed` and `turb` inputs
using matching `for` and `id` attributes, and add a visible `:focus-visible`
style for range inputs to replace the removed outline. Preserve the existing
slider appearance and apply the focus treatment to both WebKit and Firefox
controls.
In `@build/artifact-demo/neon-snake.html`:
- Around line 212-217: Update the win handling around placeFood() to call the
existing shared gameOver() end-of-round function instead of setting state and
overlay elements inline. Ensure gameOver() handles both wins and collisions by
updating best, bestVal, localStorage, newBestTag, the heading, and overlay
state, while preserving the appropriate win versus collision outcome.
- Around line 332-352: Update the render order around drawParticles,
paintBackground, drawGrid, drawFood, and drawSnake: save and apply the
screen-shake translation before rendering the scene, paint the background before
drawing particles, then render the remaining scene and death flash under the
same transform. Restore the canvas context only after all scene effects have
completed.
- Around line 129-138: Update placeFood() so it only assigns food to a
coordinate confirmed not to overlap any snake segment. Replace the bounded
random retry behavior with selection from available empty cells, or return false
when no valid cell is found; never assign the last attempted occupied coordinate
or return true for it.
In `@packages/ui/src/components/AppBar.tsx`:
- Around line 26-30: Update the welcome-route comparison in the showUpdateButton
useMemo to match the browser pathname format, using the leading slash so the
update button remains hidden on "/welcome". Preserve the existing behavior for
non-welcome routes and non-browser mode.
In `@packages/ui/src/components/chat/AcpAdvancedSettings.tsx`:
- Around line 88-97: Use a safe accumulator for ACP option groups in
packages/ui/src/components/chat/AcpAdvancedSettings.tsx lines 88-97 and
packages/ui/src/components/chat/ChatStatusBar.tsx lines 1360-1367 by replacing
the plain object with Map or Object.create(null); update the grouping access and
insertion logic in both sites so keys such as __proto__ and constructor
initialize and collect entries correctly.
In `@packages/ui/src/pages/ChatPage.tsx`:
- Around line 456-462: In packages/ui/src/pages/ChatPage.tsx, move
draft-clearing from finally blocks to the success paths of
sendMessage/queueInput and steerActiveTurn, preserving message text and
attachments when requests fail; in onCommandSubmit, call setMessage("") before
clearInput() after successful submission. Update the affected sites at lines
456-462, 496-501, and 553-558.
- Around line 620-622: Update the useEffect that resets isCancelling to also
react to sessionId changes, resetting cancellation state when navigation
switches sessions even if isGenerating remains true. Keep the existing reset
behavior for changes to isGenerating and include sessionId in the effect
dependencies.
---
Outside diff comments:
In `@apps/daemon/test/daemonSessionRoutes.test.ts`:
- Around line 890-909: Store the vi.spyOn result for interruptActiveTurn in a
local variable before exercising steerActiveTurn, then assert that variable was
called with "session-1" instead of accessing provider.interruptActiveTurn
directly. Keep the existing mock behavior and call arguments unchanged.
In `@packages/ui/src/components/AppBar.tsx`:
- Around line 90-119: Update the three window-control render predicates in
AppBar so they use isBrowser rather than !isBrowser, ensuring the controls
render in macOS browser mode while preserving the existing non-macOS behavior.
In `@packages/ui/src/components/message/MessageBlockAction.tsx`:
- Around line 47-60: Update the interval created in the rate-limit branch of the
MessageBlockAction effect to check the block age on each tick and clear the
interval once it reaches 180 seconds, preventing further setCurrentTime updates.
Preserve the existing immediate stale-block guard and cleanup behavior.
---
Minor comments:
In @.commandcode/skills/migrate-radix-to-base/menus.md:
- Line 67: Update the hideWhenDetached migration notes to document
data-anchor-hidden only on Positioner, removing Popup references and scoping the
CSS workaround selector to Positioner. Apply this change in
.commandcode/skills/migrate-radix-to-base/menus.md:67 and
.commandcode/skills/migrate-radix-to-base/overlays.md:231.
- Line 52: Add the empty interaction value '' to the documented focus callback
types: update finalFocus in menus.md, and both initialFocus and finalFocus in
overlays.md. Preserve the existing interaction types and callback signatures
while including '' in each relevant InteractionType union.
In `@packages/ui/src/components/chat/composables/useChatStatusBarAcpConfig.ts`:
- Line 87: Update handleAcpConfigOptionsReady so the accepted conversationId
branch clears isAcpSessionConfigLoading by calling
setIsAcpSessionConfigLoading(false) when the pushed session configuration is
applied.
🪄 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 Plus
Run ID: ceadea82-57f8-4e7a-896d-7b8ce336afad
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (57)
.agents/skills/migrate-radix-to-base/SKILL.md.agents/skills/migrate-radix-to-base/class-mapping.md.agents/skills/migrate-radix-to-base/consumer-props.md.agents/skills/migrate-radix-to-base/disclosure.md.agents/skills/migrate-radix-to-base/display-misc.md.agents/skills/migrate-radix-to-base/form-controls.md.agents/skills/migrate-radix-to-base/menus.md.agents/skills/migrate-radix-to-base/overlays.md.agents/skills/migrate-radix-to-base/universal-patterns.md.agents/skills/migrate-radix-to-base/wrapper-shapes.md.commandcode/skills/migrate-radix-to-base/SKILL.md.commandcode/skills/migrate-radix-to-base/class-mapping.md.commandcode/skills/migrate-radix-to-base/consumer-props.md.commandcode/skills/migrate-radix-to-base/disclosure.md.commandcode/skills/migrate-radix-to-base/display-misc.md.commandcode/skills/migrate-radix-to-base/form-controls.md.commandcode/skills/migrate-radix-to-base/menus.md.commandcode/skills/migrate-radix-to-base/overlays.md.commandcode/skills/migrate-radix-to-base/universal-patterns.md.commandcode/skills/migrate-radix-to-base/wrapper-shapes.mdapps/daemon/src/host/acp-provider-execution.tsapps/daemon/test/acpProviderExecution.test.tsapps/daemon/test/daemonSessionRoutes.test.tsapps/desktop/components.jsonapps/desktop/src/main/routes/settings/settingsAdapter.tsbuild/artifact-demo/nebula-flow.htmlbuild/artifact-demo/neon-snake.htmlpackages/backend-core/src/dispatch/settings/settingsAdapter.tspackages/shared-contracts/src/routes/settings.routes.tspackages/ui/package.jsonpackages/ui/settings/components/CommonSettings.tsxpackages/ui/shadcn/components/ui/badge.tsxpackages/ui/shadcn/components/ui/button-group.tsxpackages/ui/shadcn/components/ui/button.tsxpackages/ui/shadcn/components/ui/input.tsxpackages/ui/shadcn/components/ui/label.tsxpackages/ui/shadcn/components/ui/separator.tsxpackages/ui/src/assets/style.csspackages/ui/src/components/AppBar.tsxpackages/ui/src/components/chat/AcpAdvancedSettings.tsxpackages/ui/src/components/chat/ChatInputBox.tsxpackages/ui/src/components/chat/ChatInputToolbar.tsxpackages/ui/src/components/chat/ChatStatusBar.tsxpackages/ui/src/components/chat/MessageList.tsxpackages/ui/src/components/chat/MessageListRow.tsxpackages/ui/src/components/chat/composables/useChatStatusBarAcpConfig.tspackages/ui/src/components/markdown/useMarkdownLinkNavigation.tspackages/ui/src/components/message/MessageBlockAction.tsxpackages/ui/src/components/message/MessageBlockContent.tsxpackages/ui/src/components/message/MessageItemAssistant.tsxpackages/ui/src/pages/ChatPage.tsxpackages/ui/src/pages/NewThreadPage.tsxpackages/ui/src/routeTree.gen.tspackages/ui/src/stores/artifact.tspackages/ui/src/stores/ui/message.tspackages/ui/src/stores/uiSettingsStore.tsskills-lock.json
💤 Files with no reviewable changes (1)
- packages/ui/src/assets/style.css
| Apply these across ALL class strings (className, cva definitions, cn calls), | ||
| including app code. They are safe, mechanical rewrites. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restrict class rewrites to verified migrated components.
The global scope can rewrite custom application hooks and intentionally untouched third-party components.
.agents/skills/migrate-radix-to-base/class-mapping.md#L3-L4: require selector-producer verification..commandcode/skills/migrate-radix-to-base/class-mapping.md#L3-L4: apply the same scope restriction.
📍 Affects 2 files
.agents/skills/migrate-radix-to-base/class-mapping.md#L3-L4(this comment).commandcode/skills/migrate-radix-to-base/class-mapping.md#L3-L4
🤖 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 @.agents/skills/migrate-radix-to-base/class-mapping.md around lines 3 - 4,
Restrict the class rewrites in the class-mapping guidance to verified migrated
components by requiring selector-producer verification before changing className
values, cva definitions, or cn calls; do not rewrite custom application hooks or
intentionally untouched third-party components. Apply this same scope
restriction in .agents/skills/migrate-radix-to-base/class-mapping.md lines 3-4
and .commandcode/skills/migrate-radix-to-base/class-mapping.md lines 3-4.
| | Radix pattern | Base UI equivalent | | ||
| | --- | --- | | ||
| | `asChild` (`boolean`, `false`) | `render` (`ReactElement \| ((props: HTMLProps, state) => ReactElement)`). No merge-onto-child boolean; pass the element or a function. | | ||
| | `dir` (`"ltr" \| "rtl"`) on roots | Dropped everywhere. Base UI reads direction from `<DirectionProvider>` (`@base-ui-components/react/direction-provider`) or the DOM `dir` attribute. | | ||
| | `forceMount` (`boolean`) | `keepMounted` (`boolean`, `false`) on `Portal` / indicator parts. Same use case (animation/SEO), presence is CSS-driven via `data-starting-style` / `data-ending-style` instead of Radix `data-state` + forced mount. | | ||
| | `onEscapeKeyDown` / `onPointerDownOutside` / `onFocusOutside` / `onInteractOutside` (content parts) | Dropped as separate props. Use `onOpenChange(open, eventDetails)` on the Root and branch on `eventDetails.reason` (`'escape-key'`, `'outside-press'`, `'focus-out'`, ...). Call `eventDetails.cancel()` to prevent the close (replaces `event.preventDefault()`). | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the current Base UI package name.
@base-ui-components/react/direction-provider is the former package path. The migration guidance targets @base-ui/react@1.6.0, so this import path can fail after migration. Change it to @base-ui/react/direction-provider and use one package namespace throughout the skill. Base UI release notes document the package rename. (base-ui.com)
#!/usr/bin/env bash
set -euo pipefail
bun pm ls `@base-ui/react`🤖 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 @.agents/skills/migrate-radix-to-base/menus.md around lines 9 - 14, Update
the DirectionProvider reference in the migration table to use the current
`@base-ui/react/direction-provider` package path instead of
`@base-ui-components/react/direction-provider`, keeping the skill’s Base UI
package namespace consistent.
| drawParticles(dt); | ||
|
|
||
| paintBackground(false); | ||
| drawGrid(); | ||
| if (state !== "start") drawFood(tGlobal); | ||
| if (state === "playing" || state === "paused" || state === "dead") drawSnake(); | ||
|
|
||
| // screen shake on death | ||
| if (shake > 0) { | ||
| ctx.save(); | ||
| ctx.translate((Math.random() - 0.5) * shake, (Math.random() - 0.5) * shake); | ||
| shake *= 0.86; | ||
| if (shake < 0.5) shake = 0; | ||
| } | ||
| // red flash on death | ||
| if (state === "dead") { | ||
| ctx.globalCompositeOperation = "source-over"; | ||
| ctx.fillStyle = `rgba(248, 60, 90, ${0.08 + Math.random() * 0.05})`; | ||
| ctx.fillRect(0, 0, W, H); | ||
| } | ||
| ctx.restore(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Correct the canvas render order.
Line 332 draws particles before Line 334 fills the canvas. The background erases every particle each frame. Lines 341-345 apply the translation after the grid, food, and snake render. Only later drawing can move, so the scene does not shake.
Save and translate the context before rendering the scene. Paint the background before drawing particles. Restore the context after all scene effects render.
🤖 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 `@build/artifact-demo/neon-snake.html` around lines 332 - 352, Update the
render order around drawParticles, paintBackground, drawGrid, drawFood, and
drawSnake: save and apply the screen-shake translation before rendering the
scene, paint the background before drawing particles, then render the remaining
scene and death flash under the same transform. Restore the canvas context only
after all scene effects have completed.
| const selectGrouped = selectEntries.reduce< | ||
| Record<string, { label: string; entries: typeof selectEntries }> | ||
| >((acc, entry) => { | ||
| const g = resolveAcpOptionGroup(entry); | ||
| if (!acc[g.key]) { | ||
| acc[g.key] = { label: g.label, entries: [] }; | ||
| } | ||
| acc[g.key].entries.push(entry); | ||
| return acc; | ||
| }, {}); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Use a safe map for ACP group keys.
A group key of __proto__, constructor, or another inherited property bypasses group initialization. The subsequent entries.push then throws during rendering. ACP option metadata supplies these keys.
packages/ui/src/components/chat/AcpAdvancedSettings.tsx#L88-L97: replace the plain-object accumulator withMap, or initialize it withObject.create(null).packages/ui/src/components/chat/ChatStatusBar.tsx#L1360-L1367: apply the same safe accumulator implementation.
📍 Affects 2 files
packages/ui/src/components/chat/AcpAdvancedSettings.tsx#L88-L97(this comment)packages/ui/src/components/chat/ChatStatusBar.tsx#L1360-L1367
🤖 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 `@packages/ui/src/components/chat/AcpAdvancedSettings.tsx` around lines 88 -
97, Use a safe accumulator for ACP option groups in
packages/ui/src/components/chat/AcpAdvancedSettings.tsx lines 88-97 and
packages/ui/src/components/chat/ChatStatusBar.tsx lines 1360-1367 by replacing
the plain object with Map or Object.create(null); update the grouping access and
insertion logic in both sites so keys such as __proto__ and constructor
initialize and collect entries correctly.
| } catch (error) { | ||
| console.error("[ChatPage] send message failed:", error); | ||
| } finally { | ||
| setMessage(""); | ||
| setAttachedFiles([]); | ||
| chatInputRef.current?.clearInput(); | ||
| schedulePostSubmitScrollToBottom(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the draft until submission succeeds.
Each catch flows into finally, which clears typed text, attachments, and the editor after a failed request. This discards the user's draft. onCommandSubmit also suppresses the editor update without calling setMessage(""), so successful command submission leaves stale controlled input state.
packages/ui/src/pages/ChatPage.tsx#L456-L462: clear the draft only aftersendMessageorqueueInputsucceeds.packages/ui/src/pages/ChatPage.tsx#L496-L501: preserve the draft on failure; on success, callsetMessage("")beforeclearInput().packages/ui/src/pages/ChatPage.tsx#L553-L558: clear the draft only aftersteerActiveTurnsucceeds.
📍 Affects 1 file
packages/ui/src/pages/ChatPage.tsx#L456-L462(this comment)packages/ui/src/pages/ChatPage.tsx#L496-L501packages/ui/src/pages/ChatPage.tsx#L553-L558
🤖 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 `@packages/ui/src/pages/ChatPage.tsx` around lines 456 - 462, In
packages/ui/src/pages/ChatPage.tsx, move draft-clearing from finally blocks to
the success paths of sendMessage/queueInput and steerActiveTurn, preserving
message text and attachments when requests fail; in onCommandSubmit, call
setMessage("") before clearInput() after successful submission. Update the
affected sites at lines 456-462, 496-501, and 553-558.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/ui/src/components/message/MessageToolbar.tsx (1)
121-353: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd
aria-labelto each icon-only Button.The affected buttons are rendered inside
TooltipTriggerand do not get an accessible name from the text that appears only inTooltipContent. Add an action-matchingaria-labelto each icon-only button.🤖 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 `@packages/ui/src/components/message/MessageToolbar.tsx` around lines 121 - 353, Each icon-only Button in MessageToolbar.tsx (lines 121-353), including save, cancel, retry, variant navigation, copy, image copy, trace, fork, edit, and delete actions, needs an action-matching aria-label; also add the corresponding labels to the icon-only buttons in MessageBlockImage.tsx (lines 151-164) and MessageBlockToolCallImagePreview.tsx (lines 117-130). Do not rely on TooltipContent for accessible naming.packages/ui/shadcn/components/ui/tabs.tsx (1)
34-44: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve automatic tab activation.
Base UI
Tabs.ListdefaultsactivateOnFocustofalse, while the previous Radix tabs behaved automatically. SetactivateOnFocus = trueinTabsListand pass it toTabsPrimitive.Listto keep keyboard activation consistent for consumers such asRemoteSettingsandEmojiPicker.🤖 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 `@packages/ui/shadcn/components/ui/tabs.tsx` around lines 34 - 44, Update the TabsList component to default activateOnFocus to true and pass that prop through to TabsPrimitive.List, preserving automatic keyboard tab activation for existing consumers.
🧹 Nitpick comments (3)
packages/ui/tsconfig.app.json (1)
51-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
#-prefixed aliases for internal modules.The new bare
shadcn/*alias enables imports that violate the repository convention. Retain#shadcn/*and update all new callers.
packages/ui/tsconfig.app.json#L51-L53: removeshadcn/*.packages/ui/shadcn/components/ui/card.tsx#L3-L3: use#shadcn/lib/utils.packages/ui/shadcn/components/ui/checkbox.tsx#L3-L3: use#shadcn/lib/utils.packages/ui/shadcn/components/ui/dialog.tsx#L4-L5: use#shadcn/lib/utilsand#shadcn/components/ui/button.packages/ui/shadcn/components/ui/scroll-area.tsx#L4-L4: use#shadcn/lib/utils.packages/ui/shadcn/components/ui/select.tsx#L6-L6: use#shadcn/lib/utils.As per coding guidelines, use
#-prefixed aliases for internal modules.🤖 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 `@packages/ui/tsconfig.app.json` around lines 51 - 53, Replace the bare shadcn/* path alias in packages/ui/tsconfig.app.json (lines 51-53) by retaining only the `#shadcn/`* alias, then update imports in packages/ui/shadcn/components/ui/card.tsx (line 3), checkbox.tsx (line 3), dialog.tsx (lines 4-5), scroll-area.tsx (line 4), and select.tsx (line 6) to use `#shadcn/lib/utils`; in dialog.tsx, also use `#shadcn/components/ui/button`.Source: Coding guidelines
packages/ui/shadcn/components/ui/context-menu.tsx (2)
128-129: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMerge the submenu class with
cn.
propsspreads after the literalclassName. A callerclassNametherefore replaces"shadow-lg"instead of extending it. Merge both values.♻️ Proposed refactor
-function ContextMenuSubContent({ ...props }: React.ComponentProps<typeof ContextMenuContent>) { - return <ContextMenuContent data-slot="context-menu-sub-content" className="shadow-lg" side="right" {...props} />; +function ContextMenuSubContent({ className, ...props }: React.ComponentProps<typeof ContextMenuContent>) { + return ( + <ContextMenuContent + data-slot="context-menu-sub-content" + className={cn("shadow-lg", className)} + side="right" + {...props} + /> + ); }🤖 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 `@packages/ui/shadcn/components/ui/context-menu.tsx` around lines 128 - 129, Update ContextMenuSubContent to merge the default "shadow-lg" class with props.className via the existing cn utility, ensuring caller classes extend rather than replace the submenu styling while preserving the remaining props behavior.
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRestore the
#shadcn/...alias in shadcn imports. Vite aliases both#shadcnandshadcntoshadcn, but the project guideline uses#-prefixed aliases for internal modules, while existing consumers import through#shadcn/.... Bring these primitive imports back to#shadcn/...so the internal module scheme is consistent.🤖 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 `@packages/ui/shadcn/components/ui/context-menu.tsx` at line 4, Restore the `#shadcn/`... alias for primitive imports in context-menu.tsx, dropdown-menu.tsx, field.tsx, radio-group.tsx, slider.tsx, spinner.tsx, table.tsx, and textarea.tsx at the listed import sites. Replace each unprefixed shadcn/... path with its `#shadcn/`... equivalent while preserving the imported symbols and other imports.Source: Coding guidelines
🤖 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 `@packages/ui/components.json`:
- Around line 15-20: Restore the # prefix for every internal alias value in
packages/ui/components.json (lines 15-20). Update the cn imports in
packages/ui/shadcn/components/ui/accordion.tsx (line 3), popover.tsx (line 6),
skeleton.tsx (line 1), tabs.tsx (line 6), and tooltip.tsx (line 3) to use the
`#shadcn/lib/utils` convention consistently.
In `@packages/ui/shadcn/components/ui/alert-dialog.tsx`:
- Around line 6-7: Update the imports in
packages/ui/shadcn/components/ui/alert-dialog.tsx lines 6-7 to use the `#shadcn/`*
alias. Apply the same alias change to all four imports in
packages/ui/shadcn/components/ui/input-group.tsx lines 4-7 and the utility
import in packages/ui/shadcn/components/ui/kbd.tsx line 1; make no other
changes.
- Around line 125-127: Update AlertDialogAction to compose through
AlertDialogPrimitive.Close.render, matching AlertDialogCancel, while preserving
its existing Button styling, data-slot, className handling, and props. Ensure
confirmation actions notify the alert dialog close primitive so controlled
open/onOpenChange state is updated before or alongside the action handler.
In `@packages/ui/shadcn/components/ui/dropdown-menu.tsx`:
- Around line 38-45: Remove the w-(--anchor-width) utility from the class list
passed to MenuPrimitive.Popup in the dropdown menu content component, preserving
the existing min-w-32 sizing and other styles so labels do not wrap based on
trigger width.
In `@packages/ui/shadcn/components/ui/slider.tsx`:
- Around line 5-16: Update Slider to resolve scalar and array value/defaultValue
inputs into the correct thumb values, preserving a single-number input as one
thumb instead of falling back to [min, max]. Use the resolved value when
configuring SliderPrimitive.Root while retaining the existing min/max defaults
and thumbAlignment behavior.
In `@packages/ui/src/components/message/ImageActionContextMenu.tsx`:
- Line 42: Update the wrapper rendered by ContextMenuTrigger in
ImageActionContextMenu so it is layout-transparent and preserves the preview
button as the grid item, then verify other grid and flex call sites retain their
existing child structure and layout behavior.
In `@packages/ui/vite.config.ts`:
- Line 56: Use one `#-prefixed` internal alias convention: remove the bare shadcn
alias in packages/ui/vite.config.ts at lines 56-56, and update imports in
packages/ui/shadcn/components/ui/alert.tsx lines 4-4, button-group.tsx lines
5-6, empty.tsx lines 3-3, label.tsx lines 3-3, progress.tsx lines 3-3, and
sheet.tsx lines 4-5 to use `#shadcn/`* paths for cn, Separator, and Button.
---
Outside diff comments:
In `@packages/ui/shadcn/components/ui/tabs.tsx`:
- Around line 34-44: Update the TabsList component to default activateOnFocus to
true and pass that prop through to TabsPrimitive.List, preserving automatic
keyboard tab activation for existing consumers.
In `@packages/ui/src/components/message/MessageToolbar.tsx`:
- Around line 121-353: Each icon-only Button in MessageToolbar.tsx (lines
121-353), including save, cancel, retry, variant navigation, copy, image copy,
trace, fork, edit, and delete actions, needs an action-matching aria-label; also
add the corresponding labels to the icon-only buttons in MessageBlockImage.tsx
(lines 151-164) and MessageBlockToolCallImagePreview.tsx (lines 117-130). Do not
rely on TooltipContent for accessible naming.
---
Nitpick comments:
In `@packages/ui/shadcn/components/ui/context-menu.tsx`:
- Around line 128-129: Update ContextMenuSubContent to merge the default
"shadow-lg" class with props.className via the existing cn utility, ensuring
caller classes extend rather than replace the submenu styling while preserving
the remaining props behavior.
- Line 4: Restore the `#shadcn/`... alias for primitive imports in
context-menu.tsx, dropdown-menu.tsx, field.tsx, radio-group.tsx, slider.tsx,
spinner.tsx, table.tsx, and textarea.tsx at the listed import sites. Replace
each unprefixed shadcn/... path with its `#shadcn/`... equivalent while preserving
the imported symbols and other imports.
In `@packages/ui/tsconfig.app.json`:
- Around line 51-53: Replace the bare shadcn/* path alias in
packages/ui/tsconfig.app.json (lines 51-53) by retaining only the `#shadcn/`*
alias, then update imports in packages/ui/shadcn/components/ui/card.tsx (line
3), checkbox.tsx (line 3), dialog.tsx (lines 4-5), scroll-area.tsx (line 4), and
select.tsx (line 6) to use `#shadcn/lib/utils`; in dialog.tsx, also use
`#shadcn/components/ui/button`.
🪄 Autofix
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 Plus
Run ID: f4b03d56-3fa1-41d6-a40f-7bea2178fa2d
📒 Files selected for processing (96)
apps/daemon/src/host/acp-provider-execution.tspackages/ui/components.jsonpackages/ui/settings/components/AboutUsSettings.tsxpackages/ui/settings/components/AcpDiagnostics.tsxpackages/ui/settings/components/AcpSettings.tsxpackages/ui/settings/components/ArgosAgentsSettings.tsxpackages/ui/settings/components/BedrockProviderSettingsDetail.tsxpackages/ui/settings/components/BuiltinKnowledgeSettings.tsxpackages/ui/settings/components/DataSettings.tsxpackages/ui/settings/components/DifyKnowledgeSettings.tsxpackages/ui/settings/components/FastGptKnowledgeSettings.tsxpackages/ui/settings/components/GeminiSafetyConfig.tsxpackages/ui/settings/components/KnowledgeFile.tsxpackages/ui/settings/components/KnowledgeFileItem.tsxpackages/ui/settings/components/McpSettings.tsxpackages/ui/settings/components/MemoryManagerPanel.tsxpackages/ui/settings/components/NotificationsHooksSettings.tsxpackages/ui/settings/components/OllamaProviderSettingsDetail.tsxpackages/ui/settings/components/ProviderApiConfig.tsxpackages/ui/settings/components/ProviderModelList.tsxpackages/ui/settings/components/RagflowKnowledgeSettings.tsxpackages/ui/settings/components/RemoteSettings.tsxpackages/ui/settings/components/ScheduledTasksSettings.tsxpackages/ui/settings/components/VertexProviderSettingsDetail.tsxpackages/ui/settings/components/VoiceAIProviderConfig.tsxpackages/ui/settings/components/common/ProxySettingsSection.tsxpackages/ui/settings/components/display/FontSettingsSection.tsxpackages/ui/settings/components/prompt/CustomPromptSettingsSection.tsxpackages/ui/settings/components/prompt/SystemPromptSettingsSection.tsxpackages/ui/settings/main.tsxpackages/ui/shadcn/components/ui/accordion.tsxpackages/ui/shadcn/components/ui/alert-dialog.tsxpackages/ui/shadcn/components/ui/alert.tsxpackages/ui/shadcn/components/ui/badge.tsxpackages/ui/shadcn/components/ui/button-group.tsxpackages/ui/shadcn/components/ui/button.tsxpackages/ui/shadcn/components/ui/card.tsxpackages/ui/shadcn/components/ui/checkbox.tsxpackages/ui/shadcn/components/ui/collapsible.tsxpackages/ui/shadcn/components/ui/context-menu.tsxpackages/ui/shadcn/components/ui/dialog.tsxpackages/ui/shadcn/components/ui/dropdown-menu.tsxpackages/ui/shadcn/components/ui/empty.tsxpackages/ui/shadcn/components/ui/field.tsxpackages/ui/shadcn/components/ui/input-group.tsxpackages/ui/shadcn/components/ui/input.tsxpackages/ui/shadcn/components/ui/kbd.tsxpackages/ui/shadcn/components/ui/label.tsxpackages/ui/shadcn/components/ui/popover.tsxpackages/ui/shadcn/components/ui/progress.tsxpackages/ui/shadcn/components/ui/radio-group.tsxpackages/ui/shadcn/components/ui/scroll-area.tsxpackages/ui/shadcn/components/ui/select.tsxpackages/ui/shadcn/components/ui/separator.tsxpackages/ui/shadcn/components/ui/sheet.tsxpackages/ui/shadcn/components/ui/skeleton.tsxpackages/ui/shadcn/components/ui/slider.tsxpackages/ui/shadcn/components/ui/spinner.tsxpackages/ui/shadcn/components/ui/switch.tsxpackages/ui/shadcn/components/ui/table.tsxpackages/ui/shadcn/components/ui/tabs.tsxpackages/ui/shadcn/components/ui/textarea.tsxpackages/ui/shadcn/components/ui/tooltip.tsxpackages/ui/src/components/ConnectionIndicator.tsxpackages/ui/src/components/FolderPicker.tsxpackages/ui/src/components/WindowSideBar.tsxpackages/ui/src/components/WorkspaceSelector.tsxpackages/ui/src/components/chat-input/McpIndicator.tsxpackages/ui/src/components/chat/AcpAdvancedSettings.tsxpackages/ui/src/components/chat/ChatInputToolbar.tsxpackages/ui/src/components/chat/ChatStatusBar.tsxpackages/ui/src/components/chat/ChatTopBar.tsxpackages/ui/src/components/chat/MessageList.tsxpackages/ui/src/components/chat/composables/useChatStatusBarAcpConfig.tspackages/ui/src/components/emoji-picker/EmojiPicker.tsxpackages/ui/src/components/mcp-config/components/McpPromptPanel.tsxpackages/ui/src/components/mcp-config/components/McpResourceViewer.tsxpackages/ui/src/components/mcp-config/components/McpServerCard.tsxpackages/ui/src/components/mcp-config/components/McpServers.tsxpackages/ui/src/components/mcp-config/components/McpToolPanel.tsxpackages/ui/src/components/mcp/McpSamplingDialog.tsxpackages/ui/src/components/message/ImageActionContextMenu.tsxpackages/ui/src/components/message/MessageBlockAction.tsxpackages/ui/src/components/message/MessageBlockImage.tsxpackages/ui/src/components/message/MessageBlockToolCallImagePreview.tsxpackages/ui/src/components/message/MessageItemAssistant.tsxpackages/ui/src/components/message/MessageToolbar.tsxpackages/ui/src/components/settings/ModelCheckDialog.tsxpackages/ui/src/components/settings/OpenAIImageGenerationSettingsFields.tsxpackages/ui/src/components/settings/TtsSettingsFields.tsxpackages/ui/src/components/workspace/WorkspaceFileNode.tsxpackages/ui/src/main.tsxpackages/ui/src/pages/ChatPage.tsxpackages/ui/src/pages/NewThreadPage.tsxpackages/ui/tsconfig.app.jsonpackages/ui/vite.config.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- packages/ui/src/components/message/MessageBlockAction.tsx
- packages/ui/src/components/message/MessageItemAssistant.tsx
- packages/ui/src/components/chat/MessageList.tsx
- packages/ui/src/pages/NewThreadPage.tsx
- packages/ui/shadcn/components/ui/button.tsx
- packages/ui/src/components/chat/composables/useChatStatusBarAcpConfig.ts
- packages/ui/src/components/chat/AcpAdvancedSettings.tsx
- packages/ui/shadcn/components/ui/separator.tsx
- apps/daemon/src/host/acp-provider-execution.ts
- packages/ui/src/components/chat/ChatInputToolbar.tsx
- packages/ui/shadcn/components/ui/input.tsx
- packages/ui/src/components/chat/ChatStatusBar.tsx
- packages/ui/shadcn/components/ui/badge.tsx
| function AlertDialogAction({ className, ...props }: React.ComponentProps<typeof Button>) { | ||
| return <Button data-slot="alert-dialog-action" className={cn(className)} {...props} />; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file and related files =="
fd -a 'alert-dialog\.tsx|dialog\.tsx' packages/ui/shadcn/components/ui 2>/dev/null || true
echo "== inspect target file outline =="
ast-grep outline packages/ui/shadcn/components/ui/alert-dialog.tsx || true
echo "== relevant sections =="
sed -n '1,180p' packages/ui/shadcn/components/ui/alert-dialog.tsx
echo "== usages of AlertDialogAction =="
rg -n "AlertDialogAction|AlertDialog\.(Open|Close|Cancel|Trigger|Action)" packages -g '*.tsx' -g '*.ts' | head -200Repository: dvaJi/argos
Length of output: 10404
🌐 Web query:
@base-ui/react alert-dialog Close render property Close.Props
💡 Result:
In the @base-ui/react Alert Dialog component, AlertDialog.Close accepts a render prop that allows you to replace or compose the underlying element [1]. The render prop is defined as follows: render: ReactElement | function This prop allows you to replace the component's default HTML element (a element by default) with a different tag or compose it with another component [1]. It accepts either a ReactElement or a function that returns the element to be rendered [1]. Additionally, there is a related nativeButton prop: nativeButton: boolean (default: true) This prop specifies whether the component renders a native element when replacing it via the render prop. You should set this to false if the rendered element is not a button (for example, if you are rendering an tag) [1]. Summary of AlertDialog.Close Props: - render: Customizes the element rendering [1]. - nativeButton: Indicates if the rendered element is a native button, which helps with correct accessibility and behavior when using the render prop [1].
Citations:
🌐 Web query:
Base UI React alert-dialog primitive close component documentation
💡 Result:
In Base UI for React, the AlertDialog.Close component is a primitive used to render a button that closes the alert dialog when clicked [1][2]. Usage To implement the close functionality, include the AlertDialog.Close component within the AlertDialog.Popup [3][1]: import { AlertDialog } from '@base-ui/react/alert-dialog'; <AlertDialog.Root> <AlertDialog.Trigger /> <AlertDialog.Portal> <AlertDialog.Backdrop /> <AlertDialog.Viewport> <AlertDialog.Popup> <AlertDialog.Title /> <AlertDialog.Description /> <AlertDialog.Close>Close</AlertDialog.Close> </AlertDialog.Popup> </AlertDialog.Viewport> </AlertDialog.Portal> </AlertDialog.Root> Imperative Closing In addition to the declarative AlertDialog.Close component, you can close the dialog imperatively using the actionsRef prop on the AlertDialog.Root component [1][4]. The actionsRef object provides a close method [1][2]: const actionsRef = React.useRef(null); // Close the dialog imperatively actionsRef.current?.close; <AlertDialog.Root actionsRef={actionsRef}>... </AlertDialog.Root> When using the AlertDialog.createHandle method, you can also access a close method on the handle object, which is useful for controlling the dialog from external triggers [1].
Citations:
- 1: https://base-ui.com/react/components/alert-dialog
- 2: https://mui-base-ui.mintlify.app/api/alert-dialog
- 3: https://base-ui.com/react/components/alert-dialog.md
- 4: https://github.com/mui/base-ui/blob/d81ec002/packages/react/src/alert-dialog/root/AlertDialogRoot.tsx
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package versions =="
if [ -f package.json ]; then
node -e 'const p=require("./package.json"); console.log(JSON.stringify({workspaces:p.workspaces, devDependencies:p.devDependencies, dependencies:p.dependencies}, null, 2))' | rg -n "`@base-ui/react`|bun|react" -C 2 || true
fi
echo "== lock/package manager files =="
git ls-files '*lock*' '.bun*' 'pnpm-lock.yaml' 'yarn.lock' package.json package-lock.json 2>/dev/null | head -100
echo "== inspect consumers around dismiss logic =="
for f in \
packages/ui/settings/components/KnowledgeFileItem.tsx \
packages/ui/settings/components/ProviderRateLimitConfig.tsx \
packages/ui/settings/components/AcpSettings.tsx \
packages/ui/settings/components/MemoryManagerPanel.tsx \
packages/ui/settings/components/DataSettings.tsx \
packages/ui/src/components/ui/MessageDialog.tsx \
packages/ui/settings/components/BuiltinKnowledgeSettings.tsx \
packages/ui/settings/components/skills/SkillInstallDialog.tsx \
packages/ui/settings/components/prompt/SystemPromptSettingsSection.tsx \
packages/ui/settings/components/prompt/CustomPromptSettingsSection.tsx
do
if [ -f "$f" ]; then
echo "--- $f ---"
rg -n -A 20 -B 15 "AlertDialogAction|AlertDialogClose|AlertDialogCancel|AlertDialog|open|isOpen|handleClose|setOpen|confirm|delete|disable" "$f" || true
fi
done
echo "== any explicit root actionsRef usage =="
rg -n "actionsRef|openState|setOpen|set\s*=[" packages/ui -g '*.tsx' -g '*.ts' | rg -n "Alert.*Dialog|AlertDialog|alert-dialog|openState|isOpen|setOpen|actionsRef" | head -200Repository: dvaJi/argos
Length of output: 50368
🌐 Web query:
@base-ui/react alert-dialog version 1.0.0 AlertDialog.Close render prop source
💡 Result:
In @base-ui/react, the AlertDialog.Close component supports a render prop, which allows you to override the default HTML element (a button) or compose the component with custom React elements [1][2]. The render prop accepts either a ReactElement or a function [1][2]. When using a function, it provides access to the component's internal props and state, giving you complete control over how the component is rendered [2]. For implementation, Base UI components utilize an internal hook called useRender [3]. This hook manages the merging of external props with internal behavior, ensuring that event handlers are correctly merged, className and style properties are combined, and other props are applied to the underlying DOM node [2][3]. To use the render prop for AlertDialog.Close, you can provide it directly to the component: <AlertDialog.Close render={(props) => ( <button {...props} className="custom-close-button"> Close
Citations:
- 1: https://base-ui.com/react/components/alert-dialog
- 2: https://base-ui.com/react/handbook/composition
- 3: https://base-ui.com/react/utils/use-render
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate lock entries for base-ui =="
rg -n '"(`@base-ui/react`|base-ui)"|`@base-ui/react`' bun.lock package.json -S | head -100 || true
echo "== dependency files =="
git ls-files | rg '(^|/)(package\.json|bun\.lock|tsconfig|.*config\..*)$' | head -200Repository: dvaJi/argos
Length of output: 2587
Keep confirm actions linked to the close primitive.
AlertDialogCancel uses AlertDialogPrimitive.Close.render, while AlertDialogAction calls Button directly. For controlled AlertDialog open={...}/onOpenChange={...} dialogs, use the same Close composition or update every consumer to close the dialog before handling confirmation.
🤖 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 `@packages/ui/shadcn/components/ui/alert-dialog.tsx` around lines 125 - 127,
Update AlertDialogAction to compose through AlertDialogPrimitive.Close.render,
matching AlertDialogCancel, while preserving its existing Button styling,
data-slot, className handling, and props. Ensure confirmation actions notify the
alert dialog close primitive so controlled open/onOpenChange state is updated
before or alongside the action handler.
| function Slider({ className, defaultValue, value, min = 0, max = 100, ...props }: SliderPrimitive.Root.Props) { | ||
| const _values = Array.isArray(value) ? value : Array.isArray(defaultValue) ? defaultValue : [min, max]; | ||
|
|
||
| return ( | ||
| <SliderPrimitive.Root | ||
| className={cn("data-horizontal:w-full data-vertical:h-full", className)} | ||
| data-slot="slider" | ||
| defaultValue={defaultValue} | ||
| value={value} | ||
| min={min} | ||
| max={max} | ||
| className={cn( | ||
| "relative flex w-full touch-none items-center select-none data-disabled:opacity-50 data-vertical:h-full data-vertical:min-h-40 data-vertical:w-auto data-vertical:flex-col", | ||
| className, | ||
| )} | ||
| thumbAlignment="edge" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Base UI React Slider Root thumbAlignment prop
💡 Result:
The thumbAlignment prop on the Base UI React Slider.Root component determines how thumbs are aligned relative to the Slider.Control element when a thumb's value is at its minimum or maximum [1][2][3]. This prop accepts the following values [1][2]: - center (default): The center of the thumb is aligned with the edge of the control [1][2]. - edge: The thumb is inset within the control, ensuring its edge is aligned with the edge of the control [1][2]. This prevents the thumb from overflowing the control's bounds [1][2]. - edge-client-only: Functions identically to "edge" but defers rendering until after React hydration on the client, which can help reduce the initial bundle size [1][2]. This functionality was introduced to enable inset positioning for slider thumbs [4]. When using "edge" or "edge-client-only", the component effectively enters an inset mode [5], often requiring adjustments such as increasing the track height to match the thumb size to achieve a desired visual effect [4].
Citations:
- 1: https://base-ui.com/react/components/slider.md
- 2: https://base-ui.com/react/components/slider
- 3: https://cdn.jsdelivr.net/npm/@base-ui/react@1.3.0/esm/slider/root/SliderRoot.d.ts
- 4: [slider] Add
thumbAlignmentprop mui/base-ui#2540 - 5: https://deepwiki.com/mui/base-ui/3.3-slider-component
Handle scalar value and defaultValue.
Use the resolved value to select between array and single-number thumbs; the current Array.isArray check for a number falls back to [min, max] and renders two thumbs instead of one. thumbAlignment is available on Slider.Root.
🤖 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 `@packages/ui/shadcn/components/ui/slider.tsx` around lines 5 - 16, Update
Slider to resolve scalar and array value/defaultValue inputs into the correct
thumb values, preserving a single-number input as one thumb instead of falling
back to [min, max]. Use the resolved value when configuring SliderPrimitive.Root
while retaining the existing min/max defaults and thumbAlignment behavior.
…improved rendering of assistant message activities - Introduced MessageTurnFold component to handle the rendering of assistant message activities in a collapsible format. - Removed MessageBlockActivityGroup component and its associated logic. - Updated MessageItemAssistant to utilize MessageTurnFold instead of the deprecated MessageBlockActivityGroup. - Refactored messageActivityGroups to accommodate the new turn-fold structure, including changes to how activity blocks are grouped and rendered. - Enhanced MessageBlockThink to improve performance and reduce unnecessary re-renders. - Added a shared highlighter for code blocks in markdown rendering.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/ui/src/components/message/messageActivityGroups.ts (1)
109-120: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMake the plan dedup key explicit instead of routing through
stableId.For plan blocks,
stableIdexists only to bypass the!stableIdearly return at line 110; line 114 then discards it. A plan block that carries a realidstill collapses into the single"plan"key, so two distinct identified plans would merge. Handle plan blocks in one branch to state that intent directly.♻️ Proposed refactor
for (const block of blocks) { - const stableId = block.id ?? block.tool_call?.id ?? (block.type === "plan" ? "__plan__" : undefined); - if (!stableId) { - result.push(block); - continue; - } - const dedupKey = block.type === "plan" ? "plan" : `${block.type}:${stableId}`; + let dedupKey: string; + if (block.type === "plan") { + dedupKey = "plan"; + } else { + const stableId = block.id ?? block.tool_call?.id; + if (!stableId) { + result.push(block); + continue; + } + dedupKey = `${block.type}:${stableId}`; + } const existing = keyToIndex.get(dedupKey);🤖 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 `@packages/ui/src/components/message/messageActivityGroups.ts` around lines 109 - 120, Update the deduplication flow around stableId and dedupKey so plan blocks are handled explicitly without deriving or discarding a synthetic stableId. Ensure identified non-plan blocks continue using their type-and-ID key, while plan blocks use the single plan key only when that is the intended behavior and do not merge distinct identified plans.
🤖 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 `@packages/ui/src/components/message/MessageBlockThink.tsx`:
- Around line 64-81: Update getThinkCollapseSetting and setThinkCollapseSetting
so an in-flight getSetting("think_collapse") result cannot overwrite a value
written by a newer toggle. Apply the fetched value only while thinkCollapseCache
remains null, or invalidate the pending read when setThinkCollapseSetting writes
a value; preserve retry behavior for failed requests and ensure the mounted
block receives the latest toggle value.
In `@packages/ui/src/components/message/MessageTurnFold.tsx`:
- Around line 192-198: Update toggleExpanded so the setIsExpanded updater only
computes and returns the next boolean state; move onToggleCollapse invocation
into the event handler using the derived next value, preserving the existing
callback semantics while keeping the state updater pure.
---
Nitpick comments:
In `@packages/ui/src/components/message/messageActivityGroups.ts`:
- Around line 109-120: Update the deduplication flow around stableId and
dedupKey so plan blocks are handled explicitly without deriving or discarding a
synthetic stableId. Ensure identified non-plan blocks continue using their
type-and-ID key, while plan blocks use the single plan key only when that is the
intended behavior and do not merge distinct identified plans.
🪄 Autofix
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 Plus
Run ID: e3696122-ccdd-478d-b2ab-03a8ffde8625
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
opencode.jsonpackages/ui/package.jsonpackages/ui/src/components/chat/MessageList.tsxpackages/ui/src/components/markdown/CodeBlock.tsxpackages/ui/src/components/markdown/MarkdownRenderer.tsxpackages/ui/src/components/markdown/code-highlight.csspackages/ui/src/components/markdown/highlight.tspackages/ui/src/components/message/MessageBlockAction.tsxpackages/ui/src/components/message/MessageBlockActivityGroup.tsxpackages/ui/src/components/message/MessageBlockThink.tsxpackages/ui/src/components/message/MessageItemAssistant.tsxpackages/ui/src/components/message/MessageTurnFold.tsxpackages/ui/src/components/message/messageActivityGroups.ts
💤 Files with no reviewable changes (1)
- packages/ui/src/components/message/MessageBlockActivityGroup.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/ui/src/components/message/MessageBlockAction.tsx
- packages/ui/src/components/chat/MessageList.tsx
… chat status bar grouping, and refine message handling in chat page
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -22,6 +23,7 @@ interface ChatInputToolbarProps { | |||
|
|
|||
| const ChatInputToolbar: FC<ChatInputToolbarProps> = ({ | |||
There was a problem hiding this comment.
React Doctor · react-doctor/no-many-boolean-props (warning)
Component "ChatInputToolbar" takes 7 on/off props (isGenerating, isCancelling, hasInput…), which is hard to combine & test. Split it into smaller components or named variants.
Fix → Split boolean-heavy APIs into smaller components or named variants so combinations stay testable.
| @@ -70,10 +72,10 @@ const ChatInputToolbar: FC<ChatInputToolbarProps> = ({ | |||
| }, [isGenerating, hasActiveInput]); | |||
|
|
|||
| const primaryTooltip = useMemo(() => { | |||
There was a problem hiding this comment.
React Doctor · react-doctor/react-compiler-no-manual-memoization (warning)
React Compiler can cache this value automatically. Verify that removing useMemo preserves behavior before simplifying it.
Fix → Profile compiler-managed code and remove useMemo, useCallback, or memo only when the manual cache no longer carries behavioral or performance intent.
| .filter((group) => group.models.length > 0); | ||
| }, [modelSearchKeyword, modelGroups]); | ||
|
|
||
| const modelDisplaySections = useMemo<ModelDisplaySection[]>(() => { |
There was a problem hiding this comment.
React Doctor · react-doctor/react-compiler-no-manual-memoization (warning)
React Compiler can cache this value automatically. Verify that removing useMemo preserves behavior before simplifying it.
Fix → Profile compiler-managed code and remove useMemo, useCallback, or memo only when the manual cache no longer carries behavioral or performance intent.
|
|
||
| const resolveCaptureParentId = (messageId: string, parentId?: string): string | undefined => { | ||
| const messageItems = displayMessages; | ||
| const resolveCaptureParentId = useCallback((messageId: string, parentId?: string): string | undefined => { |
There was a problem hiding this comment.
React Doctor · react-doctor/react-compiler-no-manual-memoization (warning)
React Compiler can cache this function automatically. Verify that removing useCallback preserves behavior before simplifying it.
Fix → Profile compiler-managed code and remove useMemo, useCallback, or memo only when the manual cache no longer carries behavioral or performance intent.
| const resolvedParentId = resolveCaptureParentId(messageId, parentId); | ||
| await captureMessage({ messageId, parentId: resolvedParentId, fromTop, modelInfo }); | ||
| }; | ||
| const handleCopyImage = useCallback( |
There was a problem hiding this comment.
React Doctor · react-doctor/react-compiler-no-manual-memoization (warning)
React Compiler can cache this function automatically. Verify that removing useCallback preserves behavior before simplifying it.
Fix → Profile compiler-managed code and remove useMemo, useCallback, or memo only when the manual cache no longer carries behavioral or performance intent.
| ); | ||
| }; | ||
|
|
||
| export const MessageTurnFold = memo(MessageTurnFoldBase); |
There was a problem hiding this comment.
React Doctor · react-doctor/react-compiler-no-manual-memoization (warning)
React Compiler can cache this component output automatically. Verify that removing memo() preserves behavior before simplifying it.
Fix → Profile compiler-managed code and remove useMemo, useCallback, or memo only when the manual cache no longer carries behavioral or performance intent.
| }, [isReadOnlySession, isGenerating, isCancelling, sessionId, chatClient]); | ||
|
|
||
| useEffect(() => { | ||
| if (!isGenerating) setIsCancelling(false); |
There was a problem hiding this comment.
React Doctor · react-doctor/no-adjust-state-on-prop-change (warning)
This effect adjusts state after a prop changes, so users briefly see the stale value.
Fix → Remove the adjustment effect by deriving values during render, resetting the component with a key, or updating related state in the event that changes the prop. Avoid tracking the previous prop in more state, which preserves the duplication. See https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes
| }, [isReadOnlySession, isGenerating, isCancelling, sessionId, chatClient]); | ||
|
|
||
| useEffect(() => { | ||
| if (!isGenerating) setIsCancelling(false); |
There was a problem hiding this comment.
React Doctor · react-hooks-js/set-state-in-effect (warning)
This synchronous effect update causes an extra render: Calling setState synchronously within an effect can trigger cascading renders. Prefer deriving or initializing the value before render. If the effect must read a browser API after mount, treat this as advisory or suppress it with // react-doctor-disable-next-line react-hooks-js/set-state-in-effect.
Fix → Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. In general, the body of an effect should do one or both of the following:
- Update external systems with the latest state from React.
- Subscribe for updates from some external system, calling setState in a callback function when external state changes.
Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. (https://react.dev/learn/you-might-not-need-an-effect).
|
|
||
| useEffect(() => { | ||
| // A session change must not carry over the previous session's cancelling state. | ||
| setIsCancelling(false); |
There was a problem hiding this comment.
React Doctor · react-doctor/no-adjust-state-on-prop-change (warning)
This effect adjusts state after a prop changes, so users briefly see the stale value.
Fix → Remove the adjustment effect by deriving values during render, resetting the component with a key, or updating related state in the event that changes the prop. Avoid tracking the previous prop in more state, which preserves the duplication. See https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes
|
|
||
| useEffect(() => { | ||
| // A session change must not carry over the previous session's cancelling state. | ||
| setIsCancelling(false); |
There was a problem hiding this comment.
React Doctor · react-hooks-js/set-state-in-effect (warning)
This synchronous effect update causes an extra render: Calling setState synchronously within an effect can trigger cascading renders. Prefer deriving or initializing the value before render. If the effect must read a browser API after mount, treat this as advisory or suppress it with // react-doctor-disable-next-line react-hooks-js/set-state-in-effect.
Fix → Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. In general, the body of an effect should do one or both of the following:
- Update external systems with the latest state from React.
- Subscribe for updates from some external system, calling setState in a callback function when external state changes.
Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. (https://react.dev/learn/you-might-not-need-an-effect).
build-check has been red on master: @argos/ui build failed. Two root causes, both surfacing after the deps update (#42) once format:check was fixed.
Verified: @argos/ui build green (~11s), full desktop build 3/3 tasks, lint PASS.
Summary by CodeRabbit