Merge the thinking-off measurement

ROUTING-EVAL-31-07-2026.md

@@ -66,15 +66,64 @@ Three things this run settles:
    `запиши что…` phrasings toward fact, and that suspicion stands — all five `ru-note-*`
    cases now land on fact. Tracked as Vikunja #375.
 
-**Thinking off is the best configuration measured so far**, on both accuracy and latency
+The `thinking off` column above read as the best configuration measured so far (Vikunja #376).
-(Vikunja #376). That is worth understanding before flipping: routing is a short
+**It was wrong** — see the controlled re-run below. Ignore that column.
-classification into a fixed enum with grammar-constrained output, so there is little to
-reason about, and the thinking trace mostly gives a small model room to talk itself out of
-the right answer. Phrasing is a different job and needs measuring separately.
 
 Still `6 / 6` missed clarify — the router has no way to say "I don't know" (Vikunja #359).
 That is unchanged by anything here.
 
+## Thinking off — 31-07-2026, controlled re-run (Vikunja #376)
+
+The "thinking off wins by 6 points" observation above **does not hold**. It was a measurement
+artefact, and the earlier table's `thinking off` column should be ignored.
+
+The thinking-off variant was scored by a hand-rolled HTTP client living in the test file
+instead of `llm.Client`. That copy did not send `repeat_penalty`, which the real router does
+send (`routeRepeatPenalty = 1.15`). So the two columns differed on two axes at once, and the
+one that mattered was the penalty, not the thinking mode.
+
+Re-measured with everything else held equal — same fixture, same prompt, same grammar, same
+sampling, same idle box, the three configurations run back to back and never concurrently:
+
+| | llm-only, thinking on | llm-only, thinking off | cascade+llm |
+|---|---|---|---|
+| intent-only accuracy | 59.2% (45/76) | 59.2% (45/76) | 61.8% (47/76) |
+| full accuracy (intent+slots+gate) | 38.2% (29/76) | 38.2% (29/76) | 57.9% (44/76) |
+| route errors | 3 | 3 | 0 |
+| grammar violations | 3 (all 3 route errors) | 3 (same 3 cases) | 0 |
+| missed clarify | 5 / 6 | 5 / 6 | 5 / 6 |
+| p50 latency | 836ms | 920ms | 810ms |
+| p95 latency | 1.41s | 2.00s | 1.31s |
+
+Thinking off is not just a tie on the headline numbers — it is identical case for case, with
+the same confusion matrix and the same three unparseable replies. The latency difference is
+run-to-run noise on one box, and it points the wrong way here.
+
+The reason is simpler than any accuracy argument: **this llama-server build ignores the
+request-level thinking switch for this model.** Probed directly against the running server
+with `chat_template_kwargs.enable_thinking = false`, `chat_template_kwargs.thinking = false`
+and top-level `reasoning_budget = 0` — all three return a byte-identical answer with the
+thinking trace still in `reasoning_content`, and the server reports the prompt prefix as
+cached, meaning the rendered template did not change. There was never anything being turned
+off, which is also why the numbers match exactly.
+
+Nothing was defaulted. `internal/llm` still has no `chat_template_kwargs` field, `VoiceConfig`
+has no thinking flag, and `deploy/mavend.json` is unchanged. The misleading third
+configuration is removed from `internal/router/eval` so the table it produced cannot be quoted
+again.
+
+Two caveats worth saying out loud:
+
+- **The fixture is 76 cases.** A 6-point difference on 76 cases is roughly 4-5 cases and would
+  not have been worth trusting even if it had reproduced. This one was exactly 0 cases, which
+  is a much easier call.
+- **This is one server build and one checkpoint** (`b9351`, Qwen3.5-0.8B Q4_K_M). If the
+  #122 checkpoint or a newer llama.cpp does honour the switch, the question reopens — but it
+  reopens as an unmeasured question, not as a 6-point win.
+
+Phrasing was **not** measured. Whether thinking helps there is still open, and now also blocked
+on the same "can we even turn it off" question.
+
 ## Findings
 
 ### 1. The resident model does route better — 50.0% vs 36.8%
@@ -145,11 +194,11 @@ Note the grammar's `string ::= "\"" ([^"\\] | "\\" .)* "\""` is unbounded, so no
 
 ### 7. Two hypotheses tested and closed
 
-- **Thinking mode is a non-issue.** Qwen3.5's template defaults `thinking = 1`, so
+- **Thinking mode is a non-issue.** Confirmed twice now, the second time properly — see the
-  grammar-constrained JSON lands in `reasoning_content` with `content` empty —
+  controlled re-run section. Grammar-constrained JSON lands in `reasoning_content` with
-  `llm.Client`'s fallback handles it. A `thinking off` run scored *identically* (18/76,
+  `content` empty and `llm.Client`'s fallback handles it; the request-level switch does
-  48.7%, same p50). `internal/llm` deliberately does **not** grow a `chat_template_kwargs`
+  nothing on this build. `internal/llm` deliberately does **not** grow a
-  field.
+  `chat_template_kwargs` field.
 - **Runaway array repetition does not reproduce.** An isolated smoke test with a stripped
   grammar emitted `{"intent":"reminder"}` until `MaxTokens`; under the real `routeSystem`
   prompt the few-shot examples anchor it to one object. 2 errors in 76, not 76.

internal/router/eval/llmrouter_test.go

@@ -1,11 +1,8 @@
 package eval
 
 import (
-	"bytes"
 	"context"
-	"encoding/json"
 	"fmt"
-	"net/http"
 	"os"
 	"strings"
 	"testing"
@@ -29,13 +26,20 @@ import (
 // a bake-off across checkpoints (#278, #250) produces tables you can tell
 // apart. Point the variable at one server at a time.
 //
-// Three configurations, because "the LLM router" is ambiguous and the three
+// Two configurations, because "the LLM router" is ambiguous and the two numbers
-// numbers answer different questions:
+// answer different questions:
 //
-//	llm-only        — the model alone. Measures the prompt + grammar contract.
+//	llm-only    — the model alone. Measures the prompt + grammar contract.
-//	cascade+llm     — what #320 would actually ship: stage-0 grammar, then the
+//	cascade+llm — what #320 would actually ship: stage-0 grammar, then the
-//	                  model, then the classifier as the failure floor.
+//	              model, then the classifier as the failure floor.
-//	llm-no-thinking — diagnostic only, not a shippable path (see below).
+//
+// There used to be a third, "thinking off", which looked 6 points better. It is
+// gone: it was measured with a hand-rolled HTTP client that quietly dropped
+// repeat_penalty, so the gap was the missing penalty and not the thinking mode.
+// Re-measured with everything else held equal, thinking off scores exactly the
+// same, case for case — and a direct probe shows this llama-server build ignores
+// enable_thinking / reasoning_budget for this model anyway, so there was nothing
+// to turn off. Full write-up in ROUTING-EVAL-31-07-2026.md (Vikunja #376).
 func TestLLMRouterBaseline(t *testing.T) {
 	base := os.Getenv("MAVEN_LLM_URL")
 	if base == "" {
@@ -96,104 +100,18 @@ func TestLLMRouterBaseline(t *testing.T) {
 	}
 	t.Log("\n" + repCascade.String() + repCascade.Failures())
 
-	// llm-no-thinking: same prompt and grammar with the chat template's
-	// thinking mode off. Qwen3.5's template defaults thinking=1, so under a
-	// grammar the constrained JSON lands in reasoning_content with content
-	// empty — llm.Client's ReasoningContent fallback is what makes the router
-	// work at all today, by accident rather than design.
-	//
-	// MEASURED 2026-07-31: this variant scores identically to as-deployed
-	// (18/76, 48.7% intent-only, 2 errors, same p50). Thinking mode is a
-	// non-issue under a grammar — llama.cpp constrains the same token stream
-	// either way. Kept so the question stays answered instead of being
-	// re-asked, and so internal/llm does NOT grow a chat_template_kwargs field
-	// for a problem that does not exist.
-	repNoThink, err := Score(ctx, "llm-only ("+model+", thinking off) [diagnostic]",
-		RouterFunc(func(ctx context.Context, u string, now time.Time) (router.Decision, error) {
-			d, ok, err := router.NewLLMRouter(&noThinkCompleter{base: base, http: &http.Client{Timeout: 60 * time.Second}}).Route(ctx, u, now)
-			if err != nil {
-				return d, err
-			}
-			if !ok {
-				return d, fmt.Errorf("llm router declined without an error")
-			}
-			return d, nil
-		}), f)
-	if err != nil {
-		t.Fatalf("Score no-thinking: %v", err)
-	}
-	t.Log("\n" + repNoThink.String() + repNoThink.Failures())
-
 	// Reports rather than asserts — the numbers are inputs to the #320
 	// decision, and an assertion here would be this test inventing the bar.
 	// The one thing worth failing on is a harness fault: if every single case
 	// errors, the run measured infrastructure, not routing, and the report
 	// must not be mistaken for a score.
-	for _, rep := range []Report{repLLM, repCascade, repNoThink} {
+	for _, rep := range []Report{repLLM, repCascade} {
 		if rep.Errors == rep.Total {
 			t.Errorf("%s: all %d cases errored — harness fault, not a measurement", rep.Name, rep.Total)
 		}
 	}
 }
 
-// noThinkCompleter — llm.Client with chat_template_kwargs.enable_thinking
-// false. A test-local copy rather than a change to internal/llm: whether the
-// daemon should send it is the open question, and answering it here by adding
-// the field would prejudge #320.
-type noThinkCompleter struct {
-	base string
-	http *http.Client
-}
-
-func (c *noThinkCompleter) Complete(ctx context.Context, r llm.Req) (string, error) {
-	payload := map[string]any{
-		"messages": []map[string]string{
-			{"role": "system", "content": r.System},
-			{"role": "user", "content": r.User},
-		},
-		"max_tokens":           r.MaxTokens,
-		"temperature":          0,
-		"grammar":              r.Grammar,
-		"chat_template_kwargs": map[string]any{"enable_thinking": false},
-	}
-	b, err := json.Marshal(payload)
-	if err != nil {
-		return "", err
-	}
-	req, err := http.NewRequestWithContext(ctx, "POST", c.base+"/v1/chat/completions", bytes.NewReader(b))
-	if err != nil {
-		return "", err
-	}
-	req.Header.Set("Content-Type", "application/json")
-	resp, err := c.http.Do(req)
-	if err != nil {
-		return "", err
-	}
-	defer resp.Body.Close()
-	if resp.StatusCode != 200 {
-		return "", fmt.Errorf("status %d", resp.StatusCode)
-	}
-	var out struct {
-		Choices []struct {
-			Message struct {
-				Content          string `json:"content"`
-				ReasoningContent string `json:"reasoning_content"`
-			} `json:"message"`
-		} `json:"choices"`
-	}
-	if err := json.NewDecoder(resp.Body).Decode(&out); err != nil {
-		return "", err
-	}
-	if len(out.Choices) == 0 {
-		return "", fmt.Errorf("no choices")
-	}
-	m := out.Choices[0].Message
-	if m.Content != "" {
-		return m.Content, nil
-	}
-	return m.ReasoningContent, nil
-}
-
 func ping(ctx context.Context, c *llm.Client) error {
 	ctx, cancel := context.WithTimeout(ctx, 90*time.Second)
 	defer cancel()