feat(resolver): Phase 8.3f — object destructuring rest parameter resolution (WASM + native)

[carlos-alm]

Summary

Implements Phase 8.3f — resolving rest.method() calls where rest is the rest binding from an object-destructuring function parameter — for both WASM and native engines.

Supersedes and closes PR #1350 (WASM-only implementation).

Pattern resolved:

function e4() {}
var obj = { e4 };

function f3({ e1: eee1, ...eerest }) {
    eerest.e4();  // expected edge: f3 → e4 ✓
}
f3(obj);

Resolution chain

1. Object literal property seeding — handleVarDeclaratorTypeMap seeds typeMap['obj.e4'] = { type: 'e4' } for shorthand properties in object literals.
2. Rest-param binding extraction — extractObjectRestParamBindingsWalk records { callee: 'f3', argIndex: 0, restName: 'eerest' } from function f3({ a, ...eerest }).
3. Phase 8.3f seeding — seeds typeMap['eerest'] = { type: 'obj' } by cross-referencing objectRestParamBindings × paramBindings.
4. Resolution — resolveByMethodOrGlobal: typeMap['eerest'] → 'obj'; typeMap['obj.e4'] → 'e4'; lookup → resolved edge.

WASM path: seeding happens in buildCallEdgesJS before call-edge resolution.

Native path: new buildObjectRestParamPostPass runs after buildFnRefBindingsPtsPostPass; for same-file/imported callees the native Rust engine already resolves via its own steps 1–2, so seenByPair prevents duplicates; the post-pass adds coverage for the typeMap-chain path.

Changes

