Configurable Multi-Model OpenRouter Pipeline with User Settings UI#108
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:
📝 WalkthroughWalkthroughAdds per-user OpenRouter model selection for three roles. Backend: model types/defaults, OpenRouter fetch+mapping, Convex schema and caching, slug validation, per-user modelConfig upsert/get, REST endpoints (/openrouter/refresh, /openrouter/models, GET/POST /settings/models), and threading modelConfig into infer/populate/update workflows and agents. Frontend: settings layout, Model Settings page, ModelSideSheet picker, supporting components, Convex server modules, frontend helpers to get/save config and refresh/list models, and a lucide-react dependency. Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant Backend
participant Convex
participant OpenRouter
participant Workflow
User->>Frontend: Open Model Settings
Frontend->>Backend: GET /settings/models
Backend->>Convex: get modelConfig (by user)
Convex-->>Backend: modelConfig
Backend-->>Frontend: EffectiveModelConfig
User->>Frontend: Refresh models
Frontend->>Backend: POST /openrouter/refresh
Backend->>OpenRouter: fetch models (OPENROUTER_API_KEY)
OpenRouter-->>Backend: model list
Backend->>Convex: upsertBatch(models)
Backend-->>Frontend: updated models
User->>Frontend: Select model
Frontend->>Backend: POST /settings/models
Backend->>Convex: upsertModelConfig(userId, patch)
Convex-->>Backend: saved
User->>Frontend: Start populate workflow
Frontend->>Backend: POST /populate
Backend->>Convex: getModelConfig(userId)
Convex-->>Backend: modelConfig
Backend->>Workflow: runPopulateWorkflowInBackground(authContext with modelConfig)
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
backend/src/mastra/workflows/populate.ts (1)
32-36: ⚡ Quick winConsider requiring non-empty model slugs.
These fields are declared as plain
z.string(), so an empty string passes validation. The fallback at Line 111 (?? DEFAULT_MODEL_IDS.SCHEMA_INFERENCE) only triggers onnull/undefined, so an empty slug would flow straight intoopenrouter(""). Tightening the schema to.min(1)closes this gap at the boundary.🛡️ Proposed fix
modelConfig: z.object({ - schemaInference: z.string(), - populateOrchestrator: z.string(), - investigateSubagent: z.string(), + schemaInference: z.string().min(1), + populateOrchestrator: z.string().min(1), + investigateSubagent: z.string().min(1), }),🤖 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/workflows/populate.ts` around lines 32 - 36, The modelConfig zod schema currently allows empty strings for model slugs (schemaInference, populateOrchestrator, investigateSubagent) so an empty value bypasses the fallback at DEFAULT_MODEL_IDS.SCHEMA_INFERENCE and reaches openrouter(""); update the schema to require non-empty strings (e.g., replace z.string() with z.string().min(1) for those three fields) so validation rejects empty slugs at the boundary and downstream code (functions referencing modelConfig.schemaInference / populateOrchestrator / investigateSubagent and the DEFAULT_MODEL_IDS fallback) always receives a meaningful value.backend/src/mastra/agents/populate.ts (1)
45-45: 💤 Low valueInconsistent fallback handling vs. the workflow.
The
enumeratestep usesauthContext?.modelConfig?.schemaInference ?? DEFAULT_MODEL_IDS.SCHEMA_INFERENCE, but here the sameauthContextis dereferenced with a non-null assertion and no default. SincemodelConfigis required byauthContextSchema, this is type-safe, but the divergent styles are confusing and leave the agents with no graceful fallback if the contract ever loosens. Consider mirroring the workflow's pattern for consistency.♻️ Optional consistency tweak
- const modelSlug = authContext.modelConfig!.populateOrchestrator; + const modelSlug = + authContext.modelConfig?.populateOrchestrator ?? DEFAULT_MODEL_IDS.POPULATE_ORCHESTRATOR;(requires importing
DEFAULT_MODEL_IDSfrom../../config/models.js)🤖 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/agents/populate.ts` at line 45, Replace the non-null asserted access to authContext.modelConfig!.populateOrchestrator with a safe optional-chaining + nullish-coalescing fallback so it mirrors the enumerate step (e.g., use authContext?.modelConfig?.populateOrchestrator ?? DEFAULT_MODEL_IDS.<appropriate_POPULATE_ORCHESTRATOR_KEY>), remove the "!" non-null assertion, and add an import for DEFAULT_MODEL_IDS from ../../config/models.js so the agent has a graceful default if the contract changes.frontend/components/settings/ModelSideSheet.tsx (1)
88-93: 💤 Low valueDrop
flushSynchere — a plain state reset is sufficient.Resetting
searchon open does not require synchronous flushing;flushSyncfrom inside an effect adds re-render overhead and is discouraged. A normalsetSearch("")reset will repaint correctly before the focus timeout.♻️ Suggested change
useEffect(() => { if (open) { - flushSync(() => setSearch("")); + setSearch(""); setTimeout(() => inputRef.current?.focus(), 100); } }, [open]);Also remove the now-unused
flushSyncimport on Line 4.🤖 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/ModelSideSheet.tsx` around lines 88 - 93, The useEffect currently calls flushSync to reset search on open which is unnecessary; replace flushSync(() => setSearch("")) with a plain setSearch("") inside the useEffect that runs when open changes (target the useEffect and setSearch call), keep the focus timeout on inputRef.current?.focus(), and remove the now-unused flushSync import from the module so no stale import remains.frontend/components/settings/types.ts (1)
1-7: ⚡ Quick winDuplicate
OpenRouterModeltype — consolidate to a single source.
frontend/components/settings/types.tsdefinesOpenRouterModel, butfrontend/app/dashboard/settings/models/page.tsxtypessheetModelswithOpenRouterModelfromfrontend/lib/backend.ts, andfrontend/components/settings/ModelSideSheet.tsxexpects the local./typesOpenRouterModel. Today they’re structurally identical, but this setup can mask drift. Export/import one sharedOpenRouterModeltype for the settings UI (instead of maintaining two).🤖 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/types.ts` around lines 1 - 7, There are duplicate OpenRouterModel definitions; consolidate to a single exported type and update imports: remove the local OpenRouterModel in frontend/components/settings/types.ts and instead import/export the shared OpenRouterModel from frontend/lib/backend.ts (or create a single shared types module and export it), then update references that use OpenRouterModel (e.g., sheetModels in frontend/app/dashboard/settings/models/page.tsx and ModelSideSheet in frontend/components/settings/ModelSideSheet.tsx) to import the unified type so all components reference the same symbol.
🤖 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 @.env.example:
- Around line 24-30: The default SCHEMA_INFERENCE_MODEL value is malformed
("anthropic/claude-sonnet-4-6"); update it to the correct OpenRouter slug
"anthropic/claude-sonnet-4.6" in .env.example and the corresponding default in
backend/src/env.ts (the constant/variable that supplies SCHEMA_INFERENCE_MODEL)
so both places match and schema inference (infer-schema and populate enumerate)
uses the correct model slug.
In `@backend/src/index.ts`:
- Around line 263-274: The /openrouter/refresh route is currently public;
protect it by either moving the fastify.post("/openrouter/refresh", ...) handler
into the existing protected fastify.register(...) block or add the auth
preHandler (e.g., requireAuth) to this route; ensure the handler still imports
and calls fetchModelsFromOpenRouter and upsertModelBatch but only after
authentication succeeds so external clients cannot trigger model fetches/upserts
without authorization.
In `@frontend/components/settings/ModelSideSheet.tsx`:
- Around line 221-236: The UI shows model.promptCost and model.completionCost
formatted with toFixed(2) but lacks a unit indicating these are per-1M tokens;
update the ModelSideSheet component (where model.promptCost and
model.completionCost are rendered) to include a clear unit label such as "/1M"
either in the column/header text for the cost columns or appended to the
displayed values (e.g., "$x.xx /1M") so users understand the prices are per
million tokens; ensure the label is added to the header element corresponding to
the cost columns or next to the <p> that renders model.promptCost and
model.completionCost.
In `@frontend/convex/modelConfig.ts`:
- Around line 34-39: The upsert mutation currently calls ctx.db.patch with all
args and thus overwrites fields with undefined; change the upsert implementation
to mirror upsertInternal by building a patch object that only includes defined
properties (e.g., check args.schemaInference, args.populateOrchestrator,
args.investigateSubagent and only assign those keys to the patch), then call
ctx.db.patch(existing._id, patch) so omitted fields are not cleared.
In `@frontend/convex/openRouterModels.ts`:
- Around line 12-39: The upsertBatch mutation is publicly exposed; change its
declaration from mutation({ ... }) to internalMutation({ ... }) so it becomes
backend-only (refer to the upsertBatch symbol in openRouterModels.ts), and then
update the backend caller that currently invokes
api.openRouterModels.upsertBatch to call the internal/admin-only variant using
setAdminAuth() (the backend models refresh logic that triggers upsertBatch) so
only backend requests can delete/insert the cached models.
In `@frontend/lib/backend.ts`:
- Around line 80-84: The fetch calls in frontend/lib/backend.ts (including
getModelConfig and the other fetches around the same file) rely on cookies
instead of Bearer tokens; update each backend call to obtain a JWT via
useAuth().getToken() from `@clerk/nextjs` and add an Authorization header "Bearer
<token>" to the fetch options (replace or augment existing credentials logic),
e.g., call getToken() before the fetch and include headers: { Authorization:
`Bearer ${token}`, "Content-Type": "application/json", ... } so all new model
endpoints use Bearer-token auth.
---
Nitpick comments:
In `@backend/src/mastra/agents/populate.ts`:
- Line 45: Replace the non-null asserted access to
authContext.modelConfig!.populateOrchestrator with a safe optional-chaining +
nullish-coalescing fallback so it mirrors the enumerate step (e.g., use
authContext?.modelConfig?.populateOrchestrator ??
DEFAULT_MODEL_IDS.<appropriate_POPULATE_ORCHESTRATOR_KEY>), remove the "!"
non-null assertion, and add an import for DEFAULT_MODEL_IDS from
../../config/models.js so the agent has a graceful default if the contract
changes.
In `@backend/src/mastra/workflows/populate.ts`:
- Around line 32-36: The modelConfig zod schema currently allows empty strings
for model slugs (schemaInference, populateOrchestrator, investigateSubagent) so
an empty value bypasses the fallback at DEFAULT_MODEL_IDS.SCHEMA_INFERENCE and
reaches openrouter(""); update the schema to require non-empty strings (e.g.,
replace z.string() with z.string().min(1) for those three fields) so validation
rejects empty slugs at the boundary and downstream code (functions referencing
modelConfig.schemaInference / populateOrchestrator / investigateSubagent and the
DEFAULT_MODEL_IDS fallback) always receives a meaningful value.
In `@frontend/components/settings/ModelSideSheet.tsx`:
- Around line 88-93: The useEffect currently calls flushSync to reset search on
open which is unnecessary; replace flushSync(() => setSearch("")) with a plain
setSearch("") inside the useEffect that runs when open changes (target the
useEffect and setSearch call), keep the focus timeout on
inputRef.current?.focus(), and remove the now-unused flushSync import from the
module so no stale import remains.
In `@frontend/components/settings/types.ts`:
- Around line 1-7: There are duplicate OpenRouterModel definitions; consolidate
to a single exported type and update imports: remove the local OpenRouterModel
in frontend/components/settings/types.ts and instead import/export the shared
OpenRouterModel from frontend/lib/backend.ts (or create a single shared types
module and export it), then update references that use OpenRouterModel (e.g.,
sheetModels in frontend/app/dashboard/settings/models/page.tsx and
ModelSideSheet in frontend/components/settings/ModelSideSheet.tsx) to import the
unified type so all components reference the same symbol.
🪄 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: bb82bc51-7b70-45fb-a626-035c04e32092
📒 Files selected for processing (24)
.env.examplebackend/src/config/models.tsbackend/src/env.tsbackend/src/index.tsbackend/src/mastra/agents/investigate.tsbackend/src/mastra/agents/populate.tsbackend/src/mastra/workflows/populate.tsbackend/src/pipeline/schema-inference.tsfrontend/app/dashboard/page.tsxfrontend/app/dashboard/settings/layout.tsxfrontend/app/dashboard/settings/models/page.tsxfrontend/app/dashboard/settings/page.tsxfrontend/components/settings/ModelSideSheet.tsxfrontend/components/settings/SettingsHeader.tsxfrontend/components/settings/SettingsPageLayout.tsxfrontend/components/settings/SettingsSidebar.tsxfrontend/components/settings/SettingsTile.tsxfrontend/components/settings/Skeleton.tsxfrontend/components/settings/types.tsfrontend/convex/modelConfig.tsfrontend/convex/openRouterModels.tsfrontend/convex/schema.tsfrontend/lib/backend.tsfrontend/package.json
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 `@frontend/convex/modelConfig.ts`:
- Around line 40-45: The insert uses args.userId which is undefined; update the
upsert logic so the owner comes from the authenticated identity: replace setting
insert.userId = args.userId with insert.userId = identity.subject (or
ctx.auth.identity.subject) before calling ctx.db.insert("modelConfig", insert),
and keep the conditional copies of args.schemaInference /
args.populateOrchestrator / args.investigateSubagent unchanged; ensure the
insert record uses the identity.subject string rather than any client-provided
userId.
🪄 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: 95d46f4f-5ce8-479f-9b78-c249407cd659
📒 Files selected for processing (6)
backend/src/config/models.tsbackend/src/index.tsbackend/src/mastra/workflows/populate.tsfrontend/components/settings/ModelSideSheet.tsxfrontend/convex/modelConfig.tsfrontend/convex/openRouterModels.ts
✅ Files skipped from review due to trivial changes (1)
- frontend/components/settings/ModelSideSheet.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/convex/openRouterModels.ts
- backend/src/mastra/workflows/populate.ts
- backend/src/config/models.ts
giaphutran12
left a comment
giaphutran12
left a comment
There was a problem hiding this comment.
Requesting changes for launch safety.
[P1] frontend/convex/modelConfig.ts:41 currently builds an insert with userId: args.userId, but the mutation args only define schemaInference, populateOrchestrator, and investigateSubagent on lines 20-24. args.userId does not exist, so the Convex function is broken/type-invalid and new model preference saves can insert the wrong/undefined owner.
Fix: use the authenticated identity already loaded at line 26, e.g. const insert: Record<string, string> = { userId: identity.subject };, then rerun the frontend/Convex type/build check.
…ocumentation for model preferences
giaphutran12
left a comment
giaphutran12
left a comment
There was a problem hiding this comment.
Re-reviewed the updated head (ccbb2b3). Original args.userId blocker in frontend/convex/modelConfig.ts is fixed, but this still cannot merge safely.
[P1] frontend/package.json adds lucide-react, but the lockfile is not updated. In a fresh PR worktree, bun install --frozen-lockfile fails with lockfile had changes, but lockfile is frozen. Fix: run the supported frontend install and commit the updated frontend/bun.lock.
[P1] The model refresh UI calls a protected backend route without auth. frontend/lib/backend.ts:160-163 posts to /openrouter/refresh with only { method: "POST" }, while backend/src/index.ts:381 registers /openrouter/refresh with preHandler: requireAuth. User impact: clicking Refresh in settings will get a 401 instead of refreshing models. Fix: pass the Clerk bearer token like the existing inferSchema/populate/update helpers do, or otherwise wire the settings helpers through the repo’s authenticated request pattern.
|
@giaphutran12 I've pushed the fixes. |
giaphutran12
left a comment
giaphutran12
left a comment
There was a problem hiding this comment.
Maintainer patch applied: fixed the invalid OpenRouter default slug, removed diff-check whitespace, and cleared the PR-side lint issue. Rechecked latest head: TruffleHog, OSV, and CodeRabbit are green.
2 participants
Screen.Recording.2026-05-30.at.2.40.03.PM.mov
This PR fixes the #96
1. No hardcoded production literals (except typed defaults)
pipeline/schema-inference.ts"anthropic/claude-sonnet-4-6"DEFAULT_MODEL_IDS.SCHEMA_INFERENCEmastra/workflows/populate.ts"anthropic/claude-sonnet-4-6"DEFAULT_MODEL_IDS.SCHEMA_INFERENCEmastra/agents/investigate.ts"qwen/qwen3.7-max"authContext.modelConfig.investigateSubagentmastra/agents/populate.ts"qwen/qwen3.7-max"authContext.modelConfig.populateOrchestratorenv.tscontains typed literal defaults — all other code reads fromDEFAULT_MODEL_IDSorauthContext.modelConfig.2. Backend has typed config for all three roles
config/models.tsexportsDEFAULT_MODEL_IDSwithSCHEMA_INFERENCE,POPULATE_ORCHESTRATOR,INVESTIGATE_SUBAGENT— all derived from env vars with typed fallback literals.getModelConfig(userId)returns a complete config — never null.3. OpenRouter multi-model selection (defaults + cheaper alternatives)
OpenRouter's model list is fetched with two filters:
output_modalities=text— excludes image/audio-only modelssupported_parameters=tools— only models that support tool-calling (required by Mastra agents)The
.env.exampledocuments both the default and cheaper alternatives with role annotations.4. Missing/invalid config fails with clear error
POST /settings/modelsvalidates each slug against the cachedopenRouterModelstable before saving. Invalid slugs return400:5. Settings UI persists through backend-owned API (no frontend API routes)
GET /settings/modelsPOST /settings/modelsGET /openrouter/modelsPOST /openrouter/refreshopenRouterModelslib/backend.tswhich proxies to these — no Next.js API routes.6.
.env.exampledescribes defaults without committing secretsHow It Works
Convex Tables
openRouterModels— Full OpenRouter model list, indexed bycanonicalSlug. Populated byPOST /openrouter/refresh. Auto-fetches on firstGET /openrouter/modelscall if cache is empty.modelConfig— Per-user preferences keyed byuserId(by_userindex). Partial upserts supported — only provided fields are updated.Model Resolution Flow
Excluded Models
Add model IDs here to exclude them from the settings UI picker and OpenRouter refresh — useful for deprecated or incompatible models.
Settings UI Path