diff --git a/CHANGELOG.md b/CHANGELOG.md index 55981fba..48e579b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,28 @@ ## [1.3.1] - 2026-05-29 +### fix: strict chat templates reject mid-conversation system messages (#62) + +Qwen3 / Qwen3.5 chat templates (and other strict templates) under +llama.cpp `--jinja` raise `System message must be at the beginning.` and +llama.cpp returns HTTP 400 — but only when `tools` are present, since +that's when it compiles the template to build a tool-call grammar. +SmallCode injects system-role content mid-conversation (clarifier, plan +request, planner injection, path-validation warnings, skill activation, +compaction summaries), producing a messages array with `system` entries +at positions other than 0. + +- New `src/session/message_normalizer.js#consolidateSystemMessages()` + collapses all system-role messages into a single leading system + message (preserving order, de-duplicating identical blocks) and emits + only non-system turns after it. +- Applied in both request builders (`bin/smallcode.js` and + `bin/model_client.js` `chatCompletion`) right before the body is sent, + so it catches stray system messages regardless of which path injected + them. Verified end-to-end against a Qwen3 model: every tool-bearing + request now carries exactly one system message at index 0. +- Test coverage: `test/message_normalizer.test.js` (9 cases). + ### fix: compatibility issues #57, #58, #59 Three reported environment-compatibility bugs: diff --git a/bin/model_client.js b/bin/model_client.js index 834b567e..a45f7542 100644 --- a/bin/model_client.js +++ b/bin/model_client.js @@ -35,9 +35,13 @@ async function chatCompletion(ctx) { }); const _tools = ctx.getAllTools(config); + // Collapse any mid-conversation system messages into a single leading one + // so strict chat templates (Qwen3/Qwen3.5 under llama.cpp --jinja) don't + // 400 with "System message must be at the beginning." See issue #62. + const { consolidateSystemMessages } = require('../src/session/message_normalizer'); const body = { model: target.model, - messages: [systemMsg, ...processedMessages], + messages: consolidateSystemMessages([systemMsg, ...processedMessages]), temperature: 0.1, max_tokens: 4096, }; diff --git a/bin/smallcode.js b/bin/smallcode.js index 4f8d13cb..77255e12 100755 --- a/bin/smallcode.js +++ b/bin/smallcode.js @@ -90,6 +90,7 @@ const { PluginLoader } = require('../src/plugins/loader'); const { SkillManager } = require('../src/plugins/skills'); const { SessionStore } = require('../src/session/persistence'); const { resolveReferences, formatReferencesForPrompt } = require('../src/session/references'); +const { consolidateSystemMessages } = require('../src/session/message_normalizer'); const { TokenTracker } = require('../src/session/tokens'); const { UndoStack } = require('../src/session/undo'); const { shouldInjectGitContext, getGitDiffContext } = require('../src/session/git_context'); @@ -2161,9 +2162,16 @@ async function chatCompletion(config, messages) { }); const _tools = getAllTools(config, currentToolCategory); + // Consolidate any mid-conversation system messages into a single leading + // system message. Strict chat templates (Qwen3/Qwen3.5 under llama.cpp + // --jinja) reject a `system` role anywhere but index 0 and return HTTP 400 + // when tools are present. SmallCode injects system content mid-stream + // (clarifier, plan, planner, path warnings, skills, compaction), so we + // normalize here, right before the request is built. See issue #62. + const normalizedMessages = consolidateSystemMessages([systemMsg, ...processedWithImages]); const body = { model: target.model, - messages: [systemMsg, ...processedWithImages], + messages: normalizedMessages, temperature: 0.1, max_tokens: parseInt(process.env.SMALLCODE_MAX_OUTPUT_TOKENS) || 8192, }; diff --git a/src/session/message_normalizer.js b/src/session/message_normalizer.js new file mode 100644 index 00000000..cc44b259 --- /dev/null +++ b/src/session/message_normalizer.js @@ -0,0 +1,75 @@ +// SmallCode — Message Normalizer +// +// Some chat templates (notably Qwen3 / Qwen3.5 under llama.cpp with --jinja) +// enforce that a `system` role message may only appear at index 0 of the +// messages array. Their Jinja template raises: +// +// raise_exception('System message must be at the beginning.') +// +// …and llama.cpp returns HTTP 400 BEFORE the request is processed — but only +// when `tools` are present, because that's when it compiles the template to +// build a tool-call grammar. (See issue #62.) +// +// SmallCode legitimately injects system-role content mid-conversation in +// several places: clarification instructions, plan requests, planner +// injection, path-validation warnings, skill activation, and compaction +// summaries. Each of those pushes a `{ role: 'system', content }` object into +// the live conversation history, so by the time we assemble the request the +// array can look like: +// +// [system(prompt), user, assistant, system(plan), user, system(warning), ...] +// +// This module collapses any such array into a single leading system message +// followed by only non-system turns — satisfying strict templates while +// preserving the injected instructions (they're merged into the lead system +// message, not dropped). +// +// Design notes: +// - Order is preserved: stray system messages are appended to the lead +// system content in the order they appeared, so later instructions still +// come after earlier ones. +// - Non-string content (multimodal image arrays on user turns) is never +// touched — only `role: 'system'` entries are merged, and those are +// always plain strings in this codebase. +// - Idempotent: running it on an already-normalized array is a no-op. + +'use strict'; + +/** + * Collapse all system-role messages into a single leading system message. + * + * @param {Array<{role:string, content:any}>} messages OpenAI-style messages. + * @returns {Array} A new array with exactly one system message at index 0 + * (when any system content exists), followed by all non-system messages in + * their original order. The input array is not mutated. + */ +function consolidateSystemMessages(messages) { + if (!Array.isArray(messages) || messages.length === 0) return messages; + + const systemParts = []; + const rest = []; + + for (const msg of messages) { + if (msg && msg.role === 'system' && typeof msg.content === 'string') { + const trimmed = msg.content.trim(); + if (trimmed) systemParts.push(trimmed); + } else { + rest.push(msg); + } + } + + // No system content at all → return the non-system messages unchanged. + if (systemParts.length === 0) return rest.length === messages.length ? messages : rest; + + // De-duplicate consecutive identical blocks (the same instruction can be + // re-injected across turns; collapsing avoids ballooning the lead prompt). + const deduped = []; + for (const part of systemParts) { + if (deduped[deduped.length - 1] !== part) deduped.push(part); + } + + const merged = { role: 'system', content: deduped.join('\n\n') }; + return [merged, ...rest]; +} + +module.exports = { consolidateSystemMessages }; diff --git a/test/message_normalizer.test.js b/test/message_normalizer.test.js new file mode 100644 index 00000000..e2b9f529 --- /dev/null +++ b/test/message_normalizer.test.js @@ -0,0 +1,100 @@ +// SmallCode — message normalizer tests (issue #62) +// +// Strict chat templates (Qwen3/Qwen3.5 under llama.cpp --jinja) raise +// "System message must be at the beginning." and llama.cpp 400s when a +// system-role message appears anywhere but index 0 AND tools are present. +// consolidateSystemMessages() must guarantee exactly one leading system +// message. + +'use strict'; + +const { test } = require('node:test'); +const assert = require('node:assert'); + +const { consolidateSystemMessages } = require('../src/session/message_normalizer'); + +test('merges a mid-conversation system message into the leading one', () => { + const out = consolidateSystemMessages([ + { role: 'system', content: 'base prompt' }, + { role: 'user', content: 'hi' }, + { role: 'assistant', content: 'hello' }, + { role: 'system', content: 'PLAN: do the thing' }, + { role: 'user', content: 'go' }, + ]); + // Exactly one system message, at index 0. + assert.equal(out.filter(m => m.role === 'system').length, 1); + assert.equal(out[0].role, 'system'); + assert.match(out[0].content, /base prompt/); + assert.match(out[0].content, /PLAN: do the thing/); + // Non-system turns preserved in order. + assert.deepEqual(out.slice(1).map(m => m.role), ['user', 'assistant', 'user']); +}); + +test('order of merged system parts is preserved', () => { + const out = consolidateSystemMessages([ + { role: 'system', content: 'first' }, + { role: 'user', content: 'a' }, + { role: 'system', content: 'second' }, + { role: 'system', content: 'third' }, + ]); + assert.equal(out[0].content, 'first\n\nsecond\n\nthird'); +}); + +test('no system messages → array returned unchanged in content', () => { + const input = [ + { role: 'user', content: 'a' }, + { role: 'assistant', content: 'b' }, + ]; + const out = consolidateSystemMessages(input); + assert.deepEqual(out.map(m => m.role), ['user', 'assistant']); +}); + +test('idempotent on an already-normalized array', () => { + const once = consolidateSystemMessages([ + { role: 'system', content: 'sys' }, + { role: 'user', content: 'u' }, + ]); + const twice = consolidateSystemMessages(once); + assert.deepEqual(twice, once); +}); + +test('deduplicates identical consecutive system blocks', () => { + const out = consolidateSystemMessages([ + { role: 'system', content: 'same' }, + { role: 'user', content: 'u' }, + { role: 'system', content: 'same' }, + ]); + assert.equal(out[0].content, 'same'); +}); + +test('drops empty/whitespace-only system messages', () => { + const out = consolidateSystemMessages([ + { role: 'system', content: 'real' }, + { role: 'system', content: ' ' }, + { role: 'user', content: 'u' }, + ]); + assert.equal(out[0].content, 'real'); + assert.equal(out.filter(m => m.role === 'system').length, 1); +}); + +test('preserves multimodal user content untouched', () => { + const img = { role: 'user', content: [{ type: 'text', text: 'hi' }, { type: 'image_url', image_url: { url: 'data:...' } }] }; + const out = consolidateSystemMessages([ + { role: 'system', content: 'sys' }, + img, + ]); + assert.equal(out[1], img); // same reference, unmodified +}); + +test('handles only-system input (collapses to one)', () => { + const out = consolidateSystemMessages([ + { role: 'system', content: 'a' }, + { role: 'system', content: 'b' }, + ]); + assert.equal(out.length, 1); + assert.equal(out[0].content, 'a\n\nb'); +}); + +test('empty array is a no-op', () => { + assert.deepEqual(consolidateSystemMessages([]), []); +});