fix(cue): empty-graph CTA + tighten regression coverage#924
fix(cue): empty-graph CTA + tighten regression coverage#924reachrazamair merged 2 commits intorcfrom
Conversation
Trace: useCueGraphData fetches and returns []; usePipelineLayout's restore effect early-returns on empty graphSessions and never marks pipelinesLoaded=true. The editor's `graphLoading || !pipelinesLoaded` gate then evaluated to `false || true === true`, leaving the spinner up forever instead of falling through to "Create your first pipeline". Fix gates `!pipelinesLoaded` behind `graphSessions.length > 0` so an empty fetch result correctly drops to the CTA. Surgical one-line change at the parent boundary; usePipelineLayout's hook semantics and tests stay untouched. Regression test: CuePipelineEditor.loadingGate.test.tsx covers the four combinations of graphLoading × pipelinesLoaded × graphSessions empty/non.
Closes coverage gaps for five recent fixes that shipped without regression tests, and adds a round-trip shapes harness to catch future field-loss bugs in the pipeline ↔ YAML serialization path. Regression tests for previously-uncovered fixes: - 42ac833: tab switch must clear pendingPipelineId; toast title drops the raw ISO leak in favor of locale time + ms - 96a87a1: NodeConfigPanel header mirrors the canvas's "(N)" instance suffix, isolated per pipeline - f94108e: usePipelineLayout's request-id guard wins both stale-resolves-first and fresh-resolves-first orderings - 30ab9a5: fan-in tracker forwards max chainDepth across completions in both all-complete and timeout-continue paths - d85290c: calculateNextScheduledTime resolves same-day-next-week when today's slot has already passed Round-trip harness (pipelineRoundTrip.shapes.test.ts): - Trigger fan-out preserves all targets + per-target prompts (fan_out_prompt_files distribution) - Multi-agent linear chain preserves topology + agent inputPrompts - Trigger event configs (heartbeat, scheduled, file.changed, github.pull_request) round-trip exactly - Double round-trip is structurally idempotent Comments call out two design facts surfaced while building this: edge mode (autorun/debate) is YAML-comment-only and is recovered from pipelineLayout.json sidecar, not the YAML body; chain prompts canonically live on agent.inputPrompt, not edge.prompt — so future readers don't waste time on the same dead-ends. 30 tests added across 8 files. Cue suite now at 2,164 tests / 116 files passing.
📝 WalkthroughWalkthroughThis PR adds comprehensive regression test coverage across the Cue engine, pipeline editor, and related components, including tests for scheduling behavior, fan-in depth propagation, tab navigation, loading state transitions, agent header rendering, pipeline serialization round-trips, and race conditions in layout restoration. It also includes a minor fix to the PipelineCanvas loading condition. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
Greptile SummaryThe one-line production change corrects a loading gate in Confidence Score: 4/5Safe to merge — production change is a correct one-liner; only a minor test harness fidelity gap was found. The production fix is logically sound and verified against all four gate states in the new tests. The only finding is a P2 in the round-trip test helper that leaves src/tests/renderer/components/CuePipelineEditor/utils/pipelineRoundTrip.shapes.test.ts — the Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["useCueGraphData fetch completes"] --> B{graphSessions.length > 0?}
B -- "No (empty graph)" --> C["isLoading = graphLoading || false"]
C --> D{graphLoading?}
D -- "true" --> E["Spinner (fetch in progress)"]
D -- "false" --> F["CTA: Create your first pipeline"]
B -- "Yes (has sessions)" --> G["isLoading = graphLoading || !pipelinesLoaded"]
G --> H{graphLoading?}
H -- "true" --> E
H -- "false" --> I{pipelinesLoaded?}
I -- "false" --> E2["Spinner (layout restoring)"]
I -- "true" --> J["Pipeline Editor renders"]
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/__tests__/main/cue/cue-engine.test.ts (1)
1856-1860: Test name/comment is slightly misleading vs asserted behavior.Line 1856 says “same day next week,” but this case asserts Friday of the same week (Line 1865-1866). Consider renaming for clarity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/main/cue/cue-engine.test.ts` around lines 1856 - 1860, The test name and leading comment for the spec using the case "resolves to same day next week with multi-day filter when today is the only matching day past its slot" are misleading because the assertions check that the schedule picks Friday of the same week (not next Wednesday); update the test title string and the comment inside the test to accurately describe that when Wednesday's slot is past and Friday is the next matching day, the engine resolves to the same week's Friday (preserving the "picks-earliest" semantics). Locate the spec by its name in the test suite (the string passed to it(...) around the block and the inline comment referencing "Wednesday 2026-03-11 at 23:59 — schedule fires Wed/Fri at 09:00") and change both the test name and comment to reflect "resolves to same-week Friday with multi-day filter when today is the only matching day past its slot" or equivalent clearer wording.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/__tests__/renderer/components/CueModal.test.tsx`:
- Around line 447-448: The assertion checking
capturedEditorProps.initialPipelineId is vacuous unless the editor actually
mounted; update the test to either remove that line or first assert the editor
mounted by checking the renderCount (e.g., ensure renderCount for
CuePipelineEditor is > 0) before reading capturedEditorProps, so you only
inspect initialPipelineId after confirming CuePipelineEditor rendered.
---
Nitpick comments:
In `@src/__tests__/main/cue/cue-engine.test.ts`:
- Around line 1856-1860: The test name and leading comment for the spec using
the case "resolves to same day next week with multi-day filter when today is the
only matching day past its slot" are misleading because the assertions check
that the schedule picks Friday of the same week (not next Wednesday); update the
test title string and the comment inside the test to accurately describe that
when Wednesday's slot is past and Friday is the next matching day, the engine
resolves to the same week's Friday (preserving the "picks-earliest" semantics).
Locate the spec by its name in the test suite (the string passed to it(...)
around the block and the inline comment referencing "Wednesday 2026-03-11 at
23:59 — schedule fires Wed/Fri at 09:00") and change both the test name and
comment to reflect "resolves to same-week Friday with multi-day filter when
today is the only matching day past its slot" or equivalent clearer wording.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 593b47b3-9f04-47da-abaf-fb30d20c64ec
📒 Files selected for processing (9)
src/__tests__/main/cue/cue-engine.test.tssrc/__tests__/main/cue/cue-fan-in-tracker.test.tssrc/__tests__/renderer/components/CueModal.test.tsxsrc/__tests__/renderer/components/CuePipelineEditor/CuePipelineEditor.loadingGate.test.tsxsrc/__tests__/renderer/components/CuePipelineEditor/panels/NodeConfigPanel.test.tsxsrc/__tests__/renderer/components/CuePipelineEditor/utils/pipelineRoundTrip.shapes.test.tssrc/__tests__/renderer/hooks/cue/usePipelineLayout.test.tssrc/__tests__/renderer/hooks/useCue.test.tssrc/renderer/components/CuePipelineEditor/CuePipelineEditor.tsx
| // Default tab is 'pipeline' — editor renders with no pending token. | ||
| expect(capturedEditorProps.initialPipelineId).toBeUndefined(); |
There was a problem hiding this comment.
This initial token check can pass without mounting the editor.
beforeEach already resets capturedEditorProps, and this suite earlier asserts the dashboard is the default view, so expect(capturedEditorProps.initialPipelineId).toBeUndefined() is vacuous unless you first prove CuePipelineEditor rendered. Either drop this check or use the new renderCount capture to assert the editor actually mounted before reading its props.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/__tests__/renderer/components/CueModal.test.tsx` around lines 447 - 448,
The assertion checking capturedEditorProps.initialPipelineId is vacuous unless
the editor actually mounted; update the test to either remove that line or first
assert the editor mounted by checking the renderCount (e.g., ensure renderCount
for CuePipelineEditor is > 0) before reading capturedEditorProps, so you only
inspect initialPipelineId after confirming CuePipelineEditor rendered.
1 participant
Summary
Fixes the bug where the Cue pipeline editor showed a spinner forever instead of the "Create your first pipeline" CTA when no Cue subscriptions were registered. Bundles regression coverage for five recent fixes that shipped without tests, plus a round-trip harness for the pipeline ↔ YAML serialization path.
The bug fix
useCueGraphDatareturns[]when no agent has Cue subscriptions and flipsgraphInitialLoadingtofalse. ButusePipelineLayout's restore effect early-returns on emptygraphSessionsand never markspipelinesLoaded=true. The editor'sgraphLoading || !pipelinesLoadedgate then evaluated tofalse || true === true, so the spinner never went away.One-line fix at the parent boundary: gate
!pipelinesLoadedbehindgraphSessions.length > 0so an empty fetch result correctly drops to the CTA.The
usePipelineLayouthook's semantics and existing tests stay untouched — the integration logic lives where both signals are available.Why the broader test additions
Recent Cue branch history is ~53% bugfix commits. The pattern: most bugs sit at integration boundaries (data round-trips, multi-pipeline state, race conditions) — places unit tests don't cover. This PR closes the highest-value gaps:
Regression tests for five recent fixes that shipped without coverage:
42ac8333ependingPipelineId; toast title leaked raw ISO96a87a19cf94108e7busePipelineLayoutrace whengraphSessionschange mid-fetch30ab9a5c4d85290c51calculateNextScheduledTimereturnednullfor same-day-next-weekRound-trip harness (
pipelineRoundTrip.shapes.test.ts):Mirrors the runtime's
pipelinesToYaml → yaml.load → subscriptionsToPipelinescycle, including the normalizer's prompt-file inlining (handles bothprompt_fileandfan_out_prompt_files). 10 tests covering trigger fan-out, multi-agent chains, every trigger event config (heartbeat / scheduled / file.changed / github.pull_request), and double-round-trip idempotence.While building it I uncovered two design facts now documented inline so future readers don't waste time on the same dead-ends:
mode(autorun / debate) is YAML-comment-only by design (getEdgeModeComment) and recovered from the layout sidecar, not the YAML bodyagent.inputPrompt, not on the chain edge —pipelineToYamlonly readsedge.promptfor trigger→agent edgesHonest scope note
These are bounded regression guards anchored to specific fixes — they catch reverts of the corresponding code, not novel bug paths. The round-trip harness is the most generalizable: any future field-loss in the YAML serialization path will fail it.
The 5-pick list deliberately skipped
89833d325(scheduledFiredKeys session scoping) because the registry refactor makes the bug structurally impossible — re-introducing it would require an API change that existing tests already block.Stats
Test plan
graphLoading=truepath unchanged)npm run test -- --run src/__tests__/main/cue src/__tests__/renderer/components/CuePipelineEditor src/__tests__/renderer/hooks/cue src/__tests__/renderer/components/CueModal.test.tsx src/__tests__/renderer/hooks/useCue.test.ts src/__tests__/shared/cuepassesnpx tsc --noEmitcleanSummary by CodeRabbit