Skip to content

fix: clear error when a multi-output submodel call has the wrong number of LHS outputs - #324

Open
bvdmitri wants to merge 1 commit into
mainfrom
fix/310-multioutput-lhs-count
Open

bvdmitri wants to merge 1 commit into
mainfrom
fix/310-multioutput-lhs-count

Conversation

@bvdmitri

Copy link
Copy Markdown
Member

Closes #310.

Problem

A multi-output submodel call must provide exactly as many outputs on the LHS as there are
interfaces left unspecified on the RHS. When it doesn't, the user got:

MethodError: no method matching make_node!(::GraphPPL.Composite, ::GraphPPL.Model{MetaGraphsNext.MetaGraph{Int64, Graphs.SimpleGraphs.SimpleGraph{Int64}, ...

which says nothing about the actual mistake.

Root cause

Not where #310 suggested. The issue proposed adding the check to prepare_interfaces_multi, but
that function is never reached: the dispatch sites at src/graph_engine.jl:2055/:2060 pass
static(length(rhs_interfaces) + length(lhs_interface)), and the generated make_node! methods are
defined only for the model's own StaticInt{num_interfaces}. A count mismatch means no method
matches at all
, so it fails one layer earlier than the issue assumed.

Change

Added a less specific make_node! fallback for Composite nodes with Tuple or NamedTuple LHS and
a generic StaticInt{N}, which reports the mismatch directly:

Node 'two_iface_sub' cannot be called with 2 output(s) on the left-hand side and 2 interface(s)
on the right-hand side, 4 in total.

The number of outputs on the left-hand side must be equal to the number of interfaces of
'two_iface_sub' that are left unspecified on the right-hand side. Currently specified interfaces
are: (:x, :b), but check the documentation to see the specification options.

It is strictly less specific than every generated method (generic in both fform and N), so it only
runs when dispatch would otherwise fail — no ambiguity, no shadowing. Message style follows the
existing single-output validation at :1858-1873 (lazy"...", Node '$(fform)', local n = "\n",
closing pointer to the documentation).

Tests

test/multi_and_zero_output_tests.jl currently pins the bad behaviour with
@test_throws "no method matching make_node!". That assertion is replaced with one on the readable
message, plus two new cases: too few LHS outputs, and the same mismatch in the named-output
(NamedTuple LHS) form.

Verified the existing clear errors for the named form are preserved — a wrong interface name still
matches on arity and reaches prepare_interfaces, so "Interface ':c' does not exist in ..." and the
LHS/RHS conflict error still fire (covered by the existing test items 8 and 9).

Full suite on Julia 1.12: 258/258 test items, all passing.

🤖 Generated with Claude Code

…er of LHS outputs

A multi-output submodel call must provide exactly as many outputs on the LHS as
there are interfaces left unspecified on the RHS. When it did not, the total
arity matched no generated `make_node!` method and dispatch failed with a
`MethodError` that said nothing about the real problem.

Note this happens one layer earlier than #310 assumed: `prepare_interfaces_multi`
is never reached, because the dispatch sites pass
`static(length(rhs_interfaces) + length(lhs_interface))` and the generated
methods only exist for the model's own `StaticInt{num_interfaces}`.

Adds a less specific `make_node!` fallback for `Composite` nodes with `Tuple` or
`NamedTuple` LHS that reports the mismatch, and updates the test that previously
pinned the `MethodError` text.

Closes #310.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.73%. Comparing base (4cc7c6c) to head (857ddc1).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #324      +/-   ##
==========================================
+ Coverage   90.72%   90.73%   +0.01%     
==========================================
  Files          16       16              
  Lines        2263     2266       +3     
==========================================
+ Hits         2053     2056       +3     
  Misses        210      210              

☔ 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.

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.

Multi-output submodel with wrong number of LHS outputs leaks a cryptic MethodError

1 participant