From 8ee3b76af66e7af4b8675b704666b6b9f3b6e935 Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 13 Aug 2026 11:33:05 +0400 Subject: [PATCH] Give up instead of acting on a missing slot (V-717) The clarification attempt cap bounded questions, not the action schema. A request with two required gaps could spend its budget on the first, fill it, and reach applyAction with the second still absent, so the cap acted as permission to execute a partial action. resolveClarifyAnswer now rebuilds the pending action and re-runs the canonical missingFor check after every filled gap. One remaining gap yields exactly one next question while PendingAction.CanAsk permits it. Exhaustion says the give-up line, pops only the active stack level, and performs no write or action. finishRebuilt repeats the invariant at the execution boundary, so a future dialogue caller cannot bypass it. Reminder time answers stay out of the spoken payload but ride along in the decision copy used for validation. --- JOURNAL.md | 23 ++++++++++++ cmd/mavend/clarify.go | 73 +++++++++++++++++++++++--------------- cmd/mavend/clarify_test.go | 73 ++++++++++++++++++++++++++++++++++---- docs/routing.md | 20 +++++++++++ 4 files changed, 154 insertions(+), 35 deletions(-) diff --git a/JOURNAL.md b/JOURNAL.md index 724ea13..3f1c1b3 100644 --- a/JOURNAL.md +++ b/JOURNAL.md @@ -227,3 +227,26 @@ routing heads 93/96; destination was 11/33 and 25/33 respectively, and ecosystem reach remained 28/30. The lifecycle reacquire test, focused race suite, full aggregate command, and portable no-runtime packages all pass. Measurement: `docs/evals/2026-08-13-onnx-runtime-lifecycle.md`. + +### Clarification exhaustion is fail-closed + +V-717 closes the terminal-policy hole found during the V-573 audit. A request +with two required gaps could spend its only question on the first, fill that +slot, and then reach `applyAction` with the second still absent. The attempt cap +was accidentally acting as permission to execute a partial action. + +The resolver now rebuilds the pending action and re-runs the canonical +`missingFor` schema after every filled gap. One remaining gap produces exactly +one next question only while the shared `PendingAction.CanAsk` budget permits +it. Exhaustion visibly gives up, removes only the active stack level, and makes +no write or action. `finishRebuilt` repeats the same invariant at the execution +boundary. Reminder time answers remain separate from the clean payload but are +included in the schema decision used for validation. + +The original `TestClarifySecondGapRespectsTheAttemptCap` now asserts the exact +give-up and zero reminders. New tests cover direct boundary refusal and a +two-level stack where exhausting the top appends the surviving lower question +to the same reply. The focused V-717 race cases pass in 4.529s; every clarify +case plus all 22 forced dialogue traces pass under the race detector in +26.202s; `internal/dialogue` passes under race in 2.293s. Routing contract: +`docs/routing.md` section “Required slots and attempt exhaustion”. diff --git a/cmd/mavend/clarify.go b/cmd/mavend/clarify.go index ae40ad6..27e79fa 100644 --- a/cmd/mavend/clarify.go +++ b/cmd/mavend/clarify.go @@ -419,18 +419,6 @@ func (h *reactiveHandler) resolveClarifyAnswer(ctx context.Context, text string) if stillOpen(q.Missing, whenTextOf(q), merged) { return h.reaskOrGiveUp(ctx, q, merged, text, taken), true } - // 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 - // reminder wants both a subject and a time. "напомни" with neither used to - // ask "О чём напомнить?", accept "позвонить маме", and then hand applyAction - // 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(ctx, q, intent, merged); asked { - return reply, true - } - h.completeClarifyTop(ctx) - // Rebuild the decision as if it had routed cleanly, then run it down the // normal path. Clarify is deliberately false and the intent is unchanged: // filling in an argument never grants authority, so the completed decision @@ -442,7 +430,23 @@ func (h *reactiveHandler) resolveClarifyAnswer(ctx context.Context, text string) Intent: intent, Slots: applyDialogueSlots(answer, merged), } - return h.finishClarified(ctx, dec), true + // Time answers stay out of dec.Utterance because it is also the reminder + // payload. The action schema still needs that evidence, so validate a copy + // carrying the full time exchange while executing the clean decision. + schemaDec := dec + schemaDec.Utterance = whenTextOf(q) + + // 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 + // reminder wants both a subject and a time. Re-enter the schema one question + // at a time. If the shared attempt budget is spent, askRemainingGap visibly + // gives up and removes this stack level; it must never turn an incomplete + // decision into permission to act (V-717). + if reply, handled := h.askRemainingGap(ctx, q, schemaDec); handled { + return reply, true + } + h.completeClarifyTop(ctx) + return h.finishClarified(ctx, dec, schemaDec), true } // completeClarifyTop finishes only the active question. A nested question can @@ -540,26 +544,27 @@ func foldAnswerIntoUtterance(utterance, subject string) string { return strings.TrimSpace(utterance) + " " + subject } -// askRemainingGap re-parks the request when the answer closed one gap and -// wantedSlots still names another. Returns ("", false) when the request is -// complete, when there is no question for what is left, or when she is out of -// attempts — in all three the caller runs the decision as it stands, which for -// the out-of-attempts case is the old behaviour and is the right one: she has -// already asked enough. +// askRemainingGap re-parks the request when the answer closed one gap and the +// action schema still names another. Returns ("", false) only when the request +// is complete. A remaining gap is always handled here: one next question while +// budget remains, otherwise an explicit give-up with no partial action (V-717). // // 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(ctx context.Context, q *dialogue.PendingQuestion, intent router.Intent, merged dialogue.Slots) (string, bool) { - remaining := stillMissingFor(intent, whenTextOf(q), merged) +func (h *reactiveHandler) askRemainingGap(ctx context.Context, q *dialogue.PendingQuestion, dec router.Decision) (string, bool) { + remaining := missingFor(dec) if len(remaining) == 0 { return "", false } // Attempts+1 is the question she is about to ask, and the budget is shared // with the re-ask path, so the second gap is worded like a second try. - question, ok := h.questionFor(remaining[0], q.Attempts+1, whenTextOf(q), merged, "") + merged := toDialogueSlots(dec.Slots) + question, ok := h.questionFor(remaining[0], q.Attempts+1, dec.Utterance, merged, "") if !ok || !q.CanAsk() { - return "", false + h.completeClarifyTop(ctx) + log.Printf("voice: clarify — gave up with required gap %s still open after %d question(s); no action ran", remaining[0], q.Attempts) + return clarifyGaveUp, true } // 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 @@ -581,7 +586,7 @@ func (h *reactiveHandler) askRemainingGap(ctx context.Context, q *dialogue.Pendi 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) + log.Printf("voice: clarify — one gap filled, still missing %s for intent=%s, asking again (attempt %d)", remaining[0], dec.Intent, q.Attempts+1) return question, true } @@ -613,19 +618,25 @@ func (h *reactiveHandler) reaskOrGiveUp(ctx context.Context, q *dialogue.Pending // finishClarified completes a decision whose parked gaps were already checked // by resolveClarifyAnswer. It still records the turn for a later correction; // the old path made anything completed through dialogue uncorrectable (V-573). -func (h *reactiveHandler) finishClarified(ctx context.Context, dec router.Decision) string { - return h.finishRebuilt(ctx, dec, false) +func (h *reactiveHandler) finishClarified(ctx context.Context, dec, schemaDec router.Decision) string { + return h.finishRebuilt(ctx, dec, schemaDec, false) } // finishRepaired validates a decision rebuilt from an older utterance. Unlike // resolveClarifyAnswer, repair has not passed the current slot gate, so it must // ask about any missing argument before acting (V-573). func (h *reactiveHandler) finishRepaired(ctx context.Context, dec router.Decision) string { - return h.finishRebuilt(ctx, dec, true) + return h.finishRebuilt(ctx, dec, dec, true) } -func (h *reactiveHandler) finishRebuilt(ctx context.Context, dec router.Decision, validate bool) string { - if validate && (dec.Clarify || len(missingFor(dec)) > 0) { +// finishRebuilt is the execution boundary for decisions reconstructed from +// dialogue. schemaDec is the same action with all validation evidence present; +// a clarified reminder includes the separately-held time answers there while +// dec keeps the clean reminder payload. No rebuilt action crosses this boundary +// while missingFor still names a required slot. +func (h *reactiveHandler) finishRebuilt(ctx context.Context, dec, schemaDec router.Decision, ask bool) string { + missing := missingFor(schemaDec) + if ask && (schemaDec.Clarify || len(missing) > 0) { if reply := h.hexisBeforeClarify(ctx, dec); reply != "" { return reply } @@ -633,6 +644,10 @@ func (h *reactiveHandler) finishRebuilt(ctx context.Context, dec router.Decision return question } } + if len(missing) > 0 { + log.Printf("voice: clarify — refusing incomplete rebuilt intent=%s with required gaps %v; no action ran", dec.Intent, missing) + return clarifyGaveUp + } if h.dialogueSessions != nil { now := h.now() prev := h.dialogueSessions.Get(dialogueIDOf(ctx), now) diff --git a/cmd/mavend/clarify_test.go b/cmd/mavend/clarify_test.go index ec7a47a..425a203 100644 --- a/cmd/mavend/clarify_test.go +++ b/cmd/mavend/clarify_test.go @@ -394,25 +394,86 @@ func TestClarifyAsksAboutTheSecondGapToo(t *testing.T) { // TestClarifySecondGapRespectsTheAttemptCap — the second gap spends a question // out of the same budget, so it cannot turn a capped exchange into an endless -// one. With one attempt allowed she acts on what she has instead of asking. +// one. With one attempt allowed she gives up visibly and creates nothing: the +// cap is a bound on dialogue, never a path around the action schema (V-717). func TestClarifySecondGapRespectsTheAttemptCap(t *testing.T) { ctx := context.Background() - h, _, _ := newClarifyHandler(t) + h, st, _ := newClarifyHandler(t) h.clarifyMaxAttempts = 1 if _, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{}, "напомни")); !asked { t.Fatal("expected the subject question") } reply, handled := h.resolveClarifyAnswer(ctx, "позвонить маме") - if !handled { - t.Fatal("the answer must be consumed") + if !handled || reply != clarifyGaveUp { + t.Fatalf("out of attempts she must give up visibly, handled=%v reply=%q", handled, reply) } - if reply == "Когда?" { - t.Fatal("out of attempts she must not ask a second question") + if isAnyClarifyQuestion(reply) { + t.Fatalf("out of attempts she must not ask another question: %q", reply) } if h.clarifyStore.Get(voiceDialogueID, h.now()) != nil { t.Fatal("no question may stay armed past the cap") } + if reminders, err := st.DueReminders(ctx, h.now().Add(48*time.Hour)); err != nil || len(reminders) != 0 { + t.Fatalf("an incomplete exhausted request created a reminder: reminders=%+v err=%v", reminders, err) + } +} + +// Exhausting a nested top request removes only that request and makes the +// lower flow audible again in the same reply. This is the multi-gap exhaustion +// shape, not the ordinary failed-answer path covered in repair_test.go. +func TestClarifySecondGapExhaustionResumesLowerFlow(t *testing.T) { + ctx := context.Background() + h, st, _ := newClarifyHandler(t) + older := &dialogue.PendingQuestion{ + Intent: dialogue.IntentReminder, + Slots: dialogue.Slots{Text: "позвонить маме"}, Missing: []dialogue.Slot{dialogue.SlotTime}, + Utterance: "напомни позвонить маме", Asked: h.now(), TTL: clarifyTTL, + Attempts: 1, MaxAttempts: dialogue.DefaultMaxAttempts, + } + top := &dialogue.PendingQuestion{ + Intent: dialogue.IntentReminder, Missing: []dialogue.Slot{dialogue.SlotText}, + Utterance: "напомни", Asked: h.now(), TTL: clarifyTTL, + Attempts: 1, MaxAttempts: 1, + } + h.clarifyStore.Push(voiceDialogueID, older) + h.clarifyStore.Push(voiceDialogueID, top) + + reply := h.runTurn(ctx, "купить хлеб", sourceText) + resumed, _ := clarifyResumedFor(dialogue.SlotTime) + want := withResumed(clarifyGaveUp, resumed) + if reply != want { + t.Fatalf("reply=%q, want visible top give-up followed by resumed lower question %q", reply, want) + } + if depth := h.clarifyStore.Depth(voiceDialogueID); depth != 1 { + t.Fatalf("exhausting the top request left stack depth %d, want 1", depth) + } + if got := h.clarifyStore.Get(voiceDialogueID, h.now()); got != older { + t.Fatalf("resumed flow=%+v, want the older question", got) + } + if reminders, err := st.DueReminders(ctx, h.now().Add(48*time.Hour)); err != nil || len(reminders) != 0 { + t.Fatalf("nested exhaustion partially created a reminder: reminders=%+v err=%v", reminders, err) + } +} + +// The rebuilt-action boundary repeats the schema invariant even though the +// normal resolver checked it one branch earlier. A future dialogue caller must +// not be able to bypass required slots by calling the completion wrapper. +func TestFinishClarifiedRefusesIncompleteAction(t *testing.T) { + ctx := context.Background() + h, st, _ := newClarifyHandler(t) + dec := router.Decision{ + Utterance: "напомни позвонить маме", + Stage: 2, + Intent: router.IntentReminder, + Slots: router.Slots{Text: "позвонить маме"}, + } + if reply := h.finishClarified(ctx, dec, dec); reply != clarifyGaveUp { + t.Fatalf("incomplete rebuilt action reply=%q, want %q", reply, clarifyGaveUp) + } + if reminders, err := st.DueReminders(ctx, h.now().Add(48*time.Hour)); err != nil || len(reminders) != 0 { + t.Fatalf("rebuilt-action guard allowed a partial reminder: reminders=%+v err=%v", reminders, err) + } } // TestClarifyProseHoldsThePersona — these lines are hand-written Russian that diff --git a/docs/routing.md b/docs/routing.md index 36fd639..2dba3f4 100644 --- a/docs/routing.md +++ b/docs/routing.md @@ -338,6 +338,26 @@ tests are offline and cost nothing. False clarifies 3 to 2, intent-only 74.0% to The two remaining false clarifies are the act-with-no-allowlisted-fn arm of the gate, not this rule. +### Required slots and attempt exhaustion + +A clarification budget limits questions, not the action schema (V-717). The +same `missingFor` check used on a fresh route is applied after each answer to a +parked request. If another required slot remains and `PendingAction.CanAsk` +allows it, Maven asks exactly that first gap and re-parks the request. If the +budget is spent—or that gap has no valid question—Maven says the give-up line, +pops the active request, and performs no write or action. There is no +"best-effort" partially filled execution. + +Reminder time answers are held separately from the utterance that becomes the +spoken payload. The schema check therefore receives a decision copy carrying +the full time evidence, while the action receives the clean payload decision. +The rebuilt-action boundary repeats the `missingFor` invariant before +`applyAction`, so a future dialogue caller cannot bypass it. + +At stack depth two, exhausting the active request uses `CompleteTop`: the lower +request remains parked, its answer window restarts, and its one resumed question +is appended after the visible give-up in the same reply. + ## The destination `query` was a shrug. The cascade sorted an utterance into one of seven intents,