Skip to content

Commit 367f49a

Browse files
committed
fix streaming flush to include completed tool segments
FlushStreamingText now flushes completed tool segments that appear before the current text segment. Previously, tool segments could be skipped when text was being streamed, causing them to appear out of order or be lost. Changes: - Collect and render completed tool segments before flushing text - Return tool content even when text is below flush threshold - Use shared contentBuilder to combine tool and text output
1 parent 8e7f5f2 commit 367f49a

2 files changed

Lines changed: 149 additions & 8 deletions

File tree

internal/ui/segment_flush_test.go

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,3 +283,83 @@ func TestFallbackFlushPreservesHeadingSpacing(t *testing.T) {
283283
t.Fatalf("Fallback flush output mismatch.\nReference:\n%q\nTotal:\n%q", StripANSI(reference), StripANSI(total))
284284
}
285285
}
286+
287+
func TestFlushStreamingText_FlushesCompletedToolsFirst(t *testing.T) {
288+
// Test that FlushStreamingText flushes completed tool segments
289+
// that appear before the current text segment.
290+
width := 80
291+
renderMd := func(s string, w int) string {
292+
return RenderMarkdown(s, w)
293+
}
294+
295+
tracker := NewToolTracker()
296+
297+
// Add a tool segment and mark it complete
298+
tracker.HandleToolStart("tool-1", "test_tool", "test info")
299+
tracker.HandleToolEnd("tool-1", true)
300+
301+
// Add an incomplete text segment with content
302+
tracker.AddTextSegment("Some streaming text.\n\n", width)
303+
304+
// FlushStreamingText should flush the tool first, then the text
305+
res := tracker.FlushStreamingText(0, width, renderMd)
306+
307+
output := res.ToPrint
308+
309+
// Verify tool segment content appears in output
310+
stripped := stripAnsiForTest(output)
311+
if !strings.Contains(stripped, "●") {
312+
t.Error("Expected tool indicator (●) in output, tool segment was not flushed")
313+
}
314+
if !strings.Contains(stripped, "test_tool") {
315+
t.Error("Expected 'test_tool' in output")
316+
}
317+
if !strings.Contains(stripped, "Some streaming text") {
318+
t.Error("Expected 'Some streaming text' in output")
319+
}
320+
321+
// Verify tool appears before text
322+
toolIdx := strings.Index(stripped, "●")
323+
textIdx := strings.Index(stripped, "Some streaming text")
324+
if toolIdx > textIdx {
325+
t.Errorf("Tool should appear before text. Tool at %d, text at %d", toolIdx, textIdx)
326+
}
327+
328+
// Verify the tool segment is marked as flushed
329+
if !tracker.Segments[0].Flushed {
330+
t.Error("Tool segment should be marked as flushed")
331+
}
332+
}
333+
334+
func TestFlushStreamingText_ReturnsToolsEvenWhenTextBelowThreshold(t *testing.T) {
335+
// Test that completed tools are returned even when text is below threshold
336+
width := 80
337+
338+
tracker := NewToolTracker()
339+
340+
// Add a tool segment and mark it complete
341+
tracker.HandleToolStart("tool-1", "test_tool", "test info")
342+
tracker.HandleToolEnd("tool-1", true)
343+
344+
// Add an incomplete text segment with minimal content
345+
tracker.AddTextSegment("Hi", width)
346+
347+
// Use high threshold so text won't be flushed, but tool should still be
348+
res := tracker.FlushStreamingText(1000, width, nil)
349+
350+
output := res.ToPrint
351+
stripped := stripAnsiForTest(output)
352+
353+
// Tool should be flushed even though text is below threshold
354+
if !strings.Contains(stripped, "●") {
355+
t.Error("Expected tool indicator (●) in output even when text is below threshold")
356+
}
357+
if !strings.Contains(stripped, "test_tool") {
358+
t.Error("Expected 'test_tool' in output")
359+
}
360+
361+
// Text should NOT be in output since it's below threshold
362+
if strings.Contains(stripped, "Hi") {
363+
t.Error("Text should not be flushed when below threshold")
364+
}
365+
}

internal/ui/tool_tracker.go

Lines changed: 69 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,33 @@ func (t *ToolTracker) FlushStreamingText(threshold int, width int, renderMd func
495495
return FlushStreamingTextResult{}
496496
}
497497

