fix(resolution): decouple the JS/TS family from the AST-cache-bypass set (#3326) - #3537
abhay-codes07 wants to merge 1 commit into
Conversation
![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).
Graphify review — findings
Decouples JS/TS language-family membership from AST-cache policy by introducing _JS_FAMILY_SUFFIXES as the single source of truth for "is this a JS/TS file", with _JS_CACHE_BYPASS_SUFFIXES now a separate copy (not an alias) so the two can diverge. Points _collect_js_symbol_resolution_facts at the new family constant so which files produce INFERRED resolution edges no longer moves when the cache-bypass set changes. Adds tests asserting the two sets are distinct mutable objects, that resolution gates on the family constant, and that cross-file .ts import resolution still works.
No blocking issues surfaced.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1913 functions depend on the 170 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 591 callers, 44 callees - new:
_rebuild_code()— 116 callers, 51 callees - new:
_extract_generic()— 18 callers, 26 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
_resolve_js_module_path()— 34 callers, 9 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
extract_objc()— 27 callers, 9 callees - …and 35 more — each is listed as a finding
Verification — 1913 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: 842 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
35 of 276 test file(s) selected (13%) via static blast radius.
tests/test_astro_extraction.py— impacttests/test_build.py— impacttests/test_cjs_module_extension.py— impacttests/test_cpp_nested_and_cli.py— impacttests/test_dotnet.py— impacttests/test_extract.py— impacttests/test_forwarding_review_findings.py— impacttests/test_import_extension_resolution.py— impacttests/test_indirect_dispatch.py— impacttests/test_indirect_dispatch_assign_return.py— impacttests/test_indirect_dispatch_getattr.py— impacttests/test_js_exported_scalar_bindings.py— impacttests/test_js_family_cache_decoupled.py— changed-testtests/test_languages.py— impacttests/test_multilang.py— impacttests/test_package_json_subpath_imports.py— impacttests/test_pascal.py— impacttests/test_pascal_resolution.py— impacttests/test_phantom_external_import.py— impacttests/test_python_import_resolution.py— impacttests/test_python_parse_memoization.py— impacttests/test_python_underscore_resolution.py— impacttests/test_rationale.py— impacttests/test_resolve_memoization.py— impacttests/test_ruby_resolution.py— impacttests/test_scala_self_type.py— impacttests/test_source_key_memoization.py— impacttests/test_src_layout_import_resolution.py— impacttests/test_swift_computed_properties.py— impacttests/test_trailing_newline_not_a_syntax_error.py— impacttests/test_ts_new_expression_calls.py— impacttests/test_typescript_module_extensions.py— impacttests/test_unmapped_at_alias_resolution.py— impacttests/test_vue_extraction.py— impacttests/test_walk_python_tree_iterative.py— impact
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
· 1 grounded finding(s) anchored inline below; 42 more finding(s) on lines outside this diff (see the check run).
| @@ -1891,7 +1891,7 @@ def _ts_walk_class_members(class_node, source: bytes, path: Path, class_nid: str | |||
| def _collect_js_symbol_resolution_facts(paths: list[Path], facts: _SymbolResolutionFacts) -> None: | |||
There was a problem hiding this comment.
_collect_js_symbol_resolution_facts()
fans out to 26 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
…set (Graphify-Labs#3326) _collect_js_symbol_resolution_facts selected which files go through JS/TS cross-file resolution by membership in _JS_CACHE_BYPASS_SUFFIXES — a cache policy constant. The two answer different questions ("is this JS/TS?" vs "should this skip the AST cache?") but shared one set, so a change made for caching reasons silently changed which files produced INFERRED edges, with nothing at either call site to reveal it. Introduce _JS_FAMILY_SUFFIXES as the language-membership source of truth and gate resolution on it; keep _JS_CACHE_BYPASS_SUFFIXES as an independent cache policy (a copy, so the two move independently). Values coincide today, so behavior is unchanged — this removes the hidden coupling and unblocks making JS/TS cacheable without dropping resolution edges. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q
fd3864c to
cf97114
Compare
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. No changes could be formally verified in this run.
Graphify review — findings
Decouples JS/TS language-family membership from AST-cache policy by introducing _JS_FAMILY_SUFFIXES as the single source of truth for "is this a JS/TS file", with _JS_CACHE_BYPASS_SUFFIXES now a separate copy that can diverge without silently changing which files produce INFERRED resolution edges. Retargets the file filter in _collect_js_symbol_resolution_facts to key on the family constant instead of the cache-bypass set. Adds tests asserting the two sets are independent objects, that resolution selects by family and never references the cache constant, and that .ts cross-file import resolution still works end to end.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1913 functions depend on the 170 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 591 callers, 44 callees - new:
_rebuild_code()— 116 callers, 51 callees - new:
_extract_generic()— 18 callers, 26 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
_resolve_js_module_path()— 34 callers, 9 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
extract_objc()— 27 callers, 9 callees - …and 35 more — each is listed as a finding
Verification — 1913 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: 842 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
35 of 276 test file(s) selected (13%) via static blast radius.
tests/test_astro_extraction.py— impacttests/test_build.py— impacttests/test_cjs_module_extension.py— impacttests/test_cpp_nested_and_cli.py— impacttests/test_dotnet.py— impacttests/test_extract.py— impacttests/test_forwarding_review_findings.py— impacttests/test_import_extension_resolution.py— impacttests/test_indirect_dispatch.py— impacttests/test_indirect_dispatch_assign_return.py— impacttests/test_indirect_dispatch_getattr.py— impacttests/test_js_exported_scalar_bindings.py— impacttests/test_js_family_cache_decoupled.py— changed-testtests/test_languages.py— impacttests/test_multilang.py— impacttests/test_package_json_subpath_imports.py— impacttests/test_pascal.py— impacttests/test_pascal_resolution.py— impacttests/test_phantom_external_import.py— impacttests/test_python_import_resolution.py— impacttests/test_python_parse_memoization.py— impacttests/test_python_underscore_resolution.py— impacttests/test_rationale.py— impacttests/test_resolve_memoization.py— impacttests/test_ruby_resolution.py— impacttests/test_scala_self_type.py— impacttests/test_source_key_memoization.py— impacttests/test_src_layout_import_resolution.py— impacttests/test_swift_computed_properties.py— impacttests/test_trailing_newline_not_a_syntax_error.py— impacttests/test_ts_new_expression_calls.py— impacttests/test_typescript_module_extensions.py— impacttests/test_unmapped_at_alias_resolution.py— impacttests/test_vue_extraction.py— impacttests/test_walk_python_tree_iterative.py— impact
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Formal verification
Could not verify: Could not verify \_collect\_js\_symbol\_resolution\_facts.
The verifier did not have enough to check \_collect\_js\_symbol\_resolution\_facts, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `facts` is annotated `_SymbolResolutionFacts` — outside the synthesizable primitive/collection set
· 1 grounded finding(s) anchored inline below; 42 more finding(s) on lines outside this diff (see the check run).
| @@ -1891,7 +1891,7 @@ def _ts_walk_class_members(class_node, source: bytes, path: Path, class_nid: str | |||
| def _collect_js_symbol_resolution_facts(paths: list[Path], facts: _SymbolResolutionFacts) -> None: | |||
There was a problem hiding this comment.
_collect_js_symbol_resolution_facts()
fans out to 26 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
Closes #3326.
The bug
_JS_CACHE_BYPASS_SUFFIXESwas doing two unrelated jobs:extract.pyuses it to decide whether a file skips the AST cache (a cache policy).resolution.py's_collect_js_symbol_resolution_factsuses it to decide which files go through JS/TS cross-file symbol resolution (a language-membership fact).Because both read the one set, a change made for caching reasons silently changes which files produce INFERRED edges — and nothing at either call site reveals the link. The reporter hit this on a ~2,300-file pnpm monorepo: emptying the set while testing JS/TS caching dropped graph edges from 21,969 to 17,019 with no error and the node count barely moving.
The change
Introduce
_JS_FAMILY_SUFFIXESas the single source of truth for "is this a JS/TS-family file", and gate resolution on it._JS_CACHE_BYPASS_SUFFIXESstays as an independent cache policy — a copy, not an alias, so the two sets move independently. The values coincide today, so behavior is unchanged; this removes the hidden coupling (exactly the reporter's suggested fix) and unblocks a real follow-up — making JS/TS files cacheable — without silently losing resolution edges.Tests
tests/test_js_family_cache_decoupled.py— the family and cache-bypass sets are distinct objects and mutating one does not leak into the other; the resolution collector references_JS_FAMILY_SUFFIXESand not the cache-bypass constant; a.tscross-file import call still resolves (behavior unchanged); and the family contains the JS/TS suffixes. The three decoupling assertions fail onv8; the JS/TS + resolution suites are otherwise unchanged.🤖 Generated with Claude Code
https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q