feat: sglang provider added#164
Conversation
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis change introduces support for the "SGLang" (SGL) AI provider throughout the codebase, including core logic, provider implementation, configuration, validation, documentation, and UI. It adds the SGL provider integration, updates provider handling logic, and incorporates SGL into testing, documentation, and user interface components. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant Bifrost Core
participant SGLProvider
participant SGL API
User->>UI: Selects SGL as provider and configures it
UI->>Bifrost Core: Sends provider config (with/without API key, base URL)
Bifrost Core->>SGLProvider: Initializes SGLProvider with config
User->>UI: Initiates chat completion
UI->>Bifrost Core: Sends chat completion request (provider=SGL)
Bifrost Core->>SGLProvider: Calls ChatCompletion method
SGLProvider->>SGL API: Sends formatted request
SGL API-->>SGLProvider: Returns response
SGLProvider-->>Bifrost Core: Returns parsed response
Bifrost Core-->>UI: Returns chat completion result
UI-->>User: Displays chat completion
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (3)
core/schemas/bifrost.go (1)
39-50: Minor ordering nitThe enum is roughly alphabetical; inserting
SGLafterGroqbreaks that pattern. Consider keeping the sequence sorted for readability.transports/bifrost-http/ui/404.html (1)
118-122: Whitespace-only change to a generated/minified file adds pointless churnThis newline tweak doesn’t affect rendering, yet it bloats the diff for a 900-line minified artifact. Whenever possible, keep build artifacts out of version control or run a formatter that strips trailing-whitespace deltas before commit.
transports/bifrost-http/ui/404/index.html (1)
118-122: Same no-op whitespace diff as sibling fileIdentical observation: the change is noise only. Consider excluding these compiled/static assets or normalising them in CI to avoid needless diffs.
Pratham-Mishra04
force-pushed
the
07-16-feat_sglang_provider_added
branch
from
0f92c4e to
2ab6aae
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (3)
transports/bifrost-http/ui/mcp-clients/index.html (1)
1-122: Consider dropping generated UI build artifacts from VCSThis file looks like a minified/serialized Next .js build output. Committing such artefacts clutters the diff, makes code-review noise (e.g. this change is just an extra newline) and often causes merge conflicts. Prefer generating them in CI/CD or as part of your release pipeline instead of tracking them in Git.
transports/bifrost-http/ui/_next/static/chunks/app/providers/page-b25fa0ce16423da1.js (2)
1-30: Remove redundant"use strict"wrapper & avoidthisinside static helpersBiome flags two real issues at the very top of the bundle:
"use strict"is injected although ES-modules are strict by default.this.patternis referenced inside other static methods ofclass Dr; in a static contextthisequals the class, but it’s clearer and less error-prone to call the method via the class name itself (Dr.pattern).- "use strict"; + /* eslint-disable */ // generated bundle – strict mode already implied static email(e,msg="Must be a valid email"){ - return this.pattern(e, /^[A-Z0-9._%+-]+@[A-Z0-9.-]+\.[A-Z]{2,}$/i, msg) + return Dr.pattern(e, /^[A-Z0-9._%+-]+@[A-Z0-9.-]+\.[A-Z]{2,}$/i, msg) }Net effect: smaller bundle, removes minor visual debt, and future-proofs the helpers if they’re ever copy-pasted outside the class.
24-26: Edge-case:arrayUnique()silently breaks on non-arrays
arrayUniquedoesnew Set(e)unconditionally; wheneisnull/undefinedthe constructor still returns an empty set, but you also compare againste.lengthwhich isundefined, yieldingfalseand an error message that may confuse users (“Must have unique items” even though the field is empty).Consider short-circuiting to
truewhen the value is falsy:static arrayUnique(arr,msg="Must have unique items"){ - return {isValid:(arr?.length)===new Set(arr).size, message:msg} + if (!arr || arr.length===0) return {isValid:true, message:msg}; + return {isValid:arr.length===new Set(arr).size, message:msg}; }
♻️ Duplicate comments (5)
ui/README.md (1)
93-93: Maintain alphabetical order in provider listFor consistency with other docs and quicker scanning, keep provider names sorted alphabetically.
- **Supported Providers**: OpenAI, Azure OpenAI, Anthropic, AWS Bedrock, Cohere, Google Vertex AI, Mistral, Ollama, Groq, SGLang + **Supported Providers**: Anthropic, AWS Bedrock, Azure OpenAI, Cohere, Google Vertex AI, Groq, Mistral, Ollama, OpenAI, SGLangtransports/README.md (1)
56-56: Alphabetise provider names in feature tableThe list is now out of order; re-ordering improves readability and matches other docs.
-| **🔄 Multi-Provider Support** | OpenAI, Anthropic, Azure, Bedrock, Vertex, Cohere, Mistral, Ollama, Groq, SGLang | +| **🔄 Multi-Provider Support** | Anthropic, Azure, Bedrock, Cohere, Groq, Mistral, Ollama, OpenAI, SGLang, Vertex |docs/usage/providers.md (1)
17-18: Keep table rows in alphabetical orderSame nit as before – move “SGLang” after “Ollama” and before “Vertex”, and tidy the column spacing.
-| **Groq** | Mixtral, Llama, Gemma | Enterprise AI platform | ✅ | -| **SGLang** | Qwen | Enterprise AI platform | ✅ | +| **Groq** | Mixtral, Llama, Gemma | Enterprise AI platform | ✅ | +| **SGLang** | Qwen | Enterprise AI platform | ✅ |core/utils.go (1)
15-15: Update comment to reflect SGL provider.The function logic correctly adds
schemas.SGLas a keyless provider, but the comment should be updated to mention SGL alongside Vertex and Ollama.Apply this diff to update the comment:
-// Some providers like Vertex and Ollama are keyless and don't require API keys. +// Some providers like Vertex, Ollama, and SGL are keyless and don't require API keys.core/providers/sgl.go (1)
85-88: Fix the incorrect comment referencing "Ollama".The comment should reference "SGL" instead of "Ollama".
- // BaseURL is required for Ollama + // BaseURL is required for SGL if config.NetworkConfig.BaseURL == "" { return nil, fmt.Errorf("base_url is required for sgl provider") }
Pratham-Mishra04
force-pushed
the
07-16-feat_sglang_provider_added
branch
from
2ab6aae to
ebb8ad6
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
transports/bifrost-http/integrations/utils.go (1)
567-577:validProvidersarray forgot to include the newly-addedschemas.SGL
ParseModelStringrelies onvalidProvidersfor prefix validation.
With the current map, model strings such as"sgl/my-model"will silently fall back to “no provider” and the request will be routed to the default (openai) provider – very hard to debug.Add the missing entry:
var validProviders = map[schemas.ModelProvider]bool{ schemas.OpenAI: true, schemas.Azure: true, schemas.Anthropic: true, schemas.Bedrock: true, schemas.Cohere: true, schemas.Vertex: true, schemas.Mistral: true, schemas.Ollama: true, + schemas.SGL: true, }
♻️ Duplicate comments (10)
transports/bifrost-http/ui/mcp-clients/index.txt (1)
20-20: Generated bundle diff still clutters the PRThe change is just a new hydration key. Please exclude these serialized
.txtbundle artifacts (or gate them behind a build flag) to keep diffs readable.transports/bifrost-http/ui/docs/index.txt (1)
19-19: Generated bundle diff still clutters the PRSame as earlier feedback: internal hydration-key churn adds no reviewable value. Consider removing these files from version control or generating them conditionally.
transports/README.md (1)
56-56: Provider list still not alphabetisedThe provider list order is inconsistent with other docs (see previous review). Alphabetical ordering improves scan-ability.
ui/README.md (1)
93-93: Provider list still not alphabetisedSame nit as before – keeping the list alphabetical helps quick scanning.
docs/usage/providers.md (1)
17-18: Table alignment and alphabetical ordering issues persistThe SGLang entry should be placed alphabetically between Ollama and Vertex AI, and the table alignment inconsistency still needs to be addressed.
core/utils.go (1)
15-15: Consider updating the comment to reflect SGL provider inclusion.The comment mentions "Some providers like Vertex and Ollama are keyless" but the function now also includes SGL. While this was noted in a previous review, the comment could be updated for accuracy.
transports/bifrost-http/handlers/providers.go (1)
253-255: Same duplication inUpdateProviderSee previous comment – the validation branch here should reuse the same helper.
ui/lib/types/config.ts (1)
4-4: Keep union members alphabetically sorted
'sgl'was appended at the very end; inserting it before'vertex'keeps diffs tidier.-export type ModelProvider = 'openai' | 'azure' | 'anthropic' | 'bedrock' | 'cohere' | 'vertex' | 'mistral' | 'ollama' | 'groq' | 'sgl' +export type ModelProvider = 'openai' | 'azure' | 'anthropic' | 'bedrock' | 'cohere' | 'mistral' | 'ollama' | 'sgl' | 'vertex' | 'groq'transports/bifrost-http/ui/providers/index.txt (1)
11-12: Generated bundle – recommend excluding from gitOnly hashed file names changed; committing these bloats diffs.
transports/bifrost-http/ui/config/index.txt (1)
20-21: Generated bundle – recommend excluding from gitSame reasoning as above; no functional value in source control.
Merge activity
|
akshaydeo
merged commit ccdc619
into
07-16-feat_groq_provider_added
2 participants
No description provided.