adds vk based integration tests #1252
Conversation
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThe changes introduce virtual key (VK) support across the testing infrastructure and HTTP transport layer. Configuration files enable governance with VK entries, test utilities add VK parametrization for cross-provider scenarios, test methods are refactored to conditionally inject x-bf-vk headers, and HTTP context storage normalizes virtual key value handling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
🧪 Test Suite AvailableThis PR can be tested by a repository admin. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/integrations/tests/utils/parametrize.py (1)
8-8: Remove unused import.The
Unionimport is not used in this file. The code uses the modern|union operator (Python 3.10+) instead ofUnion[...]syntax.🔎 Proposed fix
-from typing import List, Tuple, Union +from typing import List, Tuple
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
tests/integrations/config.jsontests/integrations/config.ymltests/integrations/tests/test_openai.pytests/integrations/tests/utils/config_loader.pytests/integrations/tests/utils/parametrize.pytransports/bifrost-http/handlers/plugins.gotransports/bifrost-http/lib/ctx.gotransports/bifrost-http/server/server.go
💤 Files with no reviewable changes (1)
- transports/bifrost-http/server/server.go
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)
Files:
tests/integrations/config.ymltests/integrations/config.jsontransports/bifrost-http/lib/ctx.gotests/integrations/tests/utils/config_loader.pytransports/bifrost-http/handlers/plugins.gotests/integrations/tests/utils/parametrize.pytests/integrations/tests/test_openai.py
🧠 Learnings (5)
📚 Learning: 2025-12-09T17:07:42.007Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.
Applied to files:
transports/bifrost-http/lib/ctx.gotransports/bifrost-http/handlers/plugins.go
📚 Learning: 2025-12-29T11:54:55.836Z
Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 1153
File: framework/configstore/rdb.go:2221-2246
Timestamp: 2025-12-29T11:54:55.836Z
Learning: In Go reviews, do not flag range-over-int patterns like for i := range n as compile-time errors, assuming Go 1.22+ semantics. Only flag actual range-capable values (slices, arrays, maps, channels, strings) and other compile-time issues. This applies to all Go files across the repository.
Applied to files:
transports/bifrost-http/lib/ctx.gotransports/bifrost-http/handlers/plugins.go
📚 Learning: 2025-12-12T08:25:02.629Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1000
File: transports/bifrost-http/integrations/router.go:709-712
Timestamp: 2025-12-12T08:25:02.629Z
Learning: In transports/bifrost-http/**/*.go, update streaming response handling to align with OpenAI Responses API: use typed SSE events such as response.created, response.output_text.delta, response.done, etc., and do not rely on the legacy data: [DONE] termination marker. Note that data: [DONE] is only used by the older Chat Completions and Text Completions streaming APIs. Ensure parsers, writers, and tests distinguish SSE events from the [DONE] sentinel and handle each event type accordingly for correct stream termination and progress updates.
Applied to files:
transports/bifrost-http/lib/ctx.gotransports/bifrost-http/handlers/plugins.go
📚 Learning: 2025-12-29T09:14:16.633Z
Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 888
File: transports/bifrost-http/handlers/middlewares.go:246-256
Timestamp: 2025-12-29T09:14:16.633Z
Learning: In the bifrost HTTP transport, fasthttp.RequestCtx is the primary context carrier and should be passed directly to functions that expect a context.Context. Do not convert to context.Context unless explicitly required. Ensure tracer implementations and related components are designed to accept fasthttp.RequestCtx directly, and document this architectural decision for maintainers.
Applied to files:
transports/bifrost-http/lib/ctx.gotransports/bifrost-http/handlers/plugins.go
📚 Learning: 2025-12-24T07:38:16.990Z
Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1095
File: tests/integrations/tests/test_google.py:2030-2030
Timestamp: 2025-12-24T07:38:16.990Z
Learning: In Python tests under tests/integrations/tests, allow a fixture parameter named test_config in test functions even if unused; do not flag it as unused. This is an internal convention to ensure consistency for integration tests.
Applied to files:
tests/integrations/tests/test_openai.py
🧬 Code graph analysis (3)
transports/bifrost-http/lib/ctx.go (1)
core/schemas/bifrost.go (1)
BifrostContextKeyVirtualKey(123-123)
tests/integrations/tests/utils/parametrize.py (1)
tests/integrations/tests/utils/config_loader.py (3)
get_config(455-460)is_virtual_key_configured(441-448)is_virtual_key_configured(512-514)
tests/integrations/tests/test_openai.py (3)
tests/integrations/tests/utils/parametrize.py (2)
get_cross_provider_params_with_vk_for_scenario(50-101)format_provider_model(126-141)tests/integrations/tests/utils/config_loader.py (6)
get_config(455-460)get_integration_url(112-131)get_integration_url(463-464)get_api_config(197-199)get_virtual_key(426-439)get_virtual_key(507-509)core/schemas/bifrost.go (1)
OpenAI(35-35)
🪛 Ruff (0.14.10)
tests/integrations/tests/test_openai.py
268-268: Unused method argument: test_config
(ARG002)
286-286: Unused method argument: test_config
(ARG002)
303-303: Unused method argument: test_config
(ARG002)
323-323: Unused method argument: test_config
(ARG002)
347-347: Unused method argument: test_config
(ARG002)
392-392: Unused method argument: test_config
(ARG002)
411-411: Unused method argument: test_config
(ARG002)
427-427: Unused method argument: test_config
(ARG002)
443-443: Unused method argument: test_config
(ARG002)
574-574: Unused method argument: test_config
(ARG002)
875-875: Unused method argument: test_config
(ARG002)
893-893: Unused method argument: test_config
(ARG002)
911-911: Unused method argument: test_config
(ARG002)
942-942: Unused method argument: test_config
(ARG002)
1008-1008: Unused method argument: test_config
(ARG002)
1168-1168: Unused method argument: test_config
(ARG002)
1212-1212: Unused method argument: test_config
(ARG002)
1248-1248: Unused method argument: test_config
(ARG002)
1283-1283: Unused method argument: test_config
(ARG002)
1330-1330: Unused method argument: test_config
(ARG002)
1379-1379: Unused method argument: test_config
(ARG002)
1432-1432: Unused method argument: test_config
(ARG002)
1489-1489: Unused method argument: test_config
(ARG002)
1529-1529: Unused method argument: test_config
(ARG002)
1751-1751: Unused method argument: test_config
(ARG002)
1826-1826: Unused method argument: test_config
(ARG002)
1897-1897: Unused method argument: test_config
(ARG002)
1956-1956: Unused method argument: test_config
(ARG002)
2002-2002: Unused method argument: test_config
(ARG002)
2083-2083: Unused method argument: test_config
(ARG002)
2214-2214: Unused method argument: test_config
(ARG002)
2237-2237: Unused method argument: test_config
(ARG002)
2363-2363: Unused method argument: test_config
(ARG002)
2484-2484: Unused method argument: test_config
(ARG002)
🔇 Additional comments (12)
transports/bifrost-http/lib/ctx.go (1)
209-212: Good fix: Standardizes virtual key context storage.This change ensures the virtual key is stored under
schemas.BifrostContextKeyVirtualKeyconstant instead of using the header name string as the context key. This aligns with how VKs from other headers (authorization, x-api-key, x-goog-api-key) are already stored at lines 219, 225, and 229, enabling consistent retrieval by governance and other components.tests/integrations/config.json (2)
154-175: LGTM: VK testing infrastructure enabled correctly.The configuration changes enable config_store, add governance with a test virtual key, and configure the client to enforce governance. The virtual key value
"sk-bf-test-key"aligns withconfig.yml, enabling consistent VK-based integration testing.
183-184: LGTM: Governance enabled for VK testing.Correctly enables governance and enforces the governance header requirement, allowing integration tests to validate VK functionality.
tests/integrations/config.yml (1)
698-702: LGTM: Virtual key configuration added.The virtual key configuration enables VK-based testing with the value matching
config.json. This allows integration tests to run scenarios both with and without the VK header.tests/integrations/tests/utils/config_loader.py (2)
426-448: LGTM: Virtual key accessors implemented correctly.The
get_virtual_key()andis_virtual_key_configured()methods safely handle missing configuration and disabled states. The implementation correctly checks theenabledflag and returns empty string as a fallback, making it easy for tests to conditionally enable VK scenarios.
507-514: LGTM: Convenience functions added.The module-level convenience functions properly delegate to the ConfigLoader instance, maintaining consistency with the existing pattern in the file.
transports/bifrost-http/handlers/plugins.go (1)
316-316: TheIsCustomfield difference between line 245 (true) and line 316 (false) is intentional and correctly reflects the semantic difference. The configstore layer automatically recalculatesIsCustombased on the plugin'sPathfield: plugins without a custom path are marked as non-custom (false), while plugins with a path are marked as custom (true). The handler's explicit values are overridden by this automatic calculation, making the explicit assignments at both lines semantically correct for their respective contexts.Likely an incorrect or invalid review comment.
tests/integrations/tests/utils/parametrize.py (2)
50-101: LGTM!The VK parametrization logic correctly doubles test coverage when virtual keys are configured, ensuring both standard and VK-enabled scenarios are tested. The dummy tuple handling prevents pytest errors when no providers are available.
104-123: LGTM!Clear and well-documented test ID formatting utility. The examples in the docstring are helpful.
tests/integrations/tests/test_openai.py (3)
198-224: LGTM!The VK-enabled client factory correctly conditionally injects the
x-bf-vkheader only when bothvk_enabled=Trueand a valid virtual key is configured. The logic properly handles the case where VK is empty or unconfigured.
265-282: LGTM!The test correctly adopts the VK testing pattern: parametrizing with
vk_enabled, creating a provider-specific client with the VK flag, and using it for API calls. This ensures governance and virtual key functionality are validated.
283-301: LGTM!The VK testing pattern is applied consistently across all cross-provider parameterized tests. Each test correctly:
- Uses
get_cross_provider_params_with_vk_for_scenariofor parametrization- Accepts the
vk_enabledparameter- Creates a provider-specific client with
get_provider_openai_client(provider, vk_enabled=vk_enabled)This ensures comprehensive testing of both standard API key and virtual key authentication paths.
Also applies to: 302-319, 573-617, 874-891, 1168-1206, 2683-2701
Merge activity
|
Summary
Add virtual key (VK) testing support to integration tests, enabling tests to run with and without the
x-bf-vkheader to validate governance functionality.Changes
config.ymlfor cross-provider testingvk_enabledflag for all cross-provider testsx-bf-vkheaderType of change
Affected areas
How to test
Run the integration tests with the updated configuration:
cd tests/integrations python -m pytest tests/test_openai.py -vThe tests will now run twice for each provider/model combination when virtual key testing is enabled - once with the standard API key and once with the virtual key header.
Breaking changes
Security considerations
This PR improves testing of the governance and virtual key functionality, which is a security-related feature. The virtual key implementation in the HTTP transport was fixed to use the correct context key.
Checklist