feat(wave2): B2 dual locations for cross-file resolved calls (#81) - #133
Merged
Conversation
Implementation plan for surfacing both the user's call site and the
underlying SDK call site for cross-file-resolved API calls. Threads a
CallTrace { callSite, resolvedSite, hops } structure through the scanner,
intelligence graph, and sidebar webview. Notes #113 corpus fixtures as
blocked (separate extension-benchmark repo).
https://claude.ai/code/session_015jMBgSvEt44xMvfWKzLoXi
Defines ResolvedLocation and CallTrace interfaces plus a degenerate directTrace() constructor for direct (non-propagated) API calls. Foundation for B2 dual-locations (issue #81). https://claude.ai/code/session_015jMBgSvEt44xMvfWKzLoXi
📝 WalkthroughWalkthroughImplements Wave 2 B2: threads a ChangesWave 2 B2: Dual-Location Call Tracing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/ast/cross-file-resolver.ts (2)
361-369:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep
sourceFilepinned to the underlying SDK file.This overwrites
AstCallMatch.sourceFilewith the immediate callee file on every hop. For wrapper-of-a-wrapper cases, that regresses the origin from the real SDK file to the middle wrapper, so downstreamcrossFileOriginno longer matchestrace.resolvedSite.Proposed fix
return { ...callee, line: callerLine, frequency: isMiddleware ? "single" : callerFrequency, loopContext: isMiddleware ? false : callerLoopContext, isMiddleware: isMiddleware || callee.isMiddleware, crossFile: true, - sourceFile: calleeFilePath, + sourceFile: callee.sourceFile ?? calleeFilePath, trace, }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ast/cross-file-resolver.ts` around lines 361 - 369, The current code always sets sourceFile to the immediate calleeFilePath, which loses the original SDK origin across wrapper hops; update the construction of the new AstCallMatch so that sourceFile preserves the underlying SDK file when available (e.g., use callee.sourceFile when callee is already a cross-file match or callee.sourceFile is set, otherwise fall back to calleeFilePath) so crossFileOrigin continues to match trace.resolvedSite across wrapper-of-wrapper cases.
643-651:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid fabricating caller spans from
callee.linefor middleware traces.
findMiddlewareUseLine()returnsnullfor multilineapp.use(...)forms, and this fallback then writes acallSitein the caller file using a line number taken from the callee file. That makes the new UI link jump to unrelated code instead of the actual registration site.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ast/cross-file-resolver.ts` around lines 643 - 651, The code is fabricating a caller span by falling back to callee.line when ctx.middlewareUseLineByName.get(mwName) is null; stop using callee.line as a fallback. Change the call that currently does useLine ?? callee.line to instead pass a null/undefined line (e.g., useLine) and pointSpan(useLine), and update cloneWithCallerContext (and any code that consumes its line/callSite) to accept null/undefined and skip creating a caller callSite when the middleware use line is unknown; keep symbols to edit: useLine (ctx.middlewareUseLineByName), tryPush, cloneWithCallerContext, and pointSpan so no fabricated caller spans are produced.src/scan-results.ts (1)
483-492:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBackfill
callTraceon remote call sites too.These fallbacks only run when a local/synthetic call site is appended.
remote.map(...)leaves preexistingendpoint.callSitesuntouched, so a remote-only direct detection can still reach the UI withcallTrace === undefined, which breaks the B2 “direct calls have hops=0” contract.Suggested fix
export function mergeRemoteAndLocalEndpoints( remote: EndpointRecord[], localCalls: ApiCallInput[], projectId: string, scanId: string ): EndpointRecord[] { - const merged = remote.map((endpoint) => ({ ...endpoint, scope: endpoint.scope ?? classifyEndpointScope(endpoint.url) })); + const merged = remote.map((endpoint) => ({ + ...endpoint, + scope: endpoint.scope ?? classifyEndpointScope(endpoint.url), + callSites: endpoint.callSites.map((site) => ({ + ...site, + callTrace: site.callTrace ?? directTrace(site.file, site.span ?? pointSpan(site.line)), + })), + }));Also applies to: 538-547, 571-580
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/scan-results.ts` around lines 483 - 492, The remote-only call-site mapping leaves callTrace undefined for some entries; update the logic that processes remote.map(...) (the code that pushes into endpoint.callSites) to ensure callTrace is backfilled the same way as for local/synthetic sites by setting callTrace: call.callTrace ?? directTrace(call.file, call.span ?? pointSpan(call.line)); do this for all remote mapping sites (the other occurrences noted around the other similar blocks) so every endpoint.callSites entry has a defined callTrace and preserves the "direct calls have hops=0" contract.
🧹 Nitpick comments (1)
src/test/ast-cross-file-resolver.test.ts (1)
317-354: ⚡ Quick winAdd a 2-hop trace assertion alongside the new B2 coverage.
These checks only lock down the 1-hop case.
cloneWithCallerContext()deriveshopsfromcallee.trace?.hops, so a wrapper-of-a-wrapper regression could still slip through even though this file already has a barrel fixture. Extending that scenario to asserttrace.hops === 2and thatresolvedSitestill points at the deepest SDK file would cover the recursive contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/ast-cross-file-resolver.test.ts` around lines 317 - 354, Add a second test case that exercises a 2-hop propagation: create an intermediate wrapper file (e.g., a wrapper that calls callAI) between "app.ts" and "lib/ai.ts", call the wrapper from app.ts, run runCrossFileResolution over all three PerFileResult fixtures, find the propagated match for app.ts, then assert trace!.hops === 2 and that trace!.resolvedSite.file === "lib/ai.ts" with trace!.resolvedSite.span.startLine === 4 (and keep/assert trace!.callSite.file === "app.ts" and the callSite span pointing at the app call). This verifies cloneWithCallerContext-derived hops propagate recursively and that resolvedSite still points to the deepest SDK location.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/plans/2026-05-30-wave2-b2-dual-locations.md`:
- Line 731: The Self-Review sentence claiming the stable-ID hash “uses
resolvedSite only” contradicts the B3 rationale that IDs are position-free and
that re-keying on resolvedSite.file was rejected; update the wording to state
that B3 uses a position-free stable ID hash (which preserves IDs across
call-site moves) and explicitly note that re-keying on resolvedSite.file was
rejected (see Task 6 regression test and Task 9 documentation) so the sentence
aligns with the documented B3 rationale and reasoning around resolvedSite and
resolvedSite.file.
In `@package.json`:
- Line 201: The long chained command in the "test:scanner" script is fragile and
must be replaced by a Node runner; create a new runner (e.g.,
src/test/run-scanner-tests.ts compiled to dist-test/test/run-scanner-tests.js)
that exports or runs a list/array named tests containing each compiled test path
(e.g., "dist-test/test/scanner-patterns.test.js", ...,
"dist-test/test/call-trace.test.js"), iterate that array and spawn each test
with spawnSync(process.execPath, [testPath], { stdio: "inherit" }) and exit with
the failing status if any test returns non-zero, then update package.json
"test:scanner" to first run the two tsc builds and then call node
dist-test/test/run-scanner-tests.js (keeping the package script short).
In `@src/intelligence/builder.ts`:
- Around line 204-205: The callTrace paths are not normalized before being
stored, causing mismatches with FileNode.id and ApiCallNode.filePath; update the
code that sets callTrace (where call.callTrace is assigned alongside
normalizeCrossFileOrigin) to walk call.callTrace and normalize all file path
fields (e.g., callTrace.callSite.file and callTrace.resolvedSite.file) using the
same normalizer used for crossFileOrigin (normalizeCrossFileOrigin or its
underlying normalize function), then store the normalized callTrace instead of
the verbatim object so callTrace paths consistently match
FileNode.id/ApiCallNode.filePath.
In `@src/scanner/call-trace.ts`:
- Around line 25-28: The directTrace function creates two ResolvedLocation
objects that still share the same nested SourceSpan because resolvedSite: {
...loc } only shallow-clones loc; fix directTrace by cloning the nested span as
well so callSite.span and resolvedSite.span are distinct (e.g., create const
spanClone = { ...span } or use a deep/structured clone and build loc and
resolvedSite using separate span objects) and return { callSite: { file, span },
resolvedSite: { file, span: spanClone }, hops: 0 } so future mutations won't
alias each other.
In `@src/test/call-trace.test.ts`:
- Around line 10-16: The test currently only checks deep equality and can pass
if callSite and resolvedSite are the same object; update the "directTrace: hops
is 0 and both sites are equal" test to also assert they are distinct objects by
adding an identity check: after obtaining trace via
directTrace("services/chat.ts", span) and the existing deepEqual assertions, add
an assertion using assert.notStrictEqual(trace.callSite, trace.resolvedSite) (or
equivalent identity check) to ensure callSite and resolvedSite are non-aliased
while keeping the deep equality assertions to verify content.
---
Outside diff comments:
In `@src/ast/cross-file-resolver.ts`:
- Around line 361-369: The current code always sets sourceFile to the immediate
calleeFilePath, which loses the original SDK origin across wrapper hops; update
the construction of the new AstCallMatch so that sourceFile preserves the
underlying SDK file when available (e.g., use callee.sourceFile when callee is
already a cross-file match or callee.sourceFile is set, otherwise fall back to
calleeFilePath) so crossFileOrigin continues to match trace.resolvedSite across
wrapper-of-wrapper cases.
- Around line 643-651: The code is fabricating a caller span by falling back to
callee.line when ctx.middlewareUseLineByName.get(mwName) is null; stop using
callee.line as a fallback. Change the call that currently does useLine ??
callee.line to instead pass a null/undefined line (e.g., useLine) and
pointSpan(useLine), and update cloneWithCallerContext (and any code that
consumes its line/callSite) to accept null/undefined and skip creating a caller
callSite when the middleware use line is unknown; keep symbols to edit: useLine
(ctx.middlewareUseLineByName), tryPush, cloneWithCallerContext, and pointSpan so
no fabricated caller spans are produced.
In `@src/scan-results.ts`:
- Around line 483-492: The remote-only call-site mapping leaves callTrace
undefined for some entries; update the logic that processes remote.map(...) (the
code that pushes into endpoint.callSites) to ensure callTrace is backfilled the
same way as for local/synthetic sites by setting callTrace: call.callTrace ??
directTrace(call.file, call.span ?? pointSpan(call.line)); do this for all
remote mapping sites (the other occurrences noted around the other similar
blocks) so every endpoint.callSites entry has a defined callTrace and preserves
the "direct calls have hops=0" contract.
---
Nitpick comments:
In `@src/test/ast-cross-file-resolver.test.ts`:
- Around line 317-354: Add a second test case that exercises a 2-hop
propagation: create an intermediate wrapper file (e.g., a wrapper that calls
callAI) between "app.ts" and "lib/ai.ts", call the wrapper from app.ts, run
runCrossFileResolution over all three PerFileResult fixtures, find the
propagated match for app.ts, then assert trace!.hops === 2 and that
trace!.resolvedSite.file === "lib/ai.ts" with trace!.resolvedSite.span.startLine
=== 4 (and keep/assert trace!.callSite.file === "app.ts" and the callSite span
pointing at the app call). This verifies cloneWithCallerContext-derived hops
propagate recursively and that resolvedSite still points to the deepest SDK
location.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 664ae8c1-a7da-4530-b468-13f4f729537c
📒 Files selected for processing (19)
docs/accuracy/traceability.mddocs/superpowers/plans/2026-05-30-wave2-b2-dual-locations.mddocs/superpowers/plans/PROGRESS.mdpackage.jsonsrc/analysis/types.tssrc/ast/ast-scanner.tssrc/ast/cross-file-resolver.tssrc/intelligence/__tests__/builder.test.tssrc/intelligence/builder.tssrc/intelligence/types.tssrc/scan-results.tssrc/scanner/call-trace.tssrc/scanner/core-scanner.tssrc/test/ast-cross-file-resolver.test.tssrc/test/call-trace.test.tssrc/test/endpoint-id.test.tssrc/test/scan-results.test.tswebview/src/components/ResultsPage.tsxwebview/src/types.ts
| 1. *Propagated detections carry both spans + hop count* → Tasks 2 (resolver), 4 (EndpointCallSite), 5 (ApiCallNode). ✓ | ||
| 2. *Direct detections have hops=0 and equal spans* → `directTrace` (Task 1) applied as the fallback in Task 4; tested. ✓ | ||
| 3. *Webview shows both with clear labels* → Tasks 7 (types) + 8 (UI). Manual EDH annotated `[~]`. ✓ | ||
| 4. *Stable IDs hash uses resolvedSite only so refactoring the wrapper doesn't reset state* → satisfied by B3's position-free hash + Task 6 regression test; reasoning documented in Task 9. ✓ (Interpreted as "call-site moves don't reset the ID", which B3 already guarantees; re-keying on `resolvedSite.file` was rejected as a benchmark/aggregation hazard and noted explicitly.) |
There was a problem hiding this comment.
Fix AC-4 wording contradiction in Self-Review.
Line 731 says the stable-ID hash “uses resolvedSite only,” which conflicts with the plan text above that explicitly rejects re-keying on resolvedSite.file. Please align this sentence with the actual B3 rationale.
Suggested wording
-4. *Stable IDs hash uses resolvedSite only so refactoring the wrapper doesn't reset state* → satisfied by B3's position-free hash + Task 6 regression test; reasoning documented in Task 9. ✓ (Interpreted as "call-site moves don't reset the ID", which B3 already guarantees; re-keying on `resolvedSite.file` was rejected as a benchmark/aggregation hazard and noted explicitly.)
+4. *Stable IDs remain call-site-stable across wrapper refactors* → satisfied by B3's position-free hash + Task 6 regression test; reasoning documented in Task 9. ✓ (Re-keying on `resolvedSite.file` was explicitly rejected as a benchmark/aggregation hazard.)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 4. *Stable IDs hash uses resolvedSite only so refactoring the wrapper doesn't reset state* → satisfied by B3's position-free hash + Task 6 regression test; reasoning documented in Task 9. ✓ (Interpreted as "call-site moves don't reset the ID", which B3 already guarantees; re-keying on `resolvedSite.file` was rejected as a benchmark/aggregation hazard and noted explicitly.) | |
| 4. *Stable IDs remain call-site-stable across wrapper refactors* → satisfied by B3's position-free hash + Task 6 regression test; reasoning documented in Task 9. ✓ (Re-keying on `resolvedSite.file` was explicitly rejected as a benchmark/aggregation hazard.) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-05-30-wave2-b2-dual-locations.md` at line 731,
The Self-Review sentence claiming the stable-ID hash “uses resolvedSite only”
contradicts the B3 rationale that IDs are position-free and that re-keying on
resolvedSite.file was rejected; update the wording to state that B3 uses a
position-free stable ID hash (which preserves IDs across call-site moves) and
explicitly note that re-keying on resolvedSite.file was rejected (see Task 6
regression test and Task 9 documentation) so the sentence aligns with the
documented B3 rationale and reasoning around resolvedSite and resolvedSite.file.
| "build:dashboard": "cd dashboard && npm run build && rm -rf ../dashboard-dist && cp -r dist ../dashboard-dist", | ||
| "test": "npm run test:scanner", | ||
| "test:scanner": "tsc -p tsconfig.scanner-tests.json && tsc -p tsconfig.benchmark.json && node dist-test/test/scanner-patterns.test.js && node dist-test/test/workspace-scanner.test.js && node dist-test/test/workspace-file-access.test.js && node dist-test/test/endpoint-classification.test.js && node dist-test/test/local-waste-detector.test.js && node dist-test/test/chat-providers.test.js && node dist-test/test/fingerprint-registry.test.js && node dist-test/test/pricing-sync.test.js && node dist-test/test/ast-parser-loader.test.js && node dist-test/test/ast-call-visitor.test.js && node dist-test/test/ast-import-resolver.test.js && node dist-test/test/ast-scanner.test.js && node dist-test/test/ast-python.test.js && node dist-test/test/ast-frequency-analyzer.test.js && node dist-test/test/ast-cache-detector.test.js && node dist-test/test/ast-batch-detector.test.js && node dist-test/test/ast-concurrency-detector.test.js && node dist-test/test/ast-cross-file-resolver.test.js && node dist-test/test/a1-multi-hop-wrappers.test.js && node dist-test/intelligence/__tests__/builder.test.js && node dist-test/intelligence/__tests__/clusters.test.js && node dist-test/intelligence/__tests__/compression.test.js && node dist-test/intelligence/__tests__/export.test.js && node dist-test/test/api-client.test.js && node dist-test/test/key-management.test.js && node dist-test/test/ast-parser-loader-fallback.test.js && node dist-test/intelligence/__tests__/cost-utils.test.js && node dist-test/test/intelligence-compression-async.test.js && node dist-test/test/webview-provider-dispatch.test.js && node dist-test/test/extension-activation.test.js && node dist-test/test/source-span.test.js && node dist-test/test/url-template.test.js && node dist-test/test/enclosing-function.test.js && node dist-test/test/endpoint-id.test.js && node dist-test/test/parity.test.js && node dist-test/test/a6-object-literal-fps.test.js && node dist-test/test/a2-const-fold.test.js && node dist-test/test/a7-url-path-fallback.test.js && node dist-test/test/c1-pr2-cache-tightening.test.js && node dist-test/test/c1-pr3-batch-tightening.test.js && node dist-test/src/test/benchmark-schema.test.js && node dist-test/src/test/benchmark-metrics.test.js && node dist-test/src/test/benchmark-baseline-sort.test.js && node dist-test/test/c1-pr4-rate-limit-tightening.test.js && node dist-test/test/c1-pr4-batch-residual.test.js && node dist-test/test/pre-a-scanfiles-resolution.test.js && node dist-test/test/pre-b-export-const-tracking.test.js && node dist-test/test/a3-barrel-reexports.test.js && node dist-test/test/a5-factory-di-aliased.test.js && node dist-test/test/wave6-pr1-submit-filter.test.js && node dist-test/test/scan-publishing-handler.test.js && node dist-test/test/config.test.js && node dist-test/test/scan-id.test.js && node dist-test/test/a3-default-import-threading.test.js && node dist-test/test/factory-with-args.test.js && node dist-test/test/ast-inline-parallel.test.js && node dist-test/test/scan-results.test.js && node dist-test/test/chat-handler-merge.test.js", | ||
| "test:scanner": "tsc -p tsconfig.scanner-tests.json && tsc -p tsconfig.benchmark.json && node dist-test/test/scanner-patterns.test.js && node dist-test/test/workspace-scanner.test.js && node dist-test/test/workspace-file-access.test.js && node dist-test/test/endpoint-classification.test.js && node dist-test/test/local-waste-detector.test.js && node dist-test/test/chat-providers.test.js && node dist-test/test/fingerprint-registry.test.js && node dist-test/test/pricing-sync.test.js && node dist-test/test/ast-parser-loader.test.js && node dist-test/test/ast-call-visitor.test.js && node dist-test/test/ast-import-resolver.test.js && node dist-test/test/ast-scanner.test.js && node dist-test/test/ast-python.test.js && node dist-test/test/ast-frequency-analyzer.test.js && node dist-test/test/ast-cache-detector.test.js && node dist-test/test/ast-batch-detector.test.js && node dist-test/test/ast-concurrency-detector.test.js && node dist-test/test/ast-cross-file-resolver.test.js && node dist-test/test/a1-multi-hop-wrappers.test.js && node dist-test/intelligence/__tests__/builder.test.js && node dist-test/intelligence/__tests__/clusters.test.js && node dist-test/intelligence/__tests__/compression.test.js && node dist-test/intelligence/__tests__/export.test.js && node dist-test/test/api-client.test.js && node dist-test/test/key-management.test.js && node dist-test/test/ast-parser-loader-fallback.test.js && node dist-test/intelligence/__tests__/cost-utils.test.js && node dist-test/test/intelligence-compression-async.test.js && node dist-test/test/webview-provider-dispatch.test.js && node dist-test/test/extension-activation.test.js && node dist-test/test/source-span.test.js && node dist-test/test/url-template.test.js && node dist-test/test/enclosing-function.test.js && node dist-test/test/endpoint-id.test.js && node dist-test/test/parity.test.js && node dist-test/test/a6-object-literal-fps.test.js && node dist-test/test/a2-const-fold.test.js && node dist-test/test/a7-url-path-fallback.test.js && node dist-test/test/c1-pr2-cache-tightening.test.js && node dist-test/test/c1-pr3-batch-tightening.test.js && node dist-test/src/test/benchmark-schema.test.js && node dist-test/src/test/benchmark-metrics.test.js && node dist-test/src/test/benchmark-baseline-sort.test.js && node dist-test/test/c1-pr4-rate-limit-tightening.test.js && node dist-test/test/c1-pr4-batch-residual.test.js && node dist-test/test/pre-a-scanfiles-resolution.test.js && node dist-test/test/pre-b-export-const-tracking.test.js && node dist-test/test/a3-barrel-reexports.test.js && node dist-test/test/a5-factory-di-aliased.test.js && node dist-test/test/wave6-pr1-submit-filter.test.js && node dist-test/test/scan-publishing-handler.test.js && node dist-test/test/config.test.js && node dist-test/test/scan-id.test.js && node dist-test/test/a3-default-import-threading.test.js && node dist-test/test/factory-with-args.test.js && node dist-test/test/ast-inline-parallel.test.js && node dist-test/test/scan-results.test.js && node dist-test/test/chat-handler-merge.test.js && node dist-test/test/call-trace.test.js", |
There was a problem hiding this comment.
Split test:scanner into a runner script to avoid shell command-length failures.
This single chained command is long enough to be fragile on Windows shells (command-length limits) and hard to maintain. Move the test list into a Node runner (or a generated manifest) and keep package.json script short.
Proposed refactor
- "test:scanner": "tsc -p tsconfig.scanner-tests.json && tsc -p tsconfig.benchmark.json && node dist-test/test/scanner-patterns.test.js && ... && node dist-test/test/call-trace.test.js",
+ "test:scanner": "tsc -p tsconfig.scanner-tests.json && tsc -p tsconfig.benchmark.json && node dist-test/test/run-scanner-tests.js",// src/test/run-scanner-tests.ts (compiled to dist-test/test/run-scanner-tests.js)
import { spawnSync } from "node:child_process";
const tests = [
"dist-test/test/scanner-patterns.test.js",
// ...all other test files...
"dist-test/test/call-trace.test.js",
];
for (const t of tests) {
const r = spawnSync(process.execPath, [t], { stdio: "inherit" });
if (r.status !== 0) process.exit(r.status ?? 1);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` at line 201, The long chained command in the "test:scanner"
script is fragile and must be replaced by a Node runner; create a new runner
(e.g., src/test/run-scanner-tests.ts compiled to
dist-test/test/run-scanner-tests.js) that exports or runs a list/array named
tests containing each compiled test path (e.g.,
"dist-test/test/scanner-patterns.test.js", ...,
"dist-test/test/call-trace.test.js"), iterate that array and spawn each test
with spawnSync(process.execPath, [testPath], { stdio: "inherit" }) and exit with
the failing status if any test returns non-zero, then update package.json
"test:scanner" to first run the two tsc builds and then call node
dist-test/test/run-scanner-tests.js (keeping the package script short).
Comment on lines
204
to
+205
| crossFileOrigin: normalizeCrossFileOrigin(call.crossFileOrigin), | ||
| callTrace: call.callTrace, |
There was a problem hiding this comment.
Normalize callTrace paths before storing them in the snapshot.
file and crossFileOrigin.file are normalized here, but callTrace.callSite.file and callTrace.resolvedSite.file are copied verbatim. That leaves the snapshot with mixed path formats on Windows / ./ inputs, so callTrace can stop lining up with FileNode.id and ApiCallNode.filePath.
Suggested fix
function normalizeCrossFileOrigin(
origin: ApiCallInput["crossFileOrigin"]
): { file: string; functionName: string } | null {
if (!origin) return null;
return {
file: normalizeRepoPath(origin.file),
functionName: origin.functionName,
};
}
+
+function normalizeCallTrace(
+ trace: ApiCallInput["callTrace"]
+): ApiCallInput["callTrace"] {
+ if (!trace) return undefined;
+ return {
+ callSite: {
+ ...trace.callSite,
+ file: normalizeRepoPath(trace.callSite.file),
+ },
+ resolvedSite: {
+ ...trace.resolvedSite,
+ file: normalizeRepoPath(trace.resolvedSite.file),
+ },
+ hops: trace.hops,
+ };
+}
@@
streaming: Boolean(call.streaming),
isMiddleware: Boolean(call.isMiddleware),
crossFileOrigin: normalizeCrossFileOrigin(call.crossFileOrigin),
- callTrace: call.callTrace,
+ callTrace: normalizeCallTrace(call.callTrace),
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| crossFileOrigin: normalizeCrossFileOrigin(call.crossFileOrigin), | |
| callTrace: call.callTrace, | |
| function normalizeCrossFileOrigin( | |
| origin: ApiCallInput["crossFileOrigin"] | |
| ): { file: string; functionName: string } | null { | |
| if (!origin) return null; | |
| return { | |
| file: normalizeRepoPath(origin.file), | |
| functionName: origin.functionName, | |
| }; | |
| } | |
| function normalizeCallTrace( | |
| trace: ApiCallInput["callTrace"] | |
| ): ApiCallInput["callTrace"] { | |
| if (!trace) return undefined; | |
| return { | |
| callSite: { | |
| ...trace.callSite, | |
| file: normalizeRepoPath(trace.callSite.file), | |
| }, | |
| resolvedSite: { | |
| ...trace.resolvedSite, | |
| file: normalizeRepoPath(trace.resolvedSite.file), | |
| }, | |
| hops: trace.hops, | |
| }; | |
| } | |
| crossFileOrigin: normalizeCrossFileOrigin(call.crossFileOrigin), | |
| callTrace: normalizeCallTrace(call.callTrace), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/intelligence/builder.ts` around lines 204 - 205, The callTrace paths are
not normalized before being stored, causing mismatches with FileNode.id and
ApiCallNode.filePath; update the code that sets callTrace (where call.callTrace
is assigned alongside normalizeCrossFileOrigin) to walk call.callTrace and
normalize all file path fields (e.g., callTrace.callSite.file and
callTrace.resolvedSite.file) using the same normalizer used for crossFileOrigin
(normalizeCrossFileOrigin or its underlying normalize function), then store the
normalized callTrace instead of the verbatim object so callTrace paths
consistently match FileNode.id/ApiCallNode.filePath.
Comment on lines
+25
to
+28
| export function directTrace(file: string, span: SourceSpan): CallTrace { | ||
| const loc: ResolvedLocation = { file, span }; | ||
| // Distinct object per site so a future in-place mutation of one can't alias the other. | ||
| return { callSite: loc, resolvedSite: { ...loc }, hops: 0 }; |
There was a problem hiding this comment.
Clone span for both direct-trace locations.
resolvedSite: { ...loc } only clones the outer object. callSite.span and resolvedSite.span still point at the same SourceSpan, so mutating one nested span mutates the other and breaks the "distinct location" guarantee.
Proposed fix
export function directTrace(file: string, span: SourceSpan): CallTrace {
- const loc: ResolvedLocation = { file, span };
- // Distinct object per site so a future in-place mutation of one can't alias the other.
- return { callSite: loc, resolvedSite: { ...loc }, hops: 0 };
+ return {
+ callSite: { file, span: { ...span } },
+ resolvedSite: { file, span: { ...span } },
+ hops: 0,
+ };
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/scanner/call-trace.ts` around lines 25 - 28, The directTrace function
creates two ResolvedLocation objects that still share the same nested SourceSpan
because resolvedSite: { ...loc } only shallow-clones loc; fix directTrace by
cloning the nested span as well so callSite.span and resolvedSite.span are
distinct (e.g., create const spanClone = { ...span } or use a deep/structured
clone and build loc and resolvedSite using separate span objects) and return {
callSite: { file, span }, resolvedSite: { file, span: spanClone }, hops: 0 } so
future mutations won't alias each other.
Comment on lines
+10
to
+16
| run("directTrace: hops is 0 and both sites are equal", () => { | ||
| const span = pointSpan(12, 4); | ||
| const trace = directTrace("services/chat.ts", span); | ||
| assert.equal(trace.hops, 0); | ||
| assert.deepEqual(trace.callSite, { file: "services/chat.ts", span }); | ||
| assert.deepEqual(trace.resolvedSite, trace.callSite); | ||
| }); |
There was a problem hiding this comment.
Assert that callSite and resolvedSite are distinct objects.
This helper’s contract is stronger than deep equality: it intentionally returns non-aliased site objects so later mutation of one cannot affect the other. The current test would still pass if both fields referenced the same object.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test/call-trace.test.ts` around lines 10 - 16, The test currently only
checks deep equality and can pass if callSite and resolvedSite are the same
object; update the "directTrace: hops is 0 and both sites are equal" test to
also assert they are distinct objects by adding an identity check: after
obtaining trace via directTrace("services/chat.ts", span) and the existing
deepEqual assertions, add an assertion using
assert.notStrictEqual(trace.callSite, trace.resolvedSite) (or equivalent
identity check) to ensure callSite and resolvedSite are non-aliased while
keeping the deep equality assertions to verify content.
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements B2 dual locations (closes #81) — the Wave 2 (Traceability) feature. For cross-file-resolved API calls, the extension now surfaces both the user's call site (where they call the wrapper) and the underlying SDK call site (where the wrapper makes the real request), with both as click targets in the sidebar.
A
CallTrace { callSite, resolvedSite, hops }(each site is aResolvedLocation { file, span }) is threaded end-to-end:src/scanner/call-trace.ts(new) — sharedCallTrace/ResolvedLocationtypes +directTrace()degenerate constructor (hops=0, both sites equal).cross-file-resolver.ts— populatesAstCallMatch.tracefor propagated/middleware/factory matches, with workspace-relative paths for both sites and correct hop counting (wrapper-of-wrapper carries the original SDK site forward).core-scanner.ts→ApiCallInput.callTrace, thenscan-results.tspopulatesEndpointCallSite.callTraceat all three call-site construction sites (direct calls get a degenerate trace viadirectTrace), andbuilder.tsmirrors it ontoApiCallNode.callTrace.webview/src/types.ts+ResultsPage.tsx) — Endpoints view opens the call site by default and shows a "↳ underlying call" link only whenhops > 0. Direct calls behave exactly as before (single link).Acceptance criteria (#81)
hops = 0and equal sitescomputeEndpointIdexcludes line/column/span) + a regression test. The hash is intentionally not re-keyed onresolvedSite.file(would collapse distinct callers into one endpoint and risk a benchmark detection-metric regression).Scope / non-goals
sdk://change toisHighConfidenceEndpointUrlintroduced mid-implementation was caught and reverted; the test was reworked to use a realistic URL.)extension-benchmarkrepo. Once they land, refreshbenchmark/baseline.json. Documented indocs/accuracy/traceability.md.Test plan
npm run build— clean (dashboard + webview + extension)npm run test:scanner— exit 0, full suite green incl. 7 new B2 tests (call-trace, resolver propagated/direct, scan-results cross-file/direct, builder, endpoint-id AC-4)npm run benchmark— CI-only here (fixtures repo not accessible in this environment); Δ expected +0.00pp since the change is additive metadataDocs
Plan:
docs/superpowers/plans/2026-05-30-wave2-b2-dual-locations.md· acceptance + #113 note indocs/accuracy/traceability.md·PROGRESS.mdWave 2 → 🟡.https://claude.ai/code/session_015jMBgSvEt44xMvfWKzLoXi
Generated by Claude Code
Summary by CodeRabbit
New Features
Documentation