Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Review Summary by QodoDynamic AI model discovery and embedding model dimension validation
WalkthroughsDescription• Implement dynamic AI model discovery from configured gateway instead of static list - Backend queries gateway API with timeout and filters non-generation models - Frontend fetches discovered models and allows manual entry with normalization • Restrict embedding model selection to 1536-dimension vectors for index compatibility - Add embedding model validation and fallback logic - Remove unsupported text-embedding-3-large option from UI • Improve model ID validation and error messages for better user guidance - Support both provider/model format and raw model IDs - Enhance diagnostic messages for invalid configurations • Refactor AI gateway utilities for consistent provider label detection - Extract helper functions for API key, base URL, and provider detection - Support multiple gateway configurations (OpenAI, Vercel, custom) Diagramflowchart LR
A["AI Gateway"] -->|"fetch /models"| B["discoverAvailableAIModels"]
B -->|"filter & dedupe"| C["listAvailableModels action"]
C -->|"discovered models"| D["Frontend useAIAgentSectionConvex"]
D -->|"display dropdown + input"| E["AIAgentSection UI"]
F["Embedding Model"] -->|"validate dimensions"| G["resolveContentEmbeddingModel"]
G -->|"1536-dim compatible"| H["Save to aiAgentSettings"]
File Changes1. apps/web/src/app/settings/hooks/useSettingsSectionsConvex.ts
|
Code Review by Qodo
1.
|
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR upgrades AI model selection by discovering available generation models dynamically from the configured AI gateway, normalizing/validating model IDs, and centralizing embedding model defaults/resolution so backend modules and the UI handle models consistently.
Changes:
- Replace static model list with authenticated, gateway-backed model discovery (with dedupe + fallback behavior).
- Normalize/validate generation model IDs (supporting both
provider/modeland raw IDs) and align frontend settings UI with discovered models. - Centralize content embedding model defaults/dimensions and apply consistent fallback logic across embedding/suggestions/knowledge paths.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/convex/tests/embeddingModels.test.ts | Adds coverage for embedding model compatibility + fallback behavior. |
| packages/convex/tests/aiAgentRuntimeSafety.test.ts | Updates diagnostics expectations for new model-format rules and gateway env. |
| packages/convex/tests/aiAgent.test.ts | Updates listAvailableModels callsite to action + adds fallback preservation test. |
| packages/convex/convex/suggestions.ts | Uses centralized embedding model resolution/defaults. |
| packages/convex/convex/schema/operationsReportingTables.ts | Makes messageId optional and adds attemptStatus to AI response reporting. |
| packages/convex/convex/schema/operationsAiTables.ts | Replaces hard-coded vector dimensions with shared constant. |
| packages/convex/convex/schema/inboxConversationTables.ts | Adds aiTurnSequence to conversation schema. |
| packages/convex/convex/lib/embeddingModels.ts | Introduces shared embedding model constants + compatibility helpers. |
| packages/convex/convex/lib/aiGateway.ts | Adds base URL/key normalization and provider-label inference. |
| packages/convex/convex/embeddings.ts | Uses centralized embedding model resolution/defaults for generation. |
| packages/convex/convex/aiAgentActionsKnowledge.ts | Uses centralized embedding model resolution/defaults for runtime knowledge. |
| packages/convex/convex/aiAgentActions.ts | Updates model parsing/diagnostics to support raw model IDs + inferred provider label. |
| packages/convex/convex/aiAgent.ts | Adds gateway model discovery and uses embedding model resolution across settings/queries. |
| packages/convex/convex/_generated/api.d.ts | Regenerates API types to include new module exports. |
| apps/web/src/app/settings/hooks/useSettingsSectionsConvex.ts | Switches available-models fetch from query to action and wires it into settings hook. |
| apps/web/src/app/settings/AIAgentSection.tsx | Adds discovered-model dropdown + manual model ID entry and trims model on save. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+28
to
+30
| return isContentEmbeddingModelSupportedByCurrentIndex(model) | ||
| ? model?.trim() || DEFAULT_CONTENT_EMBEDDING_MODEL | ||
| : DEFAULT_CONTENT_EMBEDDING_MODEL; |
There was a problem hiding this comment.
resolveContentEmbeddingModel can return a non-canonical model ID (it trims but doesn’t normalize casing). Because isContentEmbeddingModelSupportedByCurrentIndex lowercases before comparing, inputs like "Text-Embedding-3-Small" will be treated as supported yet returned as-is, which can lead to downstream embedding calls using a model ID the provider doesn’t recognize. Consider returning the canonical constant (DEFAULT_CONTENT_EMBEDDING_MODEL / LEGACY_CONTENT_EMBEDDING_MODEL) based on the normalized value, or otherwise normalizing to the exact supported IDs before returning.
Suggested change
| return isContentEmbeddingModelSupportedByCurrentIndex(model) | |
| ? model?.trim() || DEFAULT_CONTENT_EMBEDDING_MODEL | |
| : DEFAULT_CONTENT_EMBEDDING_MODEL; | |
| const normalized = normalizeModelName(model); | |
| if (normalized === "" || normalized === DEFAULT_CONTENT_EMBEDDING_MODEL) { | |
| return DEFAULT_CONTENT_EMBEDDING_MODEL; | |
| } | |
| if (normalized === LEGACY_CONTENT_EMBEDDING_MODEL) { | |
| return LEGACY_CONTENT_EMBEDDING_MODEL; | |
| } | |
| return DEFAULT_CONTENT_EMBEDDING_MODEL; |
Comment on lines
+120
to
+132
| function createAvailableAIModel(value: string | undefined): AvailableAIModel | null { | ||
| const normalizedId = normalizeAvailableModelId(value); | ||
| if (!normalizedId) { | ||
| return null; | ||
| } | ||
|
|
||
| const [provider, ...modelParts] = normalizedId.split("/"); | ||
| const model = modelParts.join("/") || normalizedId; | ||
| return { | ||
| id: normalizedId, | ||
| name: model, | ||
| provider, | ||
| }; |
There was a problem hiding this comment.
createAvailableAIModel assumes the ID always contains a / and treats the first segment as the provider. When selectedModel is a raw model ID (which the UI now allows), this will set provider to the raw ID itself and name to the full ID, producing misleading entries (and potentially breaking provider-based UI grouping later). Consider detecting the no-slash case and assigning a sensible provider label (e.g., the gateway provider label / "gateway") while keeping name as the raw model ID.
Comment on lines
275
to
279
| modelString: string, | ||
| environment: { aiGatewayApiKey?: string } = { | ||
| environment: { aiGatewayApiKey?: string; aiGatewayProviderLabel?: string } = { | ||
| aiGatewayApiKey: process.env.AI_GATEWAY_API_KEY, | ||
| aiGatewayProviderLabel: getAIGatewayProviderLabel(), | ||
| } |
There was a problem hiding this comment.
getAIConfigurationDiagnostic accepts environment.aiGatewayProviderLabel (and even sets a default), but the function never actually uses that value when deriving the provider for raw model IDs (it calls getAIGatewayProviderLabel() directly instead). This makes the parameter misleading and prevents callers/tests from controlling provider label resolution. Consider using environment.aiGatewayProviderLabel ?? getAIGatewayProviderLabel() when parts.length !== 2, or remove the parameter if it’s not needed.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 35 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+358
to
+363
| .catch((error) => { | ||
| console.error("Failed to load available AI models:", error); | ||
| if (!cancelled) { | ||
| setAvailableModels(undefined); | ||
| } | ||
| }); |
There was a problem hiding this comment.
availableModels uses undefined for both the initial loading state and the error fallback (catch sets it back to undefined). This makes the UI treat a failed fetch as "Loading discovered models..." indefinitely, with no way to distinguish failure vs loading. Consider tracking an explicit error/loaded state (e.g., availableModels: null | AvailableAIAgentModel[] or a separate availableModelsStatus) and rendering an appropriate message/fallback when discovery fails.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces significant improvements to AI model selection and discovery in the workspace settings UI and backend. The main changes include dynamic discovery of available AI models from the configured AI gateway, improved normalization and validation of model IDs, and consistent handling of embedding models. The UI now allows users to select from discovered models or enter a raw model ID, with enhanced feedback and error handling. Backend logic has been refactored to support these features and ensure robust model management.
AI Model Discovery and Selection Improvements
listAvailableModelsas an authenticated action) and the frontend, which now fetches and displays discovered models. [1] [2] [3] [4] [5]Model Normalization and Validation
Embedding Model Consistency
DEFAULT_CONTENT_EMBEDDING_MODELandresolveContentEmbeddingModelacross all relevant backend modules, ensuring consistent embedding model handling for both knowledge and content embeddings. [1] [2] [3] [4] [5] [6] [7]Minor Improvements and Cleanup
messageIdingetConversationResponses, improving data integrity.These changes collectively make AI model selection more flexible, robust, and user-friendly, while ensuring backend and frontend consistency.