Repository navigation
Fix #818: Extraction hardening for python - #872
Merged
Merged
Conversation
Hardens all four extraction gauntlets for python. This was a fresh sweep (no pre-flagged known finding, unlike javascript/typescript/java/go) and surfaced three real bugs, all sharing one root cause: - func_start/args/class_start all shared a flat `[^\]]*` PEP 695 (3.12+) generic-parameter step-over, truncating at the FIRST `]` and breaking any nested-bracket type bound, e.g. `def Foo[T: Sequence[int]](x: T) -> T:`. This is the square-bracket variant of the same Rule 11 bug class already fixed for java's/ typescript's angle-bracket generics. Widened to `\[(?:[^\[\]]|\[[^\[\]]*\])*\]`. - class_start's version of the bug didn't fail the whole match (the class name is captured before the generic step-over) -- it silently dropped the base-class capture instead, same failure shape as java's #816 class_start bug. crucible_check.py showed a ZERO diff -- confirmed expected, not a red flag: PEP 695 has zero real-world adoption anywhere in the ~80-repo corpus yet (grep confirms no python file uses `def/class NAME[...]` syntax). Verification relied entirely on regex-level empirical checks (tests/extraction/tools/verify_candidates.py) plus ReDoS scaling sweeps, consistent with recurring bug class 8's guidance that a zero diff can mean the corpus doesn't exercise a feature, not that the fix is wrong. Also confirmed a genuinely positive finding, not a bug: python's Mode C (_slice_by_indentation) already shields strings/comments via an index-aligned shield BEFORE matching func_start against the code -- so the string-literal false positive confirmed at the regex level (same shape as the recurring gap already found in js/ts/java/go) is NOT a live bug in python's actual pipeline. Documented as the reference implementation the eventual Mode-B (_slice_by_braces) broadening follow-up should mirror. Migrates python's test cases out of the four monolithic dict files into tests/extraction/languages/test_python.py, and updates the epic + methodology doc with the new findings (recurring bug classes 15-17). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Part of the extraction-hardening epic (#813). Closes #818.
Hardens all four extraction gauntlets (
func_start,args,class_start,_dependency_capture) for python pertests/extraction/how_to_harden_extraction.md. Unlike javascript/typescript/java/go, this issue had no pre-flagged known bug — a fresh sweep surfaced three real bugs, all sharing one root cause:func_start/args/class_startall shared a flat[^\]]*PEP 695 (3.12+) generic-parameter step-over, truncating at the first]and breaking any nested-bracket type bound, e.g.def Foo[T: Sequence[int]](x: T) -> T:. This is the square-bracket variant of the same "Rule 11" bug class already fixed for java's/typescript's angle-bracket generics. Widened to\[(?:[^\[\]]|\[[^\[\]]*\])*\].class_start's version of the bug didn't fail the whole match (the class name is captured before the generic step-over) — it silently dropped the base-class capture instead, same failure shape as java's Extraction hardening: java #816class_startbug.Also confirmed a genuinely positive finding, not a bug: python's Mode C (
_slice_by_indentation) already shields strings/comments via an index-aligned shield before matchingfunc_startagainst the code — so the string-literal false positive confirmed at the regex level (same shape as the recurring gap already found in js/ts/java/go) is not a live bug in python's actual pipeline. Documented as the reference implementation the eventual Mode-B (_slice_by_braces) broadening follow-up should mirror.Differential Scan
Ran
python tests/tools/crucible_check.py: zero diff across the ~80-repo corpus. Confirmed this is expected, not a red flag — grep across the corpus's python files shows zero usage ofdef/class NAME[...]PEP 695 syntax anywhere yet (it's a very new, 3.12+-only feature). Per the epic's own recurring bug class 8 ("a zero diff can mean the corpus doesn't exercise the feature, not that the fix is wrong"), verification relied entirely on regex-level empirical checks (verify_candidates.py) plus ReDoS scaling sweeps, all confirmed linear. No golden master changes in this PR.Test plan
pytest tests/extraction/languages/test_python.py(60 new tests) — run via barepytestfrom an unrelated CWD to reproduce CI's invocation stylepytest tests/core_engine/ tests/extraction/ -q— 3464 passed, 1 known pre-existing tiktoken-environment failure, 1 xfailedpython tests/tools/audit_check.py— clean (only the pre-existing, already-trackedguidestar_lens.pymypy findings, unrelated to this change)python tests/tools/crucible_check.py— PASS on both venvs (zero diff, as expected — see above)tests/extraction/languages/test_python.py🤖 Generated with Claude Code