move getvirtual keys to use paginated virtual keys to handle large no of keys - #3957
Conversation
|
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR implements internal pagination for virtual key retrieval to address PostgreSQL parameter limits, adds an encryption key to a configuration example, and normalizes a go.mod dependency. The main change introduces bounded paging in ChangesVirtual Keys Pagination, Config, and Dependencies
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Confidence Score: 5/5The change is safe to merge: it touches only the internal loading path for virtual keys, introduces no schema changes, and the new keyset cursor correctly handles the degenerate case where all rows share the same created_at timestamp. The keyset pagination logic is correct — (created_at, id) is a stable total order (primary key guarantees uniqueness), the cursor condition mirrors the ORDER BY exactly, and the early-exit on a short page avoids a final empty-result round trip. The test deliberately exercises the worst-case tie-break scenario. No correctness bugs were found. No files require special attention, though a future migration adding a composite index on (created_at, id) would improve keyset-scan efficiency for very large virtual-key tables. Important Files Changed
Reviews (6): Last reviewed commit: "1.5.7 changelogs (#3907)" | Re-trigger Greptile |
9d8cb81 to
d5f078e
Compare
d5f078e to
3b37267
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
5850388 to
2b750ad
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@examples/configs/withconfigstorelogsstorepostgres/config.json`:
- Line 3: Replace the hard-coded "encryption_key" value in config.json with an
environment-backed reference: stop embedding the literal under the
"encryption_key" property and instead read from an env var (e.g.
BIFROST_ENCRYPTION_KEY). Update the configuration consumer (where
"encryption_key" is parsed/used) to resolve process.env.BIFROST_ENCRYPTION_KEY
at runtime and fail fast with a clear error if it's missing; keep the property
name "encryption_key" in the JSON but document that its value must come from the
environment in examples and tests.
In `@framework/configstore/rdb.go`:
- Around line 2574-2603: The current GetVirtualKeys loop using offset-based
pagination is unstable under concurrent writes; replace it with keyset
pagination: change GetVirtualKeys to iterate by passing a cursor (lastCreatedAt,
lastID) into getVirtualKeysPage and have getVirtualKeysPage query with the same
ORDER BY ("governance_virtual_keys.created_at ASC, governance_virtual_keys.id
ASC") and a WHERE clause equivalent to (created_at, id) > (?, ?) to fetch the
next page, keeping Limit but removing Offset; handle the initial empty cursor
case, stop when fewer than limit rows returned, and ensure
preloadVirtualKeyBaseRelations is applied and edge cases where created_at ties
are disambiguated by id are covered so traversal is mutation-safe.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 23ffeb0b-d42b-4169-a76c-ede339817ad8
📒 Files selected for processing (4)
examples/configs/withconfigstorelogsstorepostgres/config.jsonframework/configstore/rdb.goframework/configstore/rdb_test.gotransports/go.mod
This release (core v1.5.15, framework v1.3.15) fixes missing `Authorization` header forwarding on streaming requests for the Ollama and SGL providers, ensuring authenticated requests behave correctly during streaming. - Ollama streaming text and chat completion requests now correctly forward the configured API key as an `Authorization: Bearer` header (#3906) - SGL provider now sends the `Authorization` header on streaming requests (#3307) (thanks [@hensapir](https://github.com/hensapir)!) - Governance and Logging APIs: removed the `from_memory` query parameter; virtual key and config list APIs now return consistent DB-backed results, with VK names batch-fetched in a single query (#3903) - Bumped core to v1.5.15, framework to v1.3.15, transports to v1.5.7, and all dependent plugins to their respective patch versions - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [x] Plugins - [ ] UI (React) - [ ] Docs Validate that Ollama and SGL streaming requests include the `Authorization: Bearer` header when an API key is configured. ```sh go version go test ./... ``` Configure an Ollama or SGL provider with an API key and issue a streaming chat or text completion request. Inspect outbound request headers to confirm `Authorization: Bearer <key>` is present. - [ ] Yes - [x] No Closes #3906 Closes #3307 Closes #3903 These fixes ensure that API keys configured for Ollama and SGL providers are correctly forwarded on streaming requests. Previously, the `Authorization` header was silently dropped on streaming paths, meaning requests could reach upstream providers without authentication credentials. - [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) - [x] I verified the CI pipeline passes locally if applicable <!-- This is an auto-generated comment: release notes by coderabbit.ai --> * **Bug Fixes** * Fixed authorization header handling for Ollama streaming requests. * Fixed authorization header forwarding for SGL provider streaming requests. * Improved consistency in virtual key and configuration list API responses by removing unnecessary query parameters. * **Chores** * Updated component versions across the platform. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/maximhq/bifrost/pull/3907?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
a29424b to
75f63a9
Compare
Merge activity
|
## Summary Releases core v1.5.16, framework v1.3.16, transports v1.5.8, and bumps all plugins to pick up the new core and framework versions. This release adds `file://` scheme support for pricing URLs, paginated virtual key fetching, and fixes several correctness issues across Bedrock, OpenAI→Anthropic conversion, MCP stdio, and streaming responses. ## Changes - **File scheme pricing URLs** — Pricing source URLs now accept `file://`, enabling local filesystem pricing data for air-gapped and self-hosted deployments (#4045) - **Paginated virtual key fetch** — Virtual key retrieval is now paginated to avoid loading all keys into memory at once for large deployments (#3957) - **Preserve model pool entries on pricing reload** — Non-pricing model pool entries are no longer dropped when pricing data is reloaded (#3999) - **Bedrock `outputAssessments` type** — Corrected the type of `outputAssessments` in Bedrock response structs (#4028) - **`Model` field in `TextCompletionChunkResponse`** — Added the missing `Model` field to text completion chunk responses (#3970) - **Orphaned tool results in OpenAI→Anthropic conversion** — Orphaned tool results no longer cause rejections from the Anthropic API (#3919) - **MCP inline stdio env assignments** — MCP stdio server configs now correctly parse inline environment variable assignments (#3861) ## Type of change - [x] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [x] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go version go test ./... ``` - To test `file://` pricing URLs, configure a pricing URL using the `file:///path/to/pricing.json` scheme and verify pricing data loads correctly from the local file. - To test paginated virtual key fetch, create a large number of virtual keys and confirm they are all returned correctly without memory issues. - To test orphaned tool results, send a request through the OpenAI→Anthropic conversion flow containing a tool result with no matching tool call and verify it is accepted rather than rejected. - To test MCP inline stdio env, configure an MCP stdio server with inline env var assignments (e.g. `KEY=value`) and verify the server starts correctly. ## Breaking changes - [ ] Yes - [x] No ## Related issues Closes #4045, #3957, #3999, #4028, #3970, #3919, #3861 ## Security considerations No new security implications. This release does not touch auth, secrets handling, or sandboxing. ## 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 bumps the Go toolchain version from `1.26.3` to `1.26.4` across all modules and CI workflows, and cuts a new release (`core` v1.5.17, `framework` v1.3.17, `transports` v1.5.9, `plugins/compat` v0.1.16, `plugins/governance` v1.5.17, and associated plugin versions) incorporating a large batch of features and fixes accumulated since the previous release. ## Changes - **Go 1.26.4** — Updated `go-version` in all GitHub Actions workflows (`e2e-tests`, `helm-release`, `pr-tests`, `release-cli`, `release-pipeline`, `snyk`) and all `go.mod` files (core, framework, transports, cli, all plugins, examples, and test modules). - **Core (v1.5.17)** — OpenAI compaction support, multi-customer logs and usage tracking, multiple team/business unit support, `request_headers` wildcard pattern capture for OTel and Maxim plugins, xAI `x_search` tool, fetch URL validation with SSRF hardening, `file://` pricing URL scheme, virtual key provider fan-out filtering, and a broad set of fixes including Anthropic prompt cache key, empty thinking block stripping, OpenAI stream usage event cleanup, Gemini numeric schema constraints, stale connection retries, Azure Claude diagnostic strip, and passthrough budget handling. - **Framework (v1.3.17)** — Scope-aware budgets and limits wired from model configs, provider-level governance, multiple customer budget support with `calendar_aligned` windows, paginated virtual key fetch, `config.json` source-of-truth flow, FTS index cap reduction, sync worker drift fix, cascade deletes for model configs, and high-scale virtual key flow improvements. - **Transports (v1.5.9)** — Full changelog covering all of the above plus UI improvements (log navigation, customer detail sheet, `BudgetDisplay` component, inline loading shell, materialized view alias), SCIM provisioning fields, Helm/config schema additions (`roles`, `per_user_oauth`), client IP resolution from forwarded headers, and dependency upgrades (`recharts` to 3.8.1, `golang.org/x` CVE remediation). - **Plugins** — `governance` v1.5.17 adds team budget/rate-limit exporters, ghost node reconciliation fix, and VK double usage counting fix; `logging` v1.5.17 adds wildcard header capture and file attachment rendering; `otel` v1.2.17 adds `disable_content_logging` and multiple collectors support; `maxim` v1.6.17 adds `request_headers` wildcard capture; `compat` v0.1.16 fixes `max_tokens` preservation during param filtering. ## Type of change - [ ] Bug fix - [x] 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 Go version go version # should report go1.26.4 # Run core tests cd core && go test ./... # Run framework tests cd framework && go test ./... # Run transports tests cd transports && go test ./... # Run plugin tests cd plugins/governance && go test ./... cd plugins/logging && go test ./... cd plugins/otel && go test ./... # UI cd ui pnpm i pnpm build pnpm test ``` ## Breaking changes - [ ] Yes - [x] No ## Related issues #4053, #4066, #4041, #4012, #3976, #3947, #3991, #4045, #3957, #3938, #3937, #3939, #3981, #3998, #3997, #4092, #4091, #4079, #4080, #4086, #3929, #3994, #4028, #3970, #3919, #3861, #3664, #3999, #4088, #4070, #4051, #4043, #4057, #4023, #3941, #3955, #4024, #3956, #3967, #3925, #3992, #3900 ## Security considerations - Fetch URL validation hardened against SSRF by tightening IP checks for private networks and link-local addresses (#4092, #3947, #3991). - Transitive `golang.org/x` dependencies (crypto, net, sys, text) bumped to address Docker Scout CVEs (#3900). ## 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) - [x] I verified the CI pipeline passes locally if applicable <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * OpenAI compaction, multi-customer/team logstore support, request-header wildcard capture, enhanced governance (provider-level & scope-aware limits), disable-content-logging option, support for multiple OpenTelemetry collectors, SSRF hardening and URL validation. * **Chores** * Bumped Go toolchain across modules and updated component/plugin version releases. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## ✨ Features - **OpenAI Compaction** — Added OpenAI conversation compaction support across core, framework, logging, and the API surface (#4053) - **Multi-Customer & Org Hierarchy** — Logs and usage tracking now support multiple customers, teams, and business units, including business unit CRUD, team assignment, and governance endpoints in the OpenAPI spec (#4066, #4041, #4082) - **Provider-Level Governance** — Budgets & limits are now scope-aware and can be applied at the virtual-key top level and per provider, wired from the model configs table, with UI filters for scope and providers (#3938, #3937, #3939, #3981, #3962) - **Customer Budgets** — Customers support multiple budgets and `calendar_aligned` budget windows (#3998, #3997) - **Virtual Key Attribution & Controls** — Added a `created_by` user attribution column and a `blacklisted_models` column for virtual key provider configs (#3672, #3653) - **Request Header Capture** — OTel and Maxim observability plugins capture `request_headers` by pattern, with wildcard support (e.g. `x-custom-*`); logging gained the same wildcard header capture (#4012, #3958) - **OTel Content Controls & Collectors** — New `disable_content_logging` option drops message/tool content from exported spans, plus support for multiple OTel collectors (#4064, #3894) - **xAI x_search** — Added xAI `x_search` tool support (#3976) - **URL Validation** — Added fetch URL validation with private-network configuration and link-local blocking (#3947, #3991) - **File Scheme Pricing URLs** — Pricing source URLs now accept the `file://` scheme for air-gapped and self-hosted deployments (#4045) - **Paginated Virtual Keys** — Virtual key fetching is paginated to handle deployments with very large numbers of keys (#3957) - **Client IP Resolution** — Resolve client IP from `X-Forwarded-For`/`X-Real-IP` headers - **SCIM Provisioning** — Added `attributeType`/`attributeValue` SCIM provisioning fields - **Helm/Config Schema** — Added `roles` RBAC governance config and `per_user_oauth` MCP auth to the Helm chart and config schema (#4004, #4009) - **Log Navigation UI** — Added a "View logs" menu item to customer, team, and virtual key tables, clickable links in log detail views, a customer detail sheet, and a reusable `BudgetDisplay` component (#4073, #4054, #4026, #4055) - **Faster First Paint** — Added an inline loading shell to `#root` before React mounts (#4063) - **Materialized View Alias** — Added an `alias` column to the materialized view with filter support (#4078) ## 🐞 Fixed - **Fetch URL IP Checks** — Hardened fetch URL IP checks against SSRF (#4092) - **Mantle Model Matching** — Broadened Mantle model matching to all `gpt` variants (#4091) - **Empty Thinking Blocks** — Strip thinking blocks when the signature is empty (#4079) - **OpenAI Stream Usage** — Removed usage from the `responses.created` event in the OpenAI stream (#4080) - **Prompt Cache Key** — Set the prompt cache key from the Anthropic integration (#4086) - **Upstream Failure Status** — Map upstream connection failures to 502 instead of 400 (#3929) (thanks [@chris-colinsky](https://github.com/chris-colinsky)!) - **Gemini Schema Constraints** — Accept numeric schema integer constraints for Gemini (#3994) (thanks [@yanhao98](https://github.com/yanhao98)!) - **Files Provider Param** — Accept the `?provider=` query param on `GET /v1/files` (#3971) (thanks [@alexef](https://github.com/alexef)!) - **Optional Batch Model** — Made the `model` field optional on `POST /v1/batches` (#3973) (thanks [@alexef](https://github.com/alexef)!) - **Helm Azure Config** — Added missing `azure_key_config` fields to the Helm schema (#3996) (thanks [@axelray-dev](https://github.com/axelray-dev)!) - **Text Completion Chunk Model** — Added the missing `Model` field to `TextCompletionChunkResponse` (#3970) (thanks [@kuishou68](https://github.com/kuishou68)!) - **MCP Inline stdio Env** — MCP stdio server configs accept inline environment variable assignments (#3861) (thanks [@Shushmitaaaa](https://github.com/Shushmitaaaa)!) - **Orphaned Tool Results** — Orphaned tool results in the OpenAI to Anthropic conversion flow are no longer rejected by the Anthropic API (#3919) - **Node Usage Reconciliation** — Added a monotonic `inc_number` log cursor so node usage reconciliation does not skip late async log writes (#3664) - **Bedrock Output Assessments** — Corrected the type of `outputAssessments` in Bedrock responses (#4028) - **Model Pool Pricing Reloads** — Preserve non-pricing model pool entries across pricing reloads (#3999) - **Ghost Node Reconciliation** — Replicate the VK hierarchy flow for ghost node reconciliation (#4088) - **VK Double Usage Counting** — Fixed double usage counting when creating a virtual key (#4070) - **Model Config Lifecycle** — Cascade deletes for model configs and removal of stale in-memory model configs (#4051, #4043) - **FTS Index Cap** — Reduced the FTS index `left()` cap from 800k to 250k chars to stay within the tsvector limit (#4057) - **Sync Worker Drift** — Reduced the sync worker ticker period to 5m to prevent threshold drift (#4023) - **Passthrough** — Fixed passthrough budgets, gated passthrough models per VK, model extraction for Azure passthrough, and restricted fallbacks/provider selection to the VK boundary (#3941, #3988, #3983, #3924) - **Provider Response Headers** — Strip provider response headers and add a content-type filter (#3955, #4024) - **Stream Handling** — Drain non-SSE stream readers and retry stale connections (#3956, #3967) - **Azure Claude** — Strip Azure diagnostic property for Claude models (#3925) - **Compat max_tokens** — Preserve chat `max_tokens` during param filtering (#3992) - **Raw Request Flag** — Removed the raw request flag from providers that don't support it (#4058) - **UI Fixes** — Standardized page container layout, virtual key model configs UI, and dashboard chart tooltips (#4046, #4052, #4044) ## 🔧 Maintenance - **Dependency Upgrades** — Bumped transitive `golang.org/x` dependencies (crypto, net, sys, text) for Docker Scout CVE remediation and `recharts` to 3.8.1; cascaded version bumps across all modules (#3900, #4003)

Summary
GetVirtualKeyspreviously fetched all virtual keys in a single query with GORM preloads. When the number of keys grows large, this triggers PostgreSQL's extended protocol parameter limit, causing the query to fail. This PR fixes the issue by internally paginating through results in bounded pages of 1,000 rows at a time, transparently assembling the full result set before returning it to callers.Changes
GetVirtualKeysnow delegates toGetVirtualKeysPaginatedin a loop using a fixed internal page size (virtualKeyInternalPageSize = 1000), accumulating results until all rows are retrieved.TestGetVirtualKeysUsesInternalPagination) was added that creates more than one full internal page of virtual keys and asserts thatGetVirtualKeysreturns all of them in the correct order.Type of change
Affected areas
How to test
The new test creates 1,005 virtual keys (one full internal page plus five extras) and verifies that
GetVirtualKeysreturns all 1,005 rows with the first and last IDs in the expected order.Screenshots/Recordings
N/A
Breaking changes
Related issues
Security considerations
No auth, secrets, or PII implications. The change only affects how rows are batched internally during a database read.
Checklist
docs/contributing/README.mdand followed the guidelinesSummary by CodeRabbit
Release Notes
New Features
Improvements
Tests
Chores