feat(wave4): recover unbatched_parallel FN; defer Python batch FN (#117) - #132
Conversation
|
Warning Review limit reached
More reviews will be available in 55 minutes and 26 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR implements Wave 4 recall recovery by introducing a new ChangesUnbatched Parallel Finding Type Rollout
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docs/superpowers/specs/2026-05-30-wave4-recall-recovery-design.md (1)
29-46: ⚡ Quick winTrack A design describes work that was reverted.
Similar to the implementation plan, this design spec describes Track A as if it will be implemented, but the PR summary states this work was "implemented then reverted" due to corpus-labeling inconsistencies that produced 6 false positives.
Consider adding a note at the beginning of the Track A section indicating the outcome, or moving this section to an "Attempted approaches" appendix.
🤖 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/specs/2026-05-30-wave4-recall-recovery-design.md` around lines 29 - 46, The Track A section for the Python cross-function batch FN in docs/superpowers/specs/2026-05-30-wave4-recall-recovery-design.md reports work that was implemented then reverted; update the Track A text (the description around the detectSequentialBatching design in src/scanner/python-waste-detector.ts) to clearly state the outcome by adding a short note at the start of the Track A subsection indicating the implementation was reverted due to corpus-labeling inconsistencies producing false positives, or move the entire Track A subsection into an "Attempted approaches" appendix and add a one-line pointer from the main section; ensure the note references the same identifiers (Track A, detectSequentialBatching, and python-waste-detector.ts) so readers can correlate the design and its reverted status.docs/superpowers/plans/2026-05-30-wave4-recall-recovery.md (1)
42-192: ⚡ Quick winTrack A implementation was reverted but the plan document still describes it as active.
The PR summary states that the Python cross-function batch FN recovery was "implemented then reverted" due to 6 false positives caused by corpus-labeling inconsistencies. However, this plan document still describes Track A (lines 42-192) as if it will be implemented, including detailed steps, test cases, and commit instructions.
Consider adding a note at the beginning of the Track A section indicating that this track was attempted but reverted, with a reference to the outcome documented in
docs/accuracy/findings.mdor the PR summary. This will prevent future readers from attempting to execute these steps or being confused about why the code doesn't match the plan.📝 Suggested documentation clarification
Add a note at the start of the Track A section:
## Track A — Python cross-function batch FN +> **OUTCOME (2026-05-30):** This track was implemented and then reverted. The cross-function `(provider, methodChain)` batching pass recovered the target FN but produced 6 FPs on structurally-identical unlabeled sibling files. The issue is a corpus-labeling inconsistency, not recoverable by detector code alone. See `docs/accuracy/findings.md` C1 calibration table for details. This section is retained for reference but should not be re-executed. + **File:** `src/scanner/python-waste-detector.ts`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/plans/2026-05-30-wave4-recall-recovery.md` around lines 42 - 192, The Track A section currently describes implementing cross-function batching (steps referencing detectSequentialBatching, python-waste-detector.ts, and tests in src/test/python-waste-detector.test.ts) as active work but that implementation was reverted; update the Track A header so it clearly indicates the cross-function batch FN was attempted and reverted due to false positives, add a short note at the start of the Track A block referencing the PR summary and docs/accuracy/findings.md for details, and mark the plan steps as historical (e.g., prepend "REVERTED:" or "ARCHIVED:" and a one-line rationale) so future readers won't try to execute detectSequentialBatching changes or the test additions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/specs/2026-05-30-wave4-recall-recovery-design.md`:
- Line 58: The spec currently lists the savings multiplier for
unbatched_parallel as 0.18 but the implementation and plan use 0.20; update the
spec value to 0.20 to match the implementation by changing the
unbatched_parallel entry in the wave4-recall-recovery-design document to 0.20 so
it aligns with BASE_MULTIPLIERS in src/scan-results.ts (and the plan doc).
---
Nitpick comments:
In `@docs/superpowers/plans/2026-05-30-wave4-recall-recovery.md`:
- Around line 42-192: The Track A section currently describes implementing
cross-function batching (steps referencing detectSequentialBatching,
python-waste-detector.ts, and tests in src/test/python-waste-detector.test.ts)
as active work but that implementation was reverted; update the Track A header
so it clearly indicates the cross-function batch FN was attempted and reverted
due to false positives, add a short note at the start of the Track A block
referencing the PR summary and docs/accuracy/findings.md for details, and mark
the plan steps as historical (e.g., prepend "REVERTED:" or "ARCHIVED:" and a
one-line rationale) so future readers won't try to execute
detectSequentialBatching changes or the test additions.
In `@docs/superpowers/specs/2026-05-30-wave4-recall-recovery-design.md`:
- Around line 29-46: The Track A section for the Python cross-function batch FN
in docs/superpowers/specs/2026-05-30-wave4-recall-recovery-design.md reports
work that was implemented then reverted; update the Track A text (the
description around the detectSequentialBatching design in
src/scanner/python-waste-detector.ts) to clearly state the outcome by adding a
short note at the start of the Track A subsection indicating the implementation
was reverted due to corpus-labeling inconsistencies producing false positives,
or move the entire Track A subsection into an "Attempted approaches" appendix
and add a one-line pointer from the main section; ensure the note references the
same identifiers (Track A, detectSequentialBatching, and
python-waste-detector.ts) so readers can correlate the design and its reverted
status.
🪄 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: 34a55e25-cf70-4a67-8c76-78ac6e12b51c
📒 Files selected for processing (18)
benchmark/baseline.jsondashboard/src/lib/types.tsdashboard/src/pages/Suggestions.tsxdocs/accuracy/findings.mddocs/superpowers/plans/2026-05-30-wave4-recall-recovery.mddocs/superpowers/specs/2026-05-30-wave4-recall-recovery-design.mdpackage.jsonsrc/analysis/types.tssrc/ast/waste/batch-detector.tssrc/intelligence/INTEL_CONTEXT.mdsrc/intelligence/compression.tssrc/intelligence/scorer.tssrc/scan-results.tssrc/scanner/local-waste-detector.tssrc/test/ast-inline-parallel.test.tssrc/test/local-waste-detector.test.tswebview/src/components/ResultsPage.tsxwebview/src/types.ts
…overy # Conflicts: # package.json
…e.css The `<<<<<<< HEAD` / `=======` markers (empty HEAD side, no closing marker) had been committed to theme.css since 7de2c63 and sat identically on every branch. They predate this PR; removing them keeps the pulseGlow @Keyframes intact. Dashboard build verified clean.
The spec listed unbatched_parallel's BASE_MULTIPLIERS value as 0.18, but both the plan and src/scan-results.ts ship 0.20 (matching batch, which is also 0.20). Correct the stale spec value. Addresses CodeRabbit review.
2 participants
Summary
Wave 4 recall-recovery (#117) targeted the two C1 false negatives intentionally deferred by PR #111. One of two recovered.
Closes #117 — its remaining scope (the deferred Python
batchFN below) is now tracked in #134.unbatched_parallelFN — recovered. Added a dedicatedunbatched_parallelSuggestionType, threaded it through every consumer (3 union decls,SAVINGS_MULTIPLIERS,FINDING_TITLE/LABEL_BY_TYPE, webview/dashboard label+icon maps,COST_LEAK_FINDING_TYPES). Both the AST detector (detectInlineParallel) and the regex fallback (detectInlineParallelFinding) now emit it, and theBOUNDED_REPLICATIONguard was removed from the inline-parallel path — theinlineParallelCapableflag is the precision control (kept indetectBatch, where bounded replication is still legitimately suppressed).batchFN — deferred (reverted). A cross-function(provider, methodChain)batching pass was implemented but fired on structurally-identical sibling helper files (openai_helper.py,cohere_helper.py) that the corpus labels inconsistently — 6 FPs, droppingbatchprecision to 14.3%. No AST signal distinguishes the labeled-TP case from the unlabeled-but-identical ones, so this is a corpus-labeling inconsistency, not a detector gap. The pass was reverted;python-waste-detector.tsis byte-identical tomain. Follow-up tracked in [benchmark corpus] Resolve Python cross-functionbatchFN: relabel sibling helpers or accept as unrecoverable #134 (corpus relabel-or-accept decision onextension-benchmark).Also kept (independent fix):
package.jsonnow wirespython-waste-detector.test.jsintotest:scanner— it was compiled but never executed in CI.Merge maintenance
main(the B2 callTrace feature, feat(wave2): B2 dual locations for cross-file resolved calls (#81) #133) into this branch. Sole conflict was a both-sides-appended collision on thetest:scannerline inpackage.json, resolved as a union (kept bothpython-waste-detector.test.jsandcall-trace.test.js). Verified:build:ext,build:webview,test:scanner(fail 0, incl. the merged-in B2callTracetests).dashboard/src/styles/theme.css(empty HEAD side, no closing marker) that had sat onmainsince7de2c63— unrelated to this PR but cleaned up here. Dashboard build verified clean.Benchmark result
unbatched_parallelbatchNo metric dropped >1pp.
baseline.jsonregenerated to lock the recovery.Test Plan
npm run test:scanner— 415 pass / 0 failnpm run build— clean (extension + webview + dashboard)npm run benchmark— exit 0, gate green,unbatched_parallelTP1/FP0/FN0Docs
Spec:
docs/superpowers/specs/2026-05-30-wave4-recall-recovery-design.mdPlan:
docs/superpowers/plans/2026-05-30-wave4-recall-recovery.mdC1 calibration table updated in
docs/accuracy/findings.md.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
[[Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/recost-dev/extension/pull/132?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)