diff --git a/cmd/mavend/clarify.go b/cmd/mavend/clarify.go index 33a6816..1743350 100644 --- a/cmd/mavend/clarify.go +++ b/cmd/mavend/clarify.go @@ -489,15 +489,19 @@ func (h *reactiveHandler) noteSuspended(ctx context.Context, q *dialogue.Pending 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) + log.Printf("voice: clarify — letting the question about %s go: %d asides in a row, %d rides in all", q.Missing[0], q.Suspends, q.Rides) return } q.Suspends++ + // Rides is the same event counted without the reset (V-663). Incremented + // beside Suspends and never anywhere else, so the two cannot disagree about + // what happened, only about how much of it they remember. + q.Rides++ 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 (suspend %d of %d)", q.Missing[0], q.Suspends, dialogue.MaxSuspends) + log.Printf("voice: clarify — is its own request; suspending the question about %s and resuming it in the same reply (suspend %d of %d, ride %d of %d)", q.Missing[0], q.Suspends, dialogue.MaxSuspends, q.Rides, dialogue.MaxRides) } // foldAnswerIntoUtterance appends an answered subject to the original words, @@ -540,6 +544,11 @@ func (h *reactiveHandler) askRemainingGap(ctx context.Context, q *dialogue.Pendi // 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. + // + // Rides IS carried, and that is the whole point of it (V-663). This is the + // same request under a second question, not a new one, so the turns it has + // already ridden still count against it. Dropping the field here is exactly + // the re-basing that let one question ride twenty-six replies. h.clarifyStore.Put(dialogueIDOf(ctx), &dialogue.PendingQuestion{ Intent: q.Intent, Slots: merged, @@ -550,6 +559,7 @@ func (h *reactiveHandler) askRemainingGap(ctx context.Context, q *dialogue.Pendi TTL: clarifyTTL, Attempts: q.Attempts + 1, MaxAttempts: q.MaxAttempts, + Rides: q.Rides, }) log.Printf("voice: clarify — one gap filled, still missing %s for intent=%s, asking again (attempt %d)", remaining[0], intent, q.Attempts+1) return question, true diff --git a/cmd/mavend/turnrole_test.go b/cmd/mavend/turnrole_test.go index 07fdc69..6243033 100644 --- a/cmd/mavend/turnrole_test.go +++ b/cmd/mavend/turnrole_test.go @@ -372,4 +372,68 @@ func TestAnAnsweredGapResetsTheSuspendBudget(t *testing.T) { if q.Suspends != 0 { t.Fatalf("answering a gap must reset the suspend budget: suspends = %d", q.Suspends) } + // The ride it already took is carried across the re-park (V-663). Resetting + // both counters here is what let one question ride twenty-six replies. + if q.Rides != 1 { + t.Fatalf("the aside it already took was forgotten: rides = %d", q.Rides) + } +} + +// TestTwoBoundsCannotRearmEachOther — V-663. +// +// MaxSuspends landed and the measurement did not move: twenty-six of 140 turns +// carried a tail before it and twenty-six after. This is the shape it misses, +// taken from the 2026-08-08 run, where one question rode turns 7 to 13. +// +// An aside spends no attempt, so MaxAttempts never reaches it. A turn that +// reads as a failed answer zeroes Suspends, so MaxSuspends never reaches the +// asides either. Alternating the two rearms each bound with the other's +// traffic. Rides counts both kinds and is never reset, so it is what ends this. +func TestTwoBoundsCannotRearmEachOther(t *testing.T) { + ctx := context.Background() + h, _ := 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) + } + + // Two asides. Each one rides and neither spends an attempt. + for i := 0; i < 2; i++ { + reply := h.handleText(ctx, "web", "какие у меня напоминания?") + if !strings.HasSuffix(reply, resumed) { + t.Fatalf("aside %d: the question must come back, got %q", i+1, reply) + } + } + q := h.clarifyStore.Get(id, h.now()) + if q == nil || q.Rides != 2 || q.Suspends != 2 { + t.Fatalf("after two asides: %+v", q) + } + + // A failed answer. It spends an attempt and puts Suspends back to zero, so + // the old bound starts over. Rides does not. + h.handleText(ctx, "web", "спасибо") + if q = h.clarifyStore.Get(id, h.now()); q == nil || q.Suspends != 0 { + t.Fatalf("the failed answer did not rearm the old bound: %+v", q) + } + if q.Rides != 2 { + t.Fatalf("the rides it already took were forgotten: %+v", q) + } + + // Three more asides. Suspends only reaches three, one short of firing, and + // would go back to zero on the next pleasantry. Rides passes MaxRides and + // the request goes, out loud. + h.handleText(ctx, "web", "какие у меня напоминания?") + h.handleText(ctx, "web", "какие у меня напоминания?") + reply := h.handleText(ctx, "web", "какие у меня напоминания?") + if !strings.Contains(reply, clarifyDropped) { + t.Fatalf("the question rode four asides and 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") + } } diff --git a/docs/design.md b/docs/design.md index bdffc12..130f55d 100644 --- a/docs/design.md +++ b/docs/design.md @@ -308,6 +308,29 @@ The count is of CONSECUTIVE step-asides. It resets the moment he answers, in too. "Позвонить маме" against a question about the time is still him in the exchange. The retry it costs is bound enough on its own. +#### And it may ride four turns in all + +Decided 2026-08-08 (V-663), because the bound above did not move the number it +was written for. Twenty-six of 140 turns carried a tail before it landed and +twenty-six carried one after. + +Two bounds rearm each other. An aside spends no attempt, so `MaxAttempts` never +reaches it. A turn that reads as a failed answer zeroes `Suspends`, so +`MaxSuspends` never reaches the asides. Alternating them, each bound is restored +by the other's traffic. Measured on 2026-08-08: one question about a reminder's +day rode turns 7 to 13. It ended only because turn 14 was a new request. + +`PendingQuestion.Rides` counts the same event as `Suspends` with the resets +taken out. It is set once, incremented only in `noteSuspended`, carried across +the re-park in `askRemainingGap`, and read by nothing that could lower it. +`MaxRides` is 4, one looser than `MaxSuspends` so that the tighter statement +about a run stays reachable. + +This is a bound, not a cure. It ends the measured ride one turn early. Most of +that ride's length is attempts, spent because `classifyTurnRole` reads "спасибо" +and "привет" as failed answers to a question about a day. That is the next +thing to fix and it is not a bound. + 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: diff --git a/internal/dialogue/clarify.go b/internal/dialogue/clarify.go index ed59791..84fd69b 100644 --- a/internal/dialogue/clarify.go +++ b/internal/dialogue/clarify.go @@ -47,6 +47,11 @@ type PendingQuestion struct { // charging it a retry is the V-554 shape. See CanResume for why it is // counted at all. Suspends int + // Rides counts every turn this question has ridden out on the end of + // someone else's reply, over the whole life of the request. Unlike Suspends + // it is never reset and never re-based, which is the only property that + // matters about it (V-663). + Rides int } // MaxSuspends — how many times one question may step aside and come back before @@ -63,10 +68,52 @@ type PendingQuestion struct { // is that he has moved on and has not said so. const MaxSuspends = 3 +// MaxRides — how many turns one question may ride out on the end of an +// unrelated reply, counted over its whole life (V-663). +// +// MaxSuspends did not move the measurement it was written for. Twenty-six of +// 140 turns carried a tail before it landed and twenty-six carried one after. +// Every bound on this question is rearmed by something ordinary: +// +// - The TTL is an inactivity timer, and both noteSuspended and reaskOrGiveUp +// restart it, so it cannot arrive while he keeps talking. +// - Suspends is zeroed by any turn that reads as an answer, which is where +// "спасибо" and "привет" land. It resets before anything is known to have +// been filled. +// - askRemainingGap builds a fresh question for the second gap, so a reminder +// with two gaps gets a new allowance halfway through. +// +// So Suspends only bites on four strictly consecutive side queries with nothing +// chat-like between them, which is not the shape real conversation has. Rides is +// the same idea with the resets taken out: set once, incremented, carried +// across a re-park, and read by nothing that could lower it. +// +// The shape it is aimed at is measured, not imagined. In the 2026-08-08 run one +// question about a reminder's day rode turns 7 to 13 and ended only because +// turn 14 was a new request. Three asides, then two turns that read as failed +// answers, then two more asides. The asides spend no attempt and the answers +// reset Suspends, so the two bounds take turns being rearmed by the other's +// traffic. +// +// Four, not three. It has to be looser than MaxSuspends or that bound is dead +// code, because Rides is never lower than Suspends and would always fire first. +// +// Do not read this as a fix for the whole ride. It ends the measured one a turn +// early and no more. Most of that ride's length is attempts, spent by turns +// like "спасибо" and "привет" being read as failed answers to a question about +// a day. That is a defect in classifyTurnRole and not in any bound here. +const MaxRides = 4 + // 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 } +// +// Two bounds, and they answer different questions. Suspends asks whether he has +// walked away from this exchange in the last few turns. Rides asks whether this +// question has been riding long enough that the answer is no regardless. +func (q *PendingQuestion) CanResume() bool { + return q.Suspends < MaxSuspends && q.Rides < MaxRides +} // 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