test(integration): extend prototype-method-resolution to cover both engines

[carlos-alm]

Summary

• Removes the WASM-only engine pin added in test(integration): pin prototype-method-resolution test to WASM engine #1400 (PR that pinned to WASM because the published binary predated the Rust prototype extraction from feat(resolver): resolve prototype-based method calls, spread/iteration callbacks, func-prop this-dispatch, and object-rest param dispatch (JS) #1331)
• Restructures the test to build the fixture with both engines in separate temp dirs, matching the pattern in issue-1336-object-rest-param-resolution.test.ts
• All 6 assertions (3 WASM + 3 native) pass — the native extractor already implements match_js_prototype_methods in crates/codegraph-core/src/extractors/javascript.rs
• The CI preflight job builds the native addon from source before running tests, so this will pass there too

Test plan

• WASM: emits Dog.bark as a method definition
• WASM: resolves d.bark() to Dog.bark via typeMap receiver type
• WASM: resolves (new Dog(...)).bark() inline-new receiver call to Dog.bark
• Native: emits Dog.bark as a method definition
• Native: resolves d.bark() to Dog.bark via typeMap receiver type
• Native: resolves (new Dog(...)).bark() inline-new receiver call to Dog.bark

Closes #1421

[claude]

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

---

PR Review

• Read changed test file
• Read related reference test file (issue-1336 pattern)
• Phase 1: Root cause analysis
• Phase 2: Configuration change detection
• Phase 3: Test integrity
• Phase 4: Code quality
• Phase 5: Backlog compliance
• Post final assessment

---

Root Cause Analysis

Problem Being Solved: PR #1400 had pinned prototype-method-resolution.test.ts to WASM-only because the published native binary at the time predated the Rust prototype-extraction fix from #1331. That pin was a temporary workaround. Now that the native extractor implements match_js_prototype_methods in crates/codegraph-core/src/extractors/javascript.rs and CI builds the addon from source, the pin is no longer justified. This PR removes the pin and expands the test to cover both engines.

Why This Approach: Structurally mirrors the established dual-engine test pattern from issue-1336-object-rest-param-resolution.test.ts — separate tmpWasm/tmpNative dirs, separate buildGraph calls in beforeAll, and interleaved WASM:/Native: it-blocks. That's the correct pattern for this kind of parity test.

Risk Assessment: Low. This is a pure test expansion — no production code changes. The only risk would be if the native extractor is not yet correct and the tests fail in CI. The PR description claims all 6 pass; CI will be the ground truth.

---

Phase 2: Configuration Change Detection

No linting rules, TypeScript flags, test timeouts, or build configuration were touched. No concerns.

---

Phase 3: Test Integrity

The assertions are at least as strict as before. The original test had 3 assertions for WASM; this PR keeps all 3 and adds 3 identical assertions for native. No assertion was softened, skipped, or made conditional.

One minor observation:

Line 52 — native engine not explicitly pinned:

await buildGraph(tmpNative, { incremental: false, skipRegistry: true, engine: 'native' });

This correctly passes engine: 'native'. Compare with the reference file (issue-1336, line 59) where the native build omits engine: and relies on the default 'auto'. The explicit engine: 'native' here is actually more correct for a parity test and a slight improvement over the reference pattern.

Line 98 — WASM d.bark() assertion is looser than the makeAndBark assertion:

expect(edges.find((e) => e.tgt === 'Dog.bark')).toBeDefined();

This only checks that some edge targets Dog.bark, not that it specifically originates from the top-level scope where d.bark() is called. The makeAndBark assertion (e.src === 'makeAndBark' && e.tgt === 'Dog.bark') is tighter. The looser form is acceptable here because there's only one other call site, but tightening it to also assert e.src (e.g., the module-level scope name) would future-proof it against fixtures that add more callers.

This is a minor quality point, not a blocker.

---

Phase 4: Code Quality

The test is clean and follows the established pattern faithfully. The readCallEdges and getNode helpers are simple, well-scoped, and correctly use readonly: true + try/finally for safe DB access (prototype-method-resolution.test.ts:61–76, 78–87).

No concerns.

---

Phase 5: Backlog Compliance

• Zero-dep ✓ — No new runtime dependencies.
• Foundation-aligned ✓ — Pure test coverage for existing local-graph functionality. No cloud, no agent behavior, no API contract changes.
• Problem-fit: 4 — Engine parity testing directly prevents the class of bug where AI agents get different (hallucinated) call graphs depending on which engine is active. Correct prototype-method edges are core to blast-radius analysis.
• Breaking: No — No production code changed.
• Tier: 1

---

Final Assessment

## Root Cause Analysis
Problem Being Solved: Remove a temporary WASM-only engine pin that was added
  as a workaround for a stale binary, and expand the test to cover both engines.
Why This Approach: Mirrors the canonical dual-engine test pattern already
  established in issue-1336. Correct and consistent.
Risk Assessment: Low — test-only change. CI is the arbiter.

## Backlog Compliance
- Zero-dep: ✓
- Foundation-aligned: ✓
- Problem-fit: 4 — Engine parity for prototype resolution directly affects
  graph correctness and AI blast-radius analysis.
- Breaking: No
- Tier: 1

## Critical Concerns
None.

