-
Notifications
You must be signed in to change notification settings - Fork 46.2k
fix(frontend): use DS Dialog on old builder #11643
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
β Deploy Preview for auto-gpt-docs-dev canceled.
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughTwo legacy dialog components in the build interface were refactored to use a unified Dialog API. ExpandableOutputDialog now supports an enhanced rendering mode controlled by a feature flag, while RunnerInputUI migrated to the new Dialog structure with simplified layout and controlled open/close handling. Changes
Estimated code review effortπ― 3 (Moderate) | β±οΈ ~20 minutes
Poem
Pre-merge checks and finishing touchesβ Failed checks (1 warning)
β Passed checks (2 passed)
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 |
β Deploy Preview for auto-gpt-docs canceled.
|
|
Here's the code health analysis summary for commits Analysis Summary
|
|
@coderabbitai review |
β Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and canβt be posted inline due to platform limitations.
β οΈ Outside diff range comments (1)
autogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/ExpandableOutputDialog.tsx (1)
10-17: Use Phosphor Icons instead of lucide-react.Per coding guidelines, only Phosphor Icons should be used in the frontend. Replace
ClipboardandMaximize2from lucide-react with their Phosphor equivalents.π Proposed fix
-import { Clipboard, Maximize2 } from "lucide-react"; +import { ArrowsOut, ClipboardText } from "@phosphor-icons/react";Then update usages:
<Maximize2 size={20} />β<ArrowsOut size={20} /><Clipboard size={14} />β<ClipboardText size={14} /><Clipboard size={16} />β<ClipboardText size={16} />Based on learnings, only Phosphor Icons should be used in frontend code.
β»οΈ Duplicate comments (1)
autogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/RunnerInputUI.tsx (1)
67-79: Potential double invocation ofdoClose.Both the
controlled.sethandler and theonCloseprop invokedoClose(). Depending on the Dialog implementation, this may causedoCloseto be called twice when the dialog closes. Consider removing the redundantonClose={doClose}prop since the controlled handler already handles the close logic.π Proposed fix
<Dialog title="Run your agent" controlled={{ isOpen, set: (open) => { if (!open) doClose(); }, }} - onClose={doClose} styling={{ maxWidth: "56rem", width: "90vw", }} >Please verify the Dialog component's implementation to confirm whether both handlers are invoked on close:
#!/bin/bash # Search for Dialog component implementation to understand the onClose/controlled interaction ast-grep --pattern $'function Dialog($_) { $$$ }' # Also search for the Dialog component definition rg -n "onClose|controlled" --type=tsx -A3 -B3 -g '**/Dialog.tsx'
π§Ή Nitpick comments (3)
autogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/ExpandableOutputDialog.tsx (3)
140-151: Same potential doubleonCloseinvocation as RunnerInputUI.The
controlled.sethandler callsonClose()whenopenis false, andonCloseis also passed directly. Consider removing the redundantonCloseprop.π Proposed fix
controlled={{ isOpen, set: (open) => { if (!open) onClose(); }, }} - onClose={onClose} styling={{
100-113: Add error handling for clipboard operation.The
navigator.clipboard.writeText()call can fail (e.g., due to permissions or secure context requirements). Consider adding a.catch()handler to notify the user of failures.π Proposed fix
navigator.clipboard.writeText(formattedData).then(() => { toast({ title: `"${beautifyString(pinName)}" data copied to clipboard!`, duration: 2000, }); - }); + }).catch(() => { + toast({ + title: "Failed to copy to clipboard", + variant: "destructive", + duration: 2000, + }); + });
205-217: Same clipboard error handling concern.The per-item copy operation should also handle clipboard failures for consistency.
π Proposed fix
navigator.clipboard .writeText(itemData) .then(() => { toast({ title: `Item ${index + 1} copied to clipboard!`, duration: 2000, }); + }) + .catch(() => { + toast({ + title: "Failed to copy to clipboard", + variant: "destructive", + duration: 2000, + }); });
π Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
π Files selected for processing (2)
autogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/ExpandableOutputDialog.tsx(2 hunks)autogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/RunnerInputUI.tsx(2 hunks)
π§° Additional context used
π Path-based instructions (8)
autogpt_platform/frontend/**/*.{ts,tsx}
π CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/frontend/**/*.{ts,tsx}: Always run pnpm install before frontend development, then use pnpm dev to start development server on port 3000
For frontend code formatting and linting, always run pnpm formatIf adding protected frontend routes, update
frontend/lib/supabase/middleware.ts
autogpt_platform/frontend/**/*.{ts,tsx}: Use generated API hooks from@/app/api/__generated__/endpoints/for data fetching in frontend
Use function declarations (not arrow functions) for components and handlers in frontend
Only use Phosphor Icons in frontend; never use other icon libraries
Never usesrc/components/__legacy__/*or deprecatedBackendAPIin frontend
Files:
autogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/RunnerInputUI.tsxautogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/ExpandableOutputDialog.tsx
autogpt_platform/frontend/**/*.{ts,tsx,json}
π CodeRabbit inference engine (.github/copilot-instructions.md)
Use Node.js 21+ with pnpm package manager for frontend development
Files:
autogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/RunnerInputUI.tsxautogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/ExpandableOutputDialog.tsx
autogpt_platform/frontend/src/**/*.{ts,tsx}
π CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/frontend/src/**/*.{ts,tsx}: Use generated API hooks from @/app/api/generated/endpoints/ (generated via Orval from backend OpenAPI spec). Pattern: use{Method}{Version}{OperationName} (e.g., useGetV2ListLibraryAgents). Regenerate with: pnpm generate:api. Never use deprecated BackendAPI or src/lib/autogpt-server-api/*
Use function declarations for components and handlers (not arrow functions). Only arrow functions for small inline lambdas (map, filter, etc.)
Use PascalCase for components, camelCase with use prefix for hooks
No barrel files or index.ts re-exports in frontend
For frontend render errors, use component. For mutation errors, display with toast notifications. For manual exceptions, use Sentry.captureException()
Default to client components (use client). Use server components only for SEO or extreme TTFB needs. Use React Query for server state via generated hooks. Co-locate UI state in components/hooks
Files:
autogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/RunnerInputUI.tsxautogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/ExpandableOutputDialog.tsx
autogpt_platform/frontend/**/*.{js,ts,jsx,tsx}
π CodeRabbit inference engine (AGENTS.md)
Format frontend code using
pnpm format
Files:
autogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/RunnerInputUI.tsxautogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/ExpandableOutputDialog.tsx
autogpt_platform/frontend/**
π CodeRabbit inference engine (autogpt_platform/CLAUDE.md)
autogpt_platform/frontend/**: Install frontend dependencies usingpnpm iinstead of npm
Generate API client from OpenAPI spec usingpnpm generate:api
Regenerate API client hooks usingpnpm generate:apiwhen OpenAPI spec changes
Files:
autogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/RunnerInputUI.tsxautogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/ExpandableOutputDialog.tsx
autogpt_platform/frontend/src/**/*.tsx
π CodeRabbit inference engine (autogpt_platform/CLAUDE.md)
Use design system components from
src/components/(atoms, molecules, organisms) in frontend
Files:
autogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/RunnerInputUI.tsxautogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/ExpandableOutputDialog.tsx
autogpt_platform/frontend/src/app/**/*.tsx
π CodeRabbit inference engine (autogpt_platform/CLAUDE.md)
Create frontend pages in
src/app/(platform)/feature-name/page.tsxwith correspondingusePageName.tshook and localcomponents/subfolder
Files:
autogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/RunnerInputUI.tsxautogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/ExpandableOutputDialog.tsx
autogpt_platform/frontend/**/*.{ts,tsx,css}
π CodeRabbit inference engine (autogpt_platform/CLAUDE.md)
Use only Tailwind CSS for styling in frontend, with design tokens and Phosphor Icons
Files:
autogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/RunnerInputUI.tsxautogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/ExpandableOutputDialog.tsx
π§ Learnings (3)
π Learning: 2025-11-25T08:49:03.583Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/CLAUDE.md:0-0
Timestamp: 2025-11-25T08:49:03.583Z
Learning: Applies to autogpt_platform/frontend/**/*.{ts,tsx} : Never use `src/components/__legacy__/*` or deprecated `BackendAPI` in frontend
Applied to files:
autogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/RunnerInputUI.tsx
π Learning: 2025-11-25T08:48:33.246Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T08:48:33.246Z
Learning: Applies to autogpt_platform/frontend/src/components/**/*.tsx : Separate frontend component render logic from data/behavior. Structure: ComponentName/ComponentName.tsx + useComponentName.ts + helpers.ts. Small components (3-4 lines) can be inline. Render-only components can be direct files without folders
Applied to files:
autogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/ExpandableOutputDialog.tsx
π Learning: 2025-11-25T08:49:03.583Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: autogpt_platform/CLAUDE.md:0-0
Timestamp: 2025-11-25T08:49:03.583Z
Learning: Applies to autogpt_platform/frontend/src/components/**/*.{ts,tsx} : Separate render logic from data/behavior in components
Applied to files:
autogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/ExpandableOutputDialog.tsx
𧬠Code graph analysis (1)
autogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/RunnerInputUI.tsx (1)
autogpt_platform/frontend/src/app/(platform)/library/agents/[id]/components/OldAgentLibraryView/components/agent-run-draft-view.tsx (1)
AgentRunDraftView(46-724)
π Additional comments (7)
autogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/RunnerInputUI.tsx (3)
1-8: LGTM!Import paths are correct. The Dialog component is properly imported from the design system molecules path, and AgentRunDraftView is imported from the appropriate location.
33-64: LGTM!The callback wrappers properly sequence the run/schedule actions before closing the dialog, and the dependency arrays are correctly specified.
81-93: LGTM!The content structure is clean. The conditional prop passing for
doRun/onRunanddoCreateSchedule/onCreateSchedulecorrectly handles both scenarios where handlers are provided and when they're not.autogpt_platform/frontend/src/app/(platform)/build/components/legacy-builder/ExpandableOutputDialog.tsx (4)
41-98: LGTM!The
outputItemsmemoization logic correctly prepares data for the enhanced renderer with proper fallback toTextRenderer. The metadata extraction from object values is handled defensively.
117-139: LGTM!The title layout with the Enhanced Rendering toggle is well-structured. The Switch has proper accessibility with matching
htmlForandidattributes, and the feature flag gating is correctly applied.
176-242: LGTM!The conditional rendering logic correctly handles all three states (enhanced renderer, standard renderer, and empty data). The layout structure is clean and the ScrollArea provides proper overflow handling for long content.
245-262: LGTM!The footer correctly displays the item count and conditionally shows the "Copy All" button only when enhanced rendering is disabled (since enhanced mode has its own
OutputActions).
Changes ποΈ
Use the Design System
<Dialog />on the old builder, which supports long content scrolling ( the current one does not, causing issues in graphs with many run inputs )...Checklist π
For code changes:
Summary by CodeRabbit
New Features
Improvements
βοΈ Tip: You can customize this high-level summary in your review settings.