diff --git a/cmd/mavend/clarify.go b/cmd/mavend/clarify.go new file mode 100644 index 0000000..4d4a3f1 --- /dev/null +++ b/cmd/mavend/clarify.go @@ -0,0 +1,180 @@ +package main + +import ( + "context" + "log" + "time" + + "github.com/kami/maven/internal/dialogue" + "github.com/kami/maven/internal/router" +) + +// clarifyTTL — how long a parked question stays answerable. Same 90s as the +// confirm gate, for the same reason: an answer is a same-breath gesture, and a +// stale question must not eat an unrelated later utterance. +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. +// +// 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 +// decision keeps the canned "не поняла" reply — inventing a question for noise +// is worse than admitting she missed it. +var wantedSlots = map[router.Intent][]dialogue.Slot{ + router.IntentReminder: {dialogue.SlotTime}, + router.IntentFact: {dialogue.SlotKey}, + router.IntentAct: {dialogue.SlotFn}, +} + +// clarifyQuestions — one short question per missing slot. +// +// These are fixed templates, not model output. The resident model is a 0.8B; it +// would wander, and a question whose wording changes every time is harder to +// answer than a blunt one that always reads the same. They are infinitive +// questions, so there is no gender agreement to get wrong; the feminine +// self-reference lives in the reply she gives when she drops the request. +var clarifyQuestions = map[dialogue.Slot]string{ + dialogue.SlotTime: "На когда напомнить?", + dialogue.SlotKey: "Что записать?", + dialogue.SlotFn: "Что сделать?", +} + +// clarifyDropped — she asked once, the answer still did not fill the gap, so +// the request is gone. Said plainly, once, with no second question. +const clarifyDropped = "Не разобрала — скажи целиком, пожалуйста." + +// missingFor returns the slots a decision still needs, most important first. +// Empty ⇒ there is nothing identifiable to ask about. +func missingFor(dec router.Decision) []dialogue.Slot { + return dialogue.StillMissing(wantedSlots[dec.Intent], toDialogueSlots(dec.Slots)) +} + +// clarifyQuestion picks the one question to ask for a clarify decision. Returns +// ("", 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. +func clarifyQuestion(dec router.Decision) (dialogue.Slot, string, bool) { + missing := missingFor(dec) + if len(missing) == 0 { + return "", "", false + } + q, ok := clarifyQuestions[missing[0]] + if !ok { + return "", "", false + } + return missing[0], q, true +} + +// askClarify parks the request and returns the question to ask instead of the +// canned "не поняла". Returns ("", false) when there is nothing to ask about, so +// the caller falls back to the canned reply. +func (h *reactiveHandler) askClarify(dec router.Decision) (string, bool) { + if h.clarifyStore == nil { + return "", false + } + slot, question, ok := clarifyQuestion(dec) + if !ok { + return "", false + } + h.clarifyStore.Put(voiceDialogueID, &dialogue.PendingQuestion{ + Intent: dialogue.Intent(dec.Intent), + Slots: toDialogueSlots(dec.Slots), + Missing: []dialogue.Slot{slot}, + Utterance: dec.Utterance, + Asked: h.now(), + TTL: clarifyTTL, + Attempts: 1, // asked once; MaxAttempts is 1, so there is no second ask + }) + log.Printf("voice: clarify — asked about %s for intent=%s", slot, dec.Intent) + return question, true +} + +// resolveClarifyAnswer reads an utterance as the answer to a parked question. +// Returns ("", false) when no live question is parked (or it expired), so the +// caller routes the utterance normally as a fresh request. Sibling of +// resolveConfirm and checked in the same place. +// +// The answer is parsed with the same extractor the router uses, for the intent +// she parked — no second parser. If it still does not fill the gap the request +// is dropped: she does not ask again. +func (h *reactiveHandler) resolveClarifyAnswer(ctx context.Context, text string) (string, bool) { + if h.clarifyStore == nil { + return "", false + } + q := h.clarifyStore.Get(voiceDialogueID, h.now()) + if q == nil { + return "", false + } + // One shot either way: the question is consumed whether or not the answer + // works, so a failed answer can't leave the question armed. + h.clarifyStore.Delete(voiceDialogueID) + + intent := router.Intent(q.Intent) + answer := h.extractor.Extract(ctx, intent, text, h.now()) + merged := q.Answer(text, toDialogueSlots(answer)) + if len(dialogue.StillMissing(q.Missing, merged)) > 0 { + log.Printf("voice: clarify — answer %q did not fill %v, dropping", text, q.Missing) + return clarifyDropped, 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 + // still meets the allowlist and the destructive-act confirm gate in + // applyAction exactly like any other decision. + dec := router.Decision{ + Utterance: q.Utterance, + Stage: 2, + Intent: intent, + Slots: applyDialogueSlots(answer, merged), + } + return h.finishClarified(ctx, dec), true +} + +// finishClarified runs a completed decision through the same steps a freshly +// routed one takes: remember the turn, act, then phrase. +func (h *reactiveHandler) finishClarified(ctx context.Context, dec router.Decision) string { + if h.dialogueSessions != nil { + now := h.now() + prev := h.dialogueSessions.Get(voiceDialogueID, now) + dec = followUpMerge(prev, dec, now) + h.rememberTurn(prev, dec, now) + } + reply := h.applyAction(ctx, dec) + if reply == "" { + reply = h.replier.Reply(dec) + } + return reply +} + +// rememberTurn stores this turn as the dialogue session the next follow-up +// inherits from, carrying up to 4 prior turns of history for anaphora. Capped so +// one long conversation can't grow the session unboundedly. +func (h *reactiveHandler) rememberTurn(prev *dialogue.Session, dec router.Decision, now time.Time) { + var history []dialogue.Turn + if prev != nil { + history = append(history, dialogue.Turn{ + Intent: prev.Intent, + Slots: prev.Slots, + Text: prev.Slots.Text, + }) + maxHist := len(prev.History) + if maxHist > 3 { + maxHist = 3 + } + history = append(history, prev.History[:maxHist]...) + } + ttl := time.Duration(0) // use the store default (2 min) + if dec.Intent == router.IntentChat { + ttl = 15 * time.Minute // conversational turns should last longer + } + h.dialogueSessions.Put(voiceDialogueID, &dialogue.Session{ + Intent: dialogue.Intent(dec.Intent), + Slots: toDialogueSlots(dec.Slots), + Timestamp: now, + TTL: ttl, + History: history, + }) +} diff --git a/cmd/mavend/clarify_test.go b/cmd/mavend/clarify_test.go new file mode 100644 index 0000000..d83ec68 --- /dev/null +++ b/cmd/mavend/clarify_test.go @@ -0,0 +1,238 @@ +package main + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/kami/maven/internal/dialogue" + "github.com/kami/maven/internal/ipc" + "github.com/kami/maven/internal/router" + "github.com/kami/maven/internal/store" + "github.com/kami/maven/internal/tool" + "github.com/kami/maven/internal/voice" +) + +// newClarifyHandler builds a handler with the clarify path wired and no model: +// stub date parser, the real fact parser, and a matcher over whatever tools the +// test enabled. `now` is fixed so TTL behaviour is testable. +func newClarifyHandler(t *testing.T) (*reactiveHandler, *store.Store, *time.Time) { + t.Helper() + st := newTestStore(t) + api := ipc.NewStoreAPI(st) + now := time.Date(2026, 7, 31, 9, 0, 0, 0, time.UTC) + matcher := tool.NewMatcher(api) + h := &reactiveHandler{ + api: api, + dataStore: st, + tools: tool.NewExecutor(api, 2*time.Second), + matcher: matcher, + replier: voice.NewStubReplier(), + now: func() time.Time { return now }, + dialogueSessions: dialogue.NewSessionStore(2 * time.Minute), + clarifyStore: dialogue.NewClarifyStore(clarifyTTL), + extractor: router.Extractor{ + Time: router.StubDateTimeParser{}, + Acts: matcher, + Facts: router.DefaultFactParser{}, + }, + } + return h, st, &now +} + +func clarifyDec(intent router.Intent, slots router.Slots, utterance string) router.Decision { + return router.Decision{Utterance: utterance, Stage: 3, Intent: intent, Slots: slots, Clarify: true} +} + +// TestClarifyQuestionForMissingSlot pins which question goes with which gap, and +// which intents get no question at all. +func TestClarifyQuestionForMissingSlot(t *testing.T) { + cases := []struct { + name string + dec router.Decision + want string + asked bool + }{ + {"reminder without a time", clarifyDec(router.IntentReminder, router.Slots{Text: "напомни позвонить маме"}, "напомни позвонить маме"), "На когда напомнить?", true}, + {"fact without a key", clarifyDec(router.IntentFact, router.Slots{Text: "запиши"}, "запиши"), "Что записать?", true}, + {"act without a fn", clarifyDec(router.IntentAct, router.Slots{Text: "сделай это"}, "сделай это"), "Что сделать?", true}, + {"reminder that already has a time", clarifyDec(router.IntentReminder, router.Slots{HasTime: true}, "напомни в 11"), "", false}, + {"chat is never worth a question", clarifyDec(router.IntentChat, router.Slots{Text: "мгм"}, "мгм"), "", false}, + {"query is never worth a question", clarifyDec(router.IntentQuery, router.Slots{Text: "а"}, "а"), "", false}, + } + for _, tc := range cases { + _, got, asked := clarifyQuestion(tc.dec) + if asked != tc.asked || got != tc.want { + t.Errorf("%s: got (%q, %v), want (%q, %v)", tc.name, got, asked, tc.want, tc.asked) + } + } +} + +// TestClarifyReminderCompletesOnAnswer is the whole point of the feature: she +// asks for the missing time and the answer creates the reminder. +func TestClarifyReminderCompletesOnAnswer(t *testing.T) { + ctx := context.Background() + h, st, _ := newClarifyHandler(t) + + question, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{Text: "напомни позвонить маме"}, "напомни позвонить маме")) + if !asked || question != "На когда напомнить?" { + t.Fatalf("expected the time question, got %q asked=%v", question, asked) + } + + reply, handled := h.resolveClarifyAnswer(ctx, "в 11:00") + if !handled { + t.Fatal("the answer to an open question must be consumed as an answer") + } + if reply == clarifyDropped { + t.Fatalf("a good answer must not drop the request: %q", reply) + } + + reminders, err := st.DueReminders(ctx, h.now().Add(48*time.Hour)) + if err != nil || len(reminders) != 1 { + t.Fatalf("clarified reminder was not created: reminders=%v err=%v", reminders, err) + } + if !strings.Contains(reminders[0].Payload, "маме") { + t.Fatalf("the reminder lost the original request: %q", reminders[0].Payload) + } + if h.clarifyStore.Get(voiceDialogueID, h.now()) != nil { + t.Fatal("the question must be cleared once answered") + } +} + +// TestClarifyFactCompletesOnAnswer — the fact path, where the answer carries +// both the key and the value. +func TestClarifyFactCompletesOnAnswer(t *testing.T) { + ctx := context.Background() + h, st, _ := newClarifyHandler(t) + + if _, asked := h.askClarify(clarifyDec(router.IntentFact, router.Slots{Text: "запиши"}, "запиши")); !asked { + t.Fatal("a fact with no key should be asked about") + } + if reply, handled := h.resolveClarifyAnswer(ctx, "пил воду"); !handled || reply == clarifyDropped { + t.Fatalf("answer should complete the fact, handled=%v reply=%q", handled, reply) + } + if fact, err := st.LatestFact(ctx, "water"); err != nil || fact.Key != "water" { + t.Fatalf("clarified fact was not written: fact=%+v err=%v", fact, err) + } +} + +// TestClarifyAnswerAfterTTLIsANewRequest — a late answer is not an answer. +func TestClarifyAnswerAfterTTLIsANewRequest(t *testing.T) { + ctx := context.Background() + h, st, now := newClarifyHandler(t) + + if _, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { + t.Fatal("expected a question") + } + *now = now.Add(clarifyTTL + time.Second) + + if reply, handled := h.resolveClarifyAnswer(ctx, "в 11:00"); handled { + t.Fatalf("an answer past the TTL must fall through to normal routing, got %q", reply) + } + if reminders, err := st.DueReminders(ctx, now.Add(48*time.Hour)); err != nil || len(reminders) != 0 { + t.Fatalf("expired question must not create anything: reminders=%v err=%v", reminders, err) + } +} + +// TestClarifyUnclearAnswerDropsWithoutAskingAgain — MaxAttempts is 1. +func TestClarifyUnclearAnswerDropsWithoutAskingAgain(t *testing.T) { + ctx := context.Background() + h, st, _ := newClarifyHandler(t) + + if _, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { + t.Fatal("expected a question") + } + reply, handled := h.resolveClarifyAnswer(ctx, "ну не знаю") + if !handled || reply != clarifyDropped { + t.Fatalf("an unclear answer should drop the request, handled=%v reply=%q", handled, reply) + } + if strings.Contains(reply, "?") { + t.Fatalf("she must not ask a second question: %q", reply) + } + if h.clarifyStore.Get(voiceDialogueID, h.now()) != nil { + t.Fatal("a dropped request must leave no armed question") + } + if reminders, err := st.DueReminders(ctx, h.now().Add(48*time.Hour)); err != nil || len(reminders) != 0 { + t.Fatalf("a dropped request must not create anything: reminders=%v err=%v", reminders, err) + } +} + +// TestClarifiedActOffAllowlistIsStillRefused — clarification fills in an +// argument, it never grants authority. +func TestClarifiedActOffAllowlistIsStillRefused(t *testing.T) { + ctx := context.Background() + h, st, _ := newClarifyHandler(t) + marker := filepath.Join(t.TempDir(), "not-allowed-ran") + + if _, asked := h.askClarify(clarifyDec(router.IntentAct, router.Slots{Text: "сделай это"}, "сделай это")); !asked { + t.Fatal("an act with no fn should be asked about") + } + reply, handled := h.resolveClarifyAnswer(ctx, "rm "+marker) + if !handled { + t.Fatal("the answer should be consumed") + } + if strings.Contains(reply, "готово") { + t.Fatalf("an act that is not on the allowlist must not report success: %q", reply) + } + if _, err := os.Stat(marker); !os.IsNotExist(err) { + t.Fatalf("a clarified act off the allowlist ran anyway: %v", err) + } + if tools, err := st.ListTools(ctx, "enabled"); err != nil || len(tools) != 0 { + t.Fatalf("clarify must not enable a tool: tools=%+v err=%v", tools, err) + } +} + +// TestClarifiedDestructiveActStillNeedsConfirm — the confirm gate survives the +// clarify path. +func TestClarifiedDestructiveActStillNeedsConfirm(t *testing.T) { + ctx := context.Background() + h, st, _ := newClarifyHandler(t) + marker := filepath.Join(t.TempDir(), "destructive-ran") + if err := st.EnableTool(ctx, "delete_backups", []string{"touch", marker}, true, "test", h.now()); err != nil { + t.Fatal(err) + } + + if _, asked := h.askClarify(clarifyDec(router.IntentAct, router.Slots{Text: "сделай это"}, "сделай это")); !asked { + t.Fatal("expected a question") + } + reply, handled := h.resolveClarifyAnswer(ctx, "delete_backups") + if !handled { + t.Fatal("the answer should be consumed") + } + if !strings.Contains(reply, "да") || h.pending == nil { + t.Fatalf("a clarified destructive act must still park a confirm: reply=%q pending=%+v", reply, h.pending) + } + if _, err := os.Stat(marker); !os.IsNotExist(err) { + t.Fatalf("a clarified destructive act ran before confirmation: %v", err) + } +} + +// TestNoQuestionWhenNothingIsMissing — noise keeps the canned reply, so she +// never invents a question for nothing. +func TestNoQuestionWhenNothingIsMissing(t *testing.T) { + h, _, _ := newClarifyHandler(t) + for _, dec := range []router.Decision{ + clarifyDec(router.IntentChat, router.Slots{Text: "эм"}, "эм"), + clarifyDec(router.IntentQuery, router.Slots{Text: "ммм"}, "ммм"), + clarifyDec(router.IntentNote, router.Slots{Text: "..."}, "..."), + } { + if question, asked := h.askClarify(dec); asked { + t.Fatalf("intent %s should keep the canned reply, got %q", dec.Intent, question) + } + } + if h.clarifyStore.Get(voiceDialogueID, h.now()) != nil { + t.Fatal("noise must not park a question") + } +} + +// TestNoPendingQuestionFallsThrough — with nothing parked, an utterance routes +// normally. +func TestNoPendingQuestionFallsThrough(t *testing.T) { + h, _, _ := newClarifyHandler(t) + if reply, handled := h.resolveClarifyAnswer(context.Background(), "напомни в 11:00"); handled { + t.Fatalf("no open question ⇒ must not be treated as an answer, got %q", reply) + } +} diff --git a/cmd/mavend/voice.go b/cmd/mavend/voice.go index 97f5535..f1ed178 100644 --- a/cmd/mavend/voice.go +++ b/cmd/mavend/voice.go @@ -228,6 +228,8 @@ func wireVoice(cfg *config.Config, coreAPI ipc.CoreAPI, phr phraser.Phraser, mem // ----- dialogue (multi-turn slot carry-over; 2-min follow-up window) ----- dialogueSessions := dialogue.NewSessionStore(2 * time.Minute) + clarifyStore := dialogue.NewClarifyStore(clarifyTTL) + timeParser := router.NewPythonDateParser() // ----- replier (LLM-backed when the engine is on, Stub floor otherwise) ----- replier := voice.Replier(voice.NewStubReplier()) @@ -252,8 +254,10 @@ func wireVoice(cfg *config.Config, coreAPI ipc.CoreAPI, phr phraser.Phraser, mem memStore: memStore, dataStore: dataStore, dialogueSessions: dialogueSessions, + clarifyStore: clarifyStore, + extractor: router.Extractor{Time: timeParser, Acts: matcher, Facts: router.DefaultFactParser{}}, queryMinScore: cfg.Voice.QueryMinScore, - timeParser: router.NewPythonDateParser(), + timeParser: timeParser, ecosystem: eco, } @@ -306,6 +310,14 @@ type reactiveHandler struct { // box → one session slot, keyed voiceDialogueID). nil ⇒ no carry-over. dialogueSessions *dialogue.SessionStore + // clarifyStore parks the request behind an open question she asked (see + // clarify.go). nil ⇒ she falls back to the canned "не поняла" reply. + clarifyStore *dialogue.ClarifyStore + + // extractor parses the answer to an open question, with the same parsers + // the router's own stage-2 uses. + extractor router.Extractor + // pending destructive-act confirmation. A destructive act replies with a // "выполнить X? да/нет" prompt and parks here; the NEXT utterance is read as // the y/n answer. ponytail: single slot, single-user box — a second act @@ -380,6 +392,13 @@ func (h *reactiveHandler) HandlePushToTalk(ctx context.Context, req voice.PushTo return h.reply(ctx, reply, nil) } + // 1b2. clarify answer — if she asked a question last turn, this utterance is + // its answer, not a fresh command. After the confirm check: a y/n gate is + // armed by her own prompt and is the narrower claim on the utterance. + if reply, handled := h.resolveClarifyAnswer(ctx, text); handled { + return h.reply(ctx, reply, nil) + } + // 1c. quiet-hours toggle — keyword match, not classifier-dependent. // "тихий режим" / "quiet on" would route through the classifier // unreliably (it's a command, not a free-form query), so we match it @@ -409,34 +428,16 @@ func (h *reactiveHandler) HandlePushToTalk(ctx context.Context, req voice.PushTo prev := h.dialogueSessions.Get(voiceDialogueID, now) dec = followUpMerge(prev, dec, now) if !dec.Clarify { - // Build history: carry over up to 4 prior turns for cross-intent - // reference. The most recent prior turn is prepended to history. - var history []dialogue.Turn - if prev != nil { - history = append(history, dialogue.Turn{ - Intent: prev.Intent, - Slots: prev.Slots, - Text: prev.Slots.Text, // the prior turn's utterance - }) - // Cap history depth so one long conversation can't grow - // the session unboundedly. - maxHist := len(prev.History) - if maxHist > 3 { - maxHist = 3 - } - history = append(history, prev.History[:maxHist]...) - } - ttl := time.Duration(0) // use default (2 min) - if dec.Intent == router.IntentChat { - ttl = 15 * time.Minute // conversational turns should last longer - } - h.dialogueSessions.Put(voiceDialogueID, &dialogue.Session{ - Intent: dialogue.Intent(dec.Intent), - Slots: toDialogueSlots(dec.Slots), - Timestamp: now, - TTL: ttl, - History: history, - }) + h.rememberTurn(prev, dec, now) + } + } + + // 2c. 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 { + if question, asked := h.askClarify(dec); asked { + return h.reply(ctx, question, nil) } } @@ -467,6 +468,11 @@ func (h *reactiveHandler) handleText(ctx context.Context, text string) string { return reply } + // 1b2. clarify answer — same check as HandlePushToTalk. + if reply, handled := h.resolveClarifyAnswer(ctx, text); handled { + return reply + } + // 2. router — classify the utterance. dec, err := h.router.Route(ctx, text, h.now()) if err != nil { @@ -484,30 +490,14 @@ func (h *reactiveHandler) handleText(ctx context.Context, text string) string { prev := h.dialogueSessions.Get(voiceDialogueID, now) dec = followUpMerge(prev, dec, now) if !dec.Clarify { - var history []dialogue.Turn - if prev != nil { - history = append(history, dialogue.Turn{ - Intent: prev.Intent, - Slots: prev.Slots, - Text: prev.Slots.Text, - }) - maxHist := len(prev.History) - if maxHist > 3 { - maxHist = 3 - } - history = append(history, prev.History[:maxHist]...) - } - ttl := time.Duration(0) - if dec.Intent == router.IntentChat { - ttl = 15 * time.Minute - } - h.dialogueSessions.Put(voiceDialogueID, &dialogue.Session{ - Intent: dialogue.Intent(dec.Intent), - Slots: toDialogueSlots(dec.Slots), - Timestamp: now, - TTL: ttl, - History: history, - }) + h.rememberTurn(prev, dec, now) + } + } + + // 2c. clarify — same as HandlePushToTalk: ask about the one missing thing. + if dec.Clarify { + if question, asked := h.askClarify(dec); asked { + return question } }