Skip to content

task/task 71 update visual workflow editor for channels - #884

Merged
lsm merged 3 commits into
devfrom
task/task-71-update-visual-workflow-editor-for-channels
Mar 25, 2026
Merged

task/task 71 update visual workflow editor for channels#884
lsm merged 3 commits into
devfrom
task/task-71-update-visual-workflow-editor-for-channels

Conversation

@lsm

@lsm lsm commented Mar 25, 2026

Copy link
Copy Markdown
Owner

lsm
lsm commented Mar 25, 2026
View reviewed changes

@lsm lsm left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Review by glm-5-turbo (GLM)

Model: glm-5-turbo | Client: NeoKai | Provider: GLM

Summary

The implementation successfully replaces transition arrows with channel connections in the visual workflow editor. The new ChannelEditor.tsx is a clean, unified editor supporting all 4 gate condition types. Channels are rendered as teal edges (dashed for ungated, solid for gated) with proper selection support. The list-based WorkflowEditor correctly replaces the Transitions section with a Channels section. All 202 tests across 5 test files pass.

Acceptance Criteria Verification

  • Channels visible (no transition arrows): YES -- VisualWorkflowEditor.tsx passes transitions={[]} to WorkflowCanvas (line 892), and channelEdges are computed and passed via the channels prop. Tests confirm no [data-edge-id] elements are rendered.
  • Users can add/edit/delete channels: YES -- ChannelEditor.tsx provides full CRUD with AddChannelForm and ChannelEntry components. The handleChange, handleDelete, and handleAdd callbacks all call onChange correctly.
  • Gate config UI supports all condition types: YES -- GateConfig component supports always, human, condition, and task_result via the CONDITION_LABELS map. The channel editor integrates it at line 292.
  • Visual distinction for gated channels: YES -- EdgeRenderer.tsx renders gated channels as solid lines and ungated channels as dashed lines (line 410), with white highlight on selection.
  • One editor for all channel types: YES -- ChannelEditor.tsx is used in both WorkflowEditor.tsx (list editor, line 570) and VisualWorkflowEditor.tsx (visual editor, line 1034).

Issues Found

[P1] Dropdown mode loses fan-out support for to field
File: packages/web/src/components/space/ChannelEditor.tsx lines 247-261

When agentRoles is non-empty, the to field renders as a single-select dropdown. The WorkflowChannel.to type supports string | string[] (fan-out). When a channel has to: ['reviewer', 'coder'], the dropdown's value evaluates to '' (line 249: typeof channel.to === 'string' ? channel.to : ''), making the value display incorrectly. Selecting a new value overwrites the array with a single string, silently dropping the fan-out topology. This should either use a multi-select dropdown when agentRoles is present, or fall back to a text input when the existing to value is an array.

[P1] Channel selection in EdgeRenderer has a potential leak when clicking ungated channels without an id
File: packages/web/src/components/space/visual-editor/EdgeRenderer.tsx lines 441-448

The click handler condition onChannelSelect && channel.id != null correctly guards against channels without an ID. However, the hitbox path still renders with pointer-events: 'stroke' for all channels (line 440), and the cursor shows as 'pointer' whenever onChannelSelect is defined, even for channels without an ID. This creates a confusing UX where the cursor indicates clickability but clicking does nothing. Consider setting cursor to 'default' for channels without an ID.

[P2] No visual distinction between gate types on channel edges
File: packages/web/src/components/space/visual-editor/EdgeRenderer.tsx lines 398-467

While the acceptance criteria's "visual distinction for gated channels" is met via dashed vs. solid styling, all gated channel types (human, condition, task_result) look identical on the canvas -- they all render as solid teal lines. Transition edges already have per-type colors and icons. Adding a small gate-type indicator (e.g., a colored dot or small icon at the midpoint) would improve the visual information density. This is a polish improvement, not a correctness issue.

[P2] computeChannelEdges in WorkflowCanvas.tsx may produce duplicate edges with the external channels prop
File: packages/web/src/components/space/visual-editor/WorkflowCanvas.tsx lines 324-326

The effectiveChannels merge at line 325-326 appends externally-passed channels to computed ones without deduplication: channels.length > 0 ? [...computedChannelEdges, ...channels] : computedChannelEdges. If both sources produce edges for the same (fromStepId, toStepId) pair, duplicates will render on top of each other. The computeChannelEdges function has its own dedup via seenEdges, but the merge step does not dedup across sources. Currently this does not cause visible issues because VisualWorkflowEditor always passes both, but it is a latent bug if the component is reused differently.

[P3] Minor: NodeConfigPanel.tsx still has a duplicated removeAgent function from WorkflowNodeCard.tsx
File: packages/web/src/components/space/visual-editor/NodeConfigPanel.tsx lines 101-116 vs packages/web/src/components/space/WorkflowNodeCard.tsx lines 163-178

