Skip to content

feat: enhance model catalog with provider-prefixed model support and cross-provider routing#1325

Merged
akshaydeo merged 1 commit intomainfrom
01-14-feat_provider_preffixed_model_matching_enhancements_and_doc_updates
Jan 14, 2026
Merged

feat: enhance model catalog with provider-prefixed model support and cross-provider routing#1325
akshaydeo merged 1 commit intomainfrom
01-14-feat_provider_preffixed_model_matching_enhancements_and_doc_updates

Conversation

@Pratham-Mishra04
Copy link
Copy Markdown
Collaborator

Summary

Enhanced the Model Catalog with improved cross-provider model resolution and validation, enabling more flexible routing configurations including provider-prefixed model names in allowed lists.

Changes

  • Added comprehensive cross-provider model resolution logic to identify all providers that can serve a specific model
  • Implemented IsModelAllowedForProvider method to validate models against allowed lists with support for provider-prefixed entries
  • Updated governance plugin to use the new model validation method for consistent handling across routing mechanisms
  • Enhanced documentation with detailed examples of cross-provider routing scenarios and provider-prefixed model configurations
  • Added support for special provider formats (OpenRouter, Vertex, Groq, Bedrock) in model resolution

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (Next.js)
  • Docs

How to test

Test provider-prefixed model configurations with governance routing:

# Configure a virtual key with provider-prefixed allowed models
curl -X POST http://localhost:8080/api/v1/virtual-keys \
  -H "Content-Type: application/json" \
  -d '{
    "name": "test-prefixed-models",
    "provider_configs": [
      {
        "provider": "openrouter",
        "allowed_models": ["openai/gpt-4o", "anthropic/claude-3-5-sonnet"]
      }
    ]
  }'

# Test with a model that matches the prefix
curl -X POST http://localhost:8080/v1/chat/completions \
  -H "x-bf-vk: test-prefixed-models" \
  -d '{"model": "gpt-4o", "messages": [{"role": "user", "content": "Hello"}]}'
# Should succeed and route to OpenRouter

# Test with a model that doesn't match any prefix
curl -X POST http://localhost:8080/v1/chat/completions \
  -H "x-bf-vk: test-prefixed-models" \
  -d '{"model": "gpt-4-turbo", "messages": [{"role": "user", "content": "Hello"}]}'
# Should fail with "model not allowed for any configured provider"

Breaking changes

  • No

Related issues

Improves cross-provider routing capabilities discussed in various community threads.

Security considerations

No security implications. This change enhances model validation logic but doesn't affect authentication or authorization.

Checklist

  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)

Copy link
Copy Markdown
Collaborator Author

Pratham-Mishra04 commented Jan 14, 2026

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 14, 2026

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Enhanced model validation and cross-provider resolution, plus dynamic model pool management.
  • Bug Fixes

    • Fixed weighted provider routing to correctly handle provider-prefixed model names in allowed lists.
  • Documentation

    • Expanded Model Catalog and Provider Routing docs: cross-provider resolution steps, governance/load‑balancing interactions, fallbacks, examples, thread-safety, and best practices.
  • Chores

    • Removed legacy MCP integration and related tooling.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Adds cross-provider model resolution and validation via a new ModelCatalog.IsModelAllowedForProvider method and integrates it into governance (BudgetResolver now accepts ModelCatalog); extensive docs expanded for model-catalog/provider-routing behaviors; removes MCP integration (deletes core/mcp.go); updates governance constructor usage in tests and changelogs.

Changes

Cohort / File(s) Summary
Documentation
docs/architecture/framework/model-catalog.mdx, docs/providers/provider-routing.mdx
Major expansions: cross-provider resolution, provider-prefixed model rules, model discovery/validation, usage examples (governance, load-balancing), sync/fallback behaviors, and best practices.
Framework Model Catalog
framework/modelcatalog/main.go
Added IsModelAllowedForProvider(provider schemas.ModelProvider, model string, allowedModels []string) bool implementing empty vs non-empty allowedModels semantics, direct and provider-prefixed matching, and use of GetModelsForProvider/GetProvidersForModel.
Governance Plugin (integration & logic)
plugins/governance/main.go, plugins/governance/resolver.go
BudgetResolver now accepts a ModelCatalog dependency (constructor signature changed). isModelAllowed delegates to ModelCatalog when present; preserved fallback when catalog is nil.
Governance Tests
plugins/governance/resolver_test.go, plugins/governance/tracker_test.go
Updated NewBudgetResolver calls to pass an extra nil argument to match the new constructor signature.
Core removal
core/mcp.go
Deleted file and all MCP integration: types, constructors, connections, tool discovery/registration, execution, and cleanup helpers removed.
Changelogs / Metadata
framework/changelog.md, plugins/governance/changelog.md, transports/changelog.md
Added feature notes documenting improved provider-prefixed model matching and routing fixes.

Sequence Diagram

sequenceDiagram
    participant Client as Request
    participant Governance as Governance Plugin
    participant BudgetResolver as BudgetResolver
    participant ModelCatalog as ModelCatalog
    participant Provider as Provider Catalog

    Client->>Governance: incoming request (model)
    Governance->>BudgetResolver: loadBalanceProvider(provider, model, allowedModels)
    BudgetResolver->>ModelCatalog: IsModelAllowedForProvider(provider, model, allowedModels)
    alt allowedModels is empty
        ModelCatalog->>ModelCatalog: GetProvidersForModel(model)
        ModelCatalog->>Provider: Query provider support (direct, OpenRouter, Vertex, Groq, Bedrock)
        Provider-->>ModelCatalog: provider supports model?
        ModelCatalog->>ModelCatalog: contains target provider?
    else allowedModels non-empty
        ModelCatalog->>Provider: GetModelsForProvider(provider)
        Provider-->>ModelCatalog: provider's model list
        alt direct match
            ModelCatalog->>ModelCatalog: match found -> allowed
        else provider-prefixed entry
            ModelCatalog->>ModelCatalog: parse prefix, compare model part -> allowed?
        end
    end
    ModelCatalog-->>BudgetResolver: isAllowed (bool)
    BudgetResolver-->>Governance: validation result
    Governance-->>Client: routing decision
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • v1.4.0 #1153: Updates to BudgetResolver constructor and model-allowance logic that align with this PR's governance → model-catalog integration.

Suggested reviewers

  • danpiths
  • roroghost17

Poem

🐰
I hopped through catalogs, bright and spry,
Found prefixed names that caught my eye,
Governance nudged, routes sprang to light,
Models hopped home across the night,
A rabbit cheers: catalogs unite! 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding provider-prefixed model support and cross-provider routing to the model catalog.
Description check ✅ Passed The description covers all required sections including summary, changes, type of change, affected areas, testing instructions, and breaking changes. All mandatory sections are complete.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 01-14-feat_provider_preffixed_model_matching_enhancements_and_doc_updates

🧹 Recent nitpick comments
plugins/governance/tracker_test.go (1)

28-29: Constructor signature updates look correct.

All test functions correctly updated to use the new NewBudgetResolver(store, nil, logger) signature. Passing nil for modelCatalog triggers the fallback simple string-matching behavior in isModelAllowed, preserving existing test semantics.

Consider adding dedicated tests for IsModelAllowedForProvider integration when modelCatalog is non-nil to verify cross-provider routing logic.

Also applies to: 63-64, 97-98, 160-161

docs/architecture/framework/model-catalog.mdx (1)

253-253: Optional: Consider hyphenating "load-balancing" as a compound adjective.

The static analysis tool suggests hyphenating "load-balancing" when used as a compound adjective before "plugins". Both forms are commonly used in technical documentation, so this is a minor stylistic preference.


📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d245177 and 6f506cb.

📒 Files selected for processing (11)
  • core/mcp.go
  • docs/architecture/framework/model-catalog.mdx
  • docs/providers/provider-routing.mdx
  • framework/changelog.md
  • framework/modelcatalog/main.go
  • plugins/governance/changelog.md
  • plugins/governance/main.go
  • plugins/governance/resolver.go
  • plugins/governance/resolver_test.go
  • plugins/governance/tracker_test.go
  • transports/changelog.md
💤 Files with no reviewable changes (1)
  • core/mcp.go
🚧 Files skipped from review as they are similar to previous changes (5)
  • framework/changelog.md
  • plugins/governance/changelog.md
  • plugins/governance/resolver_test.go
  • transports/changelog.md
  • plugins/governance/main.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:

  • plugins/governance/tracker_test.go
  • framework/modelcatalog/main.go
  • plugins/governance/resolver.go
  • docs/architecture/framework/model-catalog.mdx
  • docs/providers/provider-routing.mdx
