diff --git a/cmd/mavend/clarify_test.go b/cmd/mavend/clarify_test.go index 84228e3..85a9df6 100644 --- a/cmd/mavend/clarify_test.go +++ b/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) + } + } +} diff --git a/cmd/mavend/voice.go b/cmd/mavend/voice.go index fe12f19..2a949bc 100644 --- a/cmd/mavend/voice.go +++ b/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 - // and park the request (clarify.go); otherwise the replier's canned reply - // stands. - if dec.Clarify { + // 7. clarify — something she needs is missing. If one named thing is missing, + // ask about it and park the request (clarify.go); otherwise the replier's + // canned reply stands. + // + // 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) } diff --git a/cmd/mavend/voicewire.go b/cmd/mavend/voicewire.go index 520d300..dddef03 100644 --- a/cmd/mavend/voicewire.go +++ b/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: "расскажи про diff --git a/internal/router/barecapture.go b/internal/router/barecapture.go new file mode 100644 index 0000000..83f0e2a --- /dev/null +++ b/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 +} diff --git a/internal/router/barecapture_test.go b/internal/router/barecapture_test.go new file mode 100644 index 0000000..b8973ec --- /dev/null +++ b/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) + } + } +}