From 43a7375f63014f95e64be5ea042ecc66d77f1c6a Mon Sep 17 00:00:00 2001
From: Alphaxiaoteng <198982749+Copilot@users.noreply.github.com>
Date: Fri, 4 Sep 2026 01:32:49 +0800
Subject: [PATCH] Improve error for misused positional argument (#1984)

Calling an op with a non-Value positional argument that was meant to be
an attribute/keyword (e.g. `op.LeakyRelu(x, 1.0)`) used to surface a
cryptic `AttributeError: 'float' object has no attribute '_add_usage'`
raised deep inside onnx_ir.

Validate positional inputs in `BuilderBase.call_op` and raise a clear,
actionable `TypeError` that names the op and suggests passing the value
as a keyword argument. Legitimate scalar constants are unaffected because
they are promoted to constants before this point when the builder enables
CAST_INPUTS (the FULL feature set used by `@script`).

Fixes #1984
---
 onnxscript/_internal/builder_test.py | 30 ++++++++++++++++++++++
 onnxscript/_internal/tape_builder.py | 37 ++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/onnxscript/_internal/builder_test.py b/onnxscript/_internal/builder_test.py
index 451d19fd3d..449fe1dae3 100644
--- a/onnxscript/_internal/builder_test.py
+++ b/onnxscript/_internal/builder_test.py
@@ -12,6 +12,7 @@
 import onnxscript._internal.builder as builder
 import onnxscript.testing
 from onnxscript import script
+from onnxscript._internal.tape_builder import BuilderFeature, TapeBuilder
 from onnxscript.onnx_types import DOUBLE, FLOAT, INT64
 
 _default_opset_version = 23
@@ -1040,6 +1041,35 @@ def test_none_input_with_custom_domain(self):
         self.assertIs(node.inputs[2], y)
         self.assertIsNotNone(result)
 
+    def test_non_value_positional_argument_raises_clear_type_error(self):
+        """GitHub issue #1984: passing a constant where an attribute/keyword was
+        expected (e.g. ``op.LeakyRelu(x, 1.0)``) used to raise a cryptic
+        ``AttributeError`` from onnx_ir (``'float' object has no attribute
+        '_add_usage'``). It should now raise a clear ``TypeError`` that tells
+        the user to pass the value as a keyword argument.
+        """
+        x = ir.Value(name="x", shape=[1, 3, 4, 4], type=ir.TensorType(ir.DataType.FLOAT))
+
+        # Default (NONE-feature) builder: no scalar promotion, so the float is a
+        # genuine misuse of a positional argument.
+        with self.assertRaises(TypeError) as cm:
+            TapeBuilder().LeakyRelu(x, 1.0)
+        message = str(cm.exception)
+        self.assertIn("LeakyRelu", message)
+        self.assertIn("keyword", message)
+        # Make sure we did not regress to the old cryptic AttributeError.
+        self.assertNotIn("_add_usage", message)
+
+        # The same clear error is produced for other ops (e.g. Add).
+        with self.assertRaises(TypeError):
+            TapeBuilder().Add(x, 1.0)
+
+        # Legitimate scalar constants must still be promoted to constants when
+        # the builder enables CAST_INPUTS (the FULL feature set used by
+        # ``@script``), so valid usage is unaffected.
+        out = TapeBuilder(features=BuilderFeature.FULL).Add(x, 1.0)
+        self.assertIsInstance(out, ir.Value)
+
     def test_call_creates_single_function_node(self):
         """Test that GraphBuilder.call creates a single function call node."""
         op, x, y = _create_builder_with_inputs()
diff --git a/onnxscript/_internal/tape_builder.py b/onnxscript/_internal/tape_builder.py
index a8a6a6435a..c99ef4e615 100644
--- a/onnxscript/_internal/tape_builder.py
+++ b/onnxscript/_internal/tape_builder.py
@@ -380,6 +380,43 @@ def call_op(
         # 6. Determine outputs
         output_values = self._adapt_outputs(outputs, op_type)
 
+        # 6.5 Validate positional inputs.
+        # A non-Value positional argument (e.g. a constant passed where an
+        # attribute/keyword was expected, as in ``op.LeakyRelu(x, 1.0)``) would
+        # otherwise surface as a cryptic ``AttributeError`` raised deep inside
+        # onnx_ir when it tries to use the argument as a graph value. Raise a
+        # clear, actionable ``TypeError`` instead.
+        # Lists/tuples of values are allowed (variadic inputs may be grouped).
+        for index, arg in enumerate(args):
+            if arg is None or isinstance(arg, ir.Value):
+                continue
+            if isinstance(arg, (list, tuple)) and all(
+                a is None or isinstance(a, ir.Value) for a in arg
+            ):
+                continue
+            if schema is not None:
+                # Suggest the most likely attribute name from the schema so the
+                # user knows how to pass the value correctly.
+                attributes = getattr(schema, "attributes", None) or ()
+                attr_hint = ""
+                if index < len(attributes):
+                    attr_hint = (
+                        f" It looks like this should be the "
+                        f"'{attributes[index].name}' attribute; pass it as a "
+                        f"keyword (e.g. op.{op_type}(x, "
+                        f"{attributes[index].name}={arg!r}))."
+                    )
+                raise TypeError(
+                    f"{op_type}() got a non-Value positional argument "
+                    f"{arg!r} at position {index + 1}.{attr_hint}"
+                )
+            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}))."
+            )
+
         # 7. Build the node
         if name is None:
             name = self._generate_node_name(op_type)