fix(ts): CALLS edge collector attribution for HOF/callback patterns (#1166)#1179
fix(ts): CALLS edge collector attribution for HOF/callback patterns (#1166)#1179abhigyanpatwari wants to merge 1 commit into
Conversation
…nction Two roots in `findEnclosingFunctionId` (parse-worker) and the parallel `findEnclosingFunction` (call-processor): A. `genericFuncName` scanned `arrow_function` / `function_expression` children for the first identifier and returned it. For unparenthesized arrows like `file => processFile(file)` the first identifier is the parameter `file`, so calls inside got attributed to a phantom `Function file` ID and emitted dangling CALLS edges that never showed up in `(:Function)-[:CALLS]->()` queries. B. `tsExtractFunctionName` only named arrows whose parent was `variable_declarator`. Object-property arrows like `addItem: (item) => set(...)` (Zustand stores, TanStack queryFn, React Context providers, config objects) live under a `pair`, so they were treated as anonymous. With no named ancestor up to the file, every call inside fell back to the File and became invisible to `context()` / `impact()`. Fix: - `genericFuncName` returns null for anonymous JS/TS function-likes — the language hook is authoritative. - `tsExtractFunctionName` resolves names from `pair` parents (property_identifier / string keys; computed keys stay anonymous). - Mirror the new shape in `TYPESCRIPT_QUERIES` / `JAVASCRIPT_QUERIES` / the scope-resolution query so pair-with-arrow becomes a Function declaration node — call sourceIds resolve to a real graph node. Adds 18 unit tests pinning attribution and definition behaviour for plain helpers, `arr.map(x => fn(x))`, Promise constructor callbacks, Zustand-style nested HOFs, TanStack query factories, string-keyed pairs, and computed-key anonymity. Fixes #1166 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@abhigyanpatwari hi man appreciate your job! Could you also check my pr for this problem? #1175 i think our combined approach would be best |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 93a0be2. Configure here.
| // Object property pair: `{ addItem: (item) => ... }`. | ||
| // tree-sitter-typescript uses `pair`; tree-sitter-javascript also exposes | ||
| // `pair`. (Older grammars used `property_assignment`; we accept both.) | ||
| if (parent.type === 'pair' || parent.type === 'property_assignment') { |
There was a problem hiding this comment.
Queries missing property_assignment patterns cause dangling edges
Low Severity
tsExtractFunctionName accepts property_assignment parents (in addition to pair) and will return a function name for arrows under them, but the tree-sitter query patterns in TYPESCRIPT_QUERIES, JAVASCRIPT_QUERIES, and the scope-resolution query only add pair-based @definition.function / @declaration.function patterns — no property_assignment equivalents. If a grammar produces property_assignment nodes, findEnclosingFunctionId would construct a Function ID that has no corresponding definition-phase node, producing exactly the dangling CALLS edges this PR aims to eliminate.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 93a0be2. Configure here.
CI Report❌ Some checks failed Pipeline Status
Test Results
✅ All 7686 tests passed 1 test(s) skipped — expand for details
Code CoverageTests
📋 View full run · Generated by CI |
|
Hey @ReidenXerx , thanks for flagging your PR, agreed, your scope-resolution fixes (pass2AttachDeclarations anchor placement, JSX-as-CALLS, the Variable exclusion in isCallerAnchorLabel, and the findExportByName callable preference) are the right architectural treatment for the registry-primary path. Mine is narrower. Would you mind pulling this branch (fix/issue-1166-calls-edges) into #1175 so yours becomes the umbrella PR? I'll |
ofc my man i will do |
|
@abhigyanpatwari merged your Feel free to close this PR whenever — #1175 supersedes it. Thanks for the parallel work and the clear factoring of Bug A / Bug B; made the consolidation trivial. |
|
Let's close this PR @abhigyanpatwari as @ReidenXerx merged your changes and we can proceed with his PR. |
Single line-length fix in `gitnexus/src/core/ingestion/languages/typescript.ts` flagged by `quality / format` CI on commit ef96603. The unformatted block came from the merge of upstream PR abhigyanpatwari#1179 (`fix/issue-1166-calls-edges`) where the `pair`-with-arrow / `pair`-with-string-key handling was added; prettier wanted the `.find` callback inlined onto a single line. No behavior change. Pre-commit hook would have caught this locally if the husky postinstall step had been able to write `.git/config` on this dev machine. Made-with: Cursor
…r arrow Addresses the medium-severity finding in @abhigyanpatwari's review of abhigyanpatwari#1175: the four `pair`-with-arrow patterns in `query.ts` anchored `@declaration.function` on the outer `pair` node instead of the inner `arrow_function` / `function_expression`. For multi-action object literals like Zustand's persist((set) => ({ addItem: (item) => doA(item), removeItem: (item) => doB(item), fetchData: () => doC(), })) `pass2AttachDeclarations.atPosition(pair.startLine, pair.startCol)` resolved to the *parent* `(set) => ({...})` callback's scope (because the pair node starts at the property-key token, before the inner arrow's `@scope.function` range). All three pair-function defs landed in the same parent's `ownedDefs`, and `resolveCallerGraphId.ownedDefs.find(...)` returned the FIRST one — `addItem` — for every walk-up. Calls inside `removeItem` and `fetchData` mis-attributed to `addItem`; those two functions had zero outgoing CALLS edges in the registry-primary path. Single-pair fixtures (`bump` in `store.ts`, `queryFn` in `query-hook.ts`) masked the defect because there is no ambiguity when only one Function-like def lives in the parent's `ownedDefs` — `find()` is deterministic over a single-element set. Fix: move the `@declaration.function` anchor from the outer `pair` to the inner `arrow_function` / `function_expression`, mirroring the `lexical_declaration` patterns above (`const fn = () => {}`). The def then lands in the arrow's own scope's `ownedDefs`, the `rangesEqual(anchor.range, innermost.range)` auto-hoist promotes the binding to the parent scope (so importers + lookups still find the name in the surrounding scope), and each pair-arrow becomes an independent caller anchor in the walk. Tests: * Updated `useFeature → fetchData` expectation to `queryFn → fetchData` in `typescript-hof-callbacks.test.ts`. The new attribution is structurally correct: `fetchData()` is called from inside the named pair-arrow `queryFn: () => fetchData()`. The pre-fix expectation only worked because the pair-pattern bug rerouted the walk past the syntactic owner. * Added `multi-action-store.ts` fixture with three pair-arrows (`addItem` / `removeItem` / `fetchData`) plus three top-level call targets (`doA` / `doB` / `doC`). Four new tests pin per-action attribution: positive (each action calls its own target), negative (no sibling leakage), exact-set (the full pair set is what we expect), and the regression fingerprint (`addItem → doB` MUST be empty). Validation: * `REGISTRY_PRIMARY_TYPESCRIPT=1 vitest run` on typescript-hof-callbacks (12 tests, +4 new), typescript-jsx-as-call (7), typescript (236), typescript-finalize, typescript-cross-file-imports, call-attribution-issue-1166 (18), all scope-resolution unit suites: 886/886 pass on registry-primary AND legacy DAG paths. * Legacy DAG attribution was already correct via @abhigyanpatwari's `tsExtractFunctionName` pair-parent handling (abhigyanpatwari#1179, merged into this PR earlier); this fix brings the registry-primary path to the same behavior, restoring parity for multi-action objects. * `npx prettier --check .`, `tsc --noEmit`, and `eslint` clean on the three modified/added files. Made-with: Cursor
3 participants
Summary
Fixes #1166. Two roots in the call-attribution code path (
findEnclosingFunctionIdinparse-worker.ts+ the parallelfindEnclosingFunctionincall-processor.ts) caused ~75% of CALLS edges to disappear in HOF / callback patterns:genericFuncNamescannedarrow_function/function_expressionchildren for the first identifier and returned it. For unparenthesized arrows likefile => processFile(file)the first identifier is the parameterfile, so calls inside got attributed to a syntheticFunction fileand emitted dangling CALLS edges. The user's(:Function)-[:CALLS]->()queries never saw them.tsExtractFunctionNameonly named arrows whose parent wasvariable_declarator.addItem: (item) => set(...)(Zustand actions, TanStackqueryFn, React Context providers, config objects) lives under apair— treated as anonymous. With no named ancestor up to the file, calls inside fell back to the File and were invisible tocontext()/impact().Fix:
genericFuncNamereturnsnullfor anonymous JS/TS function-likes — the language hook is authoritative.tsExtractFunctionNameresolves names frompairparents (property_identifier/stringkeys; computed keys stay anonymous).TYPESCRIPT_QUERIES/JAVASCRIPT_QUERIES/ the scope-resolution query so pair-with-arrow becomes a@definition.functionnode — call sourceIds resolve to a real graph node.Why this fixes the user's data
For the issue's
file-upload.tsrepro:processFile → fileToDataUrl(was missed viaawait fileToDataUrl(file)inside an outer arrow) → now captured.processSelectedFiles → processFileinsidePromise.all(files.map(file => processFile(file)))→ was attributed to phantomFunction file; now correctly attributed toprocessSelectedFiles.For Zustand-style stores:
addItem,fetchData, etc. become Function nodes via the newpaircapture pattern.set,doSomething,api.fetch, …) attribute to the action name instead of the file.The reproducer in the issue (Zustand
useStore = create<State>()(devtools(persist((set, get) => ({ addItem: ..., fetchData: ... }))))) now resolves all expected edges; calls in the top-level expression (create/devtools/persist) correctly stay file-attributed because they live in the value ofuseStore, not inside a function body.Test plan
test/unit/call-attribution-issue-1166.test.tscovering Bug A (genericFuncNamefor anonymous arrows,.map(x => fn(x)), parenthesized callbacks), Bug B (object-property arrows, function expressions, string keys, computed-key anonymity, Zustand and TanStack patterns), and regression guards (plain helpers, Promise constructor callbacks, top-level module-init expressions).@definition.functioncaptures so call sourceIds resolve to real Function nodes.tsc --noEmitclean.Files
src/core/ingestion/utils/ast-helpers.ts—genericFuncNameearly-return forarrow_function/function_expression.src/core/ingestion/languages/typescript.ts—tsExtractFunctionNamehandlespairparents.src/core/ingestion/tree-sitter-queries.ts— pair-with-arrow patterns inTYPESCRIPT_QUERIESandJAVASCRIPT_QUERIES.src/core/ingestion/languages/typescript/query.ts— same patterns mirrored in the scope-resolution query.test/unit/call-attribution-issue-1166.test.ts— new test file.🤖 Generated with Claude Code
Note
Medium Risk
Updates TypeScript/JavaScript function-name inference and tree-sitter capture patterns, which can change how call graphs and symbol IDs are generated across many files; mistakes would mainly impact analysis accuracy rather than runtime security.
Overview
Fixes TS/JS call attribution in higher-order-function and callback-heavy code by stopping
genericFuncNamefrom inventing names forarrow_function/function_expressionand instead relying on declarative context.Extends TypeScript’s
extractFunctionNameto name arrow/functions assigned to object properties (e.g.{ addItem: (...) => ... }, including string keys) while leaving computed keys anonymous, and mirrors this in bothTYPESCRIPT_QUERIES/JAVASCRIPT_QUERIESand the TypeScript scope query so these property functions are emitted as real@definition.functionnodes.Adds a focused regression test suite for issue #1166 covering the previously-missed patterns (Promise/map callbacks, Zustand/TanStack-style objects) and guarding against phantom parameter-named functions.
Reviewed by Cursor Bugbot for commit 93a0be2. Bugbot is set up for automated code reviews on this repo. Configure here.