feat(meetings): auto-split over-cap recordings on import instead of 413 - #9300
Conversation
|
Intent: Turn the meetings import's hard 413 refusal of an over-cap recording into a transparent success: split the audio at the pauses between utterances into cap-sized segments, transcribe each through the existing transcribe_audio, and stitch them into one transcript whose seams are ordinary line breaks. |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
Design Review (Fable 5) — 🟡 CONCERNSDesign-level review of Design-Verdict: CONCERNS Sound split design on the probing route, but the silent-truncation cause it's built to defeat stays live in the decoder for three other callers. Watch
Suggestions
[DESIGN-REVIEWED] caf4c2a |
First Principles Review (Fable 5) — 🟡 CONCERNSPremise-level review of All verification is done: I grepped consumers (both new functions have exactly one production caller), confirmed no existing audio-split mechanism ( First-Principles-Verdict: CONCERNS The split is justified and bounded, but the declared deferral skips a smaller cause-level fix: the decoder itself could refuse loudly, closing all 3 silently-truncating siblings. What this change shipsIntent: let a user import a recording longer than the one-hour STT cap and get one complete transcript instead of a 413. ADDITION.
Watch
[FIRST-PRINCIPLES-REVIEWED] caf4c2a |
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsI've analyzed the diff and the single candidate. Let me verify the key claim in CANDIDATE 1 — whether the worst-case product in The candidate concerns Falsifying it against the code:
The two extremes are mutually exclusive because one decode gates both. I cannot re-derive a concrete input (requirement (a)) that produces a product large enough to cause a multi-second event-loop stall within a plausible No other hunk grounds a finding at the 80+ bar: the memory-bounding readers ( No findings. [OPUS-REVIEWED] caf4c2a Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
28fc2b1 to
b531a36
Compare
self-added: yes
|
b531a36 to
b01352e
Compare
self-added: yes
|
b01352e to
909548f
Compare
self-added: yes
|
909548f to
11b69a7
Compare
self-added: yes
|
|
|
5f38490 to
06f8e80
Compare
self-added: yes
|
self-added: yes
|
06f8e80 to
ccc9aa0
Compare
self-added: yes
|
The local/Apple STT decoders stop reading at _MAX_AUDIO_SECS (3600s) without saying so, so before PR #5741's guard an over-cap recording transcribed only its first hour and returned 200 -- silent partial import. #5741 turned that into a 413 recording_too_long refusal, correct but pushing the split onto the user. This makes the import auto-split instead: the audio is cut into cap-sized segments at the pauses between utterances (ffmpeg silencedetect; hard cut at the cap only when a window has no silence), each segment transcribed through the existing transcribe_audio, and the per-segment transcripts joined with newlines. The join feeds the route's split_transcript exactly as a whole transcript does, so the result is ONE stitched transcript whose seams are ordinary line breaks -- and it still passes MAX_IMPORT_LINES / MAX_TRANSCRIPT_CHARS there, so splitting is not a way around the total-size ceiling. Any segment that fails to extract or transcribes empty refuses the whole import (502); nothing is dispatched. Refs #8272
ccc9aa0 to
caf4c2a
Compare
Problem / Motivation
Import a recording longer than one hour into a meeting and the request is refused
with
413 recording_too_long. The user then has to find an audio editor, cut thefile into pieces by hand, and import each piece one at a time. A multi-hour
all-hands or workshop recording - the input the import feature is most valuable
for - is exactly the input it turns away.
Why it matters
The one-hour ceiling is not a product decision the user should have to work
around. It is an implementation detail of the local speech-to-text decoder: both
of its decode paths stop reading at
_MAX_AUDIO_SECS(3600s) and neither says itdid. Before the guard that returns the 413 (PR #5741), an over-cap recording was
transcribed to only its first hour and returned
200- a transcript that endsearly with no sign it is incomplete, which then gets trusted as the whole meeting.
That silent partial import is the worst case, and the 413 exists to prevent it.
This change removes the manual work without reopening that hole: long audio gets
the same transparent treatment an oversized pasted image already gets when it is
auto-resized to fit.
What changed (motivation -> approach -> change)
The decoder truncates silently at the cap, so the recording must not be handed to
it whole. The import route already probes the duration before transcribing (to
catch exactly this); that probe's
exceedsbranch used to raise the 413. It nowsplits instead.
The split is on the audio, not the transcript. One ffmpeg pass locates the
pauses with
silencedetect. A pure function then picks each cut at the last pausebefore the running cap boundary, within a 30-second window, so a word straddling
the boundary is never cut - the cut lands in the silence between utterances. When
a window holds no pause (continuous speech across the whole window), the cut falls
hard on the cap: a rare, documented seam that may split one word, still strictly
better than refusing the whole recording. Because the segments are
non-overlapping (cut in the silence between utterances), the stitch is a plain
space join with nothing to de-duplicate. Each segment is decoded to its own WAV
and transcribed through the ordinary
transcribe_audio, so every segment gets thesame provider dispatch, sensitive-path guard and redaction a whole recording does.
The user gets one transcript, not several. The joined text feeds the route's
existing
split_transcriptexactly as a whole transcript does, so each segment'slines become ordinary transcript lines and the seams are invisible. That same
split_transcriptstill enforcesMAX_IMPORT_LINESandMAX_TRANSCRIPT_CHARS, sosplitting is never a way around the total-size ceiling - a stitched result past the
line budget is still refused whole with 413.
If any segment fails to decode or transcribes empty, the whole import is refused
(
502) and nothing is dispatched: a partial import that reported success would bethe same silent-data-loss defect this fixes, one level up. The split is
local-only. Apple has the same one-hour ceiling but fails loudly at it, and its
speech helper runs in a sandbox that cannot read the segment files, so an over-cap
Apple recording keeps its prior
413refusal rather than being split. AWSTranscribe has no silent ceiling (it refuses an oversized payload loudly), so it
is not probed and not split. An indeterminate duration probe still refuses
retryably (
503), because the split needs the same decode the probe could notcomplete.
An empty or failed segment refuses the whole import. A segment can fail at two
steps: the ffmpeg extract (a decode error) or the recogniser (a per-segment
timeout, or the shared recogniser being swapped mid-import).
transcribe_audioreports both a genuine recogniser failure and a legitimately silent segment as the
same empty result, so the two cannot be told apart here. Rather than risk skipping
a segment whose recogniser actually failed and silently dropping a real spoken
span, any empty segment refuses the whole import (502) with nothing dispatched.
That is the same silent-partial defect this fixes, one level down, so it is refused
loudly instead. A recording under the size cap can span at most a fixed number of
cap-sized segments, so a crafted file reporting a far-future duration is refused
before any cut is built. Each split records one
allowedSEL audit line(
split:over-Nmin, via the route's existingaudit()sink) so the incident trailcarries an over-cap import as a durable record.
Cost, measured. The split runs one silence scan of the whole recording, then per
segment one ffmpeg extract and one recogniser decode, all sequential inside the
import request under the STT
timeout_secs(default 300s) per operation. The scanis a null decode with an RMS filter and no model, so it is ~1000x real time: a
2-hour recording scans in ~7s and a 4-hour in ~15s (measured with the host ffmpeg).
The local recogniser runs at a 0.007-0.011 real-time factor, so a full 1-hour
segment decodes in ~25-40s; the extract is a few seconds. Every operation clears
the 300s budget with wide margin, and a 2-4 hour meeting -- the input this feature
is named for -- completes in one to three minutes. The bound: a 512 MiB file at a
low bitrate (~12 kbps) is ~95 hours of audio, capped at 512 one-hour segments, so
the worst case is ~512 x ~34s ~= 4.9 hours holding one HTTP request. A recording of
tens of hours therefore holds the request for a long time; past 512 cap-sized
segments it is refused before any cut is built. This is a synchronous split, not a
background job queue -- that would be a different change and is out of scope.
Scope this PR does NOT cover, deferred: the same silent-truncation cause (the local
decoder stops at
_MAX_AUDIO_SECSwithout saying so) still reaches the three othertranscribe_audiocallers that do not probe duration --slack/events.py:1644,dashboard/handlers/core.py:1334, andmessaging/attachments.py:575. Those admitsmaller uploads (25 MiB, still hours at voice-memo bitrates) so a long memo there
transcribes only its first hour and reports success. A general fix inside
transcribe_audiois larger (its segment staging must sit under the guard-exemptvoice-runtime root) and is left to a follow-up; this PR cures the cause only on the
meetings import route, which is the one that probes.
flowchart LR A[import over-cap recording]:::ctx --> B{probe: exceeds cap?}:::ctx B -->|before| C[413 recording_too_long]:::removed B -->|now| D[cut at pauses into<br/>cap-sized segments]:::added D --> E[transcribe each<br/>segment]:::added E --> F[join with spaces =<br/>one transcript]:::added F --> G[dispatch into meeting]:::ctx classDef added fill:#DCFCE7,stroke:#16A34A,color:#14532D,stroke-width:2px classDef changed fill:#FEF3C7,stroke:#D97706,color:#78350F,stroke-width:2px classDef removed fill:#FEE2E2,stroke:#DC2626,color:#7F1D1D,stroke-dasharray:4 3 classDef ctx fill:#E0F2FE,stroke:#0284C7,color:#0C4A6E linkStyle 1 stroke:#DC2626,stroke-dasharray:4 3 linkStyle 2,3,4,5 stroke:#16A34A,stroke-width:2pxadded / changed / removed / unchanged: green / amber / red / blue
A recording over the cap used to be refused; now it is cut at its pauses, each piece transcribed, and the pieces joined into one transcript the meeting receives.
Tests
Ran with (Python, single file, no parallelism):
test_transcribe.py::TestChooseSegmentCuts- the boundary rules as a purefunction: cuts at the last pause before the cap, hard-cuts at the cap when a
window has no pause, advances from each cut so the loop terminates, and every
segment stays within the cap.
test_transcribe.py::TestTranscribeOversizedInSegments- cut/transcribe/stitchcontrol flow, space join in order, that a segment which fails to EXTRACT or
produces a falsy transcript (decode-empty or a recogniser failure, which cannot
be told apart) refuses the whole import (never a silent skip), and that an
implausible/far-future duration is refused before any cut is built (the
segment-count ceiling); each segment WAV is removed after transcription.
test_meetings_audio_import.py- the route now splits an over-cap recording andanswers
200(was 413); a failed split answers502with nothing dispatched; astitched result over
MAX_IMPORT_LINESis still413; segments are staged underthe request's own snapshot dir.
test_spawn_audit.py- the two new ffmpeg spawns are classified in the audit.prove.pyreportsPROVEN: reverting the production hunks while keeping the testhunks makes an assertion fail, so the tests catch the change.
Manual verification
N/A - unit coverage sufficient. The ffmpeg seam (silence scan, segment extract) is
the same spawn shape as the existing duration probe and decode, tested at the
function boundary; the boundary rules are a pure function tested directly.
Related Issues
Closes #8272