Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-07-18
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
## Context

The TUI conversation panel in `src/tui/app.js` and `src/tui/conversationPanel.js` uses a pattern where messages are stored in a React state array and updated by cloning the array and mutating the last element. This approach was the only viable option before the adoption of `ink-scroll-view` which replaced the custom virtualized renderer (`messages.js`). However, the pattern has become a liability:

- Every streaming tick clones the full messages array and updates the last element, creating GC pressure
- The `ConversationPanel` owns both rendering and scroll logic, making it 365 lines and tightly coupled
- When a message is added, all messages are re-rendered even though only the new message should render
- App.js contains ~12 scattered `setMessages(prev => {...})` call sites across `handleChat`, `handleCommand`, `handleInterrupt`, and `handleNewSession`

The `ink-scroll-view` library (already in use) provides virtualized scrolling, meaning the React tree no longer needs to track every line. This makes a component-based approach (one component per message) viable — the ScrollView handles virtualization efficiently.

## Goals / Non-Goals

**Goals:**
- Each message is a standalone `MessageBubble` component with its own `useState`
- `MessageList` provides an imperative API (`addMessage`, `updateMessage`, `clear`, `setMessages`)
- Scroll management moves from `ConversationPanel` to `MessageList`
- App.js replaces array-mutation state updates with imperative ref calls
- Streaming text appears in real-time; user messages appear immediately
- No visual regression — same colors, layout, behavior as current UI

**Non-Goals:**
- Changing message styling or visual design
- Adding new message types (images, cards, etc.)
- Adding message search or filtering
- Changing the session loading/persistence mechanism
- Adding message reactions, editing, or deletion
- Migrating away from React state in App.js entirely (messages state remains as sync boundary)

## Decisions

### Decision 1: Imperative Update via Forwarded Ref
Use `React.forwardRef` on `MessageBubble` to expose an `update(partialState)` method. `MessageList` stores refs in a `Map<string, RefObject>`. When `MessageList.updateMessage(id, updates)` is called, it looks up the ref and calls its update method.

