fix(lua): emit an imports edge for bare require() statements (#3320) - #3448
fix(lua): emit an imports edge for bare require() statements (#3320)#3448shivamtiwari3 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 3 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Adds recognition of bare Lua require("m") statements as import edges, so a Neovim init.lua written as a list of unassigned requires now produces imports edges instead of none — previously only local x = require("m") was matched. Introduces a new import_call_types config category that dispatches import-carrying call nodes to the import handler without the early return that import_types performs, so those nodes still flow into class/function/call dispatch and Lua call extraction is unaffected; it runs both at module level and inside function bodies, catching lazy requires at any depth. Module names are read via the call's callee and string-content child rather than a regex scan, keeping nested forms like require("a").setup(require("b")) from double-reporting and stripping quotes/brackets from the id.
Worth a look
- require call inside variable_declaration also matched as bare function_call causing duplicate edges —
graphify/extract.py:1062· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Bare Lua require import edge emitted twice (top-level walk + function-body walk) —
graphify/extractors/engine.py:5316· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Adding import_call_types shifts LanguageConfig positional constructor arguments —
graphify/extractors/models.py:21· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2351 functions depend on the 680 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 572 callers, 43 callees - new:
_rebuild_code()— 115 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:
dispatch_command()— 2 callers, 124 callees - new:
extract_objc()— 27 callers, 9 callees - new:
_get_extractor()— 26 callers, 6 callees - …and 39 more — each is listed as a finding
Verification — 2351 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: 2186 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
263 of 263 test file(s) selected (100%) via static blast radius.
Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.
tests/test_affected_cli.py— full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_agents_platform.py— full-run-safetytests/test_analyze.py— full-run-safetytests/test_anthropic_custom_endpoint.py— full-run-safetytests/test_antigravity_install.py— full-run-safetytests/test_apm_fallback_version.py— full-run-safetytests/test_architecture_doc.py— full-run-safetytests/test_astro_extraction.py— impact, full-run-safetytests/test_astro_import_ids.py— impact, full-run-safetytests/test_atomic_canvas_export.py— full-run-safetytests/test_atomic_version_stamp.py— full-run-safetytests/test_atomic_writes.py— full-run-safetytests/test_backend_extras.py— full-run-safetytests/test_benchmark.py— full-run-safetytests/test_benchmark_raw_graph.py— full-run-safetytests/test_build.py— impact, full-run-safetytests/test_build_merge_hyperedges_and_prune.py— full-run-safetytests/test_build_merge_shrink_guard.py— full-run-safetytests/test_builtin_global_type_refs.py— impact, full-run-safetytests/test_cache.py— full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— full-run-safetytests/test_carried_hyperedge_remap.py— full-run-safetytests/test_case_sensitive_resolution.py— impact, full-run-safetytests/test_charmap_encoding.py— full-run-safetytests/test_chunking.py— full-run-safetytests/test_cjs_module_extension.py— impact, full-run-safetytests/test_claude_cli_backend.py— full-run-safetytests/test_claude_md.py— full-run-safetytests/test_cli_broken_pipe.py— full-run-safetytests/test_cli_export.py— full-run-safetytests/test_cli_help.py— full-run-safetytests/test_cluster.py— full-run-safetytests/test_codebuddy.py— full-run-safetytests/test_community_hub_labels.py— full-run-safetytests/test_community_labels_skill.py— full-run-safetytests/test_confidence.py— full-run-safetytests/test_corrupt_graph_json.py— full-run-safetytests/test_cpp_nested_and_cli.py— impact, full-run-safetytests/test_cpp_objc_cross_file_calls.py— impact, full-run-safetytests/test_cpp_preprocess.py— full-run-safetytests/test_cross_extension_reexport_self_cycle.py— impact, full-run-safetytests/test_cross_language_call_resolution.py— impact, full-run-safetytests/test_cross_repo_member_calls.py— impact, full-run-safetytests/test_cross_repo_shared_types.py— full-run-safetytests/test_csharp_call_site_generic_args.py— impact, full-run-safetytests/test_csharp_enum_members.py— impact, full-run-safetytests/test_csharp_field_generic_args.py— impact, full-run-safetytests/test_csharp_generic_callsites.py— impact, full-run-safety- … and 213 more
non-code file(s) changed (
CHANGELOG.md) → running the full suite for safety (a code graph can't see config/fixture/data deps)
changed code file(s) with no mapped test (
CHANGELOG.md) — a coverage gap or a missing link — running the full suite rather than only the selected tests
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 \_import\_lua.
The verifier did not have enough to check \_import\_lua, 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: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, 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 `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 grounded finding(s) anchored inline below; 46 more finding(s) on lines outside this diff (see the check run).
| @@ -3274,6 +3274,14 @@ def ensure_named_node(name: str, line: int) -> str: | |||
| def walk(node, parent_class_nid: str | None = None) -> None: | |||
There was a problem hiding this comment.
walk()
fans out to 58 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
…y-Labs#3320) `_import_lua` was only reachable from `variable_declaration`, so `local x = require("m")` produced an `imports` edge and a bare `require("m")` statement produced nothing. tree-sitter-lua parses the bare form as a plain `function_call`, and the whole Neovim config idiom require("config.lazy") require("config.options") is written that way — a 14-file LazyVim config came out with 13 nodes and 2 edges, none of them imports. `function_call` cannot simply be added to `import_types`: that branch returns without walking children, which would have dropped every Lua `calls` edge. Add `import_call_types` instead — dispatched to the same handler, but falling through so class/function/call dispatch still sees the node. The module-level walk and the call walk both consult it, so a lazy require inside a function body is captured too, matching what `_require_imports_js` already does for CommonJS. `_import_lua` reads the module from the call's own callee and arguments rather than regex-scanning the node text, so a nested `require("a").setup(require("b"))` reports each module once instead of attributing the inner one twice.
d3e93ee to
f3d49a2
Compare
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Adds import extraction for bare Lua require("m") statements — the unassigned form every Neovim init.lua uses, which tree-sitter parses as a plain function_call and which previously yielded no imports edge (only local x = require(...) was recognized). A new import_call_types config field dispatches such call nodes to the import handler without the early return import_types performs, so the walk still descends into the call for class/function/call extraction; _lua_bare_require_module matches on the callee require and pulls the module name from the string node, so nested forms like require("a").setup(require("b")) report each genuine require exactly once. Handles both the top-level and lazy-inside-a-function-body cases, and supports the require "m" / require [[m]] spellings.
Worth a look
- Bare require import emitted twice: dispatched in both module-level and function-body walk —
graphify/extractors/engine.py:5314· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- LanguageConfig positional constructor arguments shifted —
graphify/extractors/models.py:26· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2351 functions depend on the 680 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 572 callers, 43 callees - new:
_rebuild_code()— 115 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:
dispatch_command()— 2 callers, 124 callees - new:
extract_objc()— 27 callers, 9 callees - new:
_get_extractor()— 26 callers, 6 callees - …and 39 more — each is listed as a finding
Verification — 2351 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: 2186 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
263 of 263 test file(s) selected (100%) via static blast radius.
Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.
tests/test_affected_cli.py— full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_agents_platform.py— full-run-safetytests/test_analyze.py— full-run-safetytests/test_anthropic_custom_endpoint.py— full-run-safetytests/test_antigravity_install.py— full-run-safetytests/test_apm_fallback_version.py— full-run-safetytests/test_architecture_doc.py— full-run-safetytests/test_astro_extraction.py— impact, full-run-safetytests/test_astro_import_ids.py— impact, full-run-safetytests/test_atomic_canvas_export.py— full-run-safetytests/test_atomic_version_stamp.py— full-run-safetytests/test_atomic_writes.py— full-run-safetytests/test_backend_extras.py— full-run-safetytests/test_benchmark.py— full-run-safetytests/test_benchmark_raw_graph.py— full-run-safetytests/test_build.py— impact, full-run-safetytests/test_build_merge_hyperedges_and_prune.py— full-run-safetytests/test_build_merge_shrink_guard.py— full-run-safetytests/test_builtin_global_type_refs.py— impact, full-run-safetytests/test_cache.py— full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— full-run-safetytests/test_carried_hyperedge_remap.py— full-run-safetytests/test_case_sensitive_resolution.py— impact, full-run-safetytests/test_charmap_encoding.py— full-run-safetytests/test_chunking.py— full-run-safetytests/test_cjs_module_extension.py— impact, full-run-safetytests/test_claude_cli_backend.py— full-run-safetytests/test_claude_md.py— full-run-safetytests/test_cli_broken_pipe.py— full-run-safetytests/test_cli_export.py— full-run-safetytests/test_cli_help.py— full-run-safetytests/test_cluster.py— full-run-safetytests/test_codebuddy.py— full-run-safetytests/test_community_hub_labels.py— full-run-safetytests/test_community_labels_skill.py— full-run-safetytests/test_confidence.py— full-run-safetytests/test_corrupt_graph_json.py— full-run-safetytests/test_cpp_nested_and_cli.py— impact, full-run-safetytests/test_cpp_objc_cross_file_calls.py— impact, full-run-safetytests/test_cpp_preprocess.py— full-run-safetytests/test_cross_extension_reexport_self_cycle.py— impact, full-run-safetytests/test_cross_language_call_resolution.py— impact, full-run-safetytests/test_cross_repo_member_calls.py— impact, full-run-safetytests/test_cross_repo_shared_types.py— full-run-safetytests/test_csharp_call_site_generic_args.py— impact, full-run-safetytests/test_csharp_enum_members.py— impact, full-run-safetytests/test_csharp_field_generic_args.py— impact, full-run-safetytests/test_csharp_generic_callsites.py— impact, full-run-safety- … and 213 more
non-code file(s) changed (
CHANGELOG.md) → running the full suite for safety (a code graph can't see config/fixture/data deps)
changed code file(s) with no mapped test (
CHANGELOG.md) — a coverage gap or a missing link — running the full suite rather than only the selected tests
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 \_import\_lua.
The verifier did not have enough to check \_import\_lua, 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: not verifiable: all 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, 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 `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 grounded finding(s) anchored inline below; 46 more finding(s) on lines outside this diff (see the check run).
| @@ -3274,6 +3274,14 @@ def ensure_named_node(name: str, line: int) -> str: | |||
| def walk(node, parent_class_nid: str | None = None) -> None: | |||
There was a problem hiding this comment.
walk()
fans out to 58 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
Fixes #3320.
The bug
_import_luais only reachable from Luavariable_declarationnodes, solocal x = require("m")yields animportsedge and a barerequire("m")statement yields nothing. tree-sitter-lua parses the bare form as a plainfunction_call.That is the form every Neovim config is written in:
so the reporter's 14-file LazyVim config produced 13 nodes and 2 edges — both
contains, zero imports.Why
import_typescould not just be widenedwalk()handlesimport_typesand then returns, skipping the node's children. Addingfunction_callthere would have routed every Lua call through the import handler and dropped every Luacallsedge.So this adds
LanguageConfig.import_call_types: node types dispatched toimport_handlerwithout the early return, leaving class/function/call dispatch intact. Lua sets it to{"function_call"}.Both walks consult it:
walk()— top-level bare requires;walk_calls()— a lazy require inside a function body, which is equally common in Neovim config. This mirrors the existing_require_imports_jsblock directly above it, which already handles CommonJS lazy requires at arbitrary depth for the same reason.Precision
For the call form,
_import_luareads the module name from the call's own callee (identifier==require) and itsargumentsstring, instead of regex-scanning the node text. That keeps a nestedrequire("a").setup(require("b"))from attributing the inner module twice: the outer call's callee is an index expression, so only the two genuinerequirecalls match, once each.require("m"),require "m"andrequire [[m]]are all covered.The
variable_declarationpath is untouched, so existing graphs do not change.Tests
tests/test_lua_bare_require.py, 8 cases: the bare form emits the edge; bare and assigned forms produce identical targets; the no-parens form; every entry in a list of requires is reported; a lazy require in a function body; a non-require call emits nothing; call extraction still works alongside a bare require; and the edge is emitted exactly once (the two walks do not double-count).Full suite: 5501 passed, 3 pre-existing failures unrelated to this change (
test_extract_code_only_cli, twotest_ollamabackend-detection tests — all environment-dependent and failing identically onv8without this patch).🤖 Generated with Claude Code