fix: honor model_parameters_url changes in config.json like pricing_url

[jeremym-tanium]

Summary

model_parameters_url set in config.json is ignored on restart once the DB value is populated, unlike pricing_url which already follows config.json on change. This makes model_parameters_url behave the same way.

Changes

• In ResolveFrameworkPricingConfig, add a "file hash changed → file overrides DB" branch for model_parameters_url, mirroring the existing pricing_url branch.
• Include model_parameters_url in the set of fields that trigger the framework-config hash, so a change to model_parameters_url alone is detected. Previously only pricing_url/sync/MCP triggered it, even though GenerateFrameworkConfigHash already incorporates model_parameters_url — so without this the override branch above could never fire for a model_parameters_url-only config.
• Add unit tests in TestResolveFrameworkPricingConfig for both: file-overrides-DB on change, and the model_parameters_url-only case.

Type of change

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

Affected areas

• Core (Go)
• Transports (HTTP)
• Providers/Integrations
• Plugins
• UI (React)
• Docs

How to test

cd transports
go test ./bifrost-http/lib/ -run TestResolveFrameworkPricingConfig -v

Ran the targeted unit tests locally against a workspace build; both new sub-tests pass, and both fail without the fix (the stale DB value is retained). The full suite needs live provider credentials, so I ran the targeted package tests only.

Breaking changes

• Yes
• No

Behavior only changes for a config that sets model_parameters_url and later changes it — that change is now honored. Configs that don't change it behave identically.

Related issues

Closes #4863

Security considerations

None. Config reconciliation only; no change to auth or request/response data paths.

[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: 5aa221e8-e4ba-46c4-9ee2-f1d3a0dabc5f

📥 Commits

Reviewing files that changed from the base of the PR and between 4143576 and 79ccb9d.

📒 Files selected for processing (2)

• transports/bifrost-http/lib/config.go
• transports/bifrost-http/lib/config_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

• transports/bifrost-http/lib/config.go
• transports/bifrost-http/lib/config_test.go

---

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes

  • Improved detection of framework pricing configuration changes, including updates to model_parameters_url.
  • File-provided model_parameters_url can now correctly override stored values when appropriate, preventing stale pricing-related settings.
  • Ensures unresolved model_parameters_url values (e.g., env literals) do not overwrite a valid stored database value.

• Chores

  • Updated the changelog entry to document the configuration fix.

Walkthrough

This PR updates ResolveFrameworkPricingConfig so model_parameters_url participates in file-change detection and can override the stored DB value when config.json changes. It also adds tests for the new reconciliation paths and a changelog entry.

Changes

model_parameters_url reconciliation fix

Layer / File(s)	Summary
Hash gating and DB override logic for model_parameters_url transports/bifrost-http/lib/config.go	Expands hash computation to include model_parameters_url when resolved file data is available and changes DB backfill to prefer the file value when the file hash changed.
Tests and changelog for model_parameters_url override transports/bifrost-http/lib/config_test.go, transports/changelog.md	Adds subtests covering file-over-DB override behavior, hash computation when only model_parameters_url is present, unresolved env literals, and a changelog entry.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• maximhq/bifrost#3608: Both PRs modify ResolveFrameworkPricingConfig to reconcile and persist model_parameters_url with DB/file precedence.
• maximhq/bifrost#3610: Both PRs update the same config hash mechanism that decides whether file-derived values override the DB.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: honoring model_parameters_url config changes like pricing_url.
Description check	✅ Passed	The description covers the problem, fix, tests, issue link, and required template sections well enough.
Linked Issues check	✅ Passed	The code changes match issue #4863 by detecting model_parameters_url changes, overriding the DB on file change, and adding regression tests.
Out of Scope Changes check	✅ Passed	No unrelated code changes are evident beyond a supporting changelog entry.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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.}

[greptile-apps]

Confidence Score: 5/5

Safe to merge — the change is a narrow, targeted fix confined to config reconciliation logic with no impact on auth, request/response paths, or streaming.

The fix is a direct mirror of the already-tested pricing_url override branch applied to model_parameters_url. Phase ordering is preserved, the nil guard in Phase 4 catches invariant violations, and three new sub-tests cover the primary regression, the hash-computation prerequisite, and the env-unresolved safety case.

No files require special attention.

Important Files Changed

