Compare commits

..

1 Commits

Author SHA1 Message Date
kami 64e5f3bdc1 Score the chat, query and knowledge phrasing paths (#395)
The nudge fixture only covered nudges. The shared persona block now goes into
five prompts, and the three conversational ones were unmeasured — those are the
long free-form replies where a persona break is most likely.

Adds talk_v1.json (27 Russian cases, 9 per path) and ScoreTalk, reporting
per-path as well as per-check so a chat regression can be told apart from a
knowledge one. Reuses the persona checks; the nudge-only ones (length, mood,
no questions) are left out, since a chat reply is allowed 1-3 sentences and a
follow-up question. The LLM run is opt-in on MAVEN_LLM_URL as before.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ
2026-07-31 16:28:11 +04:00
6 changed files with 14 additions and 245 deletions
@@ -1,33 +0,0 @@
package eval
import (
"strings"
"testing"
)
// TestAddressReportsEveryBreak — the real reply from a nudge eval run broke in
// two ways at once and the check named only the plural. Both must print: a
// half-reported failure reads as a milder problem than it is.
func TestAddressReportsEveryBreak(t *testing.T) {
body := "Смотрите на его потребление воды."
res := checkAddress(body)
if res.Pass {
t.Fatalf("checkAddress passed %q", body)
}
for _, want := range []string{"смотрите", "его"} {
if !strings.Contains(res.Detail, want) {
t.Errorf("detail %q does not name %q", res.Detail, want)
}
}
}
// One word repeated is one problem, so the detail must not say it twice.
func TestAddressDeduplicates(t *testing.T) {
res := checkAddress("Вам стоит поесть, вам это нужно.")
if res.Pass {
t.Fatal("expected failure")
}
if n := strings.Count(res.Detail, "formal"); n != 1 {
t.Errorf("detail repeats the same break %d times: %q", n, res.Detail)
}
}
+7 -28
View File
@@ -434,26 +434,14 @@ func looksVerb(w string) bool {
func checkAddress(body string) Result {
words := addressWordRE.FindAllString(strings.ToLower(body), -1)
// Every break, not just the first. A bad reply usually breaks in more than
// one way at once — "Смотрите на его потребление воды" is a plural imperative
// AND third person about him — and reporting only the first hid the second,
// which made the failure look milder than it was.
var breaks []string
seen := map[string]bool{}
add := func(msg string) {
if seen[msg] {
return // the same word twice in one message is one problem, not two
}
seen[msg] = true
breaks = append(breaks, msg)
}
for i, w := range words {
if formalPronouns[w] {
add(fmt.Sprintf("formal %q — she says ты/тебя/тебе", w))
return Result{CheckAddress, false,
fmt.Sprintf("formal %q — she says ты/тебя/тебе", w)}
}
if pluralVerb(w) && !(i > 0 && prepositions[words[i-1]]) {
add(fmt.Sprintf("plural imperative %q — she uses the singular", w))
return Result{CheckAddress, false,
fmt.Sprintf("plural imperative %q — she uses the singular", w)}
}
}
@@ -467,26 +455,17 @@ func checkAddress(body string) Result {
if !unicode.Is(unicode.Cyrillic, []rune(p)[0]) && !isLatinWord(p) {
continue // punctuation
}
// pluralVerb as well as looksVerb: looksVerb knows the imperative in
// -й/-йте but not the -те plural ("смотрите"), so "Смотрите на его
// потребление воды" counted "смотрите" as the person being talked
// about and the "его" never printed. Third time a verb form has
// blinded this check — if a fourth turns up, the antecedent test
// wants a real morphology table, not another suffix.
if notAnAntecedent[p] || prepositions[p] || thirdPersonHim[p] || looksVerb(p) || pluralVerb(p) {
if notAnAntecedent[p] || prepositions[p] || thirdPersonHim[p] || looksVerb(p) {
continue
}
named = true
break
}
if !named {
add(fmt.Sprintf("third person %q with nobody else named — she talks to him, not about him", w))
return Result{CheckAddress, false,
fmt.Sprintf("third person %q with nobody else named — she talks to him, not about him", w)}
}
}
if len(breaks) > 0 {
return Result{CheckAddress, false, strings.Join(breaks, " + ")}
}
return Result{CheckAddress, true, ""}
}
+4 -16
View File
@@ -132,19 +132,10 @@ func TestLLMTalkBaseline(t *testing.T) {
p := phraser.NewLLMPhraserAt(base, cfg)
defer p.Close()
// Unreachable server is fatal here, not a logged warning, and that differs
// from the nudge test on purpose. PhraseNudge returns its errors, so a dead
// server there shows up honestly in the Errors column. PhraseChat and
// PhraseQuery do NOT: they swallow every failure and return a canned string
// ("поговорили.", "не знаю.", "вот что я нашла: …"). So on these three paths
// a dead server produces a full report with 0 errors and a terrible score —
// a number that looks like bad phrasing and is really no phrasing at all.
// Refusing to score without a confirmed model is the only guard available
// until the phraser reports its failures (Vikunja #397).
model, err := llm.ModelID(ctx, base)
if err != nil {
t.Fatalf("no model at %s: %v — refusing to score, these paths hide their errors "+
"and would report a plausible-looking result off a dead server", base, err)
t.Logf("could not read model id from %s: %v — report will say %q", base, err, llm.UnknownModel)
model = llm.UnknownModel
}
t.Logf("scoring model %s at %s", model, base)
@@ -154,10 +145,7 @@ func TestLLMTalkBaseline(t *testing.T) {
}
t.Log("\n" + rep.String() + "\nreplies:\n" + rep.Replies() + "\nfailures:\n" + rep.Failures())
// And again afterwards: the run takes minutes, and a server that died or got
// OOM-killed halfway through would leave the first cases scored and the rest
// silently canned. Checking only at the start would not catch that.
if _, err := llm.ModelID(ctx, base); err != nil {
t.Fatalf("model at %s went away during the run: %v — the score above is not trustworthy", base, err)
if rep.Errors == rep.Total {
t.Errorf("all %d cases errored — harness fault, not a measurement", rep.Total)
}
}
+3 -4
View File
@@ -7,7 +7,6 @@
"The owner is a man, addressed informally as ty, living alone with a home server. Every utterance is written the way he actually talks to her.",
"chat-formality-bait and chat-about-me exist to provoke the two persona breaks the nudge eval caught: the formal vy/vas plural, and talking about him in the third person.",
"want_any fragments are stems so Russian declension does not defeat the on-topic check. They are lowercased before comparison.",
"want_any is a plain substring test, so a fragment that is too short passes by accident: \"ты\" matches inside \"работы\", \"нет\" inside \"интернет\". Keep every fragment to three or more letters of a real stem.",
"Notes are written as the store would have them: short, first person, no punctuation discipline."
],
"cases": [
@@ -31,7 +30,7 @@
"id": "chat-about-me",
"path": "chat",
"utterance": "расскажи обо мне",
"want_any": ["теб"],
"want_any": [ы", "тебя", еб"],
"tags": ["persona-bait", "third-person"],
"note": "Baits the third person: she should say 'ты живёшь один', not 'он живёт один', as if reporting to somebody else."
},
@@ -119,7 +118,7 @@
"path": "query",
"utterance": "сколько я заплатил за домен?",
"notes": ["домен продлевается в марте", "хостинг оплачен на год вперёд"],
"want_any": ["домен", "не зна", "не указ"],
"want_any": ["домен", "не зна", "не указ", "нет"],
"tags": ["notes", "negative"],
"note": "The notes do not contain the price. The prompt tells her to say so; a made-up number is the failure being watched for."
},
@@ -204,7 +203,7 @@
"id": "know-dont-know",
"path": "knowledge",
"utterance": "как зовут моего соседа снизу?",
"want_any": ["не зна", "не мог"],
"want_any": ["не зна", "не мог", "нет"],
"tags": ["general", "negative"],
"note": "Unanswerable without notes. Admitting it beats inventing a name; watching for the invention."
},
-124
View File
@@ -1,124 +0,0 @@
package phraser
import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/kami/maven/internal/loop"
)
// grammarSpy stands in for llama-server: it records the grammar field of every
// request and always answers with a contract-shaped reply.
type grammarSpy struct {
srv *httptest.Server
grammars []string
}
func newGrammarSpy(t *testing.T) *grammarSpy {
t.Helper()
s := &grammarSpy{}
s.srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var req chatReq
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
t.Errorf("spy: decode request: %v", err)
}
s.grammars = append(s.grammars, req.Grammar)
w.Header().Set("Content-Type", "application/json")
w.Write([]byte(`{"choices":[{"message":{"content":"{\"response\": \"ага\", \"mood\": \"neutral\"}"}}]}`))
}))
t.Cleanup(s.srv.Close)
return s
}
// callAllPhrasingPaths hits every path that expects the JSON contract.
func callAllPhrasingPaths(t *testing.T, p *LLMPhraser) {
t.Helper()
ctx := context.Background()
if _, err := p.PhraseNudge(ctx, loop.Candidate{Rule: loop.WaterRule(), Severity: loop.Sev1}); err != nil {
t.Fatalf("PhraseNudge: %v", err)
}
if _, err := p.PhraseChat(ctx, "привет", nil); err != nil {
t.Fatalf("PhraseChat: %v", err)
}
// Both branches: no notes (general knowledge) and with notes (grounded).
if _, err := p.PhraseQuery(ctx, "сколько воды я выпил", nil); err != nil {
t.Fatalf("PhraseQuery (no notes): %v", err)
}
if _, err := p.PhraseQuery(ctx, "сколько воды я выпил", []string{"два литра"}); err != nil {
t.Fatalf("PhraseQuery (notes): %v", err)
}
}
func TestGrammarIsAttachedToEveryPhrasingRequest(t *testing.T) {
if strings.TrimSpace(responseGrammar) == "" {
t.Fatal("responseGrammar is empty")
}
spy := newGrammarSpy(t)
p := NewLLMPhraserAt(spy.srv.URL, Config{})
callAllPhrasingPaths(t, p)
if len(spy.grammars) != 4 {
t.Fatalf("expected 4 requests, got %d", len(spy.grammars))
}
for i, g := range spy.grammars {
if g != responseGrammar {
t.Errorf("request %d carries grammar %q, want responseGrammar", i, g)
}
}
}
func TestNoGrammarConfigDisablesIt(t *testing.T) {
spy := newGrammarSpy(t)
p := NewLLMPhraserAt(spy.srv.URL, Config{NoGrammar: true})
callAllPhrasingPaths(t, p)
for i, g := range spy.grammars {
if g != "" {
t.Errorf("request %d still carries a grammar with NoGrammar set: %q", i, g)
}
}
}
// The grammar's string rule must accept any codepoint, not just ASCII. Replies
// are Russian: an ASCII-only class would constrain the model into empty replies.
func TestGrammarStringRuleIsNotASCIIOnly(t *testing.T) {
if !strings.Contains(responseGrammar, `([^"\\] | "\\" ["\\/bfnrt])`) {
t.Error("string rule is not the any-codepoint-except-quote-and-backslash class; Cyrillic replies would be impossible")
}
}
// What the grammar describes must survive the parser that reads it back — a
// 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)
if want := "он сказал \"привет\" и ушёл.\nвот так."; text != want {
t.Errorf("response = %q, want %q", text, want)
}
if mood != "confused" {
t.Errorf("mood = %q, want confused", mood)
}
}
// Every mood the grammar permits is one the contract knows, and all five are there.
func TestGrammarMoodEnumMatchesTheContract(t *testing.T) {
for _, m := range []string{"neutral", "happy", "thinking", "tired", "confused"} {
if !strings.Contains(responseGrammar, `"\"`+m+`\""`) {
t.Errorf("mood %q missing from the grammar", m)
}
}
// No sixth mood: the enum line lists exactly five alternatives.
for _, line := range strings.Split(responseGrammar, "\n") {
if strings.HasPrefix(line, "mood") {
if n := strings.Count(line, "|") + 1; n != 5 {
t.Errorf("mood rule lists %d alternatives, want 5: %s", n, line)
}
}
}
}
-40
View File
@@ -45,13 +45,6 @@ type Config struct {
// address him, the time) fresh for each turn. See internal/persona.
// nil ⇒ no block, the prompts stand alone.
ContextBlock func() string
// NoGrammar turns the GBNF constraint off (zero value ⇒ grammar ON).
// The escape hatch exists because the target resident model — the
// locally CPT'd Qwen3-1.7B — does not exist yet: if its chat template
// ever fights the grammar, the fix should be a config flip on the
// deploy box, not a code change and a rebuild.
NoGrammar bool
}
func DefaultConfig(modelPath string) Config {
@@ -297,7 +290,6 @@ func (p *LLMPhraser) chatWithMessages(ctx context.Context, msgs []chatMsg, maxTo
Messages: msgs,
Temperature: 0.7,
MaxTokens: maxTokens,
Grammar: p.grammar(),
}
body, err := json.Marshal(req)
if err != nil {
@@ -377,37 +369,6 @@ type chatReq struct {
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"`
}
// responseGrammar — GBNF constraining the model to the documented phrasing
// contract and nothing else: {"response": "<text>", "mood": "<enum>"}.
//
// 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 — 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.
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} "\""
ws ::= [ \t\n]*
`
// 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 {
@@ -432,7 +393,6 @@ func (p *LLMPhraser) chatWithSystem(ctx context.Context, system, user string, ma
},
Temperature: 0.7,
MaxTokens: maxTokens,
Grammar: p.grammar(),
}
body, err := json.Marshal(req)
if err != nil {