Skip to content

fix(wasm-worker): wire paramBindings, returnTypeMap, callAssignments through worker boundary#1352

Merged
carlos-alm merged 1 commit into
mainfrom
fix/wasm-worker-boundary-1348
Jun 6, 2026
Merged

fix(wasm-worker): wire paramBindings, returnTypeMap, callAssignments through worker boundary#1352
carlos-alm merged 1 commit into
mainfrom
fix/wasm-worker-boundary-1348

Conversation

@carlos-alm

Copy link
Copy Markdown
Contributor

Summary

  • SerializedExtractorOutput in wasm-worker-protocol.ts was missing paramBindings, returnTypeMap, and callAssignments — three fields the JS/TS extractor populates but that were silently dropped at the Worker thread boundary
  • Added all three to SerializedExtractorOutput (with returnTypeMap encoded as Array<[string, TypeMapEntry]> tuples, matching the existing typeMap pattern)
  • Wired them through serializeExtractorOutput in wasm-worker-entry.ts and deserializeResult in wasm-worker-pool.ts

Closes #1348

…through worker boundary

These three ExtractorOutput fields were populated by the JS/TS extractor
but never included in SerializedExtractorOutput, causing them to be silently
dropped when results crossed the Worker thread boundary. The main thread
always received undefined for these fields regardless of what the worker
computed, breaking cross-file return-type propagation, param binding
resolution, and call-assignment tracking in the WASM engine.

Fixes #1348
@claude

claude Bot commented Jun 6, 2026

Copy link
Copy Markdown

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


PR Review

  • Gather context
  • Read changed files
  • Analyze root cause & approach
  • Review code quality
  • Post final review

Root Cause Analysis

Problem Being Solved: Three fields populated by the JS/TS extractor (paramBindings, returnTypeMap, callAssignments) were silently dropped at the Worker thread boundary because SerializedExtractorOutput in wasm-worker-protocol.ts didn't declare them. This means any JS/TS analysis using the WASM engine (fallback path) was missing inter-procedural type propagation, cross-file call assignment data, and parameter-flow points-to analysis — all of which drive Phase 8 resolution features.

Why This Approach: Correct and minimal. The serialization pattern for returnTypeMap (encode as Array<[string, TypeMapEntry]> tuples) mirrors the existing typeMap pattern already established in this file. All three fields are purely additive — no schema change, no wire format breakage for existing fields.

Risk Assessment: Low. The changes are purely additive. No existing deserialization path is modified; new fields are only wired in when present (?.length / ?.size guards). The symmetry with existing typeMap / fnRefBindings / newExpressions is solid.


Backlog Compliance

  • Zero-dep: ✓ — no new runtime dependencies
  • Foundation-aligned: ✓ — purely internal plumbing within the WASM engine path; no cloud calls, no external tools, no agent behavior
  • Problem-fit: 5 — directly fixes the WASM engine producing wrong results for inter-procedural resolution (hallucinated/missed edges), which is exactly the "AI loses its way in large codebases" problem this tool addresses
  • Breaking: No — purely additive fields, backward-compatible serialization
  • Tier: 1

Code Quality

The implementation is clean and consistent with existing patterns. A few observations:

wasm-worker-protocol.ts:69-71 — The three new fields match the optional pattern of fnRefBindings and newExpressions already in the interface. Good.

