Skip to content

Fix the two inject-r4 minors in the OP1 stream scan - #593

Draft
MaxGhenis wants to merge 9 commits into
masterfrom
openpop-op1-scan-fixes-20261101
Draft

MaxGhenis wants to merge 9 commits into
masterfrom
openpop-op1-scan-fixes-20261101

Conversation

@MaxGhenis

@MaxGhenis MaxGhenis commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

WIP. Follow-up to #591. Review inject-r4 approved the OP1 stream scan with two minor findings, and #591 answered them by wording in §6 of docs/design/open_population_evidence/op1_report.md. This PR fixes both in tests/open_population/test_stream_injection.py and drops the exceptions from §6. Review scanfix-r1 asked for a bound on the new hint walk, and review scanfix-r2 found two gaps in the first bound. This PR answers both.

What changes

Literal statement suites in annotations. At 6413d842, _embedded skips annotations and the hint checks parse only expressions, so v: exec('global f\ndef f(x):\n return isinstance(x, R)\n') runs a working check when something reads the annotation, and the scan gives no hit. _embedded now passes each hint to _hint_code:

  • A code call whose literal parses as an expression stays with the hint checks, so ForwardRef("ProjectionRNGRegistry") stays trusted.
  • Any other literal goes through _expanded, with the memo, depth and unexpanded set a code call outside an annotation uses.
  • Each string in the hint that parses as an expression is one more level of code, as typing.get_type_hints evaluates it, and _hint_code looks inside it for code calls and further strings. That includes the pieces a folded or spelled string is built from, because a name the module rebinds can run one of them. A string that starts with * reads as typing compiles it, (*x,)[0].
  • The bound (reviews scanfix-r1 and scanfix-r2): the walk puts a string at a sixth level in unexpanded instead of following it, follows each text once per hint, as typing's recursive guard does, and spends from a second budget that all of a module's hints share. A hint the walk can't finish, past that budget or on a text too deep for the parser, reports dynamic-code in place of all its walk found. When the failure comes inside what a hint runs, the outermost hint takes it.
  • The scan walks no spelled copy, so the real hint stands for it in unexpanded, and _kind tests that set first, because a string hint is a constant.
  • walked_hints needed no change, because hits() already walks annotation calls.

module_path recursion. One loop unwinds attributes and sys.modules[...] subscripts from the outermost link inward, and a second replays them from the base. A chain that raised RecursionError at 1000 links now passes at 3000.

Two adjacent changes the first fix needs.

  • _prune keeps each hint whole. With the old pruning, a pruned scan missed a suite that an unpruned scan reported, in a literal under a call the folder does not fold through.
  • A code call on unreadable text spelled in a hint's literal, as in def g(v: 'exec(S)'), reports dynamic-code at the hint. Without that, limit class 4 would need a new exception.

Three over-approximations, stated in §6 and pinned by tests. The scan never asks whether Python evaluates an annotation, so a local variable's hint, which Python never runs, gives the same hit. It reads every string in a hint that parses as an expression, so a string Python keeps as data (a Literal value, Annotated metadata, a list item, a piece of a folded string) gives the hit its code would give. And a hint past the walk's budget leaves that budget spent, so each later hint in the module that holds an expression reports too.

Review scanfix-r1

Verdict REVISE at f8d64f8b, with one major, three minor and two nit findings. Each fix:

  1. Major: no bound on the hint walk. A 2,011-character hint that spells itself ran the folder past its budget. A 1002-level chain of string hints ended in a RecursionError that _apply absorbed, and the scan gave no hit. Fixed by the level bound and the per-hint guard. Both sources are now tests.
  2. Minor: data strings in a hint read as code. Fixed in part. The walk skipped pieces of folded and spelled literals, which removed the hit from a piece of an exec literal, and review scanfix-r2 showed that skip loses a working check, so it has gone. I kept reading every other string rather than only typing's places. A conditional, an and or a tuple index with a run-time index each pick a string that get_type_hints runs, and the folder cannot tell which one, so a typing-places walk misses all three silently. A false hit fails the pin loudly, so §6 states the over-approximation and three cases pin it.
  3. Minor: no async return hint in the tests. _FORCED_HINTS gains the place, and the sweep's two async mutants now die.
  4. Minor: limit class 1 omitted hint values the folder cannot read. Stated, and pinned as an intended miss.
  5. Nit: the parity test's or. It now checks each lost hit's replacement on its line.
  6. Nit: the mix test ran one scan. It runs both.

