feat: add limit and query params to filter data endpoints for server-side search and pagination - #3567
Merged
akshaydeo merged 1 commit intoMay 18, 2026
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds parameterized filtering and limiting to log store filter-data queries. ChangesFilter-data limit and query parameter propagation
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
Contributor
Author
This was referenced May 18, 2026
Merged
impoiler
changed the title
limit and query params to filter data endpoints for server-side search and pagination
impoiler
marked this pull request as ready for review
May 18, 2026 12:00
Contributor
Confidence Score: 4/5Safe to merge with a targeted fix: search returns incorrect results when The
Important Files Changed
Reviews (3): Last reviewed commit: "feat: add search functionality to the fi..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
framework/logstore/rdb.go (1)
2860-2887:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFilter split routing engines, not just the raw CSV column.
Searching
routing_engines_usedbeforestrings.Splitstill lets unrelated sibling values leak into the result. For example, a row containinga,bqueried withbwill currently return bothaandb.🎯 Proposed fix
// Each row may contain comma-separated values; deduplicate across all rows uniqueEngines := make(map[string]struct{}) + lowerQ := strings.ToLower(strings.TrimSpace(query)) for _, raw := range rawValues { for _, engine := range strings.Split(raw, ",") { engine = strings.TrimSpace(engine) - if engine != "" { + if engine != "" && (lowerQ == "" || strings.Contains(strings.ToLower(engine), lowerQ)) { uniqueEngines[engine] = struct{}{} } } } engines := make([]string, 0, len(uniqueEngines)) @@ - if len(engines) > limit { + if limit > 0 && len(engines) > limit { engines = engines[:limit] }🤖 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 `@framework/logstore/rdb.go` around lines 2860 - 2887, The current row-level filter on "routing_engines_used" lets sibling values leak (e.g. row "a,b" matches query "b" but you then return "a"), so stop filtering rows and instead filter after splitting: remove the call that applies applyLikeFilter to q (the row-level filter on routing_engines_used), retrieve rawValues as you do now, then when iterating rawValues and splitting into engine (the loop that trims and dedups into uniqueEngines), only add an engine when query == "" OR the engine itself matches the query using the same LIKE semantics (case-insensitive substring or your existing applyLikeFilter semantics applied at the string level); keep deduping via uniqueEngines, then sort and apply the limit to engines as before.
🤖 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.go`:
- Around line 2770-2776: The DISTINCT queries (e.g., the query built in the
GetDistinct/GetAvailable model methods using
s.db.WithContext(ctx).Model(&Log{})...Distinct("model")) apply LIMIT without a
stable ORDER, causing nondeterministic subsets; fix by applying a deterministic
Order (for example Order("model ASC") or Order("timestamp DESC", "model ASC") as
appropriate) on the query (call q = q.Order(...)) before calling Limit(...). Do
this change in this method and mirror the same pattern for the other capped
distinct/available methods in this file (the other GetDistinct*/GetAvailable*
functions) so the LIMIT is always applied to a stable ordering while preserving
use of applyLikeFilter.
- Around line 3004-3017: The loop enforces `limit` while ranging over the
unordered `keyValues` map, causing nondeterministic key selection; instead,
build a deterministic slice of keys (e.g., collect keys from `keyValues`, sort
them with sort.Strings) and then iterate that sorted keys slice to populate
`result` and apply the `limit`; use the existing `keyValues`, `result`, `limit`,
and `vals` variables (from the current block in rdb.go) so behavior is stable
across runs.
In `@plugins/logging/operations.go`:
- Around line 1168-1172: In GetAvailableMCPVirtualKeys, the logger.Error call
incorrectly uses the %w verb (meant for fmt.Errorf wrapping); update the error
formatting in the error branch of GetAvailableMCPVirtualKeys to use %v (or %s)
instead of %w so the log message is idiomatic — locate the p.logger.Error(...)
line in the GetAvailableMCPVirtualKeys function and replace the %w specifier
with %v while preserving the rest of the message and the err argument.
---
Outside diff comments:
In `@framework/logstore/rdb.go`:
- Around line 2860-2887: The current row-level filter on "routing_engines_used"
lets sibling values leak (e.g. row "a,b" matches query "b" but you then return
"a"), so stop filtering rows and instead filter after splitting: remove the call
that applies applyLikeFilter to q (the row-level filter on
routing_engines_used), retrieve rawValues as you do now, then when iterating
rawValues and splitting into engine (the loop that trims and dedups into
uniqueEngines), only add an engine when query == "" OR the engine itself matches
the query using the same LIKE semantics (case-insensitive substring or your
existing applyLikeFilter semantics applied at the string level); keep deduping
via uniqueEngines, then sort and apply the limit to engines as before.
🪄 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: 1e8efeeb-4d13-47f5-bd8d-d2c946efd727
📒 Files selected for processing (8)
framework/logstore/hybrid.goframework/logstore/matviews.goframework/logstore/rdb.goframework/logstore/rdb_postgres_perf_test.goframework/logstore/store.goplugins/logging/operations.goplugins/logging/utils.gotransports/bifrost-http/handlers/logging.go
impoiler
force-pushed
the
05-18-fix_remove_the_500_distinct_values_cap_from_the_matviews_filter_queries_to_avoid_confusion_when_data_is_missing
branch
from
May 18, 2026 13:01
5b2de1c to
d7383b9
Compare
impoiler
force-pushed
the
05-18-feat_add_search_functionality_to_the_filter_apis
branch
from
May 18, 2026 13:01
18f5adf to
e811375
Compare
Contributor
Merge activity
|
akshaydeo
changed the base branch from
05-18-fix_remove_the_500_distinct_values_cap_from_the_matviews_filter_queries_to_avoid_confusion_when_data_is_missing
to
graphite-base/3567
May 18, 2026 13:40
akshaydeo
dismissed
coderabbitai[bot]’s stale review
May 18, 2026 13:40
The base branch was changed.
akshaydeo
force-pushed
the
05-18-feat_add_search_functionality_to_the_filter_apis
branch
from
May 18, 2026 13:41
e811375 to
d02fc74
Compare
akshaydeo
deleted the
05-18-feat_add_search_functionality_to_the_filter_apis
branch
May 18, 2026 13:43
Comment on lines
+2754
to
+2760
| func (s *RDBLogStore) applyLikeFilter(q *gorm.DB, column, search string) *gorm.DB { | ||
| pattern := "%" + search + "%" | ||
| if s.db.Dialector.Name() == "postgres" { | ||
| return q.Where(fmt.Sprintf("%s ILIKE ?", column), pattern) | ||
| } | ||
| return q.Where(fmt.Sprintf("%s LIKE ?", column), pattern) | ||
| } |
Contributor
There was a problem hiding this comment.
% and _ in the user-supplied q value are not escaped before the pattern is built. When a user searches for a value like gpt_4, the pattern becomes %gpt_4% where _ is a single-character wildcard, so it incorrectly matches gptX4, gpt14, etc. Similarly, a query of q=% bypasses the filter entirely and returns all values. All commonly-used DB dialects support ESCAPE to treat these characters literally.
Suggested change
| func (s *RDBLogStore) applyLikeFilter(q *gorm.DB, column, search string) *gorm.DB { | |
| pattern := "%" + search + "%" | |
| if s.db.Dialector.Name() == "postgres" { | |
| return q.Where(fmt.Sprintf("%s ILIKE ?", column), pattern) | |
| } | |
| return q.Where(fmt.Sprintf("%s LIKE ?", column), pattern) | |
| } | |
| func (s *RDBLogStore) applyLikeFilter(q *gorm.DB, column, search string) *gorm.DB { | |
| escaped := strings.NewReplacer(`%`, `\%`, `_`, `\_`).Replace(search) | |
| pattern := "%" + escaped + "%" | |
| if s.db.Dialector.Name() == "postgres" { | |
| return q.Where(fmt.Sprintf("%s ILIKE ? ESCAPE '\\'", column), pattern) | |
| } | |
| return q.Where(fmt.Sprintf("%s LIKE ? ESCAPE '\\'", column), pattern) | |
| } |
| Pluck("model", &models).Error; err != nil { | ||
| Where("model != ''") | ||
| if query != "" { | ||
| q = q.Where("model ILIKE ?", "%"+query+"%") |
Contributor
There was a problem hiding this comment.
The same "%" + query + "%" pattern used throughout the matview methods (getDistinctModelsFromMatView, getDistinctAliasesFromMatView, getDistinctStopReasonsFromMatView, getDistinctKeyPairsFromMatView, getDistinctRoutingEnginesFromMatView) does not escape % or _ in the user-supplied query. A search for gpt_4 produces ILIKE '%gpt_4%' where _ is a single-character wildcard, returning false positives like gpt14. Since these paths are Postgres-only, using ESCAPE '\' with pre-escaped characters (matching the fix suggested for applyLikeFilter) would resolve the issue across all matview methods.
akshaydeo
pushed a commit
that referenced
this pull request
May 20, 2026
…ver-side search and pagination (#3567) ## Summary All "get distinct / available" filter-data methods now accept `limit int` and `query string` parameters, enabling server-side search filtering and result capping. Previously these methods returned unbounded result sets with no way to narrow results by a search term, which could cause large memory allocations and slow queries on tables with many distinct values. ## Changes - Added `limit` and `query` parameters to all `GetDistinct*` and `GetAvailable*` methods across the `LogStore` interface, `RDBLogStore`, `HybridLogStore`, `LoggerPlugin`, `LogManager`, and `PluginLogManager`. - On the database layer, a `LIKE`/`ILIKE` filter is applied when `query` is non-empty, and `LIMIT` is pushed down to the query so the DB engine caps the result set rather than Go-side slicing. - A helper `applyLikeFilter` was added to `RDBLogStore` to emit `ILIKE` on Postgres and `LIKE` on other dialects. - Materialized-view paths (`getDistinct*FromMatView`) were updated in the same way — `ILIKE` filtering and `LIMIT` are applied before results are returned. - `GetDistinctRoutingEngines` (both raw and matview paths) now sorts results and truncates to `limit` after the in-process comma-split deduplication step. - `GetDistinctMetadataKeys` applies an in-process case-insensitive substring filter on both key names and values when `query` is set, and caps the number of returned keys at `limit`. - The `/api/logs/filterdata` and `/api/mcp/logs/filterdata` HTTP handlers now read an optional `q` query parameter and pass it through to all downstream calls with a `defaultFilterDataLimit` of 1000. - Cache bypass: when `q` is non-empty the filter-data cache is skipped entirely (both read and write), since search results are user-specific and should not be shared across callers. - Existing performance tests updated to pass `(ctx, 1000, "")` to match the new signatures. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./framework/logstore/... ./plugins/logging/... ./transports/bifrost-http/... ``` To validate search filtering end-to-end: 1. Start the server with a populated log database. 2. Call `GET /api/logs/filterdata?dimensions=models&q=gpt` — the response should only contain model names matching `gpt`. 3. Call the same endpoint without `q` — the full list (up to 1000) should be returned and cached. 4. Call `GET /api/mcp/logs/filterdata?dimensions=tool_names&q=search` — only matching tool names should be returned and the result should not be written to cache. ## Breaking changes - [x] Yes All `LogStore`, `LogManager`, and `LoggerPlugin` method signatures have changed. Any external implementations of these interfaces must add `limit int, query string` to the affected methods. ## Related issues ## Security considerations The `query` string is passed to the database as a parameterised `LIKE`/`ILIKE` pattern (`?` placeholder), so there is no SQL injection risk. The `%` wildcards are added in Go before binding, not interpolated into the query string directly. ## 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
akshaydeo
added a commit
that referenced
this pull request
May 20, 2026
## Summary This PR cuts the `v1.5.11` / `v1.3.11` release across core, framework, and all plugins, and introduces a new Claude skill (`release-checklist`) for pre-release migration safety auditing. ## Changes - **`release-checklist` skill** — Adds `.claude/skills/release-checklist/SKILL.md`, a read-only pre-release audit tool that scans Go-defined database migrations changed in a release for high-scale deadlock/lock-contention risks and boot-time-blocking operations. It produces a structured `PASS`/`WARN`/`FAIL` report with a concrete remediation plan per finding. The skill is designed to grow via an extensible Checks Registry. - **Version bumps** — `core` → `1.5.11`, `framework` → `1.3.11`, `transports` → `1.5.3`, `plugins/governance` → `1.5.11`, `plugins/logging` → `1.5.11`, `plugins/semanticcache` → `1.5.11`, `plugins/otel` → `1.2.11`, `plugins/maxim` → `1.6.11`, `plugins/prompts` → `1.0.11`, and remaining plugins bumped accordingly. - **Changelogs populated** — All per-package changelogs updated with the full set of features and fixes shipping in this release. Key highlights in this release: - Temporary access tokens for scoped, time-limited API access - MCP per-user OAuth flow refactor - Bedrock Mantle inference engine support - Azure Realtime provider with enriched session tracking - Direct access control (DAC) and virtual key rotation - Cluster-aware log metadata and per-node usage aggregation - Feature flag framework - Config-hash-based file value override of DB on restart - Semantic cache plugin rewrite - Numerous streaming stability, Bedrock, Anthropic, and Gemini fixes - AWS SDK and dependency security updates ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [x] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Verify version files reflect the new release cat core/version # expect 1.5.11 cat framework/version # expect 1.3.11 cat transports/version # expect 1.5.3 # Core/Transports go test ./... ``` To exercise the new `release-checklist` skill, invoke it via Claude with: ``` /release-checklist origin/dev...HEAD ``` Expected output: a structured report with `PASS`/`WARN`/`FAIL` per check and a Remediation Plan table for any findings. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues #3603, #3565, #3489, #3334, #3335, #3435, #3554, #3590, #3444, #3198, #3581, #3610, #3599, #3567, #3382, #3461 and others listed in the changelogs. ## Security considerations - AWS SDK and dependency security updates are included (#3461). - `FullyRedacted()` for proxy passwords and `MarshalForStorage()` for `ProxyConfig` prevent partial secret leakage in API responses (#3445). - The `release-checklist` skill is strictly read-only and never modifies files. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
Merged
This was referenced May 21, 2026
18 tasks
18 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
All "get distinct / available" filter-data methods now accept
limit intandquery stringparameters, enabling server-side search filtering and result capping. Previously these methods returned unbounded result sets with no way to narrow results by a search term, which could cause large memory allocations and slow queries on tables with many distinct values.Changes
limitandqueryparameters to allGetDistinct*andGetAvailable*methods across theLogStoreinterface,RDBLogStore,HybridLogStore,LoggerPlugin,LogManager, andPluginLogManager.LIKE/ILIKEfilter is applied whenqueryis non-empty, andLIMITis pushed down to the query so the DB engine caps the result set rather than Go-side slicing.applyLikeFilterwas added toRDBLogStoreto emitILIKEon Postgres andLIKEon other dialects.getDistinct*FromMatView) were updated in the same way —ILIKEfiltering andLIMITare applied before results are returned.GetDistinctRoutingEngines(both raw and matview paths) now sorts results and truncates tolimitafter the in-process comma-split deduplication step.GetDistinctMetadataKeysapplies an in-process case-insensitive substring filter on both key names and values whenqueryis set, and caps the number of returned keys atlimit./api/logs/filterdataand/api/mcp/logs/filterdataHTTP handlers now read an optionalqquery parameter and pass it through to all downstream calls with adefaultFilterDataLimitof 1000.qis non-empty the filter-data cache is skipped entirely (both read and write), since search results are user-specific and should not be shared across callers.(ctx, 1000, "")to match the new signatures.Type of change
Affected areas
How to test
go test ./framework/logstore/... ./plugins/logging/... ./transports/bifrost-http/...To validate search filtering end-to-end:
GET /api/logs/filterdata?dimensions=models&q=gpt— the response should only contain model names matchinggpt.q— the full list (up to 1000) should be returned and cached.GET /api/mcp/logs/filterdata?dimensions=tool_names&q=search— only matching tool names should be returned and the result should not be written to cache.Breaking changes
All
LogStore,LogManager, andLoggerPluginmethod signatures have changed. Any external implementations of these interfaces must addlimit int, query stringto the affected methods.Related issues
Security considerations
The
querystring is passed to the database as a parameterisedLIKE/ILIKEpattern (?placeholder), so there is no SQL injection risk. The%wildcards are added in Go before binding, not interpolated into the query string directly.Checklist
docs/contributing/README.mdand followed the guidelines