Repository navigation
feat(provider): Add support to pass fallback provider through stream … - #880
Conversation
|
@BoraYaswanthReddy is attempting to deploy a commit to the Sachin Sharma's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe PR introduces per-call overrides for fallback routing by extending StreamOptions with fallbackProvider and fallbackModel fields. In neurolink.ts, these options are read alongside environment variables (FALLBACK_PROVIDER, FALLBACK_MODEL) and merged with the model config route to determine the final fallback behavior, with fallbackSource tracking the override origin. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment Tip CodeRabbit can generate a title for your PR based on the changes.Add |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/neurolink.ts (1)
6166-6185:⚠️ Potential issue | 🟠 MajorTreat empty fallback env/option values as unset.
At Line [6183] and Line [6184],
??will treat""as a valid override. IfFALLBACK_PROVIDER/FALLBACK_MODEL(or option values) are empty strings, fallback resolution can select invalid values and fail instead of usingmodelConfigRoute.Proposed fix
- const optFallbackProvider = enhancedOptions.fallbackProvider; - const optFallbackModel = enhancedOptions.fallbackModel; - const envFallbackProvider = process.env.FALLBACK_PROVIDER; - const envFallbackModel = process.env.FALLBACK_MODEL; + const optFallbackProvider = enhancedOptions.fallbackProvider?.trim() || undefined; + const optFallbackModel = enhancedOptions.fallbackModel?.trim() || undefined; + const envFallbackProvider = process.env.FALLBACK_PROVIDER?.trim() || undefined; + const envFallbackModel = process.env.FALLBACK_MODEL?.trim() || undefined;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/neurolink.ts` around lines 6166 - 6185, The code uses nullish coalescing (??) for optFallbackProvider/optFallbackModel and envFallbackProvider/envFallbackModel which treats empty strings as valid overrides; change the fallback selection to treat empty string as unset by preferring the first non-empty string. Update the fallbackRoute construction (the object merging modelConfigRoute) to use truthy checks (e.g., optFallbackProvider if non-empty else envFallbackProvider if non-empty else modelConfigRoute.provider, same for model) or a small helper like firstNonEmpty(optFallbackProvider, envFallbackProvider, modelConfigRoute.provider) so empty "" values do not override modelConfigRoute.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/neurolink.ts`:
- Around line 6191-6195: The fallbackSource assignment is only checking
optFallbackProvider/envFallbackProvider so it reports "model_config" even when
fallbackModel is overridden; update the logic that sets fallbackSource (the
variable defined with optFallbackProvider and envFallbackProvider) to also
consider optFallbackModel and envFallbackModel: treat source as "options" if
either optFallbackProvider or optFallbackModel is set, "env" if either
envFallbackProvider or envFallbackModel is set, otherwise "model_config", and
update any related log/messages that read fallbackSource accordingly.
---
Outside diff comments:
In `@src/lib/neurolink.ts`:
- Around line 6166-6185: The code uses nullish coalescing (??) for
optFallbackProvider/optFallbackModel and envFallbackProvider/envFallbackModel
which treats empty strings as valid overrides; change the fallback selection to
treat empty string as unset by preferring the first non-empty string. Update the
fallbackRoute construction (the object merging modelConfigRoute) to use truthy
checks (e.g., optFallbackProvider if non-empty else envFallbackProvider if
non-empty else modelConfigRoute.provider, same for model) or a small helper like
firstNonEmpty(optFallbackProvider, envFallbackProvider,
modelConfigRoute.provider) so empty "" values do not override modelConfigRoute.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8ff927c6-8076-4bc8-a887-5dd03a583bac
📒 Files selected for processing (2)
src/lib/neurolink.tssrc/lib/types/streamTypes.ts
539b60e to
b110118
Compare
Fixed |
Pull Request
Description
What does this PR do?
Adds support for specifying a fallback provider through stream options or environment variables, improving reliability when primary providers are unavailable.
Type of Change
Changes Made
src/lib/neurolink.tsto support fallback provider configuration via stream options and environment variablessrc/lib/types/streamTypes.tswith new type definitions for fallback provider supportBreaking Changes
Testing
Code Quality
Commit Message Format
type(scope): descriptionfeatproviderCommit:
feat(provider): Add support to pass fallback provider through stream options or envDependencies
Summary by CodeRabbit