diff --git a/cmd/mavend/actions_query.go b/cmd/mavend/actions_query.go index eb54c00..236f3fb 100644 --- a/cmd/mavend/actions_query.go +++ b/cmd/mavend/actions_query.go @@ -398,6 +398,11 @@ func (h *reactiveHandler) queryWeather(ctx context.Context, t *queryTurn) (strin if errors.Is(err, weather.ErrNotConfigured) { return phraser.Q(phraser.QueryWeatherOff, nil), true } + if errors.Is(err, weather.ErrLocationUnknown) { + // He named a place and the geocoder does not have it. Saying so beats + // reading out the default city's temperature (Vikunja #421). + return "не знаю такого города — " + loc + ".", true + } if err != nil { log.Printf("voice: weather: %v", err) return phraser.Q(phraser.QueryFailWeather, nil), true diff --git a/cmd/mavend/clarify.go b/cmd/mavend/clarify.go index 591ff9a..1604d9d 100644 --- a/cmd/mavend/clarify.go +++ b/cmd/mavend/clarify.go @@ -106,11 +106,11 @@ func trimClarifyExpired(s string) string { // out, and "" when nothing was parked. Call it right after // resolveClarifyAnswer: a live question is answered there, an expired one is // only reported here — the words themselves still go on to be routed fresh. -func (h *reactiveHandler) clarifyExpiredNotice() string { +func (h *reactiveHandler) clarifyExpiredNotice(ctx context.Context) string { if h.clarifyStore == nil { return "" } - if !h.clarifyStore.TakeExpired(voiceDialogueID, h.now()) { + if !h.clarifyStore.TakeExpired(dialogueIDOf(ctx), h.now()) { return "" } log.Printf("voice: clarify — parked question expired, telling him and routing the words fresh") @@ -157,7 +157,7 @@ func clarifyQuestion(dec router.Decision) (dialogue.Slot, string, bool) { // 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) { +func (h *reactiveHandler) askClarify(ctx context.Context, dec router.Decision) (string, bool) { if h.clarifyStore == nil { return "", false } @@ -165,7 +165,7 @@ func (h *reactiveHandler) askClarify(dec router.Decision) (string, bool) { if !ok { return "", false } - h.clarifyStore.Put(voiceDialogueID, &dialogue.PendingQuestion{ + h.clarifyStore.Put(dialogueIDOf(ctx), &dialogue.PendingQuestion{ Intent: dialogue.Intent(dec.Intent), Slots: toDialogueSlots(dec.Slots), Missing: []dialogue.Slot{slot}, @@ -192,7 +192,7 @@ func (h *reactiveHandler) resolveClarifyAnswer(ctx context.Context, text string) if h.clarifyStore == nil { return "", false } - q := h.clarifyStore.Get(voiceDialogueID, h.now()) + q := h.clarifyStore.Get(dialogueIDOf(ctx), h.now()) if q == nil { return "", false } @@ -206,9 +206,9 @@ func (h *reactiveHandler) resolveClarifyAnswer(ctx context.Context, text string) // 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 + return h.reaskOrGiveUp(ctx, q, merged, text), true } - h.clarifyStore.Delete(voiceDialogueID) + h.clarifyStore.Delete(dialogueIDOf(ctx)) // 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 @@ -217,7 +217,7 @@ func (h *reactiveHandler) resolveClarifyAnswer(ctx context.Context, text string) // 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 { + if reply, asked := h.askRemainingGap(ctx, q, intent, merged); asked { return reply, true } @@ -261,7 +261,7 @@ func foldAnswerIntoUtterance(utterance, subject string) string { // 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) { +func (h *reactiveHandler) askRemainingGap(ctx context.Context, q *dialogue.PendingQuestion, intent router.Intent, merged dialogue.Slots) (string, bool) { remaining := dialogue.StillMissing(wantedSlots[intent], merged) if len(remaining) == 0 { return "", false @@ -270,7 +270,7 @@ func (h *reactiveHandler) askRemainingGap(q *dialogue.PendingQuestion, intent ro if !ok || !q.CanAsk() { return "", false } - h.clarifyStore.Put(voiceDialogueID, &dialogue.PendingQuestion{ + h.clarifyStore.Put(dialogueIDOf(ctx), &dialogue.PendingQuestion{ Intent: q.Intent, Slots: merged, Missing: []dialogue.Slot{remaining[0]}, @@ -287,13 +287,13 @@ func (h *reactiveHandler) askRemainingGap(q *dialogue.PendingQuestion, intent ro // 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". -func (h *reactiveHandler) reaskOrGiveUp(q *dialogue.PendingQuestion, merged dialogue.Slots, text string) string { +func (h *reactiveHandler) reaskOrGiveUp(ctx context.Context, q *dialogue.PendingQuestion, merged dialogue.Slots, text string) string { question := "" if len(q.Missing) > 0 { question = clarifyQuestions[q.Missing[0]] } if question == "" || !q.CanAsk() { - h.clarifyStore.Delete(voiceDialogueID) + h.clarifyStore.Delete(dialogueIDOf(ctx)) log.Printf("voice: clarify — gave up on %v after %d question(s), answer was %q", q.Missing, q.Attempts, text) return clarifyGaveUp } @@ -302,7 +302,7 @@ func (h *reactiveHandler) reaskOrGiveUp(q *dialogue.PendingQuestion, merged dial q.Slots = merged q.Attempts++ q.Asked = h.now() - h.clarifyStore.Put(voiceDialogueID, q) + h.clarifyStore.Put(dialogueIDOf(ctx), q) log.Printf("voice: clarify — answer %q did not fill %v, asking again (attempt %d)", text, q.Missing, q.Attempts) return question } diff --git a/cmd/mavend/clarify_test.go b/cmd/mavend/clarify_test.go index 8b377e1..db1f140 100644 --- a/cmd/mavend/clarify_test.go +++ b/cmd/mavend/clarify_test.go @@ -81,7 +81,7 @@ func TestClarifyReminderCompletesOnAnswer(t *testing.T) { ctx := context.Background() h, st, _ := newClarifyHandler(t) - question, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{Text: "напомни позвонить маме"}, "напомни позвонить маме")) + question, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{Text: "напомни позвонить маме"}, "напомни позвонить маме")) if !asked || question != "Когда?" { t.Fatalf("expected the time question, got %q asked=%v", question, asked) } @@ -112,7 +112,7 @@ func TestClarifyFactCompletesOnAnswer(t *testing.T) { ctx := context.Background() h, st, _ := newClarifyHandler(t) - if _, asked := h.askClarify(clarifyDec(router.IntentFact, router.Slots{Text: "запиши"}, "запиши")); !asked { + if _, asked := h.askClarify(ctx, 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 == clarifyGaveUp { @@ -128,7 +128,7 @@ func TestClarifyAnswerAfterTTLIsANewRequest(t *testing.T) { ctx := context.Background() h, st, now := newClarifyHandler(t) - if _, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { t.Fatal("expected a question") } *now = now.Add(clarifyTTL + time.Second) @@ -147,7 +147,7 @@ func TestClarifyAsksThreeTimesThenSaysSo(t *testing.T) { ctx := context.Background() h, st, _ := newClarifyHandler(t) - if _, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { t.Fatal("expected a first question") } // Two more unclear answers ⇒ two more questions (3 asks in total). @@ -185,7 +185,7 @@ func TestClarifyMaxAttemptsIsConfigurable(t *testing.T) { h, _, _ := newClarifyHandler(t) h.clarifyMaxAttempts = 1 - if _, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { t.Fatal("expected a question") } if reply, handled := h.resolveClarifyAnswer(ctx, "ну не знаю"); !handled || reply != clarifyGaveUp { @@ -199,7 +199,7 @@ func TestClarifyRestatedAnswerWins(t *testing.T) { ctx := context.Background() h, st, _ := newClarifyHandler(t) - if _, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{Text: "напомни позвонить маме"}, "напомни позвонить маме")); !asked { + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{Text: "напомни позвонить маме"}, "напомни позвонить маме")); !asked { t.Fatal("expected a question") } // First answer parses, but re-park it by hand as if she had asked again: @@ -232,7 +232,7 @@ func TestClarifiedActOffAllowlistIsStillRefused(t *testing.T) { h, st, _ := newClarifyHandler(t) marker := filepath.Join(t.TempDir(), "not-allowed-ran") - if _, asked := h.askClarify(clarifyDec(router.IntentAct, router.Slots{Text: "сделай это"}, "сделай это")); !asked { + if _, asked := h.askClarify(ctx, 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) @@ -260,7 +260,7 @@ func TestClarifiedDestructiveActStillNeedsConfirm(t *testing.T) { t.Fatal(err) } - if _, asked := h.askClarify(clarifyDec(router.IntentAct, router.Slots{Text: "сделай это"}, "сделай это")); !asked { + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentAct, router.Slots{Text: "сделай это"}, "сделай это")); !asked { t.Fatal("expected a question") } reply, handled := h.resolveClarifyAnswer(ctx, "delete_backups") @@ -284,7 +284,7 @@ func TestNoQuestionWhenNothingIsMissing(t *testing.T) { clarifyDec(router.IntentQuery, router.Slots{Text: "ммм"}, "ммм"), clarifyDec(router.IntentNote, router.Slots{Text: "..."}, "..."), } { - if question, asked := h.askClarify(dec); asked { + if question, asked := h.askClarify(context.Background(), dec); asked { t.Fatalf("intent %s should keep the canned reply, got %q", dec.Intent, question) } } @@ -296,29 +296,29 @@ func TestNoQuestionWhenNothingIsMissing(t *testing.T) { // TestClarifyExpiryIsAnnouncedAndWordsStillRoute — his answer lands after the // TTL: she must say the old request is gone AND still answer the new words. func TestClarifyExpiryIsAnnouncedAndWordsStillRoute(t *testing.T) { - ctx := context.Background() + ctx := withDialogueID(context.Background(), dialogueIDFor(sourceText, "")) h, _, now := newClarifyHandler(t) emb := router.NewHashEmbedder(1024) h.embedder = emb h.router = buildRouter(emb, h.matcher, 0.55, nil) - if _, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { t.Fatal("expected a question") } *now = now.Add(clarifyTTL + time.Second) - reply := h.handleText(ctx, "как дела") + reply := h.handleText(ctx, "", "как дела") if !isClarifyExpired(reply) { t.Fatalf("expired question must be announced first, got %q", reply) } if trimClarifyExpired(reply) == "" { t.Fatalf("the new words must still be answered, got only the notice: %q", reply) } - if h.clarifyStore.Get(voiceDialogueID, h.now()) != nil { + if h.clarifyStore.Get(textDialogueID, h.now()) != nil { t.Fatal("the expired question must be gone") } // The notice is said once, not on every later utterance. - if reply := h.handleText(ctx, "как дела"); isClarifyExpired(reply) { + if reply := h.handleText(ctx, "", "как дела"); isClarifyExpired(reply) { t.Fatalf("notice repeated on a later turn: %q", reply) } } @@ -340,7 +340,7 @@ func TestClarifyAsksAboutTheSecondGapToo(t *testing.T) { ctx := context.Background() h, st, _ := newClarifyHandler(t) - question, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{}, "напомни")) + question, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{}, "напомни")) if !asked || question != "О чём напомнить?" { t.Fatalf("expected the subject question, got %q asked=%v", question, asked) } @@ -380,7 +380,7 @@ func TestClarifySecondGapRespectsTheAttemptCap(t *testing.T) { h, _, _ := newClarifyHandler(t) h.clarifyMaxAttempts = 1 - if _, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{}, "напомни")); !asked { + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{}, "напомни")); !asked { t.Fatal("expected the subject question") } reply, handled := h.resolveClarifyAnswer(ctx, "позвонить маме") @@ -440,10 +440,10 @@ func TestClarifyProseHoldsThePersona(t *testing.T) { // The confirm turn used to return before the notice was even computed, so he // answered the confirm and never heard that the older request was let go. func TestExpiryNoticeSurvivesAConfirmTurn(t *testing.T) { - ctx := context.Background() + ctx := withDialogueID(context.Background(), dialogueIDFor(sourceText, "")) h, _, now := newClarifyHandler(t) - if _, asked := h.askClarify(clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { t.Fatal("expected a question") } // A confirm parked with a longer life than the question, so only the @@ -451,7 +451,7 @@ func TestExpiryNoticeSurvivesAConfirmTurn(t *testing.T) { h.pending = &pendingAct{fn: "delete_backups", phrase: "удалить бэкапы", expiry: now.Add(time.Hour)} *now = now.Add(clarifyTTL + time.Second) - reply := h.handleText(ctx, "нет") + reply := h.handleText(ctx, "", "нет") if !isClarifyExpired(reply) { t.Fatalf("the expired question must be announced on a confirm turn too, got %q", reply) } @@ -461,7 +461,7 @@ func TestExpiryNoticeSurvivesAConfirmTurn(t *testing.T) { if h.pending != nil { t.Fatal("the confirm must still have been consumed") } - if h.clarifyStore.Get(voiceDialogueID, h.now()) != nil { + if h.clarifyStore.Get(textDialogueID, h.now()) != nil { t.Fatal("the expired question must be gone") } } @@ -476,7 +476,7 @@ func TestClarifySubjectAnswerFillsRatherThanClobbers(t *testing.T) { h, st, _ := newClarifyHandler(t) at := h.now().Add(2 * time.Hour) - question, asked := h.askClarify(clarifyDec(router.IntentReminder, + question, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{Time: at, HasTime: true}, "напомни в 11")) if !asked || question != "О чём напомнить?" { t.Fatalf("expected the subject question, got %q asked=%v", question, asked) @@ -501,3 +501,52 @@ func TestClarifySubjectAnswerFillsRatherThanClobbers(t *testing.T) { t.Fatalf("the answer clobbered the original request: %q", reminders[0].Payload) } } + +// TestClarifyIsPerConversation — the parked question belongs to the reach that +// was asked. Before this the clarify store had one global key, so a question +// asked in the web chat and never answered captured the next utterance from +// telegram, or from the mic, and answered it against a request the speaker had +// never made (Vikunja #466). +func TestClarifyIsPerConversation(t *testing.T) { + h, _, _ := newClarifyHandler(t) + web := withDialogueID(context.Background(), dialogueIDFor(sourceText, "web")) + telegram := withDialogueID(context.Background(), dialogueIDFor(sourceText, "telegram:42")) + + if _, asked := h.askClarify(web, clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { + t.Fatal("expected a question on the web conversation") + } + if _, handled := h.resolveClarifyAnswer(telegram, "в 11:00"); handled { + t.Fatal("a question asked on the web must not eat a telegram utterance") + } + if _, handled := h.resolveClarifyAnswer(voiceCtx(), "в 11:00"); handled { + t.Fatal("a question asked on the web must not eat what he says at the mic") + } + if reply, handled := h.resolveClarifyAnswer(web, "в 11:00"); !handled || reply == clarifyGaveUp { + t.Fatalf("the asker's own answer must land, handled=%v reply=%q", handled, reply) + } +} + +// voiceCtx — the mic's conversation, which carries no id of its own. +func voiceCtx() context.Context { + return withDialogueID(context.Background(), dialogueIDFor(sourceVoice, "")) +} + +// TestARestartExpiresTheParkedQuestion pins the Vikunja #385 decision: the +// question dies with the process, and she does not claim to have let it go — +// the words that follow are routed as a fresh request. Restarting is modelled +// the way the daemon does it, by building a second handler over the same store. +func TestARestartExpiresTheParkedQuestion(t *testing.T) { + h, _, _ := newClarifyHandler(t) + ctx := voiceCtx() + if _, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{Text: "напомни"}, "напомни")); !asked { + t.Fatal("expected a question before the restart") + } + + restarted, _, _ := newClarifyHandler(t) + if _, handled := restarted.resolveClarifyAnswer(ctx, "в 11:00"); handled { + t.Fatal("a question parked before the restart must not eat the next utterance") + } + if notice := restarted.clarifyExpiredNotice(ctx); notice != "" { + t.Fatalf("notice = %q, want silence: nothing survived to expire", notice) + } +} diff --git a/cmd/mavend/ecosystem_acts.go b/cmd/mavend/ecosystem_acts.go index 9a0cdff..dc1e6fc 100644 --- a/cmd/mavend/ecosystem_acts.go +++ b/cmd/mavend/ecosystem_acts.go @@ -550,11 +550,14 @@ func (h *reactiveHandler) handleHexisAct(ctx context.Context, dec router.Decisio // Resolve the utterance text as an entity reference through Nexus. An // ambiguous match must stop and clarify — never guess a mutation target. + // The name comes from entityReferenceText, not straight from the Text slot: + // the model transliterates Latin names as it routes (Vikunja #476). + subject := entityReferenceText(dec) started := h.now() - entityID, displayName, ambiguous, err := h.ecosystem.resolveEntityReference(ctx, dec.Slots.Text, nil) + entityID, displayName, ambiguous, err := h.ecosystem.resolveEntityReference(ctx, subject, nil) if err != nil { h.recordEcosystemTrace(ctx, "nexus", "resolve", traceStatusForError(err), started, - mergeFields(traceErrorFields(err), map[string]any{"subject": redactSubject(dec.Slots.Text)})) + mergeFields(traceErrorFields(err), map[string]any{"subject": redactSubject(subject)})) if unauthorizedEcosystemError(err) { return phraser.A(phraser.EcoDenied, serviceVars(serviceNexus)) } @@ -571,7 +574,7 @@ func (h *reactiveHandler) handleHexisAct(ctx context.Context, dec router.Decisio } if entityID == "" { h.recordEcosystemTrace(ctx, "nexus", "resolve", traceNotFound, started, - map[string]any{"subject": redactSubject(dec.Slots.Text)}) + map[string]any{"subject": redactSubject(subject)}) return "" } h.recordEcosystemTrace(ctx, "nexus", "resolve", traceOK, started, @@ -605,10 +608,21 @@ func (h *reactiveHandler) handleHexisAct(ctx context.Context, dec router.Decisio } verbLower := strings.ToLower(verb) + // With no allowlisted fn the verb is a whole phrase ("restart status muzick + // indexer"), which no capability name ever contains. Read it the other way + // round then: the phrase is the haystack and the capability name is what we + // look for in it (Vikunja #476). Only when the fn slot is empty — a matched + // fn is a single verb and containment already means what it says. + loose := !dec.Slots.HasFn var matches []*hexisclient.Capability for i, c := range caps { - if strings.Contains(strings.ToLower(c.Name), verbLower) || - (c.Description != "" && strings.Contains(strings.ToLower(c.Description), verbLower)) { + name := strings.ToLower(c.Name) + hit := strings.Contains(name, verbLower) || + (c.Description != "" && strings.Contains(strings.ToLower(c.Description), verbLower)) + if loose && name != "" && strings.Contains(verbLower, name) { + hit = true + } + if hit { matches = append(matches, &caps[i]) } } @@ -685,3 +699,29 @@ func (h *reactiveHandler) execHexis(ctx context.Context, capID, capName, entityI }) return phraser.A(phraser.ActDoneEntity, map[string]string{"name": displayName}) } + +// hexisBeforeClarify gives an entity-shaped act one chance at Hexis before she +// asks what to do. +// +// The stage-3 gate thins an act that never matched an allowlisted fn, so +// "перезапусти muzick indexer" was answered with "Что сделать?" and the Hexis +// path was never entered — the capability existed and no utterance could reach +// it (Vikunja #476). Hexis is exactly where an act with no local fn belongs: +// the verb is matched against the capabilities Hexis registers for the entity, +// not against the allowlist. +// +// Narrow on purpose. Only an act, only when the fn slot is still empty, and +// only when Hexis is wired — a box with no ecosystem asks the question it +// always asked. A "" back means Nexus knew no such entity or Hexis had no +// matching capability, and then she asks after all. Authority is unchanged: +// resolution stops on ambiguity and a mutating capability still goes through +// the spoken confirm in handleHexisAct. +func (h *reactiveHandler) hexisBeforeClarify(ctx context.Context, dec router.Decision) string { + if h.ecosystem == nil || h.ecosystem.hexis == nil { + return "" + } + if dec.Intent != router.IntentAct || dec.Slots.HasFn || dec.Slots.Text == "" { + return "" + } + return h.handleHexisAct(ctx, dec) +} diff --git a/cmd/mavend/entityname.go b/cmd/mavend/entityname.go new file mode 100644 index 0000000..bc42b75 --- /dev/null +++ b/cmd/mavend/entityname.go @@ -0,0 +1,59 @@ +package main + +import ( + "regexp" + "strings" + "unicode" + + "github.com/kami/maven/internal/router" +) + +// latinRun matches a run of Latin-script words — the shape a service, host or +// project name takes in a Russian sentence. Digits, dot, dash and underscore +// ride along because "muzick-indexer" and "nginx.conf" are one name, not two. +var latinRun = regexp.MustCompile(`[A-Za-z][A-Za-z0-9._-]*(?:\s+[A-Za-z][A-Za-z0-9._-]*)*`) + +// hasLatin reports whether s carries a Latin letter. +func hasLatin(s string) bool { + for _, r := range s { + if unicode.In(r, unicode.Latin) { + return true + } + } + return false +} + +// entityReferenceText is the name Nexus is asked to resolve. +// +// Normally that is the router's Text slot, which is the verb phrase the model +// wrote. But the resident model rewrites a Russian utterance as it routes, and +// on the way it transliterates: "перезапусти muzick indexer" came back as +// "перезагрузить музик индексер" (Vikunja #476). Nexus is then asked for a +// service nobody has ever named, so the act cannot resolve its target even +// with every gate open. +// +// The recovery is deliberately narrow. Only when the utterance holds a Latin +// run and the model's Text holds none has a name certainly been rewritten — +// then the longest Latin run in his own words is the reference. Anything else +// keeps the Text slot, so an English utterance and a Russian entity name are +// both untouched. Un-transliterating the Cyrillic back is not attempted: the +// surface form he said is right there, and guessing at a reverse mapping would +// invent a second name to be wrong about. +func entityReferenceText(dec router.Decision) string { + text := dec.Slots.Text + if hasLatin(text) || !hasLatin(dec.Utterance) { + return text + } + longest := "" + for _, m := range latinRun.FindAllString(dec.Utterance, -1) { + if len(m) > len(longest) { + longest = m + } + } + longest = strings.TrimSpace(longest) + // A single stray letter is not a name. + if len(longest) < 2 { + return text + } + return longest +} diff --git a/cmd/mavend/entityname_test.go b/cmd/mavend/entityname_test.go new file mode 100644 index 0000000..05e7700 --- /dev/null +++ b/cmd/mavend/entityname_test.go @@ -0,0 +1,133 @@ +package main + +import ( + "context" + "strings" + "testing" + + "github.com/kami/maven/internal/router" +) + +// TestEntityReferenceText pins when his own words win over the model's. +func TestEntityReferenceText(t *testing.T) { + for _, tc := range []struct { + name string + utterance string + text string + want string + }{ + { + name: "the model transliterated the name", + utterance: "перезапусти muzick indexer", + text: "перезагрузить музик индексер", + want: "muzick indexer", + }, + { + name: "it kept the name, so nothing to repair", + utterance: "перезапусти muzick indexer", + text: "перезагрузить muzick indexer", + want: "перезагрузить muzick indexer", + }, + { + name: "an all-Russian entity name is not a rewrite", + utterance: "перезапусти домашний сервер", + text: "перезагрузить домашний сервер", + want: "перезагрузить домашний сервер", + }, + { + name: "an English turn never enters the recovery", + utterance: "restart muzick indexer", + text: "restart muzick indexer", + want: "restart muzick indexer", + }, + { + name: "the longest Latin run is the name", + utterance: "а перезапусти-ка nginx на muzick-indexer, пожалуйста", + text: "перезагрузить нгинкс", + want: "muzick-indexer", + }, + { + name: "one stray letter is not a name", + utterance: "перезапусти сервер a", + text: "перезагрузить сервер", + want: "перезагрузить сервер", + }, + } { + t.Run(tc.name, func(t *testing.T) { + dec := router.Decision{Utterance: tc.utterance, Slots: router.Slots{Text: tc.text}} + if got := entityReferenceText(dec); got != tc.want { + t.Fatalf("entityReferenceText = %q, want %q", got, tc.want) + } + }) + } +} + +// TestNexusIsAskedForTheNameHeSaid — the defect end to end (Vikunja #476): the +// router hands over a transliterated Text, and Nexus must still be asked about +// the service that exists. +func TestNexusIsAskedForTheNameHeSaid(t *testing.T) { + ctx := context.Background() + nexus := newFakeNexus(t, fixtureNexusResolved("ent_muzick", "Muzick indexer", "service")) + hexis := newFakeHexis(t, restartCaps(), fixtureHexisExecuted("exec_1", "succeeded")) + h := ecoHandler(t, nexus, nil, hexis) + + dec := router.Decision{ + Utterance: "перезапусти muzick indexer", + Intent: router.IntentAct, + Slots: router.Slots{Text: "перезагрузить музик индексер", Fn: "restart", HasFn: true}, + } + h.handleHexisAct(ctx, dec) + + reqs := nexus.Requests() + if len(reqs) == 0 { + t.Fatal("nexus was never asked") + } + body := string(reqs[0].Body) + if !strings.Contains(body, "muzick indexer") { + t.Fatalf("nexus resolve body = %s, want the name he said", body) + } +} + +// TestAnEntityActReachesHexisInsteadOfAsking — the second half of #476. The +// stage-3 gate thins an act with no allowlisted fn, and that question used to +// be the whole turn, so the Hexis path was unreachable from voice or chat. +func TestAnEntityActReachesHexisInsteadOfAsking(t *testing.T) { + ctx := context.Background() + nexus := newFakeNexus(t, fixtureNexusResolved("ent_muzick", "Muzick indexer", "service")) + hexis := newFakeHexis(t, restartCaps(), fixtureHexisExecuted("exec_1", "succeeded")) + h := ecoHandler(t, nexus, nil, hexis) + + dec := router.Decision{ + Utterance: "перезапусти muzick indexer", + Intent: router.IntentAct, + Stage: 3, + Clarify: true, + Slots: router.Slots{Text: "restart status muzick indexer"}, + } + reply := h.hexisBeforeClarify(ctx, dec) + if reply == "" { + t.Fatal("a resolvable entity act must reach hexis rather than fall through to the question") + } + if hexis.Count("", "/api/v1") == 0 { + t.Fatal("hexis was never contacted") + } +} + +// TestClarifyStillAsksWithoutHexis — the narrowing. No ecosystem, no change: +// she asks exactly what she asked before. +func TestClarifyStillAsksWithoutHexis(t *testing.T) { + h, _, _ := newClarifyHandler(t) + dec := router.Decision{ + Utterance: "перезапусти muzick indexer", + Intent: router.IntentAct, + Stage: 3, + Clarify: true, + Slots: router.Slots{Text: "перезагрузить музик индексер"}, + } + if reply := h.hexisBeforeClarify(context.Background(), dec); reply != "" { + t.Fatalf("no hexis must mean no reply, got %q", reply) + } + if _, asked := h.askClarify(voiceCtx(), dec); !asked { + t.Fatal("she must still ask what to do") + } +} diff --git a/cmd/mavend/followup.go b/cmd/mavend/followup.go index a8b0d75..2ec0f49 100644 --- a/cmd/mavend/followup.go +++ b/cmd/mavend/followup.go @@ -1,17 +1,64 @@ package main import ( + "context" "time" "github.com/kami/maven/internal/dialogue" "github.com/kami/maven/internal/router" ) -// voiceDialogueID — the single dialogue-session key. This is a single-user box -// (ponytail), so one slot suffices; a second speaker would need per-speaker ids, -// which waits on voice-print attribution (see PROGRESS multi-user deferral). +// voiceDialogueID — the dialogue-session key for the microphone, and the +// clarify key for it too. This is a single-user box (ponytail), so one slot +// suffices; a second speaker would need per-speaker ids, which waits on +// voice-print attribution (see PROGRESS multi-user deferral). const voiceDialogueID = "voice" +// textDialogueID — the clarify key for a text turn that named no conversation. +// Separate from the mic: an old client that sends no id still must not answer +// a question she asked out loud. +const textDialogueID = "text" + +// dialogueKey — the context key carrying the id of the conversation this turn +// belongs to. It rides the context rather than a parameter for the same reason +// the correlation id does: every step of the turn needs it, most of them only +// to hand to the next one, and threading it by hand would put it in six +// clarify signatures that have nothing else to say about it. +type dialogueKey struct{} + +// dialogueIDFor builds the id a turn is held under: the conversation the reach +// named, qualified by the tap it arrived on, or the tap's own fallback when it +// named none. +// +// A parked clarifying question used to be held under voiceDialogueID no matter +// where the turn came from, so one unanswerable question captured the next +// three utterances from anywhere. Three independent curl sessions fed a +// capture attempt that had already failed, and a reminder among them was lost +// (Vikunja #466). +func dialogueIDFor(src turnSource, conversation string) string { + if conversation != "" { + return string(src) + ":" + conversation + } + if src == sourceVoice { + return voiceDialogueID + } + return textDialogueID +} + +// withDialogueID tags a turn with that id. +func withDialogueID(ctx context.Context, id string) context.Context { + return context.WithValue(ctx, dialogueKey{}, id) +} + +// dialogueIDOf reads it back. Falls back to the microphone's slot, which is +// what an unthreaded caller — a test, an internal replay — gets. +func dialogueIDOf(ctx context.Context) string { + if id, ok := ctx.Value(dialogueKey{}).(string); ok && id != "" { + return id + } + return voiceDialogueID +} + // toDialogueSlots and applyDialogueSlots are the only bridge between // router.Slots and dialogue.Slots. dialogue must not import router (import // cycle), so the two structs are hand-kept copies and every field has to be diff --git a/cmd/mavend/morning_nudge_test.go b/cmd/mavend/morning_nudge_test.go new file mode 100644 index 0000000..8f4fc07 --- /dev/null +++ b/cmd/mavend/morning_nudge_test.go @@ -0,0 +1,39 @@ +package main + +import ( + "strings" + "testing" + + "github.com/kami/maven/internal/morning" +) + +// TestMorningNudgeBodySeparatesOptional — the one message a routine is allowed +// per day says what was not done, then what he could still do (Vikunja #473). +func TestMorningNudgeBodySeparatesOptional(t *testing.T) { + cand := morning.Candidate{ + Routine: morning.Routine{Name: "утро"}, + Missing: []morning.Item{ + {Key: "meds", Label: "таблетки"}, + {Key: "stretch", Label: "растяжка", Optional: true}, + }, + } + body := morningNudgeBody(cand) + if !strings.Contains(body, "не сделано — таблетки") { + t.Fatalf("the required item must be named as not done: %q", body) + } + if !strings.Contains(body, "если будет время — растяжка") { + t.Fatalf("the optional item must read softer: %q", body) + } + if strings.Contains(body, "не сделано — таблетки, растяжка") { + t.Fatalf("optional must not be folded into the required list: %q", body) + } + + // Nothing optional missing: the sentence is what it always was. + only := morning.Candidate{ + Routine: morning.Routine{Name: "утро"}, + Missing: []morning.Item{{Key: "meds", Label: "таблетки"}}, + } + if got, want := morningNudgeBody(only), "утро: не сделано — таблетки"; got != want { + t.Fatalf("morningNudgeBody = %q, want %q", got, want) + } +} diff --git a/cmd/mavend/reactive_notes_test.go b/cmd/mavend/reactive_notes_test.go index 66f93dd..b53512c 100644 --- a/cmd/mavend/reactive_notes_test.go +++ b/cmd/mavend/reactive_notes_test.go @@ -2,12 +2,14 @@ package main import ( "context" + "strings" "testing" "time" "github.com/kami/maven/internal/ipc" "github.com/kami/maven/internal/memory" "github.com/kami/maven/internal/router" + "github.com/kami/maven/internal/store" "github.com/kami/maven/internal/tool" "github.com/kami/maven/internal/voice" ) @@ -85,3 +87,38 @@ func TestReactiveNotesReminders(t *testing.T) { } }) } + +// TestSpokenTaskCaptureFilesATask — the whole path, from the utterance to the +// task table. It went dead when the router started claiming the marker as an +// act: capture rides the note intent, so nothing below actionNote was ever +// reached and every capture answered "Что сделать?" (Vikunja #467). +func TestSpokenTaskCaptureFilesATask(t *testing.T) { + ctx := context.Background() + st := newTestStore(t) + api := ipc.NewStoreAPI(st) + now := time.Now() + emb := router.NewHashEmbedder(1024) + matcher := tool.NewMatcher(api) + h := &reactiveHandler{ + api: api, + embedder: emb, + router: buildRouter(emb, matcher, 0.55, nil), + replier: voice.NewStubReplier(), + now: func() time.Time { return now }, + memStore: memory.NewInMemoryStore(), + dataStore: st, + } + + reply := h.handleText(ctx, "web", "добавь в задачи купить молоко") + if !strings.Contains(reply, "купить молоко") { + t.Fatalf("capture did not claim the turn: %q", reply) + } + open, err := st.ListTasks(ctx, store.TaskOpen) + if err != nil || len(open) != 1 { + t.Fatalf("task was not filed: tasks=%v err=%v", open, err) + } + // The words he said, not the model's rewrite of them. + if open[0].Text != "купить молоко" { + t.Fatalf("task text was rewritten: %q", open[0].Text) + } +} diff --git a/cmd/mavend/simulator_test.go b/cmd/mavend/simulator_test.go index 2d1b295..bdedaf6 100644 --- a/cmd/mavend/simulator_test.go +++ b/cmd/mavend/simulator_test.go @@ -1043,3 +1043,26 @@ func TestSimulatorRefusesBackwardsSteps(t *testing.T) { t.Errorf("the clock moved to %s on a refused step, it must stay at 09:00", got) } } + +// TestSimulatorRoutesWithTheDeployedSeeds — the scenarios must replay against +// the classifier the deploy runs, not an empty one. +// +// They did not. The seed path was relative to the working directory, which is +// cmd/mavend under `go test`, so every file failed to open and the whole +// simulator scored three green scenarios with zero examples loaded (Vikunja +// #465). The count is asserted rather than logged, because a silent zero is +// exactly the failure that hid here for as long as it did. +func TestSimulatorRoutesWithTheDeployedSeeds(t *testing.T) { + cls := router.NewClassifier(router.NewHashEmbedder(1024)) + seedClassifier(cls) + total := 0 + for _, intent := range cls.Intents() { + total += len(cls.Examples(intent)) + } + if total == 0 { + t.Fatalf("no seed examples loaded from %s — the simulator would route on nothing", seedPath()) + } + if len(cls.Intents()) != 7 { + t.Fatalf("seeded %d intents, want all 7", len(cls.Intents())) + } +} diff --git a/cmd/mavend/tick.go b/cmd/mavend/tick.go index ab5987b..4266a0e 100644 --- a/cmd/mavend/tick.go +++ b/cmd/mavend/tick.go @@ -769,11 +769,7 @@ func (t *tickLoop) fireMorningRoutines(ctx context.Context, now time.Time, state facts := t.gatherMorningFacts(ctx) for _, cand := range morning.Due(t.morningRoutines, facts, t.morningLast, now) { - labels := make([]string, len(cand.Missing)) - for i, it := range cand.Missing { - labels[i] = it.Label - } - body := fmt.Sprintf("%s: не сделано — %s", cand.Routine.Name, strings.Join(labels, ", ")) + body := morningNudgeBody(cand) pn := delivery.PhrasedNudge{ Candidate: loop.Candidate{ Rule: loop.Rule{Name: "morning:" + cand.Routine.Name, Severity: loop.Severity(cand.Routine.Severity)}, @@ -789,6 +785,26 @@ func (t *tickLoop) fireMorningRoutines(ctx context.Context, now time.Time, state } } +// morningNudgeBody words the one message a routine gets per day. Required +// items are what she says was not done; optional ones follow, worded as +// something he could still do rather than something he owes (Vikunja #473). +// Operator text, not phrased by the model, for the same reason it always was: +// a checklist item must not be invented. +func morningNudgeBody(cand morning.Candidate) string { + labels := func(items []morning.Item) string { + out := make([]string, len(items)) + for i, it := range items { + out[i] = it.Label + } + return strings.Join(out, ", ") + } + body := fmt.Sprintf("%s: не сделано — %s", cand.Routine.Name, labels(morning.Required(cand.Missing))) + if opt := morning.OptionalOnly(cand.Missing); len(opt) > 0 { + body += fmt.Sprintf(". если будет время — %s", labels(opt)) + } + return body +} + // gatherMorningFacts reads the latest fact for every item's fact_key across // all configured morning routines. Shared by fireMorningRoutines (nudge // decision) and morningStatus (read-only query) so the two paths can never @@ -994,7 +1010,7 @@ type daemonAPI struct { getTrace func() *loop.TickTrace getMorningStatus func(ctx context.Context) []ipc.MorningRoutineStatus getDayPlan func(ctx context.Context) ipc.DayPlan - chatFn func(ctx context.Context, text string) string + chatFn func(ctx context.Context, conversation, text string) string getMCPServers func() []ipc.MCPServerStatus getEvents func(n int) []ipc.IntakeEvent } @@ -1010,11 +1026,11 @@ func (d *daemonAPI) RecentEvents(ctx context.Context, n int) ([]ipc.IntakeEvent, return d.getEvents(n), nil } -func (d *daemonAPI) Chat(ctx context.Context, text string) (string, error) { +func (d *daemonAPI) Chat(ctx context.Context, conversation, text string) (string, error) { if d.chatFn == nil { return "", errors.New("mavend: chat not available") } - return d.chatFn(ctx, text), nil + return d.chatFn(ctx, conversation, text), nil } // MCPServers — the configured MCP servers and their health (Vikunja #251). diff --git a/cmd/mavend/voice.go b/cmd/mavend/voice.go index c86a09a..e7f4764 100644 --- a/cmd/mavend/voice.go +++ b/cmd/mavend/voice.go @@ -220,9 +220,9 @@ func (h *reactiveHandler) upgradeAPI(api ipc.CoreAPI) { // handleText — the core reactive path without stt/tts. Used by the IPC Chat // endpoint (and eventually by telegram). Splits out the audio bookends from // HandlePushToTalk so text channels share the same routing logic. -func (h *reactiveHandler) handleText(ctx context.Context, text string) string { +func (h *reactiveHandler) handleText(ctx context.Context, conversation, text string) string { log.Printf("voice: handleText: %q", text) - return h.runTurn(ctx, text, sourceText) + return h.runTurn(withDialogueID(ctx, dialogueIDFor(sourceText, conversation)), text, sourceText) } // turnSource — which channel this utterance arrived on, in the same provenance @@ -255,7 +255,7 @@ func (h *reactiveHandler) runTurn(ctx context.Context, text string, src turnSour // early. He can be asked a question, walk off, come back and say "да" to a // confirm that is still parked; computing the notice after that return meant // he answered the confirm and never heard that the older request was let go. - expiredNotice := h.clarifyExpiredNotice() + expiredNotice := h.clarifyExpiredNotice(ctx) // 2. confirm turn — if a destructive act is parked, this utterance is its // y/n answer, not a fresh command. Handled before routing so "да" doesn't @@ -351,7 +351,10 @@ func (h *reactiveHandler) runTurn(ctx context.Context, text string, src turnSour // and park the request (clarify.go); otherwise the replier's canned reply // stands. if dec.Clarify { - if question, asked := h.askClarify(dec); asked { + if reply := h.hexisBeforeClarify(ctx, dec); reply != "" { + return withNotice(expiredNotice, reply) + } + if question, asked := h.askClarify(ctx, dec); asked { return withNotice(expiredNotice, question) } } diff --git a/cmd/mavend/voicewire.go b/cmd/mavend/voicewire.go index 363a17e..7658f15 100644 --- a/cmd/mavend/voicewire.go +++ b/cmd/mavend/voicewire.go @@ -238,7 +238,10 @@ func wireVoice(cfg *config.Config, coreAPI ipc.CoreAPI, phr phraser.Phraser, mem // ----- dialogue (multi-turn slot carry-over; 2-min follow-up window) ----- // Store-backed when the daemon passes a store, so a restart mid-conversation // keeps the thread (Vikunja #363). Sessions past their TTL are dropped on - // load, never revived. Clarify's parked question stays in memory only. + // load, never revived. Clarify's parked question stays in memory only, and + // that is a decision rather than an omission (Vikunja #385, docs/design.md): + // a restart expires it, so the thread comes back and the open question does + // not. var dialogueSessions *dialogue.SessionStore if dataStore != nil { dialogueSessions = dialogue.NewPersistentSessionStore(2*time.Minute, dataStore) @@ -384,6 +387,11 @@ func buildRouter(emb router.Embedder, acts router.ActMatcher, threshold float64, grammars = append(grammars, router.NarrativeQueryGrammars()...) grammars = append(grammars, router.ListGrammars()...) grammars = append(grammars, router.ReminderGrammar()) + // Last, and it matches any utterance shape — its Build is the filter. An + // explicit capture marker beats the model, which called it an act and + // rewrote the task text (Vikunja #467). After the rules above because a + // marker never collides with a clock or agenda question. + grammars = append(grammars, router.TaskCaptureGrammar()) return router.New(router.Config{ Grammars: grammars, Classifier: cls, @@ -397,11 +405,34 @@ func buildRouter(emb router.Embedder, acts router.ActMatcher, threshold float64, }) } -// seedDir is the directory containing intent seed files. Each file is named -// .txt and contains one training example per line (blank lines and -// lines starting with # are ignored). Relative to the working directory. +// seedDir is the directory containing intent seed files, relative to the repo +// root. Each file is named .txt and holds one training example per +// line (blank lines and lines starting with # are ignored). const seedDir = "models/seeds" +// seedPath resolves seedDir against the working directory, walking up until it +// finds it. The daemon runs from the repo root and the first candidate hits. +// +// A test does not: `go test ./cmd/mavend/` runs with the working directory at +// cmd/mavend, so every open failed and the simulator scenarios replayed a whole +// scripted day against a classifier holding zero examples (Vikunja #465). They +// passed, which is the part that matters — a green simulator was not exercising +// the routing the deploy runs, and a regression in the seed set could not have +// shown up there. +// +// Bounded at five levels, so a daemon started somewhere without the seeds logs +// the same failure it always did rather than walking to the filesystem root. +func seedPath() string { + dir := seedDir + for i := 0; i < 5; i++ { + if st, err := os.Stat(dir); err == nil && st.IsDir() { + return dir + } + dir = filepath.Join("..", dir) + } + return seedDir +} + // seedClassifier floors the embedded examples so the cold-boot path // doesn't return ErrNoIntents. Loads examples from seedDir — one file per // intent (act.txt, reminder.txt, fact.txt, note.txt, query.txt). When the @@ -426,11 +457,11 @@ func seedClassifier(c *router.Classifier) { } total += n } - log.Printf("voice: loaded %d seed examples from %s", total, seedDir) + log.Printf("voice: loaded %d seed examples from %s", total, seedPath()) } func loadSeedFile(c *router.Classifier, intent router.Intent) (int, error) { - path := filepath.Join(seedDir, string(intent)+".txt") + path := filepath.Join(seedPath(), string(intent)+".txt") f, err := os.Open(path) if err != nil { return 0, fmt.Errorf("open %s: %w", path, err) diff --git a/cmd/mavend/weatherq.go b/cmd/mavend/weatherq.go index 344e2fa..04f67f9 100644 --- a/cmd/mavend/weatherq.go +++ b/cmd/mavend/weatherq.go @@ -1,10 +1,13 @@ // Package main — weatherq.go holds the weather-query keyword helpers: does -// this utterance ask about weather at all, and which city (if any) did it -// name. Both are plain substring/lookup matching, not NLU — extend this file -// rather than voice.go for anything in that shape. +// this utterance ask about weather at all, and which place (if any) did he +// name. Both are plain keyword matching, not NLU — extend this file rather +// than voice.go for anything in that shape. package main -import "strings" +import ( + "regexp" + "strings" +) // isWeatherQuery returns true if the utterance is about weather. func isWeatherQuery(u string) bool { @@ -19,40 +22,49 @@ func isWeatherQuery(u string) bool { strings.Contains(lower, "temperature") } -// weatherCities — the city names an utterance may name explicitly, as -// lowercase substrings mapped to the provider's spelling. This is a -// convenience for "какая погода в Лондоне", NOT a source of default truth: -// nothing here is used unless he actually said it. -var weatherCities = map[string]string{ - "москв": "Moscow", - "moscow": "Moscow", - "питер": "Saint Petersburg", - "spb": "Saint Petersburg", - "петербур": "Saint Petersburg", - "лондон": "London", - "london": "London", - "париж": "Paris", - "paris": "Paris", - "берлин": "Berlin", - "berlin": "Berlin", - "нью-йорк": "New York", - "new york": "New York", +// weatherPlace — the place he named, after "в"/"во"/"in". One or two words, +// letters and dashes only, so "в Нижнем Новгороде" and "in New York" both +// come through whole and "в 5 утра" does not. +var weatherPlace = regexp.MustCompile(`(?i)(?:^|\s)(?:в|во|in)\s+([\p{L}-]+(?:\s+[\p{L}-]+)?)`) + +// weatherNonPlaces — words that follow "в" in a weather question and are not +// cities. "какая погода в доме" is the smart-home sensor, not Open-Meteo, and +// "тепло в комнате" is the same question about the same room. +var weatherNonPlaces = map[string]bool{ + "доме": true, "квартире": true, "комнате": true, "спальне": true, + "гостиной": true, "кухне": true, "гараже": true, "офисе": true, + "выходные": true, "субботу": true, "воскресенье": true, "понедельник": true, + "вторник": true, "среду": true, "четверг": true, "пятницу": true, + "обед": true, "обеде": true, "утро": true, "утром": true, "вечер": true, + "вечером": true, "ночь": true, "ночью": true, "целом": true, "принципе": true, } -// extractWeatherLocation returns the city he named, or the configured default +// extractWeatherLocation returns the place he named, or the configured default // when he named none. It returns "" when he named none AND no default is // configured — the caller must then say it does not know. // -// It used to return "Moscow" in that case. That is a made-up answer presented -// as fact: reading out Moscow's temperature to someone who is not in Moscow is -// wrong in exactly the way maven must never be wrong. voice.weather -// .default_location is the only source of an unstated location. +// It used to be a hand-written table of six cities in two spellings each +// (Vikunja #421). Anything outside it — Kazan, Tbilisi — was dropped silently +// and answered for the default location, which reads as a correct answer about +// the wrong place. There is a geocoder behind this now: internal/weather +// already calls Open-Meteo's geocoding endpoint for every lookup, so any place +// it knows is a place he can ask about, and the table bought nothing. +// +// A named place that the geocoder cannot resolve is the caller's problem to +// report, not this function's to hide. +// +// It used to return "Moscow" when he named nothing. That is a made-up answer +// presented as fact. voice.weather.default_location is the only source of an +// unstated location. func extractWeatherLocation(u, defaultLoc string) string { - lower := strings.ToLower(u) - for substr, name := range weatherCities { - if strings.Contains(lower, substr) { - return name - } + m := weatherPlace.FindStringSubmatch(u) + if m == nil { + return defaultLoc } - return defaultLoc + place := strings.TrimSpace(m[1]) + first := strings.ToLower(strings.Fields(place)[0]) + if weatherNonPlaces[first] { + return defaultLoc + } + return place } diff --git a/cmd/mavend/weatherq_test.go b/cmd/mavend/weatherq_test.go new file mode 100644 index 0000000..b41ca9e --- /dev/null +++ b/cmd/mavend/weatherq_test.go @@ -0,0 +1,33 @@ +package main + +import "testing" + +// TestExtractWeatherLocation — any place he names comes through, not just the +// six that used to be in a table (Vikunja #421). +func TestExtractWeatherLocation(t *testing.T) { + cases := []struct { + utterance string + def string + want string + }{ + // The cities the table had, and the ones it silently dropped. + {"какая погода в Москве", "Berlin", "Москве"}, + {"какая погода в Казани", "Berlin", "Казани"}, + {"погода в Тбилиси?", "Berlin", "Тбилиси"}, + {"what's the weather in New York", "Berlin", "New York"}, + {"тепло в Нижнем Новгороде?", "Berlin", "Нижнем Новгороде"}, + // He named nothing: the configured default, and nothing at all when + // there is no default. + {"какая сегодня погода", "Berlin", "Berlin"}, + {"какая сегодня погода", "", ""}, + // "в" followed by something that is not a place stays the default — + // the house sensors and the day words answer elsewhere. + {"тепло в комнате?", "Berlin", "Berlin"}, + {"какая погода в выходные", "Berlin", "Berlin"}, + } + for _, c := range cases { + if got := extractWeatherLocation(c.utterance, c.def); got != c.want { + t.Errorf("extractWeatherLocation(%q, %q) = %q, want %q", c.utterance, c.def, got, c.want) + } + } +} diff --git a/cmd/mavweb/handlers_test.go b/cmd/mavweb/handlers_test.go index 6b56c51..45db011 100644 --- a/cmd/mavweb/handlers_test.go +++ b/cmd/mavweb/handlers_test.go @@ -54,7 +54,9 @@ type fakeCore struct { revertErr error // for handleNotifications tests - nudgesErr error + nudgesErr error + attempts []ipc.DeliveryAttempt + attemptStatus string // for handleHistory tests historyFacts []ipc.Fact @@ -77,7 +79,7 @@ func (f *fakeCore) MCPServers(context.Context) ([]ipc.MCPServerStatus, error) { return f.mcpServers, f.mcpErr } -func (f *fakeCore) Chat(_ context.Context, text string) (string, error) { +func (f *fakeCore) Chat(_ context.Context, _, text string) (string, error) { f.chatText = text if f.chatErr != nil { return "", f.chatErr @@ -1251,3 +1253,35 @@ func TestHandleWS_AssertedSession_PassesGate(t *testing.T) { t.Fatalf("status = 403 on an asserted session; body=%s", rr.Body.String()) } } + +func (f *fakeCore) DeliveryAttempts(_ context.Context, status string, _ int) ([]ipc.DeliveryAttempt, error) { + f.attemptStatus = status + return f.attempts, nil +} + +// TestHandleNotifications_ShowsTheOutbox — the outbox was written and never +// read, so a dropped or failed send was invisible (Vikunja #390). +func TestHandleNotifications_ShowsTheOutbox(t *testing.T) { + done := time.Date(2026, 8, 4, 9, 0, 30, 0, time.UTC) + core := &fakeCore{ + attempts: []ipc.DeliveryAttempt{ + {Kind: "nudge", Rule: "care-check", Channel: "telegram", Status: "dropped", + Created: done.Add(-30 * time.Second), Completed: &done}, + {Kind: "reminder", ReminderID: 7, Channel: "voice", Status: "pending", Created: done}, + }, + } + rr := httptest.NewRecorder() + handleNotifications(rr, httptest.NewRequest(http.MethodGet, "/notifications?status=dropped", nil), core) + if rr.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", rr.Code, rr.Body.String()) + } + if core.attemptStatus != "dropped" { + t.Errorf("status filter = %q, want it passed through", core.attemptStatus) + } + body := rr.Body.String() + for _, want := range []string{"care-check", "dropped", "reminder #7", "Delivery outbox"} { + if !strings.Contains(body, want) { + t.Errorf("rendered outbox missing %q", want) + } + } +} diff --git a/cmd/mavweb/main.go b/cmd/mavweb/main.go index be7e5eb..9d8368c 100644 --- a/cmd/mavweb/main.go +++ b/cmd/mavweb/main.go @@ -893,12 +893,59 @@ func handleNotifications(w http.ResponseWriter, r *http.Request, core ipc.CoreAP http.Error(w, "notifications error: "+err.Error(), http.StatusBadGateway) return } + // The outbox, on the page that already answers "what did she send". + // A failed or dropped attempt is why she went quiet, and until now it was + // recorded and unreadable (Vikunja #390). Filter with ?status=dropped. + status := r.URL.Query().Get("status") + attempts, err := core.DeliveryAttempts(ctx, status, 50) + if err != nil { + // The nudge list is still worth showing, so this is a note on the page + // rather than a dead page. + log.Printf("notifications: delivery attempts: %v", err) + } w.Header().Set("Content-Type", "text/html; charset=utf-8") - if err := notificationsTmpl.Execute(w, map[string]any{"Nudges": nudges}); err != nil { + if err := notificationsTmpl.Execute(w, map[string]any{ + "Nudges": nudges, + "Attempts": deliveryRows(attempts), + "Status": status, + }); err != nil { log.Printf("notifications template: %v", err) } } +// deliveryRow is one outbox line, with every timestamp already formatted so +// the template holds no date logic — same shape as taskRow. +type deliveryRow struct { + Kind string + Target string + Channel string + Status string + Created string + Completed string +} + +func deliveryRows(as []ipc.DeliveryAttempt) []deliveryRow { + out := make([]deliveryRow, 0, len(as)) + for _, a := range as { + target := a.Rule + if target == "" && a.ReminderID != 0 { + target = "reminder #" + strconv.FormatInt(a.ReminderID, 10) + } + row := deliveryRow{ + Kind: a.Kind, + Target: target, + Channel: a.Channel, + Status: a.Status, + Created: a.Created.Format("02.01 15:04"), + } + if a.Completed != nil { + row.Completed = a.Completed.Format("15:04") + } + out = append(out, row) + } + return out +} + func handleReminders(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI) { if core == nil { http.Error(w, "reminders disabled (no -core)", http.StatusServiceUnavailable) @@ -1678,7 +1725,13 @@ func handleChatAPI(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI, ses http.Redirect(w, r, "/chat", http.StatusSeeOther) return } - reply, err := core.Chat(r.Context(), text) + // One conversation id for the whole web chat, and a different one from + // telegram or the mic. A parked question belongs to the reach that was + // asked; before this, a clarify nobody answered on the web ate the next + // utterance spoken at the mic (Vikunja #466). This server has no + // per-browser session, so every browser tab is the same conversation — + // which is right for a single-owner box. + reply, err := core.Chat(r.Context(), "web", text) if err != nil { log.Printf("chat api: %v", err) http.Redirect(w, r, "/chat", http.StatusSeeOther) diff --git a/cmd/mavweb/notifications.html b/cmd/mavweb/notifications.html index 8d81cd9..4ebf044 100644 --- a/cmd/mavweb/notifications.html +++ b/cmd/mavweb/notifications.html @@ -14,5 +14,27 @@
no notifications yet
check back later or ask maven a question
{{end}} +

Delivery outbox

+

+ every send is recorded before it leaves, so a failure is visible rather than silent. + all · + dropped · + failed · + pending · + unknown +

+{{if .Attempts}}
+ +{{range .Attempts}} + + + + + + +{{end}}
startedkindrulechannelstatusfinished
{{.Created}}{{.Kind}}{{.Target}}{{.Channel}}{{.Status}}{{.Completed}}
+{{else}}
+
no delivery attempts{{if .Status}} with status {{.Status}}{{end}}
+
{{end}} {{template "shellBottom"}} diff --git a/docs/design.md b/docs/design.md index f29a2ad..f5d84ed 100644 --- a/docs/design.md +++ b/docs/design.md @@ -223,6 +223,30 @@ Not alternatives — layers: Router contract: `[{"intent":, key?, value?, text?, verb?}, ...]` over 7 intents (`fact, reminder, note, query, act, chat, system`). +#### A restart expires a parked question + +Decided 2026-08-04 (Vikunja #385). The follow-up dialogue session survives a +restart; the clarify question parked behind it does not, and neither do the +three yes/no confirms in `voice.go`. `ClarifyStore` stays in memory. + +Three reasons, in the order they settle it: + +- The clock stops meaning anything. A parked question carries a 90s TTL and an + attempt count. A restart is a gap of unknown length, so a restored question is + either already dead or pretending to be young. +- Restoring the question restores the request behind it. He asked for something, + she asked back, and then the daemon went away. Acting on that minutes later, + against words he has probably given up on, is the misroute the stage 3 gate + exists to avoid. +- She does not announce it either. The expiry notice needs to know a question + was parked, and knowing that across a restart means storing it. One sentence, + in the rare window where he speaks within 90s of a restart, does not pay for a + marker that outlives the thing it describes. His next words route fresh, which + is the correct answer with or without the notice. + +So the notice stays what it is: the in-process TTL case, where she really did +wait and really did let go. + ### save-where — the two-memory routing axis One discriminator: **does the loop evaluate a predicate against it?** diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index 8de46ac..f84b564 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -311,7 +311,7 @@ func TestGate_IpcServer_CheckWiredThroughSocket(t *testing.T) { if fake.writes != 0 { t.Errorf("auth refused but CoreAPI was called %d time(s); refused calls must not reach CoreAPI", fake.writes) } - _, err = cli.Chat(context.Background(), "привет") + _, err = cli.Chat(context.Background(), "web", "привет") if !errors.Is(err, ipc.ErrForbidden) { t.Errorf("wire: chat from unenrolled uid = %v; want ipc.ErrForbidden", err) } @@ -344,7 +344,7 @@ func TestGate_IpcServer_ChatAllowedForEnrolledCaller(t *testing.T) { t.Fatalf("dial: %v", err) } t.Cleanup(func() { _ = cli.Close() }) - reply, err := cli.Chat(context.Background(), "привет") + reply, err := cli.Chat(context.Background(), "web", "привет") if err != nil { t.Fatalf("Chat: %v", err) } @@ -373,7 +373,7 @@ func (r *recordingAPI) WriteFact(_ context.Context, _ ipc.WriteFactReq) (int64, return int64(r.writes), nil } -func (r *recordingAPI) Chat(_ context.Context, text string) (string, error) { +func (r *recordingAPI) Chat(_ context.Context, _, text string) (string, error) { r.chats++ return "echo: " + text, nil } diff --git a/internal/config/config.go b/internal/config/config.go index ff72b31..990ae1e 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -601,6 +601,9 @@ type MorningRoutineItemConfig struct { Key string `json:"key"` FactKey string `json:"fact_key"` Label string `json:"label"` + // Optional — this one being skipped does not earn a nudge. Default false, + // so a routine written before 04-08-2026 keeps behaving as it did. + Optional bool `json:"optional,omitempty"` } // QuietHoursConfig — a recurring daily quiet-window. Times are local to the @@ -1734,7 +1737,7 @@ func morningRoutinesFromConfig(mc []MorningRoutineConfig) []morning.Routine { for i, r := range mc { items := make([]morning.Item, len(r.Items)) for j, it := range r.Items { - items[j] = morning.Item{Key: it.Key, FactKey: it.FactKey, Label: it.Label} + items[j] = morning.Item{Key: it.Key, FactKey: it.FactKey, Label: it.Label, Optional: it.Optional} } weekdays := make([]time.Weekday, len(r.Weekdays)) for j, w := range r.Weekdays { diff --git a/internal/dialogue/clarify.go b/internal/dialogue/clarify.go index e3a49c3..98850c2 100644 --- a/internal/dialogue/clarify.go +++ b/internal/dialogue/clarify.go @@ -57,6 +57,14 @@ func (q *PendingQuestion) CanAsk() bool { // ClarifyStore holds the parked questions. Same shape and locking as // SessionStore: keyed by dialogue id, expired entries dropped on read. +// +// Memory only, deliberately, unlike SessionStore — a restart expires every +// parked question and she does not announce that it happened (Vikunja #385, +// written down in docs/design.md). The 90s TTL and the attempt count measure a +// pause in one conversation, and a restart is a gap of unknown length, so a +// restored question would either be dead already or lying about its age. His +// next words route fresh, which is the right answer with or without a notice. +// Do not give this store a persister without re-arguing that. type ClarifyStore struct { mu sync.RWMutex questions map[string]*PendingQuestion diff --git a/internal/ipc/api.go b/internal/ipc/api.go index 1584055..1eb8178 100644 --- a/internal/ipc/api.go +++ b/internal/ipc/api.go @@ -60,6 +60,19 @@ type Nudge struct { OutcomeTs *int64 `json:"outcome_ts,omitempty"` } +// DeliveryAttempt — one row of the delivery outbox. Times are formatted by the +// reader; Completed is nil while the attempt is still pending. +type DeliveryAttempt struct { + ID int64 `json:"id"` + Kind string `json:"kind"` + Rule string `json:"rule,omitempty"` + ReminderID int64 `json:"reminder_id,omitempty"` + Channel string `json:"channel"` + Status string `json:"status"` + Created time.Time `json:"created"` + Completed *time.Time `json:"completed,omitempty"` +} + // Note — a recall/preference item; ranked by embedding cosine on query. // Score is set by QueryNotes (0 on the write path). type Note struct { @@ -521,6 +534,12 @@ type outcomesReq struct { type nReq struct { N int `json:"n"` } + +// deliveryAttemptsReq — the outbox read. Status is empty for every status. +type deliveryAttemptsReq struct { + Status string `json:"status,omitempty"` + N int `json:"n"` +} type kindNReq struct { Kind string `json:"kind"` N int `json:"n"` @@ -593,8 +612,14 @@ type MCPServerStatus struct { } // chatReq / chatResp — text chat round-trip for the IPC Chat method. +// +// Conversation names the thread this utterance belongs to: a mavweb session, a +// telegram chat. It is opaque to the daemon and only has to be stable for one +// conversation and distinct across them. Empty is allowed and means "the +// unattributed text tap", which is what an old client sends. type chatReq struct { - Text string `json:"text"` + Text string `json:"text"` + Conversation string `json:"conversation,omitempty"` } type chatResp struct { Reply string `json:"reply"` @@ -679,6 +704,9 @@ type CoreAPI interface { RecentActiveFactsByKind(ctx context.Context, kind string, n int) ([]Fact, error) CalendarEvents(ctx context.Context, from, to time.Time) ([]Fact, error) RecentNudges(ctx context.Context, n int) ([]Nudge, error) + // DeliveryAttempts reads the outbox, newest first. An empty status means + // every status (Vikunja #390). + DeliveryAttempts(ctx context.Context, status string, n int) ([]DeliveryAttempt, error) // RecentEcosystemTraces reads the ecosystem call log, which lives in its // own table so machine-rate traces never crowd out human-rate facts. @@ -759,7 +787,12 @@ type CoreAPI interface { // Chat routes a text utterance through the reactive handler's core path // (router → dialogue → action → replier) and returns the reply text. // No audio or stt/tts — for text channels (mavweb, telegram). - Chat(ctx context.Context, text string) (string, error) + // + // conversation names the thread. A parked clarifying question is held per + // conversation, so an unanswered question on one reach cannot eat the next + // utterance from another (Vikunja #466). Empty means the unattributed text + // tap and is still one conversation of its own, separate from the mic. + Chat(ctx context.Context, conversation, text string) (string, error) // RecentEvents returns the daemon's unified intake journal, newest first // (Vikunja #283) — one envelope per thing that arrived, whatever direction diff --git a/internal/ipc/client.go b/internal/ipc/client.go index 5d3bd38..4a36322 100644 --- a/internal/ipc/client.go +++ b/internal/ipc/client.go @@ -68,6 +68,7 @@ var readOnlyMethods = map[Method]bool{ MethodRecentActiveFacts: true, MethodCalendarEvents: true, MethodRecentNudges: true, + MethodDeliveryAttempts: true, MethodRecentEcoTraces: true, MethodQueryNotes: true, MethodRecentNotes: true, @@ -373,6 +374,14 @@ func (c *Client) RecentEcosystemTraces(ctx context.Context, n int) ([]EcosystemT return out, nil } +func (c *Client) DeliveryAttempts(ctx context.Context, status string, n int) ([]DeliveryAttempt, error) { + var out []DeliveryAttempt + if err := c.call(ctx, MethodDeliveryAttempts, deliveryAttemptsReq{Status: status, N: n}, &out); err != nil { + return nil, err + } + return out, nil +} + func (c *Client) RecentNudges(ctx context.Context, n int) ([]Nudge, error) { var out []Nudge if err := c.call(ctx, MethodRecentNudges, nReq{N: n}, &out); err != nil { @@ -623,9 +632,9 @@ func (c *Client) AcceptProposedRoutine(ctx context.Context, id int64) error { return c.call(ctx, MethodAcceptProposedRoutine, acceptProposedRoutineReq{ID: id}, nil) } -func (c *Client) Chat(ctx context.Context, text string) (string, error) { +func (c *Client) Chat(ctx context.Context, conversation, text string) (string, error) { var r chatResp - if err := c.call(ctx, MethodChat, chatReq{Text: text}, &r); err != nil { + if err := c.call(ctx, MethodChat, chatReq{Text: text, Conversation: conversation}, &r); err != nil { return "", err } return r.Reply, nil diff --git a/internal/ipc/ipc_test.go b/internal/ipc/ipc_test.go index 5ba6654..5b8ca2b 100644 --- a/internal/ipc/ipc_test.go +++ b/internal/ipc/ipc_test.go @@ -401,7 +401,7 @@ func TestChatViaClient(t *testing.T) { } t.Cleanup(func() { _ = cli.Close() }) - reply, err := cli.Chat(context.Background(), "привет") + reply, err := cli.Chat(context.Background(), "web", "привет") if err != nil { t.Fatalf("Chat: %v", err) } @@ -417,7 +417,7 @@ type chatTestAPI struct { UnimplementedCoreAPI } -func (a *chatTestAPI) Chat(ctx context.Context, text string) (string, error) { +func (a *chatTestAPI) Chat(ctx context.Context, _, text string) (string, error) { if text == "привет" { return "и тебе привет!", nil } diff --git a/internal/ipc/server.go b/internal/ipc/server.go index 8e991e4..ae596a5 100644 --- a/internal/ipc/server.go +++ b/internal/ipc/server.go @@ -173,6 +173,25 @@ func (a *storeAPI) RecentNudges(ctx context.Context, n int) ([]Nudge, error) { return out, nil } +func (a *storeAPI) DeliveryAttempts(ctx context.Context, status string, n int) ([]DeliveryAttempt, error) { + as, err := a.s.ListDeliveryAttempts(ctx, status, n) + if err != nil { + return nil, mapErr(err) + } + out := make([]DeliveryAttempt, len(as)) + for i, at := range as { + out[i] = DeliveryAttempt{ + ID: at.ID, Kind: at.Kind, Rule: at.Rule, ReminderID: at.ReminderID, + Channel: at.Channel, Status: at.Status, Created: at.Created, + } + if at.HasComplete { + t := at.Completed + out[i].Completed = &t + } + } + return out, nil +} + func (a *storeAPI) WriteNote(ctx context.Context, ts time.Time, text string, embedding []float32, source string) (int64, error) { id, err := a.s.WriteNote(ctx, ts, text, embedding, source) return id, mapErr(err) @@ -240,7 +259,7 @@ func (a *storeAPI) RevertFact(ctx context.Context, key string) (int64, error) { return newID, mapErr(err) } -func (a *storeAPI) Chat(ctx context.Context, text string) (string, error) { +func (a *storeAPI) Chat(ctx context.Context, conversation, text string) (string, error) { return "", errors.New("store: chat not available via direct store API") } @@ -863,6 +882,16 @@ var methodTable = map[Method]handlerFunc{ } return out, nil }), + MethodDeliveryAttempts: withParams(func(ctx context.Context, api CoreAPI, p deliveryAttemptsReq) ([]DeliveryAttempt, error) { + out, err := api.DeliveryAttempts(ctx, p.Status, p.N) + if err != nil { + return nil, err + } + if out == nil { + out = []DeliveryAttempt{} + } + return out, nil + }), MethodRecentNudges: withParams(func(ctx context.Context, api CoreAPI, p nReq) ([]Nudge, error) { out, err := api.RecentNudges(ctx, p.N) if err != nil { @@ -974,7 +1003,7 @@ var methodTable = map[Method]handlerFunc{ return map[string]int64{"new_id": newID}, nil }), MethodChat: withParams(func(ctx context.Context, api CoreAPI, p chatReq) (chatResp, error) { - reply, err := api.Chat(ctx, p.Text) + reply, err := api.Chat(ctx, p.Conversation, p.Text) return chatResp{Reply: reply}, err }), MethodTickTrace: withoutParams(func(ctx context.Context, api CoreAPI) (TickTrace, error) { diff --git a/internal/ipc/unimplemented.go b/internal/ipc/unimplemented.go index 4aea172..4279c67 100644 --- a/internal/ipc/unimplemented.go +++ b/internal/ipc/unimplemented.go @@ -68,6 +68,9 @@ func (UnimplementedCoreAPI) RecentActiveFactsByKind(ctx context.Context, kind st func (UnimplementedCoreAPI) CalendarEvents(ctx context.Context, from, to time.Time) ([]Fact, error) { return nil, ErrNotImplemented } +func (UnimplementedCoreAPI) DeliveryAttempts(ctx context.Context, status string, n int) ([]DeliveryAttempt, error) { + return nil, ErrNotImplemented +} func (UnimplementedCoreAPI) RecentNudges(ctx context.Context, n int) ([]Nudge, error) { return nil, ErrNotImplemented } @@ -141,6 +144,6 @@ func (UnimplementedCoreAPI) MCPServers(ctx context.Context) ([]MCPServerStatus, func (UnimplementedCoreAPI) DayPlan(ctx context.Context) (DayPlan, error) { return DayPlan{}, ErrNotImplemented } -func (UnimplementedCoreAPI) Chat(ctx context.Context, text string) (string, error) { +func (UnimplementedCoreAPI) Chat(ctx context.Context, conversation, text string) (string, error) { return "", ErrNotImplemented } diff --git a/internal/ipc/wire.go b/internal/ipc/wire.go index c55a7ec..50e2f15 100644 --- a/internal/ipc/wire.go +++ b/internal/ipc/wire.go @@ -28,6 +28,7 @@ const ( MethodRecentActiveFacts Method = "recent_active_facts_by_kind" MethodCalendarEvents Method = "calendar_events" MethodRecentNudges Method = "recent_nudges" + MethodDeliveryAttempts Method = "delivery_attempts" MethodRecentEcoTraces Method = "recent_ecosystem_traces" MethodWriteNote Method = "write_note" MethodQueryNotes Method = "query_notes" diff --git a/internal/memory/recalleval/recalleval.go b/internal/memory/recalleval/recalleval.go index 1691b49..e1be017 100644 --- a/internal/memory/recalleval/recalleval.go +++ b/internal/memory/recalleval/recalleval.go @@ -90,9 +90,44 @@ func Load() (Fixture, error) { if len(f.Cases) == 0 { return Fixture{}, fmt.Errorf("fixture has no cases") } + if err := checkIDs(f); err != nil { + return Fixture{}, err + } return f, nil } +// checkIDs refuses a fixture where a case note and a filler note share an id. +// +// Every case is scored over its own notes plus the whole filler set, and the +// two stores disagree about what a repeated id means: the sqlite store upserts +// on it, the in-memory store appends. So one collision makes a case score +// differently on the two backends, and it reads as an embedder or gate +// difference, which is the one thing this harness exists to measure (Vikunja +// #386). It was dodged once by hand during #373 by renaming two ids. +// +// Checked in Load rather than in the test, so every caller of the fixture is +// covered and not only the one that remembers to look. +func checkIDs(f Fixture) error { + filler := make(map[string]bool, len(f.Filler)) + for _, n := range f.Filler { + if n.ID == "" { + return fmt.Errorf("filler note with an empty id") + } + if filler[n.ID] { + return fmt.Errorf("duplicate filler note id %q", n.ID) + } + filler[n.ID] = true + } + for _, c := range f.Cases { + for _, n := range c.Notes { + if filler[n.ID] { + return fmt.Errorf("case %s: note id %q collides with a filler note", c.ID, n.ID) + } + } + } + return nil +} + // NewStore builds an empty store for one case, plus a function to release it. // A factory rather than a store because every case needs a clean index — notes // from case A must not be visible to case B's query. diff --git a/internal/memory/recalleval/recalleval_test.go b/internal/memory/recalleval/recalleval_test.go index e9cea1c..f8b324e 100644 --- a/internal/memory/recalleval/recalleval_test.go +++ b/internal/memory/recalleval/recalleval_test.go @@ -337,3 +337,28 @@ func marginSweep(t *testing.T, emb router.Embedder, f Fixture) string { } return b.String() } + +// TestFillerIDCollisionIsRefused — the guard that keeps a fixture edit from +// looking like a backend difference (Vikunja #386). +func TestFillerIDCollisionIsRefused(t *testing.T) { + f := Fixture{ + SchemaVersion: SchemaVersion, + Cases: []Case{{ID: "ru-001", Notes: []StoredNote{{ID: "f1", Text: "..."}}}}, + Filler: []StoredNote{{ID: "f1", Text: "..."}}, + } + if err := checkIDs(f); err == nil { + t.Fatal("a case note reusing a filler id must be refused") + } + f.Filler = append(f.Filler, StoredNote{ID: "f1", Text: "..."}) + if err := checkIDs(Fixture{SchemaVersion: SchemaVersion, Filler: f.Filler}); err == nil { + t.Fatal("a duplicate filler id must be refused") + } + ok := Fixture{ + SchemaVersion: SchemaVersion, + Cases: []Case{{ID: "ru-001", Notes: []StoredNote{{ID: "n1", Text: "..."}}}}, + Filler: []StoredNote{{ID: "f1", Text: "..."}}, + } + if err := checkIDs(ok); err != nil { + t.Fatalf("a clean fixture must pass: %v", err) + } +} diff --git a/internal/morning/morning.go b/internal/morning/morning.go index 4034bef..418ee44 100644 --- a/internal/morning/morning.go +++ b/internal/morning/morning.go @@ -30,6 +30,18 @@ type Item struct { Key string FactKey string Label string // RU text surfaced when this item is still missing. + // Optional — a missing one is not worth a nudge on its own. + // + // Every item was implicitly required until 04-08-2026, because there was + // no field, so a skipped stretch read exactly like skipped medication and + // #280's first behaviour could not hold (Vikunja #473). A checklist where + // everything is mandatory is a checklist he learns to ignore. + // + // It changes two things and nothing else: an all-optional routine never + // nudges, and a nudge that does fire names the optional stragglers after + // the required ones, in softer words. Evidence, the window and the day + // plan treat both kinds alike — a missing optional item is still missing. + Optional bool } // Routine — one daily checklist. WindowStart/WindowEnd are "HH:MM" local @@ -60,12 +72,37 @@ type Status struct { } // Candidate — a routine that's due for its one-per-day nag: the window has -// reached NudgeAt and at least one item is still unevidenced. +// reached NudgeAt and at least one REQUIRED item is still unevidenced. Missing +// carries the optional stragglers too, so the one message she is allowed per +// day per routine can mention them; they never cause it. type Candidate struct { Routine Routine Missing []Item } +// Required reports the missing items that are not optional. The nudge fires on +// these; the rest ride along. +func Required(missing []Item) []Item { + var out []Item + for _, it := range missing { + if !it.Optional { + out = append(out, it) + } + } + return out +} + +// OptionalOnly is the other half of Required. +func OptionalOnly(missing []Item) []Item { + var out []Item + for _, it := range missing { + if it.Optional { + out = append(out, it) + } + } + return out +} + // Validate reports the first structural problem with a routine set: missing // name/items, an unparseable HH:MM, an inverted window, a duplicate item key // within a routine, or an out-of-range weekday. Called at config load so a @@ -191,7 +228,11 @@ func Due(routines []Routine, facts map[string]store.Fact, last map[string]time.T missing = append(missing, it) } } - if len(missing) == 0 { + // A day where only the optional items were skipped is a fine day, and + // nagging about it is what teaches him to stop listening (Vikunja + // #473). The optional ones still travel in Missing so the message can + // mention them when it is being sent anyway. + if len(Required(missing)) == 0 { continue } if prev, seen := last[r.Name]; seen && sameDay(prev, now) { diff --git a/internal/morning/morning_test.go b/internal/morning/morning_test.go index 5609bff..1a9385e 100644 --- a/internal/morning/morning_test.go +++ b/internal/morning/morning_test.go @@ -182,3 +182,40 @@ func TestDueRespectsExplicitNudgeAt(t *testing.T) { t.Fatalf("expected candidate at explicit nudge_at, got %d", len(out)) } } + +// TestOptionalItemsDoNotEarnANudge — behaviour 1 of #280, which could not hold +// while every item was implicitly required (Vikunja #473). +func TestOptionalItemsDoNotEarnANudge(t *testing.T) { + r := Routine{ + Name: "утро", + WindowStart: "07:00", + WindowEnd: "10:00", + Items: []Item{ + {Key: "meds", FactKey: "meds", Label: "таблетки"}, + {Key: "stretch", FactKey: "stretch", Label: "растяжка", Optional: true}, + }, + } + now := time.Date(2026, 8, 4, 10, 0, 0, 0, time.UTC) + took := map[string]store.Fact{"meds": {Key: "meds", Ts: now.Add(-2 * time.Hour)}} + + // Only the stretch was skipped: nothing to say. + if due := Due([]Routine{r}, took, map[string]time.Time{}, now); len(due) != 0 { + t.Fatalf("an optional item alone must not nudge, got %+v", due) + } + // The medication was skipped: she says so, and mentions the stretch too. + due := Due([]Routine{r}, map[string]store.Fact{}, map[string]time.Time{}, now) + if len(due) != 1 { + t.Fatalf("a missing required item must nudge, got %+v", due) + } + if got := Required(due[0].Missing); len(got) != 1 || got[0].Key != "meds" { + t.Fatalf("Required = %+v, want the meds item alone", got) + } + if got := OptionalOnly(due[0].Missing); len(got) != 1 || got[0].Key != "stretch" { + t.Fatalf("OptionalOnly = %+v, want the stretch item alone", got) + } + // The window still reports it as missing — optional is not invisible. + st := Evaluate(r, map[string]store.Fact{}, now.Add(-time.Hour)) + if len(st.Missing) != 2 { + t.Fatalf("Evaluate must still list both, got %+v", st.Missing) + } +} diff --git a/internal/pattern/detector.go b/internal/pattern/detector.go index 741d81f..cdaaf8d 100644 --- a/internal/pattern/detector.go +++ b/internal/pattern/detector.go @@ -53,9 +53,31 @@ const MinOnPatternFraction = 0.7 // a repeat. False negatives cost one more observation and nothing else. const MinEvents = 4 +// MinIntervalDays — the fastest rhythm that may be called a routine. Two +// hours. +// +// Without a floor, four taps of the same key minutes apart give intervals near +// 0.002 days. They all sit inside the ±50% band by construction, so the +// detector proposed a routine and PhraseRoutine worded it as "каждый день" +// (Vikunja #468). The damage outlives the mistake: UNIQUE(action, object) +// means dismissing the bogus proposal burns that pair permanently, so the real +// routine behind it can never be proposed again. +// +// Two hours rather than a day, because a genuine habit can run several times a +// day — meals, water, a break. Anything faster than that is not a habit she +// should be proposing to remind him about; the loop rules already cover that +// range, and they are rules, not guesses. It is checked against the median, so +// one quick repeat inside a real rhythm still counts. +// +// The other half of this is that hand-QA of the detector was unsafe: seeding a +// pattern the obvious way, four chat turns in a row, poisoned the very pair +// being tested. +const MinIntervalDays = 2.0 / 24.0 + // Detect checks whether a sequence of events for the same action+object // forms a stable recurring pattern. Returns a ProposedRoutine when: // - At least MinEvents events exist (≥3 intervals) +// - The median interval is at least MinIntervalDays // - At least MinOnPatternFraction of the intervals sit within // MaxIntervalRatio of the median interval // @@ -88,8 +110,8 @@ func Detect(events []Event) (*ProposedRoutine, error) { } center := medianFloat(intervals) - if center <= 0 { - return nil, nil + if center <= 0 || center < MinIntervalDays { + return nil, nil // a burst, not a rhythm — see MinIntervalDays } // Keep the intervals that sit inside the band around the median. The diff --git a/internal/pattern/detector_test.go b/internal/pattern/detector_test.go index 056a3f6..88fb4de 100644 --- a/internal/pattern/detector_test.go +++ b/internal/pattern/detector_test.go @@ -216,3 +216,46 @@ func TestDetectMedianBandNotExtremes(t *testing.T) { }) } } + +// A burst is not a habit. Four taps of the same key minutes apart give +// intervals near 0.002 days, all inside the ±50% band by construction, so the +// detector called it a daily routine (Vikunja #468). Dismissing that proposal +// burns the action+object pair permanently, which also made hand-QA of the +// detector unsafe. +func TestDetectRejectsABurst(t *testing.T) { + base := time.Date(2026, 8, 4, 9, 0, 0, 0, time.UTC) + var events []Event + for i := 0; i < 4; i++ { + events = append(events, Event{ + Action: "refill", Object: "cat_water", + Ts: base.Add(time.Duration(i) * 7 * time.Minute), + }) + } + r, err := Detect(events) + if err != nil { + t.Fatalf("Detect: %v", err) + } + if r != nil { + t.Fatalf("four taps minutes apart proposed a routine every %.3f days", r.IntervalDays) + } +} + +// The floor is two hours, not a day: a habit that runs several times a day is +// still a habit. +func TestDetectKeepsASeveralTimesADayHabit(t *testing.T) { + base := time.Date(2026, 8, 4, 8, 0, 0, 0, time.UTC) + var events []Event + for i := 0; i < 5; i++ { + events = append(events, Event{ + Action: "drink", Object: "water", + Ts: base.Add(time.Duration(i) * 4 * time.Hour), + }) + } + r, err := Detect(events) + if err != nil { + t.Fatalf("Detect: %v", err) + } + if r == nil { + t.Fatal("a four-hour rhythm over five events is a habit, got nil") + } +} diff --git a/internal/phraser/eval/checks.go b/internal/phraser/eval/checks.go index fdbbe48..586928a 100644 --- a/internal/phraser/eval/checks.go +++ b/internal/phraser/eval/checks.go @@ -186,12 +186,23 @@ func checkFeminine(body string) Result { // Second pass: self-reference with the pronoun dropped — "напомнил тебе", // "проверил за тебя". A masculine past-tense verb whose object is HIM can // only be her speaking about herself. + // + // Two guards, both from a false positive on the talk fixture: "ты заплатил + // за домен до марта" scored as her drift and cost the run a point it had + // earned (Vikunja #462). He is male, so a past-tense verb governed by "ты" + // must be masculine. And a bare "за" is not evidence of anything — "за + // домен" is a price, "за тебя" is her doing something on his behalf — so it + // only counts when he is the one it points at. for i, w := range words { - if !masculinePast(w) || i+1 >= len(words) { + if !masculinePast(w) || i+1 >= len(words) || governedByYou(words, i) { continue } next := words[i+1] - if next == "тебе" || next == "тебя" || next == "за" { + aboutHim := next == "тебе" || next == "тебя" + if next == "за" && i+2 < len(words) && (words[i+2] == "тебя" || words[i+2] == "тебе") { + aboutHim = true + } + if aboutHim { return Result{CheckFeminine, false, fmt.Sprintf("masculine self-reference %q before %q", w, next)} } @@ -665,3 +676,19 @@ func checkEllipsis(body string) Result { } return Result{CheckEllipsis, true, ""} } + +// governedByYou reports whether "ты" stands close enough in front of the verb +// at index i to be its subject. Three words, the same window checkFeminine's +// first pass uses after "я", and it stops at a first-person pronoun so "ты +// просил, я напомнил" still trips. +func governedByYou(words []string, i int) bool { + for j := i - 1; j >= 0 && j >= i-3; j-- { + switch words[j] { + case "ты": + return true + case "я": + return false + } + } + return false +} diff --git a/internal/phraser/eval/eval_test.go b/internal/phraser/eval/eval_test.go index 3342793..150f12b 100644 --- a/internal/phraser/eval/eval_test.go +++ b/internal/phraser/eval/eval_test.go @@ -106,6 +106,12 @@ func TestChecksCatchWhatTheyClaim(t *testing.T) { {"masculine predicative", "я должен сказать: попей воды.", CheckFeminine}, // The other direction: HE is male, so second-person masculine is right. {"second person masculine ok", "ты не пил воду четыре часа.", ""}, + // The recorded false positive: "заплатил" sits before "за", and the + // second pass read that as her dropping the pronoun. The subject is + // "ты" and he is male, so the reply is right (Vikunja #462). + {"second person masculine before за", "ты заплатил за домен до марта, а воду пить всё равно надо.", ""}, + // The same shape she really does get wrong still trips. + {"masculine on his behalf", "проверил за тебя — воды не было четыре часа.", CheckFeminine}, // The real observed failure: she addressed him as a woman. {"feminine second person", "ты давно не отдыхала — попей воды.", CheckHisGender}, {"feminine second person no dash", "ты пила воду четыре часа назад.", CheckHisGender}, diff --git a/internal/router/eval/ru_routing_v1.json b/internal/router/eval/ru_routing_v1.json index 68d5b1d..823fe37 100644 --- a/internal/router/eval/ru_routing_v1.json +++ b/internal/router/eval/ru_routing_v1.json @@ -73,6 +73,7 @@ { "id": "ru-note-003", "utterance": "заметка про настройку vlan на свитче", "lang": "ru", "intent": "note", "tags": ["homelab"] }, { "id": "ru-note-004", "utterance": "запиши идею: гидропоника на балконе", "lang": "ru", "intent": "note" }, { "id": "ru-note-005", "utterance": "запиши что сосед просил номер электрика", "lang": "ru", "intent": "note" }, + { "id": "ru-note-006", "utterance": "добавь в задачи купить молоко", "lang": "ru", "intent": "note", "tags": ["capture"], "note": "an explicit capture marker — the model called it an act and rewrote the payload (Vikunja #467), stage 0 claims it" }, { "id": "en-note-001", "utterance": "note: rotate the kuma api key", "lang": "en", "intent": "note", "tags": ["homelab"] }, { "id": "ru-sys-001", "utterance": "сколько сейчас времени в киеве", "lang": "ru", "intent": "system", "tags": ["time"] }, diff --git a/internal/router/task.go b/internal/router/task.go index c85059b..96b0e2e 100644 --- a/internal/router/task.go +++ b/internal/router/task.go @@ -1,6 +1,9 @@ package router -import "strings" +import ( + "regexp" + "strings" +) // Task capture and task listing, matched deterministically (Vikunja #130). // @@ -201,3 +204,47 @@ func IsTaskListQuery(text string) bool { } return false } + +// TaskCaptureGrammar — stage 0 for an explicit capture marker, so the resident +// model never sees it (Vikunja #467). +// +// Capture was built to ride the note intent, deliberately: #130 said no eighth +// intent, and while the classifier was routing, a note-shaped utterance with a +// marker in it reached actionNote and captureTaskFromNote claimed it there. The +// router pre-empted that. Measured 2026-08-02: "добавь в задачи купить молоко" +// routed act, so captureTaskFromNote was never consulted, the act arm found no +// allowlisted fn, and the gate asked "Что сделать?". Every capture utterance +// tried filed nothing. +// +// The model also rewrote the payload on the way — "купить молоко" came back as +// "сделать покупку молока". A task must read as the words he said, which is a +// second reason to answer this before the model rather than to prompt around +// it. +// +// The marker list is data (task_phrases.json) and the parse strips urgency, so +// the pattern here matches any utterance and the decision is ParseTaskCapture's +// to make — same shape as the wake-word act grammar, which also matches broadly +// and refuses in Build. Intent stays note: the daemon's note path is where +// capture lives, and nothing about the contract with the model changes. +func TaskCaptureGrammar() Grammar { + return Grammar{ + Name: "task-capture", + Pattern: regexp.MustCompile(`(?s)^\s*(.+)$`), + Build: func(m []string) (Decision, bool) { + c, ok := ParseTaskCapture(m[1]) + if !ok { + return Decision{}, false // not a capture — fall through + } + return Decision{ + Stage: 0, + Intent: IntentNote, + Confidence: 1.0, + // The capture text, not the raw utterance: it is what the + // clarify gate reads as the payload. captureTaskFromNote + // re-parses the utterance itself, so the task text comes from + // the same place either way. + Slots: Slots{Text: c.Text}, + }, true + }, + } +} diff --git a/internal/router/task_phrases.json b/internal/router/task_phrases.json index f70eff3..6ed9250 100644 --- a/internal/router/task_phrases.json +++ b/internal/router/task_phrases.json @@ -27,6 +27,9 @@ "добавь в список", "добавь задачу", "запиши в задачи", + "запиши в список дел", + "запиши в список задач", + "запиши в список", "запиши задачу", "новая задача", "поставь задачу", diff --git a/internal/router/task_test.go b/internal/router/task_test.go index 75503f5..83e5ffd 100644 --- a/internal/router/task_test.go +++ b/internal/router/task_test.go @@ -85,3 +85,37 @@ func TestIsTaskListQuery(t *testing.T) { } } } + +// TestTaskCaptureGrammarClaimsTheMarker — the capture marker is answered at +// stage 0, so the model never gets to call it an act (Vikunja #467). +func TestTaskCaptureGrammarClaimsTheMarker(t *testing.T) { + g := TaskCaptureGrammar() + captures := map[string]string{ + "добавь в задачи купить молоко": "купить молоко", + "запиши в список дел купить хлеб": "купить хлеб", + "поставь задачу вынести мусор": "вынести мусор", + "добавь в задачи срочно оплатить дом": "оплатить дом", + } + for in, want := range captures { + m := g.Pattern.FindStringSubmatch(in) + if m == nil { + t.Fatalf("%q did not match the grammar pattern", in) + } + d, ok := g.Build(m) + if !ok { + t.Fatalf("%q must be claimed as a capture", in) + } + if d.Intent != IntentNote || d.Slots.Text != want { + t.Errorf("%q → intent=%s text=%q, want note/%q", in, d.Intent, d.Slots.Text, want) + } + } + // Everything without a marker falls through, including a marker with no + // task after it and a question about the list. + for _, in := range []string{"надо бы поспать", "добавь в задачи", "какие у меня задачи?", "перезапусти nginx"} { + if m := g.Pattern.FindStringSubmatch(in); m != nil { + if _, ok := g.Build(m); ok { + t.Errorf("%q must fall through to the cascade", in) + } + } + } +} diff --git a/internal/store/delivery.go b/internal/store/delivery.go index b0b46df..4c016c7 100644 --- a/internal/store/delivery.go +++ b/internal/store/delivery.go @@ -87,3 +87,68 @@ func (s *Store) ReconcileStaleDeliveryAttempts(ctx context.Context, now time.Tim } return int(n), nil } + +// DeliveryAttempt — one row of the outbox, as a reader sees it. +type DeliveryAttempt struct { + ID int64 + Kind string // nudge|reminder + Rule string // set for nudges + ReminderID int64 // set for reminders + Channel string + Status string // one of the Delivery* constants + Created time.Time + Completed time.Time // zero while pending + HasComplete bool +} + +// ListDeliveryAttempts returns recent attempts, newest first. An empty status +// means every status; anything else filters on it. +// +// The table was write-only until 04-08-2026: rows were recorded and nothing +// could read them, so the tests for #368 and #370 had to reach past the store +// into store.DB, which is the tell (Vikunja #390). A durable record nobody can +// read answers no question, and "why did Maven go quiet" is supposed to be a +// query rather than a mystery. +// +// Status is the filter that earns its place, because the two questions actually +// asked are "what got dropped" and "what is still pending". Neither is +// answerable by reading the whole list on a busy day. +func (s *Store) ListDeliveryAttempts(ctx context.Context, status string, limit int) ([]DeliveryAttempt, error) { + if limit <= 0 { + limit = 50 + } + q := `SELECT id, kind, rule, reminder_id, channel, status, created_ts, completed_ts + FROM delivery_attempts` + args := []any{} + if status != "" { + q += ` WHERE status = ?` + args = append(args, status) + } + q += ` ORDER BY created_ts DESC, id DESC LIMIT ?` + args = append(args, limit) + + rows, err := s.db.QueryContext(ctx, q, args...) + if err != nil { + return nil, fmt.Errorf("list delivery attempts: %w", err) + } + defer rows.Close() + + var out []DeliveryAttempt + for rows.Next() { + var a DeliveryAttempt + var created int64 + var completed *int64 + if err := rows.Scan(&a.ID, &a.Kind, &a.Rule, &a.ReminderID, &a.Channel, &a.Status, &created, &completed); err != nil { + return nil, fmt.Errorf("list delivery attempts: scan: %w", err) + } + a.Created = time.UnixMilli(created) + if completed != nil { + a.Completed, a.HasComplete = time.UnixMilli(*completed), true + } + out = append(out, a) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("list delivery attempts: %w", err) + } + return out, nil +} diff --git a/internal/store/delivery_test.go b/internal/store/delivery_test.go index 4a7e23d..d261448 100644 --- a/internal/store/delivery_test.go +++ b/internal/store/delivery_test.go @@ -32,3 +32,53 @@ func TestDroppedDeliveryAttemptRoundTrips(t *testing.T) { t.Fatalf("status: want %q, got %q", DeliveryDropped, status) } } + +// TestListDeliveryAttempts — the read path the outbox lacked until #390. The +// two questions it must answer are "what was dropped" and "what is pending". +func TestListDeliveryAttempts(t *testing.T) { + ctx := context.Background() + s := newTestStore(t) + base := time.Date(2026, 8, 4, 9, 0, 0, 0, time.UTC) + + sent, err := s.BeginDeliveryAttempt(ctx, "nudge", "water", 0, "telegram", "h1", base) + if err != nil { + t.Fatal(err) + } + if err := s.CompleteDeliveryAttempt(ctx, sent, DeliverySent, base.Add(time.Second)); err != nil { + t.Fatal(err) + } + dropped, err := s.BeginDeliveryAttempt(ctx, "nudge", "care", 0, "telegram", "h2", base.Add(time.Minute)) + if err != nil { + t.Fatal(err) + } + if err := s.CompleteDeliveryAttempt(ctx, dropped, DeliveryDropped, base.Add(time.Minute)); err != nil { + t.Fatal(err) + } + if _, err := s.BeginDeliveryAttempt(ctx, "reminder", "", 7, "voice", "h3", base.Add(2*time.Minute)); err != nil { + t.Fatal(err) + } + + all, err := s.ListDeliveryAttempts(ctx, "", 10) + if err != nil || len(all) != 3 { + t.Fatalf("ListDeliveryAttempts = %d rows, err=%v, want 3", len(all), err) + } + // Newest first. + if all[0].Kind != "reminder" || all[0].ReminderID != 7 { + t.Fatalf("newest row is %+v, want the reminder", all[0]) + } + if all[0].HasComplete { + t.Fatalf("a pending row must have no completion time: %+v", all[0]) + } + if !all[2].HasComplete || !all[2].Completed.Equal(base.Add(time.Second)) { + t.Fatalf("completed row lost its time: %+v", all[2]) + } + + only, err := s.ListDeliveryAttempts(ctx, DeliveryDropped, 10) + if err != nil || len(only) != 1 || only[0].Rule != "care" { + t.Fatalf("dropped filter = %+v, err=%v", only, err) + } + pending, err := s.ListDeliveryAttempts(ctx, DeliveryPending, 10) + if err != nil || len(pending) != 1 || pending[0].Kind != "reminder" { + t.Fatalf("pending filter = %+v, err=%v", pending, err) + } +} diff --git a/internal/store/migrations.go b/internal/store/migrations.go index 8a34ed2..12081d9 100644 --- a/internal/store/migrations.go +++ b/internal/store/migrations.go @@ -240,6 +240,35 @@ ALTER TABLE reminders ADD COLUMN next_fire_ts INTEGER;`, // #2 // The live-only unique index is the tasks one, per list: saying "молоко" // twice before the shop keeps one row, saying it again next week after the // last one was crossed off writes a new one. + + // #20 — unstick the routines accepted before the fire-forever fix + // (Vikunja #377, follow-up to #366). Accepting used to leave accepted_ts + // NULL and a live one-shot reminder behind, and the tick loop skips a row + // with no accepted_ts, so every non-weekly routine accepted before that fix + // has been silent ever since. + // + // Three statements, in this order, per stuck row: adopt created_ts as the + // acceptance time, cancel the reminder that is still holding the schedule, + // then let go of it. Cancelling before clearing matters — clearing first + // loses the only pointer to the reminder and leaves it to fire on its own. + // + // created_ts rather than a fresh timestamp because a migration has no + // clock, and because the first interval should be measured from when he + // said yes. A routine whose interval has already elapsed nudges on the next + // tick, which is what being unstuck looks like. + // + // Weekly rows are included deliberately. Theirs was the case that kept + // working, because the cron reminder reschedules itself — so leaving them + // alone would give them both a cron reminder and a tick-loop schedule for + // one habit, and he would hear it twice. + `UPDATE reminders + SET status = 'cancelled' + WHERE status = 'pending' + AND id IN (SELECT reminder_id FROM proposed_routines + WHERE status = 'accepted' AND accepted_ts IS NULL AND reminder_id IS NOT NULL); + UPDATE proposed_routines + SET accepted_ts = created_ts, reminder_id = NULL + WHERE status = 'accepted' AND accepted_ts IS NULL;`, } // migrate applies every migration with a number greater than the DB's current diff --git a/internal/store/migrations_test.go b/internal/store/migrations_test.go index e0b2663..7aa80ec 100644 --- a/internal/store/migrations_test.go +++ b/internal/store/migrations_test.go @@ -3,6 +3,7 @@ package store import ( "context" "testing" + "time" ) func userVersion(t *testing.T, s *Store) int { @@ -80,3 +81,69 @@ func TestCollapsedCalendarKeysAreDropped(t *testing.T) { t.Fatalf("%d calendar rows left, want the 2 that identify their event", got) } } + +// TestStuckRoutinesAreBackfilled — routines accepted before the fire-forever +// fix have accepted_ts NULL and a live reminder, so the tick loop skips them +// and they have been silent ever since (Vikunja #377). The migration touches +// live reminders, which is why it is tested against a real store. +func TestStuckRoutinesAreBackfilled(t *testing.T) { + ctx := context.Background() + s := newTestStore(t) + created := time.Date(2026, 7, 1, 9, 0, 0, 0, time.UTC) + + rem, err := s.CreateReminder(ctx, created.Add(time.Hour), "полить цветы", "") + if err != nil { + t.Fatal(err) + } + healthy, err := s.CreateReminder(ctx, created.Add(2*time.Hour), "не трогать", "") + if err != nil { + t.Fatal(err) + } + if _, err := s.db.ExecContext(ctx, + `INSERT INTO proposed_routines (action, object, interval_days, status, created_ts, reminder_id, accepted_ts) + VALUES ('water', 'plants', 7, 'accepted', ?, ?, NULL)`, + created.UnixMilli(), rem); err != nil { + t.Fatal(err) + } + // An already-healthy accepted row, and a still-open proposal: neither is + // this migration's business. + if _, err := s.db.ExecContext(ctx, + `INSERT INTO proposed_routines (action, object, interval_days, status, created_ts, accepted_ts) + VALUES ('feed', 'cat', 1, 'accepted', ?, ?)`, + created.UnixMilli(), created.UnixMilli()); err != nil { + t.Fatal(err) + } + + if _, err := s.db.ExecContext(ctx, migrations[18]); err != nil { + t.Fatalf("migration 19: %v", err) + } + + accepted, err := s.ListAcceptedRoutines(ctx) + if err != nil || len(accepted) != 2 { + t.Fatalf("ListAcceptedRoutines = %d rows, err=%v, want 2", len(accepted), err) + } + stuck := accepted[0] + if stuck.Object != "plants" { + stuck = accepted[1] + } + if stuck.AcceptedTs == nil || !stuck.AcceptedTs.Equal(created) { + t.Fatalf("accepted_ts = %v, want the creation time", stuck.AcceptedTs) + } + if stuck.ReminderID != nil { + t.Fatalf("reminder_id = %v, want it let go", stuck.ReminderID) + } + // The reminder it was holding is cancelled, and nothing else is. + var status string + if err := s.db.QueryRowContext(ctx, `SELECT status FROM reminders WHERE id = ?`, rem).Scan(&status); err != nil { + t.Fatal(err) + } + if status != ReminderCancelled { + t.Fatalf("linked reminder status = %q, want cancelled", status) + } + if err := s.db.QueryRowContext(ctx, `SELECT status FROM reminders WHERE id = ?`, healthy).Scan(&status); err != nil { + t.Fatal(err) + } + if status != "pending" { + t.Fatalf("unrelated reminder status = %q, want it untouched", status) + } +} diff --git a/internal/weather/openmeteo.go b/internal/weather/openmeteo.go index 2bc246e..4eedf2d 100644 --- a/internal/weather/openmeteo.go +++ b/internal/weather/openmeteo.go @@ -97,7 +97,59 @@ func (p *OpenMeteoProvider) CurrentWeather(ctx context.Context, location string) }, nil } +// locationCandidates — the spellings to try for a place taken out of a spoken +// sentence, in order. He says "какая погода в Казани", so the word arrives in +// the prepositional case and the geocoder wants the nominative (Vikunja #421). +// +// Two cheap reversals cover most of what he says: a final "е" is usually a +// nominative "а" (Москве → Москва) or nothing at all (Лондоне → Лондон), and a +// final "и" is usually a soft sign (Казани → Казань). Indeclinable names — +// Тбилиси, Сочи, Осло — are already nominative and the first candidate answers. +// +// Nothing here is a guess about the weather: a wrong candidate finds no city +// and the caller says so. It only decides which strings are worth asking about. +func locationCandidates(location string) []string { + out := []string{location} + add := func(s string) { + if s == "" || s == location { + return + } + for _, seen := range out { + if seen == s { + return + } + } + out = append(out, s) + } + r := []rune(location) + if len(r) < 4 { + return out + } + stem := string(r[:len(r)-1]) + switch r[len(r)-1] { + case 'е', 'Е': + add(stem + "а") + add(stem) + case 'и', 'И': + add(stem + "ь") + add(stem) + case 'у', 'У', 'ю', 'Ю': + add(stem + "а") + } + return out +} + func (p *OpenMeteoProvider) geocode(ctx context.Context, location string) (lat, lon float64, name string, err error) { + for _, cand := range locationCandidates(location) { + lat, lon, name, err = p.geocodeOne(ctx, cand) + if err == nil { + return lat, lon, name, nil + } + } + return 0, 0, "", err +} + +func (p *OpenMeteoProvider) geocodeOne(ctx context.Context, location string) (lat, lon float64, name string, err error) { u := fmt.Sprintf("https://geocoding-api.open-meteo.com/v1/search?name=%s&count=1&language=ru&format=json", url.QueryEscape(location)) req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil) if err != nil { @@ -121,7 +173,7 @@ func (p *OpenMeteoProvider) geocode(ctx context.Context, location string) (lat, } if len(geo.Results) == 0 { - return 0, 0, "", fmt.Errorf("location %q not found", location) + return 0, 0, "", fmt.Errorf("%w: %q", ErrLocationUnknown, location) } r := geo.Results[0] diff --git a/internal/weather/openmeteo_test.go b/internal/weather/openmeteo_test.go index e2c1917..b51aae3 100644 --- a/internal/weather/openmeteo_test.go +++ b/internal/weather/openmeteo_test.go @@ -83,3 +83,29 @@ func TestStubProvider(t *testing.T) { t.Fatalf("StubProvider: want ErrNotConfigured, got %v", err) } } + +// TestLocationCandidates — he speaks the prepositional case and the geocoder +// wants the nominative (Vikunja #421). +func TestLocationCandidates(t *testing.T) { + cases := map[string][]string{ + "Москве": {"Москве", "Москва", "Москв"}, + "Казани": {"Казани", "Казань", "Казан"}, + "Лондоне": {"Лондоне", "Лондона", "Лондон"}, + "Тбилиси": {"Тбилиси", "Тбились", "Тбилис"}, + "Berlin": {"Berlin"}, + "Уфе": {"Уфе"}, // too short to strip — asked as spoken + } + for in, want := range cases { + got := locationCandidates(in) + if len(got) != len(want) { + t.Errorf("locationCandidates(%q) = %v, want %v", in, got, want) + continue + } + for i := range got { + if got[i] != want[i] { + t.Errorf("locationCandidates(%q) = %v, want %v", in, got, want) + break + } + } + } +} diff --git a/internal/weather/weather.go b/internal/weather/weather.go index 71a2f45..b10e83f 100644 --- a/internal/weather/weather.go +++ b/internal/weather/weather.go @@ -7,6 +7,13 @@ import ( var ErrNotConfigured = errors.New("weather: not configured") +// ErrLocationUnknown — the geocoder has no such place. A named city that does +// not resolve must read differently from a provider outage: one is "I do not +// know that place", the other is "I could not reach the service", and +// answering for the default location instead is the defect this replaces +// (Vikunja #421). +var ErrLocationUnknown = errors.New("weather: location not found") + type Weather struct { Location string `json:"location"` Temperature float64 `json:"temperature"`