fix: address review feedback on model roles

Author: elkaix

apps/pythinker-code/src/tui/commands/config.ts

@@ -482,7 +482,7 @@ function resolveWorkspaceConfigPath(input: string, workDir: string): string {
 
 export async function handleModelCommand(host: SlashCommandHost, args: string): Promise<void> {
   const requestedAlias = args.trim();
-  const tokens = requestedAlias.split(/\s+/).filter(Boolean);
+  const tokens = requestedAlias.split(/\s+/u).filter(Boolean);
   const config = await host.harness.getConfig({ reload: true });
   const roles = [...new Set([...BUILT_IN_MODEL_ROLES, ...Object.keys(config.modelRoles ?? {})])]
     .filter((role) => role.length > 0 && role !== 'default');
@@ -505,7 +505,12 @@ export async function handleModelCommand(host: SlashCommandHost, args: string):
       return;
     }
     if (tokens.length === 1) {
-      showModelPicker(host, config.modelRoles?.[role], undefined, { assignToRole: role });
+      const picker = showModelPicker(host, config.modelRoles?.[role], undefined, {
+        assignToRole: role,
+      });
+      if (picker !== undefined) {
+        void refreshModelsForOpenPicker(host, picker, config.modelRoles?.[role]);
+      }
       return;
     }
   }

apps/pythinker-code/test/tui/commands/model-roles.test.ts

@@ -81,6 +81,7 @@ describe('/model roles', () => {
     const { host, session, setConfig } = makeHost();
 
     await handleModelCommand(host, 'small');
+    expect(host.authFlow.refreshProviderModels).toHaveBeenCalledOnce();
     mountedPicker(host).handleInput(ENTER);
 
     await vi.waitFor(() => {

docs/configuration/config-files.md

@@ -158,7 +158,7 @@ You can also switch models temporarily without touching the config file — by s
 
 ## `model_roles`
 
-Each entry in the `model_roles` table locks a model alias to a named role. The built-in roles are `small`, `implementer`, and `advisor`; any other key defines a custom role. Values must be aliases defined in `models`; an empty string clears the role.
+Each entry in the `model_roles` table locks a model alias to a named role. The built-in roles are `small`, `implementer`, and `advisor`; any other key except the reserved `default` defines a custom role. Values must be aliases defined in `models`; an empty string clears the role.
 
 ```toml
 [model_roles]
@@ -169,7 +169,7 @@ advisor = "reviewer-model"
 
 Roles take effect in two places:
 
-- Wherever a subagent model alias is accepted (the `Agent` and `DynamicWorkflow` tool `model` arguments, and agent profile frontmatter), a `@<role>` reference such as `@small` resolves to the locked alias. An unassigned or unresolvable role falls back to the normal model precedence.
+- Wherever a subagent model alias is accepted (the `Agent` and `DynamicWorkflow` tool `model` arguments, and agent profile frontmatter), a `@<role>` reference such as `@small` resolves to the locked alias. An unassigned or unresolvable role falls back to the parent agent's model.
 - When `implementer` is assigned, it becomes the default model for subagents that do not set an explicit or profile model. Subagents of those subagents inherit the same default.
 
 Inside the TUI, `/model <role>` assigns a role from the model picker, `/model <role> clear` removes it, and `/model roles` lists the current assignments. See [Slash commands](../reference/slash-commands.md).

packages/agent-core/src/config/schema.ts

@@ -270,11 +270,16 @@ export const McpServerConfigSchema = z.preprocess((raw) => {
 
 export type McpServerConfig = z.infer<typeof McpServerConfigSchema>;
 
+const ModelRolesSchema = z.record(z.string(), z.string()).refine(
+  (roles) => !Object.hasOwn(roles, 'default'),
+  { message: '"default" is a reserved model role name' },
+);
+
 export const PythinkerConfigSchema = z.object({
   providers: z.record(z.string(), ProviderConfigSchema).default({}),
   defaultProvider: z.string().optional(),
   defaultModel: z.string().optional(),
-  modelRoles: z.record(z.string(), z.string()).optional(),
+  modelRoles: ModelRolesSchema.optional(),
   outputStyle: z.string().trim().min(1).optional(),
   models: z.record(z.string(), ModelAliasSchema).optional(),
   thinking: ThinkingConfigSchema.optional(),
@@ -320,7 +325,7 @@ export const PythinkerConfigPatchSchema = z
     providers: z.record(z.string(), ProviderConfigPatchSchema).optional(),
     defaultProvider: z.string().optional(),
     defaultModel: z.string().optional(),
-    modelRoles: z.record(z.string(), z.string()).optional(),
+    modelRoles: ModelRolesSchema.optional(),
     outputStyle: z.string().trim().min(1).optional(),
     models: z.record(z.string(), ModelAliasPatchSchema).optional(),
     thinking: ThinkingConfigPatchSchema.optional(),

packages/agent-core/src/tools/builtin/collaboration/dynamic-workflow.ts

@@ -102,7 +102,7 @@ export const DynamicWorkflowToolInputSchema = z
       .min(1)
       .optional()
       .describe(
-        'Model alias for every subagent in this workflow, so the orchestrator can run on one model while the workers run on a cheaper or faster one. References such as @small, @implementer, @advisor, and @<custom-role> resolve through configured model roles and fall back to normal precedence when unassigned. Defaults to the subagent type profile model, then this agent model.',
+        'Model alias for every subagent in this workflow, so the orchestrator can run on one model while the workers run on a cheaper or faster one. References such as @small, @implementer, @advisor, and @<custom-role> resolve through configured model roles and fall back to normal precedence when unassigned. Defaults to the subagent type profile model, then the configured implementer model role, then this agent model.',
       ),
     effort: z
       .string()

packages/agent-core/test/config/model-roles.test.ts

@@ -1,8 +1,24 @@
 import { describe, expect, it } from 'vitest';
 
-import { expandModelRef, resolveModelRoleAlias } from '../../src/config';
+import {
+  expandModelRef,
+  PythinkerConfigPatchSchema,
+  PythinkerConfigSchema,
+  resolveModelRoleAlias,
+} from '../../src/config';
 
 describe('model roles', () => {
+  it('rejects the reserved default role in full and patch configs', () => {
+    expect(
+      PythinkerConfigSchema.safeParse({ modelRoles: { default: 'x' } }).success,
+    ).toBe(false);
+    expect(
+      PythinkerConfigPatchSchema.safeParse({ modelRoles: { default: 'x' } }).success,
+    ).toBe(false);
+    expect(PythinkerConfigSchema.safeParse({ modelRoles: { custom: 'x' } }).success).toBe(true);
+    expect(PythinkerConfigPatchSchema.safeParse({ modelRoles: { custom: 'x' } }).success).toBe(true);
+  });
+
   it('resolves assigned roles and treats empty assignments as cleared', () => {
     const config = {
       modelRoles: {