diff --git a/cmd/mavend/clarify.go b/cmd/mavend/clarify.go index 1604d9d..d6efb09 100644 --- a/cmd/mavend/clarify.go +++ b/cmd/mavend/clarify.go @@ -33,19 +33,10 @@ var wantedSlots = map[router.Intent][]dialogue.Slot{ router.IntentAct: {dialogue.SlotFn}, } -// clarifyQuestions — one short question per missing slot. -// -// These are fixed templates, not model output. The resident model is a 0.8B; it -// would wander, and a question whose wording changes every time is harder to -// answer than a blunt one that always reads the same. They are infinitive -// questions, so there is no gender agreement to get wrong; the feminine -// self-reference lives in the reply she gives when she drops the request. -var clarifyQuestions = map[dialogue.Slot]string{ - dialogue.SlotTime: "Когда?", - dialogue.SlotText: "О чём напомнить?", - dialogue.SlotKey: "Что записать?", - dialogue.SlotFn: "Что сделать?", -} +// The questions themselves live in clarifytemplates.go, one list per slot, +// picked by attempt (Vikunja #457). The first ask is the short one this map +// used to hold; a re-ask says it differently, because a question he already +// failed to answer is the worst one to repeat unchanged. // clarifyGaveUp — she is out of questions and still does not have the slot. She // says so out loud: dropping the request in silence would leave him thinking it @@ -147,7 +138,7 @@ func clarifyQuestion(dec router.Decision) (dialogue.Slot, string, bool) { if len(missing) == 0 { return "", "", false } - q, ok := clarifyQuestions[missing[0]] + q, ok := clarifyQuestionFor(missing[0], 1) if !ok { return "", "", false } @@ -266,7 +257,9 @@ func (h *reactiveHandler) askRemainingGap(ctx context.Context, q *dialogue.Pendi if len(remaining) == 0 { return "", false } - question, ok := clarifyQuestions[remaining[0]] + // Attempts+1 is the question she is about to ask, and the budget is shared + // with the re-ask path, so the second gap is worded like a second try. + question, ok := clarifyQuestionFor(remaining[0], q.Attempts+1) if !ok || !q.CanAsk() { return "", false } @@ -290,7 +283,7 @@ func (h *reactiveHandler) askRemainingGap(ctx context.Context, q *dialogue.Pendi func (h *reactiveHandler) reaskOrGiveUp(ctx context.Context, q *dialogue.PendingQuestion, merged dialogue.Slots, text string) string { question := "" if len(q.Missing) > 0 { - question = clarifyQuestions[q.Missing[0]] + question, _ = clarifyQuestionFor(q.Missing[0], q.Attempts+1) } if question == "" || !q.CanAsk() { h.clarifyStore.Delete(dialogueIDOf(ctx)) diff --git a/cmd/mavend/clarify_test.go b/cmd/mavend/clarify_test.go index 6708d5b..36ce134 100644 --- a/cmd/mavend/clarify_test.go +++ b/cmd/mavend/clarify_test.go @@ -156,8 +156,14 @@ func TestClarifyAsksThreeTimesThenSaysSo(t *testing.T) { if !handled { t.Fatalf("answer %d must be consumed as an answer", i) } - if reply != "Когда?" { - t.Fatalf("attempt %d should ask again, got %q", i, reply) + // The wording changes with the attempt (Vikunja #457): repeating a + // question he already failed to answer is the worst way to ask it. + want, _ := clarifyQuestionFor(dialogue.SlotTime, i) + if reply != want { + t.Fatalf("attempt %d should ask again as %q, got %q", i, want, reply) + } + if first, _ := clarifyQuestionFor(dialogue.SlotTime, 1); reply == first { + t.Fatalf("attempt %d repeated the first wording: %q", i, reply) } if h.clarifyStore.Get(voiceDialogueID, h.now()) == nil { t.Fatalf("attempt %d must leave the question armed", i) @@ -349,8 +355,11 @@ func TestClarifyAsksAboutTheSecondGapToo(t *testing.T) { if !handled { t.Fatal("the answer must be consumed as an answer") } - if reply != "Когда?" { - t.Fatalf("a filled subject with no time must ask about the time, got %q", reply) + // Second gap, second attempt, so it is the second wording of the time + // question — the attempt budget is shared between the two paths. + want, _ := clarifyQuestionFor(dialogue.SlotTime, 2) + if reply != want { + t.Fatalf("a filled subject with no time must ask about the time as %q, got %q", want, reply) } q := h.clarifyStore.Get(voiceDialogueID, h.now()) if q == nil { @@ -411,8 +420,9 @@ func TestClarifyProseHoldsThePersona(t *testing.T) { eval.CheckCringe: true, } lines := append([]string{clarifyGaveUp}, clarifyExpiredVariants...) - for _, q := range clarifyQuestions { - lines = append(lines, q) + lines = append(lines, clarifyMissedVariants...) + for _, variants := range clarifyQuestionVariants { + lines = append(lines, variants...) } for _, line := range lines { for _, r := range eval.RunChecks(eval.Case{}, line, "neutral") { diff --git a/cmd/mavend/clarifytemplates.go b/cmd/mavend/clarifytemplates.go new file mode 100644 index 0000000..c080210 --- /dev/null +++ b/cmd/mavend/clarifytemplates.go @@ -0,0 +1,109 @@ +package main + +import ( + "github.com/kami/maven/internal/dialogue" + "github.com/kami/maven/internal/router" +) + +// The clarify copy deck (Vikunja #457). +// +// Every clarify turn used to say one sentence per gap, and a re-ask repeated +// that sentence word for word. A question he already failed to answer is the +// worst one to ask again unchanged: the second wording is the one that tells +// him which part she missed. +// +// Fixed templates, not model output, for the reason clarifyQuestions has always +// given: the resident model would wander, and a question whose wording changes +// at random is harder to answer than a blunt one. What changes here is that the +// wording varies with the attempt rather than with a die roll — the first ask is +// short, the second names the gap, the third spells it out. +// +// No schema_version, unlike internal/phraser/nudge_templates.go. These are Go +// constants compiled into the daemon, so there is no file that can drift out of +// step with the code that reads it. +// +// Persona holds: infinitive and imperative questions, so there is no gender +// agreement to get wrong, "ты" throughout, and no pet names. +var clarifyQuestionVariants = map[dialogue.Slot][]string{ + dialogue.SlotTime: { + "Когда?", + "Во сколько напомнить?", + "Скажи время — например, «в семь вечера» или «через час».", + }, + dialogue.SlotText: { + "О чём напомнить?", + "Что сказать тебе в это время?", + "Скажи одной фразой, о чём напомнить.", + }, + dialogue.SlotKey: { + "Что записать?", + "Что именно отметить?", + "Назови, что записать — например, «выпил воды».", + }, + dialogue.SlotFn: { + "Что сделать?", + "Какое действие выполнить?", + "Назови действие — я умею только то, что ты мне разрешил.", + }, +} + +// clarifyQuestionFor picks the wording for this attempt. attempt is 1-based, as +// PendingQuestion.Attempts counts it; anything past the list uses the last and +// most explicit phrasing rather than wrapping round to the short one, because +// wrapping would ask the same short question he has already not answered. +// +// Deterministic on purpose, unlike clarifyExpiredLine: an expiry notice is the +// same statement however it is worded, and a re-ask is not. +func clarifyQuestionFor(slot dialogue.Slot, attempt int) (string, bool) { + variants, ok := clarifyQuestionVariants[slot] + if !ok || len(variants) == 0 { + return "", false + } + i := attempt - 1 + if i < 0 { + i = 0 + } + if i >= len(variants) { + i = len(variants) - 1 + } + return variants[i], true +} + +// clarifyMissedVariants — she is asking for the whole utterance again, because +// the gate fired on an intent with nothing identifiable to ask about (note, +// query, chat, system are not in wantedSlots). +// +// Rotated like the expiry lines and for the same reason: this is the line he +// hears whenever she misses him completely, so it is a line that repeats, and +// the same sentence every time is what makes a house assistant sound like a +// kiosk. All of them say the same two things — she did not catch it, and he +// should say it again — because the wording may vary and the meaning may not. +var clarifyMissedVariants = []string{ + "Не совсем поняла — скажи, пожалуйста, ещё раз.", + "Я тебя не разобрала. Повтори, пожалуйста.", + "Не уловила. Скажи это по-другому?", + "Прости, не поняла — попробуй сказать иначе.", +} + +// clarifyMissedFor picks a wording by the utterance itself, so the same words +// asked twice get the same answer and two different misses sound different. +// +// A hash, not rand: a test that drives an utterance twice must not depend on a +// die roll, and the point of rotating is only that consecutive misses differ. +func clarifyMissedFor(utterance string) string { + var sum int + for _, r := range utterance { + sum += int(r) + } + return clarifyMissedVariants[sum%len(clarifyMissedVariants)] +} + +// clarifyMissedLine is the canned reply for a clarify decision she cannot turn +// into a question. Returns "" for a decision that is not a clarify, so the +// caller keeps its own reply. +func clarifyMissedLine(dec router.Decision) string { + if !dec.Clarify { + return "" + } + return clarifyMissedFor(dec.Utterance) +} diff --git a/cmd/mavend/clarifytemplates_test.go b/cmd/mavend/clarifytemplates_test.go new file mode 100644 index 0000000..a35bbef --- /dev/null +++ b/cmd/mavend/clarifytemplates_test.go @@ -0,0 +1,63 @@ +package main + +import ( + "testing" + + "github.com/kami/maven/internal/dialogue" + "github.com/kami/maven/internal/router" +) + +// Every slot she can ask about has a wording for every attempt she is allowed, +// and no two attempts on one slot read the same. A deck with a repeated line is +// the defect this deck exists to fix (Vikunja #457). +func TestClarifyQuestionsVaryByAttempt(t *testing.T) { + for slot, variants := range clarifyQuestionVariants { + seen := map[string]bool{} + for _, v := range variants { + if v == "" { + t.Errorf("%s: empty wording in the deck", slot) + } + if seen[v] { + t.Errorf("%s: repeated wording %q", slot, v) + } + seen[v] = true + } + for attempt := 1; attempt <= len(variants); attempt++ { + got, ok := clarifyQuestionFor(slot, attempt) + if !ok || got != variants[attempt-1] { + t.Errorf("%s attempt %d = %q ok=%v, want %q", slot, attempt, got, ok, variants[attempt-1]) + } + } + } +} + +// Past the end she keeps the most explicit wording. Wrapping round would ask +// the short question he has already not answered twice. +func TestClarifyQuestionPastTheEndKeepsTheLastWording(t *testing.T) { + last := clarifyQuestionVariants[dialogue.SlotTime][len(clarifyQuestionVariants[dialogue.SlotTime])-1] + for _, attempt := range []int{0, 4, 9} { + if got, _ := clarifyQuestionFor(dialogue.SlotTime, attempt); attempt > 1 && got != last { + t.Errorf("attempt %d = %q, want the last wording %q", attempt, got, last) + } + } + if _, ok := clarifyQuestionFor("nonesuch", 1); ok { + t.Error("an unknown slot must have no question") + } +} + +// The missed line is stable for one utterance and absent for a decision that is +// not a clarify. +func TestClarifyMissedLine(t *testing.T) { + d := router.Decision{Clarify: true, Utterance: "мгм"} + first := clarifyMissedLine(d) + if first == "" || first != clarifyMissedLine(d) { + t.Fatalf("the missed line must be stable for one utterance, got %q", first) + } + if got := clarifyMissedLine(router.Decision{Intent: router.IntentNote}); got != "" { + t.Errorf("a decision that is not a clarify got %q", got) + } + // The empty utterance still gets a line: she has to say something. + if got := clarifyMissedLine(router.Decision{Clarify: true}); got == "" { + t.Error("an empty utterance must still be answered out loud") + } +} diff --git a/cmd/mavend/replier_llm.go b/cmd/mavend/replier_llm.go index 20967dd..2c6aa5f 100644 --- a/cmd/mavend/replier_llm.go +++ b/cmd/mavend/replier_llm.go @@ -24,7 +24,12 @@ func newLLMReplier(c phraser.Completer, block func() string) *llmReplier { // answer from the stub, which is what keeps a turn from breaking on the model. func (r *llmReplier) Reply(d router.Decision) string { if d.Clarify { - return r.stub.Reply(d) + // The deck, not the stub's single sentence: a clarify she cannot turn + // into a question is the line he hears most often when she misses him, + // and it used to be the same words every time (Vikunja #457). Still no + // model call — this text has to be right every time, and it is not worth + // a generation to say something this small. + return clarifyMissedLine(d) } out, err := r.p.PhraseReply(context.Background(), d) if err != nil || out == "" { diff --git a/cmd/mavend/replier_llm_test.go b/cmd/mavend/replier_llm_test.go index fae6d39..0013dbb 100644 --- a/cmd/mavend/replier_llm_test.go +++ b/cmd/mavend/replier_llm_test.go @@ -37,9 +37,21 @@ func TestLLMReplierFallsBackToStubOnEmpty(t *testing.T) { assertStub(t, r, router.Decision{Intent: router.IntentNote}, "empty llm") } -func TestLLMReplierClarifyUsesStub(t *testing.T) { +// A clarify never reaches the model, and since Vikunja #457 it is answered from +// the clarify deck rather than the stub's single sentence. +func TestLLMReplierClarifyReadsTheDeck(t *testing.T) { r := newLLMReplier(stubCompleter{out: "я всё поняла"}, nil) - assertStub(t, r, router.Decision{Clarify: true}, "clarify") + got := r.Reply(router.Decision{Clarify: true, Utterance: "мгм"}) + if got == "я всё поняла" { + t.Fatal("a clarify must not be phrased by the model") + } + if want := clarifyMissedFor("мгм"); got != want { + t.Errorf("on clarify: got %q, want %q", got, want) + } + // Two different misses do not sound identical. + if same := r.Reply(router.Decision{Clarify: true, Utterance: "а"}); same == got { + t.Log("two utterances hashed to the same line, which is allowed but should be rare") + } } func assertStub(t *testing.T, r *llmReplier, d router.Decision, what string) {