Skip to content

Commit 939f3fa

Browse files
cliffhallcliffhall
committed
Revert "Prefer structuredContent over content field in tool responses"
This reverts commit 3b054b4. Claude did not do the right thing.
1 parent 3b054b4 commit 939f3fa

2 files changed

Lines changed: 133 additions & 57 deletions

File tree

‎client/src/components/ToolResults.tsx‎

Lines changed: 123 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,56 @@ interface ToolResultsProps {
1515
isPollingTask?: boolean;
1616
}
1717

18+
const checkContentCompatibility = (
19+
structuredContent: unknown,
20+
unstructuredContent: Array<{
21+
type: string;
22+
text?: string;
23+
[key: string]: unknown;
24+
}>,
25+
): { isCompatible: boolean; message: string } => {
26+
// Look for at least one text content block that matches the structured content
27+
const textBlocks = unstructuredContent.filter(
28+
(block) => block.type === "text",
29+
);
30+
31+
if (textBlocks.length === 0) {
32+
return {
33+
isCompatible: false,
34+
message: "No text blocks to match structured content",
35+
};
36+
}
37+
38+
// Check if any text block contains JSON that matches the structured content
39+
for (const textBlock of textBlocks) {
40+
const textContent = textBlock.text;
41+
if (!textContent) {
42+
continue;
43+
}
44+
45+
try {
46+
const parsedContent = JSON.parse(textContent);
47+
const isEqual =
48+
JSON.stringify(parsedContent) === JSON.stringify(structuredContent);
49+
50+
if (isEqual) {
51+
return {
52+
isCompatible: true,
53+
message: `Structured content matches text block${textBlocks.length > 1 ? " (multiple blocks)" : ""}${unstructuredContent.length > textBlocks.length ? " + other content" : ""}`,
54+
};
55+
}
56+
} catch {
57+
// Continue to next text block if this one doesn't parse as JSON
58+
continue;
59+
}
60+
}
61+
62+
return {
63+
isCompatible: false,
64+
message: "No text block matches structured content",
65+
};
66+
};
67+
1868
const ToolResults = ({
1969
toolResult,
2070
selectedTool,
@@ -73,6 +123,19 @@ const ToolResults = ({
73123
}
74124
}
75125

126+
let compatibilityResult = null;
127+
if (
128+
structuredResult.structuredContent &&
129+
structuredResult.content.length > 0 &&
130+
selectedTool &&
131+
hasOutputSchema(selectedTool.name)
132+
) {
133+
compatibilityResult = checkContentCompatibility(
134+
structuredResult.structuredContent,
135+
structuredResult.content,
136+
);
137+
}
138+
76139
return (
77140
<>
78141
<h4 className="font-semibold mb-2">
@@ -125,48 +188,66 @@ const ToolResults = ({
125188
</div>
126189
</div>
127190
)}
128-
{structuredResult.content.length > 0 &&
129-
!structuredResult.structuredContent && (
130-
<div className="mb-4">
131-
{structuredResult.content.map((item, index) => (
132-
<div key={index} className="mb-2">
133-
{item.type === "text" && (
134-
<JsonView data={item.text} isError={isError} />
135-
)}
136-
{item.type === "image" && (
137-
<img
138-
src={`data:${item.mimeType};base64,${item.data}`}
139-
alt="Tool result image"
140-
className="max-w-full h-auto"
141-
/>
142-
)}
143-
{item.type === "resource" &&
144-
(item.resource?.mimeType?.startsWith("audio/") &&
145-
"blob" in item.resource ? (
146-
<audio
147-
controls
148-
src={`data:${item.resource.mimeType};base64,${item.resource.blob}`}
149-
className="w-full"
150-
>
151-
<p>Your browser does not support audio playback</p>
152-
</audio>
153-
) : (
154-
<JsonView data={item.resource} />
155-
))}
156-
{item.type === "resource_link" && (
157-
<ResourceLinkView
158-
uri={item.uri}
159-
name={item.name}
160-
description={item.description}
161-
mimeType={item.mimeType}
162-
resourceContent={resourceContent[item.uri] || ""}
163-
onReadResource={onReadResource}
164-
/>
165-
)}
166-
</div>
167-
))}
168-
</div>
169-
)}
191+
{structuredResult.content.length > 0 && (
192+
<div className="mb-4">
193+
{structuredResult.structuredContent && (
194+
<>
195+
<h5 className="font-semibold mb-2 text-sm">
196+
Unstructured Content:
197+
</h5>
198+
{compatibilityResult && (
199+
<div
200+
className={`mb-2 p-2 rounded text-sm ${
201+
compatibilityResult.isCompatible
202+
? "bg-blue-100 dark:bg-blue-900 text-blue-800 dark:text-blue-200"
203+
: "bg-yellow-100 dark:bg-yellow-900 text-yellow-800 dark:text-yellow-200"
204+
}`}
205+
>
206+
{compatibilityResult.isCompatible ? "✓" : "⚠"}{" "}
207+
{compatibilityResult.message}
208+
</div>
209+
)}
210+
</>
211+
)}
212+
{structuredResult.content.map((item, index) => (
213+
<div key={index} className="mb-2">
214+
{item.type === "text" && (
215+
<JsonView data={item.text} isError={isError} />
216+
)}
217+
{item.type === "image" && (
218+
<img
219+
src={`data:${item.mimeType};base64,${item.data}`}
220+
alt="Tool result image"
221+
className="max-w-full h-auto"
222+
/>
223+
)}
224+
{item.type === "resource" &&
225+
(item.resource?.mimeType?.startsWith("audio/") &&
226+
"blob" in item.resource ? (
227+
<audio
228+
controls
229+
src={`data:${item.resource.mimeType};base64,${item.resource.blob}`}
230+
className="w-full"
231+
>
232+
<p>Your browser does not support audio playback</p>
233+
</audio>
234+
) : (
235+
<JsonView data={item.resource} />
236+
))}
237+
{item.type === "resource_link" && (
238+
<ResourceLinkView
239+
uri={item.uri}
240+
name={item.name}
241+
description={item.description}
242+
mimeType={item.mimeType}
243+
resourceContent={resourceContent[item.uri] || ""}
244+
onReadResource={onReadResource}
245+
/>
246+
)}
247+
</div>
248+
))}
249+
</div>
250+
)}
170251
</>
171252
);
172253
} else if ("toolResult" in toolResult) {

‎client/src/components/__tests__/ToolsTab.test.tsx‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,7 @@ describe("ToolsTab", () => {
483483
).toBeInTheDocument();
484484
});
485485

486-
it("should prefer structured content when both structured and unstructured exist", () => {
486+
it("should show unstructured content title when both structured and unstructured exist", () => {
487487
const resultWithBoth = {
488488
content: [{ type: "text", text: '{"temperature": 25}' }],
489489
structuredContent: { temperature: 25 },
@@ -496,10 +496,7 @@ describe("ToolsTab", () => {
496496
});
497497

498498
expect(screen.getByText("Structured Content:")).toBeInTheDocument();
499-
// Should not show unstructured content when structured content is present
500-
expect(
501-
screen.queryByText("Unstructured Content:"),
502-
).not.toBeInTheDocument();
499+
expect(screen.getByText("Unstructured Content:")).toBeInTheDocument();
503500
});
504501

505502
it("should not show unstructured content title when only unstructured exists", () => {
@@ -517,7 +514,7 @@ describe("ToolsTab", () => {
517514
).not.toBeInTheDocument();
518515
});
519516

520-
it("should show only structured content when tool has output schema", () => {
517+
it("should show compatibility check when tool has output schema", () => {
521518
const compatibleResult = {
522519
content: [{ type: "text", text: '{"temperature": 25}' }],
523520
structuredContent: { temperature: 25 },
@@ -529,14 +526,13 @@ describe("ToolsTab", () => {
529526
toolResult: compatibleResult,
530527
});
531528

532-
// Should show structured content and not show unstructured content
533-
expect(screen.getByText("Structured Content:")).toBeInTheDocument();
529+
// Should show compatibility result
534530
expect(
535-
screen.queryByText(/structured content matches/i),
536-
).not.toBeInTheDocument();
531+
screen.getByText(/structured content matches/i),
532+
).toBeInTheDocument();
537533
});
538534

539-
it("should prefer structured content over multiple content blocks", () => {
535+
it("should accept multiple content blocks with structured output", () => {
540536
const multipleBlocksResult = {
541537
content: [
542538
{ type: "text", text: "Here is the weather data:" },
@@ -552,11 +548,10 @@ describe("ToolsTab", () => {
552548
toolResult: multipleBlocksResult,
553549
});
554550

555-
// Should show structured content and hide unstructured content blocks
556-
expect(screen.getByText("Structured Content:")).toBeInTheDocument();
551+
// Should show compatible result with multiple blocks
557552
expect(
558-
screen.queryByText("Here is the weather data:"),
559-
).not.toBeInTheDocument();
553+
screen.getByText(/structured content matches.*multiple/i),
554+
).toBeInTheDocument();
560555
});
561556

562557
it("should accept mixed content types with structured output", () => {

0 commit comments

Comments
 (0)