Skip to content

Commit d4e6212

Browse files
committed
fix: restore reflection dynamicKind for .call/.apply/.bind (docs check acknowledged)
Issue #1778: /parity found WASM emits dyn=0 (plain static call) while native emits dyn=1/dynamicKind='reflection' for identifier.call(...)/.apply(...)/ .bind(...) call sites, even though both fully resolve the target (confidence 1). Root cause: PR #1693 (closing #1687) made the WASM extractor unconditionally drop dynamic/dynamicKind for these calls on identifier receivers, to fix a narrow dedup-collision bug in build-edges.ts's dynZeroEdgeRows upgrade path. That fix overcorrected — it silenced the `reflection` DynamicKind for every identifier-based .call/.apply/.bind, not just the collision case, diverging from native (which never changed). Chose Option A (revert + fix narrowly), not Option B (mirror WASM's drop in Rust): ADR-002 defines `reflection` as informational metadata independent of resolution/confidence, queryable via `codegraph roles --dynamic` — dropping it for .call/.apply/.bind would erase real information about a genuinely reflective invocation mechanism, and native's classification was never wrong. Changes: - src/extractors/javascript.ts: extractMemberExprCallInfo once again tags .call/.apply/.bind on identifier receivers as dynamic:true, dynamicKind:'reflection', matching the member-expression branch and native. - src/domain/graph/builder/stages/build-edges.ts: emitDirectCallEdgesForCall's dyn=0 -> dyn=1 upgrade (dynZeroEdgeRows) now compares source lines instead of firing on any dynamicKind-tagged collision. It only upgrades when the incoming call's line is EARLIER than the recorded dyn=0 call's line — which only happens when the query path's two-phase collection (query matches, then a supplementary walk pass for bare decorators, #1683) reorders a genuinely-earlier call to arrive later. A later .call/.apply/.bind to an already-called target is collected in the same phase as the direct call (true source order already holds), so it must not flip an established dyn=0 edge — matching native's plain first-recorded-wins dedup. - crates/codegraph-core/src/extractors/javascript.rs: no behavior change (native was already correct); added 4 unit tests pinning the identifier/member-expression reflection tagging so this doesn't regress. - tests/parsers/javascript.test.ts, tests/engines/dynamic-call-ffi.test.ts: updated to assert the restored dynamic/reflection tagging at the parser level. - tests/integration/issue-1778-reflection-dynamic-kind-parity.test.ts: new engine-parity tests (wasm + native) covering the #1778 minimal repro (no prior direct call -> dyn=1), the original #1687 dedup scenario (direct call then .call() -> single dyn=0 edge), the reverse-order case, and a #1683 regression guard (bare decorator before call-expression decorator still upgrades to dyn=1). Verified: `node scripts/parity-compare.mjs` shows zero edge diffs across all 42 fixtures (previously 6/8/12 diffs on dynamic-javascript/javascript/ jelly-micro). Resolution-benchmark precision/recall unchanged for every language. Full npm test suite green (3589 passed). cargo test green (479 passed, +4 new). Fixes #1778 Impact: 6 functions changed, 13 affected
1 parent ee6362c commit d4e6212

6 files changed

Lines changed: 343 additions & 53 deletions

File tree

crates/codegraph-core/src/extractors/javascript.rs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4246,6 +4246,50 @@ mod tests {
42464246
assert!(dynamic_calls.is_empty());
42474247
}
42484248