Filename	Overview
transports/bifrost-http/lib/config.go	Two targeted changes: hash-gate condition extended to include model_parameters_url, and Phase 3 DB-override logic added to mirror pricing_url — logic is correct and consistent with the existing pattern.
transports/bifrost-http/lib/config_test.go	Three new sub-tests added to TestResolveFrameworkPricingConfig covering file-overrides-DB, model_parameters_url-only hash trigger, and unresolved-env guard — all three scenarios are meaningful and well-constructed.
transports/changelog.md	Changelog entry added for the bug fix.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ResolveFrameworkPricingConfig] --> B[Phase 1: Resolve file config\nenv substitution, validation]
    B --> C{fileModelParametersURL != nil\n&& !skipModelParamsURLBackfill?}
    C -- Yes --> D[Phase 2: resolvedModelParametersURL\n= fileModelParametersURL]
    C -- No --> E[Phase 2: resolvedModelParametersURL\n= default]
    D --> F[Phase 3: Compute fileHash\nnow includes model_parameters_url\nin gate condition]
    E --> F
    F --> G{fileHash != storedHash\nfileChanged?}
    G -- No --> H[DB wins:\nresolvedModelParametersURL\n= dbConfig.ModelParametersURL]
    G -- Yes --> I{dbConfig.ModelParametersURL\n!= nil && != empty?}
    I -- Yes --> J{fileChanged &&\nfileModelParametersURL != nil &&\n!skipModelParamsURLBackfill?}
    J -- Yes --> K[File wins: Phase 1 value\nkept, needsDBUpdate = true]
    J -- No --> H
    I -- No --> L[needsDBUpdate = true\nbackfill from file]
    K --> M[Phase 4: nil guard]
    H --> M
    L --> M
    M --> N[Return normalized table\n+ model catalog config]

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[ResolveFrameworkPricingConfig] --> B[Phase 1: Resolve file config\nenv substitution, validation]
    B --> C{fileModelParametersURL != nil\n&& !skipModelParamsURLBackfill?}
    C -- Yes --> D[Phase 2: resolvedModelParametersURL\n= fileModelParametersURL]
    C -- No --> E[Phase 2: resolvedModelParametersURL\n= default]
    D --> F[Phase 3: Compute fileHash\nnow includes model_parameters_url\nin gate condition]
    E --> F
    F --> G{fileHash != storedHash\nfileChanged?}
    G -- No --> H[DB wins:\nresolvedModelParametersURL\n= dbConfig.ModelParametersURL]
    G -- Yes --> I{dbConfig.ModelParametersURL\n!= nil && != empty?}
    I -- Yes --> J{fileChanged &&\nfileModelParametersURL != nil &&\n!skipModelParamsURLBackfill?}
    J -- Yes --> K[File wins: Phase 1 value\nkept, needsDBUpdate = true]
    J -- No --> H
    I -- No --> L[needsDBUpdate = true\nbackfill from file]
    K --> M[Phase 4: nil guard]
    H --> M
    L --> M
    M --> N[Return normalized table\n+ model catalog config]

Loading

_{Reviews (2): Last reviewed commit: "fix: guard model_parameters_url override..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

transports/bifrost-http/lib/config_test.go (1)

17514-17565: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Consider adding a regression test for the skipModelParamsURLBackfill + unrelated-field-change interaction.

None of the new (or existing) subtests cover the case where model_parameters_url has an unresolved env.* reference while pricing_url (or another hashed field) changes in the same restart. As flagged in config.go (lines 3952-3961), this combination can currently persist the raw unresolved literal into the DB. A subtest setting Pricing.ModelParametersURL = &"env.MISSING_VAR" alongside a changed Pricing.PricingURL, then asserting resolvedModelParametersURL still reflects dbConfig.ModelParametersURL, would pin down the fix once applied.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@transports/bifrost-http/lib/config_test.go` around lines 17514 - 17565, Add a
regression subtest in ResolveFrameworkPricingConfig coverage for the
skipModelParamsURLBackfill case when model_parameters_url is an unresolved env
reference but another hashed field like pricing_url changes in the same restart.
Use the existing ResolveFrameworkPricingConfig and
configstore.GenerateFrameworkConfigHash paths to set Pricing.ModelParametersURL
to env.MISSING_VAR and change Pricing.PricingURL, then assert the unresolved
literal is not written back and the normalized result keeps
dbConfig.ModelParametersURL as the resolvedModelParametersURL source. This
should live alongside the current pricing/model_parameters_url override subtests
in config_test.go.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@transports/bifrost-http/lib/config.go`:
- Around line 3952-3961: The model_parameters_url override path in config
resolution is missing the same backfill guard used by the MCPLibraryURL branch,
which can persist an unresolved env.* literal into the DB when fileChanged is
triggered by another field. Update the conditional in the model_parameters_url
handling within the config resolution flow to also require
!skipModelParamsURLBackfill before treating fileChanged as a reason to overwrite
the DB, mirroring the surrounding backfill logic and keeping unresolved values
from being persisted.

---

Nitpick comments:
In `@transports/bifrost-http/lib/config_test.go`:
- Around line 17514-17565: Add a regression subtest in
ResolveFrameworkPricingConfig coverage for the skipModelParamsURLBackfill case
when model_parameters_url is an unresolved env reference but another hashed
field like pricing_url changes in the same restart. Use the existing
ResolveFrameworkPricingConfig and configstore.GenerateFrameworkConfigHash paths
to set Pricing.ModelParametersURL to env.MISSING_VAR and change
Pricing.PricingURL, then assert the unresolved literal is not written back and
the normalized result keeps dbConfig.ModelParametersURL as the
resolvedModelParametersURL source. This should live alongside the current
pricing/model_parameters_url override subtests in config_test.go.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1fda2098-525e-4dfb-a605-485d4da4ec8f

📥 Commits

Reviewing files that changed from the base of the PR and between 97c8a21 and 4143576.

📒 Files selected for processing (3)

• transports/bifrost-http/lib/config.go
• transports/bifrost-http/lib/config_test.go
• transports/changelog.md