From 4a7aed9b0d02606194a82e308a666c6d57c3eaba Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 8 Sep 2026 01:00:04 +0000 Subject: [PATCH 1/2] Fail closed when Grok STT coerces bool start to 1.0 Grok STT used float(start or 0.0), so true became 1.0 and a missing start dumped the board at t=0. Require JSON numbers before writing timing.json. Co-authored-by: jmjava --- milestones/README.md | 8 ++-- milestones/grok-stt-start-end.md | 45 +++++++++++++++++++ milestones/validate-stream-probe.md | 2 +- src/docgen/ai_client.py | 24 +++++++--- tests/test_ai_client.py | 70 +++++++++++++++++++++++++++++ 5 files changed, 140 insertions(+), 9 deletions(-) create mode 100644 milestones/grok-stt-start-end.md diff --git a/milestones/README.md b/milestones/README.md index 0a1e466..9035343 100644 --- a/milestones/README.md +++ b/milestones/README.md @@ -5,11 +5,13 @@ repositories that install `docgen` and maintain their own demo bundle. The library no longer ships an in-repo dogfood; consumers are the integration test of record. -**Active:** **[validate-stream-probe.md](validate-stream-probe.md)** — -validate stream/drift checks must not trust ffprobe stdout when the -probe exits non-zero. +**Active:** **[grok-stt-start-end.md](grok-stt-start-end.md)** — +Grok STT word/segment `start` / `end` must be JSON numbers, not bools. **Shipped:** +- **[validate-stream-probe.md](validate-stream-probe.md)** — + validate stream/drift checks must not trust ffprobe stdout when the + probe exits non-zero (#145). - **[whisper-prompt-caps.md](whisper-prompt-caps.md)** — timing-enrichment whisper count caps must not coerce bools to 1 (#144). diff --git a/milestones/grok-stt-start-end.md b/milestones/grok-stt-start-end.md new file mode 100644 index 0000000..0e10a89 --- /dev/null +++ b/milestones/grok-stt-start-end.md @@ -0,0 +1,45 @@ +# Milestone: Grok STT start/end must be JSON numbers + +**Status:** Active +**PR:** (this PR) +**Depends on:** `milestones/validate-stream-probe.md` (PR #145), +`milestones/timing-start-end.md` (PR #134), +`milestones/tts-empty-audio.md` + +## Problem + +`load_bundle_timing` / Manim loaders already require JSON-number `start` / +`end` (#134). Grok STT ingest still does: + +```python +"start": float(w.get("start") or 0.0), +"end": float(w.get("end") or 0.0), +``` + +`bool` is a subclass of `int`: `start: true` becomes **1.0**. Missing / +null `start` waits at **0.0** and dumps the board. `duration: true` +becomes **1.0** for the no-words segment fallback. + +That corrupt payload is what `docgen timestamps --engine whisper` +writes into `timing.json` when `ai.provider` is grok. + +## Goal + +Present word / segment `start` / `end` and present `duration` must be +JSON numbers (not bool). Missing `duration` still falls back to the last +word end. Explicit `start: 0` / `duration: 0` stay 0. + +## Done when + +- [ ] Bool / missing / string word `start` raises `AIError` +- [ ] `start: 0.0` still maps to `0.0` +- [ ] Present `duration: true` raises `AIError` +- [ ] `ruff check src/ tests/` +- [ ] `pytest tests/` +- [ ] `docgen benchmark` (no clock change; meets baseline) + +## Out of scope + +- OpenAI whisper-1 SDK path (typed `.start` / `.end` attributes) +- Requiring `end >= start` +- `words` / `segments` list typing beyond skipping non-object rows diff --git a/milestones/validate-stream-probe.md b/milestones/validate-stream-probe.md index 0835ae2..d1c878e 100644 --- a/milestones/validate-stream-probe.md +++ b/milestones/validate-stream-probe.md @@ -1,6 +1,6 @@ # Milestone: validate stream/drift probes must honor ffprobe returncode -**Status:** Active +**Status:** Shipped **PR:** [#145](https://github.com/jmjava/documentation-generator/pull/145) **Depends on:** `milestones/whisper-prompt-caps.md` (PR #144), `milestones/pages-ffprobe-returncode.md` (PR #143), diff --git a/src/docgen/ai_client.py b/src/docgen/ai_client.py index 9704a5b..238a19b 100644 --- a/src/docgen/ai_client.py +++ b/src/docgen/ai_client.py @@ -632,6 +632,16 @@ def _grok_tts( output_path.write_bytes(body) +def _stt_json_number(value: Any, *, label: str) -> float: + """Require a JSON number so bools/strings do not become fake timestamps.""" + if isinstance(value, bool) or not isinstance(value, (int, float)): + raise AIError( + f"xAI STT {label} must be a JSON number, not {type(value).__name__} " + f"({value!r})" + ) + return float(value) + + def _grok_stt(audio_path: Path, settings: AISettings) -> dict[str, Any]: if not settings.api_key: raise AIError(f"xAI STT needs an API key. {settings.auth_help()}") @@ -658,12 +668,16 @@ def _grok_stt(audio_path: Path, settings: AISettings) -> dict[str, Any]: continue words.append( { - "start": float(w.get("start") or 0.0), - "end": float(w.get("end") or 0.0), + "start": _stt_json_number(w.get("start"), label="words[].start"), + "end": _stt_json_number(w.get("end"), label="words[].end"), "word": token, } ) - duration = float(parsed.get("duration") or (words[-1]["end"] if words else 0.0)) + raw_dur = parsed.get("duration") + if raw_dur is None: + duration = words[-1]["end"] if words else 0.0 + else: + duration = _stt_json_number(raw_dur, label="duration") segments = parsed.get("segments") if not isinstance(segments, list) or not segments: segments = ( @@ -674,8 +688,8 @@ def _grok_stt(audio_path: Path, settings: AISettings) -> dict[str, Any]: else: segments = [ { - "start": float(s.get("start") or 0.0), - "end": float(s.get("end") or 0.0), + "start": _stt_json_number(s.get("start"), label="segments[].start"), + "end": _stt_json_number(s.get("end"), label="segments[].end"), "text": str(s.get("text") or ""), } for s in segments diff --git a/tests/test_ai_client.py b/tests/test_ai_client.py index 01898d0..00b1cc3 100644 --- a/tests/test_ai_client.py +++ b/tests/test_ai_client.py @@ -238,6 +238,76 @@ def test_grok_stt_maps_words(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> assert result["segments"][0]["text"] == "Hello world" +def test_grok_stt_zero_start_is_not_replaced(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("DOCGEN_AI_PROVIDER", "grok") + monkeypatch.setenv("XAI_API_KEY", "xai-test") + cfg = _cfg(tmp_path, {}) + mp3 = tmp_path / "n.mp3" + mp3.write_bytes(b"fake-mp3") + body = json.dumps( + { + "text": "Hello", + "words": [{"text": "Hello", "start": 0.0, "end": 0.4}], + } + ).encode() + with patch("docgen.ai_client._http_with_retries", return_value=body): + result = transcribe_audio(mp3, cfg=cfg) + assert result["words"][0]["start"] == 0.0 + assert result["words"][0]["end"] == 0.4 + + +def test_grok_stt_bool_start_raises(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("DOCGEN_AI_PROVIDER", "grok") + monkeypatch.setenv("XAI_API_KEY", "xai-test") + cfg = _cfg(tmp_path, {}) + mp3 = tmp_path / "n.mp3" + mp3.write_bytes(b"fake-mp3") + body = json.dumps( + { + "text": "Hello", + "words": [{"text": "Hello", "start": True, "end": 0.4}], + } + ).encode() + with patch("docgen.ai_client._http_with_retries", return_value=body): + with pytest.raises(AIError, match="words\\[\\]\\.start must be a JSON number"): + transcribe_audio(mp3, cfg=cfg) + + +def test_grok_stt_missing_start_raises(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("DOCGEN_AI_PROVIDER", "grok") + monkeypatch.setenv("XAI_API_KEY", "xai-test") + cfg = _cfg(tmp_path, {}) + mp3 = tmp_path / "n.mp3" + mp3.write_bytes(b"fake-mp3") + body = json.dumps( + { + "text": "Hello", + "words": [{"text": "Hello", "end": 0.4}], + } + ).encode() + with patch("docgen.ai_client._http_with_retries", return_value=body): + with pytest.raises(AIError, match="words\\[\\]\\.start must be a JSON number"): + transcribe_audio(mp3, cfg=cfg) + + +def test_grok_stt_bool_duration_raises(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("DOCGEN_AI_PROVIDER", "grok") + monkeypatch.setenv("XAI_API_KEY", "xai-test") + cfg = _cfg(tmp_path, {}) + mp3 = tmp_path / "n.mp3" + mp3.write_bytes(b"fake-mp3") + body = json.dumps( + { + "text": "Hello", + "duration": True, + "words": [{"text": "Hello", "start": 0.0, "end": 0.4}], + } + ).encode() + with patch("docgen.ai_client._http_with_retries", return_value=body): + with pytest.raises(AIError, match="duration must be a JSON number"): + transcribe_audio(mp3, cfg=cfg) + + def test_unknown_provider_raises() -> None: with pytest.raises(ValueError, match="Unknown AI provider"): from docgen.ai_client import normalize_provider From 5259341883bd034b7317a27f9120bd1c368013ab Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 8 Sep 2026 01:00:58 +0000 Subject: [PATCH 2/2] Record PR #146 and local gate results for grok-stt-start-end ruff green; pytest 812 passed, 1 skipped; docgen benchmark meets baseline. Co-authored-by: jmjava --- milestones/grok-stt-start-end.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/milestones/grok-stt-start-end.md b/milestones/grok-stt-start-end.md index 0e10a89..9bbf3bc 100644 --- a/milestones/grok-stt-start-end.md +++ b/milestones/grok-stt-start-end.md @@ -1,7 +1,7 @@ # Milestone: Grok STT start/end must be JSON numbers **Status:** Active -**PR:** (this PR) +**PR:** [#146](https://github.com/jmjava/documentation-generator/pull/146) **Depends on:** `milestones/validate-stream-probe.md` (PR #145), `milestones/timing-start-end.md` (PR #134), `milestones/tts-empty-audio.md` @@ -31,12 +31,12 @@ word end. Explicit `start: 0` / `duration: 0` stay 0. ## Done when -- [ ] Bool / missing / string word `start` raises `AIError` -- [ ] `start: 0.0` still maps to `0.0` -- [ ] Present `duration: true` raises `AIError` -- [ ] `ruff check src/ tests/` -- [ ] `pytest tests/` -- [ ] `docgen benchmark` (no clock change; meets baseline) +- [x] Bool / missing / string word `start` raises `AIError` +- [x] `start: 0.0` still maps to `0.0` +- [x] Present `duration: true` raises `AIError` +- [x] `ruff check src/ tests/` +- [x] `pytest tests/` (812 passed, 1 skipped) +- [x] `docgen benchmark` (no clock change; meets baseline) ## Out of scope