From d8efb667c7518df5f80e0e71630b9d7ae8707b47 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 11 Aug 2026 21:02:16 +0400 Subject: [PATCH 1/2] Refuse a heads_path that is the embedder's own model file (V-692) CLAUDE.md, internal/config/voice.go and docs/routing.md all say the routing heads graph is a fine-tuned copy of the embedder, never the embedder's own file. 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, which reads as ordinary drift rather than as a misconfiguration. validateVoice now refuses it at load. Both paths are cleaned and made absolute first, so "./m.onnx" and "$PWD/m.onnx" are one path, and then compared with os.SameFile, which catches a copy that is a symlink or a hard link. A path that does not stat is left to the loader, whose error message is better than this check can give. Refusing to start is deliberate and it differs from the loader's treatment of a broken weights file, which logs and leaves the heads nil on purpose. That case is a missing accelerator. This one is a working file in the wrong role, and a daemon that cannot route well should say so rather than answer worse. deploy/mavend.json points the two keys at different files, so the live config still starts. --- internal/config/voice.go | 48 ++++++++++++++++++++++++ internal/config/voice_test.go | 70 +++++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+) create mode 100644 internal/config/voice_test.go 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 +} From 240d53a96af3645fbe9e0da064aaabec0a91a78d Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 11 Aug 2026 21:02:31 +0400 Subject: [PATCH 2/2] Give the stage 0 grammar set one home (V-693) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit buildRouter held the real set and baselineGrammars in 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. They had already drifted: BareCaptureGrammar went into the daemon with V-557 and never into the fixture, so every routing measurement since has scored a set nobody runs. That is the failure CLAUDE.md warns about by name, and a diff test would have caught it one grammar late. The list moves to router.StageZeroGrammars in internal/router/stagezero.go, with the ordering comments, which are the load-bearing part. buildRouter and the fixture both call it. One list cannot drift from itself. Measured before and after on the 96-case fixture: classifier+onnx 72/96, 75.0% intent, 33.3% destination, identical either way, and the deterministic claim and reach hash ratchets do not move. So the missing grammar cost no measurable accuracy. That is the point rather than a reprieve: the fixture had been scoring the wrong set for four days and nothing could say so. The invariants caveat is deleted, both entries, since V-692 landed the other guard in the previous commit. The reasoning for both now sits in docs/routing.md beside the subsystem, which is where a fix's durable record belongs. Unrelated and pre-existing: TestONNXPersonalBoundary fails on "я рассказывал тебе про байкал?" (personal 0.9068, world 0.9413) at the merge base too. --- CLAUDE.md | 6 ++- cmd/mavend/voicewire.go | 42 ++------------------ docs/caveats/CLAUDE.md | 8 ++-- docs/caveats/invariants.md | 27 ------------- docs/routing.md | 22 ++++++++--- internal/router/eval/eval_test.go | 33 +++------------- internal/router/stagezero.go | 65 +++++++++++++++++++++++++++++++ 7 files changed, 98 insertions(+), 105 deletions(-) delete mode 100644 docs/caveats/invariants.md create mode 100644 internal/router/stagezero.go 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/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 +}