## Minor Suggestions (non-blocking)
- tests/integration/prototype-method-resolution.test.ts:98,114 — Consider
  tightening the "resolves d.bark()" assertion to also assert the source
  node (e.g. the module-level scope), matching the precision of the
  makeAndBark assertion on lines 103/119. Not required for this PR.

## Final Recommendation
- Rating: ⭐⭐⭐⭐ (4/5)
- Action: APPROVE
- Reasoning: Clean, focused removal of a justified-but-temporary workaround.
  All assertions maintained or strengthened. Pattern is consistent with the
  codebase's established dual-engine test convention. The one minor looseness
  in the d.bark() assertion is pre-existing and not introduced by this PR.

[greptile-apps]

Greptile Summary

This PR upgrades the prototype-method-resolution integration test from a WASM-only pin to a full dual-engine test suite, removing the TODO(#1381) workaround introduced in #1400 now that the native extractor implements match_js_prototype_methods.

• Restructures the fixture from a file-map object to a bare FIXTURE_CODE string, writes it into two separate temp directories, and runs both buildGraph calls concurrently via Promise.all.
• Expands the test suite from 3 WASM-only assertions to 6 assertions (3 WASM + 3 native), and tightens the d.bark() edge assertion to pin src === 'proto.js' (the file-scope node), consistent with the reply to the previous review comment.

Confidence Score: 5/5

Safe to merge — this is a well-scoped test-only change that removes a documented workaround and adds coverage for an engine that already ships the fix.

The change touches only a single integration test file, adds no production code, and the dual-engine setup with parallel builds and tighter per-scope assertions is sound. The beforeAll/afterAll cleanup pattern is consistent with the rest of the test suite.

No files require special attention.

Important Files Changed

Filename	Overview
tests/integration/prototype-method-resolution.test.ts	Removes WASM-only engine pin, adds dual-engine setup with parallel buildGraph calls and 6 well-scoped assertions; no logic or correctness issues found.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[beforeAll] --> B[mkdtempSync: cg-1317-wasm-]
    A --> C[mkdtempSync: cg-1317-native-]
    B --> D[writeFileSync proto.js]
    C --> E[writeFileSync proto.js]
    D --> F[Promise.all]
    E --> F
    F --> G[buildGraph tmpWasm - engine: wasm]
    F --> H[buildGraph tmpNative - engine: native]
    G --> I[WASM: graph.db]
    H --> J[Native: graph.db]
    I --> K[WASM: emits Dog.bark as method]
    I --> L[WASM: d.bark → Dog.bark via proto.js scope]
    I --> M[WASM: makeAndBark → Dog.bark inline-new]
    J --> N[Native: emits Dog.bark as method]
    J --> O[Native: d.bark → Dog.bark via proto.js scope]
    J --> P[Native: makeAndBark → Dog.bark inline-new]

Loading

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

[greptile-apps]

The two buildGraph calls in beforeAll run sequentially — the native build doesn't start until the WASM build finishes. Because both fixtures are written to separate temp dirs before either build begins, they can safely run in parallel with Promise.all, matching the pattern used in issue-1336-object-rest-param-resolution.test.ts. Parallelizing reduces total test-setup wall time.

Suggested change

	tmpWasm = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1317-wasm-'));
	fs.writeFileSync(path.join(tmpWasm, 'proto.js'), FIXTURE_CODE);
	await buildGraph(tmpWasm, { incremental: false, skipRegistry: true, engine: 'wasm' });
	tmpNative = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1317-native-'));
	fs.writeFileSync(path.join(tmpNative, 'proto.js'), FIXTURE_CODE);
	await buildGraph(tmpNative, { incremental: false, skipRegistry: true, engine: 'native' });
	tmpWasm = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1317-wasm-'));
	fs.writeFileSync(path.join(tmpWasm, 'proto.js'), FIXTURE_CODE);
	tmpNative = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1317-native-'));
	fs.writeFileSync(path.join(tmpNative, 'proto.js'), FIXTURE_CODE);
	await Promise.all([
	buildGraph(tmpWasm, { incremental: false, skipRegistry: true, engine: 'wasm' }),
	buildGraph(tmpNative, { incremental: false, skipRegistry: true, engine: 'native' }),
	]);

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]

Done. Both buildGraph calls now run concurrently via Promise.all — the fixture files are written to their separate temp dirs before either build starts, so there is no shared state and the parallelization is safe. The d.bark() assertions have also been tightened to assert src === "proto.js" (the file-level scope node that owns the module-level call), matching the precision of the makeAndBark assertions.

[carlos-alm]

@greptileai

[carlos-alm]

@claude

[claude]

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

---

Review complete

• Read current test file — all changes already applied
• Assess state — no implementation needed

---

The file is already in its final form. Both suggestions from the Greptile review have been addressed:

Promise.all parallelization (prototype-method-resolution.test.ts:52–55): The two buildGraph calls now run concurrently. Fixture files are written to their separate temp dirs before either build starts, so there is no shared state.

Tightened d.bark() assertions (lines 101, 117): Both now assert e.src === 'proto.js', matching the precision of the makeAndBark assertions. This pins the source node to the file-level scope that owns the module-level d.bark() call, not just any edge that happens to target Dog.bark.

The test is clean and matches the established dual-engine pattern. Nothing further needed.

View job