feat(platform): translator + model gating + chat attachment retrieve#1586
Conversation
…icit paths - Add Translator agent config with reflection pass and i18n (de/fr) - Enforce model_access policy on implicit default/fallback model paths in unified_chat so blocked models can't be reached via governance default or SDK retry; add getAccessibleModelsInternal query - Drop qwen3-vl-32b-instruct from chat-agent supportedModels - Model selector: updates + tests, plus "noModelsAvailable" string (en/de/fr) - Upload policy editor: layout/structure changes
…data Chat-uploaded files are auto-indexed into the RAG service but do not create a documents-hub row, so document_retrieve previously failed with "Document not found" for files users drop into chat. Fall back to a fileMetadata lookup (org-scoped) when the hub miss would otherwise throw, then proceed with the existing RAG content fetch. Surface distinct errors for still-indexing vs indexing-failed states so agents can retry or stop appropriately. Tool description updated to advertise the chat-attachment path and indexing-state semantics.
📝 WalkthroughWalkthroughThis PR implements model-access governance, updates agent configurations, and enhances model selection UI behavior. Changes include: removing an older Qwen model from the chat-agent configuration and adding a new Translator agent; refactoring model-selector component to auto-pin single available models, display "No models available" warnings, and clearing stale model overrides; enhancing document retrieval with a fallback path for chat-uploaded files using file metadata; adding model-access enforcement in unified chat to filter agent models by organizational allowlists; introducing a new internal governance query for accessible models; and adding corresponding i18n translations across three languages. Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 3
🤖 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/model-selector.tsx`:
- Around line 179-195: The selector auto-pins a single model into
selectedModelOverrides inside the useEffect (effectiveAgent?.name,
filteredModels, setSelectedModelOverride, isImageGenAgent) but never clears that
synthetic auto-pick when filteredModels later expands, so the UI/backend remain
stuck; to fix, record that the override was auto-pinned (e.g., maintain a small
autoPinned map keyed by effectiveAgent.name when you call
setSelectedModelOverride during the single-model branch) and then in the same
useEffect clear that auto-pinned override (call
setSelectedModelOverride(effectiveAgent.name, null)) whenever
filteredModels.length > 1 and the current override exists in
selectedModelOverrides and is marked as auto-pinned; update the autoPinned map
when you explicitly clear or when the user manually changes the override so user
choices are preserved.
In
`@services/platform/app/features/settings/governance/components/upload-policy-editor.tsx`:
- Around line 127-156: The toggle is updated optimistically in
handleToggleEnabled by calling setEnabled(checked) before saveConfig, but
saveConfig swallows errors so a failed upsert (upsertMutation.mutateAsync)
leaves the UI inconsistent; change handleToggleEnabled to save first and only
setEnabled on success (or revert to previous value on failure) — call
saveConfig(buildConfig(checked)) and, if it throws, call
setEnabled(previousValue) (or setEnabled(!checked)) and surface the error (or
rethrow) so the toast/error path can run; reference functions:
handleToggleEnabled, saveConfig, setEnabled, upsertMutation.mutateAsync.
In `@services/platform/convex/agents/unified_chat.ts`:
- Around line 143-162: The test harness must stub the new governance query so
implicit-model requests don't fail; in the TTFT test update the stub for
internal.governance.internal_queries.getAccessibleModelsInternal (used by
unified_chat's runQuery) to return the model IDs corresponding to the agent's
configResult.supportedModels (use stripModelRefQualifier if necessary) so
accessiblePlain contains the supported model IDs and accessibleRefs is
non-empty; ensure the test's Convex/mocking setup intercepts runQuery for
getAccessibleModelsInternal and returns that list before invoking the unified
chat flow that may call
internal.threads.internal_mutations.clearGenerationStatus.
🪄 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: 2bf7dd10-78e0-411b-b33a-a1b8fa74798e
📒 Files selected for processing (13)
examples/agents/chat-agent.jsonexamples/agents/translator.jsonservices/platform/app/features/chat/components/__tests__/model-selector.test.tsxservices/platform/app/features/chat/components/model-selector.tsxservices/platform/app/features/settings/governance/components/upload-policy-editor.tsxservices/platform/convex/agent_tools/documents/__tests__/document_retrieve_tool.test.tsservices/platform/convex/agent_tools/documents/document_retrieve_tool.tsservices/platform/convex/agent_tools/documents/helpers/retrieve_document.tsservices/platform/convex/agents/unified_chat.tsservices/platform/convex/governance/internal_queries.tsservices/platform/messages/de.jsonservices/platform/messages/en.jsonservices/platform/messages/fr.json
| useEffect(() => { | ||
| if (!effectiveAgent?.name) return; | ||
| const override = selectedModelOverrides[effectiveAgent.name]; | ||
| if (override && !filteredModels.includes(override)) { | ||
| setSelectedModelOverride(effectiveAgent.name, null); | ||
| return; | ||
| } | ||
| if (!override && !isImageGenAgent && filteredModels.length === 1) { | ||
| setSelectedModelOverride(effectiveAgent.name, filteredModels[0]); | ||
| } | ||
| }, [ | ||
| effectiveAgent?.name, | ||
| filteredModels, | ||
| selectedModelOverrides, | ||
| setSelectedModelOverride, | ||
| isImageGenAgent, | ||
| ]); |
There was a problem hiding this comment.
Auto-pinned single-model overrides stay stuck after the choice set widens.
Lines 186-187 persist the sole model into selectedModelOverrides, and chat-layout-context.tsx keeps that value for 24 hours. When governance/provider data later exposes multiple models again, nothing clears this synthetic override, so the selector stays pinned to the old model and the backend no longer uses Auto/fallback resolution until the user manually switches back.
🤖 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 179 - 195, The selector auto-pins a single model into
selectedModelOverrides inside the useEffect (effectiveAgent?.name,
filteredModels, setSelectedModelOverride, isImageGenAgent) but never clears that
synthetic auto-pick when filteredModels later expands, so the UI/backend remain
stuck; to fix, record that the override was auto-pinned (e.g., maintain a small
autoPinned map keyed by effectiveAgent.name when you call
setSelectedModelOverride during the single-model branch) and then in the same
useEffect clear that auto-pinned override (call
setSelectedModelOverride(effectiveAgent.name, null)) whenever
filteredModels.length > 1 and the current override exists in
selectedModelOverrides and is marked as auto-pinned; update the autoPinned map
when you explicitly clear or when the user manually changes the override so user
choices are preserved.
| const saveConfig = useCallback( | ||
| async (config: UploadPolicyConfig) => { | ||
| try { | ||
| await upsertMutation.mutateAsync({ | ||
| organizationId, | ||
| policyType: 'upload_policy', | ||
| config, | ||
| }); | ||
| toast({ title: t('uploadPolicy.saved'), variant: 'success' }); | ||
| } catch { | ||
| toast({ | ||
| title: t('uploadPolicy.saveFailed'), | ||
| variant: 'destructive', | ||
| }); | ||
| } | ||
| }, | ||
| [organizationId, upsertMutation, toast, t], | ||
| ); | ||
|
|
||
| const handleSave = useCallback(async () => { | ||
| try { | ||
| await upsertMutation.mutateAsync({ | ||
| organizationId, | ||
| policyType: 'upload_policy', | ||
| config: buildConfig(), | ||
| }); | ||
| toast({ title: t('uploadPolicy.saved'), variant: 'success' }); | ||
| } catch { | ||
| toast({ | ||
| title: t('uploadPolicy.saveFailed'), | ||
| variant: 'destructive', | ||
| }); | ||
| } | ||
| }, [organizationId, buildConfig, upsertMutation, toast, t]); | ||
| await saveConfig(buildConfig(enabled)); | ||
| }, [saveConfig, buildConfig, enabled]); | ||
|
|
||
| const handleToggleEnabled = useCallback((checked: boolean) => { | ||
| setEnabled(checked); | ||
| }, []); | ||
| const handleToggleEnabled = useCallback( | ||
| (checked: boolean) => { | ||
| setEnabled(checked); | ||
| void saveConfig(buildConfig(checked)); | ||
| }, | ||
| [saveConfig, buildConfig], | ||
| ); |
There was a problem hiding this comment.
Revert the toggle when the auto-save fails.
Line 152 updates enabled optimistically, but saveConfig swallows the mutation error and only shows a toast. If the write fails, the switch stays in the new position even though the persisted policy never changed.
Proposed fix
const saveConfig = useCallback(
async (config: UploadPolicyConfig) => {
try {
await upsertMutation.mutateAsync({
organizationId,
policyType: 'upload_policy',
config,
});
toast({ title: t('uploadPolicy.saved'), variant: 'success' });
+ return true;
} catch {
toast({
title: t('uploadPolicy.saveFailed'),
variant: 'destructive',
});
+ return false;
}
},
[organizationId, upsertMutation, toast, t],
);
@@
const handleToggleEnabled = useCallback(
- (checked: boolean) => {
+ async (checked: boolean) => {
+ const previousEnabled = enabled;
setEnabled(checked);
- void saveConfig(buildConfig(checked));
+ const saved = await saveConfig(buildConfig(checked));
+ if (!saved) {
+ setEnabled(previousEnabled);
+ }
},
- [saveConfig, buildConfig],
+ [enabled, saveConfig, buildConfig],
);🤖 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/upload-policy-editor.tsx`
around lines 127 - 156, The toggle is updated optimistically in
handleToggleEnabled by calling setEnabled(checked) before saveConfig, but
saveConfig swallows errors so a failed upsert (upsertMutation.mutateAsync)
leaves the UI inconsistent; change handleToggleEnabled to save first and only
setEnabled on success (or revert to previous value on failure) — call
saveConfig(buildConfig(checked)) and, if it throws, call
setEnabled(previousValue) (or setEnabled(!checked)) and surface the error (or
rethrow) so the toast/error path can run; reference functions:
handleToggleEnabled, saveConfig, setEnabled, upsertMutation.mutateAsync.
| const accessiblePlain = await ctx.runQuery( | ||
| internal.governance.internal_queries.getAccessibleModelsInternal, | ||
| { | ||
| organizationId: args.organizationId, | ||
| userId: authUserId, | ||
| modelIds: configResult.supportedModels.map(stripModelRefQualifier), | ||
| }, | ||
| ); | ||
| const accessibleSet = new Set(accessiblePlain); | ||
| const accessibleRefs = configResult.supportedModels.filter((ref) => | ||
| accessibleSet.has(stripModelRefQualifier(ref)), | ||
| ); | ||
| if (accessibleRefs.length === 0) { | ||
| await ctx.runMutation( | ||
| internal.threads.internal_mutations.clearGenerationStatus, | ||
| { threadId: args.threadId, streamId: preAllocatedStreamId }, | ||
| ); | ||
| throw new Error( | ||
| "No model in this agent is permitted by your organization's model access policy.", | ||
| ); |
There was a problem hiding this comment.
Keep the TTFT test harness in sync with this new query.
This branch makes getAccessibleModelsInternal mandatory for every implicit-model request, but convex/agents/__tests__/unified_chat_ttft.test.ts still doesn't stub it. CI is already failing here with No model in this agent is permitted... before the TTFT assertions run, so the new governance dependency needs the matching test fixture update before merge.
🧰 Tools
🪛 GitHub Check: Unit
[failure] 160-160: [server] convex/agents/tests/unified_chat_ttft.test.ts > chatWithAgent — TTFT parallelization > eliminates resolveAgentConfig action hop by reading files directly
Error: No model in this agent is permitted by your organization's model access policy.
❯ handler convex/agents/unified_chat.ts:160:15
❯ convex/agents/tests/unified_chat_ttft.test.ts:272:5
[failure] 160-160: [server] convex/agents/tests/unified_chat_ttft.test.ts > chatWithAgent — TTFT parallelization > still scrubs PII before startChat when PII is enabled
Error: No model in this agent is permitted by your organization's model access policy.
❯ handler convex/agents/unified_chat.ts:160:15
❯ convex/agents/tests/unified_chat_ttft.test.ts:250:5
[failure] 160-160: [server] convex/agents/tests/unified_chat_ttft.test.ts > chatWithAgent — TTFT parallelization > calls PII query and agent config resolution concurrently
Error: No model in this agent is permitted by your organization's model access policy.
❯ handler convex/agents/unified_chat.ts:160:15
❯ convex/agents/tests/unified_chat_ttft.test.ts:204:5
🤖 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 143 - 162, The
test harness must stub the new governance query so implicit-model requests don't
fail; in the TTFT test update the stub for
internal.governance.internal_queries.getAccessibleModelsInternal (used by
unified_chat's runQuery) to return the model IDs corresponding to the agent's
configResult.supportedModels (use stripModelRefQualifier if necessary) so
accessiblePlain contains the supported model IDs and accessibleRefs is
non-empty; ensure the test's Convex/mocking setup intercepts runQuery for
getAccessibleModelsInternal and returns that list before invoking the unified
chat flow that may call
internal.threads.internal_mutations.clearGenerationStatus.
… tests The model-access enforcement added in 443aecd calls a new internal query on implicit-model paths; the TTFT test's runQuery mock returned null for unrecognized names, producing an empty accessible set and throwing. Echo back the requested modelIds so every supported model is treated as accessible.
1 participant
Summary
Two commits on a WIP branch that together cover three independent improvements:
qwen3-vl-32b-instructfrom chat-agent supported models.unified_chatnow applies themodel_accessgovernance policy to default/fallback model resolution (previously only the user-selected path was checked), so a blocked model can't leak through a governance default or an SDK retry. AddsgetAccessibleModelsInternalquery and a correspondingnoModelsAvailablestring (en/de/fr). Model selector gains tests and an empty-state. Minor upload-policy-editor layout refactor.document_retrievereads chat attachments — chat-uploaded files are auto-indexed into RAG but never create adocuments-hub row, so the existing tool threw "Document not found" on them. Added afileMetadata-based fallback (org-scoped) with distinct errors for still-indexing vs indexing-failed states. Tool description updated to advertise the chat-attachment path; 7 new unit tests covering the fallback branches.Test plan
document_retrievesucceeds via the fileMetadata fallback (content comes back, not "Document not found").document_retrieve; team ACL behavior unchanged.model_accesspolicy blocking a model, confirm that model does NOT appear via default fallback or SDK retry paths inunified_chat; the model selector shows the empty-state string when no models are accessible.npx tsc --noEmit,npm run lint --workspace=@tale/platform, and thedocument_retrieve_toolvitest suite all green.Summary by CodeRabbit
New Features
Localization
Bug Fixes