Skip to content

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

Merged
magyargergo merged 2 commits into
abhigyanpatwari:mainfrom
ReidenXerx:fix/typescript-hoc-wrapped-components
May 3, 2026
Merged

fix(typescript): name HOC-wrapped const declarations (forwardRef / memo / useCallback / useMemo / observer)#1261
magyargergo merged 2 commits into
abhigyanpatwari:mainfrom
ReidenXerx:fix/typescript-hoc-wrapped-components

Conversation

@ReidenXerx

@ReidenXerx ReidenXerx commented May 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes the dominant remaining 0%-capture pattern from the original
issue #1166 audit: HOC-wrapped variable declarations like
const Button = React.forwardRef((props, ref) => { ... }),
const Card = memo((props) => ...),
const handleClick = useCallback(() => ..., []),
const computed = useMemo(() => ..., []). Adds the missing
tree-sitter patterns + extends tsExtractFunctionName so each wrapped
arrow becomes a named Function node with calls inside its body
correctly attributed to the const, not the file.

Motivation / context

Follow-up to PR #1175 (issue #1166), which fixed three HOF /
callback families (Promise.all(.map(...)), useQuery({ queryFn: ... }) pair-arrows, Zustand multi-action stores, JSX-as-call). After
that landed and Sourcerer-fe was re-indexed, the residual 0%-capture
audit identified one more dominant pattern: HOC-wrapped const
declarations
. They share a common AST shape

lexical_declaration
  variable_declarator
    name: identifier "X"                ← we want this name
    value: call_expression
      function: identifier | member_expression  ← any HOC callee
      arguments: arguments
        arrow_function | function_expression    ← actual code

— and pre-fix none matched any @declaration.function pattern in
either the registry-primary query.ts or the legacy
tree-sitter-queries.ts. The legacy DAG’s tsExtractFunctionName
only walked variable_declarator and pair parents — arguments
parents fell through with funcName = null, so calls inside the
arrow walked up past the empty arrow scope and attributed to the
File.

Pattern coverage in real React UI codebases (Sourcerer-fe audit,
~525 source files, post-PR-#1175):

Wrapper Affected const X = ... declarations
React.forwardRef / forwardRef ~57
React.memo / memo ~21
useCallback ~161
useMemo ~57
Total HOC-wrapped consts ~296

All ~296 declarations registered as anonymous arrows pre-fix. Every
function call inside their bodies (cn(...), helper(...),
imported callee invocations, render-time JSX) attributed to the
file, making gitnexus_context / gitnexus_impact blind to the
outgoing call surface of every shadcn/Radix component, every
memoised React component, and every hook callback. The dominant UI
authoring style for modern React/TypeScript codebases.

This PR closes the gap.

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.)

Scope & constraints

