Skip to content

feat: add RankingLimit filter with all/limit query params to cap or remove ranking row limits - #5624

Merged
akshaydeo merged 5 commits into
devfrom
07-28-fix_dashboard_rankings_limit_support_backend
Jul 28, 2026
Merged

akshaydeo merged 5 commits into
devfrom
07-28-fix_dashboard_rankings_limit_support_backend

Conversation

@impoiler

@impoiler impoiler commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Ranking queries (GetModelRankings, GetUserRankings, GetDimensionRankings) were previously hard-coded to return at most defaultMaxRankingsLimit (100) rows with no way for callers to override that cap. This made it impossible for the dashboard export to retrieve the full ranked list. This PR introduces a RankingLimit field on SearchFilters and exposes all / limit query parameters on the ranking HTTP endpoints so callers can request a custom cap or an uncapped result.

Changes

  • Added RankingLimit *int to SearchFilters and an EffectiveRankingLimit(defaultLimit int) int helper that resolves nil → store default, <= 0 → no cap, and > 0 → explicit cap.
  • Extracted a shared applyRankingLimit(q *gorm.DB, filters SearchFilters) *gorm.DB helper in rdb.go that replaces the inline .Limit(defaultMaxRankingsLimit) calls in GetModelRankings, GetUserRankings, and GetDimensionRankings.
  • Applied the same applyRankingLimit pattern to the materialized-view-backed ranking paths in matviews.go, which previously had no limit at all.
  • Added ParseRankingLimit in the HTTP handler layer to parse all=true (uncapped, for exports) and limit=<n> (explicit positive cap) query parameters, writing a 400 on invalid input.
  • Wired ParseRankingLimit into getModelRankings, getDimensionRankings, and getDashboard.
  • Added rdb_ranking_limit_test.go with an in-memory SQLite store that seeds more rows than the default cap and verifies all four cases: default cap, explicit limit, limit above row count, and all=true.

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

go test ./framework/logstore/... -run TestGetDimensionRankingsRespectsRankingLimit
go test ./framework/logstore/... -run TestGetModelRankingsRespectsRankingLimit
go test ./...

To exercise the HTTP layer manually, call a ranking endpoint with the new parameters:

# Return at most 10 rows
GET /api/logs/rankings?limit=10

# Return every ranked entity (export use-case)
GET /api/logs/rankings?all=true

# Invalid inputs return HTTP 400
GET /api/logs/rankings?limit=0
GET /api/logs/rankings?all=maybe

Breaking changes

  • Yes
  • No

Related issues

Security considerations

The limit parameter is validated to be a positive integer before use; all=true is validated as a boolean. No user-supplied value is interpolated into SQL — the value is passed to GORM's .Limit() method only.

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

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Ranking queries now accept caller-controlled limits through HTTP filters. The effective limit is applied to RDB aggregations and PostgreSQL matview queries, with tests covering default, explicit, oversized, and uncapped ranking requests.

Changes

Ranking limit propagation

Layer / File(s) Summary
Ranking limit contract and HTTP parsing
framework/logstore/tables.go, transports/bifrost-http/handlers/logging.go
SearchFilters adds RankingLimit and resolves default, positive, and uncapped values. Ranking handlers parse all and limit parameters and reject invalid input.
RDB ranking limit application and validation
framework/logstore/rdb.go, framework/logstore/rdb_ranking_limit_test.go
Model, user, and dimension aggregations use the effective ranking limit. SQLite tests cover default, explicit, oversized, uncapped, and grouped ranking results.
Matview ranking limit application
framework/logstore/matviews.go
Model, user, and dimension matview queries apply the shared limit helper before fetching results.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant HTTPClient
  participant RankingHandlers
  participant RDBLogStore
  participant RankingQuery
  HTTPClient->>RankingHandlers: Send all or limit parameters
  RankingHandlers->>RDBLogStore: Invoke ranking endpoint with SearchFilters
  RDBLogStore->>RankingQuery: Apply effective ranking limit
  RankingQuery-->>RDBLogStore: Return capped or uncapped rankings
  RDBLogStore-->>RankingHandlers: Return ranking results
