Ask up to three times, and say so when she gives up (review of PR 1) #22
@@ -263,6 +263,10 @@ type VoiceConfig struct {
|
||||
// default if unset.
|
||||
QueryMinScore float64 `json:"query_min_score,omitempty"`
|
||||
|
||||
// ClarifyMaxAttempts — how many clarifying questions she may ask about one
|
||||
// request before she gives up and says she did not understand. Default 3.
|
||||
ClarifyMaxAttempts int `json:"clarify_max_attempts,omitempty"`
|
||||
|
||||
// Persona — optional prompt prefix that tunes maven's character. Prepended
|
||||
// to every LLM system prompt (nudge phrasing, note queries, general
|
||||
// knowledge). Empty string ⇒ current hardcoded persona (feminine-gendered
|
||||
@@ -390,7 +394,9 @@ const (
|
||||
DefaultAutotuneInterval = 10 * time.Minute
|
||||
DefaultRouterThreshold = 0.55
|
||||
DefaultQueryMinScore = 0.55
|
||||
DefaultToolTimeout = 30 * time.Second
|
||||
// DefaultClarifyMaxAttempts — see dialogue.DefaultMaxAttempts.
|
||||
DefaultClarifyMaxAttempts = 3
|
||||
DefaultToolTimeout = 30 * time.Second
|
||||
|
||||
DefaultFactEnrichmentInterval = 30 * time.Second
|
||||
)
|
||||
@@ -472,6 +478,9 @@ func (c *Config) applyDefaults() {
|
||||
if c.Voice.QueryMinScore <= 0 {
|
||||
c.Voice.QueryMinScore = DefaultQueryMinScore
|
||||
}
|
||||
if c.Voice.ClarifyMaxAttempts <= 0 {
|
||||
c.Voice.ClarifyMaxAttempts = DefaultClarifyMaxAttempts
|
||||
}
|
||||
if c.Voice.ToolTimeout <= 0 {
|
||||
c.Voice.ToolTimeout = Duration(DefaultToolTimeout)
|
||||
}
|
||||
|
||||
@@ -17,10 +17,11 @@ const (
|
||||
SlotText Slot = "text" // Slots.Text
|
||||
)
|
||||
|
||||
// MaxAttempts is 1 because Maven is not a nag (DESIGN.md § Non-goals). She asks
|
||||
// one clarifying question. If the answer still leaves the slot empty she drops
|
||||
// the request instead of asking again.
|
||||
const MaxAttempts = 1
|
||||
// DefaultMaxAttempts — how many questions she may ask about one request.
|
||||
// Three, because after three tries the likely problem is that she misheard the
|
||||
// whole request, not one slot — so another question about that slot won't help.
|
||||
// Configurable: voice.clarify_max_attempts.
|
||||
const DefaultMaxAttempts = 3
|
||||
|
||||
// PendingQuestion is what Maven holds while she waits for an answer to an open
|
||||
// question. Unlike the yes/no confirms in cmd/mavend/voice.go, the answer here
|
||||
@@ -32,7 +33,17 @@ type PendingQuestion struct {
|
||||
Utterance string // the user's original raw words
|
||||
Asked time.Time
|
||||
TTL time.Duration
|
||||
Attempts int // questions already asked; capped by MaxAttempts
|
||||
Attempts int // questions already asked
|
||||
// MaxAttempts caps Attempts. 0 ⇒ DefaultMaxAttempts.
|
||||
MaxAttempts int
|
||||
}
|
||||
|
||||
// maxAttempts is MaxAttempts with the default filled in.
|
||||
func (q *PendingQuestion) maxAttempts() int {
|
||||
if q.MaxAttempts <= 0 {
|
||||
return DefaultMaxAttempts
|
||||
}
|
||||
return q.MaxAttempts
|
||||
}
|
||||
|
||||
func (q *PendingQuestion) IsExpired(now time.Time) bool {
|
||||
@@ -41,12 +52,9 @@ func (q *PendingQuestion) IsExpired(now time.Time) bool {
|
||||
|
||||
// CanAsk reports whether Maven may ask another question about this request.
|
||||
func (q *PendingQuestion) CanAsk() bool {
|
||||
return q.Attempts < MaxAttempts
|
||||
return q.Attempts < q.maxAttempts()
|
||||
}
|
||||
|
||||
// TODO: the daemon will phrase the question text from Missing (one short ru
|
||||
// question per Slot, feminine self-reference) and speak it here.
|
||||
|
||||
// ClarifyStore holds the parked questions. Same shape and locking as
|
||||
// SessionStore: keyed by dialogue id, expired entries dropped on read.
|
||||
type ClarifyStore struct {
|
||||
@@ -67,8 +75,7 @@ func NewClarifyStore(defaultTTL time.Duration) *ClarifyStore {
|
||||
}
|
||||
}
|
||||
|
||||
// TODO: the daemon will Put a question here when Decision.Clarify fires, in
|
||||
// place of the flat "не разобрала" reply (cmd/mavend/voice.go).
|
||||
// Put parks a question. Called on a clarify decision (cmd/mavend/clarify.go).
|
||||
func (s *ClarifyStore) Put(id string, q *PendingQuestion) {
|
||||
if q.TTL <= 0 {
|
||||
q.TTL = s.defaultTTL
|
||||
@@ -78,8 +85,7 @@ func (s *ClarifyStore) Put(id string, q *PendingQuestion) {
|
||||
s.mu.Unlock()
|
||||
}
|
||||
|
||||
// TODO: the daemon will Get on the next turn, parse that turn into Slots, call
|
||||
// Answer, and Delete — the open-question twin of resolveConfirm.
|
||||
// Get returns the live parked question, or nil when there is none.
|
||||
func (s *ClarifyStore) Get(id string, now time.Time) *PendingQuestion {
|
||||
s.mu.RLock()
|
||||
q, ok := s.questions[id]
|
||||
@@ -101,44 +107,45 @@ func (s *ClarifyStore) Delete(id string) {
|
||||
}
|
||||
|
||||
// Answer merges the slots parsed from the user's answer into the parked ones.
|
||||
// Only the slots listed in Missing are filled, and an already filled slot is
|
||||
// never overwritten — the answer completes the original request, it does not
|
||||
// restate it. Parsing the answer text into `answer` is the caller's job; this
|
||||
// package must stay free of internal/router.
|
||||
// Only the slots listed in Missing are touched. Within those, a value the answer
|
||||
// carries WINS over what was parked: she asked about this slot, so «нет, в пять»
|
||||
// after «в три» must replace the time, not be thrown away.
|
||||
//
|
||||
// This is the clarify answer only. A correction in a fresh turn ("вообще-то
|
||||
// перенеси на пять") is a different code path (followUpMerge) — not here.
|
||||
//
|
||||
// Parsing the answer text into `answer` is the caller's job; this package must
|
||||
// stay free of internal/router.
|
||||
func (q *PendingQuestion) Answer(text string, answer Slots) Slots {
|
||||
out := q.Slots
|
||||
for _, slot := range q.Missing {
|
||||
switch slot {
|
||||
case SlotTime:
|
||||
if !out.HasTime && answer.HasTime {
|
||||
if answer.HasTime {
|
||||
out.Time = answer.Time
|
||||
out.HasTime = true
|
||||
}
|
||||
case SlotKey:
|
||||
if !out.HasKey && answer.HasKey {
|
||||
if answer.HasKey {
|
||||
out.Key = answer.Key
|
||||
out.HasKey = true
|
||||
}
|
||||
case SlotValue:
|
||||
if out.Value == "" && answer.Value != "" {
|
||||
if answer.Value != "" {
|
||||
out.Value = answer.Value
|
||||
}
|
||||
case SlotFn:
|
||||
if !out.HasFn && answer.HasFn {
|
||||
if answer.HasFn {
|
||||
out.Fn = answer.Fn
|
||||
out.HasFn = true
|
||||
if len(out.Args) == 0 {
|
||||
out.Args = append([]string(nil), answer.Args...)
|
||||
}
|
||||
out.Args = append([]string(nil), answer.Args...)
|
||||
}
|
||||
case SlotText:
|
||||
if out.Text == "" {
|
||||
if answer.Text != "" {
|
||||
out.Text = answer.Text
|
||||
} else {
|
||||
// No parse for a text slot — the raw answer IS the text.
|
||||
out.Text = text
|
||||
}
|
||||
if answer.Text != "" {
|
||||
out.Text = answer.Text
|
||||
} else if out.Text == "" {
|
||||
// No parse for a text slot — the raw answer IS the text.
|
||||
out.Text = text
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package dialogue
|
||||
|
||||
import (
|
||||
"reflect"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
@@ -89,12 +90,13 @@ func TestAnswerFillsOnlyMissingSlots(t *testing.T) {
|
||||
want: Slots{Text: "напомни позвонить", Time: answerTime, HasTime: true},
|
||||
},
|
||||
{
|
||||
name: "does not overwrite a filled time",
|
||||
// He restated it: «нет, в пять». The new value wins.
|
||||
name: "a restated time overwrites the parked one",
|
||||
parked: Slots{Time: other, HasTime: true},
|
||||
missing: []Slot{SlotTime},
|
||||
text: "в три",
|
||||
text: "нет, в три",
|
||||
answer: Slots{Time: answerTime, HasTime: true},
|
||||
want: Slots{Time: other, HasTime: true},
|
||||
want: Slots{Time: answerTime, HasTime: true},
|
||||
},
|
||||
{
|
||||
name: "ignores slots that were not missing",
|
||||
@@ -121,12 +123,12 @@ func TestAnswerFillsOnlyMissingSlots(t *testing.T) {
|
||||
want: Slots{Fn: "restart", Args: []string{"nginx"}, HasFn: true},
|
||||
},
|
||||
{
|
||||
name: "keeps existing args when fn was already known",
|
||||
name: "a restated fn replaces the fn and its args",
|
||||
parked: Slots{Fn: "restart", Args: []string{"nginx"}, HasFn: true},
|
||||
missing: []Slot{SlotFn},
|
||||
text: "останови postgres",
|
||||
answer: Slots{Fn: "stop", Args: []string{"postgres"}, HasFn: true},
|
||||
want: Slots{Fn: "restart", Args: []string{"nginx"}, HasFn: true},
|
||||
want: Slots{Fn: "stop", Args: []string{"postgres"}, HasFn: true},
|
||||
},
|
||||
{
|
||||
name: "raw answer becomes the text when nothing was parsed",
|
||||
@@ -157,36 +159,34 @@ func TestAnswerFillsOnlyMissingSlots(t *testing.T) {
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
q := &PendingQuestion{Slots: tc.parked, Missing: tc.missing, Asked: base}
|
||||
got := q.Answer(tc.text, tc.answer)
|
||||
if got.Time != tc.want.Time || got.HasTime != tc.want.HasTime ||
|
||||
got.Key != tc.want.Key || got.HasKey != tc.want.HasKey ||
|
||||
got.Value != tc.want.Value || got.Text != tc.want.Text ||
|
||||
got.Fn != tc.want.Fn || got.HasFn != tc.want.HasFn {
|
||||
// Whole-struct compare: a new field in Slots is covered for free.
|
||||
if got := q.Answer(tc.text, tc.answer); !reflect.DeepEqual(got, tc.want) {
|
||||
t.Fatalf("Answer = %+v, want %+v", got, tc.want)
|
||||
}
|
||||
if len(got.Args) != len(tc.want.Args) {
|
||||
t.Fatalf("Args = %v, want %v", got.Args, tc.want.Args)
|
||||
}
|
||||
for i := range got.Args {
|
||||
if got.Args[i] != tc.want.Args[i] {
|
||||
t.Fatalf("Args = %v, want %v", got.Args, tc.want.Args)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestCanAskCapsAtOneQuestion(t *testing.T) {
|
||||
if MaxAttempts != 1 {
|
||||
t.Fatalf("MaxAttempts = %d, want 1 (Maven asks once, she is not a nag)", MaxAttempts)
|
||||
func TestCanAskAllowsThreeQuestionsByDefault(t *testing.T) {
|
||||
if DefaultMaxAttempts != 3 {
|
||||
t.Fatalf("DefaultMaxAttempts = %d, want 3", DefaultMaxAttempts)
|
||||
}
|
||||
q := &PendingQuestion{Asked: base}
|
||||
if !q.CanAsk() {
|
||||
t.Fatal("a fresh question should be askable")
|
||||
q := &PendingQuestion{Asked: base} // MaxAttempts unset ⇒ the default
|
||||
for i := 0; i < 3; i++ {
|
||||
if !q.CanAsk() {
|
||||
t.Fatalf("question %d should be allowed", i+1)
|
||||
}
|
||||
q.Attempts++
|
||||
}
|
||||
q.Attempts = MaxAttempts
|
||||
if q.CanAsk() {
|
||||
t.Fatal("the question should not be asked twice")
|
||||
t.Fatal("a fourth question must not be allowed")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCanAskHonoursConfiguredMax(t *testing.T) {
|
||||
q := &PendingQuestion{Asked: base, MaxAttempts: 1, Attempts: 1}
|
||||
if q.CanAsk() {
|
||||
t.Fatal("MaxAttempts 1 means one question only")
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user