feat(dashboards): add UI pagination to DataTable, fix agent-sessions truncation#1811
feat(dashboards): add UI pagination to DataTable, fix agent-sessions truncation#1811
Conversation
…ns truncation Add reusable pagination controls (Prev/Next, page info) to DataTable legacy API. Defaults to 100 rows/page; controls only appear when there are multiple pages. Setting pageSize=0 disables pagination for existing callers that pass large datasets they've already paginated server-side. Also fixes agent-sessions-content.tsx: the hardcoded ?limit=500 fetch of session logs is replaced with fetchAllPaginated, preventing silent truncation when sessions exceed 500 rows. Closes #1717 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ 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 latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/app/internal/agent-sessions/agent-sessions-content.tsx (1)
44-60:⚠️ Potential issue | 🟠 MajorSurface paginated enrichment failures instead of dropping them.
If the fetch on Line 45 fails, this still returns
ok: trueand renders agent sessions without the PR/cost/title enrichment. That makes the dashboard silently partial again, even though the PR objective calls for clear error/partial-data indicators when pagination fails. Please propagate a warning/error state to the UI (DataSourceBanneror an inline notice) instead of swallowinglogsResult.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/internal/agent-sessions/agent-sessions-content.tsx` around lines 44 - 60, The current code swallows partial/failed pagination by treating logsResult as success and silently building logsByBranch; update the fetch-and-enrich flow to detect incomplete pagination (e.g., a returned flag or mismatched counts from fetchAllPaginated) and propagate that state into the UI via an error/warning flag (render a DataSourceBanner or inline notice). Concretely: after calling fetchAllPaginated<SessionRow> (fetchAllPaginated -> logsResult), check for an explicit incomplete/partial/failure indicator on logsResult (or compare expected totals vs received items) and set a local warning state (e.g., logsFetchWarning) that the component uses to render DataSourceBanner; still build logsByBranch from available items but ensure the UI shows the warning when pagination didn't fully succeed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/ui/data-table.tsx`:
- Around line 47-48: The DataTable prop change made pagination opt-out by
default; restore opt-in behavior by setting pageSize's default to disabled (0 or
undefined) so existing callers without pageSize keep no pagination. Update the
DataTable component's prop default for pageSize (and any related handling inside
the DataTable render logic that reads pageSize) to treat missing value as
0/disabled, and ensure any internal pagination logic (e.g., where pageSize is
used to compute pages) respects 0 as “no pagination.” This keeps legacy callers
unaffected and requires explicit pageSize where pagination is desired.
---
Outside diff comments:
In `@apps/web/src/app/internal/agent-sessions/agent-sessions-content.tsx`:
- Around line 44-60: The current code swallows partial/failed pagination by
treating logsResult as success and silently building logsByBranch; update the
fetch-and-enrich flow to detect incomplete pagination (e.g., a returned flag or
mismatched counts from fetchAllPaginated) and propagate that state into the UI
via an error/warning flag (render a DataSourceBanner or inline notice).
Concretely: after calling fetchAllPaginated<SessionRow> (fetchAllPaginated ->
logsResult), check for an explicit incomplete/partial/failure indicator on
logsResult (or compare expected totals vs received items) and set a local
warning state (e.g., logsFetchWarning) that the component uses to render
DataSourceBanner; still build logsByBranch from available items but ensure the
UI shows the warning when pagination didn't fully succeed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0a8137ed-3552-4165-85d3-7d4aa9c18a68
📒 Files selected for processing (2)
apps/web/src/app/internal/agent-sessions/agent-sessions-content.tsxapps/web/src/components/ui/data-table.tsx
| /** Number of rows per page. Defaults to 100. Set to 0 to disable pagination. */ | ||
| pageSize?: number |
There was a problem hiding this comment.
Keep pagination opt-in for the legacy DataTable API.
Defaulting pageSize to 100 changes every existing caller that does not pass this prop. Tables like apps/web/src/app/internal/system-health/open-prs-table.tsx:180-187 and apps/web/src/app/internal/session-insights/insights-table.tsx:72-77 will now paginate after 100 rows without any call-site change, so this is a shared-component regression rather than a targeted dashboard adoption. Prefer keeping the default disabled and passing pageSize explicitly where you want pagination.
[suggested_recommended_refactor]
Proposed change
- /** Number of rows per page. Defaults to 100. Set to 0 to disable pagination. */
+ /** Number of rows per page. Omit or set to 0 to disable pagination. */
pageSize?: number
@@
- pageSize = 100,
+ pageSize = 0,Also applies to: 176-177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/ui/data-table.tsx` around lines 47 - 48, The
DataTable prop change made pagination opt-out by default; restore opt-in
behavior by setting pageSize's default to disabled (0 or undefined) so existing
callers without pageSize keep no pagination. Update the DataTable component's
prop default for pageSize (and any related handling inside the DataTable render
logic that reads pageSize) to treat missing value as 0/disabled, and ensure any
internal pagination logic (e.g., where pageSize is used to compute pages)
respects 0 as “no pagination.” This keeps legacy callers unaffected and requires
explicit pageSize where pagination is desired.
OAGr
added
claude-working
|
This PR shares 2 file(s) with PR #1822:
Coordinate to avoid merge conflicts. Posted by PR Patrol — informational only. |
1 participant
Summary
DataTablelegacy API (DataTableWithDataProps). Defaults to 100 rows/page; controls only appear when there are 2+ pages. SetpageSize=0to disable pagination (backwards-compatible for callers that have already paginated server-side).agent-sessions-content.tsxwas fetching/api/sessions?limit=500with a hardcoded ceiling. Replaced withfetchAllPaginatedto exhaust all pages, preventing silent truncation when session logs exceed 500 rows./internal/statementsand/internal/property-exploreralready correctly paginate through all API results. No truncation./api/sessions/insightsverification: Endpoint usesINSIGHTS_LIMIT = 5000with an explicit truncation warning in logs. Safe for current data volumes.Test plan
tsc --noEmit— clean, no errorspnpm test— 2675 tests passpnpm crux validate gate --fix— passedCloses #1717
Summary by CodeRabbit