fix(cue): enable multi-agent pipeline output passing and polish editor UX#572
Merged
fix(cue): enable multi-agent pipeline output passing and polish editor UX#572
Conversation
Pipeline-generated chain subscriptions were missing the {{CUE_SOURCE_OUTPUT}}
template variable, so upstream agent output never reached downstream agents.
Added ensureSourceOutputVariable() helper and applied it in buildChain().
Add includeUpstreamOutput toggle to agent nodes so users can opt out of
auto-injected {{CUE_SOURCE_OUTPUT}}. Update NodeConfigPanel with contextual
placeholder, checkbox, and placement hint when agent has incoming agent edges.
Strip auto-injected prefix in yamlToPipeline to prevent accumulation on
round-trip. 5 new tests covering toggle and prefix stripping.
Add outputTruncated boolean to both single-source and fan-in completion event payloads so downstream consumers know when source output was clipped at 5000 chars. Remove debug console.log from claude-output-parser that was logging raw usage data to stdout in production.
Debate and autorun edge modes have no backend implementation yet. Mark them as disabled with "(Soon)" label and tooltip to set correct user expectations.
End-to-end tests verifying the full flow: pipelineToYamlSubscriptions
generates {{CUE_SOURCE_OUTPUT}} in chain prompts, and
substituteTemplateVariables resolves it to actual source output.
Covers normal, empty, and multi-content scenarios.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds opt-in upstream-output injection/stripping via {{CUE_SOURCE_OUTPUT}} with ensureSourceOutputVariable and includeUpstreamOutput; UI controls for incoming-agent edges and auto-include; propagate outputTruncated through engine/fan-in and onLog hooks; tighten numeric filters and YAML validation; expand tests; enable Cmd+T to create AI tab. Changes
Sequence DiagramsequenceDiagram
participant UpAgent as UpstreamAgent
participant Engine as CUEEngine
participant PtoY as pipelineToYaml
participant Resolver as TemplateResolver
participant DownAgent as DownstreamAgent
participant YtoP as yamlToPipeline
UpAgent->>Engine: emit completion(stdout)
Engine->>Engine: compute output & truncated flag
Engine->>PtoY: emit agent.completed subscription (includeUpstreamOutput?)
PtoY->>PtoY: ensureSourceOutputVariable(prompt) when injecting
PtoY->>Resolver: deliver subscription containing {{CUE_SOURCE_OUTPUT}}
Resolver->>DownAgent: resolved prompt with injected upstream output
DownAgent->>YtoP: persist/import pipeline
YtoP->>YtoP: strip auto-injected "{{CUE_SOURCE_OUTPUT}}" prefix on import
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 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 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 |
Collaborator
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
Greptile SummaryThis PR fixes a core multi-agent pipeline bug where downstream agents never received upstream output because Three issues were found during review:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as Pipeline Editor UI
participant ToYaml as pipelineToYaml.ts
participant YAML as cue.yaml
participant Engine as CueEngine
participant Agent1 as Upstream Agent
participant Agent2 as Downstream Agent
UI->>ToYaml: Save pipeline (includeUpstreamOutput=true)
ToYaml->>ToYaml: ensureSourceOutputVariable(prompt)<br/>→ prepends {{CUE_SOURCE_OUTPUT}}
ToYaml->>YAML: Write subscription with<br/>prompt: "{{CUE_SOURCE_OUTPUT}}\n\nUser prompt"
Agent1-->>Engine: notifyAgentCompleted(stdout)
Engine->>Engine: slice(-5000), set outputTruncated flag
Engine->>Agent2: CueEvent { sourceOutput, outputTruncated }
Engine->>Engine: substituteTemplateVariables<br/>{{CUE_SOURCE_OUTPUT}} → actual output
Engine->>Agent2: Final prompt with upstream output injected
UI->>ToYaml: Load YAML back (yamlToPipeline)
ToYaml->>ToYaml: Strip "{{CUE_SOURCE_OUTPUT}}\n\n" prefix<br/>restore inputPrompt for display
ToYaml->>UI: AgentNodeData { inputPrompt (stripped) }
Note over UI,Agent2: ⚠ includeUpstreamOutput:false is NOT<br/>restored on load — setting is lost
Note over Engine: ⚠ outputTruncated missing in<br/>fan-in timeout (continue) path
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
350
to
+356
| if (sub.prompt) { | ||
| (targetNode.data as AgentNodeData).inputPrompt = sub.prompt; | ||
| // Strip auto-injected {{CUE_SOURCE_OUTPUT}} prefix to prevent accumulation on round-trip | ||
| const AUTO_PREFIX = '{{CUE_SOURCE_OUTPUT}}\n\n'; | ||
| const strippedPrompt = sub.prompt.startsWith(AUTO_PREFIX) | ||
| ? sub.prompt.slice(AUTO_PREFIX.length) | ||
| : sub.prompt; | ||
| (targetNode.data as AgentNodeData).inputPrompt = strippedPrompt || undefined; |
There was a problem hiding this comment.
includeUpstreamOutput: false lost on YAML round-trip
When a user toggles "Auto-include upstream output" off (includeUpstreamOutput: false), the generated YAML omits the {{CUE_SOURCE_OUTPUT}}\n\n prefix. However, when that YAML is loaded back, this code only strips the prefix if it's present — it never sets includeUpstreamOutput: false on the reconstructed node. The loaded node therefore has includeUpstreamOutput: undefined, which pipelineToYaml.ts treats as true (targetData.includeUpstreamOutput !== false is true for undefined).
Result: The user's opt-out is silently overridden on the next save — the prefix is auto-added back, and the checkbox in the UI shows "checked" even though the user had unchecked it.
The fix is to infer includeUpstreamOutput: false when the prompt for an agent.completed chain subscription does not contain {{CUE_SOURCE_OUTPUT}} at all:
if (sub.prompt) {
const AUTO_PREFIX = '{{CUE_SOURCE_OUTPUT}}\n\n';
const hasVariable = sub.prompt.toUpperCase().includes('{{CUE_SOURCE_OUTPUT}}');
if (sub.prompt.startsWith(AUTO_PREFIX)) {
const strippedPrompt = sub.prompt.slice(AUTO_PREFIX.length);
(targetNode.data as AgentNodeData).inputPrompt = strippedPrompt || undefined;
// includeUpstreamOutput stays undefined (treated as true) — correct
} else if (!hasVariable) {
// User explicitly placed no variable → they opted out
(targetNode.data as AgentNodeData).inputPrompt = sub.prompt || undefined;
(targetNode.data as AgentNodeData).includeUpstreamOutput = false;
} else {
// Variable is present but not as an auto-prefix (user-placed)
(targetNode.data as AgentNodeData).inputPrompt = sub.prompt;
}
}
Comment on lines
+165
to
171
| cursor: comingSoon ? 'default' : 'pointer', | ||
| transition: 'all 0.15s', | ||
| opacity: comingSoon ? 0.5 : 1, | ||
| pointerEvents: comingSoon ? 'none' : undefined, | ||
| }} | ||
| title={comingSoon ? 'Coming soon' : undefined} | ||
| > |
There was a problem hiding this comment.
pointer-events: none suppresses the "Coming soon" tooltip
The title attribute tooltip is a native browser tooltip that requires the element to receive mouseover events. Setting pointerEvents: 'none' on the button prevents all pointer events from reaching it, so the tooltip defined on line 170 (title={comingSoon ? 'Coming soon' : undefined}) will never be shown to users.
Additionally, the if (comingSoon) return; guard in the onClick handler (line 147) is now dead code — the click can never fire when pointer-events: none is set.
To display the tooltip, move pointer-events: none to the inner content and keep the outer button as the event target, or wrap the button in a container that renders the tooltip:
style={{
// ...
cursor: comingSoon ? 'default' : 'pointer',
opacity: comingSoon ? 0.5 : 1,
// Remove: pointerEvents: comingSoon ? 'none' : undefined,
}}
onClick={() => {
if (comingSoon) return; // keep this guard
// ...
}}
title={comingSoon ? 'Coming soon' : undefined}![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/main/cue/cue-engine.ts`:
- Around line 712-713: The fan-in timeout/continue completion payloads omit the
outputTruncated flag, causing inconsistent downstream handling; update the
timeout and continue payload builders in cue-engine (the fan-in timeout/continue
branch near the fan-in completion logic where completions are collected and a
payload is created) to include outputTruncated: completions.some(c =>
c.truncated) (same expression used for the normal fan-in output), ensuring both
timeout and continue payloads propagate the same flag (modify the payload
objects constructed in the timeout and continue handlers so they include
outputTruncated).
In `@src/renderer/components/CuePipelineEditor/panels/EdgeConfigPanel.tsx`:
- Around line 165-170: The current style uses pointerEvents: 'none' when
comingSoon is true which prevents the title/tooltip from showing; in
EdgeConfigPanel.tsx remove pointerEvents: comingSoon ? 'none' : undefined from
the style and instead guard actions via the component's click handler (check the
comingSoon flag in the onClick handler of the button/element) or set a
disabled/aria-disabled state so the element still receives hover events for the
title/tooltip; keep cursor and opacity logic (cursor: comingSoon ? 'default' :
'pointer', opacity: comingSoon ? 0.5 : 1) and ensure title={comingSoon ? 'Coming
soon' : undefined} remains to show the tooltip.
In `@src/renderer/components/CuePipelineEditor/panels/NodeConfigPanel.tsx`:
- Around line 517-521: The placeholder text currently only checks
hasIncomingAgentEdges and therefore can claim "Upstream output is auto-included"
even when the node's includeUpstreamOutput is false; update the placeholder
logic in NodeConfigPanel to consider both hasIncomingAgentEdges and the node's
includeUpstreamOutput flag (the state/prop tied to the checkbox named
includeUpstreamOutput) so it only shows the "auto-included via
{{CUE_SOURCE_OUTPUT}}" message when hasIncomingAgentEdges === true AND
includeUpstreamOutput === true; locate the placeholder usage near the
input/textarea render in the NodeConfigPanel component and adjust the
conditional to reflect both symbols.
In `@src/renderer/components/CuePipelineEditor/utils/yamlToPipeline.ts`:
- Around line 351-356: The current logic only removes the auto-injected prefix
AUTO_PREFIX ('{{CUE_SOURCE_OUTPUT}}\n\n') so bare tokens like
'{{CUE_SOURCE_OUTPUT}}' remain; update the stripping in the yamlToPipeline
conversion (the block that computes AUTO_PREFIX, strippedPrompt and assigns
(targetNode.data as AgentNodeData).inputPrompt) to also detect and remove the
bare token variant (e.g., BARE_TOKEN = '{{CUE_SOURCE_OUTPUT}}') — handle cases
where sub.prompt equals the bare token or starts with the bare token followed by
optional newlines, then slice those characters out before assigning inputPrompt
(use the existing strippedPrompt variable logic to set undefined when empty).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 733e4d52-9a81-40aa-a6bf-eac2d54c4917
📒 Files selected for processing (12)
src/__tests__/main/cue/cue-completion-chains.test.tssrc/__tests__/renderer/components/CuePipelineEditor/utils/pipelineChainIntegration.test.tssrc/__tests__/renderer/components/CuePipelineEditor/utils/pipelineToYaml.test.tssrc/__tests__/renderer/components/CuePipelineEditor/utils/yamlToPipeline.test.tssrc/main/cue/cue-engine.tssrc/main/parsers/claude-output-parser.tssrc/renderer/components/CuePipelineEditor/CuePipelineEditor.tsxsrc/renderer/components/CuePipelineEditor/panels/EdgeConfigPanel.tsxsrc/renderer/components/CuePipelineEditor/panels/NodeConfigPanel.tsxsrc/renderer/components/CuePipelineEditor/utils/pipelineToYaml.tssrc/renderer/components/CuePipelineEditor/utils/yamlToPipeline.tssrc/shared/cue-pipeline-types.ts
💤 Files with no reviewable changes (1)
- src/main/parsers/claude-output-parser.ts
Comment on lines
+712
to
713
| outputTruncated: completions.some((c) => c.truncated), | ||
| }, |
There was a problem hiding this comment.
Propagate outputTruncated in fan-in timeout/continue payloads as well.
outputTruncated is now set for fan-in completion, but the timeout-continue completion payload (Line 762 onward) omits it. That makes downstream event handling inconsistent depending on timeout path.
Suggested fix
const event: CueEvent = {
id: crypto.randomUUID(),
type: 'agent.completed',
timestamp: new Date().toISOString(),
triggerName: sub.name,
payload: {
completedSessions: completions.map((c) => c.sessionId),
timedOutSessions: timedOutSources,
sourceSession: completions.map((c) => c.sessionName).join(', '),
sourceOutput: completions.map((c) => c.output).join('\n---\n'),
+ outputTruncated: completions.some((c) => c.truncated),
partial: true,
},
};📝 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
| outputTruncated: completions.some((c) => c.truncated), | |
| }, | |
| const event: CueEvent = { | |
| id: crypto.randomUUID(), | |
| type: 'agent.completed', | |
| timestamp: new Date().toISOString(), | |
| triggerName: sub.name, | |
| payload: { | |
| completedSessions: completions.map((c) => c.sessionId), | |
| timedOutSessions: timedOutSources, | |
| sourceSession: completions.map((c) => c.sessionName).join(', '), | |
| sourceOutput: completions.map((c) => c.output).join('\n---\n'), | |
| outputTruncated: completions.some((c) => c.truncated), | |
| partial: true, | |
| }, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/cue/cue-engine.ts` around lines 712 - 713, The fan-in
timeout/continue completion payloads omit the outputTruncated flag, causing
inconsistent downstream handling; update the timeout and continue payload
builders in cue-engine (the fan-in timeout/continue branch near the fan-in
completion logic where completions are collected and a payload is created) to
include outputTruncated: completions.some(c => c.truncated) (same expression
used for the normal fan-in output), ensuring both timeout and continue payloads
propagate the same flag (modify the payload objects constructed in the timeout
and continue handlers so they include outputTruncated).
…e token stripping, timeout payload
- Remove pointerEvents: 'none' from coming-soon edge buttons so title
tooltip still shows on hover (onClick guard already prevents action)
- Update NodeConfigPanel placeholder to check both hasIncomingAgentEdges
AND includeUpstreamOutput before claiming output is auto-included
- Strip bare {{CUE_SOURCE_OUTPUT}} token (without trailing \n\n) in
yamlToPipeline round-trip
- Add outputTruncated to fan-in timeout continue-mode payload for
consistency with normal fan-in and single-source paths
Collaborator
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/__tests__/main/cue/cue-completion-chains.test.ts (1)
159-213: Add one exact-limit boundary test (5000 chars).Nice above/below-limit coverage. Adding an exact-5000-char case would lock the intended
>truncation semantics and guard against off-by-one regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/main/cue/cue-completion-chains.test.ts` around lines 159 - 213, Add a new unit test in the same suite that sends exactly 5000 characters to CueEngine.notifyAgentCompleted and asserts the engine does not truncate: createMockConfig/subscriptions as in the other tests, instantiate CueEngine(deps), start it, call notifyAgentCompleted('agent-a', { stdout: 'x'.repeat(5000) }), then inspect the request from deps.onCueRun mock and assert (event.payload.sourceOutput as string).length === 5000 and event.payload.outputTruncated === false so the boundary behavior (">5000" triggers truncation) is enforced; reference CueEngine.notifyAgentCompleted and the deps.onCueRun mock to locate where to add the test.
🤖 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__/main/cue/cue-completion-chains.test.ts`:
- Around line 159-213: Add a new unit test in the same suite that sends exactly
5000 characters to CueEngine.notifyAgentCompleted and asserts the engine does
not truncate: createMockConfig/subscriptions as in the other tests, instantiate
CueEngine(deps), start it, call notifyAgentCompleted('agent-a', { stdout:
'x'.repeat(5000) }), then inspect the request from deps.onCueRun mock and assert
(event.payload.sourceOutput as string).length === 5000 and
event.payload.outputTruncated === false so the boundary behavior (">5000"
triggers truncation) is enforced; reference CueEngine.notifyAgentCompleted and
the deps.onCueRun mock to locate where to add the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 326de079-83c7-4796-b672-73889f35e936
📒 Files selected for processing (6)
src/__tests__/main/cue/cue-completion-chains.test.tssrc/__tests__/renderer/components/CuePipelineEditor/utils/yamlToPipeline.test.tssrc/main/cue/cue-engine.tssrc/renderer/components/CuePipelineEditor/panels/EdgeConfigPanel.tsxsrc/renderer/components/CuePipelineEditor/panels/NodeConfigPanel.tsxsrc/renderer/components/CuePipelineEditor/utils/yamlToPipeline.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/tests/renderer/components/CuePipelineEditor/utils/yamlToPipeline.test.ts
- src/main/cue/cue-engine.ts
…oduction readiness Fix NaN bug in numeric filter comparisons where non-numeric payload values silently passed through >=, <=, >, < filters. Route file watcher errors through engine logger instead of console.error. Add validation for duplicate/empty subscription names, HH:MM time ranges, interval upper bounds, and label field parsing. 26 new tests (408 total).
Collaborator
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/cue/cue-file-watcher.ts (1)
19-19: Preserve error context in the watcher logging path.At Line 74 and Line 76, only a formatted string is emitted. Consider passing the original
errorobject through the callback/fallback so stack/context is not lost.Proposed change
+import type { BaseLogLevel } from '../../shared/logger-types'; export interface CueFileWatcherConfig { watchGlob: string; projectRoot: string; debounceMs: number; onEvent: (event: CueEvent) => void; triggerName: string; - onLog?: (level: string, message: string) => void; + onLog?: (level: BaseLogLevel, message: string, data?: unknown) => void; } @@ watcher.on('error', (error) => { const message = `[CUE] File watcher error for "${triggerName}": ${error}`; if (config.onLog) { - config.onLog('error', message); + config.onLog('error', message, error); } else { - console.error(message); + console.error(message, error); } });As per coding guidelines: "Use Sentry utilities for explicit reporting ... to report exceptions with context and notable events."
Also applies to: 72-77
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/cue/cue-file-watcher.ts` at line 19, The watcher currently emits only a formatted string via the onLog callback (onLog?: (level: string, message: string) => void) which drops the original Error context; change the onLog signature to accept an optional Error (e.g., onLog?: (level: string, message: string, err?: Error) => void), update the places in cue-file-watcher where errors are reported (the watcher error/logging paths around the existing log calls) to pass the original error object as the third argument, and ensure any fallback logger/consumer (where onLog is invoked) forwards that Error to Sentry or logs err.stack so the stack/context is preserved.
🤖 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/main/cue/cue-filter.ts`:
- Around line 47-64: Numeric comparisons in cue-filter.ts are vulnerable because
Number('') and Number(null) coerce to 0; replace direct Number(...) + isNaN
checks with a strict helper (e.g., toComparableNumber) that first rejects
null/undefined/empty-string operands and then converts and validates via
Number.isFinite(); update each branch that uses Number(payloadValue) /
Number(filterValue.slice(...)) (the '>=' / '<=' / '>' / '<' handling) to call
toComparableNumber for both threshold and payload and return false if the helper
indicates an invalid operand before performing the numeric comparison.
In `@src/main/cue/cue-yaml-loader.ts`:
- Around line 254-260: The current validation for heartbeat intervals (using the
variables sub and prefix in cue-yaml-loader.ts) allows NaN because it only
checks typeof and bounds; update the guard inside the validation block that
currently reads `typeof sub.interval_minutes !== 'number' ||
sub.interval_minutes <= 0 || sub.interval_minutes > 10080` to also require the
value be finite by adding a Number.isFinite(sub.interval_minutes) check (negated
to preserve the existing error branch), so the condition rejects NaN and
Infinity before pushing the error message.
---
Nitpick comments:
In `@src/main/cue/cue-file-watcher.ts`:
- Line 19: The watcher currently emits only a formatted string via the onLog
callback (onLog?: (level: string, message: string) => void) which drops the
original Error context; change the onLog signature to accept an optional Error
(e.g., onLog?: (level: string, message: string, err?: Error) => void), update
the places in cue-file-watcher where errors are reported (the watcher
error/logging paths around the existing log calls) to pass the original error
object as the third argument, and ensure any fallback logger/consumer (where
onLog is invoked) forwards that Error to Sentry or logs err.stack so the
stack/context is preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 08da562f-afca-480e-ab20-80dca81d8682
📒 Files selected for processing (7)
src/__tests__/main/cue/cue-file-watcher.test.tssrc/__tests__/main/cue/cue-filter.test.tssrc/__tests__/main/cue/cue-yaml-loader.test.tssrc/main/cue/cue-engine.tssrc/main/cue/cue-file-watcher.tssrc/main/cue/cue-filter.tssrc/main/cue/cue-yaml-loader.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/cue/cue-engine.ts
The inputMode === 'ai' guard on the newTab shortcut prevented Cmd+T (Ctrl+T on Windows) from working when a terminal tab was focused. Removed the guard so the shortcut works regardless of active mode.
… in validation
Replace Number()+isNaN() with a toComparableNumber() helper that rejects
null, empty/whitespace strings, NaN, and Infinity via Number.isFinite().
Fixes edge case where Number('') and Number(null) silently coerce to 0
and pass numeric filters. Add Number.isFinite() guard to interval_minutes
validation. 7 new tests (415 total).
Collaborator
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/hooks/keyboard/useMainKeyboardHandler.ts (1)
610-623:⚠️ Potential issue | 🟠 MajorCmd+T in terminal mode should also flip session
inputModeto'ai'.At Line 612 and Line 618, the new tab session is applied as returned by
createTab, butcreateTab(src/renderer/utils/tabHelpers.ts, Line 321-368) does not setinputMode. So in terminal mode, the AI tab can be created while the session remains in terminal mode.💡 Suggested fix
if (ctx.isTabShortcut(e, 'newTab')) { e.preventDefault(); const result = ctx.createTab(ctx.activeSession, { saveToHistory: ctx.defaultSaveToHistory, showThinking: ctx.defaultShowThinking, }); if (result) { ctx.setSessions((prev: Session[]) => - prev.map((s: Session) => (s.id === ctx.activeSession!.id ? result.session : s)) + prev.map((s: Session) => + s.id === ctx.activeSession!.id + ? { + ...result.session, + inputMode: 'ai', + } + : s + ) ); // Auto-focus the input so user can start typing immediately ctx.setActiveFocus('main'); setTimeout(() => ctx.inputRef.current?.focus(), FOCUS_AFTER_RENDER_DELAY_MS); trackShortcut('newTab'); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/keyboard/useMainKeyboardHandler.ts` around lines 610 - 623, The new-tab keyboard handler uses ctx.createTab(...) and then updates sessions via ctx.setSessions but does not ensure the new session's inputMode is set to 'ai', so creating a tab while in terminal mode preserves terminal inputMode; after receiving result from ctx.createTab in useMainKeyboardHandler (the block that calls ctx.setSessions and trackShortcut('newTab')), ensure the session inserted or replaced has inputMode: 'ai' — e.g., derive the session object from result.session, set session.inputMode = 'ai' (or use a shallow copy with inputMode:'ai') before calling ctx.setSessions / ctx.setActiveSession; reference ctx.createTab, result.session, ctx.setSessions, and ctx.activeSession when making this change.
🧹 Nitpick comments (2)
src/__tests__/renderer/hooks/useMainKeyboardHandler.test.ts (1)
1571-1608: Strengthen the terminal-mode Cmd+T test to validate full behavior, not just invocation.Line 1607 only checks that
createTabran. This misses regressions in the actual UX path (setSessions,setActiveFocus('main'), delayedinputRef.focus()).✅ Suggested test hardening
describe('tab shortcuts in terminal mode', () => { it('Cmd+T creates a new AI tab even when in terminal mode', () => { + vi.useFakeTimers(); const { result } = renderHook(() => useMainKeyboardHandler()); const mockCreateTab = vi.fn().mockReturnValue({ session: { id: 'session-1', aiTabs: [], activeTabId: 'new-tab' }, }); const mockSetSessions = vi.fn(); + const mockSetActiveFocus = vi.fn(); + const mockFocus = vi.fn(); result.current.keyboardHandlerRef.current = createUnifiedTabContext({ isTabShortcut: (_e: KeyboardEvent, actionId: string) => actionId === 'newTab', createTab: mockCreateTab, setSessions: mockSetSessions, - setActiveFocus: vi.fn(), - inputRef: { current: { focus: vi.fn() } }, + setActiveFocus: mockSetActiveFocus, + inputRef: { current: { focus: mockFocus } }, activeSession: { id: 'session-1', aiTabs: [{ id: 'ai-tab-1', name: 'AI Tab 1', logs: [] }], activeTabId: 'ai-tab-1', @@ // Cmd+T should work regardless of inputMode expect(mockCreateTab).toHaveBeenCalled(); + expect(mockSetSessions).toHaveBeenCalled(); + expect(mockSetActiveFocus).toHaveBeenCalledWith('main'); + vi.advanceTimersByTime(50); + expect(mockFocus).toHaveBeenCalled(); + vi.useRealTimers(); }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/hooks/useMainKeyboardHandler.test.ts` around lines 1571 - 1608, The test currently only asserts createTab was called; extend the assertion to validate the full UX flow from useMainKeyboardHandler: ensure mockSetSessions was called with the new session data returned by mockCreateTab (verify the sessions include the new tab), verify the setActiveFocus mock was called with 'main', and confirm the inputRef.current.focus mock was invoked (use fake timers or advance timers if the focus is called after a setTimeout). Reference the keyboardHandlerRef returned by useMainKeyboardHandler, and the mocks mockCreateTab, mockSetSessions, setActiveFocus, and inputRef to locate where to add these assertions and any timer handling.src/__tests__/main/cue/cue-filter.test.ts (1)
179-244: Good test coverage for numeric edge cases.The test suite comprehensively validates the new strict coercion behavior. One optional addition to consider:
🧪 Optional: Add test for -Infinity and NaN literals
it('rejects Infinity payload value with numeric filter', () => { expect(matchesFilter({ size: Infinity }, { size: '>0' })).toBe(false); }); + + it('rejects -Infinity payload value with numeric filter', () => { + expect(matchesFilter({ size: -Infinity }, { size: '<0' })).toBe(false); + }); + + it('rejects NaN payload value with numeric filter', () => { + expect(matchesFilter({ size: NaN }, { size: '>0' })).toBe(false); + }); + + it('rejects Infinity in filter threshold', () => { + expect(matchesFilter({ size: 100 }, { size: '>Infinity' })).toBe(false); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/main/cue/cue-filter.test.ts` around lines 179 - 244, Add tests to the "NaN handling in numeric comparisons" suite to cover -Infinity and NaN literals: (1) assert matchesFilter({ size: -Infinity }, { size: '>0' }) returns false, (2) assert matchesFilter({ size: NaN }, { size: '>0' }) returns false, and (3) assert matchesFilter({ size: 100 }, { size: '>NaN' }) returns false (i.e., reject NaN threshold); place these alongside the existing cases referencing the matchesFilter function so the strict coercion behavior covers -Infinity and NaN both as payload values and as thresholds.
🤖 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/main/cue/cue-yaml-loader.ts`:
- Around line 234-240: The duplicate-check currently uses the raw sub.name so
names with surrounding whitespace slip past; in the validation block around
seenNames, normalize the name by computing a trimmed string (e.g., const
normalized = String(sub.name).trim()) and use that normalized value for the
empty-string check, duplicate check against seenNames, and for adding to
seenNames; keep the original error messages using prefix but reference
normalized when testing/adding to seenNames and when interpolating the duplicate
name.
---
Outside diff comments:
In `@src/renderer/hooks/keyboard/useMainKeyboardHandler.ts`:
- Around line 610-623: The new-tab keyboard handler uses ctx.createTab(...) and
then updates sessions via ctx.setSessions but does not ensure the new session's
inputMode is set to 'ai', so creating a tab while in terminal mode preserves
terminal inputMode; after receiving result from ctx.createTab in
useMainKeyboardHandler (the block that calls ctx.setSessions and
trackShortcut('newTab')), ensure the session inserted or replaced has inputMode:
'ai' — e.g., derive the session object from result.session, set
session.inputMode = 'ai' (or use a shallow copy with inputMode:'ai') before
calling ctx.setSessions / ctx.setActiveSession; reference ctx.createTab,
result.session, ctx.setSessions, and ctx.activeSession when making this change.
---
Nitpick comments:
In `@src/__tests__/main/cue/cue-filter.test.ts`:
- Around line 179-244: Add tests to the "NaN handling in numeric comparisons"
suite to cover -Infinity and NaN literals: (1) assert matchesFilter({ size:
-Infinity }, { size: '>0' }) returns false, (2) assert matchesFilter({ size: NaN
}, { size: '>0' }) returns false, and (3) assert matchesFilter({ size: 100 }, {
size: '>NaN' }) returns false (i.e., reject NaN threshold); place these
alongside the existing cases referencing the matchesFilter function so the
strict coercion behavior covers -Infinity and NaN both as payload values and as
thresholds.
In `@src/__tests__/renderer/hooks/useMainKeyboardHandler.test.ts`:
- Around line 1571-1608: The test currently only asserts createTab was called;
extend the assertion to validate the full UX flow from useMainKeyboardHandler:
ensure mockSetSessions was called with the new session data returned by
mockCreateTab (verify the sessions include the new tab), verify the
setActiveFocus mock was called with 'main', and confirm the
inputRef.current.focus mock was invoked (use fake timers or advance timers if
the focus is called after a setTimeout). Reference the keyboardHandlerRef
returned by useMainKeyboardHandler, and the mocks mockCreateTab,
mockSetSessions, setActiveFocus, and inputRef to locate where to add these
assertions and any timer handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ceb7c444-1667-4588-a377-8b57fbf4d02e
📒 Files selected for processing (6)
src/__tests__/main/cue/cue-filter.test.tssrc/__tests__/main/cue/cue-yaml-loader.test.tssrc/__tests__/renderer/hooks/useMainKeyboardHandler.test.tssrc/main/cue/cue-filter.tssrc/main/cue/cue-yaml-loader.tssrc/renderer/hooks/keyboard/useMainKeyboardHandler.ts
…n new tab Trim-normalize subscription names before duplicate detection so names with surrounding whitespace are correctly caught. Ensure Cmd+T sets inputMode to 'ai' so creating a tab from terminal mode switches to AI view. Add -Infinity/NaN edge case tests for filter coercion.
Collaborator
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
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
{{CUE_SOURCE_OUTPUT}}was missing from generated chain promptsincludeUpstreamOutputtoggle, contextual placeholder, and placement hint to the agent config paneloutputTruncatedboolean to completion event payloads (single-source and fan-in) so consumers know when output was clipped at 5K charsconsole.logfromclaude-output-parser.tsthat was logging raw usage data in productionRoot Cause
pipelineToYaml.ts→buildChain()generated chain subscriptions withprompt: targetData.inputPrompt ?? ''— never injecting{{CUE_SOURCE_OUTPUT}}. The engine correctly populatedsourceOutputin the event payload (visible in the activity dashboard), but the generated prompt never referenced it, so the downstream agent's prompt was blind to the upstream output.Changes
Core fix (
pipelineToYaml.ts)ensureSourceOutputVariable()helper that auto-prepends{{CUE_SOURCE_OUTPUT}}\n\nunless the prompt already contains it (case-insensitive)buildChain()now calls this helper, respecting the newincludeUpstreamOutputtoggleUX improvements (
NodeConfigPanel.tsx,CuePipelineEditor.tsx,cue-pipeline-types.ts)includeUpstreamOutput?: booleantoAgentNodeData(default:true){{CUE_SOURCE_OUTPUT}}placement hinthasIncomingAgentEdgesin the editor and passed it throughRound-trip safety (
yamlToPipeline.ts){{CUE_SOURCE_OUTPUT}}\n\nprefix when reconstructing pipeline from YAML to prevent accumulation on repeated save/load cyclesEngine refinements (
cue-engine.ts,claude-output-parser.ts)outputTruncatedflag to single-source and fan-in completion event payloadstruncatedfield toFanInSourceCompletioninterfaceconsole.logthat was dumping raw usage JSON to stdoutEdge mode polish (
EdgeConfigPanel.tsx)pointer-events: none, and "Coming soon" tooltipTest plan
npx tsc --noEmitclean (only pre-existing errors in unrelated file){{CUE_SOURCE_OUTPUT}}in generated YAMLSummary by CodeRabbit
New Features
Bug Fixes
Tests
Chores