Skip to content

Commit e53a7ce

Browse files
elinscottclaude
andcommitted
Pin enum serialize behavior; guard key collisions; document set limitation
The read side does not round-trip Enums: node-graph's coerce_inputs_from_spec records structured_type extras only for dataclass/pydantic/TypedDict, never for Enum sockets, so a task body declaring a plain Enum input receives the bare value (isinstance False, x == Color.RED False), not the member. Correct the _flatten_enums docstring (was claiming a round-trip that does not happen) and add test_body_receives_bare_value_not_member, which drives a real wg.run() and asserts the body sees the bare value - it flips loudly if node-graph later adds enum reconstruction. set/frozenset are not descended into: a set fails in general_serializer regardless of contents (not JSON-serializable, no registered serializer), so flattening enums inside one would not help. Document this and pin it with test_flatten_leaves_sets_untouched. Guard dict-key flattening: two distinct keys collapsing to the same flattened value (an Enum member and its bare value, or two members sharing a .value) now raises instead of silently dropping an entry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 3460651 commit e53a7ce

2 files changed

Lines changed: 83 additions & 9 deletions

File tree

src/aiida_workgraph/serialization.py

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,43 @@
1212
def _flatten_enums(value: Any) -> Any:
1313
"""Replace ``Enum`` members with their bare ``.value``, recursively.
1414
15-
``node_graph`` declares that an enum-typed socket's serialized form is
16-
its bare value (``socket_spec`` records ``structured_type`` extras so
17-
``coerce_inputs_from_spec`` can rebuild the member before a task body
18-
runs). The write path has to honour that contract: without this,
19-
``serialize_ports`` hands the raw ``Enum`` instance to
20-
``general_serializer``, which has no serializer for it and fails at
21-
submit time. ``isinstance`` also matches wrapt proxies (TaggedValue)
22-
around enum members.
15+
``general_serializer`` (aiida-pythonjob) has no serializer for ``Enum``
16+
members: a raw ``Enum`` -- or one nested in a dict/list/tuple -- reaches
17+
it and the whole submission fails at submit time with an opaque
18+
serialization error. Collapsing members to their ``.value`` here keeps
19+
the payload JSON-serializable so ``serialize_ports`` succeeds. The
20+
``isinstance`` check also matches wrapt proxies (node-graph's
21+
``TaggedValue``) wrapping an enum member.
22+
23+
This is a one-way flattening, *not* a round-trip. Under the pinned
24+
``node_graph`` the read side (``coerce_inputs_from_spec``) rebuilds only
25+
structured *models* -- dataclass/pydantic/TypedDict -- and records no
26+
``structured_type`` extra for ``Enum`` sockets, so it never reconstructs
27+
the member. Task bodies therefore receive the bare value, not the
28+
``Enum`` they declared. This is harmless for ``IntEnum`` / ``str, Enum``
29+
consumed by value or equality, but a body that tests ``x is Color.RED``
30+
or ``x == Color.RED`` against a *plain* ``Enum`` sees ``False``. See
31+
``tests/test_serializer.py::test_body_receives_bare_value_not_member``.
32+
33+
``set``/``frozenset`` are deliberately left untouched: a set fails in
34+
``general_serializer`` regardless of its contents (not JSON-serializable,
35+
no registered serializer), so descending into one to flatten enums would
36+
not make it serializable -- see
37+
``tests/test_serializer.py::test_flatten_leaves_sets_untouched``.
2338
"""
2439
if isinstance(value, Enum):
2540
return _flatten_enums(value.value)
2641
if isinstance(value, dict):
27-
return {_flatten_enums(k): _flatten_enums(v) for k, v in value.items()}
42+
out: Dict[Any, Any] = {}
43+
for k, v in value.items():
44+
flat_key = _flatten_enums(k)
45+
if flat_key in out:
46+
# Two distinct keys (e.g. an Enum member and its bare value,
47+
# or two members sharing a ``.value``) collapse to one; raise
48+
# rather than silently drop an entry.
49+
raise ValueError(f'Enum key flattening collision: multiple keys map to {flat_key!r}')
50+
out[flat_key] = _flatten_enums(v)
51+
return out
2852
if isinstance(value, list):
2953
return [_flatten_enums(v) for v in value]
3054
if isinstance(value, tuple):

tests/test_serializer.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,30 @@ def test_flatten_wrapt_proxied_enum_member():
9494
assert _flatten_enums({proxied: proxied}) == {'red': 'red'}
9595

9696

97+
def test_flatten_dict_key_collision_raises():
98+
"""Two distinct keys collapsing to the same flattened key raises rather than
99+
silently dropping an entry. Here ``Color.RED`` (a plain Enum, so distinct in
100+
identity and hash from the bare string) and ``'red'`` are separate keys that
101+
both flatten to ``'red'``."""
102+
payload = {Color.RED: 1, 'red': 2}
103+
assert len(payload) == 2 # distinct keys before flattening
104+
with pytest.raises(ValueError, match='collision'):
105+
_flatten_enums(payload)
106+
107+
108+
def test_flatten_leaves_sets_untouched():
109+
"""``set``/``frozenset`` are not descended into: they fail in
110+
``general_serializer`` regardless of contents, so flattening enums inside
111+
them would not make them serializable. Pin that they pass through unchanged
112+
(still holding Enum members)."""
113+
s = {Color.RED, Color.BLUE}
114+
out = _flatten_enums(s)
115+
assert out is s
116+
assert Color.RED in out
117+
fs = frozenset({Priority.LOW})
118+
assert _flatten_enums(fs) is fs
119+
120+
97121
def test_serialize_ports_accepts_enum_value(aiida_profile):
98122
"""End-to-end through the adapter: an enum-valued namespace entry serializes
99123
without raising, landing as the flattened bare value (needs a profile so the
@@ -105,3 +129,29 @@ def test_serialize_ports_accepts_enum_value(aiida_profile):
105129
out = AiidaSerializationAdapter().serialize_ports({'c': Color.RED, 'n': 3}, spec, store=False)
106130
assert isinstance(out['c'], orm.Str)
107131
assert out['c'].value == 'red'
132+
133+
134+
@task(outputs=['type_name', 'is_enum', 'eq_member'])
135+
def _observe_enum(c: Color) -> dict:
136+
"""Report what the body actually receives for an Enum-typed input."""
137+
return {
138+
'type_name': type(c).__name__,
139+
'is_enum': isinstance(c, Color),
140+
'eq_member': c == Color.RED,
141+
}
142+
143+
144+
def test_body_receives_bare_value_not_member(aiida_profile):
145+
"""Pin the real round-trip behaviour under the pinned ``node_graph``: a task
146+
body declaring a *plain* ``Enum`` input receives the BARE value, not the
147+
member. ``node_graph`` records ``structured_type`` extras only for
148+
dataclass/pydantic/TypedDict, so ``coerce_inputs_from_spec`` never rebuilds
149+
an ``Enum``. If node-graph later adds enum reconstruction, this test flips
150+
loudly and the docstring/PR claim must be revisited."""
151+
wg = WorkGraph('enum_roundtrip')
152+
t = wg.add_task(_observe_enum, name='observe', c=Color.RED)
153+
wg.run()
154+
assert wg.state == 'FINISHED'
155+
assert t.outputs['type_name'].value.value == 'str'
156+
assert t.outputs['is_enum'].value.value is False
157+
assert t.outputs['eq_member'].value.value is False

0 commit comments

Comments
 (0)