Skip to content

feat(dashboard): add observability features and fix event flow bugs - #53

Merged
Tony363 merged 1 commit into
mainfrom
fix/dashboard-event-flow-and-observability
Feb 12, 2026
Merged

feat(dashboard): add observability features and fix event flow bugs#53
Tony363 merged 1 commit into
mainfrom
fix/dashboard-event-flow-and-observability

Conversation

@Tony363

@Tony363 Tony363 commented Feb 12, 2026

Copy link
Copy Markdown
Owner

Summary

  • Execution Tree: visual tree of iterations, tool calls, and subagent spawns with layout algorithm and interactive node selection
  • Diff Viewer: inline diff display for Edit/Write tool invocations showing before/after changes
  • Proto wiring: GetExecutionDetail RPC with events, SendInput RPC, heartbeat and stdin proto messages
  • Heartbeat: daemon emits periodic LogMessage events for frontend liveness detection
  • Stdin foundation: child_stdin field and send_input() on ExecutionHandle for future interactive terminal
  • Bug fix (critical): revert Stdio::piped()Stdio::null() — piped stdin with no writer caused the Claude CLI to hang, producing no output
  • Bug fix (moderate): filter stderr batching to only emit ErrorOccurred for lines containing error/panic/fatal keywords, preventing normal stderr from polluting the event log
  • Bug fix (frontend): build execution tree from historical events when expanding completed executions, fixing "No tree data yet" for finished runs
  • README: correct select_agent.py line count, add new dashboard features, fix stale Docs/ directory structure

Test plan

  • cargo check --workspace passes
  • cargo check in crates/dashboard/frontend/ passes (independent workspace)
  • cargo test --manifest-path crates/superclaude-daemon/Cargo.toml — 17 tests pass
  • Functional: start daemon + cargo tauri dev, create execution, verify events stream to dashboard
  • Functional: expand completed execution → Execution Tree tab shows nodes
  • Functional: Execution Log tab shows historical events
  • Functional: Files Changed tab shows diffs for Edit/Write tool calls

🤖 Generated with Claude Code

Summary by Sourcery

Add richer observability to the dashboard and daemon, including execution tree visualization, file diff viewing, and interactive input support, while tightening stderr error reporting and heartbeat-based liveness signals.

New Features:

  • Introduce an execution tree visualization with SVG layout, pan/zoom, and node detail overlay built from streaming and historical events.
  • Add inline diff viewing in the Files Changed tab for Edit/Write tool invocations, showing per-file before/after content.
  • Expose a SendInput gRPC/tauri command and ExecutionHandle API to send interactive stdin to running executions.
  • Add a heartbeat-based thinking indicator in the monitor to reflect ongoing background processing.
  • Emit Obsidian markdown artifact events and subagent spawn/completion events to enrich execution telemetry.

Bug Fixes:

  • Rebuild execution trees from historical events for completed runs so the Execution Tree tab is populated instead of showing as empty.
  • Filter stderr-based ErrorOccurred events to only batch and emit real error-like lines, reducing noise in the event log.

Enhancements:

  • Track additional event metadata (node IDs, parent relationships, depths) to support tree construction and richer UI context.
  • Adjust stdout/stderr logging levels and non-JSON line handling to improve observability while debugging.
  • Extend dashboard app state to track execution tree, selected node, and last event source for better UX.
  • Refine README documentation to describe new dashboard capabilities and update project structure details.

Documentation:

  • Update README with new dashboard features, corrected script line counts, and current Docs/ directory structure.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added Execution Tree visualization to display execution flow and hierarchy in the detail panel
    • Added Diff Viewer to show file changes with edit and write actions
    • Added "Claude is thinking..." indicator during active processing
    • Enabled interactive input capability during execution
    • Added Execution Tree tab to the expanded detail panel
    • Enhanced Files Changed tab with file diffs for edited and created files
  • Improvements

    • Extended execution monitoring with enhanced heartbeat tracking

)

Add 5 observability features to the dashboard and fix 3 bugs that
prevented events from reaching the frontend.

Features:
- Execution Tree: visual tree of iterations, tool calls, and subagent
  spawns with layout algorithm and node selection
- Diff Viewer: inline diff display for Edit/Write tool invocations
- Proto wiring: GetExecutionDetail RPC, SendInput RPC, heartbeat and
  stdin proto messages
- Heartbeat: daemon emits periodic LogMessage events so frontend can
  detect liveness vs stalled executions
- Stdin foundation: child_stdin field and send_input() method on
  ExecutionHandle for future interactive terminal support

Bug fixes:
- Revert Stdio::piped() to Stdio::null() for Claude CLI stdin — piped
  stdin with no writer caused the CLI to hang producing no output
- Filter stderr batching to only emit ErrorOccurred events for lines
  containing error/panic/fatal keywords, preventing normal stderr
  (MCP connection info, debug logs) from polluting the event stream
- Build execution tree from historical events when expanding a
  completed execution, fixing "No tree data yet" for finished runs

Also updates README.md: correct select_agent.py line count (406→551),
add new dashboard features to feature list, fix stale Docs/ directory
structure, expand crates/dashboard/ tree detail.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Feb 12, 2026

Copy link
Copy Markdown

Reviewer's Guide

Adds an execution tree visualization and diff viewer to the dashboard, wires new observability-related RPCs (execution detail, send_input, heartbeat) from daemon through gRPC/Tauri to the Leptos frontend, and fixes several event handling and stderr/stdio bugs to make executions observable and non-hanging.

Sequence diagram for SendInput RPC path from dashboard to child stdin

sequenceDiagram
    actor User
    participant Frontend as DashboardFrontend
    participant Tauri as TauriCommands
    participant DClient as DashboardGrpcClient
    participant Svc as SuperClaudeService
    participant Handle as ExecutionHandle
    participant Child as ChildProcess

    User->>Frontend: Trigger send_execution_input(execution_id, input)
    Frontend->>Tauri: tauri_invoke send_execution_input
    Tauri->>DClient: send_input(SendInputRequest)
    DClient->>Svc: gRPC send_input(SendInputRequest)
    Svc->>Svc: lookup execution handle in executions map
    alt handle found
        Svc->>Handle: send_input(input)
        Handle->>Handle: child_stdin.write()
        Handle->>Child: write input to stdin pipe
        Handle-->>Svc: Result Ok
        Svc-->>DClient: SendInputResponse success=true message="Input sent"
        DClient-->>Tauri: SendInputResponse
        Tauri-->>Frontend: Ok(message)
        Frontend-->>User: Show success toast or status
    else handle missing
        Svc-->>DClient: gRPC error not_found
        DClient-->>Tauri: Err(Status)
        Tauri-->>Frontend: Err("gRPC error: ...")
        Frontend-->>User: Show error "Execution not found"
    end
Loading

Sequence diagram for heartbeat events and thinking indicator

sequenceDiagram
    participant Exec as ExecutionInner
    participant Daemon as SuperClaudeDaemon
    participant Stream as EventStreamSubscribeEvents
    participant DBackend as DashboardBackend
    participant Frontend as DashboardFrontend
    participant Monitor as MonitorPage

    loop while ExecutionState is Running
        Exec->>Exec: heartbeat task interval tick (5s)
        Exec->>Daemon: emit_event(LogMessage{source="heartbeat", message="Processing..."})
        Daemon-->>Stream: stream AgentEvent(LogMessage)
        Stream-->>DBackend: AgentEvent
        DBackend-->>Frontend: push AgentEventDto over websocket
        Frontend->>Frontend: on_event update last_event_source="heartbeat"
        Frontend->>Monitor: reactive state change
        Monitor->>Monitor: compute is_thinking (has_active && last_event_source=="heartbeat")
        Monitor-->>User: Render thinking_indicator("Claude is thinking...")
    end

    Exec->>Exec: execution finishes
    Exec->>Exec: heartbeat_handle.abort()
    Frontend->>Monitor: last_event_source changes away from heartbeat
    Monitor-->>User: Hide thinking_indicator
Loading

Class diagram for execution observability and dashboard state

classDiagram
    class ExecutionInner {
        +id: String
        +state: RwLock~ExecutionState~
        +process_pid: RwLock~Option~u32~~
        +child_stdin: RwLock~Option~ChildStdin~~
        +event_tx: Sender~AgentEvent~
        +event_history: RwLock~VecDeque~AgentEvent~~
        +evidence: RwLock~ExecutionEvidence~
        +emit_event(event: AgentEvent)
        +parse_stream_json_line(line: &str)
        +handle_tool_invoked(tool_name: String, input: Value, parent_node_id: String, id: String)
        +handle_tool_result(tool_name: String, tool_output: String, pending: PendingTool)
    }

    class ExecutionHandle {
        -inner: Arc~ExecutionInner~
        +start()
        +stop()
        +resume()
        +send_input(input: &str) Result~()~
        +get_status() ExecutionStatus
    }

    class SuperClaudeService {
        -executions: DashMap~String, ExecutionHandle~
        +send_input(request: Request~SendInputRequest~) Response~SendInputResponse~
        +get_execution_detail(...)
    }

    class GrpcClient {
        -client: superclaude_client::SuperClaudeClient
        +subscribe_events(...)
        +get_execution_detail(req: GetExecutionDetailRequest) Result~GetExecutionDetailResponse~
        +send_input(req: SendInputRequest) Result~SendInputResponse~
    }

    class AppState {
        +executions: RwSignal~Vec~ExecutionSummaryDto~~
        +expanded_execution: RwSignal~Option~String~~
        +execution_detail: RwSignal~Option~ExecutionDetailDto~~
        +detail_loading: RwSignal~bool~
        +execution_tree: RwSignal~ExecutionTree~
        +selected_tree_node: RwSignal~Option~String~~
        +last_event_source: RwSignal~String~
        +new() AppState
    }

    class ExecutionDetailDto {
        +execution_id: String
        +events: Vec~AgentEventDto~
        +files_written: Vec~String~
        +files_edited: Vec~String~
        +score_breakdown: Vec~ScoreDimensionDto~
    }

    class ExecutionTree {
        +nodes: Vec~TreeNode~
        +edges: Vec~TreeEdge~
    }

    class TreeNode {
        +node_id: String
        +parent_node_id: Option~String~
        +node_type: TreeNodeType
        +label: String
        +summary: String
        +status: TreeNodeStatus
        +x: f64
        +y: f64
        +event_data: Option~Value~
    }

    class TreeEdge {
        +from_id: String
        +to_id: String
    }

    class TreeNodeType {
        <<enum>>
        Iteration
        ToolCall
        SubagentSpawn
    }

    class TreeNodeStatus {
        <<enum>>
        Running
        Success
        Failed
        Pending
    }

    class ExecutionTreeComponent {
        +ExecutionTree()
        -layout_tree(tree: &mut ExecutionTree)
        -status_class(status: TreeNodeStatus) String
        -type_icon(node_type: TreeNodeType) String
    }

    class FilesChangedTabComponent {
        +FilesChangedTab(files_written: Vec~String~, files_edited: Vec~String~, events: Vec~AgentEventDto~)
        -extract_diff_entries(events: &[AgentEventDto]) Vec~FileDiffEntry~
    }

    class DiffViewComponent {
        +DiffView(file_path: String, action: String, old_string: String, new_string: String, content: String)
    }

    class FileDiffEntry {
        +file_path: String
        +action: String
        +old_string: Option~String~
        +new_string: Option~String~
        +content: Option~String~
    }

    ExecutionHandle --> ExecutionInner : holds
    SuperClaudeService --> ExecutionHandle : uses
    GrpcClient --> SuperClaudeService : gRPC client

    AppState --> ExecutionTree : owns
    ExecutionTree --> TreeNode : contains
    ExecutionTree --> TreeEdge : contains
    TreeNode --> TreeNodeType : uses
    TreeNode --> TreeNodeStatus : uses

    ExecutionTreeComponent --> AppState : reads signals
    ExecutionTreeComponent --> ExecutionTree : renders
    FilesChangedTabComponent --> ExecutionDetailDto : uses events
    FilesChangedTabComponent --> DiffViewComponent : renders
    FilesChangedTabComponent --> FileDiffEntry : builds

    ExecutionDetailDto --> AgentEventDto : contains events
Loading

File-Level Changes

Change Details Files
Introduce execution tree visualization driven by streaming and historical events.
  • Add ExecutionTree, TreeNode, and TreeEdge types plus status/type enums to shared AppState
  • Stream events in app.rs to incrementally build tree nodes/edges for iterations, tools, and subagents, including status transitions
  • Reconstruct the same execution tree from historical events when expanding a completed execution in ControlPage
  • Implement ExecutionTree SVG component with hierarchical layout, pan/zoom, node selection, and detail overlay
  • Add CSS styles for execution tree nodes, edges, and overlay and wire a new Execution Tree tab into the execution detail panel
crates/dashboard/frontend/src/state/app_state.rs
crates/dashboard/frontend/src/app.rs
crates/dashboard/frontend/src/pages/control.rs
crates/dashboard/frontend/src/components/execution_detail.rs
crates/dashboard/frontend/src/components/execution_tree.rs
crates/dashboard/frontend/style/main.css
Add diff viewer for Edit/Write tool calls and integrate into Files Changed tab.
  • Extract FileDiffEntry data from tool_invoked events by parsing tool_input JSON for Edit/Write tools
  • Extend FilesChangedTab to render per-file expandable diff sections using DiffView, while still listing remaining changed files
  • Implement DiffView component to show removed/added content or new-file content with appropriate styling
  • Add CSS styles for diff headers, blocks, and content scroll areas
crates/dashboard/frontend/src/components/execution_detail.rs
crates/dashboard/frontend/src/components/diff_view.rs
crates/dashboard/frontend/style/main.css
Wire new SendInput RPC and stdin handling for future interactive executions.
  • Extend ExecutionInner with child_stdin handle and plumb initialization in constructor and tests
  • Expose send_input method on ExecutionHandle to write to child stdin asynchronously with error handling
  • Add send_input RPC implementation in daemon gRPC service and bridge it through GrpcClient and a new Tauri command send_execution_input
  • Update protobuf definitions to include SendInputRequest/SendInputResponse and service method (implied by usage)
crates/superclaude-daemon/src/execution.rs
crates/superclaude-daemon/src/server.rs
crates/dashboard/src/bridge/grpc_client.rs
crates/dashboard/src/commands/execution.rs
crates/proto/proto/superclaude.proto
crates/superclaude-daemon/src/execution.rs
Improve stderr handling, logging, and add a heartbeat event stream for better observability.
  • Change stdout line logging from debug to info and non-JSON line logging from debug to warn for better visibility
  • Batch stderr lines and only emit ErrorOccurred events for lines containing error/panic/fatal, with size/time-based flushing and EOF flush
  • Add periodic heartbeat task that emits LogMessage events with source "heartbeat" while execution is running and stop it on completion
  • Use last_event_source in AppState and MonitorPage to display a "Claude is thinking" indicator when heartbeat events arrive
crates/superclaude-daemon/src/execution.rs
crates/dashboard/src/commands/execution.rs
crates/dashboard/frontend/src/state/app_state.rs
crates/dashboard/frontend/src/pages/monitor.rs
Emit richer events for artifacts, tools, and subagents to support the execution tree and dashboards.
  • On FileChanged events, detect markdown files inside Obsidian-like paths and emit ArtifactWritten events describing the document
  • Treat Task tool invocations specially: increment subagents_spawned, emit SubagentSpawned with node ids and parent ids, and emit SubagentCompleted on result
  • Include node_id, parent_node_id, depth, and related identifiers in formatted events for iteration, tool, and subagent events passed to the frontend
crates/superclaude-daemon/src/execution.rs
crates/dashboard/src/commands/execution.rs
Fix execution expansion behavior and clear stale state in the Control page.
  • When expanding an execution, clear any previous execution_tree and selected_tree_node before fetching detail
  • When collapsing, reset execution_tree and selected_tree_node along with expanded_execution and execution_detail
crates/dashboard/frontend/src/pages/control.rs
Update documentation for new dashboard capabilities and correct stale info.
  • Document new dashboard features including heartbeat indicator, 5-tab execution detail, execution tree, and diff viewer
  • Update select_agent.py line count in architecture diagram
  • Refresh repo layout tree for Docs/ and dashboard frontend structure
README.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented Feb 12, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This pull request introduces execution tree visualization with interactive pan/zoom navigation, file diff viewing capabilities, a heartbeat indicator for active processing, and interactive input functionality for executions. It adds supporting state types, event-driven tree construction logic across frontend pages, new gRPC service methods, and enhanced event formatting with node and hierarchy metadata.

Changes