wasm-worker-entry.ts:807-813 — The serialization correctly uses .size for returnTypeMap (it's a Map) and .length for the array fields. The spread-conditional pattern ...(condition ? { field: value } : {}) is consistent with fnRefBindings/newExpressions above it. Correct.

wasm-worker-pool.ts:111-117 — Deserialization reconstructs returnTypeMap as a Map<string, TypeMapEntry> from the tuple array — exactly matching how typeMap is deserialized at lines 91-92. The .length guards prevent empty arrays from creating empty Maps unnecessarily. Correct.

One minor observation: The deserializeResult function at line 93 constructs out with only the 5 required fields, then conditionally assigns optional ones. This is consistent with the existing pattern for fnRefBindings / newExpressions. No issue.

No test coverage for the new fields — there are no new tests specifically exercising that paramBindings, returnTypeMap, and callAssignments survive the worker round-trip. Given the PR is fixing a silent data-loss bug, a regression test (e.g. parse a small JS fixture through the pool and assert the three fields are present on the result) would prevent this from regressing. That said, the fix is mechanical enough and the existing worker tests likely cover the overall path — this is a recommendation, not a blocker.


Critical Concerns

None. The fix is correct and tightly scoped.


Final Recommendation

  • Rating: ⭐⭐⭐⭐ (4/5)
  • Action: APPROVE
  • Reasoning: Fixes a real silent data-loss bug at the worker boundary. The approach is correct, consistent with established patterns, and carries no risk of breakage. The only gap is the absence of a targeted regression test for the three newly-wired fields — worth a follow-up issue but not a blocker for this fix.

@greptile-apps

greptile-apps Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a silent data-loss bug where paramBindings, returnTypeMap, and callAssignments — all populated by the JS/TS extractor — were dropped at the WASM worker thread boundary because SerializedExtractorOutput lacked those fields.

  • wasm-worker-protocol.ts: Adds returnTypeMap?: Array<[string, TypeMapEntry]>, callAssignments?: CallAssignment[], and paramBindings?: ParamBinding[] to SerializedExtractorOutput, with top-level imports for the two new types.
  • wasm-worker-entry.ts: Serializes the three fields in serializeExtractorOutput, correctly using .size for the Map and .length for the arrays, following the existing typeMap/fnRefBindings pattern.
  • wasm-worker-pool.ts: Deserializes the fields in deserializeResult, reconstructing returnTypeMap as a Map<string, TypeMapEntry> from the tuple array — symmetric with the serializer and identical to the existing typeMap reconstruction.

Confidence Score: 5/5

Safe to merge — the change is a targeted, symmetric addition to a well-established serialization/deserialization pair, and all three new fields carry only plain-primitive data.

The serializer correctly uses .size for the Map check and .length for array checks. The deserializer reconstructs returnTypeMap as a proper Map using the same tuple-iteration pattern already in use for typeMap. CallAssignment and ParamBinding are pure-primitive POJOs, so structured clone handles them without any conversion. The change is fully symmetric across all three files and follows existing conventions throughout.

No files require special attention.

Important Files Changed

Filename Overview
src/domain/wasm-worker-protocol.ts Adds three missing optional fields to SerializedExtractorOutput; imports are correct and types match ExtractorOutput in types.ts.
src/domain/wasm-worker-entry.ts Serializes the three new fields using the correct emptiness checks (.size for Map, .length for arrays), consistent with the pre-existing typeMap and fnRefBindings patterns.
src/domain/wasm-worker-pool.ts Deserializes the three new fields correctly; returnTypeMap is reconstructed as a Map from tuple entries, mirroring the existing typeMap reconstruction exactly.

Sequence Diagram

sequenceDiagram
    participant W as WASM Worker (wasm-worker-entry.ts)
    participant P as Protocol (SerializedExtractorOutput)
    participant M as Main Thread (wasm-worker-pool.ts)

    W->>W: JS/TS extractor populates ExtractorOutput
    W->>W: serializeExtractorOutput()
    Note over W: typeMap → Array tuples
    Note over W: returnTypeMap → Array tuples (if size>0)
    Note over W: callAssignments passthrough (if length>0)
    Note over W: paramBindings passthrough (if length>0)
    W-->>P: postMessage(SerializedExtractorOutput)
    Note over W,P: structured clone crosses worker boundary
    P-->>M: message received
    M->>M: deserializeResult()
    Note over M: typeMap tuples → new Map()
    Note over M: returnTypeMap tuples → new Map() (if length>0)
    Note over M: callAssignments assigned directly
    Note over M: paramBindings assigned directly
    M->>M: returns ExtractorOutput with all fields populated
Loading

Reviews (1): Last reviewed commit: "fix(wasm-worker): wire paramBindings, re..." | Re-trigger Greptile

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Codegraph Impact Analysis

5 functions changed3 callers affected across 2 files

  • serializeExtractorOutput in src/domain/wasm-worker-entry.ts:790 (2 transitive callers)
  • deserializeResult in src/domain/wasm-worker-pool.ts:89 (1 transitive callers)
  • SerializedExtractorOutput.returnTypeMap in src/domain/wasm-worker-protocol.ts:69 (0 transitive callers)
  • SerializedExtractorOutput.callAssignments in src/domain/wasm-worker-protocol.ts:70 (0 transitive callers)
  • SerializedExtractorOutput.paramBindings in src/domain/wasm-worker-protocol.ts:71 (0 transitive callers)

@carlos-alm carlos-alm merged commit 6031440 into main Jun 6, 2026
24 checks passed
@carlos-alm carlos-alm deleted the fix/wasm-worker-boundary-1348 branch June 6, 2026 08:34
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 6, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(wasm-worker): paramBindings, returnTypeMap, callAssignments silently dropped at worker thread boundary

1 participant