fix(ui): improve sub-agent tree rendering, skill loading, and lifecycle management

[lavaman131]

Summary

Fixes multiple UI issues related to sub-agent tree rendering, skill loading indicators, and post-task text streaming. Key improvements include eager agent tree rendering, deduplication of skill load indicators, smarter post-task text suppression, and proper grouping of sequentially spawned sub-agents.

Key Changes

Sub-agent Tree Rendering (src/ui/index.ts)

• Eager agent creation: Create ParallelAgent entries immediately on tool.start for Task tools instead of waiting for subagent.start event
  • Trees now appear instantly when agents are spawned
  • When subagent.start fires later, merge with real subagentId to prevent duplicates
• Lifecycle completion: Finalize agent status on tool.complete for agents that skip subagent.complete event
  • Ensures agents transition from "running" → "completed" even without SDK completion event
  • Sets duration, clears currentTool, and marks status as completed

Skill Loading Deduplication (src/ui/chat.tsx, src/ui/commands/skill-commands.ts)

• Track loaded skills per session using loadedSkillsRef to prevent duplicate "loaded" UI indicators
• Add <skill-loaded> directive to expanded skill prompts to prevent model from re-invoking Skill tool
• Fix skill name resolution to fall back to input.name when input.skill is unavailable

Post-Task Text Suppression (src/ui/index.ts)

• Replace aggressive substring matching with prefix-based accumulator
  • Old logic: matched any substring, incorrectly suppressed genuine follow-up text
  • New logic: only suppresses text that sequentially matches cached result from the start
• Prevents legitimate model responses from being swallowed after sub-agent tasks complete

Agent Tree Grouping (src/ui/chat.tsx)

• Group sub-agents by content offset from Task tool invocations
• Sequentially spawned agents now render as separate trees at chronological positions
• Prevents flattening of sequential agent groups into single tree

Task Tool Hiding (src/ui/chat.tsx)

• Hide Task tool calls from main conversation view (similar to HITL tools)
• Sub-agents shown via ParallelAgentsTree component instead
• Tool traces still available in detail view (Ctrl+O)

UI Polish (src/ui/components/parallel-agents-tree.tsx)

• Remove result summary display from completed agents (reduces visual clutter)

Research Documentation

• Add comprehensive DAG orchestration research document for Ralph workflow system
• Documents blockedBy dependency enforcement, parallel worker dispatch, and file concurrency

Technical Details

Eager Agent Pattern: When a Task tool starts, immediately create a placeholder agent with the tool ID. When the SDK's subagent.start event arrives, update the existing entry in-place with the real subagent ID rather than creating a duplicate.

Skill Loading State: The loadedSkillsRef tracks which skills have been loaded in the current session. Only the first invocation shows the "loaded" indicator; subsequent invocations are silent unless there's an error.

Text Suppression Logic: After a Task tool completes, the model may echo back the JSON result as streaming text. The new prefix-based accumulator tracks incoming text character-by-character, only suppressing if it matches the cached result sequentially from the beginning. Once non-matching text arrives, suppression is cleared.

Agent Grouping: Uses Task tool contentOffsetAtStart to group agents. Agents with the same offset (spawned in parallel) share a tree. Agents with different offsets (spawned sequentially with intervening text) get separate trees.

Breaking Changes

None

Test Plan

• Verify sub-agent trees appear immediately when Task tools are invoked (no delay until subagent.start)
• Verify no duplicate agent entries when both tool.start and subagent.start fire
• Verify completed agents show correct status even if subagent.complete never fires
• Verify skill /commands only show "loaded" indicator on first invocation per session
• Verify skill name displays correctly for skills using input.name instead of input.skill
• Verify model follows expanded skill instructions without re-invoking the Skill tool
• Verify post-task streaming text is not incorrectly suppressed (genuine follow-up text flows through)
• Verify sequentially spawned sub-agents render as separate trees at correct content positions
• Verify Task tool calls don't appear in main conversation view

[claude]

PR Review: fix(ui): improve sub-agent tree rendering, skill loading, and lifecycle management

Overview

This PR addresses several interconnected UI issues: sub-agent tree rendering timing, skill loading deduplication, post-task text suppression accuracy, and agent tree grouping. The changes are well-organized across three commits with clear purposes.

---

Code Quality & Best Practices

Strengths:

