diff --git a/package.json b/package.json index cff8ee1..2f0e188 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ "build": "tsc && node scripts/copy-plugin-assets.mjs", "dev": "tsc --watch", "start": "node dist/index.js", - "test": "npm run build && node --test --test-reporter=spec test/local.mjs test/skills.local.mjs test/repair.mjs test/exa.local.mjs test/audit-batch.local.mjs", + "test": "npm run build && node --test --test-reporter=spec test/local.mjs test/skills.local.mjs test/repair.mjs test/model-resolver.mjs test/exa.local.mjs test/audit-batch.local.mjs", "test:e2e": "npm run build && node --test --test-reporter=spec test/e2e.mjs", "test:free-models": "npm run build && node --test --test-reporter=spec test/free-model-matrix.mjs", "test:all": "npm run test && npm run test:e2e", diff --git a/src/agent/llm.ts b/src/agent/llm.ts index 26756e5..847bde6 100644 --- a/src/agent/llm.ts +++ b/src/agent/llm.ts @@ -390,21 +390,47 @@ export function classifyToolCallFailure( export function isRoleplayedJsonToolCallText(text: string): boolean { const trimmed = text.trim(); - if (!trimmed.startsWith('{') || !trimmed.endsWith('}')) return false; + if (!trimmed) return false; + + // Cheap pre-check: every shape we accept either starts with `{` or begins + // with a known XML wrapper. Avoid JSON.parse on long prose. + const looksJson = trimmed.startsWith('{') && trimmed.endsWith('}'); + const looksXml = /^/i.test(trimmed); + if (!looksJson && !looksXml) return false; + + if (looksXml) { + // Match `{...}`, including the multi-call variant. + return /[\s\S]*?<\/tool_call(s)?>/i.test(trimmed); + } try { - const parsed = JSON.parse(trimmed) as Record; - return ( - parsed !== null && - typeof parsed === 'object' && - !Array.isArray(parsed) && - parsed.type === 'function' && - typeof parsed.name === 'string' && - ('parameters' in parsed || 'arguments' in parsed) - ); + const parsed = JSON.parse(trimmed) as unknown; + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) return false; + const obj = parsed as Record; + + // OpenAI envelope: assistant message that forgot to use the tool_calls + // channel and dumped the whole message JSON into the text stream. + if (Array.isArray(obj.tool_calls)) return true; + + // Hermes/Claude roleplay envelope (`type: "function"` + name + arguments). + if ( + obj.type === 'function' && + typeof obj.name === 'string' && + ('parameters' in obj || 'arguments' in obj) + ) { + return true; + } + + // Bare Hermes envelope (no `type`): what nvidia/llama-4-maverick, + // nvidia/qwen3-next, and similar small models emit. The matching + // `name` + `parameters|arguments` pair is enough for the scavenger. + if (typeof obj.name === 'string' && ('parameters' in obj || 'arguments' in obj)) { + return true; + } } catch { - return false; + // Fall through to false; not roleplayed tool-call JSON. } + return false; } /** @@ -985,9 +1011,16 @@ export class ModelClient { const trimmed = currentText.trimStart(); if (!trimmed) return; - // Nemotron Omni leaks reasoning prose into the text channel without - // tags. Hold the buffer for end-of-stream stripping. - textEmission.mode = isNemotronProse || trimmed.startsWith('{') ? 'hold' : 'stream'; + // Hold the buffer when the model is roleplaying a tool call as text + // (free-tier llama/qwen/nemotron variants frequently do this) so + // the loop's scavenger can recover it instead of streaming junk + // to the user. Three shapes we recognize: + // 1. {…} roleplay JSON envelope + // 2. … XML + // 3. Nemotron Omni prose (handled separately below) + const looksLikeRoleplayJson = trimmed.startsWith('{'); + const looksLikeRoleplayXml = /^/i.test(trimmed); + textEmission.mode = isNemotronProse || looksLikeRoleplayJson || looksLikeRoleplayXml ? 'hold' : 'stream'; if (textEmission.mode === 'stream') { onStreamDelta?.({ type: 'text', text: currentText }); } diff --git a/src/commands/start.ts b/src/commands/start.ts index 209914f..a998aa6 100644 --- a/src/commands/start.ts +++ b/src/commands/start.ts @@ -13,7 +13,7 @@ import { interactiveSession } from '../agent/loop.js'; import { allCapabilities, createSubAgentCapability } from '../tools/index.js'; import { validateToolDescriptions } from '../tools/validate.js'; import { launchInkUI } from '../ui/app.js'; -import { pickModel, resolveModel } from '../ui/model-picker.js'; +import { pickModel, resolveModel, resolveModelStrict } from '../ui/model-picker.js'; import { loadMcpConfig } from '../mcp/config.js'; import { connectMcpServers, disconnectMcpServers, getMcpServerInstructions } from '../mcp/client.js'; import { ensureCodegraphIndex } from '../mcp/codegraph.js'; @@ -692,9 +692,16 @@ async function runWithBasicUI( continue; } if (input.startsWith('/model ')) { - const newModel = resolveModel(input.slice(7).trim()); - agentConfig.model = newModel; - console.error(chalk.green(` Model → ${newModel}`)); + const resolved = resolveModelStrict(input.slice(7).trim()); + if (!resolved.ok) { + console.error(chalk.yellow(` ${resolved.suggestion}`)); + continue; + } + agentConfig.model = resolved.id; + console.error(chalk.green(` Model → ${resolved.id}`)); + if (resolved.viaShortcut) { + console.error(chalk.dim(` (alias ${input.slice(7).trim()} → ${resolved.id})`)); + } continue; } // /retry — resend last prompt @@ -837,9 +844,18 @@ async function handleSlashCommand( case '/model': { const newModel = parts[1]; if (newModel) { - config.model = resolveModel(newModel); - config.baseModel = config.model; - console.error(chalk.green(` Model → ${config.model}`)); + const resolved = resolveModelStrict(newModel); + if (!resolved.ok) { + console.error(chalk.yellow(` ${resolved.suggestion}`)); + return null; + } + config.model = resolved.id; + config.baseModel = resolved.id; + const suffix = resolved.viaShortcut + ? chalk.dim(` (alias ${newModel} → ${resolved.id})`) + : ''; + console.error(chalk.green(` Model → ${resolved.id}`)); + if (suffix) console.error(suffix); return null; } const picked = await pickModel(config.model); diff --git a/src/ui/model-picker.ts b/src/ui/model-picker.ts index e4347c4..8af76d6 100644 --- a/src/ui/model-picker.ts +++ b/src/ui/model-picker.ts @@ -86,6 +86,9 @@ export const MODEL_SHORTCUTS: Record = { 'qwen-coder': 'nvidia/llama-4-maverick', 'qwen-think': 'nvidia/llama-4-maverick', maverick: 'nvidia/llama-4-maverick', + llama: 'nvidia/llama-4-maverick', + 'llama-4': 'nvidia/llama-4-maverick', + 'llama-4-maverick': 'nvidia/llama-4-maverick', 'gpt-oss': 'nvidia/llama-4-maverick', 'gpt-oss-small': 'nvidia/llama-4-maverick', 'mistral-small': 'nvidia/llama-4-maverick', @@ -120,11 +123,54 @@ export const MODEL_SHORTCUTS: Record = { }; /** - * Resolve a model name — supports shortcuts. + * Resolve a model name — supports shortcuts. Returns the canonical model id. + * + * If the input matches a shortcut, the shortcut's target is returned. If the + * input is already a fully-qualified `provider/model` id (contains a `/`), it + * is returned verbatim so the gateway can validate it. Bare, unknown aliases + * (e.g. `llama3`, `foo`) resolve to themselves too, but the gateway will + * reject them — callers that care about a clean error should branch on + * {@link resolveModelStrict} instead. */ export function resolveModel(input: string): string { - const lower = input.trim().toLowerCase(); - return MODEL_SHORTCUTS[lower] || input.trim(); + const trimmed = input.trim(); + if (!trimmed) return trimmed; + const lower = trimmed.toLowerCase(); + return MODEL_SHORTCUTS[lower] || trimmed; +} + +/** + * Strict variant of {@link resolveModel} — used by the `/model` handler so + * an unknown bare alias surfaces a clean error in the UI instead of + * forwarding `llama` to the gateway and getting back `HTTP 400: Unknown + * model: llama` two turns later. + * + * Recognised: + * - Any entry in {@link MODEL_SHORTCUTS} (case-insensitive). + * - Any id of the form `provider/model` (e.g. `anthropic/claude-sonnet-4.6`). + */ +export function resolveModelStrict( + input: string, +): { ok: true; id: string; viaShortcut: boolean } | { ok: false; suggestion: string } { + const trimmed = input.trim(); + if (!trimmed) { + return { ok: false, suggestion: 'Empty model name. Try /model sonnet or /model free.' }; + } + const lower = trimmed.toLowerCase(); + if (Object.prototype.hasOwnProperty.call(MODEL_SHORTCUTS, lower)) { + return { ok: true, id: MODEL_SHORTCUTS[lower]!, viaShortcut: true }; + } + if (trimmed.includes('/')) { + return { ok: true, id: trimmed, viaShortcut: false }; + } + const known = Object.keys(MODEL_SHORTCUTS).sort(); + const head = known.slice(0, 6).join(', '); + return { + ok: false, + suggestion: + `Unknown model alias: "${trimmed}". Use a shortcut like ${head}, ` + + `or a full id like anthropic/claude-sonnet-4.6.`, + }; } // ─── Curated Model List for Picker ───────────────────────────────────────── diff --git a/test/model-resolver.mjs b/test/model-resolver.mjs new file mode 100644 index 0000000..251c585 --- /dev/null +++ b/test/model-resolver.mjs @@ -0,0 +1,79 @@ +/** + * Unit tests for the model resolver used by the `/model` handler and the + * start command shortcut path. Pure functions, no LLM calls, no fs. + * + * Built into dist/ui/ by `tsc`. + */ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; + +const { resolveModel, resolveModelStrict } = await import('../dist/ui/model-picker.js'); + +test('resolveModel: shortcut maps to canonical id', () => { + assert.equal(resolveModel('sonnet'), 'anthropic/claude-sonnet-4.6'); + assert.equal(resolveModel(' SONNET '), 'anthropic/claude-sonnet-4.6'); + assert.equal(resolveModel('free'), 'nvidia/llama-4-maverick'); + assert.equal(resolveModel('llama'), 'nvidia/llama-4-maverick'); + assert.equal(resolveModel('llama-4-maverick'), 'nvidia/llama-4-maverick'); +}); + +test('resolveModel: full provider/model id is passed through', () => { + assert.equal(resolveModel('anthropic/claude-sonnet-4.6'), 'anthropic/claude-sonnet-4.6'); + assert.equal(resolveModel('openai/gpt-5.5'), 'openai/gpt-5.5'); + assert.equal(resolveModel('moonshot/kimi-k2.7'), 'moonshot/kimi-k2.7'); +}); + +test('resolveModel: unknown bare alias is returned verbatim (caller decides)', () => { + // The legacy soft resolver keeps the input so callers like the + // gateway-error path can still print the bad id verbatim. The strict + // resolver below is the one the `/model` handler uses to reject. + assert.equal(resolveModel('foo'), 'foo'); + assert.equal(resolveModel('llama3'), 'llama3'); + assert.equal(resolveModel(''), ''); +}); + +test('resolveModelStrict: shortcut maps to canonical id and flags viaShortcut', () => { + const r = resolveModelStrict('llama'); + assert.equal(r.ok, true); + if (r.ok) { + assert.equal(r.id, 'nvidia/llama-4-maverick'); + assert.equal(r.viaShortcut, true); + } +}); + +test('resolveModelStrict: full id is accepted but not flagged as shortcut', () => { + const r = resolveModelStrict('anthropic/claude-sonnet-4.6'); + assert.equal(r.ok, true); + if (r.ok) { + assert.equal(r.id, 'anthropic/claude-sonnet-4.6'); + assert.equal(r.viaShortcut, false); + } +}); + +test('resolveModelStrict: unknown bare alias is rejected with a helpful suggestion', () => { + const r = resolveModelStrict('foo'); + assert.equal(r.ok, false); + if (!r.ok) { + assert.match(r.suggestion, /Unknown model alias: "foo"/); + assert.match(r.suggestion, /anthropic\/claude-sonnet-4\.6/); + } + + const r2 = resolveModelStrict('llama3'); + assert.equal(r2.ok, false); + if (!r2.ok) { + assert.match(r2.suggestion, /Unknown model alias: "llama3"/); + } +}); + +test('resolveModelStrict: empty input is rejected', () => { + const r = resolveModelStrict(''); + assert.equal(r.ok, false); +}); + +test('resolveModelStrict: case-insensitive shortcut lookup', () => { + const r = resolveModelStrict(' FrEe '); + assert.equal(r.ok, true); + if (r.ok) { + assert.equal(r.id, 'nvidia/llama-4-maverick'); + } +}); \ No newline at end of file diff --git a/test/repair.mjs b/test/repair.mjs index d3e3760..8faba40 100644 --- a/test/repair.mjs +++ b/test/repair.mjs @@ -325,3 +325,42 @@ test('ToolCallRepair: scans both reasoning and content channels', () => { assert.equal(calls.length, 2); assert.equal(report.scavenged, 2); }); + +// ─── isRoleplayedJsonToolCallText detector (loop gating) ───────────────── + +const { isRoleplayedJsonToolCallText } = await import('../dist/agent/llm.js'); + +test('isRoleplayedJsonToolCallText: rejects empty / non-JSON prose', () => { + assert.equal(isRoleplayedJsonToolCallText(''), false); + assert.equal(isRoleplayedJsonToolCallText('hi there'), false); + assert.equal(isRoleplayedJsonToolCallText('{"foo":1'), false); +}); + +test('isRoleplayedJsonToolCallText: rejects non-tool-call JSON', () => { + assert.equal(isRoleplayedJsonToolCallText('{"hello":"world"}'), false); + assert.equal(isRoleplayedJsonToolCallText('{"name":42,"parameters":{}}'), false); +}); + +test('isRoleplayedJsonToolCallText: accepts Hermes roleplay envelope', () => { + const text = '{"type":"function","name":"Glob","parameters":{"pattern":"*"}}'; + assert.equal(isRoleplayedJsonToolCallText(text), true); +}); + +test('isRoleplayedJsonToolCallText: accepts bare {name,parameters} (free-tier llama/qwen)', () => { + const text = '{"name":"Glob","parameters":{"pattern":"*","path":"/home/TheCheetah11/Blockrun/Franklin"}}'; + assert.equal(isRoleplayedJsonToolCallText(text), true); +}); + +test('isRoleplayedJsonToolCallText: accepts OpenAI {tool_calls:[…]} envelope', () => { + const text = '{"tool_calls":[{"function":{"name":"Glob","arguments":"{\\"pattern\\":\\"*\\"}"}}]}'; + assert.equal(isRoleplayedJsonToolCallText(text), true); +}); + +test('isRoleplayedJsonToolCallText: accepts XML wrapper', () => { + const text = '{"name":"Glob","parameters":{"pattern":"*"}}'; + assert.equal(isRoleplayedJsonToolCallText(text), true); +}); + +test('isRoleplayedJsonToolCallText: rejects XML that is not a tool-call wrapper', () => { + assert.equal(isRoleplayedJsonToolCallText('hello'), false); +});