mirror of
https://github.com/Significant-Gravitas/AutoGPT.git
synced 2026-04-30 03:00:41 -04:00
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
This commit is contained in:
@@ -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<HTMLTextAreaElement>) {
|
||||
@@ -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<HTMLDivElement>;
|
||||
}
|
||||
|
||||
@@ -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<typeof p, { type: "text" }> => 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 (
|
||||
<ActionItem
|
||||
key={key}
|
||||
action={action}
|
||||
nodes={nodes}
|
||||
onApply={() => onApplyAction(action)}
|
||||
/>
|
||||
);
|
||||
return <ActionItem key={key} action={action} nodes={nodes} />;
|
||||
})}
|
||||
</div>
|
||||
)}
|
||||
@@ -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 (
|
||||
<div className="flex items-start justify-between gap-2 rounded bg-white p-2 text-xs shadow-sm">
|
||||
<span className="leading-tight text-slate-700">{label}</span>
|
||||
<button
|
||||
onClick={handleApply}
|
||||
disabled={applied}
|
||||
className={cn(
|
||||
"shrink-0 rounded px-2 py-0.5 text-xs font-medium transition-colors",
|
||||
applied
|
||||
? "bg-green-100 text-green-700"
|
||||
: "bg-violet-600 text-white hover:bg-violet-700",
|
||||
)}
|
||||
>
|
||||
{applied ? "Applied" : "Apply"}
|
||||
</button>
|
||||
<span className="shrink-0 rounded bg-green-100 px-2 py-0.5 text-xs font-medium text-green-700">
|
||||
Applied
|
||||
</span>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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(<BuilderChatPanel />);
|
||||
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", () => {
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<typeof p, { type: "text" }> => p.type === "text",
|
||||
)
|
||||
.map((p) => p.text)
|
||||
.join("");
|
||||
const text = extractTextFromParts(last.parts);
|
||||
const parsed = parseGraphActions(text);
|
||||
const seen = new Set<string>();
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user