Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces real-time streaming for sub-flow iterations in buildAgentflow.ts, allowing results to be emitted via sseStreamer as they complete rather than waiting for the entire batch. The review feedback suggests simplifying the implementation by using the existing loop index instead of a new counter to manage newline separators. This change would also ensure consistency between the streamed output and the final persisted message by including empty results in the stream.
|
|
||
| // Initialize array to collect results from iterations | ||
| const iterationResults: string[] = [] | ||
| let successfulCount = 0 // drives newline separator between streamed items |
There was a problem hiding this comment.
This variable can be removed if the loop index i is used to handle the newline separator logic, as suggested in the iteration processing logic below.
| if (subFlowResult) { | ||
| iterationResults.push(subFlowResult.text || '') | ||
| if (subFlowResult.text) { | ||
| // Stream each result as it completes rather than batching at the end. | ||
| // Sub-flows run with isRecursive=true, so inner nodes (e.g. DirectReply) | ||
| // never reach isLastNode=true and never call streamTokenEvent themselves. | ||
| if (isLastNode && sseStreamer) { | ||
| sseStreamer.streamTokenEvent(chatId, (successfulCount > 0 ? '\n' : '') + subFlowResult.text) | ||
| } | ||
| successfulCount++ | ||
| } | ||
| } |
There was a problem hiding this comment.
The current streaming logic skips empty results, which creates a discrepancy with the final output generated by iterationResults.join('\n') (which includes empty strings as empty lines).
To maintain consistency between the real-time stream and the final persisted message, consider streaming every result. Using the loop index i instead of successfulCount ensures the newline separator correctly matches the join behavior.
if (subFlowResult) {
const resultText = subFlowResult.text || ''
iterationResults.push(resultText)
if (isLastNode && sseStreamer) {
// Stream each result (including empty ones) to match the final joined output
sseStreamer.streamTokenEvent(chatId, (i > 0 ? '\n' : '') + resultText)
}
}References
- A suggested change, even if valid, may be deferred if it pertains to logic that is not being modified in the current pull request.
There was a problem hiding this comment.
@HenryHengZJ @jocelynlin-wd
There are 2 approaches to resolving the streaming bug:
Approach 1 — Errors excluded from content (Ref: 328d652)
Aligns iteration with every other v2 node (errors go to execution log only, not chat content), but
this changes the non-streaming API response and persisted chat history for existing flows.
Approach 2 — Errors included in content, not in stream (Changes made in this PR)
Preserves the pre-fix API response and chat history (errors visible on reload), but what was streamed
live differs from what gets saved — a user watching live sees fewer results than a user reading
history.
Decision: Should iteration errors be part of content (and therefore streamed to keep live and history
consistent), or excluded from content entirely to match all v2 node behavior ?
1 participant
Before:
Before.Streaming.Fix.mov
After:
After.streaming.fix.mov