⚡ Bolt: Optimize listing audit engine findings lookup#60
⚡ Bolt: Optimize listing audit engine findings lookup#60projectamazonph wants to merge 1 commit into
Conversation
- Convert reference findings array to Map in findingsAccuracy and severityCalibration to avoid O(N*M) nested scans. - Implemented comprehensive engine.test.ts suite for listing audit grading. Co-authored-by: projectamazonph <286085559+projectamazonph@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughThe listing-audit engine now indexes reference findings by field for accuracy and severity checks. New Vitest tests cover successful and failed grading scenarios and verify generated findings include title and bullets. ChangesListing audit engine
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 2
🤖 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/engine/listing-audit/engine.test.ts`:
- Around line 28-35: Replace all em dashes in the affected test fixture strings,
including the product title and bullets around the referenced sections, with
commas or parentheses while preserving the fixture’s meaning and assertions.
In `@src/engine/listing-audit/engine.ts`:
- Around line 86-91: Update the reference-finding indexes in the main audit
matching logic and severityCalibration so duplicate ListingAuditFinding entries
for the same field are preserved, matching the previous ref.find behavior by
retaining the first finding or indexing fields as arrays. Add a regression case
in engine.test.ts covering duplicate reviews findings produced by
generateAutoFindings and verifying accuracy and severity results.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e93c545f-c232-419e-9c5f-a48f41f6cc06
📒 Files selected for processing (2)
src/engine/listing-audit/engine.test.tssrc/engine/listing-audit/engine.ts
| title: 'Bamboo Cutting Board with Deep Juice Groove — Eco-Friendly Kitchen Chopping Board for Vegetables, Meat, Cheese | Large 16x12 inch with Non-Slip Feet | Durable, Dishwasher Safe', | ||
| bullets: [ | ||
| 'DEEP JUICE GROOVE — Catches liquids from meat, fruit, and vegetables. No more countertop spills.', | ||
| 'ECO-FRIENDLY BAMBOO — Sustainably sourced, naturally antibacterial. Lasts longer than plastic.', | ||
| 'LARGE 16x12 INCH — Fits a whole meal prep. Reversible design doubles your workspace.', | ||
| 'NON-SLIP FEET — Stays put on the counter. No more chasing the board while chopping.', | ||
| 'DISHWASHER SAFE — Easy cleanup. Oil monthly with food-grade mineral oil for best results.', | ||
| ], |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace em dashes in the test fixtures.
These new title and bullet strings contain em dashes. Replace them with commas or parentheses.
As per coding guidelines: “Do not use emojis in code or commit messages, and do not use em-dashes. Use periods, commas, or parentheses instead.”
Also applies to: 63-70
🤖 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/engine/listing-audit/engine.test.ts` around lines 28 - 35, Replace all em
dashes in the affected test fixture strings, including the product title and
bullets around the referenced sections, with commas or parentheses while
preserving the fixture’s meaning and assertions.
Source: Coding guidelines
| // Bolt optimization: Map reference findings by field to replace O(N*M) nested lookups with O(1) lookups | ||
| const refMap = new Map(ref.map((f) => [f.field, f])); | ||
|
|
||
| let truePositives = 0; | ||
| for (const sf of student) { | ||
| const match = ref.find((rf) => rf.field === sf.field); | ||
| const match = refMap.get(sf.field); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve duplicate reference findings when indexing.
ListingAuditFinding.field is not unique, and generateAutoFindings can produce two reviews findings when both review count and rating are below their thresholds. new Map(ref.map(...)) keeps only the last entry, changing the previous ref.find(...) behavior and potentially producing incorrect accuracy and severity scores.
Retain the first finding per field or index each field to an array, and add a regression case in src/engine/listing-audit/engine.test.ts.
Proposed fix
- const refMap = new Map(ref.map((f) => [f.field, f]));
+ const refMap = new Map<ListingAuditFinding['field'], ListingAuditFinding>();
+ for (const finding of ref) {
+ if (!refMap.has(finding.field)) {
+ refMap.set(finding.field, finding);
+ }
+ }Apply the same indexing logic in severityCalibration.
Also applies to: 110-116
🤖 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/engine/listing-audit/engine.ts` around lines 86 - 91, Update the
reference-finding indexes in the main audit matching logic and
severityCalibration so duplicate ListingAuditFinding entries for the same field
are preserved, matching the previous ref.find behavior by retaining the first
finding or indexing fields as arrays. Add a regression case in engine.test.ts
covering duplicate reviews findings produced by generateAutoFindings and
verifying accuracy and severity results.
Understood. Acknowledging that this work is duplicate/obsolete in favor of #43, and stopping work on this task. |
1 participant
This PR optimizes the listing audit scoring engine's comparison logic.
💡 What: Changed the nested lookup pattern from O(NM) linear search (
array.find()) to O(N + M) using Map-based indexing.🎯 Why: In interactive scenarios, comparing student findings with scenario reference findings iteratively via inner linear lookups is highly inefficient. Mapping the reference items by field key once allows O(1) constant-time lookup.
📊 Impact: Eliminates redundant loops, reducing complexity from O(NM) to O(N + M) and making the grading logic extremely fast.
🔬 Measurement: Verified with a newly implemented unit test suite
src/engine/listing-audit/engine.test.tsshowing all 3 tests passing.PR created automatically by Jules for task 14346817668686831647 started by @projectamazonph
Summary by CodeRabbit
Bug Fixes
Tests