Add precomputed user statistics with delta-based updates - #184
Conversation
…ge with delta-based user stats (#182) - Convert Cloud Functions from plain JS to TypeScript with proper Nx project (cloud-functions) - Add esbuild build, Jest tests, and ESLint integration - Define UserStats model with emptyUserStats factory in @pu-stats/models - Implement pure delta-calculation logic (applyDelta) for create/update/delete - Add updateUserStatsOnPushupWrite Cloud Function trigger - Add Firestore security rules for userStats/{userId} (owner read, admin SDK write) - Update firebase.json to deploy from build output (dist/cloud-functions) - Add cloud-functions scope to enforce-module-boundaries ESLint rules - 32 unit tests covering berlinParts, periodKeys, streaks, heatmap, bestDay/bestEntry https://claude.ai/code/session_01WUgLPxZ66o25ij4BYjqC9H
- Add rebuildFromEntries pure function for full-scan backfill (4 new tests)
- Add rebuildUserStats admin-callable Cloud Function (single user or all)
- Add UserStatsApiService in @pu-stats/data-access for reading userStats/{userId}
- Export UserStatsApiService from library barrel
https://claude.ai/code/session_01WUgLPxZ66o25ij4BYjqC9H
- Add totalDays field to UserStats model for allTimeAvg calculation - DashboardStore: prefer precomputed UserStats for todayTotal, weekReps, monthReps, and currentStreak with client-side fallback - AnalysisStore: expose heatmapData from precomputed UserStats - Update component tests to provide UserStatsApiService mock - rebuildFromEntries now computes totalDays from unique entry dates https://claude.ai/code/session_01WUgLPxZ66o25ij4BYjqC9H
📝 WalkthroughWalkthroughConverts Cloud Functions to Node 24 TypeScript/esbuild, adds transactional delta-backed server-side Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client as Web Client
participant Firestore as Firestore DB
participant CFn as Cloud Functions
User->>Client: Create/Update pushup
Client->>Firestore: Write pushups/{pushupId}
Firestore->>CFn: Trigger onDocumentWritten(pushups/{pushupId})
CFn->>Firestore: Read userStats/{userId} (transaction)
Firestore-->>CFn: existing userStats or null
CFn->>CFn: applyDelta (period keys, heatmap, streak, totals)
CFn->>Firestore: Commit transactional write to userStats/{userId}
Client->>Firestore: Read userStats/{userId}
Firestore-->>Client: Return computed userStats
Client->>Client: Update local store (userStats, heatmap)
sequenceDiagram
participant Admin
participant CFn as Cloud Functions
participant Firestore as Firestore DB
Admin->>CFn: Call rebuildUserStats(optional userId)
CFn->>Firestore: Query pushups for user(s)
Firestore-->>CFn: Return pushup entries
CFn->>CFn: rebuildFromEntries (sort, aggregate, streak, bests)
CFn->>Firestore: Write rebuilt userStats documents
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
![sentry[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| }); | ||
|
|
||
| const weekReps = computed(() => { | ||
| const us = userStats(); | ||
| if (us) return us.weeklyReps; | ||
|
|
||
| // Fallback: client-side computation | ||
| const today = new Date(); | ||
| const dayOfWeek = (today.getDay() + 6) % 7; // Monday = 0 | ||
| const monday = new Date(today); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| if (dailyReps > 0 && keys.dailyKey === (base.dailyKey || keys.dailyKey)) { | ||
| if (!bestDay || dailyReps > bestDay.total) { | ||
| bestDay = { date: keys.dailyKey, total: dailyReps }; | ||
| } | ||
| } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12134d84ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const updated = applyDelta(existing, { | ||
| userId, | ||
| repsDelta, | ||
| entriesDelta, | ||
| timestamp, |
There was a problem hiding this comment.
This transaction applies a single applyDelta with one timestamp and repsDelta = newReps - oldReps. If a pushup is edited to a different timestamp/day (supported via PushupUpdate.timestamp), the old day/week/month/heatmap bucket is never decremented correctly and timestamp-only edits (repsDelta = 0) can zero or misplace period counters, so stats drift until a full rebuild.
Useful? React with 👍 / 👎.
| const allTimeDays = computed( | ||
| () => userStats()?.totalDays ?? allTimeStats().meta.days | ||
| ); |
There was a problem hiding this comment.
The dashboard now prefers userStats.totalDays, but the new delta implementation keeps totalDays unchanged (it is only accurate after rebuild), so this value remains 0 or stale during normal create/update/delete flows. Once userStats exists, allTimeDays and therefore allTimeAvg become incorrect for affected users.
Useful? React with 👍 / 👎.
| const weekReps = computed(() => { | ||
| const us = userStats(); | ||
| if (us) return us.weeklyReps; | ||
|
|
There was a problem hiding this comment.
This returns server-side period totals unconditionally; unlike todayTotal, there is no check that the stored weeklyKey/monthlyKey is still the current period. After week/month rollover without a new write, users will see previous-period totals instead of 0 until another pushup event refreshes userStats.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds a server-precomputed UserStats document and Cloud Functions delta-update pipeline, then wires the web app to consume these stats (with client fallbacks).
Changes:
- Introduces
UserStatsmodel + API service to fetchuserStats/{userId}from Firestore. - Adds Cloud Functions delta/rebuild logic and Firestore trigger to maintain
userStats. - Updates dashboard/analysis stores and emulator/build config (esbuild + TS, rules, firebase.json).
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/app/stats/shell/stats-dashboard.component.spec.ts | Adds UserStatsApiService mock provider for component tests. |
| web/src/app/stats/shell/analysis-page.component.spec.ts | Adds UserStatsApiService + UserContextService mocks for analysis page tests. |
| web/src/app/stats/dashboard.store.ts | Uses UserStatsApiService + resource to prefer precomputed stats (with fallbacks). |
| web/src/app/stats/analysis.store.ts | Adds UserStatsApiService resource and exposes server heatmap data. |
| libs/stats/src/lib/models/user-stats.models.ts | New UserStats interface + emptyUserStats initializer. |
| libs/stats/src/lib/models/user-stats.models.spec.ts | Tests the UserStats model initializer and shape. |
| libs/stats/src/lib/models/stats.models.ts | Re-exports user-stats.models. |
| libs/data-access/src/lib/api/user-stats-api.service.ts | New Firestore read service for userStats/{userId}. |
| libs/data-access/src/lib/api/user-stats-api.service.spec.ts | Tests for UserStatsApiService. |
| libs/data-access/src/index.ts | Exports UserStatsApiService. |
| eslint.config.mjs | Adds Nx module-boundaries rule for scope:cloud-functions. |
| data-store/project.json | Emulators now include functions; adds build dependencies + deploy target. |
| data-store/functions/tsconfig.spec.json | Jest TS config for functions tests. |
| data-store/functions/tsconfig.json | Base TS config for functions project. |
| data-store/functions/tsconfig.app.json | App TS config for functions build. |
| data-store/functions/src/user-stats-delta.ts | Pure delta + rebuild logic for UserStats. |
| data-store/functions/src/user-stats-delta.spec.ts | Large test suite for delta/rebuild logic. |
| data-store/functions/src/types.d.ts | Declares web-push module types. |
| data-store/functions/src/index.ts | Migrates functions to TS/ESM syntax and adds user-stats trigger + admin rebuild callable. |
| data-store/functions/project.json | Nx project for functions with esbuild + jest. |
| data-store/functions/package.json | Updates node engine + entrypoint naming for built functions. |
| data-store/functions/jest.config.cts | Jest config + coverage thresholds for functions project. |
| data-store/firestore.rules | Allows owner-read of userStats/{userId} (Admin SDK write-only). |
| data-store/firebase.json | Points functions source to built output + sets nodejs24 runtime. |
Comments suppressed due to low confidence (1)
data-store/functions/src/index.ts:1106
updateUserStatsOnPushupWritepicks a singletimestamp(prefersafter.timestamp) and applies arepsDelta. If an update changestimestamp(UI sends timestamp on update), you must remove the old contribution (old timestamp/day/week/heatmap slot) and add the new one; current logic won’t (andrepsDeltacan be 0). Handlebefore.timestamp !== after.timestampas “delete old + create new” (or triggerrebuildFromEntries).
| const currentStreak = computed(() => { | ||
| const us = userStats(); | ||
| if (us) return us.currentStreak; | ||
|
|
||
| // Fallback: client-side computation |
There was a problem hiding this comment.
currentStreak returns userStats.currentStreak without checking whether the streak is still valid today. If the last entry was >1 day ago, server-side currentStreak will remain non-zero unless recomputed. Use lastEntryDate vs today/yesterday to decide whether to use userStats.currentStreak, otherwise return 0 / fall back to client calc.
| const weekReps = computed(() => { | ||
| const us = userStats(); | ||
| if (us) return us.weeklyReps; |
There was a problem hiding this comment.
weekReps returns userStats.weeklyReps unconditionally. If userStats.weeklyKey isn’t the current ISO week (e.g., no entries this week or an older entry was edited), this will show stale data. Compare against the current week key and fall back to computing from entries when it doesn’t match.
| const weekReps = computed(() => { | |
| const us = userStats(); | |
| if (us) return us.weeklyReps; | |
| const getCurrentIsoWeekKey = () => { | |
| const today = new Date(); | |
| const date = new Date(today); | |
| date.setHours(0, 0, 0, 0); | |
| const day = (date.getDay() + 6) % 7; | |
| date.setDate(date.getDate() + 3 - day); | |
| const isoYear = date.getFullYear(); | |
| const firstThursday = new Date(isoYear, 0, 4); | |
| firstThursday.setHours(0, 0, 0, 0); | |
| const firstThursdayDay = (firstThursday.getDay() + 6) % 7; | |
| firstThursday.setDate(firstThursday.getDate() + 3 - firstThursdayDay); | |
| const week = | |
| 1 + | |
| Math.round( | |
| (date.getTime() - firstThursday.getTime()) / (7 * 24 * 60 * 60 * 1000) | |
| ); | |
| return `${isoYear}-W${String(week).padStart(2, '0')}`; | |
| }; | |
| const weekReps = computed(() => { | |
| const us = userStats(); | |
| if (us && us.weeklyKey === getCurrentIsoWeekKey()) return us.weeklyReps; |
| const monthReps = computed(() => { | ||
| const us = userStats(); | ||
| if (us) return us.monthlyReps; | ||
|
|
||
| // Fallback: client-side computation |
There was a problem hiding this comment.
monthReps returns userStats.monthlyReps unconditionally. If userStats.monthlyKey isn’t the current month (YYYY-MM), this will show stale data. Compare against the current month key and fall back to computing from entries when it doesn’t match.
| const parts = berlinParts(timestamp); | ||
| const keys = periodKeys(parts); | ||
| const slot = heatmapSlot(parts.weekday, parts.hour); | ||
|
|
||
| // ── Aggregates ────────────────────────────────────────────────────── | ||
| const total = Math.max(0, base.total + repsDelta); | ||
| const totalEntries = Math.max(0, base.totalEntries + entriesDelta); | ||
|
|
||
| const dailyReps = | ||
| base.dailyKey === keys.dailyKey | ||
| ? Math.max(0, base.dailyReps + repsDelta) | ||
| : repsDelta > 0 | ||
| ? repsDelta | ||
| : 0; |
There was a problem hiding this comment.
applyDelta derives dailyKey/weeklyKey/monthlyKey from the mutated entry’s timestamp and rewrites the stored keys/counters. Editing/deleting an old entry (or backdating an entry) can overwrite “current” period stats. Derive current period keys from nowIso (Berlin-local) and only apply repsDelta when the entry belongs to that current period; otherwise leave period counters unchanged.
|
|
||
| // ── Best day ──────────────────────────────────────────────────────── | ||
| let bestDay = base.bestDay ? { ...base.bestDay } : null; | ||
| if (dailyReps > 0 && keys.dailyKey === (base.dailyKey || keys.dailyKey)) { |
There was a problem hiding this comment.
bestDay update is gated by keys.dailyKey === (base.dailyKey || keys.dailyKey), which blocks updating bestDay when processing the first entry of a new day (because base.dailyKey is still yesterday). Compute bestDay based on the updated day total for keys.dailyKey without referencing base.dailyKey.
| if (dailyReps > 0 && keys.dailyKey === (base.dailyKey || keys.dailyKey)) { | |
| if (dailyReps > 0) { |
| // totalDays: only accurate via rebuild; delta keeps existing value | ||
| const totalDays = base.totalDays; |
There was a problem hiding this comment.
totalDays is never updated in delta mode (const totalDays = base.totalDays). This makes UserStats.totalDays stale/0 unless a rebuild is run, but the dashboard prefers userStats.totalDays. Either maintain it incrementally (requires per-day counts) or avoid reading it from UserStats until a rebuild/backfill guarantees correctness.
| // totalDays: only accurate via rebuild; delta keeps existing value | |
| const totalDays = base.totalDays; | |
| // ── Total active days ─────────────────────────────────────────────── | |
| const previousDailyTotal = Number( | |
| (base.heatmap as Record<string, number> | undefined)?.[keys.dailyKey] ?? 0 | |
| ); | |
| const totalDays = | |
| (base.totalDays ?? 0) + (newReps > 0 && previousDailyTotal <= 0 ? 1 : 0); |
| /** | ||
| * Return an empty UserStats object. | ||
| */ | ||
| export function emptyUserStats(userId: string): UserStats { | ||
| return { | ||
| userId, | ||
| total: 0, | ||
| totalEntries: 0, | ||
| totalDays: 0, |
There was a problem hiding this comment.
emptyUserStats is duplicated here and in @pu-stats/models (libs/stats/src/lib/models/user-stats.models.ts). Duplicating the initializer risks drift; prefer importing and reusing the shared helper from the models package so both sides stay in sync.
| const { getDoc } = jest.requireMock('@angular/fire/firestore'); | ||
| const mockStats = { | ||
| userId: 'test-uid', | ||
| total: 500, | ||
| totalEntries: 25, | ||
| dailyReps: 30, | ||
| dailyKey: '2026-04-05', | ||
| }; |
There was a problem hiding this comment.
This test’s mockStats doesn’t match the UserStats contract (many required fields missing), but UserStatsApiService casts Firestore data to UserStats. Use a complete UserStats mock (preferred), or explicitly type the mock as Partial<UserStats> and adjust the service to merge defaults/validate before returning.
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/app/stats/dashboard.store.ts (1)
176-200:⚠️ Potential issue | 🟠 MajorGuard weekly/monthly totals with their period keys.
Line 178 and Line 200 return the stored counters unconditionally. If the calendar rolls over before the next write, the dashboard will keep showing the previous week/month until another entry arrives. Use the same
key matches current periodcheck thattodayTotal()already applies fordailyKey, then fall through to the existing entries-based calculation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/app/stats/dashboard.store.ts` around lines 176 - 200, Guard using stored weekly/monthly counters by verifying their period key matches the current period before returning them: in the computed weekReps and monthReps, after const us = userStats() check that us exists AND that us.weeklyKey (for weekReps) / us.monthlyKey (for monthReps) equals the current period key (compute the same weekly/monthly key logic used by todayTotal() or derive it from the monday/sunday or year-month values); only return us.weeklyReps or us.monthlyReps when the key matches, otherwise fall through to the existing entries-based fallback calculation.
🧹 Nitpick comments (3)
data-store/functions/src/types.d.ts (1)
13-16: UpdatesendNotificationtyping to match the web-push library API.Current typing is stricter than the actual web-push API. The
payloadparameter is optional (not required), the function accepts anoptionsargument, and the return type should bePromise<WebPushResponse>instead ofPromise<unknown>.Corrected typing
declare module 'web-push' { interface PushSubscription { endpoint: string; keys: { p256dh: string; auth: string }; } + interface SendNotificationOptions { + [key: string]: unknown; + } + + interface WebPushResponse { + statusCode: number; + headers: Record<string, string>; + body: string; + } + function setVapidDetails( subject: string, publicKey: string, privateKey: string ): void; function sendNotification( subscription: PushSubscription, - payload: string + payload?: string | Buffer, + options?: SendNotificationOptions - ): Promise<unknown>; + ): Promise<WebPushResponse>; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data-store/functions/src/types.d.ts` around lines 13 - 16, The sendNotification declaration is too strict: update the signature of sendNotification(subscription: PushSubscription, payload?: string, options?: PushOptions): Promise<WebPushResponse>; so that payload is optional, an options parameter is accepted, and the return type is Promise<WebPushResponse>; ensure WebPushResponse (and PushOptions if needed) are imported or declared in the same types.d.ts and update the function declaration in the file accordingly.data-store/functions/src/index.ts (1)
1182-1194: Consider memory optimization for large-scale rebuild.When rebuilding all users, this loads the entire
pushupscollection into memory to extract unique userIds. For a service with many users/entries, this could cause memory pressure.A more scalable approach would be to query distinct userIds or iterate through
userConfigsinstead:💡 Alternative approach using userConfigs
// Rebuild for ALL users - const allPushups = await db.collection('pushups').get(); - const userIds = new Set<string>(); - for (const doc of allPushups.docs) { - const uid = doc.data().userId; - if (uid) userIds.add(uid); - } + // Query userConfigs instead of loading all pushups + const userConfigsSnap = await db.collection('userConfigs').get(); + const userIds = userConfigsSnap.docs.map((doc) => doc.id); let totalRebuilt = 0; - for (const userId of userIds) { + for (const userId of userIds) { await rebuildForUser(userId); totalRebuilt++; }Note: This assumes all users with pushups have a
userConfigsdocument. If not, the current approach is safer but less scalable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data-store/functions/src/index.ts` around lines 1182 - 1194, The current loop loads the entire pushups collection via db.collection('pushups').get() (variable allPushups) which can OOM on large datasets; instead iterate user identifiers without materializing all pushup docs e.g. query userConfigs (or stream pushups in pages) to collect userIds and then call rebuildForUser(userId); update the logic that builds userIds (replace the allPushups.docs iteration) to use a paginated/streaming query or a direct userConfigs collection scan so only small batches are kept in memory while still invoking rebuildForUser for each unique userId.data-store/functions/src/user-stats-delta.ts (1)
372-394: Consider deduplicatingemptyUserStatsacross packages.This function is duplicated identically in both:
libs/stats/src/lib/models/user-stats.models.tsdata-store/functions/src/user-stats-delta.tsThe function is accessible to
data-store/functionsvia the monorepo's@pu-stats/modelspath mapping. To consolidate, first exportemptyUserStatsfromlibs/stats/src/index.ts, then import it from@pu-stats/modelsin the cloud-functions file. The implementation is consistent across both locations and already used 21 times throughout the codebase.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data-store/functions/src/user-stats-delta.ts` around lines 372 - 394, The emptyUserStats implementation is duplicated; export the existing emptyUserStats function (and its UserStats type) from the stats package public index (so it is part of the package API) and remove the local copy in user-stats-delta; then update the cloud-functions file to import emptyUserStats and UserStats from `@pu-stats/models` (replace the local function with the imported symbol) so all uses reference the single exported implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@data-store/firebase.json`:
- Around line 3-4: The functions.source value points outside the project
("../dist/cloud-functions"), which Firebase CLI rejects; update functions.source
in firebase.json to a path inside the data-store project (e.g.,
"dist/cloud-functions" or "functions") and ensure your build outputs the
compiled functions into that in-repo directory (move or reconfigure the build
target), leaving the runtime ("nodejs24") unchanged.
In `@libs/data-access/src/lib/api/user-stats-api.service.spec.ts`:
- Around line 33-45: The test currently uses
jest.requireMock('@angular/fire/firestore') to grab getDoc dynamically; replace
that with the statically imported getDoc from the top of the spec and use
jest.mocked(getDoc) to set up its mock. Locate the test "returns UserStats when
document exists" and change the getDoc assignment to use the imported symbol and
call jest.mocked(getDoc).mockResolvedValueOnce({...}) so the spec follows the
repo's static mock convention.
In `@web/src/app/stats/analysis.store.ts`:
- Around line 112-116: The resource currently calls
store._userStatsApi.getUserStats with store._user.userIdSafe() even when it
returns an empty string; update userStatsResource so params() and/or the loader
first check store._user.userIdSafe() and skip the query when it's falsy (e.g.,
return null/undefined or resolve to an empty value) instead of calling
store._userStatsApi.getUserStats(''); specifically guard in the loader that
calls firstValueFrom(store._userStatsApi.getUserStats(params.userId)) to only
invoke getUserStats when params.userId is non-empty, and ensure the loader
returns a safe no-op value when userId is empty.
In `@web/src/app/stats/dashboard.store.ts`:
- Around line 151-154: The computed currentStreak should expire stale
server-side streaks by checking the user's lastEntryDate before returning
userStats().currentStreak; update the currentStreak computed to get us =
userStats(), read us.lastEntryDate and if that date is older than yesterday
(i.e., not within the most recent day/consecutive window) return 0 instead of
us.currentStreak, otherwise return us.currentStreak; reference the currentStreak
computed, userStats(), and lastEntryDate when making the change.
---
Outside diff comments:
In `@web/src/app/stats/dashboard.store.ts`:
- Around line 176-200: Guard using stored weekly/monthly counters by verifying
their period key matches the current period before returning them: in the
computed weekReps and monthReps, after const us = userStats() check that us
exists AND that us.weeklyKey (for weekReps) / us.monthlyKey (for monthReps)
equals the current period key (compute the same weekly/monthly key logic used by
todayTotal() or derive it from the monday/sunday or year-month values); only
return us.weeklyReps or us.monthlyReps when the key matches, otherwise fall
through to the existing entries-based fallback calculation.
---
Nitpick comments:
In `@data-store/functions/src/index.ts`:
- Around line 1182-1194: The current loop loads the entire pushups collection
via db.collection('pushups').get() (variable allPushups) which can OOM on large
datasets; instead iterate user identifiers without materializing all pushup docs
e.g. query userConfigs (or stream pushups in pages) to collect userIds and then
call rebuildForUser(userId); update the logic that builds userIds (replace the
allPushups.docs iteration) to use a paginated/streaming query or a direct
userConfigs collection scan so only small batches are kept in memory while still
invoking rebuildForUser for each unique userId.
In `@data-store/functions/src/types.d.ts`:
- Around line 13-16: The sendNotification declaration is too strict: update the
signature of sendNotification(subscription: PushSubscription, payload?: string,
options?: PushOptions): Promise<WebPushResponse>; so that payload is optional,
an options parameter is accepted, and the return type is
Promise<WebPushResponse>; ensure WebPushResponse (and PushOptions if needed) are
imported or declared in the same types.d.ts and update the function declaration
in the file accordingly.
In `@data-store/functions/src/user-stats-delta.ts`:
- Around line 372-394: The emptyUserStats implementation is duplicated; export
the existing emptyUserStats function (and its UserStats type) from the stats
package public index (so it is part of the package API) and remove the local
copy in user-stats-delta; then update the cloud-functions file to import
emptyUserStats and UserStats from `@pu-stats/models` (replace the local function
with the imported symbol) so all uses reference the single exported
implementation.
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: a916db51-718c-4a21-9e50-54bd2813a251
📒 Files selected for processing (24)
data-store/firebase.jsondata-store/firestore.rulesdata-store/functions/jest.config.ctsdata-store/functions/package.jsondata-store/functions/project.jsondata-store/functions/src/index.tsdata-store/functions/src/types.d.tsdata-store/functions/src/user-stats-delta.spec.tsdata-store/functions/src/user-stats-delta.tsdata-store/functions/tsconfig.app.jsondata-store/functions/tsconfig.jsondata-store/functions/tsconfig.spec.jsondata-store/project.jsoneslint.config.mjslibs/data-access/src/index.tslibs/data-access/src/lib/api/user-stats-api.service.spec.tslibs/data-access/src/lib/api/user-stats-api.service.tslibs/stats/src/lib/models/stats.models.tslibs/stats/src/lib/models/user-stats.models.spec.tslibs/stats/src/lib/models/user-stats.models.tsweb/src/app/stats/analysis.store.tsweb/src/app/stats/dashboard.store.tsweb/src/app/stats/shell/analysis-page.component.spec.tsweb/src/app/stats/shell/stats-dashboard.component.spec.ts
- Fix firebase.json: move functions-dist inside data-store/ (Firebase CLI rejects parent-relative paths) - Fix bestDay bug: remove flawed dailyKey guard that blocked updates on new days - Add period-key validation: weekReps/monthReps only use UserStats when keys match current week/month; streak checks lastEntryDate freshness - Fix totalDays: maintain incrementally (increment on new day, decrement when day goes to zero reps) - Handle timestamp-changing updates: split into undo-old + apply-new deltas so day/week/month/heatmap buckets stay correct - Remove emptyUserStats duplication: re-export from @pu-stats/models - Guard against empty userId in dashboard/analysis store loaders - Fix test: use jest.mocked(getDoc) instead of jest.requireMock, complete UserStats mock shape https://claude.ai/code/session_01WUgLPxZ66o25ij4BYjqC9H
…, and pitfalls - Document cloud-functions Nx project (TS + esbuild + Jest) - Add firebase.json deploy path constraint (must be inside project dir) - Add delta-based aggregation pitfalls (timestamp changes, totalDays, bestDay) - Add precomputed data staleness validation pattern (period key checks) - Add @angular/fire/firestore jest.mock pitfall - Update dependency graph, module boundaries, project names, domain models https://claude.ai/code/session_01WUgLPxZ66o25ij4BYjqC9H
| // totalDays: increment when a new day appears, decrement when a day goes to 0 | ||
| let totalDays = base.totalDays; | ||
| const isNewDay = base.dailyKey !== keys.dailyKey; | ||
| if (isNewDay && dailyReps > 0) { | ||
| totalDays += 1; | ||
| } else if (!isNewDay && dailyReps === 0 && base.dailyReps > 0) { | ||
| totalDays = Math.max(0, totalDays - 1); | ||
| } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 26 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (2)
data-store/functions/src/index.ts:1126
- In the
timestampChangedbranch,oldTimestamp/newTimestampare typed asstring | undefined, butapplyDeltarequirestimestamp: string. TS won’t narrow based on the booleantimestampChanged, so this should be guarded or use non-null assertions after an explicit check (e.g. early-return if either is missing).
data-store/functions/src/index.ts:1212 rebuildUserStats(all users) loads the entirepushupscollection and then rebuilds users sequentially. This can exceed function time/memory limits as data grows. Consider paginating/streaming, deriving userIds fromuserConfigs, and/or processing users in bounded parallel batches (or via Cloud Tasks) to make backfills reliable.
| "outDir": "../../dist/out-tsc", | ||
| "types": ["node"] | ||
| }, | ||
| "include": ["src/**/*.ts"], |
There was a problem hiding this comment.
tsconfig.app.json only includes src/**/*.ts, so local declaration files like src/types.d.ts are not part of the app build. If web-push has no installed typings, the functions build will fail. Include src/**/*.d.ts (or explicitly include src/types.d.ts) in the app tsconfig.
| "include": ["src/**/*.ts"], | |
| "include": ["src/**/*.ts", "src/**/*.d.ts"], |
| if (entriesDelta >= 0) { | ||
| streakState = updateStreak( | ||
| base.currentStreak, | ||
| base.lastEntryDate, | ||
| parts.isoDate | ||
| ); | ||
| } else { | ||
| // Delete — streak recalculation requires full scan (handled by backfill). |
There was a problem hiding this comment.
applyDelta updates streaks on every create/update (entriesDelta >= 0) using updateStreak(base.lastEntryDate, parts.isoDate). If an older entry is edited (or a timestamp is moved backwards), daysBetween becomes negative and updateStreak resets the streak and lastEntryDate to an older date, corrupting streak state. Streak updates should only occur when entryDate >= lastEntryDate (or when processing in chronological order), otherwise keep streak unchanged or trigger a rebuild.
| if (entriesDelta >= 0) { | |
| streakState = updateStreak( | |
| base.currentStreak, | |
| base.lastEntryDate, | |
| parts.isoDate | |
| ); | |
| } else { | |
| // Delete — streak recalculation requires full scan (handled by backfill). | |
| const canAdvanceStreak = | |
| entriesDelta >= 0 && | |
| (base.lastEntryDate === null || parts.isoDate >= base.lastEntryDate); | |
| if (canAdvanceStreak) { | |
| streakState = updateStreak( | |
| base.currentStreak, | |
| base.lastEntryDate, | |
| parts.isoDate | |
| ); | |
| } else { | |
| // Delete or backdated/out-of-order write — incremental streak update | |
| // is unsafe; keep current state. Full rebuild handled elsewhere. |
| // totalDays: increment when a new day appears, decrement when a day goes to 0 | ||
| let totalDays = base.totalDays; | ||
| const isNewDay = base.dailyKey !== keys.dailyKey; | ||
| if (isNewDay && dailyReps > 0) { | ||
| totalDays += 1; | ||
| } else if (!isNewDay && dailyReps === 0 && base.dailyReps > 0) { | ||
| totalDays = Math.max(0, totalDays - 1); | ||
| } |
There was a problem hiding this comment.
totalDays tracking here can become incorrect on updates/deletes for dates other than base.dailyKey (and when timestamps move), because the code only knows about the current dailyKey/dailyReps. Since totalDays is consumed by the dashboard (average/metadata), this will lead to wrong UI values. Consider maintaining per-day counts (e.g. a map of day→count) or falling back to rebuildFromEntries when a mutation affects a non-current day / a day potentially drops to zero.
| // totalDays: increment when a new day appears, decrement when a day goes to 0 | |
| let totalDays = base.totalDays; | |
| const isNewDay = base.dailyKey !== keys.dailyKey; | |
| if (isNewDay && dailyReps > 0) { | |
| totalDays += 1; | |
| } else if (!isNewDay && dailyReps === 0 && base.dailyReps > 0) { | |
| totalDays = Math.max(0, totalDays - 1); | |
| } | |
| // totalDays: derive from per-day totals to stay correct for updates/deletes | |
| // on any day and for timestamp moves between days. | |
| const totalDays = Object.values(heatmap).reduce( | |
| (count, reps) => count + (reps > 0 ? 1 : 0), | |
| 0 | |
| ); |
| const todayTotal = computed(() => { | ||
| const us = userStats(); | ||
| if (us && us.dailyKey === toLocalIsoDate(new Date())) { | ||
| return us.dailyReps; | ||
| } | ||
| // Fallback: compute from entries |
There was a problem hiding this comment.
The staleness checks compare server-computed Berlin period keys (dailyKey, weeklyKey, monthlyKey) against keys computed in the browser’s local timezone (toLocalIsoDate(new Date()), currentIsoWeekKey(), currentMonthKey()). For users not in Europe/Berlin, these comparisons can fail around day/week/month boundaries and force the expensive entry-based fallbacks, undermining the goal of precomputed stats. Consider computing comparison keys in the same timezone as the backend (Europe/Berlin) or switching backend stats to user timezone.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@data-store/functions/src/user-stats-delta.ts`:
- Around line 175-177: The current incremental update code (in applyDelta)
recalculates period keys and streaks using berlinParts/periodKeys/heatmapSlot
from the touched entry's timestamp, which allows historical edits/backfills to
rewind dailyKey/weeklyKey/monthlyKey, reset streak state, and overcount
totalDays; change applyDelta to detect non-latest-timestamp writes and, for
those cases, do not perform the incremental path but instead trigger a full
rebuild or require persisted per-day metadata to compute safe diffs; concretely,
add a check in applyDelta (and the branches that call
berlinParts/periodKeys/heatmapSlot and update totalDays/streak) to compare the
touched timestamp against the current snapshot window and either (A) enqueue a
rebuild job for user-stats or (B) use stored per-day summaries to compute
idempotent deltas, ensuring only append-latest writes use the existing
incremental logic.
- Around line 180-181: When computing the updated totals in user-stats-delta
(using base.total, repsDelta, base.totalEntries, entriesDelta -> total and
totalEntries), ensure that if totalEntries === 0 you return
emptyUserStats(userId) instead of carrying over previous streak/best-entry
fields; modify the code path in the function that builds the return object (and
the analogous block that updates stats around the other totals handling) to
short-circuit and return emptyUserStats(userId) when computed totalEntries is
zero so no ghost metadata remains.
- Around line 168-175: In applyDelta, do not seed missing existing state with
emptyUserStats(userId); instead detect when existing === null and either fail
fast or call rebuildFromEntries(userId) to obtain the real aggregate before
applying the delta; modify applyDelta to early-return or throw when existing is
null (for a fail-fast path) or synchronously invoke rebuildFromEntries to
produce a proper UserStats and then continue using that result (referencing
applyDelta, emptyUserStats, and rebuildFromEntries to locate the change).
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 2e6aeae5-1754-4815-b7f7-a02b6ab29074
📒 Files selected for processing (10)
CLAUDE.mddata-store/.gitignoredata-store/firebase.jsondata-store/functions/project.jsondata-store/functions/src/index.tsdata-store/functions/src/user-stats-delta.spec.tsdata-store/functions/src/user-stats-delta.tslibs/data-access/src/lib/api/user-stats-api.service.spec.tsweb/src/app/stats/analysis.store.tsweb/src/app/stats/dashboard.store.ts
✅ Files skipped from review due to trivial changes (4)
- data-store/.gitignore
- libs/data-access/src/lib/api/user-stats-api.service.spec.ts
- data-store/functions/project.json
- data-store/functions/src/user-stats-delta.spec.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- data-store/firebase.json
- web/src/app/stats/analysis.store.ts
- web/src/app/stats/dashboard.store.ts
- data-store/functions/src/index.ts
|
View your CI Pipeline Execution ↗ for commit 3327625
☁️ Nx Cloud last updated this comment at |
|
Visit the preview URL for this PR (updated for commit 3327625): https://pushup-stats--pr184-claude-implement-iss-awbuhj1m.web.app (expires Sun, 12 Apr 2026 19:07:51 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 81c1346f18296c5c147c6260f1a724af4cbbd57e |
| base.weeklyKey === keys.weeklyKey | ||
| ? Math.max(0, base.weeklyReps + repsDelta) | ||
| : repsDelta > 0 | ||
| ? repsDelta | ||
| : 0; | ||
| const monthlyReps = | ||
| base.monthlyKey === keys.monthlyKey | ||
| ? Math.max(0, base.monthlyReps + repsDelta) | ||
| : repsDelta > 0 | ||
| ? repsDelta | ||
| : 0; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…ghost stats (#182) - Fix CRITICAL: deleting entry from past week/month no longer resets current period reps to 0 — keep stored values when entry's period differs - Fix: editing backdated entries no longer corrupts streak — only advance streak for chronological writes (entryDate >= lastEntryDate) - Fix: return emptyUserStats when totalEntries reaches 0 (no ghost metadata) - Fix: preserve dailyKey/weeklyKey/monthlyKey when entry is from different period - Fix: tsconfig.app.json now includes src/**/*.d.ts for web-push types - Add 3 regression tests: past-period delete, zero-entries reset, backdated edit https://claude.ai/code/session_01WUgLPxZ66o25ij4BYjqC9H
| return Math.round((bd.getTime() - ad.getTime()) / 86_400_000); | ||
| } | ||
|
|
||
| function currentIsoWeekKey(): string { | ||
| const d = new Date(); | ||
| d.setHours(0, 0, 0, 0); | ||
| const day = (d.getDay() + 6) % 7; | ||
| d.setDate(d.getDate() + 3 - day); | ||
| const isoYear = d.getFullYear(); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 26 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
data-store/functions/src/index.ts:1125
- In the
timestampChangedbranch,oldTimestamp/newTimestampare typed asstring | undefinedandtimestampChanged(a boolean) does not narrow them for TypeScript. Passing them intoapplyDelta(expectsstring) should be a TS error. Add an explicit guard inside the branch (or!assertions) so the compiler can prove they’re defined.
data-store/functions/src/index.ts:1212 rebuildUserStats“ALL users” path scans the entirepushupscollection and then rebuilds each user sequentially (1 query per user + write). This is likely to hit the 540s timeout / Firestore rate limits as the dataset grows. Consider batching with bounded concurrency, and/or deriving userIds from a smaller collection (e.g.userConfigs) plus pagination.
| // totalDays: increment when a new day appears, decrement when current day goes to 0 | ||
| let totalDays = base.totalDays; | ||
| if (sameDay && dailyReps === 0 && base.dailyReps > 0) { | ||
| totalDays = Math.max(0, totalDays - 1); | ||
| } else if (!sameDay && repsDelta > 0) { |
There was a problem hiding this comment.
totalDays update is not correct for unique-day counting: else if (!sameDay && repsDelta > 0) totalDays += 1 will overcount on backdated creates/updates (and on timestamp-changed updates) even if that date already has entries. Since the dashboard now prefers userStats.totalDays, this will skew averages. Consider only incrementing on chronological day rollovers (e.g. when the stored dailyKey advances), or maintain per-day counts (map) and/or trigger rebuildFromEntries() for backdated/timestamp-changed writes.
| // totalDays: increment when a new day appears, decrement when current day goes to 0 | |
| let totalDays = base.totalDays; | |
| if (sameDay && dailyReps === 0 && base.dailyReps > 0) { | |
| totalDays = Math.max(0, totalDays - 1); | |
| } else if (!sameDay && repsDelta > 0) { | |
| // totalDays: decrement when current day goes to 0; increment only on chronological day rollover | |
| let totalDays = base.totalDays; | |
| const dayAdvancedChronologically = | |
| !sameDay && repsDelta > 0 && (!base.dailyKey || dailyKey > base.dailyKey); | |
| if (sameDay && dailyReps === 0 && base.dailyReps > 0) { | |
| totalDays = Math.max(0, totalDays - 1); | |
| } else if (dayAdvancedChronologically) { |
- AnalysisTeaserCardComponent: add refreshTrigger input that forces statsResource reload when incremented - StatsDashboardComponent: track refreshCounter signal, increment on createEntry, addQuickEntry, and LiveDataStore tick - AnalysisStore: add refreshAll() method to reload all resources - AnalysisPageComponent: add LiveDataStore.updateTick() effect to auto-refresh chart/stats when new entries arrive via websocket https://claude.ai/code/session_01WUgLPxZ66o25ij4BYjqC9H
| const dailyReps = sameDay | ||
| ? Math.max(0, base.dailyReps + repsDelta) | ||
| : repsDelta > 0 | ||
| ? repsDelta // new period starts with this entry's reps | ||
| : base.dailyReps; // different period delete/update → keep current | ||
| const dailyKey = sameDay || repsDelta > 0 ? keys.dailyKey : base.dailyKey; | ||
|
|
||
| const weeklyReps = sameWeek |
There was a problem hiding this comment.
Bug: The applyDelta function incorrectly resets dailyReps when a user edits an old entry, causing today's rep count to be lost and replaced by the rep delta.
Severity: HIGH
Suggested Fix
Modify the logic in applyDelta to differentiate between creating a new entry and updating an old one. The condition should also check entriesDelta. For example, only reset dailyReps when !sameDay && repsDelta > 0 && entriesDelta > 0. When updating an old entry (entriesDelta === 0), the dailyReps and dailyKey for the current day should not be modified.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: data-store/functions/src/user-stats-delta.ts#L194-L201
Potential issue: When a user edits a push-up entry from a previous day to increase the
rep count, the `applyDelta` function incorrectly handles the update. The logic at lines
194-201 checks if `!sameDay && repsDelta > 0` and, if true, sets `dailyReps =
repsDelta`. This is wrong for an update, where `repsDelta` is just the change in reps,
not the total. As a result, the user's statistics for the current day are overwritten
with the small delta value, and the `dailyKey` is incorrectly changed to the old entry's
date, effectively losing all of today's progress.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
web/src/app/stats/shell/stats-dashboard.component.ts (1)
121-123: Extract a helper forrefreshAll + counter bumpto prevent drift.The same two-line refresh sequence appears in three places. A small helper keeps behavior consistent when refresh logic evolves.
♻️ Proposed refactor
export class StatsDashboardComponent { @@ readonly refreshCounter = signal(0); + + private triggerRefresh(): void { + this.store.refreshAll(); + this.refreshCounter.update((c) => c + 1); + } @@ effect(() => { if (!isPlatformBrowser(this.platformId)) return; const tick = this.live.updateTick(); if (!tick) return; - this.store.refreshAll(); - this.refreshCounter.update((c) => c + 1); + this.triggerRefresh(); }); @@ async createEntry(entry: { @@ await firstValueFrom(this.api.createPushup(entry)); - this.store.refreshAll(); - this.refreshCounter.update((c) => c + 1); + this.triggerRefresh(); } @@ async addQuickEntry(reps: number) { @@ await firstValueFrom( this.api.createPushup({ @@ }) ); - this.store.refreshAll(); - this.refreshCounter.update((c) => c + 1); + this.triggerRefresh(); } }Also applies to: 162-164, 182-184
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/app/stats/shell/stats-dashboard.component.ts` around lines 121 - 123, The repeated two-line sequence calling this.store.refreshAll(); followed by this.refreshCounter.update((c) => c + 1); should be extracted into a single helper method (e.g., add a private method refreshAllAndBump() on the StatsDashboardComponent) and replace each occurrence with a call to that helper to avoid behavioral drift; implement refreshAllAndBump() to call this.store.refreshAll() then this.refreshCounter.update((c) => c + 1), and update the three call sites that currently invoke store.refreshAll() + refreshCounter.update() to call refreshAllAndBump() instead.data-store/functions/src/user-stats-delta.ts (1)
240-264: Consider documenting thebestDayandtotalDayslimitations inline.The delta logic for
bestDay(lines 242-245) usesdailyReps, which only reflects the delta entry's reps when!sameDay. Similarly,totalDays(line 263) increments unconditionally for any new day, even if that day already has entries from a non-chronological backfill.Both are acceptable given the rebuild fallback, but a brief inline comment would help future maintainers understand this is intentional rather than a bug.
📝 Suggested documentation
// ── Best day ──────────────────────────────────────────────────────── + // Note: For non-chronological writes to a different day, dailyReps only + // reflects this delta, not the true day total. Rebuild corrects this. let bestDay = base.bestDay ? { ...base.bestDay } : null; if (dailyReps > 0) {- // totalDays: increment when a new day appears, decrement when current day goes to 0 + // totalDays: increment when a new day appears, decrement when current day goes to 0. + // Non-chronological writes may over/under-count; rebuild corrects this. let totalDays = base.totalDays;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data-store/functions/src/user-stats-delta.ts` around lines 240 - 264, Add brief inline comments near the bestDay and totalDays update blocks explaining the known limitations: state that bestDay uses the local dailyReps (which will reflect only the delta when !sameDay) so a backfilled/non-chronological entry may not correctly update historical bestDay until a full rebuild, and that totalDays increments for any new day when repsDelta>0 (potentially double-counting during non-chronological backfills) but is acceptable because the rebuild fallback will correct totals; reference the existing symbols bestDay, base.bestDay, dailyReps, sameDay, repsDelta, and base.totalDays so maintainers can easily find and understand the intentional behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@data-store/functions/src/user-stats-delta.ts`:
- Around line 240-264: Add brief inline comments near the bestDay and totalDays
update blocks explaining the known limitations: state that bestDay uses the
local dailyReps (which will reflect only the delta when !sameDay) so a
backfilled/non-chronological entry may not correctly update historical bestDay
until a full rebuild, and that totalDays increments for any new day when
repsDelta>0 (potentially double-counting during non-chronological backfills) but
is acceptable because the rebuild fallback will correct totals; reference the
existing symbols bestDay, base.bestDay, dailyReps, sameDay, repsDelta, and
base.totalDays so maintainers can easily find and understand the intentional
behavior.
In `@web/src/app/stats/shell/stats-dashboard.component.ts`:
- Around line 121-123: The repeated two-line sequence calling
this.store.refreshAll(); followed by this.refreshCounter.update((c) => c + 1);
should be extracted into a single helper method (e.g., add a private method
refreshAllAndBump() on the StatsDashboardComponent) and replace each occurrence
with a call to that helper to avoid behavioral drift; implement
refreshAllAndBump() to call this.store.refreshAll() then
this.refreshCounter.update((c) => c + 1), and update the three call sites that
currently invoke store.refreshAll() + refreshCounter.update() to call
refreshAllAndBump() instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: c93bf2d9-c6ab-428b-84ad-90c47ad75f48
📒 Files selected for processing (10)
CLAUDE.mddata-store/functions/src/index.tsdata-store/functions/src/user-stats-delta.spec.tsdata-store/functions/src/user-stats-delta.tsdata-store/functions/tsconfig.app.jsonweb/src/app/stats/analysis.store.tsweb/src/app/stats/components/analysis-teaser-card/analysis-teaser-card.component.tsweb/src/app/stats/shell/analysis-page.component.tsweb/src/app/stats/shell/stats-dashboard.component.htmlweb/src/app/stats/shell/stats-dashboard.component.ts
✅ Files skipped from review due to trivial changes (3)
- data-store/functions/tsconfig.app.json
- CLAUDE.md
- data-store/functions/src/user-stats-delta.spec.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/app/stats/analysis.store.ts
- data-store/functions/src/index.ts
Summary
Introduces a new
UserStatsmodel and delta-calculation system to precompute and maintain user statistics (total reps, streaks, heatmaps, best days, etc.) via Cloud Functions. Statistics are updated atomically on every pushup create/update/delete operation, eliminating the need for expensive client-side aggregations.Key Changes
New Models & Types
UserStatsmodel (libs/stats/src/lib/models/user-stats.models.ts): Server-side precomputed statistics including totals, period aggregates (daily/weekly/monthly), streaks, heatmaps, and performance metricsCore Delta Logic
user-stats-delta.ts(data-store/functions/src/user-stats-delta.ts): Pure, side-effect-free functions for:berlinParts(): Extract Berlin-local date/time from ISO timestampsisoWeekFromYmd(): Compute ISO week numbersperiodKeys(): Build daily/weekly/monthly period keysheatmapSlot(): Format weekday+hour heatmap slotsdaysBetween(): Calculate calendar day differencesupdateStreak(): Manage consecutive-day streaksapplyDelta(): Core logic for create (+reps), update (diff), and delete (-reps) operationsrebuildFromEntries(): Rebuild stats from scratch for backfill and streak recalculation after deletesComprehensive Test Coverage
user-stats-delta.spec.ts) covering:Cloud Functions Integration
data-store/functions/src/index.ts): Converted from CommonJS to ES modules with full type safetyUserStatsApiService(libs/data-access/src/lib/api/user-stats-api.service.ts): Client-side service to fetch precomputed stats from FirestoreuserStats/{userId}collection (Admin SDK write-only)Store Integration
DashboardStoreandAnalysisStoreupdated to consume precomputedUserStatsvia newUserStatsApiServiceBuild & Configuration
data-store/functions/project.jsonwith esbuild and Jest configurationfirebase.jsonto point to compiled Cloud Functions outputpackage.jsonto use Node 24 and correct entry pointImplementation Details
Delta-based updates: Instead of recalculating all stats on every change, the system applies incremental deltas:
+repsand+1 entry±(newReps - oldReps)diff-repsand-1 entryPeriod reset logic: When a user moves to a new day/week/month, period counters reset while totals accumulate. Streak updates only on creates/updates; deletes preserve the streak (conservative approach).
Timezone handling: All date calculations use Berlin timezone (Europe/Berlin) via
Intl.DateTimeFormatto ensure consistent day boundaries across regions.Rebuild capability:
rebuildFromEntries()allows full recalculation from raw pushup entries for backfill operations and streak recovery after deletes.https://claude.ai/code/session_01WUgLPxZ66o25ij4BYjqC9H
Summary by CodeRabbit
New Features
Infrastructure
Tests / Documentation