Tell him when an expired clarify question is dropped

Vikunja #382. A parked clarifying question past its TTL was discarded
silently on read; now she says the old request is gone and the newly
spoken words are still routed as a fresh utterance.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ
This commit is contained in:
kami
2026-07-31 12:51:47 +04:00
parent dc70a5a7ab
commit 214a4032cf
5 changed files with 124 additions and 13 deletions
+34
View File
@@ -45,6 +45,40 @@ var clarifyQuestions = map[dialogue.Slot]string{
// landed. Feminine self-reference ("поняла"), as everywhere.
const clarifyGaveUp = "Прости, я не поняла. Скажи, пожалуйста, по-другому."
// clarifyExpired — his answer came after the TTL, so the parked request is
// already gone. Same tone as clarifyGaveUp, different reason: too much time
// passed, not "I did not understand". Feminine self-reference ("ждала",
// "отпустила"); he is addressed with a plain imperative.
const clarifyExpired = "Прости, я слишком долго ждала ответа и отпустила прошлую просьбу. Если она ещё нужна, скажи заново."
// clarifyExpiredNotice returns that line when a parked question had just timed
// 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 {
if h.clarifyStore == nil {
return ""
}
if !h.clarifyStore.TakeExpired(voiceDialogueID, h.now()) {
return ""
}
log.Printf("voice: clarify — parked question expired, telling him and routing the words fresh")
return clarifyExpired
}
// withNotice glues the expiry notice in front of this turn's reply. One turn
// carries one reply on the wire, so the notice cannot be a message of its own —
// but neither the notice nor the fresh answer may be dropped.
func withNotice(notice, reply string) string {
if notice == "" {
return reply
}
if reply == "" {
return notice
}
return notice + " " + reply
}
// missingFor returns the slots a decision still needs, most important first.
// Empty ⇒ there is nothing identifiable to ask about.
func missingFor(dec router.Decision) []dialogue.Slot {
+30
View File
@@ -289,6 +289,36 @@ 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()
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 {
t.Fatal("expected a question")
}
*now = now.Add(clarifyTTL + time.Second)
reply := h.handleText(ctx, "как дела")
if !strings.HasPrefix(reply, clarifyExpired) {
t.Fatalf("expired question must be announced first, got %q", reply)
}
if strings.TrimSpace(strings.TrimPrefix(reply, clarifyExpired)) == "" {
t.Fatalf("the new words must still be answered, got only the notice: %q", reply)
}
if h.clarifyStore.Get(voiceDialogueID, 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, "как дела"); strings.Contains(reply, clarifyExpired) {
t.Fatalf("notice repeated on a later turn: %q", reply)
}
}
// TestNoPendingQuestionFallsThrough — with nothing parked, an utterance routes
// normally.
func TestNoPendingQuestionFallsThrough(t *testing.T) {
+23 -13
View File
@@ -402,9 +402,17 @@ func (h *reactiveHandler) HandlePushToTalk(ctx context.Context, req voice.PushTo
return h.reply(ctx, reply, nil)
}
// 1b2. clarify answer — if she asked a question last turn, this utterance is
// its answer, not a fresh command. After the confirm check: a y/n gate is
// armed by her own prompt and is the narrower claim on the utterance.
// 1b2. expired clarify — a question was parked but its TTL ran out, so the
// request behind it is gone. Say that out loud (see clarify.go) and carry
// on: these words are still routed as a fresh utterance below, with the
// notice glued in front of whatever the fresh routing answers. Checked
// BEFORE the answer path: reading a parked question drops an expired one.
expiredNotice := h.clarifyExpiredNotice()
// 1b3. clarify answer — if she asked a live question last turn, this
// utterance is its answer, not a fresh command. After the confirm check: a
// y/n gate is armed by her own prompt and is the narrower claim on the
// utterance.
if reply, handled := h.resolveClarifyAnswer(ctx, text); handled {
return h.reply(ctx, reply, nil)
}
@@ -414,7 +422,7 @@ func (h *reactiveHandler) HandlePushToTalk(ctx context.Context, req voice.PushTo
// unreliably (it's a command, not a free-form query), so we match it
// before routing. Same pattern as the confirm turn above.
if reply, handled := h.resolveQuietToggle(ctx, text); handled {
return h.reply(ctx, reply, nil)
return h.reply(ctx, withNotice(expiredNotice, reply), nil)
}
// 2. router — classify the utterance.
@@ -423,10 +431,10 @@ func (h *reactiveHandler) HandlePushToTalk(ctx context.Context, req voice.PushTo
// ErrNoIntents ⇒ classifier unseeded (cold boot). reply with a
// "still warming up" rather than a wire error.
if errors.Is(err, router.ErrNoIntents) {
return h.reply(ctx, "я ещё не понимаю свободную речь — скоро научусь.", nil)
return h.reply(ctx, withNotice(expiredNotice, "я ещё не понимаю свободную речь — скоро научусь."), nil)
}
log.Printf("voice: router error: %v", err)
return h.reply(ctx, "не получилось разобрать команду.", nil)
return h.reply(ctx, withNotice(expiredNotice, "не получилось разобрать команду."), nil)
}
// 2b. dialogue — fill this turn's missing slots from a prior same-intent
@@ -447,7 +455,7 @@ func (h *reactiveHandler) HandlePushToTalk(ctx context.Context, req voice.PushTo
// stands.
if dec.Clarify {
if question, asked := h.askClarify(dec); asked {
return h.reply(ctx, question, nil)
return h.reply(ctx, withNotice(expiredNotice, question), nil)
}
}
@@ -463,7 +471,7 @@ func (h *reactiveHandler) HandlePushToTalk(ctx context.Context, req voice.PushTo
// 5. tts — synthesise the reply text; return to the voice server which
// ships it back on the conn.
return h.reply(ctx, replyText, nil)
return h.reply(ctx, withNotice(expiredNotice, replyText), nil)
}
// handleText — the core reactive path without stt/tts: confirm check →
@@ -478,7 +486,9 @@ func (h *reactiveHandler) handleText(ctx context.Context, text string) string {
return reply
}
// 1b2. clarify answer — same check as HandlePushToTalk.
// 1b2/1b3. expired clarify then clarify answer — same order and reasons as
// HandlePushToTalk.
expiredNotice := h.clarifyExpiredNotice()
if reply, handled := h.resolveClarifyAnswer(ctx, text); handled {
return reply
}
@@ -487,10 +497,10 @@ func (h *reactiveHandler) handleText(ctx context.Context, text string) string {
dec, err := h.router.Route(ctx, text, h.now())
if err != nil {
if errors.Is(err, router.ErrNoIntents) {
return "я ещё не понимаю свободную речь — скоро научусь."
return withNotice(expiredNotice, "я ещё не понимаю свободную речь — скоро научусь.")
}
log.Printf("voice: handleText router error: %v", err)
return "не получилось разобрать команду."
return withNotice(expiredNotice, "не получилось разобрать команду.")
}
log.Printf("voice: route result: intent=%s slots=%+v", dec.Intent, dec.Slots)
@@ -507,7 +517,7 @@ func (h *reactiveHandler) handleText(ctx context.Context, text string) string {
// 2c. clarify — same as HandlePushToTalk: ask about the one missing thing.
if dec.Clarify {
if question, asked := h.askClarify(dec); asked {
return question
return withNotice(expiredNotice, question)
}
}
@@ -519,7 +529,7 @@ func (h *reactiveHandler) handleText(ctx context.Context, text string) string {
if replyText == "" {
replyText = h.replier.Reply(dec)
}
return replyText
return withNotice(expiredNotice, replyText)
}
// applyAction — executes the router's Decision. Intent-by-intent:
+15
View File
@@ -100,6 +100,21 @@ func (s *ClarifyStore) Get(id string, now time.Time) *PendingQuestion {
return q
}
// TakeExpired reports whether a question was parked here but its TTL ran out,
// and drops it. Get drops such a question silently, which leaves the user
// thinking his request is still alive — the caller uses this to tell him it is
// gone before treating his words as a fresh utterance.
func (s *ClarifyStore) TakeExpired(id string, now time.Time) bool {
s.mu.Lock()
defer s.mu.Unlock()
q, ok := s.questions[id]
if !ok || !q.IsExpired(now) {
return false
}
delete(s.questions, id)
return true
}
func (s *ClarifyStore) Delete(id string) {
s.mu.Lock()
delete(s.questions, id)
+22
View File
@@ -60,6 +60,28 @@ func TestClarifyStoreGetPutDelete(t *testing.T) {
}
}
// TestClarifyStoreTakeExpired — TakeExpired reports (and drops) only a question
// whose TTL ran out.
func TestClarifyStoreTakeExpired(t *testing.T) {
s := NewClarifyStore(time.Minute)
if s.TakeExpired("voice", base) {
t.Fatal("nothing parked ⇒ nothing expired")
}
s.Put("voice", &PendingQuestion{Missing: []Slot{SlotTime}, Asked: base, TTL: time.Minute})
if s.TakeExpired("voice", base.Add(30*time.Second)) {
t.Fatal("a live question must not report as expired")
}
if s.Get("voice", base.Add(30*time.Second)) == nil {
t.Fatal("a live question must survive TakeExpired")
}
if !s.TakeExpired("voice", base.Add(2*time.Minute)) {
t.Fatal("a stale question must report as expired")
}
if s.TakeExpired("voice", base.Add(2*time.Minute)) {
t.Fatal("TakeExpired must drop the question, so the second call is false")
}
}
func TestNewClarifyStoreDefaultTTL(t *testing.T) {
s := NewClarifyStore(0)
q := &PendingQuestion{Asked: base}