feat(DAG): warn when several func nodes write to the same var node - #84
Merged
Merged
Conversation
Two FuncNodes can share an 'out', in which case the DAG silently computes both and only the last value is visible. DAG.__post_init__ now emits a DuplicateOutsWarning naming the shadowed var nodes and the func nodes involved. Behaviour is otherwise unchanged (a warning, not an error, since existing dags may rely on it). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review of #84: make the duplicate-out check a DAG field (on_duplicate_outs, default warn_on_duplicate_outs, alternatives ignore_duplicate_outs / raise_on_duplicate_outs or any callable), fix stacklevel so the warning points at the user's call site, don't re-warn for dags derived from a dag that already warned (copy, partial, __getitem__, ch_funcs, add_edge, ch_names), and name the node whose value is visible in the message. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Second review round of #84: - derived dags keep the source dag's on_duplicate_outs strategy, wrapped so it only hears about duplications that are NEW (only_new_duplicate_outs), which also catches a renaming copy() that collapses two outs into one; - the warning's stacklevel is computed (first frame outside meshed/i2), so it points at the user's code for a delegating custom strategy and for code_to_dag too; - '+' and sum() report only the duplications the union creates; - raise_on_duplicate_outs reuses the formatted message; the new symbols are exported from meshed; annotation and docstrings corrected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #40
Two
FuncNodes can share anout: the DAG computes both and only the last one's value is visible, silently.This PR detects that and reports it through a strategy seam:
DAGgets anon_duplicate_outsfield (defaultwarn_on_duplicate_outs, which raises aDuplicateOutsWarning). Alternatives shipped:ignore_duplicate_outs,raise_on_duplicate_outs, or any callable(duplicates, *, dag_name=None).duplicate_outs(func_nodes)is the public detector. All are exported frommeshed.stacklevelis computed (first frame outsidemeshed/i2), so it points at the code that built the dag — including throughcode_to_dagand through a custom strategy that delegates to the default.dag[...],copy,partial,ch_funcs,add_edge,+,sum) keep the source's strategy but only hear about duplications that are new, so a pre-existing duplication is reported once, at the place it was created. A renamingcopy(renamer=...)that collapses two outs into one is reported.on_duplicate_outs=raise_on_duplicate_outsis there for those who want it).Tests:
meshed/tests/test_duplicate_outs.py(13 tests: explicit sharedout, same__name__, no warning for distinct outs, the three strategies, derived dags silent, union warns, strategy survives derivation, opt-out survives derivation, renaming copy that creates a duplicate, warning location) plus doctests.Review: three rounds with an independent refute-review agent. Round 1 found the warning was unattributable (
<string>:9), re-fired up to 4x on derived dags, broke-W errorconsumers from inside meshed internals, and had no opt-out; round 2 found the seam was lost on derivation, the baked-instacklevelbroke for delegating strategies,+/sumstill warned from internals,code_to_dagpointed atmakers.py, and a renaming copy could create a silent duplicate. All fixed here, each with a test.Dependents: test suites of the 10 local dependents present on this box (allude, dagapp, dotsci, extrude, front, guided, lookbook, theremin, titbit, uf) give identical results with this branch and with master (their pre-existing failures/collection errors are unchanged). meshed's own suite: 204 passed, 1 skipped, 1 pre-existing failure (
test_hybrid_dagneeds optionalhttp2py/extrude). The full suite also passes under-W error::meshed.dag.DuplicateOutsWarning, i.e. meshed itself never trips its own warning.🤖 Generated with Claude Code