Skip to content

Commit 6144097

Browse files
committed
fix(agents): preserve arun and mcp tool output
1 parent 9c37a67 commit 6144097

4 files changed

Lines changed: 78 additions & 16 deletions

File tree

src/frontend/src/components/core/chatComponents/ToolOutputDisplay.tsx

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,19 @@ function FormattedOutput({ value }: { value: JSONValue }) {
8080

8181
type Tab = "result" | "metadata";
8282

83+
function isMcpCallToolResultArtifact(
84+
value: JSONValue | undefined,
85+
): value is Record<string, JSONValue> {
86+
return (
87+
value !== null &&
88+
value !== undefined &&
89+
typeof value === "object" &&
90+
!Array.isArray(value) &&
91+
Array.isArray(value.content) &&
92+
typeof value.isError === "boolean"
93+
);
94+
}
95+
8396
/** Tab control sized for the inside of a tool-call card. Matches the
8497
* underline-on-active pattern used across assistant-ui / Claude /
8598
* ChatGPT — quiet by default, the active tab gets a 2px underline in
@@ -118,9 +131,10 @@ function TabButton({
118131
* - LangChain ToolMessage envelope with non-standard metadata keys
119132
* (additional_kwargs, response_metadata, type, ...) gets a 2-tab UI:
120133
* "Result" prefers the structured `.artifact` and falls back to the
121-
* model-facing `.content`; "Metadata" shows the rest of the envelope
122-
* as pretty JSON. Plumbing keys (name, id, tool_call_id, status) are
123-
* suppressed because the accordion trigger already surfaces them.
134+
* model-facing `.content`. MCP `CallToolResult` artifacts are protocol
135+
* envelopes, so their readable content stays in Result and the raw
136+
* artifact stays in Metadata. Plumbing keys (name, id, tool_call_id,
137+
* status) are suppressed because the accordion trigger surfaces them.
124138
* - Anything else (simple string, plain dict, unwrappable envelope)
125139
* falls through to FormattedOutput directly under the eyebrow —
126140
* no tabs, just the body. */
@@ -137,17 +151,20 @@ export function ToolOutputDisplay({ output }: { output: JSONValue }) {
137151
);
138152
}
139153

140-
const result = output.artifact ?? output.content;
154+
const artifact = output.artifact;
155+
const isMcpArtifact = isMcpCallToolResultArtifact(artifact);
156+
const result = artifact != null && !isMcpArtifact ? artifact : output.content;
141157
// Strip the standard ToolMessage plumbing keys from the metadata view —
142158
// the accordion trigger already shows tool name and status, and id /
143159
// tool_call_id aren't useful in the UI. What remains is the actually
144160
// interesting custom metadata (additional_kwargs, response_metadata,
145-
// type, custom fields). Artifact is the primary result, so don't duplicate
146-
// it in the Metadata tab.
161+
// type, custom fields). Component artifacts are the primary result, while
162+
// raw MCP protocol artifacts remain metadata behind readable content.
147163
const metadata = Object.fromEntries(
148-
Object.entries(output).filter(
149-
([k]) => !TOOL_MESSAGE_KEYS_PLUS_CONTENT.has(k),
150-
),
164+
Object.entries(output).filter(([key, value]) => {
165+
if (key === "artifact") return value != null && isMcpArtifact;
166+
return !TOOL_MESSAGE_KEYS_PLUS_CONTENT.has(key);
167+
}),
151168
);
152169
const hasMetadata = Object.keys(metadata).length > 0;
153170

@@ -205,12 +222,12 @@ export function ToolOutputDisplay({ output }: { output: JSONValue }) {
205222
);
206223
}
207224

