feat: Phase 1b — SSE step events, event rotation, lane animation - #102
Merged
Conversation
…tion Emit workflow.step.started/completed/failed events from the workflow executor so the dashboard can track work items flowing through the pipeline. Add event file rotation (archive at 5k, retain 2.5k) to prevent unbounded growth. Dashboard now renders animated run dots on stages, marching-dash flow connectors, and a HUD run counter. 20+ new JS tests cover run lifecycle, failure paths, and concurrent runs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdded per-run workflow state and UI visualizations, emitted step lifecycle control events from the executor, introduced event-log rotation for control events, and wired a dispatch panel to trigger workflows from the dashboard. Changes
Sequence Diagram(s)sequenceDiagram
participant WE as WorkflowExecutor
participant STATE as State/Events
participant SSE as SSE Server
participant DASH as Dashboard(app.js)
participant UI as Dashboard UI
WE->>STATE: append_control_event(workflow.step.started...)
WE->>WE: execute step
WE->>STATE: append_control_event(workflow.step.completed|failed...)
STATE->>STATE: maybe_rotate_events() (archive if >5000)
STATE->>SSE: events available for streaming
SSE->>DASH: SSE: workflow.* events
DASH->>DASH: processWorkflowEvent() (update appState.runs)
DASH->>UI: renderPipeline() -> update run-dots, connectors, hud-runs
UI-->>DASH: DOM updated
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Adds a "+ run" button in the dashboard that opens a dispatch form with workflow selector, optional issue number input, and a dispatch button. Calls POST /v1/actions/run to trigger the workflow. Workflows are fetched from GET /v1/workflows on panel open. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
dashboard/style.css (1)
232-259: Honorprefers-reduced-motionfor the new animations.The pulsing dot and marching connectors are always animated. Please add a reduced-motion override so motion-sensitive users can keep the dashboard usable.
💡 Minimal fix
+@media (prefers-reduced-motion: reduce) { + .run-dot-active, + .flow-connector.flowing { + animation: none; + } +}Also applies to: 395-397
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dashboard/style.css` around lines 232 - 259, Add a prefers-reduced-motion media query that disables the animations for motion-sensitive users: inside a `@media` (prefers-reduced-motion: reduce) block set animation: none and remove transform/opacity animations for .run-dot-active (which uses `@keyframes` dot-pulse) and .flow-connector.flowing (which uses `@keyframes` flow-march), and also ensure any shadow/visual-only motion for .run-dot-failed is non-animated; reference .run-dot-active, .run-dot-failed, .flow-connector.flowing, `@keyframes` dot-pulse and `@keyframes` flow-march when applying these overrides.dashboard/test.html (1)
285-289: Consider documenting or explicitly handlingworkflow.step.completed.This test validates that
step.completeddoesn't change the run status, which is correct. However,processWorkflowEventhas no explicit handler forworkflow.step.completed— it works only because the else-if chain falls through. Since the backend emits this event (per PR objectives), adding an explicit case (even as a no-op comment) would clarify intent for future maintainers:} else if (evt.event === "workflow.step.completed") { // No state change needed; run continues }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dashboard/test.html` around lines 285 - 289, The test shows workflow.step.completed currently leaves run status as "running" because processWorkflowEvent falls through; add an explicit handler for this event in processWorkflowEvent (e.g., an else if branch checking evt.event === "workflow.step.completed") and make it a no-op with a clarifying comment so intent is clear to future maintainers while preserving current behavior (also reference testRuns["run-1"].status and the existing test using processWorkflowEvent to verify no state change).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dashboard/app.js`:
- Around line 68-72: The renderer is treating staged/terminal runs as active
because UI animation logic doesn't distinguish run states; update rendering to
separate active runs from terminal runs by checking each run's status (e.g.,
run.status or run.state) before applying pulsing/connector animations so only
runs with status "running" animate, and ensure terminal runs remain static until
they're removed by the existing setTimeout that deletes from appState.runs and
calls scheduleRender(); adjust any rendering functions (where runs are
iterated—references: appState.runs, scheduleRender(), and the setTimeout removal
block) to use this status check so completed/failed runs do not pulse or keep
connectors marching.
In `@src/automation/mod.rs`:
- Around line 789-804: The started-event emission (append_control_event with
event "workflow.step.started") is only in the sequential branch, so when
explicit_dep_mode uses execute_control_dag() ensure_running/review/land steps
never publish it; update execute_control_dag (and any helper that begins step
execution) to call the same append_control_event snippet when a step transitions
to running — specifically add the workflow.step.started append_control_event
call inside execute_control_dag before executing a step (or inside the function
that dispatches/marks a step as started), using the same fields: workspace from
self.config.workspace.name, agent from step_agent_name(step), timestamp
Utc::now(), correlation_id run_id, and data including workflow.name, step_index,
step_type_name(step), and total_steps (workflow.steps.len()) so the dashboard
receives started events for ensure_running, review, land and other control-DAG
steps.
- Around line 563-570: The match arm in step_agent_name for ResolvedStep::Review
returns the source agent, which misroutes review events; change the
ResolvedStep::Review arm inside function step_agent_name to return the reviewer
session identifier instead (e.g., Some("reviewer") or the existing
REVIEWER/REVIEW_SESSION constant if one exists) so review steps are attributed
to the reviewer lane rather than the source agent.
- Around line 2095-2116: The code currently collapses StepStatus::Warning into
failure; update the branch that sets event_name (where sr.status is checked) to
handle StepStatus::Warning separately (e.g., set event_name =
"workflow.step.warning" when sr.status == StepStatus::Warning) before falling
back to "workflow.step.failed" for actual failures, leaving append_control_event
and ControlEvent payload unchanged so the dashboard receives a distinct warning
event rather than a failed event; modify the conditional around sr.status in the
same block that builds event_name and calls append_control_event.
In `@src/state/mod.rs`:
- Around line 929-944: The rotation routine maybe_rotate_events currently does
an unlocked read/modify/write which can drop concurrent events from
append_control_event() and may clobber another rotation with the same-second
archive name; fix by taking an interprocess lock (e.g., an advisory file lock)
around the entire rotation snapshot-and-rotate in maybe_rotate_events and any
writer paths (append_control_event) so reads and appends serialize, use the
snapshot (read the entire file while holding the lock) to compute split, write
the archive and the reduced active file to temporary paths and then atomically
rename into place (avoid rewriting the live file), and make archive names unique
(include subsecond timestamp or a UUID) to prevent same-second collisions.
---
Nitpick comments:
In `@dashboard/style.css`:
- Around line 232-259: Add a prefers-reduced-motion media query that disables
the animations for motion-sensitive users: inside a `@media`
(prefers-reduced-motion: reduce) block set animation: none and remove
transform/opacity animations for .run-dot-active (which uses `@keyframes`
dot-pulse) and .flow-connector.flowing (which uses `@keyframes` flow-march), and
also ensure any shadow/visual-only motion for .run-dot-failed is non-animated;
reference .run-dot-active, .run-dot-failed, .flow-connector.flowing, `@keyframes`
dot-pulse and `@keyframes` flow-march when applying these overrides.
In `@dashboard/test.html`:
- Around line 285-289: The test shows workflow.step.completed currently leaves
run status as "running" because processWorkflowEvent falls through; add an
explicit handler for this event in processWorkflowEvent (e.g., an else if branch
checking evt.event === "workflow.step.completed") and make it a no-op with a
clarifying comment so intent is clear to future maintainers while preserving
current behavior (also reference testRuns["run-1"].status and the existing test
using processWorkflowEvent to verify no state change).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fab3fd66-8fc3-42e5-afe1-21ddee84c4d1
📒 Files selected for processing (6)
dashboard/app.jsdashboard/index.htmldashboard/style.cssdashboard/test.htmlsrc/automation/mod.rssrc/state/mod.rs
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dashboard/app.js`:
- Around line 40-50: The handler currently only seeds appState.runs on
"workflow.started" so late-joined clients ignore "workflow.step.*" events;
update the event handling (the code around the evt.event checks for
"workflow.started" and other step events like
"workflow.step.started"/"workflow.step.completed") to create a minimal run
object when a step event arrives and appState.runs[id] is missing: populate id,
status: "running", stepIndex from evt.data.step_index (or 0), totalSteps from
evt.data.total_steps, workflowName from evt.data.workflow_name (or ""), and
startedAt from evt.timestamp (or null); ensure this same seeding logic is
applied to the other step-event branches referenced around lines 53-54 so runs
appear after refresh/SSE reconnect.
In `@dashboard/index.html`:
- Around line 42-45: The dispatch controls lack accessible names and the status
span won't be announced; add explicit accessible labels for the select and input
(use <label for="dispatch-workflow"> and <label for="dispatch-issue"> or
aria-label attributes on elements with ids dispatch-workflow and
dispatch-issue), ensure the dispatch button (id dispatch-go) has an accessible
name (visible text or aria-label), and make the status element
(`#dispatch-status`) live to assistive tech by adding a role or aria-live (e.g.,
role="status" or aria-live="polite") so updates are announced.
In `@dashboard/style.css`:
- Around line 329-395: The dispatch row overflows on small screens because
desktop widths (min-width: 140px, input width: 100px) aren't overridden; add a
mobile media query targeting `#dispatch-panel/.dispatch-form` that enables
flex-wrap (or switches .dispatch-form to column), set .dispatch-form select and
.dispatch-form input to use max-width: 100% / min-width: auto (or width: 100%)
so they shrink to the container, and make `#dispatch-go` and .dispatch-toggle
adapt (full-width or auto with flex-shrink) so the action/status don't get
pushed off-screen; update rules for .dispatch-form.open, .dispatch-form select,
.dispatch-form input, `#dispatch-go`, .dispatch-toggle and `#dispatch-panel` within
that media query.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2b0f3386-d009-4a50-8cb5-1e2879dbf2ad
📒 Files selected for processing (3)
dashboard/app.jsdashboard/index.htmldashboard/style.css
- Dashboard: filter terminal runs from stage dot rendering - Route review step events to reviewer agent instead of source - Emit workflow.step.started in control-DAG execution path - Distinguish Warning from Failed in step completion events - Protect event log rotation with atomic rename Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Mar 21, 2026
This was referenced Mar 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
workflow.step.started,workflow.step.completed,workflow.step.failedevents emitted fromWorkflowExecutor::executewith step index, type, agent, and workflow metadataevents.jsonlauto-rotates at 5k events — archives oldest, retains 2.5k in active fileappState.runstracks workflow runs by correlation_id; animated green dots on stages, red dots for failures, marching-dash flow connectors between active stagesTest plan
cargo test— 327 tests pass (326 unit + 1 integration)cargo clippy -- -D warnings— cleancargo fmt -- --check— cleantt run, verify dots appear and animate through stagesdashboard/test.htmlin browser, verify all JS tests pass🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Style