diff --git a/internal/dialogue/clarify.go b/internal/dialogue/clarify.go index 98850c2..4f5408a 100644 --- a/internal/dialogue/clarify.go +++ b/internal/dialogue/clarify.go @@ -38,21 +38,34 @@ type PendingQuestion struct { MaxAttempts int } -// maxAttempts is MaxAttempts with the default filled in. -func (q *PendingQuestion) maxAttempts() int { - if q.MaxAttempts <= 0 { - return DefaultMaxAttempts +// Action reads the parked question as the typed action it is assembling +// (pending.go). Derived rather than stored: the question's fields stay the one +// copy of the truth, so a caller that fills them the old way cannot end up with +// a capability that disagrees with the intent. +func (q *PendingQuestion) Action() PendingAction { + return PendingAction{ + Capability: CapabilityFor(q.Intent), + Slots: q.Slots, + Missing: q.Missing, + Utterance: q.Utterance, + Asked: q.Asked, + TTL: q.TTL, + Attempts: q.Attempts, + MaxAttempts: q.MaxAttempts, } - return q.MaxAttempts } +// IsExpired and CanAsk answer through the action, so there is exactly one copy +// of the TTL and attempt-cap rules and the widening cannot drift from them. func (q *PendingQuestion) IsExpired(now time.Time) bool { - return now.After(q.Asked.Add(q.TTL)) + a := q.Action() + return a.IsExpired(now) } // CanAsk reports whether Maven may ask another question about this request. func (q *PendingQuestion) CanAsk() bool { - return q.Attempts < q.maxAttempts() + a := q.Action() + return a.CanAsk() } // ClarifyStore holds the parked questions. Same shape and locking as @@ -66,11 +79,26 @@ func (q *PendingQuestion) CanAsk() bool { // next words route fresh, which is the right answer with or without a notice. // Do not give this store a persister without re-arguing that. type ClarifyStore struct { - mu sync.RWMutex - questions map[string]*PendingQuestion + mu sync.RWMutex + // stacks — one stack of parked questions per dialogue id, newest last. It + // was a single question per id until V-559; a side query has to be able to + // suspend the active flow and find it still there afterwards (V-561 does + // the suspending, this only holds the room for it). + stacks map[string][]*PendingQuestion defaultTTL time.Duration } +// MaxStackDepth — how many parked questions one dialogue id may hold. +// +// Two, not three. One is the flow he is in, one is the thing he interrupted it +// with, and out loud he does not nest deeper than that: a side query inside a +// side query is a shape typed conversation has and spoken conversation does +// not. The bound is also a promise — every level she keeps is a level she must +// be able to SPEAK when it dies (clarifyGaveUp, clarifyExpiredVariants), and +// two lines of "and the other thing I dropped" is already the limit of what a +// reply can carry. +const MaxStackDepth = 2 + func NewClarifyStore(defaultTTL time.Duration) *ClarifyStore { if defaultTTL <= 0 { // Short, like confirmTTL in voice.go: a clarifying question is a @@ -78,27 +106,58 @@ func NewClarifyStore(defaultTTL time.Duration) *ClarifyStore { defaultTTL = 90 * time.Second } return &ClarifyStore{ - questions: make(map[string]*PendingQuestion), + stacks: make(map[string][]*PendingQuestion), defaultTTL: defaultTTL, } } -// Put parks a question. Called on a clarify decision (cmd/mavend/clarify.go). +// Put parks a question, replacing the one on top. Called on a clarify decision +// (cmd/mavend/clarify.go), and it is still what the daemon uses: re-asking the +// same request is a new question about the SAME action, so it overwrites rather +// than growing the stack. Push is the deeper one, and nothing calls it yet. func (s *ClarifyStore) Put(id string, q *PendingQuestion) { - if q.TTL <= 0 { - q.TTL = s.defaultTTL - } + s.fillTTL(q) s.mu.Lock() - s.questions[id] = q - s.mu.Unlock() + defer s.mu.Unlock() + stack := s.stacks[id] + if len(stack) == 0 { + s.stacks[id] = []*PendingQuestion{q} + return + } + stack[len(stack)-1] = q } -// Get returns the live parked question, or nil when there is none. -func (s *ClarifyStore) Get(id string, now time.Time) *PendingQuestion { +// Push suspends whatever is parked and puts q on top. The returned question is +// one the depth bound forced out of the bottom of the stack, and the caller MUST +// tell him about it — a parked request that dies without a word leaves him +// thinking it landed, which is the whole reason clarifyGaveUp exists. nil is the +// ordinary case. +func (s *ClarifyStore) Push(id string, q *PendingQuestion) *PendingQuestion { + s.fillTTL(q) + s.mu.Lock() + defer s.mu.Unlock() + stack := append(s.stacks[id], q) + var dropped *PendingQuestion + if len(stack) > MaxStackDepth { + dropped = stack[0] + stack = stack[1:] + } + s.stacks[id] = stack + return dropped +} + +// Peek returns the live question on top, or nil when there is none. Expired +// entries below it are left alone: TakeExpired is what reports those, and +// dropping one here would be the silent death this store is careful about. +func (s *ClarifyStore) Peek(id string, now time.Time) *PendingQuestion { s.mu.RLock() - q, ok := s.questions[id] + stack := s.stacks[id] + var q *PendingQuestion + if len(stack) > 0 { + q = stack[len(stack)-1] + } s.mu.RUnlock() - if !ok { + if q == nil { return nil } if q.IsExpired(now) { @@ -108,27 +167,81 @@ func (s *ClarifyStore) Get(id string, now time.Time) *PendingQuestion { return q } +// Get is Peek under the name every caller already uses. Kept because a clarify +// answer is always about the top of the stack, so the two are the same call. +func (s *ClarifyStore) Get(id string, now time.Time) *PendingQuestion { + return s.Peek(id, now) +} + +// Pop takes the live question off the top and returns it, so the flow beneath +// becomes current again. nil when the top is empty or expired — an expired top +// is dropped along with the rest of the stack, exactly as Peek does, because the +// clock that killed it has been running for everything underneath too. +func (s *ClarifyStore) Pop(id string, now time.Time) *PendingQuestion { + s.mu.Lock() + stack := s.stacks[id] + if len(stack) == 0 { + s.mu.Unlock() + return nil + } + q := stack[len(stack)-1] + if q.IsExpired(now) { + delete(s.stacks, id) + s.mu.Unlock() + return nil + } + if len(stack) == 1 { + delete(s.stacks, id) + } else { + s.stacks[id] = stack[:len(stack)-1] + } + s.mu.Unlock() + return q +} + +// Depth — how many questions are parked for this id, expired ones included. +// Diagnostic; the arbiter in V-560 reads it to know it is inside a flow. +func (s *ClarifyStore) Depth(id string) int { + s.mu.RLock() + defer s.mu.RUnlock() + return len(s.stacks[id]) +} + // 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. +// +// It looks at the top only, and drops the whole stack when that one is dead: one +// notice is what a reply can carry, and anything parked under a question that +// timed out has been waiting at least as long. 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) { + stack := s.stacks[id] + if len(stack) == 0 || !stack[len(stack)-1].IsExpired(now) { return false } - delete(s.questions, id) + delete(s.stacks, id) return true } +// Delete drops every question parked for this id. The old single-slot Delete +// under the old name: at depth one the two are the same, and a caller that means +// "this exchange is over" means all of it. func (s *ClarifyStore) Delete(id string) { s.mu.Lock() - delete(s.questions, id) + delete(s.stacks, id) s.mu.Unlock() } +// fillTTL applies the store default to a question parked without one. +func (s *ClarifyStore) fillTTL(q *PendingQuestion) { + if q.TTL <= 0 { + q.TTL = s.defaultTTL + } +} + // Answer merges the slots parsed from the user's answer into the parked ones. // 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 «нет, в пять»