Enhance model profile handling in backend configuration and runtime.#197
Enhance model profile handling in backend configuration and runtime.#197
Conversation
Introduce optional model profile catalog path and default profile ID in environment settings. Update chat service and orchestrator to utilize model profiles for provider-specific routing and capabilities.
📝 WalkthroughWalkthroughBackend model-profile catalog and resolver added (YAML, env override, default selection). GenerationRequest gains optional Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Orchestrator as Chat Orchestrator
participant Resolver as Model Profile Resolver
participant Planner as Chat Planner
participant Runtime as Generation Runtime
participant VoltAgent as VoltAgent
Client->>Orchestrator: POST /chat (selector or model)
Orchestrator->>Resolver: resolve(selector)
Resolver->>Resolver: lookup by id / tier / provider+model<br/>or synthesize passthrough/legacy
Resolver-->>Orchestrator: ModelProfile (provider, providerModel, capabilities)
Orchestrator->>Planner: execute(provider, providerModel, capabilities)
Planner->>Runtime: generate(model, provider, capabilities, search?)
Runtime->>VoltAgent: generateText(provider-prefixed model, search if supported)
VoltAgent-->>Runtime: generation result
Runtime-->>Planner: normalized result
Planner-->>Orchestrator: plan
Orchestrator->>Orchestrator: coerce plan for surface (web policy)
Orchestrator-->>Client: response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
…od to accept metadata and improving author label handling for better clarity in message formatting.
…nings for better debugging of profile ID mismatches.
… existing plan properties for improved plan handling.
… raw model. Default to 'openai' if provider is not specified, improving robustness in profile resolution logic.
…, enhancing flexibility in request handling. Adjust warning message for profile ID mismatches to improve clarity in error reporting.
…file ID handling, ensuring clear error reporting for data integrity.
…ucing a structured warning format. Update the warning emission logic to include metadata for better context in logs. Adjust tests to validate new warning structure and ensure proper fallback behavior for model profile resolution.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/backend/src/services/modelProfileResolver.ts (2)
50-52: Return a fresh capabilities object for synthetic profiles.
RAW_MODEL_PROFILE_CAPABILITIESis reused by reference. If any downstream code mutatesprofile.capabilities, that mutation can leak into other synthetic profiles.Proposed tweak
-const RAW_MODEL_PROFILE_CAPABILITIES: ModelProfile['capabilities'] = { +const RAW_MODEL_PROFILE_CAPABILITIES: ModelProfile['capabilities'] = Object.freeze({ canUseSearch: false, -}; +}); ... - capabilities: RAW_MODEL_PROFILE_CAPABILITIES, + capabilities: { ...RAW_MODEL_PROFILE_CAPABILITIES },Also applies to: 148-148
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/services/modelProfileResolver.ts` around lines 50 - 52, RAW_MODEL_PROFILE_CAPABILITIES is a shared object reused by reference, so mutations to profile.capabilities can leak across synthetic profiles; change code that assigns RAW_MODEL_PROFILE_CAPABILITIES to each synthetic profile (where profile.capabilities is set) to return a fresh object instead (e.g., clone via {...RAW_MODEL_PROFILE_CAPABILITIES} or a small factory function like getDefaultCapabilities()), and update any places that construct ModelProfile['capabilities'] to use that clone/factory so each profile gets its own independent capabilities object.
178-194: Avoid “not found” warnings whenDEFAULT_PROFILE_IDis intentionally unset.Line 187 currently warns on missing profile even when the default profile ID is omitted by design. For optional config, that creates noisy startup warnings in valid setups.
Proposed tweak
const resolveDefaultProfile = (): ModelProfile => { - const configuredDefault = catalogById.get(defaultProfileId); + const hasConfiguredDefaultId = defaultProfileId.trim().length > 0; + const configuredDefault = hasConfiguredDefaultId + ? catalogById.get(defaultProfileId) + : undefined; if (configuredDefault?.enabled) { return configuredDefault; } if (configuredDefault && !configuredDefault.enabled) { emitWarning(warn, { message: 'Configured DEFAULT_PROFILE_ID is disabled. Falling back to first enabled catalog profile.', meta: { defaultProfileId, }, }); - } else { + } else if (hasConfiguredDefaultId) { emitWarning(warn, { message: 'Configured DEFAULT_PROFILE_ID was not found. Falling back to first enabled catalog profile.', meta: { defaultProfileId, }, }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/services/modelProfileResolver.ts` around lines 178 - 194, The current else branch emits a "not found" warning even when DEFAULT_PROFILE_ID is intentionally unset; update the logic in modelProfileResolver.ts around the configuredDefault check (references: configuredDefault, defaultProfileId, emitWarning, warn) so that you only call emitWarning about a missing profile when defaultProfileId is non-empty/defined (e.g., truthy), and skip emitting any warning when the config value is intentionally omitted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/backend/src/services/modelProfileResolver.ts`:
- Around line 50-52: RAW_MODEL_PROFILE_CAPABILITIES is a shared object reused by
reference, so mutations to profile.capabilities can leak across synthetic
profiles; change code that assigns RAW_MODEL_PROFILE_CAPABILITIES to each
synthetic profile (where profile.capabilities is set) to return a fresh object
instead (e.g., clone via {...RAW_MODEL_PROFILE_CAPABILITIES} or a small factory
function like getDefaultCapabilities()), and update any places that construct
ModelProfile['capabilities'] to use that clone/factory so each profile gets its
own independent capabilities object.
- Around line 178-194: The current else branch emits a "not found" warning even
when DEFAULT_PROFILE_ID is intentionally unset; update the logic in
modelProfileResolver.ts around the configuredDefault check (references:
configuredDefault, defaultProfileId, emitWarning, warn) so that you only call
emitWarning about a missing profile when defaultProfileId is non-empty/defined
(e.g., truthy), and skip emitting any warning when the config value is
intentionally omitted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 81b03dd1-b8c9-421e-ab8b-16032502d9ec
📒 Files selected for processing (3)
packages/backend/src/services/chatOrchestrator.tspackages/backend/src/services/modelProfileResolver.tspackages/backend/test/modelProfileCatalog.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/backend/test/modelProfileCatalog.test.ts
- packages/backend/src/services/chatOrchestrator.ts
Introduce optional model profile catalog path and default profile ID in environment settings. Update chat service and orchestrator to utilize model profiles for provider-specific routing and capabilities.
Summary by CodeRabbit
New Features
Contracts
Tests