From b18f6085944753fc3fb232646712302178da6d31 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 4 Aug 2026 00:41:16 +0400 Subject: [PATCH] mavend, eval: use the phrasing errors the phraser now returns (V-397) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Call sites take the fallback text and log the error instead of treating a canned string as success. phraseSource drops the text entirely — its callers hold the passage and read it back better than "вот что я нашла: ". The talk scorer's before-and-after model probe (the #395 workaround) goes; the run now fails only when every case errored, which is the honest "nothing was measured" condition. TalkFixture gets its own schema version so the two fixtures can be versioned apart. Co-Authored-By: Claude Opus 5 --- cmd/mavend/actions.go | 4 ++++ cmd/mavend/actions_query.go | 8 +++++++- cmd/mavend/worldmodel.go | 4 ++++ internal/phraser/eval/talk.go | 10 ++++++++-- internal/phraser/eval/talk_test.go | 27 +++++++++++---------------- 5 files changed, 34 insertions(+), 19 deletions(-) diff --git a/cmd/mavend/actions.go b/cmd/mavend/actions.go index f6a9956..d262111 100644 --- a/cmd/mavend/actions.go +++ b/cmd/mavend/actions.go @@ -58,9 +58,13 @@ func (h *reactiveHandler) actionChat(ctx context.Context, dec router.Decision) s // Conversational: build history from dialogue session (prior user turns) // and let the LLM respond from general knowledge + context. history := h.chatHistory() + // The phraser hands back its own fallback text alongside the error, so the + // turn survives a dead server and the failure still reaches the log. reply, err := h.phraser.PhraseChat(ctx, dec.Utterance, history) if err != nil { log.Printf("voice: chat: %v", err) + } + if reply == "" { return "поговорили." } return reply diff --git a/cmd/mavend/actions_query.go b/cmd/mavend/actions_query.go index c349d2d..f2b65d0 100644 --- a/cmd/mavend/actions_query.go +++ b/cmd/mavend/actions_query.go @@ -445,7 +445,13 @@ func (h *reactiveHandler) queryMemory(ctx context.Context, t *queryTurn) (string // A note is phrased in Maven's voice; a fact is read back as it was // stored. if hit.Meta["type"] == "note" { - if reply, perr := h.phraser.PhraseQuery(ctx, t.dec.Utterance, []string{text}); perr == nil && reply != "" { + reply, perr := h.phraser.PhraseQuery(ctx, t.dec.Utterance, []string{text}) + switch { + case perr != nil: + // Reading the note back verbatim beats the phraser's own fallback, + // which only wraps the same text in "вот что я нашла:". + log.Printf("voice: recall phrase: %v", perr) + case reply != "": return reply, true } } diff --git a/cmd/mavend/worldmodel.go b/cmd/mavend/worldmodel.go index c6c712f..b76c63d 100644 --- a/cmd/mavend/worldmodel.go +++ b/cmd/mavend/worldmodel.go @@ -54,7 +54,11 @@ func (h *reactiveHandler) phraseSource(ctx context.Context, name, utterance stri log.Printf("voice: %s: no world model, reading the source back instead", name) return "" case err != nil: + // The resident phraser answers this call with its fallback text and the + // error together. Drop the text: these callers hold the passage itself + // and read it back better than "вот что я нашла: " does. log.Printf("voice: %s: phrase: %v", name, err) + return "" } return reply } diff --git a/internal/phraser/eval/talk.go b/internal/phraser/eval/talk.go index 6d239f8..08a4681 100644 --- a/internal/phraser/eval/talk.go +++ b/internal/phraser/eval/talk.go @@ -78,6 +78,12 @@ type TalkCase struct { Note string `json:"note,omitempty"` } +// TalkSchemaVersion — the version this loader understands. Separate from the +// nudge fixture's SchemaVersion: the two fixtures have different shapes and +// change on different days, and one shared constant would force a bump on the +// fixture that did not move. +const TalkSchemaVersion = 1 + // TalkFixture — the versioned envelope, same gating as Fixture. type TalkFixture struct { SchemaVersion int `json:"schema_version"` @@ -92,8 +98,8 @@ func LoadTalk() (TalkFixture, error) { if err := json.Unmarshal(talkFixtureJSON, &f); err != nil { return TalkFixture{}, fmt.Errorf("parse talk fixture: %w", err) } - if f.SchemaVersion != SchemaVersion { - return TalkFixture{}, fmt.Errorf("talk fixture schema_version %d, want %d", f.SchemaVersion, SchemaVersion) + if f.SchemaVersion != TalkSchemaVersion { + return TalkFixture{}, fmt.Errorf("talk fixture schema_version %d, want %d", f.SchemaVersion, TalkSchemaVersion) } if len(f.Cases) == 0 { return TalkFixture{}, fmt.Errorf("talk fixture has no cases") diff --git a/internal/phraser/eval/talk_test.go b/internal/phraser/eval/talk_test.go index 7b993c9..72b4810 100644 --- a/internal/phraser/eval/talk_test.go +++ b/internal/phraser/eval/talk_test.go @@ -142,19 +142,13 @@ 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). + // The model id names the run in the report. Since Vikunja #397 every path + // returns its errors, so a server that dies mid-run shows up in the Errors + // column instead of scoring as bad phrasing — the before-and-after probe that + // used to stand in for that is gone. 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.Fatalf("no model at %s: %v", base, err) } t.Logf("scoring model %s at %s", model, base) @@ -169,10 +163,11 @@ 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) + // A run where nothing was phrased is not a low score, it is no measurement. + if rep.Errors == rep.Total { + t.Fatalf("every case errored — nothing was measured, the score above is not a phrasing result") + } + if rep.Errors > 0 { + t.Logf("%d/%d cases errored — those are model failures, not phrasing failures", rep.Errors, rep.Total) } }