mirror of
https://github.com/getcompanion-ai/co-mono.git
synced 2026-04-20 16:05:11 +00:00
tui-double-buffer: Implement smart differential rendering with terminal abstraction
- Create Terminal interface abstracting stdin/stdout operations for dependency injection - Implement ProcessTerminal for production use with process.stdin/stdout - Implement VirtualTerminal using @xterm/headless for accurate terminal emulation in tests - Fix TypeScript imports for @xterm/headless module - Move all component files to src/components/ directory for better organization - Add comprehensive test suite with async/await patterns for proper render timing - Fix critical TUI differential rendering bug when components grow in height - Issue: Old content wasn't properly cleared when component line count increased - Solution: Clear each old line individually before redrawing, ensure cursor at line start - Add test verifying terminal content preservation and text editor growth behavior - Update tsconfig.json to include test files in type checking - Add benchmark test comparing single vs double buffer performance The implementation successfully reduces flicker by only updating changed lines rather than clearing entire sections. Both TUI implementations maintain the same interface for backward compatibility.
This commit is contained in:
parent
923a9e58ab
commit
afa807b200
19 changed files with 1591 additions and 344 deletions
132
todos/done/2025-08-10-14-40-44-tui-double-buffer-analysis.md
Normal file
132
todos/done/2025-08-10-14-40-44-tui-double-buffer-analysis.md
Normal file
|
|
@ -0,0 +1,132 @@
|
|||
# TUI Double Buffer Implementation Analysis
|
||||
|
||||
## Current Architecture
|
||||
|
||||
### Core TUI Rendering System
|
||||
- **Location:** `/Users/badlogic/workspaces/pi-mono/packages/tui/src/tui.ts`
|
||||
- **render()** method (lines 107-150): Traverses components, calculates keepLines
|
||||
- **renderToScreen()** method (lines 354-429): Outputs to terminal with differential rendering
|
||||
- **Terminal output:** Single `writeSync()` call at line 422
|
||||
|
||||
### Component Interface
|
||||
```typescript
|
||||
interface ComponentRenderResult {
|
||||
lines: string[];
|
||||
changed: boolean;
|
||||
}
|
||||
|
||||
interface ContainerRenderResult extends ComponentRenderResult {
|
||||
keepLines: number; // Lines from top that are unchanged
|
||||
}
|
||||
```
|
||||
|
||||
### The Flicker Problem
|
||||
|
||||
**Root Cause:**
|
||||
1. LoadingAnimation (`packages/agent/src/renderers/tui-renderer.ts`) updates every 80ms
|
||||
2. Calls `ui.requestRender()` on each frame, marking itself as changed
|
||||
3. Container's `keepLines` logic stops accumulating once any child changes
|
||||
4. All components below animation must re-render completely
|
||||
5. TextEditor always returns `changed: true` for cursor updates
|
||||
|
||||
**Current Differential Rendering:**
|
||||
- Moves cursor up by `(totalLines - keepLines)` lines
|
||||
- Clears everything from cursor down with `\x1b[0J`
|
||||
- Writes all lines after `keepLines` position
|
||||
- Creates visible flicker when large portions re-render
|
||||
|
||||
### Performance Bottlenecks
|
||||
|
||||
1. **TextEditor (`packages/tui/src/text-editor.ts`):**
|
||||
- Always returns `changed: true` (lines 122-125)
|
||||
- Complex `layoutText()` recalculates wrapping every render
|
||||
- Heavy computation for cursor positioning and highlighting
|
||||
|
||||
2. **Animation Cascade Effect:**
|
||||
- Single animated component forces all components below to re-render
|
||||
- Container stops accumulating `keepLines` after first change
|
||||
- No isolation between independent component updates
|
||||
|
||||
3. **Terminal I/O:**
|
||||
- Single large `writeSync()` call for all changing content
|
||||
- Clears and redraws entire sections even for minor changes
|
||||
|
||||
### Existing Optimizations
|
||||
|
||||
**Component Caching:**
|
||||
- TextComponent: Stores `lastRenderedLines[]`, compares arrays
|
||||
- MarkdownComponent: Uses `previousLines[]` comparison
|
||||
- WhitespaceComponent: `firstRender` flag
|
||||
- Components properly detect and report changes
|
||||
|
||||
**Render Batching:**
|
||||
- `requestRender()` uses `process.nextTick()` to batch updates
|
||||
- Prevents multiple renders in same tick
|
||||
|
||||
## Double Buffer Solution
|
||||
|
||||
### Architecture Benefits
|
||||
- Components already return `{lines, changed}` - no interface changes needed
|
||||
- Clean separation between rendering (back buffer) and output (terminal)
|
||||
- Single `writeSync()` location makes implementation straightforward
|
||||
- Existing component caching remains useful
|
||||
|
||||
### Implementation Strategy
|
||||
|
||||
**TuiDoubleBuffer Class:**
|
||||
1. Extend current TUI class
|
||||
2. Maintain front buffer (last rendered lines) and back buffer (new render)
|
||||
3. Override `renderToScreen()` with line-by-line diffing algorithm
|
||||
4. Batch consecutive changed lines to minimize writeSync() calls
|
||||
5. Position cursor only at changed lines, not entire sections
|
||||
|
||||
**Line-Level Diffing Algorithm:**
|
||||
```typescript
|
||||
// Pseudocode
|
||||
for (let i = 0; i < maxLines; i++) {
|
||||
if (frontBuffer[i] !== backBuffer[i]) {
|
||||
// Position cursor at line i
|
||||
// Clear line
|
||||
// Write new content
|
||||
// Or batch with adjacent changes
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Expected Benefits
|
||||
|
||||
1. **Reduced Flicker:**
|
||||
- Only changed lines are redrawn
|
||||
- Animation updates don't affect static content below
|
||||
- TextEditor cursor updates don't require full redraw
|
||||
|
||||
2. **Better Performance:**
|
||||
- Fewer terminal control sequences
|
||||
- Smaller writeSync() payloads
|
||||
- Components can cache aggressively
|
||||
|
||||
3. **Preserved Functionality:**
|
||||
- No changes to existing components
|
||||
- Backward compatible with current TUI class
|
||||
- Can switch between single/double buffer modes
|
||||
|
||||
### Test Plan
|
||||
|
||||
Create comparison tests:
|
||||
1. `packages/tui/test/single-buffer.ts` - Current implementation
|
||||
2. `packages/tui/test/double-buffer.ts` - New implementation
|
||||
3. Both with LoadingAnimation above TextEditor
|
||||
4. Measure render() timing and visual flicker
|
||||
|
||||
### Files to Modify
|
||||
|
||||
**New Files:**
|
||||
- `packages/tui/src/tui-double-buffer.ts` - New TuiDoubleBuffer class
|
||||
|
||||
**Test Files:**
|
||||
- `packages/tui/test/single-buffer.ts` - Test current implementation
|
||||
- `packages/tui/test/double-buffer.ts` - Test new implementation
|
||||
|
||||
**No Changes Needed:**
|
||||
- Component implementations (already support caching and change detection)
|
||||
- Component interfaces (already return required data)
|
||||
92
todos/done/2025-08-10-14-40-44-tui-double-buffer.md
Normal file
92
todos/done/2025-08-10-14-40-44-tui-double-buffer.md
Normal file
|
|
@ -0,0 +1,92 @@
|
|||
# TUI Double Buffer Implementation
|
||||
|
||||
**Status:** Done
|
||||
**Agent PID:** 74014
|
||||
|
||||
## Original Todo
|
||||
- tui: we get tons of flicker in the text editor component. specifically, if we have an animated component above the editor, the editor needs re-rendering completely. Different strategy:
|
||||
- keep a back buffer and front buffer. a buffer is a list of lines.
|
||||
- on Tui.render()
|
||||
- render a new back buffer, top to bottom. components can cache previous render results and return that as a single list of lines if nothing changed
|
||||
- compare the back buffer with the front buffer. for each line that changed
|
||||
- position the cursor at that line
|
||||
- clear the line
|
||||
- render the new line
|
||||
- batch multiple subsequent lines that changed so we do not have tons of writeSync() calls
|
||||
- Open questions:
|
||||
- is this faster and procudes less flicker?
|
||||
- If possible, we should implement this as a new TuiDoubleBuffer class. Existing components should not need changing, as they already report if they changed and report their lines
|
||||
- Testing:
|
||||
- Create a packages/tui/test/single-buffer.ts file: it has a LoadingAnimation like in packages/agent/src/renderers/tui-renderer.ts inside a container as the first child, and a text editor component as the second child, which is focused.
|
||||
- Create a packages/tui/test/double-buffer.ts file: same setup
|
||||
- Measure timing of render() for both
|
||||
|
||||
## Description
|
||||
Implement a double-buffering strategy for the TUI rendering system to eliminate flicker when animated components (like LoadingAnimation) are displayed above interactive components (like TextEditor). The solution will use line-by-line diffing between a front buffer (previous render) and back buffer (current render) to only update changed lines on the terminal, replacing the current section-based differential rendering.
|
||||
|
||||
*Read [analysis.md](./analysis.md) in full for detailed codebase research and context*
|
||||
|
||||
## Implementation Plan
|
||||
- [x] Create TuiDoubleBuffer class extending Container with same interface as TUI (`packages/tui/src/tui-double-buffer.ts`)
|
||||
- [x] Implement line-by-line diffing algorithm in overridden renderToScreen() method
|
||||
- [x] Add batching logic to group consecutive changed lines for efficient terminal writes
|
||||
- [x] Create test file with current single-buffer implementation (`packages/tui/test/single-buffer.ts`)
|
||||
- [x] Create test file with new double-buffer implementation (`packages/tui/test/double-buffer.ts`)
|
||||
- [x] Add timing measurements to both test files to compare performance
|
||||
- [x] Manual test: Run both test files to verify reduced flicker in double-buffer version
|
||||
- [x] Manual test: Verify existing TUI functionality still works with original class
|
||||
- [x] Fix cursor positioning bug in double-buffer implementation (stats appear at top, components don't update)
|
||||
- [x] Add write function parameter to both TUI classes for testability
|
||||
- [x] Create VirtualTerminal class for testing ANSI output
|
||||
- [x] Create verification test that compares both implementations
|
||||
- [x] Redesign double-buffer with proper cursor tracking to fix duplicate content issue
|
||||
- [x] Implement component-based rendering with unique IDs to handle reordering
|
||||
|
||||
## Additional Work Completed
|
||||
|
||||
### Terminal Abstraction & Testing Infrastructure
|
||||
- [x] Created Terminal interface abstracting stdin/stdout operations (`packages/tui/src/terminal.ts`)
|
||||
- [x] Implemented ProcessTerminal for production use with process.stdin/stdout
|
||||
- [x] Implemented VirtualTerminal using @xterm/headless for accurate terminal emulation in tests
|
||||
- [x] Fixed @xterm/headless TypeScript imports (changed from wildcard to proper named imports)
|
||||
- [x] Added test-specific methods to VirtualTerminal (flushAndGetViewport, writeSync)
|
||||
- [x] Updated TUI class to accept Terminal interface via constructor for dependency injection
|
||||
|
||||
### Component Organization
|
||||
- [x] Moved all component files to `packages/tui/src/components/` directory
|
||||
- [x] Updated all imports in index.ts and test files to use new paths
|
||||
|
||||
### Test Suite Updates
|
||||
- [x] Created comprehensive test suite for VirtualTerminal (`packages/tui/test/virtual-terminal.test.ts`)
|
||||
- [x] Updated TUI rendering tests to use async/await pattern for proper render timing
|
||||
- [x] Fixed all test assertions to work with exact output (no trim() allowed per user requirement)
|
||||
- [x] Fixed xterm newline handling (discovered \r\n requirement vs just \n)
|
||||
- [x] Added test for preserving existing terminal content when TUI starts and handles component growth
|
||||
|
||||
### Build Configuration
|
||||
- [x] Updated root tsconfig.json to include test files for type checking
|
||||
- [x] Ensured monorepo-wide type checking covers all source and test files
|
||||
|
||||
### Bug Fixes
|
||||
- [x] Fixed TUI differential rendering bug when components grow in height
|
||||
- Issue: Old content wasn't properly cleared when component line count increased
|
||||
- Solution: Clear each old line individually before redrawing, ensure cursor at line start
|
||||
- This prevents line-wrapping artifacts when the text editor grows (e.g., SHIFT+ENTER adding lines)
|
||||
|
||||
## Notes
|
||||
- Successfully implemented TuiDoubleBuffer class with line-by-line diffing
|
||||
- Complete redesign with proper cursor tracking:
|
||||
- Tracks actual cursor position separately from buffer length
|
||||
- Clear separation between screenBuffer and new render
|
||||
- Removed console.log/stdout.write interceptors per user request
|
||||
- Terminal abstraction enables proper testing without mocking process.stdin/stdout
|
||||
- VirtualTerminal provides accurate terminal emulation using xterm.js
|
||||
- Test results show significant reduction in flicker:
|
||||
- Single-buffer: Uses clear-down (`\x1b[0J`) which clears entire sections
|
||||
- Double-buffer: Uses clear-line (`\x1b[2K`) only for changed lines
|
||||
- Animation updates only affect the animation line, not the editor below
|
||||
- Performance similar between implementations (~0.4-0.6ms per render)
|
||||
- Both TUI and TuiDoubleBuffer maintain the same interface for backward compatibility
|
||||
- Can be used as drop-in replacement: just change `new TUI()` to `new TuiDoubleBuffer()`
|
||||
- All 22 tests passing with proper async handling and exact output matching
|
||||
- Fixed critical rendering bug in TUI's differential rendering for growing components
|
||||
|
|
@ -1,20 +1,3 @@
|
|||
- tui: we get tons of flicker in the text editor component. specifically, if we have an animated component above the editor, the editor needs re-rendering completely. Different strategy:
|
||||
- keep a back buffer and front buffer. a buffer is a list of lines.
|
||||
- on Tui.render()
|
||||
- render a new back buffer, top to bottom. components can cache previous render results and return that as a single list of lines if nothing changed
|
||||
- compare the back buffer with the front buffer. for each line that changed
|
||||
- position the cursor at that line
|
||||
- clear the line
|
||||
- render the new line
|
||||
- batch multiple subsequent lines that changed so we do not have tons of writeSync() calls
|
||||
- Open questions:
|
||||
- is this faster and procudes less flicker?
|
||||
- If possible, we should implement this as a new TuiDoubleBuffer class. Existing components should not need changing, as they already report if they changed and report their lines
|
||||
- Testing:
|
||||
- Create a packages/tui/test/single-buffer.ts file: it has a LoadingAnimation like in packages/agent/src/renderers/tui-renderer.ts inside a container as the first child, and a text editor component as the second child, which is focused.
|
||||
- Create a packages/tui/test/double-buffer.ts file: same setup
|
||||
- Measure timing of render() for both
|
||||
|
||||
- agent: improve reasoning section in README.md
|
||||
|
||||
- agent: ultrathink to temporarily set reasoning_effort?
|
||||
|
|
@ -44,7 +27,7 @@
|
|||
|
||||
- agent: token usage gets overwritten with each message that has usage data. however, if the latest data doesn't have a specific usage field, we record undefined i think? also, {"type":"token_usage" "inputTokens":240,"outputTokens":35,"totalTokens":275,"cacheReadTokens":0,"cacheWriteTokens":0} doesn't contain reasoningToken? do we lack initialization? See case "token_usage": in renderers. probably need to check if lastXXX > current and use lastXXX.
|
||||
|
||||
-agent: groq responses api throws on second message
|
||||
- agent: groq responses api throws on second message
|
||||
```
|
||||
➜ pi-mono git:(main) ✗ npx tsx packages/agent/src/cli.ts --base-url https://api.groq.com/openai/v1 --api-key $GROQ_API_KEY --model openai/gpt-oss-120b --api responses
|
||||
>> pi interactive chat <<<
|
||||
|
|
@ -82,6 +65,4 @@
|
|||
|
||||
- agent: start a new agent session. when i press CTRL+C, "Press Ctrl+C again to exit" appears above the text editor followed by an empty line. After about 1 second, the empty line disappears. We should either not show the empty line, or always show the empty line. Maybe Ctrl+C info should be displayed below the text editor.
|
||||
|
||||
- tui: npx tsx test/demo.ts, using /exit or pressing CTRL+C does not work to exit the demo.
|
||||
|
||||
- agent: we need to make system prompt and tools pluggable. We need to figure out the simplest way for users to define system prompts and toolkits. A toolkit could be a subset of the built-in tools, a mixture of a subset of the built-in tools plus custom self-made tools, maybe include MCP servers, and so on. We need to figure out a way to make this super easy. users should be able to write their tools in whatever language they fancy. which means that probably something like process spawning plus studio communication transport would make the most sense. but then we were back at MCP basically. And that does not support interruptibility, which we need for the agent. So if the agent invokes the tool and the user presses escape in the interface, then the tool invocation must be interrupted and whatever it's doing must stop, including killing all sub-processes. For MCP this could be solved for studio MCP servers by, since we spawn those on startup or whenever we load the tools, we spawn a process for an MCP server and then reuse that process for subsequent tool invocations. If the user interrupts then we could just kill that process, assuming that anything it's doing or any of its sub-processes will be killed along the way. So I guess tools could all be written as MCP servers, but that's a lot of overhead. It would also be nice to be able to provide tools just as a bash script that gets some inputs and return some outputs based on the inputs Same for Go apps or TypeScript apps invoked by MPX TSX. just make the barrier of entry for writing your own tools super fucking low. not necessarily going full MCP. but we also need to support MCP. So whatever we arrive at, we then need to take our built-in tools and see if those can be refactored to work with our new tools
|
||||
Loading…
Add table
Add a link
Reference in a new issue