fix(tui): Fix garbled output when content exceeds viewport

- Implemented new renderLineBased method that properly handles scrollback boundaries
- Fixed ANSI code preservation in MarkdownComponent line wrapping
- Added comprehensive test to reproduce and verify the fix
- Root cause: PARTIAL rendering strategy couldn't position cursor in scrollback
- Solution: Component-agnostic line comparison with proper viewport boundary handling
This commit is contained in:
Mario Zechner 2025-08-11 14:17:46 +02:00
parent 1d9b77298c
commit 6e40c5d761
6 changed files with 485 additions and 106 deletions

View file

@ -0,0 +1,93 @@
# TUI Garbled Output Analysis
## Problem Description
When reading multiple README.md files and then sending a new message, the TUI displays garbled output. This happens for both renderDifferential and renderDifferentialSurgical methods, affecting any model (not just gpt-5).
## Rendering System Overview
### Three Rendering Strategies
1. **SURGICAL Updates** - Updates only changed lines (1-2 lines typical)
2. **PARTIAL Re-render** - Clears from first change to end, re-renders tail
3. **FULL Re-render** - Clears scrollback and screen, renders everything
### Key Components
- **TUI Class** (`packages/tui/src/tui.ts`): Main rendering engine
- **Container Class**: Manages child components, auto-triggers re-renders
- **TuiRenderer** (`packages/agent/src/renderers/tui-renderer.ts`): Agent's TUI integration
- **Event System**: Event-driven updates through AgentEvent
## Root Causes Identified
### 1. Complex ANSI Code Handling
- MarkdownComponent line wrapping has issues with ANSI escape sequences
- Code comment at line 203: "Need to wrap - this is complex with ANSI codes"
- ANSI codes can be split across render operations, causing corruption
### 2. Race Conditions in Rapid Updates
When processing multiple tool calls:
- Multiple containers change simultaneously
- Content added both above and within viewport
- Surgical renderer handles structural changes while maintaining cursor position
- Heavy ANSI content (colored tool output, markdown) increases complexity
### 3. Cursor Position Miscalculation
- Rapid updates can cause cursor positioning logic errors
- Content shifts due to previous renders not properly accounted for
- Viewport vs scrollback buffer calculations can become incorrect
### 4. Container Change Detection Timing
- Recent fix (192d8d2) addressed container clear detection
- But rapid component addition/removal still may leave artifacts
- Multiple render requests debounced but may miss intermediate states
## Specific Scenario Analysis
### Sequence When Issue Occurs:
1. User sends "read all README.md files"
2. Multiple tool calls execute rapidly:
- glob() finds files
- Multiple read() calls for each README
3. Long file contents displayed with markdown formatting
4. User sends new message while output still rendering
5. New components added while previous render incomplete
### Visual Artifacts Observed:
- Text overlapping from different messages
- Partial ANSI codes causing color bleeding
- Editor borders duplicated or misaligned
- Content from previous render persisting
- Line wrapping breaking mid-word with styling
## Related Fixes
- Commit 1d9b772: Fixed ESC interrupt handling race conditions
- Commit 192d8d2: Fixed container change detection for clear operations
- Commit 2ec8a27: Added instructional header to chat demo
## Test Coverage Gaps
- No tests for rapid multi-tool execution scenarios
- Missing tests for ANSI code handling across line wraps
- No stress tests for viewport overflow with rapid updates
- Layout shift artifacts test exists but limited scope
## Recommended Solutions
### 1. Improve ANSI Handling
- Fix MarkdownComponent line wrapping to preserve ANSI codes
- Ensure escape sequences never split across operations
- Add ANSI-aware string measurement utilities
### 2. Add Render Queuing
- Implement render operation queue to prevent overlaps
- Ensure each render completes before next begins
- Add render state tracking
### 3. Enhanced Change Detection
- Track render generation/version numbers
- Validate cursor position before surgical updates
- Add checksums for rendered content verification
### 4. Comprehensive Testing
- Create test simulating exact failure scenario
- Add stress tests with rapid multi-component updates
- Test ANSI-heavy content with line wrapping
- Verify viewport calculations under load

View file

@ -0,0 +1,35 @@
# Fix TUI Garbled Output When Sending Multiple Messages
**Status:** InProgress
**Agent PID:** 54802
## Original Todo
agent/tui: "read all README.md files except in node_modules". wait for completion, then send a new message. Getting garbled output. this happens for both of the renderDifferential and renderDifferentialSurgical methods. We need to emulate this in a test and get to the bottom of it.
## Description
Fix the TUI rendering corruption that occurs when sending multiple messages in rapid succession, particularly after tool calls that produce large outputs. The issue manifests as garbled/overlapping text when new messages are sent while previous output is still being displayed.
*Read [analysis.md](./analysis.md) in full for detailed codebase research and context*
## Implementation Plan
[how we are building it]
- [x] Create test to reproduce the issue: Simulate rapid tool calls with large outputs followed by new message
- [x] Fix ANSI code handling in MarkdownComponent line wrapping (packages/tui/src/components/markdown-component.ts:203-276)
- [x] Implement new line-based rendering strategy that properly handles scrollback and viewport boundaries
- [x] Add comprehensive test coverage for multi-message scenarios
- [ ] User test: Run agent, execute "read all README.md files", wait for completion, send new message, verify no garbled output
## Notes
- Successfully reproduced the issue with test showing garbled text overlay
- Fixed ANSI code handling in MarkdownComponent line wrapping
- Root cause: PARTIAL rendering strategy incorrectly calculated cursor position when content exceeded viewport
- When content is in scrollback, cursor can't reach it (can only move within viewport)
- Old PARTIAL strategy tried to move cursor 33 lines up when only 30 were possible
- This caused cursor to land at wrong position (top of viewport instead of target line in scrollback)
- Solution: Implemented new `renderLineBased` method that:
- Compares old and new lines directly (component-agnostic)
- Detects if changes are in scrollback (unreachable) or viewport
- For scrollback changes: does full clear and re-render
- For viewport changes: moves cursor correctly within viewport bounds and updates efficiently
- Handles surgical line-by-line updates when possible for minimal redraws
- Test now passes - no more garbled output when messages exceed viewport!