Skip to content

fix(typescript): capture missed CALLS edges from HOF callbacks and JSX#1175

Merged
abhigyanpatwari merged 7 commits into
abhigyanpatwari:mainfrom
ReidenXerx:fix/typescript-hof-callbacks-and-jsx-as-call
Apr 30, 2026
Merged

fix(typescript): capture missed CALLS edges from HOF callbacks and JSX#1175
abhigyanpatwari merged 7 commits into
abhigyanpatwari:mainfrom
ReidenXerx:fix/typescript-hof-callbacks-and-jsx-as-call

Conversation

@ReidenXerx

@ReidenXerx ReidenXerx commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Fixes #1166.

Summary

This is the umbrella PR for issue #1166Function nodes had zero outgoing CALLS edges for ~73% of all TypeScript functions in real React + TanStack + Zustand codebases. The umbrella consolidates two complementary fix-sets that hit the same root cause from different angles:

  • Registry-primary scope-resolution (originally PR fix(typescript): capture missed CALLS edges from HOF callbacks and JSX #1175 / @ReidenXerx) — closes two architectural gaps in the migrated TS path: HOF/arrow-callback caller-attribution (query.ts arrow anchor + finalize-algorithm.ts callable preference + ids.ts Variable exclusion) and JSX-as-call edge emission (TSX-only query.ts suffix + captures.ts phantom-ACCESSES suppression).
  • Legacy Call-Resolution DAG + named object-property arrows (originally PR fix(ts): CALLS edge collector attribution for HOF/callback patterns (#1166) #1179 / @abhigyanpatwari, merged in via bb6aa0c8) — fixes parallel attribution gaps in the legacy path that the registry-primary fix can't reach: genericFuncName returning phantom Function IDs for unparenthesized arrows like file => fn(file) (Bug A), and tsExtractFunctionName failing to name arrows under pair parents like addItem: (item) => set(...) (Bug B). Adds 4 pair-with-arrow declaration patterns to the registry-primary query.ts so the two paths agree on which nodes are functions.

Per AGENTS.md, TypeScript is in MIGRATED_LANGUAGES so the registry-primary path is authoritative for production behavior — but the CI parity gate runs both paths per migrated language on every PR, so the legacy-path fixes were a hard requirement to unify the two paths' definition sets and keep the gate green.

On the bug reporter's repo (Sourcerer-fe, scoped to src/ production code only, with src/types/ declarations excluded — the repo has zero tests / .d.ts / stories / build artifacts in the index): FunctionFunction CALLS edges grow from 818 to 1,378 (+560, +68.5%). Function-node count grows from 1,185 to 1,560 because the legacy-path fix promotes 375 previously-anonymous object-property arrows (Zustand actions, TanStack queryFn, React Context configs) into proper named functions with their own caller scopes — every one of those is a brand-new gitnexus_impact target that didn't exist on main.

Motivation / context

Issue #1166 reported quantified observations: Function nodes were indexed correctly (100%) but the CALLS edge collector silently dropped most call expressions when the call sat inside a callback passed as an argument to another function. In one typical TS+React codebase: 75% of all functions had zero outgoing CALLS, 56% were fully orphaned, and some file classes (Zustand stores, React providers) hit 0% capture. Downstream effect: gitnexus_context() returned {} for hot symbols, and gitnexus_impact("Badge", direction: "upstream") returned risk: LOW, impactedCount: 0 for components used 15+ times — silently mis-reporting blast radius.

The orphan rate was driven by four cooperating issues across both call-resolution pipelines:

Registry-primary path (TypeScript is in MIGRATED_LANGUAGES, so this is authoritative)

  1. HOF / arrow-callback attribution. pass2AttachDeclarations placed the Function def for arrow-typed declarations on the wrapping module scope (the @declaration.function anchor was the outer lexical_declaration, whose start lay before the inner arrow's scope). resolveCallerGraphId then walked up past the empty arrow scope into the module and grabbed the first Function-like def in ownedDefs — frequently the wrong one. Calls inside Promise.all(items.map(x => fn(x))), useQuery({ queryFn: () => fetchData() }), or create()(devtools(persist((set) => ({ ... })))) were either lost or attributed to a sibling. As a follow-on, findExportByName returned whichever def appeared first in localDefs — frequently the Variable def TS emits alongside every const fn = () => {}, breaking cross-file callee discovery whenever the import resolved to the non-callable. As a third follow-on, the caller walk-up's class-fallback considered any linkable label as a valid caller anchor, including Variable, which produced phantom Function:create → Function:create self-loops at module scope (Zustand store-creation idiom).

  2. JSX as a CALLS edge. <Foo /> is syntactic sugar for Foo(props) invoked at runtime by React, but the TS scope query had no jsx_* patterns, so React-component invocations were invisible to the call graph. Every <Component /> in every .tsx file produced zero outgoing edges from the enclosing function (and zero incoming edges to the component) — the dominant cause of the .tsx-vs-.ts asymmetry in the orphan rate.

Legacy Call-Resolution DAG (still runs in parallel for the CI parity gate)

  1. Bug A — phantom Function IDs. genericFuncName scanned arrow_function / function_expression children for the first identifier and returned it. For unparenthesized arrows like file => processFile(file) the first identifier was the parameter file, so calls inside got attributed to a synthetic Function file and emitted dangling CALLS edges. User queries like (:Function)-[:CALLS]->() never saw them.

  2. Bug B — anonymous object-property arrows. tsExtractFunctionName only named arrows whose parent was variable_declarator. addItem: (item) => set(...) (Zustand actions, TanStack queryFn, React Context providers, config objects) lives under a pair parent — treated as anonymous. With no named ancestor up to the file, calls inside fell back to the File and were invisible to context() / impact().

The four fixes are orthogonal in failure-mode but cooperate in coverage: registry-primary fixes (1) and (2) restore proper attribution for named arrows; legacy fixes (3) and (4) ensure the parallel path agrees on what counts as a named function and which scope to attribute under. Crucially, fix (4) also adds 4 pair-with-arrow patterns to the registry-primary query.ts so the two paths produce the same set of Function nodes — the CI parity gate would have failed otherwise.

This PR is the umbrella consolidation of two contributors' work:

References:

Areas touched

  • gitnexus/ (CLI / core / MCP server)
  • gitnexus-web/ (Vite / React UI)
  • .github/ (workflows, actions)
  • eval/ or other tooling
  • Docs / agent config only (AGENTS.md, CLAUDE.md, .cursor/, llms.txt, etc.)

Files (umbrella):

Registry-primary path (@ReidenXerx):

  • gitnexus/src/core/ingestion/languages/typescript/query.ts — arrow-anchor inner-node move + TSX_JSX_QUERY_SUFFIX
  • gitnexus/src/core/ingestion/languages/typescript/captures.ts — JSX cases in shouldEmitReadMember + arity-synthesis JSX comment
  • gitnexus/src/core/ingestion/scope-resolution/graph-bridge/ids.tsisCallerAnchorLabel Variable exclusion
  • gitnexus-shared/src/scope-resolution/finalize-algorithm.tsfindExportByName callable preference

Legacy DAG path + name-resolution alignment (@abhigyanpatwari, merged from #1179):

  • gitnexus/src/core/ingestion/languages/typescript.tstsExtractFunctionName resolves names from pair parents
  • gitnexus/src/core/ingestion/languages/typescript/query.ts — adds 4 pair-with-arrow patterns to mirror the legacy queries (registry-primary half of fix B)
  • gitnexus/src/core/ingestion/tree-sitter-queries.ts — same pair-with-arrow patterns in TYPESCRIPT_QUERIES / JAVASCRIPT_QUERIES (legacy-path half)
  • gitnexus/src/core/ingestion/utils/ast-helpers.tsgenericFuncName returns null for anonymous JS/TS function-likes

Tests + fixtures:

  • gitnexus/test/integration/resolvers/typescript-hof-callbacks.test.ts (8 assertions, registry-primary path)
  • gitnexus/test/integration/resolvers/typescript-jsx-as-call.test.ts (7 assertions, registry-primary path)
  • gitnexus/test/unit/call-attribution-issue-1166.test.ts (18 assertions, legacy DAG path)
  • gitnexus/test/fixtures/lang-resolution/typescript-hof-callbacks/ (6 fixtures)
  • gitnexus/test/fixtures/lang-resolution/typescript-jsx-as-call/ (8 fixtures)

Scope & constraints

In scope

  • HOF / arrow-callback caller-attribution for TypeScript (3 cooperating fixes: arrow-anchor in tree-sitter query, findExportByName callable preference, isCallerAnchorLabel Variable exclusion).
  • JSX-as-call CALLS-edge emission for TSX files (TSX-only query suffix + shouldEmitReadMember JSX cases to suppress phantom ACCESSES on <Foo.Bar />).
  • Two new integration test files (typescript-hof-callbacks.test.ts 8 assertions, typescript-jsx-as-call.test.ts 7 assertions) plus 13 minimal fixtures pinning the exact patterns from the bug report (TanStack useQuery/useMutation, Zustand create()(devtools(persist(...))), Promise.all(map), new Promise(executor), self-closing JSX, paired JSX, namespaced <Foo.Bar />, nested JSX, lowercase HTML negative test, combined HOF+JSX const F = () => <X />).

Explicitly out of scope / not done here (the residual .tsx orphans after this PR — kept off this PR's surface so review stays focused):

  • External-only callers (~25% of remaining .tsx orphans) — function bodies whose only calls are to node_modules symbols (twMerge, dayjs, useState). Working as intended; would require indexing dependencies. Better addressed at the UX layer ("external-only" label, not "orphan").
  • Bare function-reference passing (~10%) — queryFn: fetchData without an arrow wrapper. Not syntactically a call at the use site; deferred for a follow-up that introduces a WIRES_TO edge type or an indirect: true flag on CALLS.
  • Destructured-from-hook-return + prop-callback invocation (~30%) — const { fn } = useStore(); fn() and function F({ onChange }) { onChange(x) }. Both require return-type tracking and/or React data-flow analysis. Larger projects; deferred.
  • End-to-end positive resolution of <Container.Title /> through an object-literal namespace (export const Container = { Title }). The JSX capture is verified in tests (member-call interception suppresses the phantom ACCESSES; the receiver doesn't collapse to the wrong target). The full positive-edge to the leaf Title requires the existing compound-receiver resolver to chain through object-literal namespaces — pre-existing gap, orthogonal to JSX-as-call.

Implementation notes

Fix 1 — HOF / arrow-callback attribution (3 cooperating changes)

File Change
gitnexus/src/core/ingestion/languages/typescript/query.ts @declaration.function anchor moved from the wrapping lexical_declaration to the inner arrow_function / function_expression so anchor.range aligns with @scope.function and pass2AttachDeclarations lands the def on the arrow's own scope.
gitnexus-shared/src/scope-resolution/finalize-algorithm.ts findExportByName now prefers callable / class-like defs (Function/Method/Constructor/Class/Interface/Enum/Struct/Record/Trait/Namespace/Module/TypeAlias) over Variable when localDefs contains both for the same name. (TS emits two defs per const fn = () => {} — a Function and a Variable; capture order silently decided which an importer bound to.) The behavior is a tie-break: when only one def with the name exists, the existing first-match path fires unchanged.
gitnexus/src/core/ingestion/scope-resolution/graph-bridge/ids.ts New isCallerAnchorLabel predicate (a strict subset of isLinkableLabel) restricted to Function / Method / Constructor / Class / Interface / Struct / Enum. Variable / Property are no longer valid caller anchors — module-level call expressions inside const X = expr(args) declarations correctly fall through to the File-node fallback instead of being mis-attributed to a sibling Function via simpleKey (the Zustand create()(devtools(persist(...))) regression).

Fix 2 — JSX as CALLS edge (2 changes)

File Change
gitnexus/src/core/ingestion/languages/typescript/query.ts Adds a TSX_JSX_QUERY_SUFFIX constant (compiled only against the tsx grammar — jsx_* node types don't exist in the plain typescript grammar, so embedding them in the shared query would throw Query.InvalidNodeType). Captures jsx_self_closing_element and jsx_opening_element as @reference.call.free / @reference.call.member. Lowercase HTML elements (<div>, <span>) are filtered with the (#match? @reference.name "^[A-Z]") predicate so they don't emit edges to nonexistent targets; member-form (<Foo.Bar />) is unfiltered (HTML element names can't contain dots, so any .-form is unambiguously a component). The closing tag is intentionally NOT captured — each JSX element should emit exactly one CALLS edge per use site.
gitnexus/src/core/ingestion/languages/typescript/captures.ts shouldEmitReadMember extended with jsx_self_closing_element / jsx_opening_element parent cases. Without this, <Foo.Bar />'s inner member_expression would emit BOTH a @reference.call.member (the new path) AND a @reference.read.member (the generic catch-all), producing a redundant phantom ACCESSES edge alongside the CALLS edge. Verified by the JSX integration test.

Sourcerer-fe end-to-end validation

I built the patched CLI from this branch and re-analyzed the reporter's Sourcerer-fe repo at the same commit, three times — clean main, registry-primary fixes only (PR #1175 alone), umbrella with legacy-DAG fixes merged in (this PR). Sourcerer-fe is a Next.js + Capacitor mobile app; the indexer respects .gitignore and skips android/, ios/, node_modules/, out/, .next/. Of the 525 indexed files, 508 live in src/ and the remaining 17 are root configs (next.config.js, tailwind.config.ts, etc.); the repo has zero test files, zero .d.ts, zero stories. Metrics below are scoped to src/ only, with src/types/ declarations excluded — purely production code patterns.

Metric Clean main Registry only (#1175) Umbrella (this PR) Δ vs main
Function nodes (in src/) 1,185 1,185 1,560 +375
FunctionFunction CALLS edges 818 1,322 1,378 +560 (+68.5%)
All CALLS edges (incl. File-anchored) 979 1,528 1,541 +562 (+57.4%)
.tsx no-outgoing orphans 521 405 471 −50
.ts no-outgoing orphans 347 309 586 +239 (see note)

Why the .ts orphan count grows in the umbrella column: the legacy-path fix (Bug B) promotes 375 previously-anonymous object-property arrows in .ts files (Zustand store actions, TanStack hook configs, React Context setup objects) into proper named Function nodes. Many of those promoted leaves are tiny one-liners that legitimately have zero outgoing calls (addItem: (item) => set(...) only calls set, which is itself a parameter), so they appear as orphans-by-numerator while not being a regression. The right way to read the table: function count grew by 375 because that many new gitnexus_impact targets exist that never existed before, and Function→Function edges still grew by 560 — 56 of those edges originate from the newly-promoted leaves and 504 from existing functions that were previously misattributing or invisible.

Concrete bug-report cases that now work:

Registry-primary path (HOF + JSX):

  • TanStack hookssrc/api-modules/notifications/push-notifications.hooks.ts: pre-fix 0/16 captures (matches the bug report's "0%" hook number). Post-fix: arrow-form mutationFn: (data) => registerDevice(data) correctly emits useRegisterDevice → registerDevice and 4 sibling hook-to-API edges.
  • React component invocationssrc/components/subscription/subscription-management.tsx::getStatusBadge: previously orphan (its only "call" was <Badge variant=... />). Post-fix: getStatusBadge → Badge is captured. Across the codebase, Badge now has 15 incoming CALLS edges from .tsx files. Top JSX-restored receivers: cn (83×), usePlatform (37×), Loader (35×), toast (31×), useToast (23×), Badge (15×), TopBarWrapper/TopBarPortal (11× each), Lucide icons like Share2 (10×) — all 0 incoming on clean main.
  • Zustand mis-attributionuseStore = create()(devtools(persist((set) => ({ ... })))) no longer produces phantom Function:create → Function:create self-loops. Module-level call expressions correctly fall through to the File node when no enclosing function scope owns them (covered by the Zustand-fixture assertions in typescript-hof-callbacks.test.ts).

Legacy DAG path (Bugs A + B):

  • Phantom Function file IDs eliminatedprocessSelectedFiles → processFile inside Promise.all(files.map(file => processFile(file))) previously attributed to a synthetic Function file (the parameter name). Now correctly attributes to processSelectedFiles.
  • Object-property arrows promotedaddItem: (item) => set(...) and TanStack queryFn: () => fetchData() now appear as named Function nodes; calls inside their bodies attribute to the action name instead of the file. The reporter's specific Zustand reproducer from CALLS edge collector misses ~75% of functions; concentrated in HOF/callback patterns #1166 (useStore = create<State>()(devtools(persist((set, get) => ({ addItem: ..., fetchData: ... }))))) now resolves all expected edges.

Testing & verification

Template's standard checklist:

  • cd gitnexus && npm test — ran the suites covering changed code paths (see custom rows below). Skipped the full npm test because the 146 kotlin.test.ts failures it would surface are a pre-existing local environment issue (missing optional tree-sitter-kotlin native binding, documented in AGENTS.md § Gotchas) that reproduces on main at dafda284 without these patches. CI runs the full suite in a clean container where the binding is present.
  • cd gitnexus && npm run test:integration — covered the relevant subset (changes are confined to TS scope-resolution; integration entry points listed below).
  • cd gitnexus && npx tsc --noEmit — clean
  • cd gitnexus-web && npx tsc -b --noEmit — clean (this PR doesn't touch gitnexus-web/, but ran per checklist to confirm the gitnexus-shared re-exports stay binary-compatible after the finalize-algorithm.ts change)
  • cd gitnexus-web && npm test — skipped (web not changed)
  • Manual / Playwright E2E — N/A

Specific commands run on the umbrella branch (after merging #1179):

  • cd gitnexus && npx vitest run test/integration/resolvers/typescript-hof-callbacks.test.ts8/8 ✓ (registry-primary HOF assertions)
  • cd gitnexus && npx vitest run test/integration/resolvers/typescript-jsx-as-call.test.ts7/7 ✓ (registry-primary JSX assertions)
  • cd gitnexus && npx vitest run test/unit/call-attribution-issue-1166.test.ts18/18 ✓ (legacy-DAG attribution + Bug A / Bug B assertions, from fix(ts): CALLS edge collector attribution for HOF/callback patterns (#1166) #1179)
  • cd gitnexus && npx vitest run test/integration/resolvers/typescript.test.ts236/236 ✓ (no regressions in either path)
  • cd gitnexus && npx vitest run test/integration/resolvers/api-deep-flow.test.ts11/11 ✓
  • cd gitnexus && npx vitest run test/unit/scope-resolution test/unit/scope-extractor test/unit/typescript613/613 ✓
  • Combined regression run — all five files in one vitest run: 280/280 ✓ (registry-primary integration + legacy-DAG unit + existing TS suite all coexist with both fix sets active)
  • cd gitnexus-shared && npx tsc --noEmit — clean
  • Pre-commit hook (.husky/pre-commit) clean during umbrella commits
  • End-to-end: re-analyzed Sourcerer-fe three times (clean main, registry-primary only, umbrella) with patched CLIs; captured the orphan-rate / edge-count numbers in the table above; spot-checked per-file deltas against the specific patterns from CALLS edge collector misses ~75% of functions; concentrated in HOF/callback patterns #1166 (file-upload.ts, use-gateway-queries.ts, grouped-file-uploads-store.tsx, push-notifications.hooks.ts, subscription-management.tsx).

Risk & rollout

Risk: LOW.

  • Scope is fully contained in the TypeScript scope-resolution path. No behavior change for any other indexed language. The shared library change (finalize-algorithm.ts::findExportByName) is a tie-break refinement when both a Function and a Variable definition share a name within the same module — a TypeScript-specific situation; in every other migrated language tested, no callable/variable name collisions of this shape occur in localDefs, so the fallback path (first-match) still fires unchanged.
  • The new isCallerAnchorLabel predicate in graph-bridge/ids.ts strictly narrows the existing isLinkableLabel predicate (it's a subset). The only legacy behavior it removes is "if the only owned def at module scope is a Variable, attribute the call to it as a caller" — a path that produced phantom self-loops in practice (the Zustand regression covered by the test).
  • The TSX-only JSX query suffix is appended at TSX-grammar compile time (in getTsScopeQuery); plain .ts files don't see it. The TSX grammar has been the indexer's .tsx path since MIGRATED_LANGUAGES flipped — no parser swap.
  • Impact analysis (gitnexus_impact) on getTsScopeQuery, shouldEmitReadMember, emitTsScopeCaptures, resolveCallerGraphId, and findExportByName returned risk: LOW for all five — direct callers are localized, no execution flows downstream, no cross-module fan-out.
  • The legacy-path genericFuncName change is a strict narrowing: it returns null for anonymous JS/TS function-likes where it previously returned the wrong identifier (the first parameter). Anonymous arrows now correctly fall through to the language hook instead of producing phantom Function IDs.
  • The legacy-path tsExtractFunctionName change extends naming to pair parents (object-property arrows). Computed keys ([K]: () => ...) intentionally stay anonymous to avoid resolving runtime expressions. No previously-named function changes its name.
  • The 4 new pair-with-arrow patterns added to the registry-primary query.ts are new pattern matches (not edits to existing patterns) — they only fire on object-property arrows, which previously matched no declaration pattern at all in that path. This is what keeps the CI parity gate green: both pipelines now agree on the set of Function nodes.
  • 0 regressions across the 280 TypeScript-relevant integration + unit tests (8 HOF + 7 JSX + 18 issue-1166 + 236 typescript + 11 api-deep-flow) and 613 scope-resolution unit tests on the merged branch.

Breaking changes: none. No public API changes (CLI flags, MCP tools, edge types, node labels, schema). The graph emits more edges than before — strictly additive — and eliminates a small number of phantom self-loops. Consumers of the graph (existing gitnexus_impact / gitnexus_query / gitnexus_context callers) will see strictly more truthful results.

Migrations: none required to ship. No DB schema change, no node/edge property additions.

Index refresh: required for users to see the new edges. Existing indexed repositories will need npx gitnexus analyze to surface the additional CALLS edges in their graph (no --force needed — the analyzer detects the file/grammar change and re-extracts). Without re-analyzing, existing graphs will continue to render the pre-fix edge set (correct behavior — staleness check writes a marker, see AGENTS.md § "If any GitNexus tool warns the index is stale").

Release notes: lands under 🐛 Bug Fixes (per .github/release.yml, type=fix).

Rollback. Each of the source-level changes is independently revertible without breaking the others:

Registry-primary path (commits 7be595d3, 851d2ab7):

Revert this Effect
gitnexus/src/core/ingestion/languages/typescript/query.ts HOF chunk (arrow-anchor patterns ~lines 151-170) Restores legacy outer-lexical_declaration anchor; HOF-callback caller-attribution returns to pre-fix behavior. JSX and legacy-path fixes still active.
gitnexus-shared/src/scope-resolution/finalize-algorithm.ts (findExportByName) Returns to first-match behavior across languages.
gitnexus/src/core/ingestion/scope-resolution/graph-bridge/ids.ts (isCallerAnchorLabel) Reverts to isLinkableLabel for caller-walk fallback; phantom self-loop case (Zustand) returns.
gitnexus/src/core/ingestion/languages/typescript/query.ts TSX_JSX_QUERY_SUFFIX (constant + concatenation in getTsScopeQuery) JSX-as-call disabled; HOF and legacy-path fixes still active.
gitnexus/src/core/ingestion/languages/typescript/captures.ts (jsx cases in shouldEmitReadMember) Phantom ACCESSES edges on <Foo.Bar /> re-appear; CALLS edges unaffected.

Legacy DAG path (commit 93a0be23, originally PR #1179):

Revert this Effect
gitnexus/src/core/ingestion/utils/ast-helpers.ts (genericFuncName returning null for anonymous JS/TS) Phantom Function IDs from unparenthesized arrow parameters re-appear in the legacy path.
gitnexus/src/core/ingestion/languages/typescript.ts (tsExtractFunctionName pair parent handling) Object-property arrows fall back to anonymous in the legacy path; calls inside revert to File attribution.
gitnexus/src/core/ingestion/tree-sitter-queries.ts pair-with-arrow patterns Legacy queries no longer emit Function nodes for object-property arrows.
gitnexus/src/core/ingestion/languages/typescript/query.ts 4 pair-with-arrow patterns Registry-primary path no longer produces Function nodes for object-property arrows. Reverting just this without also reverting the legacy-side patterns will fail the CI parity gate — the two paths must agree on the Function set.

A full revert is git revert <merge-commit> for the umbrella merge bb6aa0c8 plus the registry-primary commits. Each commit is independently revertible with git revert <sha>. No migrations / data-shape changes. Re-running npx gitnexus analyze after a revert restores the pre-fix edge set.

Checklist

  • PR body meets repo minimum length (workflow may label short descriptions)
  • If AGENTS.md / overlays changed: headers, scope block, and changelog updated per project conventions — N/A, no overlay files in this PR
  • No secrets, tokens, or machine-specific paths committed — verified by scanning the diff for /Users/, /home/, sk-, ghp_, github_pat_, password, api_key (none found)

Two distinct gaps in the TypeScript scope-resolution path were silently
dropping call edges in real-world React + TanStack + Zustand codebases.
On the bug reporter's repo (Sourcerer-fe, 1185 src/ functions), 504
missing Function->Function CALLS edges are now captured (+61.6%) and
the no-outgoing-CALLS orphan rate drops from 73.2% to 60.3%.

HOF / arrow-callback caller-attribution (3 cooperating fixes):
  - typescript/query.ts: @declaration.function anchor moved from the
    wrapping lexical_declaration to the inner arrow_function /
    function_expression, so anchor.range aligns with @scope.function and
    pass2AttachDeclarations lands the def on the arrow's own scope.
  - finalize-algorithm.ts: findExportByName prefers callable / class-
    like defs over Variable when localDefs contains both for the same
    name (TS emits two defs per `const fn = () => {}`).
  - graph-bridge/ids.ts: resolveCallerGraphId's walk-up class-fallback
    now uses isCallerAnchorLabel restricted to Function / Method /
    Constructor / Class / Interface / Struct / Enum, so module-level
    calls fall through to the File node instead of mis-attributing to
    sibling Variable defs (the Zustand `create()(devtools(...))`
    phantom-self-loop regression).

JSX as a CALLS edge (2 cooperating fixes):
  - typescript/query.ts: new TSX_JSX_QUERY_SUFFIX (TSX-grammar only)
    captures jsx_self_closing_element / jsx_opening_element as
    @reference.call.free / @reference.call.member. PascalCase predicate
    filters native HTML elements (<div>, <span>) so they don't emit
    edges to nonexistent targets.
  - typescript/captures.ts: shouldEmitReadMember extended with
    jsx_self_closing_element / jsx_opening_element parent cases to
    suppress phantom ACCESSES edges on member-form JSX names.

Tests: 8 HOF assertions + 7 JSX assertions across two new integration
test files plus 13 minimal fixtures. typescript.test.ts (236),
api-deep-flow.test.ts (11), and scope-resolution / scope-extractor unit
tests (613) pass with no regressions.

Made-with: Cursor
@vercel

vercel Bot commented Apr 28, 2026

Copy link
Copy Markdown

@ReidenXerx is attempting to deploy a commit to the NexusCore Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

github-actions Bot commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 6m 23s —— View job


PR #1175 Review — fix(typescript): capture missed CALLS edges from HOF callbacks and JSX

Tasks completed:

  • Read reference docs (DoD.md, AGENTS.md, GUARDRAILS.md, CONTRIBUTING.md, TESTING.md, ARCHITECTURE.md)
  • Read changed source files (query.ts, captures.ts, finalize-algorithm.ts, ids.ts)
  • Read test files and fixtures
  • Analyze HOF/arrow-callback attribution path
  • Analyze JSX query suffix and TSX-only compilation
  • Analyze shared findExportByName tie-breaker
  • Analyze isCallerAnchorLabel narrowing
  • Assess test quality and coverage gaps
  • Assess non-TypeScript regression risk
  • Assess CI/validation completeness
  • Produce findings and final verdict

Review bar for this PR

  • HOF/arrow-callback calls are attributed to the correct enclosing arrow/function scope through pass2AttachDeclarationsresolveCallerGraphId on the registry-primary path.
  • const fn = () => {} exports resolve to the callable Function def (not the parallel Variable def) when imported cross-file.
  • Module-level call expressions correctly fall back to the File node and never produce phantom self-loops against a sibling function.
  • JSX component usage emits exactly one CALLS edge per element; lowercase HTML tags (<div>, <span>, <button>) are filtered at the query level.
  • <Foo.Bar /> emits a CALLS edge via the member-call path and does NOT emit a duplicate ACCESSES edge.
  • Plain .ts query compilation is unaffected; JSX patterns are TSX-only via a separate constant compiled against the TSX grammar.
  • Shared findExportByName callable preference is safe for all currently migrated languages and does not change single-def behavior.
  • Tests exercise the real runPipelineFromRepo integration path and would fail if any of the five independent source changes were reverted.

Understanding of the change

What failures this PR addresses: Two independent TypeScript scope-resolution gaps caused ~73% of Function nodes in real React+TanStack+Zustand codebases to have zero outgoing CALLS edges.

Why HOF/arrow callbacks were misattributed: The @declaration.function anchor was placed on the outer lexical_declaration (col 0), not the inner arrow_function. pass2AttachDeclarations uses atPosition(startLine, startCol) to find the innermost scope; with the anchor at col 0 the module scope is innermost, so the Function def landed in the module scope's ownedDefs. resolveCallerGraphId then walked up past the empty arrow scope into the module scope and grabbed the first Function-like def in ownedDefs — often the wrong function entirely.

Why JSX needs a CALLS edge: <Foo /> is Foo(props) at runtime. Without JSX patterns, gitnexus_impact("Badge", direction: "upstream") returned risk: LOW, impactedCount: 0 for a component invoked 15+ times — the call graph was blind to React component usage.

Registry-primary paths affected: getTsScopeQuery (query compilation), emitTsScopeCaptures (capture emission and read-member suppression), pass2AttachDeclarations (anchor → scope placement, via query fix), findExportByName (import resolution tie-break), resolveCallerGraphId (caller attribution). The legacy Call-Resolution DAG is untouched.

Shared contract change: findExportByName in gitnexus-shared now prefers callable/type-like defs over Variable when the same simple name appears in localDefs. This is a tie-break only (single-def behavior unchanged), and the parallel Function+Variable emission is TypeScript-specific.

What must remain unchanged: Plain .ts query compilation; non-TypeScript languages' finalize behavior; the Variable def still exists in localDefs (for backward compat) — only import resolution preference changes; ACCESSES edges for ordinary member reads.


Findings

[medium] toBeGreaterThan(0) in combined HOF+JSX test masks duplicate-edge regressions

  • Category: Test quality / DoD §2.7
  • Files: gitnexus/test/integration/resolvers/typescript-jsx-as-call.test.ts:121
  • Issue: expect(calls.length).toBeGreaterThan(0) for the Wrapped → Foo assertion. If the JSX query suffix accidentally emits duplicate CALLS edges for the same site (e.g., both jsx_self_closing_element and a generic call pattern firing), this test passes while the regression goes undetected. The DoD explicitly prohibits bounds-only assertions that mask regressions.
  • Why it matters here: Duplicate CALLS edges inflate blast-radius counts in gitnexus_impact and degrade graph quality. This is exactly the kind of subtle double-count this review was asked to detect.
  • Recommended fix: Change to expect(calls.length).toBe(1). A single JSX self-closing element is exactly one logical invocation. Fix this →

[medium] Zustand "all sources must be File" test is vacuously true when no edges are emitted

  • Category: Test quality / DoD §2.7
  • Files: gitnexus/test/integration/resolvers/typescript-hof-callbacks.test.ts:111–126
  • Issue: The test iterates for (const c of calls) and asserts each edge's source is ['File', 'store.ts']. If calls is empty, the loop body never executes and the test passes unconditionally. This makes it a weak regression guard: any change that silences ALL CALLS edges from store.ts would pass.
  • Why it matters here: The Zustand phantom self-loop fix is the hardest regression to detect. The anti-self-loop test above it (lines 100–108) is the real guard; this one adds false confidence. Additionally, since the HOF fix now correctly attributes calls inside named arrow bodies to those functions (not the File), if the fixture ever gains a resolvable call inside create's body, the assertion toEqual(['File', 'store.ts']) would become a false failure.
  • Recommended fix: Add an explicit check that the calls array is non-empty before the loop, or replace with a structural assertion. Alternatively, scope the test to only module-level calls (those on the const useStore = ... RHS) and add a positive assertion that calls inside create's body would source from create. For this PR the fixture is safe as-is, but the test is brittle.

[low] CI test suite pending — full validation not yet confirmed

  • Category: Validation / DoD §4.2
  • Files: N/A
  • Issue: The CI run (ID 25069944392) was in_progress at review time; no completed pass/fail result was available. The PR also did not run cd gitnexus && npm test locally — the explanation (Kotlin native binding failures on main) is credible but unverified from this review's vantage point.
  • Why it matters here: The DoD requires npm test to pass for any gitnexus/ change. The new tests are in test/integration/resolvers/ and should be auto-discovered by Vitest; they will run in CI. This is not blocking if CI comes back clean, but the validation claim cannot be marked satisfied until the run completes.
  • Recommended fix: Confirm CI green before merging. No code change needed.

[low] findExportByName comment calls out TypeScript by name in shared code

  • Category: Maintainability / DoD §2.3
  • Files: gitnexus-shared/src/scope-resolution/finalize-algorithm.ts:743–761
  • Issue: The comment explains the tie-break using a TypeScript-specific example (const fn = () => {}) and links to typescript-hof-callbacks.test.ts. The implementation itself is language-agnostic and correct, but a future contributor adding a non-TS language could be confused about whether this is a TS-only contract.
  • Why it matters here: finalize-algorithm.ts is shared infra. DoD §2.2 warns against "language-specific logic leaked into shared ingestion code without a documented architectural reason."
  • Recommended fix: Rephrase the comment to note "TypeScript is the first known language where this arises, but the rule is applied generically" — the current comment already says "Languages like TypeScript…" which is close; just ensure future readers understand it's a general tie-break, not a TS-specific hack.

[low] No arity metadata synthesized for JSX @reference.call.free captures

  • Category: Correctness (minor) / DoD §2.1
  • Files: gitnexus/src/core/ingestion/languages/typescript/captures.ts:243–269
  • Issue: The arity synthesis loop searches for a call_expression or new_expression at the anchor's range. For JSX captures, the anchor is jsx_self_closing_element / jsx_opening_element, so callNode is always null and @reference.arity is never synthesized. JSX calls enter the registry without arity metadata, falling back to name-only resolution.
  • Why it matters here: In practice, React components are almost never overloaded in GitNexus's graph model, so arity-less resolution is correct for JSX. But the implicit behavior gap is undocumented — if a future component is overloaded, JSX calls would always match arity-0, not the actual component. Not a correctness issue for the current codebase.
  • Recommended fix: Add a comment in captures.ts noting that JSX call anchors don't carry arity metadata and explaining why this is acceptable. No code change needed now.

Definition of Done check

DoD item Status
HOF/arrow calls attributed to the correct enclosing arrow scope via real registry-primary path satisfied — anchor moved to inner arrow_function; pass2AttachDeclarations places def in arrow's own ownedDefs; tested via runPipelineFromRepo integration path
const fn = () => {} exports resolve to callable Function def without breaking Variable behavior satisfiedfindExportByName tie-break is additive; Variable def still present in localDefs; tested cross-file via useFeature → fetchData fixture
Module-level calls fall back to File and produce no phantom self-loops satisfiedisCallerAnchorLabel excludes Variable/Property; anti-self-loop test with exact toEqual([]) assertion
JSX emits exactly one CALLS edge per element; lowercase HTML filtered partially satisfied — negative HTML test is exact; double-count prevention is verified for paired JSX (toHaveLength(1)) but NOT for the combined HOF+JSX case (uses toBeGreaterThan(0) instead of toBe(1))
<Foo.Bar /> emits CALLS not phantom ACCESSES satisfiedshouldEmitReadMember JSX cases verified; phantom ACCESSES test uses toEqual([])
Plain .ts query compilation unaffected; JSX patterns TSX-only satisfiedTSX_JSX_QUERY_SUFFIX is a separate constant; compiled only with TSX_GRAMMAR; separate _tsQuery and _tsxQuery singletons
Shared finalize behavior compatible with non-TypeScript languages satisfied — tie-break only fires when two defs for the same name exist in localDefs; only TS's dual-emit creates that situation; Python/C# unaffected
Tests prove the real changed paths and would fail on revert partially satisfied — integration path is real (runPipelineFromRepo); all five source changes have fixture coverage; the toBeGreaterThan(0) assertion in the combined test is the one gap

Validation assessment

Tests added/updated:

  • typescript-hof-callbacks.test.ts — 8 assertions over 6 new fixtures; covers direct call, Promise.all/map, new Promise executor, TanStack useQuery, Zustand phantom self-loop, cross-file arrow-export resolution.
  • typescript-jsx-as-call.test.ts — 7 assertions over 8 new fixtures; covers self-closing, paired, nested, member, HTML-negative, phantom ACCESSES negative, combined HOF+JSX.

Validation claimed by PR:

  • 8/8 HOF integration tests ✓
  • 7/7 JSX integration tests ✓
  • 236/236 existing typescript.test.ts
  • 11/11 api-deep-flow.test.ts
  • 613/613 scope-resolution + scope-extractor unit tests ✓
  • gitnexus, gitnexus-shared, gitnexus-web typechecks clean ✓
  • Pre-commit hook clean ✓
  • Sourcerer-fe before/after end-to-end metric improvement ✓

Validated from diff / CI evidence:

  • TSX-only suffix path (_tsxQuery vs _tsQuery singletons) is structurally correct; cannot compile jsx_* node types against plain TS grammar.
  • shouldEmitReadMember JSX cases correctly check parent.childForFieldName('name')?.id — same pattern used successfully for call_expression and new_expression in the pre-existing cases.
  • findExportByName callable-preference loop is correct: first callable match returns immediately; non-callable match saves as fallback only; single-def case is unchanged.
  • isCallerAnchorLabel is a strict subset of what resolveCallerGraphId already did in its first-check pass (Function/Method/Constructor); the class-fallback now adds Class/Interface/Struct/Enum only.
  • Test helpers use runPipelineFromRepo (the real CLI pipeline), not mocks.
  • namespaced.tsx fixture declares export const Container = { Title } — the known object-literal namespace gap is correctly documented as out of scope and the test only asserts negatives, not a false positive.

Missing or not-verifiable:

  • CI test suite still pending — could not confirm full Vitest run with Kotlin binding present.
  • gitnexus-shared npm run build — only npx tsc --noEmit is mentioned; dist build not explicitly confirmed (DoD §4.4).
  • Full npm test locally — Kotlin binding explanation is credible but unverifiable. The focused per-file runs cited in the PR body are the practical substitute.

Whether missing validation blocks production readiness: The CI in-progress is the only material gap. If CI comes back green (which is expected given the extensive per-file pre-validation), no blocker remains. gitnexus-shared npm run build should be verified before merge.


Final verdict

production-ready with minor follow-ups

The two core fixes are mechanically correct, architecturally sound, and properly placed in the registry-primary path. The HOF anchor change resolves the root cause (not a symptom patch); the JSX suffix correctly exploits tree-sitter's grammar isolation guarantees; the findExportByName tie-break is language-agnostic and additive; and isCallerAnchorLabel's Variable exclusion is the right semantic constraint backed by a strict-subset argument. The 8+7 integration tests exercise the real pipeline path and would fail on independent revert of each of the five changed files.

The two medium findings are test-quality issues only: the toBeGreaterThan(0) assertion in the combined HOF+JSX test should be toBe(1) to catch duplicate-edge regressions (the most subtle correctness concern in JSX-as-call), and the Zustand "all sources File" test has a vacuous-pass path. Neither affects production runtime behavior. The low findings (CI pending, missing dist-build confirmation, documentation, arity comment) are similarly non-blocking.

Recommend: fix toBeGreaterThan(0)toBe(1) before merge, confirm CI green, confirm gitnexus-shared npm run build passes, then ship.


@github-actions

github-actions Bot commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

CI Report

All checks passed

Pipeline Status

Stage Status Details
✅ Typecheck success tsc --noEmit
✅ Tests success unit tests, 3 platforms
✅ E2E success gitnexus-web changes only

Test Results

Tests Passed Failed Skipped Duration
7707 7704 0 1 317s

✅ All 7704 tests passed

1 test(s) skipped — expand for details
  • withTestLbugDB(lock-retry) > withLbugDb retry behaviour > propagates non-BUSY errors immediately without retrying
  • withTestLbugDB(lock-retry) > withLbugDb retry behaviour > throws after max retry attempts
  • buildTypeEnv > known limitations (documented skip tests) > Ruby block parameter: users.each { |user| } — closure param inference, different feature

Code Coverage

Tests

Metric Coverage Covered Base Delta Status
Statements 77% 22720/29504 76.82% 📈 +0.2 🟢 ███████████████░░░░░
Branches 65.97% 14652/22209 65.81% 📈 +0.2 🟢 █████████████░░░░░░░
Functions 81.99% 2213/2699 81.57% 📈 +0.4 🟢 ████████████████░░░░
Lines 79.85% 20499/25670 79.67% 📈 +0.2 🟢 ███████████████░░░░░

📋 View full run · Generated by CI

…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 abhigyanpatwari#1166

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@magyargergo

Copy link
Copy Markdown
Collaborator

@ReidenXerx can you please look into these findings? 🙏

Also I so there's a double contribution by @abhigyanpatwari on the other PR you commented on.

@abhigyanpatwari

Copy link
Copy Markdown
Owner

Hi @magyargergo , I asked @ReidenXerx to rebase of my PR, since this PR is covering more stuff

…assertions

Addresses the automated review findings on PR abhigyanpatwari#1175:

- prettier --write the 3 files flagged by `quality / format` CI check
  (query.ts, typescript-hof-callbacks.test.ts, typescript-jsx-as-call.test.ts).

- [medium] typescript-jsx-as-call.test.ts: tighten the combined HOF+JSX
  assertion from `toBeGreaterThan(0)` to `toHaveLength(1)`. A single
  `<Foo />` is one logical invocation; the bounds-only assertion would
  have masked a duplicate-CALLS-edge regression (e.g. if both
  `jsx_self_closing_element` and a generic call pattern matched the
  same site).

- [medium] typescript-hof-callbacks.test.ts: replace the vacuously-true
  `for (c of calls) expect(...)` Zustand assertion with a structural
  one. Old form passed unconditionally when `calls` was empty (any
  change that silenced ALL CALLS edges from store.ts would have
  slipped through). New form asserts both: (a) at least one File-rooted
  edge exists (proving the `isCallerAnchorLabel` fallback fires), and
  (b) no edge sources from anything else (proving the fallback fires
  exclusively).

- [low] finalize-algorithm.ts (`findExportByName`): rephrase the
  comment to make the language-agnostic nature of the tie-break rule
  explicit. The implementation was already correct for all migrated
  languages; only the comment overplayed the TypeScript specificity.

- [low] captures.ts (arity synthesis): add a comment explaining why
  JSX call anchors (`jsx_self_closing_element` / `jsx_opening_element`)
  intentionally don't synthesize `@reference.arity`. Name-only
  resolution is correct for React (components aren't overloaded in the
  current graph model); a JSX-aware synthesizer counting jsx_attribute
  children would be needed if that ever changes.

No production behavior change. All 8/8 HOF + 7/7 JSX + 236/236
typescript + 11/11 api-deep-flow integration tests still pass.
gitnexus and gitnexus-shared typechecks clean.

Made-with: Cursor
@ReidenXerx

Copy link
Copy Markdown
Contributor Author

@magyargergo Thanks for triggering the review — addressed everything in 851d2ab. Quick summary:

[medium] toBeGreaterThan(0)toHaveLength(1) for combined HOF+JSX
Caught — a single <Foo /> is one logical invocation, and the bounds-only assertion would have masked any duplicate-CALLS-edge regression (e.g. if jsx_self_closing_element and a generic call pattern ever both matched the same site). Tightened in typescript-jsx-as-call.test.ts:121.

[medium] Zustand "all sources are File" was vacuously true on empty calls
Replaced the for (c of calls) expect(...) loop with a structural assertion that's regression-proof either way:

  • fromOther (anything not File-rooted) must be [] — same correctness invariant as before
  • fromFile.length must be >0 — proves the isCallerAnchorLabel fallback actually fires, not just that we silenced everything

[low] findExportByName comment generality
Reworked the comment in gitnexus-shared/src/scope-resolution/finalize-algorithm.ts to make the language-agnostic nature of the tie-break rule explicit (TypeScript is the first language where this matters in practice, but the rule applies generically — single-def behavior is unchanged for all languages).

[low] JSX has no arity metadata — undocumented behavior
Added a comment in captures.ts explaining why JSX call anchors intentionally don't synthesize @reference.arity (React components aren't overloaded in the current graph model, so name-only dispatch is correct; a JSX-aware synthesizer counting jsx_attribute children would be needed if that ever changes).

quality / format CI failure
Fixed — ran npx prettier --write on the 3 flagged files. The pre-commit hook didn't fire on my end because I had a permissions issue on .git/config when the husky install would have run (separate sandbox quirk on my workstation, not a project issue).

All 262 affected integration tests still pass locally (8/8 HOF + 7/7 JSX + 236/236 typescript + 11/11 api-deep-flow), gitnexus and gitnexus-shared typechecks clean. Re-running CI should be green now.

Re: @abhigyanpatwari's note — yes, I'm happy to coordinate on the other PR if there's anything to align on.

… fix/typescript-hof-callbacks-and-jsx-as-call
@ReidenXerx

Copy link
Copy Markdown
Contributor Author

@abhigyanpatwari Done — merged your fix/issue-1166-calls-edges branch into this one as bb6aa0c8. Clean auto-merge, no manual conflict resolution needed.

Done:

  • ✅ Merged your branch (commit 93a0be23) into fix(typescript): capture missed CALLS edges from HOF callbacks and JSX #1175
  • ✅ Updated PR title/description to umbrella scope: Fixes #1166, Closes #1179, both contributors credited, full per-path breakdown of all 4 root causes
  • ✅ Re-ran the full TS test surface on the merged branch:
    • 8/8 typescript-hof-callbacks (registry-primary)
    • 7/7 typescript-jsx-as-call (registry-primary)
    • 18/18 call-attribution-issue-1166 (your tests, legacy DAG)
    • 236/236 typescript.test.ts (no regressions either side)
    • 11/11 api-deep-flow.test.ts
    • 613/613 scope-resolution + scope-extractor + typescript unit tests
    • Combined run: 280/280 ✓ (registry-primary + legacy-DAG fixes coexist cleanly)
  • gitnexus and gitnexus-shared typecheck clean
  • ✅ Re-validated end-to-end on Sourcerer-fe with the umbrella build

Updated headline numbers (Sourcerer-fe src/):

Metric Clean main Registry only Umbrella
Function nodes 1,185 1,185 1,560 (+375)
F→F CALLS edges 818 1,322 1,378 (+560, +68.5%)

The +375 function nodes come from your pair-with-arrow capture promoting Zustand actions, TanStack queryFn, and React Context configs from anonymous-into-the-File status to first-class named functions — every one of those is a brand-new gitnexus_impact target that didn't exist before.

The TSX/JSX-as-call addition (registry-primary) and your pair-with-arrow patterns (mirrored in both query modules) work together: object-property arrows that previously dissolved into the File now have proper scope identity, and JSX usages of those arrow-typed components emit CALLS edges — the entire <Container.Title /> and addItem: () => <Foo /> style flows through end-to-end.

cc @magyargergo — all of your earlier review findings remain addressed (commit 851d2ab7); the umbrella merge layered on top doesn't touch any of those tests.

Pushed bb6aa0c8. Once a maintainer approves the workflow run, CI should be fully green.

Introduced new scripts in package.json for GitNexus analysis:
- `gitnexus:refresh`: analyzes with embeddings and skills.
- `gitnexus:full`: forces analysis with embeddings and skills.

No production behavior changes. This enhances the development workflow for GitNexus users.
@github-actions

github-actions Bot commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 8m 11s —— View job


PR #1175 Review — fix(typescript): capture missed CALLS edges from HOF callbacks and JSX

Tasks:

  • Read reference docs (DoD.md, AGENTS.md, GUARDRAILS.md, CONTRIBUTING.md, TESTING.md, ARCHITECTURE.md)
  • Inspect registry-primary arrow/function-expression anchoring (query.ts)
  • Inspect HOF / callback attribution path
  • Inspect shared findExportByName callable preference (finalize-algorithm.ts)
  • Inspect caller-anchor narrowing (ids.ts)
  • Inspect JSX-as-CALLS query suffix (query.ts)
  • Inspect JSX capture handling and ACCESSES suppression (captures.ts)
  • Inspect legacy DAG genericFuncName fix (ast-helpers.ts)
  • Inspect legacy TypeScript object-property arrow naming (typescript.ts)
  • Inspect query parity for pair-with-arrow patterns
  • Inspect JSX resolution behavior
  • Inspect tests and fixtures
  • Inspect non-TypeScript and shared regression risk
  • Inspect package.json change
  • Inspect performance and scaling
  • Inspect validation
  • Inspect scope and maintainability
  • Produce findings and final verdict

Review bar for this PR

  • TypeScript HOF / arrow-callback calls are attributed to the correct enclosing function scope through pass2AttachDeclarationsresolveCallerGraphId on the registry-primary path.
  • Legacy DAG caller attribution no longer creates phantom function IDs from unparenthesized arrow parameters.
  • Object-property arrows are named consistently across registry-primary and legacy paths; computed/runtime keys remain anonymous.
  • JSX component usage emits exactly one CALLS edge per use site; lowercase HTML tags filtered at query level.
  • <Foo.Bar /> emits a member-call CALLS edge without a duplicate phantom ACCESSES edge.
  • const fn = () => {} exports resolve to the callable Function def, not the parallel Variable def, without breaking single-def or non-TypeScript behavior.
  • Shared gitnexus-shared finalize tie-break does not affect any non-TypeScript language.
  • Tests exercise the real registry-primary and legacy pipeline paths and would fail if each source change were independently reverted.

Understanding of the change

Issue #1166 reported that ~73% of Function nodes in real React+TanStack+Zustand TypeScript codebases had zero outgoing CALLS edges, making gitnexus_impact and gitnexus_context return meaningless results for hot symbols.

Root causes (four, addressed in two fix sets):

Registry-primary path (authoritative for TypeScript since it's in MIGRATED_LANGUAGES):

  1. HOF/arrow anchor misplacement. @declaration.function anchored on the outer lexical_declaration (col 0), so pass2AttachDeclarations.atPosition(startLine=0, startCol=0) resolved to the module scope, not the arrow's own scope. resolveCallerGraphId walking up from calls inside the arrow hit the module scope's ownedDefs and grabbed the first Function-like def — often the wrong one. Fixed by moving the anchor to the inner arrow_function / function_expression.
  2. findExportByName returning the Variable def. After (1), the Function def's anchor moved later in source, causing the Variable def (from @declaration.variable on the same lexical_declaration) to appear first in localDefs. Importers bound to the non-callable Variable. Fixed by a callable-preference tie-break.
  3. isCallerAnchorLabel accepting Variable. Module-level const X = expr() calls walked up to module scope, found the Variable def for X, resolved it by simpleKey to a sibling Function with the same name, and emitted a self-loop. Fixed by restricting valid caller anchors to Function/Method/Constructor/Class/Interface/Struct/Enum.
  4. JSX invisible. No jsx_* patterns in the query; every <Component /> in .tsx produced zero CALLS edges. Fixed by a TSX-grammar-only query suffix.

Legacy DAG path (runs in parallel for CI parity gate):

  • Bug A: genericFuncName returned the first identifier child of an arrow_function — for file => fn(file) that was the parameter file, creating a phantom Function file. Fixed by returning null for arrow_function/function_expression.
  • Bug B: tsExtractFunctionName only named arrows with variable_declarator parents. Pair-parent arrows (addItem: (item) => set(...)) were anonymous. Fixed by adding pair/property_assignment handling. Mirrored in both tree-sitter-queries.ts (legacy) and query.ts (registry-primary) to keep the parity gate green.

Shared contract changed: findExportByName in gitnexus-shared now prefers callable/type-like defs over Variable when the same simple name exists in localDefs. Single-def case unchanged; non-TypeScript languages unaffected (no dual-emit situation exists there).

Must remain unchanged: Plain .ts query compilation; non-TypeScript language behavior; ACCESSES for ordinary member reads; arity/overload resolution for non-JSX calls.


Findings

[medium] Pair-with-arrow @declaration.function anchored on pair, not inner arrow — multi-action stores misattribute calls in the registry-primary path

  • Category: Correctness — caller attribution, DoD §2.1

  • Files: gitnexus/src/core/ingestion/languages/typescript/query.ts:176–190

  • Issue: The four pair-with-arrow patterns anchor @declaration.function on the outer pair node:

    (pair
      key: (property_identifier) @declaration.name
      value: (arrow_function)) @declaration.function   ;; ← on pair, not arrow

    pass2AttachDeclarations.atPosition(pair.startLine, pair.startCol) resolves to the parent scope (e.g., the persist-callback (set) => ... scope), not the inner arrow's own scope, because the pair key starts before the arrow's @scope.function range. All pair-function defs from the same object literal therefore land in the same parent scope's ownedDefs. When resolveCallerGraphId walks up from calls inside any of those arrows it does scope.ownedDefs.find(d => d.type === 'Function'), which returns the first Function def in the parent scope — always the same one, regardless of which pair arrow the call originated in.

    Concrete regression: a Zustand slice with two or more actions (addItem, removeItem, fetchData) in the same persist((set) => ({ ... })) body will attribute calls inside removeItem and fetchData to addItem. The promoted Function nodes (Function:removeItem, Function:fetchData) exist in the graph but carry zero outgoing CALLS edges from the registry-primary path.

    The exact fix applied to const fn = () => {} (moving @declaration.function to the inner arrow_function) was intentionally NOT applied to the pair patterns, and the integration fixture uses single-pair objects (bump, queryFn) which mask the defect.

    Note: the legacy path (tsExtractFunctionName + findEnclosingFunctionId parent-walk) handles multi-pair correctly — the unit test at call-attribution-issue-1166.test.ts:243–264 asserts addItem → doSomething and fetchData → fetch both correctly. The two paths diverge for multi-pair objects; the parity gate does not catch this because it compares which Function nodes exist, not edge attribution.

  • Why it matters here: Zustand stores with 2+ actions are the dominant pattern cited in the bug report ("Zustand store files hit 0% CALLS-edge capture"). This PR claims to fix them, and in the Sourcerer-fe metrics table the umbrella PR promotes +375 object-property arrows to named Function nodes. But if those functions' CALLS edges are all mis-attributed to the first sibling, gitnexus_context("removeItem") still returns zero outgoing edges and gitnexus_impact("doB", direction:"upstream") still doesn't reach removeItem.

  • Recommended fix: Move @declaration.function from the outer pair to the inner arrow_function / function_expression in all 4 pair patterns — mirroring what was done for lexical_declaration:

    (pair
      key: (property_identifier) @declaration.name
      value: (arrow_function) @declaration.function)    ;; anchor on inner arrow
    
    (pair
      key: (property_identifier) @declaration.name
      value: (function_expression) @declaration.function)

    This places each pair-function def in its own arrow scope. The auto-hoist in pass2AttachDeclarations (rangesEqual(anchor.range, innermost.range)) will then hoist the name binding to the parent scope as usual, keeping lookup correct. Update the integration test expectation from useFeature → fetchData to queryFn → fetchData (or add queryFn as an intermediate). Add a multi-pair fixture to the HOF suite. Fix this →


[low] CI still in-progress — full validation unconfirmed

  • Category: Validation, DoD §4.2
  • Files: N/A
  • Issue: At review time both the CI run (25112171041) and Docker Build (25112171084) were in_progress — triggered at 2026-04-29T13:37:19Z, the merge commit from bb6aa0c8. No completed result is available. The previous run (25069944392) had a typecheck failure (formatting), resolved in 851d2ab. The PR's claim of 280/280 tests passing is from local per-file runs, which skip the Kotlin binding suite.
  • Why it matters here: DoD §4.2 requires cd gitnexus && npm test to pass for any gitnexus/ change. "Full CI is in-progress" is not the same as "CI is green." The new integration tests are in test/integration/resolvers/ and should be auto-discovered by Vitest in CI. JavaScript query changes in tree-sitter-queries.ts should be exercised by CI's full suite.
  • Recommended fix: Confirm CI green before merging. No code change needed.

[low] package.json adds two convenience scripts outside the PR's stated scope

  • Category: Scope, DoD §3 / §6 "Not-Done signals"
  • Files: package.json:10–11
  • Issue: gitnexus:refresh and gitnexus:full call gitnexus as a global command. These are developer-experience scripts, not related to the TypeScript scope-resolution fix. They assume gitnexus is globally installed (not npx gitnexus), diverging from the npx gitnexus analyze convention documented in AGENTS.md. The DoD explicitly flags diffs that contain changes outside the stated task scope.
  • Why it matters here: Minor — no correctness risk — but it's unrelated churn in a diff that should be surgical. The scripts belong in a separate docs/tooling PR.
  • Recommended fix: Remove from this PR and land separately.

Definition of Done check

DoD item Status
HOF/arrow calls attributed to the correct enclosing arrow scope (registry-primary pass2AttachDeclarationsresolveCallerGraphId) partially satisfiedconst fn = () => {} fully fixed; pair-with-arrow multi-action objects misattribute in the registry-primary path (see medium finding)
Legacy DAG phantom Function IDs from arrow parameters eliminated satisfiedgenericFuncName returns null for arrow_function/function_expression; unit tests pin both forms
Object-property arrows named consistently across registry-primary and legacy paths partially satisfied — single-pair objects correct on both paths; multi-pair attribution diverges in registry-primary
JSX emits exactly one CALLS edge per use; lowercase HTML filtered satisfied#match? @reference.name "^[A-Z]" predicate verified; paired JSX uses toHaveLength(1); closing tag not captured
<Foo.Bar /> emits member-call CALLS, no phantom ACCESSES satisfiedshouldEmitReadMember JSX cases correct; toEqual([]) assertion on ACCESSES
const fn = () => {} exports resolve to callable def satisfiedfindExportByName callable preference additive; cross-file integration test confirms Function def selected over Variable
Shared gitnexus-shared finalize behavior compatible with non-TypeScript languages satisfied — tie-break only fires when dual-emit exists (TypeScript-specific); single-def path unchanged
Tests prove each fix path and fail on independent revert partially satisfied — 5 of 7 changes have robust exact-value test coverage; pair-with-arrow multi-action misattribution is untested; CI unconfirmed

Validation assessment

Tests added/updated:

  • typescript-hof-callbacks.test.ts — 8 assertions, real runPipelineFromRepo path; covers single-pair Zustand, TanStack, Promise patterns
  • typescript-jsx-as-call.test.ts — 7 assertions with toHaveLength(1) / toEqual([]) exact guards; combined HOF+JSX probe
  • call-attribution-issue-1166.test.ts — 18 assertions over legacy DAG path; covers multi-pair correctly in legacy path
  • All fixtures are minimal and realistic; no network/clock dependencies

Claimed by PR:

  • 280/280 TypeScript-relevant tests (8+7+18+236+11) ✓ local
  • 613/613 scope-resolution/unit tests ✓ local
  • gitnexus + gitnexus-shared + gitnexus-web typecheck ✓
  • Pre-commit hook ✓
  • End-to-end Sourcerer-fe three-way before/after analysis

Verified from diff:

  • TSX grammar isolation is structurally sound — _tsQuery and _tsxQuery are separate singletons; jsx_* patterns only in TSX_GRAMMAR compilation; tsCachedTreeMatchesGrammar guards against grammar mismatch on cached trees
  • shouldEmitReadMember JSX cases use the correct childForFieldName('name')?.id pattern (same as the pre-existing call/new_expression cases)
  • findExportByName callable-preference loop correctly short-circuits on first callable match and saves first non-callable as fallback
  • isCallerAnchorLabel is a strict subset of the pre-existing walk's first-check pass; no new lookups
  • genericFuncName null-return for arrow/function_expression is correct; named function declarations still resolve via childForFieldName('name')
  • tsExtractFunctionName pair key resolution handles identifier, string-fragment, and computed-key (returns null) correctly
  • CI report from run 25069944392: 7681/7681 tests, typecheck failure (formatting) addressed in 851d2ab

Missing / unverifiable:

  • CI for current merge commit (25112171041) still in-progress — cannot confirm full suite with Kotlin binding present
  • gitnexus-shared npm run build (dist build) not explicitly confirmed — DoD §4.4 requires it
  • Multi-pair integration test missing — registry-primary path's pair-with-arrow misattribution for 2+ actions is neither tested nor documented as a known limitation
  • JavaScript behavior in tree-sitter-queries.ts — pair patterns added to JAVASCRIPT_QUERIES but no JS-specific integration test added; covered implicitly by shared infrastructure

Whether missing validation blocks production readiness: The CI in-progress is the primary gate. The multi-pair misattribution (medium finding) is a correctness gap in the authoritative path that should be addressed before merge — it would affect the most impactful use case (Zustand stores with 2+ actions) that the PR explicitly claims to fix.


Final verdict

not production-ready (one medium correctness issue requiring a fix before merge)

The registry-primary and JSX fixes are architecturally sound: the anchor move for const fn = () => {}, the findExportByName tie-break, isCallerAnchorLabel, and the TSX-only JSX suffix are all mechanically correct with good test coverage. The legacy path fixes are solid and the unit tests demonstrate correct multi-pair attribution there.

However, the four pair-with-arrow patterns in query.ts place @declaration.function on the outer pair node rather than on the inner arrow_function/function_expression. This causes all pair-function defs in a multi-action object to land in the same parent scope, and resolveCallerGraphId's ownedDefs.find() then attributes all calls to the first pair function. Multi-action Zustand stores — the largest class of "0% capture rate" files in the bug report — would still produce incorrect attribution for all but the first action in the registry-primary path. The fix is surgical (four patterns in query.ts, matching what was done for lexical_declaration), and a multi-pair fixture should be added to prove it. The package.json scope creep is minor and separately addressable. Confirm CI green and gitnexus-shared npm run build before final merge.


@magyargergo

Copy link
Copy Markdown
Collaborator

@ReidenXerx please look at these findings and ensure that the CI is healthy 🙏

@magyargergo

Copy link
Copy Markdown
Collaborator

@ReidenXerx thank you for your impressive work! We appreciate you taking on this challenge! 💪🏻

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
@ReidenXerx

Copy link
Copy Markdown
Contributor Author

@ReidenXerx please look at these findings and ensure that the CI is healthy 🙏

it would be really much easier for me if you could let me rerun ci pipelines by myself

@ReidenXerx

Copy link
Copy Markdown
Contributor Author

@ReidenXerx thank you for your impressive work! We appreciate you taking on this challenge! 💪🏻

thanks! i think your product is gem in bunch of people who tries sell useless "ai neighbourhood" services
and i really want to make it better

@magyargergo

Copy link
Copy Markdown
Collaborator

Come and join us on Discord and we can talk about the details ;)

Comment on lines +176 to +190
(pair
key: (property_identifier) @declaration.name
value: (arrow_function)) @declaration.function

(pair
key: (property_identifier) @declaration.name
value: (function_expression)) @declaration.function

(pair
key: (string (string_fragment) @declaration.name)
value: (arrow_function)) @declaration.function

(pair
key: (string (string_fragment) @declaration.name)
value: (function_expression)) @declaration.function

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same anchor placement as the lexical_declaration fix above (lines 154-169) needs to apply here. The @declaration.function capture currently sits on the outer pair node, so pass2AttachDeclarations lands the def in the parent scope's ownedDefs. For multi-action objects like { addItem: ..., removeItem: ... }, all defs end up in the same parent scope, and resolveCallerGraphId's ownedDefs.find() attributes every call to the first sibling. Single-pair fixtures (like bump) mask this because there is no ambiguity, but a 2+ action Zustand store would attribute calls inside removeItem to addItem.

Could you move @declaration.function to the inner arrow_function / function_expression (mirroring lines 154-169) and add a multi-pair fixture that asserts each action's calls attribute to its own name? Same shape as the existing bump fixture but with a second action calling something different.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 66ad5c49. Moved @declaration.function from the outer pair to the inner arrow_function / function_expression in all 4 pair patterns, mirroring the lexical_declaration block above. The rangesEqual(anchor.range, innermost.range) auto-hoist in pass2AttachDeclarations now does the right thing — def in the arrow's own scope, binding hoisted to the parent.

Multi-pair regression coverage:

  • New fixture multi-action-store.ts with three pair-arrows (addItem / removeItem / fetchData) and three top-level call targets (doA / doB / doC) — exactly the Zustand shape you described.
  • Four new assertions in typescript-hof-callbacks.test.ts: positive (addItem → doA, removeItem → doB, fetchData → doC), negative (no addItem → doB, no addItem → doC, no other cross-leaks), the exact-set invariant (full pair set is {addItem→doA, removeItem→doB, fetchData→doC}), and the regression fingerprint (addItem → doB MUST be []).

Also updated the useFeature → fetchData expectation to queryFn → fetchData per your "or add queryFn as an intermediate" suggestion. 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 — exactly the divergence from the legacy DAG you flagged. The useFeature → useQuery edge stays asserted (that call IS in useFeature's body).

Local validation on this commit:

  • REGISTRY_PRIMARY_TYPESCRIPT=1 vitest runtypescript-hof-callbacks (12, +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.
  • Same suite without the env flag (legacy DAG path): 886/886 pass — the tsExtractFunctionName pair-parent handling from fix(ts): CALLS edge collector attribution for HOF/callback patterns (#1166) #1179 was already correct for multi-pair attribution; this just brings the registry-primary path to parity.
  • npx prettier --check . clean on tracked files; npx tsc --noEmit clean; npx eslint clean on the touched files.

…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
@ReidenXerx

ReidenXerx commented Apr 30, 2026

Copy link
Copy Markdown
Contributor Author

Hey @abhigyanpatwari — thanks for the thorough review. Pushed 66ad5c49 addressing the medium finding plus a quick note on the two lows.

[medium] Pair-with-arrow anchor placement → fixed in 66ad5c49

You were exactly right — the four pair patterns anchored @declaration.function on the outer pair node instead of the inner arrow_function / function_expression, and the parity gate didn't catch it because it compares which Function nodes exist, not edge attribution. Single-pair fixtures (bump, queryFn) masked the defect because find() is deterministic over a single-element set.

Inline thread: #1175 (comment)

Summary of changes:

File Change
gitnexus/src/core/ingestion/languages/typescript/query.ts Moved @declaration.function from outer pair to inner arrow_function / function_expression in all 4 pair patterns. Added a long anchor-discipline comment explaining why (mirroring the lexical_declaration block above).
gitnexus/test/fixtures/lang-resolution/typescript-hof-callbacks/src/multi-action-store.ts New 3-action Zustand-shape fixture (addItem → doA, removeItem → doB, fetchData → doC) — the exact regression case the bug report flagged.
gitnexus/test/integration/resolvers/typescript-hof-callbacks.test.ts +4 multi-pair attribution tests (positive, negative, exact-set, regression fingerprint). Updated useFeature → fetchData expectation to the structurally-correct queryFn → fetchData.

Validation on 66ad5c49:

  • REGISTRY_PRIMARY_TYPESCRIPT=1 vitest run — typescript-hof-callbacks (12, +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.
  • Same suite without the env flag (legacy DAG path): 886/886 pass — your tsExtractFunctionName pair-parent handling already had multi-pair attribution correct on the legacy side; this just brings registry-primary to parity.
  • npx prettier --check ., npx tsc --noEmit, and npx eslint clean on the touched files.

[low] CI in-progress → should be green now

Both this commit and the previous ca3d5202 (which fixed the prettier warning on typescript.ts:107 from the #1179 merge) need maintainer "Approve and run". When it runs, the quality / format job that failed on ef96603f should now pass — verified locally with the exact npx prettier --check . command CI uses. Whenever you have a moment to approve, that'd be appreciated.

(Re your "let me rerun" comment from earlier — totally agreed, having to poke you for every CI run is a pain on both sides. The first-time-contributor gate seems to re-prompt on every push. Once one of these gets merged the gate should drop.)

[low] package.json scope creep → I'll separate

The gitnexus:refresh and gitnexus:full scripts in ef96603f are unrelated to the TypeScript fix — i decided to include it because i use it on all my projects


Net result: the four pair-arrow patterns now anchor exactly like the lexical_declaration patterns above them, multi-action Zustand stores attribute calls per-action on the registry-primary path, and the parity gate stays green because the legacy path was already correct.

@abhigyanpatwari abhigyanpatwari merged commit 883091f into abhigyanpatwari:main Apr 30, 2026
19 of 20 checks passed
magyargergo pushed a commit that referenced this pull request May 3, 2026
…mo / useCallback / useMemo / observer) (#1261)

* fix(typescript): name HOC-wrapped const declarations (forwardRef / memo / useCallback / useMemo / observer / debounce)

Follow-up to issue #1166 / PR #1175. After fixing HOF callbacks (Promise
fan-out, queryFn pair-arrows, multi-action Zustand stores) and JSX-as-call,
the dominant residual 0%-capture pattern in real React UI codebases was
the HOC-wrapped variable declaration:

  const Button = React.forwardRef((props, ref) => { ... })
  const Card = memo((props) => { ... })
  const handleClick = useCallback(() => { ... }, [])
  const computed = useMemo(() => { ... }, [])
  const debouncedSearch = debounce((q) => { ... }, 250)

All share the AST shape `lexical_declaration > variable_declarator >
call_expression > arguments > arrow_function`. Pre-fix, neither the
registry-primary `query.ts` nor the legacy `tree-sitter-queries.ts` had
a `@declaration.function` pattern matching this shape, and the legacy
DAG's `tsExtractFunctionName` only walked `variable_declarator` and
`pair` parents — `arguments` parents fell through with `funcName = null`.

Result: every shadcn/Radix component, every memoised React component,
and every `useCallback` / `useMemo` callback bound to a const registered
as anonymous; calls inside attributed to the file. Sourcerer-fe audit:
~296 declarations affected (~57 forwardRef + ~21 memo + ~161 useCallback
+ ~57 useMemo).

Fix:
  - 4 new tree-sitter patterns in `languages/typescript/query.ts`
    (registry-primary), anchored on the inner arrow_function /
    function_expression — same anchor discipline as the existing
    `lexical_declaration` and `pair` patterns from PR #1175.
  - 8 mirrored patterns in `tree-sitter-queries.ts` (4 in
    TYPESCRIPT_QUERIES, 4 in JAVASCRIPT_QUERIES) for the legacy DAG
    and the CI parity gate.
  - New `arguments`-parent branch in `tsExtractFunctionName` that
    walks `arguments → call_expression → variable_declarator` and
    returns the const's name. Three guards keep it strictly scoped
    to HOC-wrapped declarations; bare statement-level HOC calls fall
    through anonymous.

Tests:
  - 11 integration tests + 9 minimal TS/TSX fixtures exercising
    forwardRef / memo / useCallback / useMemo / observer / debounce,
    with positive (named-Function + correct CALLS edge), negative
    (no phantom Functions for unbound HOCs, no phantom self-loops,
    no first-sibling-wins leakage), and cross-pollination assertions.
  - 8 new unit tests in `call-attribution-issue-1166.test.ts`
    pinning the legacy-DAG path: 6 attribution tests + 2
    @definition.function capture tests.

Trade-off documented inline: chained array-method declarations
(`const x = arr.find((y) => p(y))`) match the same shape and produce
a mostly-harmless phantom `Function:x` with one outgoing edge. The
false-positive cost is negligible vs. the React UI coverage gain.

Verification: - 11/11 typescript-hoc-wrapped (registry-primary)
  - 26/26 call-attribution-issue-1166 (8 new + 18 pre-existing)
  - 266/266 across all 4 typescript resolver test files (registry)
  - 236/236 typescript.test.ts on legacy DAG (CI parity gate)
  - 1693/1693 across all non-Kotlin/Swift resolver test files
  - tsc --noEmit clean; prettier clean; eslint clean (no new warnings)
Co-authored-by: Cursor <cursoragent@cursor.com>

* test(typescript): pin documented HOC trade-offs and close var-form parity gap

Addresses the four findings on PR #1261 (Claude bot review for #1261).
All findings flagged missing assertion tests for behaviour already documented
in code comments — none reported a real bug. The verdict was
"production-ready with minor follow-ups"; these tests strengthen the
documentation-to-test contract.

[medium #1] Array-method false-positive
  Pin `const found = items.find((item) => predicate(item))` →
  `predicate.attributedTo === 'found'` as an accepted FP. The const is a
  value, never invoked, so no incoming CALLS edge ever points at it; the
  outgoing edge is a minor mis-attribution we accept rather than maintain
  a HOC allowlist.

[medium #2] Nested HOCs (`memo(forwardRef(...))`) — no phantom Function:Wrapped
  Two integration tests in `typescript-hoc-wrapped.test.ts`:
    1. `Wrapped` is NOT a Function node (the outer call's first arg is a
       call_expression, not an arrow — no @declaration.function pattern
       matches the outer shape).
    2. The deepest arrow's `helper()` call is NOT attributed to
       Function:Wrapped (the deepest arrow is anonymous because
       call_expression.parent is `arguments`, not `variable_declarator`),
       and no Function-sourced CALLS originate from `nested.tsx`.

[medium #3] Multi-arrow argument dedup
  Pin `const x = call(() => first(), () => second())` — both arrows share
  the same `arguments → call_expression → variable_declarator` ancestor
  chain on the legacy DAG, so both attribute to "x". Documents the
  registry-primary dedup story alongside.

[low #4] `var X = HOC(...)` parity gap
  Registry-primary `query.ts` had `(variable_declaration ...)` HOC patterns
  but legacy `tree-sitter-queries.ts` (TS + JS) did not. Closes the gap by
  mirroring two `(variable_declaration ...)` HOC patterns into both legacy
  sections so the parity gate stays tight even if a codebase mixes
  `var X = HOC(...)` with `const X = HOC(...)`.

Validation
  - Targeted: 41/41 (28 unit + 13 integration) on registry-primary.
  - Broader TS suite: 60/60 across 4 resolver test files.
  - CI parity gate (`typescript.test.ts`): 236/236 on legacy DAG and 236/236
    on registry-primary.
  - Prettier clean. ESLint clean (5 pre-existing non-null-assertion
    warnings in the test file, unrelated). tsc --noEmit clean.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CALLS edge collector misses ~75% of functions; concentrated in HOF/callback patterns

3 participants