fix(native): persist this/super dispatch via hybrid WASM post-pass#1337
Conversation
…1326) The native orchestrator resolves typed receiver calls but does not persist raw unresolved call site receiver info (this/super) to the DB, so runPostNativeCha could not resolve this.method() or super.method() calls. Add runPostNativeThisDispatch: after the Rust pipeline completes, WASM-re- parses JS/TS/TSX files to collect call sites with this/super receivers, then resolves them through the DB class hierarchy (extends edges) using the existing resolveThisDispatch function. Only runs when extends edges exist. Removes the skipIf(engine === 'native') guards on the this-dispatch and super-dispatch integration tests — both engines now produce identical edges for ConcreteWorker.doWork → ConcreteWorker.prepare and Lion.speak → Animal.speak. The two CHA transitive skips remain (pending abstract_class_ declaration fix in a future native binary). Closes #1326
Greptile SummaryThis PR adds
Confidence Score: 5/5Safe to merge — the post-pass is well-scoped, the two previously identified bugs (SQLite NULL ordering, spurious 'self' receiver) were fixed in fd15d3c, and no new issues were found. The new post-pass correctly builds the parents map from extends edges, restricts WASM re-parsing to only files in the class hierarchy (not the whole project), uses the already-proven resolveThisDispatch function, deduplicates against existing edges via the seen set, and re-classifies roles for newly-reached target methods. The early-return path and the normal return path both pass a valid thisDispatchMs to formatNativeTimingResult. The optional thisDispatchMs field in BuildResult is backward-compatible. The integration tests now exercise both engines on the same assertions. No files require special attention — the native-orchestrator changes are the core of the PR and they look correct after the fd15d3c fixes. Important Files Changed
Sequence DiagramsequenceDiagram
participant Rust as Rust Pipeline
participant Native as tryNativeOrchestrator (TS)
participant WASM as parseFilesWasmForBackfill
participant DB as SQLite DB
participant CHA as resolveThisDispatch
Rust->>DB: Insert nodes + edges (resolves typed receivers, no this/super raw sites)
Native->>DB: runPostNativeCha (CHA/RTA expansion)
Native->>DB: SELECT extends edges → build parents map
DB-->>Native: "{ child_name, parent_name } rows"
Native->>DB: SELECT DISTINCT file FROM extends-edge nodes (hierarchy files only)
DB-->>Native: relFiles[]
Native->>WASM: parseFilesWasmForBackfill(absFiles, rootDir)
WASM-->>Native: Map relPath ExtractorOutput with call.receiver info
loop "Each call site with receiver = this or super"
Native->>DB: findCallerByLineStmt (innermost method at call.line)
DB-->>Native: "{ id, name } callerRow"
Native->>CHA: resolveThisDispatch(call.name, callerRow.name, receiver, chaCtx, lookup)
CHA->>DB: lookup.byName(qualified method name)
DB-->>CHA: matching nodes
CHA-->>Native: target nodes[]
Native->>DB: batchInsertEdges (calls, cha)
end
Native->>DB: classifyNodeRoles (re-classify target files)
Native->>Native: backfillEdgeTechniques
Native-->>Caller: BuildResult (with thisDispatchMs)
Reviews (11): Last reviewed commit: "Merge branch 'main' into fix/native-cha-..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Find the innermost containing method/function for a call at `line` in `file`. | ||
| // NULL end_line sorts last in SQLite ASC → only selected when no bounded node exists. | ||
| const findCallerByLineStmt = db.prepare(` | ||
| SELECT id, name FROM nodes | ||
| WHERE file = ? AND kind IN ('method', 'function') | ||
| AND line <= ? AND (end_line IS NULL OR end_line >= ?) | ||
| ORDER BY (end_line - line) ASC | ||
| LIMIT 1 | ||
| `); |
There was a problem hiding this comment.
end_line is absent
The comment says "NULL end_line sorts last in SQLite ASC", but SQLite actually sorts NULLs first in ASC order (NULLs are considered less than any other value). When any method/function node has end_line IS NULL, the expression (end_line - line) evaluates to NULL, which ranks before every positive span — so that unbounded node is picked by LIMIT 1 instead of the tightest-bounded enclosing method. The intent (use unbounded nodes only as a last resort) is directly contradicted by the actual SQLite behaviour, meaning this/super edges can silently be attributed to the wrong caller.
Fix: replace the ORDER BY with COALESCE(end_line - line, 999999999) ASC or (end_line - line) ASC NULLS LAST (supported since SQLite 3.30.0).
There was a problem hiding this comment.
Fixed — replaced (end_line - line) ASC with COALESCE(end_line - line, 999999999) ASC. SQLite ASC orders NULLs first so the original query would pick unbounded nodes before any bounded node, inverting the intent. The COALESCE sentinel pushes them last, which is what the comment was trying to say. Committed in fd15d3c.
| if (call.receiver !== 'this' && call.receiver !== 'self' && call.receiver !== 'super') | ||
| continue; |
There was a problem hiding this comment.
'self' as a this-like receiver for JS/TS-family files may produce false-positive dispatch edges. In JavaScript and TypeScript self is not a class-instance receiver — it refers to the global WindowOrWorkerGlobalScope object. Any call of the form self.postMessage() or similar inside a Worker would be misidentified as a this-dispatch and resolved against the class hierarchy, yielding spurious calls edges. If self support is intentional (e.g. for CoffeeScript or other transpiled sources), a comment explaining the rationale would help future readers avoid accidentally widening this further.
| if (call.receiver !== 'this' && call.receiver !== 'self' && call.receiver !== 'super') | |
| continue; | |
| if (call.receiver !== 'this' && call.receiver !== 'super') | |
| continue; |
There was a problem hiding this comment.
Fixed — removed 'self' from the receiver check in runPostNativeThisDispatch. This function only runs on JS/TS-family files where self is WindowOrWorkerGlobalScope, not a class instance. Keeping it would have produced spurious dispatch edges for any self.postMessage() or similar Worker call. Applied the suggested fix plus an explanatory comment. Committed in fd15d3c.
…f receiver SQLite ASC ordering puts NULL values first, so (end_line - line) ASC would pick unbounded nodes before any bounded node — inverting the intent. Replace with COALESCE(end_line - line, 999999999) ASC so unbounded nodes sort last. Also remove 'self' from the this/super receiver filter in runPostNativeThisDispatch. In JS/TS files 'self' refers to WindowOrWorkerGlobalScope, not a class instance — including it would produce spurious dispatch edges from Worker call sites.
Codegraph Impact Analysis4 functions changed → 6 callers affected across 6 files
|
…files only On a full native build, runPostNativeThisDispatch was WASM-re-parsing every JS/TS file in the project, adding a costly second parse pass on top of the native Rust parse (measured: +358% ms/file on codegraph itself). Narrow the file set to only files that appear in the class inheritance graph (sources and targets of 'extends' edges). Files outside the hierarchy have no class relationship, so this/super calls in them either resolve locally or are skipped by resolveThisDispatch anyway — WASM re-parsing them adds cost with zero benefit. Also replace the hardcoded 0.1 confidence penalty with the CHA_DISPATCH_PENALTY named constant (already imported), matching every other CHA confidence calculation in native-orchestrator.ts and build-edges.ts. Fixes: regression-guard failure "Build ms/file: 3.6 → 16.5 (+358%)" (#1337)
|
Addressed Greptile's remaining finding from the summary: replaced the hardcoded Also fixed the performance regression caught by the benchmark gate: the WASM re-parse on full native builds was scanning every JS/TS file in the project (+358% ms/file). Narrowed the file set to only files that participate in the class inheritance graph (sources and targets of Committed in 4a5c5b9. |
…Ms timing Add a comment to the incremental-build branch of runPostNativeThisDispatch documenting the known gap: if a parent-class method is replaced (new node ID) but the child file is unchanged, the stale super.method() edge is not refreshed until the next full rebuild. Add wall-clock timing for the this/super dispatch post-pass. The function now returns the elapsed milliseconds (Promise<number>), and the result is threaded through formatNativeTimingResult as a new thisDispatchMs phase. For large class hierarchies the WASM re-parse can be non-trivial, so surfacing it in build diagnostics makes performance regressions visible.
|
Addressed the two remaining findings from the Greptile summary: Incremental build limitation (lines 643–645): Added a comment to the Timing capture: Changed Committed in 773caf0. |
…ss (#1337) The Rust orchestrator runs role classification before the post-passes, so target methods (e.g. Animal.speak, ConcreteWorker.prepare) that had no callers at Rust build time were classified dead or dead-ffi. runPostNativeThisDispatch inserted the correct call edges but never re-ran classifyNodeRoles, leaving stale role labels visible to dead-code detection and API boundary analysis. Mirror the pattern used after runPostNativeCha: change the return type from Promise<number> to Promise<{ elapsedMs: number; targetIds: Set<number> }>, collect target node IDs while building newEdges, then look up the affected files and call classifyNodeRoles on them — same chunk-and-dedupe pattern as the CHA post-pass.
…ch-1326' into fix/native-cha-this-super-dispatch-1326
|
Addressed the missing role re-classification finding from the Greptile summary: The Rust orchestrator classifies node roles before the post-passes, so target methods like Fix: changed the return type from |
1 participant
Summary
this,super) to the DB, sothis.method()andsuper.method()dispatch was silently dropped on the native pathrunPostNativeThisDispatch: after the Rust pipeline completes, WASM-re-parses JS/TS/TSX files to collect call sites withthis/superreceivers, then resolves them through the DB class hierarchy (extendsedges) using the existingresolveThisDispatchfunctionskipIf(engine === 'native')guards on thethis-dispatchandsuper-dispatchintegration tests — both engines now produce identical edgesTest plan
this-dispatch: emits ConcreteWorker.doWork → ConcreteWorker.preparepasses for both wasm and nativesuper-dispatch: emits Lion.speak → Animal.speakpasses for both wasm and nativeabstract_class_declarationfix)Closes #1326