Allow user to select provider - #138
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR shifts local setup, model storage, and workflow execution from a single OpenRouter path to a provider-scoped LLM path. It adds provider definitions, verification, and model discovery for multiple hosted and local providers. It stores provider-aware model and reasoning settings in Convex. It updates Mastra agents and schema inference to use the active provider configuration. The setup and settings UI now supports provider selection, custom model IDs, reasoning controls, and OAuth gating. Documentation and dev/runtime files are updated for the new local mode. Sequence Diagram(s)sequenceDiagram
participant SetupPage
participant Backend
participant LLMProvider
participant Convex
SetupPage->>Backend: POST /local-setup/llm-provider
Backend->>LLMProvider: verify provider configuration
LLMProvider-->>Backend: verification result
Backend->>Convex: persist provider metadata
Convex-->>Backend: saved credentials
Backend-->>SetupPage: setup status and model data
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ 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.
…dels Resolves the two review blockers on selecting a provider/model: 1. Hard validation on save. POST /settings/models now rejects a model slug the current provider doesn't offer (400) instead of warning and saving it, so incompatible selections fail at save time rather than during schema-inference/populate/update. Local/custom OpenAI-compatible providers (custom, ollama, lmstudio) whose catalogs can't be enumerated stay exempt. Validation fails closed (502 "try again") if the provider list can't be fetched. Logic centralized in findUnsupportedModelSlugs(). The frontend now surfaces the rejection inline in the model side sheet. 2. Auth guard on GET /llm-provider/models. The route exercised configured provider credentials and rate limits while being public; it now runs requireAuth first (passes through in local mode for setup, enforces a Clerk token in prod). getLlmProviderModels() threads the token from authenticated callers; the local-mode setup flow calls it without one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks for the thorough review! Addressed the two blockers in 56f68ab: 1. Model selection could save incompatible slugs → now a hard validation error. 2. 3. Curated per-provider allowlist — deliberately not doing this here. Verified with |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (4)
backend/src/config/llm.ts (1)
207-259: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer the validated
envmodule over rawprocess.envfor provider base URLs.
env(from../env.js) is already imported and used forSCHEMA_INFERENCE_MODEL/IS_LOCAL_MODEin this file, but every base-URL lookup here (process.env.OPENROUTER_BASE_URL,process.env.XAI_BASE_URL, etc.) bypasses it. As per path instructions, backend config should be read consistently through validated environment access; routing these throughenvwould keep validation/typing consistent across the file. As per path instructions, "Read required service configuration from environment variables, including Convex, Clerk, LLM, and TinyFish credentials; do not hardcode secrets."🤖 Prompt for 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. In `@backend/src/config/llm.ts` around lines 207 - 259, Update defaultBaseUrlForLlmProvider to read every provider base URL from the already imported validated env module instead of process.env, including OPENROUTER_BASE_URL, GOOGLE_GENERATIVE_AI_BASE_URL, XAI_BASE_URL, and the remaining provider-specific settings. Preserve each existing fallback URL and the undefined result for unsupported providers.Source: Path instructions
backend/src/local-credentials.ts (1)
365-391: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winLoad provider statuses concurrently.
This loop serializes a Convex query and potential keychain request for every provider, adding avoidable setup-page latency. Resolve the provider entries with
Promise.allbefore constructingproviderStatuses.🤖 Prompt for 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. In `@backend/src/local-credentials.ts` around lines 365 - 391, Update the provider-status construction around LLM_PROVIDER_TYPES to resolve each localCredentialForLlmProvider call concurrently with Promise.all, including the existing credential-to-status mapping for each provider. After all entries resolve, construct providerStatuses from the resulting provider/status pairs while preserving the current status fields and ordering.frontend/lib/backend.ts (1)
162-170: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winHardcoded default OpenRouter model may drift from the setup wizard's provider-defaults source.
saveOpenRouterApiKeyhardcodesdefaultModel: "anthropic/claude-sonnet-4.6", whilefrontend/app/setup/page.tsx's equivalent save path derivesdefaultModelfrom a sharedproviderCopy.defaultModelobject. If the canonical OpenRouter default model changes, only one of these two call sites would need to be updated — the other would silently keep saving the stale value for users going through the settings-page credential flow instead of setup.♻️ Suggested fix: source the default from one place
-export async function saveOpenRouterApiKey( - apiKey: string, -): Promise<LocalSetupStatus> { - return saveLlmProviderConfig({ - provider: "openrouter", - apiKey, - defaultModel: "anthropic/claude-sonnet-4.6", - }); -} +export async function saveOpenRouterApiKey( + apiKey: string, +): Promise<LocalSetupStatus> { + return saveLlmProviderConfig({ + provider: "openrouter", + apiKey, + defaultModel: OPENROUTER_DEFAULT_MODEL, // shared constant, single source of truth + }); +}🤖 Prompt for 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. In `@frontend/lib/backend.ts` around lines 162 - 170, Update saveOpenRouterApiKey to obtain the OpenRouter default model from the shared providerCopy.defaultModel source used by the setup page, instead of hardcoding "anthropic/claude-sonnet-4.6". Ensure both credential-save flows use the same canonical default value.frontend/convex/localCredentials.ts (1)
83-98: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueClearing
llmBaseUrl/llmDefaultModelis coupled tollmProviderbeing present, not to the provider actually changing.Any caller that passes
llmProvider(even unchanged) without resendingllmBaseUrl/llmDefaultModelwill wipe those fields. Today's only visible caller (saveLlmProviderConfiginfrontend/lib/backend.ts) always sendsdefaultModelalongsideprovider, so this isn't currently exploitable from the code shown, but the mutation itself has no guard against a future caller (e.g. a re-verification path) passingllmProvideralone.🤖 Prompt for 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. In `@frontend/convex/localCredentials.ts` around lines 83 - 98, The llmPatch construction in the local credentials mutation incorrectly clears llmBaseUrl and llmDefaultModel whenever llmProvider is supplied, even if unchanged. Compare the incoming provider with the stored provider and only clear or update those fields when the provider actually changes; preserve existing values when the provider is unchanged and omitted auxiliary fields should remain untouched. Apply the corresponding guard to llmInsert as needed for new records.
🤖 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 `@backend/src/index.ts`:
- Around line 722-767: Protect the `/local-setup/llm-provider` route before any
provider verification, persistence, or activation by moving it into the scoped
Fastify plugin whose `preHandler` is `requireAuth` (or otherwise applying that
existing setup capability). Do not rely on `env.IS_LOCAL_MODE` as authorization;
preserve the route’s validation and setup behavior after authentication.
In `@backend/src/local-credentials.ts`:
- Around line 102-125: Update the local credential lookup around the keyless
provider branch to call getKeychainCredential(service) before returning an empty
apiKey for custom, Ollama, or LM Studio endpoints. Reuse any stored keychain
credential when present, and only return the apiKey: "" fallback when no
keychain credential exists; preserve the existing endpoint metadata.
In `@backend/src/mastra/tools/dataset-tools.ts`:
- Around line 61-72: Update rowDataCellsToRecord and its callers to normalize
column names before building the record, detect duplicate normalized keys, and
return a structured tool failure before any data mutation occurs. Preserve
successful conversion for unique columns and apply the same validation to the
related flows around the referenced ranges, including cleanDataKeys handling.
In `@backend/src/pipeline/schema-inference.ts`:
- Line 13: Correct the spelling errors in the schema-inference prompt within the
primary-key guidance: replace “unqiue” with “unique,” “guarenteed” with
“guaranteed,” “thigns” with “things,” and “guarentee” with “guarantee.”
In `@frontend/app/setup/page.tsx`:
- Around line 368-371: Update the onSaved handler in the setup page to clear the
provider-scoped modelConfig whenever credentials change, before updating status
and closing the modal. Ensure stale model slugs cannot keep modelsConfigured
true while the new provider configuration loads.
In `@frontend/components/settings/llm-providers.tsx`:
- Around line 459-480: Update the showExperimentalProviders state initialization
and related effect in the provider selector to derive the initial opt-in from
whether value is experimental, preserving saved experimental providers on mount.
Ensure the OpenRouter fallback runs only from handleExperimentalChange when the
user explicitly unchecks the option, not during initial rendering or value
synchronization.
In `@frontend/components/settings/LocalCredentialsPanel.tsx`:
- Around line 596-608: The initialBaseUrl helpers in
frontend/components/settings/LocalCredentialsPanel.tsx lines 596-608 and
frontend/app/setup/page.tsx lines 779-791 must preserve the active base URL when
services.llmProviders is unavailable. Before using the provider-map value, fall
back to the matching active provider from aggregate services.llm or the inferred
local preset, then retain the existing default/custom-provider behavior in both
helpers.
In `@frontend/convex/localCredentials.ts`:
- Around line 4-47: Replace the duplicated provider literals with one shared
provider source of truth and derive all validators and TypeScript unions from
it. Update frontend/convex/localCredentials.ts (lines 4-47),
frontend/convex/schema.ts (lines 138-164 and 165-219),
frontend/convex/modelConfig.ts (lines 6-41), and frontend/lib/backend.ts (lines
66-82) so serviceValidator, llmProviderValidator, schema provider fields,
LlmProvider, providerValidator, and LlmProviderType all reuse the shared
definition without independent provider lists.
In `@frontend/lib/openrouter-oauth.ts`:
- Around line 55-85: Update isLocalIpv4Hostname to classify only 192.168.0.0/16
as local instead of all addresses beginning with 192. Extend isLocalIpv6Hostname
to recognize the unspecified address :: and IPv4-mapped loopback addresses such
as ::ffff:127.0.0.1, while preserving existing private and loopback
classifications.
In `@README.md`:
- Line 167: Remove the blank line within the blockquote in README.md, keeping
all blockquote lines contiguous to satisfy markdownlint MD028.
- Around line 153-164: Update provider documentation to match the implemented
selector: in README.md lines 153-164 explain that local runtimes require no API
key and models are chosen through the model picker; revise README.md line 96 and
line 249 to list the full supported providers or explicitly mark the examples as
non-exhaustive; update backend/CLAUDE.md line 51 likewise to include the full
local provider set or use “including”.
---
Nitpick comments:
In `@backend/src/config/llm.ts`:
- Around line 207-259: Update defaultBaseUrlForLlmProvider to read every
provider base URL from the already imported validated env module instead of
process.env, including OPENROUTER_BASE_URL, GOOGLE_GENERATIVE_AI_BASE_URL,
XAI_BASE_URL, and the remaining provider-specific settings. Preserve each
existing fallback URL and the undefined result for unsupported providers.
In `@backend/src/local-credentials.ts`:
- Around line 365-391: Update the provider-status construction around
LLM_PROVIDER_TYPES to resolve each localCredentialForLlmProvider call
concurrently with Promise.all, including the existing credential-to-status
mapping for each provider. After all entries resolve, construct providerStatuses
from the resulting provider/status pairs while preserving the current status
fields and ordering.
In `@frontend/convex/localCredentials.ts`:
- Around line 83-98: The llmPatch construction in the local credentials mutation
incorrectly clears llmBaseUrl and llmDefaultModel whenever llmProvider is
supplied, even if unchanged. Compare the incoming provider with the stored
provider and only clear or update those fields when the provider actually
changes; preserve existing values when the provider is unchanged and omitted
auxiliary fields should remain untouched. Apply the corresponding guard to
llmInsert as needed for new records.
In `@frontend/lib/backend.ts`:
- Around line 162-170: Update saveOpenRouterApiKey to obtain the OpenRouter
default model from the shared providerCopy.defaultModel source used by the setup
page, instead of hardcoding "anthropic/claude-sonnet-4.6". Ensure both
credential-save flows use the same canonical default value.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0460788e-90dd-44d2-bc63-10cc2d10418d
⛔ Files ignored due to path filters (19)
backend/package-lock.jsonis excluded by!**/package-lock.jsonfrontend/public/logos/providers/anthropic-icon.svgis excluded by!**/*.svgfrontend/public/logos/providers/anthropic.svgis excluded by!**/*.svgfrontend/public/logos/providers/deepinfra.svgis excluded by!**/*.svgfrontend/public/logos/providers/deepseek.svgis excluded by!**/*.svgfrontend/public/logos/providers/fireworks-ai.svgis excluded by!**/*.svgfrontend/public/logos/providers/google-g.svgis excluded by!**/*.svgfrontend/public/logos/providers/groq.svgis excluded by!**/*.svgfrontend/public/logos/providers/huggingface.svgis excluded by!**/*.svgfrontend/public/logos/providers/lmstudio.svgis excluded by!**/*.svgfrontend/public/logos/providers/mistral-ai.svgis excluded by!**/*.svgfrontend/public/logos/providers/ollama.svgis excluded by!**/*.svgfrontend/public/logos/providers/openai-icon.svgis excluded by!**/*.svgfrontend/public/logos/providers/openai.svgis excluded by!**/*.svgfrontend/public/logos/providers/openrouter-wordmark.svgis excluded by!**/*.svgfrontend/public/logos/providers/openrouter.svgis excluded by!**/*.svgfrontend/public/logos/providers/qwen.svgis excluded by!**/*.svgfrontend/public/logos/providers/together-ai.svgis excluded by!**/*.svgfrontend/public/logos/providers/xai.svgis excluded by!**/*.svg
📒 Files selected for processing (32)
CLAUDE.mdREADME.mdbackend/CLAUDE.mdbackend/package.jsonbackend/prompts/schema-inference.txtbackend/src/config/llm.tsbackend/src/config/models.tsbackend/src/env.tsbackend/src/index.tsbackend/src/local-credential-types.tsbackend/src/local-credentials.tsbackend/src/mastra/agents/investigate.tsbackend/src/mastra/agents/populate.tsbackend/src/mastra/agents/refresh.tsbackend/src/mastra/tools/dataset-tools.tsbackend/src/mastra/tools/investigate-tool.tsbackend/src/mastra/workflows/populate.tsbackend/src/mastra/workflows/update.tsbackend/src/pipeline/schema-inference.tsbackend/src/pipeline/types.tsfrontend/Dockerfile.devfrontend/app/dashboard/settings/models/page.tsxfrontend/app/setup/page.tsxfrontend/components/settings/LocalCredentialsPanel.tsxfrontend/components/settings/ModelSideSheet.tsxfrontend/components/settings/llm-providers.tsxfrontend/convex/localCredentials.tsfrontend/convex/modelConfig.tsfrontend/convex/schema.tsfrontend/lib/backend.tsfrontend/lib/openrouter-oauth.tsmakefiles/Makefile
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@backend/src/index.ts`:
- Around line 842-847: Move the route containing the inline requireAuth(req,
reply) call into the scoped protected Fastify plugin, or configure requireAuth
as the route’s preHandler. Remove the direct authentication invocation and
preserve the existing handler behavior while ensuring every request is
authenticated before execution.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9edffa17-4199-4aed-aa7b-1620251c601b
📒 Files selected for processing (5)
backend/src/config/models.tsbackend/src/index.tsfrontend/app/dashboard/settings/models/page.tsxfrontend/components/settings/ModelSideSheet.tsxfrontend/lib/backend.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/lib/backend.ts
- backend/src/config/models.ts
- frontend/components/settings/ModelSideSheet.tsx
- frontend/app/dashboard/settings/models/page.tsx
Two confirmed false-rejection regressions in the new hard-validation path: - qwen: fetchModelsForCurrentLlmProvider returns a static 8-entry stub, not a live catalog, so valid DashScope slugs (qwen-turbo, qwen-vl-max, future releases) were hard-rejected at save time despite working at runtime. qwen's catalog isn't enumerable, so it now joins custom/ollama/lmstudio in providerAllowsAnyModelSlug (validation skipped; slug accepted as-is). - openrouter: models are served from a persistent Convex cache with no TTL, so a cache predating a model's release would falsely reject a currently-offered slug. findUnsupportedModelSlugs now refreshes the cache once on a miss and re-checks before rejecting. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Refresh every hardcoded default model to a current, verified provider API slug (researched against official docs, 2026-07-21): - OpenRouter env defaults: claude-sonnet-5 (schema/orchestrator), claude-haiku-4.5 (subagent) - OpenAI: gpt-5.6-terra / gpt-5.6-luna (Sol/Terra/Luna tiers; the old gpt-5.4-mini naming was wrong) - Anthropic: claude-sonnet-5 / claude-haiku-4-5 - xAI: grok-4.5 - DeepSeek: deepseek-v4-pro / deepseek-v4-flash (deepseek-chat is being deprecated 2026-07-24) - Qwen: qwen3-max / qwen3.5-flash; refreshed static picker list - Mistral: mistral-medium-latest / mistral-small-latest - Groq: gpt-oss-120b / gpt-oss-20b - Together / DeepInfra / Fireworks / HF: GLM 5.2 and DeepSeek V4 slugs - Gemini kept at gemini-3.5-flash (3.6 Flash unverified on official docs) Also fixes the provider modal snapping back to OpenRouter on open: the Experimental Providers toggle now initializes from the current selection, and removes a stray double blank line in the model settings page. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Send HTTP-Referer + X-Title + X-OpenRouter-Categories on OpenRouter model calls so BigSet's usage (including local-mode users on their own keys) rolls up under one app on OpenRouter's public rankings, marketplace, and per-app analytics. Applied in the single createLanguageModel OpenRouter path, so it covers every generation call. Defaults to https://bigset.tinyfish.ai / "TinyFish BigSet" / "personal-agent", overridable via OPENROUTER_APP_URL / OPENROUTER_APP_TITLE / OPENROUTER_APP_CATEGORIES. https://openrouter.ai/docs/app-attribution Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Every hardcoded default was re-verified against the provider's own docs and moved onto a current model. The main theme is dropping the weak "cheap tier" picks: the investigate subagent does real web research, and models like gpt-oss-20b, claude-haiku-4.5, and Qwen3.5-9B are far below the bar now that $0.1-0.2/1M-input models score near-frontier on agentic benchmarks. - OpenAI: gpt-5.6-luna for all roles ($0.20/$1.20 makes terra hard to justify). - Anthropic: claude-sonnet-5 for all roles; Haiku 4.5 is $1/$5 for markedly less capability, so it is bad value rather than a cheap tier. - Google: gemini-3.6-flash (supersedes 3.5-flash, cheaper and stronger). - DeepSeek: deepseek-v4-flash for all roles — per DeepSeek's own benchmarks V4-Flash-0731 outperforms V4-Pro (Preview), and the Responses API does not support v4-pro at all. - Qwen: qwen3.7-max / qwen3.6-plus; refreshed the static DashScope picker list. - Mistral: mistral-medium-latest (Medium 3.5) for all roles. - Groq: openai/gpt-oss-120b for all roles — 20b is no longer worth shipping, and 120b is the strongest text model GroqCloud offers in production. - Together / DeepInfra / Fireworks: DeepSeek-V4-Flash-0731, which matches GLM-5.2 on intelligence at roughly a tenth of the price. - Hugging Face: DeepSeek-V4-Flash-0731 for the subagent. - OpenRouter env default subagent: openai/gpt-5.6-luna. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to the slug refresh: each provider's catalog was researched against its own docs rather than inferred, which turned up several picks that were wrong or unsafe. Corrections to the previous commit: - Fireworks: back off the `-0731` snapshot. It resolves and reports "live", but it has no row in the serverless pricing table that Fireworks calls the source of truth, so it may be dedicated-only. The unsuffixed `deepseek-v4-flash` is confirmed serverless with function calling. - Hugging Face: pin the router to `:deepinfra`. A bare repo id auto-routes to the fastest provider, and structured-output support varies per provider for the same model — Novita and Fireworks serve this one without it, which would break schema inference intermittently and non-reproducibly. - Qwen: qwen3.7-max is absent from Alibaba's JSON-mode support list, so schema inference moves to qwen3.6-plus. DashScope also bills every token at the tier the request's total input lands in, so a growing agent transcript can reprice the whole call; qwen3.7-max and qwen3.5-flash are the only two flat to 1M and now take the orchestrator and subagent roles. - Mistral: Large 3 is cheaper than Medium 3.5 and rated by Mistral as the stronger generalist, so it takes schema inference; Medium stays on the orchestrator as their agent-tuned model; Small 4 keeps tool calling and structured outputs at a fraction of the cost for the subagent. - Anthropic: orchestrator to claude-opus-5 (flat 1M pricing, no long-context surcharge, highest measured capability). - OpenAI: Sol for the two low-volume roles, Luna for the subagent fan-out. - OpenRouter env: schema inference to anthropic/claude-opus-5. Also teach the slug validator about the Hugging Face router's `<repo>:<target>` addressing. Its /v1/models listing only returns bare repo ids, so a pinned slug would be rejected on save even though it is the more precise address. Confirmed unchanged: DeepSeek (v4-flash beats Preview-tier v4-pro, is 3x cheaper, has 5x the concurrency the subagent fan-out needs, and is the only one the Responses API supports), Groq (gpt-oss-120b is the only production model with both tool calling and structured output), Google, xAI, Together, DeepInfra. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every role now runs at an explicit reasoning level instead of inheriting whatever the provider defaults to. That mattered more than it sounds: three separate providers default to expensive settings we were silently paying for — Anthropic defaults `effort: high` on Opus 5 and Sonnet 5, Gemini 3.6 Flash defaults to high dynamic thinking billed at the output rate, and OpenAI's GPT-5.6 default is undocumented entirely. Providers share no vocabulary here. xAI exposes two rungs, Anthropic five, Mistral only off/on, and Qwen isn't a ladder at all — a thinking toggle plus a token budget. So the UI and stored config speak one canonical 5-stop scale (none/low/medium/high/max) that the backend projects onto whatever each provider actually accepts. Providers whose SDK exposes no knob are marked unsupported and send nothing, rather than risking a 400 on an unrecognised parameter; the settings UI disables the control for them instead of pretending it applies. Defaults are inverse to model strength, since a role on a frontier model needs less deliberation to reach the same answer than the same role on a cheap one. Role shape matters too: schema inference is one short structured call, while the orchestrator and research subagents run long tool loops where premature termination is the common failure. So Opus 5 orchestrates at medium while gpt-oss-120b — the weakest default in the table, with no parallel tool calling — gets high on both agent roles. Levels are stored as an override, and absent means auto: the provider/role default is re-resolved at request time, so moving a role to a weaker model raises its reasoning without the user touching anything. Picking a level pins it; "Reset to auto" hands it back. Plumbing note: the level is baked into the model with the AI SDK's `defaultSettingsMiddleware` inside `createLanguageModel`, so `generateText`, the Mastra orchestrator, and the subagents all inherit it with no call-site changes. It travels through the workflow on `authContext` so every agent built during a run uses the level the request resolved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Same per-role slider as dashboard settings, alongside each model picker during
first-run setup, so the choice is visible where models are first chosen rather
than only after onboarding.
Also fixes a latent bug from the previous commit: setup's model save still
wrote a bare slug into the role, which is now an object of {model, reasoning,
reasoningOverridden}. The computed-key spread meant TypeScript widened the
write instead of rejecting it, so this only would have surfaced at runtime as
a role losing its reasoning level the moment its model changed.
Selecting a model now re-reads the config in setup too, so a role on auto
picks up the reasoning default for the model just chosen instead of keeping
the previous model's level.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # backend/src/mastra/tools/investigate-tool.ts
The "testing row extraction" commit (ef9de13) landed a spike on main: a 582-line GitHub-specific extractor plus a playwright-core dependency. This broke `node scripts/build-release.mjs`. The release bundler runs esbuild with `packages: "bundle"`, which tried to inline playwright-core and failed three ways: it requires chromium-bidi (not an installed dependency) and bundles its own chokidar, which pulls in the native fsevents.node binary that esbuild has no loader for. Remove the extractor, its call site in investigate-tool, and the playwright-core dependency. The extractor also branched on GitHub specifically, so it was not a general solution worth keeping. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Frontend: 19 advisories -> 0. - npm audit fix cleared the OpenTelemetry chain, brace-expansion, js-yaml, dompurify, protobufjs, ws, convex and posthog-js. - Bump the exact next pin 16.2.6 -> 16.3.0 (minor), which clears the next, postcss and sharp advisories. - Replace the abandoned xlsx@0.18.5 with @e965/xlsx@0.20.3, a maintained SheetJS republish. SheetJS moved distribution off npm, so the xlsx package there is frozen and permanently unpatched. Our usage is write-only (aoa_to_sheet/writeFile), so neither the prototype pollution nor the ReDoS advisory was reachable, but this keeps the audit clean. Backend: 16 advisories -> 6, all 6 highs resolved. - npm audit fix cleared find-my-way, ip-address, js-yaml, shell-quote, brace-expansion and fast-uri. - Bump the hono override 4.12.25 -> 4.12.34 for the CORS ReDoS advisory. The 6 remaining backend advisories (2 low, 4 moderate) are all unreachable in shipped code and have no non-regressive fix: - The 4 moderate are the mastra -> @mastra/deployer -> @hono/node-ws -> @hono/node-server chain. npm's only offered fix is a semver-major downgrade of mastra 1.23 -> 1.3.19. The esbuild metafile for the release bundle contains zero hono, @modelcontextprotocol/sdk and @mastra/deployer inputs, so this is dev-only (Mastra Studio), and the advisory is Windows serve-static path traversal. - @ai-sdk/provider-utils is pinned exactly at 3.0.30 by @mastra/core's multi-version shim; the 3.x line ends at 3.0.31, still within the advisory range, so no fixed version exists. - @mastra/core appears to be a prerelease-range false positive: the flagged range ends at 0.24.10-alpha.0 but 1.57.0 is installed. Both clear once Mastra updates its own dependencies. Verified: tsc --noEmit clean, eslint 0 errors, and node scripts/build-release.mjs produces a 36.2 MB artifact. Note: frontend/bun.lock is not regenerated here — it needs a bun install to match the updated package.json. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The release artifact built cleanly but was unusable: `convex deploy` failed on the user's machine with Could not resolve "../lib/llm-provider-types.js" convex/schema.ts, convex/modelConfig.ts and convex/localCredentials.ts all import shared types from the sibling frontend/lib directory, but copyConvexRuntime() only copied frontend/convex and package.json, so that file never shipped. Broken since d1ae1ef, which introduced the first convex -> lib import. This class of bug is invisible to the build: `convex deploy` only runs at install time, so a dangling import in the packaged tree surfaces on the user's machine rather than in CI. Copying lib/ fixes this instance; verifyConvexPackage() prevents the next one by resolving every relative import in the packaged Convex tree and failing the build if any dangle. Verified by removing the lib/ copy and confirming the build fails with all three unresolved imports (exit 1), then, with the fix in place, running the real installer command against a throwaway local backend: node node_modules/convex/bin/main.js deploy --url ... --admin-key ... => Deployed Convex functions
helperHref pointed at fireworks.ai/account/api-keys, which is not the key management page. The actual one is app.fireworks.ai/settings/users/api-keys. The setup wizard reads the same entry via llmProviderOption(), so this corrects the link in both the settings page and setup flow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The range input set appearance:none but never styled ::-webkit-slider-thumb, so WebKit rendered no thumb at all -- there was nothing to drag. Inline styles cannot express pseudo-elements, so the component as written could never have styled one. The 4px track meant the entire hit target was a 4px strip, which is why clicks so often landed on nothing. Reasoning effort is five discrete named steps, so it is a segmented control, not a slider: five real buttons with full-height hit targets, ordered None -> Max so the scale still reads as a scale. Arrow keys and Home/End work via radiogroup/radio semantics with a roving tabindex. Also: - "Auto" was wrong. reasoningOverridden:false means the level came from the provider/role default, not from anything adaptive. It now reads "Default" / "Reset to default". - A pinned level renders filled and an inherited one renders highlighted next to the Default pill, so which level runs stays separable from who chose it. - SettingsTile had no hover state and no pointer cursor, so the model row gave no sign it was clickable. Each role is now a bordered card with the model row on top and reasoning below a divider, so the model reads as the primary control rather than a heading for the slider. - Converted from inline styles to Tailwind, matching the other settings components. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@coderabbitai @greptileai and maybe @copilot too |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (10)
backend/src/config/llm.ts (2)
461-467: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCorrect the "No knob" comment above
openrouter.
REASONING_PROVIDER_OPTIONS.openrouterdefines a full projection at lines 280-286, soreasoningSupported("openrouter")returnstrue. The comment states the opposite for the entry directly below it. Move the comment so it only covers the providers whose projection isnull.♻️ Proposed comment fix
- // No knob — recorded so every provider resolves to a concrete level and the - // UI has something to show if support is added later. openrouter: { schemaInference: "low", populateOrchestrator: "medium", investigateSubagent: "high", }, + // No knob — recorded so every provider resolves to a concrete level and the + // UI has something to show if support is added later. togetherai: {🤖 Prompt for 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. In `@backend/src/config/llm.ts` around lines 461 - 467, Move the “No knob” comment away from REASONING_PROVIDER_OPTIONS.openrouter, since its full projection makes reasoningSupported("openrouter") true; place it immediately before the provider entries whose projection is null, preserving the openrouter configuration unchanged.
27-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRead the OpenRouter attribution values through the validated
envmodule.Lines 533-578 in this same file read base URLs from
env. These three attribution values readprocess.envdirectly. That split makes the environment surface harder to audit and bypasses the central declaration inbackend/src/env.ts. AddOPENROUTER_APP_URL,OPENROUTER_APP_TITLE, andOPENROUTER_APP_CATEGORIEStoenv.tsand read them from there.🤖 Prompt for 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. In `@backend/src/config/llm.ts` around lines 27 - 32, Move OPENROUTER_APP_URL, OPENROUTER_APP_TITLE, and OPENROUTER_APP_CATEGORIES into the validated env module declaration, then update the corresponding constants in llm configuration to read from env instead of process.env while preserving their existing defaults and behavior.backend/src/index.ts (2)
753-756: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTwo new protected routes register outside the scoped plugin. Both routes authenticate through an inline
preHandler: requireAuthon the top-levelfastifyinstance. The/settings/modelsroutes in the same file register oninstance, the scoped plugin. The shared root cause is route registration at the wrong instance, which splits the authentication boundary across two patterns.
backend/src/index.ts#L753-L756: movePOST /local-setup/llm-providerinto the scoped plugin and remove the inlinepreHandler.backend/src/index.ts#L881-L884: moveGET /llm-provider/modelsinto the same scoped plugin and remove the inlinepreHandler.As per coding guidelines, "Register new protected Fastify routes inside the scoped plugin whose
preHandlerisrequireAuth."🤖 Prompt for 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. In `@backend/src/index.ts` around lines 753 - 756, Move the POST /local-setup/llm-provider route at backend/src/index.ts lines 753-756 and the GET /llm-provider/models route at lines 881-884 into the scoped plugin registered on instance, alongside the /settings/models routes. Remove each inline preHandler: requireAuth and rely on the scoped plugin’s authentication boundary.Source: Coding guidelines
891-896: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign the upstream-failure status with
POST /settings/models.
POST /settings/modelsreturns502when the provider model list cannot be fetched. This route returns500for the same failure from the same function. Use502here so clients can distinguish an upstream provider failure from a backend fault.🤖 Prompt for 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. In `@backend/src/index.ts` around lines 891 - 896, Update the catch block handling the current LLM provider model-loading failure to return HTTP 502 instead of 500, matching POST /settings/models while preserving the existing error logging and response body.backend/src/mastra/tools/dataset-tools.ts (1)
77-91: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueBuild the record with a null prototype.
rowis an object literal. A cell whose column normalizes to__proto__does not create an own property; the assignment targets the prototype instead and the value is dropped.Object.hasOwnthen does not report a repeat of that column. A null-prototype record makes every column name a plain data key.♻️ Proposed refactor
- const row: Record<string, string> = {}; + const row: Record<string, string> = Object.create(null);🤖 Prompt for 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. In `@backend/src/mastra/tools/dataset-tools.ts` around lines 77 - 91, Initialize the row record in the data-row construction flow with a null prototype instead of an object literal. Update the `row` declaration used by the loop so `__proto__` and other special column names are stored as own data properties while preserving the existing `Object.hasOwn` duplicate detection and return behavior.frontend/components/settings/ReasoningControl.tsx (1)
52-76: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGuard the key handler while the control is disabled, and allow the keyboard to pin the current level.
Two behaviours in
handleKeyDown:
- The handler ignores
disabled. A radio that receives focus and is then disabled during a save can still commit a change through arrow keys in some browsers. Return early whendisabledis true.- When
clamped === selectedIndexthe handler returns without callingonChange. A mouse click on the selected segment does callonChangeand pins the inherited default. Keyboard users cannot pin the current level. Commit the value instead of returning when the level is not yet overridden.♻️ Proposed change
function handleKeyDown(event: React.KeyboardEvent<HTMLDivElement>) { + if (disabled) return; const deltas: Record<string, number> = { @@ event.preventDefault(); const clamped = Math.min(Math.max(next, 0), REASONING_LEVELS.length - 1); - if (clamped === selectedIndex) return; + if (clamped === selectedIndex && overridden) return; onChange(REASONING_LEVELS[clamped]);🤖 Prompt for 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. In `@frontend/components/settings/ReasoningControl.tsx` around lines 52 - 76, Update handleKeyDown to return immediately when disabled is true, before processing keyboard input. When clamped equals selectedIndex, still call onChange if the current level has not yet been overridden, so keyboard input can pin the inherited default; retain the no-op behavior for an already overridden current level.frontend/convex/modelConfig.ts (1)
77-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared patch-building logic.
upsertandupsertInternalnow contain the same patch construction, the same clearing rules, and the same insert/patch branch. Only the user id source differs. Extract one helper that takesctx,userId, andargs, then call it from both mutations. The public mutation can then be reduced to the identity check plus the helper call.Also applies to: 140-158
🤖 Prompt for 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. In `@frontend/convex/modelConfig.ts` around lines 77 - 95, Extract the duplicated patch construction and insert/patch branching from upsert and upsertInternal into a shared helper accepting ctx, userId, and args. Preserve the existing reasoning-field clearing behavior and provider updates, then have both mutations call the helper; keep the public upsert limited to its identity check and helper invocation.frontend/app/setup/page.tsx (1)
150-178: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePass the auth token to
getLlmProviderModels.The dashboard call site passes a bearer token (
frontend/app/dashboard/settings/models/page.tsxLine 201). This call site passes none. The setup page runs only in local mode, so the request currently succeeds. Passing the token keeps both call sites uniform and avoids a failure if the local-mode exemption is later removed.🤖 Prompt for 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. In `@frontend/app/setup/page.tsx` around lines 150 - 178, Update loadProviderModels to pass the available authentication bearer token to getLlmProviderModels, matching the dashboard settings call site. Ensure the token is available in the callback scope and include it in the callback dependencies as needed.frontend/convex/schema.ts (1)
154-163: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDeploy the schema change and consider constraining the reasoning fields.
Two points on this segment:
- The new
by_user_providerindex and the three reasoning fields require a deploy. Runmake convex-pushfrom the project root, because the self-hosted Convex instance does not hot-reload. As per coding guidelines: "After editing any file infrontend/convex/, runmake convex-pushfrom the project root".- The reasoning fields accept any string. The canonical scale is fixed (
none | low | medium | high | max). A literal union validator would reject invalid levels at write time and keep the stored data aligned with the scale.♻️ Optional: constrain the reasoning fields
+const reasoningLevelValidator = v.union( + v.literal("none"), + v.literal("low"), + v.literal("medium"), + v.literal("high"), + v.literal("max"), +);- schemaInferenceReasoning: v.optional(v.string()), - populateOrchestratorReasoning: v.optional(v.string()), - investigateSubagentReasoning: v.optional(v.string()), + schemaInferenceReasoning: v.optional(reasoningLevelValidator), + populateOrchestratorReasoning: v.optional(reasoningLevelValidator), + investigateSubagentReasoning: v.optional(reasoningLevelValidator),🤖 Prompt for 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. In `@frontend/convex/schema.ts` around lines 154 - 163, Update schemaInferenceReasoning, populateOrchestratorReasoning, and investigateSubagentReasoning to use optional literal validators restricted to none, low, medium, high, and max instead of optional strings. Then run make convex-push from the project root to deploy the new by_user_provider index and schema changes to the self-hosted Convex instance.Source: Coding guidelines
frontend/app/dashboard/settings/models/page.tsx (1)
18-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueShare
modelListCacheKeybetween the two pages.
frontend/app/setup/page.tsxLines 47-56 defines the same cache-key derivation, with a nullable status parameter. Two copies of the key format will drift and then the caches will disagree. Move one implementation into a shared module, for examplefrontend/components/settings/, and import it in both pages.🤖 Prompt for 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. In `@frontend/app/dashboard/settings/models/page.tsx` around lines 18 - 26, Move the shared cache-key derivation from modelListCacheKey into a reusable module, preserving its provider, baseUrl, defaultModel, and verifiedAt ordering and fallbacks. Update both the dashboard settings page and setup page to import and use the shared function, supporting the nullable status parameter required by setup.
🤖 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 `@backend/src/env.ts`:
- Around line 64-68: Swap the production defaults for
POPULATE_ORCHESTRATOR_MODEL and SCHEMA_INFERENCE_MODEL in the environment
configuration so they follow LLM_PROVIDER_DEFAULT_MODELS_BY_ROLE: use the
stronger Anthropic model for the populate orchestrator and the cheaper model for
schema inference. Leave INVESTIGATE_SUBAGENT_MODEL unchanged.
In `@backend/src/index.ts`:
- Around line 1169-1180: Update reasoningOrAuto and its surrounding comment to
reject unknown or off-scale reasoning values with a 400 response instead of
returning undefined. Ensure valid reasoning levels still populate the
configuration and preserve the existing handling for explicitly omitted values,
so invalid inputs cannot silently leave the stored override unchanged.
In `@frontend/app/dashboard/settings/models/page.tsx`:
- Around line 139-162: Update the reasoning-save failure handling in
frontend/app/dashboard/settings/models/page.tsx lines 139-162 and
frontend/app/setup/page.tsx lines 229-251: remove the full-configuration
snapshot and use a functional state update to restore only the affected role’s
previous reasoning value and reasoningOverridden flag, preserving concurrent
updates to other roles.
- Around line 284-294: Update the disabled condition on the ReasoningControl in
the role configuration rendering to also include the model-saving flag
isSavingModel, while preserving the existing savingReasoningRole check.
---
Nitpick comments:
In `@backend/src/config/llm.ts`:
- Around line 461-467: Move the “No knob” comment away from
REASONING_PROVIDER_OPTIONS.openrouter, since its full projection makes
reasoningSupported("openrouter") true; place it immediately before the provider
entries whose projection is null, preserving the openrouter configuration
unchanged.
- Around line 27-32: Move OPENROUTER_APP_URL, OPENROUTER_APP_TITLE, and
OPENROUTER_APP_CATEGORIES into the validated env module declaration, then update
the corresponding constants in llm configuration to read from env instead of
process.env while preserving their existing defaults and behavior.
In `@backend/src/index.ts`:
- Around line 753-756: Move the POST /local-setup/llm-provider route at
backend/src/index.ts lines 753-756 and the GET /llm-provider/models route at
lines 881-884 into the scoped plugin registered on instance, alongside the
/settings/models routes. Remove each inline preHandler: requireAuth and rely on
the scoped plugin’s authentication boundary.
- Around line 891-896: Update the catch block handling the current LLM provider
model-loading failure to return HTTP 502 instead of 500, matching POST
/settings/models while preserving the existing error logging and response body.
In `@backend/src/mastra/tools/dataset-tools.ts`:
- Around line 77-91: Initialize the row record in the data-row construction flow
with a null prototype instead of an object literal. Update the `row` declaration
used by the loop so `__proto__` and other special column names are stored as own
data properties while preserving the existing `Object.hasOwn` duplicate
detection and return behavior.
In `@frontend/app/dashboard/settings/models/page.tsx`:
- Around line 18-26: Move the shared cache-key derivation from modelListCacheKey
into a reusable module, preserving its provider, baseUrl, defaultModel, and
verifiedAt ordering and fallbacks. Update both the dashboard settings page and
setup page to import and use the shared function, supporting the nullable status
parameter required by setup.
In `@frontend/app/setup/page.tsx`:
- Around line 150-178: Update loadProviderModels to pass the available
authentication bearer token to getLlmProviderModels, matching the dashboard
settings call site. Ensure the token is available in the callback scope and
include it in the callback dependencies as needed.
In `@frontend/components/settings/ReasoningControl.tsx`:
- Around line 52-76: Update handleKeyDown to return immediately when disabled is
true, before processing keyboard input. When clamped equals selectedIndex, still
call onChange if the current level has not yet been overridden, so keyboard
input can pin the inherited default; retain the no-op behavior for an already
overridden current level.
In `@frontend/convex/modelConfig.ts`:
- Around line 77-95: Extract the duplicated patch construction and insert/patch
branching from upsert and upsertInternal into a shared helper accepting ctx,
userId, and args. Preserve the existing reasoning-field clearing behavior and
provider updates, then have both mutations call the helper; keep the public
upsert limited to its identity check and helper invocation.
In `@frontend/convex/schema.ts`:
- Around line 154-163: Update schemaInferenceReasoning,
populateOrchestratorReasoning, and investigateSubagentReasoning to use optional
literal validators restricted to none, low, medium, high, and max instead of
optional strings. Then run make convex-push from the project root to deploy the
new by_user_provider index and schema changes to the self-hosted Convex
instance.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 11987d61-34b0-4414-b243-6616b00c55a3
⛔ Files ignored due to path filters (1)
backend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (30)
README.mdbackend/CLAUDE.mdbackend/package.jsonbackend/src/config/llm.tsbackend/src/config/models.tsbackend/src/env.tsbackend/src/index.tsbackend/src/local-credentials.tsbackend/src/mastra/agents/investigate.tsbackend/src/mastra/agents/populate.tsbackend/src/mastra/agents/refresh.tsbackend/src/mastra/tools/dataset-tools.tsbackend/src/mastra/tools/investigate-tool.tsbackend/src/mastra/workflows/populate.tsbackend/src/pipeline/schema-inference.tsfrontend/app/dashboard/settings/models/page.tsxfrontend/app/setup/page.tsxfrontend/components/settings/LocalCredentialsPanel.tsxfrontend/components/settings/ReasoningControl.tsxfrontend/components/settings/SettingsTile.tsxfrontend/components/settings/llm-providers.tsxfrontend/convex/localCredentials.tsfrontend/convex/modelConfig.tsfrontend/convex/schema.tsfrontend/lib/backend.tsfrontend/lib/export.tsfrontend/lib/llm-provider-types.tsfrontend/lib/openrouter-oauth.tsfrontend/package.jsonscripts/build-release.mjs
🚧 Files skipped from review as they are similar to previous changes (10)
- backend/CLAUDE.md
- backend/package.json
- backend/src/mastra/agents/populate.ts
- README.md
- frontend/convex/localCredentials.ts
- frontend/lib/openrouter-oauth.ts
- backend/src/mastra/tools/investigate-tool.ts
- frontend/components/settings/llm-providers.tsx
- frontend/components/settings/LocalCredentialsPanel.tsx
- backend/src/local-credentials.ts
| process.env.SCHEMA_INFERENCE_MODEL ?? "anthropic/claude-opus-5", | ||
| POPULATE_ORCHESTRATOR_MODEL: | ||
| process.env.POPULATE_ORCHESTRATOR_MODEL ?? "qwen/qwen3.7-max", | ||
| process.env.POPULATE_ORCHESTRATOR_MODEL ?? "anthropic/claude-sonnet-5", | ||
| INVESTIGATE_SUBAGENT_MODEL: | ||
| process.env.INVESTIGATE_SUBAGENT_MODEL ?? "qwen/qwen3.7-max", | ||
| process.env.INVESTIGATE_SUBAGENT_MODEL ?? "openai/gpt-5.6-luna", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The production role defaults invert the model-strength rule used in backend/src/config/llm.ts.
LLM_PROVIDER_DEFAULT_MODELS_BY_ROLE assigns the stronger model to populateOrchestrator and the cheaper model to schemaInference. For Anthropic it uses claude-sonnet-5 for schema inference and claude-opus-5 for the orchestrator (backend/src/config/llm.ts lines 116-118). These production defaults do the opposite: Opus 5 runs the single short structured call, and Sonnet 5 runs the 80-step orchestrator. That raises cost on the cheap role and weakens the role that needs the most capability.
Confirm the intent. If the rule in llm.ts holds, swap the two values.
🐛 Proposed fix
- process.env.SCHEMA_INFERENCE_MODEL ?? "anthropic/claude-opus-5",
+ process.env.SCHEMA_INFERENCE_MODEL ?? "anthropic/claude-sonnet-5",
POPULATE_ORCHESTRATOR_MODEL:
- process.env.POPULATE_ORCHESTRATOR_MODEL ?? "anthropic/claude-sonnet-5",
+ process.env.POPULATE_ORCHESTRATOR_MODEL ?? "anthropic/claude-opus-5",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| process.env.SCHEMA_INFERENCE_MODEL ?? "anthropic/claude-opus-5", | |
| POPULATE_ORCHESTRATOR_MODEL: | |
| process.env.POPULATE_ORCHESTRATOR_MODEL ?? "qwen/qwen3.7-max", | |
| process.env.POPULATE_ORCHESTRATOR_MODEL ?? "anthropic/claude-sonnet-5", | |
| INVESTIGATE_SUBAGENT_MODEL: | |
| process.env.INVESTIGATE_SUBAGENT_MODEL ?? "qwen/qwen3.7-max", | |
| process.env.INVESTIGATE_SUBAGENT_MODEL ?? "openai/gpt-5.6-luna", | |
| process.env.SCHEMA_INFERENCE_MODEL ?? "anthropic/claude-sonnet-5", | |
| POPULATE_ORCHESTRATOR_MODEL: | |
| process.env.POPULATE_ORCHESTRATOR_MODEL ?? "anthropic/claude-opus-5", | |
| INVESTIGATE_SUBAGENT_MODEL: | |
| process.env.INVESTIGATE_SUBAGENT_MODEL ?? "openai/gpt-5.6-luna", |
🤖 Prompt for 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.
In `@backend/src/env.ts` around lines 64 - 68, Swap the production defaults for
POPULATE_ORCHESTRATOR_MODEL and SCHEMA_INFERENCE_MODEL in the environment
configuration so they follow LLM_PROVIDER_DEFAULT_MODELS_BY_ROLE: use the
stronger Anthropic model for the populate orchestrator and the cheaper model for
schema inference. Leave INVESTIGATE_SUBAGENT_MODEL unchanged.
| // An explicit level is stored as an override; "auto" (or anything off the | ||
| // scale) clears it so the provider/role default applies again. | ||
| const reasoningOrAuto = (value: unknown) => | ||
| isReasoningLevel(value) ? value : undefined; | ||
| const config = { | ||
| schemaInference: typeof body.schemaInference === "string" ? body.schemaInference.trim() || undefined : undefined, | ||
| populateOrchestrator: typeof body.populateOrchestrator === "string" ? body.populateOrchestrator.trim() || undefined : undefined, | ||
| investigateSubagent: typeof body.investigateSubagent === "string" ? body.investigateSubagent.trim() || undefined : undefined, | ||
| schemaInferenceReasoning: reasoningOrAuto(body.schemaInferenceReasoning), | ||
| populateOrchestratorReasoning: reasoningOrAuto(body.populateOrchestratorReasoning), | ||
| investigateSubagentReasoning: reasoningOrAuto(body.investigateSubagentReasoning), | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
An off-scale reasoning value is a silent no-op, not a reset.
The comment states that a value off the scale clears the override. reasoningOrAuto returns undefined for such a value, and upsertModelConfig treats undefined as "leave the stored value untouched" (backend/src/config/models.ts lines 531-532). Only a strict null sets clearReasoning. A client that sends "medum" therefore keeps the old override and receives a success response.
Either reject unknown reasoning values with 400, or map them to the clear path. Then align the comment with the chosen behavior.
🐛 Proposed fix: reject unknown values
+ const reasoningFields = [
+ "schemaInferenceReasoning",
+ "populateOrchestratorReasoning",
+ "investigateSubagentReasoning",
+ ] as const;
+ for (const field of reasoningFields) {
+ const value = body[field];
+ if (value !== undefined && value !== null && !isReasoningLevel(value)) {
+ return reply
+ .code(400)
+ .send({ error: `Invalid reasoning level for ${field}` });
+ }
+ }
const reasoningOrAuto = (value: unknown) =>
isReasoningLevel(value) ? value : undefined;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // An explicit level is stored as an override; "auto" (or anything off the | |
| // scale) clears it so the provider/role default applies again. | |
| const reasoningOrAuto = (value: unknown) => | |
| isReasoningLevel(value) ? value : undefined; | |
| const config = { | |
| schemaInference: typeof body.schemaInference === "string" ? body.schemaInference.trim() || undefined : undefined, | |
| populateOrchestrator: typeof body.populateOrchestrator === "string" ? body.populateOrchestrator.trim() || undefined : undefined, | |
| investigateSubagent: typeof body.investigateSubagent === "string" ? body.investigateSubagent.trim() || undefined : undefined, | |
| schemaInferenceReasoning: reasoningOrAuto(body.schemaInferenceReasoning), | |
| populateOrchestratorReasoning: reasoningOrAuto(body.populateOrchestratorReasoning), | |
| investigateSubagentReasoning: reasoningOrAuto(body.investigateSubagentReasoning), | |
| }; | |
| // An explicit level is stored as an override; "auto" (or anything off the | |
| // scale) clears it so the provider/role default applies again. | |
| const reasoningFields = [ | |
| "schemaInferenceReasoning", | |
| "populateOrchestratorReasoning", | |
| "investigateSubagentReasoning", | |
| ] as const; | |
| for (const field of reasoningFields) { | |
| const value = body[field]; | |
| if (value !== undefined && value !== null && !isReasoningLevel(value)) { | |
| return reply | |
| .code(400) | |
| .send({ error: `Invalid reasoning level for ${field}` }); | |
| } | |
| } | |
| const reasoningOrAuto = (value: unknown) => | |
| isReasoningLevel(value) ? value : undefined; | |
| const config = { | |
| schemaInference: typeof body.schemaInference === "string" ? body.schemaInference.trim() || undefined : undefined, | |
| populateOrchestrator: typeof body.populateOrchestrator === "string" ? body.populateOrchestrator.trim() || undefined : undefined, | |
| investigateSubagent: typeof body.investigateSubagent === "string" ? body.investigateSubagent.trim() || undefined : undefined, | |
| schemaInferenceReasoning: reasoningOrAuto(body.schemaInferenceReasoning), | |
| populateOrchestratorReasoning: reasoningOrAuto(body.populateOrchestratorReasoning), | |
| investigateSubagentReasoning: reasoningOrAuto(body.investigateSubagentReasoning), | |
| }; |
🤖 Prompt for 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.
In `@backend/src/index.ts` around lines 1169 - 1180, Update reasoningOrAuto and
its surrounding comment to reject unknown or off-scale reasoning values with a
400 response instead of returning undefined. Ensure valid reasoning levels still
populate the configuration and preserve the existing handling for explicitly
omitted values, so invalid inputs cannot silently leave the stored override
unchanged.
| const previous = effectiveConfig; | ||
| setSavingReasoningRole(role.key); | ||
| setSaveError(null); | ||
| // Optimistic: the control should track the click, not the round-trip. | ||
| setEffectiveConfig((prev) => | ||
| prev | ||
| ? { | ||
| ...prev, | ||
| [role.key]: { | ||
| ...prev[role.key as keyof typeof prev], | ||
| ...(level ? { reasoning: level } : {}), | ||
| reasoningOverridden: level !== null, | ||
| }, | ||
| } | ||
| : prev, | ||
| ); | ||
| try { | ||
| const token = await getToken(); | ||
| if (!token) throw new Error("Not authenticated"); | ||
| await saveModelConfig({ [role.key]: model.canonicalSlug }, token); | ||
| setEffectiveConfig((prev: EffectiveModelConfig | null) => | ||
| prev ? { ...prev, [role.key]: model.canonicalSlug } : null | ||
| await saveModelConfig({ [`${role.key}Reasoning`]: level }, token); | ||
| // Clearing needs the server's recomputed default, which we can't derive. | ||
| if (level === null) setModelConfigReloadKey((key) => key + 1); | ||
| } catch (err) { | ||
| setEffectiveConfig(previous); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Roll back only the failed role, not the whole configuration. Both handlers capture the complete configuration object before the request and restore that snapshot on failure. Each role has its own control, and only the saving role is disabled, so a user can start a second role's save while the first is in flight. A failure then discards the other role's successful update.
frontend/app/dashboard/settings/models/page.tsx#L139-L162: drop theprevioussnapshot and restore onlyrole.keyinside a functionalsetEffectiveConfigupdate.frontend/app/setup/page.tsx#L229-L251: apply the same change tosetModelConfig, restoring only the affected role'sreasoningandreasoningOverridden.
📍 Affects 2 files
frontend/app/dashboard/settings/models/page.tsx#L139-L162(this comment)frontend/app/setup/page.tsx#L229-L251
🤖 Prompt for 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.
In `@frontend/app/dashboard/settings/models/page.tsx` around lines 139 - 162,
Update the reasoning-save failure handling in
frontend/app/dashboard/settings/models/page.tsx lines 139-162 and
frontend/app/setup/page.tsx lines 229-251: remove the full-configuration
snapshot and use a functional state update to restore only the affected role’s
previous reasoning value and reasoningOverridden flag, preserving concurrent
updates to other roles.
| <ReasoningControl | ||
| value={roleConfig.reasoning} | ||
| overridden={roleConfig.reasoningOverridden} | ||
| disabled={savingReasoningRole === role.key} | ||
| unsupportedReason={ | ||
| reasoningSupported | ||
| ? undefined | ||
| : "This provider doesn't expose a reasoning control, so effort is left to the model's own default." | ||
| } | ||
| onChange={(level) => void saveReasoningForRole(role, level)} | ||
| /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Disable the reasoning control while a model save is in flight.
disabled covers only savingReasoningRole === role.key. A model save sets isSavingModel and then bumps modelConfigReloadKey, which re-reads the configuration. A reasoning change made in that window can be overwritten by the reload. The setup page includes the model-saving flag in the same check (frontend/app/setup/page.tsx Line 396).
🐛 Proposed fix
- disabled={savingReasoningRole === role.key}
+ disabled={
+ isSavingModel || savingReasoningRole === role.key
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <ReasoningControl | |
| value={roleConfig.reasoning} | |
| overridden={roleConfig.reasoningOverridden} | |
| disabled={savingReasoningRole === role.key} | |
| unsupportedReason={ | |
| reasoningSupported | |
| ? undefined | |
| : "This provider doesn't expose a reasoning control, so effort is left to the model's own default." | |
| } | |
| onChange={(level) => void saveReasoningForRole(role, level)} | |
| /> | |
| <ReasoningControl | |
| value={roleConfig.reasoning} | |
| overridden={roleConfig.reasoningOverridden} | |
| disabled={ | |
| isSavingModel || savingReasoningRole === role.key | |
| } | |
| unsupportedReason={ | |
| reasoningSupported | |
| ? undefined | |
| : "This provider doesn't expose a reasoning control, so effort is left to the model's own default." | |
| } | |
| onChange={(level) => void saveReasoningForRole(role, level)} | |
| /> |
🤖 Prompt for 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.
In `@frontend/app/dashboard/settings/models/page.tsx` around lines 284 - 294,
Update the disabled condition on the ReasoningControl in the role configuration
rendering to also include the model-saving flag isSavingModel, while preserving
the existing savingReasoningRole check.
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.
Modernized default model slugs
Every hardcoded default was researched against the provider's own docs — model list, pricing table, and the function-calling / structured-output capability pages — rather than inferred from naming patterns. Two themes: drop the weak "cheap tier" picks (the investigate subagent does real web research, so a bottom-tier model there is where quality leaks), and prefer models whose pricing shape suits a long agent loop.
anthropic/claude-opus-5anthropic/claude-sonnet-5openai/gpt-5.6-lunagpt-5.6-solgpt-5.6-solgpt-5.6-lunaclaude-sonnet-5claude-opus-5claude-sonnet-5gemini-3.6-flashgrok-4.5deepseek-v4-flashqwen3.6-plusqwen3.7-maxqwen3.5-flashmistral-large-latestmistral-medium-latestmistral-small-latestopenai/gpt-oss-120bdeepseek-ai/DeepSeek-V4-Flash-0731deepseek-ai/DeepSeek-V4-Flash-0731accounts/fireworks/models/deepseek-v4-flashdeepseek-ai/DeepSeek-V4-Flash:deepinfraWhy these, specifically
minimalthinking, and Google explicitly documents that this causes premature tool termination on multi-step tasks — exactly the subagent's workload.deepseek-v4-proentirely. V4-Flash is the officially released model (Pro is still Preview and unbenchmarked), is ~3x cheaper, has 5x the concurrency limit the 3-way subagent fan-out needs, and is the only one the Responses API supports.qwen3.7-maxandqwen3.5-flashare flat to 1M, hence their roles. Schema inference avoidsqwen3.7-maxbecause it is missing from Alibaba's JSON-mode support list, whileqwen3.6-plusis on it. The static picker list was refreshed andqwen-longremoved — it does not appear in the International supported-models list.gpt-oss-120bis the only production model with both tool calling and real structured output; the Llama models are JSON-object-mode only, anything stronger is preview-tier, and thegroq/compoundsystems reject user-provided tools outright. Droppinggpt-oss-20bwas load-bearing, not cosmetic — Fireworks lists its function calling as Not supported.-0731snapshot. It resolves and reportslive, but it has no row in the serverless pricing table that Fireworks calls the source of truth, so it may be dedicated-only. The unsuffixed slug is confirmed end-to-end.:deepinfrapin is load-bearing. A bare repo id auto-routes to the fastest provider, and structured-output support varies per provider for the same model: DeepInfra has it, Novita and Fireworks-ai do not. Left bare, schema inference would fail intermittently and non-reproducibly.Validator change
The slug validator now understands the Hugging Face router's
<repo>:<routing-target>addressing. That listing only ever returns bare repo ids, so a pinned slug would be rejected on save even though it is the more precise — and for structured output, the more correct — address.OpenRouter app attribution
OpenRouter model calls now send
HTTP-Referer,X-Title, andX-OpenRouter-Categories(app attribution) so BigSet's usage — including local-mode users on their own keys — rolls up under one app on OpenRouter's public rankings, marketplace, and per-app analytics. Applied in the singlecreateLanguageModelOpenRouter path, so it covers every generation call. Defaults tohttps://bigset.tinyfish.ai/TinyFish BigSet/personal-agent, overridable viaOPENROUTER_APP_URL/OPENROUTER_APP_TITLE/OPENROUTER_APP_CATEGORIES.Misc fixes