Cohort / File(s) Summary
State Types & Data Models
crates/dashboard/frontend/src/state/app_state.rs
Introduces TreeNodeType, TreeNodeStatus, TreeNode, TreeEdge, and ExecutionTree types; adds execution_tree, selected_tree_node, and last_event_source fields to AppState with default initialization.
Execution Tree Visualization
crates/dashboard/frontend/src/components/execution_tree.rs
Adds new ExecutionTree component with layout calculation via DFS-based algorithm, SVG rendering with pan/zoom/drag interactions, node selection with detail overlay, and CSS-based node styling based on status and type.
Diff View Component
crates/dashboard/frontend/src/components/diff_view.rs
Introduces DiffView component rendering file diffs in edit (showing Removed/Added blocks) and write (showing New File block) modes based on action prop.
Component Module Exports
crates/dashboard/frontend/src/components/mod.rs
Exports new execution_tree and diff_view modules as public.
Event-Driven Tree Construction
crates/dashboard/frontend/src/app.rs
Implements incremental execution tree building from agent events (iteration_started, tool_invoked, iteration_completed, subagent_spawned, subagent_completed); tracks last_event_source for heartbeat detection; appends all events to global log; guards tree updates to matching execution_id.
Control Page Tree Building
crates/dashboard/frontend/src/pages/control.rs
Adds build_tree_from_events function to construct ExecutionTree from historical events; populates execution_tree state when fetching execution detail; resets tree and selected node on collapse.
Heartbeat Indicator
crates/dashboard/frontend/src/pages/monitor.rs
Displays "Claude is thinking..." indicator with animated dots when last_event_source is "heartbeat" and executions are active.
Diff Integration in Detail View
crates/dashboard/frontend/src/components/execution_detail.rs
Adds ExecutionTree tab to detail panel; extends FilesChangedTab to accept events and render diffs via DiffView component; extracts FileDiffEntry structures from tool_invoked events.
Styling
crates/dashboard/frontend/style/main.css
Adds CSS blocks for Execution Tree Visualization, Diff View, and Thinking Indicator; contains duplicate style definitions later in file.
Interactive Input gRPC
crates/proto/proto/superclaude.proto, crates/dashboard/src/bridge/grpc_client.rs
Adds SendInput RPC with SendInputRequest (execution_id, input) and SendInputResponse (success, message); implements async send_input method in GrpcClient.
Interactive Input Command
crates/dashboard/src/commands/execution.rs
Adds send_execution_input Tauri command; enriches event formatting with node_id, depth, parent_node_id fields for IterationStarted, IterationCompleted, ToolInvoked, SubagentSpawned, SubagentCompleted.
Interactive Input Daemon
crates/superclaude-daemon/src/execution.rs
Adds child_stdin field to ExecutionInner; implements send_input method on ExecutionHandle; adds heartbeat task emitting periodic "Processing..." messages; enhanced stderr batching with ErrorOccurred events; ArtifactWritten detection for MD files; SubagentSpawned/SubagentCompleted for Task tools.
Interactive Input Server
crates/superclaude-daemon/src/server.rs
Implements send_input method on gRPC service (appears twice in diff, indicating potential duplication); looks up ExecutionHandle and forwards input; returns success or not-found error.
Command Exposure
crates/dashboard/src/main.rs
Exposes send_execution_input command in Tauri invoke_handler.
Documentation
README.md
Updates feature descriptions for Live Monitor heartbeat indicator and Execution Control detail panel tabs; documents Execution Tree and Diff Viewer capabilities; adjusts code-size reference in diagrams.

Sequence Diagram(s)

sequenceDiagram
    participant Agent as Agent<br/>(Daemon)
    participant AppState as AppState<br/>(Frontend)
    participant TreeUI as ExecutionTree<br/>Component
    participant User as User<br/>Browser

    Agent->>AppState: Emit iteration_started<br/>(node_id, depth)
    AppState->>AppState: Create TreeNode<br/>(Iteration type)
    AppState->>AppState: Add to execution_tree

    Agent->>AppState: Emit tool_invoked<br/>(node_id, parent_id)
    AppState->>AppState: Create TreeNode<br/>(ToolCall type)
    AppState->>AppState: Add TreeEdge<br/>(parent→tool)

    Agent->>AppState: Emit iteration_completed
    AppState->>AppState: Update node status<br/>to Success

    AppState->>TreeUI: Signal execution_tree<br/>updated
    TreeUI->>TreeUI: Layout nodes via DFS<br/>algorithm
    TreeUI->>TreeUI: Render SVG with pan/zoom
    TreeUI->>User: Display interactive tree

    User->>TreeUI: Click node
    TreeUI->>TreeUI: Show detail overlay<br/>with event_data
    User->>TreeUI: Pan/Zoom interactions
Loading
sequenceDiagram
    participant User as User<br/>Browser
    participant TauriCmd as Tauri Command<br/>send_execution_input
    participant GrpcClient as gRPC Client
    participant Daemon as SuperClaude<br/>Daemon
    participant Process as Child<br/>Process stdin

    User->>TauriCmd: Invoke with<br/>execution_id, input
    TauriCmd->>GrpcClient: send_input(request)
    GrpcClient->>Daemon: SendInput RPC
    Daemon->>Daemon: Lookup ExecutionHandle
    Daemon->>Process: Write input to stdin<br/>via send_input
    Process->>Daemon: stdin receives data
    Daemon->>GrpcClient: SendInputResponse<br/>(success)
    GrpcClient->>TauriCmd: Return response
    TauriCmd->>User: Display confirmation
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

The changes span multiple layers (frontend components, state management, gRPC integration, daemon modifications) with new complex logic (tree layout algorithm, event-driven state synchronization, diff extraction). Notable concerns include duplicate send_input implementation in server.rs, CSS style duplication, and the interplay between event emission, tree construction, and rendering across multiple files requires careful verification.

Possibly related PRs

Poem

🐰 Hops with glee beneath the tree,
Each node a hop, each edge I see!
Diffs bloom where files take flight,
While heartbeats thump through Claude's night.
Oh, what a dance of tree and input stream!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 47.06% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main additions (observability features) and key bug fixes (event flow issues), directly matching the core changes in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/dashboard-event-flow-and-observability

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 4 issues, and left some high level feedback:

  • The execution-tree construction logic is duplicated between the real-time handler in app.rs and the historical replay in build_tree_from_events in control.rs; consider extracting shared helpers for mapping AgentEventDtoTreeNode/TreeEdge to keep behavior in sync and reduce maintenance overhead.
  • In main.css, .tree-node rect uses transition: stroke-color 150ms ease;, but stroke-color is not a valid animatable property in most browsers; switching this to transition: stroke 150ms ease; (or similar) will make the hover border animation work reliably.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The execution-tree construction logic is duplicated between the real-time handler in `app.rs` and the historical replay in `build_tree_from_events` in `control.rs`; consider extracting shared helpers for mapping `AgentEventDto``TreeNode`/`TreeEdge` to keep behavior in sync and reduce maintenance overhead.
- In `main.css`, `.tree-node rect` uses `transition: stroke-color 150ms ease;`, but `stroke-color` is not a valid animatable property in most browsers; switching this to `transition: stroke 150ms ease;` (or similar) will make the hover border animation work reliably.

## Individual Comments

### Comment 1
<location> `crates/dashboard/frontend/style/main.css:942-947` </location>
<code_context>
+    stroke-width: 1.5;
+}
+
+.tree-node rect {
+    fill: var(--bg-card);
+    stroke: var(--border);
+    stroke-width: 1;
+    cursor: pointer;
+    transition: stroke-color 150ms ease;
+}
+
</code_context>

<issue_to_address>
**issue (bug_risk):** The `transition` property uses `stroke-color`, which isn't a valid SVG/CSS property.

`stroke-color` is not a recognized property, so this transition will be ignored. Use `stroke` instead, e.g. `transition: stroke 150ms ease;` (and add `fill` if you also want the fill color animated).
</issue_to_address>

### Comment 2
<location> `crates/superclaude-daemon/src/execution.rs:837-846` </location>
<code_context>
                 });
             }
