feat(models): add streaming support and utility function - #338
Conversation
Introduced a `streaming` property to model and provider definitions to clearly indicate support for streaming. Added a utility function `getModelStreamingSupport` to determine streaming compatibility for model-provider combinations. Updated related files to use the centralized logic for improved maintainability.
WalkthroughThe changes introduce a new helper function to determine streaming support for models and providers, update the data structures to include mandatory streaming flags at the model-provider level, and refactor related logic across the codebase to utilize the new function. Exports and error messages are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant UI/Route
participant ModelsHelper
participant ModelsData
Client->>UI/Route: Request streaming model info
UI/Route->>ModelsHelper: getModelStreamingSupport(modelName, providerId)
ModelsHelper->>ModelsData: Look up model and provider streaming flags
ModelsData-->>ModelsHelper: Return streaming support status
ModelsHelper-->>UI/Route: Return boolean
UI/Route-->>Client: Respond with streaming support info
sequenceDiagram
participant User
participant ChatAPI
participant ModelsHelper
participant ModelsData
User->>ChatAPI: Request streaming chat with model/provider
ChatAPI->>ModelsHelper: getModelStreamingSupport(modelName, providerId)
ModelsHelper->>ModelsData: Lookup streaming flags
ModelsData-->>ModelsHelper: Return streaming support status
ModelsHelper-->>ChatAPI: Return boolean
alt Streaming not supported
ChatAPI-->>User: Return error: Model/provider does not support streaming
else Streaming supported
ChatAPI-->>User: Proceed with streaming chat
end
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
apps/ui/src/routes/playground.tsx (1)
169-180: 🛠️ Refactor suggestion
⚠️ Potential issueStreaming falsely disabled when model is prefixed with provider
selectedModelmay contain strings like"openai/gpt-4o-mini".
Passing that verbatim togetModelStreamingSupport(with no provider ID) fails the lookup and returnsfalse, so the UI falls back to non-streaming even though the backend accepts streaming.- const supportsStreaming = getModelStreamingSupport(selectedModel); + // De-compose "<provider>/<model>" if the user selects a provider-prefixed entry + let modelName = selectedModel; + let providerId: string | undefined; + if (selectedModel.includes("/")) { + const parts = selectedModel.split("/"); + providerId = parts.shift(); + modelName = parts.join("/"); + } + const supportsStreaming = getModelStreamingSupport(modelName, providerId);
🧹 Nitpick comments (5)
apps/gateway/src/api.e2e.ts (1)
43-51: Avoid copy-pasted streaming logic – call the central helper insteadThe inline
some()check duplicates the logic already encapsulated ingetModelStreamingSupport, risking divergence when the helper evolves.-import { models, providers } from "@llmgateway/models"; +import { models, providers, getModelStreamingSupport } from "@llmgateway/models"; @@ -const streamingModels = testModels.filter((m) => - m.providers.some((p: any) => { - if (p.streaming !== undefined) { - return p.streaming; - } - const provider = providers.find((pr) => pr.id === p.providerId); - return provider?.streaming; - }), -); +const streamingModels = testModels.filter((m) => + m.providers.some((p: any) => + getModelStreamingSupport(m.model, p.providerId), + ), +);packages/models/src/models.ts (1)
39-43:ModelDefinition.streamingis never consulted by the helper
getModelStreamingSupportonly inspects the provider-level flag and, if absent, falls back to the provider definition – it never looks atmodel.streaming. Either:
- Wire the fallback into the helper, or
- Drop the property to avoid dead config.
Keeping unused metadata invites confusion.
packages/models/src/helpers.ts (3)
18-26: Avoid O(n²) look-ups by pre-computing a provider → streaming mapInside the
.some()you perform a linearproviders.findfor every provider mapping.
For large provider lists this turns the overall complexity into O(p²). Building a simple map once per function call drops the inner look-up to O(1) and keeps the whole branch O(p).- return modelInfo.providers.some((provider: ProviderModelMapping) => { - // Check model-level streaming first, then fall back to provider-level - if (provider.streaming !== undefined) { - return provider.streaming; - } - // Fall back to provider-level streaming support - const providerInfo = providers.find((p) => p.id === provider.providerId); - return providerInfo?.streaming === true; - }); + const providerStreaming = new Map( + providers.map((p) => [p.id, p.streaming === true]), + ); + + return modelInfo.providers.some((provider: ProviderModelMapping) => { + if (provider.streaming !== undefined) { + return provider.streaming; + } + return providerStreaming.get(provider.providerId) === true; + });
30-44: DRY: Extract repeated fallback logic into a tiny helperLines 30-44 repeat the same precedence check implemented above (model-level, then provider-level). Pulling this into an internal helper (
supportsProviderStreaming(mapping)) eliminates duplication, keeps both branches aligned, and makes future fixes one-shot.
7-45: Unit-test the precedence matrixGiven the three-level precedence (model absent → provider absent → explicit
falseoverriding providertrue) a regression here would be hard to spot manually. Recommend adding table-driven tests that cover:• model with provider-level
true | false | undefined
• provider-only streaming flagstrue | false | undefined
• specified vs. unspecifiedproviderIdHappy to draft the tests if useful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/gateway/src/api.e2e.ts(1 hunks)apps/gateway/src/chat/chat.ts(2 hunks)apps/ui/src/routes/playground.tsx(1 hunks)packages/models/src/helpers.ts(1 hunks)packages/models/src/index.ts(1 hunks)packages/models/src/models.ts(30 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/gateway/src/chat/chat.ts (1)
packages/models/src/helpers.ts (1)
getModelStreamingSupport(7-45)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build / run
- GitHub Check: e2e / run
🔇 Additional comments (2)
packages/models/src/index.ts (1)
4-4: Public re-export looks fine.Exposing
getModelStreamingSupportfrom the package root keeps consumers unaware of the helpers file layout. No issues spotted.apps/gateway/src/chat/chat.ts (1)
1111-1116: 👍 Switched to model + provider level streaming guardGreat move from provider-only checks to
getModelStreamingSupport.
This prevents false positives when a provider offers mixed streaming capabilities across models.
Log response body and throw an error when the status code is not 200 in the API end-to-end test. This improves debugging and test reliability.
Added `baseModelName` to track the base model name for streaming support validation. Updated streaming checks to use the base model instead of the provider-specific model name, ensuring proper compatibility checks and error handling.
Ensure `baseModelName` accurately reflects the final `usedModel` after routing. Fallback to `usedModel` if no corresponding model definition is found.
Removed redundant `baseModelName` assignments and centralized the logic using optional chaining and fallback to `usedModel` for clarity and maintainability.
Removed optional streaming definition in model configurations and ensured all models explicitly specify streaming support. Updated model entries accordingly to include streaming properties.
1 participant
Introduced a
streamingproperty to model and provider definitions to clearly indicate support for streaming. Added a utility functiongetModelStreamingSupportto determine streaming compatibility for model-provider combinations. Updated related files to use the centralized logic for improved maintainability.Summary by CodeRabbit