diff --git a/cmd/mavend/clarify.go b/cmd/mavend/clarify.go index 1271b8d..33a6816 100644 --- a/cmd/mavend/clarify.go +++ b/cmd/mavend/clarify.go @@ -6,7 +6,6 @@ import ( "math/rand" "strings" "time" - "unicode" "unicode/utf8" "github.com/kami/maven/internal/dialogue" @@ -162,12 +161,13 @@ func withNotice(notice, reply string) string { // напоминание?" — answer first, then the open question. A question in front of // its own answer would read as ignoring what he asked. // -// A statement's full stop is folded into a comma, so the two acts read as one -// sentence — that is the owner's own punctuation, "в Риме сейчас ..., на какое -// время поставить напоминание?". An answer that is ITSELF a question keeps its -// mark and the resume starts a new sentence: she sometimes answers a side query -// by asking him to say it again, and "переформулировать?, на какое время" folds -// two questions into one unreadable line. +// Two sentences, not one (V-654). This used to fold the answer's full stop into +// a comma, on the strength of the owner having written it that way once. Spliced +// onto a real answer it reads as one run-on thought — "вот что я нашла: вайфай +// пароль лежит в ящике стола, на какое время поставить напоминание?" — and the +// question disappears into the tail of a sentence about something else. A reply +// with no terminator of its own is given one, so the join never depends on how +// the phraser chose to end. // // A resume with no answer in front of it is just the question. func withResumed(reply, resumed string) string { @@ -178,23 +178,17 @@ func withResumed(reply, resumed string) string { if reply == "" { return resumed } - if strings.HasSuffix(reply, "?") { - return reply + " " + resumed + if !endsSentence(reply) { + reply += "." } - if trimmed := strings.TrimRight(reply, ".!"); trimmed != "" { - reply = trimmed - } - return reply + ", " + lowerFirst(resumed) + return reply + " " + resumed } -// lowerFirst lowercases the opening rune, so a deck line written as a standalone -// sentence reads as the second half of one. Only the first rune: "На какое -// время" must become "на какое время" and nothing else in it may move. -func lowerFirst(s string) string { - for i, r := range s { - return string(unicode.ToLower(r)) + s[i+utf8.RuneLen(r):] - } - return s +// endsSentence reports whether s already closes itself. The ellipsis counts: a +// trailing "…" is a deliberate end, and a full stop after it reads as a typo. +func endsSentence(s string) bool { + r, _ := utf8.DecodeLastRuneInString(s) + return strings.ContainsRune(".!?…", r) } // missingFor returns the slots a decision still needs, most important first. @@ -381,6 +375,14 @@ func (h *reactiveHandler) resolveClarifyAnswer(ctx context.Context, text string) return "", false } + // He is answering, so the run of step-asides is over (V-654). Reset here + // rather than where a gap is FILLED: "позвонить маме" against a question + // about the time gives her nothing she asked for and still means he is in + // the exchange, and the retry it costs is bound enough on its own. The + // counter is for the case the bounds miss — he asked for other things and + // never came back. + q.Suspends = 0 + merged := q.Answer(text, toDialogueSlots(answer)) // Fold a newly answered subject into the raw utterance. Downstream actions // phrase from Utterance, not from the text slot — actionReminder stores it @@ -467,6 +469,14 @@ func (h *reactiveHandler) noteDropped(ctx context.Context) { // // A slot with no resumed wording (clarifyResumedFor says so) resumes nothing and // says nothing. She must not claim to be holding a question she cannot re-ask. +// +// Suspension is bounded, since V-654. Neither of the two things above is a +// limit: no attempt is spent, and restarting the clock means the TTL cannot +// arrive while he keeps talking. So the count is the only thing that ends it, +// and past MaxSuspends she lets the request go and says so with the same line +// every other drop uses. The rule is unchanged — a question ends by being +// answered or by being let go out loud — this only recognises three unrelated +// requests in a row as the second of those. func (h *reactiveHandler) noteSuspended(ctx context.Context, q *dialogue.PendingQuestion) { rt := turnRouteFrom(ctx) if rt == nil || len(q.Missing) == 0 { @@ -476,11 +486,18 @@ func (h *reactiveHandler) noteSuspended(ctx context.Context, q *dialogue.Pending if !ok { return } + if !q.CanResume() { + h.clarifyStore.Delete(dialogueIDOf(ctx)) + h.noteDropped(ctx) + log.Printf("voice: clarify — the question about %s stepped aside %d times; letting the request go", q.Missing[0], q.Suspends) + return + } + q.Suspends++ q.Asked = h.now() h.clarifyStore.Put(dialogueIDOf(ctx), q) rt.resume = question rt.suspended = true - log.Printf("voice: clarify — is its own request; suspending the question about %s and resuming it in the same reply", q.Missing[0]) + log.Printf("voice: clarify — is its own request; suspending the question about %s and resuming it in the same reply (suspend %d of %d)", q.Missing[0], q.Suspends, dialogue.MaxSuspends) } // foldAnswerIntoUtterance appends an answered subject to the original words, @@ -520,6 +537,9 @@ func (h *reactiveHandler) askRemainingGap(ctx context.Context, q *dialogue.Pendi if !ok || !q.CanAsk() { return "", false } + // Suspends is not carried, and by this point it is already zero: the answer + // path resets it (V-654). Left off the literal so the zero is stated where + // the struct is built, rather than inherited from a field nobody names. h.clarifyStore.Put(dialogueIDOf(ctx), &dialogue.PendingQuestion{ Intent: q.Intent, Slots: merged, diff --git a/cmd/mavend/clarify_test.go b/cmd/mavend/clarify_test.go index 732c306..946c393 100644 --- a/cmd/mavend/clarify_test.go +++ b/cmd/mavend/clarify_test.go @@ -740,3 +740,53 @@ func TestACompleteTurnStillDoesNotAsk(t *testing.T) { } } } + +// TestTheResumedQuestionIsItsOwnSentence — V-654. The re-ask used to be spliced +// onto the answer with a comma, so a real answer and an unrelated open question +// read as one run-on thought and the question vanished into its tail. +func TestTheResumedQuestionIsItsOwnSentence(t *testing.T) { + const resumed = "На какое время поставить напоминание?" + cases := []struct { + name string + reply string + want string + }{ + { + // The measured line, shortened. Two sentences, and the question keeps + // its capital. + name: "a statement keeps its full stop", + reply: "Вайфай пароль лежит в ящике стола.", + want: "Вайфай пароль лежит в ящике стола. " + resumed, + }, + { + name: "a reply with no terminator is given one", + reply: "Вайфай пароль лежит в ящике стола", + want: "Вайфай пароль лежит в ящике стола. " + resumed, + }, + { + // She sometimes answers a side query by asking him to say it again. + // Two questions, and neither may swallow the other. + name: "a question keeps its mark", + reply: "Можешь переформулировать?", + want: "Можешь переформулировать? " + resumed, + }, + { + name: "an ellipsis is already an ending", + reply: "Не уверена…", + want: "Не уверена… " + resumed, + }, + { + name: "a resume with no answer in front of it is just the question", + reply: "", + want: resumed, + }, + } + for _, tc := range cases { + if got := withResumed(tc.reply, resumed); got != tc.want { + t.Errorf("%s: withResumed(%q) = %q, want %q", tc.name, tc.reply, got, tc.want) + } + } + if got := withResumed("Готово.", ""); got != "Готово." { + t.Errorf("nothing to resume must leave the reply alone, got %q", got) + } +} diff --git a/cmd/mavend/turnrole_test.go b/cmd/mavend/turnrole_test.go index c230519..07fdc69 100644 --- a/cmd/mavend/turnrole_test.go +++ b/cmd/mavend/turnrole_test.go @@ -279,3 +279,97 @@ func TestTheTurnIsRoutedOnce(t *testing.T) { t.Fatalf("the pipeline routed again and got something else: %+v vs %+v", second, first) } } + +// TestASuspendedQuestionDoesNotRideForever — V-654, the measured failure of +// 2026-08-07 (docs/evals/2026-08-07-week-of-usage-transcript.md, t=51 to t=58). +// +// A side query suspends the parked question, spends no attempt and restarts the +// TTL. Nothing else bounded it, so one unfilled time slot came back on the end +// of six consecutive unrelated replies and stopped only when a seventh turn +// happened to read as a failed answer. Three step-asides, then she lets it go +// and says so. +func TestASuspendedQuestionDoesNotRideForever(t *testing.T) { + ctx := context.Background() + h, st := newRoutingClarifyHandler(t) + id := dialogueIDFor(sourceText, "web") + resumed, _ := clarifyResumedFor(dialogue.SlotTime) + + if reply := h.handleText(ctx, "web", "напомни позвонить маме"); !strings.Contains(reply, "?") { + t.Fatalf("expected the time question, got %q", reply) + } + + // Three questions of his own. Each one is answered as itself and each one + // brings the open question back, exactly as V-561 asks. + asides := []string{ + "о чём мы вчера говорили?", + "какие у меня напоминания?", + "сколько времени?", + } + for i, text := range asides { + reply := h.handleText(ctx, "web", text) + if !strings.HasSuffix(reply, resumed) { + t.Fatalf("side query %d: the question must come back, got %q", i+1, reply) + } + if strings.Contains(reply, clarifyDropped) { + t.Fatalf("side query %d: nothing was let go yet, so nothing may say so: %q", i+1, reply) + } + q := h.clarifyStore.Get(id, h.now()) + if q == nil { + t.Fatalf("side query %d: the question was dropped early", i+1) + } + if q.Attempts != 1 { + t.Fatalf("side query %d: a step-aside spent an attempt: %d", i+1, q.Attempts) + } + if q.Suspends != i+1 { + t.Fatalf("side query %d: suspends = %d, want %d", i+1, q.Suspends, i+1) + } + } + + // The fourth. She has stepped aside as often as she is willing to, so the + // request goes — out loud, and without the question on the tail. + reply := h.handleText(ctx, "web", "что у меня сегодня?") + if !strings.Contains(reply, clarifyDropped) { + t.Fatalf("the request was let go in silence: %q", reply) + } + if strings.HasSuffix(reply, resumed) { + t.Fatalf("a question she has let go must not be asked again: %q", reply) + } + if h.clarifyStore.Get(id, h.now()) != nil { + t.Fatal("the question must be gone once she has said she let it go") + } + if reminders, err := st.DueReminders(ctx, h.now().Add(48*time.Hour)); err != nil || len(reminders) != 0 { + t.Fatalf("a reminder was invented for a time nobody gave: %v err=%v", reminders, err) + } +} + +// TestAnAnsweredGapResetsTheSuspendBudget — the counter measures CONSECUTIVE +// step-asides. He filled a gap, so the run is broken and the next question +// starts with its full allowance: a long exchange he is engaged with must not +// run out of patience on his behalf. +func TestAnAnsweredGapResetsTheSuspendBudget(t *testing.T) { + ctx := context.Background() + h, _ := newRoutingClarifyHandler(t) + id := dialogueIDFor(sourceText, "web") + + // A bare "напомни" is missing both halves, so answering the subject re-parks + // the request with a question about the time. + if reply := h.handleText(ctx, "web", "напомни"); !strings.Contains(reply, "?") { + t.Fatalf("expected a question, got %q", reply) + } + if reply := h.handleText(ctx, "web", "какие у меня напоминания?"); reply == "" { + t.Fatal("the side query must be answered as itself") + } + if q := h.clarifyStore.Get(id, h.now()); q == nil || q.Suspends != 1 { + t.Fatalf("the side query was not counted: %+v", q) + } + if reply := h.handleText(ctx, "web", "позвонить маме"); reply == "" { + t.Fatal("the answer must be consumed") + } + q := h.clarifyStore.Get(id, h.now()) + if q == nil { + t.Fatal("a reminder still needs its time, so a question must be parked") + } + if q.Suspends != 0 { + t.Fatalf("answering a gap must reset the suspend budget: suspends = %d", q.Suspends) + } +} diff --git a/docs/design.md b/docs/design.md index 13e7b37..bdffc12 100644 --- a/docs/design.md +++ b/docs/design.md @@ -1,6 +1,6 @@ # Maven — Design -*Last verified: 2026-08-02 @ 7079a24. Living doc: correct it in place, do not append.* +*Last verified: 2026-08-07 @ beb093a. Living doc: correct it in place, do not append.* > Folded 2026-07-30 from `SPEC.md` (north star, 2026-07-03), `maven.md` > (consolidated decisions, 2026-06-30) and `ROADMAP.md` (execution plan, @@ -282,6 +282,39 @@ Three reasons, in the order they settle it: So the notice stays what it is: the in-process TTL case, where she really did wait and really did let go. +#### A parked question may step aside three times + +Decided 2026-08-07 (V-654). A side query or an aside suspends the parked +question instead of dropping it. The words are answered as themselves, and the +question comes back on the end of the same reply. + +Neither bound on a question reaches that path. No attempt is spent, because a +side query is not a failed answer, so `MaxAttempts` never applies. +`noteSuspended` also restarts the 90s clock, since she is about to speak the +question again. So the TTL cannot arrive while he keeps talking. + +Measured on 2026-08-07: one unfilled time slot rode the tail of six consecutive +unrelated replies. It stopped only when a seventh turn happened to read as a +failed answer. See `docs/evals/2026-08-07-week-of-usage.md`. + +`PendingQuestion.Suspends` counts the step-asides. `MaxSuspends` is 3, matching +`DefaultMaxAttempts`. Past it she lets the request go, with the same +`clarifyDropped` line every other drop uses. The owner's rule is unchanged. A +question still ends by being answered or by being let go out loud. This only +recognises three unrelated requests in a row as the second of those. + +The count is of CONSECUTIVE step-asides. It resets the moment he answers, in +`resolveClarifyAnswer`. An answer that gives her nothing she asked for resets it +too. "Позвонить маме" against a question about the time is still him in the +exchange. The retry it costs is bound enough on its own. + +The re-ask is also two sentences rather than one. It used to be spliced onto the +answer with a comma. On a real answer that buries the question in the tail of +one run-on thought: + +> вот что я нашла: вайфай пароль лежит в ящике стола, на какое время поставить +> напоминание? + ### save-where — the two-memory routing axis One discriminator: **does the loop evaluate a predicate against it?** diff --git a/internal/dialogue/clarify.go b/internal/dialogue/clarify.go index 9337095..ed59791 100644 --- a/internal/dialogue/clarify.go +++ b/internal/dialogue/clarify.go @@ -41,8 +41,33 @@ type PendingQuestion struct { Attempts int // questions already asked // MaxAttempts caps Attempts. 0 ⇒ DefaultMaxAttempts. MaxAttempts int + // Suspends counts how many times this question has stepped aside for + // something he asked instead, and come back on the end of the answer. It is + // deliberately NOT an attempt: a side query is not a failed answer, and + // charging it a retry is the V-554 shape. See CanResume for why it is + // counted at all. + Suspends int } +// MaxSuspends — how many times one question may step aside and come back before +// she lets the request go (V-654). +// +// It exists because suspension had no bound of any kind. A side query spends no +// attempt, so MaxAttempts never applies to it, and it restarts the 90s clock, so +// the TTL never arrives either. Measured on 2026-08-07: one unfilled time slot +// rode the end of six consecutive unrelated replies and stopped only when a +// seventh turn happened to read as a failed answer. +// +// Three, matching DefaultMaxAttempts, and for the same reason. Once he has +// asked for three other things without touching the question, the likely truth +// is that he has moved on and has not said so. +const MaxSuspends = 3 + +// CanResume reports whether this question may step aside once more. False ⇒ the +// caller lets the request go and says so; it must never simply stop resuming, +// because a question dropped in silence reads as one that was answered. +func (q *PendingQuestion) CanResume() bool { return q.Suspends < MaxSuspends } + // Action reads the parked question as the typed action it is assembling // (pending.go). Derived rather than stored: the question's fields stay the one // copy of the truth, so a caller that fills them the old way cannot end up with