fix: indirect JS/TS call resolution for dispatch tables and callbacks#1985
Open
Tefa-hup wants to merge 1 commit into
Open
fix: indirect JS/TS call resolution for dispatch tables and callbacks#1985Tefa-hup wants to merge 1 commit into
Tefa-hup wants to merge 1 commit into
Conversation
- Fix indirect function reference resolution in JavaScript/TypeScript for: - Callbacks passed by name in arrays/objects (dispatch tables) - Assignment/return of function values - getAttr reflective calls - Added regression tests in test_indirect_dispatch.py - The fix was verified against the full test suite (only unrelated pre-existing failures) Closes #xxxx (لو هتعمل Issue)
Tefa-hup
marked this pull request as draft
July 18, 2026 02:10
Tefa-hup
marked this pull request as ready for review
July 18, 2026 02:12
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
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.
Ready-to-submit GitHub issue (or PR description) for Graphify-Labs/graphify
Repo: https://github.com/Graphify-Labs/graphify (redirects from https://github.com/safishamsi/graphify)
Diff is against
main@8cee776cee7d5dc3279085797bde2d2dcce21e36(2026-07-17).I don't have push/fork access to this repo from my GitHub account, so I could not open
this as an issue or PR directly. Steps to submit it yourself:
git apply graphify-js-indirect-call-fix.diff(from the repo root — the diff alreadyhas correct paths for
graphify/extractors/engine.pyandtests/test_indirect_dispatch.py).Graphify-Labs/graphify:mainusing thetitle/body below (or open it as an issue first if you'd rather report it without a PR).
Title
JS/TS AST extractor: two false-positive
indirect_calledge sources (for-of destructuring,module.exports = {...})Body
Found while auditing a
graphifyrun against a real Node.js codebase: severalindirect_calledges in the graph did not correspond to any real call in the source. Root-caused to two distinct bugs ingraphify/extractors/engine.py, both in the JS/TS indirect-dispatch resolution path.Bug 1 —
for (const [a, b] of x)loop variables aren't recognized as local bindings_js_local_bound_names'swalk()(around L1151) only special-casesvariable_declaratornodes to collect bound names. But afor_in_statement's loop variable(s) live directly in itsleftfield — never wrapped in avariable_declarator:So a destructured loop var like
countinfor (const [file, count] of Object.entries(x))is never added to the function's local-shadow set. A later bare read of it (if (count >= 3)) then gets treated as a reference to a same-named module-level function, and if a same-named callable exists ANYWHERE in the corpus, the cross-file resolver (extract.py~L4905-4927) manufactures a phantomindirect_calledge to it — confidence_score 0.8, i.e. presented as fairly confident.Concretely: a file with
for (const [file, count] of editCounts) { if (count >= 3) {...} }and an unrelatedJsonFileBackend.count()method defined elsewhere in the repo produced a graph edge claiming the first function calls the second. It doesn't — there's no textual reference between them at all.Bug 2 —
module.exports = { a, b, c }misidentified as a dispatch tableThe module-level dispatch-table scanner (
_scan_js_module_dispatch, ~L4386) treats any top-levelobject/arrayliteral as a potential handler-registry, via_js_dispatch_value_idents. The standard CommonJS export idiommodule.exports = { init, consolidate, stats };has exactly that shape (an object of bare/shorthand identifiers), so every plain CommonJS module hits this and gets a spuriousindirect_calledge from the file node to each of its own exported functions — misrepresenting an export list as a runtime dispatch table.Fix
Two changes, both scoped to
graphify/extractors/engine.py(see attached diff):_js_local_bound_names.walk(), add a branch forfor_in_statementthat pulls identifiers out of itsleftfield via the existing_js_collect_pattern_identshelper (handles both destructured and bare-identifier loop vars)._scan_js_module_dispatch, detect when anassignment_expression's LHS is the whole exports surface (module.exportsor bareexports— notmodule.exports.NAME/exports.NAME, which can legitimately hold a real dispatch table under a named property) and skip dispatch-scanning that object directly, while still walking its children so a genuine nested table under a named property is still found.Verification
extract()on the four files that surfaced this in a real repo:indirect_calledges went from 9 (all false positives) to 0, with zero change in node count and only those 9 phantom edges removed from the edge count.tests/test_indirect_dispatch.py(for-of destructured var, for-in bare var,module.exports = {...}exemption, bareexports = {...}exemption, and a control test confirmingmodule.exports.NAME = {...}/ named sub-property assignment still resolves as a real dispatch table — the exemption is intentionally narrow).tests/test_indirect_dispatch.py: 31/31 pass with the patch.test_obsidian_filename_cap,test_ollama_retry_cap,test_skillgen,test_watch) reproduce identically with the patch reverted (confirmed viagit stash) — unrelated to this change, likely Windows/environment-specific.Files in this folder
graphify-js-indirect-call-fix.diff— the patch (engine.py fix + new tests), apply withgit applyfrom repo root.engine_patched.py— the full post-patchgraphify/extractors/engine.pyfor reference/diffing.