From dd3225a5c478b80f20bdce85682f4edf7439b8b5 Mon Sep 17 00:00:00 2001 From: majdyz Date: Wed, 8 Apr 2026 07:43:22 +0000 Subject: [PATCH] fix(frontend/builder): address review comments on chat panel - Validate node existence before connect_nodes in handleApplyAction - Add cleanup guard to session creation effect to prevent state updates after unmount - Extract extractTextFromParts helper to deduplicate text extraction - Remove dead code in ActionItem (applied state was always true) - Remove redundant setTimeout scroll in handleSend (useEffect handles it) - Update test to match simplified ActionItem --- .../BuilderChatPanel/BuilderChatPanel.tsx | 52 +++---------------- .../__tests__/BuilderChatPanel.test.tsx | 7 +-- .../components/BuilderChatPanel/helpers.ts | 9 ++++ .../BuilderChatPanel/useBuilderChatPanel.ts | 21 +++++--- 4 files changed, 30 insertions(+), 59 deletions(-) diff --git a/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/BuilderChatPanel.tsx b/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/BuilderChatPanel.tsx index cd089e84fd..ee19a37b44 100644 --- a/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/BuilderChatPanel.tsx +++ b/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/BuilderChatPanel.tsx @@ -11,7 +11,7 @@ import { } from "@phosphor-icons/react"; import { KeyboardEvent, useEffect, useRef, useState } from "react"; import type { CustomNode } from "../FlowEditor/nodes/CustomNode/CustomNode"; -import { GraphAction } from "./helpers"; +import { GraphAction, extractTextFromParts } from "./helpers"; import { useBuilderChatPanel } from "./useBuilderChatPanel"; interface Props { @@ -32,7 +32,6 @@ export function BuilderChatPanel({ className, isGraphLoaded }: Props) { sessionId, nodes, parsedActions, - handleApplyAction, } = useBuilderChatPanel({ isGraphLoaded }); const [inputValue, setInputValue] = useState(""); @@ -53,9 +52,6 @@ export function BuilderChatPanel({ className, isGraphLoaded }: Props) { if (!text || !canSend) return; setInputValue(""); sendMessage({ text }); - setTimeout(() => { - messagesEndRef.current?.scrollIntoView({ behavior: "smooth" }); - }, 50); } function handleKeyDown(e: KeyboardEvent) { @@ -82,7 +78,6 @@ export function BuilderChatPanel({ className, isGraphLoaded }: Props) { sessionError={sessionError} nodes={nodes} parsedActions={parsedActions} - onApplyAction={handleApplyAction} messagesEndRef={messagesEndRef} /> @@ -136,7 +131,6 @@ interface MessageListProps { sessionError: boolean; nodes: CustomNode[]; parsedActions: GraphAction[]; - onApplyAction: (action: GraphAction) => void; messagesEndRef: React.RefObject; } @@ -146,7 +140,6 @@ function MessageList({ sessionError, nodes, parsedActions, - onApplyAction, messagesEndRef, }: MessageListProps) { return ( @@ -165,12 +158,7 @@ function MessageList({ )} {messages.map((msg) => { - const textParts = msg.parts - .filter( - (p): p is Extract => p.type === "text", - ) - .map((p) => p.text) - .join(""); + const textParts = extractTextFromParts(msg.parts); if (!textParts) return null; @@ -199,14 +187,7 @@ function MessageList({ action.type === "update_node_input" ? `${action.nodeId}:${action.key}` : `${action.source}:${action.sourceHandle}->${action.target}:${action.targetHandle}`; - return ( - onApplyAction(action)} - /> - ); + return ; })} )} @@ -219,22 +200,10 @@ function MessageList({ function ActionItem({ action, nodes, - onApply, }: { action: GraphAction; nodes: CustomNode[]; - onApply: () => void; }) { - // The AI applies changes server-side via edit_agent; the canvas refreshes - // automatically via invalidateQueries. The button starts in the applied state - // to reflect that changes are already live — not pending user confirmation. - const [applied, setApplied] = useState(true); - - function handleApply() { - onApply(); - setApplied(true); - } - const nodeName = (id: string) => nodes.find((n) => n.id === id)?.data.title ?? id; @@ -246,18 +215,9 @@ function ActionItem({ return (
{label} - + + Applied +
); } diff --git a/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/__tests__/BuilderChatPanel.test.tsx b/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/__tests__/BuilderChatPanel.test.tsx index e740b6b99e..b02a00726e 100644 --- a/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/__tests__/BuilderChatPanel.test.tsx +++ b/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/__tests__/BuilderChatPanel.test.tsx @@ -122,7 +122,7 @@ describe("BuilderChatPanel", () => { expect(screen.getByText("Applied")).toBeDefined(); }); - it("shows pre-applied actions as disabled", () => { + it("shows applied badge for actions", () => { const action = { type: "update_node_input" as const, nodeId: "1", @@ -136,10 +136,7 @@ describe("BuilderChatPanel", () => { }), ); render(); - const button = screen.getByRole("button", { - name: "Applied", - }) as HTMLButtonElement; - expect(button.disabled).toBe(true); + expect(screen.getByText("Applied")).toBeDefined(); }); it("calls sendMessage when the user submits a message", () => { diff --git a/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/helpers.ts b/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/helpers.ts index 820f9d8355..431469ca87 100644 --- a/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/helpers.ts +++ b/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/helpers.ts @@ -45,6 +45,15 @@ export function serializeGraphForChat( return parts.join("\n\n"); } +export function extractTextFromParts( + parts: ReadonlyArray<{ type: string; text?: string }>, +): string { + return parts + .filter((p): p is { type: "text"; text: string } => p.type === "text") + .map((p) => p.text) + .join(""); +} + export function parseGraphActions(text: string): GraphAction[] { const actions: GraphAction[] = []; const jsonBlockRegex = /```(?:json)?\s*\n?([\s\S]*?)\n?```/g; diff --git a/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/useBuilderChatPanel.ts b/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/useBuilderChatPanel.ts index d70975ddb1..880dc8f973 100644 --- a/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/useBuilderChatPanel.ts +++ b/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/useBuilderChatPanel.ts @@ -12,6 +12,7 @@ import { useEdgeStore } from "../../stores/edgeStore"; import { useNodeStore } from "../../stores/nodeStore"; import { GraphAction, + extractTextFromParts, parseGraphActions, serializeGraphForChat, } from "./helpers"; @@ -52,23 +53,29 @@ export function useBuilderChatPanel({ useEffect(() => { if (!isOpen || sessionId || isCreatingSession || sessionError) return; + let cancelled = false; + async function createSession() { setIsCreatingSession(true); try { const res = await postV2CreateSession(null); + if (cancelled) return; if (res.status === 200) { setSessionId(res.data.id); } else { setSessionError(true); } } catch { - setSessionError(true); + if (!cancelled) setSessionError(true); } finally { - setIsCreatingSession(false); + if (!cancelled) setIsCreatingSession(false); } } createSession(); + return () => { + cancelled = true; + }; }, [isOpen, sessionId, isCreatingSession, sessionError]); const transport = useMemo( @@ -118,12 +125,7 @@ export function useBuilderChatPanel({ const assistantMessages = messages.filter((m) => m.role === "assistant"); const last = assistantMessages[assistantMessages.length - 1]; if (!last) return []; - const text = last.parts - .filter( - (p): p is Extract => p.type === "text", - ) - .map((p) => p.text) - .join(""); + const text = extractTextFromParts(last.parts); const parsed = parseGraphActions(text); const seen = new Set(); return parsed.filter((action) => { @@ -194,6 +196,9 @@ export function useBuilderChatPanel({ }, }); } else if (action.type === "connect_nodes") { + const sourceExists = nodes.some((n) => n.id === action.source); + const targetExists = nodes.some((n) => n.id === action.target); + if (!sourceExists || !targetExists) return; addEdge({ id: `${action.source}:${action.sourceHandle}->${action.target}:${action.targetHandle}`, source: action.source,