Skip to content

feat(ui): implement graph-to-API serialization for workflow canvas save/load#1111

Open
geoffjay wants to merge 2 commits intoissue-1098from
issue-1100
Open

feat(ui): implement graph-to-API serialization for workflow canvas save/load#1111
geoffjay wants to merge 2 commits intoissue-1098from
issue-1100

Conversation

@geoffjay
Copy link
Copy Markdown
Owner

Add graphToWorkflows() to serialize canvas graph to CreateWorkflowRequest[], workflowsToGraph() to deserialize API responses to React Flow nodes/edges (with agent dedup), validateGraph() for pre-save validation, and localStorage layout persistence. 36 tests covering round-trips, shared agents, edge cases.

Closes #1100

@geoffjay geoffjay added the review-agent Used to invoke a review by an agent tracking this label label Apr 14, 2026
Copy link
Copy Markdown
Owner Author

@geoffjay geoffjay left a comment

Choose a reason for hiding this comment

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

Review: feat(ui): implement graph-to-API serialization for workflow canvas save/load

Stack position: issue-1100 (#1111) stacked on issue-1098 (#1110 — also has review-agent). #1110 should be reviewed and merged first.

Overview

This PR adds the serialization layer between React Flow's node/edge graph model and the orchestrator API's CreateWorkflowRequest shape. It includes full round-trip support (graph→API and API→graph), localStorage-backed layout persistence, and a comprehensive 532-line test suite covering ~35 cases.

What looks good

  • Agent deduplication is correct. workflowsToGraph uses agentNodeMap: Map<string, Node> keyed on agent-${agentId} and checks agentNodeMap.has(agentNodeId) before creating a node — agents shared across multiple workflows appear exactly once on the canvas.

  • Layout key is order-independent. layoutStorageKey sorts IDs before hashing with djb2, so ['b','a'] and ['a','b'] produce the same key. The three tests for this in serialization.test.ts confirm it.

  • graphToWorkflows throws, validateGraph returns. The asymmetry is intentional: callers who want a pre-flight check use validateGraph; callers who want an all-or-nothing conversion use graphToWorkflows. The tests cover both paths correctly.

  • localStorage calls are guarded. Both saveLayout and loadLayout wrap localStorage access in try/catch, which correctly handles private-browsing environments and quota errors.

  • Test isolation is solid. beforeEach/afterEach(() => localStorage.clear()) prevents layout tests from bleeding into each other. Test helpers (makeTriggerNode, makeAgentNode, makeEdge, makeWorkflow, makeAgent) are minimal and reusable.

Non-blocking notes

  1. loadLayout casts without runtime validation (JSON.parse(raw) as CanvasLayout). If stored layout data is stale (e.g., schema changed between deploys), this will silently accept a malformed object and crash downstream when layout fields are destructured. Consider a lightweight shape-check or a try/catch around the usage site. This is consistent with the pattern in PR #1113 — worth addressing in a follow-up for both.

  2. index.ts re-exports everything. The barrel export added in this PR makes all serialization internals (LAYOUT_TRIGGER_X, LAYOUT_ROW_HEIGHT, etc.) part of the public surface of the canvas module. If these constants are implementation details, consider exporting only the functions. Low urgency since this is internal UI code.

  3. workflowsToGraph uses agent?.config?.model optional chaining, which silently produces undefined if the agent has no model set. This is safe for now but worth documenting — a node with an undefined model label may render oddly depending on how #1110's AgentNode renders the prop.

Verdict

No blocking issues. The serialization logic is correct, the tests are thorough, and the code follows the patterns established in the rest of the canvas module. Approving pending merge of parent #1110.

@geoffjay geoffjay added merge-queue Approved by reviewer, queued for merge by conductor and removed review-agent Used to invoke a review by an agent tracking this label labels Apr 17, 2026
- graphToWorkflows(): converts each trigger->agent edge to CreateWorkflowRequest
- workflowsToGraph(): converts Workflow[] to React Flow nodes/edges with agent dedup
- validateGraph(): detects unconnected nodes, missing config, invalid edges
- CanvasLayout interface for storing node positions + viewport in localStorage
- saveLayout/loadLayout with stable hash-based key for layout persistence
- Workflow name auto-derived as [trigger_type]-to-[agent_name]
- 36 tests: single/multi workflow, shared agents, round-trip, edge cases, layout
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-queue Approved by reviewer, queued for merge by conductor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant