From 1b76fa8205c5355e7e568bc81c638667fefa807b Mon Sep 17 00:00:00 2001 From: claude Date: Wed, 5 Aug 2026 23:34:25 +0400 Subject: [PATCH] an act with nothing on the other end says so (V-556) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit askClarify parked "Что сделать?" whatever was on the other end. With an empty allowlist that question has no answer: she asks, fails, asks again and gives up, three turns spent on a request she could have declined in the first one. Empty allowlist now names the gap and parks nothing. A non-empty one still asks, and names what she can run, capped at six, so the question is answerable. --- cmd/mavend/clarify.go | 14 +++++++ cmd/mavend/clarify_test.go | 74 ++++++++++++++++++++++++++++++++-- cmd/mavend/clarifytemplates.go | 32 +++++++++++++++ 3 files changed, 117 insertions(+), 3 deletions(-) diff --git a/cmd/mavend/clarify.go b/cmd/mavend/clarify.go index 837933d..35f8573 100644 --- a/cmd/mavend/clarify.go +++ b/cmd/mavend/clarify.go @@ -156,6 +156,20 @@ func (h *reactiveHandler) askClarify(ctx context.Context, dec router.Decision) ( if !ok { return "", false } + // An act is the one gap that may have nothing on the other end. Ask only + // when she has something to run, and say what it is (Vikunja #556). + if slot == dialogue.SlotFn { + var allow []string + if h.matcher != nil { + allow = h.matcher.Allowlist() + } + reply, ask := fnClarify(allow, 1) + if !ask { + log.Printf("voice: clarify — act with %d allowlisted fns; naming the gap instead of asking", len(allow)) + return reply, reply != "" + } + question = reply + } h.clarifyStore.Put(dialogueIDOf(ctx), &dialogue.PendingQuestion{ Intent: dialogue.Intent(dec.Intent), Slots: toDialogueSlots(dec.Slots), diff --git a/cmd/mavend/clarify_test.go b/cmd/mavend/clarify_test.go index 66e235c..904dbac 100644 --- a/cmd/mavend/clarify_test.go +++ b/cmd/mavend/clarify_test.go @@ -232,11 +232,15 @@ func TestClarifyRestatedAnswerWins(t *testing.T) { } // TestClarifiedActOffAllowlistIsStillRefused — clarification fills in an -// argument, it never grants authority. +// argument, it never grants authority. One tool is enabled so there is a +// question to park at all (Vikunja #556); the answer names something else. func TestClarifiedActOffAllowlistIsStillRefused(t *testing.T) { ctx := context.Background() h, st, _ := newClarifyHandler(t) marker := filepath.Join(t.TempDir(), "not-allowed-ran") + if err := st.EnableTool(ctx, "uptime", []string{"true"}, false, "test", h.now()); err != nil { + t.Fatal(err) + } if _, asked := h.askClarify(ctx, clarifyDec(router.IntentAct, router.Slots{Text: "сделай это"}, "сделай это")); !asked { t.Fatal("an act with no fn should be asked about") @@ -251,7 +255,7 @@ func TestClarifiedActOffAllowlistIsStillRefused(t *testing.T) { if _, err := os.Stat(marker); !os.IsNotExist(err) { t.Fatalf("a clarified act off the allowlist ran anyway: %v", err) } - if tools, err := st.ListTools(ctx, "enabled"); err != nil || len(tools) != 0 { + if tools, err := st.ListTools(ctx, "enabled"); err != nil || len(tools) != 1 { t.Fatalf("clarify must not enable a tool: tools=%+v err=%v", tools, err) } } @@ -571,7 +575,11 @@ func TestARestartExpiresTheParkedQuestion(t *testing.T) { // line. None of them was ever an answer. func TestClarifyStepsAsideForItsOwnRequest(t *testing.T) { ctx := context.Background() - h, _, _ := newClarifyHandler(t) + h, st, _ := newClarifyHandler(t) + // One enabled tool, so there is a question to park (Vikunja #556). + if err := st.EnableTool(ctx, "uptime", []string{"true"}, false, "test", h.now()); err != nil { + t.Fatal(err) + } if _, asked := h.askClarify(ctx, clarifyDec(router.IntentAct, router.Slots{Text: "выключи свет в спальне"}, "выключи свет в спальне")); !asked { t.Fatal("an act with no fn should be asked about") @@ -620,3 +628,63 @@ func TestClarifyQuestionShapedAnswerThatFillsTheGapStillLands(t *testing.T) { t.Fatalf("reminder was not created: reminders=%v err=%v", reminders, err) } } + +// TestActWithNothingOnTheOtherEndNamesTheGap — Vikunja #556. With no tool +// enabled, "Что сделать?" has no answer he could give, so she says so and parks +// nothing rather than spending three turns on a request she cannot fulfil. +func TestActWithNothingOnTheOtherEndNamesTheGap(t *testing.T) { + ctx := context.Background() + h, _, _ := newClarifyHandler(t) + + reply, spoken := h.askClarify(ctx, clarifyDec(router.IntentAct, router.Slots{Text: "выключи свет"}, "выключи свет")) + if !spoken || reply == "" { + t.Fatal("an act with nothing on the other end must still say something") + } + if strings.Contains(reply, "?") { + t.Errorf("reply = %q, want a statement, not a question", reply) + } + if h.clarifyStore.Get(voiceDialogueID, h.now()) != nil { + t.Error("nothing to ask about, so nothing may be parked") + } +} + +// TestActClarifyNamesWhatSheCanDo — the other half. With tools enabled the +// question stands, and it names them so it is answerable. +func TestActClarifyNamesWhatSheCanDo(t *testing.T) { + ctx := context.Background() + h, st, _ := newClarifyHandler(t) + for _, name := range []string{"uptime", "disk"} { + if err := st.EnableTool(ctx, name, []string{"true"}, false, "test", h.now()); err != nil { + t.Fatal(err) + } + } + + reply, asked := h.askClarify(ctx, clarifyDec(router.IntentAct, router.Slots{Text: "сделай это"}, "сделай это")) + if !asked { + t.Fatal("with tools enabled she should still ask") + } + for _, want := range []string{"uptime", "disk"} { + if !strings.Contains(reply, want) { + t.Errorf("reply = %q, want it to name %q", reply, want) + } + } + if h.clarifyStore.Get(voiceDialogueID, h.now()) == nil { + t.Error("the request must be parked so the answer can land") + } +} + +// TestFnClarifyCapsTheListItRecites — a spoken sentence cannot carry twelve +// names, which is how many this deployment enables. +func TestFnClarifyCapsTheListItRecites(t *testing.T) { + allow := []string{"a", "b", "c", "d", "e", "f", "g", "h"} + reply, ask := fnClarify(allow, 1) + if !ask { + t.Fatal("a non-empty allowlist is still a question") + } + if strings.Contains(reply, "g") || strings.Contains(reply, "h") { + t.Errorf("reply = %q, want the list capped at %d", reply, namedActsCap) + } + if !strings.Contains(reply, "…") { + t.Errorf("reply = %q, want it to admit the list was cut", reply) + } +} diff --git a/cmd/mavend/clarifytemplates.go b/cmd/mavend/clarifytemplates.go index c080210..e0d64c6 100644 --- a/cmd/mavend/clarifytemplates.go +++ b/cmd/mavend/clarifytemplates.go @@ -1,6 +1,8 @@ package main import ( + "strings" + "github.com/kami/maven/internal/dialogue" "github.com/kami/maven/internal/router" ) @@ -47,6 +49,36 @@ var clarifyQuestionVariants = map[dialogue.Slot][]string{ }, } +// namedActsCap bounds how many capability names one question may recite. Six is +// what a spoken sentence carries; past that the list stops being an answer and +// becomes a wall he has to hold in his head. +const namedActsCap = 6 + +// fnClarify decides what to say about an act whose capability is missing, and +// whether the request is worth parking (Vikunja #556). +// +// The old deck asked "Что сделать?" whatever was on the other end. With an empty +// allowlist that question has no answer: nothing he says can match, so she asks, +// fails, asks again and gives up — three turns spent on a request she could have +// declined in the first one. So an empty allowlist names the gap and parks +// nothing, and a non-empty one asks a question he can actually answer by naming +// what she has. +func fnClarify(allow []string, attempt int) (reply string, ask bool) { + if len(allow) == 0 { + return "Я пока ничего не умею делать — мне не разрешён ни один инструмент.", false + } + q, ok := clarifyQuestionFor(dialogue.SlotFn, attempt) + if !ok { + return "", false + } + named := allow + tail := "" + if len(named) > namedActsCap { + named, tail = named[:namedActsCap], "…" + } + return q + " Я умею: " + strings.Join(named, ", ") + tail + ".", true +} + // clarifyQuestionFor picks the wording for this attempt. attempt is 1-based, as // PendingQuestion.Attempts counts it; anything past the list uses the last and // most explicit phrasing rather than wrapping round to the short one, because