**Rationale:** Ink supports `React.forwardRef` (it's a React feature). This gives the same imperative control that the previous approach had with `messagesRef` mutations, but through React's component model instead of a plain mutable array.

**Alternatives considered:**
- **Context provider:** Pass a central update function via React Context. Adds overhead for a simple parent→child pattern. Refs are more direct.
- **State-driven updates:** Keep all state in `MessageList`, pass down props. This works for simple cases but makes streaming updates (cursor character cycling) harder because every prop change re-renders the entire list of bubbles.
- **Event emitter:** Use a simple pub/sub. Adds a dependency and indirection. Refs are standard React.

### Decision 2: Internal streamingId Counter in MessageBubble
Each `MessageBubble` tracks a `streamingId` state counter. Streaming updates increment this counter (e.g., every tick of the cursor animation or every streaming character). This ensures Ink re-renders the component even when content hasn't changed visually.

**Rationale:** Ink may batch renders. If the content string is identical, Ink might skip re-render. The counter acts as a "bump" to force updates. This is the same pattern some Ink apps use for cursor animation.

**Alternatives considered:**
- **useEffect dependency:** Use a separate `key` prop on the component. Changes the component identity on every render, losing scroll position and DOM refs. Not compatible with ink-scroll-view which needs stable children IDs.
- **CSS animation:** Cursor blinking via CSS. Ink supports limited CSS. The `\u2588` character is fine, but we need a reliable way to force re-render for smooth cursor.

### Decision 3: MessageList Owns ScrollView and Scroll Logic
All scroll management (throttle, resize, manual scroll detection, keyboard scroll) moves from `ConversationPanel` to `MessageList`. `ConversationPanel` becomes a 10-line wrapper.

**Rationale:** The ScrollView is part of `MessageList`'s visual output. Having MessageList own scroll behavior keeps the component self-contained. ConversationPanel can forward the ScrollView ref to App.js for keyboard navigation via a `scrollRef` prop or method.

**Alternatives considered:**
- **Keep scroll in ConversationPanel:** Would require passing MessageList ref to ConversationPanel for scroll-to-bottom. More indirection.

### Decision 4: Keep messages useState in App.js as Sync Boundary
The `messages` state in App.js is not removed. It stays as the source of truth for session persistence and is synced into MessageList via an `initialize(msgs)` call when a session loads.

**Rationale:** MessageList needs to know what messages to display on session restore. The App.js → MessageList interface is: initial state via prop, subsequent updates via imperative methods. This hybrid approach is simpler than making MessageList the sole source of truth and dealing with session state synchronization complexity.

## Risks / Trade-offs

1. **[Risk: Ink forwardRef compatibility]** Ink might not properly forward refs on functional components.
→ [Mitigation: Test with a simple forwarded-ref component first. If it fails, fall back to a callback ref pattern or a MessageContext pattern.]

2. **[Risk: Scroll regression]** The scroll behavior (throttle, manual-detection) is non-trivial. Moving it could introduce regressions.
→ [Mitigation: Port the scroll code verbatim from ConversationPanel.js (lines 260-353), only changing the ref targets. Test with the existing `tests/unit/tui/conversationPanel.test.js` as reference.]

3. **[Risk: Bubble ID stability]** Message IDs must be stable across renders for `updateMessage` to find the right bubble.
→ [Mitigation: Use a monotonic counter (assigned at add time) rather than random IDs. Map ID → ref, not ref → ID.]

## Migration Plan

1. Create `messageBubble.js` and `messageList.js` with all new code
2. Update `conversationPanel.js` to be a thin wrapper
3. Update `app.js` — replace all `setMessages` with imperative calls
4. Run tests and fix any failures
5. Run `npm run lint` and fix any lint issues
6. Run `npm start` briefly to verify the app boots without errors

## Open Questions

1. Should the cursor character be sourced globally (context) or per-component (prop from MessageList from app props)?
2. Is a monotonic counter sufficient for bubble IDs, or should we use `crypto.randomUUID()` for uniqueness across session loads? (Answer for now: monotonic counter is simpler and sufficient within a session.)
3. How should `MessageBubble` handle empty content during streaming (just a blinking cursor)?
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
## Why

The current TUI message rendering uses a mutable array pattern that fights React's rendering model: messages are stored via state (`useState`), but updates are done by cloning the full array (`[...prev]`) and mutating the last element. This happens on every streaming tick, causing memory allocation churn, unnecessary re-renders of unchanged messages, and unreliable scroll behavior. The streaming handler clones the array on every character to append a cursor character, compounding the issue.

## What Changes

- **New:** `src/tui/messageBubble.js` — a self-contained MessageBubble component with internal state (content, streaming, reasoningContent, activeToolCall, toolCallDisplay, streamingId) and an imperative update method exposed via forwarded ref
- **New:** `src/tui/messageList.js` — a MessageList component that manages an array of MessageBubble instances, provides imperative addMessage/updateMessage/clear/setMessages methods, and owns scroll management (ScrollView, throttle, resize, manual scroll detection)
- **Modified:** `src/tui/conversationPanel.js` — simplified to a thin wrapper that renders MessageList, no longer owns scroll logic
- **Modified:** `src/tui/app.js` — replaces all `setMessages([...])` calls with imperative `messageListRef.current.addMessage()` and `.updateMessage()` calls; removes streaming array mutations across ~12 call sites
- **No breaking API changes** for external users of the TUI — the App component still accepts the same props (config, registry, sessionState, etc.)

## Capabilities

### New Capabilities
- **component-message-bubbles**: Component-based message rendering where each message is a standalone MessageBubble with its own state and imperative update API, eliminating array-spread-and-mutate rendering patterns

### Modified Capabilities
- **tui-conversation**: Requirement behavior unchanged, but implementation moves from data-driven rendering to component-instance management. The "conversation persistence on interruption" requirement still applies.
- **tui-scroll-view**: The scroll management requirement shifts from ConversationPanel to MessageList. MessageList becomes the entity that "handles keyboard scroll input", "handles terminal resize", and "owns scroll state". ConversationPanel remains the visual container but delegates scroll responsibility.

## Impact

- **Files created:** `src/tui/messageBubble.js`, `src/tui/messageList.js`
- **Files modified:** `src/tui/conversationPanel.js`, `src/tui/app.js`
- **Files unchanged:** `src/tui/messages.js` (reused for `getRoleLabel`, utilities)
- **Test files to update:** `tests/unit/tui/` — all TUI tests that mock `setMessages` or the messages state will need to mock the new imperative API
- **No new dependencies** — all existing dependencies (ink, ink-scroll-view, React) are sufficient
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
## ADDED Requirements

### Requirement: MessageBubble manages its own state
The MessageBubble component SHALL maintain internal state for content, streaming status, reasoning content, active tool call, and tool call display via useState. Each bubble is a standalone React functional component.

#### Scenario: MessageBubble renders initial content
- **WHEN** a MessageBubble is created with role, content, and time
- **THEN** it renders the role label and content area with the initial content

#### Scenario: MessageBubble updates content imperatively
- **WHEN** the parent calls the exposed update() method with a content update
- **THEN** the bubble's internal state is updated and it re-renders with the new content

#### Scenario: MessageBubble shows streaming indicator
- **WHEN** a MessageBubble's internal streaming state is true
- **THEN** it appends a cursor character to its content and re-renders on each update cycle

#### Scenario: MessageBubble renders reasoning content
- **WHEN** a MessageBubble's internal reasoningContent is present and role is "assistant"
- **THEN** it renders the reasoning content as muted text below the main content

#### Scenario: MessageBubble renders active tool call
- **WHEN** a MessageBubble's internal activeToolCall is present
- **THEN** it renders a "Running: <name>" indicator below the main content

#### Scenario: MessageBubble renders tool call display output
- **WHEN** a MessageBubble's internal toolCallDisplay is present
- **THEN** it renders each line of tool call display output below the main content

### Requirement: MessageList provides imperative message management API
The MessageList component SHALL expose an imperative ref API containing addMessage(role, content, options), updateMessage(id, updates), clear(), and setMessages(msgs) methods.

#### Scenario: addMessage creates a new bubble
- **WHEN** addMessage("user", text) is called
- **THEN** a new MessageBubble instance is created at the end of the message list

#### Scenario: updateMessage updates an existing bubble
- **WHEN** updateMessage(id, { content: "new text" }) is called with a valid id
- **THEN** the MessageBubble with that id updates its content and re-renders

#### Scenario: updateMessage with streaming flag shows cursor
- **WHEN** updateMessage(id, { streaming: true }) is called
- **THEN** the MessageBubble sets its streaming state and begins showing a cursor

#### Scenario: clear removes all bubbles
- **WHEN** clear() is called
- **THEN** all MessageBubble instances are removed and the list shows "No messages yet"

#### Scenario: setMessages initializes from a data array
- **WHEN** setMessages([{ role: "assistant", content: "Hello" }]) is called
- **THEN** all existing bubbles are cleared and new bubbles are created for each message

### Requirement: MessageList owns scroll management
The MessageList component SHALL manage ScrollView rendering, auto-scroll-to-bottom with throttle, terminal resize handling, and manual scroll detection internally.

#### Scenario: Auto-scroll during streaming
- **WHEN** a message is streaming and its content changes
- **THEN** MessageList scrolls to the bottom of the ScrollView with a 100ms throttle

#### Scenario: Suppress auto-scroll during user manual scroll
- **WHEN** the user scrolls up away from the bottom and is not streaming
- **THEN** MessageList does NOT auto-scroll until the user returns to bottom or streaming resumes

#### Scenario: Immediate scroll on streaming completion
- **WHEN** streaming completes (streaming state set to false)
- **THEN** MessageList immediately scrolls to the bottom (no throttle)

#### Scenario: Remeasure on terminal resize
- **WHEN** the terminal is resized
- **THEN** MessageList's ScrollView ref calls remeasure() to update measured heights

### Requirement: MessageList enforces rendering window
The MessageList component SHALL render at most the last 100 messages (MAX_RENDER_MESSAGES = 100) to prevent performance degradation with very long conversations, while all messages remain available via the imperative API.

#### Scenario: Long conversation limits visible bubbles
- **WHEN** MessageList contains 200 messages and renders
- **THEN** only the last 100 MessageBubble components are rendered in the React tree

#### Scenario: AddMessage after window is full still works
- **WHEN** the window is full and addMessage() is called
- **THEN** a new message is added and the oldest visible message is replaced
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
## MODIFIED Requirements

### Requirement: Conversation panel uses ScrollView for rendering messages
The MessageList component SHALL render messages inside a `ScrollView` component from `ink-scroll-view` rather than manually slicing a messages array. ConversationPanel delegates all rendering to MessageList and does not directly render messages.

#### Scenario: ScrollView wraps message list
- **WHEN** the UI renders a conversation
- **THEN** the ScrollView from `ink-scroll-view` is the container inside MessageList, which is rendered by ConversationPanel

#### Scenario: Messages receive unique keys
- **WHEN** the ScrollView renders its children
- **THEN** each message element has a unique `key` prop (derived from message ID or index)

### Requirement: app.js no longer manages scroll state directly via setMessages
The `app.js` component SHALL NOT update messages via array cloning and mutation (`setMessages(prev => { const cloned = [...prev]; ... })`). Instead, app.js calls imperative methods on the MessageList ref for all message updates.

#### Scenario: Scroll state is removed from app.js
- **WHEN** `app.js` is inspected for scroll-related state and mutations
- **THEN** no `setMessages` callbacks that clone and mutate the messages array exist in the file

#### Scenario: ConversationPanel receives simplified props
- **WHEN** `app.js` renders ConversationPanel
- **THEN** it passes an initial `messages` array or `initialize` callback and `assistantName`, and uses a ref (`messageListRef`) for imperative updates
Loading