⚡ Bolt: Optimize badge evaluation with LazyCriteriaCache - #105
⚡ Bolt: Optimize badge evaluation with LazyCriteriaCache#105projectamazonph wants to merge 4 commits into
Conversation
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. |
|
Warning Review limit reached
Next review available in: 52 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughChangesBadge query cache
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant evaluateBadges
participant LazyCriteriaCache
participant Database
evaluateBadges->>LazyCriteriaCache: request criteria data
LazyCriteriaCache->>Database: execute missing query
Database-->>LazyCriteriaCache: return result
LazyCriteriaCache-->>evaluateBadges: reuse cached result
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Pull request overview
This PR optimizes the badge evaluation engine by introducing a per-evaluation lazy cache of database query promises, reducing redundant DB roundtrips while iterating over many unearned published badges.
Changes:
- Added
LazyCriteriaCacheinsrc/lib/badges.tsto coalesce repeated reads (user record, lesson completion count, tool session count) within a singleevaluateBadges()run. - Updated
checkCriteria()paths to use the lazy cache instead of issuing repeated rawdb.*queries. - Added a unit test asserting
db.user.findUniqueis only called once when multiple criteria depend on the same user record.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/lib/badges.ts | Introduces LazyCriteriaCache and routes criteria evaluation DB reads through it to reduce redundant queries. |
| src/lib/tests/badges.test.ts | Adds a regression test ensuring repeated user lookups are coalesced during badge evaluation. |
| .jules/bolt.md | Documents the performance lesson and the caching approach used for loop-based evaluators. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| getToolSessionsCount(userId: string, scopeToolType?: string): Promise<number> { | ||
| const key = scopeToolType || '__ALL__'; | ||
| const existing = this.toolSessionsPromises.get(key); | ||
| if (existing) { | ||
| return existing; |
| { id: 'b1', title: 'Streak 1', criteria: JSON.stringify({ type: 'streak_days', threshold: 7 }), xpReward: 30, description: '', icon: '', tier: 'SILVER', isPublished: true, deletedAt: null }, | ||
| { id: 'b2', title: 'XP 1', criteria: JSON.stringify({ type: 'xp_threshold', threshold: 100 }), xpReward: 50, description: '', icon: '', tier: 'BRONZE', isPublished: true, deletedAt: null }, |
Co-authored-by: projectamazonph <286085559+projectamazonph@users.noreply.github.com>
Co-authored-by: projectamazonph <286085559+projectamazonph@users.noreply.github.com>
![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 @.jules/bolt.md:
- Around line 7-9: Update the “Redundant N+1 DB Queries in Loop-Based
Evaluators” entry to state that caching combines repeated queries, while
tool-session queries remain O(R) in the worst case when each rule has a distinct
scope.toolType. Replace “query promise” with a plain-language definition such as
a pending database request, and define “database roundtrip” as one request to
the database; do not claim all roundtrips become O(1).
In `@src/lib/badges.ts`:
- Around line 60-85: Update getToolSessionsCount and the toolSessionsPromises
map to use undefined as the unscoped cache key instead of the "__ALL__" string,
while preserving distinct string keys for every scoped tool type including
"__ALL__". Add a regression test that evaluates unscoped and "__ALL__"-scoped
queries and verifies each uses its own cached result.
🪄 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: ca38a3a0-8f61-4f7e-b98e-85d25a08fc39
📒 Files selected for processing (4)
.jules/bolt.mdpackage.jsonsrc/lib/__tests__/badges.test.tssrc/lib/badges.ts
| ## 2026-07-17 - [Redundant N+1 DB Queries in Loop-Based Evaluators] | ||
| **Learning:** Evaluators checking multiple rules (such as checkCriteria in the badge engine) can generate redundant database queries for identical user records or resource aggregates when looping over each rule. | ||
| **Action:** Use a transient, local `LazyCriteriaCache` (caching query promises rather than resolved values) during the evaluation lifecycle. This coalesces identical database queries into a single database call, safely changing database roundtrips from O(R) to O(1) where R is the number of rules. |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
Correct the query-complexity claim.
Tool-session counts use one query for each distinct scope.toolType. The query count is O(R) in the worst case when every rule has a different scope. The entry must not state that all database roundtrips become O(1).
Use plain language. Define technical terms such as “query promise” and “database roundtrip.”
Proposed fix
-**Action:** Use a transient, local `LazyCriteriaCache` (caching query promises rather than resolved values) during the evaluation lifecycle. This coalesces identical database queries into a single database call, safely changing database roundtrips from O(R) to O(1) where R is the number of rules.
+**Action:** Create one local `LazyCriteriaCache` for each evaluation. Store a database request when it starts, then reuse it when another rule needs the same data. Lesson counts and user data use one request each. Tool-session counts use one request for each different tool type.As per coding guidelines, use direct, plain-spoken language and define jargon.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## 2026-07-17 - [Redundant N+1 DB Queries in Loop-Based Evaluators] | |
| **Learning:** Evaluators checking multiple rules (such as checkCriteria in the badge engine) can generate redundant database queries for identical user records or resource aggregates when looping over each rule. | |
| **Action:** Use a transient, local `LazyCriteriaCache` (caching query promises rather than resolved values) during the evaluation lifecycle. This coalesces identical database queries into a single database call, safely changing database roundtrips from O(R) to O(1) where R is the number of rules. | |
| ## 2026-07-17 - [Redundant N+1 DB Queries in Loop-Based Evaluators] | |
| **Learning:** Evaluators checking multiple rules (such as checkCriteria in the badge engine) can generate redundant database queries for identical user records or resource aggregates when looping over each rule. | |
| **Action:** Create one local `LazyCriteriaCache` for each evaluation. Store a database request when it starts, then reuse it when another rule needs the same data. Lesson counts and user data use one request each. Tool-session counts use one request for each different tool type. |
🤖 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 @.jules/bolt.md around lines 7 - 9, Update the “Redundant N+1 DB Queries in
Loop-Based Evaluators” entry to state that caching combines repeated queries,
while tool-session queries remain O(R) in the worst case when each rule has a
distinct scope.toolType. Replace “query promise” with a plain-language
definition such as a pending database request, and define “database roundtrip”
as one request to the database; do not claim all roundtrips become O(1).
Source: Coding guidelines
| private toolSessionsPromises = new Map<string, Promise<number>>(); | ||
| private userPromise: Promise<{ streakDays: number; xp: number } | null> | null = null; | ||
|
|
||
| getCompletedCount(userId: string): Promise<number> { | ||
| if (!this.completedCountPromise) { | ||
| this.completedCountPromise = db.lessonProgress.count({ | ||
| where: { userId, status: 'COMPLETED' }, | ||
| }); | ||
| } | ||
| return this.completedCountPromise!; | ||
| } | ||
|
|
||
| getToolSessionsCount(userId: string, scopeToolType?: string): Promise<number> { | ||
| const key = scopeToolType || '__ALL__'; | ||
| const existing = this.toolSessionsPromises.get(key); | ||
| if (existing) { | ||
| return existing; | ||
| } | ||
| const p = db.toolSession.count({ | ||
| where: { | ||
| userId, | ||
| status: 'GRADED', | ||
| ...(scopeToolType ? { toolType: scopeToolType } : {}), | ||
| }, | ||
| }); | ||
| this.toolSessionsPromises.set(key, p); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep the unscoped cache key separate from all tool types.
scope.toolType can equal "__ALL__" because it is a string. An unscoped rule and a rule scoped to "__ALL__" then share one cache entry. The first query controls both results. This can create or omit persisted badge awards and XP.
Use undefined as the unscoped Map key. Add a regression test that evaluates both scopes.
Proposed fix
- private toolSessionsPromises = new Map<string, Promise<number>>();
+ private toolSessionsPromises = new Map<string | undefined, Promise<number>>();
...
- const key = scopeToolType || '__ALL__';
+ const key = scopeToolType;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private toolSessionsPromises = new Map<string, Promise<number>>(); | |
| private userPromise: Promise<{ streakDays: number; xp: number } | null> | null = null; | |
| getCompletedCount(userId: string): Promise<number> { | |
| if (!this.completedCountPromise) { | |
| this.completedCountPromise = db.lessonProgress.count({ | |
| where: { userId, status: 'COMPLETED' }, | |
| }); | |
| } | |
| return this.completedCountPromise!; | |
| } | |
| getToolSessionsCount(userId: string, scopeToolType?: string): Promise<number> { | |
| const key = scopeToolType || '__ALL__'; | |
| const existing = this.toolSessionsPromises.get(key); | |
| if (existing) { | |
| return existing; | |
| } | |
| const p = db.toolSession.count({ | |
| where: { | |
| userId, | |
| status: 'GRADED', | |
| ...(scopeToolType ? { toolType: scopeToolType } : {}), | |
| }, | |
| }); | |
| this.toolSessionsPromises.set(key, p); | |
| private toolSessionsPromises = new Map<string | undefined, Promise<number>>(); | |
| private userPromise: Promise<{ streakDays: number; xp: number } | null> | null = null; | |
| getCompletedCount(userId: string): Promise<number> { | |
| if (!this.completedCountPromise) { | |
| this.completedCountPromise = db.lessonProgress.count({ | |
| where: { userId, status: 'COMPLETED' }, | |
| }); | |
| } | |
| return this.completedCountPromise!; | |
| } | |
| getToolSessionsCount(userId: string, scopeToolType?: string): Promise<number> { | |
| const key = scopeToolType; | |
| const existing = this.toolSessionsPromises.get(key); | |
| if (existing) { | |
| return existing; | |
| } | |
| const p = db.toolSession.count({ | |
| where: { | |
| userId, | |
| status: 'GRADED', | |
| ...(scopeToolType ? { toolType: scopeToolType } : {}), | |
| }, | |
| }); | |
| this.toolSessionsPromises.set(key, p); |
🤖 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/lib/badges.ts` around lines 60 - 85, Update getToolSessionsCount and the
toolSessionsPromises map to use undefined as the unscoped cache key instead of
the "__ALL__" string, while preserving distinct string keys for every scoped
tool type including "__ALL__". Add a regression test that evaluates unscoped and
"__ALL__"-scoped queries and verifies each uses its own cached result.
… pnpm setup failure Co-authored-by: projectamazonph <286085559+projectamazonph@users.noreply.github.com>
⚡ Bolt: Optimize badge evaluation with LazyCriteriaCache
💡 What
Introduced a transient
LazyCriteriaCacheinsidesrc/lib/badges.tsto cache database query promises during a single badge evaluation cycle. Updated thecheckCriteriaevaluation pathways (lesson completion, tool session count, and user streak/XP metrics) to retrieve counts and user records from this lazy cache rather than firing raw database calls on every unearned badge.🎯 Why
When evaluating badges upon lesson/quiz/tool completions or user sign-in, the system iterates through all published unearned badges. Under the old implementation, for every unearned badge evaluated, the system queried the database redundantly (e.g., executing N database roundtrips to retrieve the same user profile information, or multiple lesson/session counts). This O(R) query complexity (where R is the number of rules/badges) caused unnecessary database load.
📊 Impact
streakDays/xpor completed lesson count only once per evaluation cycle).🔬 Measurement
Verified via targeted unit tests in
src/lib/__tests__/badges.test.tschecking that database queries (specificallydb.user.findUnique) are called exactly once during evaluations with multiple unearned badge criteria. All existing and new tests pass cleanly.PR created automatically by Jules for task 9680293478510324938 started by @projectamazonph
Summary by CodeRabbit
Performance
Bug Fixes
Tests
Chores