enable core tests #738
Open
enable core tests #738
Conversation
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
## Summary Refactored the Config struct methods in the bifrost-http transport to use consistent receiver variable naming, replacing `s` with `c` throughout the codebase for better readability and consistency. Also removed unused code related to MCP configuration restoration. ## Changes - Standardized receiver variable naming from `s` to `c` across all Config methods - Removed unused `getRestoredMCPConfig` function that was no longer needed - Removed a commented line about loading governance config - Improved code consistency and maintainability ## Type of change - [ ] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (Next.js) - [ ] Docs ## How to test Verify that all functionality continues to work as expected: ```sh # Core/Transports go version go test ./transports/bifrost-http/... ``` ## Breaking changes - [x] No - [ ] Yes ## Related issues Improves code quality and maintainability. ## Security considerations No security implications as this is a refactoring change only. ## 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
## Summary This PR refactors the database table structures in the framework/configstore package by moving table definitions into a dedicated tables subpackage. It also introduces a new framework configuration structure to support centralized framework settings. Closes #523 ## Changes - Created a new `framework/configstore/tables` package to house all database table definitions - Split the monolithic `tables.go` file into multiple smaller, focused files organized by entity type - Added a new `FrameworkConfig` structure to support centralized framework configuration - Enhanced the pricing module with configurable URL and sync interval settings - Updated all references to table structures throughout the codebase to use the new package path ## Type of change - [x] Refactor - [x] Feature ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Plugins ## How to test ```sh # Core/Transports go version go test ./... ``` ## Breaking changes - [x] No ## Security considerations No security implications as this is primarily a code organization refactor with minimal functional changes. ## Checklist - [x] I added/updated tests where appropriate - [x] I verified builds succeed (Go and UI)
## Summary
Added a health check endpoint that validates connections to the config store, log store, and vector store, making it suitable for Kubernetes liveness and readiness probes.
## Changes
- Added a new `/health` endpoint to the HTTP API
- Implemented `Ping()` methods in all store interfaces (config, log, vector)
- Added implementations for Redis, RDB, and Weaviate stores
- Fixed naming inconsistency from `ApiKey` to `APIKey` in Weaviate config
- Updated OpenAPI documentation to include the new health endpoint
## Type of change
- [x] Feature
- [x] Refactor
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [x] Docs
## How to test
Test the health endpoint with a running server:
```sh
# Start the server
go run main.go
# Test the health endpoint
curl http://localhost:8000/health
# Expected response:
# {"status":"ok"}
# Test with unavailable stores (by stopping databases)
# Expected response:
# {"error":"config store not available"}
```
## Breaking changes
- [x] No
## Security considerations
The health endpoint doesn't expose sensitive information, only returning a status code and simple message about store availability.
## Checklist
- [x] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
…om_readme removes active releases from readme
## Summary Fix for the virtual key header handling in the Bifrost HTTP transport, ensuring proper context key usage. Closes #632 ## Changes - Updated the context key for the "x-bf-vk" header from a string-based key to the proper schema constant `schemas.BifrostContextKeyVirtualKeyHeader` - Removed trailing whitespace in the code - Bumped version from 1.3.0 to 1.3.1 - Updated changelog to reflect the bug fix ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (Next.js) - [ ] Docs ## How to test Verify that the "x-bf-vk" header is properly processed in HTTP requests: ```sh # Core/Transports go version go test ./... # Test with a request containing the x-bf-vk header curl -X POST http://localhost:8000/v1/chat/completions \ -H "Content-Type: application/json" \ -H "x-bf-vk: your-virtual-key" \ -d '{"model": "gpt-3.5-turbo", "messages": [{"role": "user", "content": "Hello"}]}' ``` ## Breaking changes - [ ] Yes - [x] No ## Related issues Fixes the "x-bf-vk" missing error mentioned in the changelog. ## Security considerations This fix ensures proper handling of virtual key headers which could impact authentication and routing. ## 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
## Summary Standardize parameter order and improve code consistency across the codebase by fixing function signatures, renaming variables, and adding documentation. ## Changes - Standardized parameter order in `tryRequest` function to consistently place context first - Renamed context key from `BifrostContextKeyVirtualKeyHeader` to `BifrostContextKeyVirtualKey` for clarity - Improved variable naming in methods by using consistent parameter names (e.g., `br` instead of `r` for BifrostRequest) - Added missing documentation for functions and methods - Added database migration for new columns in the logs table - Fixed context key types across plugins to use the standard `BifrostContextKey` type ## Type of change - [ ] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [ ] UI (Next.js) - [ ] Docs ## How to test ```sh # Core/Transports go version go test ./... ``` ## Breaking changes - [ ] Yes - [x] No ## Security considerations No security implications as this is a refactoring change that maintains the same functionality. ## Checklist - [x] I added/updated tests where appropriate - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable
…_responses_stream fix: handle panic in gemini responses stream
## Summary Enhance the ListModels functionality to aggregate results from multiple API keys, improving model discovery and availability. ## Changes - Modified `ListModels` interface to accept multiple keys instead of a single key - Implemented key aggregation logic that combines model lists from multiple API keys - Added deduplication of models based on model ID when aggregating results - Created a best-effort approach that continues processing remaining keys even if some fail - Added a new helper function `getAllSupportedKeys` to retrieve all valid keys for a provider - Updated all provider implementations to support the multi-key approach ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (Next.js) - [ ] Docs ## How to test Test the ListModels endpoint with multiple API keys configured for the same provider: ```sh # Core/Transports go version go test ./... # Test with multiple keys for a provider (e.g., OpenAI) curl -X GET "http://localhost:8000/v1/models?provider=openai" -H "Authorization: Bearer your-bifrost-token" ``` ## Breaking changes - [ ] Yes - [x] No ## Related issues Improves model discovery by aggregating results from multiple API keys. ## Security considerations The implementation maintains the same security model as before, with proper error handling to avoid leaking sensitive information. ## Checklist - [x] I added/updated tests where appropriate - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable
…ized request handling (#724) ## Summary Refactored the Cohere provider to use a unified request handling approach and improved response object pooling for better performance and memory efficiency. ## Changes - Implemented `completeRequest` method in Cohere provider to centralize API request handling - Added object pooling for Cohere embedding responses to reduce memory allocations - Standardized response handling across all Cohere API endpoints - Added id, object, and model fields to Chat Completion responses from Bedrock and Cohere providers - Updated list models functionality to use all available keys ## Type of change - [ ] Bug fix - [x] Feature - [x] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (Next.js) - [ ] Docs ## How to test Test the Cohere provider with various request types to ensure proper functionality: ```sh # Core/Transports go version go test ./... # Test Cohere provider specifically go test ./core/providers -run TestCohereProvider ``` ## Breaking changes - [x] No - [ ] Yes ## Related issues Improves performance and standardizes response handling across providers. ## Security considerations No security implications as this is an internal refactoring of existing functionality. ## Checklist - [x] I added/updated tests where appropriate - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable
…pi (#702) ## Summary Implement proper type handling for Anthropic tools in the schema conversion layer, adding support for specialized tool types like computer use preview, web search, and local shell. ## Changes - Added `ResponsesToolType` as a proper string type with constants for all supported tool types - Enhanced Anthropic tool conversion to properly handle specialized tool types: - Computer use preview tools - Web search tools - Local shell (bash) tools - Added proper type definitions for Anthropic-specific tool structures - Fixed type conversion in the chat tool mux to use the new type system ## Type of change - [x] Feature - [x] Refactor ## Affected areas - [x] Core (Go) - [x] Providers/Integrations ## How to test ```sh # Core/Transports go version go test ./... ``` Test with Anthropic API calls that use specialized tools like computer use preview, web search, and local shell to verify proper conversion between Bifrost and Anthropic formats. ## Breaking changes - [x] No ## Related issues Improves tool handling for Anthropic provider integration. ## Checklist - [x] I added/updated tests where appropriate - [x] I verified builds succeed (Go and UI)
…725) ## Summary Add support for storing and displaying Responses API input history in logs, enabling better tracking and debugging of Responses API requests. ## Changes - Added a new `responses_input_history` column to the logs database table - Created a database migration to add the new column - Modified the logging plugin to extract and store Responses API input messages - Updated the UI to display Responses input history in log details - Renamed `LogResponsesOutputView` to `LogResponsesMessageView` for reuse with both input and output - Fixed an issue in the post-hook runner by clearing `ChatResponse` after converting to `ResponsesStreamResponse` ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [x] UI (Next.js) - [ ] Docs ## How to test 1. Make Responses API requests 2. Check logs to verify input history is properly stored and displayed 3. Verify that both input and output messages are properly displayed in the UI ```sh # Core/Transports go version go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` ## Breaking changes - [x] Yes - [ ] No The database schema has been updated with a new column. Users should run the application to trigger the migration. ## Security considerations No new security implications. The implementation follows the same patterns as existing log storage. ## 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
## Summary This PR updates the PreRequestCallback interface to include a Bifrost context parameter, allowing callbacks to modify the context before it's passed to Bifrost. It also adds a rawBody parameter to provide access to the original request body. ## Changes - Modified the `PreRequestCallback` function signature to include `bifrostCtx *context.Context` and `rawBody []byte` parameters - Updated all implementations of `PreRequestCallback` in various integration files (anthropic.go, genai.go, openai.go) - Added a new router.go file that contains the centralized router implementation with the updated interface - Moved most of the code from utils.go to router.go for better organization - Simplified utils.go to contain only utility functions ## Type of change - [ ] Bug fix - [x] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (Next.js) - [ ] Docs ## How to test Test that pre-request callbacks can now modify the Bifrost context: ```sh # Core/Transports go version go test ./... # Test with a custom integration that uses the PreRequestCallback # to modify the Bifrost context before processing ``` ## Breaking changes - [x] Yes - [ ] No This changes the signature of the `PreRequestCallback` interface. Any custom implementations will need to be updated to match the new signature. ## Related issues Enables better context manipulation in pre-request callbacks, which helps with request processing customization. ## Security considerations No additional security implications. The change maintains the existing security model while providing more flexibility. ## 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
…raw request passthrough and response gzip decompression and anthropic integration passthrough (#731) ## Summary Add support for passthrough mode to allow direct forwarding of requests to providers without modifying the request body. This enables advanced use cases like Claude Code and other provider-specific features that require exact request formats. ## Changes - Added context keys to support passthrough mode: `BifrostContextKeySkipKeySelection`, `BifrostContextKeyExtraHeaders`, `BifrostContextKeyURLPath`, and `BifrostContextKeyRequestBody` - Implemented passthrough detection for Anthropic API requests - Refactored provider code to check for raw request bodies in context before converting - Added support for preserving original URL paths and headers - Improved error handling for unsupported operations with standardized messages - Fixed handling of response bodies with different content encodings (gzip) - Added support for skipping key selection when appropriate ## Type of change - [ ] Bug fix - [x] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (Next.js) - [ ] Docs ## How to test Test passthrough mode with Anthropic API: ```sh # Test with Claude Code request curl -X POST http://localhost:8080/anthropic/v1/messages \ -H "Content-Type: application/json" \ -H "anthropic-version: 2023-06-01" \ -d '{ "model": "claude-3-opus-20240229", "messages": [{"role": "user", "content": "Write a simple Python function to calculate Fibonacci numbers"}], "max_tokens": 1000 }' # Test with standard API key curl -X POST http://localhost:8080/anthropic/v1/messages \ -H "Content-Type: application/json" \ -H "x-api-key: your-api-key" \ -H "anthropic-version: 2023-06-01" \ -d '{ "model": "claude-3-opus-20240229", "messages": [{"role": "user", "content": "Hello"}], "max_tokens": 100 }' ``` ## Breaking changes - [ ] Yes - [x] No ## Related issues Enables support for Claude Code and other provider-specific features that require exact request formats. ## Security considerations The implementation preserves security by: - Filtering out hop-by-hop headers that shouldn't be forwarded - Maintaining proper authentication mechanisms - Only allowing passthrough for specific providers and endpoints ## 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
…requests (#732) ## Summary Implement proper context cancellation for HTTP requests to prevent resource leaks and wasted provider quota when clients disconnect, especially during streaming operations. ## Changes - Added cancellable context support in `ConvertToBifrostContext()` which now returns both a context and a cancel function - Ensured all HTTP handlers properly defer the cancel function to clean up resources - Modified streaming handlers to immediately cancel upstream provider requests when clients disconnect - Added detailed comments explaining the context cancellation pattern and its importance - Fixed potential resource leaks in streaming handlers by properly handling write errors ## Type of change - [x] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (Next.js) - [ ] Docs ## How to test Verify that streaming requests are properly cancelled when clients disconnect: ```sh # Start Bifrost server go run cmd/bifrost/main.go # In another terminal, start a streaming request and then cancel it (Ctrl+C) curl -N http://localhost:8000/v1/chat/completions \ -H "Content-Type: application/json" \ -d '{"model": "gpt-3.5-turbo", "messages": [{"role": "user", "content": "Write a very long story"}], "stream": true}' # Verify in server logs that the upstream request was cancelled ``` ## Breaking changes - [ ] Yes - [x] No ## Related issues Addresses resource leaks and wasted provider quota when clients disconnect during streaming requests. ## Security considerations Improves resource management by ensuring provider requests are cancelled when no longer needed, preventing potential resource exhaustion. ## Checklist - [x] I added/updated tests where appropriate - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable
## Summary - Adds dynamic plugin architecture - Add auth for dashboard and inference calls Closes #246 Closes #518 ## Changes - What was changed and why - Any notable design decisions or trade-offs ## Type of change - [x] Bug fix - [x] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (Next.js) - [x] Docs ## How to test Describe the steps to validate this change. Include commands and expected outcomes. ```sh # Core/Transports go version go test ./... # UI cd ui pnpm i || npm i pnpm test || npm test pnpm build || npm run build ``` If adding new configs or environment variables, document them here. ## Screenshots/Recordings If UI changes, add before/after screenshots or short clips. ## Breaking changes - [ ] Yes - [x] No If yes, describe impact and migration instructions. ## Related issues Link related issues and discussions. Example: Closes #123 ## Security considerations Note any security implications (auth, secrets, PII, sandboxing, etc.). ## 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
version bumps
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughA release script block is activated to execute provider tests during the core release process, replacing previously commented-out commands with live test execution in the tests/core-providers directory. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![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: 0
🧹 Nitpick comments (2)
.github/workflows/scripts/release-core.sh (2)
43-46: Unclear test pattern in-runflag.The pattern
go test -v -run .uses a single.regex character, which matches any single character in test names. This is unclear and non-idiomatic. To run all tests, use an empty string or omit the-runflag entirely.-go test -v -run . +go test -vAlternatively, for explicit all-test matching:
-go test -v -run . +go test -v -run ''
43-46: Add directory existence validation and improve path handling.The script navigates to
tests/core-providerswithout validating the directory exists. Additionally, usingcd ../..with hardcoded relative paths is fragile if the directory structure changes or the script is executed from a different location.Consider using
pushd/popdfor safer directory handling:-echo "🔧 Running core provider tests..." -cd tests/core-providers -go test -v -run . -cd ../.. +echo "🔧 Running core provider tests..." +if [[ ! -d "tests/core-providers" ]]; then + echo "❌ tests/core-providers directory not found" + exit 1 +fi +pushd tests/core-providers > /dev/null +go test -v +popd > /dev/null
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/scripts/release-core.sh(1 hunks)
🔇 Additional comments (1)
.github/workflows/scripts/release-core.sh (1)
39-46: Clarify whether both core tests and provider tests should run.Line 41 shows commented-out core package tests (
go test -v ./...), while lines 43-46 activate provider tests. The PR title is "enable core tests," but only provider tests are now active. Verify whether:
- Both test blocks should be active (core and provider tests)
- Only provider tests should run
- The commented core tests should be uncommented
If both test types should run, uncomment line 41 as well. If only one is intended, consider removing the other block entirely for clarity.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Briefly explain the purpose of this PR and the problem it solves.
Changes
Type of change
Affected areas
How to test
Describe the steps to validate this change. Include commands and expected outcomes.
If adding new configs or environment variables, document them here.
Screenshots/Recordings
If UI changes, add before/after screenshots or short clips.
Breaking changes
If yes, describe impact and migration instructions.
Related issues
Link related issues and discussions. Example: Closes #123
Security considerations
Note any security implications (auth, secrets, PII, sandboxing, etc.).
Checklist
docs/contributing/README.mdand followed the guidelines