feat: model_parameters_url configuration via config JSON and Helm chart - #3609
Merged
akshaydeo merged 4 commits intoMay 20, 2026
Conversation
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds an optional ChangesModel Parameters URL Pricing Configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Author
This was referenced May 20, 2026
impoiler
marked this pull request as ready for review
May 20, 2026 06:36
impoiler
force-pushed
the
05-20-feat_made_model_parameters_url_configurable
branch
from
May 20, 2026 06:37
aa5a41d to
a329ffb
Compare
impoiler
force-pushed
the
05-20-feat_model_parameters_url_configuration_via_config_json_and_helm_chart
branch
from
May 20, 2026 06:37
3754f4e to
27cb862
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
helm-charts/bifrost/templates/_helpers.tpl (1)
337-345:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude
model_parameters_urlin the pricing-block emission guard.
model_parameters_urlis mapped, butframework.pricingis only emitted whenpricing_urlorpricing_sync_intervalexists. If onlymodelParametersUrlis set, the new field is silently dropped.💡 Proposed fix
-{{- if or $pricing.pricing_url $pricing.pricing_sync_interval }} +{{- if or $pricing.pricing_url $pricing.pricing_sync_interval $pricing.model_parameters_url }} {{- $_ := set $framework "pricing" $pricing }} {{- end }}🤖 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 `@helm-charts/bifrost/templates/_helpers.tpl` around lines 337 - 345, The template currently sets $pricing.model_parameters_url but only emits $framework.pricing when $pricing.pricing_url or $pricing.pricing_sync_interval exist, causing model_parameters_url to be dropped; update the emission guard for $framework "pricing" (the or condition that checks $pricing.pricing_url and $pricing.pricing_sync_interval) to also check $pricing.model_parameters_url so that if only model_parameters_url is present the set $framework "pricing" $pricing call still runs; locate the block that uses the or guard and include $pricing.model_parameters_url in that condition.
🤖 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.
Outside diff comments:
In `@helm-charts/bifrost/templates/_helpers.tpl`:
- Around line 337-345: The template currently sets $pricing.model_parameters_url
but only emits $framework.pricing when $pricing.pricing_url or
$pricing.pricing_sync_interval exist, causing model_parameters_url to be
dropped; update the emission guard for $framework "pricing" (the or condition
that checks $pricing.pricing_url and $pricing.pricing_sync_interval) to also
check $pricing.model_parameters_url so that if only model_parameters_url is
present the set $framework "pricing" $pricing call still runs; locate the block
that uses the or guard and include $pricing.model_parameters_url in that
condition.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 703741aa-2aed-40f8-b2c2-2a28a71aa665
📒 Files selected for processing (6)
examples/configs/withframework/config.jsonexamples/dockers/data/config.jsonhelm-charts/bifrost/templates/_helpers.tplhelm-charts/bifrost/values.schema.jsonhelm-charts/bifrost/values.yamltransports/config.schema.json
Contributor
Confidence Score: 4/5Safe to merge once the guard condition in the Helm template is updated to include The guard condition at line 343 of helm-charts/bifrost/templates/_helpers.tpl — the guard condition needs to include Important Files Changed
Reviews (3): Last reviewed commit: "feat: model_parameters_url configuration..." | Re-trigger Greptile |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
helm-charts/bifrost/templates/_helpers.tpl (1)
343-345:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude
model_parameters_urlin the pricing-object emission guard.
model_parameters_urlis set on Line 338, but the guard on Line 343 only checkspricing_urlandpricing_sync_interval. With onlymodelParametersUrlconfigured,framework.pricingis dropped from rendered config.As per coding guidelines: "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."Proposed fix
-{{- if or $pricing.pricing_url $pricing.pricing_sync_interval }} +{{- if or $pricing.pricing_url $pricing.pricing_sync_interval $pricing.model_parameters_url }} {{- $_ := set $framework "pricing" $pricing }} {{- end }}🤖 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 `@helm-charts/bifrost/templates/_helpers.tpl` around lines 343 - 345, The guard that emits framework.pricing only checks $pricing.pricing_url and $pricing.pricing_sync_interval, so if only model_parameters_url is set the block is skipped; update the conditional used before calling set (the if or ...) to also include $pricing.model_parameters_url (i.e. if or $pricing.pricing_url $pricing.pricing_sync_interval $pricing.model_parameters_url) so framework.pricing is preserved when model_parameters_url is present while keeping the existing set $framework "pricing" $pricing behavior.
🤖 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.
Outside diff comments:
In `@helm-charts/bifrost/templates/_helpers.tpl`:
- Around line 343-345: The guard that emits framework.pricing only checks
$pricing.pricing_url and $pricing.pricing_sync_interval, so if only
model_parameters_url is set the block is skipped; update the conditional used
before calling set (the if or ...) to also include $pricing.model_parameters_url
(i.e. if or $pricing.pricing_url $pricing.pricing_sync_interval
$pricing.model_parameters_url) so framework.pricing is preserved when
model_parameters_url is present while keeping the existing set $framework
"pricing" $pricing behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 168bbdc5-ca52-432a-8b08-865df30c39e3
📒 Files selected for processing (6)
examples/configs/withframework/config.jsonexamples/dockers/data/config.jsonhelm-charts/bifrost/templates/_helpers.tplhelm-charts/bifrost/values.schema.jsonhelm-charts/bifrost/values.yamltransports/config.schema.json
✅ Files skipped from review due to trivial changes (1)
- helm-charts/bifrost/values.yaml
impoiler
force-pushed
the
05-20-feat_model_parameters_url_configuration_via_config_json_and_helm_chart
branch
from
May 20, 2026 08:29
27cb862 to
685afb8
Compare
impoiler
force-pushed
the
05-20-feat_made_model_parameters_url_configurable
branch
from
May 20, 2026 08:29
a329ffb to
09a6db3
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
helm-charts/bifrost/templates/_helpers.tpl (1)
343-345:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude
model_parameters_urlin the pricing object emission guard.
model_parameters_urlis set at Line 338, but the guard at Line 343 only checkspricing_urlandpricing_sync_interval. IfmodelParametersUrlis the only configured pricing field,framework.pricingis dropped from rendered config.💡 Proposed fix
-{{- if or $pricing.pricing_url $pricing.pricing_sync_interval }} +{{- if or $pricing.pricing_url $pricing.pricing_sync_interval $pricing.model_parameters_url }} {{- $_ := set $framework "pricing" $pricing }} {{- end }}🤖 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 `@helm-charts/bifrost/templates/_helpers.tpl` around lines 343 - 345, The guard that assigns framework.pricing only checks $pricing.pricing_url and $pricing.pricing_sync_interval but misses $pricing.model_parameters_url, so if model_parameters_url is the only pricing field the pricing object is omitted; update the conditional that wraps the set call (the if or ... block that currently references $pricing.pricing_url and $pricing.pricing_sync_interval) to also include $pricing.model_parameters_url so $_ := set $framework "pricing" $pricing executes when model_parameters_url is present.
🤖 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.
Outside diff comments:
In `@helm-charts/bifrost/templates/_helpers.tpl`:
- Around line 343-345: The guard that assigns framework.pricing only checks
$pricing.pricing_url and $pricing.pricing_sync_interval but misses
$pricing.model_parameters_url, so if model_parameters_url is the only pricing
field the pricing object is omitted; update the conditional that wraps the set
call (the if or ... block that currently references $pricing.pricing_url and
$pricing.pricing_sync_interval) to also include $pricing.model_parameters_url so
$_ := set $framework "pricing" $pricing executes when model_parameters_url is
present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bcdd3309-a6fc-4310-95b7-5280b6dd9887
📒 Files selected for processing (6)
examples/configs/withframework/config.jsonexamples/dockers/data/config.jsonhelm-charts/bifrost/templates/_helpers.tplhelm-charts/bifrost/values.schema.jsonhelm-charts/bifrost/values.yamltransports/config.schema.json
✅ Files skipped from review due to trivial changes (1)
- examples/dockers/data/config.json
BearTS
force-pushed
the
05-20-feat_model_parameters_url_configuration_via_config_json_and_helm_chart
branch
from
May 20, 2026 09:00
685afb8 to
9c719ee
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
helm-charts/bifrost/templates/_helpers.tpl (1)
337-345:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude
model_parameters_urlin the pricing object guardLine 343 should also check
$pricing.model_parameters_url; otherwise Line 337-339 can be dropped from rendered config when it’s the only pricing field set.Suggested fix
-{{- if or $pricing.pricing_url $pricing.pricing_sync_interval }} +{{- if or $pricing.pricing_url $pricing.pricing_sync_interval $pricing.model_parameters_url }} {{- $_ := set $framework "pricing" $pricing }} {{- end }}As per coding guidelines: "
**: 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".🤖 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 `@helm-charts/bifrost/templates/_helpers.tpl` around lines 337 - 345, The template currently sets $pricing.model_parameters_url but the guard that assigns $framework.pricing only checks $pricing.pricing_url and $pricing.pricing_sync_interval, so if model_parameters_url is the only pricing field it will be dropped; update the conditional that calls set $framework "pricing" $pricing to include $pricing.model_parameters_url in the or check (i.e., ensure the guard tests $pricing.model_parameters_url along with $pricing.pricing_url and $pricing.pricing_sync_interval) so the pricing object is preserved when model_parameters_url is the sole field.
🤖 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.
Outside diff comments:
In `@helm-charts/bifrost/templates/_helpers.tpl`:
- Around line 337-345: The template currently sets $pricing.model_parameters_url
but the guard that assigns $framework.pricing only checks $pricing.pricing_url
and $pricing.pricing_sync_interval, so if model_parameters_url is the only
pricing field it will be dropped; update the conditional that calls set
$framework "pricing" $pricing to include $pricing.model_parameters_url in the or
check (i.e., ensure the guard tests $pricing.model_parameters_url along with
$pricing.pricing_url and $pricing.pricing_sync_interval) so the pricing object
is preserved when model_parameters_url is the sole field.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 023b3e5d-4d8d-4870-92ec-d70d342a7349
📒 Files selected for processing (6)
examples/configs/withframework/config.jsonexamples/dockers/data/config.jsonhelm-charts/bifrost/templates/_helpers.tplhelm-charts/bifrost/values.schema.jsonhelm-charts/bifrost/values.yamltransports/config.schema.json
✅ Files skipped from review due to trivial changes (3)
- examples/configs/withframework/config.json
- transports/config.schema.json
- examples/dockers/data/config.json
Contributor
Merge activity
|
akshaydeo
changed the base branch from
05-20-feat_made_model_parameters_url_configurable
to
graphite-base/3609
May 20, 2026 09:05
akshaydeo
dismissed
coderabbitai[bot]’s stale review
May 20, 2026 09:05
The base branch was changed.
akshaydeo
deleted the
05-20-feat_model_parameters_url_configuration_via_config_json_and_helm_chart
branch
May 20, 2026 09:06
akshaydeo
pushed a commit
that referenced
this pull request
May 20, 2026
…rt (#3609) ## Summary Briefly explain the purpose of this PR and the problem it solves. ## Changes - What was changed and why - Any notable design decisions or trade-offs ## 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 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 - [ ] 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 - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] 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
3 tasks
Merged
This was referenced Jun 2, 2026
Closed
18 tasks
This was referenced Jul 2, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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