From 9145b831003b5980a29d7d31e98a1ee4cc76275e Mon Sep 17 00:00:00 2001 From: kami Date: Fri, 31 Jul 2026 02:20:28 +0400 Subject: [PATCH 1/4] Add the clarify data layer: a parked question with one missing slot The router can already say "I am not sure" (Decision.Clarify) but the daemon had nowhere to keep the request while it asked. PendingQuestion holds the original slots, ClarifyStore parks one per dialogue id with a 90s TTL, and Answer fills only the slots that were missing so an answer can never rewrite what she already understood. Logic that uses this comes next. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ --- internal/dialogue/clarify.go | 149 ++++++++++++++++++++++++++++++ internal/dialogue/clarify_test.go | 93 +++++++++++++++++++ 2 files changed, 242 insertions(+) create mode 100644 internal/dialogue/clarify.go create mode 100644 internal/dialogue/clarify_test.go diff --git a/internal/dialogue/clarify.go b/internal/dialogue/clarify.go new file mode 100644 index 0000000..699fb3c --- /dev/null +++ b/internal/dialogue/clarify.go @@ -0,0 +1,149 @@ +package dialogue + +import ( + "sync" + "time" +) + +// Slot names one piece of information a turn needs. A question is always about +// exactly one of these. +type Slot string + +const ( + SlotTime Slot = "time" + SlotKey Slot = "key" + SlotFn Slot = "fn" + SlotText Slot = "text" +) + +// MaxAttempts — she asks once and then drops the request. Asking twice about +// the same utterance reads as nagging, and the non-goals forbid that. +const MaxAttempts = 1 + +// PendingQuestion — a request she could not act on, parked while she waits for +// the one missing piece. The original slots are kept so the answer only has to +// carry the gap, not the whole request again. +type PendingQuestion struct { + Intent Intent + Slots Slots + Missing []Slot + Utterance string // the original request, so the answer inherits its wording + Asked time.Time + TTL time.Duration + Attempts int +} + +// IsExpired — an answer that arrives after the TTL is a new request, not an +// answer. Same reasoning as the confirm gate: a stale question must not eat an +// unrelated later utterance. +func (q *PendingQuestion) IsExpired(now time.Time) bool { + return now.After(q.Asked.Add(q.TTL)) +} + +func (q *PendingQuestion) CanAsk() bool { + return q.Attempts < MaxAttempts +} + +// Answer merges the parsed answer into the parked slots. It fills only the +// slots that were missing when the question was asked — an answer can never +// overwrite something she already understood, so a stray word in the answer +// cannot silently change the request. +func (q *PendingQuestion) Answer(text string, answer Slots) Slots { + out := q.Slots + for _, slot := range q.Missing { + switch slot { + case SlotTime: + if !out.HasTime && answer.HasTime { + out.Time, out.HasTime = answer.Time, true + } + case SlotKey: + if !out.HasKey && answer.HasKey { + out.Key, out.HasKey = answer.Key, true + } + case SlotFn: + if !out.HasFn && answer.HasFn { + out.Fn, out.Args, out.HasFn = answer.Fn, answer.Args, true + } + case SlotText: + if out.Text == "" { + out.Text = text + } + } + } + if out.Text == "" { + out.Text = text + } + return out +} + +// StillMissing returns the wanted slots that the given slots do not fill, in +// the order they were wanted. Empty result ⇒ the request can be acted on. +func StillMissing(want []Slot, s Slots) []Slot { + var out []Slot + for _, slot := range want { + filled := false + switch slot { + case SlotTime: + filled = s.HasTime + case SlotKey: + filled = s.HasKey + case SlotFn: + filled = s.HasFn + case SlotText: + filled = s.Text != "" + } + if !filled { + out = append(out, slot) + } + } + return out +} + +// ClarifyStore holds the parked questions. One entry per dialogue id; a new +// question overwrites the old one (last-asked wins, single-user box). +type ClarifyStore struct { + mu sync.Mutex + questions map[string]*PendingQuestion + defaultTTL time.Duration +} + +func NewClarifyStore(defaultTTL time.Duration) *ClarifyStore { + if defaultTTL <= 0 { + defaultTTL = 90 * time.Second + } + return &ClarifyStore{ + questions: make(map[string]*PendingQuestion), + defaultTTL: defaultTTL, + } +} + +// Get returns the live question for id, or nil. An expired question is dropped +// on read so the caller never sees one. +func (c *ClarifyStore) Get(id string, now time.Time) *PendingQuestion { + c.mu.Lock() + defer c.mu.Unlock() + q, ok := c.questions[id] + if !ok { + return nil + } + if q.IsExpired(now) { + delete(c.questions, id) + return nil + } + return q +} + +func (c *ClarifyStore) Put(id string, q *PendingQuestion) { + if q.TTL <= 0 { + q.TTL = c.defaultTTL + } + c.mu.Lock() + c.questions[id] = q + c.mu.Unlock() +} + +func (c *ClarifyStore) Delete(id string) { + c.mu.Lock() + delete(c.questions, id) + c.mu.Unlock() +} diff --git a/internal/dialogue/clarify_test.go b/internal/dialogue/clarify_test.go new file mode 100644 index 0000000..f991e02 --- /dev/null +++ b/internal/dialogue/clarify_test.go @@ -0,0 +1,93 @@ +package dialogue + +import ( + "testing" + "time" +) + +var clarifyNow = time.Date(2026, 7, 31, 10, 0, 0, 0, time.UTC) + +func TestPendingQuestionAnswerFillsOnlyMissing(t *testing.T) { + fireAt := clarifyNow.Add(time.Hour) + q := &PendingQuestion{ + Intent: IntentReminder, + Slots: Slots{Key: "mom", HasKey: true, Text: "напомни позвонить маме"}, + Missing: []Slot{SlotTime}, + } + got := q.Answer("в 11", Slots{Time: fireAt, HasTime: true, Key: "other", HasKey: true}) + if !got.HasTime || !got.Time.Equal(fireAt) { + t.Fatalf("missing time slot not filled: %+v", got) + } + if got.Key != "mom" { + t.Fatalf("answer overwrote a filled slot: key=%q", got.Key) + } + if got.Text != "напомни позвонить маме" { + t.Fatalf("answer overwrote the original text: %q", got.Text) + } +} + +func TestPendingQuestionAnswerKeepsGapWhenAnswerIsEmpty(t *testing.T) { + q := &PendingQuestion{Intent: IntentReminder, Missing: []Slot{SlotTime}} + got := q.Answer("не знаю", Slots{}) + if got.HasTime { + t.Fatal("empty answer must not fill the slot") + } + if len(StillMissing(q.Missing, got)) != 1 { + t.Fatal("StillMissing should report the unfilled slot") + } +} + +func TestStillMissing(t *testing.T) { + cases := []struct { + name string + want []Slot + slots Slots + left int + }{ + {"all filled", []Slot{SlotTime, SlotKey}, Slots{HasTime: true, HasKey: true}, 0}, + {"time gap", []Slot{SlotTime}, Slots{HasKey: true}, 1}, + {"fn gap", []Slot{SlotFn}, Slots{}, 1}, + {"text filled", []Slot{SlotText}, Slots{Text: "hi"}, 0}, + {"nothing wanted", nil, Slots{}, 0}, + } + for _, tc := range cases { + if got := StillMissing(tc.want, tc.slots); len(got) != tc.left { + t.Errorf("%s: got %v, want %d left", tc.name, got, tc.left) + } + } +} + +func TestClarifyStoreExpiry(t *testing.T) { + s := NewClarifyStore(90 * time.Second) + s.Put("voice", &PendingQuestion{Asked: clarifyNow, Missing: []Slot{SlotTime}}) + + if s.Get("voice", clarifyNow.Add(30*time.Second)) == nil { + t.Fatal("question inside the TTL should be live") + } + if s.Get("voice", clarifyNow.Add(2*time.Minute)) != nil { + t.Fatal("question past the TTL should be dropped") + } + if s.Get("voice", clarifyNow) != nil { + t.Fatal("an expired question must be deleted on read, not linger") + } +} + +func TestClarifyStoreDefaultTTL(t *testing.T) { + s := NewClarifyStore(0) + q := &PendingQuestion{Asked: clarifyNow} + s.Put("voice", q) + if q.TTL != 90*time.Second { + t.Fatalf("default TTL not applied: %v", q.TTL) + } +} + +func TestCanAskOnce(t *testing.T) { + q := &PendingQuestion{} + if !q.CanAsk() { + t.Fatal("a fresh question should be askable") + } + q.Attempts = MaxAttempts + if q.CanAsk() { + t.Fatal("she must not ask twice") + } +} From af35ec36291d99b2b6a0ab69a421348b5c1b1573 Mon Sep 17 00:00:00 2001 From: kami Date: Fri, 31 Jul 2026 02:22:18 +0400 Subject: [PATCH 2/4] Work out which slot is missing and phrase one short question MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A table per intent (reminder needs a time, fact needs a key, act needs a fn) plus one fixed Russian question per slot. Templates, not model output: a 0.8B would wander and a question that rewords itself is harder to answer. Note, query, chat and system get no question — for those a clarify decision keeps the canned reply rather than inventing a question for noise. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ --- cmd/mavend/clarify.go | 66 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 cmd/mavend/clarify.go diff --git a/cmd/mavend/clarify.go b/cmd/mavend/clarify.go new file mode 100644 index 0000000..eff681f --- /dev/null +++ b/cmd/mavend/clarify.go @@ -0,0 +1,66 @@ +package main + +import ( + "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 +} From cfd38d53cfc05eae79624035d987efc182d76e14 Mon Sep 17 00:00:00 2001 From: kami Date: Fri, 31 Jul 2026 02:26:11 +0400 Subject: [PATCH 3/4] Ask the question, then act on the answer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a clarify decision with one identifiable gap she now asks instead of saying "не поняла", and parks the request. The next utterance is parsed as the answer with the router's own extractor and the completed decision runs through applyAction like any other — so a clarified act still needs the allowlist and still hits the destructive confirm gate. An answer that does not fill the gap drops the request; she never asks twice. Also pulls the session-store block that HandlePushToTalk and handleText both had into rememberTurn, since the clarify path needed a third copy. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ --- cmd/mavend/clarify.go | 114 ++++++++++++++++++++++++++++++++++++++++++ cmd/mavend/voice.go | 96 ++++++++++++++++------------------- 2 files changed, 157 insertions(+), 53 deletions(-) diff --git a/cmd/mavend/clarify.go b/cmd/mavend/clarify.go index eff681f..4d4a3f1 100644 --- a/cmd/mavend/clarify.go +++ b/cmd/mavend/clarify.go @@ -1,6 +1,8 @@ package main import ( + "context" + "log" "time" "github.com/kami/maven/internal/dialogue" @@ -64,3 +66,115 @@ func clarifyQuestion(dec router.Decision) (dialogue.Slot, string, bool) { } 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/voice.go b/cmd/mavend/voice.go index 9bbd226..8e59a86 100644 --- a/cmd/mavend/voice.go +++ b/cmd/mavend/voice.go @@ -226,6 +226,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()) @@ -250,8 +252,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, } @@ -304,6 +308,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 @@ -378,6 +390,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 @@ -407,34 +426,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) } } @@ -465,6 +466,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 { @@ -482,30 +488,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 } } From 2f4257e19474318eb08e220d48a34c5b9670ac62 Mon Sep 17 00:00:00 2001 From: kami Date: Fri, 31 Jul 2026 02:29:04 +0400 Subject: [PATCH 4/4] Test the clarify round-trip end to end at the daemon level Covers: a reminder with no time is asked about and completes on the answer; the same for a fact; an answer past the TTL falls through as a fresh utterance; a second unclear answer drops the request with no second question; a clarified act off the allowlist neither runs nor gets enabled; a clarified destructive act still parks a confirm; noise keeps the canned reply. No model, no network. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ --- cmd/mavend/clarify_test.go | 238 +++++++++++++++++++++++++++++++++++++ 1 file changed, 238 insertions(+) create mode 100644 cmd/mavend/clarify_test.go 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) + } +}