feat: persist logs page-size preference in localStorage and exclude error_details from list query - #5327
Conversation
📝 WalkthroughWalkthroughThe log list query no longer returns ChangesLog list projection
Logs pagination preference
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant User
participant LogsTable
participant PageSizePreference
participant localStorage
participant Pagination
User->>LogsTable: select rows-per-page value
LogsTable->>PageSizePreference: update selected page size
PageSizePreference->>localStorage: persist selected page size
LogsTable->>Pagination: update limit and reset offset
Pagination->>LogsTable: render updated page controls
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
error_details from list queryerror_details from list query
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (4): Last reviewed commit: "feat: Configurable page size for logs ta..." | Re-trigger Greptile |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ui/lib/hooks/useTablePageSizePreference.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the
"use client"directive — this project uses TanStack Router, not Next.js App Router.The
"use client"directive is a Next.js-specific construct that has no effect in the current Vite/TanStack Router toolchain. Introducing it can confuse contributors about the project's routing framework. Based on learnings, the UI code underui/should not include Next.js App Router conventions such as"use client"directives.♻️ Proposed fix
-"use client"; - import { useCallback, useEffect, useState } from "react";🤖 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 `@ui/lib/hooks/useTablePageSizePreference.ts` at line 1, Remove the top-level "use client" directive from useTablePageSizePreference.ts, leaving the hook implementation unchanged and preserving the existing TanStack Router/Vite conventions.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.
Nitpick comments:
In `@ui/lib/hooks/useTablePageSizePreference.ts`:
- Line 1: Remove the top-level "use client" directive from
useTablePageSizePreference.ts, leaving the hook implementation unchanged and
preserving the existing TanStack Router/Vite conventions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3533eba4-125c-4b0d-bfca-5d1e8628c25c
📒 Files selected for processing (3)
framework/logstore/rdb.goui/app/workspace/logs/views/logsTable.tsxui/lib/hooks/useTablePageSizePreference.ts
90632b8 to
7df1167
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/lib/hooks/useTablePageSizePreference.ts`:
- Around line 27-31: Update useTablePageSizePreference to restrict the
defaultPageSize, hydrated localStorage value, and setPageSize input to values
included in DEFAULT_PAGE_SIZE_OPTIONS. Fall back to the supported default when
validation fails, and ensure the returned pageSize never reaches LogsDataTable
as an unsupported value.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 29fb0cb0-ca8a-4dc8-b9f2-b1c982f014d1
📒 Files selected for processing (3)
framework/logstore/rdb.goui/app/workspace/logs/views/logsTable.tsxui/lib/hooks/useTablePageSizePreference.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- ui/app/workspace/logs/views/logsTable.tsx
- framework/logstore/rdb.go
7df1167 to
000ff52
Compare
Merge activity
|
The base branch was changed.
000ff52 to
785f9cc
Compare
…error_details` from list query (#5327) ## Summary This PR fixes a Cloud Run 500 error caused by the `/api/logs` response exceeding the 32MB body limit when many error-status log rows are returned, and replaces the auto-calculated table page size with a user-controlled, localStorage-persisted page size preference. ## Changes - **Exclude `error_details` from the logs list query**: The `error_details` column is dropped from `listSelectColumns` in `rdb.go` because it can carry unbounded provider error payloads. With 25+ such rows, the combined response body can exceed Cloud Run's 32MB limit and return a 500. The full error detail remains available via the individual log detail endpoint (`GET /api/logs/{id}`). - **Replace auto-sized page size with a user preference**: The `useTablePageSize` hook (which inferred page size from container height) is replaced by `useTablePageSizePreference`, which stores the user's chosen page size in localStorage under a per-table key (e.g. `bifrost.logs.pageSize`). The preference is hydrated lazily after mount to avoid SSR/hydration mismatches and defaults to 25. - **Add a "Rows per page" dropdown to the logs table footer**: A `ComboboxSelect` with options `[10, 25, 50, 100, 200]` is added to the pagination controls, allowing users to explicitly choose how many rows to load per page. The selection is persisted across sessions. ## Type of change - [x] Bug fix - [x] Feature ## Affected areas - [x] Core (Go) - [x] UI (React) ## How to test 1. Navigate to the logs table in the UI. 2. Confirm a "Rows per page" dropdown appears in the pagination footer with options 10, 25, 50, 100, and 200. 3. Select a page size, reload the page, and verify the selection is restored from localStorage. 4. Confirm that log list responses no longer include `error_details` in the payload, and that the full error is still visible when opening an individual log entry. 5. Verify that workspaces with many error-status logs no longer trigger 500 responses from `/api/logs`. ```sh # Core go test ./framework/logstore/... # UI cd ui pnpm i pnpm test pnpm build ``` ## Screenshots/Recordings Before: Page size was inferred from container height with no user control. After: A "Rows per page" dropdown is shown in the pagination bar, and the selection persists across page reloads. ## Breaking changes - [x] No The `error_details` field is removed from list responses but remains available on the detail endpoint. Clients relying on `error_details` in the list response will need to fetch individual log entries to retrieve it. ## Related issues Closes the Cloud Run 32MB body limit 500 error on `/api/logs`. ## Security considerations No new auth, secrets, or PII exposure. Removing `error_details` from the list response reduces the amount of potentially sensitive provider error data sent in bulk responses. ## 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
…error_details` from list query (#5327) ## Summary This PR fixes a Cloud Run 500 error caused by the `/api/logs` response exceeding the 32MB body limit when many error-status log rows are returned, and replaces the auto-calculated table page size with a user-controlled, localStorage-persisted page size preference. ## Changes - **Exclude `error_details` from the logs list query**: The `error_details` column is dropped from `listSelectColumns` in `rdb.go` because it can carry unbounded provider error payloads. With 25+ such rows, the combined response body can exceed Cloud Run's 32MB limit and return a 500. The full error detail remains available via the individual log detail endpoint (`GET /api/logs/{id}`). - **Replace auto-sized page size with a user preference**: The `useTablePageSize` hook (which inferred page size from container height) is replaced by `useTablePageSizePreference`, which stores the user's chosen page size in localStorage under a per-table key (e.g. `bifrost.logs.pageSize`). The preference is hydrated lazily after mount to avoid SSR/hydration mismatches and defaults to 25. - **Add a "Rows per page" dropdown to the logs table footer**: A `ComboboxSelect` with options `[10, 25, 50, 100, 200]` is added to the pagination controls, allowing users to explicitly choose how many rows to load per page. The selection is persisted across sessions. ## Type of change - [x] Bug fix - [x] Feature ## Affected areas - [x] Core (Go) - [x] UI (React) ## How to test 1. Navigate to the logs table in the UI. 2. Confirm a "Rows per page" dropdown appears in the pagination footer with options 10, 25, 50, 100, and 200. 3. Select a page size, reload the page, and verify the selection is restored from localStorage. 4. Confirm that log list responses no longer include `error_details` in the payload, and that the full error is still visible when opening an individual log entry. 5. Verify that workspaces with many error-status logs no longer trigger 500 responses from `/api/logs`. ```sh # Core go test ./framework/logstore/... # UI cd ui pnpm i pnpm test pnpm build ``` ## Screenshots/Recordings Before: Page size was inferred from container height with no user control. After: A "Rows per page" dropdown is shown in the pagination bar, and the selection persists across page reloads. ## Breaking changes - [x] No The `error_details` field is removed from list responses but remains available on the detail endpoint. Clients relying on `error_details` in the list response will need to fetch individual log entries to retrieve it. ## Related issues Closes the Cloud Run 32MB body limit 500 error on `/api/logs`. ## Security considerations No new auth, secrets, or PII exposure. Removing `error_details` from the list response reduces the amount of potentially sensitive provider error data sent in bulk responses. ## 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
This PR fixes a Cloud Run 500 error caused by the
/api/logsresponse exceeding the 32MB body limit when many error-status log rows are returned, and replaces the auto-calculated table page size with a user-controlled, localStorage-persisted page size preference.Changes
error_detailsfrom the logs list query: Theerror_detailscolumn is dropped fromlistSelectColumnsinrdb.gobecause it can carry unbounded provider error payloads. With 25+ such rows, the combined response body can exceed Cloud Run's 32MB limit and return a 500. The full error detail remains available via the individual log detail endpoint (GET /api/logs/{id}).useTablePageSizehook (which inferred page size from container height) is replaced byuseTablePageSizePreference, which stores the user's chosen page size in localStorage under a per-table key (e.g.bifrost.logs.pageSize). The preference is hydrated lazily after mount to avoid SSR/hydration mismatches and defaults to 25.ComboboxSelectwith options[10, 25, 50, 100, 200]is added to the pagination controls, allowing users to explicitly choose how many rows to load per page. The selection is persisted across sessions.Type of change
Affected areas
How to test
error_detailsin the payload, and that the full error is still visible when opening an individual log entry./api/logs.Screenshots/Recordings
Before: Page size was inferred from container height with no user control.
After: A "Rows per page" dropdown is shown in the pagination bar, and the selection persists across page reloads.
Breaking changes
The
error_detailsfield is removed from list responses but remains available on the detail endpoint. Clients relying onerror_detailsin the list response will need to fetch individual log entries to retrieve it.Related issues
Closes the Cloud Run 32MB body limit 500 error on
/api/logs.Security considerations
No new auth, secrets, or PII exposure. Removing
error_detailsfrom the list response reduces the amount of potentially sensitive provider error data sent in bulk responses.Checklist
docs/contributing/README.mdand followed the guidelines