Allow user to select provider#138
Conversation
|
Important Review skippedDraft detected. 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:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
MMeteorL
left a comment
MMeteorL
left a comment
There was a problem hiding this comment.
Great job! The main layering is appropriate: credentials, provider verification, model creation, and Mastra wiring stay in the backend. Frontend remains UI/config. The provider abstraction is large but coherent. Timeouts are handled for verification/model-list fetches. Malformed schema output still has a retry path. Tool failures are mostly captured and logged. The weak point is saving models that may not support the required tool/schema behavior.
However, there are several blockers that we may need to address here:
1: Model selection can save incompatible model IDs and fail later at runtime.
frontend/components/settings/ModelSideSheet.tsx (line 170) always exposes a free-form “Custom model slug”, and backend/src/index.ts (line 884) only warns when the slug is not returned by the current provider, then saves it anyway. The later compatibility check in backend/src/config/models.ts (line 192) is mostly prefix/shape based, so values like unknown OpenRouter slugs or nonexistent gpt-* names can pass and only fail during schema inference/populate/update. This affects correctness, reliability, and product trust. I’d make unsupported selections a hard validation error except for explicitly custom/local endpoints.
2: /llm-provider/models is public and can trigger provider API calls using configured credentials.
backend/src/index.ts (line 841) is not behind requireAuth and has no local-mode guard. It can call fetchModelsForCurrentLlmProvider(), which may use stored/env provider credentials. Even if it does not expose secrets directly, it lets unauthenticated callers consume provider/model-list rate limits and exercise configured external endpoints. I’d either require auth, restrict it to local mode where setup needs it, or serve only cached public-safe model data from this route.
3: The provider/model surface is much broader than the current agent/tool reliability envelope.
The implementation puts many direct providers and local/custom endpoints behind an “Experimental Providers” opt-in, which is a good start, but the backend still treats broad text-model lists as selectable for Mastra agents. BigSet’s Mastra tool workflow depends on reliable tool-calling/schema behavior, and many older or non-tool-optimized models will not handle the tool protocol consistently. I recommend curating a small allowlist of known-compatible models per provider, with provider-specific defaults for each role, rather than exposing most returned text models plus arbitrary slugs. From what we tested, deepseek-v4-pro, Qwen3.7 are reliable models, as well as the more expensive latest Claude or OpenAI models.
| /> | ||
| </div> | ||
| )} | ||
| <form onSubmit={handleCustomSlugSubmit} className="flex items-center gap-2"> |
There was a problem hiding this comment.
This always exposes a free-form “Custom model slug.” Values like unknown OpenRouter slugs or nonexistent gpt-* names can pass and only fail during schema inference/populate/update. This affects correctness, reliability, and product trust. I’d make unsupported selections a hard validation error except for explicitly custom/local endpoints.
| return reply.code(400).send({ | ||
| error: `Invalid model slug "${slug}" for ${role}. Refresh the model list first or choose a different model.`, | ||
| }); | ||
| req.log.warn({ role, slug }, "Saving model slug that was not returned by the current LLM provider"); |
There was a problem hiding this comment.
This warns when the slug is not returned by the current provider, then saves it anyway. Values like unknown OpenRouter slugs or nonexistent gpt-* names can pass and only fail during schema inference/populate/update. This affects correctness, reliability, and product trust. I’d make unsupported selections a hard validation error except for explicitly custom/local endpoints.
| ); | ||
| } | ||
|
|
||
| function isProviderTextModelId( |
There was a problem hiding this comment.
The compatibility check is mostly prefix/shape based. Values like unknown OpenRouter slugs or nonexistent gpt-* names can pass and only fail during schema inference/populate/update. This affects correctness, reliability, and product trust. I’d make unsupported selections a hard validation error except for explicitly custom/local endpoints.
| if (toValidate.length > 0) { | ||
| try { | ||
| const models = await getCachedModels(); | ||
| const models = await fetchModelsForCurrentLlmProvider(); |
There was a problem hiding this comment.
This call is not behind requireAuth and has no local-mode guard. It can call fetchModelsForCurrentLlmProvider(), which may use stored/env provider credentials. Even if it does not expose secrets directly, it lets unauthenticated callers consume provider/model-list rate limits and exercise configured external endpoints. I’d either require auth, restrict it to local mode where setup needs it, or serve only cached public-safe model data from this route.
3 participants
Summary
This PR rebuilds local setup for a multi-provider model world instead of assuming OpenRouter is the only path.
What changed
When a provider exposes a compatible model-list endpoint, BigSet fetches available models so users can choose from a list instead of manually entering exact model slugs. Local and custom providers do not assume default models, so setup now asks users to choose the models BigSet should use for each role.