refactor: extract dashboard tab data-fetching into dedicated TabView components - #3797
Conversation
|
Warning Review limit reached
More reviews will be available in 36 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR refactors the Dashboard page from a monolithic component with local RTK query orchestration into a modular tab-view architecture. Five new tab view components encapsulate data fetching and expose imperative ref APIs. The Dashboard page aggregates these refs to fetch and export data across all active tabs. ChangesDashboard Tab View Refactor
Sequence Diagram(s)sequenceDiagram
participant DashboardPage
participant TabViewRefs as Tab View Refs<br/>(Overview, Provider, etc.)
participant RTKQueries as RTK Query Hooks
participant TabPresentational as Tab Components<br/>(MCPTab, OverviewTab, etc.)
DashboardPage->>DashboardPage: parse filters from URL
DashboardPage->>TabViewRefs: pass filters, active state, pdfMode
Note over TabViewRefs: active = true for current tab
TabViewRefs->>RTKQueries: skip = false, fetch data
RTKQueries-->>TabViewRefs: histogram, rankings, stats
DashboardPage->>TabViewRefs: getDashboardData() for export
TabViewRefs-->>DashboardPage: aggregated Partial<DashboardData>
DashboardPage->>TabViewRefs: handlePreloadData() on PDF export
TabViewRefs->>RTKQueries: loadData() triggers all lazy queries
DashboardPage->>DashboardPage: handlePdfExport() enables pdfMode
DashboardPage->>TabPresentational: unhide force-mounted tabs, measure
TabPresentational-->>DashboardPage: rendered DOM ready for capture
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
cf7fb07 to
787e939
Compare
8089ce8 to
0ad0728
Compare
Confidence Score: 4/5Export paths have correctness issues with inactive tabs returning null data; the refactor itself is clean but export functionality needs resolution before merging The tab view components are well-structured. Risk is in getDashboardData and handlePdfExport: inactive tabs (skip=true) return null from getData(), and modelData is emitted by both OverviewTabView and ModelRankingsTabView causing one to overwrite the other. PDF export calls handlePreloadData before setPdfMode(true), so inactive tab refs are null and loadData() is never called for them. page.tsx — getDashboardData and handlePdfExport Important Files Changed
Reviews (6): Last reviewed commit: "fix: fetch only active tabs data in dash..." | Re-trigger Greptile |
eac19fe to
4d50443
Compare
11e3a09 to
18253b0
Compare
18253b0 to
be859cc
Compare
4d50443 to
ba36910
Compare
a957ea9 to
c6a89b9
Compare
712f0b2 to
75c1e18
Compare
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 `@ui/app/workspace/dashboard/page.tsx`:
- Around line 348-394: The PDF export's section ordering in handlePdfExport is
out of sync with the UI tab order: update the ids array inside handlePdfExport
so the element IDs follow the same visual tab sequence (move
"dashboard-section-user-rankings" to appear before
"dashboard-section-customer-rankings" and "dashboard-section-bu-rankings"), then
return the mapped elements as before; ensure you only change the order of the
IDs in the ids constant so exported PDF pages match the on-screen tabs.
🪄 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
Run ID: 7273bbc2-3fbb-442e-971d-b71468a54e3a
📒 Files selected for processing (8)
ui/app/workspace/dashboard/components/tabViews/dimensionRankingsTabView.tsxui/app/workspace/dashboard/components/tabViews/mcpTabView.tsxui/app/workspace/dashboard/components/tabViews/modelRankingsTabView.tsxui/app/workspace/dashboard/components/tabViews/overviewTabView.tsxui/app/workspace/dashboard/components/tabViews/providerUsageTabView.tsxui/app/workspace/dashboard/page.tsxui/lib/store/apis/logsApi.tsui/lib/store/apis/mcpLogsApi.ts
Merge activity
|
The base branch was changed.
c6a89b9 to
230db16
Compare
…` components (#3797) ## Summary Refactors the dashboard page by extracting per-tab data fetching logic into dedicated `*TabView` wrapper components, replacing the large collection of manual state variables, lazy query hooks, and `ensure*DataLoaded` ref-tracking functions that previously lived directly in `page.tsx`. ## Changes - Introduced five new `forwardRef` tab view components (`OverviewTabView`, `ProviderUsageTabView`, `MCPTabView`, `ModelRankingsTabView`, `DimensionRankingsTabView`), each owning its own RTK Query subscriptions, loading state, and `getData`/`loadData` imperative handle. - Replaced ~300 lines of manual state, lazy hooks, fetch functions, and generation-counter deduplication logic in `page.tsx` with refs to the new tab view components. Export data aggregation now iterates over those refs rather than a large `useMemo` object. - Each tab view uses an `active` prop (driven by the current tab value or `pdfMode`) to skip fetching when the tab is not visible, delegating the skip logic to RTK Query's built-in `skip` option instead of manual `fetchedRef`/`loadingRef` guards. - Exported `useGetModelRankingsQuery`, `useGetDimensionRankingsQuery`, `useGetMCPCostHistogramQuery`, and `useGetMCPTopToolsQuery` non-lazy hooks from the store APIs to support the new subscription-based approach. - Moved `sanitizeSeriesLabels` into the tab views that need it (`OverviewTabView`, `ProviderUsageTabView`) rather than keeping it in the page. - Reordered the "User Rankings" tab trigger to appear before "Customer Rankings" and "BU Rankings" in the tab list. - Removed the background warm-up `setTimeout` effect and the filter-change `useEffect` that reset all fetch flags, as RTK Query cache invalidation now handles re-fetching automatically. ## Type of change - [ ] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh cd ui pnpm i || npm i pnpm build || npm run build ``` 1. Open the dashboard and verify each tab loads its data correctly when first visited. 2. Change a filter (e.g., time range or provider) and confirm all visible and previously loaded tabs refresh. 3. Trigger a CSV and PDF export and confirm all tab data is included. 4. Verify PDF export correctly force-mounts all tabs and captures each section. ## Screenshots/Recordings No visual changes expected; this is a pure refactor of data-fetching logic. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations None. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable

Summary
Refactors the dashboard page by extracting per-tab data fetching logic into dedicated
*TabViewwrapper components, replacing the large collection of manual state variables, lazy query hooks, andensure*DataLoadedref-tracking functions that previously lived directly inpage.tsx.Changes
forwardReftab view components (OverviewTabView,ProviderUsageTabView,MCPTabView,ModelRankingsTabView,DimensionRankingsTabView), each owning its own RTK Query subscriptions, loading state, andgetData/loadDataimperative handle.page.tsxwith refs to the new tab view components. Export data aggregation now iterates over those refs rather than a largeuseMemoobject.activeprop (driven by the current tab value orpdfMode) to skip fetching when the tab is not visible, delegating the skip logic to RTK Query's built-inskipoption instead of manualfetchedRef/loadingRefguards.useGetModelRankingsQuery,useGetDimensionRankingsQuery,useGetMCPCostHistogramQuery, anduseGetMCPTopToolsQuerynon-lazy hooks from the store APIs to support the new subscription-based approach.sanitizeSeriesLabelsinto the tab views that need it (OverviewTabView,ProviderUsageTabView) rather than keeping it in the page.setTimeouteffect and the filter-changeuseEffectthat reset all fetch flags, as RTK Query cache invalidation now handles re-fetching automatically.Type of change
Affected areas
How to test
Screenshots/Recordings
No visual changes expected; this is a pure refactor of data-fetching logic.
Breaking changes
Related issues
Security considerations
None.
Checklist
docs/contributing/README.mdand followed the guidelines