208-
// `content` and `artifact` belong in the Result tab; the rest of the
209-
// canonical plumbing fields aren't worth exposing — keep this set local
210-
// rather than re-exporting yet another constant.
225+
// `content` belongs in the Result tab; the canonical plumbing fields aren't
226+
// worth exposing — keep this set local rather than re-exporting another
227+
// constant. Artifact routing depends on whether it is component data or an
228+
// MCP protocol envelope, so it is handled separately above.
211229
const TOOL_MESSAGE_KEYS_PLUS_CONTENT = new Set([
212230
"content",
213-
"artifact",
214231
"name",
215232
"id",
216233
"tool_call_id",

src/frontend/src/components/core/chatComponents/__tests__/ContentDisplay.test.tsx

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { render, screen } from "@testing-library/react";
1+
import { fireEvent, render, screen } from "@testing-library/react";
22
import type {
33
CitationContent,
44
ContentBlockItem,
@@ -394,6 +394,38 @@ describe("ContentDisplay", () => {
394394
expect(screen.getAllByRole("tab")).toHaveLength(2);
395395
});
396396

397+
it("keeps MCP content as the result and its protocol artifact in metadata", async () => {
398+
const tool = {
399+
type: "tool_use",
400+
name: "mcp_search",
401+
tool_input: {},
402+
output: {
403+
content: "readable MCP output",
404+
artifact: {
405+
meta: null,
406+
content: [{ type: "text", text: "readable MCP output" }],
407+
structuredContent: null,
408+
isError: false,
409+
},
410+
additional_kwargs: {},
411+
response_metadata: {},
412+
type: "tool",
413+
name: "mcp_search",
414+
tool_call_id: "toolu_mcp",
415+
status: "success",
416+
},
417+
} as unknown as ContentBlockItem;
418+
419+
render(<ContentDisplay content={tool} chatId="t-t-mcp-artifact" />);
420+
421+
expect(screen.getByText("readable MCP output")).toBeInTheDocument();
422+
423+
fireEvent.click(screen.getByRole("tab", { name: "Metadata" }));
424+
const metadata = await screen.findByTestId("code-tabs");
425+
expect(metadata).toHaveTextContent('"artifact": {');
426+
expect(metadata).toHaveTextContent('"isError": false');
427+
});
428+
397429
it("renders the error body in a destructive-toned panel", () => {
398430
// No eyebrow label — the surrounding accordion already says
399431
// "tool call" and the destructive bg + color carries the meaning.

src/lfx/src/lfx/base/tools/component_tool.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ async def _arun(
6363
run_manager: AsyncCallbackManagerForToolRun | None = None,
6464
**kwargs: Any,
6565
) -> tuple[Any, Any | None]:
66+
if self.coroutine is None:
67+
# StructuredTool falls back to self._run, which already returns the
68+
# content-and-artifact tuple. Wrapping it again would nest the tuple.
69+
return await super()._arun(*args, config=config, run_manager=run_manager, **kwargs)
6670
content = await super()._arun(*args, config=config, run_manager=run_manager, **kwargs)
6771
return self._content_and_artifact(content)
6872

src/lfx/tests/unit/custom/component/test_tool_dataframe_return.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,6 @@ def test_agent_tool_preserves_dataframe_as_artifact():
105105
]
106106

107107

108-
@pytest.mark.asyncio
109108
async def test_sync_agent_tool_preserves_dataframe_as_artifact_when_awaited():
110109
"""Async agent execution of a sync tool must not wrap the response tuple twice."""
111110
component = DataFrameProducerComponent()
@@ -128,6 +127,17 @@ async def test_sync_agent_tool_preserves_dataframe_as_artifact_when_awaited():
128127
]
129128

130129

130+
async def test_sync_tool_arun_returns_dataframe_not_wrapped_tuple():
131+
"""Direct arun calls on sync tools must preserve the legacy raw-result contract."""
132+
component = DataFrameProducerComponent()
133+
table_tool = next(t for t in ComponentToolkit(component=component).get_tools() if t.name == "get_table")
134+
135+
result = await table_tool.arun({"query": "test"})
136+
137+
assert isinstance(result, pd.DataFrame), f"Expected pandas DataFrame, got {type(result).__name__}: {result!r}"
138+
assert result["col1"].tolist() == [1, 2, 3]
139+
140+
131141
def test_tool_returns_text_for_message():
132142
"""Tool wrapping a Message output must still return text (not change behavior)."""
133143
component = DataFrameProducerComponent()
@@ -193,7 +203,6 @@ async def test_tool_handles_positional_args_async():
193203
assert "my_async_query" in result
194204

195205

196-
@pytest.mark.asyncio
197206
async def test_async_agent_tool_preserves_dataframe_as_artifact():
198207
"""Async component tools must preserve structured output under the same Agent contract."""
199208
component = AsyncTextComponent()

0 commit comments

Comments
 (0)