• Clean separation of concerns across commits
• Good use of TypeScript with appropriate type annotations (as const for status literals)
• Defensive coding patterns (fallback to agentsOffset ?? 0, null checks)
• The <skill-loaded> directive pattern is a clever way to prevent model re-invocation

Suggestions:

1. src/ui/index.ts:514-528 - The eager agent creation duplicates logic from the subagent.start handler. Consider extracting a helper function:

   function createAgentEntry(id: string, type: string, task: string, startedAt: string): ParallelAgent

2. src/ui/chat.tsx:1953-1954 - The loadedSkillsRef comment could be clearer about why this deduplication matters (prevents visual flicker, not just UI clutter).

3. src/ui/index.ts:1001-1040 - The prefix-based suppression accumulator logic is complex. A brief inline comment explaining the algorithm would help future maintainers understand why substring matching was problematic.

---

Potential Bugs or Issues

1. Agent ID collision risk (src/ui/index.ts:517):

   id: toolId,  // Uses toolId as initial ID

   Later merged with real subagentId on subagent.start. If subagent.start never fires (network issue, agent crash), the toolId remains as the permanent ID. This is handled correctly, but worth documenting the invariant.

2. src/ui/chat.tsx:3198-3209 - When finalizing a previous streaming message, the filter removes empty placeholders:

   .filter((msg: ChatMessage) =>
     !(msg.id === prevStreamingId && !msg.content.trim())
   )

   Edge case: If the previous message has only whitespace content (e.g., " "), it gets removed. This seems intentional but verify it doesn't cause issues with messages that legitimately start with whitespace.

3. src/ui/commands/skill-commands.ts:1696-1401 - The <skill-loaded> directive could be confused with XML tags in the expanded skill content. If a skill prompt contains </skill-loaded>, it could prematurely close the directive. Consider using a more unique delimiter or CDATA-style wrapping.

---

Performance Considerations

1. Map operations in buildContentSegments (src/ui/chat.tsx:1321-1356):

   • Creating a new Map<number, ParallelAgent[]> on every render could be costly with many agents
   • The current implementation is O(n) which is reasonable, but could benefit from memoization if agents array is large

2. src/ui/index.ts:990-1040 - String prefix matching with cachedResult.startsWith(candidate):

   • For very long results (e.g., large file contents), this check runs on every text chunk
   • Consider truncating cachedResult for comparison purposes after a reasonable length (e.g., 5000 chars)

3. Parallel agent state updates (src/ui/index.ts:637-685):

   • Multiple .map() calls over state.parallelAgents could be consolidated when updating agent status

---

Security Concerns

No security issues identified. The changes are purely UI-related and don't handle user input in security-sensitive ways.

---

Test Coverage

Missing tests for new functionality:

1. Skill loading deduplication - No tests for loadedSkillsRef behavior:

   • First invocation shows indicator
   • Subsequent invocations are suppressed
   • Errors always show
   • Session reset clears the set

2. Eager agent creation - No tests for the tool.start → subagent.start merge path:

   • Agent appears immediately on tool.start
   • Correct merge when subagent.start fires
   • Proper finalization when subagent.complete is skipped

3. Post-task text suppression - No tests for the prefix accumulator:

   • Echoed result text is suppressed
   • Genuine follow-up text passes through
   • Whitespace handling during accumulation

4. Agent tree grouping - No tests for multiple trees at different offsets

Existing test coverage is maintained - The registry.test.ts changes for skill name fallback (input.name) are covered implicitly.

---

Documentation

The research document (research/docs/2026-02-15-ralph-dag-orchestration-blockedby.md) is thorough and well-structured. However:

1. Line 172: The worker path discrepancy is documented (~/.atomic/workflows/{session_id} vs ~/.atomic/workflows/sessions/{session_id}) but not fixed in this PR. Consider adding a follow-up issue or fixing it now.

2. The PR description is excellent - clear bullet points with specific behavioral changes and a comprehensive test plan.

---

Summary

Category	Assessment
Code Quality	✅ Good - clean, well-typed, defensive
Bugs/Issues	⚠️ Minor edge cases noted
Performance	⚠️ Some optimization opportunities
Security	✅ No concerns
Test Coverage	❌ New functionality lacks unit tests

Recommendation: This PR is ready to merge with one request:

Required: Add unit tests for the skill loading deduplication and post-task text suppression changes. These are complex enough that regression risk is real without tests.

Optional: Address the performance suggestions and document the eager agent ID lifecycle.

---

Reviewed with attention to the atomic codebase patterns and conventions.