fix(api): invalidate analytics cache on conversation create/delete#370
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Review limit reached
Next review available in: 32 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ 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 |
The dashboard analytics endpoint (GET /v1/projects/:id/analytics)
caches results in AUTH_CACHE for 60-300s under
analytics:public:{projectId}:{range}, but createConversation and
deleteConversation never busted it. Deleting conversations showed
stale, too-high counts for up to 5 minutes.
Explicit invalidation on write is simplest and correct here: the
range affected isn't known ahead of time, so bust all three cached
ranges (7d/30d/90d) for the project via a new
lib/analytics-cache.ts helper, fire-and-forget via executionCtx.
Closes #352
Co-Authored-By: Duyet Le <me@duyet.net>
Co-Authored-By: duyetbot <bot@duyet.net>
a3ba43a to
8a929b6
Compare
Plan 006 is superseded: main shipped a stronger fix for the same issue (#289/#290) in aaf05ac (PR #355) after this plan's ce9a1fa baseline. The Idempotency-Key claim is now embedded atomically in the same d1.batch() as the mutation, rather than plan 006's separate claim-then-mutate step. Plan 007 was unaffected (#355 guarded a different code path) and ships as #382. Plan 012's hard blocker dissolved: PR #380 implemented #344 and wrote docs/webhooks.md with the timestamped signature scheme in the same PR, so the docs were written against the correct format. Plan 012 is now UI-only. Three plans went stale within hours of being written, each differently: 008 was wrong on arrival, 006 was overtaken by unrelated work, 012 by its own dependency being solved better elsewhere. The drift check guards only the first, and only when the executor's baseline is the pinned commit. Also records that #370 and #380 both edit createConversation/deleteConversation and will conflict, plus a suggested merge order. Co-Authored-By: Duyet Le <me@duyet.net> Co-Authored-By: duyetbot <bot@duyet.net>
* docs(plans): track the plans directory and add a status index The 10 implementation plans were untracked, so they existed only on one machine while every PR referenced them by path, and no worktree could see them. Each plan also instructs its executor to "update this plan's row in plans/README.md" — a file that never existed, which three separate executors hit and correctly flagged rather than fabricating. Adds plans/README.md: status table with issue links, the cross-plan dependency graph, and a note that plan 008 is superseded (its target design preserves the /watch deadlock in #360, so its own regression test would hang). Co-Authored-By: Duyet Le <me@duyet.net> Co-Authored-By: duyetbot <bot@duyet.net> * docs(plans): add plans 011-014 for the held product/architecture issues Plans for the four issues deliberately held out of the parallel fix batch because they are product decisions rather than defined fixes: - 011 dashboard UI for the 4 unexposed primitives (#284) - 012 webhooks docs + dashboard UI (#285) - 013 demo sandbox / pre-signup try-it (#287) - 014 write atomicity via db.batch() (#342) 014 confirms D1 batch() semantics against Cloudflare docs: statements execute sequentially as one all-or-nothing SQL transaction, but all statements must be prepared before the call — no reading a result mid-batch and branching on it. All three call sites are batchable as-is; updateConversationMessageCount is already a blind relative SQL update rather than a read-then-write. Co-Authored-By: Duyet Le <me@duyet.net> Co-Authored-By: duyetbot <bot@duyet.net> * docs(plans): mark 006 superseded and 012 docs-done; record merge order Plan 006 is superseded: main shipped a stronger fix for the same issue (#289/#290) in aaf05ac (PR #355) after this plan's ce9a1fa baseline. The Idempotency-Key claim is now embedded atomically in the same d1.batch() as the mutation, rather than plan 006's separate claim-then-mutate step. Plan 007 was unaffected (#355 guarded a different code path) and ships as #382. Plan 012's hard blocker dissolved: PR #380 implemented #344 and wrote docs/webhooks.md with the timestamped signature scheme in the same PR, so the docs were written against the correct format. Plan 012 is now UI-only. Three plans went stale within hours of being written, each differently: 008 was wrong on arrival, 006 was overtaken by unrelated work, 012 by its own dependency being solved better elsewhere. The drift check guards only the first, and only when the executor's baseline is the pinned commit. Also records that #370 and #380 both edit createConversation/deleteConversation and will conflict, plus a suggested merge order. Co-Authored-By: Duyet Le <me@duyet.net> Co-Authored-By: duyetbot <bot@duyet.net> --------- Co-authored-by: duyetbot <bot@duyet.net>
Summary
Closes #352.
The dashboard analytics endpoint (
GET /v1/projects/:id/analytics,packages/api/src/routes/analytics.ts) caches its aggregated result inAUTH_CACHE(KV) per project+range for 60-300s under the keyanalytics:public:{projectId}:{range}. Nothing ever busted that key on writes, so after a user deleted (or created) conversations, the dashboard could show stale — and in the delete case, too-high — counts for up to 5 minutes with no way to force a refresh.Fix chosen: explicit invalidation on write
Three options were on the table: explicit invalidation, a shorter TTL, or a cache-key version bump. Explicit invalidation is the simplest fix that's actually correct:
Since the cache key is range-specific but a write doesn't know which range(s) a user has open, the fix busts all three known ranges (
7d/30d/90d) for the project on write. This is centralized in a newpackages/api/src/lib/analytics-cache.ts(analyticsCacheKey,invalidateAnalyticsCache) so the key format has one source of truth (routes/analytics.ts now uses it too) and is called from bothcreateConversationanddeleteConversationinservices/conversations.ts— matching exactly the two write paths the issue names. Invalidation is fire-and-forget viaexecutionCtx.waitUntil, consistent with how the route already populates the cache.Scope notes (not fixed, flagged per review guidance)
Cache-Control: publicon genuinely static, unauthenticated routes (/apihealth check,/llms.txt,/agents.md,/openapi.json); the analytics routes require a Clerk session or API key (Authorization/session cookie present), and per the Workers Cache docs/comments in that commit, requests carrying auth headers are never cached at that layer. So Workers Cache is not serving stale analytics — only theAUTH_CACHEKV layer this PR fixes was the culprit.services/mcp-conversations.ts, used by the MCP toolstore_conversation/delete) with its owncreateConversation/deleteConversation, kept intentionally separate fromservices/conversations.tsper an existing code comment (different response contracts, no webhook trigger). It writes to the sameconversations/messagestables, so it has the same latent analytics-cache-staleness gap this issue described — just not the two functions the issue named. Left unaddressed here to keep this fix scoped to what's asked; worth a small follow-up if the MCP write path is a live traffic source for dashboard analytics viewers.Changes
packages/api/src/lib/analytics-cache.ts(new):ANALYTICS_CACHE_RANGES,analyticsCacheKey(),invalidateAnalyticsCache().packages/api/src/routes/analytics.ts: useanalyticsCacheKey()instead of an inline template literal (DRY with the new invalidation helper).packages/api/src/services/conversations.ts:createConversationanddeleteConversationnow accept the KV cache binding (anddeleteConversationnow also takesprojectId+executionCtx, needed to invalidate) and callinvalidateAnalyticsCacheafter a successful write.packages/api/src/routes/conversations/crud.ts: threadc.env.AUTH_CACHE(and for delete,existing.projectId+c.executionCtx) through to the service calls.packages/api/test/analytics.test.ts: removed the manualclearAnalyticsCache()test workaround (its own comment said "creating a conversation does not invalidate it" — that was the bug) and added a new test,"reflects deleted conversations immediately, with no stale cache (issue #352)", that populates the cache, deletes a conversation, and asserts the very next fetch (no manual cache clear) shows the decremented counts — this is the exact regression test for the reported bug. The two existing create-path tests were also tightened to assert no manual cache clear is needed anymore.Test plan
bunx biome check packages/api/src/— clean (one pre-existing, unrelated formatting diagnostic inlib/validation.tspredates this branch; confirmed viagit show main:...validation.ts | biome check --stdin-file-path=validation.ts -, left untouched per surgical-changes convention)bunx tsc --noEmit -p packages/api/tsconfig.json— no errorscd packages/api && bunx vitest run— 354/354 tests passed (25 test files), including the new/updated analytics cache-invalidation testsbun buildintentionally not run per instructionsCo-Authored-By: Duyet Le me@duyet.net
Co-Authored-By: duyetbot bot@duyet.net