Review scanfix-r2

Verdict REVISE at 7a830f39, with two major, one minor and one nit finding, plus two surviving mutants. Each answer:

  1. Major: skipping a folded subtree loses a working check. The folder takes str to mean the builtin. After str = eval, the hint (str("exec(...)"), "int")[1] folds to "int", Python runs the piece, and the skip left that piece unread. r2 proposed that the folder refuse any name the module rebinds. I removed the skip instead, so the walk reads every string and piece. builtins.str = eval, globals()['str'] = eval, exec('str = eval') and a star import each rebind a name without an assignment a list of bindings reads directly, and that folder change would reach every other check in such a module. A false hit fails the pin loudly, so the two piece cases go back to isinstance, pinned and stated.
  2. Major: width aborts the scan. A wide hint ran the module's budget out with a ScanError, where the base gave no error. r2 proposed one budget per _hint_code call. I built that first, and a probe showed that a hint inside what a hint runs got a fresh budget, so the work multiplied at each of five levels. Each module now has one second budget, which all its hints share, nested ones included, and a hint the walk can't finish reports dynamic-code.
  3. Minor: the sixth-level invariant left out the guard's exception. Stated, and three cases pin the order.
  4. Nit: typing's guard. The report and docstring now give the scan's own argument, not typing's.
  5. The two surviving mutants each have a case.

I found two more while answering these. A string that starts with * ran its check and gave no hit at every revision, and _spelled now reads it as typing compiles it. A callee known only by name, such as run = exec, gave no hit at every revision, and limit class 1 now states it, with four pinned misses.

Invariants

  1. A pruned scan and an unpruned scan give the same hits on every source (test_pruning_leaves_every_invented_scan_unchanged, test_pruning_leaves_scans_unchanged_for_any_literal, test_pruning_keeps_each_hint_whole, and every hint test runs both).
  2. module_path agrees with a recursive reference on every chain the recursion survives: all 26,180 chains up to three links, and 200 Hypothesis chains of four to eight.
  3. A working check in a hint reports its own kind down to five levels and dynamic-code past them, for all 127 mixes of exec and eval at the four places, and for strings and string hints.
  4. A hint gives every check an exec statement gives, except name-string hits, and annotation-read in place of a namespace read or vars of a module on the same line.
  5. A literal in a hint that parses as an expression is never parsed as a suite itself: the hint checks read it, and the scan looks inside it for code calls and further strings.
  6. On the real tree the scan gives exactly the pinned hit.
  7. The walk through a hint's strings follows at most five levels and each text once, and it spends from one budget per module, so the text it parses stays within a fixed multiple of the module's text and that budget (test_scan_bounds_hints_inside_what_a_hint_runs, test_scan_shares_the_walk_budget_across_a_module).
  8. A string at a sixth level reports dynamic-code, whatever the module binds, unless the walk has already followed that text in this hint.
  9. Every failure of the walk leaves a hit on the outermost hint, and no mark outlives the tree it names (test_a_hint_past_the_walk_budget_marks_only_itself, test_a_failure_inside_what_a_hint_runs_goes_to_the_outermost_hint).
  10. A module whose walk stays within the budget spends nothing from the module's own budget for that walk (test_scan_keeps_a_hint_within_the_walk_budget_off_the_module_budget).

Evidence