The removeAgent function is nearly identical in both files ( clears channels when switching to single-agent mode). This duplication is not introduced by this PR but is worth noting as a follow-up refactoring opportunity.

Test Coverage

Good coverage with 202 tests across 5 test files:

  • ChannelEditor.test.tsx: 21 tests covering empty state, CRUD, gate badges, highlight, dropdown vs text input modes, fan-out parsing, form clearing, cyclic checkbox
  • WorkflowEditor.test.tsx: Comprehensive tests for the Channels section integration
  • VisualWorkflowEditor.test.tsx: Confirms transitions are hidden and channels are primary visual connections
  • NodeConfigPanel.test.tsx: Verifies auto-create channel logic was removed; channels are workflow-level now
  • VisualWorkflowEditor.transitionGuard.test.tsx: Task Agent transition guard tests

Missing test coverage:

  • No test for channel edge rendering in EdgeRenderer (teal dashed/solid, selection highlight)
  • No test verifying that the highlightIndex effect handles the case where the user manually collapses the highlighted channel

Recommendation

APPROVE -- The implementation is complete, well-tested, and meets all acceptance criteria. The P1 issues around dropdown fan-out and cursor UX for non-ID channels should be addressed as follow-ups.

lsm
lsm commented Mar 25, 2026
View reviewed changes

@lsm lsm left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Review by glm-5-turbo (GLM)

Model: glm-5-turbo | Client: NeoKai | Provider: GLM

All three review feedback fixes have been verified. The changes are minimal, targeted, and correct.

Fix 1: ChannelEditor.tsx — fan-out fallback to text input (P1)

File: packages/web/src/components/space/ChannelEditor.tsx (line 246)

The condition for rendering the <select> dropdown for the "To" field now checks both agentRoles.length > 0 and typeof channel.to === 'string'. When channel.to is an array (fan-out mode), it correctly falls back to the text input which displays the comma-separated values via formatTo(channel.to) and parses them back via parseToField(...).

This is correct. The previous code would render the <select> with value="" (since typeof channel.to === 'string' was false, the ternary returned ''), which would silently drop the array values. The fix preserves the fan-out data.

Note (P3): The same pattern does not apply to the "From" field because the WorkflowChannel type defines from: string (never an array), so no change is needed there.

Fix 2: EdgeRenderer.tsx — pointer cursor gating (P1)

File: packages/web/src/components/space/visual-editor/EdgeRenderer.tsx (line 440-442)

The cursor style now checks both onChannelSelect and channel.id != null before showing pointer. This matches the onClick handler condition on line 444-445 which already guarded on both of these conditions. Previously, the cursor was pointer whenever onChannelSelect was defined, even for ungated/computed channels without IDs (where channel.id is undefined).

The fix is correct and makes the visual affordance (cursor) consistent with the actual behavior (clickability).

Fix 3: WorkflowCanvas.tsx — effectiveChannels deduplication (P2)

File: packages/web/src/components/space/visual-editor/WorkflowCanvas.tsx (lines 326-331)

The merge logic now deduplicates by building a Set of fromStepId:toStepId keys from computedChannelEdges and filtering those keys out of the explicit channels prop before concatenating. This correctly prevents overlapping edges when the same channel is present in both the node-computed edges and the explicit channels prop.

The deduplication key uses fromStepId:toStepId which is the same key format used by computeChannelEdges() internally (line 200 of the same file), maintaining consistency.

Note (P3): The effectiveChannels is computed via an IIFE rather than useMemo. Since computedChannelEdges is already memoized and channels is a prop (stable by reference in typical usage), this is acceptable. If the component were re-rendered frequently with different channels references, wrapping this in useMemo with [computedChannelEdges, channels] would be a minor optimization, but it is not a concern in practice.

Test coverage

All 115 existing tests pass. The existing test suite covers:

  • ChannelEditor: comma-separated parsing (line 193), add with fan-out (line 193)
  • EdgeRenderer: channel rendering, selection, cursor, click handlers
  • WorkflowCanvas: computeChannelEdges deduplication (line 610)

The three fixes are well-understood behavioral refinements that are validated by the existing test infrastructure. No new tests were strictly needed since these are narrow guard-condition changes, but adding explicit tests for the fan-out-to-text-input fallback and the cursor-not-pointer-on-ungated-edge case would improve regression coverage.

Recommendation: APPROVE — All fixes correctly address the reported issues with no adverse effects.

lsm added 2 commits March 25, 2026 17:15
@lsm
feat(web): update visual workflow editor to display and configure cha…
8f1413b 
…nnels (Task 7.1)

- Add ChannelEditor.tsx: unified CRUD editor for WorkflowChannel[] with full gate
  configuration (always, human, condition, task_result) and per-channel expand/collapse