498+
// First: flush any completed tool segments before the text segment.
499+
// Tool calls are always processed before text continues, so any tool segments
500+
// that appear before the current text segment are already complete.
501+
var contentBuilder strings.Builder
502+
var toolsToFlush []*Segment
503+
for i := 0; i < segIdx; i++ {
504+
s := &t.Segments[i]
505+
if s.Type == SegmentTool && !s.Flushed && s.ToolStatus != ToolPending {
506+
toolsToFlush = append(toolsToFlush, s)
507+
s.Flushed = true
508+
}
509+
}
510+
511+
if len(toolsToFlush) > 0 {
512+
toolContent := RenderSegments(toolsToFlush, width, -1, nil, true)
513+
if t.HasFlushed {
514+
toolContent = stripLeadingBlankLine(toolContent)
515+
prefix := t.LeadingSeparator(toolsToFlush[0].Type)
516+
if prefix != "" {
517+
toolContent = prefix + toolContent
518+
}
519+
}
520+
contentBuilder.WriteString(toolContent)
521+
t.HasFlushed = true
522+
t.LastFlushedType = toolsToFlush[len(toolsToFlush)-1].Type
523+
}
524+
498525
// Get current text length
499526
textLen := 0
500527
if seg.TextBuilder != nil {
@@ -507,6 +534,10 @@ func (t *ToolTracker) FlushStreamingText(threshold int, width int, renderMd func
507534
unflushedLen := textLen - seg.FlushedPos
508535
if unflushedLen < threshold {
509536
debugFlushf("stream skip seg=%d reason=below-threshold textLen=%d flushedPos=%d unflushedLen=%d threshold=%d", segIdx, textLen, seg.FlushedPos, unflushedLen, threshold)
537+
// Return any tool content we already flushed, even if text doesn't need flushing
538+
if contentBuilder.Len() > 0 {
539+
return FlushStreamingTextResult{ToPrint: contentBuilder.String()}
540+
}
510541
return FlushStreamingTextResult{}
511542
}
512543

@@ -532,25 +563,32 @@ func (t *ToolTracker) FlushStreamingText(threshold int, width int, renderMd func
532563
if safeBoundary <= seg.FlushedPos {
533564
// No new committed blocks to flush yet
534565
debugFlushf("stream skip seg=%d reason=no-committed committed=%d flushedPos=%d", segIdx, safeBoundary, seg.FlushedPos)
566+
// Return any tool content we already flushed
567+
if contentBuilder.Len() > 0 {
568+
return FlushStreamingTextResult{ToPrint: contentBuilder.String()}
569+
}
535570
return FlushStreamingTextResult{}
536571
}
537572
rendered := seg.StreamRenderer.RenderedUnflushed()
538573
if rendered == "" {
539574
debugFlushf("stream skip seg=%d reason=empty-rendered committed=%d flushedPos=%d", segIdx, safeBoundary, seg.FlushedPos)
575+
// Return any tool content we already flushed
576+
if contentBuilder.Len() > 0 {
577+
return FlushStreamingTextResult{ToPrint: contentBuilder.String()}
578+
}
540579
return FlushStreamingTextResult{}
541580
}
542581

543-
var b strings.Builder
544582
if t.HasFlushed && seg.FlushedPos == 0 {
545-
b.WriteString(t.LeadingSeparator(SegmentText))
583+
contentBuilder.WriteString(t.LeadingSeparator(SegmentText))
546584
}
547585

548586
// Strip leading blank line if we've already flushed content,
549587
// since tea.Printf adds a newline after each flush
550588
if t.HasFlushed {
551589
rendered = stripLeadingBlankLine(rendered)
552590
}
553-
b.WriteString(rendered)
591+
contentBuilder.WriteString(rendered)
554592

555593
// Update tracker state
556594
seg.FlushedPos = safeBoundary
@@ -562,7 +600,7 @@ func (t *ToolTracker) FlushStreamingText(threshold int, width int, renderMd func
562600
debugFlushf("stream flush seg=%d committed=%d flushedPos=%d renderedUnflushedLen=%d flushedRenderedPos=%d", segIdx, safeBoundary, seg.FlushedPos, len(rendered), seg.FlushedRenderedPos)
563601

564602
return FlushStreamingTextResult{
565-
ToPrint: b.String(),
603+
ToPrint: contentBuilder.String(),
566604
}
567605
}
568606

@@ -571,51 +609,74 @@ func (t *ToolTracker) FlushStreamingText(threshold int, width int, renderMd func
571609
if safeBoundary <= seg.FlushedPos {
572610
// No safe boundary found, can't flush yet
573611
debugFlushf("stream skip seg=%d reason=no-safe-boundary flushedPos=%d textLen=%d", segIdx, seg.FlushedPos, len(fullText))
612+
// Return any tool content we already flushed
613+
if contentBuilder.Len() > 0 {
614+
return FlushStreamingTextResult{ToPrint: contentBuilder.String()}
615+
}
574616
return FlushStreamingTextResult{}
575617
}
576618

577619
// Fallback for when no streaming renderer is available (should be rare)
578620
toFlush := fullText[seg.FlushedPos:safeBoundary]
579621
if toFlush == "" {
580622
debugFlushf("stream skip seg=%d reason=empty-toFlush safeBoundary=%d flushedPos=%d", segIdx, safeBoundary, seg.FlushedPos)
623+
// Return any tool content we already flushed
624+
if contentBuilder.Len() > 0 {
625+
return FlushStreamingTextResult{ToPrint: contentBuilder.String()}
626+
}
581627
return FlushStreamingTextResult{}
582628
}
583629

584630
if renderMd == nil {
585631
debugFlushf("stream skip seg=%d reason=no-renderer safeBoundary=%d flushedPos=%d", segIdx, safeBoundary, seg.FlushedPos)
632+
// Return any tool content we already flushed
633+
if contentBuilder.Len() > 0 {
634+
return FlushStreamingTextResult{ToPrint: contentBuilder.String()}
635+
}
586636
return FlushStreamingTextResult{}
587637
}
588638

589639
// Render the full committed markdown so inter-block spacing is preserved.
590640
renderedAll := renderMd(fullText[:safeBoundary], width)
591641
if renderedAll == "" {
592642
debugFlushf("stream skip seg=%d reason=empty-rendered-fallback safeBoundary=%d flushedPos=%d", segIdx, safeBoundary, seg.FlushedPos)
643+
// Return any tool content we already flushed
644+
if contentBuilder.Len() > 0 {
645+
return FlushStreamingTextResult{ToPrint: contentBuilder.String()}
646+
}
593647
return FlushStreamingTextResult{}
594648
}
595649

596650
rendered := renderedAll
597651
if seg.FlushedRenderedPos > 0 {
598652
if seg.FlushedRenderedPos >= len(renderedAll) {
599653
debugFlushf("stream skip seg=%d reason=rendered-pos>=len safeBoundary=%d flushedRenderedPos=%d renderedLen=%d", segIdx, safeBoundary, seg.FlushedRenderedPos, len(renderedAll))
654+
// Return any tool content we already flushed
655+
if contentBuilder.Len() > 0 {
656+
return FlushStreamingTextResult{ToPrint: contentBuilder.String()}
657+
}
600658
return FlushStreamingTextResult{}
601659
}
602660
rendered = renderedAll[seg.FlushedRenderedPos:]
603661
}
604662
if rendered == "" {
605663
debugFlushf("stream skip seg=%d reason=empty-rendered-slice safeBoundary=%d flushedPos=%d", segIdx, safeBoundary, seg.FlushedPos)
664+
// Return any tool content we already flushed
665+
if contentBuilder.Len() > 0 {
666+
return FlushStreamingTextResult{ToPrint: contentBuilder.String()}
667+
}
606668
return FlushStreamingTextResult{}
607669
}
608670

609-
var b strings.Builder
610671
// Only add leading separator if this is the very FIRST flush for this segment.
611672
if t.HasFlushed && seg.FlushedPos == 0 {
612-
b.WriteString(t.LeadingSeparator(SegmentText))
673+
contentBuilder.WriteString(t.LeadingSeparator(SegmentText))
613674
}
614675
// Strip leading blank line since tea.Printf adds a newline after each flush
615676
if t.HasFlushed {
616677
rendered = stripLeadingBlankLine(rendered)
617678
}
618-
b.WriteString(rendered)
679+
contentBuilder.WriteString(rendered)
619680

620681
// Update flushed position/state
621682
seg.FlushedPos = safeBoundary
@@ -625,7 +686,7 @@ func (t *ToolTracker) FlushStreamingText(threshold int, width int, renderMd func
625686
debugFlushf("stream flush seg=%d safeBoundary=%d flushedPos=%d toFlushLen=%d renderedLen=%d", segIdx, safeBoundary, seg.FlushedPos, len(toFlush), len(rendered))
626687

627688
return FlushStreamingTextResult{
628-
ToPrint: b.String(),
689+
ToPrint: contentBuilder.String(),
629690
}
630691
}
631692

0 commit comments

Comments
 (0)