Skip to content

fix(conversations): Stop rendering raw tool call JSON as message content#117065

Merged
obostjancic merged 3 commits into
masterfrom
ognjenbostjancic/tet-2448-weird-rendering-of-tool-calls
Jun 8, 2026
Merged

fix(conversations): Stop rendering raw tool call JSON as message content#117065
obostjancic merged 3 commits into
masterfrom
ognjenbostjancic/tet-2448-weird-rendering-of-tool-calls

Conversation

@obostjancic

@obostjancic obostjancic commented Jun 8, 2026

Copy link
Copy Markdown
Member

When an assistant response contains only tool calls without text, parseAssistantContent would fall through to gen_ai.response.objectand render the raw tool call JSON as message content. This caused:

  1. Raw JSON blobs appearing in conversation messages
  2. Duplicate rendering of tool calls (badges + JSON content)

Fix by detecting tool-call-only outputs in extractAssistantOutput andreturning null instead of falling through to response attributes. Also creates assistant messages for tool-call-only turns so the tool call badges still render, and skips the content area when message content is empty.

Before:
CleanShot 2026-06-08 at 13 07 55

After:
CleanShot 2026-06-08 at 13 07 55

Closes TET-2448

When an assistant response contains only tool calls without text,
parseAssistantContent would fall through to gen_ai.response.object
and render the raw tool call JSON as message content. This caused:

1. Raw JSON blobs appearing in conversation messages
2. Duplicate rendering of tool calls (badges + JSON content)

Fix by detecting tool-call-only outputs in extractAssistantOutput and
returning null instead of falling through to response attributes. Also
create assistant messages for tool-call-only turns so the badges still
render, and skip the content area when message content is empty.

Closes TET-2448
Co-Authored-By: Claude <noreply@anthropic.com>
@linear-code

linear-code Bot commented Jun 8, 2026

Copy link
Copy Markdown

TET-2448

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jun 8, 2026
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

📊 Type Coverage Diff

✅ no issues found

Comment on lines +129 to +145
{message.content !== '' && (
<StyledClippedBox
clipHeight={200}
buttonProps={{variant: 'secondary', size: 'xs'}}
collapsible
>
<Container padding="md">
<MessageText size="sm" align="left">
<AIContentRenderer
text={message.content}
inline
autoCollapseLimit={10}
/>
</MessageText>
</Container>
</StyledClippedBox>
)}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 high · possible confidence

Null Content Guard — The guard message.content !== '' only skips rendering when content is exactly an empty string. If content can be null or undefined at runtime (the old code set content: turn.assistantContent which could be null), the condition evaluates to true and AIContentRenderer receives a null/undefined text prop, which may throw or render incorrectly. The new code sets content: turn.assistantContent ?? '' so this should be safe for newly created messages, but any cached or legacy message objects with content: null would bypass the guard.

@sharvan9515

Copy link
Copy Markdown

✅ Implements the stated intent

Intent: Fix raw tool-call JSON appearing as message content by detecting tool-call-only assistant outputs and returning null instead of falling through to response attributes, and skip rendering the content area when content is empty.

This PR fixes an issue where tool-call-only assistant outputs were rendering raw JSON as message content. The fix detects when an assistant message contains only tool calls (no text) and returns null from parseAssistantContent, then guards the content rendering in MessagesPanel to skip when content is empty. Finding [1] identifies a critical gap: the guard checks message.content !== '' but doesn't handle the case where content is null, which would still attempt to render and likely cause an error.

Findings (1): 1 high · 📍 1 commented inline on changed lines

