refactor(frontend): agent-path playground chrome off antd - #5873
refactor(frontend): agent-path playground chrome off antd#5873ardaerzin wants to merge 6 commits into
Conversation
|
@coderabbitai review |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe Playground replaces selected Ant Design controls with Agenta UI primitives. Single-agent layouts now use a controlled, resizable ChangesPlayground UI migration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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 |
3ec8109 to
baa1f8e
Compare
52698fd to
8a4399f
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
web/oss/src/components/Playground/Components/MainLayout/index.tsx (2)
273-275: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider persisting the agent pane width instead of holding it per mount.
agentPaneSizelives in component state, so a remount or reload resets the pane to 440px. The comparable chat panel stores its width in a Jotai atom (rightPanelWidthAtominweb/oss/src/components/AgentChatSlice/components/RightPanel/RightPanelSplit.tsx). The repository guidelines ask foratomWithStoragefor persisted UI preferences, with keys prefixed byagenta:.As per coding guidelines: "Use
atomWithStoragefor persisted preferences, UI state, recently used items, and form drafts; prefix storage keys withagenta:."Source: Coding guidelines
408-410: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the duplicated config placeholder and drop the
--ag-c-*literal.Lines 408-410 repeat the placeholder markup from lines 558-560, including
var(--ag-c-FFFFFF). The guidelines forbid--ag-c-*literals. Extract one small placeholder component and use a semantic surface token so both branches stay in sync.As per coding guidelines: "Consume theme colors through Ant Design semantic tokens, Tailwind color utilities, or supported
var(--ag-color*)variables; do not use raw hex colors or--ag-c-*literals."Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 35c8a95f-5991-47a7-931b-fbeebd1fa9d5
📒 Files selected for processing (15)
web/oss/src/components/Playground/Components/AgentCommitNotice.tsxweb/oss/src/components/Playground/Components/AgentRevisionSelector/index.tsxweb/oss/src/components/Playground/Components/AlwaysAllowedNotice.tsxweb/oss/src/components/Playground/Components/MainLayout/index.tsxweb/oss/src/components/Playground/Components/Modals/CommitVariantChangesModal/assets/CommitVariantChangesButton/index.tsxweb/oss/src/components/Playground/Components/Modals/CommitVariantChangesModal/assets/types.d.tsweb/oss/src/components/Playground/Components/Modals/DeployVariantModal/assets/DeployVariantButton/types.d.tsweb/oss/src/components/Playground/Components/Modals/DeployVariantModal/assets/DeployVariantModalContent/index.tsxweb/oss/src/components/Playground/Components/Modals/DeployVariantModal/types.d.tsweb/oss/src/components/Playground/Components/Modals/RefinePromptModal/assets/InstructionsPanel.tsxweb/oss/src/components/Playground/Components/PlaygroundGenerations/assets/GatewayToolExecuteButton.tsxweb/oss/src/components/Playground/Components/PlaygroundHeader/index.tsxweb/oss/src/components/Playground/Components/PlaygroundVariantConfig/assets/PlaygroundVariantConfigHeader.tsxweb/oss/src/components/Playground/Components/PlaygroundVariantConfig/index.tsxweb/oss/src/components/Playground/Components/ProviderKeyNotice.tsx
| <SplitPane | ||
| paneSide="start" | ||
| paneSize={configCollapsed ? 0 : agentPaneSize} | ||
| paneMin={300} | ||
| paneMax={440} | ||
| fillMin={420} | ||
| animate={animateSplit} | ||
| barHidden={configCollapsed} | ||
| // Controlled width: the drag must write through per tick, or the pane | ||
| // only snaps at pointer-up. | ||
| onResize={(size) => setAgentPaneSize(size)} | ||
| onResizeEnd={(size) => setAgentPaneSize(size)} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate the SplitPane implementation and inspect its props and clamping behavior.
fd -i 'SplitPane*' --type f
fd -i 'SplitPane*' --type f --exec ast-grep outline {} --items all
rg -nP -C4 '\b(paneMin|paneMax|barHidden|resizable|fillMin)\b' --glob '**/SplitPane*'Repository: Agenta-AI/agenta
Length of output: 1358
🏁 Script executed:
#!/bin/bash
set -e
echo "== Locate relevant files =="
fd -i 'index\.tsx|MainLayout' --type f
fd -i 'RightPanelSplit\.tsx|SplitPanelLayout\.tsx' --type f
echo
echo "== SplitPane references in MainLayout =="
rg -n "SplitPane|paneMin|paneMax|barHidden|resizable|agentPaneSize|configCollapsed" web/oss/src/components/Playground/Components/MainLayout/index.tsx || true
echo
echo "== SplitPanelLayout outline =="
ast-grep outline web/packages/agenta-ui/src/components/presentational/layout/SplitPanelLayout.tsx || true
echo
echo "== SplitPanelLayout implementation =="
cat -n web/packages/agenta-ui/src/components/presentational/layout/SplitPanelLayout.tsx
echo
echo "== RightPanelSplit relevant code =="
cat -n web/oss/src/components/AgentChatSlice/components/RightPanel/RightPanelSplit.tsx | sed -n '1,220p'
echo
echo "== package exports for `@agenta/ui/ui` =="
fd -i 'package.json|ui\.ts|index\.(ts|tsx)$' web/packages/agenta-ui --type f --print0 | xargs -0 rg -n "SplitPane|presentational/layout|components/presentational/|ui:" || trueRepository: Agenta-AI/agenta
Length of output: 32830
🏁 Script executed:
#!/bin/bash
set -e
echo "== UI package dependency scripts =="
cat -n web/packages/agenta-ui/package.json | sed -n '1,120p'
echo
echo "== SplitPane implementation =="
cat -n web/packages/agenta-ui/src/components/ui/split-pane.tsx
echo
echo "== MainLayout related lines =="
cat -n web/oss/src/components/Playground/Components/MainLayout/index.tsx | sed -n '350,385p'Repository: Agenta-AI/agenta
Length of output: 20864
Disable division resizing and size bounds while the config pane is collapsed.
paneMin={300} keeps 0 outside the SplitPane clamp when paneSize flips from agentPaneSize to 0, so the 0-width collapsed state will be clamped back up. barHidden only hides the divider from render; it still leaves the layout gap and keyboard/pointer resize behavior gated by resizable, which defaults to true. Gate both the collapse and the resizable bounds with configCollapsed.
| {isLoading ? ( | ||
| <tr> | ||
| <td colSpan={3} className="px-3 py-3"> | ||
| <Skeleton className="h-16 w-full" /> | ||
| </td> | ||
| </tr> | ||
| ) : ( | ||
| (data as DeployModalEnvRow[]).map((record) => ( |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
content="web/oss/src/components/Playground/Components/Modals/DeployVariantModal/assets/DeployVariantModalContent/index.tsx"
atom="web/oss/src/components/Playground/Components/Modals/DeployVariantModal/assets/DeployVariantModalContent/tableDataAtom.ts"
rg -n -C4 '\bDeployVariantModalContent\b|isLoading=' \
web/oss/src/components/Playground/Components/Modals/DeployVariantModal
sed -n '1,160p' "$atom"
sed -n '1,110p' "$content"Repository: Agenta-AI/agenta
Length of output: 11645
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
api="web/oss/src/components/Playground/Components/Modals/DeployVariantModal/assets/DeployVariantButton/index.tsx"
index="web/oss/src/components/Playground/Components/Modals/DeployVariantModal/index.tsx"
env_atoms="web/oss/src/oss/state/environment/appEnvironmentAtoms.ts"
sed -n '1,180p' "$api"
printf '\n--- modal index loading-related context ---\n'
rg -n -C5 "propsIsLoading|isLoading|onOpen|open|isEnvironmentsLoading|environment" "$index" "$env_atoms"Repository: Agenta-AI/agenta
Length of output: 11328
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- appEnvironmentAtoms files ---'
fd -a 'appEnvironmentAtoms\.ts$|appEnvironmentAtoms\.tsx' web/oss/src | sed 's#^\./##'
file="$(fd 'appEnvironmentAtoms\.ts$' web/oss/src | head -n 1)"
if [ -n "$file" ]; then
sed -n '1,220p' "$file"
else
printf 'No appEnvironmentAtoms.ts file found.\n'
fi
printf '\n--- loading-state and appEnvironmentsLoadableAtom users ---\n'
rg -n -C4 "appEnvironmentsLoadableAtom|useAppEnvironments|isLoading:|propsIsLoading|publishMutationAtom" web/oss/src/components web/oss/src/oss/state/ || trueRepository: Agenta-AI/agenta
Length of output: 50373
Align the modal loading state with the environment rows.
publishMutationAtom.isPending controls the Skeleton, while deployModalEnvironmentsTableAtom reads appEnvironmentsLoadableAtom. The Skeleton row is skipped while the button may still be disabled and the table rows are placeholders, so users can select a deployment target while the environments are still loading. Use the environment loadable state for both Skeleton and selectable rows, or pass that shared state through DeployVariantModal.
| onKeyDown={(e) => { | ||
| // Enter sends; Shift+Enter breaks the line (the Sender's contract). | ||
| if (e.key === "Enter" && !e.shiftKey) { | ||
| e.preventDefault() | ||
| handleSubmit(inputValue) | ||
| } | ||
| }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Guard the Enter handler against IME composition.
The removed Sender suppressed submission while an input method editor composed text. The new handler submits on any Enter without shiftKey. A user who types Chinese, Japanese, or Korean confirms a composition candidate with Enter, so the panel submits an incomplete guideline and clears the textarea.
🐛 Proposed fix
onKeyDown={(e) => {
// Enter sends; Shift+Enter breaks the line (the Sender's contract).
- if (e.key === "Enter" && !e.shiftKey) {
+ if (e.key === "Enter" && !e.shiftKey && !e.nativeEvent.isComposing) {
e.preventDefault()
handleSubmit(inputValue)
}
}}📝 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.
| onKeyDown={(e) => { | |
| // Enter sends; Shift+Enter breaks the line (the Sender's contract). | |
| if (e.key === "Enter" && !e.shiftKey) { | |
| e.preventDefault() | |
| handleSubmit(inputValue) | |
| } | |
| }} | |
| onKeyDown={(e) => { | |
| // Enter sends; Shift+Enter breaks the line (the Sender's contract). | |
| if (e.key === "Enter" && !e.shiftKey && !e.nativeEvent.isComposing) { | |
| e.preventDefault() | |
| handleSubmit(inputValue) | |
| } | |
| }} |
| <DropdownMenu> | ||
| <DropdownMenuTrigger asChild> | ||
| <Button | ||
| variant="outline" | ||
| size="icon-sm" | ||
| aria-label="Tool call options" | ||
| className="-ml-px rounded-l-none" | ||
| > | ||
| <CaretDown size={12} /> | ||
| </Button> | ||
| </DropdownMenuTrigger> | ||
| <DropdownMenuContent align="end"> | ||
| <DropdownMenuItem onSelect={() => handleExecute(p, false)}> | ||
| Call tool | ||
| </DropdownMenuItem> | ||
| </DropdownMenuContent> | ||
| </DropdownMenu> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Block the dropdown action while a tool call is in flight.
Dropdown.Button disabled both halves of the control through its loading prop. The new caret Button and the DropdownMenuItem have no loading gate. A user can select "Call tool" while the primary call is still running. Two concurrent executeToolCall requests then race: executingId holds one value, the first finally clears the loading indicator, and both responses call onUpdateToolResponse for the same tool call.
🐛 Proposed fix
+ {(() => {
+ const isExecuting = executingId === (p.callId || p.name || "default")
+ return (
<DropdownMenu>
<DropdownMenuTrigger asChild>
<Button
variant="outline"
size="icon-sm"
aria-label="Tool call options"
+ disabled={isExecuting}
className="-ml-px rounded-l-none"
>
<CaretDown size={12} />
</Button>
</DropdownMenuTrigger>
<DropdownMenuContent align="end">
- <DropdownMenuItem onSelect={() => handleExecute(p, false)}>
+ <DropdownMenuItem
+ disabled={isExecuting}
+ onSelect={() => handleExecute(p, false)}
+ >
Call tool
</DropdownMenuItem>
</DropdownMenuContent>
</DropdownMenu>
+ )
+ })()}A cleaner form is to compute isExecuting once per payload above the returned markup, or to extract a small per-payload component.
📝 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.
| <DropdownMenu> | |
| <DropdownMenuTrigger asChild> | |
| <Button | |
| variant="outline" | |
| size="icon-sm" | |
| aria-label="Tool call options" | |
| className="-ml-px rounded-l-none" | |
| > | |
| <CaretDown size={12} /> | |
| </Button> | |
| </DropdownMenuTrigger> | |
| <DropdownMenuContent align="end"> | |
| <DropdownMenuItem onSelect={() => handleExecute(p, false)}> | |
| Call tool | |
| </DropdownMenuItem> | |
| </DropdownMenuContent> | |
| </DropdownMenu> | |
| {(() => { | |
| const isExecuting = executingId === (p.callId || p.name || "default") | |
| return ( | |
| <DropdownMenu> | |
| <DropdownMenuTrigger asChild> | |
| <Button | |
| variant="outline" | |
| size="icon-sm" | |
| aria-label="Tool call options" | |
| disabled={isExecuting} | |
| className="-ml-px rounded-l-none" | |
| > | |
| <CaretDown size={12} /> | |
| </Button> | |
| </DropdownMenuTrigger> | |
| <DropdownMenuContent align="end"> | |
| <DropdownMenuItem | |
| disabled={isExecuting} | |
| onSelect={() => handleExecute(p, false)} | |
| > | |
| Call tool | |
| </DropdownMenuItem> | |
| </DropdownMenuContent> | |
| </DropdownMenu> | |
| ) | |
| })()} |
| <SimpleTooltip title="Agent"> | ||
| <span className="flex h-6 w-6 shrink-0 items-center justify-center rounded-md bg-[var(--ant-color-fill-secondary)] text-[var(--ag-c-13C2C2)]"> | ||
| <Robot size={15} weight="fill" /> | ||
| </span> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use a supported theme color token.
Line 674 introduces var(--ag-c-13C2C2). Replace this literal with a semantic token or Tailwind color utility. Verify the replacement in light and dark appearances.
As per coding guidelines, use supported theme colors and do not use --ag-c-* literals. Based on learnings, validate changed color tokens in both appearances.
Sources: Coding guidelines, Learnings
| <AgentNameInline | ||
| workflowId={renameWorkflowId} | ||
| name={agentName} | ||
| onRenamed={setDisplayAgentName} | ||
| /> | ||
| ) : ( | ||
| <Typography className="truncate whitespace-nowrap text-[16px] leading-[18px] font-[600]"> | ||
| <span className="truncate whitespace-nowrap text-[16px] font-[600] leading-[18px] text-colorText"> | ||
| {agentName || "Agent"} | ||
| </Typography> | ||
| </span> | ||
| )} | ||
| {rootEntityId ? ( | ||
| <> | ||
| <Divider orientation="vertical" className="!mx-1 h-5" /> | ||
| <span | ||
| aria-hidden | ||
| className="mx-1 h-5 w-px shrink-0 bg-colorBorderSecondary" | ||
| /> | ||
| <AgentRevisionSelector variantId={rootEntityId} /> | ||
| </> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
target="web/oss/src/components/EntityIdentity/AgentNameInline.tsx"
sed -n '1,110p' "$target"
rg -n -C2 'from ["'\'']antd|<Typography\b' "$target"Repository: Agenta-AI/agenta
Length of output: 3747
Replace the Ant Design imports in web/oss/src/components/EntityIdentity/AgentNameInline.tsx.
AgentNameInline imports {Input, Typography} from "antd"; and renders Typography, so replacing PlaygroundHeader with frontend primitives leaves this component with an Ant Design dependency.
The Skeleton was driven by the publish mutation, but the rows come from the environments query. Until that query settled the table rendered PLACEHOLDER_ROWS (dev/stage/prod) as pickable radio rows — and those names are not the real environment slugs (development/staging/production), so an early pick submitted a bogus environmentSlug. The table selector now reports whether its rows are placeholders, and the content shows the skeleton until real environments arrive.
…-flight flag Only the primary button was gated on executingId, so the dropdown's "Call tool" could start a second execution while the first was still running: executingId holds a single value, the first finally clears the indicator, and both responses write back for the same tool call. The busy condition is derived once now and gates the menu item too.
…osition The antd Sender this textarea replaced suppressed submission while an input method editor was composing. Without the guard, a CJK user confirming a candidate with Enter submits a half-written guideline and loses the draft. Matches the RichChatInput SubmitPlugin, which reads event.isComposing.
The playground header is antd-free but still pulled antd in through AgentNameInline. Ports it the way the rest of the header went: the shared @agenta/ui Input (status -> aria-invalid, onPressEnter -> an Enter branch in onKeyDown that the IME guard skips) and a plain span with text-colorText in place of Typography. Same markup, same geometry.
bg-[var(--ant-color-fill-secondary)] also breaks on the mobile app, which emits no --ant-* variables. bg-colorFillSecondary is the app's own token (and what PlaygroundLoadingShell already uses for the same badge); text-cyan-6 is the antd cyan/6 the --ag-c-13C2C2 legacy literal resolved to in both themes, so the rendered colour is unchanged.
1 participant
The agent-path playground chrome drops antd so the same chrome can render inside
/m.Not run in a browser — static gates only (
pnpm lint-fix24/24,tsc --noEmitcleanfor
@agenta/shared,ui,entities,entity-ui,settings-ui,oss,ee,mobile).Stacked on
mobile/chat-and-shell; review only this lane's diff.