Repository navigation
Conversation
…l-graph loops (#713) PR #136 added a per-file guard to the Python extractor so one pathological file could not abort the whole repo's extraction, and named the JS / C / Ruby / PHP extractors as a tracked follow-up: "The same latent unguarded-loop pattern in the JS / C / Ruby / PHP extractors is a tracked follow-up, out of scope here." #170 closed the C / Ruby / PHP half. The JS one stayed open because that parser is implemented in JavaScript, so `_process_file_guarded` was never ported to it. `TypeScriptAnalyzer.analyzeFiles` guards Step 1, the file-ADDING loop, only. Steps 2 and 3 -- extraction and call-graph building -- were bare, and the nearest catch above them is the CLI's outer handler, which prints and `process.exit(1)`. So a single file throwing inside the ts-morph tree walk took the whole repository's JS/TS unit inventory to zero, reported as a parse failure rather than as a skipped file. Measured at the base: a file holding `return 1+1+...+1` (50k terms) is added successfully and then raises `RangeError: Maximum call stack size exceeded` in both loops, while the sibling file in the same project extracts cleanly -- the whole repo's units were lost to one file's stack depth. For a SAST tool that is a total false negative for the language on that repo; the Python, C, Ruby and PHP parsers all degrade instead. Both loop bodies now carry the sibling contract: log the file plus the error type on stderr, count the file ONCE in `files_with_errors`, continue. `statistics.files_processed` / `statistics.files_with_errors` join the analyzer's output so the degradation is not stderr-only, mirroring the sibling parsers' key names, and `files_processed + files_with_errors` equals the files iterated by construction -- the Python guard's bucket invariant: a file that crashed is counted once as an error and never as processed. A file whose EXTRACTION threw is skipped by the call-graph loop. That is not tidiness: it was measured. Running Step 3 over a file whose extraction failed yields functions=2 / callGraph=4 on a two-file repo, with `boom.js:one` and `boom.js:two` present in the graph and absent from the inventory -- fabricated units, and the `len(callGraph) === len(functions)` lockstep that tests/parsers/javascript/test_call_graph_companion.py and two sibling test files assert breaks, because the Step-4 backstop can only ADD missing companions, never drop surplus ones. Those keys would also propagate into the resolved `call_graph` / `reverse_call_graph` built in Step 5. A throw in the call-graph loop alone is isolated the other way round: the file keeps the units it already yielded and loses only its edges, which the Step-4 backstop fills with an empty list. The CLI prints one summary line when any file failed, exactly as the sibling extractors' CLI does (parsers/python/function_extractor.py:1143-1144). This matters more here than it reads: a guarded parse exits 0, and core/parser_adapter.py books a 0-unit parse as a successful one with no advisory of its own, so without the summary a repository whose every JS file is pathological would be indistinguishable from a repository containing no JS at all. Before this change that case produced `exit 1` and, in multi-language mode, the `[Parser] DEGRADED` advisory; the summary line is what keeps the signal loud now that the parse succeeds. Declared limitation, asserted rather than hidden (test_deep_member_chain_degrades_instead_of_aborting): a deep member-access chain (`a.b.b.b...`) overflows while the TypeScript binder walks the SHARED program, so every file's extraction throws, not only the pathological one -- measured, including when the good file is extracted first, so it is not state poisoning from the earlier failure. The guard still turns a hard failure (exit 1, no output written, the Python adapter raising RuntimeError and aborting the entire scan) into a reported, counted degradation, but it does not recover the other files' units for that shape. The regression suite drives the real entry points: the production CLI (`--files-from` / `--output`, the shape parsers/javascript/test_pipeline.py::run_typescript_analyzer uses) for the real-file witnesses, and `analyzeFiles` for the injected ones. The injected pair mirrors the template the Python extractor's robustness suite settled on after its own fixture stopped being pathological on a newer parser: a test that relies only on a pathological fixture goes vacuously green wherever the fixture no longer crashes.
gadievron
requested review from
dgeyshis,
shahar-davidson and
sounil
as code owners
October 8, 2026 15:02
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.
What
The JS/TS analyzer's extraction and call-graph loops were unguarded — one pathological file aborted the whole repo's parse (
exit 1, every unit lost). The sibling Python/C/Ruby/PHP extractors got per-file guards in #136/#170; the JS one stayed open because it is implemented in JavaScript, so_process_file_guardedwas never ported. This ports that contract.The fix
extractFunctionsFromFile/buildCallGraphForFile): a throwing file is logged, counted once infiles_with_errors, and skipped — the parse continues, the sibling files' units survive.len(callGraph) === len(functions)).Files with errors: N of M, the sibling CLIs' own contract): a degraded parse exits 0, so without it a repo whose every file is pathological was indistinguishable from a repo with no JS files.statistics.files_with_errors+ the honest denominator: Step-1 failures (unreadable/missing files) count as errors, never absences; the denominator is the distinct resolved input set so duplicate/aliased lists cannot inflatefiles_processed.The witnesses (all RED at
fc886b35, purely assertion-red: 4 failed / 0 passed)7 tests in
tests/parsers/javascript/test_issue713_loop_guards.py, driving the real CLI (--files-from/--output, the production pipeline's shape) andanalyzeFilesitself with injected throws: the pathological file, the isolated+bucketed extraction and call-graph throws, the declared binder-overflow residual (degrade-never-abort), the Step-1 bucket (locked + missing files), and the alias-dedupe lock (kills both denominator mutants).Verification
tests/parsers/javascript/); the 46-fileanalyzer_outputcontract battery green; full tree 4413 passed / 34 skipped with only the 2 pre-existingtest_llm_sdk_contract_floorenv-pin failures — identical by name at base and head...///alias bypass, round 4 the missing regression lock — all fixed with executed RED/GREEN or mutant-kill receipts.closure=1, failing=7/7/7/4).Fixes #713