fix(ui): resolve TUI streaming rendering and background agent lifecycle issues - #265
Merged
Conversation
…ment Extract streaming event handling from the monolithic chat component into dedicated, testable modules: - parts/stream-pipeline.ts: unified event reducer for text, thinking, tool, HITL, and agent streaming events - utils/loading-state.ts: completion summary and loading indicator logic - utils/background-agent-footer.ts: active background agent resolution - utils/background-agent-termination.ts: Ctrl+F double-press termination - utils/background-agent-tree-hints.ts: parallel agents header hints - components/background-agent-footer.tsx: footer status component Additional fixes: - Normalize Windows line endings (CRLF) in markdown text handling - Apply text normalization to task tool result parsing - Expand guards with hasActiveForegroundAgents and shouldFinalizeDeferredStream Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
List environment provisioning issues causing test failures for protocol ordering, claude rendering, unified event parity, copilot client, and opencode events test suites. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Harden tool completion timing and preserve HITL responses when syncing tool parts. Improve streaming output rendering by removing text-part status prefixes, normalizing reasoning duration labels, and converting markdown task checkboxes to unicode symbols for reliable TUI display. Add focused tests covering duration formatting, invalid startedAt handling, and markdown checkbox normalization. Assistant-model: GitHub Copilot CLI Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…verage Assistant-model: openai/gpt-5.3-codex
These files were used during development and are no longer needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…line Propagate provider-native thinking source keys (block index, reasoning ID, part ID) through all three SDK clients (Claude, Copilot, OpenCode) and into the UI streaming pipeline. - Add thinkingSourceKey to MessageDeltaEventData and stream metadata - Track thinking source lifecycle (create/update/finalize/drop) with diagnostics support - Validate thinking-meta events against message ID and stream generation to prevent stale/cross-source bleed - Build stable React render keys from reasoning source identity - Filter pending ask-user questions from message bubble rendering - Add comprehensive tests for source identity, interleaving, and validation across all SDK clients Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add research documents for thinking source identity tracking and background agents UI, plus the implementation spec for thinking tag stream grouping. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…iminated union type
- Remove old BackgroundTerminationDecision interface from background-agent-termination.ts
- Import and re-export BackgroundTerminationDecision from background-agent-contracts.ts
- Update getBackgroundTerminationDecision to return discriminated union:
- { action: 'none' } when no active background agents
- { action: 'warn', message: '...' } on first press
- { action: 'terminate', message: '...' } on second press
- Update chat.tsx to use new discriminated union pattern
- Update all tests to match new return type
- All tests passing, no type errors
- Import and use BACKGROUND_FOOTER_CONTRACT in footer files - Replace hardcoded 'ctrl+f terminate' with contract value - Add contract validation tests - Add test for footer visibility threshold Tasks #7 + #8 complete. All footer UX now driven by the canonical contract, eliminating hardcoded behavior.
- Add TuiBackgroundTerminationEvent interface with action, activeAgentCount, interruptedCount - Add trackBackgroundTermination method to TuiTelemetrySessionTracker - Track noop/warn/execute counters for session summary - Include counters in TuiSessionEndEvent and TuiSessionSummary - Supports observability for Ctrl+F keyboard termination flow Related to tasks #11 and #12 in workflow
…transitions - Add console.debug call after decision computation with pressCount and activeAgents - Add debug log in none/noop branch - Add debug log in terminate branch with interruptedIds and remainingCount - Add debug log in warn/armed branch - All logs prefixed with [background-termination] for filtering - Uses console.debug for structured logging Related to task #11 in workflow
…mination - Create background-agent-keybinding-nonconflict.test.ts - Verify Ctrl+O (transcript toggle) does NOT conflict with Ctrl+F (termination) - Verify Ctrl+C (interruption) does NOT conflict with Ctrl+F (termination) - Test modifier exclusion (Ctrl+Shift+F, Ctrl+Meta+F not detected) - Comprehensive test of all common Ctrl+key combos (a-z) - All 8 tests pass with 32 expect() calls
Add comprehensive test suite verifying background agent contract functions produce deterministic, consistent results invariant across runtime paths (dev via 'bun run' vs compiled production binary). Per spec (specs/background-agents-ui-issue-258-parity-hardening.md), dev and production runtime paths share startChatUI entry point. Contract functions are pure JavaScript with no runtime-conditional branching. Test coverage: - Contract constants frozen/deterministic (BACKGROUND_FOOTER_CONTRACT, BACKGROUND_TREE_HINT_CONTRACT) - Pure function determinism (getBackgroundTerminationDecision, interruptActiveBackgroundAgents, getActiveBackgroundAgents, buildParallelAgentsHeaderHint, formatBackgroundAgentFooterStatus) - Idempotency (multiple calls with same args yield same result) - No environment-conditional branching (no process.env/import.meta checks) - Module import stability (all exports accessible with expected types) - Function signature stability (parameter counts remain consistent) This is a 'canary' test documenting and enforcing invariance rather than testing complex logic. Issue #258 Task #20
…tracts - Create fixture-based acceptance tests at background-agent-acceptance.test.ts - Validate exact footer text/behavior: 'ctrl+f terminate' hint, agent count visibility - Validate Ctrl+F double-press flow: warn → terminate → agent termination confirmation - Validate tree hints: running/complete/default states with exact wording - Test cross-surface consistency: ctrl+f/ctrl+o references, 'terminate' keyword - Test UX polish: separator style (·), lowercase keybindings, pluralization - All 21 acceptance tests pass, providing machine-readable screenshot equivalents - Tests serve as canonical specification for issue #258 expected behavior
…tation (task #22) - Add 'test:contracts' script to package.json for running contract parity tests - Document CI enforcement in background-agent-contracts.ts JSDoc - Contract tests automatically run in CI via 'bun test' command - Lefthook pre-commit hook runs 'bun test --bail' which includes contract tests - All 116 contract parity tests passing (provider, runtime, acceptance, etc.)
#1) - Add background detection for input.mode === 'background' at line 644 (tool.start handler) - Add background detection for input.mode === 'background' at line 704 (tool.start handler) - Add background detection for fallbackInput?.mode === 'background' at line 1091-1093 (subagent.start handler) This ensures Copilot's built-in task tool mode parameter is properly detected in addition to the existing run_in_background flag.
…ents (task #2) Relaxes the second correlation guard in the subagent.start handler to also allow session-owned events, not just events with pendingTaskEntry or sdkCorrelationMatch. This supports SDKs like Copilot that dispatch custom agents without a Task tool, by allowing session-owned events during active streaming. Changes: - Modified line ~1073 in src/ui/index.ts to include '&& !sessionOwned' check - Updated comment to explain the rationale for session-owned event allowance Testing: - All 1676 tests pass - No type errors (bun typecheck passes)
…ation - Add toolCallId field to subagent.start events for agent parts - Use part.callID as primary correlation ID, fallback to part.id - Enables SDK correlation in UI layer for agent event tracking - Matches correlation pattern used in tool parts - All tests passing (1676 tests)
…ation (task #9) - Add 35 tests verifying the relaxed correlation guard logic - Tests cover both guards at lines 1068 and 1073 in src/ui/index.ts - Verify session-owned events pass through without pendingTaskEntry or SDK correlation - Verify non-session-owned events without correlation are still blocked - Add real-world scenario tests for Copilot, Claude, and OpenCode flows - Add edge case tests and regression tests for existing flows - All 1711 tests pass including new guard relaxation tests
Add temporary debug logging at key event emission points in OpenCode SDK client: - tool.start events: log toolName, toolId, and hasToolInput - subagent.start from agent parts: log partType, subagentId, subagentType, toolCallId - subagent.start from subtask parts: log partType, subagentId, subagentType Debug logging is gated behind process.env.ATOMIC_DEBUG flag. This enables runtime verification of: - Whether tool.start fires with correct toolName (Task vs task) - Whether subagent.start fires from agent/subtask part types - What fields are present in the event data No logic changes, only observability improvements for development.
- Add test for subagent.started → subagent.start with enriched data (toolCallId, task) - Add test for task fallback priority: description → prompt → agentName - Add test for subagent.completed → subagent.complete with success: true - Add test for subagent.failed → subagent.complete with success: false and error - All 10 tests passing, verifying event mapping logic in copilot.ts
…icate agent trees - Add separateAndInterruptAgents helper to only interrupt foreground agents while preserving background agents during Ctrl+C - Guard mergeParallelAgentsIntoParts to skip when agent parts already exist from streaming, preventing duplicate agent tree rendering - Preserve background agents across resetParallelTracking during interrupt - Fix background termination (Ctrl+F) to clear agents from state and abort SDK session only when not streaming - Update footer resolver and contracts to use consistent naming conventions - Update all related tests to match new behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nt filtering Replace inline `a.background && a.status === "background"` filter patterns with the shared getActiveBackgroundAgents utility across all occurrences in chat.tsx for consistency and maintainability. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
lavaman131
force-pushed
the
fix/tui-streaming-rendering
branch
from
February 23, 2026 06:56
9c7f51e to
2536309
Compare
claude
Bot
changed the title
PR Review: fix(ui): fix TUI streaming rendering and background agent managementThis is a substantial PR (10,948 additions, 714 deletions across 60+ files) that addresses multiple related issues (#259, #258, #254, #248, #231) around streaming content rendering and background agent lifecycle management. Overall Assessment: Approve with Minor SuggestionsThe PR demonstrates excellent architectural decisions with well-structured code, comprehensive test coverage, and clear documentation. The code quality is high overall. Strengths1. Excellent Code Architecture
2. Strong Test Coverage
3. Contract-Driven Development
4. SDK Enhancements
Suggestions for Improvement1. Code Duplication: 2. Magic Strings: Consider extracting the keybinding string ( 3. Type Safety: In Potential Issues to Verify1. WeakMap Memory: The 2. Duration Calculation: The invalid date handling in Performance Considerations
Final VerdictApprove - Well-architected PR with strong test coverage and clear documentation. Minor suggestions are optional improvements. Key improvements:
|
Merged
lavaman131
added a commit
that referenced
this pull request
Mar 26, 2026
…265) * refactor(ui): extract stream pipeline and add background agent management Extract streaming event handling from the monolithic chat component into dedicated, testable modules: - parts/stream-pipeline.ts: unified event reducer for text, thinking, tool, HITL, and agent streaming events - utils/loading-state.ts: completion summary and loading indicator logic - utils/background-agent-footer.ts: active background agent resolution - utils/background-agent-termination.ts: Ctrl+F double-press termination - utils/background-agent-tree-hints.ts: parallel agents header hints - components/background-agent-footer.tsx: footer status component Additional fixes: - Normalize Windows line endings (CRLF) in markdown text handling - Apply text normalization to task tool result parsing - Expand guards with hasActiveForegroundAgents and shouldFinalizeDeferredStream Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(research): add branch task breakdown for TUI streaming rendering Document the grouped issues (#259, #258, #254, #248, #231) being addressed on the fix/tui-streaming-rendering branch with rationale for their grouping under the streaming content rendering pipeline. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: track cross-agent E2E dependency blockers List environment provisioning issues causing test failures for protocol ordering, claude rendering, unified event parity, copilot client, and opencode events test suites. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(ui): resolve streaming render inconsistencies Harden tool completion timing and preserve HITL responses when syncing tool parts. Improve streaming output rendering by removing text-part status prefixes, normalizing reasoning duration labels, and converting markdown task checkboxes to unicode symbols for reliable TUI display. Add focused tests covering duration formatting, invalid startedAt handling, and markdown checkbox normalization. Assistant-model: GitHub Copilot CLI Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(ui): add thinking stream interleaving and handoff integration coverage Assistant-model: openai/gpt-5.3-codex * chore: remove resolved issues tracker and debug screenshot These files were used during development and are no longer needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(sdk,ui): add thinking source identity tracking to streaming pipeline Propagate provider-native thinking source keys (block index, reasoning ID, part ID) through all three SDK clients (Claude, Copilot, OpenCode) and into the UI streaming pipeline. - Add thinkingSourceKey to MessageDeltaEventData and stream metadata - Track thinking source lifecycle (create/update/finalize/drop) with diagnostics support - Validate thinking-meta events against message ID and stream generation to prevent stale/cross-source bleed - Build stable React render keys from reasoning source identity - Filter pending ask-user questions from message bubble rendering - Add comprehensive tests for source identity, interleaving, and validation across all SDK clients Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: add thinking tag stream grouping research and spec Add research documents for thinking source identity tracking and background agents UI, plus the implementation spec for thinking tag stream grouping. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor: align getBackgroundTerminationDecision with canonical discriminated union type - Remove old BackgroundTerminationDecision interface from background-agent-termination.ts - Import and re-export BackgroundTerminationDecision from background-agent-contracts.ts - Update getBackgroundTerminationDecision to return discriminated union: - { action: 'none' } when no active background agents - { action: 'warn', message: '...' } on first press - { action: 'terminate', message: '...' } on second press - Update chat.tsx to use new discriminated union pattern - Update all tests to match new return type - All tests passing, no type errors * refactor: align footer resolver and component with canonical contract - Import and use BACKGROUND_FOOTER_CONTRACT in footer files - Replace hardcoded 'ctrl+f terminate' with contract value - Add contract validation tests - Add test for footer visibility threshold Tasks #7 + #8 complete. All footer UX now driven by the canonical contract, eliminating hardcoded behavior. * feat(telemetry): add background termination tracking and metrics - Add TuiBackgroundTerminationEvent interface with action, activeAgentCount, interruptedCount - Add trackBackgroundTermination method to TuiTelemetrySessionTracker - Track noop/warn/execute counters for session summary - Include counters in TuiSessionEndEvent and TuiSessionSummary - Supports observability for Ctrl+F keyboard termination flow Related to tasks #11 and #12 in workflow * feat(ui): add structured debug logs for background termination state transitions - Add console.debug call after decision computation with pressCount and activeAgents - Add debug log in none/noop branch - Add debug log in terminate branch with interruptedIds and remainingCount - Add debug log in warn/armed branch - All logs prefixed with [background-termination] for filtering - Uses console.debug for structured logging Related to task #11 in workflow * test(ui): add parent callback integration tests for background agent termination * test(ui): add Ctrl+O non-conflict integration test for background termination - Create background-agent-keybinding-nonconflict.test.ts - Verify Ctrl+O (transcript toggle) does NOT conflict with Ctrl+F (termination) - Verify Ctrl+C (interruption) does NOT conflict with Ctrl+F (termination) - Test modifier exclusion (Ctrl+Shift+F, Ctrl+Meta+F not detected) - Comprehensive test of all common Ctrl+key combos (a-z) - All 8 tests pass with 32 expect() calls * test(ui): add E2E provider parity matrix tests for background agent contracts * test(ui): add E2E runtime parity tests for background agent contracts Add comprehensive test suite verifying background agent contract functions produce deterministic, consistent results invariant across runtime paths (dev via 'bun run' vs compiled production binary). Per spec (specs/background-agents-ui-issue-258-parity-hardening.md), dev and production runtime paths share startChatUI entry point. Contract functions are pure JavaScript with no runtime-conditional branching. Test coverage: - Contract constants frozen/deterministic (BACKGROUND_FOOTER_CONTRACT, BACKGROUND_TREE_HINT_CONTRACT) - Pure function determinism (getBackgroundTerminationDecision, interruptActiveBackgroundAgents, getActiveBackgroundAgents, buildParallelAgentsHeaderHint, formatBackgroundAgentFooterStatus) - Idempotency (multiple calls with same args yield same result) - No environment-conditional branching (no process.env/import.meta checks) - Module import stability (all exports accessible with expected types) - Function signature stability (parameter counts remain consistent) This is a 'canary' test documenting and enforcing invariance rather than testing complex logic. Issue #258 Task #20 * test(ui): add acceptance tests for issue #258 background agent UX contracts - Create fixture-based acceptance tests at background-agent-acceptance.test.ts - Validate exact footer text/behavior: 'ctrl+f terminate' hint, agent count visibility - Validate Ctrl+F double-press flow: warn → terminate → agent termination confirmation - Validate tree hints: running/complete/default states with exact wording - Test cross-surface consistency: ctrl+f/ctrl+o references, 'terminate' keyword - Test UX polish: separator style (·), lowercase keybindings, pluralization - All 21 acceptance tests pass, providing machine-readable screenshot equivalents - Tests serve as canonical specification for issue #258 expected behavior * chore(ci): add contract parity test script and CI enforcement documentation (task #22) - Add 'test:contracts' script to package.json for running contract parity tests - Document CI enforcement in background-agent-contracts.ts JSDoc - Contract tests automatically run in CI via 'bun test' command - Lefthook pre-commit hook runs 'bun test --bail' which includes contract tests - All 116 contract parity tests passing (provider, runtime, acceptance, etc.) * feat(ui): add mode==='background' detection for Copilot task tool (task #1) - Add background detection for input.mode === 'background' at line 644 (tool.start handler) - Add background detection for input.mode === 'background' at line 704 (tool.start handler) - Add background detection for fallbackInput?.mode === 'background' at line 1091-1093 (subagent.start handler) This ensures Copilot's built-in task tool mode parameter is properly detected in addition to the existing run_in_background flag. * feat(ui): relax subagent.start correlation guard for session-owned events (task #2) Relaxes the second correlation guard in the subagent.start handler to also allow session-owned events, not just events with pendingTaskEntry or sdkCorrelationMatch. This supports SDKs like Copilot that dispatch custom agents without a Task tool, by allowing session-owned events during active streaming. Changes: - Modified line ~1073 in src/ui/index.ts to include '&& !sessionOwned' check - Updated comment to explain the rationale for session-owned event allowance Testing: - All 1676 tests pass - No type errors (bun typecheck passes) * feat(sdk): add toolCallId to OpenCode agent part events for UI correlation - Add toolCallId field to subagent.start events for agent parts - Use part.callID as primary correlation ID, fallback to part.id - Enables SDK correlation in UI layer for agent event tracking - Matches correlation pattern used in tool parts - All tests passing (1676 tests) * feat(sdk): enrich Copilot subagent.started event with toolCallId and task (task #3) * test(ui): add comprehensive unit tests for subagent.start guard relaxation (task #9) - Add 35 tests verifying the relaxed correlation guard logic - Tests cover both guards at lines 1068 and 1073 in src/ui/index.ts - Verify session-owned events pass through without pendingTaskEntry or SDK correlation - Verify non-session-owned events without correlation are still blocked - Add real-world scenario tests for Copilot, Claude, and OpenCode flows - Add edge case tests and regression tests for existing flows - All 1711 tests pass including new guard relaxation tests * feat(sdk): add debug logging for OpenCode event verification (task #5) Add temporary debug logging at key event emission points in OpenCode SDK client: - tool.start events: log toolName, toolId, and hasToolInput - subagent.start from agent parts: log partType, subagentId, subagentType, toolCallId - subagent.start from subtask parts: log partType, subagentId, subagentType Debug logging is gated behind process.env.ATOMIC_DEBUG flag. This enables runtime verification of: - Whether tool.start fires with correct toolName (Task vs task) - Whether subagent.start fires from agent/subtask part types - What fields are present in the event data No logic changes, only observability improvements for development. * test(sdk): add comprehensive tests for Copilot subagent event mapping - Add test for subagent.started → subagent.start with enriched data (toolCallId, task) - Add test for task fallback priority: description → prompt → agentName - Add test for subagent.completed → subagent.complete with success: true - Add test for subagent.failed → subagent.complete with success: false and error - All 10 tests passing, verifying event mapping logic in copilot.ts * test(sdk): add comprehensive tests for OpenCode agent event mapping (task #8) * fix(ui): preserve background agents across interrupt and prevent duplicate agent trees - Add separateAndInterruptAgents helper to only interrupt foreground agents while preserving background agents during Ctrl+C - Guard mergeParallelAgentsIntoParts to skip when agent parts already exist from streaming, preventing duplicate agent tree rendering - Preserve background agents across resetParallelTracking during interrupt - Fix background termination (Ctrl+F) to clear agents from state and abort SDK session only when not streaming - Update footer resolver and contracts to use consistent naming conventions - Update all related tests to match new behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor(ui): use getActiveBackgroundAgents helper for background agent filtering Replace inline `a.background && a.status === "background"` filter patterns with the shared getActiveBackgroundAgents utility across all occurrences in chat.tsx for consistency and maintainability. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: lavaman131 <dev@example.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Comprehensive fix for TUI streaming rendering inconsistencies and background agent lifecycle management across all provider SDKs (Claude, OpenCode, Copilot). Addresses issues #258, #259, #254, #248, and #231 related to agent tree rendering, background agent persistence, and termination flows.
Key Changes
Streaming Rendering Fixes
mergeParallelAgentsIntoPartsto skip when agent parts already exist from streaming, preventing duplicate renderingBackground Agent Lifecycle Management
separateAndInterruptAgentshelper that only interrupts foreground agents while keeping background agents alivemode==='background'detection for Copilot's task tool (fixes Copilot background agent rendering)SDK Event Enhancements
toolCallIdandtaskfields for UI correlationInfrastructure & Telemetry
test:contracts) and CI enforcement documentationComprehensive Test Coverage
Architecture
Stream Pipeline Refactoring
stream-pipeline.tsmodule with single event reducer patternBackground Agent State Management
backgroundAgentMessageIdRefBreaking Changes
None. All changes are backward compatible.
Related Issues
Testing
Run the contract parity test suite:
Run all tests:
bun test