fix: parse provider-prefixed model string in runPreRequestRouting large-payload path - #4282
Conversation
|
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR updates the large-payload routing path in ChangesLarge-payload routing provider parsing
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 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 |
Confidence Score: 4/5The core fix is correct and the transport-layer analogue works the same way; the change is narrow and low-risk. The fix is a one-call change that delegates to a well-tested utility (ParseModelString) and the three new tests directly validate the affected paths. The only gap is that the hasRoutingRules: true branch with a provider-prefixed model in the large-payload path has no test coverage — it's a minor omission, not a defect in the fix itself. No files require special attention; the change is confined to plugins/governance/main.go and its new test file. Important Files Changed
Reviews (1): Last reviewed commit: "fix: governance routing for large payloa..." | Re-trigger Greptile |
Merge activity
|
…arge-payload path (#4282) ## Summary When a large-payload request arrives via the `PreRequestHook` path, the request body is not parsed, so `req.Model` is taken directly from metadata and may carry a provider-prefixed string like `openai/gpt-4o`. Previously, `runPreRequestRouting` passed this raw string directly as the model name without parsing the provider prefix, causing load balancing to ignore the caller's explicit routing intent. This change ensures the provider prefix is extracted and honored in the same way the transport layer handles body-having requests. ## Changes - `runPreRequestRouting` now calls `schemas.ParseModelString` on the incoming model string before constructing the synthetic `BifrostRequest`, populating both `Provider` and `Model` on the `ChatRequest` so that an explicit prefix like `openai/gpt-4o` correctly pins routing to that provider instead of being subject to VK load balancing. - Added `prerequestrouting_test.go` covering three cases: an explicit provider prefix bypasses load balancing and always resolves to the specified provider; a bare model string still goes through VK load balancing and returns provider-prefixed; and an unknown slash-containing prefix (e.g. a HuggingFace-style namespace like `meta-llama/llama-3.1-8b-instant`) is treated as part of the model name with load balancing still applied. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./plugins/governance/... ``` Expected: all three new tests pass, confirming that provider-prefixed models in the large-payload path are routed to the correct provider, bare models still load balance, and unknown slash-prefixes are preserved as model namespaces. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations No security implications. This change only affects internal routing logic for constructing synthetic requests during pre-request hook processing. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Bug Fixes** * Fixed request routing to properly preserve explicit provider specifications in model names (e.g., `provider/model` format), ensuring routing intent is maintained for large-payload requests. * **Tests** * Added comprehensive tests for pre-request routing behavior, including explicit provider prefix handling and load balancing scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai -->

Summary
When a large-payload request arrives via the
PreRequestHookpath, the request body is not parsed, soreq.Modelis taken directly from metadata and may carry a provider-prefixed string likeopenai/gpt-4o. Previously,runPreRequestRoutingpassed this raw string directly as the model name without parsing the provider prefix, causing load balancing to ignore the caller's explicit routing intent. This change ensures the provider prefix is extracted and honored in the same way the transport layer handles body-having requests.Changes
runPreRequestRoutingnow callsschemas.ParseModelStringon the incoming model string before constructing the syntheticBifrostRequest, populating bothProviderandModelon theChatRequestso that an explicit prefix likeopenai/gpt-4ocorrectly pins routing to that provider instead of being subject to VK load balancing.prerequestrouting_test.gocovering three cases: an explicit provider prefix bypasses load balancing and always resolves to the specified provider; a bare model string still goes through VK load balancing and returns provider-prefixed; and an unknown slash-containing prefix (e.g. a HuggingFace-style namespace likemeta-llama/llama-3.1-8b-instant) is treated as part of the model name with load balancing still applied.Type of change
Affected areas
How to test
go test ./plugins/governance/...Expected: all three new tests pass, confirming that provider-prefixed models in the large-payload path are routed to the correct provider, bare models still load balance, and unknown slash-prefixes are preserved as model namespaces.
Breaking changes
Related issues
Security considerations
No security implications. This change only affects internal routing logic for constructing synthetic requests during pre-request hook processing.
Checklist
docs/contributing/README.mdand followed the guidelinesSummary by CodeRabbit
Release Notes
Bug Fixes
provider/modelformat), ensuring routing intent is maintained for large-payload requests.Tests