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.