Fix javascript/typescript func_start string-literal false positive - #860
Merged
squid-protocol merged 1 commit intoJul 30, 2026
Merged
Conversation
/#815) _slice_by_braces computed func_start.finditer(code) against the raw, unshielded code -- BEFORE the string/comment-shielded safe_code existed (safe_code was only built afterward, for the brace-search step). Since javascript's/typescript's func_start regex is \b-anchored (not ^-anchored), a single-line string literal containing function-shaped text false-positive-matched, e.g. `let query = "function Foo() {";`. Fixed by matching against safe_code instead, gated to lang_id in ("javascript", "typescript") only -- NOT applied broadly to every Mode B (brace-slicing) language. Verifying the broad version against the real crucible corpus surfaced a separate, pre-existing bug in prism.py's comment/string stripping for PHP (filed as #859): at least two real PHP corpus files' code_stream already has corrupted docblock/string content that confuses this same shielding step. That's currently harmless because the brace-search step's blast radius is naturally bounded -- but matching func_start's own positions against a corrupted safe_code turns that latent corruption into wholesale loss of real functions (confirmed: one file dropped from 1 detected function to 0, with a 17x structural-magnitude blowup; another lost a runaway 1057-line false-positive satellite, which is a genuine improvement, but the corpus-wide differential scan doesn't distinguish the two cases automatically). Broadening this fix to other Mode B languages should follow #859, not precede it. Verification: the js/ts fix confirmed correct on real code; crucible_check.py clean (zero diff) with the gate in place -- the real corpus's js/ts files don't happen to exercise this specific edge case, matching the same "correctness fix the corpus doesn't trigger" pattern already seen in #735. Full core_engine/extraction suites pass (only the pre-existing, unrelated tiktoken-dependency environment failure). ruff/mypy/dead-key audits clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
squid-protocol
deleted the
fix/814-815-shared-string-shielding-func-start
branch
July 30, 2026 22:32
This was referenced Jul 30, 2026
Closed
squid-protocol
added a commit
that referenced
this pull request
Jul 31, 2026
Hardens all four extraction gauntlets for javascript. The shared string-literal false positive (recurring bug class 3) was already fixed earlier via PR #860 (gated to js/ts); this issue closes the remaining javascript-specific case expansion work and fixes one additional real bug: - func_start/args had no allowance for ES6 generator method shorthand (`*foo() {}`, `async *foo() {}`, `static *foo() {}`) -- completely invisible before this fix. Generators are a common, legitimate ES6+ feature (custom iteration protocols, iterables). The first version of this fix used a whitespace-tolerant `\*?[ \t\n]*` gap between the star and the method name, matching the style of the other modifier gaps around it. This introduced a NEW false positive, caught via crucible_check.py against real corpus code (not by any hand-written test): a JSDoc comment continuation line (`* A (storage) buffer attribute...`, where `*` is the comment's own marker and "A" is the first word of a plain-English sentence) got hallucinated as a generator method named "A" in threejs/BufferGeometry.js. Corrected by requiring the star to hug the name with zero intervening whitespace -- every real formatter (Prettier included) emits `*foo()`, never `* foo()`, so this is strictly more accurate, not a flexibility regression. crucible_check.py showed a zero diff after the corrected fix (confirmed expected: zero generator-method usage anywhere in the ~80-repo JS corpus). Migrates javascript's test cases out of the four monolithic dict files into tests/extraction/languages/test_javascript.py, and updates the epic + methodology doc with the new finding (recurring bug class 21: a fix's own whitespace tolerance can open a new false-positive path, caught only by the real-corpus check). Co-authored-by: Joe Esquibel <squid-protocol@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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
1 participant
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
func_startfalse-positive-matched a single-line string literal containing function-shaped text (let query = "function Foo() {";).detector.py's_slice_by_bracescomputedfunc_start.finditer(code)against the raw code, before building the string/comment-shieldedsafe_code(which was only used afterward, for the brace-search step).safe_codeinstead — gated tojavascript/typescriptonly, not applied broadly to every Mode B language.Why gated, not universal
Verifying the broad (all-languages) version against the real crucible corpus surfaced a separate, pre-existing bug in
prism.py's comment/string stripping for PHP — filed as #859. At least two real PHP corpus files'code_streamalready has corrupted docblock/string content that confuses this same shielding step. That's currently harmless because the brace-search's blast radius is naturally bounded, but matchingfunc_start's own positions against a corruptedsafe_codeturns that latent corruption into wholesale loss of real functions (one file dropped from 1 detected function to 0, with a 17x structural-magnitude blowup). Broadening this fix to other Mode B languages should follow #859, not precede it — noted directly in the code comment so this gate isn't accidentally "simplified" away later.Verification
crucible_check.py: clean, zero diff on both venvs — the real corpus's js/ts files don't happen to contain this specific edge case (same "correctness fix the corpus doesn't trigger" pattern as html attribute-value patterns assume double-quoted values only (single-quoted HTML never matches) #735).core_engine/extractionsuites pass (3218 passed; only the pre-existing, unrelated tiktoken-dependency environment failure).ruff format/ruff_audit.py --ci(line-shift baseline regen)/mypy_audit.py --ci/dead_key_audit.py --ci: clean.Test plan