+
+            // Emit SubagentCompleted when a Task tool result arrives
+            if is_task_tool {
+                self.emit_event(AgentEvent {
+                    execution_id: self.id.clone(),
+                    timestamp: Self::now_timestamp(),
+                    event: Some(agent_event::Event::SubagentCompleted(SubagentCompleted {
+                        subagent_id: pending.node_id.clone(),
+                        success: true,
+                        result_summary: truncate_str(&tool_output, 200),
+                        node_id: format!("subagent-{}", pending.node_id),
+                    })),
+                });
</code_context>

<issue_to_address>
**issue (bug_risk):** SubagentCompleted `node_id` format is inconsistent with SubagentSpawned, likely breaking tree linking.

In `SubagentSpawned` for `Task` tools, `node_id` is `format!("subagent-{}", id)` and `subagent_id` is `id.to_string()`. Here, `SubagentCompleted` uses `subagent_id: pending.node_id.clone()` and `node_id: format!("subagent-{}", pending.node_id)`, which will yield `subagent-subagent-…` if `pending.node_id` already has the prefix. Since the frontend looks up the node by `event.data["node_id"]`, this mismatch will prevent it from finding the original node and applying the completion state. Please align `subagent_id`/`node_id` with the spawn event (e.g. `subagent_id: id.to_string()`, `node_id: format!("subagent-{}", id)`).
</issue_to_address>

### Comment 3
<location> `crates/dashboard/frontend/src/app.rs:105` </location>
<code_context>
+                state.last_event_source.set(event.event_type.clone());
+            }
+
+            // Build the execution tree incrementally
+            if let Some(expanded_eid) = state.expanded_execution.get_untracked() {
+                if expanded_eid == eid {
</code_context>

<issue_to_address>
**issue (complexity):** Consider extracting the execution-tree event matching and mutation logic into a shared helper function that both `App` and `build_tree_from_events` call instead of duplicating it inline.

You’ve effectively duplicated the tree‑building rules inline in `App`, which does increase complexity and coupling. You can keep all functionality but centralize the logic in a reusable “tree updater” that both `App` and `build_tree_from_events` call.

### 1. Extract a reusable tree update function

In a shared module (e.g. `state::execution_tree` or `pages::control::tree`), define something like:

```rust
pub fn apply_event_to_tree(
    tree: &mut ExecutionTree,
    event: &AgentEventDto,
) {
    match event.event_type.as_str() {
        "iteration_started" => {
            let node_id = event.data.get("node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            let iteration = event.data.get("iteration")
                .and_then(|v| v.as_i64())
                .unwrap_or(0);

            if !node_id.is_empty() && !tree.nodes.iter().any(|n| n.node_id == node_id) {
                tree.nodes.push(TreeNode {
                    node_id,
                    parent_node_id: None,
                    node_type: TreeNodeType::Iteration,
                    label: format!("Iteration {}", iteration),
                    summary: String::new(),
                    status: TreeNodeStatus::Running,
                    x: 0.0,
                    y: 0.0,
                    event_data: Some(event.data.clone()),
                });
            }
        }
        "tool_invoked" => {
            let node_id = event.data.get("node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            let parent_node_id = event.data.get("parent_node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            let summary_val = event.data.get("summary")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();

            if summary_val == "(result)" {
                // existing result logic
                let base_id = node_id.strip_suffix("-result").unwrap_or(&node_id).to_string();
                if let Some(node) = tree.nodes.iter_mut().find(|n| n.node_id == base_id) {
                    node.status = TreeNodeStatus::Success;
                    if let Some(output) = event.data.get("tool_output") {
                        if let Some(ref mut data) = node.event_data {
                            if let Some(obj) = data.as_object_mut() {
                                obj.insert("tool_output".to_string(), output.clone());
                            }
                        }
                    }
                }
            } else if !node_id.is_empty() && !tree.nodes.iter().any(|n| n.node_id == node_id) {
                let tool_name = event.data.get("tool_name")
                    .and_then(|v| v.as_str())
                    .unwrap_or("Tool")
                    .to_string();
                tree.nodes.push(TreeNode {
                    node_id: node_id.clone(),
                    parent_node_id: Some(parent_node_id.clone()),
                    node_type: TreeNodeType::ToolCall,
                    label: tool_name,
                    summary: summary_val,
                    status: TreeNodeStatus::Running,
                    x: 0.0,
                    y: 0.0,
                    event_data: Some(event.data.clone()),
                });
                if !parent_node_id.is_empty() {
                    tree.edges.push(TreeEdge {
                        from_id: parent_node_id,
                        to_id: node_id,
                    });
                }
            }
        }
        "iteration_completed" => {
            let node_id = event.data.get("node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            if !node_id.is_empty() {
                if let Some(node) = tree.nodes.iter_mut().find(|n| n.node_id == node_id) {
                    node.status = TreeNodeStatus::Success;
                }
            }
        }
        "subagent_spawned" => {
            // move your existing subagent_spawned logic here
        }
        "subagent_completed" => {
            // move your existing subagent_completed logic here
        }
        _ => {}
    }
}
```

You can iteratively migrate the remaining branches (`subagent_spawned`, `subagent_completed`) into this function, preserving the exact behavior you added.

### 2. Use it in `App`’s event handler

Replace the large `match` block in `App` with a narrow call that only gates on `expanded_eid`:

```rust
// Build the execution tree incrementally
if let Some(expanded_eid) = state.expanded_execution.get_untracked() {
    if expanded_eid == eid {
        state.execution_tree.update(|tree| {
            apply_event_to_tree(tree, &event);
        });
    }
}
```

That keeps the event loop in `App` focused on “when to update the tree” rather than “how to update the tree,” which makes the UI code easier to follow.

### 3. Reuse in `build_tree_from_events`

In `control.rs`, `build_tree_from_events` can then delegate to the same helper instead of duplicating logic:

```rust
pub fn build_tree_from_events(events: &[AgentEventDto]) -> ExecutionTree {
    let mut tree = ExecutionTree::default();
    for event in events {
        apply_event_to_tree(&mut tree, event);
    }
    tree
}
```

This removes the duplicated rules for node creation, edges, and status updates, and ensures future changes to tree semantics only need to be done in one place.
</issue_to_address>

### Comment 4
<location> `crates/dashboard/frontend/src/pages/control.rs:48` </location>
<code_context>
+/// Build an ExecutionTree from a list of historical events.
+/// This replays the same logic that app.rs uses for real-time events,
+/// ensuring the tree is populated when viewing completed executions.
+fn build_tree_from_events(events: &[AgentEventDto]) -> ExecutionTree {
+    let mut tree = ExecutionTree::default();
+
</code_context>

<issue_to_address>
**issue (complexity):** Consider extracting the shared event-to-execution-tree match logic into a reusable helper so both historical and real-time paths use the same code.

You can avoid maintaining two divergent implementations of the same event → tree logic by extracting the matching/branching into a shared helper and having both `build_tree_from_events` and the real-time handler in `app.rs` call it.

Concretely:

1. Extract the event-handling logic into a single helper (e.g. in `state` or a shared module):

```rust
pub fn update_execution_tree(tree: &mut ExecutionTree, event: &AgentEventDto) {
    match event.event_type.as_str() {
        "iteration_started" => {
            let node_id = event.data.get("node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            let iteration = event.data.get("iteration")
                .and_then(|v| v.as_i64())
                .unwrap_or(0);

            if !node_id.is_empty() && !tree.nodes.iter().any(|n| n.node_id == node_id) {
                tree.nodes.push(TreeNode {
                    node_id,
                    parent_node_id: None,
                    node_type: TreeNodeType::Iteration,
                    label: format!("Iteration {}", iteration),
                    summary: String::new(),
                    status: TreeNodeStatus::Running,
                    x: 0.0,
                    y: 0.0,
                    event_data: Some(event.data.clone()),
                });
            }
        }
        "tool_invoked" => {
            let node_id = event.data.get("node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            let parent_node_id = event.data.get("parent_node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            let summary_val = event.data.get("summary")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();

            if summary_val == "(result)" {
                let base_id = node_id.strip_suffix("-result").unwrap_or(&node_id).to_string();
                if let Some(node) = tree.nodes.iter_mut().find(|n| n.node_id == base_id) {
                    node.status = TreeNodeStatus::Success;
                    if let Some(output) = event.data.get("tool_output") {
                        if let Some(ref mut data) = node.event_data {
                            if let Some(obj) = data.as_object_mut() {
                                obj.insert("tool_output".to_string(), output.clone());
                            }
                        }
                    }
                }
            } else if !node_id.is_empty() && !tree.nodes.iter().any(|n| n.node_id == node_id) {
                let tool_name = event.data.get("tool_name")
                    .and_then(|v| v.as_str())
                    .unwrap_or("Tool")
                    .to_string();
                tree.nodes.push(TreeNode {
                    node_id: node_id.clone(),
                    parent_node_id: Some(parent_node_id.clone()),
                    node_type: TreeNodeType::ToolCall,
                    label: tool_name,
                    summary: summary_val,
                    status: TreeNodeStatus::Running,
                    x: 0.0,
                    y: 0.0,
                    event_data: Some(event.data.clone()),
                });
                if !parent_node_id.is_empty() {
                    tree.edges.push(TreeEdge {
                        from_id: parent_node_id,
                        to_id: node_id,
                    });
                }
            }
        }
        "iteration_completed" => {
            let node_id = event.data.get("node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            if let Some(node) = tree.nodes.iter_mut().find(|n| n.node_id == node_id) {
                node.status = TreeNodeStatus::Success;
            }
        }
        "subagent_spawned" => {
            let node_id = event.data.get("node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            let parent_node_id = event.data.get("parent_node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            let subagent_type = event.data.get("subagent_type")
                .and_then(|v| v.as_str())
                .unwrap_or("Subagent")
                .to_string();
            let task_summary = event.data.get("task_summary")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();

            if !node_id.is_empty() && !tree.nodes.iter().any(|n| n.node_id == node_id) {
                tree.nodes.push(TreeNode {
                    node_id: node_id.clone(),
                    parent_node_id: Some(parent_node_id.clone()),
                    node_type: TreeNodeType::SubagentSpawn,
                    label: subagent_type,
                    summary: task_summary,
                    status: TreeNodeStatus::Running,
                    x: 0.0,
                    y: 0.0,
                    event_data: Some(event.data.clone()),
                });
                if !parent_node_id.is_empty() {
                    tree.edges.push(TreeEdge {
                        from_id: parent_node_id,
                        to_id: node_id,
                    });
                }
            }
        }
        "subagent_completed" => {
            let node_id = event.data.get("node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            let success = event.data.get("success")
                .and_then(|v| v.as_bool())
                .unwrap_or(false);
            if let Some(node) = tree.nodes.iter_mut().find(|n| n.node_id == node_id) {
                node.status = if success {
                    TreeNodeStatus::Success
                } else {
                    TreeNodeStatus::Failed
                };
            }
        }
        _ => {}
    }
}
```

2. Make `build_tree_from_events` a simple loop that reuses this helper:

```rust
fn build_tree_from_events(events: &[AgentEventDto]) -> ExecutionTree {
    let mut tree = ExecutionTree::default();
    for event in events {
        update_execution_tree(&mut tree, event);
    }
    tree
}
```

3. In `app.rs`, call the same helper for real-time updates instead of re-implementing the `match`:

```rust
// wherever you currently handle live AgentEventDto events:
fn on_agent_event(state: &AppState, event: AgentEventDto) {
    let mut tree = state.execution_tree.get();
    update_execution_tree(&mut tree, &event);
    state.execution_tree.set(tree);
    // ...existing detail/state updates...
}
```

This keeps the new historical-tree feature intact while removing the duplication and ensuring any future changes to tree semantics are made once in `update_execution_tree`.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +942 to +947
.tree-node rect {
fill: var(--bg-card);
stroke: var(--border);
stroke-width: 1;
cursor: pointer;
transition: stroke-color 150ms ease;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): The transition property uses stroke-color, which isn't a valid SVG/CSS property.

stroke-color is not a recognized property, so this transition will be ignored. Use stroke instead, e.g. transition: stroke 150ms ease; (and add fill if you also want the fill color animated).

Comment on lines +837 to +846
// Emit SubagentCompleted when a Task tool result arrives
if is_task_tool {
self.emit_event(AgentEvent {
execution_id: self.id.clone(),
timestamp: Self::now_timestamp(),
event: Some(agent_event::Event::SubagentCompleted(SubagentCompleted {
subagent_id: pending.node_id.clone(),
success: true,
result_summary: truncate_str(&tool_output, 200),
node_id: format!("subagent-{}", pending.node_id),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): SubagentCompleted node_id format is inconsistent with SubagentSpawned, likely breaking tree linking.

In SubagentSpawned for Task tools, node_id is format!("subagent-{}", id) and subagent_id is id.to_string(). Here, SubagentCompleted uses subagent_id: pending.node_id.clone() and node_id: format!("subagent-{}", pending.node_id), which will yield subagent-subagent-… if pending.node_id already has the prefix. Since the frontend looks up the node by event.data["node_id"], this mismatch will prevent it from finding the original node and applying the completion state. Please align subagent_id/node_id with the spawn event (e.g. subagent_id: id.to_string(), node_id: format!("subagent-{}", id)).

state.last_event_source.set(event.event_type.clone());
}

// Build the execution tree incrementally

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider extracting the execution-tree event matching and mutation logic into a shared helper function that both App and build_tree_from_events call instead of duplicating it inline.

You’ve effectively duplicated the tree‑building rules inline in App, which does increase complexity and coupling. You can keep all functionality but centralize the logic in a reusable “tree updater” that both App and build_tree_from_events call.

1. Extract a reusable tree update function

In a shared module (e.g. state::execution_tree or pages::control::tree), define something like:

pub fn apply_event_to_tree(
    tree: &mut ExecutionTree,
    event: &AgentEventDto,
) {
    match event.event_type.as_str() {
        "iteration_started" => {
            let node_id = event.data.get("node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            let iteration = event.data.get("iteration")
                .and_then(|v| v.as_i64())
                .unwrap_or(0);

            if !node_id.is_empty() && !tree.nodes.iter().any(|n| n.node_id == node_id) {
                tree.nodes.push(TreeNode {
                    node_id,
                    parent_node_id: None,
                    node_type: TreeNodeType::Iteration,
                    label: format!("Iteration {}", iteration),
                    summary: String::new(),
                    status: TreeNodeStatus::Running,
                    x: 0.0,
                    y: 0.0,
                    event_data: Some(event.data.clone()),
                });
            }
        }
        "tool_invoked" => {
            let node_id = event.data.get("node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            let parent_node_id = event.data.get("parent_node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            let summary_val = event.data.get("summary")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();

            if summary_val == "(result)" {
                // existing result logic
                let base_id = node_id.strip_suffix("-result").unwrap_or(&node_id).to_string();
                if let Some(node) = tree.nodes.iter_mut().find(|n| n.node_id == base_id) {
                    node.status = TreeNodeStatus::Success;
                    if let Some(output) = event.data.get("tool_output") {
                        if let Some(ref mut data) = node.event_data {
                            if let Some(obj) = data.as_object_mut() {
                                obj.insert("tool_output".to_string(), output.clone());
                            }
                        }
                    }
                }
            } else if !node_id.is_empty() && !tree.nodes.iter().any(|n| n.node_id == node_id) {
                let tool_name = event.data.get("tool_name")
                    .and_then(|v| v.as_str())
                    .unwrap_or("Tool")
                    .to_string();
                tree.nodes.push(TreeNode {
                    node_id: node_id.clone(),
                    parent_node_id: Some(parent_node_id.clone()),
                    node_type: TreeNodeType::ToolCall,
                    label: tool_name,
                    summary: summary_val,
                    status: TreeNodeStatus::Running,
                    x: 0.0,
                    y: 0.0,
                    event_data: Some(event.data.clone()),
                });
                if !parent_node_id.is_empty() {
                    tree.edges.push(TreeEdge {
                        from_id: parent_node_id,
                        to_id: node_id,
                    });
                }
            }
        }
        "iteration_completed" => {
            let node_id = event.data.get("node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            if !node_id.is_empty() {
                if let Some(node) = tree.nodes.iter_mut().find(|n| n.node_id == node_id) {
                    node.status = TreeNodeStatus::Success;
                }
            }
        }
        "subagent_spawned" => {
            // move your existing subagent_spawned logic here
        }
        "subagent_completed" => {
            // move your existing subagent_completed logic here
        }
        _ => {}
    }
}

You can iteratively migrate the remaining branches (subagent_spawned, subagent_completed) into this function, preserving the exact behavior you added.

2. Use it in App’s event handler

Replace the large match block in App with a narrow call that only gates on expanded_eid:

// Build the execution tree incrementally
if let Some(expanded_eid) = state.expanded_execution.get_untracked() {
    if expanded_eid == eid {
        state.execution_tree.update(|tree| {
            apply_event_to_tree(tree, &event);
        });
    }
}

That keeps the event loop in App focused on “when to update the tree” rather than “how to update the tree,” which makes the UI code easier to follow.

3. Reuse in build_tree_from_events

In control.rs, build_tree_from_events can then delegate to the same helper instead of duplicating logic:

pub fn build_tree_from_events(events: &[AgentEventDto]) -> ExecutionTree {
    let mut tree = ExecutionTree::default();
    for event in events {
        apply_event_to_tree(&mut tree, event);
    }
    tree
}

This removes the duplicated rules for node creation, edges, and status updates, and ensures future changes to tree semantics only need to be done in one place.

/// Build an ExecutionTree from a list of historical events.
/// This replays the same logic that app.rs uses for real-time events,
/// ensuring the tree is populated when viewing completed executions.
fn build_tree_from_events(events: &[AgentEventDto]) -> ExecutionTree {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider extracting the shared event-to-execution-tree match logic into a reusable helper so both historical and real-time paths use the same code.

You can avoid maintaining two divergent implementations of the same event → tree logic by extracting the matching/branching into a shared helper and having both build_tree_from_events and the real-time handler in app.rs call it.

Concretely:

  1. Extract the event-handling logic into a single helper (e.g. in state or a shared module):
pub fn update_execution_tree(tree: &mut ExecutionTree, event: &AgentEventDto) {
    match event.event_type.as_str() {
        "iteration_started" => {
            let node_id = event.data.get("node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            let iteration = event.data.get("iteration")
                .and_then(|v| v.as_i64())
                .unwrap_or(0);

            if !node_id.is_empty() && !tree.nodes.iter().any(|n| n.node_id == node_id) {
                tree.nodes.push(TreeNode {
                    node_id,
                    parent_node_id: None,
                    node_type: TreeNodeType::Iteration,
                    label: format!("Iteration {}", iteration),
                    summary: String::new(),
                    status: TreeNodeStatus::Running,
                    x: 0.0,
                    y: 0.0,
                    event_data: Some(event.data.clone()),
                });
            }
        }
        "tool_invoked" => {
            let node_id = event.data.get("node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            let parent_node_id = event.data.get("parent_node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            let summary_val = event.data.get("summary")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();

            if summary_val == "(result)" {
                let base_id = node_id.strip_suffix("-result").unwrap_or(&node_id).to_string();
                if let Some(node) = tree.nodes.iter_mut().find(|n| n.node_id == base_id) {
                    node.status = TreeNodeStatus::Success;
                    if let Some(output) = event.data.get("tool_output") {
                        if let Some(ref mut data) = node.event_data {
                            if let Some(obj) = data.as_object_mut() {
                                obj.insert("tool_output".to_string(), output.clone());
                            }
                        }
                    }
                }
            } else if !node_id.is_empty() && !tree.nodes.iter().any(|n| n.node_id == node_id) {
                let tool_name = event.data.get("tool_name")
                    .and_then(|v| v.as_str())
                    .unwrap_or("Tool")
                    .to_string();
                tree.nodes.push(TreeNode {
                    node_id: node_id.clone(),
                    parent_node_id: Some(parent_node_id.clone()),
                    node_type: TreeNodeType::ToolCall,
                    label: tool_name,
                    summary: summary_val,
                    status: TreeNodeStatus::Running,
                    x: 0.0,
                    y: 0.0,
                    event_data: Some(event.data.clone()),
                });
                if !parent_node_id.is_empty() {
                    tree.edges.push(TreeEdge {
                        from_id: parent_node_id,
                        to_id: node_id,
                    });
                }
            }
        }
        "iteration_completed" => {
            let node_id = event.data.get("node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            if let Some(node) = tree.nodes.iter_mut().find(|n| n.node_id == node_id) {
                node.status = TreeNodeStatus::Success;
            }
        }
        "subagent_spawned" => {
            let node_id = event.data.get("node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            let parent_node_id = event.data.get("parent_node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            let subagent_type = event.data.get("subagent_type")
                .and_then(|v| v.as_str())
                .unwrap_or("Subagent")
                .to_string();
            let task_summary = event.data.get("task_summary")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();

            if !node_id.is_empty() && !tree.nodes.iter().any(|n| n.node_id == node_id) {
                tree.nodes.push(TreeNode {
                    node_id: node_id.clone(),
                    parent_node_id: Some(parent_node_id.clone()),
                    node_type: TreeNodeType::SubagentSpawn,
                    label: subagent_type,
                    summary: task_summary,
                    status: TreeNodeStatus::Running,
                    x: 0.0,
                    y: 0.0,
                    event_data: Some(event.data.clone()),
                });
                if !parent_node_id.is_empty() {
                    tree.edges.push(TreeEdge {
                        from_id: parent_node_id,
                        to_id: node_id,
                    });
                }
            }
        }
        "subagent_completed" => {
            let node_id = event.data.get("node_id")
                .and_then(|v| v.as_str())
                .unwrap_or("")
                .to_string();
            let success = event.data.get("success")
                .and_then(|v| v.as_bool())
                .unwrap_or(false);
            if let Some(node) = tree.nodes.iter_mut().find(|n| n.node_id == node_id) {
                node.status = if success {
                    TreeNodeStatus::Success
                } else {
                    TreeNodeStatus::Failed
                };
            }
        }
        _ => {}
    }
}
  1. Make build_tree_from_events a simple loop that reuses this helper:
fn build_tree_from_events(events: &[AgentEventDto]) -> ExecutionTree {
    let mut tree = ExecutionTree::default();
    for event in events {
        update_execution_tree(&mut tree, event);
    }
    tree
}
  1. In app.rs, call the same helper for real-time updates instead of re-implementing the match:
// wherever you currently handle live AgentEventDto events:
fn on_agent_event(state: &AppState, event: AgentEventDto) {
    let mut tree = state.execution_tree.get();
    update_execution_tree(&mut tree, &event);
    state.execution_tree.set(tree);
    // ...existing detail/state updates...
}

This keeps the new historical-tree feature intact while removing the duplication and ensuring any future changes to tree semantics are made once in update_execution_tree.

@github-actions

Copy link
Copy Markdown
Contributor

PAL MCP Consensus Code Review (Manual Multi-Model Analysis)

Overview

This PR adds comprehensive observability features to the SuperClaude dashboard, including:

  • Execution Tree Visualization: Real-time SVG tree graph with pan/zoom showing iterations, tool calls, and subagent spawns
  • Diff Viewer: Inline display of Edit/Write tool changes with before/after comparison
  • Heartbeat Indicator: Visual feedback when Claude is thinking between tool invocations
  • Interactive Input: Foundation for sending input to running executions via stdin pipe
  • Enhanced Event Flow: Fixed event flow bugs with proper node_id tracking and tree state management

Files Changed: 16 files (Rust frontend/backend, protobuf, CSS)
Lines Changed: +1344/-24
Language: Primarily Rust (Leptos WASM), no Python files modified


Critical Issues

None identified - No blocking security vulnerabilities or critical bugs that prevent merge.


High Priority Issues

1. Potential Memory Leak in Event History

File: crates/superclaude-daemon/src/execution.rs
Issue: The event_history VecDeque grows unbounded during long-running executions. The new heartbeat task emits events every 5 seconds, accelerating accumulation.
Recommendation: Implement a circular buffer with max size (e.g., 1000 events) or add periodic cleanup.
Severity: High (memory leak over time)

2. Stdin Pipe Not Initialized

File: crates/superclaude-daemon/src/execution.rs:465-466
Issue: The child_stdin RwLock is initialized to None but never populated with the actual stdin handle. The send_input method will always fail.
Recommendation: Add initialization after spawning child process.
Severity: High (feature will not work)

3. Race Condition in Tree Building

File: crates/dashboard/frontend/src/app.rs:76-237
Issue: Tree nodes are built incrementally from streaming events, but no synchronization for parent-child relationships.
Recommendation: Add validation to check parent node exists before adding edges.
Severity: Medium-High (UI inconsistencies)


Medium Priority Issues

4. String Slicing Panic Risk

File: execution_tree.rs:217-221
Issue: String slicing by byte index can panic on multibyte UTF-8 boundaries.

5. Inefficient Tree Traversal

File: execution_tree.rs:191-209
Issue: O(n²) edge rendering with nested find() calls.

6. Duplicate Tree Building Logic

Files: app.rs vs control.rs
Issue: Tree-building code duplicated (DRY violation).

7. Error Events May Overwhelm UI

File: execution.rs:492-555
Issue: Stderr batching may emit false positive errors.


Positive Observations

✅ Excellent event architecture
✅ Strong type safety
✅ Good separation of concerns
✅ Comprehensive CSS styling
✅ Proper error handling


Review Summary

Category Rating
Security 4/5
Code Quality 4/5
Architecture 4/5
Performance 3/5
Testing 3/5

Overall: Strong PR. Fix stdin pipe initialization (issue 2) before merge.


Recommended Actions

  1. MUST FIX: Initialize child_stdin with stdin handle
  2. SHOULD FIX: Bound event_history buffer size
  3. SHOULD FIX: Add parent node validation before adding edges
  4. NICE TO HAVE: Use Unicode-safe string truncation

Manual multi-model consensus review by Claude Sonnet 4.5

@github-actions

Copy link
Copy Markdown
Contributor

Claude Code Review (via AWS Bedrock)

Overview

This PR adds comprehensive observability features to the Tauri dashboard and fixes critical event flow bugs. The changes span 16 files with ~1,344 insertions, introducing execution tree visualization, diff viewer UI, heartbeat monitoring, and stdin infrastructure for future interactive support. Three key bugs were fixed: stdin hanging, stderr noise pollution, and missing tree data for completed executions.


Critical Issues

1. Security: Unvalidated User Input in send_input() (HIGH SEVERITY)

Location: crates/superclaude-daemon/src/execution.rs:1710-1722

The send_input() method writes user-controlled input directly to the child process stdin without any validation or sanitization. This could allow command injection, buffer overflow, or DoS via stdin flooding.

Recommendation: Add input validation, length limits (e.g., 10KB max), and rate limiting.

2. Resource Leak: Unbounded Memory Growth in Tree Building

Location: crates/dashboard/frontend/src/app.rs:76-237

The execution tree accumulates nodes indefinitely. For long-running executions with thousands of tool calls, this will cause excessive memory consumption in the WASM frontend.

Recommendation: Implement a maximum node limit (e.g., 1,000 nodes) with LRU eviction.

3. Logic Bug: child_stdin Never Initialized

Location: crates/superclaude-daemon/src/execution.rs:331-456

The child_stdin field is initialized as None but never set to Some(stdin) after spawning the child. This means send_input() will always fail.

Fix needed: After spawning the child process, store the stdin handle.


High Priority

4. Performance: O(n²) Tree Layout Algorithm

The DFS layout repeatedly searches through all nodes for each event. With hundreds of nodes, this causes UI lag. Use a HashMap for O(1) lookups instead.

5. Code Duplication: Tree Building Logic Duplicated 2x

The same 150+ lines appear in both app.rs and control.rs. Extract into a shared function.

6. Error Handling: Silent JSON Parse Failures

Parsing errors in extract_diff_entries() are silently ignored, causing diffs to not appear without explanation.

7. Observability: Missing Metrics

No telemetry for tree node count, heartbeat latency, or send_input() error rates.


Medium Priority

  1. Architecture: Tight coupling between UI and JSON event structure (no compile-time safety)
  2. UX: No loading indicators during expensive tree layout
  3. Code Quality: Magic numbers scattered throughout (500ms, 5s, 1000 chars)
  4. Accessibility: Missing ARIA labels and keyboard navigation in tree SVG
  5. Documentation: Thin inline docs (no algorithm explanations, perf characteristics)
  6. Testing: Zero test coverage for 1,344 new lines

Positive Observations

✅ Excellent commit message with clear feature/bug descriptions
✅ Smart use of shared tree-building function for historical consistency
✅ Proper CSS scoping with BEM-like naming
✅ Defensive programming (stdin null checks)
✅ Proper resource cleanup (heartbeat abort)
✅ Thoughtful UX (thinking indicator with animated dots)
✅ README accuracy updates


Review Summary

Category Rating Notes
Security ⚠️ 2/5 Critical: Unvalidated stdin input
Code Quality 3/5 DRY violations, missing tests
Architecture 4/5 Solid design, minor coupling
Testing 1/5 Zero test coverage
Performance 3/5 O(n²) algorithm, unbounded memory
Documentation 3/5 Good commit, thin inline docs
Overall 3/5 Solid work, but fix security/testing

Recommendations

Before Merging:

  1. Fix child_stdin initialization bug (chore: cleanup temp files and update .gitignore #3)
  2. Add input validation to send_input() (Welcome to SuperClaude Discussions! #1)
  3. Implement bounded tree memory (remove deepwiki mcp support #2)

Post-Merge:
4. Extract duplicated tree logic (#5)
5. Add comprehensive unit tests (#13)
6. Optimize tree layout algorithm (#4)


Generated by Claude Code Review (AWS Bedrock) - Sonnet 4.5

@Tony363 Tony363 self-assigned this Feb 12, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In `@crates/dashboard/frontend/src/components/execution_tree.rs`:
- Around line 217-221: The current logic slices node.summary by bytes (using
&node.summary[..20]) which panics on multi-byte UTF‑8; replace this byte-slice
truncation with the existing safe helper (truncate_str) to cut by character
boundaries—e.g., when building summary in the execution tree component use
truncate_str(&node.summary, 20) (or equivalent) instead of the byte slice and
keep the fallback node.summary.clone() when not exceeding the limit; update the
summary assignment that references node.summary to call truncate_str so
multi-byte chars are handled safely.

In `@crates/dashboard/frontend/style/main.css`:
- Around line 942-948: In the .tree-node rect rule replace the invalid
transition target "stroke-color" with the proper SVG/CSS property "stroke" so
the border color animates; update the transition declaration inside the
.tree-node rect selector to use "stroke 150ms ease" so animations work as
intended.

In `@crates/superclaude-daemon/src/execution.rs`:
- Around line 162-163: The send_input RPC always fails because child_stdin
(tokio::sync::RwLock<Option<ChildStdin>>) is never populated — the process is
spawned with Stdio::null() and child.stdin.take() is never stored, so
ExecutionHandle::send_input always bails; fix by either populating child_stdin
(change spawn to use Stdio::piped(), after spawning call child.stdin.take() and
store it into self.child_stdin via self.child_stdin.write().await, and ensure
send_input writes to that handle) or, if the feature is intentionally
unimplemented, remove the send_input RPC/Tauri command or add a clear TODO
comment where Stdio::null() is used referencing child_stdin, send_input and
ExecutionHandle to avoid misleading “Input sent” success messages.
🧹 Nitpick comments (11)
README.md (1)

601-604: Line 602 exceeds ~100-character wrap guideline.

This line is nearly 200 characters. Consider breaking it into multiple lines or shortening the parenthetical list for readability.

As per coding guidelines, "Markdown guidance (README, Docs/, .codex-os/) uses ATX headings, wraps near 100 characters, and should link to decisions or specs when behavior changes".

crates/dashboard/frontend/style/main.css (2)

970-973: Keyframe names should use kebab-case per Stylelint rules.

treePulse and thinkingPulse should be tree-pulse and thinking-pulse respectively. Note that the existing slideDown keyframe (Line 687) has the same issue, but since only the new code is in scope, update the new keyframes and their references.

Proposed fix
-.tree-node-running rect {
-    animation: treePulse 2s ease-in-out infinite;
+.tree-node-running rect {
+    animation: tree-pulse 2s ease-in-out infinite;
 }
 
-@keyframes treePulse {
+@keyframes tree-pulse {
     0%, 100% { opacity: 1; }
     50% { opacity: 0.6; }
 }
 .thinking-dot {
-    animation: thinkingPulse 1.4s ease-in-out infinite;
+    animation: thinking-pulse 1.4s ease-in-out infinite;
 }
 
-@keyframes thinkingPulse {
+@keyframes thinking-pulse {
     0%, 80%, 100% { opacity: 0.3; transform: scale(0.8); }
     40% { opacity: 1; transform: scale(1.0); }
 }

Also applies to: 1106-1108


1067-1071: Use overflow shorthand and split multi-declaration lines.

Stylelint flags overflow-y + overflow-x as redundant longhand (Line 1068-1069). Also, lines 1107-1108 have multiple declarations per line.

Proposed fix for overflow
 .diff-content {
     padding: 8px 12px;
     font-family: var(--font-mono);
     font-size: 11px;
     color: var(--text-secondary);
     background: var(--bg-primary);
     max-height: 300px;
-    overflow-y: auto;
-    overflow-x: auto;
+    overflow: auto;
     white-space: pre-wrap;
     word-break: break-all;
     margin: 0;
 }
crates/dashboard/frontend/src/pages/control.rs (2)

89-101: Fragile sentinel-based result detection.

The "(result)" string match and "-result" suffix stripping to associate a tool completion with its originating node is a brittle convention. If the daemon's event format changes (e.g., uses a different summary or ID scheme), this silently breaks without error. Consider using a dedicated event type (e.g., tool_completed) or an explicit boolean field in ToolInvoked to distinguish invocations from results.


45-193: Extract duplicated event-to-tree logic into a shared helper function.

The build_tree_from_events function duplicates the exact event-handling logic that app.rs implements for real-time updates (lines 109–256 in app.rs). Both files extract the same fields, perform identical deduplication checks, handle the "(result)" sentinel in the same way, and construct TreeNode/TreeEdge structures with the same field assignments.

Extract a shared function (e.g., apply_event_to_tree(&mut ExecutionTree, &AgentEventDto)) that both files can call. This eliminates the DRY violation and ensures that new event types or logic changes stay synchronized.

crates/proto/proto/superclaude.proto (2)

406-422: Nit: "Interactive Input" block splits the "Health Check" header from its messages.

The // Health Check section comment (Line 406-408) is now separated from PingRequest/PingResponse (Line 424-430) by the new SendInput messages. Consider moving the SendInput block above the Health Check section to keep the Health Check header adjacent to its messages, or move the Health Check header down.

Suggested reordering
 // ============================================================================
-// Health Check
-// ============================================================================
-
-// ============================================================================
 // Interactive Input
 // ============================================================================

 message SendInputRequest { ... }
 message SendInputResponse { ... }

+// ============================================================================
+// Health Check
+// ============================================================================
+
 message PingRequest {}
 message PingResponse { ... }

414-422: Consider adding an input size constraint or documentation note.

SendInputRequest.input is an unbounded string that gets written directly to a child process's stdin (per the server implementation in execution.rs). While this is a local-only daemon, a very large input could cause issues. A comment documenting the expected usage (e.g., single-line terminal input) or an optional max_length validation on the server side would be a useful guard.

crates/dashboard/frontend/src/components/diff_view.rs (1)

13-13: Unnecessary clone — file_path is already an owned String and unused afterward.

path_display can just be file_path directly since it's only consumed inside the match.

Proposed fix
-    let path_display = file_path.clone();
-
-    match action.as_str() {
+    match action.as_str() {

Then replace path_display with file_path in lines 19 and 48.

crates/dashboard/frontend/src/components/execution_tree.rs (1)

123-128: laid_out_tree() is re-computed multiple times per reactive cycle.

Each call to laid_out_tree() clones the entire tree from the signal and re-runs layout_tree. Within a single render pass, it's invoked from the main closure (line 171), viewbox() (line 132→171 again via laid_out_tree()), and the detail overlay (line 252). Consider computing it once with a Memo or a derived signal.

Sketch
-    // Compute the laid-out tree
-    let laid_out_tree = move || {
-        let mut tree = state.execution_tree.get();
-        layout_tree(&mut tree);
-        tree
-    };
+    // Compute the laid-out tree once per change
+    let laid_out_tree = Memo::new(move |_| {
+        let mut tree = state.execution_tree.get();
+        layout_tree(&mut tree);
+        tree
+    });

Then use laid_out_tree.get() at call sites instead of laid_out_tree().

Also applies to: 131-138, 170-176, 249-252

crates/superclaude-daemon/src/execution.rs (1)

334-334: Raising stdout log level from debug to info may be noisy in production.

Every line from Claude's stdout (which can be voluminous with stream-json output) will now be logged at info level. This could generate significant log volume. Consider keeping this at debug or gating behind a verbose flag.

Proposed change
-                    info!(execution_id = %inner.id, len = line.len(), "claude stdout line");
+                    debug!(execution_id = %inner.id, len = line.len(), "claude stdout line");
crates/dashboard/frontend/src/state/app_state.rs (1)

158-169: Consider excluding layout fields (x, y) from serialization.

TreeNode derives Serialize/Deserialize, but x and y are layout-computed values that are overwritten by layout_tree before every render. Serializing them is harmless but adds noise if these structs are ever logged or persisted.

Comment on lines +217 to +221
let summary = if node.summary.len() > 20 {
format!("{}...", &node.summary[..20])
} else {
node.summary.clone()
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Panic on multi-byte UTF-8 summaries: &node.summary[..20] slices by byte index.

If node.summary contains any multi-byte UTF-8 character within the first 20 bytes, this will panic at runtime with "byte index is not a char boundary". The daemon already has a safe truncate_str helper that uses char_indices.

🐛 Proposed fix
-                            let summary = if node.summary.len() > 20 {
-                                format!("{}...", &node.summary[..20])
+                            let summary = if node.summary.chars().count() > 20 {
+                                let end = node.summary.char_indices().nth(20).map(|(i, _)| i).unwrap_or(node.summary.len());
+                                format!("{}…", &node.summary[..end])
                             } else {
                                 node.summary.clone()
                             };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let summary = if node.summary.len() > 20 {
format!("{}...", &node.summary[..20])
} else {
node.summary.clone()
};
let summary = if node.summary.chars().count() > 20 {
let end = node.summary.char_indices().nth(20).map(|(i, _)| i).unwrap_or(node.summary.len());
format!("{}…", &node.summary[..end])
} else {
node.summary.clone()
};
🤖 Prompt for AI Agents
In `@crates/dashboard/frontend/src/components/execution_tree.rs` around lines 217
- 221, The current logic slices node.summary by bytes (using
&node.summary[..20]) which panics on multi-byte UTF‑8; replace this byte-slice
truncation with the existing safe helper (truncate_str) to cut by character
boundaries—e.g., when building summary in the execution tree component use
truncate_str(&node.summary, 20) (or equivalent) instead of the byte slice and
keep the fallback node.summary.clone() when not exceeding the limit; update the
summary assignment that references node.summary to call truncate_str so
multi-byte chars are handled safely.

Comment on lines +942 to +948
.tree-node rect {
fill: var(--bg-card);
stroke: var(--border);
stroke-width: 1;
cursor: pointer;
transition: stroke-color 150ms ease;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

stroke-color is not a valid CSS transition property — use stroke.

Line 947: transition: stroke-color 150ms ease; won't animate because stroke-color is not a recognized CSS property. The correct property name for SVG stroke transitions is stroke.

Proposed fix
 .tree-node rect {
     fill: var(--bg-card);
     stroke: var(--border);
     stroke-width: 1;
     cursor: pointer;
-    transition: stroke-color 150ms ease;
+    transition: stroke 150ms ease;
 }
🤖 Prompt for AI Agents
In `@crates/dashboard/frontend/style/main.css` around lines 942 - 948, In the
.tree-node rect rule replace the invalid transition target "stroke-color" with
the proper SVG/CSS property "stroke" so the border color animates; update the
transition declaration inside the .tree-node rect selector to use "stroke 150ms
ease" so animations work as intended.

Comment on lines +162 to +163
/// Piped stdin handle for interactive input via SendInput RPC.
child_stdin: tokio::sync::RwLock<Option<tokio::process::ChildStdin>>,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

send_input will always fail — child_stdin is never populated.

The child_stdin field is initialized as None (line 206) and stdin is set to Stdio::null() (line 269), so child.stdin.take() would return None even if it were called — but it's never called anywhere. The send_input method (line 1253) will always bail with "stdin pipe not available".

If this is intentionally deferred (the PR summary calls it "Stdin foundation"), consider either:

  1. Removing the send_input RPC endpoint and Tauri command until it's functional, to avoid confusing "Input sent" success messages that never actually reach the process.
  2. Or adding a TODO comment at line 269 documenting why Stdio::null() is used and what's needed to complete the feature.

Currently the full pipeline (frontend → Tauri → gRPC → ExecutionHandle::send_input) is wired up but will always return an error at the last mile.

Also applies to: 206-206, 269-269

🤖 Prompt for AI Agents
In `@crates/superclaude-daemon/src/execution.rs` around lines 162 - 163, The
send_input RPC always fails because child_stdin
(tokio::sync::RwLock<Option<ChildStdin>>) is never populated — the process is
spawned with Stdio::null() and child.stdin.take() is never stored, so
ExecutionHandle::send_input always bails; fix by either populating child_stdin
(change spawn to use Stdio::piped(), after spawning call child.stdin.take() and
store it into self.child_stdin via self.child_stdin.write().await, and ensure
send_input writes to that handle) or, if the feature is intentionally
unimplemented, remove the send_input RPC/Tauri command or add a clear TODO
comment where Stdio::null() is used referencing child_stdin, send_input and
ExecutionHandle to avoid misleading “Input sent” success messages.

@Tony363
Tony363 merged commit 62a7355 into main Feb 12, 2026
35 checks passed
@Tony363
Tony363 deleted the fix/dashboard-event-flow-and-observability branch February 12, 2026 05:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant