diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index f33a48b..5494f74 100644 --- a/.abcd/work/DECISIONS.md +++ b/.abcd/work/DECISIONS.md @@ -1536,3 +1536,48 @@ Architecture-shaping decisions graduate to an ADR under schema-move note delegates the canonical table to `05-internals/ 02-schemas.md`; the other found the omission real but nitpick-tier) — discarded per the loop's tie-breaking rule. + +- 2026-08-16 — Bug-hunt round 46: three confirmed nitpicks, zero refuted. + `install_ffmpeg_local`'s Linux branch (`install.sh`) downloaded and + verified the johnvansickle static ffmpeg tarball, which bundles + `ffprobe` alongside `ffmpeg`, but only ever installed `ffmpeg` — + `ffprobe` was extracted and discarded. `transcribe -audio`'s offset + derivation shells out to `ffprobe` to read a recording's `creation_time` + tag; without it on PATH (guaranteed by getting-started.md's documented + no-Homebrew "local" install path on Linux), every external recording + silently fell back to a 0 offset, indistinguishable from a genuinely + missing or unreadable tag — and `docs/explanation/how-alignment-works.md` + and `docs/reference/cli.md` named only the tag-based causes. Fixed by + installing `ffprobe` from the already-downloaded tarball alongside + `ffmpeg`, and naming the missing-binary cause in both docs pages; the + macOS local-install branch still lacks `ffprobe` (evermeet publishes it + as a separate signed download this round did not add, to avoid an + unverified new network path) and is now accurately reflected as the + residual gap rather than being the whole story. Separately, + `resolveModel`'s not-found guidance (`internal/transcribe/whispercpp.go`) + and its mirror in `docs/how-to/transcribe-a-recording.md` both printed a + `curl -L --create-dirs` model-download recipe omitting `-f`/`--fail`; + without it, an HTTP error response (a moved or withdrawn model asset) + is written into the destination `.bin` file at exit 0 instead of failing + the download, so a reader gets a "successful" download that only fails, + confusingly, at whisper-cli load time — `install.sh`'s own equivalent + guidance already used `curl -fL` for the identical recipe; both sites + now match it, pinned by a new test. And the released `[0.2.0]` + `CHANGELOG.md` entry for `testimony analyze` closed with "The CLI holds + no API keys and makes no network calls" — a CLI-wide *and* unqualified + claim, unlike every other occurrence of this phrase family: three + scope the subject to `analyze` (`README.md`, `docs/reference/cli.md`, + `docs/how-to/analyse-a-session.md`) and three keep "The CLI" as + subject but qualify the predicate to "adds no network dependency" + rather than "makes no network calls" (`AGENTS.md`, + `internal/analyze/analyze.go`, `.abcd/work/CONTEXT.md`) — a narrower, + still-true claim about the analysis layer's own design rather than a + claim of zero runtime network activity anywhere in the CLI, which the + demo page's rrweb CDN load (already present at v0.2.0's release) and + transcribe's ASR model fetch both falsify. Adversarial review of this + round's own PR caught an earlier draft of this entry overclaiming that + "every other occurrence uses the scoped `analyze`-only form" — false, + since three of the six use "The CLI" as subject; corrected here to + name the qualifier that actually distinguishes them. Rescoped the + CHANGELOG bullet's subject to `analyze`, true of it, without altering + the historical record's substance. diff --git a/CHANGELOG.md b/CHANGELOG.md index f30d3fa..12111f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -796,7 +796,7 @@ Resource and process lifecycle: a versioned rubric plus the session timeline as a self-contained prompt any assistant can answer; `-ingest` validates that answer against the timeline (evidence must exist, quotes must be verbatim, status is forced to - `unverified`) into `findings.jsonl`. The CLI holds no API keys and makes no + `unverified`) into `findings.jsonl`. `analyze` holds no API keys and makes no network calls. - **`testimony review`** — records `confirmed` / `rejected` / `duplicate` verdicts append-only, never rewriting the original finding; interactive walk diff --git a/docs/explanation/how-alignment-works.md b/docs/explanation/how-alignment-works.md index e7f8284..f5907ea 100644 --- a/docs/explanation/how-alignment-works.md +++ b/docs/explanation/how-alignment-works.md @@ -16,7 +16,7 @@ The voice recording is the awkward stream. A transcription engine reports times session_time = audio_time + offset ``` -Which offset applies depends on where the audio came from. A session captured with `testimony record` needs none: capture starts at `t0`, so the offset is 0 by construction and no derivation is attempted. For an external recording (`transcribe -audio`), the offset is derived automatically when it can be: `transcribe` reads the recording's embedded creation timestamp (the `creation_time` tag recorders such as QuickTime write) and subtracts the manifest's `t0_epoch_ms`; if the tag is missing or unreadable, it falls back to 0 — correct whenever you start recording at the moment the session starts. The derived value is then persisted beside the audio in `audio.offset.json`, so a later re-run over the converted `audio.wav` reuses it instead of silently assuming 0. Whatever the path, the command prints the offset and its provenance, so the value is never silent. +Which offset applies depends on where the audio came from. A session captured with `testimony record` needs none: capture starts at `t0`, so the offset is 0 by construction and no derivation is attempted. For an external recording (`transcribe -audio`), the offset is derived automatically when it can be: `transcribe` shells out to `ffprobe` (ffmpeg's companion tool) to read the recording's embedded creation timestamp (the `creation_time` tag recorders such as QuickTime write) and subtracts the manifest's `t0_epoch_ms`; if `ffprobe` is not on PATH, or the tag is missing or unreadable, it falls back to 0 — correct whenever you start recording at the moment the session starts. The derived value is then persisted beside the audio in `audio.offset.json`, so a later re-run over the converted `audio.wav` reuses it instead of silently assuming 0. Whatever the path, the command prints the offset and its provenance, so the value is never silent. Derivation can be wrong — a recorder may stamp the wrong moment, or omit the tag. That is why sessions begin with a spoken marker: saying "session start" aloud at t0 plants a phrase that appears in the transcript at a known session time (roughly zero). If the report looks misaligned, the marker's transcript time reveals the true offset, and an explicit `-offset` overrides derivation entirely. A belt-and-braces anchor, recoverable after the fact. diff --git a/docs/how-to/transcribe-a-recording.md b/docs/how-to/transcribe-a-recording.md index eb8cc46..503c2af 100644 --- a/docs/how-to/transcribe-a-recording.md +++ b/docs/how-to/transcribe-a-recording.md @@ -51,7 +51,7 @@ whisper.cpp needs a ggml model file. `-model` accepts either: - **A model name** (default `large-v3-turbo`): Testimony looks for `ggml-.bin` in `~/.cache/whisper.cpp`, `~/.cache/whisper`, `~/.local/share/whisper.cpp`, and `~/models`. Download one there if missing: ```sh - curl -L --create-dirs -o ~/.cache/whisper.cpp/ggml-large-v3-turbo.bin \ + curl -fL --create-dirs -o ~/.cache/whisper.cpp/ggml-large-v3-turbo.bin \ https://huggingface.co/ggerganov/whisper.cpp/resolve/main/ggml-large-v3-turbo.bin ``` diff --git a/docs/reference/cli.md b/docs/reference/cli.md index b3ee7b2..16f4946 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -60,11 +60,11 @@ testimony transcribe -session DIR [-audio FILE] | `-vad` | `auto` | (whisperx) VAD method: `auto`, `silero`, or `pyannote`. `auto` picks `silero`; `pyannote` fails under newer torch versions | | `-offset` | derived | audio-to-session clock offset in seconds. When not given: with `-audio` naming a file other than the session's own `audio.wav`, derived from the recording's creation time minus the manifest's `t0_epoch_ms`, or 0 when derivation is impossible; without it (including `-audio audio.wav`), read back from `audio.offset.json` when the session has one, else 0. A non-finite value, or one beyond ±10⁹ seconds (the bound every derived or persisted offset already meets), is a usage error | -Behaviour: reads `manifest.json` (required). With `-audio`, requires ffmpeg on PATH and converts the recording to 16 kHz mono `audio.wav` in the session directory; without it (or when `-audio` points at the session's own `audio.wav`), it uses the existing `audio.wav` in place and skips the conversion. It then runs the engine, applies the offset, and writes `transcript.jsonl`. Always prints the offset it used and its provenance — one of: +Behaviour: reads `manifest.json` (required). With `-audio`, requires ffmpeg on PATH and converts the recording to 16 kHz mono `audio.wav` in the session directory; without it (or when `-audio` points at the session's own `audio.wav`), it uses the existing `audio.wav` in place and skips the conversion. Deriving the offset below additionally needs `ffprobe`, ffmpeg's companion tool, on PATH; without it derivation falls back to 0 like a missing or unreadable timestamp tag. It then runs the engine, applies the offset, and writes `transcript.jsonl`. Always prints the offset it used and its provenance — one of: - `from -offset flag` — the explicit flag, which always wins; - `derived: audio creation_time − manifest t0` — derived for an external recording; -- `default 0: audio creation time unavailable` — an external recording whose creation time could not be read; +- `default 0: audio creation time unavailable` — an external recording whose creation time could not be read, or for which `ffprobe` was not found on PATH; - `persisted: audio.wav converted from an external recording (+3.20s)` — read back from `audio.offset.json`, the printed value being the persisted offset; - `default 0: session audio.wav captured at t0` — a session whose `audio.wav` was captured here and has no sidecar. diff --git a/install.sh b/install.sh index 97364dc..2f9ff37 100644 --- a/install.sh +++ b/install.sh @@ -342,6 +342,15 @@ install_ffmpeg_local() { || { err "could not unpack ffmpeg; skipping ffmpeg"; rm -rf "$tmp2"; return; } install -m 0755 "$tmp2"/ffmpeg-*-static/ffmpeg "$INSTALL_DIR/ffmpeg" \ || { err "could not install ffmpeg into $INSTALL_DIR; skipping ffmpeg"; rm -rf "$tmp2"; return; } + # The same tarball also carries ffprobe, which transcribe -audio + # uses to derive the audio-to-session offset from a recording's + # creation_time tag; install it too so that derivation does not + # silently fall back to 0 for lack of the binary. Best-effort: + # ffmpeg itself already succeeded above, so a failure here only + # loses offset derivation, not the conversion this function exists + # to provide. + install -m 0755 "$tmp2"/ffmpeg-*-static/ffprobe "$INSTALL_DIR/ffprobe" \ + || err "could not install ffprobe into $INSTALL_DIR; offset derivation for transcribe -audio will fall back to 0" ;; esac rm -rf "$tmp2" diff --git a/internal/transcribe/transcribe_test.go b/internal/transcribe/transcribe_test.go index 4de7535..101c627 100644 --- a/internal/transcribe/transcribe_test.go +++ b/internal/transcribe/transcribe_test.go @@ -1625,6 +1625,26 @@ func TestResolveModelAcceptsRegularFile(t *testing.T) { } } +// TestResolveModelGuidanceFailsClosedOnHTTPError pins resolveModel's printed +// download recipe to curl's fail-closed flag. Without -f/--fail, curl treats +// an HTTP error response (e.g. a moved or withdrawn Hugging Face asset) as a +// successful transfer and writes the error page into the destination .bin +// file at exit 0 — silently corrupting the model the reader thinks they just +// downloaded, which then fails opaquely at whisper-cli load time rather than +// at download time. install.sh's own equivalent guidance already uses -fL +// for the identical recipe; this pins the printed guidance to match. +func TestResolveModelGuidanceFailsClosedOnHTTPError(t *testing.T) { + t.Setenv("HOME", t.TempDir()) // no cached model anywhere for resolveModel to find + + _, err := resolveModel("missing-model") + if err == nil { + t.Fatal("resolveModel of a nonexistent model must fail") + } + if !strings.Contains(err.Error(), "curl -fL --create-dirs") { + t.Fatalf("resolveModel guidance must use curl -fL (fail-closed on HTTP errors), got: %v", err) + } +} + // TestWriteOffsetSidecarFailurePreservesPrior pins the atomic sidecar write. // The previous truncating write (O_TRUNC, then write) destroyed the prior // sidecar's bytes the moment the open succeeded, so a write that failed after diff --git a/internal/transcribe/whispercpp.go b/internal/transcribe/whispercpp.go index 6cfa222..55c8c90 100644 --- a/internal/transcribe/whispercpp.go +++ b/internal/transcribe/whispercpp.go @@ -138,5 +138,5 @@ func resolveModel(model string) (string, error) { return c, nil } } - return "", fmt.Errorf("whisper.cpp model %q not found: not a file path, and %s is absent from ~/.cache/whisper.cpp, ~/.cache/whisper, ~/.local/share/whisper.cpp, and ~/models — download it, e.g.\n curl -L --create-dirs -o ~/.cache/whisper.cpp/%s https://huggingface.co/ggerganov/whisper.cpp/resolve/main/%s\nor pass an existing ggml file path via -model", model, name, name, name) + return "", fmt.Errorf("whisper.cpp model %q not found: not a file path, and %s is absent from ~/.cache/whisper.cpp, ~/.cache/whisper, ~/.local/share/whisper.cpp, and ~/models — download it, e.g.\n curl -fL --create-dirs -o ~/.cache/whisper.cpp/%s https://huggingface.co/ggerganov/whisper.cpp/resolve/main/%s\nor pass an existing ggml file path via -model", model, name, name, name) }