Merge pull request #655 from Molecule-AI/feat/issue-499-hermes-stacked-system-messages

feat(hermes): stacked system message merge + Nous sampling defaults (#499 #500)

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
+}

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")
+	}
+}