added benchmarks#79
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a local benchmark harness ( ChangesBenchmark harness and orchestration
Sequence DiagramsequenceDiagram
participant Dev as CLI / Developer
participant Bench as Benchmark Script
participant MCP as MCP Tool Registry / guided_query
participant Pine as Pinecone Mock Client
participant FS as Filesystem
Dev->>Bench: run `npm run benchmark`
Bench->>Pine: install bench Pinecone test double (search, rerank, namespaces)
Bench->>MCP: register tools / capture `guided_query` handler
Bench->>Bench: warmup scenarios
loop for each scenario & iteration
Bench->>MCP: invoke guided_query (when applicable)
MCP->>Pine: query / rerank / list namespaces
Pine-->>MCP: synthetic responses
MCP-->>Bench: tool response
end
Bench->>FS: write `benchmarks/baseline.json`
Bench-->>Dev: print ASCII table with p50/p95/p99/min/max
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #79 +/- ##
=======================================
Coverage ? 66.01%
=======================================
Files ? 33
Lines ? 1121
Branches ? 359
=======================================
Hits ? 740
Misses ? 381
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
benchmarks/latency.ts (1)
142-145: ⚡ Quick winReturn fresh hit objects per call to avoid benchmark state bleed.
Line 142–Line 145 currently return shared arrays/objects. If downstream code mutates results, later iterations can measure mutated state instead of a clean run.
Proposed fix
function buildQueryBenchClient(): PineconeClientBenchDouble { const denseHits = syntheticHits('dense', TOP_K, 0.95); const sparseHits = syntheticHits('sparse', TOP_K, 0.9); + const cloneHits = (hits: PineconeHit[]): PineconeHit[] => + hits.map((h) => ({ + ...h, + fields: { ...h.fields }, + })); const denseIndexRef = {} as SearchableIndex; const sparseIndexRef = {} as SearchableIndex; @@ client.searchIndex = async (index) => { - if (index === denseIndexRef) return denseHits; - if (index === sparseIndexRef) return sparseHits; + if (index === denseIndexRef) return cloneHits(denseHits); + if (index === sparseIndexRef) return cloneHits(sparseHits); return []; };🤖 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 `@benchmarks/latency.ts` around lines 142 - 145, The searchIndex stub returns shared arrays/objects (denseHits/sparseHits) which can be mutated across iterations; update client.searchIndex so it returns fresh copies per call (e.g., create a new array and clone each hit object) for the denseIndexRef and sparseIndexRef branches and return a new empty array for the default path to avoid benchmark state bleed; locate the client.searchIndex closure and replace the direct returns of denseHits/sparseHits with code that maps/duplicates each hit (referencing denseIndexRef, sparseIndexRef, denseHits, sparseHits, and client.searchIndex).
🤖 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 `@benchmarks/latency.ts`:
- Around line 113-120: The table is converting numeric latency values via
String(...) which discards trailing zeros; update the rendering inside the
line([...]) call so r.p50, r.p95, r.p99, r.min, and r.max are formatted with
four decimal places (e.g. use Number(...).toFixed(4) or ensure the values are
numbers and call toFixed(4)) before converting to strings; keep the name slicing
(r.name.slice(...)) unchanged and apply this formatting only to the numeric
latency fields so the table consistently shows four decimal places.
---
Nitpick comments:
In `@benchmarks/latency.ts`:
- Around line 142-145: The searchIndex stub returns shared arrays/objects
(denseHits/sparseHits) which can be mutated across iterations; update
client.searchIndex so it returns fresh copies per call (e.g., create a new array
and clone each hit object) for the denseIndexRef and sparseIndexRef branches and
return a new empty array for the default path to avoid benchmark state bleed;
locate the client.searchIndex closure and replace the direct returns of
denseHits/sparseHits with code that maps/duplicates each hit (referencing
denseIndexRef, sparseIndexRef, denseHits, sparseHits, and client.searchIndex).
🪄 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
Run ID: 08440cb8-8b58-4953-a02f-4fb5c85e7f4e
📒 Files selected for processing (3)
README.mdbenchmarks/latency.tspackage.json
Pull Request
Added
benchmarks/latency.ts
Updated
package.json
Added benchmarks/baseline.json
Updated README.md
npm run typecheck, npm run lint, npm run build, and npm test all pass. npm run ci still fails on npm run format:check because many existing src/**/*.ts files already fail Prettier in this repo; that was not changed as part of this work.
close #74
Summary by CodeRabbit
Documentation
New Features
Chores