fix(extract): remove quadratic scan in TS import-type normalization (#3359) - #3402
fix(extract): remove quadratic scan in TS import-type normalization (#3359)#3402Sagexd08 wants to merge 2 commits into
Conversation
…raphify-Labs#3359) `_normalize_ts_import_types` filtered each `import(...)` regex match against every tree-sitter `type_arguments` byte range with a nested linear scan, in both the pre-mask and post-mask passes: matches = [m for m in matches if not any(start <= m.start() < end for start, end in original_type_ranges)] That is O(matches x ranges), and `m.start()` is re-evaluated inside `any()` for every range — which is why profiles blamed `re.Match.start`/`_sre` and `sample` appeared to show a pathological regex. The regexes are all linear; the quadratic caller was the real cost. Both counts grow with file size in TypeScript that mixes generic calls (`fn<A, B>(x)` — one `type_arguments` range each) with `import(...)` types, which is the shape of generated and barrel modules in a large monorepo. On the filter alone: a 0.8MB/10k-import file took ~21s, 2.1MB/25k ~2.6min, and 4.2MB/50k ~11.8min. With several such files `extract` never finished, and a worker sat at 100% CPU holding no open files because the work is entirely in-memory after the read. Replace both scans with a binary search over start-sorted ranges plus a prefix-maximum of `end` values, so a lookup touches nesting depth rather than every range. Type-argument ranges nest but never partially overlap, so the prefix maximum is a correct stop condition; a *closed* inner sibling must not terminate the leftward walk, which a naive running-max break gets wrong. Verified against the old predicate with 500k differential cases, including non-nested random ranges: zero mismatches. Normalization output is unchanged — byte-length and line-count preserving, same masking decisions, and the runtime-vs-type disambiguation from 6baa5e0 still holds. Also restore the emptiness guard in the second pass: the range list is now an index tuple, which is always truthy, so it checks the spans it holds. The new test asserts on scaling (2x input must not ~4x the time) rather than wall-clock, so it is not machine-dependent. It fails at ~3.4x before this change and passes after.
There was a problem hiding this comment.
🟡 Changes recommended
The new _TsRangeIndex is defined as a string literal but used as a type alias in annotations, which can break Pyright/type-checking.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR addresses #3359 by removing an O(matches × ranges) nested scan in the TypeScript import(...) type normalization path, replacing it with an indexed lookup to keep extraction performance linear on large TS files.
Changes:
- Add a start-sorted range index (
starts,spans,prefix_max_end) plus_ts_ranges_containinglookup to avoid per-match linear scans. - Update
_normalize_ts_import_typesto use the new lookup in both filtering passes and restore the second-pass emptiness guard. - Add a regression test that asserts near-linear scaling behavior for large synthetic TypeScript inputs.
File summaries
| File | Description |
|---|---|
| graphify/extract.py | Replaces quadratic match-vs-range filtering with an indexed lookup via binary search + prefix max ends. |
| tests/test_ts_import_type_arguments.py | Adds a scaling-based regression test for the TS import-type normalizer performance. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| # Sorted type-argument ranges prepared for lookup: (starts, spans, prefix_max_end). | ||
| # See _ts_build_range_index. | ||
| _TsRangeIndex = "tuple[list[int], list[tuple[int, int]], list[int]]" |
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. 1 change(s) tested, no difference found (not proven).
Graphify review — findings
Replaces the linear range scans in _normalize_ts_import_types with a sorted, binary-searchable index (_ts_build_range_index / _ts_ranges_containing), fixing the O(matches × ranges) blowup that pinned a worker at 100% CPU on large monorepo files mixing many generic calls and import(...) types (#3359). The lookup binary-searches to the last range starting at or before an offset and walks left, using a prefix-maximum of end offsets to stop early while still catching enclosing ranges. Adds a scaling regression test asserting that doubling the input stays under 3× the time rather than checking wall-clock.
No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1838 functions depend on the 264 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 544 callers, 43 callees - new:
_rebuild_code()— 115 callers, 51 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
_get_extractor()— 26 callers, 6 callees - new:
run_pipeline()— 8 callers, 13 callees - new:
collect_files()— 17 callers, 6 callees - …and 27 more — each is listed as a finding
Verification — 1838 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1673 function(s) in the blast radius were not formally verified this run
Formal verification
No difference found (not proven): No behavior difference found in \_normalize\_ts\_import\_types (not a proof).
The verifier ran both versions of \_normalize\_ts\_import\_types on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 35 more finding(s) on lines outside this diff (see the check run).
The alias was written as a quoted string, which makes it a `str` *value* rather than a type. Type checkers reject it: mypy reports "Variable _TsRangeIndex is not valid as a type" at both annotation sites, plus two cascading errors where the tuple loses its type and the unpacking/indexing in _ts_ranges_containing and _normalize_ts_import_types become untyped. `typing.get_type_hints` happened to resolve it at runtime because it evaluates the annotation string and then the alias string again, so nothing failed in practice -- but that is an accident of double evaluation, not a working alias. Unquote the right-hand side. The module already has `from __future__ import annotations`, so the annotations themselves need no quoting either. No behavior change.
|
Good catch — fixed in d902820.
The last two are cascading: the tuple loses its type, so the unpacking in Worth noting it did not fail at runtime — Unquoted the right-hand side. The module already has |
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. 1 change(s) tested, no difference found (not proven).
Graphify review — findings
Replaces the linear range scans in _normalize_ts_import_types with a sorted, binary-searchable index: _ts_build_range_index precomputes range starts plus a prefix-maximum of ends, and _ts_ranges_containing binary-searches to the last candidate start and walks left, using the prefix-max to stop early once no earlier range can reach the offset. This drops the previous O(matches × ranges) behaviour that pinned a worker at 100% CPU and never finished on large monorepo files with thousands of generic calls and import(...) types (#3359). Adds a scaling test asserting that doubling the input stays under 3× the time rather than quadrupling.
No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1838 functions depend on the 264 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 544 callers, 43 callees - new:
_rebuild_code()— 115 callers, 51 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
_get_extractor()— 26 callers, 6 callees - new:
run_pipeline()— 8 callers, 13 callees - new:
collect_files()— 17 callers, 6 callees - …and 27 more — each is listed as a finding
Verification — 1838 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1673 function(s) in the blast radius were not formally verified this run
Formal verification
No difference found (not proven): No behavior difference found in \_normalize\_ts\_import\_types (not a proof).
The verifier ran both versions of \_normalize\_ts\_import\_types on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 35 more finding(s) on lines outside this diff (see the check run).
perf_counter counts wall-clock, which includes time the process is preempted off-CPU during a busy full-suite run, inflating the larger measurement and flaking the ratio. process_time counts only CPU work, isolating the algorithmic scaling. Follow-up to #3402.
|
Shipped in v0.9.56 — landed on |
Fixes #3359.
Root cause
Not a linearity-unsafe regex. Every
re.sub/pattern in the extract path is linear — I benchmarkedtextwrap.shorten,_TS_IMPORT_CALL_RE,_VUE_SCRIPT_REand the JS rescue patterns and none backtrack. The_sreframes in the reporter'ssampleoutput are the innermost frames of a quadratic caller._normalize_ts_import_types(the #3154 TypeScriptimport(...)normalizer) filters each regex match against every tree-sittertype_argumentsbyte range with a nested linear scan, in both passes:That is O(matches × ranges), and
m.start()is re-evaluated insideany()for every range — hencere.Match.startdominating profiles andsampleappearing to indict a regex.Both counts scale with file size in TypeScript that mixes generic calls (
fn<A, B>(x)→ onetype_argumentsrange each) withimport(...)types — the shape of generated and barrel modules in a large monorepo.Measured on the filter alone:
import()sitesThis accounts for every symptom in the report:
packages/hangs while other top-level dirs and a 60-file repo finish.--max-workers 1stalls earlier (500/2469 vs ~2400/2470) — one worker reaches the pathological file sooner instead of ten racing past it first.--no-dedup, 0.9.53 and 0.9.54 all reproduce — the code is in the extract path and unchanged across those releases.Fix
Replace both scans with a binary search over start-sorted ranges plus a prefix-maximum of
endvalues, so a lookup touches nesting depth instead of every range.Type-argument ranges nest but never partially overlap, so the prefix maximum is a correct stop condition. The subtle part: a closed inner sibling must not terminate the leftward walk — a naive running-max break gets this wrong and silently drops enclosing ranges.
Normalization output is unchanged: byte-length and line-count preserving, identical masking decisions, and the runtime-vs-type disambiguation from 6baa5e0 still holds.
Also restores the emptiness guard in the second pass — the range list is now an index tuple, which is always truthy, so it checks the spans it holds rather than the container.
Verification
_normalize_ts_import_typesscaling is now ~2× per doubling (was ~4×), linear out to 32k imports.Full-suite note
A whole-suite comparison against unmodified
c9f9901shows 53 vs 61 failures, but the delta is environment flakiness on Windows, not this change:serve_http/install*/hooks/watch. None is inextract,resolution, or any TypeScript path, and no failing test references_normalize_ts_import_types,_ts_ranges_containing,_ts_build_range_index, or_ts_type_argument_ranges.