Skip to content

foundation(accuracy): B1 spans + B3 stable endpoint IDs + A4 AST↔regex parity - #90

Merged
AndresL230 merged 48 commits into
mainfrom
claude/foundation-parser-accuracy-2GYtd
May 13, 2026
Merged

foundation(accuracy): B1 spans + B3 stable endpoint IDs + A4 AST↔regex parity#90
AndresL230 merged 48 commits into
mainfrom
claude/foundation-parser-accuracy-2GYtd

Conversation

@AndresL230

@AndresL230 AndresL230 commented May 12, 2026

Copy link
Copy Markdown
Contributor

Summary

This branch lands the three foundation plans from the parser-accuracy
roadmap (see docs/accuracy/README.md):

  • B1 — Span-based source locations ([Traceability] Span-based source locations (B1) #80): every detected call now
    carries a SourceSpan from AST through EndpointCallSite to the
    webview, so click-back can select the full multi-line call expression
    instead of just the start line.
  • B3 — Stable endpoint IDs ([Traceability] Stable endpoint IDs across scans (B3) #82): introduces computeEndpointId()
    (URL-template masker + enclosing-function extractor) so an endpoint's
    identity survives non-structural edits. Persisted simulator scenarios
    now reattach across re-scans; orphaned records are pruned (guarded so
    empty scans never destroy state).
  • A4 — AST↔regex parity ([Detection] AST ↔ regex parity audit and CI gate (A4) #76): a new parity runner
    (src/test/parity.ts) walks src/test/fixtures/parity/, runs both
    detection paths in isolation (no AST-coverage masking), and fails CI
    on any unannotated divergence in (provider, method, line). First
    audit fixed one regex bug (generic-http.ts now host-attributes
    known hosts via lookupHost() and drops the wrong-method GET
    fallback for multi-line fetch options) and documented two structural
    multi-line cases in docs/accuracy/PARITY.md.

Closes #76. Closes #80. Closes #82.

Merge notes

  • Merged origin/main into this branch via git merge (no rebase) so
    the SHAs referenced in commit messages, PROGRESS.md, and the A4
    handoff memory remain intact.
  • Conflicts resolved against PR Audit remediation: full P0–P3 fix pass (2026-05-11 plan) #87's handler extraction:
    • B3 stable-ID logic re-applied inside
      src/webview/scan-publishing-handler.ts::mergeRemoteAndLocalEndpoints
      (the function moved there during PR Audit remediation: full P0–P3 fix pass (2026-05-11 plan) #87).
    • pruneSavedScenariosAgainst moved to
      src/webview/simulation-handler.ts::pruneAgainst (where
      savedScenarios now lives) and is invoked from the scan
      publishing flow via a new pruneSavedScenariosAgainst context
      callback.
    • webview-provider.ts kept main's compact post-extraction shape
      plus B1's SourceSpan import and the openFile span-aware
      selection logic.
    • package.json test:scanner script combines main's
      extension-activation.test.js and this branch's parity /
      source-span / url-template / enclosing-function / endpoint-id
      tests. esbuild ^0.28 and openai ^4.104 ranges from main
      intact.
    • src/webview/simulation-handler.ts (added by PR Audit remediation: full P0–P3 fix pass (2026-05-11 plan) #87) survived
      the merge.

Notable design notes

  • Why parity isolates each path: core-scanner.scanFiles() masks
    regex output on AST-covered lines, which would silently hide most
    parity gaps. The runner deliberately calls each path on raw source.
  • Why generic-http matches are filtered in parity normalisation:
    the regex layer's bare-HTTP detection without a host map produces
    library: "generic-http", which has no provider. After the fix,
    known hosts elevate to the concrete provider id (e.g. openai).
  • Multi-line allowlist entries: line-based regex cannot stitch a
    multi-line fetch("url", { method: "POST" }) or a Python
    requests.post(\n "url",\n ...) together. AST does this
    structurally. Two astOnly: true allowlist entries with concrete
    reasons.
  • Endpoint ID stability: computeEndpointId(provider, urlTemplate, enclosingFunction, filePath) hashes the four inputs. URL template
    masker collapses numeric/uuid path segments so …/users/42 and
    …/users/99 get the same ID. Enclosing-function extractor uses
    AST when available with a 7-day persistence override during the
    rollout, documented in commit b9e54be.

Tests

  • src/test/parity.test.ts — new, runs the parity runner against the
    fixture corpus.
  • src/test/endpoint-id.test.ts — 13 cases covering hash determinism,
    refactor stability, file-path normalization, and end-to-end behaviour.
  • src/test/url-template.test.ts, src/test/enclosing-function.test.ts
    — extracted unit suites for the two ID components.
  • src/test/source-span.test.ts — span helper unit tests.
  • Existing waste-detector/AST/scanner suites updated for the now-
    required span field on AstCallMatch and EndpointCallSite.

Full suite is green locally. test:scanner invocation in package.json
now includes parity.test.js. Final line is
PASS parity (2 documented divergences, 0 unannotated).

Pending manual verification (post-merge, before sign-off)

Two acceptance criteria require a manual run in the Extension
Development Host because they exercise UI selection / persistence
across editor sessions:

  • B1 T10 (criterion Add more option for models #3 in docs/accuracy/traceability.md § B1)
    — F5 the dev host, scan a workspace containing a multi-line
    await openai.chat.completions.create({ ... }), click that
    endpoint row, confirm the selection covers from await through
    the closing ).
  • B3 T7 Step 4 (criterion Add custom frequency input for calls #5 in docs/accuracy/traceability.md
    § B3) — F5 the dev host, save a simulator scenario, edit any
    unrelated file, re-scan, confirm the scenario still loads
    (endpoint IDs survived the non-structural change).

These are gated checkboxes in docs/accuracy/traceability.md; flip
them to [x] and amend in a follow-up commit once green.

Test plan

🤖 Generated with Claude Code


Generated by Claude Code

Summary by CodeRabbit

  • New Features

    • Precise source location highlighting when opening files to API calls
    • Improved endpoint identification with collision handling
  • Bug Fixes

    • Enhanced provider resolution for built-in modules
    • Better synthetic endpoint deduplication and tracking
  • Tests

    • Added parity validation between AST and regex scanners
    • Expanded test fixtures for major providers and patterns
  • Documentation

    • Updated progress tracking and roadmap completion status
    • Added parity divergence allowlist and documentation

Review Change Stack

AndresL230 and others added 30 commits May 12, 2026 02:23
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…issue #80)

After T4 made AstCallMatch.span required, four detector test fixtures stopped
compiling. They were also missing the pre-existing required `confidence` field
on the base object — fixing both at once so the scanner-tests build clears.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
3 of 4 acceptance criteria automated-verified; criterion #3 (reveal-by-span in
the webview) awaits manual Extension Development Host verification — code is
landed at commit 69ca79d.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ures

- ast-scanner: extend isInternalImport to filter node:* prefix and bare
  built-in module names so fs/path/assert don't surface as SDK matches
- fingerprint-registry test: expect elevenlabs (11 providers) and derive
  count from ALL_PROVIDERS.length
- compression test: loosen two estimatedMonthlyCost==null assertions —
  compressClusters now computes a real cost from local pricing
- export test: add required costLeaks/providerSummary fields to two
  ExportedContext fixtures
- tsconfig.scanner-tests: exclude src/test/fixtures (recost-mock-calls
  imports SDKs not installed in the test env)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
AndresL230 and others added 15 commits May 12, 2026 03:43
…#82)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Pure parity-runner library plus the human/machine-readable allowlist doc.
The runner walks a fixture corpus, runs AST and regex paths in isolation
(without core-scanner's AST-coverage masking), normalises both result sets
to {provider, method, line} tuples, and emits divergences.

parseAllowlist() reads the YAML block in PARITY.md so the markdown doc is
the single source of truth for documented intentional divergences.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Corpus for the AST↔regex parity runner. Seven fixtures total:

Basic agreement (both paths should detect):
- openai-basic.ts: direct OpenAI SDK call
- anthropic-basic.ts: direct Anthropic SDK call
- stripe-basic.ts: direct Stripe SDK call
- fetch-known-host.ts: raw fetch() to a known host

Documented divergence / regression guards:
- wrapped-call.ts: AST follows wrapper back to SDK; regex sees only the
  wrapper invocation. Allowlisted as astOnly in PARITY.md.
- object-literal-only.ts: pricing-table-style data — both paths should
  produce zero matches (A6 regression guard).
- python-requests.py: requests.post() to a known host — both paths
  should attribute to openai via host lookup.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
First run surfaces two AST-only divergences (fetch-known-host.ts L2,
python-requests.py L4) — both cases where AST does host-based provider
attribution and regex does not. Triaged in follow-up commits.

Fixture dir is resolved back to the source tree because fixtures are
excluded from tsc compilation (tsconfig.scanner-tests.json) by design.

The test is intentionally red on this commit; follow-up commits in
Task 5 categorize and resolve each divergence.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…llback (issue #76)

generic-http.ts previously hard-coded provider: "generic-http" for every
fetch/axios/requests match, even when the URL had a known host. The AST
scanner already attributes such calls via lookupHost(). Bring the regex
path into parity by reusing the same host registry: when the URL host
maps to a known provider, emit that provider id; otherwise fall back to
"generic-http" as before.

Also: the previous fetch fallback pattern matched fetch("url"...) without
caring whether the call had an unparsed options object on subsequent
lines, which produced wrong-method GET emits for multi-line POST/PUT
calls. Tighten it to require a closing paren on the same line so the
fallback only fires for actual no-options fetches; multi-line option
objects are AST's job (separately documented in PARITY.md).

Surfaces and resolves one of the two divergences flagged by the new
parity test in src/test/parity.test.ts. The remaining multi-line cases
are documented in docs/accuracy/PARITY.md in the next commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fetch-known-host.ts and python-requests.py both contain HTTP calls whose
options object or URL argument spans multiple source lines. The regex
matchers operate one line at a time by design, so they cannot stitch
those constructs together; the AST scanner sees the full call expression
structurally and attributes correctly. Documented in the YAML allowlist
as astOnly with explicit reasons so future maintainers know why these
divergences are accepted rather than introducing speculative multi-line
regex passes.

After this commit:
  PASS parity (2 documented divergences, 0 unannotated)

Closes the iterative triage for issue #76.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Acceptance criteria in detection.md § A4 all check [x]:
  - Parity test runs in CI on every PR (test.yml → npm test → test:scanner)
  - Every divergence is fixed or annotated in PARITY.md
  - Same line reported by both paths (enforced by the runner — same-line
    disagreement always fails)

PROGRESS.md updated: A4 row, batch table, task checklist, activity log.

Final state: PASS parity (2 documented divergences, 0 unannotated).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ser-accuracy-2GYtd

# Conflicts:
#	package.json
#	src/scanner/source-span.ts
#	src/webview-provider.ts
@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@AndresL230 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 42 minutes and 21 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5232171c-1cb3-4261-a0fc-460c0f0e930d

📥 Commits

Reviewing files that changed from the base of the PR and between dc111c7 and 0a44aaa.

📒 Files selected for processing (6)
  • src/ast/ast-scanner.ts
  • src/ast/enclosing-function.ts
  • src/intelligence/__tests__/builder.test.ts
  • src/scanner/patterns/generic-http.ts
  • src/test/url-template.test.ts
  • src/webview-provider.ts
📝 Walkthrough

Walkthrough

This PR implements three interrelated roadmap items: span-based source locations (B1), stable endpoint IDs (B3), and AST↔regex parity validation (A4). It introduces source span infrastructure throughout the scanner and intelligence layers, adds deterministic endpoint ID hashing, includes comprehensive fixture-based parity testing with an allowlist, and updates the webview to support span-based file navigation.

Changes

Span Infrastructure and Source Location Tracking

Layer / File(s) Summary
Source span type definitions
src/scanner/source-span.ts, src/analysis/types.ts, src/intelligence/types.ts, src/messages.ts, src/scanner/patterns/types.ts, webview/src/types.ts
Core SourceSpan interface with explicit exclusive-end semantics (endLine/endColumn are one past the last character). Updated type signatures across analysis, intelligence, messaging, and webview contracts to include optional or required span fields.
AST enclosing function extraction and span population
src/ast/enclosing-function.ts, src/ast/ast-scanner.ts
Introduces `enclosingFunctionName(node: SyntaxNode): string
Call span extraction from Tree-sitter
src/ast/call-visitor.ts
CallInfo now computes full source span from Tree-sitter startPosition/endPosition with 1-based lines and 0-based columns. Span field represents the complete call-expression source range.
Span synthesis for regex-detected patterns
src/scanner/core-scanner.ts, src/scanner/patterns/generic-http.ts
Regex scanner synthesizes line-wide SourceSpan (column 0 to line end) for both route-definition and HTTP call matches. Generic-HTTP pattern matcher refines fetch matching to single-argument form only and attributes calls to resolved provider via URL host lookup with fallback to "generic-http".

Endpoint ID Generation and Integration

Layer / File(s) Summary
Stable endpoint ID hashing
src/scanner/endpoint-id.ts, src/scanner/url-template.ts
computeEndpointId generates deterministic ep_<hash> identifiers by hashing normalized file path, provider, method signature, enclosing function, and masked URL. maskUrlDynamicParts replaces UUIDs, numeric path segments, and template patterns with :id for template stability. FNV-1a 32-bit hash with base-36 output ensures short, URL-safe, deterministic IDs.
ID collision handling in builders
src/intelligence/builder.ts, src/scan-results.ts, src/webview/scan-publishing-handler.ts
Integrates computeEndpointId throughout: builder delegates ID generation and appends _L{line} on collision; scan-results and publishing-handler track emitted IDs and suffix with _1, _2, etc. to guarantee uniqueness within merge scope.
API call node span and ID propagation
src/intelligence/types.ts
ApiCallNode includes `span: SourceSpan

Span-Based Editor Navigation

Layer / File(s) Summary
Span-based file opening
src/webview-provider.ts, src/messages.ts, webview/src/components/ResultsPage.tsx, src/webview/simulation-handler.ts
Updates openFile message type and handler to accept optional span?: SourceSpan. Provider's handleOpenFile derives vscode.Range from span and selects the precise region; falls back to line selection if span is absent. Wires pruneSavedScenariosAgainst callback to remove saved scenarios referencing deleted endpoint IDs. ResultsPage passes span in postMessage payload.

AST↔Regex Parity Testing

Layer / File(s) Summary
Parity test infrastructure
src/test/parity.ts, src/test/parity.test.ts, docs/accuracy/PARITY.md
Defines ParityRecord, FixtureDivergence, and AllowlistEntry types. compareForFixture normalizes AST and regex outputs, groups by provider/method/line, identifies same-line disagreements. runParity iterates fixtures, filters divergences against allowlist, classifies as annotated or unannotated. parity.test.ts runner gates CI on unannotated divergences and prints detailed diagnostics.
Parity test fixtures
src/test/fixtures/parity/*
Seven fixtures covering OpenAI (SDK and native fetch), Anthropic, Stripe, Python requests, wrapped calls, and data-only patterns. Enables validation of AST and regex scanner agreement across diverse SDK types, call styles, and language patterns.

Test Updates and New Suites

Layer / File(s) Summary
Detector test helper updates
src/test/ast-batch-detector.test.ts, src/test/ast-cache-detector.test.ts, src/test/ast-call-visitor.test.ts, src/test/ast-concurrency-detector.test.ts, src/test/ast-cross-file-resolver.test.ts, src/test/python-waste-detector.test.ts
makeMatch helpers now compute span via pointSpan(line, column), populate enclosingFunction, and set default confidence to align mock objects with scanner output. Includes assertions validating single-line and multi-line span behavior.
New comprehensive test suites
src/test/source-span.test.ts, src/test/endpoint-id.test.ts, src/test/url-template.test.ts, src/test/enclosing-function.test.ts
Tests validate pointSpan/spanFromMatch correctness, computeEndpointId determinism, file-path/URL normalization behavior, maskUrlDynamicParts canonicalization, and enclosingFunctionName across JS/TS/Python function declarations, methods, arrows, and top-level calls.
Test assertion relaxation
src/intelligence/__tests__/builder.test.ts, src/intelligence/__tests__/compression.test.ts, src/test/fingerprint-registry.test.ts
Builder test ID assertion updated to match ep_[a-z0-9]+ pattern. Compression test permits estimatedMonthlyCost null or non-negative numeric value. Fingerprint registry extended to include elevenlabs provider with dynamic length assertions.

CI and Documentation

Layer / File(s) Summary
CI script and configuration
package.json, tsconfig.scanner-tests.json
test:scanner npm script expanded to run new span/ID/parity test modules. TypeScript config excludes src/test/fixtures from compilation.
Roadmap and acceptance tracking
docs/accuracy/detection.md, docs/accuracy/traceability.md, docs/superpowers/plans/PROGRESS.md, docs/accuracy/PARITY.md
A4 acceptance criteria marked complete with parity test implementation and CI wiring; B1 marked code-complete with manual EDH verification pending for UI click-span behavior; B3 marked code-complete with manual EDH pending for saved-scenario/suppressed-finding persistence. Activity log expanded with completion milestones. PARITY.md introduced documenting three AST-only divergences and allowlist usage.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

  • recost-dev/extension#87: Overlaps on SourceSpan infrastructure, scanner/source-span utilities, package.json test:scanner changes, and webview provider/scan-publishing/simulation handler logic.

🐰 A rabbit hops through the code with glee,
Spans and IDs now flow wild and free,
Parity tests keep the paths aligned,
No more silent divergences to find!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title directly summarizes the three main changes: B1 spans, B3 stable endpoint IDs, and A4 AST↔regex parity, aligning with PR objectives.
Linked Issues check ✅ Passed All three objectives from linked issues (#76, #80, #82) are implemented: parity test runner with CI integration, span propagation through all layers, and stable endpoint ID computation with persistence safeguards.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the three linked issues: span types and propagation, endpoint ID computation and application, parity testing infrastructure, and necessary test fixtures.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/foundation-parser-accuracy-2GYtd

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread src/test/parity.test.ts Fixed
Flagged by github-code-quality bot. Runtime behavior unchanged — the
parity harness uses console.error/process.exit, not assert.
@AndresL230
AndresL230 marked this pull request as ready for review May 12, 2026 15:30

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (4)
src/intelligence/__tests__/builder.test.ts (1)

230-230: ⚡ Quick win

Tighten the ID assertion to validate the full format.

Current regex is prefix-only, so malformed trailing characters still pass.

Proposed fix
-  assert.ok(calls.every((apiCall) => /^ep_[a-z0-9]+/.test(apiCall.id)));
+  assert.ok(calls.every((apiCall) => /^ep_[a-z0-9]+(?:_L\d+)?$/.test(apiCall.id)));
🤖 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/__tests__/builder.test.ts` at line 230, The test is only
asserting a prefix because the regex /^ep_[a-z0-9]+/ lacks an end anchor; update
the assertion in the calls.every((apiCall) => /^ep_[a-z0-9]+/.test(apiCall.id))
to validate the entire id (e.g., use /^ep_[a-z0-9]+$/) so malformed trailing
characters no longer pass.
src/test/source-span.test.ts (1)

32-32: ⚡ Quick win

Add newline at end of file.

Line 32 is missing a trailing newline, which violates typical file formatting conventions and may cause issues with some tools.

🤖 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/source-span.test.ts` at line 32, The file ends with the line
containing the top-level IIFE catch handler "})().catch((err) => {
console.error(err); process.exit(1); });" but is missing a trailing newline;
simply add a single newline character at end-of-file so the file ends with a
newline after that statement.
src/scanner/core-scanner.ts (1)

177-182: 💤 Low value

Consider extracting duplicated span construction.

The line-wide span creation logic is duplicated in two locations (route matching and generic HTTP matching). While not a correctness issue, extracting this into a helper would improve maintainability.

♻️ Proposed extraction
+function lineWideSpan(lineNum: number, lineText: string): SourceSpan {
+  return {
+    startLine: lineNum,
+    startColumn: 0,
+    endLine: lineNum,
+    endColumn: lineText.length,
+  };
+}
+
 for (const route of routeMatches) {
   if (!isHighConfidenceUrl(route.url)) continue;
   const key = `${entry.relativePath}:${lineNum}:${route.method}:${route.url}:${route.library}`;
   if (dedupe.has(key)) continue;
   dedupe.add(key);
-  const span: SourceSpan = {
-    startLine: lineNum,
-    startColumn: 0,
-    endLine: lineNum,
-    endColumn: line.length,
-  };
+  const span = lineWideSpan(lineNum, line);
   allCalls.push({

Also applies to: 208-213

🤖 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/core-scanner.ts` around lines 177 - 182, Extract the duplicated
line-wide SourceSpan creation into a small helper (e.g. makeLineSpan or
buildLineSpan) that accepts the line number and the line text/length and returns
a SourceSpan with startLine=lineNum, startColumn=0, endLine=lineNum,
endColumn=line.length; replace the two duplicated constructions used in the
route-matching and generic-HTTP-matching code paths in core-scanner.ts with
calls to this helper (references: SourceSpan, the span object creation block,
and the variables lineNum and line) to improve maintainability.
package.json (1)

199-199: ⚡ Quick win

Consider refactoring the test script for maintainability.

The test:scanner script is now a single 1,500+ character line chaining 30+ test files. This makes it difficult to read, maintain, and debug. If any test fails, subsequent tests don't run.

♻️ Alternative approaches

Option 1: Use a test runner script that discovers dist-test/**/*.test.js:

"test:scanner": "tsc -p tsconfig.scanner-tests.json && node scripts/run-tests.js"

Option 2: Use npm-run-all to run tests in parallel/series:

"test:scanner": "tsc -p tsconfig.scanner-tests.json && npm-run-all test:scanner:*",
"test:scanner:patterns": "node dist-test/test/scanner-patterns.test.js",
"test:scanner:workspace": "node dist-test/test/workspace-scanner.test.js",
...

Option 3: At minimum, break into multiple lines with && for readability:

"test:scanner": "tsc -p tsconfig.scanner-tests.json && node dist-test/test/scanner-patterns.test.js && node dist-test/test/workspace-scanner.test.js && ..."
🤖 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 199, The "test:scanner" npm script is an overly long
single-line command that chains many test files and is hard to maintain; split
it into a maintainable approach by replacing the single "test:scanner" entry
with one of the recommended patterns: (a) point "test:scanner" to a test runner
script (e.g., "node scripts/run-tests.js") that discovers dist-test/**/*.test.js
while still running tsc -p tsconfig.scanner-tests.json first, or (b) create
per-file scripts like "test:scanner:patterns", "test:scanner:workspace", etc.,
and make "test:scanner" run them via npm-run-all (or with "&&" in series), or
(c) at minimum break the long command into a readable chained command using
multiple && lines; update package.json's "test:scanner" key and add the helper
scripts (e.g., "test:scanner:patterns") accordingly and ensure the tsc
invocation (tsc -p tsconfig.scanner-tests.json) remains before running tests so
compiled files under dist-test are used.
🤖 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 `@src/ast/ast-scanner.ts`:
- Line 601: The propagated match construction in matches.push is collapsing
multi-line spans by using pointSpan(line, column); update the span assignment in
the propagated objects (the matches.push call) to preserve the original full
call span (use m.span if present or merge m.span with the current node/span)
instead of replacing it with pointSpan; change each occurrence that sets span:
pointSpan(...) (the ones around matches.push) to carry forward m.span (or a
merged span) so wrapper/callback/middleware propagated detections keep their
full multi-line span and precise click-selection fidelity.

In `@src/ast/enclosing-function.ts`:
- Around line 37-44: The traversal currently stops and returns null for
anonymous arrow_function/function_expression when not bound to a
variable_declarator; instead, remove the early return so traversal continues up
the tree to find an enclosing named function. In the block handling current.type
=== "arrow_function" || "function_expression" (and after checking decl?.type ===
"variable_declarator" and possibly returning the lhs identifier), do not return
null — allow the function to advance current = current.parent (or fall through)
so outer function declarations or function_declaration names can still be
discovered.

In `@src/scanner/patterns/generic-http.ts`:
- Around line 22-27: The regex in the fetch pattern (sdk: "fetch", regex:
/fetch\(\s*['"`]([^'"`\n]+)['"`]\s*\)/gi) still matches calls that have a second
argument (options) when the first arg is a template/identifier form; update the
regex to ensure it only matches single-argument fetch calls by forbidding a
comma after the first argument—e.g., add a negative lookahead like (?!\s*,)
after the closing quote/backtick/identifier so fetch(..., options) is not
matched; modify the pattern used in generic-http.ts (the regex for sdk "fetch")
accordingly and keep flags consistent.

In `@src/test/url-template.test.ts`:
- Around line 4-7: The test helper run() currently calls fn() without awaiting
async results, so asynchronous rejections escape the try/catch; change the
signature of run to accept an async function (e.g., fn: () => Promise<void>) and
await its result inside the try block (await fn()) so any returned rejected
promise is caught and causes the test to fail as intended; update any callers if
needed to pass async functions/promises to run().

In `@src/webview-provider.ts`:
- Around line 749-756: The creation of the vscode.Range from the unvalidated
span can throw and silently fail; before constructing the Range in the code that
computes range (the span ? new vscode.Range(...) : ...) and the similar block at
lines 763–766, validate and clamp span.startLine, span.endLine,
span.startColumn, and span.endColumn to the document bounds (>=1 and <= total
lines/line length) and ensure end positions are not before start positions; if
the span is out-of-bounds or invalid, fall back to the line-based Range or
undefined and log or surface a small error so the click-open path does not
silently fail.

---

Nitpick comments:
In `@package.json`:
- Line 199: The "test:scanner" npm script is an overly long single-line command
that chains many test files and is hard to maintain; split it into a
maintainable approach by replacing the single "test:scanner" entry with one of
the recommended patterns: (a) point "test:scanner" to a test runner script
(e.g., "node scripts/run-tests.js") that discovers dist-test/**/*.test.js while
still running tsc -p tsconfig.scanner-tests.json first, or (b) create per-file
scripts like "test:scanner:patterns", "test:scanner:workspace", etc., and make
"test:scanner" run them via npm-run-all (or with "&&" in series), or (c) at
minimum break the long command into a readable chained command using multiple &&
lines; update package.json's "test:scanner" key and add the helper scripts
(e.g., "test:scanner:patterns") accordingly and ensure the tsc invocation (tsc
-p tsconfig.scanner-tests.json) remains before running tests so compiled files
under dist-test are used.

In `@src/intelligence/__tests__/builder.test.ts`:
- Line 230: The test is only asserting a prefix because the regex
/^ep_[a-z0-9]+/ lacks an end anchor; update the assertion in the
calls.every((apiCall) => /^ep_[a-z0-9]+/.test(apiCall.id)) to validate the
entire id (e.g., use /^ep_[a-z0-9]+$/) so malformed trailing characters no
longer pass.

In `@src/scanner/core-scanner.ts`:
- Around line 177-182: Extract the duplicated line-wide SourceSpan creation into
a small helper (e.g. makeLineSpan or buildLineSpan) that accepts the line number
and the line text/length and returns a SourceSpan with startLine=lineNum,
startColumn=0, endLine=lineNum, endColumn=line.length; replace the two
duplicated constructions used in the route-matching and generic-HTTP-matching
code paths in core-scanner.ts with calls to this helper (references: SourceSpan,
the span object creation block, and the variables lineNum and line) to improve
maintainability.

In `@src/test/source-span.test.ts`:
- Line 32: The file ends with the line containing the top-level IIFE catch
handler "})().catch((err) => { console.error(err); process.exit(1); });" but is
missing a trailing newline; simply add a single newline character at end-of-file
so the file ends with a newline after that statement.
🪄 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: f94fe3e7-fa60-40d3-8524-3259b7300c35

📥 Commits

Reviewing files that changed from the base of the PR and between abce7a3 and dc111c7.

📒 Files selected for processing (47)
  • docs/accuracy/PARITY.md
  • docs/accuracy/detection.md
  • docs/accuracy/traceability.md
  • docs/superpowers/plans/PROGRESS.md
  • package.json
  • src/analysis/types.ts
  • src/ast/ast-scanner.ts
  • src/ast/call-visitor.ts
  • src/ast/enclosing-function.ts
  • src/intelligence/__tests__/builder.test.ts
  • src/intelligence/__tests__/compression.test.ts
  • src/intelligence/builder.ts
  • src/intelligence/types.ts
  • src/messages.ts
  • src/scan-results.ts
  • src/scanner/core-scanner.ts
  • src/scanner/endpoint-id.ts
  • src/scanner/patterns/generic-http.ts
  • src/scanner/patterns/types.ts
  • src/scanner/source-span.ts
  • src/scanner/url-template.ts
  • src/test/ast-batch-detector.test.ts
  • src/test/ast-cache-detector.test.ts
  • src/test/ast-call-visitor.test.ts
  • src/test/ast-concurrency-detector.test.ts
  • src/test/ast-cross-file-resolver.test.ts
  • src/test/enclosing-function.test.ts
  • src/test/endpoint-id.test.ts
  • src/test/fingerprint-registry.test.ts
  • src/test/fixtures/parity/anthropic-basic.ts
  • src/test/fixtures/parity/fetch-known-host.ts
  • src/test/fixtures/parity/object-literal-only.ts
  • src/test/fixtures/parity/openai-basic.ts
  • src/test/fixtures/parity/python-requests.py
  • src/test/fixtures/parity/stripe-basic.ts
  • src/test/fixtures/parity/wrapped-call.ts
  • src/test/parity.test.ts
  • src/test/parity.ts
  • src/test/python-waste-detector.test.ts
  • src/test/source-span.test.ts
  • src/test/url-template.test.ts
  • src/webview-provider.ts
  • src/webview/scan-publishing-handler.ts
  • src/webview/simulation-handler.ts
  • tsconfig.scanner-tests.json
  • webview/src/components/ResultsPage.tsx
  • webview/src/types.ts

Comment thread src/ast/ast-scanner.ts Outdated
Comment thread src/ast/enclosing-function.ts
Comment thread src/scanner/patterns/generic-http.ts
Comment thread src/test/url-template.test.ts Outdated
Comment thread src/webview-provider.ts
Five actionable findings from the review on PR #90:

- **B1 span fidelity** (`src/ast/ast-scanner.ts`): propagated
  matches in the wrapper/callback/middleware passes were collapsing
  multi-line spans to `pointSpan(line, column)`. Carry the original
  `callInfo.span` through so click-back selects the full call
  expression.
- **B3 enclosing function** (`src/ast/enclosing-function.ts`): the
  walk returned `null` immediately for anonymous `arrow_function` /
  `function_expression` not bound to a `variable_declarator`, which
  swallowed common cases like `[].forEach(x => openai.create(x))`
  inside a named function. Let traversal continue so the named
  ancestor wins. Destructured bindings still return null (no single
  name to attribute the call to).
- **Parity / generic-http** (`src/scanner/patterns/generic-http.ts`):
  the template-literal and identifier fetch patterns still matched
  the multi-arg form, so `fetch(\`url\`, { method: "POST" })` and
  `fetch(urlVar, { ... })` could emit a wrong-method GET fallback.
  Anchor both with `\s*\)` to keep them single-arg only — multi-line
  options are AST's job.
- **url-template.test.ts**: `run()` invoked `fn()` without awaiting,
  so async rejections would silently pass. Await it and accept
  `() => void | Promise<void>`.
- **webview-provider.ts handleOpenFile**: stale spans (e.g. from a
  re-scan after the file shrank) could throw inside `vscode.Range`
  and disappear into the silent catch. Clamp `startLine`, `endLine`,
  `startColumn`, `endColumn` to the document bounds so click-back
  always lands somewhere visible.

Plus one nitpick:

- `builder.test.ts` ID regex anchor: `/^ep_[a-z0-9]+/` was prefix-only;
  tighten to `/^ep_[a-z0-9]+(?:_L\d+)?$/` to also reject malformed
  trailing characters.

Skipped: the EOF-newline nitpick on source-span.test.ts (the file
already ends with `\n`), the package.json `test:scanner` length
nitpick (style only, out of scope), and the core-scanner span helper
extraction (low value per the reviewer's own classification).

Full test suite still green:
`PASS parity (2 documented divergences, 0 unannotated)`.
Comment thread src/ast/ast-scanner.ts Fixed
Comment thread src/ast/ast-scanner.ts Fixed
Flagged by github-code-quality (CodeQL). The previous fix removed all
node consumers in the callback/iteration and middleware loops; the
destructure was left over.
@AndresL230
AndresL230 merged commit 831972b into main May 13, 2026
2 checks passed
@AndresL230
AndresL230 deleted the claude/foundation-parser-accuracy-2GYtd branch May 13, 2026 03:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants