diff --git a/benchmark/baseline.json b/benchmark/baseline.json index a672c5c..484714a 100644 --- a/benchmark/baseline.json +++ b/benchmark/baseline.json @@ -3,7 +3,7 @@ "detectionRecall": 0.5147058823529411, "providerAttributionAccuracy": 0.8275862068965517, "findingPrecision": 1, - "findingRecall": 0.3333333333333333, + "findingRecall": 0.6666666666666666, "findingMetricsByType": { "batch": { "truePositives": 0, @@ -20,11 +20,11 @@ "recall": 1 }, "unbatched_parallel": { - "truePositives": 0, + "truePositives": 1, "falsePositives": 0, - "falseNegatives": 1, + "falseNegatives": 0, "precision": 1, - "recall": 0 + "recall": 1 } } } diff --git a/dashboard/src/lib/types.ts b/dashboard/src/lib/types.ts index 5bc1cb0..568c488 100644 --- a/dashboard/src/lib/types.ts +++ b/dashboard/src/lib/types.ts @@ -102,7 +102,8 @@ export type SuggestionType = | "redundancy" | "n_plus_one" | "rate_limit" - | "concurrency_control"; + | "concurrency_control" + | "unbatched_parallel"; export type Severity = "high" | "medium" | "low"; diff --git a/dashboard/src/pages/Suggestions.tsx b/dashboard/src/pages/Suggestions.tsx index cfd2e53..464c73b 100644 --- a/dashboard/src/pages/Suggestions.tsx +++ b/dashboard/src/pages/Suggestions.tsx @@ -16,6 +16,7 @@ const severityConfig: Record> = { cache: Archive, batch: Layers, + unbatched_parallel: Layers, redundancy: RefreshCw, n_plus_one: Layers, rate_limit: Zap, @@ -25,6 +26,7 @@ const typeIcons: Partial> = { const typeLabels: Record = { cache: 'Cacheable', batch: 'Batchable', + unbatched_parallel: 'Unbatched Parallel', redundancy: 'Redundant Call', n_plus_one: 'N+1 Query', rate_limit: 'Rate Limit Risk', diff --git a/dashboard/src/styles/theme.css b/dashboard/src/styles/theme.css index 4f4c23b..fcaa795 100644 --- a/dashboard/src/styles/theme.css +++ b/dashboard/src/styles/theme.css @@ -127,8 +127,6 @@ display: none; } -<<<<<<< HEAD -======= @keyframes pulseGlow { 0% { fill: #444; diff --git a/docs/accuracy/findings.md b/docs/accuracy/findings.md index f45c5f6..093e01e 100644 --- a/docs/accuracy/findings.md +++ b/docs/accuracy/findings.md @@ -38,17 +38,18 @@ Each detector has a measured false-positive rate against the benchmark corpus (D - [ ] FPR is re-measured on every benchmark CI run; regressions fail the build. - [ ] False positives that remain are by-design (documented exceptions, e.g., "we choose to flag this conservatively because the cost of missing it is high"). -### Calibration table (measured 2026-05-13 against corpus v1 — 7 fixtures, 3 expected findings; refreshed after C1 PR-3 merged) +### Calibration table (measured 2026-05-13 against corpus v1 — 7 fixtures, 3 expected findings; refreshed after C1 PR-3 merged; updated after Wave 4 / #117) | Detector (scanner `type`) | TP | FP | FN | FPR | Precision | Severity (current) | Notes | |---|---|---|---|---|---|---|---| -| `n_plus_one` | 1 | 0 | 0 | 0% | 100% | high | Only detector with a corpus TP. Sample size = 1. | -| `cache` | 0 | 0 | 0 | — | — | medium | C1 PR-2 dropped emissions from 7 to 0 — Python detector now suppresses generative endpoints + explicit write-shaped HTTP methods, AST detector buckets fetch/axios redundancy by URL. No emissions, no expected entries; row collapses to absent in `findingMetricsByType`. | -| `batch` | 0 | 1 | 1 | 100% | 0% | medium | C1 PR-3 dropped emissions from 9 to 1 by bucketing both TS and Python sequential-batching detectors by `(provider, enclosingFunction)` (calls in different functions can't be batched together) plus line-dedup for cross-file resolver expansion. The one remaining FP is `bedrock-raw-fetch/src/index.ts:5` — two sequential `await handleApi(...)` calls in `main()`, arguably a true positive that the corpus didn't label. The expected `batch` TP at `flask-mixed-providers/src/providers/anthropic_helper.py:11` is still missed (the detector no longer mis-emits at line 7, so finding-recall is unchanged at 33.33%). Sample size = 1, below per-type gate threshold. | -| `rate_limit` | 0 | 1 | 0 | 100% | 0% | low | One FP. No expected entries. | -| `concurrency_control` | — | — | — | — | — | low | Scanner emits nothing on the corpus; not in the table. See "Type-name mismatch" below. | - -The corpus labels one fan-out finding as `unbatched_parallel`; the scanner emits `concurrency_control` for the same pattern. The matcher compares type strings exactly, so the expected `unbatched_parallel` shows up as a recall miss (FN = 1) and the scanner's `concurrency_control` (if it were ever emitted on this corpus) would show up as a separate row of FPs. As of 2026-05-13 the scanner emits zero `concurrency_control` findings on the corpus, so only the FN side appears. The label gap is tracked as a corpus follow-up — either rename the expected type to `concurrency_control` or have the scanner emit `unbatched_parallel` for this specific pattern. +| `n_plus_one` | 1 | 0 | 0 | 0% | 100% | high | Only detector with a corpus TP. Sample size = 1. | +| `cache` | 0 | 0 | 0 | — | — | medium | C1 PR-2 dropped emissions from 7 to 0 — Python detector now suppresses generative endpoints + explicit write-shaped HTTP methods, AST detector buckets fetch/axios redundancy by URL. No emissions, no expected entries; row collapses to absent in `findingMetricsByType`. | +| `batch` | 0 | 0 | 1 | — | 100% | medium | C1 PR-3 bucketed sequential-batching detectors by `(provider, enclosingFunction)` + line-dedup. The prior bedrock FP (`bedrock-raw-fetch/src/index.ts:5`) is gone — live benchmark shows `batch` clean at 0 FP. The expected TP at `flask-mixed-providers/src/providers/anthropic_helper.py:11` is still missed (FN = 1, recall 0%). Wave 4 / #117 attempted a cross-function `(provider, methodChain)` batching pass to recover this FN, but it fired equally on structurally-identical sibling helpers (`openai_helper.py`, `cohere_helper.py`) that are unlabeled in `expected.json`, producing 6 FPs and dropping `batch` precision to 14.3%. Because no AST signal distinguishes the labeled-TP case from the unlabeled-but-equivalent cases, this is a **corpus-labeling inconsistency**: either the sibling helpers should also be labeled as batch findings, or the FN should be accepted as unrecoverable without cross-file call-graph signal. The cross-function pass was reverted. Tracked as a corpus follow-up, not a detector fix. | +| `unbatched_parallel` | 1 | 0 | 0 | 0% | 100% | derived | Wave 4 / #117 recovered the DALL-E `Promise.all(Array.from({length:n}).map(() => images.generate()))` FN. A dedicated `unbatched_parallel` SuggestionType was added; `detectInlineParallel` (AST) and `detectInlineParallelFinding` (regex fallback) now emit it. The `BOUNDED_REPLICATION` guard was removed from the inline-parallel path because `inlineParallelCapable` is the precision control. Severity is derived from the score-based `deriveSeverity()` pipeline (not hardcoded). AST + regex paths both emit it. | +| `rate_limit` | 0 | 1 | 0 | 100% | 0% | low | One FP. No expected entries. | +| `concurrency_control` | — | — | — | — | — | low | Scanner emits nothing on the corpus; not in the table. | + +**Wave 4 / #117 outcome (2026-05-29):** Two benchmark finding false-negatives were targeted. One shipped: the DALL-E `unbatched_parallel` FN is recovered — finding recall rose from 33.33% → 66.67%, precision held at 100%. One did not ship: the Python cross-function `batch` FN at `flask-mixed-providers/anthropic_helper.py:11` remains open. The attempted recovery pass produced 6 FPs on structurally-identical but unlabeled sibling helper files — a corpus-labeling inconsistency, not a detector gap. The pass was reverted; the `batch` row is now clean (0 FP, precision 100%) with recall 0%. Recommended corpus follow-up: either label `openai_helper.py` and `cohere_helper.py` as batch findings too, or formally accept this FN as unrecoverable without cross-file call-graph signal. Acceptance criterion "no detector with FPR > 30%" passes for `n_plus_one` and `cache` after PR-2 and effectively passes for `batch` after PR-3 (sample size 1 below per-type gate threshold; remaining FP is borderline TP). Still fails for `rate_limit` (sample size 1). Sample sizes remain small (corpus v1 has 3 expected findings total), so the FPR numbers are diagnostic, not statistically robust. Wait until the corpus grows past N ≥ 10 expected findings per type before defending an "FPR < 30%" target as final. diff --git a/docs/superpowers/plans/2026-05-30-wave4-recall-recovery.md b/docs/superpowers/plans/2026-05-30-wave4-recall-recovery.md new file mode 100644 index 0000000..1d65c14 --- /dev/null +++ b/docs/superpowers/plans/2026-05-30-wave4-recall-recovery.md @@ -0,0 +1,494 @@ +# Wave 4 — Recall Recovery Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Recover the two C1 false negatives (#117) — the Python cross-function batch FN and the DALL-E inline-parallel FN — without introducing new false positives. + +**Architecture:** Two file-disjoint tracks. Track A adds a module-scope `(provider, methodChain)` second pass to the Python sequential-batching detector. Track B lifts the `BOUNDED_REPLICATION` guard from the inline-parallel detector and gives that finding a new dedicated `unbatched_parallel` type, threaded through the `SuggestionType` union and all of its consumers. + +**Tech Stack:** TypeScript (strict), Node `node:test` runner, web-tree-sitter AST, the `../extension-benchmark` corpus + `npm run benchmark` gate. + +**Spec:** [`docs/superpowers/specs/2026-05-30-wave4-recall-recovery-design.md`](../specs/2026-05-30-wave4-recall-recovery-design.md) + +--- + +## Task 0: Capture the live benchmark baseline (BLOCKING — do first) + +**Why:** The issue says the `batch` row is currently `0/0/1`, but the older `docs/accuracy/findings.md` calibration table records a `batch` **FP** at `bedrock-raw-fetch/src/index.ts:5`. They disagree because one is stale. If the bedrock FP still exists, recovering the FN lands the row at TP 1 / FP 1 — failing the "FP 0" bar. That FP is a corpus-labeling question, not something Track A's code fixes. We must know the truth before writing code. + +**Files:** none (measurement only) + +- [ ] **Step 1: Confirm the corpus is present** + +Run: `ls ../extension-benchmark` +Expected: directories including `flask-mixed-providers`, `langchain-openai`, `bedrock-raw-fetch`. +If missing: `git clone https://github.com/recost-dev/extension-benchmark.git ../extension-benchmark` + +- [ ] **Step 2: Run the benchmark and record the two rows** + +Run: `npm run benchmark` +Capture the `findingMetricsByType` rows for `batch` and `unbatched_parallel` (TP/FP/FN each). Write the actual numbers into the PR description. + +- [ ] **Step 3: Decide the bedrock FP disposition** + +If the `batch` row shows an FP at `bedrock-raw-fetch/src/index.ts:5`: +- This is OUT OF SCOPE for the code in this plan. STOP and surface it to the human as a blocking decision: either (a) label the bedrock case a true positive in `../extension-benchmark/bedrock-raw-fetch/expected.json`, or (b) accept the row will read FP 1 and relax the acceptance bar. +- Do not silently absorb it. + +If the `batch` row is clean (`0/0/1`): proceed to Track A. + +--- + +## Track A — Python cross-function batch FN + +**File:** `src/scanner/python-waste-detector.ts` +**Test:** `src/test/python-waste-detector.test.ts` + +### Context (existing code you are extending) + +`detectSequentialBatching` (currently lines ~228–282) buckets by `(providerKey, enclosingFunction)` and requires ≥3 calls within a 30-line cluster. The fixture `flask-mixed-providers/src/providers/anthropic_helper.py` has `_client.messages.create` once in `summarize` (line 11) and once in `summarize_with_style` (line 20) — two different functions, one call each, so neither bucket fires. You will add a **second pass** at module scope. + +Helpers already in the file you will reuse: +- `makeFinding(type, filePath, line, riskScore, confidence, description, evidence[])` +- `NON_LOOP_FREQUENCIES` (the Set used by the primary pass's `continue` guard) +- `ASYNCIO_GATHER` and `CONCURRENCY_GUARD` regexes +- `betweenWindow(lines, firstLine, lastLine, padding)` + +Each `ClassifiedMatch` exposes `match.enclosingFunction`, `match.methodChain`, `match.line`, `match.frequency`, and `providerKey`. + +### Task A1: Failing test for the cross-function batch FN + +- [ ] **Step 1: Write the failing test** + +Add to `src/test/python-waste-detector.test.ts` (follow the existing `run(...)`/`assert` style in that file; the snippet below is the assertion logic — adapt the harness call to match how other tests in the file invoke the Python detector): + +```ts +run("batch: same method across two module functions → one batch finding at earliest line", () => { + const source = [ + "import anthropic", + "", + "_client = anthropic.Anthropic()", + "", + "def summarize(text):", + " msg = _client.messages.create(model='claude-3-haiku', messages=[])", + " return msg", + "", + "def summarize_with_style(text, style):", + " msg = _client.messages.create(model='claude-3-haiku', messages=[])", + " return msg", + ].join("\n"); + + const findings = runPythonWasteDetector(source, "src/providers/anthropic_helper.py"); + const batch = findings.filter((f) => f.type === "batch"); + assert.equal(batch.length, 1, "expected exactly one batch finding"); + assert.equal(batch[0].line, 6, "finding should anchor at the earliest call line"); +}); +``` + +(Use whatever existing helper the file already uses to run the detector; if it is named differently than `runPythonWasteDetector`, match the file. Line 6 here is the first `messages.create` in this synthetic source.) + +- [ ] **Step 2: Run the test, verify it fails** + +Run: `npx tsc -p tsconfig.scanner-tests.json && node dist-test/test/python-waste-detector.test.js` +Expected: FAIL — `expected exactly one batch finding` (got 0), because the function-scoped pass needs ≥3 calls per function. + +### Task A2: Implement the module-scope second pass + +- [ ] **Step 3: Add the second bucketing pass** + +In `src/scanner/python-waste-detector.ts`, inside `detectSequentialBatching`, after the existing function-scoped loop builds and emits its findings, add a second pass before `return findings;`: + +```ts + // Second pass: cross-function batching. The same (providerKey, methodChain) + // called in ≥2 distinct functions in one module is batchable even though the + // calls live in different functions. Keying on methodChain (not just provider) + // is the FP guard — different SDKs / different methods never merge (PR-3 trap). + const byMethod = new Map(); + for (const classified of matches) { + if (!NON_LOOP_FREQUENCIES.has(classified.match.frequency)) continue; + const methodChain = classified.match.methodChain ?? ""; + if (!methodChain) continue; + const key = `${classified.providerKey}::${methodChain}`; + const bucket = byMethod.get(key) ?? { providerKey: classified.providerKey, methodChain, matches: [] }; + bucket.matches.push(classified); + byMethod.set(key, bucket); + } + + for (const { providerKey, methodChain, matches: group } of byMethod.values()) { + const fns = new Set(group.map((c) => c.match.enclosingFunction ?? "")); + if (fns.size < 2) continue; // needs ≥2 distinct functions + const sorted = [...group].sort((a, b) => a.match.line - b.match.line); + const firstLine = sorted[0].match.line; + const lastLine = sorted[sorted.length - 1].match.line; + const window = betweenWindow(lines, firstLine, lastLine, 5); + if (ASYNCIO_GATHER.test(window) || CONCURRENCY_GUARD.test(window)) continue; + + // Dedupe: skip if the function-scoped pass already emitted a batch finding at this line. + if (findings.some((f) => f.type === "batch" && f.line === firstLine)) continue; + + findings.push( + makeFinding( + "batch", + filePath, + firstLine, + 4, + 0.7, + `${group.length} calls to "${methodChain}" across multiple functions in this module — consolidate into a single batched call.`, + [ + `"${methodChain}" (${providerKey}) is called in ${fns.size} functions: lines ${sorted.map((c) => c.match.line).join(", ")}.`, + "Calls share a provider and method, so they can be batched into one request.", + ] + ) + ); + } +``` + +- [ ] **Step 4: Run the test, verify it passes** + +Run: `npx tsc -p tsconfig.scanner-tests.json && node dist-test/test/python-waste-detector.test.js` +Expected: PASS. + +### Task A3: FP-guard negative test + +- [ ] **Step 5: Write the negative test** + +Add to `src/test/python-waste-detector.test.ts`: + +```ts +run("batch: different methods across functions → NO cross-function batch finding", () => { + const source = [ + "import anthropic", + "_client = anthropic.Anthropic()", + "", + "def a(text):", + " return _client.messages.create(model='m', messages=[])", + "", + "def b(text):", + " return _client.completions.create(model='m', prompt=text)", + ].join("\n"); + + const findings = runPythonWasteDetector(source, "src/providers/mixed.py"); + const batch = findings.filter((f) => f.type === "batch"); + assert.equal(batch.length, 0, "different methodChains must not merge into a batch finding"); +}); +``` + +- [ ] **Step 6: Run it, verify it passes** + +Run: `node dist-test/test/python-waste-detector.test.js` +Expected: PASS (the `methodChain` key keeps `messages.create` and `completions.create` in separate buckets, each with 1 function → below the ≥2 bar). + +- [ ] **Step 7: Run the full scanner suite** + +Run: `npm run test:scanner` +Expected: all green (no regression to existing batch/C1 tests). + +- [ ] **Step 8: Commit** + +```bash +git add src/scanner/python-waste-detector.ts src/test/python-waste-detector.test.ts +git commit -m "feat(wave4): cross-function batch detection in python waste detector (#117)" +``` + +--- + +## Track B — DALL-E inline-parallel FN + new `unbatched_parallel` type + +This track has two parts: (1) the detector change in `batch-detector.ts`, (2) registering `unbatched_parallel` across the `SuggestionType` union and every consumer. + +### Task B1: Add `unbatched_parallel` to the SuggestionType union + +**Files (3 union declarations):** +- `src/analysis/types.ts:79-85` +- `webview/src/types.ts:17` +- `dashboard/src/lib/types.ts:99-105` + +- [ ] **Step 1: Extend `src/analysis/types.ts`** + +Change: +```ts +export type SuggestionType = + | "cache" + | "batch" + | "redundancy" + | "n_plus_one" + | "rate_limit" + | "concurrency_control"; +``` +to add the new member: +```ts +export type SuggestionType = + | "cache" + | "batch" + | "redundancy" + | "n_plus_one" + | "rate_limit" + | "concurrency_control" + | "unbatched_parallel"; +``` + +- [ ] **Step 2: Extend `webview/src/types.ts`** + +Change line 17 from: +```ts +export type SuggestionType = "cache" | "batch" | "redundancy" | "n_plus_one" | "rate_limit" | "concurrency_control"; +``` +to: +```ts +export type SuggestionType = "cache" | "batch" | "redundancy" | "n_plus_one" | "rate_limit" | "concurrency_control" | "unbatched_parallel"; +``` + +- [ ] **Step 3: Extend `dashboard/src/lib/types.ts`** + +Add `| "unbatched_parallel"` to the `SuggestionType` union ending at line 105 (after `| "concurrency_control"`, keeping the trailing semicolon on the last line). + +- [ ] **Step 4: Verify the build now FAILS with an exhaustiveness error** + +Run: `npm run build:ext` +Expected: a TypeScript error in `src/intelligence/compression.ts` at `FINDING_TITLE_BY_TYPE` — it is the one `Record` (fully exhaustive, non-`Partial`) map, so it must gain the new key. **Important:** the other consumer maps below are either `Record` or `Partial>`, so the compiler will NOT flag them. They must be updated by hand — the build passing is not proof they're complete. + +### Task B2: Register `unbatched_parallel` in every consumer map + +**Files:** +- `src/scan-results.ts:31-37` (`SAVINGS_MULTIPLIERS`) +- `src/intelligence/compression.ts:38-51` (`FINDING_TITLE_BY_TYPE` exhaustive — required; `FINDING_LABEL_BY_TYPE` Partial — optional) +- `webview/src/components/ResultsPage.tsx:19-29` (`typeLabels`, `Record` — manual) +- `dashboard/src/pages/Suggestions.tsx:16-32` (`typeIcons` Partial + `typeLabels` `Record` — manual) + +- [ ] **Step 1: Savings multiplier — `src/scan-results.ts`** + +The table currently reads: +```ts +export const SAVINGS_MULTIPLIERS: Partial> = { + redundancy: 0.40, + n_plus_one: 0.35, + cache: 0.30, + batch: 0.20, + concurrency_control: 0.22, +}; +``` +Add an `unbatched_parallel` entry matching `batch` (same cost-savings family): +```ts + batch: 0.20, + unbatched_parallel: 0.20, + concurrency_control: 0.22, +``` + +- [ ] **Step 2: Intelligence titles — `src/intelligence/compression.ts`** (this is the build-breaking one) + +In `FINDING_TITLE_BY_TYPE` (the exhaustive `Record` that contains `batch: "Batching opportunity"`), add alongside `batch`: +```ts + batch: "Batching opportunity", + unbatched_parallel: "Unbatched parallel fan-out", +``` +Optionally also add to `FINDING_LABEL_BY_TYPE` (the `Partial` map) for a richer label: +```ts + unbatched_parallel: "Unbatched parallel fan-out", +``` + +- [ ] **Step 3: Webview label — `webview/src/components/ResultsPage.tsx`** + +`typeLabels` (a `Record`) currently: +```ts +const typeLabels: Record = { + n_plus_one: "n+1", + cache: "cache", + batch: "batch", + redundancy: "redundancy", + rate_limit: "rate-limit", + concurrency_control: "concurrency", + retry_storm: "retry storm", + event_amplification: "event amp", + sequential: "sequential", +}; +``` +Add alongside `batch`: +```ts + batch: "batch", + unbatched_parallel: "unbatched parallel", +``` + +- [ ] **Step 4: Dashboard icon + label — `dashboard/src/pages/Suggestions.tsx`** + +In `typeIcons` (`Partial>`, currently maps `batch: Layers`), add `unbatched_parallel: Layers,`. +In `typeLabels` (`Record`, currently maps `batch: 'Batchable'`), add `unbatched_parallel: 'Unbatched Parallel',`. + +- [ ] **Step 5: Verify the full build passes** + +Run: `npm run build` +Expected: clean (dashboard + webview + extension). The compression exhaustiveness error from B1 Step 4 is resolved; the manual maps (Steps 1, 3, 4) are filled even though the compiler didn't force them. + +- [ ] **Step 6: Commit the type plumbing** + +```bash +git add src/analysis/types.ts webview/src/types.ts dashboard/src/lib/types.ts src/scan-results.ts src/intelligence/compression.ts webview/src/components/ResultsPage.tsx dashboard/src/pages/Suggestions.tsx +git commit -m "feat(wave4): register unbatched_parallel suggestion type across consumers (#117)" +``` + +### Task B3: Failing test for the DALL-E inline-parallel FN + +**File:** `src/ast/waste/batch-detector.ts` +**Test:** `src/test/ast-batch-detector.test.ts` + +Context: `detectInlineParallel` (currently lines ~270–317) emits `type: "batch"` and has this guard at ~line 281: +```ts + // Array.from({ length: N }) is intentional bounded replication — not naive fan-out. + if (match.frequency === "parallel" && hasGuardInWindow(source, match.line, BOUNDED_REPLICATION)) return null; +``` +The DALL-E fixture matches `BOUNDED_REPLICATION` (`Array.from({ length: this.n })`), so it's suppressed. + +- [ ] **Step 1: Write the failing test** + +Add to `src/test/ast-batch-detector.test.ts` (match the file's existing `run(...)` harness and how it builds `AstCallMatch` inputs): + +```ts +run("inline-parallel: Array.from({length:n}) fan-out on an n-capable endpoint → unbatched_parallel", () => { + const source = [ + "const results = await Promise.all(", + " Array.from({ length: this.n }).map(() =>", + " this.client.images.generate(fields)", + " )", + ");", + ].join("\n"); + + const match = makeMatch({ + line: 3, + frequency: "parallel", + inlineParallelCapable: true, + provider: "openai", + methodSignature: "images.generate", + }); + + const findings = detectBatchWaste([match], source, "src/tools/dalle.ts"); + const inline = findings.filter((f) => f.type === "unbatched_parallel"); + assert.equal(inline.length, 1, "expected one unbatched_parallel finding"); +}); +``` + +(Use the file's existing match-builder helper; `makeMatch` is illustrative — match the real helper name and required fields.) + +- [ ] **Step 2: Run it, verify it fails** + +Run: `npx tsc -p tsconfig.scanner-tests.json && node dist-test/test/ast-batch-detector.test.js` +Expected: FAIL — 0 findings (guard suppresses) AND the type is `"batch"` not `"unbatched_parallel"`. + +### Task B4: Lift the guard and emit the new type + +- [ ] **Step 3: Remove the BOUNDED_REPLICATION guard from `detectInlineParallel` only** + +In `src/ast/waste/batch-detector.ts`, inside `detectInlineParallel`, delete these two lines: +```ts + // Array.from({ length: N }) is intentional bounded replication — not naive fan-out. + if (match.frequency === "parallel" && hasGuardInWindow(source, match.line, BOUNDED_REPLICATION)) return null; +``` +Leave the identical guard in `detectBatch` untouched. (`BOUNDED_REPLICATION` is still referenced by `detectBatch`, so the const stays.) + +- [ ] **Step 4: Change the emitted type** + +In the `return` object of `detectInlineParallel`, change: +```ts + type: "batch" as SuggestionType, +``` +to: +```ts + type: "unbatched_parallel" as SuggestionType, +``` +(The `id` field `local-inline_parallel-...` already names it correctly — leave it.) + +- [ ] **Step 5: Run the test, verify it passes** + +Run: `node dist-test/test/ast-batch-detector.test.js` +Expected: PASS. + +### Task B5: Regression test — guard removal didn't widen blast radius + +- [ ] **Step 6: Write the regression test** + +Add to `src/test/ast-batch-detector.test.ts`: + +```ts +run("inline-parallel: Array.from fan-out on a NON-n-capable endpoint → no unbatched_parallel", () => { + const source = [ + "const results = await Promise.all(", + " Array.from({ length: 5 }).map(() => client.chat.completions.create(body))", + ");", + ].join("\n"); + + const match = makeMatch({ + line: 2, + frequency: "parallel", + inlineParallelCapable: false, // endpoint has no n/count parameter + provider: "openai", + methodSignature: "chat.completions.create", + }); + + const findings = detectBatchWaste([match], source, "src/x.ts"); + assert.equal( + findings.filter((f) => f.type === "unbatched_parallel").length, + 0, + "inlineParallelCapable=false must not produce unbatched_parallel" + ); +}); +``` + +- [ ] **Step 7: Run it, verify it passes** + +Run: `node dist-test/test/ast-batch-detector.test.js` +Expected: PASS (the `if (!match.inlineParallelCapable) return null;` gate inside `detectInlineParallel` is the precision control that survives guard removal). + +- [ ] **Step 8: Run the full scanner suite** + +Run: `npm run test:scanner` +Expected: all green. + +- [ ] **Step 9: Commit** + +```bash +git add src/ast/waste/batch-detector.ts src/test/ast-batch-detector.test.ts +git commit -m "feat(wave4): recover DALL-E inline-parallel FN as unbatched_parallel (#117)" +``` + +--- + +## Task C: Whole-wave verification gate (after both tracks merge) + +**Files:** none (verification only) + +- [ ] **Step 1: Full build** + +Run: `npm run build` +Expected: clean across dashboard + webview + extension. + +- [ ] **Step 2: Full scanner suite** + +Run: `npm run test:scanner` +Expected: all green, including all 7 pre-existing C1 tests. + +- [ ] **Step 3: Benchmark — confirm both rows recovered** + +Run: `npm run benchmark` +Expected in `findingMetricsByType`: +- `batch` → TP 1 / FP 0 / FN 0 +- `unbatched_parallel` → TP 1 / FP 0 / FN 0 +- No per-type precision regression on any other row. + +(If `batch` shows FP 1 from the bedrock case flagged in Task 0, that's the corpus-labeling decision — resolve per Task 0 Step 3, not by changing detector code here.) + +- [ ] **Step 4: Update the C1 calibration table** + +In `docs/accuracy/findings.md`, update the C1 calibration table: `batch` and `unbatched_parallel` rows to their recovered TP/FP/FN, and note "FN recovered in Wave 4 / #117". Mark the #117 items shipped. + +- [ ] **Step 5: Commit docs** + +```bash +git add docs/accuracy/findings.md +git commit -m "docs(wave4): mark C1 false negatives recovered (#117)" +``` + +--- + +## Self-review notes + +- **Spec coverage:** Track A ↔ Python batch FN; Track B (B1–B5) ↔ DALL-E FN + `unbatched_parallel` type; Task 0 ↔ baseline/bedrock-FP risk; Task C ↔ whole-wave gates + doc update. All spec sections covered. +- **Type consistency:** new literal is `"unbatched_parallel"` everywhere (union, multiplier, labels, titles, detector emission, tests). `detectBatchWaste` / `detectInlineParallel` / `makeFinding` names match the existing source. +- **Parallelization note for the workflow:** Track A and Track B touch disjoint files. The only ordering constraint *within* Track B is B1 (union) → B2 (consumers) → B3–B5 (detector); B1's deliberately-failing build is the driver for B2. Track A is fully independent of Track B and can run concurrently. Task C is the barrier after both. diff --git a/docs/superpowers/specs/2026-05-30-wave4-recall-recovery-design.md b/docs/superpowers/specs/2026-05-30-wave4-recall-recovery-design.md new file mode 100644 index 0000000..e9770c1 --- /dev/null +++ b/docs/superpowers/specs/2026-05-30-wave4-recall-recovery-design.md @@ -0,0 +1,110 @@ +# Wave 4 — Recover the two C1 false negatives (#117) + +**Date:** 2026-05-30 +**Issue:** [#117](https://github.com/recost-dev/extension/issues/117) — `wave/4-recall-recovery`, `area/findings` +**Design note backing this work:** [`docs/accuracy/findings.md`](../../accuracy/findings.md) (C1) + +## Problem + +PR #111 closed C1 (#83) by driving the local waste detector's false positives to zero. Two labeled **false negatives** were intentionally accepted as out-of-scope at the time. Both are cases where a previously-shipped guard now suppresses a real positive: + +1. **`batch` FN** at `flask-mixed-providers/src/providers/anthropic_helper.py:11` — `_client.messages.create(...)` is called once each in two module-level functions (`summarize`, `summarize_with_style`). The same SDK method called repeatedly across functions in one module is batchable, but PR-3's `(provider, enclosingFunction)` bucketing suppresses it (calls in different functions land in different buckets, and each bucket has < 3 calls). + +2. **`unbatched_parallel` FN** at `langchain-openai/src/libs/langchain-openai/src/tools/dalle.ts:242` — `Promise.all(Array.from({ length: this.n }).map(() => this.client.images.generate(...)))` fans out `n` parallel image-generation requests with identical params. DALL-E's `images.generate` accepts `n` directly in a single request, so this is provably wasteful — but the `BOUNDED_REPLICATION` guard (PR #110) silences it, and even with the guard lifted the detector emits the wrong type string. + +Both need a finer structural signal than the AST currently uses. This wave is sequenced last in the accuracy roadmap precisely because the C1 PR-3 → PR-4 loop has historically re-introduced FPs; the guardrails below are the core of the design. + +## Goals / Non-goals + +**Goals** +- Recover both false negatives so the benchmark shows `batch` and `unbatched_parallel` rows at TP 1 / FP 0 / FN 0. +- Introduce no new false positives — per-type precision must hold on every other row. +- Keep the two fixes file-disjoint so they parallelize across two subagent tracks. + +**Non-goals** +- Corpus expansion (#113) or traceability dual-locations (#81) — separate waves. +- Reworking the `concurrency_control` detector or the existing `(provider, enclosingFunction)` batch pass; the new Python pass is additive. + +## Design + +Two recall-recovery fixes, file-disjoint, one per track. + +### Track A — Python cross-function batch FN + +**File:** `src/scanner/python-waste-detector.ts` (`detectSequentialBatching`) + +Add a **second bucketing pass** that runs after the existing `(providerKey, enclosingFunction)` pass: + +- Bucket non-loop matches by `(providerKey, methodChain)` at **module scope** (i.e. across enclosing functions). +- Fire a `batch` finding when the same `(providerKey, methodChain)` appears in **≥ 2 distinct functions** with no nearby `asyncio.gather` / concurrency-limiter guard. +- Anchor the finding at the **earliest** call line in the group (→ line 11 for the fixture, inside the ±2 line tolerance the benchmark matcher allows). +- Dedupe against the primary function-scoped pass so a cluster already flagged is not double-emitted (reuse the existing line/finding dedupe). + +**Suggestion copy:** "N calls to `{methodChain}` across multiple functions in this module — consolidate into a single batched call." + +**FP guardrail (the PR-3 trap):** keying on `methodChain` equality — not just `providerKey` — is the safeguard. Calls to different SDKs or different methods never merge into one bucket. This directly honors the issue's "must NOT re-introduce the FPs that motivated PR-3 (calls across different SDKs / different methodChains)." The "≥ 2 distinct functions" requirement (vs the primary pass's "≥ 3 in a line cluster") is the looser bar where FP risk lives, so it is gated tightly by exact-method bucketing + the concurrency-guard window check. + +### Track B — DALL-E inline-parallel FN + new `unbatched_parallel` type + +Two coupled changes. + +**1. Lift the guard.** `src/ast/waste/batch-detector.ts` — remove the `BOUNDED_REPLICATION` early-return *inside `detectInlineParallel`* only. Keep it in `detectBatch`, where there is no n-parameter signal and bounded replication may be legitimate. Rationale: `detectInlineParallel` runs only when `inlineParallelCapable` is true (the endpoint has an n/count parameter, per #116). Once that is known, `Array.from({ length: n }).map(call)` is the *symptom* of the waste, not a mitigating factor — the flag is the precision control the guard was providing elsewhere. + +**2. New `unbatched_parallel` type.** `detectInlineParallel` emits `type: "unbatched_parallel"` instead of `"batch"`. The benchmark matcher compares type strings **exactly** (no aliasing), and the corpus labels this finding `unbatched_parallel`, so the scanner must produce that literal. It is a **cost / batch-family** finding (you can save money by passing `n`), so it groups with `batch`, **not** with the reliability set (`rate_limit`, `concurrency_control`). + +**Type ripple** (traced from how `concurrency_control` threads through the codebase) — register `unbatched_parallel` in: + +- **Union (3 declarations):** `src/analysis/types.ts`, `webview/src/types.ts`, `dashboard/src/lib/types.ts` +- **Savings multiplier:** `src/scan-results.ts` `BASE_MULTIPLIERS` — value `0.20`, matching `batch` +- **Labels / icons:** `webview/src/components/ResultsPage.tsx` `TYPE_LABELS` (e.g. `"unbatched parallel"`); `dashboard/src/pages/Suggestions.tsx` (icon map + label map) +- **Intelligence titles:** `src/intelligence/compression.ts` (title maps, e.g. "Unbatched parallel fan-out") +- **Explicitly NOT** added to `RELIABILITY_FINDING_TYPES` in `src/intelligence/scorer.ts` / `src/intelligence/clusters.ts` — it is a cost finding, so it does not contribute to the reliability score. + +TypeScript exhaustiveness over the union will force every site that switches on `SuggestionType` to handle the new member; `npm run build` failing is itself a completeness check for the ripple. + +## Testing + +**Track A** — `src/test/python-waste-detector.test.ts`: +- New: two same-method calls across two functions in a module → exactly one `batch` finding anchored at the earliest line. +- Negative (FP guard): two calls of *different* methods (or *different* providers) across functions → no finding. +- Each test must fail on revert of the implementation. + +**Track B** — `src/test/ast-batch-detector.test.ts` plus the type-ripple sites: +- New: `Promise.all(Array.from({ length: n }).map(() => client.images.generate(...)))` on an `inlineParallelCapable` endpoint → one `unbatched_parallel` finding. +- Regression: a non-`inlineParallelCapable` `Array.from({ length: N })` fan-out is still suppressed (guard removal did not widen blast radius). +- Each test must fail on revert. + +**Establish the live baseline first.** The issue records the `batch` row as "currently 0/0/1", but the older `docs/accuracy/findings.md` calibration table (2026-05-13) records a `batch` **FP** at `bedrock-raw-fetch/src/index.ts:5` (two sequential `await handleApi(...)` calls, an arguably-true-positive the corpus did not label). These disagree because one is stale. **Before any code change, run `npm run benchmark` to capture the real current `batch` / `unbatched_parallel` rows.** If the bedrock FP still exists, recovering the FN yields TP 1 / FP 1 / FN 0 — failing the "FP 0" bar. That FP is *not* addressed by Track A's code change; it is a corpus-labeling question (label the bedrock case a TP, or tighten separately) and must be surfaced as a blocking finding, not silently absorbed. + +**Whole-wave gates (all must pass before merge):** +- `npm run test:scanner` green. +- `npm run build` clean (catches every UI / dashboard ripple site via exhaustiveness). +- `npm run benchmark` against `../extension-benchmark`: + - `batch` row → TP 1 / FP 0 / FN 0 (currently 0 / 1 / 1). + - `unbatched_parallel` row → TP 1 / FP 0 / FN 0 (currently 0 / 0 / 1). + - No per-type precision regression on any other row. This is the issue's real acceptance bar and the FP-risk backstop. +- All 7 existing C1 tests still pass (no regression to PR-3 / PR-4 fixtures). + +## Implementation via dynamic subagent workflow + +Mirrors the #126 Wave 3 build: + +- **Two parallel implementation tracks**, worktree-isolated: + - **Track A** — `src/scanner/python-waste-detector.ts` + its test. + - **Track B** — `src/ast/waste/batch-detector.ts` + the `unbatched_parallel` type ripple (union, multiplier, labels, intelligence titles) + its test. + - File-disjoint: the only shared surface is test infrastructure conventions; the type union is touched by B only. +- Each track: implement → self-verify (`npm run test:scanner` + targeted tests) → code-quality review subagent. +- **Barrier.** Then a final whole-impl pass: `npm run build` + `npm run benchmark`, confirm both rows recovered with zero regression, final review verdict. + +## Acceptance criteria + +- [ ] `batch` benchmark row at TP 1 / FP 0 / FN 0. +- [ ] `unbatched_parallel` benchmark row at TP 1 / FP 0 / FN 0. +- [ ] No new FPs: per-type precision stays at 100% for both rows and no other row regresses. +- [ ] All 7 existing C1 tests still pass. +- [ ] `npm run build` and `npm run test:scanner` green. + +## Risks + +- **FP re-introduction (Track A).** The cross-function pass is a looser bar than the existing cluster pass. Mitigated by exact `methodChain` bucketing, the ≥ 2-distinct-functions requirement, and the concurrency-guard window check. The benchmark precision gate is the backstop — if any other row regresses, the pass is too loose. +- **Type-ripple miss (Track B).** A missed switch site would surface the new type with a fallback label or break the build. Exhaustiveness + `npm run build` is the guard. diff --git a/package.json b/package.json index e5e84db..1897f57 100644 --- a/package.json +++ b/package.json @@ -198,7 +198,7 @@ "build:webview": "cd webview && npm run build", "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 && 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/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/python-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", "calibrate-detectors": "tsc -p tsconfig.scanner-tests.json && node dist-test/test/waste-calibration.js", "watch:ext": "node esbuild.mjs --watch", "watch:webview": "cd webview && npm run build -- --watch", diff --git a/src/analysis/types.ts b/src/analysis/types.ts index 15a453e..31933ec 100644 --- a/src/analysis/types.ts +++ b/src/analysis/types.ts @@ -86,7 +86,8 @@ export type SuggestionType = | "redundancy" | "n_plus_one" | "rate_limit" - | "concurrency_control"; + | "concurrency_control" + | "unbatched_parallel"; export type Severity = "high" | "medium" | "low"; diff --git a/src/ast/waste/batch-detector.ts b/src/ast/waste/batch-detector.ts index 252ccb1..cd131af 100644 --- a/src/ast/waste/batch-detector.ts +++ b/src/ast/waste/batch-detector.ts @@ -277,8 +277,6 @@ function detectInlineParallel( if (!BATCH_LOOP_FREQS.has(match.frequency)) return null; if (!match.inlineParallelCapable) return null; if (hasGuardInWindow(source, match.line, BATCH_GUARD)) return null; - // Array.from({ length: N }) is intentional bounded replication — not naive fan-out. - if (match.frequency === "parallel" && hasGuardInWindow(source, match.line, BOUNDED_REPLICATION)) return null; const evidence: string[] = [ `Call executes in a "${match.frequency}" context — each iteration issues a separate request.`, @@ -304,7 +302,7 @@ function detectInlineParallel( return { id: `local-inline_parallel-${filePath}:${match.line}`, - type: "batch" as SuggestionType, + type: "unbatched_parallel" as SuggestionType, severity: scoreToSeverity(score), riskScore: score, confidence, diff --git a/src/intelligence/INTEL_CONTEXT.md b/src/intelligence/INTEL_CONTEXT.md index 4274691..8b30b16 100644 --- a/src/intelligence/INTEL_CONTEXT.md +++ b/src/intelligence/INTEL_CONTEXT.md @@ -107,7 +107,7 @@ Defines every interface contract used across the pipeline. Nothing is inferred a **Frequency cost weights:** `unbounded-loop` = 3, `parallel` = 2, `polling` = 2, `bounded-loop` = 1 -**Cost leak finding types:** `cache`, `batch`, `n_plus_one`, `redundancy` (weighted by severity: high=3, medium=2, low=1) +**Cost leak finding types:** `cache`, `batch`, `unbatched_parallel`, `n_plus_one`, `redundancy` (weighted by severity: high=3, medium=2, low=1) **Reliability finding types:** `rate_limit` (+3), `concurrency_control` (+3) diff --git a/src/intelligence/compression.ts b/src/intelligence/compression.ts index e2ab763..8ba1fa8 100644 --- a/src/intelligence/compression.ts +++ b/src/intelligence/compression.ts @@ -42,12 +42,14 @@ const FINDING_TITLE_BY_TYPE: Record = { redundancy: "Repeated API pattern", n_plus_one: "N+1 risk", batch: "Batching opportunity", + unbatched_parallel: "Unbatched parallel fan-out", }; const FINDING_LABEL_BY_TYPE: Partial> = { rate_limit: "Rate-limit finding", concurrency_control: "Concurrency-control finding", cache: "Cacheable call without cache", redundancy: "Repeated API pattern", + unbatched_parallel: "Unbatched parallel fan-out", }; const SEVERITY_WEIGHT: Record = { high: 3, medium: 2, low: 1 }; diff --git a/src/intelligence/scorer.ts b/src/intelligence/scorer.ts index a01f12e..8868938 100644 --- a/src/intelligence/scorer.ts +++ b/src/intelligence/scorer.ts @@ -14,7 +14,7 @@ import { collectRealProviders } from "./provider-normalization"; const HIGH_FREQUENCY_CLASSES = new Set(["unbounded-loop", "parallel", "polling"]); const RELIABILITY_FINDING_TYPES = new Set(["rate_limit", "concurrency_control"]); -const COST_LEAK_FINDING_TYPES = new Set(["cache", "batch", "n_plus_one", "redundancy"]); +const COST_LEAK_FINDING_TYPES = new Set(["cache", "batch", "unbatched_parallel", "n_plus_one", "redundancy"]); const SEVERITY_WEIGHT: Record = { high: 3, medium: 2, low: 1 }; const FREQUENCY_COST_WEIGHT: Record = { "unbounded-loop": 3, diff --git a/src/scan-results.ts b/src/scan-results.ts index 63040b9..ef4da28 100644 --- a/src/scan-results.ts +++ b/src/scan-results.ts @@ -35,6 +35,7 @@ export const SAVINGS_MULTIPLIERS: Partial> = n_plus_one: 0.35, cache: 0.30, batch: 0.20, + unbatched_parallel: 0.20, concurrency_control: 0.22, }; diff --git a/src/scanner/local-waste-detector.ts b/src/scanner/local-waste-detector.ts index 03b98ac..4cf10ac 100644 --- a/src/scanner/local-waste-detector.ts +++ b/src/scanner/local-waste-detector.ts @@ -333,7 +333,7 @@ function detectInlineParallelFinding(relativePath: string, site: MatchedCallSite if (confidence < 0.35) return null; return { id: `local-inline_parallel-${relativePath}:${site.line}`, - type: "batch" as SuggestionType, + type: "unbatched_parallel" as SuggestionType, severity: scoreToSeverity(score), confidence, riskScore: score, @@ -452,10 +452,11 @@ function dedupeFindings(findings: Array): LocalWasteFi const deduped = new Map(); for (const finding of findings) { if (!finding) continue; - // Key on the detector-specific id so distinct detectors that share a - // SuggestionType at the same site stay separate — e.g. batch - // (`local-batch-…`) vs inline-parallel (`local-inline_parallel-…`), which - // both emit type "batch". Fall back to type:file:line if an id is missing. + // Key on the detector-specific id so distinct detectors at the same site + // stay separate — batch (`local-batch-…`) emits type "batch"; inline-parallel + // (`local-inline_parallel-…`) now emits type "unbatched_parallel" and is + // keyed on its own id, so it never collapses with a batch finding. + // Fall back to type:file:line if an id is missing. const key = finding.id || `${finding.type}:${finding.affectedFile}:${finding.line ?? 0}`; const existing = deduped.get(key); if (!existing || finding.confidence > existing.confidence) { diff --git a/src/test/ast-inline-parallel.test.ts b/src/test/ast-inline-parallel.test.ts index addcd26..a5f14ac 100644 --- a/src/test/ast-inline-parallel.test.ts +++ b/src/test/ast-inline-parallel.test.ts @@ -16,12 +16,16 @@ function makeMatch(overrides: Partial): AstCallMatch { }; } +function findInline(findings: ReturnType) { + return findings.filter((f) => f.id.includes("inline_parallel")); +} + function run(name: string, fn: () => void): void { try { fn(); console.log(`PASS ${name}`); } catch (err) { console.error(`FAIL ${name}`); throw err; } } -run("inline-parallel: inlineParallelCapable fan-out → n/count suggestion, NOT batch-endpoint text", () => { +run("inline-parallel: inlineParallelCapable fan-out → unbatched_parallel type, NOT batch-endpoint text", () => { const match = makeMatch({ inlineParallelCapable: true, frequency: "parallel", loopContext: true, line: 4 }); const source = [ "import OpenAI from 'openai';", @@ -30,6 +34,7 @@ run("inline-parallel: inlineParallelCapable fan-out → n/count suggestion, NOT "const imgs = await Promise.all(prompts.map((p) => openai.images.generate({ prompt: p })));", ].join("\n"); const findings = detectBatchWaste([match], source, "/project/src/img.ts"); + const inline = findInline(findings); assert.ok( findings.some((f) => /n\/count parameter|count parameter|single call/i.test(f.description)), `expected an inline-parallel (n/count) suggestion, got: ${JSON.stringify(findings.map((f) => f.description))}` @@ -38,6 +43,8 @@ run("inline-parallel: inlineParallelCapable fan-out → n/count suggestion, NOT !findings.some((f) => /batch request|batch endpoint|consolidate into a single batch/i.test(f.description)), `must not emit batch-endpoint text: ${JSON.stringify(findings.map((f) => f.description))}` ); + assert.equal(inline.length, 1, `expected 1 inline_parallel finding, got ${inline.length}`); + assert.equal(inline[0].type, "unbatched_parallel", `expected type unbatched_parallel, got ${inline[0].type}`); }); run("inline-parallel: a real batchCapable API in the same shape still emits batch text", () => { @@ -51,7 +58,7 @@ run("inline-parallel: a real batchCapable API in the same shape still emits batc assert.ok(findings.some((f) => f.type === "batch" && /batch/i.test(f.description)), "real batch API should still get batch text"); }); -run("inline-parallel: Array.from({length:n}) idiom stays fully suppressed", () => { +run("inline-parallel: Array.from({length:n}) idiom now flagged as unbatched_parallel", () => { const match = makeMatch({ inlineParallelCapable: true, frequency: "parallel", loopContext: true, line: 3 }); const source = [ "import OpenAI from 'openai';", @@ -59,10 +66,12 @@ run("inline-parallel: Array.from({length:n}) idiom stays fully suppressed", () = "const imgs = await Promise.all(Array.from({ length: 4 }).map(() => openai.images.generate({ prompt: 'x' })));", ].join("\n"); const findings = detectBatchWaste([match], source, "/project/src/img.ts"); - assert.equal(findings.length, 0, `expected no findings for the Array.from idiom, got: ${JSON.stringify(findings.map((f) => f.description))}`); + const inline = findInline(findings); + assert.equal(inline.length, 1, `expected 1 inline_parallel finding for Array.from idiom, got: ${JSON.stringify(findings.map((f) => f.description))}`); + assert.equal(inline[0].type, "unbatched_parallel", `expected type unbatched_parallel, got ${inline[0].type}`); }); -run("inline-parallel: inlineParallelCapable in an unbounded for-loop → n/count suggestion fires", () => { +run("inline-parallel: inlineParallelCapable in an unbounded for-loop → unbatched_parallel type fires", () => { const match = makeMatch({ inlineParallelCapable: true, frequency: "unbounded-loop", loopContext: true, line: 4 }); const source = [ "import OpenAI from 'openai';", @@ -72,8 +81,23 @@ run("inline-parallel: inlineParallelCapable in an unbounded for-loop → n/count "}", ].join("\n"); const findings = detectBatchWaste([match], source, "/project/src/img.ts"); + const inline = findInline(findings); assert.ok( findings.some((f) => /n\/count parameter|count parameter|single call/i.test(f.description)), `expected an inline-parallel suggestion for the loop case, got: ${JSON.stringify(findings.map((f) => f.description))}` ); + assert.equal(inline.length, 1, `expected 1 inline_parallel finding for loop case, got ${inline.length}`); + assert.equal(inline[0].type, "unbatched_parallel", `expected type unbatched_parallel for loop case, got ${inline[0].type}`); +}); + +run("inline-parallel: inlineParallelCapable:false → zero inline findings (precision gate)", () => { + const match = makeMatch({ inlineParallelCapable: false, frequency: "parallel", loopContext: true, line: 3 }); + const source = [ + "import OpenAI from 'openai';", + "const openai = new OpenAI();", + "const imgs = await Promise.all(Array.from({ length: 5 }).map(() => openai.images.generate({ prompt: 'x' })));", + ].join("\n"); + const findings = detectBatchWaste([match], source, "/project/src/img.ts"); + const inline = findInline(findings); + assert.equal(inline.length, 0, `expected no inline_parallel findings when inlineParallelCapable:false, got: ${JSON.stringify(inline.map((f) => f.description))}`); }); diff --git a/src/test/local-waste-detector.test.ts b/src/test/local-waste-detector.test.ts index b1a4156..0983288 100644 --- a/src/test/local-waste-detector.test.ts +++ b/src/test/local-waste-detector.test.ts @@ -69,6 +69,19 @@ run("flags inline-parallel fanout for an n/count-capable endpoint (regex-only pa const inlineParallel = findings.find((finding) => finding.id.includes("inline_parallel")); assert.ok(inlineParallel, "expected an inline-parallel finding for images.generate fanout"); assert.match(inlineParallel?.description ?? "", /n\/count parameter/); + assert.equal(inlineParallel?.type, "unbatched_parallel", "inline-parallel finding must be unbatched_parallel"); +}); + +run("regex path flags Array.from bounded-replication fanout as unbatched_parallel (guard removal regression)", () => { + const text = [ + "async function generateVariants(prompt) {", + " return Promise.all(Array.from({ length: 4 }).map(() => openai.images.generate({ prompt })));", + "}", + ].join("\n"); + const findings = detectLocalWasteFindingsInText("src/lib/variants.ts", text); + const inlineParallel = findings.find((finding) => finding.id.includes("inline_parallel")); + assert.ok(inlineParallel, "expected an inline-parallel finding for Array.from fanout over images.generate"); + assert.equal(inlineParallel?.type, "unbatched_parallel", "inline-parallel finding must be unbatched_parallel"); }); run("#112: bare 'cache' in a comment does not suppress a cache finding", () => { diff --git a/webview/src/components/ResultsPage.tsx b/webview/src/components/ResultsPage.tsx index 166e2e7..e366dc1 100644 --- a/webview/src/components/ResultsPage.tsx +++ b/webview/src/components/ResultsPage.tsx @@ -20,6 +20,7 @@ const typeLabels: Record = { n_plus_one: "n+1", cache: "cache", batch: "batch", + unbatched_parallel: "unbatched parallel", redundancy: "redundancy", rate_limit: "rate-limit", concurrency_control: "concurrency", diff --git a/webview/src/types.ts b/webview/src/types.ts index a4499a8..58f517b 100644 --- a/webview/src/types.ts +++ b/webview/src/types.ts @@ -25,7 +25,7 @@ export type EndpointStatus = | "n_plus_one_risk" | "rate_limit_risk"; -export type SuggestionType = "cache" | "batch" | "redundancy" | "n_plus_one" | "rate_limit" | "concurrency_control"; +export type SuggestionType = "cache" | "batch" | "redundancy" | "n_plus_one" | "rate_limit" | "concurrency_control" | "unbatched_parallel"; export type Severity = "high" | "medium" | "low"; export interface EndpointRecord {