diff --git a/Makefile b/Makefile index 1486a70..c3e8d7b 100644 --- a/Makefile +++ b/Makefile @@ -149,8 +149,13 @@ eval-models: # stt-fixtures — regenerate the golden STT audio in cmd/mavsttd/testdata from # the piper voices (#288). The committed WAVs are synthesised, never recorded, -# so this is the only way they should ever change. TestGoldenAudioTranscription -# then scores them against ggml-small; it self-skips when the model is absent. +# so this is the only way they should ever change. The spoken text is read out +# of testdata/golden_v1.json, so edit the transcript there and rerun this. +# +# test-stt-golden runs both golden tests: TestGoldenAudioTranscription, which +# scores the fixtures against ggml-small and self-skips when the model is +# absent, and TestGoldenFixturesAreCanonical, which checks the committed audio +# and the manifest with no model at all. stt-fixtures: ./scripts/gen-stt-fixtures.sh diff --git a/cmd/mavsttd/golden_test.go b/cmd/mavsttd/golden_test.go index 0783a83..a547e11 100644 --- a/cmd/mavsttd/golden_test.go +++ b/cmd/mavsttd/golden_test.go @@ -2,10 +2,22 @@ package main // Golden-audio STT tests (Vikunja #288). // -// These push real audio through the real whisper.cpp binding, so a bad model -// path, a wrong language hint, a broken resample or a regressed silence gate -// is caught by `make test` rather than by the owner talking to a daemon that -// mishears him. +// These push real audio through the real whisper.cpp binding. What that +// covers, precisely, is two things: the model still transcribes known speech +// well enough for the router to act on it, and the silence gate still lets real +// speech through. A regression in either shows up in `make test` rather than in +// the owner talking to a daemon that mishears him. +// +// It is worth being exact about what is NOT covered, because this comment used +// to claim more. Nothing here resamples: audio.PCMFromWAV refuses anything that +// is not 16 kHz mono s16, the fixtures arrive at 16 kHz from ffmpeg, and there +// is no conversion step between the WAV and whisper_full. Nothing here +// exercises language selection either: the hint comes out of the manifest +// already correct and goes straight into the request, so how mavsttd chooses a +// language is untested. And a wrong model path is not caught when it is the +// default one, because a box without the model skips; an explicitly set +// MAVEN_WHISPER_MODEL that does not exist is a failure, since that is a +// mistake and not an absence. // // The fixtures are piper-synthesised, not recorded — see // scripts/gen-stt-fixtures.sh. Nothing of the owner's voice is committed, and @@ -47,7 +59,10 @@ type goldenCase struct { Lang string `json:"lang"` Text string `json:"text"` Keywords []string `json:"keywords"` - MaxWER float64 `json:"max_wer"` + // MeasuredWER is what this case scored when the ceiling was last set, so + // a model swap is a diff to a recorded number rather than silence. + MeasuredWER float64 `json:"measured_wer"` + MaxWER float64 `json:"max_wer"` } type goldenManifest struct { @@ -152,6 +167,14 @@ func containsSeq(hyp, want []string) bool { return false } +// looseWordMatch reports whether got is want, or an inflection of it. +// +// A shared prefix alone is not enough. "воды" retains three runes, so "водка" +// used to satisfy the ru_fact keyword and the test passed on whisper hearing +// "выпил водки". "disk" retains "dis", which "display", "distance" and +// "discuss" all match. So the hypothesis is also capped in length: a case +// ending adds a rune or two, it does not add a syllable. Short words get no +// slack at all, because there is nothing left of them after a prefix cut. func looseWordMatch(got, want string) bool { if got == want { return true @@ -166,6 +189,13 @@ func looseWordMatch(got, want string) bool { if n < 3 || len(g) < n { return false } + extra := 2 + if len(w) <= 4 { + extra = 0 + } + if len(g) > len(w)+extra { + return false + } return string(g[:n]) == string(w[:n]) } @@ -176,6 +206,11 @@ func TestGoldenAudioTranscription(t *testing.T) { model := goldenModelPath() if _, err := os.Stat(model); err != nil { + // An explicit override that points at nothing is a mistake, not a box + // without the model. Skipping there made a typo look like a pass. + if os.Getenv("MAVEN_WHISPER_MODEL") != "" { + t.Fatalf("MAVEN_WHISPER_MODEL=%s does not exist: %v", model, err) + } t.Skipf("whisper model %s absent (%v) — set MAVEN_WHISPER_MODEL or see AGENTS.md", model, err) } @@ -192,7 +227,9 @@ func TestGoldenAudioTranscription(t *testing.T) { path := filepath.Join("testdata", c.WAV) raw, err := os.ReadFile(path) if err != nil { - t.Skipf("fixture %s absent (%v) — run scripts/gen-stt-fixtures.sh", path, err) + // Not a skip. A fixture the generator failed to write is a + // broken checkout, and skipping made `make test` green on one. + t.Fatalf("fixture %s absent (%v) — run scripts/gen-stt-fixtures.sh", path, err) } format, pcm, err := audio.PCMFromWAV(raw) if err != nil { @@ -221,9 +258,12 @@ func TestGoldenAudioTranscription(t *testing.T) { if missing := missingKeywords(c.Keywords, hyp); len(missing) > 0 { t.Errorf("%s: missing keywords %v in %q", c.WAV, missing, resp.Text) } - if wer := wordErrorRate(ref, hyp); wer > c.MaxWER { - t.Errorf("%s: WER %.2f > %.2f\n want: %q\n got: %q", c.WAV, wer, c.MaxWER, c.Text, resp.Text) + wer := wordErrorRate(ref, hyp) + if wer > c.MaxWER { + t.Errorf("%s: WER %.2f > %.2f (measured %.2f when the ceiling was set)\n want: %q\n got: %q", + c.WAV, wer, c.MaxWER, c.MeasuredWER, c.Text, resp.Text) } + t.Logf("%s: WER %.2f (ceiling %.2f, was %.2f)", c.WAV, wer, c.MaxWER, c.MeasuredWER) }) } } @@ -253,27 +293,29 @@ func TestGoldenFixturesAreCanonical(t *testing.T) { } // The fixture must clear mavsttd's own silence gate, otherwise the // model test below would be asserting on a gated empty string. - if reason := gateReason(pcmToF32(pcm), whisperSampleRate, 300, 0.01); reason != "" { + if reason := gateReason(pcmSamples(pcm), whisperSampleRate, 300, 0.01); reason != "" { t.Errorf("%s: would be gated as %s", path, reason) } if len(c.Keywords) == 0 { t.Errorf("%s: manifest case has no keywords", c.Name) } + // An empty reference makes wordErrorRate return 1 for every + // hypothesis, so the WER assertion fires with nothing useful to say. + if len(normalizeTranscript(c.Text)) == 0 { + t.Errorf("%s: manifest case has no reference text", c.Name) + } + if c.Lang != "ru" && c.Lang != "en" { + t.Errorf("%s: lang %q is not one of the two languages mavsttd is run with", c.Name, c.Lang) + } if c.MaxWER <= 0 || c.MaxWER > 1 { t.Errorf("%s: max_wer %v outside (0,1]", c.Name, c.MaxWER) } + if c.MeasuredWER > c.MaxWER { + t.Errorf("%s: measured_wer %v is above max_wer %v, so the ceiling was never met", c.Name, c.MeasuredWER, c.MaxWER) + } } } -func pcmToF32(b []byte) []float32 { - out := make([]float32, len(b)/2) - for i := range out { - s := int16(b[i*2]) | int16(b[i*2+1])<<8 - out[i] = float32(s) / 32768.0 - } - return out -} - // --- matcher unit tests (no model, no fixtures) ---------------------------- func TestNormalizeTranscript(t *testing.T) { @@ -320,4 +362,30 @@ func TestMissingKeywords(t *testing.T) { if got := missingKeywords([]string{"часть"}, hyp2); len(got) != 1 { t.Fatalf("missingKeywords = %v, want %q reported missing", got, "часть") } + // A prefix is not a word. These are different words that share one, and + // each of them used to satisfy the keyword it is paired with. + different := [][2]string{ + {"воды", "Я выпил водки."}, + {"disk", "check the display"}, + {"disk", "we should discuss it"}, + {"server", "a serverless function"}, + } + for _, d := range different { + if got := missingKeywords([]string{d[0]}, normalizeTranscript(d[1])); len(got) != 1 { + t.Errorf("keyword %q was satisfied by %q", d[0], d[1]) + } + } + // And the inflections still pass, which is the whole point of the loose + // match. + same := [][2]string{ + {"воды", "выпил воду"}, + {"напомни", "напомните мне"}, + {"календарю", "по календаре"}, + {"restart", "restarted the server"}, + } + for _, d := range same { + if got := missingKeywords([]string{d[0]}, normalizeTranscript(d[1])); len(got) != 0 { + t.Errorf("keyword %q was not matched by %q", d[0], d[1]) + } + } } diff --git a/cmd/mavsttd/testdata/golden_v1.json b/cmd/mavsttd/testdata/golden_v1.json index 870a4d5..5e7b4b9 100644 --- a/cmd/mavsttd/testdata/golden_v1.json +++ b/cmd/mavsttd/testdata/golden_v1.json @@ -1,5 +1,6 @@ { - "note": "Golden STT fixtures. Audio is piper-synthesised, not recorded — see scripts/gen-stt-fixtures.sh. Regenerate with that script; do not hand-edit `wav`.", + "note": "Golden STT fixtures. Audio is piper-synthesised, not recorded — see scripts/gen-stt-fixtures.sh, which reads `text` from this file and synthesises from it. This is the only source of the spoken words; regenerate with that script and do not hand-edit `wav`.", + "wer_note": "max_wer is set just above what each case actually measures against ggml-small, recorded in `measured_wer` on 2026-08-01. A flat 0.34 over a five-word reference tolerated two wrong words and left most of the range unguarded. A model swap should show up as a diff to these numbers, not as silence: rerun `make test-stt-golden`, read the logged transcript, and move both fields together.", "cases": [ { "name": "ru_reminder", @@ -7,7 +8,8 @@ "lang": "ru", "text": "напомни мне через час позвонить маме", "keywords": ["напомни", "час", "позвонить"], - "max_wer": 0.34 + "measured_wer": 0.0, + "max_wer": 0.1 }, { "name": "ru_fact", @@ -15,7 +17,8 @@ "lang": "ru", "text": "отметь что я выпил воды", "keywords": ["отметь", "воды"], - "max_wer": 0.34 + "measured_wer": 0.2, + "max_wer": 0.25 }, { "name": "ru_query", @@ -23,7 +26,8 @@ "lang": "ru", "text": "что у меня сегодня по календарю", "keywords": ["сегодня", "календарю"], - "max_wer": 0.34 + "measured_wer": 0.0, + "max_wer": 0.1 }, { "name": "en_act", @@ -31,7 +35,8 @@ "lang": "en", "text": "restart the web server and check the disk space", "keywords": ["restart", "server", "disk"], - "max_wer": 0.34 + "measured_wer": 0.0, + "max_wer": 0.1 } ] } diff --git a/cmd/mavsttd/whisper_handler.go b/cmd/mavsttd/whisper_handler.go index b8cdf93..bc58cbd 100644 --- a/cmd/mavsttd/whisper_handler.go +++ b/cmd/mavsttd/whisper_handler.go @@ -66,6 +66,19 @@ func gateReason(samples []float32, rate, minMs int, minRMS float64) string { return "" } +// pcmSamples converts canonical s16le little-endian PCM to the float32 range +// whisper wants. Shared with the golden tests: they used to carry their own +// copy, so a regression here (a /32767 divisor, a byte order slip) left the +// assertion that the fixtures clear the silence gate green. +func pcmSamples(b []byte) []float32 { + out := make([]float32, len(b)/2) + for i := range out { + s := int16(b[i*2]) | int16(b[i*2+1])<<8 + out[i] = float32(s) / 32768.0 + } + return out +} + func (h *whisperHandler) Transcribe(ctx context.Context, req worker.TranscribeReq) (worker.TranscribeResp, error) { if err := ctx.Err(); err != nil { return worker.TranscribeResp{}, fmt.Errorf("whisper: context done before transcribe: %w", err) @@ -75,12 +88,7 @@ func (h *whisperHandler) Transcribe(ctx context.Context, req worker.TranscribeRe return worker.TranscribeResp{}, fmt.Errorf("whisper: empty audio") } - nSamples := len(a.Bytes) / 2 - samples := make([]float32, nSamples) - for i := 0; i < nSamples; i++ { - s := int16(a.Bytes[i*2]) | int16(a.Bytes[i*2+1])<<8 - samples[i] = float32(s) / 32768.0 - } + samples := pcmSamples(a.Bytes) // Silence gate: drop non-speech before whisper hallucinates on it. if reason := gateReason(samples, whisperSampleRate, h.minMs, h.minRMS); reason != "" { @@ -111,7 +119,7 @@ func (h *whisperHandler) Transcribe(ctx context.Context, req worker.TranscribeRe ch := make(chan result, 1) cSamples := (*C.float)(unsafe.Pointer(&samples[0])) go func() { - ch <- result{code: int(C.whisper_full(h.ctx, params, cSamples, C.int(nSamples)))} + ch <- result{code: int(C.whisper_full(h.ctx, params, cSamples, C.int(len(samples))))} }() select { case r := <-ch: diff --git a/scripts/gen-stt-fixtures.sh b/scripts/gen-stt-fixtures.sh index 98d6dae..5f430dc 100755 --- a/scripts/gen-stt-fixtures.sh +++ b/scripts/gen-stt-fixtures.sh @@ -10,6 +10,13 @@ # Usage: # scripts/gen-stt-fixtures.sh # +# The spoken text is NOT written here. It is read out of +# cmd/mavsttd/testdata/golden_v1.json, which is the same file the test scores +# against. It used to live in both places, so editing this script and running +# make stt-fixtures left the manifest describing audio that no longer existed — +# and at a WER ceiling of 0.34 over a five-word reference, a one-word drift +# passed silently. Punctuation does not matter: normalizeTranscript strips it. +# # Voices are picked up from, in order, $PIPER_VOICE_RU / $PIPER_VOICE_EN, then # the repo's models/tts, then ~/esp-server/voices. The English voice is not # vendored; if it is missing the English fixture is skipped and the existing @@ -34,6 +41,26 @@ ru="$(pick_voice "${PIPER_VOICE_RU:-}" "$root/models/tts/ru_RU-irina-medium.onnx } en="$(pick_voice "${PIPER_VOICE_EN:-}" "$root/models/tts/en_US-lessac-medium.onnx" "$HOME/esp-server/voices/en_US-lessac-medium.onnx")" || en="" +manifest="$root/cmd/mavsttd/testdata/golden_v1.json" +command -v jq >/dev/null || { echo "jq is required to read $manifest" >&2; exit 1; } +[ -f "$manifest" ] || { echo "missing $manifest" >&2; exit 1; } + +# case_text — the reference transcript for one manifest case. +case_text() { + local name="$1" text + text="$(jq -r --arg n "$name" '.cases[] | select(.name==$n) | .text' "$manifest")" + [ -n "$text" ] && [ "$text" != "null" ] || { echo "no case named $name in $manifest" >&2; exit 1; } + printf '%s' "$text" +} + +# case_wav — the file name the manifest expects for one case. +case_wav() { + local name="$1" wav + wav="$(jq -r --arg n "$name" '.cases[] | select(.name==$n) | .wav' "$manifest")" + [ -n "$wav" ] && [ "$wav" != "null" ] || { echo "no case named $name in $manifest" >&2; exit 1; } + printf '%s' "$wav" +} + # synth # piper emits raw 22050 Hz s16le on stdout; ffmpeg resamples to the canonical # 16 kHz mono and writes a plain 44-byte-header WAV (-fflags bitexact keeps @@ -51,15 +78,15 @@ synth() { echo "wrote $dest ($(stat -c%s "$dest") bytes)" } -synth "$ru" "$out/ru_reminder.wav" "Напомни мне через час позвонить маме." -synth "$ru" "$out/ru_fact.wav" "Отметь, что я выпил воды." -synth "$ru" "$out/ru_query.wav" "Что у меня сегодня по календарю?" +for name in ru_reminder ru_fact ru_query; do + synth "$ru" "$out/$(case_wav "$name")" "$(case_text "$name")" +done if [ -n "$en" ]; then - # Keep the English line free of words piper spells out letter by letter — - # "nginx" comes out of lessac as "engine X", which is a TTS artefact and - # would make the fixture assert on the wrong thing. - synth "$en" "$out/en_act.wav" "Restart the web server and check the disk space." + # Keep the English line in the manifest free of words piper spells out + # letter by letter — "nginx" comes out of lessac as "engine X", which is a + # TTS artefact and would make the fixture assert on the wrong thing. + synth "$en" "$out/$(case_wav en_act)" "$(case_text en_act)" else echo "no english piper voice found — skipping en_act.wav" >&2 fi