Stop shipping half-written JSON as a reply
Two bugs, one symptom. A run of the talk eval produced replies that were
literally "{" and "{\n \"" — those strings went out as things Maven said.
First bug: the parser could not tell "the model answered in plain prose"
from "the model started a JSON object and got cut off". Both came back as
empty, and every caller then shipped the raw text. Now an unfinished object
returns an error and each caller uses its own fallback instead. Bare prose
with no JSON in it still passes through, because small models do sometimes
answer that way and the reply is fine.
Second bug, and the actual cause: the grammar capped the response field at
400 characters. I measured it against Qwen3.5-0.8B at three different token
caps — 256, 768 and 2048 — and the reply came back exactly 400 characters
every time, cut mid-word. So the token limit was never what stopped it.
The bound is 1000 now, about six Russian sentences, still low enough to cut
off a repetition loop.
Token caps go from 256 to 768 on the chat and query paths so 1000
characters of Russian actually fits. The nudge path keeps its own cap; a
nudge is meant to be one sentence.
Note: cmd/mavend/replier_llm.go has its own copy of this parser with the
same bug. Left alone here so this commit stays small — that duplicate is
Vikunja #396.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ
This commit is contained in:
@@ -0,0 +1,54 @@
|
||||
package phraser
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// A reply that starts a JSON object and never finishes it is a failed
|
||||
// generation, not a reply. Before this, the parser returned ("", "") for these
|
||||
// and every caller then shipped the raw fragment as the thing Maven said. A
|
||||
// real run produced replies of literally "{" and "{\n \"".
|
||||
func TestParseResponseMoodRejectsUnfinishedJSON(t *testing.T) {
|
||||
for _, raw := range []string{
|
||||
`{`,
|
||||
"{\n \"",
|
||||
`{"response": "неполн`,
|
||||
`{"response": "текст", "mood":`,
|
||||
} {
|
||||
text, mood, err := parseResponseMood(raw)
|
||||
if !errors.Is(err, errBrokenJSON) {
|
||||
t.Errorf("parseResponseMood(%q) err = %v, want errBrokenJSON", raw, err)
|
||||
}
|
||||
if text != "" || mood != "" {
|
||||
t.Errorf("parseResponseMood(%q) leaked %q/%q — a fragment must never come back as a reply", raw, text, mood)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Bare prose is still fine. Small models sometimes answer without any JSON at
|
||||
// all, and that reply is usable — so the new error must not swallow it.
|
||||
func TestParseResponseMoodAllowsBareProse(t *testing.T) {
|
||||
for _, raw := range []string{
|
||||
"норм, а ты как?",
|
||||
"вот что я нашла: ключ у соседа",
|
||||
} {
|
||||
text, mood, err := parseResponseMood(raw)
|
||||
if err != nil {
|
||||
t.Errorf("parseResponseMood(%q) err = %v, want nil", raw, err)
|
||||
}
|
||||
// No JSON means no fields; the caller ships raw as-is.
|
||||
if text != "" || mood != "" {
|
||||
t.Errorf("parseResponseMood(%q) = %q/%q, want empty", raw, text, mood)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The measured failure: the model wants more than 400 characters and the old
|
||||
// grammar cut it off mid-word. Guards the bound against being tightened back.
|
||||
func TestGrammarStringBoundHasRoomForARealAnswer(t *testing.T) {
|
||||
if !strings.Contains(responseGrammar, "{0,1000}") {
|
||||
t.Error("grammar string bound is not 1000; 400 truncated real replies mid-word (see the comment on responseGrammar)")
|
||||
}
|
||||
}
|
||||
@@ -97,7 +97,10 @@ func TestGrammarStringRuleIsNotASCIIOnly(t *testing.T) {
|
||||
// Russian body with an escaped quote inside, hand-built to test the contract.
|
||||
func TestGrammarShapedJSONParses(t *testing.T) {
|
||||
raw := `{"response": "он сказал \"привет\" и ушёл.\nвот так.", "mood": "confused"}`
|
||||
text, mood := parseResponseMood(raw)
|
||||
text, mood, err := parseResponseMood(raw)
|
||||
if err != nil {
|
||||
t.Fatalf("grammar-shaped JSON did not parse: %v", err)
|
||||
}
|
||||
if want := "он сказал \"привет\" и ушёл.\nвот так."; text != want {
|
||||
t.Errorf("response = %q, want %q", text, want)
|
||||
}
|
||||
|
||||
@@ -190,7 +190,12 @@ func (p *LLMPhraser) PhraseNudge(ctx context.Context, c loop.Candidate) (deliver
|
||||
if err != nil {
|
||||
return delivery.PhrasedNudge{}, err
|
||||
}
|
||||
body, mood := parseResponseMood(resp)
|
||||
body, mood, perr := parseResponseMood(resp)
|
||||
if perr != nil {
|
||||
// Truncated JSON. Not a nudge — use the plain Russian fallback.
|
||||
log.Printf("phraser: PhraseNudge: %v", perr)
|
||||
body, mood = "", ""
|
||||
}
|
||||
if body == "" {
|
||||
// fallback: try old body/summary format
|
||||
body, _ = parsePhrase(resp)
|
||||
@@ -216,11 +221,16 @@ func (p *LLMPhraser) PhraseQuery(ctx context.Context, utterance string, notes []
|
||||
// prompt is the single tested source in router.KnowledgePrompt.
|
||||
sys := persona.Prepend(p.cfg.ContextBlock, router.KnowledgePrompt())
|
||||
prompt := fmt.Sprintf("Пользователь спрашивает: \"%s\".", utterance)
|
||||
resp, err := p.chatWithSystem(ctx, sys, prompt, 256)
|
||||
resp, err := p.chatWithSystem(ctx, sys, prompt, 768)
|
||||
if err != nil || resp == "" {
|
||||
return "не знаю.", nil
|
||||
}
|
||||
if text, _ := parseResponseMood(resp); text != "" {
|
||||
text, _, perr := parseResponseMood(resp)
|
||||
if perr != nil {
|
||||
log.Printf("phraser: PhraseQuery: %v", perr)
|
||||
return "не знаю.", nil
|
||||
}
|
||||
if text != "" {
|
||||
return text, nil
|
||||
}
|
||||
return resp, nil
|
||||
@@ -233,14 +243,19 @@ func (p *LLMPhraser) PhraseQuery(ctx context.Context, utterance string, notes []
|
||||
`Он спрашивает: "%s". В твоих заметках по этому вопросу написано: "%s". Ответь ему коротко и своими словами. Если в заметках ответа нет — так и скажи.`,
|
||||
utterance, strings.Join(notes, `"; "`),
|
||||
)
|
||||
resp, err := p.chatWithSystem(ctx, sys, prompt, 256)
|
||||
if err != nil {
|
||||
resp, err := p.chatWithSystem(ctx, sys, prompt, 768)
|
||||
text, _, perr := parseResponseMood(resp)
|
||||
if err != nil || perr != nil {
|
||||
// Read the notes out rather than ship a broken fragment.
|
||||
if perr != nil {
|
||||
log.Printf("phraser: PhraseQuery: %v", perr)
|
||||
}
|
||||
if len(notes) == 1 {
|
||||
return "вот что я нашла: " + notes[0], nil
|
||||
}
|
||||
return "вот что я нашла: " + strings.Join(notes, "; "), nil
|
||||
}
|
||||
if text, _ := parseResponseMood(resp); text != "" {
|
||||
if text != "" {
|
||||
return text, nil
|
||||
}
|
||||
return resp, nil
|
||||
@@ -263,12 +278,17 @@ func (p *LLMPhraser) PhraseChat(ctx context.Context, utterance string, history [
|
||||
combined += utterance
|
||||
msgs = append(msgs, chatMsg{Role: "user", Content: strings.TrimSpace(combined)})
|
||||
|
||||
resp, err := p.chatWithMessages(ctx, msgs, 512)
|
||||
resp, err := p.chatWithMessages(ctx, msgs, 768)
|
||||
if err != nil {
|
||||
log.Printf("phraser: PhraseChat: %v", err)
|
||||
return "поговорили.", nil
|
||||
}
|
||||
if text, _ := parseResponseMood(resp); text != "" {
|
||||
text, _, perr := parseResponseMood(resp)
|
||||
if perr != nil {
|
||||
log.Printf("phraser: PhraseChat: %v", perr)
|
||||
return "поговорили.", nil
|
||||
}
|
||||
if text != "" {
|
||||
return text, nil
|
||||
}
|
||||
// fallback: plain text without JSON
|
||||
@@ -351,7 +371,12 @@ func (p *LLMPhraser) PhraseReminder(ctx context.Context, d loop.ReminderDecision
|
||||
if err != nil {
|
||||
return delivery.PhrasedReminder{}, err
|
||||
}
|
||||
body, mood := parseResponseMood(resp)
|
||||
body, mood, perr := parseResponseMood(resp)
|
||||
if perr != nil {
|
||||
// Truncated JSON. Fall through to the reminder's own text.
|
||||
log.Printf("phraser: PhraseReminder: %v", perr)
|
||||
body, mood = "", ""
|
||||
}
|
||||
if body == "" {
|
||||
// fallback: try old body/summary format
|
||||
body, _ = parsePhrase(resp)
|
||||
@@ -396,10 +421,16 @@ type chatReq struct {
|
||||
// Russian, so an ASCII-only rule would make every reply empty. The escape rule
|
||||
// is what lets the model close a string it opened with a quote inside. Length
|
||||
// is bounded so a repetition loop truncates the field, not the JSON object.
|
||||
//
|
||||
// That bound was 400 and 400 was too tight. Measured against Qwen3.5-0.8B: on
|
||||
// "почему гром слышно позже молнии?" the reply came back exactly 400 characters
|
||||
// long, cut mid-word ("Нужно записать и,"), at every token cap from 256 to 2048.
|
||||
// So the token cap was never what stopped it — this rule was. 1000 characters is
|
||||
// roughly six Russian sentences, still short enough to stop a repetition loop.
|
||||
const responseGrammar = `
|
||||
root ::= "{" ws "\"response\"" ws ":" ws string ws "," ws "\"mood\"" ws ":" ws mood ws "}"
|
||||
mood ::= "\"neutral\"" | "\"happy\"" | "\"thinking\"" | "\"tired\"" | "\"confused\""
|
||||
string ::= "\"" ([^"\\] | "\\" ["\\/bfnrt]){0,400} "\""
|
||||
string ::= "\"" ([^"\\] | "\\" ["\\/bfnrt]){0,1000} "\""
|
||||
ws ::= [ \t\n]*
|
||||
`
|
||||
|
||||
@@ -633,21 +664,39 @@ type responseMood struct {
|
||||
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. Returns
|
||||
// ("", "") when no valid JSON is found.
|
||||
func parseResponseMood(raw string) (response, mood string) {
|
||||
// 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 {
|
||||
return "", ""
|
||||
if strings.HasPrefix(cleaned, "{") {
|
||||
return "", "", errBrokenJSON
|
||||
}
|
||||
return "", "", nil
|
||||
}
|
||||
var parsed responseMood
|
||||
if err := json.Unmarshal([]byte(cleaned[start:end+1]), &parsed); err != nil {
|
||||
return "", ""
|
||||
if e := json.Unmarshal([]byte(cleaned[start:end+1]), &parsed); e != nil {
|
||||
if strings.HasPrefix(cleaned, "{") {
|
||||
return "", "", errBrokenJSON
|
||||
}
|
||||
return "", "", nil
|
||||
}
|
||||
return parsed.Response, parsed.Mood
|
||||
return parsed.Response, parsed.Mood, nil
|
||||
}
|
||||
|
||||
func parsePhrase(raw string) (body, summary string) {
|
||||
|
||||
Reference in New Issue
Block a user