diff --git a/cmd/mavend/clarify.go b/cmd/mavend/clarify.go index 5e78d6c..75d6691 100644 --- a/cmd/mavend/clarify.go +++ b/cmd/mavend/clarify.go @@ -17,7 +17,8 @@ import ( const clarifyTTL = 90 * time.Second // wantedSlots — what each intent needs before she can act on it. First entry is -// the one she asks about; the rest are only used to decide act-vs-drop. +// the one she asks about this turn; the rest are asked about on later turns, one +// per turn, as each answer lands (see askRemainingGap). // // Intents not listed here are never worth a question: note and query act on the // raw utterance, chat and system have nothing to fill in. For those a clarify @@ -25,8 +26,7 @@ const clarifyTTL = 90 * time.Second // is worse than admitting she missed it. // A reminder wants BOTH what to remind about and when. Subject first: "напомни // в 11" has a time and nothing to say at 11, and a reminder with no subject is -// not worth setting. Order here is the order she asks in — she still only asks -// about the first one missing. +// not worth setting. Order here is the order she asks in. var wantedSlots = map[router.Intent][]dialogue.Slot{ router.IntentReminder: {dialogue.SlotText, dialogue.SlotTime}, router.IntentFact: {dialogue.SlotKey}, @@ -140,7 +140,8 @@ func missingFor(dec router.Decision) []dialogue.Slot { // ("", false) when she has no idea what is missing. // // One question about one thing: if two slots are missing she asks about the -// first and lets the rest go. Two questions in a row is an interrogation. +// first only. Two questions in one breath is an interrogation. The second gap +// is picked up on the turn after the first one is answered (askRemainingGap). func clarifyQuestion(dec router.Decision) (dialogue.Slot, string, bool) { missing := missingFor(dec) if len(missing) == 0 { @@ -199,11 +200,27 @@ 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)) + // 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 "напомни" + // would fire at 11:00 saying "напомни" and nothing else. + q.Utterance = foldAnswerIntoUtterance(q.Utterance, merged.Text) if len(dialogue.StillMissing(q.Missing, merged)) > 0 { return h.reaskOrGiveUp(q, merged, text), true } h.clarifyStore.Delete(voiceDialogueID) + // One gap filled is not the same as a complete request. askClarify parks + // only the first gap, because one question per turn is the rule, but a + // reminder wants both a subject and a time. "напомни" with neither used to + // ask "О чём напомнить?", accept "позвонить маме", and then hand applyAction + // a reminder with no time, which answered "не получилось разобрать время + // напоминания." — an error for a request she never finished asking about. + // Re-enter the loop instead, one question at a time as before. + if reply, asked := h.askRemainingGap(q, intent, merged); asked { + return reply, true + } + // Rebuild the decision as if it had routed cleanly, then run it down the // normal path. Clarify is deliberately false and the intent is unchanged: // filling in an argument never grants authority, so the completed decision @@ -218,6 +235,55 @@ func (h *reactiveHandler) resolveClarifyAnswer(ctx context.Context, text string) return h.finishClarified(ctx, dec), true } +// foldAnswerIntoUtterance appends an answered subject to the original words, +// unless they already carry it. "напомни" + "позвонить маме" reads as the +// request he would have made in one breath. Nothing is appended when the +// subject is empty or already present, so re-asking the same question twice +// cannot grow the utterance. +func foldAnswerIntoUtterance(utterance, subject string) string { + subject = strings.TrimSpace(subject) + if subject == "" || strings.Contains(utterance, subject) { + return utterance + } + if strings.TrimSpace(utterance) == "" { + return subject + } + return strings.TrimSpace(utterance) + " " + subject +} + +// askRemainingGap re-parks the request when the answer closed one gap and +// wantedSlots still names another. Returns ("", false) when the request is +// complete, when there is no question for what is left, or when she is out of +// attempts — in all three the caller runs the decision as it stands, which for +// the out-of-attempts case is the old behaviour and is the right one: she has +// already asked enough. +// +// The attempt budget is shared with the re-ask path on purpose. A second gap +// costs a question exactly like a second try at the first one does, so the cap +// still bounds how many times she can speak before acting or letting go. +func (h *reactiveHandler) askRemainingGap(q *dialogue.PendingQuestion, intent router.Intent, merged dialogue.Slots) (string, bool) { + remaining := dialogue.StillMissing(wantedSlots[intent], merged) + if len(remaining) == 0 { + return "", false + } + question, ok := clarifyQuestions[remaining[0]] + if !ok || !q.CanAsk() { + return "", false + } + h.clarifyStore.Put(voiceDialogueID, &dialogue.PendingQuestion{ + Intent: q.Intent, + Slots: merged, + Missing: []dialogue.Slot{remaining[0]}, + Utterance: q.Utterance, + Asked: h.now(), + TTL: clarifyTTL, + Attempts: q.Attempts + 1, + MaxAttempts: q.MaxAttempts, + }) + log.Printf("voice: clarify — one gap filled, still missing %s for intent=%s, asking again (attempt %d)", remaining[0], intent, q.Attempts+1) + return question, true +} + // reaskOrGiveUp handles an answer that left the gap open: ask the same question // again while she has attempts left, otherwise say she did not understand and // let the request go. Never returns "" — a mute give-up reads as "done". diff --git a/cmd/mavend/clarify_test.go b/cmd/mavend/clarify_test.go index e280e32..af92268 100644 --- a/cmd/mavend/clarify_test.go +++ b/cmd/mavend/clarify_test.go @@ -330,3 +330,66 @@ func TestNoPendingQuestionFallsThrough(t *testing.T) { t.Fatalf("no open question ⇒ must not be treated as an answer, got %q", reply) } } + +// TestClarifyAsksAboutTheSecondGapToo — "напомни" with neither a subject nor a +// time. She asks about the subject, he gives it, and the request is still not +// complete. The old code handed applyAction a reminder with no time, which +// answered with a parse error for a question she never asked. +func TestClarifyAsksAboutTheSecondGapToo(t *testing.T) { + ctx := context.Background() + h, st, _ := newClarifyHandler(t) + + question, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{}, "напомни")) + if !asked || question != "О чём напомнить?" { + t.Fatalf("expected the subject question, got %q asked=%v", question, asked) + } + + reply, handled := h.resolveClarifyAnswer(ctx, "позвонить маме") + 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) + } + q := h.clarifyStore.Get(voiceDialogueID, h.now()) + if q == nil { + t.Fatal("the second gap must leave a question armed") + } + if q.Slots.Text == "" { + t.Fatalf("the re-parked question lost the answered subject: %+v", q.Slots) + } + + if reply, handled := h.resolveClarifyAnswer(ctx, "в 11:00"); !handled || reply == clarifyGaveUp { + t.Fatalf("the time answer must complete the reminder, handled=%v reply=%q", handled, reply) + } + reminders, err := st.DueReminders(ctx, h.now().Add(48*time.Hour)) + if err != nil || len(reminders) != 1 { + t.Fatalf("expected one reminder: %v err=%v", reminders, err) + } + if !strings.Contains(reminders[0].Payload, "маме") { + t.Fatalf("the reminder lost the subject: %q", reminders[0].Payload) + } +} + +// TestClarifySecondGapRespectsTheAttemptCap — the second gap spends a question +// out of the same budget, so it cannot turn a capped exchange into an endless +// one. With one attempt allowed she acts on what she has instead of asking. +func TestClarifySecondGapRespectsTheAttemptCap(t *testing.T) { + ctx := context.Background() + h, _, _ := newClarifyHandler(t) + h.clarifyMaxAttempts = 1 + + if _, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{}, "напомни")); !asked { + t.Fatal("expected the subject question") + } + reply, handled := h.resolveClarifyAnswer(ctx, "позвонить маме") + if !handled { + t.Fatal("the answer must be consumed") + } + if reply == "Когда?" { + t.Fatal("out of attempts she must not ask a second question") + } + if h.clarifyStore.Get(voiceDialogueID, h.now()) != nil { + t.Fatal("no question may stay armed past the cap") + } +}