4249+
// ── #1778: .call/.apply/.bind reflection tagging (parity pin) ───────────
4250+
//
4251+
// Pins the native extractor's classification of `.call/.apply/.bind` on both
4252+
// identifier and member-expression receivers as dynamic/reflection. This is
4253+
// the Option-A semantic from #1778: the WASM extractor previously stripped
4254+
// this tag for identifier receivers only, diverging from native. These tests
4255+
// guard against either engine's classification drifting again — the
4256+
// dedup-collision case that originally motivated the WASM regression (#1687)
4257+
// is a downstream build-edges.ts concern, not an extraction concern, so it is
4258+
// deliberately NOT re-tested here (see the JS-side pins in
4259+
// tests/integration for that).
4260+
4261+
#[test]
4262+
fn call_on_identifier_receiver_tags_reflection() {
4263+
let s = parse_js("function test(ctx) { greet.call(ctx, 'world'); }");
4264+
let c = s.calls.iter().find(|c| c.name == "greet").unwrap();
4265+
assert_eq!(c.dynamic, Some(true));
4266+
assert_eq!(c.dynamic_kind.as_deref(), Some("reflection"));
4267+
}
4268+
4269+
#[test]
4270+
fn apply_on_identifier_receiver_tags_reflection() {
4271+
let s = parse_js("function test(ctx) { greet.apply(ctx, ['world']); }");
4272+
let c = s.calls.iter().find(|c| c.name == "greet").unwrap();
4273+
assert_eq!(c.dynamic, Some(true));
4274+
assert_eq!(c.dynamic_kind.as_deref(), Some("reflection"));
4275+
}
4276+
4277+
#[test]
4278+
fn bind_on_identifier_receiver_tags_reflection() {
4279+
let s = parse_js("var bound = greet.bind(ctx);");
4280+
let c = s.calls.iter().find(|c| c.name == "greet").unwrap();
4281+
assert_eq!(c.dynamic, Some(true));
4282+
assert_eq!(c.dynamic_kind.as_deref(), Some("reflection"));
4283+
}
4284+
4285+
#[test]
4286+
fn call_on_member_expression_receiver_tags_reflection() {
4287+
let s = parse_js("obj.method.call({});");
4288+
let c = s.calls.iter().find(|c| c.name == "method").unwrap();
4289+
assert_eq!(c.dynamic, Some(true));
4290+
assert_eq!(c.dynamic_kind.as_deref(), Some("reflection"));
4291+
}
4292+
42494293
// ── #1771: object-literal value-ref extraction ──────────────────────────
42504294

42514295
#[test]

src/domain/graph/builder/stages/build-edges.ts

Lines changed: 56 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,18 @@ import { getResolved, isBarrelFile, resolveBarrelExportCached } from './resolve-
6666
type EdgeRowTuple = [number, number, string, number, number, string | null, string | null];
6767
// src tgt kind conf dyn technique dynamic_kind
6868

