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".

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")
+	}
+}