feat: expose tool node artifacts to downstream nodes in Agentflow v2#6440
feat: expose tool node artifacts to downstream nodes in Agentflow v2#6440mvanhorn wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for parsing, streaming, and returning artifacts within the DirectReply node of agentflow. It adds a new directReplyArtifacts input parameter, updates the node's execution logic to resolve artifacts from either the dedicated input or the message itself, and includes comprehensive test coverage. The review feedback highlights two key areas for improvement: preventing a potential runtime TypeError by ensuring outputContent is a string before calling .trim(), and optimizing the parseArtifacts helper to avoid unnecessary try-catch overhead on plain text messages by using a quick JSON heuristic check.
| const outputContent = messageArtifacts.length > 0 && directReplyMessage?.trim() ? ' ' : directReplyMessage | ||
|
|
||
| if (isStreamable) { | ||
| const sseStreamer: IServerSideEventStreamer = options.sseStreamer | ||
| sseStreamer.streamTokenEvent(chatId, directReplyMessage) | ||
| if (outputContent?.trim()) { | ||
| sseStreamer.streamTokenEvent(chatId, outputContent) | ||
| } |
There was a problem hiding this comment.
Calling directReplyMessage?.trim() and outputContent?.trim() can throw a TypeError at runtime if directReplyMessage is not a string (for example, if it is an array or object passed as a variable from an upstream node, which is a common pattern in Agentflow). Optional chaining (?.) only guards against null or undefined, not against other types that lack the .trim() method.
We can make this safer and simpler by:
- Setting
outputContentto' 'directly ifmessageArtifacts.length > 0(since we already know it contains artifacts and shouldn't be printed as text). - Checking
typeof outputContent === 'string'before calling.trim()when streaming.
| const outputContent = messageArtifacts.length > 0 && directReplyMessage?.trim() ? ' ' : directReplyMessage | |
| if (isStreamable) { | |
| const sseStreamer: IServerSideEventStreamer = options.sseStreamer | |
| sseStreamer.streamTokenEvent(chatId, directReplyMessage) | |
| if (outputContent?.trim()) { | |
| sseStreamer.streamTokenEvent(chatId, outputContent) | |
| } | |
| const outputContent = messageArtifacts.length > 0 ? ' ' : directReplyMessage | |
| if (isStreamable) { | |
| const sseStreamer: IServerSideEventStreamer = options.sseStreamer | |
| if (typeof outputContent === 'string' && outputContent.trim()) { | |
| sseStreamer.streamTokenEvent(chatId, outputContent) | |
| } |
| try { | ||
| artifacts = JSON.parse(trimmedValue) | ||
| } catch { | ||
| return [] | ||
| } |
There was a problem hiding this comment.
To avoid throwing and catching exceptions on every plain text message (which is the most common case for directReplyMessage), we can add a quick heuristic check to see if the string looks like JSON (e.g., starts with [ or {) before calling JSON.parse. This improves performance and prevents debugger disruption when 'Pause on caught exceptions' is enabled. This heuristic-based approach is acceptable here as the risk of false positives is very low.
if (!trimmedValue.startsWith('[') && !trimmedValue.startsWith('{')) {
return []
}
try {
artifacts = JSON.parse(trimmedValue)
} catch {
return []
}References
- A heuristic-based solution is acceptable if the risk of false positives is determined to be very low within the specific application's context.
Summary
feat: expose tool node artifacts to downstream nodes in Agentflow v2
Closes #6354
AI was used for assistance.