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.
This commit is contained in:
+23
@@ -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
|
reach remained 28/30. The lifecycle reacquire test, focused race suite, full
|
||||||
aggregate command, and portable no-runtime packages all pass. Measurement:
|
aggregate command, and portable no-runtime packages all pass. Measurement:
|
||||||
`docs/evals/2026-08-13-onnx-runtime-lifecycle.md`.
|
`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”.
|
||||||
|
|||||||
+44
-29
@@ -419,18 +419,6 @@ func (h *reactiveHandler) resolveClarifyAnswer(ctx context.Context, text string)
|
|||||||
if stillOpen(q.Missing, whenTextOf(q), merged) {
|
if stillOpen(q.Missing, whenTextOf(q), merged) {
|
||||||
return h.reaskOrGiveUp(ctx, q, merged, text, taken), true
|
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
|
// 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:
|
// normal path. Clarify is deliberately false and the intent is unchanged:
|
||||||
// filling in an argument never grants authority, so the completed decision
|
// 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,
|
Intent: intent,
|
||||||
Slots: applyDialogueSlots(answer, merged),
|
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
|
// 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
|
return strings.TrimSpace(utterance) + " " + subject
|
||||||
}
|
}
|
||||||
|
|
||||||
// askRemainingGap re-parks the request when the answer closed one gap and
|
// askRemainingGap re-parks the request when the answer closed one gap and the
|
||||||
// wantedSlots still names another. Returns ("", false) when the request is
|
// action schema still names another. Returns ("", false) only when the request
|
||||||
// complete, when there is no question for what is left, or when she is out of
|
// is complete. A remaining gap is always handled here: one next question while
|
||||||
// attempts — in all three the caller runs the decision as it stands, which for
|
// budget remains, otherwise an explicit give-up with no partial action (V-717).
|
||||||
// the out-of-attempts case is the old behaviour and is the right one: she has
|
|
||||||
// already asked enough.
|
|
||||||
//
|
//
|
||||||
// The attempt budget is shared with the re-ask path on purpose. A second gap
|
// 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
|
// 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.
|
// 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) {
|
func (h *reactiveHandler) askRemainingGap(ctx context.Context, q *dialogue.PendingQuestion, dec router.Decision) (string, bool) {
|
||||||
remaining := stillMissingFor(intent, whenTextOf(q), merged)
|
remaining := missingFor(dec)
|
||||||
if len(remaining) == 0 {
|
if len(remaining) == 0 {
|
||||||
return "", false
|
return "", false
|
||||||
}
|
}
|
||||||
// Attempts+1 is the question she is about to ask, and the budget is shared
|
// 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.
|
// 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() {
|
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
|
// 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
|
// 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,
|
MaxAttempts: q.MaxAttempts,
|
||||||
Rides: q.Rides,
|
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
|
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
|
// finishClarified completes a decision whose parked gaps were already checked
|
||||||
// by resolveClarifyAnswer. It still records the turn for a later correction;
|
// by resolveClarifyAnswer. It still records the turn for a later correction;
|
||||||
// the old path made anything completed through dialogue uncorrectable (V-573).
|
// the old path made anything completed through dialogue uncorrectable (V-573).
|
||||||
func (h *reactiveHandler) finishClarified(ctx context.Context, dec router.Decision) string {
|
func (h *reactiveHandler) finishClarified(ctx context.Context, dec, schemaDec router.Decision) string {
|
||||||
return h.finishRebuilt(ctx, dec, false)
|
return h.finishRebuilt(ctx, dec, schemaDec, false)
|
||||||
}
|
}
|
||||||
|
|
||||||
// finishRepaired validates a decision rebuilt from an older utterance. Unlike
|
// finishRepaired validates a decision rebuilt from an older utterance. Unlike
|
||||||
// resolveClarifyAnswer, repair has not passed the current slot gate, so it must
|
// resolveClarifyAnswer, repair has not passed the current slot gate, so it must
|
||||||
// ask about any missing argument before acting (V-573).
|
// ask about any missing argument before acting (V-573).
|
||||||
func (h *reactiveHandler) finishRepaired(ctx context.Context, dec router.Decision) string {
|
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 {
|
// finishRebuilt is the execution boundary for decisions reconstructed from
|
||||||
if validate && (dec.Clarify || len(missingFor(dec)) > 0) {
|
// 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 != "" {
|
if reply := h.hexisBeforeClarify(ctx, dec); reply != "" {
|
||||||
return reply
|
return reply
|
||||||
}
|
}
|
||||||
@@ -633,6 +644,10 @@ func (h *reactiveHandler) finishRebuilt(ctx context.Context, dec router.Decision
|
|||||||
return question
|
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 {
|
if h.dialogueSessions != nil {
|
||||||
now := h.now()
|
now := h.now()
|
||||||
prev := h.dialogueSessions.Get(dialogueIDOf(ctx), now)
|
prev := h.dialogueSessions.Get(dialogueIDOf(ctx), now)
|
||||||
|
|||||||
@@ -394,25 +394,86 @@ func TestClarifyAsksAboutTheSecondGapToo(t *testing.T) {
|
|||||||
|
|
||||||
// TestClarifySecondGapRespectsTheAttemptCap — the second gap spends a question
|
// TestClarifySecondGapRespectsTheAttemptCap — the second gap spends a question
|
||||||
// out of the same budget, so it cannot turn a capped exchange into an endless
|
// 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) {
|
func TestClarifySecondGapRespectsTheAttemptCap(t *testing.T) {
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
h, _, _ := newClarifyHandler(t)
|
h, st, _ := newClarifyHandler(t)
|
||||||
h.clarifyMaxAttempts = 1
|
h.clarifyMaxAttempts = 1
|
||||||
|
|
||||||
if _, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{}, "напомни")); !asked {
|
if _, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{}, "напомни")); !asked {
|
||||||
t.Fatal("expected the subject question")
|
t.Fatal("expected the subject question")
|
||||||
}
|
}
|
||||||
reply, handled := h.resolveClarifyAnswer(ctx, "позвонить маме")
|
reply, handled := h.resolveClarifyAnswer(ctx, "позвонить маме")
|
||||||
if !handled {
|
if !handled || reply != clarifyGaveUp {
|
||||||
t.Fatal("the answer must be consumed")
|
t.Fatalf("out of attempts she must give up visibly, handled=%v reply=%q", handled, reply)
|
||||||
}
|
}
|
||||||
if reply == "Когда?" {
|
if isAnyClarifyQuestion(reply) {
|
||||||
t.Fatal("out of attempts she must not ask a second question")
|
t.Fatalf("out of attempts she must not ask another question: %q", reply)
|
||||||
}
|
}
|
||||||
if h.clarifyStore.Get(voiceDialogueID, h.now()) != nil {
|
if h.clarifyStore.Get(voiceDialogueID, h.now()) != nil {
|
||||||
t.Fatal("no question may stay armed past the cap")
|
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
|
// TestClarifyProseHoldsThePersona — these lines are hand-written Russian that
|
||||||
|
|||||||
@@ -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
|
The two remaining false clarifies are the act-with-no-allowlisted-fn arm of the
|
||||||
gate, not this rule.
|
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
|
## The destination
|
||||||
|
|
||||||
`query` was a shrug. The cascade sorted an utterance into one of seven intents,
|
`query` was a shrug. The cascade sorted an utterance into one of seven intents,
|
||||||
|
|||||||
Reference in New Issue
Block a user