In scope

  • 4 new tree-sitter patterns in
    gitnexus/src/core/ingestion/languages/typescript/query.ts
    (registry-primary path) covering lexical_declaration /
    variable_declaration × arrow_function / function_expression
    inside a call_expression’s arguments. Anchored on the inner
    arrow (same anchor discipline as the existing lexical_declaration
    and pair patterns from PR fix(typescript): capture missed CALLS edges from HOF callbacks and JSX #1175) so each wrapped arrow becomes
    its own caller anchor.
  • 8 mirrored patterns in
    gitnexus/src/core/ingestion/tree-sitter-queries.ts (4 in
    TYPESCRIPT_QUERIES, 4 in JAVASCRIPT_QUERIES) — required for
    the legacy Call-Resolution DAG and the CI parity gate.
  • New arguments-parent branch in tsExtractFunctionName
    (gitnexus/src/core/ingestion/languages/typescript.ts) that
    walks arguments → call_expression → variable_declarator and
    returns the const’s name. Mirrors the resolver fix at the function
    -name layer so findEnclosingFunctionId (parse-worker) and
    findEnclosingFunction (call-processor) agree on attribution.
  • 11 integration tests
    (gitnexus/test/integration/resolvers/typescript-hoc-wrapped.test.ts)
    • 9 minimal TS/TSX fixtures
      (gitnexus/test/fixtures/lang-resolution/typescript-hoc-wrapped/)
      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
      (two HOC-wrapped consts in the same module attribute independently)
      assertions.
  • 8 new unit tests appended to
    gitnexus/test/unit/call-attribution-issue-1166.test.ts pinning
    the legacy-DAG path: 6 tsExtractFunctionName attribution tests
    (bare/member-expression callees, no first-sibling-wins, unbound
    HOC negative) + 2 @definition.function capture tests (all wrappers
    • function-expression form).

Explicitly out of scope / not done here

  • Deeply-nested HOCs like const X = memo(forwardRef((p, r) => ...)). The deepest behaviour-arrow is two levels below the outer
    variable_declarator (its arguments parent’s call_expression
    parent is itself an arguments child, not a variable_declarator),
    so neither the new patterns nor the new resolver branch match it.
    nested.tsx documents the limitation and asserts no regression
    (no phantom edges introduced). A future fix would need a recursive
    walk-up through arguments → call_expression chains and a
    policy choice for which level wins the name.
  • HOC-wrapped const NOT bound to a const/let/var — bare
    statement-level calls (useCallback(() => ..., []) discarded as a
    side-effect statement) intentionally remain anonymous and fall
    back to File-level attribution. Covered by the
    negative-bare-call.ts fixture and the
    does NOT name a bare statement-level HOC call unit test.
  • Pre-existing unrelated UI-build typing issues in
    gitnexus-web/src/components/GraphCanvas.tsx and RightPanel.tsx
    (React 19 RefObject<T | null> vs LegacyRef<T> mismatches). They
    exist on main already and are orthogonal to this fix.
  • Non-TypeScript languages. Only TS / JS are touched. Other
    language providers (Python, C#, Java, Go, …) define their own
    function-name resolution and are unaffected.

Implementation notes

Pattern shape — anchor discipline

The four registry-primary patterns follow the same anchor-on-inner-
arrow discipline established by PR #1175 for the lexical_declaration
and pair patterns:

(lexical_declaration
  (variable_declarator
    name: (identifier) @declaration.name
    value: (call_expression
      arguments: (arguments
        (arrow_function) @declaration.function))))

@declaration.function sits on the inner arrow_function, NOT
the outer lexical_declaration or call_expression. The arrow’s
range matches its own @scope.function range, so
pass2AttachDeclarations.atPosition resolves innermost to the
arrow’s own scope and rangesEqual(anchor.range, innermost.range)
triggers the auto-hoist that promotes the binding to the parent
scope (where the const X declaration lives). Without this
discipline, the def lands in the parent scope’s ownedDefs and
calls inside the arrow walk up past the empty arrow scope, losing
attribution.

Trade-off — chained array-method false positive

The patterns intentionally don’t constrain the wrapping callee.
That accepts:

  • const Button = React.forwardRef((p, r) => ...)
  • const Card = memo((p) => ...)
  • const handleClick = useCallback(() => ..., [])
  • const debouncedSearch = debounce((q) => ..., 250)

…but also matches:

  • const x = arr.find((y) => predicate(y))

— which would name the .find callback as x. The resulting
Function:x is mostly harmless: x is consumed as a value
(if (x) { ... }), never invoked as a function, so it accumulates
zero incoming CALLS edges. The one outgoing edge Function:x → predicate is a minor mis-attribution that could be filtered with a
known-array-method blocklist (.map / .filter / .find / .reduce
/ .forEach / .some / .every) — but (a) that blocklist needs
maintenance, (b) any user-defined fluent-API method with a callback
arg would still false-positive, and (c) the false-positive cost is
negligible vs. the much larger benefit of capturing forwardRef /
memo / useCallback for React UI codebases. See the long comment
block above the patterns in
gitnexus/src/core/ingestion/languages/typescript/query.ts for the
full rationale.

Trade-off — multi-arrow arguments

const x = call(arrow1, arrow2) would emit two matches with the
same name x (one per arrow). pass2AttachDeclarations pushes both
Function:x defs into their respective arrow scopes (each in its
own ownedDefs) and hoists both bindings to the parent. The
downstream registry’s qualified-name dedup collapses them via
(filePath, type, qualifiedName) — second wins. Acceptable;
multi-arrow-callback APIs are rare (new Promise(executor) is the
main one and takes a single executor).

Resolver branch

tsExtractFunctionName’s third branch:

if (parent.type === 'arguments') {
  const callExpr = parent.parent;
  if (!callExpr || callExpr.type !== 'call_expression') return { funcName: null, label: 'Function' };
  const declarator = callExpr.parent;
  if (!declarator || declarator.type !== 'variable_declarator') return { funcName: null, label: 'Function' };
  const nameNode = declarator.childForFieldName?.('name');
  if (nameNode?.type === 'identifier') return { funcName: nameNode.text, label: 'Function' };
  return { funcName: null, label: 'Function' };
}

Three guards (must be argumentscall_expression
variable_declarator) keep this strictly scoped to HOC-wrapped
declarations. Bare statement-level HOC calls (parent chain ends in
expression_statement / program) hit the variable_declarator
guard and return null, so they correctly fall through anonymous.

Testing & verification

  • cd gitnexus && npx vitest run test/integration/resolvers/typescript-hoc-wrapped.test.ts
    11/11 passed in 2.2s

  • cd gitnexus && npx vitest run test/unit/call-attribution-issue-1166.test.ts
    26/26 passed (8 new HOC tests + 18 pre-existing) in 1.0s

  • cd gitnexus && npx vitest run test/integration/resolvers/typescript.test.ts test/integration/resolvers/typescript-hof-callbacks.test.ts test/integration/resolvers/typescript-jsx-as-call.test.ts test/integration/resolvers/typescript-hoc-wrapped.test.ts
    266/266 passed on registry-primary (default) in ~38s

  • cd gitnexus && REGISTRY_PRIMARY_TYPESCRIPT=0 npx vitest run test/integration/resolvers/typescript.test.ts
    236/236 passed on legacy DAG (CI parity gate) in ~45s

  • cd gitnexus && npx vitest run test/integration/resolvers/{typescript*,javascript,python,csharp,cpp,java,go,php,ruby,rust,dart,cobol,vue}*.test.ts
    1693/1693 passed across all non-Kotlin/Swift languages in ~74s (Kotlin/Swift skipped — optional native bindings not installed locally per AGENTS.md § Gotchas)

  • cd gitnexus && npx tsc --noEmit → clean

  • cd gitnexus && npx prettier --check <touched files> → clean

  • cd gitnexus && npx eslint <touched files> → clean (5 pre-existing
    Forbidden non-null assertion warnings in the unit-test file
    matching its established style; no new errors)

  • Live re-index on Sourcerer-fe — confirmed empirical impact.

    Sourcerer-fe @ commit 9684e13, 525 source files, indexed
    twice: once via published gitnexus@1.6.3 (pre-PR-fix(typescript): capture missed CALLS edges from HOF callbacks and JSX #1175,
    pre-this-PR baseline), once via this branch (post-PR-fix(typescript): capture missed CALLS edges from HOF callbacks and JSX #1175 +
    post-this-PR; combined effect):

    Metric Pre (1.6.3) Post (this branch) Δ
    Total nodes 6,008 7,013 +1,005 (+16.7%)
    Total edges 10,832 13,416 +2,584 (+23.9%)
    Function nodes 1,197 2,133 +936 (+78.2%)
    Functions with ≥ 1 outgoing CALLS 478 756 +278 (+58.2%)
    Total CALLS edges 1,308 2,326 +1,018 (+77.8%)
    … from Function (correctly attributed) 1,154 (88.2%) 2,179 (93.7%) +1,025 (+88.8%)
    … from Method 82 (6.3%) 119 (5.1%) +37
    … from File (the smoking gun) 72 (5.5%) 27 (1.2%) −45 (−62.5%)
    ACCESSES edges 68 447 +379 (+557%)
    DEFINES edges 4,887 5,823 +936 (matches Functions)

    Headline takeaways (combined effect of PR fix(typescript): capture missed CALLS edges from HOF callbacks and JSX #1175 + this PR):

    1. +936 named Function nodes unlocked — the entire
      React-UI primitive surface that was previously invisible to
      gitnexus_context / gitnexus_impact.
    2. +1,018 CALLS edges captured. The “correctly attributed”
      CALLS share (Function-rooted) climbs from 88.2% → 93.7%.
    3. File-rooted CALLS shrink by 63% (72 → 27): calls that
      previously fell through to the file as a fallback are now
      attributed to the named function whose body they live in.

    Concrete spot-check — shadcn/Radix UI primitives

    Sample of new Function nodes in src/components/ui/ (none
    registered as Function pre-fix; the surrounding const X = ...
    was a Variable and the inner arrow was anonymous):

    | name              | file                                |
    |-------------------|-------------------------------------|
    | AvatarFallback    | src/components/ui/avatar.tsx        |
    | AvatarImage       | src/components/ui/avatar.tsx        |
    | Avatar            | src/components/ui/avatar.tsx        |
    | handleClick       | src/components/ui/back-button.tsx   |  ← useCallback
    | goToRoute         | src/components/ui/back-button.tsx   |  ← useCallback
    | BackButton        | src/components/ui/back-button.tsx   |
    | Badge             | src/components/ui/badge.tsx         |
    | Button            | src/components/ui/button.tsx        |  ← React.forwardRef
    | Calendar          | src/components/ui/calendar.tsx      |
    | IconLeft / Right  | src/components/ui/calendar.tsx      |
    | CardHeader        | src/components/ui/card.tsx          |
    | CardContent       | src/components/ui/card.tsx          |
    | CardDescription   | src/components/ui/card.tsx          |
    | CardTitle         | src/components/ui/card.tsx          |
    

    And the Button → cn outgoing CALLS edge is now resolved — the
    exact edge our integration test (forwardRef test in
    typescript-hoc-wrapped.test.ts) asserts post-fix and which the
    pre-fix gitnexus@1.6.3 binary does not produce.

    Micro-validation against this PR’s test fixture — same
    typescript-hoc-wrapped/ fixture (9 files, 7 HOC-wrapped
    consts), copied to a tiny git repo and indexed via the pre-fix
    gitnexus@1.6.3:

    Metric Pre-fix (global 1.6.3) Post-fix (this PR)
    Function nodes for HOC consts 0 7
    CALLS edges (total) 3 ≥ 11 (asserted)
    … sourced from Function 0 ≥ 11
    … sourced from File 3 0 for HOC files
    Button → cn absent present
    handleClick → doStuff absent present
    computed → doStuff absent present

    The pre-fix binary captures only 3 CALLS edges total on the
    whole fixture, all File-rooted, and the targets are the
    wrappers themselves (debounce, wrap, observer) — every
    Button → cn, Card → helper, handleClick → doStuff, etc.
    edge is absent pre-fix and present post-fix.

Risk & rollout

Risk: low-to-medium. The new query patterns and the resolver
branch are strictly additive — pre-existing patterns (plain
variable_declarator > arrow_function, pair > arrow_function,
function declarations, methods) are untouched. The only behaviour
change is for arrows whose parent is arguments and whose
arguments.parent.parent is a variable_declarator — pre-fix those
returned null/anonymous, post-fix they return the const’s name.

Known false-positive surface: chained array-method declarations
(const x = arr.find((y) => predicate(y))) get a phantom
Function:x with one outgoing edge Function:x → predicate.
Documented in query.ts and accepted as an acceptable trade-off
(see Implementation notes). No incoming-edge consequences.

Rollout:

  • No migration needed.
  • Indexed repos should re-run npx gitnexus analyze to pick up the
    new Function nodes and edges. Existing graphs are forward-
    compatible (no schema change).
  • Both registry-primary and legacy DAG paths are updated, so the CI
    parity gate
    (.github/workflows/ci-scope-parity.yml) holds for TypeScript.

Release notes (suggested entry):

[typescript-resolution] HOC-wrapped const declarations
(forwardRef, memo, useCallback, useMemo, observer,
debounce, etc.) now register as named Function nodes. Calls
inside their bodies attribute to the const, not the file. Estimated
impact on React/TypeScript codebases: ~hundreds of additional
CALLS edges per medium-sized UI repo (Sourcerer-fe audit: ~296
declarations affected).

Checklist

  • PR body meets repo minimum length
  • No AGENTS.md / overlay changes (kept out of this PR)
  • No secrets, tokens, or machine-specific paths committed

…mo / useCallback / useMemo / observer / debounce)

Follow-up to issue abhigyanpatwari#1166 / PR abhigyanpatwari#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 abhigyanpatwari#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>
@vercel

vercel Bot commented May 1, 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.

@ReidenXerx ReidenXerx marked this pull request as draft May 1, 2026 19:20
@github-actions

github-actions Bot commented May 1, 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
7843 7842 0 1 369s

✅ All 7842 tests passed

1 test(s) skipped — expand for details
  • 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.28% 23098/29887 77.02% 📈 +0.3 🟢 ███████████████░░░░░
Branches 66.17% 14812/22384 65.97% 📈 +0.2 🟢 █████████████░░░░░░░
Functions 82.31% 2271/2759 81.86% 📈 +0.5 🟢 ████████████████░░░░
Lines 80.17% 20843/25998 79.89% 📈 +0.3 🟢 ████████████████░░░░

📋 View full run · Generated by CI

@ReidenXerx

Copy link
Copy Markdown
Contributor Author

Live re-index of Sourcerer-fe (525 source files, post-PR-#1175 + this PR's HOC fix vs. pre-PR-#1175 baseline) — combined uplift on the React UI codebase that originally surfaced this regression in the bug report:

Metric Pre (1.6.3) Post (this branch) Δ
Function nodes 1,197 2,133 +936 (+78%)
CALLS edges (total) 1,308 2,326 +1,018 (+78%)
CALLS from Function (correctly attributed) 1,154 (88.2%) 2,179 (93.7%) +1,025
CALLS from File (smoking gun) 72 (5.5%) 27 (1.2%) −45 (−63%)
ACCESSES edges 68 447 +379 (+557%)

Concrete spot-check — every shadcn/Radix UI primitive in src/components/ui/ (Button, Avatar, BackButton, Badge, Calendar, Card*, etc.) is now a named Function with resolved outgoing CALLS, where pre-fix they were anonymous and their inner calls fell through to File-level attribution. The exact edge our integration test asserts (Button → cn from React.forwardRef((props, ref) => { const cls = cn(...); ... })) is now present in the live graph.

PR description updated with the full breakdown including a per-table comparison and the micro-validation against the typescript-hoc-wrapped/ fixture.

@ReidenXerx ReidenXerx marked this pull request as ready for review May 1, 2026 19:32
@github-actions

github-actions Bot commented May 2, 2026

Copy link
Copy Markdown
Contributor

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


PR #1261 Code Review — HOC-wrapped const declarations


Review bar for this PR

  • HOC-wrapped const declarations (forwardRef, memo, useCallback, useMemo, observer, debounce) must produce named Function nodes.
  • Calls inside the wrapped arrow/function expression must attribute to the named Function, not to File.
  • Registry-primary (query.ts) and legacy (tree-sitter-queries.ts) TypeScript paths must produce equivalent Function nodes for HOC-wrapped consts.
  • Legacy JavaScript JAVASCRIPT_QUERIES must be updated with parity patterns; tsExtractFunctionName (shared for JS) must apply the same arguments branch.
  • Bare statement-level HOC calls (result not bound to a const) must remain anonymous / File-attributed.
  • Nested HOCs must not introduce phantom edges or wrong attribution; known limitation must be explicitly tested, not just documented.
  • Existing TypeScript HOF, pair-arrow, JSX-as-call, Promise executor, Zustand, and TanStack patterns must not regress.

Issue and PR context

Issue #1166 identified that ~75% of CALLS edges were missing in TypeScript HOF/callback-heavy codebases. The root causes were: genericFuncName borrowing parameter identifiers as function names; and tsExtractFunctionName only walking variable_declarator / pair parents.

PR #1175 fixed pair-arrow naming, Promise fan-out, TanStack queryFn, and Zustand multi-action stores by adding pair-parent branch to tsExtractFunctionName and mirrored patterns in both query files.

This PR (#1261) closes the remaining dominant blind spot from the same audit: const X = HOC((args) => { ... }). The residual pattern shared one AST shape — lexical_declaration → variable_declarator → call_expression → arguments → arrow_function — that matched no @declaration.function capture and whose arguments-parent arrow fell through tsExtractFunctionName with funcName = null. In Sourcerer-fe (~525 files), ~296 declarations were affected.

Changed files (14):

  • gitnexus/src/core/ingestion/languages/typescript.ts — new arguments branch in tsExtractFunctionName
  • gitnexus/src/core/ingestion/languages/typescript/query.ts — 4 new registry-primary patterns
  • gitnexus/src/core/ingestion/tree-sitter-queries.ts — 8 new legacy patterns (4 TS + 4 JS)
  • 9 fixture files + 1 new integration test + 1 extended unit test

CI status: All checks passed on head SHA 8d2eae8. 7772/7773 tests passed; 1 documented skip (Ruby block param, pre-existing). Code coverage improved across all metrics. Docker green. Typecheck, Prettier, ESLint clean.

Known tradeoffs documented:

  1. Broad pattern matches const x = arr.find((y) => p(y)) → phantom Function:x → p.
  2. Multi-arrow calls (const x = call(a1, a2)) emit duplicate Function:x; second wins dedup.
  3. Nested HOCs (const X = memo(forwardRef(...))) — deepest arrow remains anonymous.

Understanding of the change

Why arrows inside arguments were anonymous before this PR:
tsExtractFunctionName walked arrow_function.parent and only had two branches: variable_declarator (direct const fn = () => {}) and pair (object property arrows). When the arrow's parent was arguments — as it is for every HOC-wrapped declaration — both branches missed and the function returned null, leaving the arrow anonymous. The findEnclosingFunctionId walk then continued up past the empty arrow scope into the module scope and selected the file as the caller anchor.

Fix structure:

  • A third if (parent.type === 'arguments') branch walks: arrow.parent → arguments → call_expression → variable_declarator → identifier and returns the const's name. Three guards ensure this is strictly scoped: the parent must be arguments, its parent call_expression (not new_expression), and that expression's parent must be variable_declarator. This correctly excludes Promise executor callbacks (new_expression guard), bare statement-level calls (expression_statement guard), and doubly-nested HOC arrows (intermediate arguments guard).
  • Registry-primary query.ts adds 4 patterns anchored on the inner arrow_function / function_expression (same anchor discipline as PR fix(typescript): capture missed CALLS edges from HOF callbacks and JSX #1175's pair patterns), covering lexical_declaration × variable_declaration × 2 function types.
  • Legacy tree-sitter-queries.ts adds 4 TS patterns + 4 JS patterns, including export_statement-wrapped forms for exported HOC consts.

Findings

[medium] Array-method false-positive is documented but has no assertion test

  • Category: Test coverage / false-positive risk
  • Files: gitnexus/src/core/ingestion/languages/typescript/query.ts, gitnexus/test/unit/call-attribution-issue-1166.test.ts
  • Issue: const found = items.find((item) => predicate(item)) matches the new HOC pattern and produces Function:found → predicate. This is documented in an extensive comment block in query.ts and accepted as a trade-off. However, there is no assertion anywhere — positive or negative — that tests this behavior. The existing unit test at line 119 (const x = files.map(file => processFile(file))) only checks genericFuncName(arrow!) = null in isolation; it does not run collectCallAttributions on this snippet, so it does not exercise the arguments branch at all. If someone runs collectCallAttributions('const found = items.find((item) => predicate(item));'), predicate will attribute to found. That is the accepted false-positive, but it is not pinned.
  • Why it matters here: gitnexus_context("found") would now show a Function node with outgoing CALLS, potentially confusing impact analysis for code that uses a common array-method callback assignment pattern. The PR description correctly assesses this as "mostly harmless" (no incoming CALLS because found is never invoked as a function), but the assessment is untested.
  • Recommended fix: Add one unit test that calls collectCallAttributions on const x = items.find((item) => predicate(item)) and explicitly asserts predicate.attributedTo === 'x' with a comment like // accepted false-positive: array-method callback named after the binding. This pins the accepted behavior and prevents a future reader from being surprised.
  • Blocks merge: No — the trade-off is documented and proportionate. This is a follow-up.

Fix this →


[medium] Nested HOC fixture (nested.tsx) exists but is not exercised by any integration test

  • Category: Test coverage / regression safety
  • Files: gitnexus/test/fixtures/lang-resolution/typescript-hoc-wrapped/src/nested.tsx, gitnexus/test/integration/resolvers/typescript-hoc-wrapped.test.ts
  • Issue: The PR description claims nested.tsx "asserts no regression (no phantom edges introduced)." However, the integration test file (typescript-hoc-wrapped.test.ts) has no test case that references Wrapped, nested.tsx, or the memo(forwardRef(...)) shape. The fixture exists, is parsed as part of the test suite's runPipelineFromRepo fixture directory, and the Wrapped const will appear in the graph — but no assertion checks what its label is, what its CALLS look like, or that no phantom Function:Wrapped node was created. The claim in the PR body is unsupported by code.
  • Why it matters here: The nested HOC pattern is the next level of complexity after this fix. If a future change accidentally starts matching the nested shape (or matching the wrong arrow), there is no test that would catch it. The DoD bar requires that tests would fail if the changed behavior regressed.
  • Recommended fix: Add one integration test: assert that Wrapped is NOT in the set of Function nodes (it should be a Variable or not appear at all), and that any CALLS edges in nested.tsx source from File, not from Function:Wrapped. This directly tests the documented current behavior ("deepest arrow stays anonymous, calls attribute to File").
  • Blocks merge: No — out-of-scope limitation is acceptable. But the test gap means the written documentation is the only evidence.

Fix this →


[medium] Multi-arrow argument dedup behavior is documented but not tested

  • Category: Test coverage / attribution correctness
  • Files: gitnexus/src/core/ingestion/languages/typescript/query.ts (comment block), integration tests
  • Issue: The PR documents that const x = call(() => first(), () => second()) would emit two Function:x candidates and the registry dedup collapses them (second wins). The consequence is that outgoing CALLS from the first arrow could be associated with the second arrow's range in the dedup collision. No test exists that: (a) verifies both arrows are found, (b) asserts dedup behavior, or (c) confirms CALLS from both (or only one) attribute to x. The claim "acceptable; multi-arrow-callback APIs are rare" is correct but relies entirely on reasoning, not evidence.
  • Why it matters here: new Promise((resolve, reject) => { doA(); }, ignoredSecondArg) is the most plausible multi-arrow case and is already safe (it uses new_expression, not call_expression). But user-defined register(setup, teardown) patterns where both arrows are assigned are real. If doA() (first arrow) disappears from the graph because the Function:x node was overwritten by the second arrow's range, that's a silent attribution loss.
  • Recommended fix: Add one unit test with two HOC arrows bound to the same const and assert either: (a) both arrows' calls attribute to x, OR (b) only one does with an explicit comment explaining which wins and why. This documents the actual behavior and enables regression detection.
  • Blocks merge: No — rare pattern, documented as accepted.

[low] variable_declaration HOC patterns in registry-primary lack mirror in legacy TS/JS queries

  • Category: Registry-primary / legacy parity
  • Files: gitnexus/src/core/ingestion/languages/typescript/query.ts, gitnexus/src/core/ingestion/tree-sitter-queries.ts
  • Issue: Registry-primary query.ts adds (variable_declaration ...) HOC patterns (covering var X = HOC(...)). Legacy TYPESCRIPT_QUERIES and JAVASCRIPT_QUERIES add only (lexical_declaration ...) and (export_statement declaration: (lexical_declaration ...)) HOC patterns. var declarations are not mirrored. This is a minor divergence.
  • Why it matters here: var in modern TypeScript/React codebases is essentially absent. The CI parity gate (REGISTRY_PRIMARY_TYPESCRIPT=0) runs the legacy path against the same TypeScript fixtures, all of which use const. No parity failure is expected. However, if a codebase does use var X = HOC(...), registry-primary would produce a Function:X that the legacy path would not — a subtle inconsistency.
  • Recommended fix: Either add matching (variable_declaration ...) HOC patterns to legacy TS/JS queries, or add a comment in the legacy query file explicitly noting the intentional omission (since var is deprecated and the fixture coverage is complete without it).
  • Blocks merge: No.

Definition of Done check

DoD item Status
HOC-wrapped consts produce named Function nodes ✅ satisfied — 7 fixtures tested in integration, unit tests verify @definition.function capture
Calls inside wrapped arrow attribute to named function, not File ✅ satisfied — integration tests assert CALLS source by name; self-loop negative tests present
Registry-primary and legacy TS paths aligned ✅ satisfied — 4+4+4+4 patterns mirrored; export_statement forms covered in legacy; variable_declaration gap is minor (see finding 4)
JS legacy query behavior updated without regressing existing JS capture ✅ satisfied — JAVASCRIPT_QUERIES updated; broad all-language test suite passed; tsExtractFunctionName shared
Bare statement-level HOC calls remain anonymous / File-attributed ✅ satisfied — negative-bare-call.ts fixture + integration + unit tests cover this
Nested HOCs do not create phantom wrong edges ⚠️ partially satisfied — nested.tsx fixture documents behavior, but no integration test assertion verifies the absence of phantom Function:Wrapped edges
Known false-positive patterns tested/documented ⚠️ partially satisfied — documented exhaustively in query.ts; tested in description but NOT asserted by any unit/integration test case
Existing HOF/JSX-as-call/Zustand/TanStack patterns unchanged ✅ satisfied — regression tests pass; new arguments branch guards correctly against Promise executor, Zustand nested chains, TanStack patterns
CI/typecheck/parity validation green ✅ satisfied

TypeScript / JavaScript attribution assessment

  • HOC-wrapped const naming: Correct. The arguments → call_expression → variable_declarator guard chain is tight. new_expression correctly excluded. Bare statement-level calls correctly excluded. Destructuring declarators ({ a, b } = ...) correctly fall through to null (no identifier name node).
  • CALLS source attribution: Correct. Anchor-on-inner-arrow discipline ensures the def lands in the arrow's own scope and hoists the binding to the parent. Both findEnclosingFunctionId (parse-worker) and findEnclosingFunction (call-processor) use tsExtractFunctionName, so both paths agree.
  • Registry-primary query patterns: Well-formed. Four patterns covering lexical_declaration × variable_declaration × arrow_function × function_expression. TSX is handled by the same patterns (the grammar switch is at the parser level, not the query level). export const is handled because tree-sitter pattern matching is positional/subtree, not anchored — (lexical_declaration ...) matches inner lexical_declaration regardless of export_statement wrapper.
  • Legacy TS/JS query patterns: Well-formed. Four TS patterns + four JS patterns including explicit export_statement wrappers. Capture names (@name, @definition.function) match legacy parser expectations. Minor: no variable_declaration HOC mirror (see finding 4).
  • tsExtractFunctionName: Third branch is correctly guarded. Shared between TS and JS providers (both use extractFunctionName: tsExtractFunctionName).
  • Exported declarations: ✅ Unit test at line 516 explicitly asserts export const Exported = forwardRef(...) is captured.
  • Function expression support: ✅ Both arrow_function and function_expression forms covered in all four pattern layers (registry-primary, legacy TS, legacy JS, resolver branch).
  • Bare-call negative behavior: ✅ Solid. expression_statement parent breaks the variable_declarator guard; integration test + unit test both verify. The wrap(memo(...)) case (doubly-nested bare calls) correctly returns null because the inner call_expression.parent is arguments not variable_declarator.
  • Nested HOC behavior: Correctly excluded — the deepest arrow's call_expression.parent is arguments (not variable_declarator) for doubly-wrapped forms. But no integration test asserts this (see finding 2).
  • Array-method false-positive tradeoff: Correctly analyzed. The Function:x from const x = arr.find(...) is harmless in terms of incoming CALLS (x is a value, never invoked). Outgoing Function:x → predicate is a minor mis-attribution. Tradeoff is proportionate. Missing assertion test (see finding 1).
  • Multi-arrow argument behavior: Dedup claim is plausible based on understanding the registry's qualified-name dedup logic. No test verifies it.

Test assessment

  • Integration fixture coverage: Strong. 7 HOC wrappers × named-Function + named-CALLS assertions. Sibling const cross-attribution test (no first-sibling-wins). Self-loop negative test. Bare-call negative test. Covers React.forwardRef (member-expression callee), bare forwardRef, memo, useCallback, useMemo, observer, debounce.
  • Unit attribution coverage: Strong. 6 new attribution tests (bare HOC, member-expression HOC, useCallback, memo, first-sibling isolation, bare statement-level negative). 2 definition-capture tests. Tests use the same collectCallAttributions harness that mirrors production findEnclosingFunctionId logic.
  • JS coverage: Indirect. JAVASCRIPT_QUERIES updated with 4 new patterns; tsExtractFunctionName shared. No dedicated JS fixture, but broad all-language test run passed. Acceptable.
  • Legacy parity coverage: ✅ PR body claims REGISTRY_PRIMARY_TYPESCRIPT=0 parity run passed (236/236). CI also ran full suite.
  • Negative tests: Solid. Bare-call negative in both unit and integration. Self-loop check. No-first-sibling assertion.
  • Would tests fail before this PR? Yes. All named-CALLS assertions (e.g., Button → cn, handleClick → doStuff) would fail because the arrows were anonymous and CALLS attributed to File. The Function-node existence test would also fail.
  • Gaps: No test for array-method false-positive, no test for multi-arrow dedup, no integration test for nested.tsx behavior.

Validation assessment

  • CI: Green on head 8d2eae8. 7772/7773 tests passed; 1 pre-existing skip documented. Code coverage improved +0.2% statements, +0.3% functions.
  • Docker: Green per PR comments.
  • Typecheck: tsc --noEmit clean per PR body and CI typecheck stage.
  • Focused tests: 11/11 HOC integration, 26/26 unit (8 new + 18 existing).
  • Broad resolver tests: 1693/1693 across all non-Kotlin/Swift languages.
  • Legacy DAG parity: Claimed 236/236 with REGISTRY_PRIMARY_TYPESCRIPT=0; not directly visible in CI output but consistent with green run.
  • Live re-index: Empirically validated on 525-file Sourcerer-fe codebase. Metrics are compelling (+936 Function nodes, −45 File-rooted CALLS) but these are auxiliary evidence, not a substitute for targeted regression tests.
  • Not verified directly: Multi-arrow dedup claim (reasoned, not tested). First-arrow CALLS loss on multi-arrow dedup (not verified by test). Nested HOC "no phantom edges" claim (asserted only by PR description, not by test code).

Compatibility and rollout assessment

  • Graph schema compatibility: No schema changes. New Function nodes and CALLS edges are additive to the existing graph model.
  • Re-index requirement: Indexed repos must re-run npx gitnexus analyze to discover the new nodes. Existing graphs remain valid (no migration needed). Clearly stated in PR.
  • Expected impact on React/TS codebases: High positive impact. HOC-wrapped declarations are the dominant authoring style for React UI primitives. Every forwardRef, memo, useCallback, useMemo const previously invisible to gitnexus_context / gitnexus_impact is now a first-class Function node with outgoing CALLS.
  • Release-note recommendation: Accurate in PR body. The note correctly scopes to HOC-wrapped consts, names the known false-positive, and quantifies the impact.
  • Known limitations / follow-ups:
    1. Deeply-nested HOCs (memo(forwardRef(...))) — out of scope, correctly documented, needs integration test assertion.
    2. Array-method callbacks (arr.find((y) => ...)) — accepted false positive, needs an assertion test to pin the behavior.
    3. Multi-arrow callback dedup (call(a1, a2)) — documented, needs an assertion test.

Final verdict

production-ready with minor follow-ups

The core attribution fix is architecturally sound and correctly implemented across all three touch points (resolver, registry-primary query, legacy queries). The arguments → call_expression → variable_declarator guard chain is tight enough to exclude new Promise, bare statement-level calls, and doubly-nested HOC chains. Anchor discipline is correct — inner-arrow placement ensures CALLS attribute to the named const, not to File. CI is green on all 7772 tests. The three medium findings are all about missing assertion tests for documented trade-offs, not about incorrect behavior. The array-method false-positive is harmless in practice (no incoming CALLS), the nested HOC limitation is correctly excluded, and multi-arrow dedup only affects a rare API shape. None of these block correctness of the primary fix. The recommended follow-up tests would strengthen the documentation-to-test contract but are not prerequisite for merge.


@magyargergo

Copy link
Copy Markdown
Collaborator

@ReidenXerx could you please look at these findings? 🙏 please ensure that the CI is healthy as well

…rity gap

Addresses the four findings on PR abhigyanpatwari#1261 (Claude bot review for abhigyanpatwari#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 abhigyanpatwari#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 abhigyanpatwari#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 abhigyanpatwari#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 abhigyanpatwari#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>
@ReidenXerx

Copy link
Copy Markdown
Contributor Author

@magyargergo thanks for the review — all four follow-ups addressed in 861cbe66. Each finding called out a missing assertion for behaviour that was only documented in code comments; the new tests pin the documented trade-offs so future readers (and future patches) can't silently change them.

What changed

# Severity Finding Fix
1 medium Array-method FP (const found = items.find((y) => p(y))Function:found → predicate) was documented but untested Unit test pinning predicate.attributedTo === 'found' with a comment explaining why we accept the FP (value-only consts have no incoming CALLS, narrowing the pattern would require an HOC allowlist)
2 medium nested.tsx fixture existed but no integration test asserted Wrapped is NOT a Function and inner CALLS source from File Two integration tests: (a) Wrapped is NOT a Function node, (b) the deepest arrow's helper() call is NOT attributed to Function:Wrapped and no Function-sourced CALLS originate from nested.tsx
3 medium Multi-arrow dedup (const x = call(a1, a2)) documented but untested Unit test pinning legacy DAG attribution (both arrows resolve to "x" via the shared arguments → call_expression → variable_declarator ancestor); comment also clarifies the registry-primary dedup story
4 low Registry-primary had (variable_declaration ...) HOC patterns but legacy TS/JS queries didn't Mirrored two (variable_declaration ...) HOC patterns into both legacy sections so var X = HOC(...) produces the same Function:X on both pipelines (real-world impact: transpiler output that downlevels constvar)

Validation

  • Targeted suite: 41/41 (28 unit + 13 integration, 3 new tests added).
  • Broader TS suite: 60/60 across 4 resolver test files (typescript-hoc-wrapped, typescript-hof-callbacks, typescript-jsx-as-call, typescript-import-resolution) on registry-primary.
  • CI parity gate target (test/integration/resolvers/typescript.test.ts): 236/236 on legacy DAG (REGISTRY_PRIMARY_TYPESCRIPT=0) and 236/236 on registry-primary (=1).
  • Prettier: clean. ESLint: clean (5 pre-existing non-null-assertion warnings in the test file, all in unchanged code). tsc --noEmit: clean.

CI should re-run automatically on the new commit. Will keep an eye on it.

@github-actions

github-actions Bot commented May 3, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 4m 17s —— View job


PR #1261 Code Review — HOC-wrapped const declarations


Review bar for this PR

  • HOC-wrapped const X = HOC((...) => { ... }) declarations must produce named Function nodes using the const's identifier.
  • Calls inside the wrapped arrow/function expression must attribute to the named function, not to File.
  • tsExtractFunctionName's new arguments branch must be narrowly guarded: arguments → call_expression → variable_declarator → identifier. Bare calls, nested HOCs, and destructuring declarations must fall through to null.
  • Registry-primary (query.ts) and legacy (tree-sitter-queries.ts) TypeScript paths must produce equivalent Function nodes; variable_declaration (var) parity must be maintained.
  • JavaScript legacy queries must carry matching HOC patterns; tsExtractFunctionName is the shared resolver for both TS and JS.
  • Known false-positive patterns (array-method callbacks, multi-arrow dedup, nested HOC limitation) must be tested or pinned, not only documented.
  • Existing TS HOF/pair-arrow/JSX-as-call/Zustand/TanStack patterns must not regress; CI, Docker, typecheck, and all focused resolver tests must be green on the head SHA.

Issue and PR context considered

Issue #1166 identified ~75% CALLS-edge miss rate in TypeScript codebases with HOF/callback idioms. Two root causes: genericFuncName borrowing parameter identifiers as function names; and tsExtractFunctionName missing the pair/arguments parent paths.

PR #1175 fixed pair-arrow naming, Promise fan-out, TanStack queryFn, and Zustand multi-action stores. After re-indexing Sourcerer-fe, the residual dominant 0%-capture pattern was const X = HOC((args) => { ... }) — ~296 declarations in ~525 files, all anonymous pre-fix.

This PR (#1261) closes the HOC-wrapped blind spot across both pipeline paths (registry-primary + legacy) and the resolver layer. 14 files changed: typescript.ts (resolver), typescript/query.ts (registry-primary), tree-sitter-queries.ts (legacy TS + JS), 9 fixtures, 1 new integration test file (13 tests), 1 extended unit test file (+28 tests in final commit).

CI status: Both CI and Docker Build & Push workflows are green on head 861cbe6 (2026-05-03). The second commit (861cbe6) addressed all four findings from the previous automated review — array-method FP assertion, nested HOC integration tests, multi-arrow dedup pin, var-form parity in legacy TS/JS queries.

Known trade-offs: (1) Array-method callbacks (arr.find) produce harmless Function:x → predicate false-positive with no incoming CALLS. (2) Multi-arrow args both resolve to the same name; dedup second-wins in registry-primary. (3) Doubly-nested HOCs (memo(forwardRef(...))) remain anonymous (deepest arrow's call_expression.parent is arguments, not variable_declarator).


Understanding of the change

Before this PR, tsExtractFunctionName only handled two parent shapes: variable_declarator (direct const fn = () => {}) and pair (object property arrows). When an arrow's parent was arguments — as it always is for const Button = React.forwardRef((p, r) => { ... }) — both branches missed and the function returned { funcName: null }. findEnclosingFunctionId then continued walking up past the empty arrow scope into the module scope, selecting File as the caller anchor. Every call inside the HOC-wrapped arrow — cn(...), helper(...), imported utilities, JSX renders — attributed to the file.

The fix adds a third resolver branch that walks arguments → call_expression → variable_declarator → identifier with three tight guards. It adds 4 registry-primary patterns (anchored on the inner arrow_function/function_expression for correct scope hoisting) and 6 legacy patterns per query section (lexical + export_statement + variable_declaration forms, anchored on the outer lexical_declaration consistent with all other legacy patterns). Both TS and JS legacy queries are updated because JS uses tsExtractFunctionName via the same methodExtractor config.


Findings

[low] Registry-primary export const handling relies on implicit subtree matching — no dedicated test exercises it via the registry-primary path

  • Category: Test coverage / registry-primary vs. legacy parity
  • Files: gitnexus/src/core/ingestion/languages/typescript/query.ts, gitnexus/test/unit/call-attribution-issue-1166.test.ts (line 516)
  • Issue: The registry-primary (lexical_declaration ...) patterns correctly match export const X = HOC(...) because tree-sitter positional matching finds lexical_declaration nested inside export_statement. The unit test at line 516 (export const Exported = forwardRef(...)) verifies @definition.function capture — but that test runs against TYPESCRIPT_QUERIES (legacy), not against the registry-primary TYPESCRIPT_SCOPE_QUERY string. The legacy path has explicit export_statement wrapper patterns; registry-primary relies on implicit subtree matching. For production this is fine (the subtree match is how tree-sitter works and the integration test fixture forward-ref.tsx uses export const Button = React.forwardRef(...), which passes), but the unit test doesn't directly exercise the registry-primary query string with an exported HOC const.
  • Why it matters here: If a future refactor adds an explicit export_statement guard to the legacy patterns without noticing that registry-primary doesn't need one, the test wouldn't catch any divergence.
  • Recommended fix: Add a comment in query.ts noting that (lexical_declaration ...) matches both exported and non-exported forms via implicit subtree matching, and that no explicit export_statement wrapper is needed. A one-liner suffices. No new test required.
  • Blocks merge: No.

No other production-readiness issues found against the current DoD bar.


Definition of Done check

DoD item Status
HOC-wrapped consts produce named Function nodes ✅ satisfied — 7 HOC-wrapped consts in fixtures, each asserted as Function by getNodesByLabel(result, 'Function'); unit test pins @definition.function captures for all 5 wrapper forms + function-expression
Calls inside wrapped arrow attribute to named function, not File ✅ satisfied — all positive CALLS tests filter by source === constName; nested HOC integration tests assert no Function-sourced CALLS from nested.tsx
Registry-primary and legacy TS paths aligned ✅ satisfied — lexical_declaration × variable_declaration × arrow_function × function_expression covered in both; export_statement forms in legacy, implicit subtree match in registry-primary (minor note above); CI parity gate claimed green
JavaScript legacy queries updated without regressing ✅ satisfied — JAVASCRIPT_QUERIES carries 6 matching HOC patterns; tsExtractFunctionName shared; broad all-language run 1693/1693
Bare statement-level HOC calls remain anonymous / File-attributed ✅ satisfied — unit test asserts doSomething.attributedTo === null; integration test asserts fromFile.length > 0; wrap(memo(...)) doubly-bare case excluded by variable_declarator guard
Nested HOCs do not create phantom wrong edges ✅ satisfied — two integration tests: Wrapped NOT in Function nodes; all nested.tsx CALLS source from File, none from Function:Wrapped
Known false-positive patterns tested/documented ✅ satisfied — array-method FP pinned in unit test (predicate.attributedTo === 'found', with explanation comment); multi-arrow pinned in unit test (first and second both attribute to x); extensive comments in query.ts and typescript.ts
Existing HOF/JSX-as-call/Zustand/TanStack patterns unchanged ✅ satisfied — new arguments branch is a third branch added after variable_declarator and pair; existing branches untouched; 266/266 TS resolver families passed
CI, Docker, typecheck, focused tests, resolver parity green ✅ satisfied — CI + Docker both green on head 861cbe6; tsc clean; 7842/7843 tests (1 pre-existing skip)

TypeScript / JavaScript attribution assessment

HOC-wrapped const naming: Correct. The arguments → call_expression → variable_declarator guard chain is tight. new_expression is excluded (the check is callExpr.type !== 'call_expression'). Bare statement-level calls are excluded (declarator.type !== 'variable_declarator' fires when parent is expression_statement). Nested HOCs are excluded (inner call_expression.parent is arguments again, not variable_declarator). Destructuring declarators are excluded (no identifier name node).

CALLS source attribution: Correct. Inner-anchor discipline in registry-primary ensures pass2AttachDeclarations.atPosition resolves to the arrow's own scope and rangesEqual triggers the binding hoist to the parent scope. tsExtractFunctionName ensures both findEnclosingFunctionId (parse-worker) and findEnclosingFunction (call-processor) agree on attribution. Integration tests assert source === constName on CALLS edges, not just Function node existence.

Registry-primary query patterns: Four patterns, anchor on inner arrow_function/function_expression. Consistent with the existing lexical_declaration and pair patterns established in PR #1175. export const handled via implicit subtree matching (verified by forward-ref.tsx fixture using export const Button). No duplication with existing direct const fn = () => {} patterns — the intermediate call_expression > arguments distinguishes them.

Legacy TS/JS query patterns: Six patterns each (lexical + export_statement + variable_declaration × 2). Anchor on outer lexical_declaration/export_statement — consistent with all pre-existing legacy patterns. @name and @definition.function capture names match legacy parser expectations. var form mirrored. No conflicts with @definition.const/@definition.variable — parse-worker dedup handles the overlap.

tsExtractFunctionName: Third branch is tightly guarded and returns cleanly at every guard. Shared between TypeScript and JavaScript providers.

Exported declarations: Integration fixture forward-ref.tsx uses export const Button = React.forwardRef(...). Unit test (line 516) covers legacy path explicitly.

Function expression support: Both arrow_function and function_expression covered at all four levels (resolver, registry-primary, legacy TS, legacy JS). Unit test const Legacy = wrap(function (x) { ... }) asserts Legacy in captured names.

Bare-call negative behavior: Solid. Both unit test (attributedTo: null) and integration test (CALLS from File) cover this. The doubly-nested bare call wrap(memo(({ y }) => { ... })) is correctly excluded because memo(...) 's call_expression.parent is arguments (of wrap), not variable_declarator.

Nested HOC behavior: Correctly excluded — documented in nested.tsx comment block, asserted by two integration tests. No phantom edges.

Array-method false-positive: const found = items.find((item) => predicate(item))Function:found → predicate. Tested and accepted. The Function:found node has zero incoming CALLS (never invoked), so it is graph-isolated on the incoming side. The outgoing Function:found → predicate is a minor mis-attribution with negligible practical impact.

Multi-arrow argument behavior: const x = call(() => first(), () => second()) → both arrows attribute to x via legacy path (tested). Registry-primary dedup story is reasoned in a comment (second-write-wins on qualified-name collision). Rare in real APIs.


Test assessment

Integration fixture coverage: 9 fixtures, 13 tests. Every positive test filters by both sourceFilePath and source, verifying attribution is correct and not merely that edges exist. Covers: React.forwardRef (member-expression callee), bare forwardRef, memo, useCallback (two sibling consts), useMemo, observer, debounce. Self-loop negative, bare-call negative (with File-attribution assertion), cross-pollination, nested HOC limitation (two tests). Function-node existence tested as a separate assertion from CALLS attribution.

Unit attribution coverage: collectCallAttributions harness mirrors the production findEnclosingFunctionId walk exactly. 6 attribution tests (bare HOC, member-expression HOC, useCallback, memo, sibling isolation, bare-statement negative) + 2 definition-capture tests + 2 trade-off pins (array-method FP, multi-arrow dedup). Test harness is robust — no brittle line/column values.

JS coverage: No dedicated JS fixture, but tsExtractFunctionName is shared and JAVASCRIPT_QUERIES carries matching patterns. Broad all-language test run (1693/1693) includes the existing JS resolver tests which pass. Acceptable — adding a minimal JS fixture would add confidence but is not a blocking gap.

Legacy parity coverage: PR claims 236/236 with REGISTRY_PRIMARY_TYPESCRIPT=0. The CI parity gate is a separate run and passes per PR description (not directly visible in the CI action output shown in comments, but consistent with the overall green status).

Negative tests: Solid. Bare-call negative is tested at both unit and integration levels. Self-loop check is explicit (filters source === constName && target === wrapperName). No-first-sibling assertion checks that no Function-sourced strays from siblings appear.

Would tests fail before this PR? Yes, conclusively. All positive CALLS assertions (Button → cn, handleClick → doStuff, etc.) would fail because the arrows were anonymous and CALLS attributed to File. The Function-node existence test for each HOC-wrapped const would also fail.


Validation assessment

  • CI (head 861cbe6): ✅ Green — confirmed via CI API. Both CI and Docker Build & Push passed on 2026-05-03.
  • Docker: ✅ Green.
  • Typecheck:tsc --noEmit clean per PR description and CI typecheck stage.
  • Focused tests: 13/13 HOC integration, 28/28 unit (covering the HOC-specific describe block); 266/266 TS resolver families.
  • Broad resolver tests: 1693/1693 across all non-Kotlin/Swift languages (Kotlin/Swift skipped per AGENTS.md § Gotchas — acceptable).
  • Legacy DAG parity: 236/236 with REGISTRY_PRIMARY_TYPESCRIPT=0 per PR description.
  • ESLint: Clean; 5 pre-existing Forbidden non-null assertion warnings in unchanged unit-test code.
  • Live re-index: Compelling empirical evidence (+936 Function nodes, −45 File-rooted CALLS, +1,018 CALLS total on Sourcerer-fe). Treated as auxiliary evidence consistent with the targeted tests.
  • Not verified directly: Registry-primary dedup second-wins claim for multi-arrow (reasoned, not tested at registry level). Acceptable given the pattern is rare and the legacy path is tested.

Compatibility and rollout assessment

Graph schema compatibility: No schema changes. New Function nodes and CALLS edges are additive.

Re-index requirement: Indexed repos must re-run npx gitnexus analyze. Existing graphs remain valid (no migration). Clearly documented in PR.

Expected impact on React/TS codebases: High positive impact. Every React.forwardRef, memo, useCallback, useMemo, observer, debounce-wrapped const — the dominant authoring style for React UI primitives — is now a first-class Function node with resolved outgoing CALLS. Shadow impact on gitnexus_context/gitnexus_impact for these nodes goes from zero to complete.

Release-note recommendation: Accurate in PR body. Correctly scopes to HOC-wrapped consts, names the array-method false-positive, quantifies the impact range.

Known limitations / follow-ups:

  1. Doubly-nested HOCs (memo(forwardRef(...))) — out of scope, explicitly documented and integration-tested as limitation. A future fix requires a policy decision on which level wins the name.
  2. Array-method callbacks (arr.find) — accepted false-positive, now pinned by unit test.
  3. Multi-arrow callback dedup (registry-primary only) — documented, legacy path pinned, rare enough not to block.
  4. No dedicated JS fixture for HOC patterns — low risk; the shared resolver and updated JS queries provide reasonable coverage via existing JS test suite.

Final verdict

production-ready

The core attribution fix is architecturally correct across all three touch points — resolver branch, registry-primary query, and legacy queries for both TS and JS. The arguments → call_expression → variable_declarator guard chain correctly excludes new_expression, bare statement-level calls, nested HOCs, and destructuring. Anchor discipline is right: inner-arrow placement in registry-primary ensures CALLS attribute to the named const, not to File. All previously documented trade-offs are now pinned by tests, not just comments — the four findings from the previous review round have been fully addressed. CI and Docker are green on head 861cbe6. The one remaining low-severity observation (implicit subtree matching for export const in registry-primary) is a documentation note, not a behavior gap. No blocking issues.

@magyargergo magyargergo merged commit 36ff151 into abhigyanpatwari:main May 3, 2026
18 of 19 checks passed
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.

2 participants