phraser: one POST, not two copies of the same forty lines (V-397)

chatWithSystem and chatWithMessages each built the request, sent it, read it and unwrapped the choice. Both now call postChat, which does that once. The copies had already drifted: only chatWithMessages logged the raw generation, so an unparseable nudge or query left nothing in the log to read. Every path logs it now, tagged with the caller.
2026-08-06 01:20:20 +04:00
parent b49755302c
commit 5fede2acb7
2 changed files with 103 additions and 129 deletions
@@ -1,12 +1,9 @@
 package phraser

 import (
-	"bytes"
 	"context"
-	"encoding/json"
 	"errors"
 	"fmt"
-	"io"
 	"log"
 	"net/http"
 	"strings"
@@ -404,70 +401,6 @@ func chatSystemPrompt(block func() string) string {
 	return persona.Prepend(block, base)
 }

-// chatWithMessages sends a full message array (system + history + current) to
-// the LLM completion endpoint. Like chatWithSystem but for an arbitrary message
-// slice — the caller owns the system prompt placement.
-func (p *LLMPhraser) chatWithMessages(ctx context.Context, msgs []chatMsg, maxTokens int) (string, error) {
-	// Same silent preference as chatWithSystem, when the array is the shape
-	// llm.Req can carry: one system turn and one user turn. PhraseChat already
-	// folds the history into a single user message (some chat templates reject
-	// consecutive user turns), so today that is every call. A longer array goes
-	// to the resident model rather than get flattened here, because flattening a
-	// conversation is a decision its owner should make.
-	if len(msgs) == 2 && msgs[0].Role == "system" && msgs[1].Role == "user" {
-		if out, ok := p.remoteChat(ctx, msgs[0].Content, msgs[1].Content, maxTokens); ok {
-			return out, nil
-		}
-	}
-	base, release, err := p.acquire()
-	if err != nil {
-		return "", err
-	}
-	defer release()
-	req := chatReq{
-		Messages:      msgs,
-		Temperature:   p.temperature(),
-		MaxTokens:     maxTokens,
-		Grammar:       p.grammar(),
-		RepeatPenalty: phraseRepeatPenalty,
-	}
-	body, err := json.Marshal(req)
-	if err != nil {
-		return "", fmt.Errorf("llm: marshal: %w", err)
-	}
-	httpReq, err := http.NewRequestWithContext(ctx, "POST", base+"/v1/chat/completions", bytes.NewReader(body))
-	if err != nil {
-		return "", fmt.Errorf("llm: request: %w", err)
-	}
-	httpReq.Header.Set("Content-Type", "application/json")
-	resp, err := p.client.Do(httpReq)
-	if err != nil {
-		return "", fmt.Errorf("llm: post: %w", err)
-	}
-	defer resp.Body.Close()
-	raw, err := io.ReadAll(resp.Body)
-	if err != nil {
-		return "", fmt.Errorf("llm: read: %w", err)
-	}
-	if resp.StatusCode != 200 {
-		return "", fmt.Errorf("llm: status %d: %s", resp.StatusCode, strings.TrimSpace(string(raw)))
-	}
-	var cr chatResp
-	if err := json.Unmarshal(raw, &cr); err != nil {
-		return "", fmt.Errorf("llm: parse: %w", err)
-	}
-	if len(cr.Choices) == 0 {
-		return "", fmt.Errorf("llm: no choices in response")
-	}
-	logIfTruncated("chat", cr.Choices[0].FinishReason, maxTokens)
-	content := cr.Choices[0].Message.Content
-	if content == "" {
-		content = cr.Choices[0].Message.ReasoningContent
-	}
-	log.Printf("llm raw content: %q", content)
-	return stripThink(content), nil
-}
-
 func (p *LLMPhraser) PhraseReminder(ctx context.Context, d loop.ReminderDecision) (delivery.PhrasedReminder, error) {
 	text := extractReminderText(d.Reminder.Payload)
 	if text == "" {
@@ -511,68 +444,6 @@ func (p *LLMPhraser) chat(ctx context.Context, userPrompt string) (string, error
 	return p.chatWithSystem(ctx, p.systemPrompt(), userPrompt, 512)
 }

-func (p *LLMPhraser) chatWithSystem(ctx context.Context, system, user string, maxTokens int) (string, error) {
-	// The workstation model first when it will take work, and silently: every
-	// caller of this helper is on the silent half of the degradation rule. It
-	// answering is not news, and it being asleep is not news either.
-	if out, ok := p.remoteChat(ctx, system, user, maxTokens); ok {
-		return out, nil
-	}
-	base, release, err := p.acquire()
-	if err != nil {
-		return "", err
-	}
-	defer release()
-	req := chatReq{
-		Messages: []chatMsg{
-			{Role: "system", Content: system},
-			{Role: "user", Content: user},
-		},
-		Temperature:   p.temperature(),
-		MaxTokens:     maxTokens,
-		Grammar:       p.grammar(),
-		RepeatPenalty: phraseRepeatPenalty,
-	}
-	body, err := json.Marshal(req)
-	if err != nil {
-		return "", fmt.Errorf("llm: marshal: %w", err)
-	}
-
-	httpReq, err := http.NewRequestWithContext(ctx, "POST", base+"/v1/chat/completions", bytes.NewReader(body))
-	if err != nil {
-		return "", fmt.Errorf("llm: request: %w", err)
-	}
-	httpReq.Header.Set("Content-Type", "application/json")
-
-	resp, err := p.client.Do(httpReq)
-	if err != nil {
-		return "", fmt.Errorf("llm: post: %w", err)
-	}
-	defer resp.Body.Close()
-
-	raw, err := io.ReadAll(resp.Body)
-	if err != nil {
-		return "", fmt.Errorf("llm: read: %w", err)
-	}
-	if resp.StatusCode != 200 {
-		return "", fmt.Errorf("llm: status %d: %s", resp.StatusCode, strings.TrimSpace(string(raw)))
-	}
-
-	var cr chatResp
-	if err := json.Unmarshal(raw, &cr); err != nil {
-		return "", fmt.Errorf("llm: parse: %w", err)
-	}
-	if len(cr.Choices) == 0 {
-		return "", fmt.Errorf("llm: no choices in response")
-	}
-	logIfTruncated("chatWithSystem", cr.Choices[0].FinishReason, maxTokens)
-	content := cr.Choices[0].Message.Content
-	if content == "" {
-		content = cr.Choices[0].Message.ReasoningContent
-	}
-	return stripThink(content), nil
-}
-
 // nudgeSystem — the phrasing contract for nudges.
 //
 // Written as filled-in examples, not as a schema with "..." in it. A 0.8B
@@ -4,7 +4,14 @@
 package phraser

 import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"fmt"
+	"io"
 	"log"
+	"net/http"
+	"strings"
 )

 type chatMsg struct {
@@ -121,3 +128,99 @@ func logIfTruncated(where, reason string, maxTokens int) {
 		log.Printf("phraser: %s hit the %d-token cap (finish_reason=length) — the reply is truncated, or the model was looping", where, maxTokens)
 	}
 }
+
+// postChat sends one message array to the resident llama-server and returns
+// what the model wrote, think block stripped.
+//
+// The only POST in the package. chatWithSystem and chatWithMessages each
+// carried their own copy of these forty lines, identical down to the error
+// strings, and the copies had already drifted twice: one logged the raw
+// content and the other did not, and one labelled a truncation "chat" where
+// the other said "chatWithSystem". Two transports mean two chances to
+// configure the sampler differently, which is exactly how #531 happened.
+//
+// where names the calling path, for the truncation line and nothing else.
+func (p *LLMPhraser) postChat(ctx context.Context, where string, msgs []chatMsg, maxTokens int) (string, error) {
+	base, release, err := p.acquire()
+	if err != nil {
+		return "", err
+	}
+	defer release()
+	body, err := json.Marshal(chatReq{
+		Messages:      msgs,
+		Temperature:   p.temperature(),
+		MaxTokens:     maxTokens,
+		Grammar:       p.grammar(),
+		RepeatPenalty: phraseRepeatPenalty,
+	})
+	if err != nil {
+		return "", fmt.Errorf("llm: marshal: %w", err)
+	}
+	httpReq, err := http.NewRequestWithContext(ctx, "POST", base+"/v1/chat/completions", bytes.NewReader(body))
+	if err != nil {
+		return "", fmt.Errorf("llm: request: %w", err)
+	}
+	httpReq.Header.Set("Content-Type", "application/json")
+	resp, err := p.client.Do(httpReq)
+	if err != nil {
+		return "", fmt.Errorf("llm: post: %w", err)
+	}
+	defer resp.Body.Close()
+	raw, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return "", fmt.Errorf("llm: read: %w", err)
+	}
+	if resp.StatusCode != 200 {
+		return "", fmt.Errorf("llm: status %d: %s", resp.StatusCode, strings.TrimSpace(string(raw)))
+	}
+	var cr chatResp
+	if err := json.Unmarshal(raw, &cr); err != nil {
+		return "", fmt.Errorf("llm: parse: %w", err)
+	}
+	if len(cr.Choices) == 0 {
+		return "", fmt.Errorf("llm: no choices in response")
+	}
+	logIfTruncated(where, cr.Choices[0].FinishReason, maxTokens)
+	content := cr.Choices[0].Message.Content
+	if content == "" {
+		content = cr.Choices[0].Message.ReasoningContent
+	}
+	// Every phrasing path logs its raw generation now. Only chatWithMessages
+	// did, so a nudge or a query that came back unparseable left nothing in the
+	// log to read (V-397).
+	log.Printf("phraser: %s raw content: %q", where, content)
+	return stripThink(content), nil
+}
+
+// chatWithSystem is the common shape: one system turn, one user turn.
+//
+// The workstation model first when it will take work, and silently: every
+// caller of this helper is on the silent half of the degradation rule. It
+// answering is not news, and it being asleep is not news either.
+func (p *LLMPhraser) chatWithSystem(ctx context.Context, system, user string, maxTokens int) (string, error) {
+	if out, ok := p.remoteChat(ctx, system, user, maxTokens); ok {
+		return out, nil
+	}
+	return p.postChat(ctx, "chatWithSystem", []chatMsg{
+		{Role: "system", Content: system},
+		{Role: "user", Content: user},
+	}, maxTokens)
+}
+
+// chatWithMessages sends a full message array (system + history + current) to
+// the LLM completion endpoint. Like chatWithSystem but for an arbitrary message
+// slice — the caller owns the system prompt placement.
+func (p *LLMPhraser) chatWithMessages(ctx context.Context, msgs []chatMsg, maxTokens int) (string, error) {
+	// Same silent preference as chatWithSystem, when the array is the shape
+	// llm.Req can carry: one system turn and one user turn. PhraseChat already
+	// folds the history into a single user message (some chat templates reject
+	// consecutive user turns), so today that is every call. A longer array goes
+	// to the resident model rather than get flattened here, because flattening a
+	// conversation is a decision its owner should make.
+	if len(msgs) == 2 && msgs[0].Role == "system" && msgs[1].Role == "user" {
+		if out, ok := p.remoteChat(ctx, msgs[0].Content, msgs[1].Content, maxTokens); ok {
+			return out, nil
+		}
+	}
+	return p.postChat(ctx, "chat", msgs, maxTokens)
+}