diff --git a/internal/phraser/broken_json_test.go b/internal/phraser/broken_json_test.go new file mode 100644 index 0000000..c8772b7 --- /dev/null +++ b/internal/phraser/broken_json_test.go @@ -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)") + } +} diff --git a/internal/phraser/grammar_test.go b/internal/phraser/grammar_test.go index fe262a6..bf6e713 100644 --- a/internal/phraser/grammar_test.go +++ b/internal/phraser/grammar_test.go @@ -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) } diff --git a/internal/phraser/llmphraser.go b/internal/phraser/llmphraser.go index d6ad483..3fc90c3 100644 --- a/internal/phraser/llmphraser.go +++ b/internal/phraser/llmphraser.go @@ -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) {