fix: preserve active time period when applying dimension filters - #5030
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 40 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR changes dashboard and logs filter handling so absolute time fields are only written when both ChangesFilter and query param preservation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Confidence Score: 5/5Safe to merge — all three files make narrowly scoped, correct changes to URL state management with no regressions to existing functionality. The time-guard logic is sound: both hasExplicitTime checks correctly require both timestamps to be present before touching time-related URL state. The timeChanged comparisons are type-compatible with the existing URL state representations. The functional search merge in logDetailView.tsx is the standard TanStack Router pattern for partial state updates, and adding offset: 0 and selected_log addresses the two edge cases flagged in prior review threads. No data-testid attributes are removed and no browser crypto APIs are introduced. No files require special attention. Important Files Changed
Reviews (16): Last reviewed commit: "fix: unwated filters resets on dashboard..." | Re-trigger Greptile |
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/logs/sheets/logDetailView.tsx`:
- Around line 743-749: The detail-sheet filter links are preserving the current
pagination state because the Link search updater spreads prev without resetting
offset. Update each search callback in logDetailView.tsx (the badge/link
handlers such as the routing_rule link) to explicitly set offset to 0 alongside
the new filter ids so navigation always lands on the first page of the filtered
results.
🪄 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: 84019987-d78e-4e5b-b784-00173895bed7
📒 Files selected for processing (3)
ui/app/workspace/dashboard/page.tsxui/app/workspace/logs/page.tsxui/app/workspace/logs/sheets/logDetailView.tsx
efee0f3 to
73efcd9
Compare
a2be949 to
d19f0cc
Compare
73efcd9 to
e121cb6
Compare
d19f0cc to
d8627f7
Compare
e121cb6 to
b9db7f9
Compare
d8627f7 to
964cf81
Compare
b9db7f9 to
47c47cc
Compare
964cf81 to
becea36
Compare
47c47cc to
3367726
Compare
becea36 to
0475258
Compare
3367726 to
7c2fe3c
Compare
0475258 to
fab2b0f
Compare
7c2fe3c to
fdc4799
Compare
fab2b0f to
a09364b
Compare
37721c8 to
f823121
Compare
f823121 to
4e48235
Compare
a09364b to
62560d6
Compare
4e48235 to
eab1035
Compare
9491a4b to
ca6e660
Compare
eab1035 to
6ae1cc0
Compare
Merge activity
|
The base branch was changed.
6ae1cc0 to
9dd49c4
Compare
## Summary
Fixes a bug where applying dimension filters (providers, models, keys, etc.) from the sidebar or header would inadvertently clear the active time period. In period mode, `newFilters` carries no `start_time`/`end_time`, so the previous logic was unconditionally writing `undefined` for those values and wiping the selected period. Time state is now only updated when an explicit start and end time are both present.
Also fixes navigation links in the log detail view that were replacing the entire search state instead of merging into it, which caused the active time range and other filters to be lost when clicking through to filtered log views.
## Changes
- `setFilters` on both the dashboard and logs pages now checks for `hasExplicitTime` before touching time-related URL state. If no explicit range is provided, the existing `period`, `start_time`, and `end_time` values are left untouched.
- `start_time`/`end_time` are now written inside the conditional `timeChanged` block rather than unconditionally, removing the path that could set them to `undefined`.
- All `search={{ ... }}` props on filter links in `logDetailView.tsx` have been changed to `search={(prev) => ({ ...prev, ... })}` so that existing URL state (including the active time range) is preserved when navigating.
## Type of change
- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (React)
- [ ] Docs
## How to test
1. Set a relative time period (e.g. "Last 24 hours") on the Logs or Dashboard page.
2. Apply a dimension filter (e.g. select a provider or model) from the sidebar or header.
3. Verify the active time period is still selected and the data is filtered correctly — it should not revert to an empty/default time range.
4. Open a log detail view, click a filter link (e.g. a routing rule, key, team, or customer).
5. Verify the logs page opens with both the clicked filter and the previously active time range applied.
```sh
cd ui
pnpm i || npm i
pnpm build || npm run build
```
## 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
…imhq#5030) ## Summary Fixes a bug where applying dimension filters (providers, models, keys, etc.) from the sidebar or header would inadvertently clear the active time period. In period mode, `newFilters` carries no `start_time`/`end_time`, so the previous logic was unconditionally writing `undefined` for those values and wiping the selected period. Time state is now only updated when an explicit start and end time are both present. Also fixes navigation links in the log detail view that were replacing the entire search state instead of merging into it, which caused the active time range and other filters to be lost when clicking through to filtered log views. ## Changes - `setFilters` on both the dashboard and logs pages now checks for `hasExplicitTime` before touching time-related URL state. If no explicit range is provided, the existing `period`, `start_time`, and `end_time` values are left untouched. - `start_time`/`end_time` are now written inside the conditional `timeChanged` block rather than unconditionally, removing the path that could set them to `undefined`. - All `search={{ ... }}` props on filter links in `logDetailView.tsx` have been changed to `search={(prev) => ({ ...prev, ... })}` so that existing URL state (including the active time range) is preserved when navigating. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test 1. Set a relative time period (e.g. "Last 24 hours") on the Logs or Dashboard page. 2. Apply a dimension filter (e.g. select a provider or model) from the sidebar or header. 3. Verify the active time period is still selected and the data is filtered correctly — it should not revert to an empty/default time range. 4. Open a log detail view, click a filter link (e.g. a routing rule, key, team, or customer). 5. Verify the logs page opens with both the clicked filter and the previously active time range applied. ```sh cd ui pnpm i || npm i pnpm build || npm run build ``` ## 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
Fixes a bug where applying dimension filters (providers, models, keys, etc.) from the sidebar or header would inadvertently clear the active time period. In period mode,
newFilterscarries nostart_time/end_time, so the previous logic was unconditionally writingundefinedfor those values and wiping the selected period. Time state is now only updated when an explicit start and end time are both present.Also fixes navigation links in the log detail view that were replacing the entire search state instead of merging into it, which caused the active time range and other filters to be lost when clicking through to filtered log views.
Changes
setFilterson both the dashboard and logs pages now checks forhasExplicitTimebefore touching time-related URL state. If no explicit range is provided, the existingperiod,start_time, andend_timevalues are left untouched.start_time/end_timeare now written inside the conditionaltimeChangedblock rather than unconditionally, removing the path that could set them toundefined.search={{ ... }}props on filter links inlogDetailView.tsxhave been changed tosearch={(prev) => ({ ...prev, ... })}so that existing URL state (including the active time range) is preserved when navigating.Type of change
Affected areas
How to test
Breaking changes
Related issues
Security considerations
None.
Checklist
docs/contributing/README.mdand followed the guidelines