Give LLM-routed decisions a real confidence so clarify can fire (#359)

Confidence was hardcoded to 1.0 for every LLM decision, and the LLM branch in Router.Route returned straight from fillSlots without ever touching the stage-3 threshold gate — so the LLM path could not produce a Clarify no matter what confidence a model reported. That is why all 6 want_clarify cases in the 77-case RU fixture were missed by every model in the bake-off. Fix reads structural signal instead of changing the (parity-locked) router prompt: a single-token utterance ("вода", "бэкап") is flagged thin evidence in llmrouter.go; a fact left keyless or an act that never resolves to an allowlisted fn, checked after fillSlots so the deterministic parsers get first crack, is flagged in router.go's new gateLLMDecision. Anything below config.DefaultRouterThreshold (0.55) now sets Clarify=true through the same path the classifier already uses. Added unit tests with a stubbed Completer proving both directions: thin cases clarify, clean multi-word/resolved-slot cases stay confident. The 77-case fixture re-run against a live llama-server is still needed to confirm the 6/6 moves — not done here, no llama-server on this box. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ
2026-07-31 23:07:32 +04:00
parent 73d13f1ea6
commit f0f7ebc9b2
4 changed files with 174 additions and 3 deletions
@@ -118,6 +118,39 @@ const routeRepeatPenalty = 1.15
 // Route return ok=false so the caller drops to the classifier cascade.
 const routeIntentUnknown = "unknown"

+// llmFullConfidence / llmThinConfidence — Vikunja #359. Confidence used to be
+// hardcoded to 1.0 for every LLM decision, so the stage-3 gate in router.go
+// never had anything to bite on and the LLM path could never produce a
+// Clarify: on the 77-case RU fixture, 6/6 want_clarify cases were missed by
+// EVERY model in the 31-07-2026 bake-off (0.8B through 2B) — proof this was a
+// code bug, not a capability ceiling.
+//
+// The fix does not touch the prompt (routeSystem is under
+// llm/check_prompt_parity.py in the training workspace; changing its text
+// creates a parity break that has to be fixed there too — see Vikunja #362).
+// Instead it reads structural signal that is already free:
+//   - a single-token utterance is thin evidence for anything a grammar
+//     didn't already catch at stage 0 — "вода" and "бэкап" alone don't say
+//     fact-vs-query or act-vs-report;
+//   - a fact with no key, or an act that never resolves to an allowlisted fn
+//     (checked in router.go, after slot-fill has had its say), is a decision
+//     with a hole in the one slot that makes it actionable.
+//
+// A model self-reporting confidence in the JSON was considered and rejected:
+// a sub-2B is not calibrated (nothing stops it saying "confident" on exactly
+// the cases it gets wrong today), and true logprobs would need a response
+// field internal/llm.Client's Complete does not currently return — see
+// internal/llm/client.go.
+//
+// llmThinConfidence sits below config.DefaultRouterThreshold (0.55) so the
+// existing stage-3 gate in Router.Route treats it exactly like a low-scoring
+// classifier result — same lane, same daemon-side clarify machinery
+// (cmd/mavend/clarify.go), no new consumer to build.
+const (
+	llmFullConfidence = 1.0
+	llmThinConfidence = 0.3
+)
+
 type routeAction struct {
 	Intent string `json:"intent"`
 	Key    string `json:"key"`
@@ -160,7 +193,15 @@ func (lr *LLMRouter) Route(ctx context.Context, utterance string, now time.Time)
 	if a.Intent == routeIntentUnknown {
 		return Decision{}, false, nil
 	}
-	d := Decision{Utterance: utterance, Stage: 1, Confidence: 1.0}
+	d := Decision{Utterance: utterance, Stage: 1, Confidence: llmFullConfidence}
+	// A single-token utterance is thin evidence: the model had nothing to
+	// disambiguate on ("вода" is a fact-or-query coin flip, "бэкап" an
+	// act-or-report one) and stage 0 would already have won on anything
+	// that pattern-matches cleanly. Flag it now; router.go's stage-3 gate
+	// (Router.Route) decides whether that trips Clarify.
+	if len(strings.Fields(utterance)) <= 1 {
+		d.Confidence = llmThinConfidence
+	}
 	switch Intent(a.Intent) {
 	case IntentFact:
 		d.Intent = IntentFact
@@ -258,4 +258,101 @@ func TestLLMFactGetsKeyFromParser(t *testing.T) {
 	if !d.Slots.HasKey || d.Slots.Key != "water" {
 		t.Fatalf("want key=water, got %+v", d.Slots)
 	}
+	if d.Clarify {
+		t.Fatalf("the parser resolved the key, this must not clarify: %+v", d)
+	}
+}
+
+// --- confidence / stage-3 gate on the LLM path (Vikunja #359) -----------------
+
+// A single-token utterance is thin evidence on its own — "вода" alone is a
+// fact/query coin flip. The gate must ask rather than guess confidently.
+func TestLLMRouterSingleTokenTripsClarify(t *testing.T) {
+	r := newLLMTestRouter(t, `{"intent":"query","text":"вода"}`)
+	d, err := r.Route(context.Background(), "вода", refNow())
+	if err != nil {
+		t.Fatalf("route: %v", err)
+	}
+	if !d.Clarify {
+		t.Fatalf("a bare single-token decision must clarify, got %+v", d)
+	}
+}
+
+// A multi-word utterance with a clean answer must not be punished — the
+// whole point is not trading the confident cases away for clarify coverage.
+func TestLLMRouterMultiWordStaysConfident(t *testing.T) {
+	r := newLLMTestRouter(t, `{"intent":"reminder","text":"позвонить маме"}`)
+	d, err := r.Route(context.Background(), "напомни позвонить маме", refNow())
+	if err != nil {
+		t.Fatalf("route: %v", err)
+	}
+	if d.Clarify {
+		t.Fatalf("a clean multi-word decision must not clarify: %+v", d)
+	}
+	if d.Confidence != llmFullConfidence {
+		t.Fatalf("want full confidence, got %v", d.Confidence)
+	}
+}
+
+// "бэкап" alone: the model guesses act, but nothing on the allowlist matches
+// "бэкап" as a verb — that must not fire a tool blind.
+func TestLLMRouterActWithoutFnTripsClarify(t *testing.T) {
+	r := newLLMTestRouter(t, `{"intent":"act","verb":"бэкап"}`)
+	d, err := r.Route(context.Background(), "бэкап сделай пожалуйста расписание", refNow())
+	if err != nil {
+		t.Fatalf("route: %v", err)
+	}
+	if d.Slots.HasFn {
+		t.Fatalf("test setup drifted: %q now resolves to an fn", d.Slots.Fn)
+	}
+	if !d.Clarify {
+		t.Fatalf("an unresolved act must clarify rather than guess: %+v", d)
+	}
+}
+
+// An act that DOES resolve to an allowlisted fn must stay confident even
+// though its own verb is single-word-ish in spirit — guard against the fn
+// check firing on the happy path.
+func TestLLMRouterActWithFnStaysConfident(t *testing.T) {
+	r := newLLMTestRouter(t, `{"intent":"act","verb":"restart nginx"}`)
+	d, err := r.Route(context.Background(), "слушай, restart nginx пожалуйста", refNow())
+	if err != nil {
+		t.Fatalf("route: %v", err)
+	}
+	if !d.Slots.HasFn {
+		t.Fatalf("test setup drifted, want fn resolved: %+v", d.Slots)
+	}
+	if d.Clarify {
+		t.Fatalf("a resolved act must not clarify: %+v", d)
+	}
+}
+
+// A fact where NEITHER the model NOR the deterministic parser can name a key
+// must clarify instead of silently writing under an empty/guessed key.
+func TestLLMRouterFactWithoutKeyTripsClarify(t *testing.T) {
+	r := newLLMTestRouter(t, `{"intent":"fact","value":"что-то"}`)
+	d, err := r.Route(context.Background(), "у меня какая-то фигня случилась вот прямо только что", refNow())
+	if err != nil {
+		t.Fatalf("route: %v", err)
+	}
+	if d.Slots.HasKey {
+		t.Fatalf("test setup drifted: parser now resolves a key for this utterance")
+	}
+	if !d.Clarify {
+		t.Fatalf("a keyless fact must clarify rather than guess: %+v", d)
+	}
+}
+
+// The whole point of #359: the classifier cascade cannot be traded away for
+// clarify coverage. A multi-word fact the parser CAN key must stay confident
+// through the full Router.Route path, not just the raw LLMRouter.
+func TestRouterLLMFactWithResolvedKeyStaysConfident(t *testing.T) {
+	r := newLLMTestRouter(t, `{"intent":"fact","text":"я выпил воду"}`)
+	d, err := r.Route(context.Background(), "я выпил воду", refNow())
+	if err != nil {
+		t.Fatalf("route: %v", err)
+	}
+	if d.Clarify {
+		t.Fatalf("a fact the parser could key must not clarify: %+v", d)
+	}
 }
@@ -89,6 +89,7 @@ func (r *Router) Route(ctx context.Context, utterance string, now time.Time) (De
 		if d, ok, err := r.llm.Route(ctx, utterance, now); err == nil && ok {
 			d.Utterance = utterance
 			r.fillSlots(ctx, &d, now)
+			r.gateLLMDecision(&d)
 			return d, nil
 		} else if err != nil {
 			log.Printf("router: llm route fell back to classifier: %v", err)
@@ -152,6 +153,35 @@ func (r *Router) fillSlots(ctx context.Context, d *Decision, now time.Time) {
 	// Stage stays 1: it says who decided the route, and that was the LLM.
 }

+// gateLLMDecision — stage 3 for the LLM path (Vikunja #359). This used to be
+// the classifier's job alone (see the threshold check at the bottom of
+// Route): the LLM branch returned straight from fillSlots and never touched
+// r.threshold at all, so a hardcoded Confidence: 1.0 in llmrouter.go could
+// never gate. Two more structural holes are checked here, after fillSlots
+// has had a chance to fill them from the deterministic parsers — checking
+// before fillSlots would flag e.g. every keyless fact the fact parser goes
+// on to resolve (TestLLMFactGetsKeyFromParser):
+//   - a fact with no key even after the parser tried — nothing to write, or
+//     worse, a confident write under the wrong key;
+//   - an act that never resolved to an allowlisted fn — a confident guess
+//     here means either silently doing nothing or, if the daemon is lax,
+//     running something never on the allowlist. Don't guess; ask.
+//
+// Anything below threshold gets the exact same Clarify=true treatment the
+// classifier path already produces — same field, same daemon-side consumer
+// (cmd/mavend/clarify.go), nothing new to wire.
+func (r *Router) gateLLMDecision(d *Decision) {
+	if d.Intent == IntentFact && !d.Slots.HasKey && d.Confidence > llmThinConfidence {
+		d.Confidence = llmThinConfidence
+	}
+	if d.Intent == IntentAct && !d.Slots.HasFn && d.Confidence > llmThinConfidence {
+		d.Confidence = llmThinConfidence
+	}
+	if d.Confidence < r.threshold {
+		d.Clarify = true
+	}
+}
+
 // CorrectMisroute — the user corrected a bad classification. Appends a new
 // example for the corrected intent (append-only — grows the classifier, no
 // retrain). Same shape as nudges.outcome tuning cooldowns: more reliable over