perf: optimize history endpoint to eliminate N+1 queries #5333
Conversation
- Batch load unique evals instead of loading same eval multiple times - Reduces database queries from N to unique_eval_count - Maintains exact functional equivalency and plugin counting logic - Typical performance improvement: 2-5x for history page load times The original implementation would call Eval.findById() + getTable() for every result row, even when multiple rows referenced the same eval. This optimization batches those calls to only load each unique eval once, then reuses the loaded data via Map lookup. Resolves performance bottleneck in /history endpoint for users with large numbers of evaluations.
|
⏩ No test execution environment matched (76a2079) View output ↗ |
TestGru AssignmentSummary
Files
Tip You can |
| } = result; | ||
| const eval_ = await Eval.findById(evalId); | ||
| invariant(eval_, `Eval with ID ${evalId} not found`); | ||
| const table = (await eval_.getTable()) || { body: [] }; |
There was a problem hiding this comment.
I'm surprised this didn't get cached
📝 WalkthroughWalkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/util/database.ts (1)
497-503: Cache key misses the description filter — returns stale/wrong cached results across different descriptionscacheKey doesn’t include description, but description participates in the WHERE clause. A request with the same limit/tag but a different description can incorrectly reuse cached data from a previous invocation.
Apply this diff:
- const cacheKey = `standalone_evals_${limit}_${tag?.key}_${tag?.value}`; + const cacheKey = `standalone_evals_${limit}_${tag?.key}_${tag?.value}_${description ?? ''}`;Also applies to: 602-604
🧹 Nitpick comments (2)
src/util/database.ts (2)
532-545: Optional: cap concurrent Eval.findById/getTable calls for very large result setsPromise.all on a large uniqueEvalIds can fan out a high number of DB/IO calls. If you expect spikes (e.g., limit >> 100), consider chunking (e.g., 16–32 concurrent) to smooth load. No need to change now if current limits are small/stable.
563-581: Optional: precompute pluginCounts once per eval to avoid O(prompts × rows) work per result rowpluginCounts is recomputed for each row and each prompt index. If table.body is large and results has multiple rows per eval (e.g., due to tags), consider precomputing counts per index once per eval and reusing them across rows.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/util/database.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/gh-cli-workflow.mdc)
Prefer not to introduce new TypeScript types; use existing interfaces whenever possible
Files:
src/util/database.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Follow consistent import order (Biome will handle import sorting)
Use consistent curly braces for all control statements
Prefer const over let; avoid var
Use object shorthand syntax whenever possible
Use async/await for asynchronous code
Use consistent error handling with proper type checks
Files:
src/util/database.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Redteam Custom Enterprise Server
- GitHub Check: Test on Node 18.x and windows-latest
- GitHub Check: Test on Node 18.x and macOS-latest
- GitHub Check: Test on Node 20.x and windows-latest
🔇 Additional comments (1)
src/util/database.ts (1)
532-545: Great N+1 fix: batched eval prefetch + map is clean and correctMoving to a unique evalId set + Promise.all and a Map lookup eliminates the N+1 pattern while keeping the rest of the flow intact. Nice work.
Apply auto-formatter to resolve style check failures in CI
Optimizes the /history endpoint by batching eval lookups to eliminate N+1 query patterns.
Problem
The getStandaloneEvals function was calling Eval.findById() + getTable() for every result row, even when multiple rows referenced the same eval. This created a performance bottleneck for users with many evaluations.
Solution
Performance Impact
Testing