diff --git a/internal/phraser/llmphraser.go b/internal/phraser/llmphraser.go index 57e6288..26bab84 100644 --- a/internal/phraser/llmphraser.go +++ b/internal/phraser/llmphraser.go @@ -444,10 +444,6 @@ func (p *LLMPhraser) PhraseNudge(ctx context.Context, c loop.Candidate) (deliver log.Printf("phraser: PhraseNudge: %v", perr) body, mood = "", "" } - if body == "" { - // fallback: try old body/summary format - body, _ = parsePhrase(resp) - } if body == "" { // The model said nothing usable. Say it in Russian anyway — this text // goes straight to a Russian piper voice, so the old "water — care" @@ -691,10 +687,6 @@ func (p *LLMPhraser) PhraseReminder(ctx context.Context, d loop.ReminderDecision log.Printf("phraser: PhraseReminder: %v", perr) body, mood = "", "" } - if body == "" { - // fallback: try old body/summary format - body, _ = parsePhrase(resp) - } if body == "" { body = text } @@ -1164,120 +1156,6 @@ func buildNudgePrompt(c loop.Candidate) string { return strings.Join(ctxParts, "\n") + "\n\n" + tail } -type responseMood struct { - Response string `json:"response"` - Mood string `json:"mood"` -} - -// errBrokenJSON — the model started a JSON object and never finished it. -// That is a failed generation, not a reply. Callers must use their fallback. -var errBrokenJSON = fmt.Errorf("phraser: model output starts as JSON but does not parse") - -// parseResponseMood extracts {"response","mood"} from LLM output, tolerant -// of thinking tokens and extra text before/after the JSON block. -// -// Three outcomes: -// - parsed fine → the fields, nil error. -// - output never looked like JSON → ("", "", nil). The caller may ship it -// as-is; small models sometimes answer in bare prose and that is fine. -// - output starts with "{" but does not parse → errBrokenJSON. The grammar -// guarantees a valid *prefix*, so a generation that hits the token cap -// mid-object comes back as a fragment like `{` or `{\n "`. Shipping that -// as a reply is the bug this error exists to stop. -func parseResponseMood(raw string) (response, mood string, err error) { - cleaned := strings.TrimSpace(raw) - start := strings.Index(cleaned, "{") - end := strings.LastIndex(cleaned, "}") - if start < 0 || end < 0 || end <= start { - if strings.HasPrefix(cleaned, "{") { - return "", "", errBrokenJSON - } - return "", "", nil - } - var parsed responseMood - if e := json.Unmarshal([]byte(escapeRawControls(cleaned[start:end+1])), &parsed); e != nil { - if strings.HasPrefix(cleaned, "{") { - return "", "", errBrokenJSON - } - return "", "", nil - } - return parsed.Response, parsed.Mood, nil -} - -// escapeRawControls escapes the control characters a model writes literally -// inside a JSON string, so a reply that is otherwise fine still parses. -// -// The grammar is what stops these being generated (Vikunja #537). This is the -// second line, for the paths that send no grammar at all — NoGrammar, and any -// remote model whose server ignores one. A raw newline is the shape that was -// measured; the rest of the range is here because the same argument covers it. -// -// Inside a string only. The first version escaped the whole object on the -// argument that JSON permits no control character outside a string either, so -// rewriting one could not do harm. That argument is wrong: JSON permits a -// newline, a tab and a return BETWEEN tokens, which is what pretty-printing is. -// Qwen3-1.7B pretty-prints — it opens `{` and writes three newlines before the -// first key — and escaping those into a literal backslash-n broke every reply -// it wrote. Measured 2026-08-05 on the talk fixture: 31 of 36 conversational -// cases came back as errBrokenJSON and answered from the stub (Vikunja #44). -func escapeRawControls(s string) string { - if !strings.ContainsFunc(s, func(r rune) bool { return r < 0x20 }) { - return s - } - var b strings.Builder - b.Grow(len(s) + 8) - inString := false - escaped := false - for _, r := range s { - switch { - case escaped: - // The character after a backslash is the model's own escape and is - // already whatever it meant to write. - escaped = false - b.WriteRune(r) - continue - case inString && r == '\\': - escaped = true - b.WriteRune(r) - continue - case r == '"': - inString = !inString - b.WriteRune(r) - continue - } - switch { - case !inString || r >= 0x20: - b.WriteRune(r) - case r == '\n': - b.WriteString(`\n`) - case r == '\r': - b.WriteString(`\r`) - case r == '\t': - b.WriteString(`\t`) - default: - fmt.Fprintf(&b, `\u%04x`, r) - } - } - return b.String() -} - -func parsePhrase(raw string) (body, summary string) { - cleaned := strings.TrimSpace(raw) - start := strings.Index(cleaned, "{") - end := strings.LastIndex(cleaned, "}") - if start < 0 || end < 0 || end <= start { - return "", "" - } - var parsed struct { - Body string `json:"body"` - Summary string `json:"summary"` - } - if err := json.Unmarshal([]byte(cleaned[start:end+1]), &parsed); err != nil { - return "", "" - } - return parsed.Body, parsed.Summary -} - func extractPort(listen string) string { _, port, _ := strings.Cut(listen, ":") if port == "" { @@ -1285,12 +1163,3 @@ func extractPort(listen string) string { } return port } - -// stripThink removes the block that Thinking-variant models emit -// before the actual response. No-op when no think block is present. -func stripThink(s string) string { - if i := strings.LastIndex(s, ""); i >= 0 { - s = strings.TrimSpace(s[i+8:]) - } - return s -} diff --git a/internal/phraser/parse.go b/internal/phraser/parse.go new file mode 100644 index 0000000..9598061 --- /dev/null +++ b/internal/phraser/parse.go @@ -0,0 +1,120 @@ +// phraser/parse.go — the reply-side of the LLM output contract: +// {"response":"...","mood":"..."} in, a string and a mood out. +// +// One parser, and every phrasing path in the repo goes through it — the six +// LLMPhraser methods, PhraseWorld, and Replier.PhraseReply, which cmd/mavend +// wraps. The contract is written down in CLAUDE.md; this file is the only +// place it is implemented, so the two halves cannot drift. +package phraser + +import ( + "encoding/json" + "fmt" + "strings" +) + +type responseMood struct { + Response string `json:"response"` + Mood string `json:"mood"` +} + +// errBrokenJSON — the model started a JSON object and never finished it. +// That is a failed generation, not a reply. Callers must use their fallback. +var errBrokenJSON = fmt.Errorf("phraser: model output starts as JSON but does not parse") + +// parseResponseMood extracts {"response","mood"} from LLM output, tolerant +// of thinking tokens and extra text before/after the JSON block. +// +// Three outcomes: +// - parsed fine → the fields, nil error. +// - output never looked like JSON → ("", "", nil). The caller may ship it +// as-is; small models sometimes answer in bare prose and that is fine. +// - output starts with "{" but does not parse → errBrokenJSON. The grammar +// guarantees a valid *prefix*, so a generation that hits the token cap +// mid-object comes back as a fragment like `{` or `{\n "`. Shipping that +// as a reply is the bug this error exists to stop. +func parseResponseMood(raw string) (response, mood string, err error) { + cleaned := strings.TrimSpace(raw) + start := strings.Index(cleaned, "{") + end := strings.LastIndex(cleaned, "}") + if start < 0 || end < 0 || end <= start { + if strings.HasPrefix(cleaned, "{") { + return "", "", errBrokenJSON + } + return "", "", nil + } + var parsed responseMood + if e := json.Unmarshal([]byte(escapeRawControls(cleaned[start:end+1])), &parsed); e != nil { + if strings.HasPrefix(cleaned, "{") { + return "", "", errBrokenJSON + } + return "", "", nil + } + return parsed.Response, parsed.Mood, nil +} + +// escapeRawControls escapes the control characters a model writes literally +// inside a JSON string, so a reply that is otherwise fine still parses. +// +// The grammar is what stops these being generated (Vikunja #537). This is the +// second line, for the paths that send no grammar at all — NoGrammar, and any +// remote model whose server ignores one. A raw newline is the shape that was +// measured; the rest of the range is here because the same argument covers it. +// +// Inside a string only. The first version escaped the whole object on the +// argument that JSON permits no control character outside a string either, so +// rewriting one could not do harm. That argument is wrong: JSON permits a +// newline, a tab and a return BETWEEN tokens, which is what pretty-printing is. +// Qwen3-1.7B pretty-prints — it opens `{` and writes three newlines before the +// first key — and escaping those into a literal backslash-n broke every reply +// it wrote. Measured 2026-08-05 on the talk fixture: 31 of 36 conversational +// cases came back as errBrokenJSON and answered from the stub (Vikunja #44). +func escapeRawControls(s string) string { + if !strings.ContainsFunc(s, func(r rune) bool { return r < 0x20 }) { + return s + } + var b strings.Builder + b.Grow(len(s) + 8) + inString := false + escaped := false + for _, r := range s { + switch { + case escaped: + // The character after a backslash is the model's own escape and is + // already whatever it meant to write. + escaped = false + b.WriteRune(r) + continue + case inString && r == '\\': + escaped = true + b.WriteRune(r) + continue + case r == '"': + inString = !inString + b.WriteRune(r) + continue + } + switch { + case !inString || r >= 0x20: + b.WriteRune(r) + case r == '\n': + b.WriteString(`\n`) + case r == '\r': + b.WriteString(`\r`) + case r == '\t': + b.WriteString(`\t`) + default: + fmt.Fprintf(&b, `\u%04x`, r) + } + } + return b.String() +} + +// stripThink removes the block that Thinking-variant models emit +// before the actual response. No-op when no think block is present. +func stripThink(s string) string { + if i := strings.LastIndex(s, ""); i >= 0 { + s = strings.TrimSpace(s[i+8:]) + } + return s +}