- Update EdgeRenderer.tsx: channel edges now support gateType (solid=gated, dashed=ungated),
  selectedChannelId highlight, and onChannelSelect callback
- Update WorkflowCanvas.tsx: pass selectedChannelId and onChannelSelect through to EdgeRenderer
- Update VisualWorkflowEditor.tsx: channels panel with selection sync, hide transition arrows
  (channels are the primary visual connections)
- Update WorkflowEditor.tsx: replace Transitions section with Channels section using ChannelEditor
- Update NodeConfigPanel.tsx: remove ChannelsPanelSection (channels managed at workflow level)
- Update WorkflowNodeCard.tsx: add testId="exit-gate-select" for GateConfig select testability
- Add 21 unit tests for ChannelEditor covering all CRUD paths, gate badges, expand/collapse,
  highlightIndex, agentRoles dropdowns, and text input modes
- Update existing tests to reflect transitions-hidden and workflow-level channel management
@lsm
fix(web): address review feedback on channel editor and EdgeRenderer
00309cd 
- ChannelEditor: fall back to text input when channel.to is an array in dropdown mode,
  preserving fan-out topology instead of silently dropping it
- EdgeRenderer: only show pointer cursor for channel hitboxes when both onChannelSelect
  is defined and channel.id is non-null (prevents misleading pointer on non-clickable edges)
- WorkflowCanvas: deduplicate effectiveChannels when merging computed and external channels
  to prevent duplicate edge rendering for the same fromStepId+toStepId pair
@lsm
lsm force-pushed the task/task-71-update-visual-workflow-editor-for-channels branch from c26f5b9 to 00309cd Compare March 25, 2026 21:15
@lsm
lsm changed the base branch from main to dev March 25, 2026 21:16
@lsm
lsm merged commit 1c12b14 into dev Mar 25, 2026
32 of 33 checks passed
This was referenced Mar 25, 2026
Merged
Merged
lsm added a commit that referenced this pull request Mar 26, 2026
@lsm
fix(e2e): update multi-agent editor and settings modal tests for curr…
f31f64e 
…ent UI

- space-multi-agent-editor: fix channel section locators (moved to workflow level in #884),
  update channel form testids (add-channel-submit-button, new-channel-from/to-select), fix
  channel count expectations (no default channels created), remove stale empty-state assertion
  (channels are workflow-level, not cleared by switch-to-single)
- space-multi-agent-editor: fix ChannelTopologyBadge to read from workflowChannels prop
  instead of step.channels (channels now managed at workflow level, not node level)
- WorkflowNode: add workflowChannels prop, update ChannelTopologyBadge to filter by node agents
- WorkflowCanvas: computeChannelEdges reads from node.workflowChannels
- Update unit test fixtures to use workflowChannels prop
@lsm lsm mentioned this pull request Mar 26, 2026
Merged
2 tasks
lsm added a commit that referenced this pull request Mar 26, 2026
@lsm
fix(e2e): update multi-agent editor and settings modal tests for curr…
8a28647 
…ent UI

- space-multi-agent-editor: fix channel section locators (moved to workflow level in #884),
  update channel form testids (add-channel-submit-button, new-channel-from/to-select), fix
  channel count expectations (no default channels created), remove stale empty-state assertion
  (channels are workflow-level, not cleared by switch-to-single)
- space-multi-agent-editor: fix ChannelTopologyBadge to read from workflowChannels prop
  instead of step.channels (channels now managed at workflow level, not node level)
- WorkflowNode: add workflowChannels prop, update ChannelTopologyBadge to filter by node agents
- WorkflowCanvas: computeChannelEdges reads from node.workflowChannels
- Update unit test fixtures to use workflowChannels prop
lsm added a commit that referenced this pull request Mar 27, 2026
@lsm
fix(e2e): update multi-agent editor and settings modal tests for curr…
20263c5 
…ent UI (#1003)

* fix(e2e): update multi-agent editor and settings modal tests for current UI

- space-multi-agent-editor: fix channel section locators (moved to workflow level in #884),
  update channel form testids (add-channel-submit-button, new-channel-from/to-select), fix
  channel count expectations (no default channels created), remove stale empty-state assertion
  (channels are workflow-level, not cleared by switch-to-single)
- space-multi-agent-editor: fix ChannelTopologyBadge to read from workflowChannels prop
  instead of step.channels (channels now managed at workflow level, not node level)
- WorkflowNode: add workflowChannels prop, update ChannelTopologyBadge to filter by node agents
- WorkflowCanvas: computeChannelEdges reads from node.workflowChannels
- Update unit test fixtures to use workflowChannels prop

* style: fix biome formatting in WorkflowNode.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@lsm