perf: coalesce polling snapshots - #343
Conversation
mira-2026
added
priority: medium
|
Warning Review limit reached
Next review available in: 20 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: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds backend coalesced snapshots, cache invalidation, ETag responses, and telemetry, while centralizing frontend polling intervals and browser-aware WebSocket refresh behavior. Tests cover snapshot semantics, conditional HTTP responses, metrics, Docker caching, and socket reconnection. ChangesBackend caching and telemetry
Frontend refresh and socket coordination
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![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
🧹 Nitpick comments (1)
backend/src/routes/cronRoutes.ts (1)
37-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate cron-list normalization logic across three files — consolidate into
openClawCronSnapshot.ts.
cronRoutes.ts,taskAutomation.ts, andtaskRoutes.tseach independently re-implement the same "extract.jobs, else.items, else[]" fallback against the payload returned bygetOpenClawCronListSnapshot(). Since all three now consume the same shared snapshot, export a singlenormalizeOpenClawCronJobs(payload): CronJob[]helper fromopenClawCronSnapshot.tsand have each call site use it.
backend/src/routes/cronRoutes.ts#L37-L43: replace localnormalizeJobswith the shared helper.backend/src/services/taskAutomation.ts#L109-L114: replace localnormalizedCronJobswith the shared helper.backend/src/routes/taskRoutes.ts#L202-L220: replace the inline jobs/items fallback infetchCronJobsByIdwith the shared helper.🤖 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 `@backend/src/routes/cronRoutes.ts` around lines 37 - 43, Duplicate cron-job normalization exists across three call sites; consolidate it into one shared helper. Export normalizeOpenClawCronJobs(payload): CronJob[] from openClawCronSnapshot.ts, then replace cronRoutes.ts lines 37-43 local normalizeJobs, taskAutomation.ts lines 109-114 local normalizedCronJobs, and taskRoutes.ts lines 202-220 inline jobs/items fallback with calls to the shared helper, preserving the existing jobs-then-items-then-empty-array behavior.
🤖 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 `@backend/src/lib/coalescedSnapshot.ts`:
- Around line 23-28: Update SnapshotEntry and the read() flow so
retryAfterMs/nextRetryAt is enforced even when entry.value is absent. Record the
last load error when `#startLoad`() fails, set nextRetryAt, and have subsequent
reads before that time reuse or propagate the recorded failure instead of
starting another load; retain normal loading after the backoff expires and
existing cached-value behavior.
In `@backend/src/routes/dockerRoutes.ts`:
- Around line 759-761: Update the updater job flows for /api/docker/updater/run
and /api/docker/updater/services/:serviceId/update to invalidate read snapshots
after completion. Add invalidateDockerReadSnapshots() in finally blocks
surrounding both job waits, or route both through shared completion handling,
while preserving existing success and error behavior.
---
Nitpick comments:
In `@backend/src/routes/cronRoutes.ts`:
- Around line 37-43: Duplicate cron-job normalization exists across three call
sites; consolidate it into one shared helper. Export
normalizeOpenClawCronJobs(payload): CronJob[] from openClawCronSnapshot.ts, then
replace cronRoutes.ts lines 37-43 local normalizeJobs, taskAutomation.ts lines
109-114 local normalizedCronJobs, and taskRoutes.ts lines 202-220 inline
jobs/items fallback with calls to the shared helper, preserving the existing
jobs-then-items-then-empty-array behavior.
🪄 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: b4690447-85e2-4253-b9cf-090880614a02
📒 Files selected for processing (32)
backend/src/http.tsbackend/src/lib/coalescedSnapshot.tsbackend/src/lib/processes.tsbackend/src/routes/agentRoutes.tsbackend/src/routes/cronRoutes.tsbackend/src/routes/dockerRoutes.tsbackend/src/routes/metricsRoutes.tsbackend/src/routes/pullRequestRoutes.tsbackend/src/routes/taskRoutes.tsbackend/src/services/agents.tsbackend/src/services/openClawCronSnapshot.tsbackend/src/services/taskAutomation.tsbackend/test/coalescedSnapshot.test.tsbackend/test/httpCaching.test.tsbackend/test/routeAndServiceBehavior.test.tsbackend/test/setup.tssrc/components/features/chat/useChatHistory.tssrc/hooks/useAgents.tssrc/hooks/useBackups.tssrc/hooks/useCron.tssrc/hooks/useDelivery.tssrc/hooks/useDocker.tssrc/hooks/useHealth.tssrc/hooks/useJobExecutions.tssrc/hooks/useMetrics.tssrc/hooks/useOpenClawSocket.tssrc/hooks/useReports.tssrc/hooks/useScheduledJobs.tssrc/lib/queryClient.tssrc/lib/refreshPolicy.tssrc/lib/socket/socketClient.tssrc/test/frontendBehavior.test.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: backend-checks
- GitHub Check: frontend-checks
- GitHub Check: Analyze JavaScript and TypeScript
🔇 Additional comments (35)
backend/src/routes/dockerRoutes.ts (1)
2-3: LGTM!Also applies to: 310-315, 376-394, 683-691, 736-757, 763-788, 832-832
backend/src/routes/metricsRoutes.ts (1)
7-16: LGTM!Also applies to: 76-85, 346-364
backend/test/coalescedSnapshot.test.ts (1)
1-111: LGTM!backend/test/httpCaching.test.ts (1)
1-39: LGTM!backend/test/routeAndServiceBehavior.test.ts (1)
2833-2848: LGTM!Also applies to: 3640-3643, 3656-3662
backend/test/setup.ts (1)
7-7: LGTM!Also applies to: 49-49
src/lib/refreshPolicy.ts (1)
1-15: LGTM!src/components/features/chat/useChatHistory.ts (1)
10-10: LGTM!Also applies to: 24-24, 247-252
src/hooks/useAgents.ts (1)
3-3: LGTM!Also applies to: 45-45, 67-67, 78-78
src/hooks/useCron.ts (1)
3-3: LGTM!Also applies to: 44-44
src/hooks/useDocker.ts (1)
3-3: LGTM!Also applies to: 210-211, 285-285, 301-301
src/hooks/useHealth.ts (1)
3-3: LGTM!Also applies to: 39-39
src/hooks/useReports.ts (1)
3-3: LGTM!Also applies to: 52-52
src/hooks/useMetrics.ts (1)
37-56: LGTM!src/hooks/useBackups.ts (1)
3-3: LGTM!Also applies to: 43-43, 56-56
src/hooks/useDelivery.ts (1)
4-4: LGTM!Also applies to: 168-168, 290-292, 337-337
src/hooks/useJobExecutions.ts (1)
8-8: LGTM!Also applies to: 84-84
src/hooks/useScheduledJobs.ts (1)
3-3: LGTM!Also applies to: 122-125, 139-144
src/lib/queryClient.ts (1)
3-6: LGTM!src/hooks/useOpenClawSocket.ts (1)
18-18: LGTM!Also applies to: 59-59, 79-100, 121-123, 171-171, 247-265, 277-277, 292-292, 314-314
src/lib/socket/socketClient.ts (1)
3-3: LGTM!Also applies to: 14-34, 89-90, 118-138, 214-224, 313-313, 328-329
src/test/frontendBehavior.test.tsx (1)
205-205: LGTM!Also applies to: 1750-1812
backend/src/routes/pullRequestRoutes.ts (4)
1-2: LGTM!Also applies to: 44-78, 80-95
109-113: LGTM!Also applies to: 131-135, 146-150, 161-163, 174-184, 195-202
211-216: LGTM!Also applies to: 250-256
265-265: LGTM!Also applies to: 274-274
backend/src/http.ts (1)
1-2: LGTM!Also applies to: 61-92
backend/src/lib/coalescedSnapshot.ts (1)
1-158: LGTM!Also applies to: 199-238
backend/src/lib/processes.ts (1)
19-35: LGTM!Also applies to: 80-80, 89-110
backend/src/services/agents.ts (1)
7-7: LGTM!Also applies to: 409-409, 483-493
backend/src/services/openClawCronSnapshot.ts (1)
1-19: LGTM!backend/src/services/taskAutomation.ts (1)
3-6: LGTM!Also applies to: 182-182
backend/src/routes/agentRoutes.ts (2)
1-2: Import summary doesn't match code:jsonis still imported and used.The line-range summary states the
jsonimport was removed, but line 1 still importsjsonfrom../http.ts, and it's used throughout this file (agentError,missingConfig, and every route handler). The code should be treated as authoritative here.
27-44: LGTM!Also applies to: 59-63, 105-105
backend/src/routes/cronRoutes.ts (1)
13-16: LGTM!Also applies to: 52-59, 86-88, 100-104, 123-127, 161-167, 191-204
![chatgpt-codex-connector[bot]](https://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: e299b1d6b7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@backend/src/services/openClawCronSnapshot.ts`:
- Around line 21-27: Update normalizeOpenClawCronJobs to validate the outer
payload and each cron entry before returning results: reject array payloads,
require jobs/items to be arrays of non-null objects, and treat an empty object
or missing/invalid list as an unavailable/error snapshot rather than a healthy
empty list. Preserve the generic return type only for validated entries and
update callers as needed to handle the rejected-payload outcome.
🪄 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: 133afc41-5a8f-4bc7-b0ab-0ade61381b57
📒 Files selected for processing (9)
backend/src/lib/coalescedSnapshot.tsbackend/src/routes/cronRoutes.tsbackend/src/routes/dockerRoutes.tsbackend/src/routes/taskRoutes.tsbackend/src/services/openClawCronSnapshot.tsbackend/src/services/taskAutomation.tsbackend/test/coalescedSnapshot.test.tsbackend/test/routeAndServiceBehavior.test.tssrc/hooks/useOpenClawSocket.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- backend/src/routes/taskRoutes.ts
- backend/test/coalescedSnapshot.test.ts
- backend/src/routes/dockerRoutes.ts
- src/hooks/useOpenClawSocket.ts
- backend/test/routeAndServiceBehavior.test.ts
- backend/src/routes/cronRoutes.ts
- backend/src/lib/coalescedSnapshot.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Analyze JavaScript and TypeScript
- GitHub Check: frontend-checks
- GitHub Check: backend-checks
🔇 Additional comments (2)
backend/src/services/openClawCronSnapshot.ts (1)
1-9: LGTM!Also applies to: 11-19, 29-32
backend/src/services/taskAutomation.ts (1)
6-9: LGTM!Also applies to: 173-175
3 participants
Summary
sessions.listrefreshesSafety
Documentation
Docs: not needed — this changes internal polling, caching, and reconnect behavior without changing user-facing or operator contracts.
Verification
bun run format:checkbun run lintbun run buildbun run test— frontend 485 passed; backend 601 passedbun run test:frontend:coverage— 94.59% (required 85%)bun run test:backend:coverage— 90.26% (required 85%)