Skip to content

Close the receipt stack's outstanding review Lows - #632

Merged
MaxGhenis merged 9 commits into
mainfrom
w4/receipt-review-lows
Oct 16, 2026
Merged

MaxGhenis merged 9 commits into
mainfrom
w4/receipt-review-lows

Conversation

@MaxGhenis

@MaxGhenis MaxGhenis commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to #571, #579 and #588, the receipt stack now on main. It collects the Lows their independent reviews left as non-blocking.

Change

False refusal, fixed (#571 round 16). A helper annotated List['Annotated[float, "weekly | monthly"]'] inside a measure function was refused, while the unquoted form bound. The typing-alias check reparsed the forward reference and read the Annotated metadata string as a type. holds_union now walks type expressions structurally:

  • it reads only the first element of Annotated;
  • it never parses Literal values;
  • it handles quoted forward references the same way.

Documented limitation (#571 round 16, narrowed after this PR's round 1). Any use of id or another refused name (globals, eval, …) as a variable is refused: a read, an assignment target or a del, even of a parameter or local, such as reading a household identifier named id. Telling a local from the builtin needs lexical-scope analysis, which this rule deliberately doesn't attempt. Positions that are not variable uses are allowed: an unread parameter or function of that spelling, an attribute (simulation.id) and a keyword argument (id=3). The docs now say exactly that and suggest renaming; a new test pins each position.

Round 1 of this PR's review (GPT-6.1 Sol, REQUEST-CHANGES) found the structural walk's Annotated exemption applied when any binding of the name was Annotated. A helper that rebinds it (from typing import Tuple as Annotated) hid a real union: the reviewer's end-to-end case bound and produced 29 or 28 pennies depending on typing's cache. Fixed in a15a98f:

  • the Literal and Annotated exemptions now need every possible binding to be that form, in both the marking pass and the walk; an ambiguous name is inspected as a type, and the reviewer's case is refused;
  • the walk and name resolution are iterative, so quoted layers deeper than the recursion limit no longer raise RecursionError; the reviewer's 32-case deep-quote probe now matches main on every case.

Four mutants (each exemption back to any-binding, and the recursive walk) are each killed by the new tests.

Regressions for surviving mutants and untested paths:

From Test Kills
#571 r16 List[Optional[Decimal]] inside a function is refused dropping Optional from the union forms
#571 r16 a certificate naming other bytes gives an uncertified receipt, not a validation error dropping the certified-bytes comparison
#571 r16 quoted and unquoted Annotated metadata with | binds reading all of Annotated's elements
#571 r19 fresh process: Final['_receipt_forward_probe'] poisoned in typing's cache before anything binds is refused (the test asserts the module really received the poisoned alias) — (pins the interpreter-constant flag)
#579 r5 a stale composition member is caught first or last dropping the member re-check
#579 r5 a current-law member that later gains changes is caught, alone, mixed and nested dropping the current-law composition's handle
#588 r4 a str subclass with its own __str__ hashes as its value str(value) instead of str.__str__(value)

Round 16 had also reported a surviving mutant that deletes module bindings in the alias resolver. The existing tests kill it at main's head (18 failures), so nothing was needed.

Tests

  • Receipt suites, editable layout (--noconftest): 936 passed, 2 skipped (round 1 added 11 tests).
  • Receipt suites plus the budget contract, CI layout (PYTHONPATH=src): 1037 passed, 7 skipped.
  • Schema check passes.

axiom: n/a: receipt provenance infrastructure, no policy encoded.

🤖 Generated with Claude Code

MaxGhenis and others added 9 commits October 15, 2026 10:55
From the reviews of #571 (rounds 16 and 19), #579 (round 5) and #588
(round 4):

- Quoted Annotated metadata inside a typing generic
  (List['Annotated[float, "weekly | monthly"]']) was falsely refused: the
  union check reparsed the forward reference and read the metadata string
  as a type. It now walks type expressions structurally, reading only the
  first element of Annotated and never parsing Literal values.
- Docs: `id` and the other refused names are refused even as parameter
  or local names (telling them from the builtin needs scope analysis).
- Regressions for surviving mutants and untested paths:
  - List[Optional[...]] inside a function is refused;
  - a certificate for other bytes gives an uncertified receipt, not an
    error;
  - a str subclass with its own __str__ hashes as its value;
  - a stale composition member is caught in any position;
  - a current-law member that later gains changes is caught, alone,
    mixed and nested;
  - in a fresh process, a Final forward reference poisoned in typing's
    cache before anything binds is refused.
  Each fails against the mutant it guards.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The function-code union check skipped Literal values and Annotated
metadata when any object a name could stand for was that form. A helper
that rebinds the name (`from typing import Tuple as Annotated`) then hid
a real type argument: `List['Annotated[float, "List[Union[Decimal,
float]]"]']` bound, and produced 29 or 28 pennies depending on what
earlier code had cached. The exemption now needs every possible binding
to be the form, in both the marking pass and the type walk; an ambiguous
name is inspected as a type.

The type walk and name resolution are now iterative, so quoted layers
deeper than the recursion limit are inspected rather than raising
RecursionError (the round-17 deep-quote probe now matches main on all 32
cases). docs/receipts.md now says what the refused-name guard checks:
uses of the name as a variable, not unread parameters, attributes or
keyword arguments.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Round 2 of the review found that the import-only resolution was treated
as exhaustive: `Annotated = Tuple` inside the function, or a nested
helper's parameter named `Annotated`, left the module's import as the
only resolved binding, so the metadata exemption hid a real union (the
same 29-or-28-pennies case). The function's code is now scanned for
every name it binds other than by import, in any scope: assignment,
loop, with, comprehension and := targets, deletions, parameters, except
and match captures, nested definitions, type parameters and global or
nonlocal declarations. Forward references are scanned too, since a :=
target or a lambda's parameters inside one bind names when it is
evaluated. A Literal or Annotated exemption needs the name to be bound
by nothing of that kind; past the size limit no exemption is granted.

Adds the round's cases (assignment, nested parameter, Literal assigned,
:= in a forward reference, lambda parameter in a forward reference) and
a directly built 3x-recursion-limit attribute chain that kills both
surviving mutants (recursive resolution, and the message without its
RecursionError fallback). Five mutants of the new guard are killed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Round 3 of the review found that an import inside the function whose
target is not loaded when the receipt is built was dropped from the
name's bindings. `import r3_unloaded_aliases as t` then left only the
module-level `typing as t`, so `t.Annotated`'s metadata was exempted,
while at run time the import bound Tuple: 29 or 28 pennies under one
identity.

Such an import, a relative import that cannot be resolved, and an
attribute of anything but a loaded module (or one it lacks now) now
resolve to an explicit unresolved marker. It never earns a Literal or
Annotated exemption, and union detection treats it as possibly a typing
generic and possibly a union, so subscripting it with a union, or
subscripting a typing generic with it, is refused. A name the code
does not import at all (a parameter, a local) still resolves to
nothing, as documented.

Adds the round's case and four more routes (a subscript of an unloaded
import, a name from an unloaded module, an unloaded module used
directly, an unresolvable relative import, an imported class's
attribute), and two bind cases (an unloaded import with no union, a
parameter's attribute subscripted with bitwise or). Seven mutants are
killed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Round 4 of the review:

Medium (new here): the Literal/Annotated exemption trusted what a name
pointed to at receipt time, but code can change that without binding
the name. Attribute stores and deletions, and the literal names given
to setattr or delattr, now count as rebinding that name wherever it
appears in a chain (sys.modules[__name__].Annotated = Tuple, typing.
Annotated = Tuple), and a function that uses any modules attribute
(sys.modules swaps) gets no exemption at all. import a.b as c now
resolves as Python binds it, through getattr(a, "b"). Stores on other
names, such as simulation.rate, change nothing.

Medium (on main): string expansion stopped silently at 100,000 nodes,
leaving later forward references unchecked. Strings that cannot build a
type (labels, codes) are no longer expanded or counted, and past the
limit the function is refused.

Low: a RecursionError escaped from the new pre-pass and the walk. Every
parse site now fails closed (no exemption, a refusal, or a possible
union).

Low: Literal values and Annotated metadata that are enum members
(HTTPStatus.OK) were refused as unresolved; the walk now skips nodes
already marked as values. A name the receipt cannot resolve is reported
as such, not as a union.

The docs say what is not followed (local aliases such as Amount =
Union[...], getattr(typing, 'List'), importlib.import_module, forward
references built at run time). Seventeen mutants, including the nine
the review found alive, are each killed; the review's A to D routes are
refused.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Round 5 of #632:

- Sixteen routes undid the Literal and Annotated exemption at run time.
  Following the reviewer's tested fix: stores, setattr/delattr names and
  any use of `modules` now count across every function of the module (a
  function can call a sibling); `modules` counts as a name, an imported
  name or a string; setattr, delattr and getattr make the exemption
  unsound unless called directly with a literal name; a local import of
  a name or module path that code stores to is rebound; and a forward
  reference naming those builtins, import_module, a refused name or a
  dunder withholds the exemption (Pydantic runs what it calls).
- A lazy loader's module __getattr__ no longer runs while a receipt is
  built: attributes are read from the module's namespace, then
  sys.modules (`import a.b as c`, `from a import b`, and attribute
  chains).
- The inspection limit no longer refuses ordinary code: a string counts
  only if it subscripts something the receipt can name or holds a nested
  string (bracketed parameter paths are skipped), and the binding scan
  counts only strings that could bind a name or build a type, not labels.
- Docs, docstring and changelog state the threat model (an honest
  producer, not a sandbox), the routes not followed, and the new
  refusals.
- Tests for each route, the five money-moving mutants (scan limit,
  nested string, chain middle, setattr and modules in forward
  references), del and delattr, the lazy loader, and the false refusals.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Round 6 of #632 found two regressions against main from round 5's rules:

- A plain label ('id', '__main__', a pandas column) removed the Literal
  and Annotated exemption, because every parseable string was checked
  for code-running names. Now only a forward reference that calls
  something is checked for them (a label runs no code); stores and
  bindings in any string still count. The bare name `modules` (a local
  variable) no longer counts; `modules` as an attribute, an imported
  name or a string still does.
- Names numpy 2 and pydantic 2 provide through a module __getattr__
  (np.polynomial, pydantic.HttpUrl) could not be resolved, so the
  verdict depended on import history. Module attributes are now read
  as Python reads them, with getattr; AttributeError falls back to the
  loaded submodule, and a loader that raises anything else, or an owner
  that is not a module, gives unresolved instead of an error. A module
  import whose __getattr__ raises no longer crashes _unwrapped (L1).
- Docs and docstring scope forward references to the function itself
  and list other functions' forward references as not followed; the
  changelog names the refusals this PR adds and drops the claim that no
  lazy loader runs.
- Tests: the honest labels and local `modules`, a lazily provided name,
  a raising loader reached through a module-level import, and the
  money-moving mutants m06, m09, m11, m12, m13/m14, m17, m20, m32, m37.
  The code change follows the reviewer's tested patch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…a call

Round 7 of #632: a forward reference with no call, such as
'[0 for t.__dict__["Annotated"] in [t.Tuple]] and int', runs __setitem__
(or __setattr__) and so can rebind a typing name, yet the round-6 change
let it keep the Literal/Annotated exemption. A store to an attribute or
subscript now counts as a call, so the dunder and `modules` rules apply
to it again (the reviewer's fix).

Also:
- docs/receipts.md and the _runtime_alias_problem docstring say a string
  that neither calls nor stores runs no code, and that the string
  'modules' and the inspection limit still count.
- _names_mutated_by_attribute's docstring no longer lists a bare name
  among the uses of `modules` that count.
- The changelog example now says "an attribute of something other than a
  module inside a typing generic or subscripted with a union".
- Tests: the reviewer's three store cases (each fails without the fix),
  and the changelog's two refusals, List[pd.DataFrame.dtypes] and
  HTTPStatus.OK.phrase[0 | 0], are now pinned.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MaxGhenis
MaxGhenis merged commit c9c6360 into main Oct 16, 2026
13 checks passed
@MaxGhenis

Copy link
Copy Markdown
Contributor Author

Merged at reviewed head 842b71d with a merge commit. Gates: gh pr checks exit 0 (13/13 pass), mergeable, not draft, no change requests. Independent review: round 8 APPROVE at 842b71d (rounds 1–7 findings fixed; the round-7 store regression has three committed tests that fail without the fix). Remaining low (tests for a02/a05/a06/r02/g04 on contrived input) is queued as a follow-up.

🤖 Generated with Claude Code

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