feat(platform): wire default model resolver and add governance tab#1390
Merged
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
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.
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
📝 WalkthroughWalkthroughThis PR introduces a governance-based default model configuration system. It adds a Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/platform/app/features/chat/components/model-selector.tsx (1)
128-136:⚠️ Potential issue | 🟠 MajorFix override reset condition to respect governance-aware defaults.
Line 131 clears the user override whenever the selected model is
filteredModels[0]. If governance default is different, users cannot explicitly choose the agent’s primary model because the override is removed and selection reverts to governance default.💡 Suggested fix
const handleSelect = useCallback( (modelId: string) => { if (!effectiveAgent?.name) return; - if (modelId === filteredModels[0]) { + const implicitDefaultModelId = + governanceDefault?.modelId && + filteredModels.includes(governanceDefault.modelId) + ? governanceDefault.modelId + : filteredModels[0]; + + if (modelId === implicitDefaultModelId) { setSelectedModelOverride(effectiveAgent.name, null); } else { setSelectedModelOverride(effectiveAgent.name, modelId); } }, - [effectiveAgent?.name, filteredModels, setSelectedModelOverride], + [ + effectiveAgent?.name, + governanceDefault, + filteredModels, + setSelectedModelOverride, + ], );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/features/chat/components/model-selector.tsx` around lines 128 - 136, The handler handleSelect currently clears the user override when modelId equals filteredModels[0], which incorrectly removes an explicit choice if the governance default differs; change the check to compare modelId against the agent's primary/governance-aware default (e.g., effectiveAgent.primaryModel or effectiveAgent.governanceDefaultModel) so you only call setSelectedModelOverride(effectiveAgent.name, null) when the user selects the agent's actual primary/governance default, otherwise call setSelectedModelOverride(effectiveAgent.name, modelId).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/platform/app/features/chat/components/chat-interface.tsx`:
- Around line 419-420: The model fallback is inconsistent: normal sends use
(selectedModelOverrides[effectiveAgent.name] ?? governanceDefault?.modelId) but
the edit-and-branch path only checks overrides. Update the edit-and-branch model
resolution to use the same fallback (use
selectedModelOverrides[effectiveAgent.name] ?? governanceDefault?.modelId) or
extract a single helper like getEffectiveModelId(effectiveAgent) and use it from
both the normal send and edit-and-branch code paths so both paths resolve the
same model when an override is missing.
In
`@services/platform/app/features/settings/governance/components/default-model-editor.test.tsx`:
- Around line 60-69: Add unit tests in default-model-editor.test.tsx that cover
the DefaultModelEditor component's happy and error paths beyond the existing axe
audit: write a happy-path test that renders <DefaultModelEditor
organizationId="org-1" />, uses userEvent to add a rule, make a valid selection,
save, and assert the expected mutation was called and UI shows success (or
updated rule list); write edge-case tests for invalid selection that attempt to
save and assert validation UI prevents the save and shows the appropriate
validation message; write a failure-path test that mocks the save/mutation to
reject (or use msw to return 500) and assert the error toast is displayed; use
the component’s exported mutation or API helper (mock the function used by
DefaultModelEditor) and query the DOM to verify add/remove rule behavior and
toast messages.
In
`@services/platform/app/features/settings/governance/components/default-model-editor.tsx`:
- Around line 109-127: The form currently can submit stale scopeId values when
scope changes (e.g., team -> role), so update handleSubmit to sanitize the draft
before calling onSave: derive a cleaned copy of draft (or call setDraft
synchronously) that deletes scopeId when scope === 'default', and clears or
validates scopeId when scope changed between 'team' and 'role' (use the existing
updateDraft logic or compare prev draft.providerName/scope if available); then
pass this cleaned object to onSave and close via onOpenChange(false) to avoid
persisting impossible rules. Use the symbols handleSubmit, updateDraft,
setDraft, draft, and onSave to locate and implement the change.
In `@services/platform/convex/agents/unified_chat.ts`:
- Around line 96-111: When you set effectiveModelId from the governance default
(from ctx.runQuery using
internal.governance.internal_queries.resolveDefaultModelInternal) validate that
effectiveModelId is actually supported by the selected agent (check against
agent.supportedModels) before passing it into resolveAgentConfig; if it isn’t
supported, either clear effectiveModelId so resolveAgentConfig won’t silently
drop it, or emit a warning via the logger indicating the governance default
modelId was incompatible with the agent’s supportedModels and will be ignored.
Ensure you reference the variables effectiveModelId and the agent object used by
resolveAgentConfig so the validation/logging happens immediately after resolving
the governance default and before calling resolveAgentConfig.
In `@services/platform/convex/governance/default_model_query.ts`:
- Around line 31-32: Wrap the call to authComponent.getAuthUser(ctx) in a
try/catch and treat any thrown error as an unauthenticated result: call
authComponent.getAuthUser(ctx) inside a try, assign to authUser if it succeeds,
and in the catch set authUser = null (no logging), then keep the existing check
(if (!authUser) return null) so failures are swallowed and the query returns
null instead of throwing; update the logic around the authUser variable in
default_model_query.ts accordingly.
In `@services/platform/convex/governance/internal_queries.ts`:
- Around line 107-120: The memberships query is using the raw JWT args.userId
and a hard cap of 100 items, which misses migrated/linked accounts and extra
pages; change the findMany call in this flow to query by the resolved Better
Auth id from member.userId (the object returned by getOrganizationMember)
instead of args.userId, and implement pagination (loop/accumulate) using
components.betterAuth.adapter.findMany until page is exhausted, increasing
paginationOpts.numItems to the repo-standard 1000 to collect all teamMember
rows, then derive teamIds from the accumulated results.
In `@services/platform/messages/en.json`:
- Around line 1451-1453: The value for settings.providers.description was
accidentally set to the literal "Description" which breaks the Providers page
subtitle; revert or replace that string in the en.json entry for
"settings.providers.description" with the intended subtitle copy (either restore
the original text from the previous commit or copy the matching value from
another locale) so it reads the proper descriptive sentence (not the field label
which lives at settings.providers.description_field).
---
Outside diff comments:
In `@services/platform/app/features/chat/components/model-selector.tsx`:
- Around line 128-136: The handler handleSelect currently clears the user
override when modelId equals filteredModels[0], which incorrectly removes an
explicit choice if the governance default differs; change the check to compare
modelId against the agent's primary/governance-aware default (e.g.,
effectiveAgent.primaryModel or effectiveAgent.governanceDefaultModel) so you
only call setSelectedModelOverride(effectiveAgent.name, null) when the user
selects the agent's actual primary/governance default, otherwise call
setSelectedModelOverride(effectiveAgent.name, modelId).
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 815befd0-b84d-4d6c-be0b-6f02a36b2211
⛔ Files ignored due to path filters (1)
services/platform/convex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (16)
services/platform/app/features/chat/components/__tests__/model-selector.test.tsxservices/platform/app/features/chat/components/chat-interface.tsxservices/platform/app/features/chat/components/model-selector.tsxservices/platform/app/features/chat/hooks/use-default-model.tsservices/platform/app/features/settings/governance/components/default-model-editor.test.tsxservices/platform/app/features/settings/governance/components/default-model-editor.tsxservices/platform/app/routes/dashboard/$id/settings/governance.tsxservices/platform/convex/agents/unified_chat.tsservices/platform/convex/governance/__tests__/resolve_default_model.test.tsservices/platform/convex/governance/default_model_query.tsservices/platform/convex/governance/internal_queries.tsservices/platform/convex/governance/mutations.tsservices/platform/messages/de-AT.jsonservices/platform/messages/de-CH.jsonservices/platform/messages/de.jsonservices/platform/messages/en.json
Comment on lines
+419
to
+420
| ? (selectedModelOverrides[effectiveAgent.name] ?? | ||
| governanceDefault?.modelId) |
Contributor
There was a problem hiding this comment.
Align model fallback logic across send entry points.
Line 419 applies governance fallback for normal sends, but edit-and-branch still resolves model from override-only (Line 470). That creates inconsistent model selection behavior depending on how the user sends.
Suggested patch
- const modelId = effectiveAgent.name
- ? selectedModelOverrides[effectiveAgent.name]
- : undefined;
+ const modelId = effectiveAgent.name
+ ? (selectedModelOverrides[effectiveAgent.name] ??
+ governanceDefault?.modelId)
+ : undefined;
...
[
editingMessage,
dataThreadId,
rootThreadId,
effectiveAgent,
selectedModelOverrides,
+ governanceDefault,
organizationId,
userContext,
editAndBranchAction,
selectNewBranch,
setPendingMessage,
],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/chat/components/chat-interface.tsx` around
lines 419 - 420, The model fallback is inconsistent: normal sends use
(selectedModelOverrides[effectiveAgent.name] ?? governanceDefault?.modelId) but
the edit-and-branch path only checks overrides. Update the edit-and-branch model
resolution to use the same fallback (use
selectedModelOverrides[effectiveAgent.name] ?? governanceDefault?.modelId) or
extract a single helper like getEffectiveModelId(effectiveAgent) and use it from
both the normal send and edit-and-branch code paths so both paths resolve the
same model when an override is missing.
Comment on lines
+60
to
+69
| describe('DefaultModelEditor', () => { | ||
| describe('accessibility', () => { | ||
| it('passes axe audit', async () => { | ||
| const { container } = render( | ||
| <DefaultModelEditor organizationId="org-1" />, | ||
| ); | ||
| await checkAccessibility(container); | ||
| }); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add functional and error-path tests beyond the axe audit.
Line 62 only verifies accessibility. Please also add happy-path and failure-path tests for rule CRUD/save behavior (e.g., add/remove rule, invalid selection, mutation failure toast) to prevent regressions in this new governance editor.
As per coding guidelines **/*.{test,spec}.{ts,tsx,js,jsx}: “Tests should cover happy paths, edge cases, and error conditions.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/settings/governance/components/default-model-editor.test.tsx`
around lines 60 - 69, Add unit tests in default-model-editor.test.tsx that cover
the DefaultModelEditor component's happy and error paths beyond the existing axe
audit: write a happy-path test that renders <DefaultModelEditor
organizationId="org-1" />, uses userEvent to add a rule, make a valid selection,
save, and assert the expected mutation was called and UI shows success (or
updated rule list); write edge-case tests for invalid selection that attempt to
save and assert validation UI prevents the save and shows the appropriate
validation message; write a failure-path test that mocks the save/mutation to
reject (or use msw to return 500) and assert the error toast is displayed; use
the component’s exported mutation or API helper (mock the function used by
DefaultModelEditor) and query the DOM to verify add/remove rule behavior and
toast messages.
Comment on lines
+33
to
+48
| const SCOPE_OPTIONS = [ | ||
| { value: 'default', label: 'Default' }, | ||
| { value: 'team', label: 'Team' }, | ||
| { value: 'role', label: 'Role' }, | ||
| ]; | ||
|
|
||
| function isScopeValue(v: string): v is DefaultModelRule['scope'] { | ||
| return SCOPE_OPTIONS.some((o) => o.value === v); | ||
| } | ||
|
|
||
| const ROLE_OPTIONS = [ | ||
| { value: 'admin', label: 'Admin' }, | ||
| { value: 'developer', label: 'Developer' }, | ||
| { value: 'editor', label: 'Editor' }, | ||
| { value: 'member', label: 'Member' }, | ||
| ]; |
Contributor
There was a problem hiding this comment.
Localize scope and role labels instead of rendering raw English/IDs.
The new editor still hardcodes Default/Team/Role/Admin/… in the option lists and renders raw rule.scope / scopeId values in the table. This PR adds locale coverage for the feature, so non-English users will still get mixed untranslated text on this screen unless these labels go through t(...).
As per coding guidelines: “Do NOT hardcode text, use the translation hooks/functions instead for user-facing UI.”
Also applies to: 422-438, 523-526
Comment on lines
+109
to
+127
| const updateDraft = useCallback((patch: Partial<DefaultModelRule>) => { | ||
| setDraft((prev) => { | ||
| const updated = { ...prev, ...patch }; | ||
| if (patch.scope === 'default') { | ||
| delete updated.scopeId; | ||
| } | ||
| if (patch.providerName && patch.providerName !== prev.providerName) { | ||
| updated.modelId = ''; | ||
| } | ||
| return updated; | ||
| }); | ||
| }, []); | ||
|
|
||
| const handleSubmit = useCallback( | ||
| (e: React.FormEvent) => { | ||
| e.preventDefault(); | ||
| onSave(draft); | ||
| onOpenChange(false); | ||
| }, |
Contributor
There was a problem hiding this comment.
Clear and validate scope-dependent fields before saving.
When an existing rule is switched from team to role (or the other way around), scopeId is preserved unless the new scope is default. handleSubmit then saves that stale value without any validation, so you can persist impossible rules like a role-scoped rule carrying a team id.
Suggested fix
const updateDraft = useCallback((patch: Partial<DefaultModelRule>) => {
setDraft((prev) => {
const updated = { ...prev, ...patch };
- if (patch.scope === 'default') {
- delete updated.scopeId;
+ if (patch.scope && patch.scope !== prev.scope) {
+ if (patch.scope === 'default') {
+ delete updated.scopeId;
+ } else {
+ updated.scopeId = '';
+ }
}
if (patch.providerName && patch.providerName !== prev.providerName) {
updated.modelId = '';
}
return updated;
@@
const handleSubmit = useCallback(
(e: React.FormEvent) => {
e.preventDefault();
+ if (!draft.providerName || !draft.modelId) return;
+ if (draft.scope !== 'default' && !draft.scopeId) return;
onSave(draft);
onOpenChange(false);
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/settings/governance/components/default-model-editor.tsx`
around lines 109 - 127, The form currently can submit stale scopeId values when
scope changes (e.g., team -> role), so update handleSubmit to sanitize the draft
before calling onSave: derive a cleaned copy of draft (or call setDraft
synchronously) that deletes scopeId when scope === 'default', and clears or
validates scopeId when scope changed between 'team' and 'role' (use the existing
updateDraft logic or compare prev draft.providerName/scope if available); then
pass this cleaned object to onSave and close via onOpenChange(false) to avoid
persisting impossible rules. Use the symbols handleSubmit, updateDraft,
setDraft, draft, and onSave to locate and implement the change.
Comment on lines
+96
to
+111
| // Resolve governance default model when no explicit model is provided | ||
| let effectiveModelId = args.modelId; | ||
| if (!effectiveModelId) { | ||
| const governanceDefault = await ctx.runQuery( | ||
| internal.governance.internal_queries.resolveDefaultModelInternal, | ||
| { | ||
| organizationId: args.organizationId, | ||
| userId: String(authUser._id), | ||
| userEmail: authUser.email, | ||
| userName: authUser.name, | ||
| }, | ||
| ); | ||
| if (governanceDefault) { | ||
| effectiveModelId = governanceDefault.modelId; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate resolveAgentConfig implementation =="
fd -i "file_actions.ts" services/platform/convex --exec sh -c '
f="$1"
echo "---- $f"
rg -n -C6 "resolveAgentConfig|modelId|supportedModels|fallback|default" "$f"
' sh {}
echo
echo "== Inspect start_chat usage paths for model validation/fallback =="
fd -i "start_chat.ts" services/platform/convex --exec sh -c '
f="$1"
echo "---- $f"
rg -n -C6 "agentConfig|modelId|supportedModels|default" "$f"
' sh {}
echo
echo "== Find tests covering unsupported governance-default model cases =="
rg -n -C4 "resolveDefaultModelInternal|governance.*default|unsupported|supportedModels|modelId" services/platform/convex --type tsRepository: tale-project/tale
Length of output: 13111
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for tests covering unsupported model or governance fallback scenarios =="
rg -n "unsupported|fallback|supportedModels.*false|governance.*model|modelId.*not.*found" services/platform/convex --type ts -i -A 3 | head -60
echo
echo "== Check for error/warning logging in resolveAgentConfig or model validation flow =="
rg -n "console\.(error|warn|log)|throw|Error|ValidationError" services/platform/convex/agents/file_actions.ts -B 2 -A 2 | grep -A 5 -B 5 "modelId\|supportedModels" | head -40
echo
echo "== Look for validation or error handling in unified_chat.ts around governance resolution =="
rg -n "resolveDefaultModelInternal|effectiveModelId|throw|Error" services/platform/convex/agents/unified_chat.ts -B 2 -A 2
echo
echo "== Check if there are unit tests for agent config resolution =="
fd -i "\.test\.ts|\.spec\.ts" services/platform/convex --exec sh -c '
f="$1"
if rg -l "resolveAgentConfig" "$f" 2>/dev/null; then
echo "---- Test file: $f"
rg -n "supportedModels|modelId|unsupported|fallback|governance" "$f" -C 3 | head -40
fi
' sh {}Repository: tale-project/tale
Length of output: 5264
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check governance resolution for validation/error handling =="
fd -i "*governance*" services/platform/convex --exec sh -c '
f="$1"
if [[ "$f" == *.ts ]]; then
echo "---- File: $f"
rg -n "resolveDefaultModelInternal|modelId|supportedModels|throw|Error" "$f" -B 1 -A 2 | head -50
fi
' sh {}
echo
echo "== Search for where governance defaults are created/validated =="
rg -n "resolveDefaultModelInternal|defaultModel|governance" services/platform/convex --type ts -B 2 -A 4 | grep -A 6 "resolveDefaultModelInternal" | head -50
echo
echo "== Check if agent config applies any validation or throws on unsupported modelId =="
sed -n '420,470p' services/platform/convex/agents/file_actions.ts
echo
echo "== Look for any logs/warnings about model fallback or governance in unified_chat.ts =="
sed -n '90,130p' services/platform/convex/agents/unified_chat.tsRepository: tale-project/tale
Length of output: 433
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check governance resolution for validation/error handling =="
fd --glob "*governance*" services/platform/convex --type f --exec sh -c '
f="$1"
if [[ "$f" == *.ts ]]; then
echo "---- File: $f"
rg -n "resolveDefaultModelInternal|modelId|supportedModels|throw|Error" "$f" -B 1 -A 2 2>/dev/null | head -40
fi
' sh {}
echo
echo "== Find resolveDefaultModelInternal definition =="
rg -n "resolveDefaultModelInternal" services/platform/convex --type ts -A 15 | head -60
echo
echo "== Read key section from file_actions.ts: resolveAgentConfig function =="
sed -n '420,470p' services/platform/convex/agents/file_actions.ts
echo
echo "== Read unified_chat.ts around governance resolution and resolveAgentConfig call =="
sed -n '90,125p' services/platform/convex/agents/unified_chat.tsRepository: tale-project/tale
Length of output: 5710
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check if there's any validation or warning when unsupported modelId is passed =="
rg -n "supportedModels.includes|modelId" services/platform/convex/agents/file_actions.ts -B 3 -A 3 | grep -A 5 -B 5 "includes"
echo
echo "== Look for tests in agents directory =="
find services/platform/convex -name "*.test.ts" -o -name "*.spec.ts" | head -10
echo
echo "== Search for any test files related to agent config or chat =="
find services/platform/convex -type f -name "*test*" -o -name "*spec*" | grep -i agent
echo
echo "== Check if there's any logging or error on model mismatch =="
rg -n "console\.|logger\.|warn\.|error\(" services/platform/convex/agents/file_actions.ts -C 2 | head -40
echo
echo "== Verify what happens when modelId is not in supportedModels (read full function) =="
sed -n '423,470p' services/platform/convex/agents/file_actions.tsRepository: tale-project/tale
Length of output: 1492
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check what toSerializableConfig returns and what default model/fallbackModels are =="
rg -n "toSerializableConfig\|fallbackModels\|config\.model" services/platform/convex/agents/config.ts -B 2 -A 5 | head -80
echo
echo "== Read the config.ts file to understand the agent config structure =="
sed -n '1,100p' services/platform/convex/agents/config.ts
echo
echo "== Check if governance modelId is expected to always be in agent's supportedModels =="
rg -n "governance\|supportedModels" services/platform/convex/governance/internal_queries.ts -B 3 -A 3 | head -60Repository: tale-project/tale
Length of output: 148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Read config.ts to understand toSerializableConfig =="
wc -l services/platform/convex/agents/config.ts
echo
echo "== Show first 150 lines of config.ts =="
head -150 services/platform/convex/agents/config.ts
echo
echo "== Show lines around toSerializableConfig function =="
rg -n "toSerializableConfig" services/platform/convex/agents/config.ts
echo
echo "== Read that function =="
sed -n '30,100p' services/platform/convex/agents/config.tsRepository: tale-project/tale
Length of output: 3460
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check full governance resolveDefaultModelInternal to see if it validates against agent =="
sed -n '92,150p' services/platform/convex/governance/internal_queries.ts
echo
echo "== Check if there's any cross-check between governance and agent supportedModels =="
rg -n "governance\|supportedModels" services/platform/convex --type ts | grep -i "governance\|agent" | head -20
echo
echo "== Look at the complete resolveDefaultModelInternal function =="
wc -l services/platform/convex/governance/internal_queries.ts
echo
echo "== Read the entire governance internal_queries.ts =="
cat services/platform/convex/governance/internal_queries.tsRepository: tale-project/tale
Length of output: 1233
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Read the complete governance internal_queries.ts file =="
cat services/platform/convex/governance/internal_queries.ts
echo
echo "== Search for any tests or documentation about governance model defaults =="
find services/platform/convex -type f \( -name "*.test.ts" -o -name "*.spec.ts" -o -name "*.md" \) | xargs grep -l "governance\|resolveDefaultModel" 2>/dev/null | head -5
echo
echo "== Check if there are any comments or design docs about the fallback behavior =="
rg -n "unsupported|fallback|governance.*model" services/platform/convex/agents -B 2 -A 2 | head -40Repository: tale-project/tale
Length of output: 5571
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Read the resolve_default_model test file =="
cat services/platform/convex/governance/__tests__/resolve_default_model.test.ts
echo
echo "== Also check the actual resolveDefaultModel implementation =="
fd -i "resolve_default_model.ts" services/platform/convex/governance -type f ! -name "*.test.ts" -exec cat {} \;
echo
echo "== Check if there are any validation errors thrown when unsupported model is used =="
rg -n "throw.*Error\|console\.error\|logger\." services/platform/convex/agents/file_actions.ts -B 2 -A 2Repository: tale-project/tale
Length of output: 6327
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find and read resolve_default_model.ts implementation =="
find services/platform/convex/governance -name "resolve_default_model.ts" -type f ! -name "*.test.ts" | head -1 | xargs cat
echo
echo "== Summary: Check what happens when governance model is not in agent supportedModels =="
echo "Scenario: governance returns modelId X, but agent only supports [Y, Z]"
echo "Location: file_actions.ts:460 - if (args.modelId && result.config.supportedModels.includes(args.modelId))"
echo "Outcome: condition fails, model override is skipped, fallback logic continues"
echo "Question: Is this documented behavior or a bug?"Repository: tale-project/tale
Length of output: 2192
Validate governance model against agent supportedModels or log when falling back.
Line 99–110 resolves a governance default modelId without verifying it's in the agent's supported list. When passed to resolveAgentConfig (line 119), an unsupported governance modelId is silently ignored (line 460: condition fails, no override applied). Either validate effectiveModelId against the agent's supportedModels before calling resolveAgentConfig, or add logging/error handling when a governance default is dropped due to agent incompatibility.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/agents/unified_chat.ts` around lines 96 - 111, When
you set effectiveModelId from the governance default (from ctx.runQuery using
internal.governance.internal_queries.resolveDefaultModelInternal) validate that
effectiveModelId is actually supported by the selected agent (check against
agent.supportedModels) before passing it into resolveAgentConfig; if it isn’t
supported, either clear effectiveModelId so resolveAgentConfig won’t silently
drop it, or emit a warning via the logger indicating the governance default
modelId was incompatible with the agent’s supportedModels and will be ignored.
Ensure you reference the variables effectiveModelId and the agent object used by
resolveAgentConfig so the validation/logging happens immediately after resolving
the governance default and before calling resolveAgentConfig.
Comment on lines
+31
to
+32
| const authUser = await authComponent.getAuthUser(ctx); | ||
| if (!authUser) return null; |
Contributor
There was a problem hiding this comment.
Treat auth lookup failures as unauthenticated.
If authComponent.getAuthUser(ctx) throws here, this query will error and break the chat-side default-model lookup instead of cleanly returning null.
Suggested fix
handler: async (ctx, args) => {
- const authUser = await authComponent.getAuthUser(ctx);
+ let authUser = null;
+ try {
+ authUser = await authComponent.getAuthUser(ctx);
+ } catch {}
if (!authUser) return null;Based on learnings: “In Convex query handlers, if authComponent.getAuthUser(ctx) fails, swallow the error (empty catch) and treat as unauthenticated without logging warnings/errors.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/governance/default_model_query.ts` around lines 31 -
32, Wrap the call to authComponent.getAuthUser(ctx) in a try/catch and treat any
thrown error as an unauthenticated result: call authComponent.getAuthUser(ctx)
inside a try, assign to authUser if it succeeds, and in the catch set authUser =
null (no logging), then keep the existing check (if (!authUser) return null) so
failures are swallowed and the query returns null instead of throwing; update
the logic around the authUser variable in default_model_query.ts accordingly.
Comment on lines
+107
to
+120
| const member = await getOrganizationMember(ctx, args.organizationId, { | ||
| userId: args.userId, | ||
| email: args.userEmail, | ||
| name: args.userName, | ||
| }); | ||
|
|
||
| const membershipsResult: BetterAuthFindManyResult<BetterAuthTeamMember> = | ||
| await ctx.runQuery(components.betterAuth.adapter.findMany, { | ||
| model: 'teamMember', | ||
| paginationOpts: { cursor: null, numItems: 100 }, | ||
| where: [{ field: 'userId', operator: 'eq', value: args.userId }], | ||
| }); | ||
|
|
||
| const teamIds = membershipsResult?.page.map((m) => m.teamId) ?? []; |
Contributor
There was a problem hiding this comment.
Resolve team memberships from the normalized member identity, not the raw auth ID.
getOrganizationMember(...) already handles the “JWT userId drifted from Better Auth” case via email fallback, but this lookup immediately reuses args.userId for teamMember rows. That means migrated/linked accounts can resolve the org member successfully and still miss every team-scoped default here. The fixed numItems: 100 cap has the same effect for memberships beyond the first page. Query memberships with the resolved Better Auth user id from member, and paginate until done (or at least align with the repo’s usual 1000-item ceiling).
Based on learnings: “In services/platform/convex/team_members.ts, the 1000-item pagination limit is sufficient for typical team sizes.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/governance/internal_queries.ts` around lines 107 -
120, The memberships query is using the raw JWT args.userId and a hard cap of
100 items, which misses migrated/linked accounts and extra pages; change the
findMany call in this flow to query by the resolved Better Auth id from
member.userId (the object returned by getOrganizationMember) instead of
args.userId, and implement pagination (loop/accumulate) using
components.betterAuth.adapter.findMany until page is exhausted, increasing
paginationOpts.numItems to the repo-standard 1000 to collect all teamMember
rows, then derive teamIds from the accumulated results.
Comment on lines
1451
to
1453
| "title": "Providers", | ||
| "description": "Manage LLM providers and their models", | ||
| "description": "Description", | ||
| "addProvider": "Add provider", |
Contributor
There was a problem hiding this comment.
Restore the Providers page description copy.
settings.providers.description sits next to the page title, but it now resolves to the literal word "Description". The field label already lives under settings.providers.description_field, so this looks like the wrong key was changed and will render a broken subtitle on the Providers page.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/messages/en.json` around lines 1451 - 1453, The value for
settings.providers.description was accidentally set to the literal "Description"
which breaks the Providers page subtitle; revert or replace that string in the
en.json entry for "settings.providers.description" with the intended subtitle
copy (either restore the original text from the previous commit or copy the
matching value from another locale) so it reads the proper descriptive sentence
(not the field label which lives at settings.providers.description_field).
- Add Zod validation for default_models policy type in upsertPolicy - Create getMyDefaultModel reactive query for client consumption - Add resolveDefaultModelInternal internal query for server-side use - Apply governance default in unified_chat when no explicit model sent - Insert governance default in model selector priority chain - Pass governance default through chat interface send path - Add DefaultModelEditor admin UI with rule CRUD, scope/team/role pickers - Add Default Models tab to governance settings page - Add i18n keys to all four locale files (en, de, de-CH, de-AT) - Add backend tests for resolveDefaultModel resolution logic - Add accessibility test for DefaultModelEditor component - Fix existing model-selector test to mock new useDefaultModel hook
larryro
force-pushed
the
feat/issue-1168-default-model
branch
from
6fbc878 to
36cf65b
Compare
…nance default is active Selecting the agent's first model cleared the user override, causing the governance default to take over. Now the override is only cleared when the user picks the effective default (governance or agent primary).
This was referenced Apr 12, 2026
This was referenced Apr 20, 2026
4 tasks
11 tasks
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
2 participants
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.
Summary
default_modelsgovernance policy end-to-end: backend validation, reactive query, server-side enforcement inunified_chat, client-side consumption in model selector and chat interface, and a new admin UI editor on the governance settings page.DefaultModelEditorcomponent (followingBudgetEditorpattern) with rule CRUD, scope/team/role pickers, provider/model selectors, and enable/disable toggle.resolveDefaultModeland accessibility tests for the new editor component.Test plan
resolveDefaultModelpriority logic (team > role > default), disabled policy, empty rules, multi-team membershipDefaultModelEditorModelSelectortest updated and passes with newuseDefaultModelmockCloses #1168
Summary by CodeRabbit
Release Notes
New Features
Localization
Tests