feat(cue): UI polish — trigger selector, flexible nodes, activity graph, bolt animation#591
Merged
feat(cue): UI polish — trigger selector, flexible nodes, activity graph, bolt animation#591
Conversation
… nodes, activity graph CUE bars, bolt pulse animation - Remove agent.completed from TriggerDrawer selector (chaining mechanism kept intact) - TriggerNode: flexible width (220–320px) with title tooltips on label and config summary - ActivityGraph: include CUE entries as stacked bar segments with cyan color and tooltip - SessionItem: pulse Zap icon via animate-pulse when Cue pipeline is actively running - SessionList: propagate activeRuns from CueSessionStatus to drive bolt animation
|
Warning Ignoring CodeRabbit configuration file changes. For security, only the configuration from the base branch is applied for open source repositories. 📝 WalkthroughWalkthroughRemoved the "Agent Done" trigger; added Maestro Cue tracking (counts + running state) across session and history UIs; made TriggerNode layout responsive with truncation tooltips; extended ActivityGraph to aggregate/render CUE counts; and added/updated tests covering these behaviors. Changes
Sequence DiagramsequenceDiagram
participant SL as SessionList
participant FS as Cue Fetch Service
participant SM as cueSessionMap (state)
participant SI as SessionItem
participant UI as Cue Indicator
SL->>FS: request cue status for sessionIds
FS-->>SL: return [{ sessionId, subscriptionCount, activeRuns }, ...]
SL->>SM: set(sessionId -> { count: subscriptionCount, active: activeRuns > 0 })
SL->>SI: render(sessionId, cueSubscriptionCount, cueActiveRun)
SI->>UI: render indicator with count + title
alt cueActiveRun === true
UI->>UI: add animate-pulse
UI->>UI: title "Maestro Cue running (X subscriptions)"
else
UI->>UI: no pulse
UI->>UI: title "Maestro Cue active (X subscriptions)"
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Contributor
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
Greptile SummaryThis PR delivers four UI-polish features for the CUE pipeline system: hiding Key changes:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[window.maestro.cue.getStatus] -->|activeRuns > 0| B{cueActiveRun = true}
A -->|activeRuns = 0| C{cueActiveRun = false}
B --> D["SessionItem\ncueSubscriptionCount prop"]
C --> D
D -->|cueActiveRun = true| E["Zap icon\nanimate-pulse\ntooltip: 'running'"]
D -->|cueActiveRun = false| F["Zap icon\nstatic\ntooltip: 'active'"]
G[HistoryEntry type = CUE] --> H[ActivityGraph bucket.cue++]
H --> I[cuePercent = bucket.cue / total * 100]
I --> J["Cyan bar segment #06b6d4\n(middle, between auto and user)"]
J --> K{hoveredIndex?}
K -->|yes| L["Tooltip: Cue row shown\n(only when cue > 0)"]
K -->|no| M["Summary title:\ntotalCue > 0 ? add ', N cue' : omit"]
N[TriggerDrawer] -->|agent.completed removed| O[6 trigger types shown]
N --> P[TriggerNode still handles\nagent.completed in EVENT_COLORS/ICONS]
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/renderer/components/History/ActivityGraph.tsx (1)
6-7: Consider centralizingCUE_COLORin shared history constants.This avoids color drift between
ActivityGraph,HistoryFilterToggle, andHistoryEntryItemover time.♻️ Suggested refactor
-/** Hardcoded CUE brand color — matches HistoryFilterToggle and HistoryEntryItem */ -const CUE_COLOR = '#06b6d4'; +// src/renderer/components/History/historyConstants.ts +export const CUE_COLOR = '#06b6d4';-import { LOOKBACK_OPTIONS } from './historyConstants'; +import { LOOKBACK_OPTIONS, CUE_COLOR } from './historyConstants';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/History/ActivityGraph.tsx` around lines 6 - 7, Extract the hardcoded CUE_COLOR constant into a shared history constants module (export const CUE_COLOR = '#06b6d4') and update ActivityGraph (where CUE_COLOR is currently defined), HistoryFilterToggle, and HistoryEntryItem to import CUE_COLOR from that module; ensure all references use the imported symbol so a single source of truth controls the color and prevents drift.src/__tests__/renderer/components/History/ActivityGraph.test.tsx (1)
352-352: Scope the"2"assertion to the Cue row to avoid accidental matches.
screen.getByText('2')is global and can become brittle as UI text evolves.♻️ Suggested test hardening
-import { render, screen, fireEvent, act } from '@testing-library/react'; +import { render, screen, fireEvent, act, within } from '@testing-library/react';- expect(screen.getByText('Cue')).toBeInTheDocument(); - expect(screen.getByText('2')).toBeInTheDocument(); // 2 CUE entries + const cueLabel = screen.getByText('Cue'); + expect(cueLabel).toBeInTheDocument(); + const cueRow = cueLabel.closest('div'); + expect(cueRow).not.toBeNull(); + expect(within(cueRow as HTMLElement).getByText('2')).toBeInTheDocument(); // 2 CUE entries🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/History/ActivityGraph.test.tsx` at line 352, The assertion using screen.getByText('2') is global and brittle; instead scope it to the Cue row by locating the Cue row first (e.g., use screen.getByRole('row', { name: /Cue/i }) or find the element that contains 'Cue') and then use within(foundRow).getByText('2') so the '2' is asserted only inside the Cue row; update the assertion in ActivityGraph.test.tsx that currently calls screen.getByText('2') to this scoped approach.src/__tests__/renderer/components/CuePipelineEditor/nodes/TriggerNode.test.tsx (2)
119-123: Strengthen selected-style assertion to avoid false positives.
borderColorstring checks against'60'are brittle because style values may be normalized. Compare selected vs unselected root styles directly instead.Proposed test hardening
it('should apply selection styling when selected', () => { - const { container } = renderTriggerNode({}, true); - - const rootDiv = container.querySelector('div[style*="min-width: 220px"]') as HTMLElement; - // Selected nodes have the full color border (not the 60% opacity variant) - expect(rootDiv.style.borderColor).not.toContain('60'); - // Selected nodes have a box shadow - expect(rootDiv.style.boxShadow).toBeTruthy(); + const { container: selectedContainer } = renderTriggerNode({}, true); + const { container: unselectedContainer } = renderTriggerNode({}, false); + + const selectedRoot = selectedContainer.querySelector('div[style*="min-width: 220px"]') as HTMLElement; + const unselectedRoot = unselectedContainer.querySelector('div[style*="min-width: 220px"]') as HTMLElement; + + expect(selectedRoot.style.boxShadow).toBeTruthy(); + expect(unselectedRoot.style.boxShadow).toBe(''); + expect(selectedRoot.style.border).not.toBe(unselectedRoot.style.border); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/CuePipelineEditor/nodes/TriggerNode.test.tsx` around lines 119 - 123, The current test checks selected node styling by searching for '60' inside rootDiv.style.borderColor which is brittle; instead query an unselected node (e.g., locate the unselected root element similarly to how rootDiv is found), read its computed styles for borderColor and boxShadow (or rootDiv.style for inline styles used in the component), and assert that rootDiv.style.borderColor !== unselectedRoot.style.borderColor and that rootDiv.style.boxShadow !== unselectedRoot.style.boxShadow (or that rootDiv has a truthy boxShadow while the unselected one is falsy) so the test directly compares selected vs unselected styles using the element identifiers rootDiv and the newly located unselectedRoot.
126-134: Addagent.completedto the event-color test matrix.
TriggerNodestill supports this event type, so excluding it here leaves one runtime path untested.Small coverage patch
const eventColors: Record<string, string> = { 'time.heartbeat': '#f59e0b', 'time.scheduled': '#8b5cf6', 'file.changed': '#3b82f6', + 'agent.completed': '#22c55e', 'github.pull_request': '#a855f7', 'github.issue': '#f97316', 'task.pending': '#06b6d4', };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/CuePipelineEditor/nodes/TriggerNode.test.tsx` around lines 126 - 134, The test matrix in the "should use correct color for each event type" test omits the 'agent.completed' event; update the eventColors object in TriggerNode.test.tsx to include an 'agent.completed' key with the same color string used by the TriggerNode implementation for that event. Locate the test's eventColors variable and add 'agent.completed': '<color-from-TriggerNode>' (use the exact hex/color literal from the TriggerNode code) so the runtime path for agent.completed is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/__tests__/renderer/components/CuePipelineEditor/nodes/TriggerNode.test.tsx`:
- Around line 119-123: The current test checks selected node styling by
searching for '60' inside rootDiv.style.borderColor which is brittle; instead
query an unselected node (e.g., locate the unselected root element similarly to
how rootDiv is found), read its computed styles for borderColor and boxShadow
(or rootDiv.style for inline styles used in the component), and assert that
rootDiv.style.borderColor !== unselectedRoot.style.borderColor and that
rootDiv.style.boxShadow !== unselectedRoot.style.boxShadow (or that rootDiv has
a truthy boxShadow while the unselected one is falsy) so the test directly
compares selected vs unselected styles using the element identifiers rootDiv and
the newly located unselectedRoot.
- Around line 126-134: The test matrix in the "should use correct color for each
event type" test omits the 'agent.completed' event; update the eventColors
object in TriggerNode.test.tsx to include an 'agent.completed' key with the same
color string used by the TriggerNode implementation for that event. Locate the
test's eventColors variable and add 'agent.completed':
'<color-from-TriggerNode>' (use the exact hex/color literal from the TriggerNode
code) so the runtime path for agent.completed is covered.
In `@src/__tests__/renderer/components/History/ActivityGraph.test.tsx`:
- Line 352: The assertion using screen.getByText('2') is global and brittle;
instead scope it to the Cue row by locating the Cue row first (e.g., use
screen.getByRole('row', { name: /Cue/i }) or find the element that contains
'Cue') and then use within(foundRow).getByText('2') so the '2' is asserted only
inside the Cue row; update the assertion in ActivityGraph.test.tsx that
currently calls screen.getByText('2') to this scoped approach.
In `@src/renderer/components/History/ActivityGraph.tsx`:
- Around line 6-7: Extract the hardcoded CUE_COLOR constant into a shared
history constants module (export const CUE_COLOR = '#06b6d4') and update
ActivityGraph (where CUE_COLOR is currently defined), HistoryFilterToggle, and
HistoryEntryItem to import CUE_COLOR from that module; ensure all references use
the imported symbol so a single source of truth controls the color and prevents
drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 892aaa6a-833a-4300-858c-d789061b4cdd
📒 Files selected for processing (9)
src/__tests__/renderer/components/CuePipelineEditor/drawers/TriggerDrawer.test.tsxsrc/__tests__/renderer/components/CuePipelineEditor/nodes/TriggerNode.test.tsxsrc/__tests__/renderer/components/History/ActivityGraph.test.tsxsrc/__tests__/renderer/components/SessionItemCue.test.tsxsrc/renderer/components/CuePipelineEditor/drawers/TriggerDrawer.tsxsrc/renderer/components/CuePipelineEditor/nodes/TriggerNode.tsxsrc/renderer/components/History/ActivityGraph.tsxsrc/renderer/components/SessionItem.tsxsrc/renderer/components/SessionList/SessionList.tsx
…ertions - Extract CUE_COLOR into historyConstants.tsx; update ActivityGraph, HistoryFilterToggle, and HistoryEntryItem to import from single source - TriggerNode selection test: compare selected vs unselected styles directly instead of brittle string containment check - TriggerNode color test: add agent.completed to event type coverage matrix - ActivityGraph CUE bucket test: scope count assertion to Cue row via within()
Contributor
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/renderer/components/History/ActivityGraph.tsx (1)
308-315: Consider showing Cue row consistently in tooltip.Auto and User rows display unconditionally (even when 0), but Cue only appears when
cue > 0. This asymmetry may be intentional for a "new feature" feel, but for consistency you could show all three rows always.♻️ Optional: Show Cue row unconditionally
- {bucketData[hoveredIndex].cue > 0 && ( - <div className="flex items-center justify-between gap-3"> - <span style={{ color: CUE_COLOR }}>Cue</span> - <span className="font-bold" style={{ color: CUE_COLOR }}> - {bucketData[hoveredIndex].cue} - </span> - </div> - )} + <div className="flex items-center justify-between gap-3"> + <span style={{ color: CUE_COLOR }}>Cue</span> + <span className="font-bold" style={{ color: CUE_COLOR }}> + {bucketData[hoveredIndex].cue} + </span> + </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/History/ActivityGraph.tsx` around lines 308 - 315, The tooltip currently conditionally renders the Cue row only when bucketData[hoveredIndex].cue > 0, which creates inconsistency with Auto and User rows; update the JSX in ActivityGraph.tsx to render the Cue row unconditionally (like Auto/User) by removing the conditional and always outputting the <div> that uses bucketData[hoveredIndex].cue and CUE_COLOR so the Cue label and value appear even when the value is 0. Ensure you reference bucketData, hoveredIndex and CUE_COLOR when making the change.src/renderer/components/History/HistoryEntryItem.tsx (1)
9-50: Consider extracting sharedgetPillColorandgetEntryIconhelpers.Both
HistoryFilterToggle.tsxand this file define identicalgetPillColorandgetEntryIconfunctions. These could be co-located withCUE_COLORinhistoryConstants.tsxto reduce duplication.♻️ Proposed consolidation
In
historyConstants.tsx:+import { Bot, User, Zap } from 'lucide-react'; +import type { Theme, HistoryEntryType } from '../../types'; + +export const getPillColor = (type: HistoryEntryType, theme: Theme) => { + switch (type) { + case 'AUTO': + return { bg: theme.colors.warning + '20', text: theme.colors.warning, border: theme.colors.warning + '40' }; + case 'USER': + return { bg: theme.colors.accent + '20', text: theme.colors.accent, border: theme.colors.accent + '40' }; + case 'CUE': + return { bg: CUE_COLOR + '20', text: CUE_COLOR, border: CUE_COLOR + '40' }; + default: + return { bg: theme.colors.bgActivity, text: theme.colors.textDim, border: theme.colors.border }; + } +}; + +export const getEntryIcon = (type: HistoryEntryType) => { + switch (type) { + case 'AUTO': return Bot; + case 'USER': return User; + case 'CUE': return Zap; + default: return Bot; + } +};Then import from both components.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/History/HistoryEntryItem.tsx` around lines 9 - 50, Extract the duplicated getPillColor and getEntryIcon implementations into historyConstants.tsx (next to CUE_COLOR), export them, and replace the local copies in HistoryEntryItem and HistoryFilterToggle with imports; ensure historyConstants.tsx also imports/export the lucide icons (Bot, User, Zap) and the types (Theme, HistoryEntryType) used by getPillColor/getEntryIcon so callers can simply import { getPillColor, getEntryIcon } and remove the duplicated functions from those components.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/renderer/components/History/ActivityGraph.tsx`:
- Around line 308-315: The tooltip currently conditionally renders the Cue row
only when bucketData[hoveredIndex].cue > 0, which creates inconsistency with
Auto and User rows; update the JSX in ActivityGraph.tsx to render the Cue row
unconditionally (like Auto/User) by removing the conditional and always
outputting the <div> that uses bucketData[hoveredIndex].cue and CUE_COLOR so the
Cue label and value appear even when the value is 0. Ensure you reference
bucketData, hoveredIndex and CUE_COLOR when making the change.
In `@src/renderer/components/History/HistoryEntryItem.tsx`:
- Around line 9-50: Extract the duplicated getPillColor and getEntryIcon
implementations into historyConstants.tsx (next to CUE_COLOR), export them, and
replace the local copies in HistoryEntryItem and HistoryFilterToggle with
imports; ensure historyConstants.tsx also imports/export the lucide icons (Bot,
User, Zap) and the types (Theme, HistoryEntryType) used by
getPillColor/getEntryIcon so callers can simply import { getPillColor,
getEntryIcon } and remove the duplicated functions from those components.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 95497b4f-9b13-44df-b1d8-a205a148c452
📒 Files selected for processing (6)
src/__tests__/renderer/components/CuePipelineEditor/nodes/TriggerNode.test.tsxsrc/__tests__/renderer/components/History/ActivityGraph.test.tsxsrc/renderer/components/History/ActivityGraph.tsxsrc/renderer/components/History/HistoryEntryItem.tsxsrc/renderer/components/History/HistoryFilterToggle.tsxsrc/renderer/components/History/historyConstants.tsx
✅ Files skipped from review due to trivial changes (1)
- src/renderer/components/History/historyConstants.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/tests/renderer/components/History/ActivityGraph.test.tsx
- src/tests/renderer/components/CuePipelineEditor/nodes/TriggerNode.test.tsx
…w unconditionally - Extract getPillColor and getEntryIcon into historyConstants.tsx; remove duplicated implementations from HistoryFilterToggle and HistoryEntryItem - Render Cue row unconditionally in ActivityGraph tooltip (consistent with Auto/User rows that always show even when count is 0) - Update test to assert Cue row shows with zero count instead of being absent
Contributor
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/components/History/ActivityGraph.tsx (1)
141-145:⚠️ Potential issue | 🟠 MajorMake the newly-clickable bars keyboard-accessible.
Including
cuein the bucket total makes CUE-only buckets actionable, but the bars are still mouse-only clickabledivs. Keyboard users still can't focus or trigger that drill-down path. Prefer a<button>here, or addtabIndex={0},outline-none, an accessible label, and Enter/Space handling that mirrors the existing click behavior.As per coding guidelines,
src/renderer/components/**/*.tsx: For focus issues, addtabIndex={0}ortabIndex={-1}and addoutline-noneclass.Also applies to: 324-345
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/History/ActivityGraph.tsx` around lines 141 - 145, ActivityGraph makes bars clickable via the onBarClick handler when total (bucketData[index].auto + bucketData[index].user + bucketData[index].cue) > 0 but the bars are plain divs and not keyboard accessible; update the interactive bar element (where you call getBucketTimeRange(index) and invoke onBarClick) to be keyboard-focusable and operable—either replace the div with a semantic <button> or add tabIndex={0}, className="outline-none", role="button", an accessible aria-label describing the bucket time range, and keyDown handlers that trigger the same logic on Enter/Space (mirror the click behavior); apply the same changes to the other bar block referenced around lines 324-345 so both mouse and keyboard users can drill down via onBarClick.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/renderer/components/History/ActivityGraph.tsx`:
- Around line 141-145: ActivityGraph makes bars clickable via the onBarClick
handler when total (bucketData[index].auto + bucketData[index].user +
bucketData[index].cue) > 0 but the bars are plain divs and not keyboard
accessible; update the interactive bar element (where you call
getBucketTimeRange(index) and invoke onBarClick) to be keyboard-focusable and
operable—either replace the div with a semantic <button> or add tabIndex={0},
className="outline-none", role="button", an accessible aria-label describing the
bucket time range, and keyDown handlers that trigger the same logic on
Enter/Space (mirror the click behavior); apply the same changes to the other bar
block referenced around lines 324-345 so both mouse and keyboard users can drill
down via onBarClick.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a53cff7b-07ad-46ac-8ec7-c717c9ff8a93
📒 Files selected for processing (6)
.coderabbit.yamlsrc/__tests__/renderer/components/History/ActivityGraph.test.tsxsrc/renderer/components/History/ActivityGraph.tsxsrc/renderer/components/History/HistoryEntryItem.tsxsrc/renderer/components/History/HistoryFilterToggle.tsxsrc/renderer/components/History/historyConstants.tsx
✅ Files skipped from review due to trivial changes (1)
- src/tests/renderer/components/History/ActivityGraph.test.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- src/renderer/components/History/HistoryFilterToggle.tsx
- src/renderer/components/History/HistoryEntryItem.tsx
- src/renderer/components/History/historyConstants.tsx
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
agent.completedfrom trigger selector — hidden from the TriggerDrawer drag-and-drop list so users can't manually create standalone "Agent Done" trigger nodes. The auto-generated chaining mechanism behind agent-to-agent edges remains fully intact.minWidth: 220px/maxWidth: 320pxinstead of a fixedwidth: 220px, gracefully growing to fit longer labels. Truncated text shows the full value via native tooltip on hover.#06b6d4), with tooltip and summary title support. CUE-only bars are clickable.animate-pulse) when the agent is actively running under a Cue pipeline, and stays static when idle. Tooltip updates to "running" vs "active" accordingly.Test plan
npx tsc --noEmit— zero type errorsnpm run test— all tests pass (349 across affected suites)Summary by CodeRabbit
New Features
Improvements
Layout