adds sync support from local files for airgapped deployments - #4574
Merged
akshaydeo merged 1 commit intoJun 20, 2026
Merged
Conversation
|
|
Contributor
|
Warning Review limit reached
More reviews will be available in 31 minutes and 5 seconds. Learn how PR review limits work. To continue reviewing without waiting, enable usage-based billing in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughAdds a new ChangesLocal file:// Datasheet Loading
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
akshaydeo
marked this pull request as ready for review
June 20, 2026 10:59
Contributor
Confidence Score: 5/5The core Go changes are a small, well-tested helper plus two call-site updates; the file:// branch is entirely additive and does not affect the existing HTTP path. The Both Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[loadPricingFromURL / loadModelParametersFromURL] --> B{url.Parse scheme}
B -- file --> C[filePathFromURL]
C --> D{parsed.Opaque != empty?}
D -- yes --> E[return Opaque\ne.g. file:./x.json]
D -- no --> F{Host != empty\nand != localhost?}
F -- yes --> G[return Host + Path\ne.g. file://../../x.json]
F -- no --> H[return Path\ne.g. file:///abs/x.json]
E & G & H --> I[os.ReadFile]
B -- http/https --> J[ValidateExternalURL]
J --> K[http.Client.Do]
K --> L[io.ReadAll]
I & L --> M[json.Unmarshal]
M --> N[applyPricingData /\napplyModelParameters]
%%{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[loadPricingFromURL / loadModelParametersFromURL] --> B{url.Parse scheme}
B -- file --> C[filePathFromURL]
C --> D{parsed.Opaque != empty?}
D -- yes --> E[return Opaque\ne.g. file:./x.json]
D -- no --> F{Host != empty\nand != localhost?}
F -- yes --> G[return Host + Path\ne.g. file://../../x.json]
F -- no --> H[return Path\ne.g. file:///abs/x.json]
E & G & H --> I[os.ReadFile]
B -- http/https --> J[ValidateExternalURL]
J --> K[http.Client.Do]
K --> L[io.ReadAll]
I & L --> M[json.Unmarshal]
M --> N[applyPricingData /\napplyModelParameters]
Reviews (3): Last reviewed commit: "adds sync support from local files for a..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
akshaydeo
force-pushed
the
06-20-adds_sync_support_from_local_files_for_airgapped_deployments
branch
from
June 20, 2026 11:33
c1419ac to
90ad24b
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@framework/modelcatalog/datasheet/testdata/model-parameters.json`:
- Line 613: The model configuration on line 613 declares mode as "embedding",
but the subsequent lines 619-759 contain chat-generation specific parameters
such as temperature, max_tokens, stop, n, and stream, which are not valid for
embedding models. To fix this, either change the mode value from "embedding" to
an appropriate chat-specific mode that supports these parameters, or remove all
chat-only control parameters from this configuration block to ensure the
parameters align with the embedding mode declaration.
- Line 61: The text-embedding-3-small model is declared with "mode": "embedding"
but includes chat completion-specific parameters that don't apply to embedding
models. Locate the text-embedding-3-small model definition in the fixture and
remove all chat completion parameters (temperature, max_tokens, stop, top_p,
frequency_penalty, logit_bias, logprobs, n, presence_penalty, seed, stream) from
its parameter list, keeping only parameters that are applicable to embedding
requests.
🪄 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: 10f356bf-6976-45e5-824f-0d7a7e5259c2
📒 Files selected for processing (7)
examples/configs/withlocalpricingfiles/README.mdexamples/configs/withlocalpricingfiles/config.jsonexamples/configs/withlocalpricingfiles/model-parameters.jsonexamples/configs/withlocalpricingfiles/pricing.jsonframework/modelcatalog/datasheet/localfiles_test.goframework/modelcatalog/datasheet/testdata/model-parameters.jsonframework/modelcatalog/datasheet/testdata/pricing.json
✅ Files skipped from review due to trivial changes (2)
- examples/configs/withlocalpricingfiles/config.json
- examples/configs/withlocalpricingfiles/README.md
🚧 Files skipped from review as they are similar to previous changes (4)
- framework/modelcatalog/datasheet/localfiles_test.go
- examples/configs/withlocalpricingfiles/pricing.json
- examples/configs/withlocalpricingfiles/model-parameters.json
- framework/modelcatalog/datasheet/testdata/pricing.json
akshaydeo
force-pushed
the
06-20-adds_sync_support_from_local_files_for_airgapped_deployments
branch
from
June 20, 2026 12:12
90ad24b to
51171a9
Compare
Contributor
Author
Merge activity
|
akshaydeo
deleted the
06-20-adds_sync_support_from_local_files_for_airgapped_deployments
branch
June 20, 2026 12:15
akshaydeo
added a commit
that referenced
this pull request
Jun 21, 2026
## Summary Adds support for loading Bifrost's pricing and model parameters datasheets from local files using the `file://` URL scheme. This resolves a connectivity issue (closes #4305) where hosts without outbound internet access, or behind HTTP proxies that block DNS resolution of `getbifrost.ai`, could not start Bifrost successfully. ## Changes - Added a new example config at `examples/configs/withlocalpricingfiles/` demonstrating how to configure `pricing_url` and `model_parameters_url` with `file://` paths, along with sample pricing and model parameters datasheets containing real entries for `gpt-4o`, `gpt-4o-mini`, `gpt-4.1`, `claude-sonnet-4-20250514`, and `text-embedding-3-small`. - Added unit tests in `framework/modelcatalog/datasheet/localfiles_test.go` that verify both datasheets load correctly from `file://` URLs without any network access or hostname resolution. A dedicated regression test (`TestLoadFromLocalFiles_NeverResolvesHostname`) guards against the `file://` scheme accidentally falling through to external URL validation. - Added `testdata/` fixtures mirroring the example datasheets for use by the tests. The `file://` scheme short-circuits external URL validation and hostname lookup entirely, reading the path directly off disk. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [x] Docs ## How to test ```sh go test ./framework/modelcatalog/datasheet/... ``` To validate the Docker example: ```sh cd examples/configs/withlocalpricingfiles docker run -p 8080:8080 \ -e OPENAI_API_KEY=sk-... \ -v "$(pwd)/config.json:/app/data/config.json" \ -v "$(pwd)/pricing.json:/opt/bifrost/pricing.json" \ -v "$(pwd)/model-parameters.json:/opt/bifrost/model-parameters.json" \ maximhq/bifrost ``` Bifrost should start without attempting to reach `getbifrost.ai`. Pricing and model parameter lookups for the bundled models should resolve correctly from the local files. **New config fields:** | Field | Example value | Description | | --- | --- | --- | | `framework.pricing.pricing_url` | `file:///opt/bifrost/pricing.json` | Local path to the pricing datasheet | | `framework.pricing.model_parameters_url` | `file:///opt/bifrost/model-parameters.json` | Local path to the model parameters datasheet | ## Breaking changes - [ ] Yes - [x] No ## Related issues Closes #4305 ## Security considerations The `file://` scheme reads arbitrary paths off disk as the process user. Operators should ensure the mounted files are not world-writable and that the container or process runs with appropriate least-privilege permissions. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
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
Adds support for loading Bifrost's pricing and model parameters datasheets from local files using the
file://URL scheme. This resolves a connectivity issue (closes #4305) where hosts without outbound internet access, or behind HTTP proxies that block DNS resolution ofgetbifrost.ai, could not start Bifrost successfully.Changes
examples/configs/withlocalpricingfiles/demonstrating how to configurepricing_urlandmodel_parameters_urlwithfile://paths, along with sample pricing and model parameters datasheets containing real entries forgpt-4o,gpt-4o-mini,gpt-4.1,claude-sonnet-4-20250514, andtext-embedding-3-small.framework/modelcatalog/datasheet/localfiles_test.gothat verify both datasheets load correctly fromfile://URLs without any network access or hostname resolution. A dedicated regression test (TestLoadFromLocalFiles_NeverResolvesHostname) guards against thefile://scheme accidentally falling through to external URL validation.testdata/fixtures mirroring the example datasheets for use by the tests.The
file://scheme short-circuits external URL validation and hostname lookup entirely, reading the path directly off disk.Type of change
Affected areas
How to test
go test ./framework/modelcatalog/datasheet/...To validate the Docker example:
Bifrost should start without attempting to reach
getbifrost.ai. Pricing and model parameter lookups for the bundled models should resolve correctly from the local files.New config fields:
framework.pricing.pricing_urlfile:///opt/bifrost/pricing.jsonframework.pricing.model_parameters_urlfile:///opt/bifrost/model-parameters.jsonBreaking changes
Related issues
Closes #4305
Security considerations
The
file://scheme reads arbitrary paths off disk as the process user. Operators should ensure the mounted files are not world-writable and that the container or process runs with appropriate least-privilege permissions.Checklist
docs/contributing/README.mdand followed the guidelines