Severity Location Issue
🟠 high static/app/views/explore/conversations/components/messagesPanel.tsx:129 Null Content Guard — The guard message.content !== '' only skips rendering when content is exactly an empty string. If content can be null or undefined at runtime (the old code set content: turn.assistantContent which could be null), the condition evaluates to true and `AIContentRendere…
📋 Basis of review

Examined control flow in conversationMessages.ts and messagesPanel.tsx, checked the new conditional rendering guard, the flush logic in mergeEmptyTurns, and the hasAssistantContent/hasToolCalls branching in turnsToMessages. Also reviewed the new test cases for correctness.

🔍 Needs human verification
  1. Confirm at runtime that assistant messages with only tool calls render the tool-call badges correctly without any content area, and that no visual regression occurs for normal text-only or mixed text+tool-call messages. 2. Verify that the mergeEmptyTurns flush (appending pending tool calls to the last result turn) does not cause duplicate tool-call badges when the last turn already has tool calls from a previous merge step. 3. Confirm that message.content !== '' is the right guard — if content can ever be undefined or null at runtime (e.g., from older data), the guard would pass and AIContentRenderer would receive a falsy value.

}

if (
const hasAssistantContent =

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 medium · likely confidence

Deduplication Bypass — The hasAssistantContent check uses seenAssistantContent.has(turn.assistantContent) to deduplicate, but when hasToolCalls is true and assistantContent is null, the block is entered via hasToolCalls without adding anything to seenAssistantContent. If a subsequent turn has the same null/empty assistantContent with tool calls, it will also pass through. More importantly, if hasAssistantContent is false (content already seen) but hasToolCalls is true, the block is entered and an assistant message is emitted with the duplicate content set to turn.assistantContent ?? ''. This means a previously-deduplicated content string could be re-emitted as an empty string, silently changing behavior from the old code where the entire block was skipped.

@sharvan9515

Copy link
Copy Markdown

✅ Implements the stated intent

Intent: Fix raw tool call JSON appearing as message content by detecting tool-call-only assistant outputs and returning null instead of falling through to response attributes, and skip rendering the content area when message content is empty.

The diff implements all described fixes: parseAssistantContent in conversationMessages.ts now returns null for tool-call-only outputs, turnsToMessages creates assistant messages with empty content for tool-call-only turns, mergeEmptyTurns flushes pending tool calls onto the last result turn, and messagesPanel.tsx conditionally renders the content area only when message.content !== ''. Tests cover all new branches.

Findings (1): 1 medium · 📍 1 commented inline on changed lines

Severity Location Issue
🟡 medium static/app/views/explore/conversations/utils/conversationMessages.ts:185 Deduplication Bypass — The hasAssistantContent check uses seenAssistantContent.has(turn.assistantContent) to deduplicate, but when hasToolCalls is true and assistantContent is null, the block is entered via hasToolCalls without adding anything to seenAssistantContent. If a subsequent tu…
📋 Basis of review

Examined control flow and logic changes in conversationMessages.ts and messagesPanel.tsx, including the new hasAssistantContent/hasToolCalls branching, the flush logic in mergeEmptyTurns, and the conditional render guard, together with the new test cases.

🔍 Needs human verification
  1. Confirm at runtime that tool-call-only assistant turns render the tool call badges correctly without any content area, and that no visual regression occurs for turns that have both text and tool calls. 2. Verify that flushing pending tool calls onto the last result turn in mergeEmptyTurns does not cause duplicate tool call badges when the last turn already has tool calls from a prior merge. 3. Confirm that message.content !== '' (strict string check) correctly handles all possible content values returned by the updated turnsToMessages, including the FILTERED sentinel.

@obostjancic obostjancic marked this pull request as ready for review June 8, 2026 11:16
@obostjancic obostjancic requested a review from a team as a code owner June 8, 2026 11:16

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c2b482b. Configure here.

timestamp: endTs,
nodeId: turn.generation.id,
toolCalls: turn.toolCalls.length > 0 ? turn.toolCalls : undefined,
toolCalls: hasToolCalls ? turn.toolCalls : undefined,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assistant dedup bypassed with tools

Medium Severity

turnsToMessages now emits an assistant message when hasToolCalls is true even if that turn’s assistantContent was already deduplicated. A later turn with the same assistant text plus tool calls still pushes a full duplicate message (same content again), regressing the existing “deduplicates assistant messages by exact content” behavior.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c2b482b. Configure here.

@obostjancic obostjancic merged commit 2177a52 into master Jun 8, 2026
70 checks passed
@obostjancic obostjancic deleted the ognjenbostjancic/tet-2448-weird-rendering-of-tool-calls branch June 8, 2026 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants