feat(media): transcribe long recordings in windows and write them to a file (#6748 item 2) by nevgenov · Pull Request #6773 · librefang/librefang

nevgenov · 2026-08-08T22:16:19Z

Implements item 2 of #6748 (windowing + out_path). Item 1 ([media] timeout_secs) is yours; item 3 (measuring the size limit after extraction) is still open on the issue and is not in this branch.

The problem

media_transcribe transcribed whole files and returned the transcript inline, so two limits that have nothing to do with file size decided how long a usable recording could be:

a single transcription request is bounded by a wall-clock timeout that does not scale with the input;
the kernel spills any tool result over [tool_results] spill_threshold_bytes (16 KB by default) to the artifact store and hands the agent a stub.

Both are reached around ten minutes of speech, at roughly 2.5 MB of extracted audio — eight times under MAX_AUDIO_BYTES and twenty times under MAX_VIDEO_BYTES.
No size limit is anywhere near being involved.

Changes

Windowing. start_sec / max_secs bound the request to one window; the response carries has_more / next_start_sec to walk the rest.
extract_media_window cuts the span with -ss placed before -i, so ffmpeg seeks instead of decoding and discarding everything ahead of the window — on a 45-minute recording that is the difference between a constant-time seek and re-decoding the whole file per chunk.
The cut lands on the same Ogg/Opus target extract_video_audio_track already produces, which subsumes both the video-extraction and .oga re-mux branches for windowed calls.
out_path. Writes the transcript as UTF-8 through workspace_sandbox::resolve_sandbox_path_ext — the same resolution web_fetch_to_file and file_write use — and returns only path, byte count, sha256 and a 200-character preview.
A window starting at 0 truncates and later windows append, so repeated calls assemble one transcript without any of it passing through the agent's context.
The reported size and digest describe the whole file, not the last window's share, so a caller that walked five windows can verify what it ended up with.
Advancing by produced length, not requested length. ogg_opus_duration_secs reads the granule position of the final Ogg page.
Opus counts granules at a fixed 48 kHz regardless of encoder rate (RFC 7845 §4), so this is exact, and the pre-skip from OpusHead is subtracted so the error does not accumulate across chunks.
A seek lands on a keyframe and a window overlapping the end of the recording is short, so an assumed edge drifts and eventually skips audio.
Both mechanisms, not either. Window size varies with how much was said: a 600 s window measured ~16.8 KB of transcript against a 16 KB threshold, so windowing alone leaves the outcome dependent on speech density and out_path is what makes it deterministic.
Schema and docs. The tool description now tells the agent to window long recordings and write them to a file; docs/src/app/agent/tools (en + zh) gained the new parameters and a worked example of the loop.

Compatibility

A call that names neither window field takes the previous path unchanged, including adding no ffmpeg pass.
transcribe_audio keeps its signature and delegates to the new transcribe_audio_window, so all seven existing call sites are untouched.
Continuation fields are emitted only for windowed calls — offering has_more on a whole-file result would invite a caller to loop on a transcript that is already complete.

Why not `ffprobe`

Duration is read out of the produced bytes rather than probed.
ffprobe is called nowhere in the kernel today, so depending on it would make it a new deployment requirement on every host for one number the Ogg container already carries.

Verification

cargo test -p librefang-runtime-media --lib — 94 passed, 4 new: granule-position duration against 1 s and 7.5 s streams, None for non-Ogg input, a window cut from the middle, and a window overlapping the end coming back short.
The ffmpeg-gated ones were re-run with --nocapture to confirm they actually execute rather than self-skip.
cargo test -p librefang-runtime --lib — 2180 passed, 7 new: window parsing and defaults, rejection of spans that cannot describe a window, character-boundary preview truncation (a byte-based cut would split a codepoint in exactly the non-Latin recordings this targets), append/truncate assembly, and that out_path cannot escape the workspace.
cargo check --workspace --lib — clean.
cargo clippy -p librefang-runtime -p librefang-runtime-media --all-targets -- -D warnings — clean.
cargo fmt --all -- --check — clean.

Not verified: no live provider round-trip. The transcription call itself is unchanged by this branch — only what is handed to it and what is done with the result.

One measurement worth carrying into item 1

The default window is 600 s because that is the largest round number that stays inside the tool-result budget on ordinary speech.
But on a self-hosted whisper-large-v3-turbo endpoint that same window took ~72 s, which is already past the current 60 s transcription timeout.
So the window default and whatever default [media] timeout_secs gets have to be chosen together — a 10-minute window is only a sane default on provider hardware fast enough to finish it in time.

Review rounds so far

Two rounds landed on the draft; both turned up real defects, recorded here so the history is legible rather than buried in resolved threads.

Appended windows fused words at every boundary. Each window's transcript arrives trimmed and they were concatenated with nothing between them, so "and then we" + "discussed the budget" wrote "and then wediscussed the budget". A window edge lands mid-sentence by design, so this was the common case. Windows are now newline-separated.
An unreadable window edge could make the walk non-terminating. consumed_secs: None used to fall back to max_secs, which reads as "full window, there is more" — the caller would advance past the end, get the same unreadable shape, and loop. Unknown now stops the walk, and consumed_secs is reported as null so "recording ended" and "edge unknown" stay distinguishable.
The rebase onto fix(media): stage video input to a file so mp4/mov audio extraction can seek #6751 turned up the important one. extract_media_window still fed the container on pipe:0 — exactly the shape fix(media): stage video input to a file so mp4/mov audio extraction can seek #6751 had just fixed — so windowed transcription of an ordinary mp4 would have reintroduced media_transcribe silently returns an empty audio track for .mp4 / .mov #6747 on a path that fix did not cover. The window now stages to a ScopedTempFile, which also makes -ss a real seek, and carries the same ogg_contains_audio guard.
The Ogg parser scanned for raw OggS magic. Opus payloads are entropy-coded, so those four bytes occur inside real audio by chance, and eight bytes of compressed payload read as a granule position would produce a plausible but wrong duration — the exact drift this contract exists to prevent. Pages are now walked by their declared lengths.
has_more / next_start_sec could disagree for a max_secs under the completeness tolerance; the condition and its test both now treat the two fields as one invariant.

One test-quality note worth stating plainly: the first version of the decoy-magic regression test did not fail under the mutation it was written for — the decoy sat too close to the end of the buffer for a naive reader to reach it, so it passed for the wrong reason. It now carries trailing padding and reports 3600s against a 1s stream when the fix is reverted.

Still open

Item 3 of Transcription of long recordings: hard-coded timeout and un-chunked results block media far below every size limit #6748 (measuring the size limit after extraction) is deliberately absent. The issue asks whether you want it folded in here or kept separate; this branch assumes separate until you say otherwise, and it is a small change in the same file either way.
Partial writes on append are documented rather than rolled back: buffering the assembled transcript to rewrite it atomically would restore, on disk, the proportional-to-recording-length cost out_path exists to remove. file_bytes / file_sha256 make a short artefact detectable, and the documented recovery is to restart the walk from start_sec = 0, which truncates.

houko

Automated daily review pass. Draft PR, item 2 of #6748 — reviewed against CLAUDE.md's changelog-fragment, testing, and prose-wrapping conventions plus the chunking/windowing correctness itself. Four findings left inline, none blocking the draft status; not approving/requesting changes per the automation's scope.

Generated by Claude Code

houko · 2026-08-09T01:28:26Z

+Windows starting at `0` begin a new file and later windows append, so repeated calls assemble one transcript without any of it passing through the agent's context.
+Both mechanisms are needed rather than either alone: window size varies with how much was said, so a fixed window straddles the spill threshold instead of staying under it, and `out_path` is what makes the outcome independent of that.
+Callers advance by the produced window length rather than the requested one, read back from the Ogg granule position — a seek lands on a keyframe and a window overlapping the end of the recording is short, so an assumed edge drifts and eventually skips audio.
+A call that names neither window field keeps its previous behaviour exactly, including adding no ffmpeg pass. (#6748) (@nevgenov)


Per changelog.d/README.md, the trailing (#N) group is what suppresses the auto-generated release-notes line for the PR that merges — "Only the last (#N) group on the bullet's last non-empty line counts... Every merged PR in the range gets a generated line unless its number appears in [that] group."

This bullet ends (#6748), which is the tracking issue, not this PR (#6773). When #6773 merges, the release flow won't recognize this fragment as covering it, so the curated prose here and a generated - feat(media): transcribe long recordings in windows and write them to a file (#6748 item 2) (#6773) (@nevgenov) line will both land in the release notes.

Worth changing the ending to (#6773) (@nevgenov) (or (#6748, #6773) (@nevgenov) if you want the issue referenced too) before this leaves draft.

Generated by Claude Code

houko · 2026-08-09T01:28:30Z

+        // lands it on the same Ogg/Opus target the two branches below produce,
+        // so neither has anything left to do. Handled first for that reason —
+        // running either of them before this would decode the whole recording
+        // just to throw most of it away.


CLAUDE.md's prose-wrapping rule ("no column limit; break only at sentence boundaries") applies to "any multi-line free-form prose comment block," not just /// doc comments — this repo's /// blocks elsewhere in this same PR correctly use one-line-per-sentence, but this new // block (and several others added here, e.g. lines 262–264, 266–272, 274–277 in tool_runner/media.rs, and lines 1304–1306, 1314–1316, 1324–1325, 1332–1333 in this file) hard-wraps at a fixed column and breaks mid-sentence instead.

Not a functional issue, just flagging so it doesn't get carried forward as the pattern for the rest of this feature — worth a pass to join these back to one-sentence-per-line before merge.

Generated by Claude Code

houko · 2026-08-09T01:28:38Z

+        // unreadable keeps the walk moving rather than stalling it — at worst
+        // it repeats or skips one window, which the caller can see in the
+        // transcript, whereas a stall is silent.
+        let consumed = outcome.consumed_secs.unwrap_or(window.max_secs);


This fallback reads backwards from a safety standpoint. consumed_secs comes back None from ogg_opus_duration_secs when the produced Ogg's last page carries no readable/finished granule (malformed trailing page, or a page whose granule is the u64::MAX "no packet finishes here" sentinel) — and the situation most likely to produce exactly that is a window cut at or past the true end of the recording, where ffmpeg's output degenerates to header-only or truncated bytes.

Falling back to window.max_secs in that case makes has_more (line 278) come back true, i.e. "assume the whole window was consumed, there's more after this." If a stream ever produces this shape at the tail — rather than the Some(~0.0) that a header-only page with granule 0 would give you, which does terminate correctly — every subsequent call re-extracts a window that's still past EOF, gets the same unreadable shape again, and next_start_sec keeps advancing by a full max_secs forever. Nothing bounds the walk in that branch.

Given the doc comment already reasons through "repeats or skips one window" as the accepted cost, it might be worth also handling "the walk doesn't stop" — e.g. defaulting the unreadable case to has_more = false (safe-stop bias) or surfacing it as a distinct field the caller can act on, rather than silently assuming full consumption.

Generated by Claude Code

houko · 2026-08-09T01:28:43Z

+            "max_secs": window.max_secs,
+            "consumed_secs": consumed,
+        });
+        response["has_more"] = serde_json::json!(has_more);


Per CLAUDE.md's integration-testing rule ("Always write integration tests at the injection site, not just the implementation site"): transcribe_window_tests covers parse_window, preview_of, and write_transcript well, but the has_more / next_start_sec computation here — the actual walk-termination logic, and per the sibling comment on this thread the part most exposed to an edge case — has no test at all, presumably because it's inline in a function that needs a live MediaEngine.

Pulling it into a small pure helper (e.g. fn window_continuation(window: MediaWindow, consumed_secs: Option<f64>) -> (bool, Option<f64>)) would make it unit-testable without a provider round-trip, and it's exactly the kind of boundary-condition logic (exact-length window, one-tick-short window, None consumed_secs) that benefits most from a table test.

Generated by Claude Code

houko

Automated review pass. Path-traversal on out_path looks handled correctly (goes through resolve_sandbox_path_ext, and it's tested — out_path_cannot_escape_the_workspace). Main finding is a real functional bug in how appended windows are joined (no separator, combined with the pre-existing .trim()), plus a few smaller robustness/test-coverage/docs nits, including two spots that don't follow the repo's one-sentence-per-line prose rule. Nothing here should block further draft iteration; flagging the window-join issue as the one worth fixing before this leaves draft, since it affects the correctness of the assembled transcript that's the whole point of out_path.

Generated by Claude Code

houko · 2026-08-09T01:58:03Z

+            response["transcript"] = serde_json::json!(transcript);
+        }
+        Some(out_path) => {
+            let written = write_transcript(


Consecutive windows are appended with no separator, and the per-window transcript is .trim()ed before it gets here (media_understanding.rs:436), which strips exactly the whitespace that would otherwise separate two chunks.

A window boundary is very likely to land mid-sentence or mid-word by design (that's the whole point of has_more/next_start_sec), so the common case — not just an edge case — is: window N's transcript ends "...and then we", window N+1's starts "discussed the budget", and write_transcript concatenates them byte-for-byte into "...and then wediscussed the budget" on disk. This directly contradicts the "functional concern" the PR description otherwise reasons carefully about (keyframe drift, granule-accurate advancing) — the assembled artifact can come out with words fused together at every single window boundary, not just occasionally.

Suggest inserting a single space (or newline) between chunks when appending (skip it only for the very first write), or at minimum note the caveat in the tool description/docs so a caller knows to post-process.

Generated by Claude Code

houko · 2026-08-09T01:58:08Z

+        .truncate(!append)
+        .open(&resolved)
+        .await
+        .map_err(|e| ToolError::upstream_msg(format!("Failed to open '{out_path}': {e}")))?;


OpenOptions::open with truncate(!append) truncates at open time, before any bytes are written. If write_all (line 435) or flush (line 438) then fails partway — disk full, I/O error — on an append call, the file is left with whatever partial bytes made it to disk with no rollback, and there's no temp-file+rename or similar safeguard.

Given the intended usage pattern is "call again with the same start_sec on failure" (per the retry reasoning already written into the has_more fallback comment above), a retry after a partial-write failure would append the full transcript again after the corrupted partial bytes, silently producing a mangled transcript rather than a clean retry. Worth at least documenting the limitation, or writing to a temp file in the same directory and renaming into place for the truncate (fresh-window) case, where atomicity is straightforward.

Not blocking for a draft, but worth tracking before this leaves draft since the feature exists specifically to make long-running, multi-call assembly reliable.

Generated by Claude Code

houko · 2026-08-09T01:58:11Z

+            response["written_to"] = serde_json::json!(written.path);
+            response["bytes_written"] = serde_json::json!(written.bytes);
+            response["sha256"] = serde_json::json!(written.sha256);
+            response["chars"] = serde_json::json!(transcript.chars().count());


Minor clarity nit: bytes_written and sha256 describe the whole assembled file (per write_transcript's doc comment), but chars (and preview) describe only the current window's transcript. An agent reading this response has no way to tell those two are different scopes just from the field names — worth a distinct name like window_chars (mirroring window.consumed_secs) or a doc-string update on the tool schema to call out the scope difference explicitly.

Generated by Claude Code

houko · 2026-08-09T01:58:15Z

+        // on a packet boundary, so an exactly-covered window still lands a few
+        // milliseconds short and a strict comparison would call every window
+        // the last one.
+        let has_more = consumed >= window.max_secs - WINDOW_COMPLETE_TOLERANCE_SECS;


The has_more/next_start_sec assembly (lines 265-287) is the part of this change most likely to have an off-by-one, and it isn't unit-tested directly — it's only reachable through tool_media_transcribe, which needs a live MediaEngine. parse_window, preview_of, and write_transcript all got extracted and unit-tested; consider doing the same here (a small fn window_advance(window, consumed) -> (bool, Option<f64>)-shaped helper) so the tolerance boundary and the consumed_secs == None fallback path get direct coverage instead of only indirect exercise through a live transcription call.

Non-blocking for a draft, flagging since it's the one branchy bit of new logic without a test.

Generated by Claude Code

houko · 2026-08-09T01:58:18Z

    /// ElevenLabs, …). There is no runtime cascade; a provider failure
    /// surfaces as `Err` to the caller.
+    ///
+    /// Transcribes the whole file. Callers that need to bound the request — anything driven by a recording whose length they do not control — want [`Self::transcribe_audio_window`] instead (#6748).


Repo CLAUDE.md prose-wrapping rule: one sentence per line in doc comments. This line has two ("Transcribes the whole file." + "Callers that need..."). Same issue at line 1248 (Requested length. The produced window is shorter...) and in docs/src/app/agent/tools/page.mdx:229 (Lower \max_secs` if a call times out. How much media fits...`). Mechanical fix, just needs a line break after each first sentence.

Generated by Claude Code

…ble edge Addresses the review on librefang#6773. Two of the findings were real defects in the first pass; the rest are naming, coverage and prose. **Appended windows fused words at every boundary.** Each window's transcript arrives trimmed (`media_understanding.rs`, right after dispatch) and windows were concatenated with nothing between them, so window N ending "and then we" followed by window N+1 starting "discussed the budget" wrote "and then wediscussed the budget" to disk. A window boundary lands mid-sentence by design, so this was the common case rather than an edge case, and it corrupted the assembled artefact that `out_path` exists to produce. Windows are now separated by a newline — visible to a reader, and inserting nothing that was not spoken, so the file stays a transcript rather than a rendering of one. Skipped ahead of the first window and when the target file is absent or empty, so a transcript never opens with a stray break. **An unreadable window edge could make the walk non-terminating.** When `ogg_opus_duration_secs` returns `None` the produced stream carried no usable granule position, and the shape most likely to produce that is a window cut at or past the true end of the recording. The old fallback substituted `max_secs`, which reads as "the window was full, there is more" — so the caller would advance past the end, get the same unreadable shape again, and loop forever. Unknown now stops the walk: at worst that costs a short read the caller can see in the transcript, which is strictly better than a walk that cannot end. `consumed_secs` is reported as null rather than a substituted number so the distinction between "recording ended" and "edge unknown" is visible. **Walk termination is now unit-testable.** The `has_more` / `next_start_sec` computation moved out of `tool_media_transcribe` into `window_continuation`, which needs no live provider, and is covered by a table over the branches that decide whether a caller stops, loops, or skips audio: full window, short-by-less-than-one-packet, genuinely short, unknown duration, and a window that produced nothing. **Response field scopes are now in the names.** `bytes_written` / `sha256` described the whole assembled file while `chars` / `preview` described only the current window, with nothing in the names to say so. They are now `file_bytes` / `file_sha256` and `window_chars` / `window_preview`, and the tool schema spells out the two scopes. **Partial writes.** Documented rather than fixed: buffering the assembled transcript to rewrite it atomically would restore, on disk, the proportional-to-recording-length cost the parameter exists to remove. What the caller gets instead is detection — `file_bytes` / `file_sha256` describe the file as it now stands — and the documented recovery is to restart the walk from `start_sec = 0`, which truncates, rather than retrying the failed window onto partial bytes. Also: the changelog fragment's trailing group named the tracking issue rather than this PR, so the release flow would have emitted a generated line beside it; it now ends `(librefang#6748, librefang#6773)`. Prose comment blocks flagged in review are rewrapped to one sentence per line — the rule covers `//` blocks, not only doc comments, and the first pass only applied it to the latter. Verification: - mutation check on both defects: removing the separator fails `windows_append_into_one_transcript`, and restoring the `unwrap_or(max_secs)` fallback fails `window_continuation_decides_when_the_walk_stops`. Each fails alone, so both fixes are individually pinned. - cargo test -p librefang-runtime --lib: 2182 passed (9 in the window suite, 2 new) - cargo test -p librefang-runtime-media --lib: 94 passed - cargo check (workspace, lib), clippy on both crates with --all-targets -D warnings, cargo fmt check: clean

houko

Mechanical CLAUDE.md-compliance pass (draft-appropriate scope only — no incomplete-feature nits).

Two inline findings on the prose-wrapping rule ("no column limit; break only at sentence boundaries"), see line comments.

One more of the same kind with no diff line to anchor to: the bodies of commits 95aacd8 and 17c1c8c are hard-wrapped at a fixed column (e.g. "Addresses the review on #6773. Two of the findings were real defects in the\nfirst pass; the rest are naming, coverage and prose.") rather than broken at sentence boundaries, which the rule explicitly calls out as applying to "Commit message bodies." Not actionable without rewriting history mid-review, flagging for awareness before this lands.

Everything else checked out: no AI/Claude/Anthropic attribution in commits or docs, commit format is conventional with no Co-Authored-By footer, and the changelog fragment (changelog.d/added/6748-transcribe-windowing-and-out-path.md) is correctly formed — new file, no leading - , one sentence per line, ends (#6748, #6773) (@nevgenov). The out_path write path goes through resolve_sandbox_path_ext with a dedicated escape test (out_path_cannot_escape_the_workspace), so no path-traversal concern.

Generated by Claude Code

houko · 2026-08-10T11:12:50Z

+    // Both are read back out of the produced bytes rather than assumed, because a seek lands on a keyframe and a window past the end of the media is short.
+
+    /// Generate `secs` seconds of tone as an Ogg/Opus stream at the same
+    /// 48 kHz mono target the extraction paths produce.


CLAUDE.md prose-wrapping rule ("no column limit; break only at sentence boundaries") — this doc comment is one sentence hard-wrapped across two lines at a fixed column rather than broken at a sentence boundary:

/// Generate `secs` seconds of tone as an Ogg/Opus stream at the same /// 48 kHz mono target the extraction paths produce.

Same pattern recurs at lines 2380-2381 (Guards the fallback contract: … / None rather than …) and 2413-2415 (The end-of-recording signal: … / short, and … / what tells the caller to stop.). Worth a second pass since the dedicated rewrap commit (378176d) covers this file's other test-module blocks but these three were missed. Suggested fix: put each sentence on its own line, however long.

Generated by Claude Code

houko · 2026-08-10T11:13:00Z

+
+    Ok(serde_json::to_string_pretty(&response)?)
+}
+


CLAUDE.md prose-wrapping rule — same issue as flagged in media_understanding.rs, but throughout the new transcribe_window_tests module here (which the rewrap commit didn't touch, since that module is new to this PR, not carried over from before). Examples of one sentence hard-wrapped at a fixed column instead of broken at sentence boundaries:

L305-306: A call that names no window field stays on the whole-file path, so / every pre-#6748 caller keeps its exact behaviour and gains no ffmpeg hop.

L322-324, L358-360, L372-375, L415-416, L456-458, L463-464, L469-471, L494-496: same pattern.

Rest of the diff (production code doc comments) already follows the one-sentence-per-line convention correctly — it's specifically this test module that reverts to fixed-column wrapping. Suggested fix: reflow each of these blocks so each sentence is its own line regardless of length.

Generated by Claude Code

houko · 2026-08-10T11:28:31Z

+
+```
+media_transcribe(path="meeting.mp4", max_secs=600, out_path="meeting.txt")
+→ { "written_to": "...", "has_more": true, "next_start_sec": 600.0, "preview": "..." }


This example response still uses "preview", but the rename commit (95aacd8) changed the actual field to window_preview (alongside file_bytes/file_sha256/window_chars) — the tool schema description in definitions.rs already reflects the new name, this worked example just wasn't updated alongside it. As written, an agent following this doc would look for a preview key that the response never contains.

Generated by Claude Code

houko · 2026-08-10T11:28:35Z

+    let has_more = consumed >= window.max_secs - WINDOW_COMPLETE_TOLERANCE_SECS;
+    WindowContinuation {
+        has_more,
+        // A window that produced nothing cannot advance the cursor; treating it as progress would spin on the same offset.
+        next_start_sec: (has_more && consumed > 0.0).then_some(window.start_sec + consumed),


For a max_secs smaller than WINDOW_COMPLETE_TOLERANCE_SECS (0.25) — which parse_window permits, it only rejects max_secs <= 0.0 — this can report has_more: true with no next_start_sec. E.g. max_secs = 0.1, consumed = 0.0: has_more = 0.0 >= 0.1 - 0.25 is true, but next_start_sec requires consumed > 0.0 (line 344), so it comes back None and the field is omitted from the response entirely (it's only set if let Some(next) = ... in the caller). The response then tells the agent there's more to transcribe with nowhere to resume from.

window_continuation_decides_when_the_walk_stops's "produced nothing" case (w(0.1), Some(0.0)) only asserts next_start_sec == None, not has_more, so this doesn't surface as a test failure. Might be worth requiring consumed > 0.0 in the has_more condition itself, not just in the next_start_sec derivation, so the two fields can't disagree.

Generated by Claude Code

houko · 2026-08-10T12:09:24Z

+
+```
+media_transcribe(path="meeting.mp4", max_secs=600, out_path="meeting.txt")
+→ { "written_to": "...", "has_more": true, "next_start_sec": 600.0, "preview": "..." }


The worked example shows the response key as "preview", but the actual field name tool_media_transcribe sets (in tool_runner/media.rs) is window_preview (alongside window_chars), not preview. Worth fixing so the doc matches what a caller actually gets back:

→ { "written_to": "...", "has_more": true, "next_start_sec": 600.0, "window_preview": "..." }

Generated by Claude Code

houko

Checked the current HEAD (378176d) against the existing review threads to avoid duplicating the 7 still-open findings from prior passes (window-join separator — already fixed in 95aacd8; non-atomic partial write; two prose-wrap spots; the has_more/next_start_sec disagreement for sub-tolerance max_secs; and the page.mdx preview→window_preview doc mismatch, already flagged twice). Path-traversal handling on out_path (resolve_sandbox_path_ext, tested), resource bounds (ffmpeg's existing 30s wall-clock cap plus MAX_AUDIO_BYTES/MAX_VIDEO_BYTES already bound window extraction regardless of requested max_secs), and ogg_opus_duration_secs's parsing of malformed/truncated Ogg pages all look sound and don't panic. One new finding below: the Chinese docs translation grew a "see below" reference to a worked long-recording example that was only added to the English page.

Generated by Claude Code

houko · 2026-08-10T13:18:09Z

 | `image_analyze(image, question)` | 对已有图像做 vision Q&A。 |
 | `media_describe(image_or_video)` | 给已有媒体素材生成 caption / 描述。 |
-| `media_transcribe(audio_or_video, language?, prompt?)` | 音频转文本；也接受视频容器,服务端会提取其中的音轨。无法识别的扩展名会被拒绝。 |
+| `media_transcribe(audio_or_video, language?, prompt?, start_sec?, max_secs?, out_path?)` | 音频转文本；也接受视频容器,服务端会提取其中的音轨。无法识别的扩展名会被拒绝。`start_sec` / `max_secs` 只转写一个时间窗口,并在响应中返回 `has_more` / `next_start_sec` 以便继续；`out_path` 会把转写结果写入工作区文件,只返回路径、字节数、sha256 和一段预览。长录音两者都需要,详见下文。 |


This row's new sentence ends "长录音两者都需要,详见下文。" ("Both are needed for long recordings, see below"), promising a walkthrough further down the page the way the English page.mdx got its new "### Transcribing a long recording" section (lines 211-231 there, with the worked media_transcribe(...) example).

No matching section exists in this file — the page goes straight from this table (ending at line 210) to ## Owner 通知 at line 213. A Chinese-reading caller following "详见下文" finds nothing to see. Either add the equivalent section here or drop "详见下文" from the row until the walkthrough is translated.

Generated by Claude Code

houko

Automated review — 1 finding (unvalidated Ogg page-magic scan in ogg_opus_duration_secs can misread a coincidental byte match inside compressed audio as a page header, silently corrupting the computed window duration). Otherwise this is a thoroughly tested, well-reasoned change (windowing, out_path sandboxing, append/truncate semantics, changelog fragment) with no other CLAUDE.md violations spotted.

Generated by Claude Code

houko · 2026-08-10T13:43:48Z

+    // Scan forward for the last capture pattern rather than seeking back from the end: page sizes vary with the segment table, so the final page's offset cannot be computed, only found.
+    let mut last_granule: Option<u64> = None;
+    let mut at = 0usize;
+    while let Some(found) = ogg[at..]


ogg_opus_duration_secs finds page boundaries by scanning for the raw 4-byte magic OggS in the byte stream, with no page-structure or CRC validation. The Ogg spec's own answer to exactly this — a compressed Opus packet's entropy-coded payload coincidentally containing the byte sequence 4F 67 67 53 — is the per-page CRC32 checksum, which this scan never checks. A coincidental match inside real audio payload (roughly file_size/2^32 odds, so unlikely on one file but not negligible at scale) would be read as a page header, and the "granule position" bytes taken from that position would be noise from the compressed audio rather than an actual granule value, silently producing a wrong consumed_secs and misleading next_start_sec / has_more for the caller — the exact class of "windows drift and audio gets skipped/duplicated" failure this feature exists to prevent. Since the function already walks every match to find the last one, validating that a candidate page has a plausible header (segment table length, or reconstructing/checking the CRC) before accepting its granule value would close this without needing ffprobe.

Generated by Claude Code

…a file Refs librefang#6748. `media_transcribe` transcribed whole files and returned the transcript inline, so two limits that have nothing to do with file size decided how long a usable recording could be: - a single transcription request is bounded by a wall-clock timeout that does not scale with the input; - the kernel spills any tool result over `[tool_results] spill_threshold_bytes` (16 KB by default) to the artifact store and hands the agent a stub. Both are reached around ten minutes of speech, at roughly 2.5 MB of extracted audio — eight times under `MAX_AUDIO_BYTES` and twenty times under `MAX_VIDEO_BYTES`, so no size limit is anywhere near being involved. ## Changes **Windowing.** `start_sec` / `max_secs` bound the request to one window; the response carries `has_more` / `next_start_sec` to walk the rest. `extract_media_window` cuts the span with `-ss` before `-i` so ffmpeg seeks rather than decoding and discarding everything ahead of the window, and lands it on the same Ogg/Opus target the video-extraction path already produces — which subsumes both the video-extraction and `.oga` re-mux branches for windowed calls. **`out_path`.** Writes the transcript as UTF-8 through `workspace_sandbox::resolve_sandbox_path_ext` — the same resolution `web_fetch_to_file` and `file_write` use — and returns only the path, byte count, sha256 and a 200-character preview. A window starting at `0` truncates, later windows append, so repeated calls assemble one transcript without any of it passing through the agent's context. The reported size and digest describe the whole file rather than the last window's share. Both are needed rather than either alone: window size varies with how much was said, so a fixed window straddles the spill threshold instead of staying under it. **Advancing by produced length, not requested length.** `ogg_opus_duration_secs` reads the granule position of the final Ogg page — Opus counts granules at a fixed 48 kHz regardless of encoder rate (RFC 7845 §4), so this is exact. A seek lands on a keyframe and a window overlapping the end is short, so an assumed edge drifts and eventually skips audio. Read out of the bytes rather than probed with `ffprobe`, which is called nowhere else in the kernel and would become a new deployment requirement for one number the container already carries. **Compatibility.** A call naming neither window field takes the previous path unchanged, including adding no ffmpeg pass. `transcribe_audio` keeps its signature and delegates to the new `transcribe_audio_window`, so all seven existing call sites are untouched. ## Verification - `cargo test -p librefang-runtime-media --lib` — 94 passed (4 new). The ffmpeg-gated ones were confirmed to actually execute rather than self-skip. - `cargo test -p librefang-runtime --lib` — 2180 passed (7 new), covering window parsing and defaults, rejection of spans that cannot describe a window, character-boundary preview truncation, append/truncate assembly, and that `out_path` cannot escape the workspace. - `cargo check --workspace --lib`, `cargo clippy` on both crates with `--all-targets -- -D warnings`, `cargo fmt --check` — all clean. ## Notes for review `extract_media_window` sits next to `extract_video_audio_track`, which librefang#6751 also rewrites; this branch is based on `main` and expects to rebase once that lands. The default window is 600 s because that is the largest round number that stays inside the tool-result budget on ordinary speech — a 600 s window measured ~16.8 KB of transcript against a 16 KB threshold. On a self-hosted endpoint the same window took ~72 s, which is already past the 60 s transcription timeout, so the window default and any future timeout default have to be chosen together rather than separately.

…ble edge Addresses the review on librefang#6773. Two of the findings were real defects in the first pass; the rest are naming, coverage and prose. **Appended windows fused words at every boundary.** Each window's transcript arrives trimmed (`media_understanding.rs`, right after dispatch) and windows were concatenated with nothing between them, so window N ending "and then we" followed by window N+1 starting "discussed the budget" wrote "and then wediscussed the budget" to disk. A window boundary lands mid-sentence by design, so this was the common case rather than an edge case, and it corrupted the assembled artefact that `out_path` exists to produce. Windows are now separated by a newline — visible to a reader, and inserting nothing that was not spoken, so the file stays a transcript rather than a rendering of one. Skipped ahead of the first window and when the target file is absent or empty, so a transcript never opens with a stray break. **An unreadable window edge could make the walk non-terminating.** When `ogg_opus_duration_secs` returns `None` the produced stream carried no usable granule position, and the shape most likely to produce that is a window cut at or past the true end of the recording. The old fallback substituted `max_secs`, which reads as "the window was full, there is more" — so the caller would advance past the end, get the same unreadable shape again, and loop forever. Unknown now stops the walk: at worst that costs a short read the caller can see in the transcript, which is strictly better than a walk that cannot end. `consumed_secs` is reported as null rather than a substituted number so the distinction between "recording ended" and "edge unknown" is visible. **Walk termination is now unit-testable.** The `has_more` / `next_start_sec` computation moved out of `tool_media_transcribe` into `window_continuation`, which needs no live provider, and is covered by a table over the branches that decide whether a caller stops, loops, or skips audio: full window, short-by-less-than-one-packet, genuinely short, unknown duration, and a window that produced nothing. **Response field scopes are now in the names.** `bytes_written` / `sha256` described the whole assembled file while `chars` / `preview` described only the current window, with nothing in the names to say so. They are now `file_bytes` / `file_sha256` and `window_chars` / `window_preview`, and the tool schema spells out the two scopes. **Partial writes.** Documented rather than fixed: buffering the assembled transcript to rewrite it atomically would restore, on disk, the proportional-to-recording-length cost the parameter exists to remove. What the caller gets instead is detection — `file_bytes` / `file_sha256` describe the file as it now stands — and the documented recovery is to restart the walk from `start_sec = 0`, which truncates, rather than retrying the failed window onto partial bytes. Also: the changelog fragment's trailing group named the tracking issue rather than this PR, so the release flow would have emitted a generated line beside it; it now ends `(librefang#6748, librefang#6773)`. Prose comment blocks flagged in review are rewrapped to one sentence per line — the rule covers `//` blocks, not only doc comments, and the first pass only applied it to the latter. Verification: - mutation check on both defects: removing the separator fails `windows_append_into_one_transcript`, and restoring the `unwrap_or(max_secs)` fallback fails `window_continuation_decides_when_the_walk_stops`. Each fails alone, so both fixes are individually pinned. - cargo test -p librefang-runtime --lib: 2182 passed (9 in the window suite, 2 new) - cargo test -p librefang-runtime-media --lib: 94 passed - cargo check (workspace, lib), clippy on both crates with --all-targets -D warnings, cargo fmt check: clean

… per line Three prose blocks in the new test modules were missed by the previous pass: the module note above the windowing tests in `media_understanding.rs`, and the doc comments on `windows_append_into_one_transcript` and `window_continuation_decides_when_the_walk_stops`. The rule covers any multi-line prose comment block, and these were written after the earlier rewrap rather than before it. Verified by scanning the whole branch diff for added comment lines carrying two sentences; the only remaining hit is a markdown table cell in `docs/src/app/agent/tools`, where a line break would break the table.

Rebased onto main now that librefang#6751 has landed, and addresses the second round of review on librefang#6773. The rebase turned up the important one. `extract_media_window` still fed the container on `pipe:0`, which is exactly the shape librefang#6751 just fixed for `extract_video_audio_track`: a pipe cannot seek, `mp4` / `mov` keep their index in a trailing `moov` atom, and the failed demux still exits 0 while emitting a headers-only Ogg. Windowed transcription of an ordinary mp4 would therefore have reintroduced librefang#6747 on a path the fix did not cover. The window now stages to a `ScopedTempFile` and passes a path, which also makes `-ss` a real seek rather than a decode of everything ahead of the window, and it gains the same `ogg_contains_audio` guard. That guard earns its place twice here: a window starting past the end of the recording is a legitimate request and produces bytes indistinguishable from a failed demux, so rejecting both keeps an empty window from being uploaded and charged for. `ogg_opus_duration_secs` no longer scans for the raw `OggS` magic. Opus payloads are entropy-coded, so the four bytes occur inside real audio by chance; taking eight bytes of compressed payload as a granule position produces a plausible but wrong duration, and the caller advances by that number — the precise "windows drift and audio is skipped" failure the produced-duration contract exists to prevent. Pages are now walked by following each page's declared length (header + segment table + the table's sum), so a candidate is only accepted where the previous page ends and payload bytes are never read as a header. A truncated tail stops the walk and reports the last real granule rather than discarding a usable answer. `has_more` and `next_start_sec` could disagree. For a `max_secs` below `WINDOW_COMPLETE_TOLERANCE_SECS` — which `parse_window` permits, it only rejects zero — a window that produced nothing still satisfied the tolerance comparison, so the response said there was more with nowhere to resume from. `consumed > 0.0` is now part of the `has_more` condition itself rather than only of the cursor, and the test asserts the two fields agree across every case rather than checking them separately, which is what let them drift apart. Docs: the English worked example still showed the pre-rename `preview` key, and the Chinese table row promised a walkthrough ("详见下文") that only existed on the English page. Both fixed — the Chinese page now carries the translated section. Remaining prose blocks in the two new test modules are rewrapped to one sentence per line. Verification: - mutation check on the Ogg parser: restoring the raw magic scan makes `ogg_opus_duration_ignores_page_magic_inside_a_payload` report 3600s where the stream is 1s, and the test fails. The first version of that test did not fail under the same mutation — the decoy sat too close to the end of the buffer for a naive reader to take it for a header, so it was passing for the wrong reason. It now carries trailing padding. - cargo test -p librefang-runtime-media --lib: 100 passed - cargo test -p librefang-runtime --lib: 2216 passed - cargo check (workspace, lib), clippy on both crates with --all-targets -D warnings, cargo fmt check: clean

houko

Automated review: this is a well-tested, carefully-reasoned PR (windowing math, keyframe-drift awareness, path-traversal reuse via resolve_sandbox_path_ext, UTF-8-safe preview truncation, changelog fragment all present and correct). One boundary-math finding on the has_more completeness check for very short windows.

Generated by Claude Code

houko · 2026-08-10T17:22:37Z

+    // `consumed > 0.0` is part of the condition rather than only of the cursor below.
+    // For a `max_secs` under the tolerance — which `parse_window` permits, it only rejects zero — the tolerance alone would call a window that produced *nothing* full, and the caller would be told there is more with nowhere to resume from.
+    // The two fields have to agree: "there is more" is only meaningful alongside a place to continue.
+    let has_more = consumed > 0.0 && consumed >= window.max_secs - WINDOW_COMPLETE_TOLERANCE_SECS;


The completeness check's threshold, window.max_secs - WINDOW_COMPLETE_TOLERANCE_SECS, goes negative whenever max_secs <= WINDOW_COMPLETE_TOLERANCE_SECS (0.25s) — and parse_window only rejects max_secs == 0, so any positive value under 0.25s is a legal request.

Once the threshold is negative, consumed >= <negative> is true for any positive consumed, not just a fully-covered window. Example: max_secs = 0.2, consumed = 0.01 (the recording genuinely ended almost immediately) → has_more still comes out true, and the caller is told to resume at start_sec + 0.01.

The comment right above this line (lines 340-343) already identifies half of this problem — "the tolerance alone would call a window that produced nothing full" — and the fix that shipped is consumed > 0.0. That guard only rules out the exact-zero case the comment describes; it doesn't rule out the general case of a small nonzero consumed that is nowhere near max_secs once max_secs itself is below the tolerance. The two fields would still "agree" (has_more and next_start_sec both present, satisfying the invariant tested in window_continuation_decides_when_the_walk_stops), but has_more itself would be wrong — telling the caller to continue a walk that has actually ended, on a window that came back far short of what was asked for.

In practice max_secs defaults to 600 and the tool description steers callers toward multi-minute windows, so this likely never fires through the tool surface as written — but window_continuation and MediaWindow are public-ish internal API (crate::media_understanding::MediaWindow), and the bound isn't enforced by parse_window beyond > 0. Worth clamping the subtraction (e.g. (window.max_secs - WINDOW_COMPLETE_TOLERANCE_SECS).max(0.0)) or requiring max_secs to exceed the tolerance in parse_window, so the boundary math holds regardless of how small a window is requested.

Generated by Claude Code

Fixed in 632f4b2 — the finding is right, and the boundary is worse than the consumed > 0.0 guard I added last round accounted for: that only closed the exact-zero case, while a hair above zero clears a negative bar just as well.

One note on the suggested remedy, since I went a different way. Clamping the subtraction at zero does not close it: with max_secs = 0.2 and consumed = 0.01, the threshold becomes 0.0 and 0.01 >= 0.0 still holds, so the same window still reports more to come. I ran the suite against that variant to be sure rather than reasoning about it — it fails the new test the same way the unclamped version does.

What does hold is capping the tolerance at half the window, so it can never exceed the quantity it is a tolerance on. Windows above half a second — every window the defaults and docs steer callers toward — keep the flat 0.25 s behaviour unchanged.

The mutation check now distinguishes the fix from both nearby wrong answers (flat tolerance, clamp-at-zero) rather than only from the original, and the new cases cover a short window that barely started, the same short window actually filled (so small windows stay walkable), and a 20 ms shortfall on a 600 s window.

The completeness check subtracted a flat 0.25 s tolerance from `max_secs`, and `parse_window` admits any positive `max_secs`, so for a window below the tolerance the threshold went negative — after which every positive `consumed` cleared it. A 0.2 s window that produced 0.01 s, because the recording ended almost immediately, still reported that there was more to transcribe. The previous `consumed > 0.0` guard only closed the exact-zero case; a hair above zero still cleared a negative bar. Clamping the threshold at zero does not close it either, since 0.01 clears 0 just as easily — verified by running the test suite against that variant. The tolerance is now capped at half the window, so it can never exceed the quantity it is a tolerance on. Windows above half a second — every window the tool's defaults and documentation steer callers toward — keep the flat 0.25 s behaviour unchanged. Verification: - mutation check against both alternatives: restoring the flat tolerance fails `window_continuation_decides_when_the_walk_stops`, and so does the clamp-at-zero variant, so the test distinguishes the fix from the two nearest wrong answers rather than merely from the original. - new cases cover a short window that barely started (must stop), the same short window actually filled (must continue, so small windows stay walkable), and a 20 ms shortfall on a 600 s window (must still count as full). - cargo test -p librefang-runtime --lib: 2216 passed - cargo check (workspace, lib), clippy with --all-targets -D warnings, cargo fmt check: clean

… an empty window Findings from an adversarial self-review pass over this branch. Four are defects; the fifth is a false claim I made in an earlier commit message, corrected below. **`out_path` was validated after the work it guards.** Resolution happened inside `write_transcript`, which runs after `transcribe_audio_window` — an ffmpeg pass and a billed provider request. On that branch the transcript is deliberately not placed in the response, so a path rejected afterwards destroyed work already paid for and the caller paid again for the retry. An absolute path outside the workspace is a routine thing for a model to emit, so this was not an exotic input. Resolution now happens before the provider call, which is also the order `web_fetch_to_file` uses — it resolves its destination ahead of the SSRF check and the fetch. **A window past the end of the recording failed the walk instead of ending it.** The guard's own comment called such a request legitimate and then returned `Err`. The walk reaches that state by itself: for a recording whose length is near a multiple of `max_secs` — 1200 s at the default 600 — the second window comes back full, so `has_more` is true and the third window starts exactly at EOF. The documented loop therefore ended in an error, having paid for one more ffmpeg pass. An empty window that started past the opening of the recording is now reported as a zero-length window, which `window_continuation` already turns into a clean stop; an empty window at offset zero still errors, since that means the input could not be decoded. **A silent stretch aborted the walk with no resume point.** An empty transcript was a hard error, which is right for a whole file and wrong for one window of a recording — a pause between speakers is ordinary. The error carried no offset, so a caller could not resume, and the assembled file simply stopped at the silence with nothing to say so. A windowed call now treats an empty transcript as an empty window and keeps its continuation fields. **The Ogg walk discarded usable granules on a truncated tail.** The loop guard promised that "whatever was read up to here is still a real granule", and the `?` on the segment-table read broke that promise for one of the two truncation shapes, returning `None` for the whole function. Separately, a granule was accepted from a page whose payload had not arrived — reporting audio the caller never received, which is the drift this contract exists to prevent. Both now break out of the walk and report the last complete page. **Correction to an earlier claim.** The commit message on `f251a5e6` says "all seven existing call sites are untouched". That is wrong twice over: on this base there are six production call sites of `transcribe_audio`, and one of them — `tool_media_transcribe` — is the one this branch rewrites. Five are untouched. The compatibility claim itself holds; the count did not. Docs: the `speech_to_text` row claimed the only difference from `media_transcribe` was the permissive MIME fallback, which stopped being true when this branch added windowing and `out_path` to only one of the two. Both locales now say so. The English walkthrough also regained the newline-separator detail that the Chinese translation already carried. Verification: - mutation checks on each new pin, all failing the intended test and only that test: restoring `?` in the segment-table read, accepting a granule from an unarrived payload, and neutralising the pre-skip subtraction. - the pre-skip case is why `ogg_opus_duration_reads_the_final_granule_position` now asserts within 0.002 s. The old 0.05 s bar was seven times wider than the 0.0065 s the subtraction is worth, so the mutation passed — the assertion was never guarding what the PR body credited it with. - `out_path_cannot_escape_the_workspace` now drives the resolver directly, which is where the check moved; it also asserts that an ordinary relative path still resolves, so the test cannot pass by rejecting everything. - cargo test -p librefang-runtime-media --lib: 101 passed; -p librefang-runtime --lib: 2216 passed - cargo check (workspace, lib), clippy on both crates with --all-targets -D warnings, cargo fmt check: clean

nevgenov · 2026-08-10T19:49:04Z

Ran an adversarial self-review pass over this branch before asking for more of your time on it. Four defects, all fixed in 5544825, plus one false statement of mine corrected.

out_path was validated after the work it guards. Resolution sat inside write_transcript, which runs after the ffmpeg pass and the billed provider request — and on that branch the transcript is deliberately not in the response, so a rejected path destroyed work already paid for and the caller paid again for the retry. An absolute path outside the workspace is routine model output, so this was not an exotic input. web_fetch_to_file had it right all along: it resolves its destination ahead of the SSRF check and the fetch. I cited that file twice as the model to follow and then inverted its order.

A window past the end of the recording failed the walk instead of ending it. The guard's own comment called such a request legitimate and then returned Err — and the walk reaches that state unaided: for a recording near a multiple of max_secs the last full window sets has_more, and the next one starts exactly at EOF. The loop the tool description prescribes therefore ended in an error. An overshoot is now a zero-length window, which window_continuation already turns into a clean stop; an empty window at offset zero still errors, since that means the input could not be decoded.

A silent stretch aborted the walk with no resume point. An empty transcript was a hard error — correct for a whole file, wrong for one window, where a pause between speakers is ordinary. The error carried no offset, so the assembled file just stopped at the silence with nothing to say so.

The Ogg walk discarded usable granules on a truncated tail, contradicting the loop guard's own promise, and separately accepted a granule from a page whose payload had not arrived. Narrow trigger — run_ffmpeg_pipe requires exit 0 — but it was a contract the comment and the commit message both stated and the code did not keep.

Correction. f251a5e6's message says "all seven existing call sites are untouched". Wrong twice: there are six production call sites of transcribe_audio on this base, and one of them is the one this branch rewrites. Five are untouched. The compatibility claim holds; the count did not.

Two notes on test quality, since both are the kind of thing that would otherwise have shipped looking covered:

ogg_opus_duration_reads_the_final_granule_position asserted within 0.05 s while the pre-skip subtraction it was credited with guarding is worth 0.0065 s — seven times narrower than the bar, so neutralising the subtraction passed. The tolerance is now 0.002 s and the mutation fails.
out_path_cannot_escape_the_workspace drove write_transcript directly, so it was structurally blind to where that call sat relative to the paid work. It now drives the resolver, and also asserts an ordinary relative path still resolves, so it cannot pass by rejecting everything.

Every fix is pinned by a test that I checked by mutation — reverting the defect fails that test and only that test.

houko

Automated review of the 2 commits pushed since the last pass (632f4b2 and 5544825, latest at 19:48:42Z). Both fix real defects (window-completeness tolerance going negative for sub-tolerance windows; out_path validated after the billed provider call; walk erroring instead of ending on an overshoot/silent window; Ogg parser discarding valid pages on a truncated tail) and are well mutation-tested for the functional side.

Two classes of finding:

One test-coverage gap: the new Ok(None) overshoot branch in extract_media_window (the core of the "end the walk on an empty window" fix) has no test hitting it — the sibling test only covers a window overlapping the end, not one fully past it.
Six hard-wrapped comment blocks (prose split mid-sentence across lines) reintroducing the exact issue flagged — and dedicated-rewrap-commit'd — earlier in this same PR's review history.

Nothing here blocks continued draft iteration; flagging before this leaves draft.

Generated by Claude Code

houko · 2026-08-10T23:41:47Z

+    if !ogg_contains_audio(&out) {
+        if window.start_sec > 0.0 {
+            return Ok(None);
+        }


New Ok(None) branch (the fix for "a window past the end failed the walk instead of ending it") has no test coverage.

The sibling test extract_media_window_past_the_end_returns_a_short_window (below, ~line 2820) only covers a window that overlaps the end — ogg_contains_audio is still true there, so it takes the Ok(Some(out)) path, never this branch. Nothing in the diff exercises a window whose start_sec is fully past the recording's true end, which is exactly the shape this branch exists for and the one window_continuation/the walk-termination logic now depends on.

Suggest a test alongside the existing one: synth a short recording (e.g. synth_ogg_opus(1.0)), call extract_media_window with start_sec at or past 1.0s, and assert Ok(None). No provider/network needed — same shape as the existing ffmpeg-gated test.

Generated by Claude Code

houko · 2026-08-10T23:41:55Z

+    // Resolved before the provider call, not inside the write below.
+    // Transcription costs an ffmpeg pass and a billed request, and on this
+    // branch the transcript exists nowhere else — it is not put in the
+    // response when a destination was named — so a path rejected afterwards
+    // destroys work that was already paid for and makes the caller pay again
+    // for the retry.
+    // `web_fetch_to_file` orders it the same way, resolving its destination
+    // ahead of the SSRF check and the fetch.


Hard-wrapped again: "Transcription costs an ffmpeg pass and a billed request, and on this branch the transcript exists nowhere else — it is not put in the response when a destination was named — so a path rejected afterwards destroys work that was already paid for and makes the caller pay again for the retry." is one sentence split across lines 253–257; same for the web_fetch_to_file sentence on 258–259. CLAUDE.md: "One sentence = one line, regardless of length" — applies to comment blocks, no column limit. Please re-join each sentence onto its own line.

Generated by Claude Code

houko · 2026-08-10T23:42:01Z

+                // Overshoot: the window began past the end of the recording.
+                // Reported as a zero-length window rather than an error, so the
+                // caller's loop ends on `has_more: false` the way the tool
+                // description tells it to, instead of on a failure it has no
+                // way to distinguish from a broken file.


Hard-wrapped: the sentence "Reported as a zero-length window rather than an error, so the caller's loop ends on has_more: false the way the tool description tells it to, instead of on a failure it has no way to distinguish from a broken file." is split across lines 304–307 instead of sitting on one line. Same prose-wrapping rule flagged in earlier passes on this PR (and in the dedicated rewrap commit a7cc9ca) — this is new prose from the latest commit that reintroduces it.

Generated by Claude Code

houko · 2026-08-10T23:42:09Z

+            // A whole file that transcribes to nothing is a failure worth
+            // surfacing. One *window* of a recording that transcribes to
+            // nothing is ordinary — a pause, a silent stretch, a gap between
+            // speakers — and erroring there would abort the walk with no
+            // resume point, stranding the rest of the recording behind a
+            // silence.


Hard-wrapped: two sentences split across lines 452–457 — "A whole file that transcribes to nothing is a failure worth surfacing." (452–453) and "One window of a recording that transcribes to nothing is ordinary — a pause, a silent stretch, a gap between speakers — and erroring there would abort the walk with no resume point, stranding the rest of the recording behind a silence." (453–457). Please put each sentence on its own line per the repo's prose-wrapping rule.

Generated by Claude Code

houko · 2026-08-10T23:42:16Z

+    // Same defence in depth as the video path: a demux that fails for want of
+    // a seek still exits 0 and still emits a headers-only container, so
+    // neither guard in `run_ffmpeg_pipe` catches it.


Hard-wrapped: "Same defence in depth as the video path: a demux that fails for want of a seek still exits 0 and still emits a headers-only container, so neither guard in run_ffmpeg_pipe catches it." is one sentence split across lines 1419–1421 (the rest of this same comment block, 1423–1425, correctly keeps one sentence per line — this opening sentence is the outlier).

Generated by Claude Code

houko · 2026-08-10T23:42:24Z

+        // A truncated tail must not discard what was already read — the guard
+        // above says so and this is the other half of it: a stream that stops
+        // inside the segment table or mid-payload leaves the granules of every
+        // complete page before it perfectly usable, and `?` here would throw
+        // them away along with the incomplete one.


Two more hard-wrapped sentences in this new hunk: lines 1493–1497 ("A truncated tail must not discard what was already read — the guard above says so and this is the other half of it: a stream that stops inside the segment table or mid-payload leaves the granules of every complete page before it perfectly usable, and ? here would throw them away along with the incomplete one.") and lines 1503–1506 ("The granule is only meaningful once the page it describes has actually arrived: accepting it from a page whose payload was cut off would report audio the caller never received, and the caller would advance past it."). Each should collapse to one line per the prose-wrapping rule.

Generated by Claude Code

houko · 2026-08-10T23:42:31Z

+    /// A stream that stops inside a page must still report the granules of the
+    /// complete pages before it — the walk's own comment promises exactly that,
+    /// and an early `?` used to discard them, stopping a caller's walk short of
+    /// the recording's end.


Same hard-wrap pattern recurs in the new test module (the exact class of thing commit a7cc9ca was dedicated to fixing last round): doc comment 2744–2747 is one sentence split across 4 lines; the comment at 2773–2775 packs two sentences across 3 lines ("Cut inside the third page's header, its segment table, and its payload in turn." / "Every one of those leaves the 1 s of complete pages before it intact, so every one must report 1 s rather than nothing."); and 2785–2787 is another single sentence split across 3 lines. Please rewrap all three to one sentence per line.

Generated by Claude Code

houko

Automated review pass, scoped to resource bounds / temp-file handling / out_path path-safety on the windowed-transcription feature.

This PR already carries an extensive, high-quality review history (multiple rounds, several real defects caught and fixed by the author) — most of the usual concerns (path traversal via out_path, temp-file cleanup via ScopedTempFile, prose-wrap and test-coverage nits) are already covered by earlier passes and don't need repeating.

One additional finding below, on the resource-bound question specifically: windowing bounds the transcription request and the result size per call, but the source-file read/base64/temp-stage cost is still paid in full on every windowed call rather than once per walk. Non-blocking — flagged for awareness, not a defect in what the PR set out to fix.

Everything else checked (git safety, commit attribution, changelog fragment presence/format, prose wrapping on newly-touched lines) matches what prior passes already reported: no AI/Claude attribution in commits, conventional commit format, changelog fragment present and correctly formed, out_path traversal handled and tested.

Generated by Claude Code

houko · 2026-08-11T00:08:56Z

@@ -229,12 +247,284 @@ pub(super) async fn tool_media_transcribe(
    let attachment = build_transcription_attachment(&resolved, &ext).await?;


Resource-bound question raised in review scope: windowing bounds the transcription request and the result size, but not the per-call cost of handling the source file itself.

Each call to tool_media_transcribe — including every window in a multi-window walk over one long recording — runs build_transcription_attachment (line 247) fresh, which re-reads the whole file from disk via read_with_size_limit(resolved, MAX_AUDIO_BYTES) and re-encodes it to base64 into the attachment. transcribe_audio_window then base64-decodes that whole attachment back into audio_bytes and, for a windowed call, passes the entire thing to extract_media_window, which stages it to a fresh ScopedTempFile before ffmpeg seeks into it with -ss.

So the seek genuinely avoids decoding the parts of the file outside the window (as the PR description correctly claims), but the disk read + base64 round-trip + temp-file staging of the whole source file happens once per window, not once per walk. For a caller walking N windows over one recording, that fixed-per-call cost is paid N times rather than being amortized — e.g. a 45-minute recording walked in 5 windows of 600s re-reads/re-stages the full file 5 times.

This is bounded, not unbounded — each call is still capped at MAX_AUDIO_BYTES / MAX_VIDEO_BYTES exactly as before, so there's no memory-safety or DoS concern — and for the sizes discussed in the PR body (~2.5 MB for ten minutes of speech) it's cheap in absolute terms. Flagging as a scaling characteristic worth being aware of (or noting in docs) rather than a defect: "windowing" caps what reaches the provider and the agent per call, but the source-file handling cost still scales with (file size) × (number of windows) for a full walk, not with file size alone.

Not blocking — the feature solves the two problems it explicitly set out to solve (timeout, spill threshold), and this is orthogonal to those.

Generated by Claude Code

…indow Addresses the review on librefang#6773. **A walk re-handled the whole recording once per window.** Every call built the attachment by reading the file, base64-encoding it, and handing that over; `transcribe_audio_window` decoded it back and `extract_media_window` copied the result to a scratch file before ffmpeg seeked into it. The seek itself was constant-time as the PR claimed, but the read, the base64 round-trip and the staging copy were not — a 45-minute recording walked in five windows paid all of that five times. A tool call already has the recording on disk and ffmpeg can seek into it directly, so a windowed call now builds a `FilePath` attachment and `extract_media_window` takes either a path or bytes, staging only the latter. An inbound attachment that carries bytes still stages, because that is what makes it seekable at all (librefang#6747). Size admission is unchanged: `build_windowed_attachment` reads the length from the file's metadata and applies the same `MAX_AUDIO_BYTES` / `MAX_VIDEO_BYTES` budget, so what is admitted is identical and only the transport differs. **Test coverage for the overshoot branch.** The `Ok(None)` path added last round — the one the walk's termination now rests on — had no test; the sibling case only covered a window *overlapping* the end, which still carries audio and takes the other branch. Writing that test surfaced a limitation worth recording rather than papering over, so it now has a test of its own that documents it. ffmpeg abandons the seek once `-ss` is far enough past the end and returns the recording **from the beginning** instead of nothing. Measured on ffmpeg 8.1 against a 1.0065 s source: `-ss` of 1.2 through 5 s all yield the 137-byte headers-only stream the guard rejects, while `-ss 30` and `-ss 120` both yield 5013 bytes carrying the first 1.013 s — real audio, which the guard passes. A walk cannot reach that: `next_start_sec` advances by the produced duration, so it overshoots by less than one window and lands in the handled range. It is reachable only when a caller supplies `start_sec` itself and is far wrong about the recording's length, and the cost is a billed request for audio already transcribed plus a duplicate of the opening appended to the transcript. `-copyts` is the obvious remedy and does suppress it, but it makes the granule absolute — a tail window at `-ss 8` of a 10 s source reports 10.007 s instead of 2.006 s — and changes what `-t` means. Both break the produced-duration contract the whole walk rests on, so it is left documented rather than half-fixed. Prose comment blocks flagged in review are rewrapped to one sentence per line. Only the blocks the review named; untouched paragraphs elsewhere in these files are deliberately left as they are. Verification: - cargo test -p librefang-runtime-media --lib: 104 passed, 3 new — the overshoot branch across four offsets, the documented far-overshoot limitation, and path-vs-bytes input producing the same window - cargo test -p librefang-runtime --lib: 2216 passed - cargo check (workspace, lib), clippy on both crates with --all-targets -D warnings, cargo fmt check: clean

…empty window Second adversarial self-review pass, over the fixes from the first one. Six findings, all real. **An undecodable audio track could look like the end of the recording.** Overshoot and failed-demux both produce headers with no audio, and the previous commit told them apart by `start_sec > 0` alone. But `start_sec` is a parameter a caller sets directly — "transcribe from the tenth minute" of a file whose audio will not decode would have been reported as a successful empty window, and the caller would have concluded there was nothing there. That is the silent failure librefang#6747 exists to make loud, reintroduced on the windowed path by a technicality. An empty window past the opening now probes the source by cutting one second from its start: audio there means the track decodes and the window really is past the end. The probe runs only when a window came back empty, which happens once at the end of a walk. **An empty window wrote a bare separator.** Both the overshoot and the silent-stretch branches reach `write_transcript` with an empty transcript and `append = true`, so the separator was written with nothing after it — a blank line in the artefact for every pause in the recording, and a trailing one at the end of every walk. In a feature whose whole point is the accuracy of the assembled file, that is not cosmetic. **`create_dir_all` still ran after the transcription.** The previous commit moved path resolution ahead of the provider call but left directory creation behind it, so a parent directory that cannot be created — permissions, a full disk — still destroyed a paid-for transcript. Same class as the original finding, narrower entrance; it now runs with the resolution. **The overshoot branch reported an empty `model`.** It sits ahead of model resolution and filled the field with an empty string, so the last step of a walk came back with `"model": ""` while every other step named the model. The field describes the configuration the call ran under, not whether it produced text. **A doc comment had migrated to the wrong function.** Splitting the resolver out of `write_transcript` left the writer's entire doc block — separators, whole-file sha256, partial-write recovery — attached to a function that writes nothing, and the writer with none. Both now carry their own. **A claim of mine was too broad.** The PR comment on the previous pass said "every fix is pinned by a test that I checked by mutation". The commit message listed exactly three mutations and was accurate; the comment generalised past it. Two branches were unpinned at the time: the `Ok(None)` overshoot (now covered) and the empty-transcript window (now covered). ## On the probe's own coverage `extract_media_window_reports_an_undecodable_source_at_any_offset` covers the reachable case — ffmpeg rejecting the input outright, which exits non-zero and errors before the guard is reached. It does **not** exercise the probe, and neutralising the probe leaves it green. The shape the probe exists for — ffmpeg exiting 0 while emitting a headers-only container from an input it accepted — could not be synthesised with file input: every undecodable source tried exits non-zero. That shape was real over a pipe (librefang#6747), so the probe stays as defence, but it is documented as unpinned rather than claimed otherwise. Verification: - mutation checks: removing the empty-transcript guard fails `an_empty_window_writes_nothing_at_all`; the probe's own mutation is documented above as not caught, deliberately and in the test's doc comment. - cargo test -p librefang-runtime-media --lib: 105 passed; -p librefang-runtime --lib: 2217 passed - cargo check (workspace, lib), clippy on both crates with --all-targets -D warnings, cargo fmt check: clean

…he real source size Third adversarial self-review pass. Six findings; the first corrects a risk assessment I got wrong in the previous commit. **The ffmpeg clamping margin is absolute, not proportional.** The last commit described the far-overshoot case as reachable only by a caller "far wrong about the recording's length". That came from measuring a 1 s source, where five seconds of slack looked like five times the length. Measured across 10 s, 60 s and 300 s sources on ffmpeg 8.1, every one behaves identically: headers-only up to about 8 s past the end, the 5524-byte opening of the recording from about 10 s past it. The margin does not scale, so on a 45-minute recording it is 0.4 % of its length — "transcribe from the fiftieth minute" of a meeting that ran forty-five is enough to get the opening back, billed, appended to `out_path` as though it were the window that was asked for, and reported with `has_more: false` as success. It is not detectable from the output. The clamped stream is ordinary audio, and comparing it against the recording's opening does not work either: Ogg page headers carry per-encode serials and CRCs, so a clamped window agrees with the head on 3 % of bytes — the same as a legitimate tail does. Knowing the source duration would settle it and there is no way to learn it here without `ffprobe`, which the kernel calls nowhere. So the constraint is stated where a caller can act on it rather than only in a test's doc comment, which an agent never sees: the `start_sec` schema description and both documentation pages now say to advance with `next_start_sec` and not to guess an offset, and what happens if one is guessed. **Windowed calls logged the source as zero bytes.** After the previous commit stopped reading the file for windowed calls, `original_size` was taken from the now-empty buffer, so every long recording — the case this feature exists for — logged `original_size=0`. It comes from the attachment now, which carries the real size either way. **Directory creation moved back next to the write.** Hoisting it into the resolver did keep it ahead of the billed call, but it turned a pure path decision into a filesystem mutation: a mistyped `out_path` materialised a directory on every rejected call, and the change had also quietly swapped `tokio::fs` for blocking `std::fs`. Both neighbours order it the way it is ordered now — `web_fetch_to_file` creates after the download, `file_write` immediately before writing. Resolution, which is the failure a caller actually hits, stays ahead of the provider call. **A test asserted the presence of a defect.** `extract_media_window_far_past_the_end_is_not_detected` pinned `is_some()`, so anyone who fixed the clamping behaviour would have been met with a red test and read it as a regression. It now asserts only that the call does not error, which holds before and after such a fix, and the doc comment carries the detail. **`parse_window` was called twice in a row.** Pure function, same answer, but the second call was redundant. **Prose blocks rewrapped, and the rule applied as a rule.** The previous commit fixed the blocks review had named and left its own new prose hard-wrapped, which is applying the rule by instruction rather than following it. The check is now mechanical — scan the branch diff for added comment lines carrying two sentences — and it comes back clean. Verification: - clamping margin measured directly across three source lengths; the byte-comparison detection idea was tested and rejected on evidence rather than assumed unworkable - cargo test -p librefang-runtime-media --lib: 105 passed; -p librefang-runtime --lib: 2217 passed - cargo check (workspace, lib), clippy on both crates with --all-targets -D warnings, cargo fmt check: clean

github-actions Bot added size/L 250-999 lines changed area/docs Documentation and guides area/runtime Agent loop, LLM drivers, WASM sandbox labels Aug 8, 2026

houko reviewed Aug 9, 2026

View reviewed changes

houko reviewed Aug 10, 2026

View reviewed changes

nevgenov added 4 commits August 10, 2026 16:51

nevgenov force-pushed the feat/6748-transcribe-chunking-out-path branch from 378176d to 117fd3d Compare August 10, 2026 14:00

nevgenov marked this pull request as ready for review August 10, 2026 14:11

houko reviewed Aug 10, 2026

View reviewed changes

nevgenov added 2 commits August 10, 2026 21:45

github-actions Bot added size/XL 1000+ lines changed and removed size/L 250-999 lines changed labels Aug 10, 2026

houko reviewed Aug 10, 2026

View reviewed changes

houko reviewed Aug 11, 2026

View reviewed changes

nevgenov added 3 commits August 11, 2026 19:17

houko enabled auto-merge (squash) August 12, 2026 00:40

houko merged commit 49a1064 into librefang:main Aug 12, 2026
36 checks passed

This was referenced Aug 18, 2026

release: v2026.8.19 #7710

Merged

fix(media): bound the extracted audio track against MAX_AUDIO_BYTES #7811

Merged

		@@ -229,12 +247,284 @@ pub(super) async fn tool_media_transcribe(
		let attachment = build_transcription_attachment(&resolved, &ext).await?;

Conversation

nevgenov commented Aug 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The problem

Changes

Compatibility

Why not ffprobe

Verification

One measurement worth carrying into item 1

Review rounds so far

Still open

Uh oh!

houko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

houko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

houko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

houko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

houko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

houko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nevgenov commented Aug 10, 2026

Uh oh!

houko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

nevgenov commented Aug 8, 2026 •

edited

Loading

Why not `ffprobe`