Skip to content

Resolve link targets inside typed dynamic entries during from_dict - #166

Open
elinscott wants to merge 3 commits into
scinode:mainfrom
elinscott:fix/dynamic-entry-from-dict
Open

elinscott wants to merge 3 commits into
scinode:mainfrom
elinscott:fix/dynamic-entry-from-dict

Conversation

@elinscott

@elinscott elinscott commented Jul 24, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

A graph whose typed dynamic output namespace entries are populated key-by-key from leaf sockets cannot be reconstructed from its dict form — which build(), run() and the daemon all rely on:

class Entry(TypedDict):
    value: int
    tag: int

@task.graph
def Produce(x: int) -> Entry:
    return Entry(value=compute(x).result, tag=compute(-x).result)

@task.graph
def Parent(items: dict) -> Annotated[dict, dynamic(Entry)]:
    entries = {}
    for label, x in items.items():
        produced = Produce(x)
        # per-key socket picking into the dynamic entry
        entries[label] = Entry(value=produced.value, tag=produced.tag)
    return entries

Parent.build(items={"a": 1, "b": 2})
# ValueError: Missing socket 'a.value' in graph_outputs.inputs and
# parent namespace is not dynamic.

The per-key links create the entry namespaces (a, b) in the live graph, but reconstruction refuses to re-create their children because the typed entry itself is not marked dynamic.

Changes

  • Extract the dotted-path input-socket resolution from links_from_dict into a Graph._resolve_or_create_input_socket helper (mechanical, first commit)
  • Walking the dotted target path materializes the entry namespace from the dynamic parent's item spec, which already creates the entry's children per that spec; the resolver then returns an existing leaf before checking dynamism, instead of refusing to create it because the typed entry itself is not dynamic (second commit)

Testing

  • New round-trip regression test in test_graph.py: a typed dynamic output namespace populated by per-key leaf links survives to_dict → from_dict
  • Unit tests covering the resolver's error branches (missing sockets under non-dynamic namespaces, descent through a leaf), which the extraction had moved out of inline code untested
  • Full suite passes with no changes to existing tests

elinscott and others added 2 commits July 24, 2026 15:56
Hoists the dynamic-namespace path-walking logic out of links_from_dict
into a reusable staticmethod so aiida-workgraph's Map-zone clone path
(_patch_cloned_tasks) can resolve dotted targets like ``pseudos.O``
into a dynamic namespace by materialising children on demand.

Fixes the AttributeError "'O' is not in this namespace" raised when a
Map zone clones a task whose dynamic-namespace input is populated by
links (not raw values) — the round-trip via ``to_dict`` was preserving
the namespace itself but losing its link-installed children.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reconstructing a graph whose typed dynamic namespace entries were
populated by per-key leaf links failed in from_dict with "Missing
socket '...' in <task>.inputs and parent namespace is not dynamic":

- the entry namespace exists only at build time, and walking the
  dotted target path materializes it from the parent's item spec
- the leaf socket created alongside the entry was then reported as
  missing because the typed entry itself is not dynamic

Resolution now returns an already-existing leaf instead of insisting
on creating it. Since run() and the daemon reconstruct via from_dict,
such graphs previously died at run start.
@elinscott
elinscott requested a review from GeigerJ2 July 24, 2026 15:52
@codecov

codecov Bot commented Jul 24, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.18750% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.82%. Comparing base (8d85e61) to head (ffde95c).

Files with missing lines Patch % Lines
tests/test_graph.py 91.17% 3 Missing ⚠️
src/node_graph/graph.py 93.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #166      +/-   ##
==========================================
+ Coverage   89.68%   89.82%   +0.14%     
==========================================
  Files          81       81              
  Lines        8984     9020      +36     
==========================================
+ Hits         8057     8102      +45     
+ Misses        927      918       -9     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

The helper extraction moved pre-existing untested error paths into
_resolve_or_create_input_socket, where they are now unit-testable.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LGsT8YPFW5jT93FPWojAYf
elinscott added a commit to elinscott/node-graph that referenced this pull request Aug 14, 2026
# Conflicts:
#	src/node_graph/graph.py
elinscott added a commit to elinscott/node-graph that referenced this pull request Aug 17, 2026
Resolution notes, all of it integration between two open pull requests:

- graph.py: scinode#151 and scinode#166 reword the same docstring paragraph of
  `_resolve_or_create_input_socket`; scinode#151's wording is kept, and scinode#166's
  already-materialised-leaf early return is taken.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant