From 0d8b54e1986afc58fcc62e6eeba5904e67a0d3c5 Mon Sep 17 00:00:00 2001 From: Ivar Vong Date: Mon, 1 Jun 2026 16:57:54 -0400 Subject: [PATCH] fix(decimal): regression tests + informative builtin clause errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A production Sentry report (issue 4c87eb50) showed `Pyex.Stdlib.DecimalModule.decimal_new/1` crashing with `FunctionClauseError` (arg0 = []) under the `defaultdict(Decimal)` accumulation pattern: the first read of any key invokes the factory with no args, hitting an unhandled clause. The crash itself was fixed in PR #46 (April 22): `decimal_new([])` now returns `{:pyex_decimal, Decimal.new(0)}` and a final `decimal_new([val])` clause returns a proper TypeError instead of crashing. But: 1. There was no targeted regression test for either the `Decimal()` no-arg constructor or the `defaultdict(Decimal)` end-to-end pattern. Add four tests in decimal_test.exs covering both, plus the multi-arg TypeError case. 2. The interpreter's FunctionClauseError rescue in `Pyex.Interpreter.Invocation` (three identical blocks in `call_builtin/4`, `call_builtin_raw/4`, `call_builtin_kw/5`) converted any unhandled-clause crash into a Python `TypeError: invalid arguments` — generic, unactionable, and bad for LLM self-healing. Replace with a diagnostic message naming the builtin (extracted via `Function.info/1`) and the shape of the args it received, e.g.: TypeError: Pyex.Builtins.builtin_len/1(int, int): no matching clause (wrong number or types of arguments) The LLM consumer now has enough to fix its call without guessing. Also add the corresponding `:erlang.fun_info/1` BIF to `@erlang_allowed` in the banned-call tracer: `Function.info/1` lowers to it, and it's a pure reflection BIF with no side effects. --- lib/pyex/banned_call_tracer.ex | 3 +- lib/pyex/interpreter/invocation.ex | 69 ++++++++++++++++++++++++++++-- test/pyex/interpreter_test.exs | 44 +++++++++++++++++++ test/pyex/stdlib/decimal_test.exs | 55 ++++++++++++++++++++++++ 4 files changed, 167 insertions(+), 4 deletions(-) diff --git a/lib/pyex/banned_call_tracer.ex b/lib/pyex/banned_call_tracer.ex index 45f0408..936bc61 100644 --- a/lib/pyex/banned_call_tracer.ex +++ b/lib/pyex/banned_call_tracer.ex @@ -133,7 +133,7 @@ defmodule Pyex.BannedCallTracer do # `setelement`, `tuple_to_list`, `list_to_tuple`). # - Pure conversions (`*_to_binary`, `binary_to_*`, `binary_part`, # `iolist_to_binary`). - # - Reflection (`function_exported`, `get_module_info`). + # - Reflection (`fun_info`, `function_exported`, `get_module_info`). # - Pure hashing / unique identifiers # (`crc32`, `phash2`, `unique_integer`, `make_ref`). # - Wall clock / VM stat reads with no side effects on the caller @@ -209,6 +209,7 @@ defmodule Pyex.BannedCallTracer do {:iolist_to_binary, 1}, {:list_to_binary, 1}, # reflection + {:fun_info, 1}, {:function_exported, 3}, {:get_module_info, 2}, # pure hashing / unique ids diff --git a/lib/pyex/interpreter/invocation.ex b/lib/pyex/interpreter/invocation.ex index 5bdef01..aa5c89d 100644 --- a/lib/pyex/interpreter/invocation.ex +++ b/lib/pyex/interpreter/invocation.ex @@ -726,7 +726,7 @@ defmodule Pyex.Interpreter.Invocation do fun.(derefed_args) rescue FunctionClauseError -> - {:exception, "TypeError: invalid arguments"} + {:exception, builtin_clause_error_message(fun, derefed_args)} e in [ArithmeticError, ArgumentError, Enum.EmptyError] -> {:exception, "TypeError: #{Exception.message(e)}"} @@ -794,7 +794,7 @@ defmodule Pyex.Interpreter.Invocation do fun.(args) rescue FunctionClauseError -> - {:exception, "TypeError: invalid arguments"} + {:exception, builtin_clause_error_message(fun, args)} e in [ArithmeticError, ArgumentError, Enum.EmptyError] -> {:exception, "TypeError: #{Exception.message(e)}"} @@ -825,7 +825,7 @@ defmodule Pyex.Interpreter.Invocation do fun.(derefed_args, derefed_kwargs) rescue FunctionClauseError -> - {:exception, "TypeError: invalid arguments"} + {:exception, builtin_clause_error_message(fun, derefed_args, derefed_kwargs)} e in [ArithmeticError, ArgumentError, Enum.EmptyError] -> {:exception, "TypeError: #{Exception.message(e)}"} @@ -859,6 +859,69 @@ defmodule Pyex.Interpreter.Invocation do end end + # Builds a diagnostic Python-level TypeError message when a builtin + # raises FunctionClauseError — i.e. the user passed args that hit no + # clause head. Includes the builtin's name (via `Function.info/1`) + # and a short summary of the arg shapes, so the LLM consuming the + # error can fix its call. The generic fallback "TypeError: invalid + # arguments" is uninformative for self-healing. + @spec builtin_clause_error_message( + (... -> term()), + [Interpreter.pyvalue()], + %{optional(String.t()) => Interpreter.pyvalue()} + ) :: String.t() + defp builtin_clause_error_message(fun, args, kwargs \\ %{}) do + name = builtin_name(fun) + arg_summary = Enum.map_join(args, ", ", &arg_type_summary/1) + kwarg_summary = if map_size(kwargs) == 0, do: "", else: ", " <> kwargs_summary(kwargs) + + "TypeError: #{name}(#{arg_summary}#{kwarg_summary}): no matching clause" <> + " (wrong number or types of arguments)" + end + + @spec builtin_name((... -> term())) :: String.t() + defp builtin_name(fun) do + info = Function.info(fun) + mod = Keyword.get(info, :module) + name = Keyword.get(info, :name) + arity = Keyword.get(info, :arity) + + cond do + mod == nil or name == nil -> "" + true -> "#{inspect(mod)}.#{name}/#{arity}" + end + end + + @spec arg_type_summary(Interpreter.pyvalue()) :: String.t() + defp arg_type_summary(value) do + case value do + v when is_integer(v) -> "int" + v when is_float(v) -> "float" + v when is_boolean(v) -> "bool" + v when is_binary(v) -> "str" + nil -> "None" + {:py_list, _, _} -> "list" + {:py_dict, _, _} -> "dict" + {:py_set, _, _} -> "set" + {:tuple, _} -> "tuple" + {:pyex_decimal, _} -> "Decimal" + {:builtin, _} -> "builtin" + {:function, _, _, _, _, _, _} -> "function" + {:class, _, _, _} -> "class" + {:instance, _, _, _} -> "instance" + {:generator, _} -> "generator" + {:iterator, _} -> "iterator" + {tag, _} when is_atom(tag) -> Atom.to_string(tag) + {tag, _, _} when is_atom(tag) -> Atom.to_string(tag) + _ -> "?" + end + end + + @spec kwargs_summary(%{optional(String.t()) => Interpreter.pyvalue()}) :: String.t() + defp kwargs_summary(kwargs) do + Enum.map_join(kwargs, ", ", fn {k, v} -> "#{k}=#{arg_type_summary(v)}" end) + end + @doc false @spec call_bound_builtin( Interpreter.pyvalue(), diff --git a/test/pyex/interpreter_test.exs b/test/pyex/interpreter_test.exs index 266d6b6..0453bdb 100644 --- a/test/pyex/interpreter_test.exs +++ b/test/pyex/interpreter_test.exs @@ -3286,4 +3286,48 @@ defmodule Pyex.InterpreterTest do assert result == 2 end end + + describe "builtin FunctionClauseError diagnostics" do + # When a builtin raises FunctionClauseError (i.e. the user passed an + # arg shape that hit no clause head), the interpreter catches it and + # surfaces a Python-level TypeError. The message must name the + # builtin and summarise the arg shapes so an LLM consumer can + # self-correct. See the original Sentry report (decimal_module + # `defp decimal_new([])` missing in a pre-PR-#46 build) for the + # motivating crash. + + test "len() with two args names the builtin and shows arg types" do + {:error, err} = + Pyex.run(""" + try: + len(1, 2) + except TypeError as e: + raise RuntimeError(str(e)) + """) + + # Message must contain enough for an LLM to fix its call: + # - module + function name + # - the arg shapes it actually got + # - that it was a clause mismatch + assert err.message =~ "builtin_len" + assert err.message =~ "int, int" + assert err.message =~ "no matching clause" + end + + test "rescued message is caught by Python try/except as TypeError" do + # Critical: the FunctionClauseError must surface as a Python + # TypeError so caller code can catch it, not as an internal + # runtime crash. + result = + Pyex.run!(""" + try: + len(1, 2) + "no-error" + except TypeError: + "caught" + """) + + assert result == "caught" + end + end end diff --git a/test/pyex/stdlib/decimal_test.exs b/test/pyex/stdlib/decimal_test.exs index 9a8d8dd..b539d2f 100644 --- a/test/pyex/stdlib/decimal_test.exs +++ b/test/pyex/stdlib/decimal_test.exs @@ -42,6 +42,61 @@ defmodule Pyex.Stdlib.DecimalTest do assert result == "5" end + test "Decimal() with no arguments returns Decimal('0')" do + # CPython: Decimal() == Decimal('0'). This is the path + # `defaultdict(Decimal)` takes on a missing key. A regression + # here would crash with FunctionClauseError in the builtin + # dispatcher — see github issue #4c87eb50 (Sentry) and the + # "defaultdict(Decimal) auto-creates Decimal('0') on missing key" + # test below for the end-to-end pattern. + result = + Pyex.run!(""" + from decimal import Decimal + str(Decimal()) + """) + + assert result == "0" + end + + test "defaultdict(Decimal) auto-creates Decimal('0') on missing key" do + # Real-world pattern from a sandboxed analytics workload: + # accumulating net positions per symbol with `defaultdict(Decimal)` + # and `+=`. The first touch of any symbol invokes the factory with + # zero arguments. + result = + Pyex.run!(""" + from decimal import Decimal + from collections import defaultdict + net = defaultdict(Decimal) + for sym, side, shares in [ + ('VTI', 'BUY', '10.5'), + ('VTI', 'SELL', '2.3'), + ('BND', 'BUY', '5.0'), + ]: + s = Decimal(shares) + if side == 'BUY': + net[sym] += s + else: + net[sym] -= s + # `net['MISSING']` exercises the no-arg factory path on a key + # that was never written to. + '|'.join(str(net[k]) for k in ['VTI', 'BND', 'MISSING']) + """) + + assert result == "8.2|5.0|0" + end + + test "Decimal with too many arguments raises TypeError" do + assert {:error, %Pyex.Error{message: msg}} = + Pyex.run(""" + from decimal import Decimal + Decimal(1, 2, 3) + """) + + assert msg =~ "TypeError" + assert msg =~ "Decimal()" + end + test "invalid Decimal literal returns InvalidOperation" do assert {:error, %Pyex.Error{message: msg}} = Pyex.run("""