Loading

Possibly related PRs

Suggested reviewers: akshaydeo

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: configurable ranking limits via RankingLimit and new all/limit query params.
Description check ✅ Passed The description matches the template well, covering summary, changes, type, affected areas, testing, breaking changes, security, and checklist.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 07-28-fix_dashboard_rankings_limit_support_backend

Comment @coderabbitai help to get the list of available commands.

@impoiler impoiler self-assigned this Jul 28, 2026
@impoiler impoiler changed the title fix: dashboard rankings limit support backend feat: add RankingLimit filter with all/limit query params to cap or remove ranking row limits Jul 28, 2026
@impoiler
impoiler force-pushed the 07-28-fix_loading_flickering_for_empty_state branch from 1697ed0 to 45e73e3 Compare July 28, 2026 15:19
@impoiler
impoiler force-pushed the 07-28-fix_dashboard_rankings_limit_support_backend branch from a1086b9 to 9e5adbb Compare July 28, 2026 15:19
@impoiler
impoiler marked this pull request as ready for review July 28, 2026 15:24
@coderabbitai
coderabbitai Bot requested a review from akshaydeo July 28, 2026 15:31

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@framework/logstore/rdb_ranking_limit_test.go`:
- Around line 73-88: Update TestGetModelRankingsRespectsRankingLimit and its
setup so seeded rows use distinct model values while retaining valid provider
data, then assert that the unlimited query returns all seeded rankings and
RankingLimit=1 returns exactly one. Ensure the assertions distinguish uncapped
from capped behavior rather than merely verifying non-empty 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: df5ad2d3-4dda-44cc-a6f8-375da9bc4136

📥 Commits

Reviewing files that changed from the base of the PR and between 45e73e3 and 9e5adbb.

📒 Files selected for processing (5)
  • framework/logstore/matviews.go
  • framework/logstore/rdb.go
  • framework/logstore/rdb_ranking_limit_test.go
  • framework/logstore/tables.go
  • transports/bifrost-http/handlers/logging.go

Comment thread framework/logstore/rdb_ranking_limit_test.go
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 28, 2026

akshaydeo commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Merge activity

  • Jul 28, 8:46 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jul 28, 8:54 PM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo
akshaydeo changed the base branch from 07-28-fix_loading_flickering_for_empty_state to graphite-base/5624 July 28, 2026 20:51
@akshaydeo
akshaydeo changed the base branch from graphite-base/5624 to dev July 28, 2026 20:54
@akshaydeo
akshaydeo dismissed coderabbitai[bot]’s stale review July 28, 2026 20:54

The base branch was changed.

@akshaydeo
akshaydeo merged commit 3341ecf into dev Jul 28, 2026
10 checks passed
@akshaydeo
akshaydeo deleted the 07-28-fix_dashboard_rankings_limit_support_backend branch July 28, 2026 20:54
akhsaul pushed a commit to akhsaul/bifrost that referenced this pull request Aug 27, 2026
…p or remove ranking row limits (maximhq#5624)

## Summary

Ranking queries (`GetModelRankings`, `GetUserRankings`, `GetDimensionRankings`) were previously hard-coded to return at most `defaultMaxRankingsLimit` (100) rows with no way for callers to override that cap. This made it impossible for the dashboard export to retrieve the full ranked list. This PR introduces a `RankingLimit` field on `SearchFilters` and exposes `all` / `limit` query parameters on the ranking HTTP endpoints so callers can request a custom cap or an uncapped result.

## Changes

- Added `RankingLimit *int` to `SearchFilters` and an `EffectiveRankingLimit(defaultLimit int) int` helper that resolves `nil` → store default, `<= 0` → no cap, and `> 0` → explicit cap.
- Extracted a shared `applyRankingLimit(q *gorm.DB, filters SearchFilters) *gorm.DB` helper in `rdb.go` that replaces the inline `.Limit(defaultMaxRankingsLimit)` calls in `GetModelRankings`, `GetUserRankings`, and `GetDimensionRankings`.
- Applied the same `applyRankingLimit` pattern to the materialized-view-backed ranking paths in `matviews.go`, which previously had no limit at all.
- Added `ParseRankingLimit` in the HTTP handler layer to parse `all=true` (uncapped, for exports) and `limit=<n>` (explicit positive cap) query parameters, writing a 400 on invalid input.
- Wired `ParseRankingLimit` into `getModelRankings`, `getDimensionRankings`, and `getDashboard`.
- Added `rdb_ranking_limit_test.go` with an in-memory SQLite store that seeds more rows than the default cap and verifies all four cases: default cap, explicit limit, limit above row count, and `all=true`.

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./framework/logstore/... -run TestGetDimensionRankingsRespectsRankingLimit
go test ./framework/logstore/... -run TestGetModelRankingsRespectsRankingLimit
go test ./...
```

