Wave 3: resolver follow-ups (#114, #115, #116) - #126
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…atches (#114) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ion (#116) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lAssignments (#115) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…h core-scanner + test coverage (#116) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR implements three resolver and detector follow-ups ( ChangesWave 3 Resolver and Detector Follow-ups
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
| const c0 = makeClient(); | ||
| const c1 = makeClient(config); | ||
| const c2 = makeClient({ apiKey: process.env.KEY }); | ||
| const c3 = makeClient(env, options); |
| const c3 = makeClient(env, options); | ||
| const c4 = makeClient( | ||
| env, | ||
| options, |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ast/cross-file-resolver.ts (1)
615-623:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThread middleware default-import context into resolver.
Line 621 hardcodes
isDefaulttofalse. For default-imported middleware (import mw from "./mw"),export defaultcallees won’t be resolved, so propagation is skipped.Suggested fix
const calleeMatches = resolveExportedMatches( mwName, resolvedFile, registry, sourceByFile, normalizedKnown, 0, - false + importEntry.isDefault );🤖 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 615 - 623, The call to resolveExportedMatches is hardcoding the isDefault flag to false, so default-imported middleware (mwName) never resolves default exports; compute an isDefault boolean at the call site based on the middleware import metadata (i.e., whether mwName was imported as a default), and pass that boolean instead of false into resolveExportedMatches(mwName, resolvedFile, registry, sourceByFile, normalizedKnown, 0, isDefault). If necessary, add an isDefault parameter to intermediate functions that lead to this call so the default-import context is threaded through from where imports are parsed to this resolver.
🧹 Nitpick comments (1)
docs/superpowers/plans/2026-05-27-wave3-resolver-followups.md (1)
712-717: ⚡ Quick winReplace machine-specific absolute paths in benchmark steps.
The commands currently depend on
/home/andresl/..., which makes the runbook non-portable for other contributors. Please switch these to repo-relative or environment-variable-based paths (for example,$REPO_ROOT) so the instructions are reproducible across dev machines.Suggested doc patch
- cd /tmp/wave3-fixtures && git fetch --depth 1 origin "$(tr -d '\n\r' < /home/andresl/Projects/recost/extension/.benchmark-fixtures-sha)" && git checkout FETCH_HEAD - cd /home/andresl/Projects/recost/extension + REPO_ROOT="$(pwd)" # run from repo root before this block + cd /tmp/wave3-fixtures && git fetch --depth 1 origin "$(tr -d '\n\r' < "$REPO_ROOT/.benchmark-fixtures-sha")" && git checkout FETCH_HEAD + cd "$REPO_ROOT"🤖 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-27-wave3-resolver-followups.md` around lines 712 - 717, Replace the machine-specific absolute paths used in the benchmark commands (notably the git fetch line containing "$(tr -d '\n\r' < /home/andresl/Projects/recost/extension/.benchmark-fixtures-sha)" and the "cd /home/andresl/Projects/recost/extension" command) with a repo-relative or environment-variable-based reference (e.g., use $REPO_ROOT or detect the repo root with git rev-parse --show-toplevel) and update the commands to read the .benchmark-fixtures-sha from that variable instead of a hardcoded home path so the runbook is portable across machines.
🤖 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.
Outside diff comments:
In `@src/ast/cross-file-resolver.ts`:
- Around line 615-623: The call to resolveExportedMatches is hardcoding the
isDefault flag to false, so default-imported middleware (mwName) never resolves
default exports; compute an isDefault boolean at the call site based on the
middleware import metadata (i.e., whether mwName was imported as a default), and
pass that boolean instead of false into resolveExportedMatches(mwName,
resolvedFile, registry, sourceByFile, normalizedKnown, 0, isDefault). If
necessary, add an isDefault parameter to intermediate functions that lead to
this call so the default-import context is threaded through from where imports
are parsed to this resolver.
---
Nitpick comments:
In `@docs/superpowers/plans/2026-05-27-wave3-resolver-followups.md`:
- Around line 712-717: Replace the machine-specific absolute paths used in the
benchmark commands (notably the git fetch line containing "$(tr -d '\n\r' <
/home/andresl/Projects/recost/extension/.benchmark-fixtures-sha)" and the "cd
/home/andresl/Projects/recost/extension" command) with a repo-relative or
environment-variable-based reference (e.g., use $REPO_ROOT or detect the repo
root with git rev-parse --show-toplevel) and update the commands to read the
.benchmark-fixtures-sha from that variable instead of a hardcoded home path so
the runbook is portable across machines.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1eb61a89-a1ff-475d-80f5-d28373d51a6e
📒 Files selected for processing (20)
docs/superpowers/plans/2026-05-27-wave3-resolver-followups.mddocs/superpowers/specs/2026-05-27-wave3-resolver-followups-design.mdpackage.jsonsrc/analysis/types.tssrc/ast/ast-scanner.tssrc/ast/cross-file-resolver.tssrc/ast/waste/batch-detector.tssrc/ast/waste/concurrency-detector.tssrc/scanner/core-scanner.tssrc/scanner/fingerprints/openai.jsonsrc/scanner/fingerprints/types.tssrc/test/a3-default-import-threading.test.tssrc/test/ast-inline-parallel.test.tssrc/test/factory-with-args.test.tssrc/test/fixtures/a3-followup/mixed-barrel/anthropic-named.tssrc/test/fixtures/a3-followup/mixed-barrel/barrel.tssrc/test/fixtures/a3-followup/mixed-barrel/consumer.tssrc/test/fixtures/a3-followup/mixed-barrel/openai-default.tssrc/test/fixtures/factory-args/consumer.tssrc/test/fixtures/factory-args/factory.ts
Summary
Wave 3 resolver/detection follow-ups left behind by PR #110, bundled into one PR (the accuracy track's
wave/3-resolver-followups):resolveExportedMatchescould not tell a default import from a named one, so a named import in a heterogeneous barrel (export { default } from "./a"+export { ask } from "./b") wrongly inherited the default re-export's provider. Threaded anisDefaultflag throughextractRelativeImports→resolveExportedMatches, split the re-export filter (default bindings follow onlyexport { default }; named/wildcard bindings never do), and encoded the dimension in the cycle-protectionvisitKey. Also fixedextractRelativeImportsto parse the default binding in mixed imports (import gen, { ask } from "x").extractFactoryCallAssignmentsmatched only no-argmakeClient(). Widened the regex somakeClient(config),makeClient({ apiKey }), multi-arg, and multi-line factory calls resolve.inlineParallelCapableflag.openai.images.generatewas flaggedbatchCapable, conflating a true batch endpoint with DALL·E's inlinen/count parameter and producing the wrong "use the batch endpoint" suggestion in a loop/fan-out. Added a distinctinlineParallelCapablefingerprint flag, reclassifiedimages.generate, suppressed the concurrency fan-out finding on either flag, and addeddetectInlineParallelemitting a "use the n/count parameter" suggestion instead.Built subagent-driven across two file-disjoint tracks (resolver / detector) in parallel worktrees, with per-track spec + code-quality review and a final whole-implementation review.
Test Plan
npm run test:scanner— full suite green, incl. 4 new tests (a3-default-import-threading,factory-with-args,ast-inline-parallel×4 cases). Each verified to fail on revert.npm run build:ext— clean compile.Follow-ups (non-blocking, to be filed)
inlineParallelCapableis intentionally not yet threaded into the intelligence graph (ApiCallNode), the regex pattern path (local-waste-detector.ts/openai-compatible.ts), the pattern dedup key, or a dashboard badge. The detectors work correctly via the AST path. Only user-visible effect: DALL·E endpoints lose the old (incorrect) "batch" dashboard badge until the badge follow-up lands.Closes #114
Closes #115
Closes #116
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
New Features
n/countparameters to fetch multiple results in a single request.Tests