-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: AI dialogue node calls MCP, AI's reply content is included in the code block of MCP call result, markdown parsing error #3846 #3894
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
Conversation
…e code block of MCP call result, markdown parsing error #3846
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| content = generate_tool_message_template(chunk[0].name, chunk[0].content) | ||
| chunk[0].content = content | ||
| yield chunk[0] | ||
| if isinstance(chunk[0], AIMessageChunk): |
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.
The provided code has a few issues and areas for improvement:
-
Redundancy: The
generate_tool_message_templatefunction is called twice within the_yield_mcp_responsemethod. This redundancy can be removed. -
Security Concerns: If user input (
reasoning_content) contains malicious JSON, it could lead to injection attacks. Consider sanitizing this input before converting it to JSON. -
Code Duplication: There are similar template string constructions in both places where you're creating tool messages. Can these be combined into a single function?
Here's an optimized version of the code:
@@ -11,7 +11,6 @@
import re
import time
from functools import reduce
-from types import AsyncGeneratorType
from typing import List, Dict
from django.db.models import QuerySet
@@ -33,13 +32,25 @@
<strong>Called MCP Tool: <em>%s</em></strong>
</summary>
-```json
%s
-```
+
</details>
"""
+def format_tool_message(content):
+ """Formats the content as a Markdown block with triple backticks."""
+ return f"```json\n{json.dumps(content, indent=4)}\n```"
def _write_context(node_variable: Dict, workflow_variable: Dict, node: INode, workflow, answer: str,
reasoning_content: str):
@@ -109,7 +120,7 @@ async def _yield_mcp_response(chat_model, message_list, mcp_servers):
response = agent.astream({"messages": message_list}, stream_mode='messages')
async for chunk in response:
if isinstance(chunk[0], ToolMessage):
- content = tool_message_template % (chunk[0].name, chunk[0].content)
+ content = format_tool_message(chunk[0].content)
chunk[0].content = content
yield chunk[0]
if isinstance(chunk[0], AIMessageChunk):Key Changes:
- Single Function for Formatting: A new
format_tool_messagefunction has been introduced to handle the formatting of Tool Message data. - Simplified Code: The code duplication between the two places has been eliminated, improving readability and maintainability.
This should help resolve the redundancies and improve security while keeping the code concise.
| console.log(nodeModel) | ||
| props.nodeModel.graphModel.addEdge({ | ||
| type: 'app-edge', | ||
| sourceNodeId: props.nodeModel.id, |
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.
There is no irregularity or issue in the provided code snippet. The function clickNodes appears to be correctly setting up data and parameters needed for adding an edge to a graph using properties passed from props. A possible optimization suggestion would be to ensure that anchorData.value?.x, width, and other variables used in the calculation have defined values in order to prevent errors at runtime.
fix: AI dialogue node calls MCP, AI's reply content is included in the code block of MCP call result, markdown parsing error #3846