To exercise the HTTP layer manually, call a ranking endpoint with the new parameters:

```sh
# Return at most 10 rows
GET /api/logs/rankings?limit=10

# Return every ranked entity (export use-case)
GET /api/logs/rankings?all=true

# Invalid inputs return HTTP 400
GET /api/logs/rankings?limit=0
GET /api/logs/rankings?all=maybe
```

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

The `limit` parameter is validated to be a positive integer before use; `all=true` is validated as a boolean. No user-supplied value is interpolated into SQL — the value is passed to GORM's `.Limit()` method only.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
occcat pushed a commit to occcat/bifrost that referenced this pull request Sep 2, 2026
…p or remove ranking row limits (maximhq#5624)

## Summary

Ranking queries (`GetModelRankings`, `GetUserRankings`, `GetDimensionRankings`) were previously hard-coded to return at most `defaultMaxRankingsLimit` (100) rows with no way for callers to override that cap. This made it impossible for the dashboard export to retrieve the full ranked list. This PR introduces a `RankingLimit` field on `SearchFilters` and exposes `all` / `limit` query parameters on the ranking HTTP endpoints so callers can request a custom cap or an uncapped result.

## Changes

- Added `RankingLimit *int` to `SearchFilters` and an `EffectiveRankingLimit(defaultLimit int) int` helper that resolves `nil` → store default, `<= 0` → no cap, and `> 0` → explicit cap.
- Extracted a shared `applyRankingLimit(q *gorm.DB, filters SearchFilters) *gorm.DB` helper in `rdb.go` that replaces the inline `.Limit(defaultMaxRankingsLimit)` calls in `GetModelRankings`, `GetUserRankings`, and `GetDimensionRankings`.
- Applied the same `applyRankingLimit` pattern to the materialized-view-backed ranking paths in `matviews.go`, which previously had no limit at all.
- Added `ParseRankingLimit` in the HTTP handler layer to parse `all=true` (uncapped, for exports) and `limit=<n>` (explicit positive cap) query parameters, writing a 400 on invalid input.
- Wired `ParseRankingLimit` into `getModelRankings`, `getDimensionRankings`, and `getDashboard`.
- Added `rdb_ranking_limit_test.go` with an in-memory SQLite store that seeds more rows than the default cap and verifies all four cases: default cap, explicit limit, limit above row count, and `all=true`.

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./framework/logstore/... -run TestGetDimensionRankingsRespectsRankingLimit
go test ./framework/logstore/... -run TestGetModelRankingsRespectsRankingLimit
go test ./...
```

To exercise the HTTP layer manually, call a ranking endpoint with the new parameters:

```sh
# Return at most 10 rows
GET /api/logs/rankings?limit=10

# Return every ranked entity (export use-case)
GET /api/logs/rankings?all=true

# Invalid inputs return HTTP 400
GET /api/logs/rankings?limit=0
GET /api/logs/rankings?all=maybe
```

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

The `limit` parameter is validated to be a positive integer before use; `all=true` is validated as a boolean. No user-supplied value is interpolated into SQL — the value is passed to GORM's `.Limit()` method only.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants