feat: add per-provider disable_model_discovery to skip live model discovery

[ragokan]

Summary

GET /v1/models triggers live model discovery (list-models) against every configured provider. For built-in providers like Azure with many regional keys this fans out per key, and the connections can pile up until /v1/models hangs (and can starve inference). Built-in providers can't opt out via custom_provider_config.allowed_requests because they aren't in SupportedBaseProviders, so there was previously no way to disable discovery for them. Full investigation in #4581.

This adds a per-provider disable_model_discovery flag. When set, FetchAndStoreLiveForKey early-returns, so /v1/models serves only the statically configured models for that provider and the per-key list-models fan-out is skipped. It composes with the existing allowed_requests skip used by custom providers.

This implements option 2 from #4581 (a per-provider opt-out for built-in providers), not the global env var. Default behaviour is unchanged — discovery stays on unless explicitly disabled.

Changes

• core/schemas/provider.go — add ProviderConfig.DisableModelDiscovery (disable_model_discovery, default false).
• transports/bifrost-http/server/server.go — FetchAndStoreLiveForKey skips the live fetch when the flag is set, alongside the existing custom-provider allowed_requests skip.
• framework/configstore — persist the field end-to-end: the configstore ProviderConfig, the providers table column, GenerateConfigHash, the load/save conversions, and an add_disable_model_discovery_column migration. Because the field defaults to false (which writes nothing to the config hash), existing rows keep a valid hash and the migration only adds the column — no backfill needed.
• transports/config.schema.json — document the field on every provider block.
• transports/schema_test/config_schema_test.go — assert the schema documents it as a boolean on every provider block.
• changelogs — core/, framework/, transports/.

Type of change

• Feature

Affected areas

• Core (Go)
• Transports (HTTP)
• Providers/Integrations

How to test

go version   # 1.26
# builds (go.work spans the modules)
go build ./core/... ./framework/... ./transports/...
# schema documents the field on every provider block
go test ./transports/schema_test/ -run TestSchemaDisableModelDiscovery
# configstore hash + migrations still green
go test ./framework/configstore/ -run 'Hash|Migration'
# schema <-> Go validators
bash .github/workflows/scripts/validate-schema-sync.sh
bash .github/workflows/scripts/validate-go-config-fields.sh

Runtime: set disable_model_discovery: true on a provider in config.json. GET /v1/models then returns only that provider's statically configured models, and the gateway no longer opens list-models connections for it on startup or per request.

Breaking changes

• No

Defaults to false; existing configs and stored config_hash values are unaffected.

Related issues

Closes #4581

Security considerations

None. The flag only suppresses outbound list-models calls; it doesn't touch auth, secrets, or request handling (and can reduce outbound connection pressure).

Checklist

• I read the contributing guidelines and followed them (docs/contributing/raising-a-pr.mdx)
• I added/updated tests where appropriate
• I updated documentation where needed (config schema + changelogs)
• I verified builds succeed (Go; UI unaffected)
• I verified the schema-sync, go-config-fields validators and configstore tests pass locally

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2e0f8e87-ec17-417e-8042-659891e61fed

📥 Commits

Reviewing files that changed from the base of the PR and between b894c09 and e1a9c34.

📒 Files selected for processing (11)

• core/changelog.md
• core/schemas/provider.go
• framework/changelog.md
• framework/configstore/clientconfig.go
• framework/configstore/migrations.go
• framework/configstore/rdb.go
• framework/configstore/tables/provider.go
• transports/bifrost-http/server/server.go
• transports/changelog.md
• transports/config.schema.json
• transports/schema_test/config_schema_test.go

---

📝 Walkthrough

Summary by CodeRabbit

• New Features
  • Added a per-provider disable_model_discovery option that skips live model discovery, allowing /v1/models to serve only statically configured models. This applies to all provider types, including built-in providers.

Walkthrough

Adds a per-provider disable_model_discovery boolean flag to the provider configuration schema, persists it through the configstore (new DB column via migration, table struct, and all RDB read/write paths), short-circuits FetchAndStoreLiveForKey when set, and extends the JSON config schema and schema validation tests accordingly.

Changes

Per-provider disable_model_discovery option

Layer / File(s)	Summary
Core and configstore ProviderConfig field definitions core/schemas/provider.go, framework/configstore/clientconfig.go, framework/configstore/tables/provider.go	DisableModelDiscovery bool field added to the core ProviderConfig, the configstore ProviderConfig struct (including Redacted() and GenerateConfigHash()), and TableProvider DB struct.
DB migration and RDB read/write paths framework/configstore/migrations.go, framework/configstore/rdb.go	Migration conditionally adds the disable_model_discovery column to the provider table with rollback support; all RDB upsert, update, add, and get paths propagate DisableModelDiscovery between ProviderConfig and TableProvider.
FetchAndStoreLiveForKey early-return transports/bifrost-http/server/server.go	Adds a DisableModelDiscovery short-circuit before issuing live discovery requests; restructures the existing CustomProviderConfig/IsOperationAllowed guard to apply only when CustomProviderConfig is non-nil.
JSON schema and coverage test transports/config.schema.json, transports/schema_test/config_schema_test.go	disable_model_discovery boolean property added to governance and all provider_with_*_config definitions in the JSON schema; TestSchemaDisableModelDiscovery validates presence, boolean typing, and parity with store_raw_request_response via a new recursive schema walker.
Changelogs core/changelog.md, framework/changelog.md, transports/changelog.md	Feature entries added to all three changelogs describing the new per-provider option.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• maximhq/bifrost#4038: Refactors the same FetchAndStoreLiveForKey / live model discovery fan-out path in transports/bifrost-http/server/server.go that this PR modifies to add the DisableModelDiscovery early-return.

Suggested reviewers

• danpiths
• akshaydeo
• Pratham-Mishra04

Poem

🐇 Hop hop, no more connection race,
A flag to skip the live-model chase.
Azure keys won't pile up the FDs,
disable_model_discovery sets us free!
Static models only, swift and neat —
The rabbit rests with tidy feat. 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'feat: add per-provider disable_model_discovery to skip live model discovery' clearly summarizes the main feature being introduced—a new configuration option that disables model discovery per provider.
Description check	✅ Passed	The description is comprehensive and follows the template structure, including summary, detailed changes across all affected files, type of change, affected areas, testing instructions, breaking changes, related issues, security considerations, and a complete checklist.
Linked Issues check	✅ Passed	The PR fully addresses issue #4581 by implementing option 2: a per-provider disable_model_discovery flag for built-in providers. The implementation includes the configuration field, persistence layer, transport-level logic to skip discovery, schema documentation, and tests—covering all coding requirements.
Out of Scope Changes check	✅ Passed	All changes are scoped to implementing the disable_model_discovery feature: configuration schemas, persistence, business logic for skipping discovery, schema validation tests, and changelog entries. No unrelated modifications are present.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

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"

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[akshaydeo]

Hey not the right solution again - we will take over it

[greptile-apps]

Confidence Score: 4/5

Safe to merge. The flag is additive with a false default, all read/write paths and the config hash are updated consistently, and the early-return guard covers every call site that would otherwise open network connections.

The implementation is complete and correct end-to-end. The two non-blocking observations are: RefreshLiveModelsForProvider still creates per-key goroutines before the guard fires (they return immediately, no network I/O), and there is no idempotency migration test comparable to the one added for store_raw_request_response. Neither affects correctness or introduces regressions.

transports/bifrost-http/server/server.go (goroutine fan-out before guard) and framework/configstore/migrations.go (missing migration test) are worth a second look, though neither blocks merging.

Important Files Changed

