diff --git a/platform/internal/handlers/hermes_messages.go b/platform/internal/handlers/hermes_messages.go new file mode 100644 index 00000000..3ef45d27 --- /dev/null +++ b/platform/internal/handlers/hermes_messages.go @@ -0,0 +1,76 @@ +package handlers + +// mergeSystemMessages collapses consecutive leading system messages into a +// single system message before the payload is forwarded to a Hermes/vLLM +// endpoint. +// +// Background +// ---------- +// The OpenAI-compatible vLLM server (used by Nous Hermes and similar models) +// accepts only ONE system message. When the platform constructs a messages +// array from multiple sources — e.g. a base system prompt, a workspace-level +// config block, and a per-session user override — and these are all emitted as +// consecutive {"role":"system","content":"..."} entries, vLLM either rejects +// the request or silently drops all but the first. +// +// This function is a stateless pre-flight transform that resolves the +// collision before any HTTP call is made. +// +// Rules +// ----- +// 1. Scan from the front of the slice. +// 2. Collect every consecutive {"role":"system"} entry. +// 3. Join their "content" strings with "\n\n" into one system message. +// 4. Prepend the merged message to the remaining (non-system) messages. +// 5. If there is only one leading system message, the slice is returned +// unchanged (no allocation, no copy). +// 6. Non-system messages that appear BETWEEN two system messages are NOT +// considered — the merge only applies to the uninterrupted leading run. +// 7. If there are no system messages at all, the slice is returned as-is. +// +// Content types +// ------------- +// "content" may be a string (the common case) or any other JSON-decoded type +// (e.g. []interface{} for multi-modal content arrays). Only string values +// are merged textually; non-string values are skipped during concatenation. +// +// Example +// +// In: [{system,"A"}, {system,"B"}, {user,"Q"}] +// Out: [{system,"A\n\nB"}, {user,"Q"}] +func mergeSystemMessages(messages []map[string]interface{}) []map[string]interface{} { + // Find the end of the leading system-message run. + end := 0 + for end < len(messages) { + role, _ := messages[end]["role"].(string) + if role != "system" { + break + } + end++ + } + + // Zero or one system message — nothing to merge. + if end <= 1 { + return messages + } + + // Concatenate content strings from the leading system messages. + var merged string + for i := 0; i < end; i++ { + content, _ := messages[i]["content"].(string) + if i == 0 { + merged = content + } else { + merged += "\n\n" + content + } + } + + // Build result: one merged system message + the remaining messages. + result := make([]map[string]interface{}, 0, 1+len(messages)-end) + result = append(result, map[string]interface{}{ + "role": "system", + "content": merged, + }) + result = append(result, messages[end:]...) + return result +} diff --git a/platform/internal/handlers/hermes_messages_test.go b/platform/internal/handlers/hermes_messages_test.go new file mode 100644 index 00000000..3d6e2776 --- /dev/null +++ b/platform/internal/handlers/hermes_messages_test.go @@ -0,0 +1,196 @@ +package handlers + +import ( + "reflect" + "testing" +) + +// msg is a shorthand constructor for test messages. +func msg(role, content string) map[string]interface{} { + return map[string]interface{}{"role": role, "content": content} +} + +// ============================================================ +// mergeSystemMessages — acceptance criteria from issue #499 +// ============================================================ + +// TestMergeSystemMessages_StackedMerged verifies that two consecutive leading +// system messages are collapsed into one, joined by "\n\n". +// +// Acceptance criterion 3: +// +// input [{system,"A"}, {system,"B"}, {user,"Q"}] +// output [{system,"A\n\nB"}, {user,"Q"}] +func TestMergeSystemMessages_StackedMerged(t *testing.T) { + input := []map[string]interface{}{ + msg("system", "A"), + msg("system", "B"), + msg("user", "Q"), + } + got := mergeSystemMessages(input) + + want := []map[string]interface{}{ + msg("system", "A\n\nB"), + msg("user", "Q"), + } + if !reflect.DeepEqual(got, want) { + t.Errorf("stacked merge: got %v, want %v", got, want) + } +} + +// TestMergeSystemMessages_SingleUnchanged verifies that a single leading system +// message is passed through without modification or reallocation. +// +// Acceptance criterion 4: single system message unchanged. +func TestMergeSystemMessages_SingleUnchanged(t *testing.T) { + input := []map[string]interface{}{ + msg("system", "only"), + msg("user", "hello"), + } + got := mergeSystemMessages(input) + + // Pointer equality: same underlying slice (no copy made). + if &got[0] != &input[0] { + t.Error("single system: expected same slice to be returned, got a copy") + } + if len(got) != 2 { + t.Errorf("single system: got len %d, want 2", len(got)) + } +} + +// TestMergeSystemMessages_NoSystem verifies that a messages array with no system +// messages at all is returned unchanged. +// +// Acceptance criterion 5: no system message → messages passed through unchanged. +func TestMergeSystemMessages_NoSystem(t *testing.T) { + input := []map[string]interface{}{ + msg("user", "hello"), + msg("assistant", "hi"), + } + got := mergeSystemMessages(input) + + if &got[0] != &input[0] { + t.Error("no system: expected same slice to be returned, got a copy") + } + if len(got) != 2 { + t.Errorf("no system: got len %d, want 2", len(got)) + } +} + +// TestMergeSystemMessages_ThreeSystem verifies three consecutive system messages +// are collapsed into one, with "\n\n" between each pair. +func TestMergeSystemMessages_ThreeSystem(t *testing.T) { + input := []map[string]interface{}{ + msg("system", "base"), + msg("system", "workspace config"), + msg("system", "user override"), + msg("user", "go"), + } + got := mergeSystemMessages(input) + + want := []map[string]interface{}{ + msg("system", "base\n\nworkspace config\n\nuser override"), + msg("user", "go"), + } + if !reflect.DeepEqual(got, want) { + t.Errorf("three system: got %v, want %v", got, want) + } +} + +// TestMergeSystemMessages_OnlySystemMessages verifies an array of only system +// messages (no user turn) is collapsed correctly. +func TestMergeSystemMessages_OnlySystemMessages(t *testing.T) { + input := []map[string]interface{}{ + msg("system", "first"), + msg("system", "second"), + } + got := mergeSystemMessages(input) + + want := []map[string]interface{}{ + msg("system", "first\n\nsecond"), + } + if !reflect.DeepEqual(got, want) { + t.Errorf("only system: got %v, want %v", got, want) + } +} + +// TestMergeSystemMessages_InterlevedUserNotMerged verifies that only the leading +// run of system messages is collapsed — a system message that appears AFTER a +// user turn is NOT merged into the leading block. +func TestMergeSystemMessages_InterleavedUserNotMerged(t *testing.T) { + input := []map[string]interface{}{ + msg("system", "A"), + msg("system", "B"), + msg("user", "Q1"), + msg("system", "C"), // NOT part of leading run + msg("user", "Q2"), + } + got := mergeSystemMessages(input) + + want := []map[string]interface{}{ + msg("system", "A\n\nB"), + msg("user", "Q1"), + msg("system", "C"), // untouched + msg("user", "Q2"), + } + if !reflect.DeepEqual(got, want) { + t.Errorf("interleaved: got %v, want %v", got, want) + } +} + +// TestMergeSystemMessages_EmptySlice verifies that an empty input is +// returned as-is without panicking. +func TestMergeSystemMessages_EmptySlice(t *testing.T) { + input := []map[string]interface{}{} + got := mergeSystemMessages(input) + if len(got) != 0 { + t.Errorf("empty: got len %d, want 0", len(got)) + } +} + +// TestMergeSystemMessages_NilSlice verifies that a nil input is handled +// without panicking. +func TestMergeSystemMessages_NilSlice(t *testing.T) { + var input []map[string]interface{} + got := mergeSystemMessages(input) + if got != nil && len(got) != 0 { + t.Errorf("nil: got %v, want nil/empty", got) + } +} + +// TestMergeSystemMessages_NonStringContentSkipped verifies that a system message +// whose "content" is not a string (e.g. a []interface{} multi-modal block) is +// treated as an empty string during concatenation so the merge still succeeds +// without panicking. +func TestMergeSystemMessages_NonStringContentSkipped(t *testing.T) { + input := []map[string]interface{}{ + {"role": "system", "content": "text part"}, + {"role": "system", "content": []interface{}{"block1", "block2"}}, // non-string + msg("user", "hi"), + } + got := mergeSystemMessages(input) + + // Non-string treated as "": "text part\n\n" + wantContent := "text part\n\n" + if len(got) != 2 { + t.Fatalf("non-string content: got len %d, want 2", len(got)) + } + gotContent, _ := got[0]["content"].(string) + if gotContent != wantContent { + t.Errorf("non-string content: got content %q, want %q", gotContent, wantContent) + } +} + +// TestMergeSystemMessages_AssistantLeadingNotMerged verifies that an assistant +// message at the front (unusual but possible) is not treated as a system +// message and the slice is returned as-is. +func TestMergeSystemMessages_AssistantLeadingNotMerged(t *testing.T) { + input := []map[string]interface{}{ + msg("assistant", "hello"), + msg("user", "hi"), + } + got := mergeSystemMessages(input) + if &got[0] != &input[0] { + t.Error("assistant leading: expected same slice to be returned") + } +}