69+
/**
70+
* Tracks a dyn=0 direct-call edge row so a later dynamicKind-tagged call to the
71+
* same (caller, target) pair can decide whether to upgrade it in-place — see
72+
* {@link emitDirectCallEdgesForCall}. `line` is the source line of the call that
73+
* produced the row, used to detect out-of-source-order collection artifacts
74+
* (bare decorators processed after call-expression matches in the query path).
75+
*/
76+
interface DynZeroEdgeEntry {
77+
idx: number;
78+
line: number;
79+
}
80+
6981
interface NodeIdStmt {
7082
get(name: string, kind: string, file: string, line: number): { id: number } | undefined;
7183
}
@@ -1395,8 +1407,12 @@ function resolveFallbackTargets(
13951407
* - If a pts edge already exists for this pair, upgrades it in-place to
13961408
* direct-call confidence and promotes to seenCallEdges.
13971409
* - If a dyn=0 edge already exists and the incoming call has an explicit
1398-
* dynamicKind (e.g. 'reflection' for bare decorators), upgrades the
1399-
* existing row to dyn=1 in-place so the semantic classification wins.
1410+
* dynamicKind AND textually precedes the recorded dyn=0 call (e.g. a bare
1411+
* decorator `@Log` reordered after `@Log()` by the query path's
1412+
* query-then-walk collection — see buildFileCallEdges), upgrades the
1413+
* existing row to dyn=1 in-place so the earlier-in-source classification
1414+
* wins, matching what native's single-pass source-order walk produces
1415+
* natively.
14001416
* - Otherwise records a new `calls` edge with `ts-native` technique.
14011417
*/
14021418
function emitDirectCallEdgesForCall(
@@ -1405,11 +1421,12 @@ function emitDirectCallEdgesForCall(
14051421
importedFrom: string | null | undefined,
14061422
isDynamic: number,
14071423
hasDynamicKind: boolean,
1424+
callLine: number,
14081425
relPath: string,
14091426
seenCallEdges: Set<string>,
14101427
ptsEdgeRows: Map<string, number>,
14111428
allEdgeRows: EdgeRowTuple[],
1412-
dynZeroEdgeRows?: Map<string, number>,
1429+
dynZeroEdgeRows?: Map<string, DynZeroEdgeEntry>,
14131430
): void {
14141431
// Sort targets by confidence descending before emitting edges.
14151432
// For multi-target calls with duplicate (source_id, target_id) pairs the
@@ -1432,14 +1449,30 @@ function emitDirectCallEdgesForCall(
14321449
if (seenCallEdges.has(edgeKey)) {
14331450
// Edge already emitted. If the incoming call carries an explicit semantic
14341451
// dynamic classification (dynamicKind set — e.g. 'reflection' for bare
1435-
// decorators) and the existing edge was recorded with dyn=0, upgrade it
1436-
// in-place so the more specific classification wins.
1437-
// Generic dynamic=true without dynamicKind (alias/callback calls) does
1438-
// NOT override dyn=0 to avoid false positives on f.call/f.bind patterns.
1452+
// decorators or .call/.apply/.bind) and the existing edge was recorded
1453+
// with dyn=0, only upgrade it in-place when the incoming call's source
1454+
// line is EARLIER than the recorded dyn=0 call's line.
1455+
//
1456+
// Why line order, not just "hasDynamicKind": the query path collects
1457+
// calls in two phases — tree-sitter query matches (callfn_node/callmem_node,
1458+
// true source order) first, then a supplementary walk pass for constructs
1459+
// the query grammar can't capture (bare decorators, object-literal
1460+
// value-refs) appended AFTERWARD regardless of true position (#1683).
1461+
// A bare `@Log` at an earlier line can therefore reach this branch AFTER
1462+
// `@Log()` at a later line already recorded dyn=0 — upgrading is correct
1463+
// there because native's single-pass source-order walk would have seen
1464+
// `@Log` first and kept dyn=1.
1465+
//
1466+
// But `.call/.apply/.bind` calls (e.g. `f(); f.call({})`, #1687/#1778) are
1467+
// ordinary call_expressions collected in the SAME query phase as the
1468+
// direct call, so true source order is already preserved: when the
1469+
// dynamic-flavored call's line is LATER than the recorded dyn=0 call, it
1470+
// is genuinely a second, later reference to the same target — native's
1471+
// dedup (first-recorded-wins, no upgrade) drops it, so WASM must too.
14391472
if (isDynamic === 1 && hasDynamicKind && dynZeroEdgeRows) {
1440-
const dynZeroIdx = dynZeroEdgeRows.get(edgeKey);
1441-
if (dynZeroIdx !== undefined) {
1442-
const row = allEdgeRows[dynZeroIdx];
1473+
const dynZeroEntry = dynZeroEdgeRows.get(edgeKey);
1474+
if (dynZeroEntry !== undefined && callLine < dynZeroEntry.line) {
1475+
const row = allEdgeRows[dynZeroEntry.idx];
14431476
if (row) row[4] = 1;
14441477
dynZeroEdgeRows.delete(edgeKey);
14451478
}
@@ -1463,10 +1496,12 @@ function emitDirectCallEdgesForCall(
14631496
seenCallEdges.add(edgeKey);
14641497
const newIdx = allEdgeRows.length;
14651498
allEdgeRows.push([caller.id, t.id, 'calls', confidence, isDynamic, 'ts-native', null]);
1466-
// Track dyn=0 edges so a later dyn=1+dynamicKind call for the same pair
1467-
// can upgrade them (e.g. bare decorator after call-expression decorator).
1499+
// Track dyn=0 edges (with their source line) so a later dyn=1+dynamicKind
1500+
// call for the same pair can decide whether to upgrade them — see the
1501+
// line-order comparison above (e.g. bare decorator reordered ahead of a
1502+
// call-expression decorator by the query path, #1683).
14681503
if (isDynamic === 0 && dynZeroEdgeRows) {
1469-
dynZeroEdgeRows.set(edgeKey, newIdx);
1504+
dynZeroEdgeRows.set(edgeKey, { idx: newIdx, line: callLine });
14701505
}
14711506
}
14721507
}
@@ -1736,11 +1771,13 @@ function buildFileCallEdges(
17361771
// no longer tracked here.
17371772
const ptsEdgeRows = new Map<string, number>();
17381773

1739-
// Tracks direct-call edges emitted with dyn=0 (edgeKey → allEdgeRows index).
1740-
// When a later call to the same target has dyn=1 (e.g. a bare decorator `@Log`
1741-
// processed after the call-expression `@Log()` in the query path), the existing
1742-
// dyn=0 row is upgraded in-place so the more specific dynamic classification wins.
1743-
const dynZeroEdgeRows = new Map<string, number>();
1774+
// Tracks direct-call edges emitted with dyn=0 (edgeKey → { row index, source line }).
1775+
// When a later call to the same target has dyn=1 and textually precedes the recorded
1776+
// call (e.g. a bare decorator `@Log` reordered after the call-expression `@Log()` by
1777+
// the query path, #1683), the existing dyn=0 row is upgraded in-place. See the line-order
1778+
// comparison in emitDirectCallEdgesForCall for why line order (not mere dynamicKind
1779+
// presence) gates the upgrade — this is also what keeps #1687/#1778 from regressing.
1780+
const dynZeroEdgeRows = new Map<string, DynZeroEdgeEntry>();
17441781

17451782
// Pre-compute the set of names that appear as lhs in fnRefBindings so that
17461783
// case (c) of the pts gate below only fires for names that are genuine
@@ -1773,6 +1810,7 @@ function buildFileCallEdges(
17731810
importedFrom,
17741811
isDynamic,
17751812
!!call.dynamicKind,
1813+
call.line,
17761814
relPath,
17771815
seenCallEdges,
17781816
ptsEdgeRows,

src/extractors/javascript.ts

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -353,9 +353,11 @@ function dispatchQueryMatch(
353353
if (callfnInfo) calls.push(callfnInfo);
354354
calls.push(...extractCallbackReferenceCalls(c.callfn_node));
355355
} else if (c.callmem_node) {
356-
// extractCallInfo → extractMemberExprCallInfo applies the plain-identifier guard for
357-
// .call/.apply/.bind: when the object is a bare identifier (e.g. `fn.call(ctx)`),
358-
// the call is emitted as static (no dynamic flag), matching the walk path and native engine.
356+
// extractCallInfo → extractMemberExprCallInfo tags .call/.apply/.bind (e.g. `fn.call(ctx)`)
357+
// as dynamic/reflection regardless of receiver shape, matching the walk path and native
358+
// engine (#1778). The #1687 dedup-collision case — the same target already reached by a
359+
// direct call from the same caller in the same scope — is resolved downstream in
360+
// build-edges.ts's emitDirectCallEdgesForCall, not here.
359361
const callInfo = extractCallInfo(c.callmem_fn!, c.callmem_node);
360362
if (callInfo) calls.push(callInfo);
361363
const cbDef = extractCallbackDefinition(c.callmem_node, c.callmem_fn);
@@ -3357,16 +3359,18 @@ function extractMemberExprCallInfo(fn: TreeSitterNode, callNode: TreeSitterNode)
33573359
};
33583360
}
33593361

3360-
// .call()/.apply()/.bind() — this-rebinding; the wrapped function is the real callee.
3361-
// When the object is a plain identifier (e.g. `f.call({})`), the target is statically
3362-
// known so we emit a static call (no dynamic flag). This keeps parity with the native
3363-
// Rust engine, which also resolves these as dyn=0, and prevents the dynZeroEdgeRows
3364-
// upgrade path in emitDirectCallEdgesForCall from wrongly converting a dyn=0 edge
3365-
// (emitted by a prior direct `f()` call) to dyn=1.
3366-
// When the object is a member_expression (e.g. `obj.method.call({})`), we still mark
3367-
// it dynamic/reflection because the inner callee requires a second resolution hop.
3362+
// .call()/.apply()/.bind() — this-rebinding; the wrapped function is the real callee, but
3363+
// invoking it through .call/.apply/.bind is a genuinely reflective mechanism (a distinct
3364+
// invocation path from a plain `f()` call), so both identifier and member-expression
3365+
// receivers are tagged dynamic/reflection — matching the native Rust engine and preserving
3366+
// the informational value of the `reflection` DynamicKind (queryable via
3367+
// `codegraph roles --dynamic`; see ADR-002). This does NOT reintroduce #1687: that bug was
3368+
// a dedup-collision in build-edges.ts (a direct `f()` edge getting wrongly flipped to dyn=1
3369+
// by a later `f.call()` to the same target in the same scope), fixed narrowly at the
3370+
// edge-emission layer in emitDirectCallEdgesForCall rather than by suppressing the tag here.
33683371
if (propText === 'call' || propText === 'apply' || propText === 'bind') {
3369-
if (obj && obj.type === 'identifier') return { name: obj.text, line: callLine };
3372+
if (obj && obj.type === 'identifier')
3373+
return { name: obj.text, line: callLine, dynamic: true, dynamicKind: 'reflection' };
33703374
if (obj && obj.type === 'member_expression') {
33713375
const innerProp = obj.childForFieldName('property');
33723376
if (innerProp)

tests/engines/dynamic-call-ffi.test.ts

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,28 +65,31 @@ describe('dynamic call classification — dynamicKind and keyExpr fields', () =>
6565
expect(c?.dynamic).toBe(true);
6666
});
6767

68-
it('resolves fn.call(ctx) as a static call — no dynamic flag (#1687)', () => {
69-
// `greet.call(ctx, 'world')` — plain-identifier receiver; target is statically known.
70-
// We emit a static call (no dynamic, no dynamicKind) to match native Rust parity and
71-
// prevent the dynZeroEdgeRows upgrade from promoting a prior dyn=0 edge to dyn=1.
68+
it('tags fn.call(ctx) as reflection kind (#1778)', () => {
69+
// `greet.call(ctx, 'world')` — plain-identifier receiver. The wrapped function
70+
// is the real callee, but invoking it via .call is a genuinely reflective
71+
// mechanism, so it's tagged dynamic/reflection — matching native Rust parity.
72+
// (Option A of #1778: the dedup-collision bug this used to work around
73+
// — #1687 — is now fixed narrowly at the build-edges.ts edge-emission layer
74+
// instead of by suppressing this tag for every identifier receiver.)
7275
const out = parseJS(`
7376
function test(ctx) { greet.call(ctx, 'world'); }
7477
`);
7578
const c = out.calls.find((c) => c.name === 'greet');
7679
expect(c).toBeDefined();
77-
expect(c?.dynamic).toBeFalsy();
78-
expect(c?.dynamicKind).toBeUndefined();
80+
expect(c?.dynamicKind).toBe('reflection');
81+
expect(c?.dynamic).toBe(true);
7982
});
8083

81-
it('resolves fn.apply(ctx, args) as a static call — no dynamic flag (#1687)', () => {
82-
// Same as .call(): plain-identifier receiver → static call.
84+
it('tags fn.apply(ctx, args) as reflection kind (#1778)', () => {
85+
// Same as .call(): plain-identifier receiver → dynamic/reflection.
8386
const out = parseJS(`
8487
function test(ctx) { greet.apply(ctx, ['world']); }
8588
`);
8689
const c = out.calls.find((c) => c.name === 'greet');
8790
expect(c).toBeDefined();
88-
expect(c?.dynamic).toBeFalsy();
89-
expect(c?.dynamicKind).toBeUndefined();
91+
expect(c?.dynamicKind).toBe('reflection');
92+
expect(c?.dynamic).toBe(true);
9093
});
9194

9295
it('tags obj[a + b]() as unresolved-dynamic kind', () => {

0 commit comments

Comments
 (0)