🧠 Learnings (6)
📚 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:

  • plugins/governance/tracker_test.go
  • framework/modelcatalog/main.go
  • plugins/governance/resolver.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:

  • plugins/governance/tracker_test.go
  • framework/modelcatalog/main.go
  • plugins/governance/resolver.go
📚 Learning: 2026-01-14T04:40:11.480Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1312
File: framework/modelcatalog/pricing.go:276-426
Timestamp: 2026-01-14T04:40:11.480Z
Learning: In the Bifrost codebase, ImageUsage and other usage types guarantee that TotalTokens is populated (computed as InputTokens + OutputTokens if providers don’t supply TotalTokens). Reviewers can rely on this invariant and should not assume TotalTokens may be missing when input/output tokens exist. When implementing tiering logic or token-based decisions, you can safely use TotalTokens without extra null/zero guards, provided you’re in a context where InputTokens and OutputTokens are present. If a branch might discard tokens, ensure the invariant is preserved or add explicit checks only where the inputs are confirmed to be valid.

Applied to files:

  • plugins/governance/tracker_test.go
  • framework/modelcatalog/main.go
  • plugins/governance/resolver.go
📚 Learning: 2026-01-14T13:30:28.760Z
Learnt from: Radheshg04
Repo: maximhq/bifrost PR: 1326
File: plugins/semanticcache/test_utils.go:545-559
Timestamp: 2026-01-14T13:30:28.760Z
Learning: In the maximhq/bifrost repository, prefer using bifrost.Ptr() to create pointers instead of the address operator (&) even when & would be valid syntactically. Apply this consistently across all code paths, including test utilities, to improve consistency and readability. Replace occurrences of &value where a *T is expected with bifrost.Ptr(value) (or an equivalent call) and ensure the function is in scope and used correctly for the target pointer type.

Applied to files:

  • plugins/governance/tracker_test.go
  • framework/modelcatalog/main.go
  • plugins/governance/resolver.go
📚 Learning: 2025-12-22T10:50:40.990Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1154
File: plugins/governance/store.go:1165-1186
Timestamp: 2025-12-22T10:50:40.990Z
Learning: In the Bifrost governance plugin, budgets and rate limits have 1:1 relationships with their parent entities (virtual keys, teams, customers). Do not assume sharing; ensure cascade deletion logic only deletes budgets/rate limits when there are no shared references. Enforce invariants in code and add tests to verify no cross-entity sharing and that cascade deletes only remove the specific child of the parent. If a counterexample arises, adjust data model or add guards.

Applied to files:

  • plugins/governance/tracker_test.go
  • plugins/governance/resolver.go
📚 Learning: 2025-12-30T05:37:48.365Z
Learnt from: Pratham-Mishra04
Repo: maximhq/bifrost PR: 1180
File: docs/features/mcp/connecting-to-servers.mdx:452-458
Timestamp: 2025-12-30T05:37:48.365Z
Learning: When reviewing documentation PRs in a Graphite-managed stack, first check related or previous PRs in the stack for feature implementations before flagging documentation as incorrect or unsupported. Documentation MDX files often reference features implemented in earlier stack PRs; verify that the documented behavior exists in earlier changes and that the docs accurately reflect the implemented state before requesting edits.

Applied to files:

  • docs/architecture/framework/model-catalog.mdx
  • docs/providers/provider-routing.mdx
🧬 Code graph analysis (3)
plugins/governance/tracker_test.go (1)
plugins/governance/resolver.go (1)
  • NewBudgetResolver (72-78)
framework/modelcatalog/main.go (2)
core/schemas/bifrost.go (1)
  • ModelProvider (30-30)
core/schemas/utils.go (1)
  • ParseModelString (21-34)
plugins/governance/resolver.go (3)
plugins/governance/store.go (1)
  • GovernanceStore (50-72)
framework/modelcatalog/main.go (1)
  • ModelCatalog (23-46)
core/schemas/provider.go (1)
  • Provider (317-366)
🪛 LanguageTool
docs/architecture/framework/model-catalog.mdx

