From 27bb9119fb6c486d0fea60b4c5a25c7cc93c4950 Mon Sep 17 00:00:00 2001 From: claude Date: Wed, 5 Aug 2026 22:34:01 +0400 Subject: [PATCH 1/3] clarify steps aside when the next turn is its own request (V-554) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A parked question consumed whatever came next. One act she could not fulfil ate three turns: "выключи свет в спальне" asked "Что сделать?", and "кто изобрёл телефон" was scored as an answer to it, then "как дела" after that. Nothing tested whether the words could be an answer. The test is two offline token checks that already existed for other callers: a question shape, or a capture verb. It fires only where the answer filled nothing, so an answer that closes the gap still lands whatever shape it has, and the retry budget is untouched — the count was never the problem. --- cmd/mavend/clarify.go | 31 ++++++++++++++++++++ cmd/mavend/clarify_test.go | 56 +++++++++++++++++++++++++++++++++++++ internal/router/question.go | 18 ++++++++++++ 3 files changed, 105 insertions(+) diff --git a/cmd/mavend/clarify.go b/cmd/mavend/clarify.go index c9188d2..837933d 100644 --- a/cmd/mavend/clarify.go +++ b/cmd/mavend/clarify.go @@ -179,6 +179,20 @@ func (h *reactiveHandler) askClarify(ctx context.Context, dec router.Decision) ( // she parked — no second parser. If it still does not fill the gap she asks // again, up to MaxAttempts; after that she says out loud that she did not // understand. She never drops the request in silence. +// isOwnRequest reports whether an utterance asks for something in its own +// right, which is what a clarify answer never does. Two offline tests over +// tokens, both already written for other callers: a question shape, and a +// capture verb. Cheap on purpose — this runs on the answer to every parked +// question, and it must not cost a model call. +// +// It is not a general relevance test. A bare noun that answers nothing ("синий" +// after "Что сделать?") is still treated as an answer and still re-asked, and +// that is the intended shape: only an utterance that carries its own request +// wins over the question in front of it. +func isOwnRequest(text string) bool { + return router.IsQuestionShaped(text) || router.CarriesCaptureVerb(text) +} + func (h *reactiveHandler) resolveClarifyAnswer(ctx context.Context, text string) (string, bool) { if h.clarifyStore == nil { return "", false @@ -191,6 +205,23 @@ func (h *reactiveHandler) resolveClarifyAnswer(ctx context.Context, text string) intent := router.Intent(q.Intent) answer := h.extractor.Extract(ctx, intent, text, h.now()) merged := q.Answer(text, toDialogueSlots(answer)) + // He moved on. A parked question used to swallow whatever came next, so one + // act she could not fulfil ate the following three turns: "выключи свет в + // спальне" asked "Что сделать?", and "кто изобрёл телефон" was scored as an + // answer to it, then "как дела" after that (Vikunja #554). Nothing checked + // whether the words could be an answer at all. + // + // Deliberately narrow. It only fires where the answer filled nothing, so a + // turn that closes the gap is still an answer whatever shape it has, and + // the retry budget is untouched — the count was never the problem. Dropping + // the question and routing the utterance as itself is what he meant either + // way: if he really was answering, he can say it again, and if he was not, + // he gets the thing he asked for instead of being asked a third time. + if len(dialogue.StillMissing(q.Missing, merged)) > 0 && isOwnRequest(text) { + h.clarifyStore.Delete(dialogueIDOf(ctx)) + log.Printf("voice: clarify — %q is its own request, not an answer to %v; dropping the question", text, q.Missing) + return "", false + } // Fold a newly answered subject into the raw utterance. Downstream actions // phrase from Utterance, not from the text slot — actionReminder stores it // as the reminder payload — so a reminder clarified out of a bare "напомни" diff --git a/cmd/mavend/clarify_test.go b/cmd/mavend/clarify_test.go index 56b0722..66e235c 100644 --- a/cmd/mavend/clarify_test.go +++ b/cmd/mavend/clarify_test.go @@ -564,3 +564,59 @@ func TestARestartExpiresTheParkedQuestion(t *testing.T) { t.Fatalf("notice = %q, want silence: nothing survived to expire", notice) } } + +// TestClarifyStepsAsideForItsOwnRequest — Vikunja #554. An act she could not +// fulfil parked "Что сделать?", and the three turns after it were scored as +// answers to that question: a world question, then "как дела", then the give-up +// line. None of them was ever an answer. +func TestClarifyStepsAsideForItsOwnRequest(t *testing.T) { + ctx := context.Background() + h, _, _ := newClarifyHandler(t) + + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentAct, router.Slots{Text: "выключи свет в спальне"}, "выключи свет в спальне")); !asked { + t.Fatal("an act with no fn should be asked about") + } + if reply, handled := h.resolveClarifyAnswer(ctx, "кто изобрёл телефон"); handled { + t.Fatalf("a world question must route as itself, got %q", reply) + } + if h.clarifyStore.Get(voiceDialogueID, h.now()) != nil { + t.Error("the parked question must be dropped, not left to eat the turn after this one") + } +} + +// TestClarifyStillRetriesOnAnAnswerThatMissed — the other half of #554, and the +// reason the test above is narrow. A bare noun answers nothing either, but it +// carries no request of its own, so she asks again as before. +func TestClarifyStillRetriesOnAnAnswerThatMissed(t *testing.T) { + ctx := context.Background() + h, _, _ := newClarifyHandler(t) + + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{Text: "напомни позвонить маме"}, "напомни позвонить маме")); !asked { + t.Fatal("expected the time question") + } + reply, handled := h.resolveClarifyAnswer(ctx, "ага") + if !handled || reply == "" { + t.Fatalf("a missed answer must still be re-asked, handled=%v reply=%q", handled, reply) + } + if h.clarifyStore.Get(voiceDialogueID, h.now()) == nil { + t.Error("the question must survive a missed answer") + } +} + +// TestClarifyQuestionShapedAnswerThatFillsTheGapStillLands — the guard runs only +// where nothing was filled. "во сколько?" is question-shaped and is also how a +// time gets said back, so an answer that closes the gap wins whatever its shape. +func TestClarifyQuestionShapedAnswerThatFillsTheGapStillLands(t *testing.T) { + ctx := context.Background() + h, st, _ := newClarifyHandler(t) + + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{Text: "напомни позвонить маме"}, "напомни позвонить маме")); !asked { + t.Fatal("expected the time question") + } + if reply, handled := h.resolveClarifyAnswer(ctx, "а что если в 11:00"); !handled || reply == clarifyGaveUp { + t.Fatalf("an answer that fills the gap must land, handled=%v reply=%q", handled, reply) + } + if reminders, err := st.DueReminders(ctx, h.now().Add(48*time.Hour)); err != nil || len(reminders) != 1 { + t.Fatalf("reminder was not created: reminders=%v err=%v", reminders, err) + } +} diff --git a/internal/router/question.go b/internal/router/question.go index 356e0af..b50a050 100644 --- a/internal/router/question.go +++ b/internal/router/question.go @@ -26,6 +26,24 @@ var ( captureVerbs = lexicon.CaptureVerbs() ) +// CarriesCaptureVerb reports whether text tells Maven to write something down. +// Sibling of IsQuestionShaped and matched over the same tokens, and the two do +// not overlap: IsQuestionShaped returns false for anything this returns true +// for, because "запиши что я пил воду" is a capture and not a question. +// +// Both exist together so a caller can ask "is this its own request?" — a +// clarify answer that asks a question or orders a capture is not an answer +// (Vikunja #554). +func CarriesCaptureVerb(text string) bool { + toks := planTokens(strings.TrimSpace(text)) + for _, v := range captureVerbs { + if hasTok(toks, v) { + return true + } + } + return false +} + // IsQuestionShaped reports whether text asks for something rather than // records it. It is a deterministic offline test over tokens, so it costs // nothing and never depends on the model that produced the routing decision. From de4c47459a763dd31f557aa706150aaa2dd51a1b Mon Sep 17 00:00:00 2001 From: claude Date: Wed, 5 Aug 2026 22:36:10 +0400 Subject: [PATCH 2/3] the personal boundary lets a narrative world question through (V-554) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "расскажи про Байкал" was refused as his by 0.0052. Every world seed opened with an interrogative, so a world question phrased as an order landed nearer "я тебе рассказывал об этом?" — the same verb about his own words. Four narrative seeds on the world side. TestONNXPersonalBoundary 25/25 -> 29/29 on held-out utterances, and the control "я рассказывал тебе про байкал?" is still his. TestONNXTopics unchanged at 34/34. --- cmd/mavend/personalboundary.go | 9 +++++++++ cmd/mavend/personalboundary_test.go | 7 +++++++ 2 files changed, 16 insertions(+) diff --git a/cmd/mavend/personalboundary.go b/cmd/mavend/personalboundary.go index d71af54..d239bdd 100644 --- a/cmd/mavend/personalboundary.go +++ b/cmd/mavend/personalboundary.go @@ -94,6 +94,15 @@ var worldSeeds = []string{ // topic seeds had already let them past the weather source. "какой сегодня курс валют", "что сегодня происходит в мире", + // The narrative shape (Vikunja #554). "расскажи про Байкал" was refused as + // his by 0.0052, and nothing here was phrased as an order rather than a + // question: every world seed above opens with an interrogative. So a world + // question that names its subject and asks for prose landed nearer "я тебе + // рассказывал об этом?", which is the same verb about his own words. + "расскажи про байкал", + "расскажи про древний рим", + "объясни как работает двигатель", + "tell me about the roman empire", } // personalBoundary holds the embedded seeds. Zero value is usable and means diff --git a/cmd/mavend/personalboundary_test.go b/cmd/mavend/personalboundary_test.go index 4c62047..cc21295 100644 --- a/cmd/mavend/personalboundary_test.go +++ b/cmd/mavend/personalboundary_test.go @@ -82,6 +82,13 @@ func TestONNXPersonalBoundary(t *testing.T) { {"какой сегодня праздник", false}, {"что интересного произошло сегодня в мире", false}, {"кто выиграл вчера матч", false}, + // The narrative shape, held out from the seeds above (Vikunja #554). + // The control is the row after them: the same verb about his own words + // is still his. + {"расскажи про эверест", false}, + {"расскажи про войну 1812 года", false}, + {"объясни что такое инфляция", false}, + {"я рассказывал тебе про байкал?", true}, } h := &reactiveHandler{recall: recallWiring{embedder: emb}} From e94c868160da77261cbe2596c54d0367ad003d01 Mon Sep 17 00:00:00 2001 From: claude Date: Wed, 5 Aug 2026 22:40:28 +0400 Subject: [PATCH 3/3] a chat prompt says which turn to answer (V-554) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prior turns were joined with newlines and nothing else, so the model got four unlabelled lines and no way to tell which one was the question. It answered an earlier one: asked "как дела" after a question about the telephone, she carried on about the telephone. Four turns live for fifteen minutes, so the line she answered was often minutes old. One user message still, because the template constraint that forced the flattening is real. The turns are labelled as his own earlier words and the current utterance is named as the one to answer. With no history the message is the utterance alone, unchanged. --- internal/phraser/llmphraser.go | 37 ++++++++++++++---- internal/phraser/llmphraser_chatmsg_test.go | 42 +++++++++++++++++++++ 2 files changed, 71 insertions(+), 8 deletions(-) create mode 100644 internal/phraser/llmphraser_chatmsg_test.go diff --git a/internal/phraser/llmphraser.go b/internal/phraser/llmphraser.go index 913210c..84d486b 100644 --- a/internal/phraser/llmphraser.go +++ b/internal/phraser/llmphraser.go @@ -512,19 +512,40 @@ func (p *LLMPhraser) PhraseQuery(ctx context.Context, utterance string, notes [] // message array from dialogue history + the current user utterance. On any LLM // error it returns both ChatFallback and the error, on the same rule as // PhraseQuery: the fallback keeps the turn alive, the error stays visible. +// chatUserMessage folds the prior turns and the current one into a single user +// message, because some chat templates (Ministral and others) reject two user +// turns in a row. That constraint is real; what was wrong is how it was met. +// +// The turns used to be joined with newlines and nothing else, so the model was +// handed four unlabelled lines and no way to tell which one it was answering +// (Vikunja #554). It answered an earlier one, or answered all of them at once: +// asked "как дела" after a question about the telephone, she carried on about +// the telephone. Four turns live for fifteen minutes, so the wrong line was +// often several minutes old. +// +// The history holds only his own utterances, never her replies, so the label +// says so and stays in the second person the persona requires. With no history +// the message is the utterance alone, which is the common case and unchanged. +func chatUserMessage(utterance string, history []dialogue.Turn) string { + prior := make([]string, 0, len(history)) + for _, t := range history { + if s := strings.TrimSpace(t.Text); s != "" { + prior = append(prior, "- "+s) + } + } + if len(prior) == 0 { + return strings.TrimSpace(utterance) + } + return "Раньше ты говорил:\n" + strings.Join(prior, "\n") + + "\n\nОтветь только на то, что ты говоришь сейчас: " + strings.TrimSpace(utterance) +} + func (p *LLMPhraser) PhraseChat(ctx context.Context, utterance string, history []dialogue.Turn) (string, error) { sys := chatSystemPrompt(p.cfg.ContextBlock) msgs := []chatMsg{ {Role: "system", Content: sys}, } - // Combine history and current utterance into one user message. - // Some model chat templates (Ministral, etc.) reject consecutive user turns. - var combined string - for _, t := range history { - combined += t.Text + "\n" - } - combined += utterance - msgs = append(msgs, chatMsg{Role: "user", Content: strings.TrimSpace(combined)}) + msgs = append(msgs, chatMsg{Role: "user", Content: chatUserMessage(utterance, history)}) resp, err := p.chatWithMessages(ctx, msgs, 768) if err != nil { diff --git a/internal/phraser/llmphraser_chatmsg_test.go b/internal/phraser/llmphraser_chatmsg_test.go new file mode 100644 index 0000000..466efb5 --- /dev/null +++ b/internal/phraser/llmphraser_chatmsg_test.go @@ -0,0 +1,42 @@ +package phraser + +import ( + "strings" + "testing" + + "github.com/kami/maven/internal/dialogue" +) + +// TestChatUserMessageMarksWhichTurnToAnswer — Vikunja #554. Four prior turns +// were joined with newlines and nothing else, so nothing in the message said +// which line was the question. +func TestChatUserMessageMarksWhichTurnToAnswer(t *testing.T) { + got := chatUserMessage("как дела", []dialogue.Turn{ + {Text: "кто изобрёл телефон"}, + {Text: "а когда это было"}, + }) + if !strings.Contains(got, "кто изобрёл телефон") { + t.Error("the prior turns must survive — they are what anaphora reads") + } + now := strings.LastIndex(got, "как дела") + if now < strings.Index(got, "кто изобрёл телефон") { + t.Error("the current utterance must come last, after the turns it follows") + } + if !strings.Contains(got, "Раньше ты говорил") { + t.Errorf("the prior turns must be labelled as prior: %q", got) + } + if strings.Contains(got, " вы ") || strings.Contains(got, "Вы ") { + t.Errorf("the persona addresses him informally: %q", got) + } +} + +// TestChatUserMessageWithNoHistoryIsJustTheUtterance — the common case must not +// grow a preamble that the model then has to see past. +func TestChatUserMessageWithNoHistoryIsJustTheUtterance(t *testing.T) { + if got := chatUserMessage(" привет ", nil); got != "привет" { + t.Errorf("chatUserMessage = %q, want %q", got, "привет") + } + if got := chatUserMessage("привет", []dialogue.Turn{{Text: " "}}); got != "привет" { + t.Errorf("a blank prior turn must not label anything: %q", got) + } +}