Skip to content

fix(graph_workflow): make to_json/from_json node types round-trip (#1839) - #1898

Open
aayushbaluni wants to merge 1 commit into
kyegomez:masterfrom
aayushbaluni:fix/1839-graphworkflow-json-nodetype
Open

fix(graph_workflow): make to_json/from_json node types round-trip (#1839)#1898
aayushbaluni wants to merge 1 commit into
kyegomez:masterfrom
aayushbaluni:fix/1839-graphworkflow-json-nodetype

Conversation

@aayushbaluni

Copy link
Copy Markdown

Description

GraphWorkflow.to_json()GraphWorkflow.from_json() round trip is broken: it raises and loses the entire graph. This fixes the serialization defect that causes it.

to_json writes the node type with str(node.type), which renders a NodeType as "NodeType.AGENT". from_json parses it with NodeType(n["type"]), which only accepts the enum value ("agent"):

>>> str(NodeType.AGENT)
'NodeType.AGENT'
>>> NodeType('NodeType.AGENT')
ValueError: 'NodeType.AGENT' is not a valid NodeType
>>> NodeType('agent')
<NodeType.AGENT: 'agent'>

Because the deserialize loop wraps each node in try/except ... continue, that ValueError is swallowed per node and every node is silently dropped. The failure then surfaces on the edges, pointing away from the real cause:

WARNING  Failed to deserialize node alpha: 'NodeType.AGENT' is not a valid NodeType
WARNING  Failed to deserialize node beta: 'NodeType.AGENT' is not a valid NodeType
ERROR    Source node 'alpha' does not exist in GraphWorkflow
ValueError: Source node 'alpha' does not exist in GraphWorkflow

Reproduced on master (3ff3475) with a two-agent workflow and one edge.

Fix

  • to_json emits the enum value ("agent"). Node.type is not coerced in __init__, so getattr(node.type, "value", node.type) also tolerates a plain string.
  • from_json parses via a small _parse_node_type helper that additionally accepts the legacy "NodeType.X" spelling, so workflows exported before this fix still load.

The change is confined to the two lines that serialize/deserialize the node type, plus the helper. I deliberately left export_summary's str(node.type) alone: it is display-only and never parsed back.

Issue

Fixes the first half of #1839.

Scope note: #1839 also reports that from_json reconstructs agents as plain dicts. That is real and still present — Agent has to_dict() but no from_dict() inverse, so restoring live Agent objects is a design decision rather than a bug fix, and it belongs in a separate PR. The issue additionally proposes deleting the ~495-line JSON path; that is a maintainer call on public API, so this PR only repairs the defect. With this change the round trip preserves ids, node types and edges instead of collapsing to an exception.

Tests

Added 3 tests to tests/structs/test_graph_workflow.py (there were previously none covering the JSON path, which is how this shipped):

  • test_to_json_node_type_round_trips — serialized type is "agent" and parses back
  • test_parse_node_type_accepts_legacy_and_canonical_forms — canonical, legacy and enum inputs; invalid input still raises
  • test_from_json_reconstructs_nodes_and_edges — topology survives the round trip

Verification

  • pytest tests/structs/test_graph_workflow.py51 passed, 11 skipped (48 passed on unmodified master, so +3 and no regressions)
  • Mutation-checked both halves: reverting to_json to str(node.type) fails test_to_json_node_type_round_trips; disabling the legacy branch in _parse_node_type fails the legacy test. The tests measure the fix rather than decorating it.
  • black --check clean; ruff check reports 215 errors on these two files both before and after my change (all pre-existing, repo-wide UP006/I001 style), so this adds none.

Dependencies

None.

Tag maintainer

@kyegomez

to_json serialized the node type with str(node.type), which renders a NodeType as 'NodeType.AGENT'. from_json parses it with NodeType(n['type']), which only accepts the enum value ('agent'), so every node raised "'NodeType.AGENT' is not a valid NodeType".

That exception was swallowed by the per-node try/except in the deserialize loop, so all nodes were silently dropped and the subsequent edge restore failed with "Source node 'alpha' does not exist in GraphWorkflow" - a confusing error that points at the edges rather than the actual serialization defect.

Serialize the enum value instead, and parse through a helper that also accepts the legacy 'NodeType.X' spelling so workflows exported before this fix still load.
@github-actions

Copy link
Copy Markdown

Hello there, thank you for opening an PR ! 🙏🏻 The team was notified and they will get back to you asap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant