-
Notifications
You must be signed in to change notification settings - Fork 0
⚡ Bolt: collapse redundant DB queries in badge evaluation #119
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
cf22832
94e2a58
ad3c4c4
fde1912
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,6 +51,54 @@ export interface BadgeEvaluationResult { | |
| totalXpGained: number; | ||
| } | ||
|
|
||
| /** | ||
| * Transient cache to collapse duplicate DB queries during badge evaluation lifecycle. | ||
| * Holds query promises rather than resolved values to optimize concurrent checks. | ||
| */ | ||
| class EvaluationCache { | ||
| private userId: string; | ||
| private lessonProgressCountPromise: Promise<number> | null = null; | ||
| private toolSessionCountPromises = new Map<string | undefined, Promise<number>>(); | ||
| private userPromise: Promise<{ streakDays: number; xp: number } | null> | null = null; | ||
|
|
||
| constructor(userId: string) { | ||
| this.userId = userId; | ||
| } | ||
|
|
||
| getLessonProgressCount(): Promise<number> { | ||
| if (!this.lessonProgressCountPromise) { | ||
| this.lessonProgressCountPromise = db.lessonProgress.count({ | ||
| where: { userId: this.userId, status: 'COMPLETED' }, | ||
| }); | ||
| } | ||
| return this.lessonProgressCountPromise; | ||
| } | ||
|
|
||
| getToolSessionCount(scopeToolType?: string): Promise<number> { | ||
| if (!this.toolSessionCountPromises.has(scopeToolType)) { | ||
| const promise = db.toolSession.count({ | ||
| where: { | ||
| userId: this.userId, | ||
| status: 'GRADED', | ||
| ...(scopeToolType ? { toolType: scopeToolType } : {}), | ||
| }, | ||
| }); | ||
| this.toolSessionCountPromises.set(scopeToolType, promise); | ||
| } | ||
| return this.toolSessionCountPromises.get(scopeToolType)!; | ||
| } | ||
|
|
||
| getUser(): Promise<{ streakDays: number; xp: number } | null> { | ||
| if (!this.userPromise) { | ||
| this.userPromise = db.user.findUnique({ | ||
| where: { id: this.userId }, | ||
| select: { streakDays: true, xp: true }, | ||
| }) as Promise<{ streakDays: number; xp: number } | null>; | ||
| } | ||
| return this.userPromise; | ||
| } | ||
| } | ||
|
|
||
|
Comment on lines
+54
to
+101
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win Add regression tests for This PR does not include tests for the new cache behavior. The supplied XP-threshold test checks one award result, but it does not verify query de-duplication. Add tests that evaluate multiple matching criteria and assert one call for repeated lesson-progress and user reads. Also test repeated and distinct As per coding guidelines, “New features must include tests; admin and business-layer features require tests.” 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| /** | ||
| * Evaluate all badges for a user against the current database state. Award any | ||
| * newly-earned ones. Idempotent — re-running with no new events returns | ||
|
|
@@ -95,6 +143,8 @@ export async function evaluateBadges( | |
| xpReward: number; | ||
| }> = []; | ||
|
|
||
| const cache = new EvaluationCache(userId); | ||
|
|
||
| for (const badge of published) { | ||
| if (alreadyAwardedSet.has(badge.id)) continue; | ||
|
|
||
|
|
@@ -106,7 +156,7 @@ export async function evaluateBadges( | |
| continue; | ||
| } | ||
|
|
||
| const qualifies = await checkCriteria(userId, criteria, event); | ||
| const qualifies = await checkCriteria(userId, criteria, event, cache); | ||
| if (qualifies) earnedNow.push(badge); | ||
| } | ||
|
|
||
|
|
@@ -146,12 +196,11 @@ async function checkCriteria( | |
| userId: string, | ||
| criteria: BadgeCriteria, | ||
| event: BadgeTrigger, | ||
| cache: EvaluationCache, | ||
| ): Promise<boolean> { | ||
| switch (criteria.type) { | ||
| case 'module_complete': { | ||
| const completedCount = await db.lessonProgress.count({ | ||
| where: { userId, status: 'COMPLETED' }, | ||
| }); | ||
| const completedCount = await cache.getLessonProgressCount(); | ||
| // Treat each completed lesson as progress toward module_complete; the | ||
| // seed threshold is 1 so this triggers after the first lesson. | ||
| return completedCount >= criteria.threshold; | ||
|
|
@@ -165,30 +214,18 @@ async function checkCriteria( | |
|
|
||
| case 'tool_sessions': { | ||
| const scopeToolType = criteria.scope?.toolType; | ||
| const count = await db.toolSession.count({ | ||
| where: { | ||
| userId, | ||
| status: 'GRADED', | ||
| ...(scopeToolType ? { toolType: scopeToolType } : {}), | ||
| }, | ||
| }); | ||
| const count = await cache.getToolSessionCount(scopeToolType); | ||
| return count >= criteria.threshold; | ||
| } | ||
|
|
||
| case 'streak_days': { | ||
| const user = await db.user.findUnique({ | ||
| where: { id: userId }, | ||
| select: { streakDays: true }, | ||
| }); | ||
| const user = await cache.getUser(); | ||
| if (!user) return false; | ||
| return user.streakDays >= criteria.threshold; | ||
| } | ||
|
|
||
| case 'xp_threshold': { | ||
| const user = await db.user.findUnique({ | ||
| where: { id: userId }, | ||
| select: { xp: true }, | ||
| }); | ||
| const user = await cache.getUser(); | ||
| if (!user) return false; | ||
| return user.xp >= criteria.threshold; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the query-complexity claim.
getToolSessionCountcreates one query per distinctscopeToolType. The total query count scales with unique tool-type scopes.State
O(1 + u), whereuis the number of distinct tool-type scopes, instead of O(1) for all rules.🤖 Prompt for AI Agents