Merge: a missing slot asks, whatever the confidence (#206)

cmd/mavend/clarify_test.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/kami/maven/internal/dialogue"
 	"github.com/kami/maven/internal/ipc"
+	"github.com/kami/maven/internal/memory"
 	"github.com/kami/maven/internal/phraser/eval"
 	"github.com/kami/maven/internal/router"
 	"github.com/kami/maven/internal/store"
@@ -646,3 +647,79 @@ func TestUnresolvedActSaysItDoesNotKnowTheCommand(t *testing.T) {
 }
 
 
+
+// newRoutingClarifyHandler wires the real cascade (hash embedder, no model) onto
+// the clarify handler, so a test can drive handleText end to end and see which
+// gate claimed the turn.
+func newRoutingClarifyHandler(t *testing.T) (*reactiveHandler, *store.Store) {
+	t.Helper()
+	h, st, _ := newClarifyHandler(t)
+	h.router = buildRouter(router.NewHashEmbedder(1024), h.matcher, 0.55, nil)
+	h.recall = recallWiring{embedder: router.NewHashEmbedder(1024), memStore: memory.NewInMemoryStore()}
+	return h, st
+}
+
+// TestIncompleteReminderAsksInsteadOfFailing — Vikunja #557. "напомни позвонить"
+// is routed confidently and is still half a request. It used to reach applyAction,
+// fail on the missing time and park nothing, so the "в семь вечера" that followed
+// was routed as a world question and web-searched.
+func TestIncompleteReminderAsksInsteadOfFailing(t *testing.T) {
+	ctx := context.Background()
+	h, st := newRoutingClarifyHandler(t)
+
+	reply := h.handleText(ctx, "web", "напомни позвонить маме")
+	want, _ := clarifyQuestionFor(dialogue.SlotTime, 1)
+	if reply != want {
+		t.Fatalf("reply = %q, want the time question %q", reply, want)
+	}
+	if h.clarifyStore.Get(dialogueIDFor(sourceText, "web"), h.now()) == nil {
+		t.Fatal("the request must be parked, or the answer has nowhere to land")
+	}
+	if reply := h.handleText(ctx, "web", "в семь вечера"); strings.Contains(reply, "нашла") {
+		t.Fatalf("the answer to her own question must not be looked up: %q", reply)
+	}
+	if reminders, err := st.DueReminders(ctx, h.now().Add(48*time.Hour)); err != nil || len(reminders) != 1 {
+		t.Fatalf("the answer did not complete the reminder: reminders=%v err=%v", reminders, err)
+	}
+}
+
+// TestBareCaptureVerbAsksWhatToRecord — the other half of #557. A bare "запиши"
+// went to the resident model as chat, which agreed to a wording change nobody
+// asked for. It is a fact with no key, and that gap has a question.
+func TestBareCaptureVerbAsksWhatToRecord(t *testing.T) {
+	ctx := context.Background()
+	h, _ := newRoutingClarifyHandler(t)
+
+	reply := h.handleText(ctx, "web", "запиши")
+	want, _ := clarifyQuestionFor(dialogue.SlotKey, 1)
+	if reply != want {
+		t.Fatalf("reply = %q, want %q", reply, want)
+	}
+	if h.clarifyStore.Get(dialogueIDFor(sourceText, "web"), h.now()) == nil {
+		t.Fatal("the request must be parked so the next utterance completes it")
+	}
+}
+
+// TestACompleteTurnStillDoesNotAsk — the gate reads a missing slot, not any
+// slot, so a request she can act on must never turn into a question. Checked on
+// the decision rather than through the cascade: what is at stake is the gate's
+// condition, and driving it through the hash embedder would measure routing.
+func TestACompleteTurnStillDoesNotAsk(t *testing.T) {
+	ctx := context.Background()
+	h, _, _ := newClarifyHandler(t)
+
+	complete := []router.Decision{
+		{Intent: router.IntentReminder, Slots: router.Slots{Text: "позвонить маме", HasTime: true}, Utterance: "напомни в 11 позвонить маме"},
+		{Intent: router.IntentFact, Slots: router.Slots{Key: "water", Value: "выпил", HasKey: true}, Utterance: "я выпил воды"},
+		{Intent: router.IntentNote, Slots: router.Slots{Text: "купить хлеб"}, Utterance: "запиши купить хлеб"},
+		{Intent: router.IntentQuery, Slots: router.Slots{Text: "что у меня сегодня"}, Utterance: "что у меня сегодня"},
+	}
+	for _, dec := range complete {
+		if gaps := missingFor(dec); len(gaps) > 0 {
+			t.Errorf("%q reads as incomplete: %v", dec.Utterance, gaps)
+		}
+		if reply, asked := h.askClarify(ctx, dec); asked {
+			t.Errorf("%q was answered with a question: %q", dec.Utterance, reply)
+		}
+	}
+}

cmd/mavend/voice.go

@@ -366,10 +366,16 @@ func (h *reactiveHandler) runTurn(ctx context.Context, text string, src turnSour
 		}
 	}
 
-	// 7. clarify — she is not sure. If one named thing is missing, ask about it
+	// 7. clarify — something she needs is missing. If one named thing is missing,
-	// and park the request (clarify.go); otherwise the replier's canned reply
+	// ask about it and park the request (clarify.go); otherwise the replier's
-	// stands.
+	// canned reply stands.
-	if dec.Clarify {
+	//
+	// Not gated on dec.Clarify alone (Vikunja #557). A turn the cascade routed
+	// confidently but incompletely skipped this entirely: "напомни позвонить"
+	// reached applyAction, failed on the missing time, parked nothing, and the
+	// "в семь вечера" that followed was web-searched as a world question. A
+	// required slot that missingFor names is a gap whatever the confidence.
+	if dec.Clarify || len(missingFor(dec)) > 0 {
 		if reply := h.hexisBeforeClarify(ctx, dec); reply != "" {
 			return withNotice(expiredNotice, reply)
 		}

cmd/mavend/voicewire.go

@@ -404,6 +404,10 @@ func buildRouter(emb router.Embedder, acts router.ActMatcher, threshold float64,
 	// board noun), and before the capture marker, which would otherwise read
 	// "убери из задач купить молоко" as a new task (Vikunja #512).
 	grammars = append(grammars, router.TaskStatusGrammar())
+	// Before the capture markers, which all need an object. A capture verb
+	// alone is a fact with no key, and the clarify path asks for it rather than
+	// letting the model invent an answer (Vikunja #557).
+	grammars = append(grammars, router.BareCaptureGrammar()...)
 	grammars = append(grammars, router.TaskCaptureGrammar())
 	// After the capture marker, so "запиши" still wins over "расскажи", and
 	// last overall because it matches on the first word alone: "расскажи про

internal/router/barecapture.go

@@ -0,0 +1,51 @@
+package router
+
+import (
+	"regexp"
+	"strings"
+)
+
+// BareCaptureGrammar — a capture verb with nothing after it is a fact she has
+// yet to hear, not a conversation (Vikunja #557).
+//
+// A bare "запиши" reached the resident model as chat, and the model answered by
+// agreeing to something nobody asked for: "Я поняла, теперь я буду говорить
+// «записала» или «записала заметку»." It read its own instruction block as the
+// subject of the turn. Whatever the routing, that reply is invented.
+//
+// The route this rule asks for is a fact with no key, which is a gap the clarify
+// path already has copy for ("Что записать?"). So the rule does not answer the
+// turn — it hands it to the one mechanism that asks.
+//
+// Wired before TaskCaptureGrammar, whose pattern needs an object, so it can
+// never claim this shape. There is nothing to disambiguate: the whole utterance
+// is one verb from a closed lexicon.
+func BareCaptureGrammar() []Grammar {
+	return []Grammar{{
+		Name:    "bare-capture",
+		Pattern: bareCapturePattern,
+		Build:   bareCaptureBuild,
+	}}
+}
+
+// Anchored at both ends, so only the verb and punctuation are in the utterance.
+// Trailing "-ка" and "пожалуйста" are the same request said politely.
+var bareCapturePattern = regexp.MustCompile(`(?i)^\s*([\p{L}]+)(?:-ка)?[,\s]*(?:пожалуйста)?[\s.!?]*$`)
+
+func bareCaptureBuild(m []string) (Decision, bool) {
+	word := strings.ToLower(strings.TrimSpace(m[1]))
+	for _, v := range captureVerbs {
+		if word != v {
+			continue
+		}
+		// No key, no text: she was told to record and not what. Confidence is
+		// 1.0 about the shape, which is all stage 0 ever claims — the gap is
+		// carried by the empty slots, not by a doubt.
+		return Decision{
+			Stage:      0,
+			Intent:     IntentFact,
+			Confidence: 1.0,
+		}, true
+	}
+	return Decision{}, false
+}

internal/router/barecapture_test.go

@@ -0,0 +1,37 @@
+package router
+
+import "testing"
+
+// TestBareCaptureIsAFactWithNoKey — Vikunja #557. The whole point is the gap:
+// she must route it as a fact she cannot write yet, so the clarify path asks.
+func TestBareCaptureIsAFactWithNoKey(t *testing.T) {
+	for _, u := range []string{"запиши", "Запиши.", "запомни, пожалуйста", "отметь!", "note", "запиши-ка"} {
+		m := bareCapturePattern.FindStringSubmatch(u)
+		if m == nil {
+			t.Errorf("%q did not match the shape", u)
+			continue
+		}
+		dec, ok := bareCaptureBuild(m)
+		if !ok {
+			t.Errorf("%q should route as a fact with no key", u)
+			continue
+		}
+		if dec.Intent != IntentFact || dec.Slots.HasKey || dec.Slots.Text != "" {
+			t.Errorf("%q built %+v, want an empty fact", u, dec)
+		}
+	}
+}
+
+// TestBareCaptureDeclinesAnythingWithAnObject — the object cases belong to the
+// capture markers and the cascade, and a lone non-capture word is not this rule.
+func TestBareCaptureDeclinesAnythingWithAnObject(t *testing.T) {
+	for _, u := range []string{"запиши что я пил воду", "добавь задачу купить хлеб", "привет", "вода", "расскажи"} {
+		m := bareCapturePattern.FindStringSubmatch(u)
+		if m == nil {
+			continue // shape already declined it
+		}
+		if _, ok := bareCaptureBuild(m); ok {
+			t.Errorf("%q must not be claimed as a bare capture", u)
+		}
+	}
+}