Merge the phraser sweep: a reminder summary cut inside a letter (#261)
Both PhraseReminder copies truncated with summary[:57] on a byte length. A Cyrillic letter is two bytes, so a Russian summary was cut at about 28 letters rather than 60, and byte 57 lands inside a letter roughly half the time. Sendable.Summary is what voicesink hands to piper and what the telegram sink posts, so the half letter was spoken and sent. The existing truncation test is ASCII-only, which is why the arithmetic survived. One shared reminderSummary counts runes now. Two phrasing paths returned an empty string with a nil error where the third had guarded it since it was written: the evidence branch of PhraseQuery and the bare-prose tail of PhraseChat. The daemon callers substitute a fallback on an empty reply, so the cost was confined to the eval, which scores an error as a failure but scored an empty reply as bad phrasing. Both return their fallback and errEmptyResponse now. Stub.PhraseReminder set no Mood where its sibling PhraseNudge documents the rule. Nothing reads it today. (V-620)
This commit is contained in:
@@ -78,3 +78,36 @@ func TestEmptyKnowledgeAnswerIsAnError(t *testing.T) {
|
||||
t.Errorf("error = %v; want it to name the empty response", err)
|
||||
}
|
||||
}
|
||||
|
||||
// The other two paths, which had no such guard. The evidence branch of
|
||||
// PhraseQuery and PhraseChat both returned ("", nil) off a server that produced
|
||||
// no tokens — an empty answer reported as a successful phrasing. The daemon's
|
||||
// callers check for the empty string and paper over it; the eval does not, and
|
||||
// scored a silent model as bad phrasing rather than as a failure.
|
||||
func TestEmptyAnswerIsAnErrorOnEveryPath(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.Write([]byte(`{"choices":[{"message":{"content":""}}]}`))
|
||||
}))
|
||||
t.Cleanup(srv.Close)
|
||||
p := NewLLMPhraserAt(srv.URL, Config{})
|
||||
|
||||
t.Run("evidence", func(t *testing.T) {
|
||||
got, err := p.PhraseQuery(context.Background(), "сколько воды я выпил", []string{"два литра"})
|
||||
if err == nil {
|
||||
t.Fatal("an empty response scored as an answer")
|
||||
}
|
||||
if !isFallback(t, fbQuerySources, "два литра", got) {
|
||||
t.Errorf("fallback text = %q, want a %q variant", got, fbQuerySources)
|
||||
}
|
||||
})
|
||||
t.Run("chat", func(t *testing.T) {
|
||||
got, err := p.PhraseChat(context.Background(), "как дела", nil)
|
||||
if err == nil {
|
||||
t.Fatal("an empty response scored as an answer")
|
||||
}
|
||||
if !isFallback(t, fbChat, "", got) {
|
||||
t.Errorf("fallback text = %q, want a %q variant", got, fbChat)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -299,6 +299,16 @@ func (p *LLMPhraser) PhraseQuery(ctx context.Context, utterance string, notes []
|
||||
if text != "" {
|
||||
return text, nil
|
||||
}
|
||||
if raw == "" {
|
||||
// Same guard the knowledge branch above has had since it was written,
|
||||
// and this branch did not: the server answered and the model wrote
|
||||
// nothing, which returned ("", nil) — an empty answer reported as a
|
||||
// successful phrasing. The daemon's callers happen to check for the
|
||||
// empty string, so it read as a silent fallback there; the eval scored
|
||||
// it as bad phrasing rather than as the failure it is, and nothing on
|
||||
// either path logged that the model had produced no tokens.
|
||||
return SourcesFallback(strings.Join(notes, "; ")), errEmptyResponse
|
||||
}
|
||||
return raw, nil
|
||||
}
|
||||
|
||||
@@ -375,7 +385,13 @@ func (p *LLMPhraser) PhraseChat(ctx context.Context, utterance string, history [
|
||||
if i := strings.IndexByte(resp, '\n'); i >= 0 {
|
||||
resp = resp[:i]
|
||||
}
|
||||
return strings.TrimSpace(resp), nil
|
||||
if resp = strings.TrimSpace(resp); resp == "" {
|
||||
// The model was up and wrote nothing. Same rule as PhraseQuery: the
|
||||
// fallback keeps the turn alive and the failure stays visible, rather
|
||||
// than ("", nil) telling the caller the chat path succeeded.
|
||||
return ChatFallback(), fmt.Errorf("phrase chat: %w", errEmptyResponse)
|
||||
}
|
||||
return resp, nil
|
||||
}
|
||||
|
||||
func (p *LLMPhraser) PhraseReminder(ctx context.Context, d loop.ReminderDecision) (delivery.PhrasedReminder, error) {
|
||||
@@ -414,11 +430,9 @@ func (p *LLMPhraser) PhraseReminder(ctx context.Context, d loop.ReminderDecision
|
||||
if mood == "" {
|
||||
mood = "neutral"
|
||||
}
|
||||
summary := body
|
||||
if len(summary) > 60 {
|
||||
summary = summary[:57] + "..."
|
||||
}
|
||||
return delivery.PhrasedReminder{Decision: d, Body: body, Summary: summary, Mood: mood}, nil
|
||||
return delivery.PhrasedReminder{
|
||||
Decision: d, Body: body, Summary: reminderSummary(body), Mood: mood,
|
||||
}, nil
|
||||
}
|
||||
|
||||
func (p *LLMPhraser) chat(ctx context.Context, userPrompt string) (string, error) {
|
||||
|
||||
@@ -115,11 +115,32 @@ func (s *Stub) PhraseReminder(_ context.Context, d loop.ReminderDecision) (deliv
|
||||
if text == "" {
|
||||
text = "reminder"
|
||||
}
|
||||
summary := text
|
||||
if len(summary) > 60 {
|
||||
summary = summary[:57] + "..."
|
||||
// Mood, for the same reason PhraseNudge sets it: the Stub is a production
|
||||
// fallback, so it owes the output contract a value. This one was left at the
|
||||
// zero value, which is not one of the five moods.
|
||||
return delivery.PhrasedReminder{
|
||||
Decision: d, Body: text, Summary: reminderSummary(text), Mood: "neutral",
|
||||
}, nil
|
||||
}
|
||||
|
||||
// summaryLimit — how much of a reminder goes to the away channels.
|
||||
const summaryLimit = 60
|
||||
|
||||
// reminderSummary shortens a reminder body to the away-channel summary.
|
||||
//
|
||||
// Counted in runes. Both copies of this counted bytes — `len(s) > 60` and
|
||||
// `s[:57]` — and on a Russian reminder that is wrong twice. A Cyrillic letter is
|
||||
// two bytes, so the cut fell at about 28 letters rather than 60; and byte 57
|
||||
// lands inside a letter about half the time, so the summary ended in half a
|
||||
// rune. That is not cosmetic: Sendable.Summary is the text voicesink hands to
|
||||
// piper and the text the telegram sink posts, so the broken byte was spoken and
|
||||
// sent.
|
||||
func reminderSummary(body string) string {
|
||||
r := []rune(body)
|
||||
if len(r) <= summaryLimit {
|
||||
return body
|
||||
}
|
||||
return delivery.PhrasedReminder{Decision: d, Body: text, Summary: summary}, nil
|
||||
return string(r[:summaryLimit-3]) + "..."
|
||||
}
|
||||
|
||||
// phraseNudge — the per-rule templates. each reads the context the predicate
|
||||
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
"unicode/utf8"
|
||||
|
||||
"github.com/kami/maven/internal/delivery"
|
||||
"github.com/kami/maven/internal/dialogue"
|
||||
@@ -185,6 +186,34 @@ func TestPhraseReminderTruncatesLongSummary(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// The same truncation, in the language she actually speaks. The test above is
|
||||
// ASCII, which is what let the byte arithmetic stand: `len(s) > 60` and `s[:57]`
|
||||
// cut a Russian reminder at about 28 letters instead of 60, and landed inside a
|
||||
// letter about half the time. Summary is what voicesink hands to piper and what
|
||||
// the telegram sink posts, so half a rune was spoken and sent.
|
||||
func TestPhraseReminderSummaryCountsRunesNotBytes(t *testing.T) {
|
||||
long := "позвонить маме и забрать посылку из пункта выдачи на соседней улице до восьми вечера"
|
||||
rd := loop.ReminderDecision{
|
||||
Reminder: store.Reminder{Payload: `{"text":"` + long + `"}`},
|
||||
State: loop.State{Now: time.Now().UTC()},
|
||||
}
|
||||
pr, _ := NewStub().PhraseReminder(context.Background(), rd)
|
||||
if !utf8.ValidString(pr.Summary) {
|
||||
t.Fatalf("summary is not valid UTF-8, it was cut mid-letter: %q", pr.Summary)
|
||||
}
|
||||
if n := utf8.RuneCountInString(pr.Summary); n > summaryLimit {
|
||||
t.Fatalf("summary = %d runes, want at most %d: %q", n, summaryLimit, pr.Summary)
|
||||
}
|
||||
// The cut must be near the limit, not near half of it. A byte count would
|
||||
// stop at 28 letters here.
|
||||
if n := utf8.RuneCountInString(pr.Summary); n < summaryLimit-5 {
|
||||
t.Fatalf("summary = %d runes, cut far too early — counted in bytes? %q", n, pr.Summary)
|
||||
}
|
||||
if pr.Mood == "" {
|
||||
t.Error("Mood is empty; the Stub is a production fallback and owes the contract a mood")
|
||||
}
|
||||
}
|
||||
|
||||
func TestPhraseReminderNonJSONPayload(t *testing.T) {
|
||||
// a payload that isn't JSON → the phraser falls back to the raw string.
|
||||
rd := loop.ReminderDecision{
|
||||
|
||||
Reference in New Issue
Block a user