diff --git a/CLAUDE.md b/CLAUDE.md index ac91ef0..2efe82c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -115,13 +115,15 @@ classifier. Every stage may decline and the next one answers. - **The classifier is the floor, not dead code.** It answers when the resident model is off, absent, or erroring. **Any model error falls through.** -- **`baselineGrammars` in `eval_test.go` mirrors `buildRouter`.** A grammar - added to one belongs in both, or the fixture scores a set nobody runs. +- **The stage 0 set lives in `router.StageZeroGrammars`**, and both `buildRouter` + and the eval fixture call it. Add a grammar there, in the right place, and read + the comment above the line you insert after. Do not restate the list anywhere. - **Go's `\b` is ASCII-only** and never fires after a Cyrillic letter. A Russian pattern needs an explicit `(\s|[?!.]|$)`. - **`PraxisGrammars()` is the only path to Praxis**, not a faster one. - **`voice.embedder.heads_path` must never point at `model_path`.** Recall depends on the resident e5-small scoring what it scored. Fine-tune a copy. + Refused at config load since V-692, symlinks included. - **Routing traces are retained 14 days**, enforced on write and again on start. - **Bump `tokenizerRev` on any change to what `encodeWord` emits**, so a tokenizer fix triggers `ReembedAll` the way swapping the model file does. diff --git a/cmd/mavend/voicewire.go b/cmd/mavend/voicewire.go index f476128..f3e4aa8 100644 --- a/cmd/mavend/voicewire.go +++ b/cmd/mavend/voicewire.go @@ -469,45 +469,9 @@ func buildRouter(emb router.Embedder, acts router.ActMatcher, threshold float64, llmR *router.LLMRouter, heads *router.RouterHeads) *router.Router { cls := router.NewClassifier(emb) seedClassifier(cls) - grammars := router.DefaultGrammars(acts) - grammars = append(grammars, router.SystemTimeDateGrammars()...) - // After the time/date rules on purpose: "какой сегодня день" is a clock - // question and must keep reaching replySystem, while "что у меня сегодня" - // is an agenda question and must not. - grammars = append(grammars, router.AgendaQueryGrammars()...) - // Same reason as the agenda rules, for the feeds: "что нового в лентах?" - // routed system and answered "пока не умею" (Vikunja #474). - // After the agenda rules, which are the narrower claim, and BEFORE the feed - // and list rules, which are not: "что такое лента" is a definition question - // and the feed rule would take it on the noun alone (V-655). - grammars = append(grammars, router.WorldQueryGrammars()...) - grammars = append(grammars, router.FeedQueryGrammar()) - // The list side of the same exposure: a phrasing with no possessive in it - // ("список дел") routed system and never reached queryTasks (Vikunja #467). - grammars = append(grammars, router.TaskListGrammar()) - grammars = append(grammars, router.ListGrammars()...) - grammars = append(grammars, router.ReminderGrammar()) - // Before the capture marker, because "отметь" is a capture verb and "отметь - // второй пункт" is not a note. The Praxis rules are the narrower claim — a - // lifecycle verb AND an item named — so they get first refusal (Vikunja #516). - grammars = append(grammars, router.PraxisGrammars()...) - // Last, and it matches any utterance shape — its Build is the filter. An - // explicit capture marker beats the model, which called it an act and - // rewrote the task text (Vikunja #467). After the rules above because a - // marker never collides with a clock or agenda question. - // After Praxis, whose bare "закрой" claim this rule cannot reach (it needs the - // board noun), and before the capture marker, which would otherwise read - // "убери из задач купить молоко" as a new task (Vikunja #512). - grammars = append(grammars, router.TaskStatusGrammar()) - // Before the capture markers, which all need an object. A capture verb - // alone is a fact with no key, and the clarify path asks for it rather than - // letting the model invent an answer (Vikunja #557). - grammars = append(grammars, router.BareCaptureGrammar()...) - grammars = append(grammars, router.TaskCaptureGrammar()) - // After the capture marker, so "запиши" still wins over "расскажи", and - // last overall because it matches on the first word alone: "расскажи про - // X" is a world question the model called a fact (Vikunja #498). - grammars = append(grammars, router.NarrativeQueryGrammars()...) + // The stage 0 set, in the router package, so the eval fixture runs the rules + // the daemon runs (V-693). Order and reasoning live with the list. + grammars := router.StageZeroGrammars(acts) return router.New(router.Config{ Grammars: grammars, Classifier: cls, diff --git a/docs/caveats/CLAUDE.md b/docs/caveats/CLAUDE.md index 46f9e49..7f1abe5 100644 --- a/docs/caveats/CLAUDE.md +++ b/docs/caveats/CLAUDE.md @@ -22,10 +22,12 @@ caveat is the pointer between them plus the trigger. Every entry below came from the 2026-08-10 deep audit (`docs/evals/2026-08-10-repo-audit.md`), except the last, which came from wiring -the gate the audit asked for. Three of the twenty findings are fixed and have no +the gate the audit asked for. Five of the twenty findings are fixed and have no entry. The unauthenticated mavgpud proxy was V-673. The 20 reachable advisories in the toolchain and `x/text` were V-682. The missing analyzers were V-694, and -what they now report is the baseline entry under V-701. +what they now report is the baseline entry under V-701. The two unguarded +invariants were V-692 and V-693, and their guards are described in +`docs/routing.md`. | limit | severity | | --- | --- | @@ -41,8 +43,6 @@ what they now report is the baseline entry under V-701. | [mavweb errors cannot be traced](transport.md#errors) | medium | | [Fact enrichment is a 20-call serial waterfall](workers.md#enrichment) | medium | | [A suppressed nudge is phrased anyway](workers.md#nudges) | medium | -| [heads_path may equal model_path](invariants.md#heads) | medium | -| [baselineGrammars is mirrored by hand](invariants.md#grammars) | medium | | [Committed absolute paths pin the build to this box](config.md#paths) | medium | | [The env example omits deployed variables](config.md#secrets) | medium | | [The analyzers pass against a baseline, not zero](dependencies.md#baseline) | medium | diff --git a/docs/caveats/invariants.md b/docs/caveats/invariants.md deleted file mode 100644 index ebab2c6..0000000 --- a/docs/caveats/invariants.md +++ /dev/null @@ -1,27 +0,0 @@ -# Unguarded invariants - -`CLAUDE.md` names these as load-bearing. Nothing enforces either one. A rule -that lives only in prose gets broken by whoever did not read the prose. Both of -these fail silently when broken. - -`tokenizerRev` and `preRouteLadder` were checked and need nothing. The rev is -baked into the embedder key, so a bump triggers re-embedding. A missing ladder -rung is observable in the decision record. - -## heads_path may equal model_path [#692] {#heads} - -Costs: the routing heads then score with the same graph the resident e5-small -uses, and recall degrades. There is no error and no log line, so it reads as -ordinary drift rather than a misconfiguration. -Revisit when: `deploy/mavend.json` is edited by hand, or a fine-tuned heads -graph is swapped in. -Workaround: check the two keys by eye. That is the whole guard today. - -## baselineGrammars is mirrored by hand [#693] {#grammars} - -Costs: the eval fixture restates the stage 0 rule set in the daemon's order, -and its own comment says so. Three test files score against it. A grammar added -to `buildRouter` alone means every routing measurement scores a set nobody -runs. `CLAUDE.md` warns about this failure by name. -Revisit when: the next stage 0 grammar is added. That is when it bites. -Workaround: add to both lists, which is what the rule already says. diff --git a/docs/routing.md b/docs/routing.md index 8745c4e..4cdfdc4 100644 --- a/docs/routing.md +++ b/docs/routing.md @@ -1,6 +1,6 @@ # Routing -*Last verified: 2026-08-09 @ 31b5093* +*Last verified: 2026-08-11 @ 25ed201* How an utterance becomes a `Decision`, why each stage exists, and what every stage has measured. `CLAUDE.md` carries the rules an agent must not break. This @@ -137,10 +137,15 @@ below. Go's `\b` is ASCII-only and never fires after a Cyrillic letter. A pattern needs an explicit `(\s|[?!.]|$)`. -`baselineGrammars` in `eval_test.go` mirrors `buildRouter` and has drifted before. -`WorldQueryGrammars` was wired into the daemon by V-655 and not into the mirror, -so the fixture scored a grammar set nobody runs. Fixed by V-659, worth 3 points -of destination. +The stage 0 set lives in `router.StageZeroGrammars` (`internal/router/stagezero.go`). +Both `buildRouter` and the eval fixture call it. The daemon and the measurement +cannot disagree about which rules exist, or in what order. + +It was two lists until V-693 and it drifted twice. V-655 wired +`WorldQueryGrammars` into the daemon and not into the fixture. That cost 3 points +of destination and V-659 fixed it. `BareCaptureGrammar` then did the same thing, +from V-557 until V-693 found it. That one moved no number, which is the point: +the fixture had been scoring a set nobody ran and nothing said so. ### Praxis lifecycle rules @@ -270,6 +275,13 @@ Three rules around it, each measured: means the heads are nil. The cascade is then byte-for-byte what shipped before them. +Pointing it at `model_path` is refused at config load (V-692). An unloadable +weights file is not fatal, because the heads are an accelerator. A working file +in the wrong role is a different thing. The heads then score with the graph the +resident embedder scored with, and recall degrades with no log line. The check +cleans and absolutises both paths, then compares them with `os.SameFile`, so a +symlinked copy is caught too. + ### The tokenizer bug the heads found `encodeWord` in `onnxembedder.go` read every long word backwards until 2026-08-08. diff --git a/internal/config/voice.go b/internal/config/voice.go index 1e9a9e3..4f3b568 100644 --- a/internal/config/voice.go +++ b/internal/config/voice.go @@ -2,6 +2,9 @@ package config import ( "errors" + "fmt" + "os" + "path/filepath" "time" ) @@ -141,10 +144,55 @@ func (c *Config) validateVoice() error { if e.ModelPath == "" || e.TokenizerPath == "" || e.LibPath == "" { return errors.New("voice.embedder: all three of model_path, tokenizer_path, lib_path must be set, or remove embedder to use the floor stub") } + if err := e.checkHeadsDistinct(); err != nil { + return err + } } return nil } +// checkHeadsDistinct refuses a heads graph that is the embedder's own file +// (V-692). The rule is stated on HeadsPath above and in CLAUDE.md, and until +// now nothing enforced it: the daemon loaded whatever the key pointed at, so +// pointing both keys at one file cost recall with no error and no log line. It +// reads as ordinary drift, which is the worst kind of misconfiguration. +// +// Refusing to start is the right trade here. The heads are an accelerator and a +// broken weights file is deliberately not fatal in voicewire.go, but this is not +// a broken file. It is a working file in the wrong role, and a daemon that +// cannot route well should say so rather than answer worse. +// +// Cleaned and made absolute first, so "./m.onnx" and "$PWD/m.onnx" are one +// path. Then SameFile, which catches the copy that is a symlink or a hard link +// to the original. A path that does not stat is left to the loader, which fails +// on it with a better message than this can give. +func (e *EmbedderConfig) checkHeadsDistinct() error { + if e.HeadsPath == "" || e.ModelPath == "" { + return nil + } + heads, model := absClean(e.HeadsPath), absClean(e.ModelPath) + same := heads == model + if !same { + hi, herr := os.Stat(heads) + mi, merr := os.Stat(model) + same = herr == nil && merr == nil && os.SameFile(hi, mi) + } + if same { + return fmt.Errorf("voice.embedder: heads_path and model_path are the same file (%s) — the heads graph is a fine-tuned copy, and scoring recall with it degrades what the resident embedder already stored", heads) + } + return nil +} + +// absClean — the comparable form of a path. Abs fails only when the working +// directory is unreadable, and a cleaned relative path is still worth comparing, +// so the error falls back rather than propagating. +func absClean(p string) string { + if abs, err := filepath.Abs(p); err == nil { + return abs + } + return filepath.Clean(p) +} + // VoiceConfig — the client↔core TCP surface + the stt/tts worker-module // seams. // diff --git a/internal/config/voice_test.go b/internal/config/voice_test.go new file mode 100644 index 0000000..86c274f --- /dev/null +++ b/internal/config/voice_test.go @@ -0,0 +1,70 @@ +package config + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// The heads graph is a fine-tuned copy of the embedder, and pointing both keys +// at one file degrades recall with no error and no log line (V-692). These are +// the shapes that used to boot clean. +func TestHeadsPathMustNotBeTheModelFile(t *testing.T) { + dir := t.TempDir() + model := filepath.Join(dir, "model.onnx") + heads := filepath.Join(dir, "heads.onnx") + for _, p := range []string{model, heads} { + if err := os.WriteFile(p, []byte("onnx"), 0o600); err != nil { + t.Fatalf("write %s: %v", p, err) + } + } + link := filepath.Join(dir, "link.onnx") + if err := os.Symlink(model, link); err != nil { + t.Fatalf("symlink: %v", err) + } + + // A path that stats and one that does not, because the guard compares the + // cleaned string before it stats anything. + for name, headsPath := range map[string]string{ + "the same path": model, + "a symlink to it": link, + "an uncleaned path": filepath.Join(dir, ".", "sub", "..", "model.onnx"), + "a path on no disk": filepath.Join(dir, "absent.onnx"), + } { + t.Run(name, func(t *testing.T) { + same := headsPath != filepath.Join(dir, "absent.onnx") + err := voiceConfigWith(t, model, headsPath) + if same && err == nil { + t.Fatal("want a startup error, got a daemon that routes worse in silence") + } + if same && !strings.Contains(err.Error(), "heads_path") { + t.Fatalf("the error does not name the key: %v", err) + } + if !same && err != nil { + t.Fatalf("a distinct heads_path was refused: %v", err) + } + }) + } + + if err := voiceConfigWith(t, model, heads); err != nil { + t.Fatalf("two distinct files were refused: %v", err) + } + if err := voiceConfigWith(t, model, ""); err != nil { + t.Fatalf("no heads at all was refused: %v", err) + } +} + +// voiceConfigWith loads a minimal enabled voice block through the real Load, so +// the test exercises the startup path and not just the check in isolation. +func voiceConfigWith(t *testing.T, model, heads string) error { + t.Helper() + body := `{"voice":{"enabled":true,"bind":"127.0.0.1:9100","embedder":{` + + `"model_path":"` + model + `","tokenizer_path":"/t.json","lib_path":"/l.so"` + if heads != "" { + body += `,"heads_path":"` + heads + `"` + } + body += `}}}` + _, err := Load(writeConfig(t, body)) + return err +} diff --git a/internal/router/eval/eval_test.go b/internal/router/eval/eval_test.go index 879388c..dd53cf6 100644 --- a/internal/router/eval/eval_test.go +++ b/internal/router/eval/eval_test.go @@ -255,35 +255,12 @@ func newBaselineClassifier(t *testing.T, emb router.Embedder) *router.Classifier return cls } -// baselineGrammars — the stage-0 rule set in the daemon's order (buildRouter in -// cmd/mavend/voicewire.go). Split out of newBaselineRouter so the claim -// measurement can run the same rules one at a time and see which of them -// contend for the same utterance, which the cascade hides by stopping at the -// first match. +// baselineGrammars — the stage 0 rule set the daemon runs, from the one place +// it is written down (V-693). It used to restate the list by hand, and by the +// time the guard was written the two had already drifted by one grammar. +// Kept as a name because the claim measurement reads it as "the baseline set". func baselineGrammars(acts router.ActMatcher) []router.Grammar { - grammars := router.DefaultGrammars(acts) - grammars = append(grammars, router.SystemTimeDateGrammars()...) - // Same order as buildRouter (voicewire.go). The fixture is only worth - // anything while its grammar set is the daemon's grammar set. - grammars = append(grammars, router.AgendaQueryGrammars()...) - // After the agenda rules and before the feed and list rules, same as - // voicewire.go: "что такое лента" is a definition question and the feed - // rule would claim it on the noun alone (V-655). Missing here until V-659, - // so the fixture was scoring a grammar set the daemon does not run. - grammars = append(grammars, router.WorldQueryGrammars()...) - grammars = append(grammars, router.FeedQueryGrammar()) - // The list side of the same exposure: a phrasing with no possessive in it - // ("список дел") routed system and never reached queryTasks (Vikunja #467). - grammars = append(grammars, router.TaskListGrammar()) - grammars = append(grammars, router.ListGrammars()...) - grammars = append(grammars, router.ReminderGrammar()) - grammars = append(grammars, router.PraxisGrammars()...) - grammars = append(grammars, router.TaskStatusGrammar()) - grammars = append(grammars, router.TaskCaptureGrammar()) - // "расскажи про X" is a world question the model called a fact, and the - // rule goes last because it matches on the first word alone (Vikunja #498). - grammars = append(grammars, router.NarrativeQueryGrammars()...) - return grammars + return router.StageZeroGrammars(acts) } // seedOrder — fixed iteration order over the corpus. Not cosmetic: a few diff --git a/internal/router/stagezero.go b/internal/router/stagezero.go new file mode 100644 index 0000000..3fad4b6 --- /dev/null +++ b/internal/router/stagezero.go @@ -0,0 +1,65 @@ +package router + +// StageZeroGrammars — the stage 0 rule set, in the order the daemon runs it. +// +// It lives here because it used to live in two places (V-693). `buildRouter` in +// cmd/mavend/voicewire.go held the real set and `baselineGrammars` in +// internal/router/eval/eval_test.go restated it by hand, in the daemon's order, +// with its own comment saying so. Three test files score against the fixture, +// and nothing compared the two lists. By 2026-08-11 they had already drifted: +// BareCaptureGrammar was in the daemon and not in the fixture, so every routing +// measurement scored a set nobody ran. That is the failure CLAUDE.md warned +// about by name, and a diff test would have caught it one grammar late. One +// list cannot drift from itself. +// +// The order is the contract, not the membership. Each rule below says why it +// sits where it sits, and a rule inserted in the wrong place changes which +// utterances the cascade never reaches. Read the comment above a line before +// moving it, and read docs/routing.md before adding one. +// +// The classifier, the extractor, the threshold and the model arm are the +// daemon's to assemble. This function returns the rules and nothing else, so +// the fixture can also run them one at a time and see which of them contend for +// the same utterance, which the cascade hides by stopping at the first match. +func StageZeroGrammars(acts ActMatcher) []Grammar { + grammars := DefaultGrammars(acts) + grammars = append(grammars, SystemTimeDateGrammars()...) + // After the time/date rules on purpose: "какой сегодня день" is a clock + // question and must keep reaching replySystem, while "что у меня сегодня" + // is an agenda question and must not. + grammars = append(grammars, AgendaQueryGrammars()...) + // Same reason as the agenda rules, for the feeds: "что нового в лентах?" + // routed system and answered "пока не умею" (Vikunja #474). + // After the agenda rules, which are the narrower claim, and BEFORE the feed + // and list rules, which are not: "что такое лента" is a definition question + // and the feed rule would take it on the noun alone (V-655). + grammars = append(grammars, WorldQueryGrammars()...) + grammars = append(grammars, FeedQueryGrammar()) + // The list side of the same exposure: a phrasing with no possessive in it + // ("список дел") routed system and never reached queryTasks (Vikunja #467). + grammars = append(grammars, TaskListGrammar()) + grammars = append(grammars, ListGrammars()...) + grammars = append(grammars, ReminderGrammar()) + // Before the capture marker, because "отметь" is a capture verb and "отметь + // второй пункт" is not a note. The Praxis rules are the narrower claim — a + // lifecycle verb AND an item named — so they get first refusal (Vikunja #516). + grammars = append(grammars, PraxisGrammars()...) + // After Praxis, whose bare "закрой" claim this rule cannot reach (it needs the + // board noun), and before the capture marker, which would otherwise read + // "убери из задач купить молоко" as a new task (Vikunja #512). + grammars = append(grammars, TaskStatusGrammar()) + // Before the capture markers, which all need an object. A capture verb + // alone is a fact with no key, and the clarify path asks for it rather than + // letting the model invent an answer (Vikunja #557). + grammars = append(grammars, BareCaptureGrammar()...) + // Last, and it matches any utterance shape — its Build is the filter. An + // explicit capture marker beats the model, which called it an act and + // rewrote the task text (Vikunja #467). After the rules above because a + // marker never collides with a clock or agenda question. + grammars = append(grammars, TaskCaptureGrammar()) + // After the capture marker, so "запиши" still wins over "расскажи", and + // last overall because it matches on the first word alone: "расскажи про + // X" is a world question the model called a fact (Vikunja #498). + grammars = append(grammars, NarrativeQueryGrammars()...) + return grammars +}