[grammar] ~253-~253: Use a hyphen to join words.
Context: ...s used internally by governance and load balancing plugins. ```go // Empty allow...

(QB_NEW_EN_HYPHEN)

🔇 Additional comments (13)
docs/providers/provider-routing.mdx (4)

28-31: Good addition of architecture documentation cross-reference.

The Info block linking to Model Catalog Architecture helps users find detailed technical documentation when needed.


85-124: Cross-provider resolution rules well documented.

The step-by-step documentation of OpenRouter, Vertex, Groq, and Bedrock special handling matches the implementation in GetProvidersForModel. This helps users understand how Bifrost discovers provider availability for models.


388-423: Provider-prefixed entries documentation is clear and accurate.

The explanation of how Bifrost strips the prefix and matches against the requested model aligns with the IsModelAllowedForProvider implementation. The step-by-step example at lines 405-413 clearly illustrates the matching process.


625-653: Excellent practical example for weighted routing via proxy provider.

This example demonstrates a real-world use case where 99% of traffic routes through OpenRouter for cost savings while maintaining 1% direct to OpenAI for fallback. The explanation of how "openai/gpt-4o" in allowed_models matches requests for "gpt-4o" is clear.

docs/architecture/framework/model-catalog.mdx (2)

9-11: Good cross-reference to Provider Routing documentation.

The Info block helps users discover detailed routing examples and cross-provider scenarios.


252-294: IsModelAllowedForProvider documentation is comprehensive and accurate.

The documentation clearly explains:

  • Empty allowedModels delegates to GetProvidersForModel (cross-provider logic)
  • Non-empty allowedModels checks direct and provider-prefixed matches
  • Provider-prefixed entries are validated against the provider's catalog

The examples demonstrate all three use cases effectively.

plugins/governance/resolver.go (3)

12-12: Import addition is correct.

The modelcatalog import is required for the new modelCatalog field in BudgetResolver.


64-78: BudgetResolver struct and constructor updates are well-designed.

The modelCatalog field as a pointer allows graceful fallback to simple string matching when the catalog is unavailable, maintaining backward compatibility with existing deployments.


162-186: isModelAllowed method correctly integrates with ModelCatalog.

The updated logic:

  1. Empty ProviderConfigs allows all models (line 165-167)
  2. When modelCatalog is available, delegates to IsModelAllowedForProvider for cross-provider resolution (lines 174-176)
  3. Fallback to simple string matching when catalog is unavailable (lines 178-181)

This maintains backward compatibility while enabling the new cross-provider routing features when the model catalog is configured.

framework/modelcatalog/main.go (4)

310-348: Comprehensive documentation for IsModelAllowedForProvider.

The extensive doc comments clearly explain:

  • Parameter semantics
  • Behavior for empty vs non-empty allowedModels
  • Provider-prefixed matching rules
  • Concrete examples for common scenarios

This level of documentation is excellent for a public API method.


349-355: Empty allowedModels case correctly delegates to GetProvidersForModel.

By calling GetProvidersForModel, this case leverages all existing cross-provider resolution logic (OpenRouter, Vertex, Groq, Bedrock special handling) without duplication.


357-383: Non-empty allowedModels case handles both direct and prefixed matches correctly.

The logic:

  1. Direct match (line 363-365): Simple string equality
  2. Prefixed match (lines 369-379):
    • Validates that the prefixed model exists in the provider's catalog (line 372)
    • Extracts model part using ParseModelString (line 374)
    • Compares with requested model (line 375)

The catalog validation step (line 372) is important—it ensures that arbitrary provider prefixes don't create false matches.


349-383: Thread safety is correctly maintained through delegation.

The method delegates to GetProvidersForModel and GetModelsForProvider, both of which acquire read locks internally. This pattern is consistent with other ModelCatalog methods and ensures thread-safe access to modelPool.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

🧪 Test Suite Available

This PR can be tested by a repository admin.

Run tests for PR #1325

@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 01-14-feat_provider_preffixed_model_matching_enhancements_and_doc_updates branch from bd950c3 to d245177 Compare January 14, 2026 09:03
@coderabbitai coderabbitai Bot requested a review from roroghost17 January 14, 2026 09:04
@Pratham-Mishra04 Pratham-Mishra04 force-pushed the 01-14-feat_provider_preffixed_model_matching_enhancements_and_doc_updates branch from d245177 to 6f506cb Compare January 14, 2026 19:52
Copy link
Copy Markdown
Contributor

akshaydeo commented Jan 14, 2026

Merge activity

  • Jan 14, 10:02 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jan 14, 10:02 PM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo akshaydeo merged commit 4f19500 into main Jan 14, 2026
9 checks passed
@akshaydeo akshaydeo deleted the 01-14-feat_provider_preffixed_model_matching_enhancements_and_doc_updates branch January 14, 2026 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants