Conversation
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/twenty-front/src/modules/ai/components/AIChatEditorSection.tsx">
<violation number="1" location="packages/twenty-front/src/modules/ai/components/AIChatEditorSection.tsx:122">
P1: Persisted model selection is global and can leak across workspaces, leading to invalid `modelId` being sent in agent chat requests.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
packages/twenty-front/src/modules/ai/components/AIChatEditorSection.tsx
Outdated
Show resolved
Hide resolved
…e management for agent chat model selection
There was a problem hiding this comment.
2 issues found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/twenty-front/src/modules/ai/hooks/useAgentChatModelId.ts">
<violation number="1" location="packages/twenty-front/src/modules/ai/hooks/useAgentChatModelId.ts:17">
P2: `DEFAULT_SMART_MODEL` user selections are incorrectly treated as unavailable because availability is checked only against `enabledModels` (which excludes virtual models). Use `isModelEnabled` for availability checks so virtual models are accepted.</violation>
</file>
<file name="packages/twenty-front/src/modules/ai/components/AIChatEditorSection.tsx">
<violation number="1" location="packages/twenty-front/src/modules/ai/components/AIChatEditorSection.tsx:172">
P2: Using `resolvedModelId` as the Select value breaks the virtual Auto model selection display. Selecting Auto is immediately normalized to the workspace smart model, so the dropdown does not preserve/show the user’s Auto choice.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
packages/twenty-front/src/modules/ai/hooks/useAgentChatModelId.ts
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/ai/components/AIChatEditorSection.tsx
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/ai/hooks/useAgentChatModelId.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/metadata-modules/ai/ai-models/types/model-family.enum.ts
Show resolved
Hide resolved
| "label": "OpenAI", | ||
| "apiKey": "{{OPENAI_API_KEY}}", | ||
| "models": [ | ||
| { |
There was a problem hiding this comment.
This file is auto generated so if you fix this you need to fix the script that generates it too
packages/twenty-front/src/modules/ai/components/AIChatEditorSection.tsx
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/ai/components/AIChatEditorSection.tsx
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/api/graphql/direct-execution/direct-execution.service.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/twenty-server/src/engine/metadata-modules/ai/ai-models/ai-providers.json">
<violation number="1">
P1: Lowercasing `modelFamily` in the provider JSON breaks enum-based model-family checks at runtime and causes incorrect billing logic (e.g. Anthropic token accounting no longer matches `ModelFamily.CLAUDE`).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
packages/twenty-server/src/engine/metadata-modules/ai/ai-models/ai-providers.json
Show resolved
Hide resolved
packages/twenty-front/src/modules/ai/hooks/useAgentChatModelId.ts
Outdated
Show resolved
Hide resolved
| import { type ClientAiModelConfig } from '~/generated-metadata/graphql'; | ||
|
|
||
| const VIRTUAL_MODEL_IDS: Set<string> = new Set([ | ||
| const DEFAULT_MODEL_SENTINEL_IDS: Set<string> = new Set([ |
There was a problem hiding this comment.
Model sentinel is also a strange name, what are we trying to say here?
There was a problem hiding this comment.
Used “sentinel” here because we already use that term on the server (isDefaultModelSentinel) for the same default IDs, but yea it looks off -- I renamed it to DEFAULT_MODEL_IDS / isDefaultModelId for clarity (no behavior change)
There was a problem hiding this comment.
Yes apparently I introduced that and I don't even remember, that's bad 🤦♂️
| GROK = 'grok', | ||
| GPT = 'GPT', | ||
| CLAUDE = 'CLAUDE', | ||
| GEMINI = 'GEMINI', |
There was a problem hiding this comment.
I thought I had made a comment on the fact that this was synced with model family in providers.json so you need to update the script but I don't see it anymore, maybe I didn't hit send?
There was a problem hiding this comment.
you did send it 🙂
i just verified locally by running the sync script (ai-sync-models-dev.ts) and it already picks modelFamily from inferModelFamily / ModelFamily -- so no script change needed -- it would have synced automatically
i’ll include the regenerated file in this PR so it stays in sync
Visual Regression Report — twenty-uiNo visual changes detected across 0 stories. |
There was a problem hiding this comment.
1 issue found across 28 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/twenty-front/src/modules/ai/components/AIChatEditorSection.tsx">
<violation number="1" location="packages/twenty-front/src/modules/ai/components/AIChatEditorSection.tsx:132">
P2: Filter out the default model by `modelId`, not by `label`, to avoid dropping distinct models with the same label.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| : t`Default`; | ||
|
|
||
| const modelsWithoutDefault = enabledModels.filter( | ||
| (model) => model.label !== workspaceSmartModel?.label, |
There was a problem hiding this comment.
P2: Filter out the default model by modelId, not by label, to avoid dropping distinct models with the same label.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-front/src/modules/ai/components/AIChatEditorSection.tsx, line 132:
<comment>Filter out the default model by `modelId`, not by `label`, to avoid dropping distinct models with the same label.</comment>
<file context>
@@ -125,9 +125,13 @@ export const AIChatEditorSection = () => {
: t`Default`;
+ const modelsWithoutDefault = enabledModels.filter(
+ (model) => model.label !== workspaceSmartModel?.label,
+ );
+
</file context>
| (model) => model.label !== workspaceSmartModel?.label, | |
| (model) => model.modelId !== workspaceSmartModel?.modelId, |


https://www.figma.com/design/xt8O9mFeLl46C5InWwoMrN/Twenty?node-id=93653-368288&t=obTG32NRidXid4lN-0
closes https://discord.com/channels/1130383047699738754/1480990726442582086