Skip to content

1680 fix tool calling in playground#1696

Merged
neelneelneel merged 25 commits intodevfrom
1680-fix-tool-calling-in-playground
Aug 26, 2025
Merged

1680 fix tool calling in playground#1696
neelneelneel merged 25 commits intodevfrom
1680-fix-tool-calling-in-playground

Conversation

@neelneelneel
Copy link
Copy Markdown
Contributor

Description

Changes Made

How to Test

  1. Steps to reproduce/test the behavior
  2. Expected outcomes

Notes

@neelneelneel neelneelneel requested a review from a team as a code owner August 12, 2025 22:06
@neelneelneel neelneelneel linked an issue Aug 12, 2025 that may be closed by this pull request
1 task
@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /describe

@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /review

@QodoAI-Agent
Copy link
Copy Markdown

Title

1680 fix tool calling in playground


User description

Description

Changes Made

How to Test

  1. Steps to reproduce/test the behavior
  2. Expected outcomes

Notes


PR Type

Enhancement, Bug fix


Description

  • Refactor chat to typed message stores

  • Fix tool-calling flow and persistence

  • Migrate FlexLayout usage to shared module

  • Add input/response UI components


Diagram Walkthrough

flowchart LR
  MsgStores["Root/Input/Response Stores"] -- "replaces" --> OldChatMessage["Legacy ChatMessage"]
  RoomStore["RoomStore"] -- "uses" --> MsgStores
  RoomStore -- "executes" --> ToolFlow["RunMCPTool / AddPlaygroundToolExecution"]
  UI["InputMessage / ResponseMessage"] -- "renders" --> MsgStores
  SharedFlex["@semoss/shared FlexLayout"] -- "replaces" --> DirectFlex["flexlayout-react direct usage"]
  ClientPanels["Client Panels (Workspace, File/Notebook/Layers)"] -- "import from" --> SharedFlex
Loading

File Walkthrough

Relevant files
Enhancement
32 files
room.store.ts
Replace ChatRoom with RoomStore; tool execution and rewrite
+261/-158
LayersPanel.tsx
Use shared FlexLayout and UI cleanups                                       
+81/-87 
FileExplorerPanel.tsx
Migrate to shared FlexLayout; simplify UI                               
+96/-110
ResponseMessage.tsx
New response message component with tools/feedback             
+295/-0 
Workspace.tsx
Integrate shared FlexLayout in workspace                                 
+13/-12 
NotebookExplorerPanel.tsx
Switch to shared FlexLayout actions/types                               
+9/-9     
abstract-message.store.ts
Add abstract message base with tree/activation                     
+144/-0 
FileEditorPanel.tsx
Rename tab via shared FlexLayout; UX tweaks                           
+10/-6   
NewRoomPage.tsx
Adopt RoomStore options and string fixes                                 
+6/-6     
chat.store.ts
Use RoomStore; adjust create/get logic                                     
+7/-7     
RoomApp.tsx
Stub app message processing; iframe title                               
+7/-5     
RoomPage.tsx
Render input/response components; loader fix                         
+21/-10 
InputMessage.tsx
New input message UI component                                                     
+60/-0   
workspace.store.ts
Workspace model via shared FlexLayout                                       
+6/-6     
OptionsPicker.tsx
Type to RoomStore options; minor cleanup                                 
+5/-5     
OptionsMenu.tsx
Options types update; checkbox handler fix                             
+4/-4     
response-message.store.ts
Add response message store with tools/rating                         
+66/-0   
root-message.store.ts
Root store with computed history traversal                             
+44/-0   
input-message.store.ts
Add input message store                                                                   
+24/-0   
RoomControls.tsx
Prop typing to RoomStore                                                                 
+2/-2     
Layout.tsx
Introduce shared Layout wrapper and types                               
[link]   
workspace.types.ts
Workspace options use shared FlexLayout types                       
+2/-2     
index.ts
Export message stores                                                                       
+5/-0     
index.ts
Remove legacy exports; export ChatStore only                         
+1/-3     
index.ts
Re-export components; remove direct flexlayout exports     
+1/-4     
index.ts
Export new message components                                                       
+1/-0     
index.ts
Export FlexLayout namespace and Layout                                     
+5/-0     
index.ts
Re-export chat, message, room stores                                         
+2/-0     
index.ts
Export RoomStore                                                                                 
+3/-0     
index.ts
Add message exports                                                                           
+2/-0     
index.ts
Export Layout module                                                                         
[link]   
flexlayout.css
Add custom FlexLayout styling                                                       
[link]   
Bug fix
1 files
types.d.ts
Align tool response schema to arrays                                         
+3/-3     
Documentation
1 files
useChat.ts
Clarify hook docs casing                                                                 
+2/-2     
Additional files
7 files
usePixel.ts +0/-1     
package.json +0/-1     
flexlayout.css +0/-147 
RoomMessage.tsx +0/-327 
index.ts +0/-1     
chat.message.ts +0/-262 
pnpm-lock.yaml +0/-3     

@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /improve

@QodoAI-Agent
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Error Handling

