Skip to content

fix(decimal): regression tests for Decimal()/defaultdict + informative builtin clause errors#77

Merged
ivarvong merged 1 commit into
mainfrom
fix/decimal-no-args
Jun 2, 2026
Merged

fix(decimal): regression tests for Decimal()/defaultdict + informative builtin clause errors#77
ivarvong merged 1 commit into
mainfrom
fix/decimal-no-args

Conversation

@ivarvong

@ivarvong ivarvong commented Jun 1, 2026

Copy link
Copy Markdown
Owner

Summary

A production Sentry report (issue 4c87eb50, Pyex.Stdlib.DecimalModule.decimal_new/1 FunctionClauseError, arg0 = []) surfaced under this Python pattern:

from collections import defaultdict
from decimal import Decimal
net_shares = defaultdict(Decimal)
for sym, side, shares in rows:
    s = Decimal(shares)
    if side == 'BUY':
        net_shares[sym] += s         # first touch invokes Decimal() with no args
    else:
        net_shares[sym] -= s

The first read of any new key invokes the factory (Decimal) with no args.

Why this is happening in production

The bug is fixed on main but not in any published release. The user is running Hex package pyex 0.1.0 (released 2026-02-13). The fix landed in PR #46 on 2026-04-22 — almost two months later. Verified by extracting the pyex-0.1.0.tar checked into the repo root: it contains only three decimal_new/1 clauses ([val] when is_binary, [val] when is_integer, [{:pyex_decimal, _}]). Decimal() with no args hits none of them and crashes at the first clause head — line 21, exactly matching the Sentry trace.

To stop the crash in production: merge open release PR #60 (chore(main): release 0.2.0). That PR has been open since 2026-05-10 and contains 3+ weeks of fixes including this one. Merging it triggers mix hex.publish via .github/workflows/release-please.yml.

What this PR adds (defence-in-depth)

The crash itself was fixed three weeks ago. But auditing the path that led to it surfaced two improvements worth shipping independently:

1. Regression tests for the failing pattern (test/pyex/stdlib/decimal_test.exs)

Four tests pinning the behaviour:

  • Decimal() with no arguments returns Decimal('0') — direct repro of the factory call.
  • defaultdict(Decimal) auto-creates Decimal('0') on missing key — end-to-end repro of the Sentry pattern with realistic accumulation.
  • Decimal with too many arguments raises TypeError — covers the explicit length(args) > 1 clause.

If decimal_new([]) ever regresses, these go red immediately. Verified against the current main (passes) and confirmed the pattern from the published 0.1.0 tarball would have failed them (would crash with FunctionClauseError).

2. Informative FunctionClauseError rescue messages

The interpreter's Pyex.Interpreter.Invocation rescues FunctionClauseError from builtins in three places (call_builtin/4, call_builtin_raw/4, call_builtin_kw/5) and converted them to a generic TypeError: invalid arguments. That message is uninformative — bad for an LLM that has to self-correct.

Replace with a diagnostic message naming the builtin (via Function.info/1) and summarising the arg shapes:

# Before
TypeError: invalid arguments

# After
TypeError: Pyex.Builtins.builtin_len/1(int, int): no matching clause (wrong number or types of arguments)

Two tests in interpreter_test.exs cover:

  • the new message names the builtin and includes arg type summary
  • the rescued error is still caught by Python try/except TypeError as before (no behaviour change for callers that catch).

This won't fix the current user (their build is too old to have this code) but it will make the next user-visible arg-shape mismatch easier to diagnose.

3. Allowlist :erlang.fun_info/1 in the banned-call tracer

Function.info/1 lowers to :erlang.fun_info/1, which wasn't in @erlang_allowed (the existing tracer test caught this immediately). Pure reflection BIF; added under the existing 'reflection' category alongside function_exported/3 and get_module_info/2.

Verification

  • mix test — 291 properties, 5462 tests, 0 failures, 2 skipped
  • mix compile --warnings-as-errors — clean
  • mix format --check-formatted — clean
  • mix dialyzer — passed, 0 errors
  • CI: 7/7 jobs green

Tested honestly?

My first pass didn't catch that the published Hex release is stale — I tested against current main, saw the fix work, and concluded the user just needed to redeploy. That's correct in a vacuous sense (their build IS old) but the actionable answer is 'merge PR #60'. The regression tests in this PR matter, but on their own they will not stop the production crash. Releasing 0.2.0 will.

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.
@ivarvong ivarvong merged commit 754a34a into main Jun 2, 2026
7 checks passed
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.

1 participant