[CSM Portal] Cap and stagger dashboard widget loading to reduce backend load - #1437
Conversation
|
Warning Review limit reached
Next review available in: 53 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds abort-signal support to backend POST requests, shared widget-fetch concurrency and retry control, and viewport-gated dashboard widget loading. It also adds tests for cancellation, retries, visibility behavior, and realistic lazy-loading scenarios. ChangesWidget request cancellation
Viewport-gated dashboard loading
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetGrid.realisticViewport.test.tsx (1)
130-138: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKey the simulated geometry to the node, not to a global observe counter.
observe()derives the row index fromnextObserveIndex, which increments on every call.disconnect()is a no-op, so a secondobserve()for the same tile consumes a new index and shifts every later tile's simulated position. React can run an effect more than once per element, for example under StrictMode or after a ref reattachment. If that happens, the expected count of 8 changes for a reason unrelated to the hook.Map each node to a stable index on first observe.
♻️ Proposed change
class GeometryIntersectionObserver { static instances: GeometryIntersectionObserver[] = []; static nextObserveIndex = 0; + static nodeIndexes = new Map<Element, number>(); callback: IntersectionObserverCallback; marginPx: number; threshold: number; @@ observe(node: Element): void { - const index = GeometryIntersectionObserver.nextObserveIndex; - GeometryIntersectionObserver.nextObserveIndex += 1; + let index = GeometryIntersectionObserver.nodeIndexes.get(node); + if (index === undefined) { + index = GeometryIntersectionObserver.nextObserveIndex; + GeometryIntersectionObserver.nextObserveIndex += 1; + GeometryIntersectionObserver.nodeIndexes.set(node, index); + } const visible = isVisible(index, this.marginPx, this.threshold);Reset the map in
beforeEachnext to the existing counter reset:GeometryIntersectionObserver.instances = []; GeometryIntersectionObserver.nextObserveIndex = 0; + GeometryIntersectionObserver.nodeIndexes = new Map();🤖 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 `@apps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetGrid.realisticViewport.test.tsx` around lines 130 - 138, Update GeometryIntersectionObserver.observe to assign each node a stable index on its first observation and reuse that index for subsequent observations of the same node, instead of incrementing nextObserveIndex on every call. Add the corresponding node-to-index map and reset it in beforeEach alongside the existing counter reset, preserving the current visibility calculation and callback behavior.apps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetGrid.lazyLoad.test.tsx (1)
149-153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFlush a macrotask before the steady-state assertions, and drop the unused mapping.
Line 153 repeats the assertion that Line 147 already awaited. No macrotask runs between them, so a wrongly-gated third fetch would not yet be visible. Flush a macrotask inside
actbefore the assertion. Apply the same flush before Line 182, which asserts the same steady state after reportingisIntersecting: false.Lines 149-150 compute
fetchedIdsfrom the request bodies but never check identity, and the length check duplicates Line 147. Either assert the two fetched widget identities, or remove the two lines.♻️ Proposed change
- const fetchedIds = postMock.mock.calls.map(([, body]) => body); - expect(fetchedIds).toHaveLength(2); - // Give the (deliberately un-triggered) third tile a chance to have - // fired if the gating didn't hold — it must still not have. - expect(postMock).toHaveBeenCalledTimes(2); + // Give the (deliberately un-triggered) third tile a chance to have + // fired if the gating didn't hold — it must still not have. + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + expect(postMock).toHaveBeenCalledTimes(2);Based on learnings, do not rely on an unflushed negative assertion to detect delayed TanStack Query requests; flush a macrotask in
actfirst.🤖 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 `@apps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetGrid.lazyLoad.test.tsx` around lines 149 - 153, Update the lazy-load test’s steady-state assertions by flushing a macrotask inside act before the assertion after the initial intersection and again before the equivalent assertion after reporting isIntersecting: false, so delayed TanStack Query requests are observable. In the fetchedIds section, either assert the two expected widget identities or remove the unused mapping and redundant length assertion, while retaining the request-count check.Source: Learnings
🤖 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
`@apps/csm-portal/webapp/src/features/csm-dashboard/utils/widgetFetchConcurrency.test.ts`:
- Around line 25-59: Update the withWidgetFetchSlot test to assert FIFO start
order by comparing the recorded started array with the original request indices
in order. Keep the existing concurrency and result assertions unchanged, and add
the assertion after all calls have completed.
---
Nitpick comments:
In
`@apps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetGrid.lazyLoad.test.tsx`:
- Around line 149-153: Update the lazy-load test’s steady-state assertions by
flushing a macrotask inside act before the assertion after the initial
intersection and again before the equivalent assertion after reporting
isIntersecting: false, so delayed TanStack Query requests are observable. In the
fetchedIds section, either assert the two expected widget identities or remove
the unused mapping and redundant length assertion, while retaining the
request-count check.
In
`@apps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetGrid.realisticViewport.test.tsx`:
- Around line 130-138: Update GeometryIntersectionObserver.observe to assign
each node a stable index on its first observation and reuse that index for
subsequent observations of the same node, instead of incrementing
nextObserveIndex on every call. Add the corresponding node-to-index map and
reset it in beforeEach alongside the existing counter reset, preserving the
current visibility calculation and callback behavior.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2285c316-4723-4aa8-aaed-7a16fba0c5d7
📒 Files selected for processing (15)
apps/csm-portal/webapp/src/api/backend/client.tsapps/csm-portal/webapp/src/features/csm-admin/dashboards/components/WidgetEditorDialog.test.tsxapps/csm-portal/webapp/src/features/csm-dashboard/api/useWidgetData.test.tsxapps/csm-portal/webapp/src/features/csm-dashboard/api/useWidgetData.tsapps/csm-portal/webapp/src/features/csm-dashboard/api/useWidgetPieData.test.tsxapps/csm-portal/webapp/src/features/csm-dashboard/api/useWidgetPieData.tsapps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetGrid.lazyLoad.test.tsxapps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetGrid.realisticViewport.test.tsxapps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetTile.test.tsxapps/csm-portal/webapp/src/features/csm-dashboard/components/DashboardWidgetTile.tsxapps/csm-portal/webapp/src/features/csm-dashboard/pages/DashboardWidgetPreviewPage.test.tsxapps/csm-portal/webapp/src/features/csm-dashboard/utils/widgetFetchConcurrency.test.tsapps/csm-portal/webapp/src/features/csm-dashboard/utils/widgetFetchConcurrency.tsapps/csm-portal/webapp/src/hooks/useElementVisibleOnce.test.tsapps/csm-portal/webapp/src/hooks/useElementVisibleOnce.ts
2 participants
Purpose
Loading a CSM dashboard fires one data-fetch request per widget (and one per pie slice) essentially simultaneously, with no coordination between tiles. On dashboards with ~20 widgets, this produces bursts of concurrent downstream calls large enough that the entity-service can't service them within its own timeout budget, producing request timeouts and 5xx responses back to the browser for a chunk of the widgets on a normal page load — this is a production-observed issue, not a hypothetical.
Goals
Approach
widgetFetchConcurrency.ts): a small hand-rolled FIFO semaphore,withWidgetFetchSlot, wraps the actual API call insideuseWidgetData/useWidgetPieData. React Query's own loading state is untouched — a queued widget simply shows its existing loading skeleton for longer. Concurrency is currently set to 1 (fully sequential) via theWIDGET_FETCH_CONCURRENCY_LIMITconstant.useElementVisibleOnce.ts): a one-shotIntersectionObserverlatch — a widget's fetch doesn't fire until its tile has been observed on-screen at least once; once visible, it stays "loaded" (no refetch on scrolling away). Falls back to "load immediately" whenIntersectionObserverisn't available in the runtime.withWidgetFetchSlot+client.ts): each widget fetch is aborted viaAbortControllerafterWIDGET_FETCH_TIMEOUT_MS(10s), and — critically — the concurrency slot is released on timeout so the next queued widget can proceed.retrypredicate on the widget queries only (not the app's global retry policy). The retry re-enters the same FIFO queue, so it naturally lands after every widget that hadn't been attempted yet — no special priority, no re-created head-of-line blocking.No dashboard config, entity-service, BFF, or Ballerina changes — frontend-only.
User stories
As a CS engineer, opening a dashboard with many widgets no longer causes several of them to fail with a backend error — widgets load progressively, a slow one degrades to a retry instead of blocking everything else, and off-screen widgets don't load until scrolled into view.
Release note
Dashboard widgets now load with bounded concurrency, are deferred until scrolled into view, and time out with a retry instead of hanging indefinitely — reducing backend load and eliminating a class of widget-load failures on dashboards with many widgets.
Documentation
N/A — internal loading-behavior change, no user-facing config or documented API surface affected.
Training
N/A — no training content affected.
Certification
N/A — no certification exam content affected.
Marketing
N/A — internal reliability fix, not a marketed feature.
Automation tests
New/updated test files:
widgetFetchConcurrency.test.ts,useElementVisibleOnce.test.ts,useWidgetData.test.tsx,DashboardWidgetGrid.lazyLoad.test.tsx,DashboardWidgetGrid.realisticViewport.test.tsx, plus updates toDashboardWidgetTile.test.tsxanduseWidgetPieDatacall sites. Full dashboard/hooks regression suite: 26 files / 266 tests pass.tsc -bandeslintclean.Covered via component-level tests exercising the real
DashboardWidgetGridfan-out (not mocked in isolation): concurrency cap actually binds under 27 concurrent callers; a realistic 20-widget/4-per-row viewport fixture proves only genuinely on-screen widgets fetch; a timeout-then-retry ordering test proves the retry fires only after other queued widgets have already resolved, not before.Security checks
eslintandtsc -bran clean insteadSamples
N/A — no new samples.
Related PRs
None.
Migrations (if applicable)
N/A — no schema or data migration.
Test environment
Verified locally via
vitest/tsc/eslint, and manually against a live local CSM backend stack (dashboard widget grids for multiple real dashboard configs), Chrome (latest).Learning
Root-caused via production Choreo logs (
csm-portal-backend+customer-entity-service) showing bursts of simultaneous/cases/searchtimeouts correlated with dashboard page loads. No new dependency added — checkedpackage.jsonforp-limit/p-queuefirst; the concurrency/timeout/retry mechanisms needed were each small enough to hand-roll instead.Summary by CodeRabbit
New Features
Bug Fixes