Commit 1 (cherry-picked from #1350): WASM/JS implementation

• src/types.ts — ObjectRestParamBinding type + ExtractorOutput.objectRestParamBindings
• src/extractors/javascript.ts — extractObjectRestParamBindingsWalk; object literal property seeding
• src/domain/graph/builder/stages/build-edges.ts — Phase 8.3f seeding in buildCallEdgesJS
• WASM worker serialization in wasm-worker-{protocol,entry,pool}.ts
• Parser unit tests (6 new) + Jelly micro-test fixture

Commit 2 (this PR): Native engine parity

• src/domain/graph/builder/stages/build-edges.ts — buildObjectRestParamPostPass wired into native call-edge path
• Updated #1336 integration test to assert both WASM and native engines produce f3 → e4

Commit 3: wasm-worker boundary fix (#1352)

• Wires paramBindings, returnTypeMap, callAssignments through WASM worker serialization boundary

Commit 4: defensive-copy typeMap in buildCallEdgesJS

• buildCallEdgesJS was taking a direct reference to symbols.typeMap; clones it before Phase 8.3f seeding to prevent seeds persisting across incremental rebuilds
• Tracks rest-param scoping gap in fix(resolver): scope Phase 8.3f typeMap key by function to avoid same-name rest-param collision #1358

Commit 5: Extend binding walk to class and object-literal methods (#1357)

• extractObjectRestParamBindingsWalk now also handles method_definition (class methods and object-literal shorthand methods) and pair nodes with function values
• Threads currentClass context so class methods emit ClassName.methodName as callee
• 4 new unit tests

Related

• Closes feat(resolver): resolve property calls on object destructuring rest parameters (JS) #1336 (original WASM issue)
• Closes fix(native): add object destructuring rest-parameter resolution (Phase 8.3f parity) #1349 (native parity)
• Closes follow-up: extend extractObjectRestParamBindingsWalk to cover class methods and object-literal methods #1357 (extend binding walk to class/object-literal methods)
• Related: fix(wasm-worker): paramBindings, returnTypeMap, callAssignments silently dropped at worker thread boundary #1348 (missing WASM worker serialization for paramBindings/returnTypeMap/callAssignments)
• Related: fix(resolver): scope Phase 8.3f typeMap key by function to avoid same-name rest-param collision #1358 (rest-param scoping gap — same restName in two functions in one file)

[claude]

Claude finished @carlos-alm's task —— View job

---

PR Review: Phase 8.3f — Object Destructuring Rest Parameter Resolution

• Gather context and understand the request
• Read changed files
• Phase 1: Root cause analysis
• Phase 2: Configuration change detection
• Phase 3: Test integrity
• Phase 4: Code quality
• Phase 5: Backlog compliance
• Post final review

---

Root Cause Analysis

Problem Being Solved: When a JS/TS function uses object-destructuring with a rest parameter (function f3({ a, ...rest }) {}), and the rest object's method is subsequently called (rest.method()), codegraph was failing to emit the call edge because neither the WASM nor native engine had a mechanism to trace rest → original argument → method lookup through the typeMap chain.

Why This Approach: The fix chains four steps: (1) seed obj.e4 → e4 in typeMap from object literal declarations, (2) record rest-param bindings from function signatures, (3) cross-reference with call-site paramBindings to seed typeMap[restName] = argName, (4) resolve via typeMap[argName.method]. This follows the existing Phase 8.3 points-to architecture rather than introducing a new mechanism — correct choice.

Risk Assessment: Medium. The WASM path has a serialization gap (see Critical Concerns below). Native path looks correct. Duplicate-prevention via seenByPair is well-designed.

---

Phase 2: Configuration Changes

No linting rules relaxed, no TypeScript strict flags disabled, no coverage thresholds changed, no build configuration modified. Clean.

---

Phase 3: Test Integrity

Tests are additive (+435/-0). New assertions are appropriately strict (expect(edge).toBeDefined(), expect(edge!.dynamic).toBe(0)). Parser unit tests verify both extraction and typeMap seeding at the unit level. The integration test verifies the full pipeline for both engines.

One concern: The WASM integration test may be validating behavior through a path that doesn't exercise the serialization roundtrip — see Critical Concerns.

---

Phase 4: Code Quality

src/extractors/javascript.ts — extractObjectRestParamBindingsWalk (line 1794)

The implementation correctly handles function_declaration, generator_function_declaration, and variable_declarator variants. However, there is no handling for method_definition (class methods) or export_default_declaration wrapping a function. Neither variant is tested. This is an acceptable scope limit but should be documented or tracked.

src/extractors/javascript.ts — handleVarDeclaratorTypeMap shorthand seeding (line 1596)

seedProtoProperties is called when valueN.type === 'object'. This correctly handles { e4, handler }. The condition is appropriately scoped — no false positives for non-object initialisers.

src/domain/graph/builder/stages/build-edges.ts — buildObjectRestParamPostPass (line 726)

The sharedLookup parameter correctly reuses a shared CallNodeLookup across post-passes, avoiding redundant closure construction. The seenByPair dedup seeds from the entire allEdgeRows snapshot (all edge kinds, not just calls), unlike buildChaPostPass which filters to calls only. This is intentional — the function needs to avoid re-emitting any edge the native engine already emitted.

Minor: The typeMap cast at line 787 — typeMap as Map<string, unknown> — works but weakens the type. This pre-existing pattern is consistent with adjacent post-passes so not a blocker.

---

Critical Concern: paramBindings Not Serialized Through WASM Worker Protocol

src/domain/wasm-worker-protocol.ts / src/domain/wasm-worker-pool.ts

SerializedExtractorOutput (protocol.ts:48) includes objectRestParamBindings (added by this PR) but does not include paramBindings. The deserializeResult function (pool.ts:89) similarly reconstructs objectRestParamBindings but never restores paramBindings.

In buildCallEdgesJS (build-edges.ts:936), Phase 8.3f is gated on both:

if (symbols.objectRestParamBindings?.length && symbols.paramBindings?.length) {

On the WASM worker path, symbols.paramBindings will always be undefined after deserialization, so Phase 8.3f seeding is silently skipped. The eerest.e4() edge will not be resolved when using the WASM engine with the worker pool (which is the default engine: 'wasm' path for non-trivial project sizes).

The WASM integration test (line 82-87) may be passing for the wrong reason. buildGraph with engine: 'wasm' dispatches to parseFilesWasm → pool.parse() → worker. If paramBindings is absent after deserialization, the Phase 8.3f condition fails silently, and the f3 → e4 edge would not be produced. The test should be verifying this scenario is actually working — not just checking the presence of an edge that may be emitted through a different resolution path (e.g., direct name lookup in resolveCallTargets).

The PR description acknowledges this under "Related: #1348 (missing WASM worker serialization for paramBindings/returnTypeMap/callAssignments)" but marks it as "related" rather than a blocking dependency. This is a problem: if #1348 is not fixed first, the WASM path for Phase 8.3f is incomplete regardless of what the integration test shows.

This needs to be resolved before merging. Either:

1. Add paramBindings to SerializedExtractorOutput + deserializeResult in this PR (simple, self-contained), or
2. Block this PR on fix(wasm-worker): paramBindings, returnTypeMap, callAssignments silently dropped at worker thread boundary #1348 and add a test that explicitly verifies symbols.paramBindings survives the worker roundtrip.

---

Backlog Compliance

• Zero-dep: ✓ — no new runtime dependencies
• Foundation-aligned: ✓ — purely local, SQLite-based, no external calls, builds on existing Phase 8.3 pts architecture
• Problem-fit: 4/5 — directly improves call-edge accuracy for a real JS pattern (object destructuring rest params), reduces hallucinated missing edges for AI agents navigating codebases with this pattern
• Breaking: No — purely additive, no API/CLI/schema changes
• Tier: 1

---

Final Recommendation

• Rating: ⭐⭐⭐☆☆ (3/5)
• Action: REQUEST CHANGES
• Reasoning: The core approach is architecturally sound, well-tested at the unit level, and correctly integrated into the native post-pass chain. However, there is a silent regression on the WASM worker path: paramBindings is not serialized through the worker protocol, so the Phase 8.3f typeMap seeding in buildCallEdgesJS will never fire when using engine: 'wasm' with the worker pool. The integration test does not catch this because it may be passing via a different resolution path. Fix is straightforward — add paramBindings to SerializedExtractorOutput and deserializeResult — and should be included in this PR rather than deferred to fix(wasm-worker): paramBindings, returnTypeMap, callAssignments silently dropped at worker thread boundary #1348.

[greptile-apps]

Greptile Summary

Implements Phase 8.3f object-destructuring rest-parameter resolution for both the WASM and native engine paths, addressing eerest.e4() → f3 → e4 call-edge generation when f3 is called as f3(obj) and obj is a shorthand object literal containing e4.

• WASM path: buildCallEdgesJS now defensively copies symbols.typeMap before Phase 8.3f seeding and seeds typeMap[restName] = { type: argName } from the cross-product of objectRestParamBindings × paramBindings.
• Native path: new buildObjectRestParamPostPass mirrors the seeding logic and filters already-resolved edges via seenByPair; wired in after buildFnRefBindingsPtsPostPass in the post-pass chain.
• Extractor: extractObjectRestParamBindingsWalk is refactored to handle generator_function_declaration, method_definition (class and object-literal shorthand methods), and pair nodes with function values, threading currentClass through class_body so class methods emit qualified ClassName.method callees. 10 new parser unit tests and a WASM+native integration test are added.

Confidence Score: 5/5

Safe to merge — all known correctness issues from prior review rounds have been addressed in this PR's commits.

The defensive new Map(...) copy in both the WASM and native paths prevents typeMap seeds from persisting into the cached ExtractorOutput across incremental runs. The native post-pass correctly initialises seenByPair from the full allEdgeRows snapshot before adding any new edges, consistent with all sibling post-passes. The dynamic: 0 assertion was added to the native integration test in this PR. The restName scoping gap (#1358) is explicitly deferred and tracked upstream.

No files require special attention.

Important Files Changed

Filename	Overview
src/domain/graph/builder/stages/build-edges.ts	Adds buildObjectRestParamPostPass (native path) mirroring the WASM Phase 8.3f seeding, with correct defensive new Map(...) copy, seenByPair deduplication from all existing edges, and a restNames pre-filter. buildCallEdgesJS now also copies symbols.typeMap before seeding, fixing the incremental-rebuild mutation bug.
src/extractors/javascript.ts	Refactors extractObjectRestParamBindingsWalk to cover generator_function_declaration, method_definition (class + object-literal shorthand), and pair nodes with function values; threads currentClass correctly through class_declaration → class_body → method_definition.
tests/integration/issue-1336-object-rest-param-resolution.test.ts	New integration test exercises both WASM and native engines end-to-end. Both assertions now include expect(edge!.dynamic).toBe(0), catching dynamic-dispatch fallbacks.
tests/parsers/javascript.test.ts	Ten new unit tests cover the extractor across all supported function forms including class methods, object-literal shorthand, pair with function value, and anonymous class.

Sequence Diagram

sequenceDiagram
    participant Extractor as javascript.ts (Extractor)
    participant TypeMap as typeMap
    participant PostPass as buildObjectRestParamPostPass / buildCallEdgesJS
    participant Resolver as resolveCallTargets

    Extractor->>TypeMap: "seed typeMap['obj.e4'] = {type:'e4'} (object literal seeding)"
    Extractor->>Extractor: "record objectRestParamBinding {callee:'f3', argIndex:0, restName:'eerest'}"
    Extractor->>Extractor: "record paramBinding {callee:'f3', argIndex:0, argName:'obj'} from f3(obj)"

    PostPass->>TypeMap: "seed typeMap['eerest'] = {type:'obj', confidence:0.65} (Phase 8.3f)"

    PostPass->>Resolver: "resolveCallTargets(call{receiver:'eerest', method:'e4'}, typeMap)"
    Resolver->>TypeMap: lookup typeMap['eerest'] → 'obj'
    Resolver->>TypeMap: lookup typeMap['obj.e4'] → 'e4'
    Resolver-->>PostPass: target node for e4

    PostPass->>PostPass: check seenByPair — dedup native engine edges
    PostPass->>PostPass: allEdgeRows.push([f3_id, e4_id, 'calls', conf, 0, 'points-to'])

Loading

_{Reviews (12): Last reviewed commit: "Merge branch 'main' into fix/native-rest..." | Re-trigger Greptile}

[greptile-apps]

Shared typeMap mutated in-place on WASM path

symbols.typeMap is assigned by reference (symbols.typeMap || new Map()), so the subsequent typeMap.set(orpb.restName, ...) call permanently writes into the stored ExtractorOutput object. The native post-pass (buildObjectRestParamPostPass) correctly copies with new Map(symbols.typeMap instanceof Map ? symbols.typeMap : []). In incremental or watch-mode pipelines where fileSymbols for unchanged files is cached across buildEdges invocations, the !typeMap.has(orpb.restName) guard will block re-seeding on the second run — meaning a changed call-site argument (e.g., f3(obj1) → f3(obj2)) won't update the stale entry for the cached file.

[carlos-alm]

Fixed in commit f3a42fe ("fix(resolver): defensive-copy typeMap in buildCallEdgesJS Phase 8.3f block"). buildCallEdgesJS now opens Phase 8.3f with const typeMap = new Map(symbols.typeMap instanceof Map ? symbols.typeMap : []); — a fresh copy — so seeds never persist into the cached ExtractorOutput across incremental/watch runs. The !typeMap.has(orpb.restName) guard is now only preventing duplicate seeding within a single buildEdges invocation, not across invocations.

[greptile-apps]

Rest-param name collision when two functions share the same rest-binding identifier

The typeMap key is orpb.restName (e.g., "rest", "opts", "props") with no function-scope qualifier. If a file contains two functions that each destructure-rest with the same name — a common pattern — the !typeMap.has(orpb.restName) guard silently discards the second seeding. Any rest.method() call in the second function will then be resolved using the first function's argument type, producing incorrect call edges. The same issue is present in the WASM path's Phase 8.3f seeding block in buildCallEdgesJS.

[carlos-alm]

This scoping collision was filed as issue #1358 and deferred — fixing it requires keying typeMap entries by function scope rather than just restName, which coordinates changes in seeding + resolution lookup. The current PR addresses the primary pattern documented in the fixture test. The gap is tracked and not lost.

[greptile-apps]

The native engine test only asserts edge is defined, omitting the dynamic: 0 check that the WASM test includes. This means a dynamic edge (which would indicate incorrect resolution) would silently pass on the native path.

Suggested change

	it('Native: emits a calls edge from f3 to e4 via eerest.e4() rest-receiver resolution', () => {
	const edges = readCallEdges(path.join(tmpNative, '.codegraph', 'graph.db'));
	const edge = edges.find((e) => e.src === 'f3' && e.tgt === 'e4');
	expect(edge).toBeDefined();
	});
	it('Native: emits a calls edge from f3 to e4 via eerest.e4() rest-receiver resolution', () => {
	const edges = readCallEdges(path.join(tmpNative, '.codegraph', 'graph.db'));
	const edge = edges.find((e) => e.src === 'f3' && e.tgt === 'e4');
	expect(edge).toBeDefined();
	expect(edge!.dynamic).toBe(0);
	});

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[carlos-alm]

Fixed — added expect(edge!.dynamic).toBe(0) to the native engine test at line 93 (commit 054c953). The assertion now matches the WASM test, so a dynamic-dispatch fallback in the native engine will be caught as a regression.

[github-actions]

Codegraph Impact Analysis

5 functions changed → 20 callers affected across 5 files

• buildObjectRestParamPostPass in src/domain/graph/builder/stages/build-edges.ts:726 (3 transitive callers)
• buildCallEdgesJS in src/domain/graph/builder/stages/build-edges.ts:1003 (3 transitive callers)
• buildEdges in src/domain/graph/builder/stages/build-edges.ts:1702 (4 transitive callers)
• extractObjectRestParamBindingsWalk in src/extractors/javascript.ts:2304 (3 transitive callers)
• walk in src/extractors/javascript.ts:2308 (24 transitive callers)

[carlos-alm]

Addressed review feedback:

• WASM path in-place typeMap mutation (raised by both Claude and Greptile): buildCallEdgesJS now creates a defensive copy of symbols.typeMap before Phase 8.3f seeding, matching the pattern already used in buildObjectRestParamPostPass. Seeds no longer persist across watch/incremental builds.

• Class method / object-literal method coverage (raised by Claude review): extractObjectRestParamBindingsWalk now handles method_definition (class methods) and pair nodes with function values (object-literal methods). Three new unit tests cover these cases.

• Rest-param scoping collision (raised by Greptile): filed as fix(resolver): scope Phase 8.3f typeMap key by function to avoid same-name rest-param collision #1358 — fixing it requires coordinated changes to seeding + resolution lookup, out of scope for this PR.

@greptileai

[greptile-apps]

Thanks for the thorough follow-up. Confirming what I see in the diff:

Addressed:

• ✅ typeMap defensive copy (WASM path) — new Map(symbols.typeMap instanceof Map ? symbols.typeMap : []) in buildCallEdgesJS now matches the native post-pass pattern. The stale-seeding bug on incremental runs is closed.
• ✅ Class method / pair coverage — method_definition and pair branches in extractObjectRestParamBindingsWalk look correct, and the new unit tests (class method Service.handle, object-literal shorthand, object-literal pair) cover the added paths.
• ✅ Scoping collision → fix(resolver): scope Phase 8.3f typeMap key by function to avoid same-name rest-param collision #1358 — Deferral is reasonable; it requires changing both seeding and resolution lookup to scope keys by caller, which is out of band here.

Still open:

The native integration test is missing the dynamic: 0 assertion (my prior comment on tests/integration/issue-1336-object-rest-param-resolution.test.ts:89-93 is still unresolved). The WASM test asserts edge!.dynamic === 0, but the native test only checks expect(edge).toBeDefined(). A dynamic edge would indicate the Rust engine fell through to a dynamic-dispatch fallback rather than a static type-chain resolution — exactly the kind of regression this test should catch. The suggestion in my prior comment still applies:

  it('Native: emits a calls edge from f3 to e4 via eerest.e4() rest-receiver resolution', () => {
    const edges = readCallEdges(path.join(tmpNative, '.codegraph', 'graph.db'));
    const edge = edges.find((e) => e.src === 'f3' && e.tgt === 'e4');
    expect(edge).toBeDefined();
    expect(edge!.dynamic).toBe(0);
  });

Everything else looks good. LGTM once that assertion is added.

[carlos-alm]

Added the missing expect(edge!.dynamic).toBe(0) assertion to the native engine test (commit 054c953). All three Greptile inline comments now have replies. @greptileai

[carlos-alm]

Addressing Claude's review feedback:

Critical Concern: paramBindings serialization — Fixed in commit 6031440 ("fix(wasm-worker): wire paramBindings, returnTypeMap, callAssignments through worker boundary (#1352)"), which was merged into this branch via 3c164f2. The SerializedExtractorOutput interface now includes paramBindings?: ParamBinding[] (protocol.ts:72) and deserializeResult restores it (pool.ts:119). Phase 8.3f's dual-gated condition (objectRestParamBindings?.length && symbols.paramBindings?.length) now fires correctly on the WASM worker path.

Class method / object-literal method coverage — Fixed in commit 31d7fe7. extractObjectRestParamBindingsWalk was extended to cover method_definition (class methods) and pair nodes (object-literal methods), with new unit tests for both.

typeMap defensive copy — Fixed in commit f3a42fe. buildCallEdgesJS now opens Phase 8.3f with const typeMap = new Map(symbols.typeMap instanceof Map ? symbols.typeMap : []), matching the native post-pass pattern.

dynamic:0 assertion in native test — Added in commit 054c953.

All four issues from the review are resolved.

[carlos-alm]

@greptileai

[carlos-alm]

Addressed Greptile's two remaining concerns:

1. Deleted integration tests — func-prop-this-dispatch.test.ts, prototype-method-resolution.test.ts, and the jelly-micro fixtures (this/, spread/, more1/) were accidentally dropped when resolving an earlier merge conflict with main (feat(resolver): resolve prototype-based method calls, spread/iteration callbacks, func-prop this-dispatch, and object-rest param dispatch (JS) #1331). All have been restored in commit e984226. The PR diff no longer deletes these files — coverage for feat(resolver): resolve this-dispatch on function-as-object property methods (JS) #1334 and feat(resolver): resolve prototype-based method calls (Foo.prototype.bar = function(){}) #1317 is intact.

2. Quote-strip global regex — line 2205 in extractObjectRestParamBindingsWalk now uses keyN.text.slice(1, -1) instead of keyN.text.replace(/['"]/g, '') to avoid stripping embedded quote characters from property key names. Fixed in commit 4d033d6.

[carlos-alm]

@greptileai

[carlos-alm]

Fixed Greptile's remaining suggestion: added a computed_property_name guard to the pair branch in extractObjectRestParamBindingsWalk (commit fa0f4d8). For computed property keys like { [Symbol.iterator]: function({ ...rest }) {} }, keyN.text would be '[Symbol.iterator]' — a callee that can never match a paramBinding — so the binding is now skipped entirely. The guard prevents wasted bindings without affecting any real resolution.

[carlos-alm]

@greptileai