Multiple catch blocks assume Error shape (e.message). In browsers, thrown values can be non-Error; accessing e.message may be undefined. Normalize error handling or cast types to avoid undefined notifications.

			color: "success",
			message: "Succesfully copied to clipboard",
		});
	} catch (e) {
		notification.add({
			color: "error",
			message: e.message,
		});
	}
};

/**
 * Copy the text
 * @param text - text to copy
 */
const recordFeedback = async (rating: boolean) => {
	try {
		await room.recordFeedback(message, rating);

		notification.add({
			color: "success",
			message: "Succesfully saved feedback",
		});
	} catch (e) {
		notification.add({
			color: "error",
			message: e.message,
		});
	}
};

/**
 * Copy the text
 * @param text - text to copy
 */
const rewriteMessage = async () => {
	try {
		await room.rewriteMessage(message);

		notification.add({
			color: "success",
			message: "Succesfully rewrote message",
		});
	} catch (e) {
		notification.add({
			color: "error",
			message: e.message,
		});
	}
Type Narrowing

In auto-execute flow, you check instance of ResponseMessageStore before iterating tools, but earlier createMessage may return InputMessageStore or ResponseMessageStore. Ensure createMessage always returns a defined instance for the given pixelMessage types and handle unexpected/unknown types to avoid runtime undefined access.

// auto execute if enabled
if (this._store.options.autoExecute) {
	if (!(responseMessage instanceof ResponseMessageStore)) {
		return;
	}

	// loop through the response and execute the tool
	// save the response
	for (const tool of responseMessage.tools) {
		await this.runTool(
			responseMessage,
			tool.id,
			tool.name,
			tool.parameters,
		);
	}
}
API Contract Changes

The code now expects pixel messages to use 'tool_responses' instead of 'toolResponse'. Verify backend responses match this schema across INPUT_TOOL_EXEC and RESPONSE_TOOL; otherwise tools mapping will fail silently.

		pixelMessage.messageId,
		"",
		pixelMessage.tool_responses.map((t) => ({
			id: t.id,
			name: t.name,
			parameters: t.arguments,
		})),
	);
} else if (pixelMessage.type === "RESPONSE_TEXT") {
	return new ResponseMessageStore(
		pixelMessage.messageId,
		pixelMessage.content,
		[],
	);
} else if (pixelMessage.type === "RESPONSE_TOOL") {
	return new ResponseMessageStore(
		pixelMessage.messageId,
		"",
		pixelMessage.tool_responses.map((t) => ({
			id: t.id,
			name: t.name,
			parameters: t.arguments,
		})),
	);

@QodoAI-Agent
Copy link
Copy Markdown

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix undefined prompt usage

rewriteMessage references prompt which is undefined, causing runtime errors and
malformed pixel calls. Pass the rewritten prompt as a parameter and interpolate that
value. Also guard against missing parentMessage before accessing its properties.

packages/playground/src/stores/room/room.store.ts [486-528]

-rewriteMessage = async (message: ResponseMessageStore): Promise<void> => {
+rewriteMessage = async (message: ResponseMessageStore, prompt: string): Promise<void> => {
 	try {
-		// turn on the loading screen
 		this.setIsLoading(true);
 
-		// get the parent message
 		const parentMessage = message.parent;
 
-		// build the context if it is there
 		let context = "";
 		if (this._store.options?.instructions) {
 			context = this._store.options?.instructions;
 		}
 
-		// get a list of tool ids
-		const tools: string[] = this._store.options.tools.map(
-			(t) => t.id,
-			[],
-		);
+		const tools: string[] = this._store.options.tools.map((t) => t.id);
 
-		// wait for the pixel to run
+		const parentMsgId =
+			parentMessage && parentMessage.id ? `parentMessageId=["${parentMessage.id}"],` : "";
+
 		const response = await this.runPixel<
 			[
 				{
 					inputMessage: PixelMessage;
 					responseMessage: PixelMessage;
 				},
 			]
-		>(
-			`AskPlayground(
+		>(`AskPlayground(
 engine=["${this._store.modelId}"],
 roomId=["${this._store.roomId}"],
 command=["<encode>${prompt}</encode>"],
 ${context ? `context=["<encode>${context}</encode>"],` : `context=[],`}
+${tools.length ? `mcpToolID=${JSON.stringify(tools)},` : "mcpToolID=[],"}
+${parentMsgId}
+paramValues=[${JSON.stringify({
+			max_new_tokens: this._store.options.tokenLength,
+			temperature: this._store.options.temperature,
+		})}]
+);`);
 
-${tools.length ? `mcpToolID=${JSON.stringify(tools)},` : "mcpToolID=[],"}
-${parentMessage.id ? `parentMessageId=["${parentMessage.id}"],` : ""}
-paramValues=[${JSON.stringify({
-					max_new_tokens: this._store.options.tokenLength,
-					temperature: this._store.options.temperature,
-				})}]
-);`,
-		);
+		if (response.errors.length > 0) {
+			throw new Error(JSON.stringify(response.errors));
+		}
 
+		const { output } = response.pixelReturn[0];
+		const responseMessage = this.createMessage(output.responseMessage);
+		parentMessage?.addChild(responseMessage);
+
+		if (this._store.options.autoExecute) {
+			if (!(responseMessage instanceof ResponseMessageStore)) {
+				return;
+			}
+			for (const tool of responseMessage.tools) {
+				await this.runTool(responseMessage, tool.id, tool.name, tool.parameters);
+			}
+		}
+	} finally {
+		this.setIsLoading(false);
+	}
+};
+
Suggestion importance[1-10]: 9

__

Why: The snippet uses an undefined prompt in the pixel command, which is a clear runtime bug; passing it as a parameter and guarding parentMessage materially improves correctness and robustness.

High
Security
Encode tool execution response

executionResponse is interpolated without encoding, risking malformed pixel or
injection if it contains quotes/newlines. Safely encode or stringify the response
before embedding in the pixel string.

packages/playground/src/stores/room/room.store.ts [607-626]

 saveTool = async (
 	message: ResponseMessageStore,
 	toolId: string,
 	toolName: string,
 	executionResponse: string,
 ): Promise<void> => {
 	try {
-		// turn on the loading screen
 		this.setIsLoading(true);
 
-		// wait for the pixel to run
+		const safeResponse = JSON.stringify(executionResponse);
+
 		const response = await this.runPixel<
 			[
 				{
 					responseMessage: PixelMessage | string;
 				},
 			]
 		>(
-			`AddPlaygroundToolExecution(roomId = [ "${this._store.roomId}" ], toolId = [ "${toolId}" ], toolName=[ "${toolName}" ], response=[ ${executionResponse} ]);`,
+			`AddPlaygroundToolExecution(roomId = [ "${this._store.roomId}" ], toolId = [ "${toolId}" ], toolName=[ "${toolName}" ], response=[ ${safeResponse} ]);`,
 		);
 
-		// throw errors
 		if (response.errors.length > 0) {
 			throw new Error(JSON.stringify(response.errors));
 		}
 
 		const { output } = response.pixelReturn[0];
-
-		// don't create a new message if it is a string. More tools need to be executed
 		if (typeof output.responseMessage === "string") {
 			return;
 		}
 
-		// create the response and link to the input
 		const responseMessage = this.createMessage(output.responseMessage);
-
 		message.addChild(responseMessage);
 	} finally {
-		// turn off the loading screen
 		this.setIsLoading(false);
 	}
 };
Suggestion importance[1-10]: 8

__

Why: Interpolating executionResponse unescaped into the pixel string risks malformed queries; JSON-stringifying it is a solid security and correctness fix aligned with the PR’s pattern of stricter error handling.

Medium
General
Stabilize message listener effect

The effect depends on notification.add, a function that can change identity, causing
unnecessary re-subscriptions and potential leaks. Wrap handleMessage with
useCallback and limit dependencies to stable references, or remove notification.add
from deps by using a ref.

packages/playground/src/components/room/RoomApp.tsx [66-70]

+const handleMessage = useCallback(async (_event: MessageEvent<{ data: Record<string, unknown> }>) => {
+	try {
+		// await room.processAppResponse();
+		console.log("TODO");
+	} catch (e) {
+		notification.add({
+			message: (e as Error).message,
+			color: "error",
+		});
+	}
+}, [notification, room]);
+
 useEffect(() => {
-	const handleMessage = async (
-		_event: MessageEvent<{ data: Record<string, unknown> }>,
-	) => {
-		try {
-			// await room.processAppResponse();
-			console.log("TODO");
-		} catch (e) {
-			notification.add({
-				message: e.message,
-				color: "error",
-			});
-		}
+	window.addEventListener("message", handleMessage as EventListener);
+	return () => {
+		window.removeEventListener("message", handleMessage as EventListener);
 	};
+}, [handleMessage]);
 
-	window.addEventListener("message", handleMessage);
-
-	return () => {
-		window.removeEventListener("message", handleMessage);
-	};
-}, [options, room.closeSidebar, notification.add]);
-
Suggestion importance[1-10]: 5

__

Why: Using a stable useCallback for the message handler reduces unnecessary add/remove cycles; it's a reasonable maintainability improvement though not critical to functionality.

Low

@neelneelneel neelneelneel merged commit 6c91380 into dev Aug 26, 2025
3 checks passed
@neelneelneel neelneelneel deleted the 1680-fix-tool-calling-in-playground branch August 26, 2025 20:44
@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /update_changelog

@QodoAI-Agent
Copy link
Copy Markdown

Changelog updates: 🔄

2025-08-26 *

Added

  • Support for MCP tool execution in the playground with tool response handling and artifact UI.
  • Shared FlexLayout integration across client and playground.

Changed

  • Refactored playground messaging and room stores to new message model and sidebar behavior.

Fixed

  • Tool calling flows in the playground and related renderer state handling.

to commit the new content to the CHANGELOG.md file, please type:
'/update_changelog --pr_update_changelog.push_changelog_changes=true'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix tool calling in playground

3 participants