Filename	Overview
core/schemas/provider.go	Adds DisableModelDiscovery bool to ProviderConfig; field placement and JSON tag are correct and consistent with existing boolean flags.
transports/bifrost-http/server/server.go	Adds DisableModelDiscovery early-return guard to FetchAndStoreLiveForKey; all three call sites benefit. RefreshLiveModelsForProvider still fans out N goroutines before the guard fires, which is harmless but slightly wasteful for the high-key-count use case this flag targets.
framework/configstore/clientconfig.go	Field added to ProviderConfig, Redacted(), and GenerateConfigHash consistently; existing rows with false keep a valid hash because false writes nothing to the hash.
framework/configstore/migrations.go	Migration adds the column with an idempotency guard and a rollback path; no deadlock risk since no index is created. A migration test comparable to the store_raw_request_response tests is missing.
framework/configstore/rdb.go	All four CRUD paths (UpdateProvidersConfig, UpdateProvider, AddProvider, GetProvidersConfig/GetProviderConfig) updated consistently with DisableModelDiscovery.
framework/configstore/tables/provider.go	Adds DisableModelDiscovery bool to TableProvider; no GORM tag needed (GORM derives column name from field name convention).
transports/config.schema.json	Field added to all eight provider blocks alongside store_raw_request_response; schema test asserts count parity between the two fields.
transports/schema_test/config_schema_test.go	New TestSchemaDisableModelDiscovery walks the schema tree and verifies the field is present as a boolean on every provider block; test logic is correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[GET /v1/models or key event] --> B[RefreshLiveModelsForProvider]
    B --> C{No keys?}
    C -- keyless --> D[FetchAndStoreLiveForKey]
    C -- has keys --> E[Fan-out: goroutine per key]
    E --> D
    D --> F{s.Config not nil?}
    F -- no --> G[Proceed to ListModels]
    F -- yes --> H[GetProviderConfigRaw]
    H --> I{DisableModelDiscovery?}
    I -- true --> J[Return early - no HTTP call]
    I -- false --> K{CustomProviderConfig blocks ListModels?}
    K -- true --> J
    K -- false --> G
    G --> L[ListModelsRequest x2 filtered + unfiltered]
    L --> M[UpsertLiveFromResponse model catalog]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[GET /v1/models or key event] --> B[RefreshLiveModelsForProvider]
    B --> C{No keys?}
    C -- keyless --> D[FetchAndStoreLiveForKey]
    C -- has keys --> E[Fan-out: goroutine per key]
    E --> D
    D --> F{s.Config not nil?}
    F -- no --> G[Proceed to ListModels]
    F -- yes --> H[GetProviderConfigRaw]
    H --> I{DisableModelDiscovery?}
    I -- true --> J[Return early - no HTTP call]
    I -- false --> K{CustomProviderConfig blocks ListModels?}
    K -- true --> J
    K -- false --> G
    G --> L[ListModelsRequest x2 filtered + unfiltered]
    L --> M[UpsertLiveFromResponse model catalog]

Loading

Comments Outside Diff (1)

1. transports/bifrost-http/server/server.go, line 939-959 (link)

   Goroutines still fan out before the DisableModelDiscovery guard

   When DisableModelDiscovery is true, RefreshLiveModelsForProvider still spawns N goroutines (one per key), each of which calls FetchAndStoreLiveForKey, acquires a read lock, checks the flag, and immediately returns. For a provider like Azure with many regional keys — the exact scenario this flag is designed for — startup and reload will still create O(keys) goroutines. Each finishes almost instantly with no network I/O, so there is no correctness issue, but the goroutine creation and scheduler overhead is unnecessary. Checking the flag once at the top of RefreshLiveModelsForProvider (before the loop) would eliminate this overhead entirely and makes the call site self-documenting.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

_{Reviews (1): Last reviewed commit: "[feat]: add per-provider disable_model_d..." | Re-trigger Greptile}

[greptile-apps]

No migration test for migrationAddDisableModelDiscoveryColumn

The structurally identical predecessor, migrationAddStoreRawRequestResponseColumn, has TestMigrationAddStoreRawRequestResponseColumn and TestMigrationAddStoreRawRequestResponseColumn_Idempotent (in migrations_test.go) which cover both the data-preservation path and the idempotency guard. This migration has neither. Since the migration only adds a column (no backfill logic), a minimal idempotency test — running migrationAddDisableModelDiscoveryColumn twice on the same in-memory SQLite DB and asserting no error — would protect against regression if the HasColumn guard is accidentally removed in a future edit.

[akshaydeo]

This is not the right way - we just need to use cached table and trigger a background sync to make sure they are in the right state.

This does not require these many changes

The merge-base changed after approval.