diff --git a/internal/router/router.go b/internal/router/router.go index 68457e3..f603ea9 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -80,6 +80,9 @@ func (r *Router) Route(ctx context.Context, utterance string, now time.Time) (De continue // grammar matched shape but not content → fall through } d.Utterance = utterance + // The grammar decided the intent; the extractor fills the slots it did + // not match (V-572). See fillMatchedSlots for why every grammar gets it. + r.fillMatchedSlots(ctx, &d, now) return d, nil } @@ -121,14 +124,37 @@ func (r *Router) Route(ctx context.Context, utterance string, now time.Time) (De return d, nil } -// fillSlots — run stage-2 extraction on an LLM decision and fill only the slots -// the model left empty. The LLM wins where it answered: it saw the sentence, the -// parsers are keyword tables. Extraction covers what the model cannot produce at -// all — a parsed reminder time and an allowlist fn. +// fillMatchedSlots — run stage-2 extraction over a decision some earlier +// claimant produced, and fill only the slots that claimant left empty. A +// matched value always wins: the claimant read the sentence, the extractor +// guesses from keyword tables. +// +// Shared by the stage-0 grammars and the LLM router, which had the same hole +// for the same reason. A grammar asserts an intent at confidence 1.0 and says +// nothing about the slots, so "напомни в 11:00 позвонить маме" arrived with +// HasTime false however plainly the hour was spoken, and the daemon read the +// silence as absence and asked "Когда?" (V-572). The alternative was ten +// grammars each re-implementing extraction. +// +// It is applied to every stage-0 decision rather than to a chosen few, because +// for every intent but reminder it is inert: Extract fills Time for a reminder, +// Fn for an act and Key for a fact, and nothing at all for query, system, note +// or chat, which is what the query, clock, agenda, feed, list, task and +// narrative rules emit. The act rules — wakeword-act and the Praxis ones — +// already carry an Fn or they do not match, so there is nothing left for the +// matcher to fill. The reminder rule is the one that gains, and its time parse +// is a cost the daemon was already paying one layer down in actionReminder. +// +// Slots.Text is deliberately NOT filled here. Extract sets it to the raw +// utterance, and a grammar that left it empty meant it: agendaQueryBuild hands +// the query chain the utterance itself, and narrativeQueryBuild's Text is the +// topic, not the sentence. // // If a reminder still has no time, leave it missing. The daemon then says it // could not read the time; inventing one would set a wrong alarm. -func (r *Router) fillSlots(ctx context.Context, d *Decision, now time.Time) { +// Returns what the extractor read, so a caller that wants more of it does not +// pay for a second extraction — the reminder parser is the expensive one. +func (r *Router) fillMatchedSlots(ctx context.Context, d *Decision, now time.Time) Slots { ex := r.extractor.Extract(ctx, d.Intent, d.Utterance, now) if !d.Slots.HasTime && ex.HasTime { d.Slots.Time, d.Slots.HasTime = ex.Time, ex.HasTime @@ -139,6 +165,14 @@ func (r *Router) fillSlots(ctx context.Context, d *Decision, now time.Time) { if !d.Slots.HasFn && ex.HasFn { d.Slots.Fn, d.Slots.Args, d.Slots.HasFn = ex.Fn, ex.Args, ex.HasFn } + return ex +} + +// fillSlots — fillMatchedSlots for an LLM decision, plus the two backfills that +// only make sense there. The LLM wins where it answered: it saw the sentence, +// the parsers are keyword tables. +func (r *Router) fillSlots(ctx context.Context, d *Decision, now time.Time) { + ex := r.fillMatchedSlots(ctx, d, now) // For an act the model returns the verb in Text ("restart nginx"), which is // often cleaner than the raw utterance ("maven, could you restart nginx"). // Try it too when the utterance did not match the allowlist. diff --git a/internal/router/router_test.go b/internal/router/router_test.go index e8970fc..9c5a285 100644 --- a/internal/router/router_test.go +++ b/internal/router/router_test.go @@ -121,6 +121,73 @@ func TestStage0GrammarFiresThroughCyrillicWakeWord(t *testing.T) { } } +// TestStage0ReminderCarriesTheHourHeSaid — "напомни в 11:00 позвонить маме" is +// the commonest reminder there is, and it used to reach the daemon with HasTime +// false, because ReminderGrammar builds its slots by hand and the router ran no +// extraction over a stage-0 decision. The daemon read the silence as absence and +// asked "Когда?" about an hour he had just said (V-572). +func TestStage0ReminderCarriesTheHourHeSaid(t *testing.T) { + r := newTestRouter(t, 0.0) + r.grammars = append(r.grammars, ReminderGrammar()) + + d, err := r.Route(context.Background(), "напомни в 11:00 позвонить маме", refNow()) + if err != nil { + t.Fatalf("route: %v", err) + } + if d.Stage != 0 || d.Intent != IntentReminder { + t.Fatalf("want stage0 reminder, got %+v", d) + } + if !d.Slots.HasTime { + t.Fatalf("the hour was spoken, so the slot must be filled: %+v", d.Slots) + } + if got, want := d.Slots.Time.Format("15:04"), "11:00"; got != want { + t.Errorf("fire time = %s, want %s", got, want) + } + // The subject is the grammar's, not the extractor's: Slots.Text is what she + // says at the hour, and Extract would have overwritten it with the sentence. + if d.Slots.Text != "в 11:00 позвонить маме" { + t.Errorf("Text = %q, want the grammar's capture", d.Slots.Text) + } +} + +// TestStage0MatchedSlotBeatsTheExtractor — a grammar that matched a literal +// pattern outranks a parser that guessed. The wake-word act names its fn from +// the remainder after the wake token; extraction over the raw utterance must not +// be able to replace it. +func TestStage0MatchedSlotBeatsTheExtractor(t *testing.T) { + r := newTestRouter(t, 0.0) + d, err := r.Route(context.Background(), "maven, restart nginx", refNow()) + if err != nil { + t.Fatalf("route: %v", err) + } + if d.Slots.Fn != "restart" || len(d.Slots.Args) != 1 || d.Slots.Args[0] != "nginx" { + t.Fatalf("matched fn was overwritten: %+v", d.Slots) + } + if d.Slots.Text != "restart nginx" { + t.Errorf("Text = %q, want the grammar's remainder", d.Slots.Text) + } +} + +// TestStage0QueryKeepsAnEmptyText — agendaQueryBuild deliberately leaves Text +// empty so the query chain reads the utterance itself. Extraction fills Time, +// Key and Fn and never Text, or every stage-0 query would start carrying the +// whole sentence in a slot that means something narrower. +func TestStage0QueryKeepsAnEmptyText(t *testing.T) { + r := newTestRouter(t, 0.0) + r.grammars = append(r.grammars, AgendaQueryGrammars()...) + + d, err := r.Route(context.Background(), "что у меня сегодня", refNow()) + if err != nil { + t.Fatalf("route: %v", err) + } + if d.Stage != 0 || d.Intent != IntentQuery { + t.Fatalf("want stage0 query, got %+v", d) + } + if d.Slots.Text != "" { + t.Errorf("Text = %q, want it left empty", d.Slots.Text) + } +} + // ----------------------------- stage 1 --------------------------------------- func TestStage1ClassifiesAct(t *testing.T) { diff --git a/internal/router/stage0.go b/internal/router/stage0.go index 82f58ab..fe0cb0a 100644 --- a/internal/router/stage0.go +++ b/internal/router/stage0.go @@ -88,11 +88,12 @@ func DefaultGrammars(actMatcher ActMatcher) []Grammar { // non-reminder time queries toward it, and the verb+action overlap pushes // actual reminders toward fact — a double contamination. Stage 0 fixes both. // -// The grammar captures the part after "напомни"/"remind me" into Slots.Text -// so the daemon's time parser can extract the fire time from it. The grammar -// itself does NOT parse time — that's the extractor's job (stage 2), but -// stage 0 skips the extractor. The daemon's applyAction fallback calls the -// time parser for stage-0 reminders that arrive without HasTime. +// The grammar captures the part after "напомни"/"remind me" into Slots.Text — +// what she says at the hour. The grammar itself does NOT parse time; that is +// the extractor's job, and since V-572 the router runs the extractor over a +// stage-0 decision too (fillMatchedSlots in router.go). Before that it did not, +// so "напомни в 11:00 позвонить маме" reached the daemon with HasTime false and +// was asked "Когда?" about an hour he had just said. func ReminderGrammar() Grammar { return Grammar{ Name: "reminder-wakeword",