Repository navigation
Conversation
A \const X = require('./m')\ binding is an import, not a module-scope data
binding. The node it produced in the importing file shadowed the class the
required file exports, so \
ew X()\ bound to the local node and the
required class got no incoming edge.
![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.
No issues found by static checks — no coupling regressions or blocking issues in the code graph. That is not the same as safe to merge: see what was not checked.
Not checked: tests were not run; formal verification proved 0 of 1 changed function(s) (1 not verified).
Formal verification. PR-changed functions: 0/1 verified (0 proven, 0 may-equivalent, 0 distinguished) · 1 not verified (1 vacuous).
Not verified on this run: \_js\_extra\_walk (vacuous: never exercised).
Graphify review — findings
Stops creating a module-scope data node for CommonJS const X = require('./m') bindings in the JS extractor, since they are imports rather than data. Previously that local node shadowed the class the required file exports, so new X() resolved to the importing file's own node and the real class got no incoming calls edge (#4621). A regression test covers the case, checking that no local Logger node is emitted and that Watcher links to the exported Logger class.
No blocking issues surfaced. 3 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 954 functions depend on the 356 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
_js_extra_walk()— 2 callers, 12 callees
Verification — 954 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: 843 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
40 of 437 test file(s) selected (9%) 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_csharp_partial_classes.py— impacttests/test_dotnet.py— impacttests/test_ejs_extractor.py— impacttests/test_ext_tagged_id_clash.py— impacttests/test_extract.py— impacttests/test_extract_php_closures.py— impacttests/test_import_extension_resolution.py— impacttests/test_indirect_call_block_scoped_shadow.py— impacttests/test_indirect_dispatch.py— impacttests/test_indirect_dispatch_assign_return.py— impacttests/test_indirect_dispatch_getattr.py— impacttests/test_java_member_calls.py— impacttests/test_js_exported_scalar_bindings.py— impacttests/test_js_member_collisions.py— impacttests/test_js_require_binding_shadow.py— impact, changed-testtests/test_languages.py— impacttests/test_multilang.py— impacttests/test_php_unparsed_casts.py— impacttests/test_python_inherited_method_calls.py— impacttests/test_python_property_reads.py— impacttests/test_python_pytest_fixture_receivers.py— impacttests/test_python_underscore_resolution.py— impacttests/test_rationale.py— impacttests/test_ruby_resolution.py— impacttests/test_scala_context_bounds.py— impacttests/test_scala_given_instances.py— impacttests/test_scala_self_type.py— impacttests/test_scala_top_level_binding.py— impacttests/test_scala_type_definition.py— impacttests/test_svelte_extraction.py— impacttests/test_swift_computed_properties.py— impacttests/test_swift_protocol_requirements.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_vue_extraction.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 \_js\_extra\_walk.
The verifier did not have enough to check \_js\_extra\_walk, 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: no capturable inputs from the test suite; property tier: not verifiable: all 264 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)
· 1 more finding(s) on lines outside this diff (see the check run).
This branch has not been deployed
1 participant
What
A CommonJS
const X = require('./m')binding no longer creates a node in the importing file.The binding is an import, not a module-scope data binding, but the extractor treated it as one. The node it created shadowed the class the required file exports, so inside the importing file
new X()andclass A extends Xbound to the local node instead of the required one, and the required class got no incoming edge:The ESM default import (
import Logger from './logger') resolves to the class in./logger, and so does avarbinding — that declaration kind creates no node.constandletwere the odd ones out.Fix
graphify/extractors/engine.py: the module-scope data-binding branch skips a declarator whose value is arequire(...)call (_find_require_call, the same helper the require-import path uses). The require's ownimports_fromedge is unaffected — the import is still recorded, it just no longer mints a second node for the same name.tests/test_js_require_binding_shadow.py: a new regression test writessrc/logger.js(aLoggerclass exported throughmodule.exports) andsrc/use.js(const Logger = require('./logger')plusnew Logger('w')), then asserts thatsrc/use.js :: Logger, andsrc/logger.js :: Logger.Verification
I could not collect the graphify test suite here: the environment has no
tree_sitter, soimport graphify.extractfails at collection. This is a pre-existing environment gap, not related to the change; CI is the judge.What I could check here is the change itself: the branch that creates module-scope data nodes now skips a
require(...)value, and the helper it uses (_find_require_call) is the one the require-import path already relies on, so arequire('./m')in any position —const X = require('./m'),const x = require('./m').y,const { a } = require('./m')— is recognised the same way.AI authorship
Prepared with MiniMax M3.1 Flash Free via the opencode agent harness (
minimax-m3.1-flash-free).Closes #4621.