fix: emit reference edges for function identifiers used as object-literal values#1898
Conversation
…eral values
codegraph roles --role dead inconsistently flagged dispatch-table handler
functions (`{ matches, resolve: someFunction }`-style arrays) as dead,
depending on an unrelated, incidental property of the referenced function:
whether it happened to call another tracked symbol internally (fanOut > 0).
Root cause: codegraph created no edge at all for a bare function identifier
used as an object-literal property VALUE. The role classifier's
`node.kind === 'function' && node.fanOut > 0` heuristic in
classifyUnreferencedNode incidentally rescued whichever handlers happened to
have nonzero fanOut, leaving the rest misclassified dead-unresolved — a
false positive that risks deleting live dispatch-table handlers during
dead-code cleanup.
Fix: both engines now extract a dynamic `calls` edge (dynamicKind /
dynamic_kind = 'value-ref') for every bare-identifier object-literal
property value and shorthand property (`{ resolve: fn }` / `{ fn }`),
attributed to the enclosing scope via the existing findCaller machinery
(falls back to the widest enclosing constant/variable binding for top-level
dispatch tables, same as any other call). Resolution is restricted to
function/method-kind targets only, so plain data references
(`{ name: SOME_CONSTANT }`) are silently dropped rather than fabricating
nonsensical edges to constants.
Reuses the existing `calls` edge kind + dynamic flag (per ADR-002's "no new
edge kind" precedent, and the existing extractCallbackReferenceCalls
mechanism from #1741) rather than inventing a new edge kind — fan-in/fan-out
(scoped to `kind IN ('calls','imports-type')` / `kind = 'calls'`) already
account for it with zero changes to structure.ts/roles.rs's median
computations.
The `fanOut > 0` heuristic in classifyUnreferencedNode is kept, narrowed by
comment: it's no longer load-bearing for the object-literal case but remains
the only rescue for sibling value-reference patterns not yet extracted as
edges (logical-or fallback defaults, ternary defaults, array-of-functions
elements).
Applied to both engines:
- WASM/TS: src/extractors/javascript.ts (collectObjectLiteralValueRefCall,
wired into the shared runCollectorWalk used by both the walk and query
extraction paths), src/domain/graph/builder/stages/build-edges.ts
(resolveFallbackTargets kind filter), src/domain/graph/builder/incremental.ts
(mirrored filter for the incremental/watch-mode path), src/types.ts (new
'value-ref' DynamicKind variant).
- Native: crates/codegraph-core/src/extractors/javascript.rs
(handle_object_literal_pair_value_ref /
handle_object_literal_shorthand_value_ref, wired into match_js_node),
crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs
(matching kind filter in process_file).
Verified against the exact repro (this repo's own PARAM_NODE_HANDLERS
dispatch table in src/ast-analysis/visitor-utils.ts): all 5 handlers now
show totalDependents=1 and role=core on both engines, with zero dead-flagged
symbols in that file. Resolution-benchmark precision/recall is unchanged
across all 34 language fixtures (javascript 100/100, typescript 95.7/93.6,
aggregate 63.8% recall, identical before and after).
Fixes #1771
docs check acknowledged — internal edge-emission/resolver bug fix, no new
commands, languages, or architecture to document.
Impact: 9 functions changed, 19 affected
Codegraph Impact Analysis9 functions changed → 19 callers affected across 6 files
|
Greptile SummaryThis PR fixes inconsistent dead-code classification for functions referenced as object-literal property values (dispatch-table pattern:
Confidence Score: 4/5Safe to merge; the core extraction and edge-filtering logic is correct on both engines and both build paths, with comprehensive parser-level and integration-level test coverage for the full-build path. The functional fix is well-reasoned and the Rust/TypeScript parity is tight. Two gaps keep the score from being higher: the
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[JS/TS source file] -->|tree-sitter parse| B{Node type?}
B -->|pair node| C[collectObjectLiteralValueRefCall\nvalue must be bare identifier]
B -->|shorthand_property_identifier| D[inline push to valueRefCalls]
C -->|BUILTIN_GLOBALS? skip| E[Call record\ndynamic=true\ndynamicKind=value-ref]
D -->|BUILTIN_GLOBALS? skip| E
E --> F{Build path}
F -->|Full build\nbuild-edges.ts| G[resolveFallbackTargets]
F -->|Incremental\nincremental.ts| H[applyCallFallbacks]
G --> I{Resolved targets}
H --> I
I -->|dynamicKind=value-ref filter| J{target.kind?}
J -->|function or method| K[Emit calls edge\ndynamic=1\ndynamic_kind=NULL in DB]
J -->|anything else| L[Silently dropped\nno edge, no flag]
K --> M[fan-in increases\nrole classifier: non-dead]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[JS/TS source file] -->|tree-sitter parse| B{Node type?}
B -->|pair node| C[collectObjectLiteralValueRefCall\nvalue must be bare identifier]
B -->|shorthand_property_identifier| D[inline push to valueRefCalls]
C -->|BUILTIN_GLOBALS? skip| E[Call record\ndynamic=true\ndynamicKind=value-ref]
D -->|BUILTIN_GLOBALS? skip| E
E --> F{Build path}
F -->|Full build\nbuild-edges.ts| G[resolveFallbackTargets]
F -->|Incremental\nincremental.ts| H[applyCallFallbacks]
G --> I{Resolved targets}
H --> I
I -->|dynamicKind=value-ref filter| J{target.kind?}
J -->|function or method| K[Emit calls edge\ndynamic=1\ndynamic_kind=NULL in DB]
J -->|anything else| L[Silently dropped\nno edge, no flag]
K --> M[fan-in increases\nrole classifier: non-dead]
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (call.dynamicKind === 'value-ref') { | ||
| // `targets` is typed without `kind` when it flows straight through from | ||
| // resolveCallTargets (call-resolver.ts's declared return type omits it), | ||
| // but every underlying CallNodeLookup method actually populates it — the | ||
| // same gap the preQualifiedTargets cast above already works around. | ||
| targets = (targets as ReadonlyArray<{ id: number; file: string; kind?: string }>).filter( | ||
| (t) => t.kind === 'function' || t.kind === 'method', | ||
| ); |
There was a problem hiding this comment.
kind field access on undeclared type
The cast targets as ReadonlyArray<{ id: number; file: string; kind?: string }> works around call-resolver.ts's return type omitting kind. If any resolution tier produces targets where kind is genuinely absent at runtime (not merely absent from the declared type), the filter silently drops those targets and no edge is emitted — with zero diagnostic signal. The PR comment and the preQualifiedTargets cast above confirm this is a known type gap in the codebase, but combining an unsafe cast with a functional gate in a single expression makes future regressions hard to detect.
1 participant
Summary
{ matches, resolve: someFunction }) got zero graph edges at all, soroles --role deadfell back to an incidentalfanOut > 0heuristic in the role classifier to rescue them — producing inconsistent results across structurally identical dispatch-table entries depending on whether each handler happened to call another tracked symbol internally (unrelated to whether the dispatch-table reference itself was real).callsedge kind +dynamic=1with a newDynamicKindvariant'value-ref', rather than inventing a new edge kind — this wires into fan-in/fan-out, cycle detection, exporters, and MCP schemas for free (fan-in/out are scoped tokind IN ('calls', 'imports-type')/kind='calls'), matches the precedent set by bug(resolver): identifier-arg dynamic call resolution fabricates false edges/cycles on name collision (e.g. communities.ts phantom cycle) #1741's identifier-arg handling, and matches ADR-002's explicit "no new edge kind" decision for dynamic-call classification.function/method(object-literal values are as often data as functions).fanOut > 0heuristic is kept, not removed — it's no longer load-bearing for object-literal values but is still the only rescue for the sibling logical-or-fallback pattern (options._fetchLatest || fetchLatestVersion) and other unextracted value-reference shapes.Closes #1771
Test plan
npm test— full suite green (3560 passed, 0 failed)npm run lint/npx tsc --noEmit— cleancargo test— 465 passedPARAM_NODE_HANDLERSentries now showtotalDependents:1,role:core, zero dead-flaggedScope: fully general, not scoped down — any bare-identifier object-literal property value or shorthand property, same-file or cross-file.
Filed #1897 for an out-of-scope finding: WASM has no equivalent to native's inline object-literal dispatch-table extraction (
({a:fnA,b:fnB})[k]()) — a genuine engine-parity gap, unrelated to this issue.Stacked on #1896 (base branch
fix/issue-1770-leiden-partition-dead-code) — only the extractor/builder/classifier/test diff is this issue's change.