From b49755302c3254bac41f30ff202bed5e45e12f4b Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 01:18:46 +0400 Subject: [PATCH] phraser: the wire structs and the grammar move to transport.go (V-397) Verbatim move of chatMsg, chatReq, chatResp, phraseRepeatPenalty, responseGrammar/ResponseGrammar, grammar() and logIfTruncated. The two senders follow. --- internal/phraser/llmphraser.go | 115 ------------------------------ internal/phraser/transport.go | 123 +++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 115 deletions(-) create mode 100644 internal/phraser/transport.go diff --git a/internal/phraser/llmphraser.go b/internal/phraser/llmphraser.go index c03e4eb..0a58948 100644 --- a/internal/phraser/llmphraser.go +++ b/internal/phraser/llmphraser.go @@ -507,121 +507,6 @@ func (p *LLMPhraser) PhraseReminder(ctx context.Context, d loop.ReminderDecision return delivery.PhrasedReminder{Decision: d, Body: body, Summary: summary, Mood: mood}, nil } -type chatMsg struct { - Role string `json:"role"` - Content string `json:"content"` -} - -type chatReq struct { - Model string `json:"model"` - Messages []chatMsg `json:"messages"` - Temperature float64 `json:"temperature"` - MaxTokens int `json:"max_tokens"` - // Grammar is llama-server's `grammar` field (GBNF). Same wiring as - // internal/llm.Req.Grammar. Empty ⇒ unconstrained sampling. - Grammar string `json:"grammar,omitempty"` - // RepeatPenalty — defence in depth behind the bounded grammar, not the fix - // for #531. This struct had no such field, so every caller through - // chatWithSystem ran at the server default of 1.0 while Replier.PhraseReply - // sent 1.3 through internal/llm and was protected by accident. Two wire - // structs that disagree about the sampler is the condition that let one - // path run away and the other not, and it should not survive as a - // difference nobody chose. - RepeatPenalty float64 `json:"repeat_penalty,omitempty"` -} - -// phraseRepeatPenalty — matches Replier.PhraseReply, which has sent 1.3 since -// it was written. The value is not tuned here and is not what stops the -// whitespace loop; the bounded ws rule is. It is here so the two phrasing -// paths sample alike. -const phraseRepeatPenalty = 1.3 - -// responseGrammar — GBNF constraining the model to the documented phrasing -// contract and nothing else: {"response": "", "mood": ""}. -// -// Without it a 0.8B answers roughly one chat turn in three with open reasoning -// as plain text ("Thinking Process:" …), which no tag-stripper can remove and -// which eats the token budget before the JSON closes. Modelled on -// routeGrammar in internal/router/llmrouter.go so the two read alike. -// -// text accepts ANY codepoint except the two JSON must escape and the control -// range — the replies are 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. -// -// The control range is excluded because a raw newline inside a JSON string is -// not JSON (Vikunja #537). The class used to be `[^"\\]`, which let the model -// write a multi-line reply that satisfied the grammar and then failed -// json.Unmarshal with "invalid character '\n' in string literal" — the object -// starts with "{", so it came back as errBrokenJSON and the case answered with -// an empty string. Sixty of the failures in the 2026-08-05 temperature sweep -// were that one error, and it never once hit the token cap, which is why the -// truncation reading was wrong. The escape alternatives are llama.cpp's own -// json.gbnf: a model that wants a line break must write \n, which parses. -// -// 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. -// -// ws is bounded for the same reason and it is the more expensive of the two. -// `*` let the model open the object and then satisfy ws with whitespace until -// max_tokens, which is 512 here: both interactive turns measured on 2026-08-04 -// decoded exactly 512 tokens and spent 24-30 seconds doing it, all of it -// whitespace (Vikunja #531). Nothing on this path sends a repeat penalty — -// chatReq had no field for one — so the sampler never broke the loop. {0,4} -// was measured: three runs, three clean stops at 33 tokens, no penalty needed. -const responseGrammar = ` -root ::= "{" ws "\"response\"" ws ":" ws string ws "," ws "\"mood\"" ws ":" ws mood ws "}" -mood ::= "\"neutral\"" | "\"happy\"" | "\"thinking\"" | "\"tired\"" | "\"confused\"" -string ::= "\"" ([^"\\\x00-\x1F] | "\\" ["\\/bfnrt] | "\\u" [0-9a-fA-F]{4}){0,1000} "\"" -ws ::= [ \t\n]{0,4} -` - -// ResponseGrammar exposes responseGrammar to the other callers that emit the -// same {"response","mood"} contract — cmd/mavend's reactive replier, which is -// parsed by the same two fields. One definition, so the two cannot drift. -const ResponseGrammar = responseGrammar - -// grammar returns the GBNF to attach to a phrasing request, or "" when the -// operator turned it off. -func (p *LLMPhraser) grammar() string { - if p.cfg.NoGrammar { - return "" - } - return responseGrammar -} - -type chatResp struct { - Choices []struct { - Message struct { - Content string `json:"content"` - Reasoning string `json:"reasoning"` - ReasoningContent string `json:"reasoning_content"` - } `json:"message"` - // FinishReason — "stop" when the model chose to end, "length" when the - // token cap cut it off. Parsed since #531, where two turns ran to the - // 512-token cap and both happened to parse anyway: the grammar had - // already closed the JSON, so a truncated generation was indistinguishable - // from a good one at every layer above this struct. - FinishReason string `json:"finish_reason"` - } `json:"choices"` -} - -// logIfTruncated says so when a generation stopped at the token cap. -// -// A cap hit is never routine. Either the model was looping, which is the #531 -// shape, or the reply was genuinely longer than maxTokens, which means she cut -// herself off mid-sentence. Both are worth a line, and neither produced one -// before: the caller sees a parsed string and cannot tell. -func logIfTruncated(where, reason string, maxTokens int) { - if reason == "length" { - log.Printf("phraser: %s hit the %d-token cap (finish_reason=length) — the reply is truncated, or the model was looping", where, maxTokens) - } -} - func (p *LLMPhraser) chat(ctx context.Context, userPrompt string) (string, error) { return p.chatWithSystem(ctx, p.systemPrompt(), userPrompt, 512) } diff --git a/internal/phraser/transport.go b/internal/phraser/transport.go new file mode 100644 index 0000000..ebeec7f --- /dev/null +++ b/internal/phraser/transport.go @@ -0,0 +1,123 @@ +// phraser/transport.go — the wire to one llama-server: the request and +// response shapes, the GBNF that binds the model to the reply contract, and +// the single POST every phrasing path in this package goes through. +package phraser + +import ( + "log" +) + +type chatMsg struct { + Role string `json:"role"` + Content string `json:"content"` +} + +type chatReq struct { + Model string `json:"model"` + Messages []chatMsg `json:"messages"` + Temperature float64 `json:"temperature"` + MaxTokens int `json:"max_tokens"` + // Grammar is llama-server's `grammar` field (GBNF). Same wiring as + // internal/llm.Req.Grammar. Empty ⇒ unconstrained sampling. + Grammar string `json:"grammar,omitempty"` + // RepeatPenalty — defence in depth behind the bounded grammar, not the fix + // for #531. This struct had no such field, so every caller through + // chatWithSystem ran at the server default of 1.0 while Replier.PhraseReply + // sent 1.3 through internal/llm and was protected by accident. Two wire + // structs that disagree about the sampler is the condition that let one + // path run away and the other not, and it should not survive as a + // difference nobody chose. + RepeatPenalty float64 `json:"repeat_penalty,omitempty"` +} + +// phraseRepeatPenalty — matches Replier.PhraseReply, which has sent 1.3 since +// it was written. The value is not tuned here and is not what stops the +// whitespace loop; the bounded ws rule is. It is here so the two phrasing +// paths sample alike. +const phraseRepeatPenalty = 1.3 + +// responseGrammar — GBNF constraining the model to the documented phrasing +// contract and nothing else: {"response": "", "mood": ""}. +// +// Without it a 0.8B answers roughly one chat turn in three with open reasoning +// as plain text ("Thinking Process:" …), which no tag-stripper can remove and +// which eats the token budget before the JSON closes. Modelled on +// routeGrammar in internal/router/llmrouter.go so the two read alike. +// +// text accepts ANY codepoint except the two JSON must escape and the control +// range — the replies are 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. +// +// The control range is excluded because a raw newline inside a JSON string is +// not JSON (Vikunja #537). The class used to be `[^"\\]`, which let the model +// write a multi-line reply that satisfied the grammar and then failed +// json.Unmarshal with "invalid character '\n' in string literal" — the object +// starts with "{", so it came back as errBrokenJSON and the case answered with +// an empty string. Sixty of the failures in the 2026-08-05 temperature sweep +// were that one error, and it never once hit the token cap, which is why the +// truncation reading was wrong. The escape alternatives are llama.cpp's own +// json.gbnf: a model that wants a line break must write \n, which parses. +// +// 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. +// +// ws is bounded for the same reason and it is the more expensive of the two. +// `*` let the model open the object and then satisfy ws with whitespace until +// max_tokens, which is 512 here: both interactive turns measured on 2026-08-04 +// decoded exactly 512 tokens and spent 24-30 seconds doing it, all of it +// whitespace (Vikunja #531). Nothing on this path sends a repeat penalty — +// chatReq had no field for one — so the sampler never broke the loop. {0,4} +// was measured: three runs, three clean stops at 33 tokens, no penalty needed. +const responseGrammar = ` +root ::= "{" ws "\"response\"" ws ":" ws string ws "," ws "\"mood\"" ws ":" ws mood ws "}" +mood ::= "\"neutral\"" | "\"happy\"" | "\"thinking\"" | "\"tired\"" | "\"confused\"" +string ::= "\"" ([^"\\\x00-\x1F] | "\\" ["\\/bfnrt] | "\\u" [0-9a-fA-F]{4}){0,1000} "\"" +ws ::= [ \t\n]{0,4} +` + +// ResponseGrammar exposes responseGrammar to the other callers that emit the +// same {"response","mood"} contract — cmd/mavend's reactive replier, which is +// parsed by the same two fields. One definition, so the two cannot drift. +const ResponseGrammar = responseGrammar + +// grammar returns the GBNF to attach to a phrasing request, or "" when the +// operator turned it off. +func (p *LLMPhraser) grammar() string { + if p.cfg.NoGrammar { + return "" + } + return responseGrammar +} + +type chatResp struct { + Choices []struct { + Message struct { + Content string `json:"content"` + Reasoning string `json:"reasoning"` + ReasoningContent string `json:"reasoning_content"` + } `json:"message"` + // FinishReason — "stop" when the model chose to end, "length" when the + // token cap cut it off. Parsed since #531, where two turns ran to the + // 512-token cap and both happened to parse anyway: the grammar had + // already closed the JSON, so a truncated generation was indistinguishable + // from a good one at every layer above this struct. + FinishReason string `json:"finish_reason"` + } `json:"choices"` +} + +// logIfTruncated says so when a generation stopped at the token cap. +// +// A cap hit is never routine. Either the model was looping, which is the #531 +// shape, or the reply was genuinely longer than maxTokens, which means she cut +// herself off mid-sentence. Both are worth a line, and neither produced one +// before: the caller sees a parsed string and cannot tell. +func logIfTruncated(where, reason string, maxTokens int) { + if reason == "length" { + log.Printf("phraser: %s hit the %d-token cap (finish_reason=length) — the reply is truncated, or the model was looping", where, maxTokens) + } +}