Improve error for misused positional argument (#1984)

[αI (Alphaxiaoteng)]

Description

Calling an op with a non-Value positional argument that was meant to be an attribute/keyword — e.g. `op.LeakyRelu(x, 1.0)` where `alpha` should be a keyword — used to surface a cryptic error raised deep inside onnx_ir:

```
AttributeError: 'float' object has no attribute '_add_usage'
```

`BuilderBase.call_op` (the single point where every op dispatch builds its node) now validates positional inputs before constructing the node and raises a clear, actionable `TypeError` that names the op and suggests passing the value as a keyword argument:

```
TypeError: LeakyRelu() got a non-Value positional argument 1.0 at position 2. If this was meant to be an attribute, pass it as a keyword argument (e.g. op.LeakyRelu(x, alpha=1.0)).
```

When a schema is available, the message additionally suggests the specific attribute name.

Why this location

The `Node`/`Value` IR was moved into the separate `onnx_ir` package, so the reporter's original patch (against `ir/_core.py`) no longer applies. `call_op` is the onnxscript-side equivalent of "the node constructor" and the right place to give users a readable error without touching the dependency.

Notes / non-regression

• Legitimate scalar constants are unaffected: under the FULL feature set used by `@script` (which enables `CAST_INPUTS`), scalars are promoted to `ir.Value` constants before this validation runs, so `op.Add(x, 1.0)` still works.
• `None` inputs (optional inputs) and lists/tuples of `ir.Value` (grouped variadic inputs) are explicitly allowed.

Tests

Added `test_non_value_positional_argument_raises_clear_type_error` to `onnxscript/_internal/builder_test.py`. The full `builder_test.py` suite (91 tests) passes.

Fixes #1984

[Justin Chu (justinchuby)]

You may use assertRaisesRegex to pin down the error message

[Justin Chu (justinchuby)]

Could you simplify the doctring to make it self-contained (no need to reference the issue)?

[Copilot]

🟡 Changes recommended

The schema-based hint logic in the new validation block can mis-handle schema.attributes (a dict) and the fallback example hard-codes alpha=..., which can produce incorrect/misleading errors.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR improves developer-facing diagnostics when an ONNX op is called with a non-ir.Value positional argument (commonly a Python scalar that was intended to be an attribute passed by keyword), by validating positional inputs in the central op-dispatch path and raising a clear TypeError instead of surfacing a cryptic error from onnx_ir.

Changes:

• Added positional-argument validation in BuilderBase.call_op to detect non-ir.Value positional inputs and raise an actionable TypeError.
• Added a regression test to ensure the new error is raised (and that scalar constant promotion under BuilderFeature.FULL is unaffected).

File summaries

File	Description
onnxscript/_internal/tape_builder.py	Adds validation and improved error messaging for misused non-Value positional args in call_op.
onnxscript/_internal/builder_test.py	Adds a unit test covering the clearer TypeError and non-regression for scalar promotion under FULL.

Review details

Suppressed comments (1)

onnxscript/_internal/tape_builder.py:417

• The schema-unavailable error message hard-codes alpha=... in the example, which is misleading for most ops (e.g. Add has no alpha attribute). Using a generic placeholder avoids suggesting an invalid attribute name.

            raise TypeError(
                f"{op_type}() got a non-Value positional argument {arg!r} at "
                f"position {index + 1}. If this was meant to be an attribute, "
                f"pass it as a keyword argument (e.g. op.{op_type}(x, "
                f"alpha={arg!r}))."

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov]

Codecov Report

❌ Patch coverage is 68.00000% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.69%. Comparing base (3ba2bf7) to head (43a7375).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
onnxscript/_internal/tape_builder.py	33.33%	6 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3028      +/-   ##
==========================================
+ Coverage   72.64%   72.69%   +0.04%     
==========================================
  Files         265      265              
  Lines       32251    32309      +58     
  Branches     3050     3058       +8     
==========================================
+ Hits        23429    23486      +57     
  Misses       7786     7786              
- Partials     1036     1037       +1     

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