Host run at 181f40ca (6 Nov, 12:36 to 13:19 UTC; the first steps started at a one-minute load under 100, the rest under 130, the mark the coordinator set that day). The next commit changes only the report.

  • tests/open_population: 812 passed and 1 strict xfail in 169.14 s, 118 more than base (all in test_stream_injection.py). The other four modules: 104 passed. Track B holdout-registry modules: 230 passed and 2 skipped. All of them in one process: 1,146 passed, 2 skipped and 1 strict xfail. Peak RSS 1.33 GB.
  • --collect-only -m unit tests/open_population: 813. Black, Ruff and git diff --check 6413d842 come back clean; the sealed-path git diff --quiet exits 0.
  • Real tree through repository_sources(): 302 modules, exactly the pinned hit (mortality_observer.py:381 type-compare), the same at 6413d842, f8d64f8b, 7a830f39 and 181f40ca.
  • Differential against 7a830f39, pruned and unpruned: the same hits on all 178 base-corpus sources. Of the 617 added sources, 23 differ in both modes, each as intended: a string that starts with *, six pieces a rebound name runs, the pieces of a folded and a spelled literal, and builtins rebound now give isinstance; six wide sources and three branching ones now report dynamic-code instead of ScanError; four sources too deep for the parser now give dynamic-code instead of MemoryError or RecursionError. Pruned and unpruned agree on all 795.
  • Mutants: 66, after an unmutated control, including seventeen for review scanfix-r2's fixes. Sixty-five fail a test. The survivor, a nested hint keeping its own failure, now dies to test_a_failure_inside_what_a_hint_runs_goes_to_the_outermost_hint at the committed module. The module came back byte for byte each time.
  • The tier counts change only in unit (+118 at base); the recount happens in the coordinator's slot after merging master.

Plan

  • Parse statement suites in annotations with the same memo, depth limit and unexpanded handling.
  • Unwind sys.modules[...] in module_path, in the same loop as attributes.
  • Add working checks that force the annotation at depths 1, 5, 6 and 7, and add the chains to LONG_CHAINS.
  • Add mutants for each fix.
  • Run the old-versus-new differential on invented sources.
  • Run the host suite and the real-tree differential.
  • Update §6: drop the exceptions Serve OP1's adapter draws from DirectRNGStreams by default #591 stated.
  • Subfleet review r1 (REVISE), findings fixed.
  • Subfleet review r2 (REVISE), findings answered.
  • Subfleet review r3.
  • Recount tier counts in the coordinator's slot.

🤖 Generated with Claude Code

Review inject-r4 found two minors that #591 stated in the report instead
of fixing. Both fixes land in the scan here.

The scan now parses a statement suite an annotation's code call holds,
with the same memo, depth limit and unexpanded handling as a suite in a
statement. Expression literals in hints stay with the hint checks.

module_path now unwinds sys.modules subscripts and attributes in one loop,
so a long chain of either cannot exhaust recursion.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vercel

vercel Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
social-security-model Ready Ready Preview Nov 6, 2026 1:24pm UTC

Request Review

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The sweep showed that no test reached _resolve from module_path, the empty
path arm or the strip in _spelled. Compare module_path with a recursive
reference on every chain of up to three links with invented re-exports, and
add working cases for a padded literal and a re-exported sys. Count levels
through every mix of exec and eval, and make the second sys.modules chain
one whose hit depends on the path. State the new behaviour in section 6.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sweep two left one survivor: dropping the argument guard crashes the
scan on exec() in a hint, and no test used that spelling.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review scanfix-r1 found that the hint walk had no bound: a hint that
spells itself ran the folder past its budget, and a 1002-level chain of
forward references ended in a swallowed RecursionError and no hit. The
walk now marks a sixth level of hint text as dynamic code, follows each
text once per hint, as typing's recursive guard does, and skips the
pieces a folded string is built from.

Tests pin the quine, the countdown at 6, 50 and 1002 levels, the level
boundary for strings and string hints, the data strings the scan reads
on purpose, the pieces it skips, a name-bound hint it misses, and async
return hints. The parity test now checks each lost hit's replacement on
its line, and the mix test runs both scans.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds the host run, the differential against f8d64f8 and the 49-mutant
sweep at 50e7e61 to the OP1 report, with review scanfix-r1's findings
and the fix for each.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review scanfix-r2 found two major gaps. Skipping a folded subtree lost a
working check under a rebound name, so the walk now reads every string and
piece in a hint. A wide hint stopped the scan with the module's budget
error, so a module's hints now share a second budget, and a hint the walk
can't finish reports dynamic-code. A star text now reads as typing reads it.
The report answers the minor finding and the nit, and states what stays.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The run from 12:36 to 13:19 UTC on 6 November, its 66 mutants, and the
three newer pending entries. This commit changes only the report.

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

This branch was successfully deployed

1 active deployment
Preview — 30b0df81 Deployed Nov 6, 2026 by vercel[bot]
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