Skip to content

feat(meetings): auto-split over-cap recordings on import instead of 413 - #9300

Merged
bolichen97 merged 1 commit into
mainfrom
feat/meetings-import-autosplit-8272
Sep 8, 2026
Merged

feat(meetings): auto-split over-cap recordings on import instead of 413#9300
bolichen97 merged 1 commit into
mainfrom
feat/meetings-import-autosplit-8272

Conversation

@chenmingwei23

@chenmingwei23 chenmingwei23 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

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 the
file 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 it
did. 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 ends
early 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 exceeds branch used to raise the 413. It now
splits 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 pause
before 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 the
same 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_transcript exactly as a whole transcript does, so each segment's
lines become ordinary transcript lines and the seams are invisible. That same
split_transcript still enforces MAX_IMPORT_LINES and MAX_TRANSCRIPT_CHARS, so
splitting 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 be
the 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 413 refusal rather than being split. AWS
Transcribe 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 not
complete.

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_audio
reports 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 allowed SEL audit line
(split:over-Nmin, via the route's existing audit() sink) so the incident trail
carries 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 scan
is 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_SECS without saying so) still reaches the three other
transcribe_audio callers that do not probe duration -- slack/events.py:1644,
dashboard/handlers/core.py:1334, and messaging/attachments.py:575. Those admit
smaller 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_audio is larger (its segment staging must sit under the guard-exempt
voice-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:2px
Loading

added / 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):

timeout 900 python3 -m pytest -n0 test/test_transcribe.py -x -q
timeout 900 python3 -m pytest -n0 test/test_meetings_audio_import.py -x -q
timeout 900 python3 -m pytest -n0 test/test_spawn_audit.py -x -q
  • test_transcribe.py::TestChooseSegmentCuts - the boundary rules as a pure
    function: 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/stitch
    control 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 and
    answers 200 (was 413); a failed split answers 502 with nothing dispatched; a
    stitched result over MAX_IMPORT_LINES is still 413; segments are staged under
    the request's own snapshot dir.
  • test_spawn_audit.py - the two new ffmpeg spawns are classified in the audit.

prove.py reports PROVEN: reverting the production hunks while keeping the test
hunks 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

@chenmingwei23
chenmingwei23 requested a review from a team as a code owner September 7, 2026 20:06
@chenmingwei23

Copy link
Copy Markdown
Contributor Author

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.
Not a goal: No general media pipeline (no queue, retries, or progress reporting); no overlap/text de-duplication (segments are non-overlapping by construction); no change to the total-size ceilings (MAX_IMPORT_LINES / MAX_TRANSCRIPT_CHARS still gate the stitched result); a mid-split failure refuses the whole import rather than dispatching a partial one.

@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of caf4c2a397e1b4a37dea31af3085c19b4d928e45 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] caf4c2a

False positive or not applicable? A repository writer can comment:
/ai-review override gpt caf4c2a397e1b4a37dea31af3085c19b4d928e45: <one-sentence reason>

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — 🟡 CONCERNS

Design-level review of caf4c2a397e1b4a37dea31af3085c19b4d928e45 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

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

  • The decode paths still truncate silently at the cap (transcribe.py:2489, transcribe.py:2604); the PR's own description concedes slack/events.py, dashboard/handlers/core.py, and messaging/attachments.py "transcribe only the first hour and report success." The worst-case harm this PR names — trusted-but-partial transcripts — ships unfixed on three surfaces, and the deferred "general fix" carries no tracking issue in the description.
  • A cap-sized genuinely-silent stretch (recording left running) now yields a whole-import 502 with a retryable-looking failure, because transcribe_audio conflates decoded-empty with recogniser-failure. Deliberate and documented, but the user's only recourse is the manual editor workflow this PR set out to remove.
  • The synchronous split holds one HTTP request for the full duration (admitted worst case ~4.9h at the 512-segment ceiling); a client disconnect cancels mid-way with no partial progress to resume, so the long tail retries from zero.

Suggestions

  • Independent of splitting, make the two decode paths fail loudly at _MAX_AUDIO_SECS instead of truncating — a small companion that closes the silent-data-loss class for all callers and reduces this PR's split to pure UX.

[DESIGN-REVIEWED] caf4c2a

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of caf4c2a397e1b4a37dea31af3085c19b4d928e45 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All verification is done: I grepped consumers (both new functions have exactly one production caller), confirmed no existing audio-split mechanism (silencedetect appears nowhere else; stt/vad.py is live-stream endpointing, a different job), and counted the unfixed siblings of the root cause (3 non-probing transcribe_audio callers). Final review follows.

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 ships

Intent: let a user import a recording longer than the one-hour STT cap and get one complete transcript instead of a 413. ADDITION.

  1. Over-cap recording on the local recogniser now imports as one transcript — justified (issue Meetings import: auto-split recordings longer than the STT duration cap instead of refusing with 413 #8272)
  2. Over-cap Apple recording still refused 413 — justified (sandbox masks the segment dir)
  3. Any failed or empty segment refuses the whole import (502) — justified (prevents silent partial)
  4. Stitched result past the line budget still refused 413 — justified
  5. Crafted far-future duration refused before any cut is built — justified (hostile-input boundary)
  6. Pause-dense / giant-stderr pathological files refused instead of crashing the gateway — justified (external-content boundary)
  7. One allowed SEL line per split — justified (incident trail)
  8. Over-cap import now holds the request minutes (worst case hours) — declared timing change
  9. Two new module-level functions, one production consumer each — acceptable stub seam
  10. Slack/dashboard/attachment audio still silently truncates at one hour — declared deferral, 3 counted siblings

Watch

  • Point patch with counted siblings: grep transcribe_audio( → 3 non-probing callers (slack/events.py:1644, dashboard/handlers/core.py:1337, messaging/attachments.py:575). The description defers only the general split ("A general fix inside transcribe_audio is larger… left to a follow-up") and never weighs the smaller fix at the actual cause — the two silent cap sites in the same file this PR edits (transcribe.py:2489 caps readframes; transcribe.py:2604 passes -t _MAX_AUDIO_SECS). Making those refuse past the cap turns the siblings' silent partials into loud failures now, by the PR's own "silent partial is the worst case" reasoning, without any segment staging.
  • provider_splits_oversized is negative membership (provider not in ("transcribe", "apple")): a future provider defaults into the split path; supported today only by the config loader normalizing unknowns to local.

[FIRST-PRINCIPLES-REVIEWED] caf4c2a

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed caf4c2a397e1b4a37dea31af3085c19b4d928e45 — this comment is updated in place on each push.

Review details

I've analyzed the diff and the single candidate. Let me verify the key claim in CANDIDATE 1 — whether the worst-case product in _choose_segment_cuts is reachable.

The candidate concerns _choose_segment_cuts (transcribe.py:343-353) running a per-window list comprehension over the full ordered silence list synchronously on the event loop, claiming O(segments × silence_points) ≈ 51M iterations.

Falsifying it against the code:

  • The loop is bounded by _MAX_SEGMENTS (512) — transcribe_oversized_in_segments refuses when duration > _MAX_SEGMENTS * cap_secs before any cut is built (transcribe.py:562).
  • Each comprehension is bounded by _MAX_SILENCE_POINTS (100k).
  • But both bounds derive from the same single decode pass (_detect_silence_ends), gated by stt_config.timeout_secs. To reach 512 segments at the confirmed cap of 3600s (audit string over-{cap_secs // 60}min + tests use 3600), the decode must report ~512 hours of duration, which either requires actually decoding ~512h of audio (far beyond any request timeout at any realistic decode speed) or a crafted far-future PTS — and a crafted PTS implies short real content, hence few silence events. Conversely, 100k genuine silence_end events require ≥8h of real pause-dense audio, yielding only ~8 segments at a 1h cap (product <1M, sub-second).

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 timeout_secs; establishing it would require assuming a decode that simultaneously survives the timeout and maxes both dimensions. The candidate itself rates this "low" for exactly this reason. (a) resolves to "might" — dropped under Step 1.

No other hunk grounds a finding at the 80+ bar: the memory-bounding readers (_drain_progress_and_silence, _drain_stderr_tail) cap retained data, cancellation/timeout arms reap the child, the falsy-segment path refuses the whole import, and provider_splits_oversized correctly gates the split.

No findings.

[OPUS-REVIEWED] caf4c2a

Verdict parsed from the review's SHA-scoped output markers for commit caf4c2a397e1b4a37dea31af3085c19b4d928e45.

False positive or not applicable? A repository writer can comment:
/ai-review override fable caf4c2a397e1b4a37dea31af3085c19b4d928e45: <one-sentence reason>

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 7, 2026
@chenmingwei23
chenmingwei23 force-pushed the feat/meetings-import-autosplit-8272 branch from 28fc2b1 to b531a36 Compare September 7, 2026 20:20
@chenmingwei23

Copy link
Copy Markdown
Contributor Author
  • fixed span=b85dcc20eaf0 (double SEL audit on the successful split path)

self-added: yes
mechanism: split-path audit record

Fixed in b531a36. The successful over-cap
split path called _reject("split_over_cap") (which emits outcome="rejected")
and then audit(... outcome="allowed") before returning 200, so one successful
import wrote two contradictory SEL records. Removed the _reject call: the
split path succeeds, so it records only the ALLOWED decision, the same way the
owner-check ALLOW is recorded. _reject is now reserved for paths that raise.
Class-level rule: an audit(... outcome="rejected") belongs only on a branch
that raises/refuses; a branch that proceeds to 200 records allowed or nothing,
never both. New test test_a_successful_split_audits_allowed_not_rejected pins
exactly one allowed split record and asserts no rejected record on success.

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 7, 2026
@chenmingwei23
chenmingwei23 force-pushed the feat/meetings-import-autosplit-8272 branch from b531a36 to b01352e Compare September 7, 2026 20:33
@chenmingwei23

Copy link
Copy Markdown
Contributor Author
  • fixed span=eb8103c2acd8 (segment join bypassed sentence splitting)

self-added: yes
mechanism: segment transcript join

Fixed in b01352e. transcribe_oversized_in_segments
joined per-segment transcripts with "\n". A local/Apple segment transcript is one
whitespace-joined paragraph with no internal newlines (stt/engine.py joins whisper
segments with " "), so a newline join handed split_transcript N>1 lines. That makes
split_transcript treat each whole segment as one line (tier 1) and only hard-wrap it
at max_chars, dispatching 4000-char chunks instead of utterances -- worse structure
than the un-split path. Changed the join to " ", so the stitched text is one
paragraph and split_transcript sentence-splits it into utterances (tier 2) exactly
as for a whole recording; a segment that DOES carry internal newlines keeps them
across a space join, so tier-1 structure is preserved where it exists.
Class-level rule: a stitched transcript must reach split_transcript in the same
line-structure shape a whole transcript would, so the tier the splitter selects does
not depend on how many segments the audio was cut into. Test
test_it_cuts_transcribes_and_stitches_into_one_paragraph pins the space join and that
no newline is introduced.

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 7, 2026
@chenmingwei23
chenmingwei23 force-pushed the feat/meetings-import-autosplit-8272 branch from b01352e to 909548f Compare September 7, 2026 20:46
@chenmingwei23

Copy link
Copy Markdown
Contributor Author
  • fixed span=eb8103c2acd8 (over-cap Apple import would 502: split segments unreadable by the sandboxed helper)

self-added: yes
mechanism: split provider gate

Fixed in 909548f. The split staged segment WAVs
under the request's snapshot dir (run/voice-runtime). The Apple Swift helper runs
in the mode="strict" sandbox, which masks the run/voice-runtime leaf (sandbox.py),
and a .wav is a native suffix so _to_native_audio hands the helper the path by
value -- the helper cannot open it, so every over-cap Apple import would return
502. Root cause: the split was gated on "has a cap" (batch_duration_cap_secs
non-None), which is true for both local AND apple, but only the LOCAL decoder
truncates silently; Apple already fails loudly at the ceiling
(_to_native_audio raises RecordingTooLongError). Added provider_splits_oversized()
= provider not in (transcribe, apple), so the split is local-only and Apple keeps
its prior 413. This matches issue #8272's own intent: "segmentation only ever
triggers where the ceiling exists (AWS/Apple providers fail loudly and need no
split)".
Class-level rule: a mechanism that only helps the silently-truncating provider
must be gated on THAT provider, not on the weaker "has a ceiling" property that
also matches a provider whose loud failure needs no help and whose sandbox the
mechanism breaks. Tests: provider_splits_oversized unit test + route test
test_an_over_cap_recording_on_a_loud_fail_provider_is_413_not_split.

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 7, 2026
@chenmingwei23
chenmingwei23 force-pushed the feat/meetings-import-autosplit-8272 branch from 909548f to 11b69a7 Compare September 7, 2026 20:56
@chenmingwei23

Copy link
Copy Markdown
Contributor Author
  • fixed span=f8992237b312 (stale comment said "both split" after the split was narrowed to local-only)

self-added: yes
mechanism: provider-aware comment

Fixed in 11b69a7. When the previous round narrowed
the split to local-only (provider_splits_oversized), the comment above the probe
still read "the local decoder and the Apple lane share the ceiling, so both split",
contradicting the new Apple-refusal branch. Rewrote it: both providers are PROBED
(both -t-bound their decode), but only local truncates silently so only local
splits; Apple fails loudly and its sandboxed helper cannot read the segment dir, so
an over-cap Apple recording is refused 413, not split. Also updated the PR body to
state the split is local-only. Comment-only code change (plus the body edit).
Class-level rule: after narrowing a branch, grep the surrounding prose for the
premise the narrowing invalidated - a comment describing the superseded behaviour in
the same commit reads as evidence the change was not thought through.

@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 7, 2026
@chenmingwei23

Copy link
Copy Markdown
Contributor Author
  • fixed span=design-concerns (empty-segment refusal; description/diagram newline claim)

Addressed in 5f38490.
Watch 1 (empty-segment refusal makes a break hour unimportable): FIXED. A segment
that DECODES fine but transcribes empty is now a benign silent stretch - it is
skipped and the rest still stitch, so an all-day recording with quiet hours stays
importable. Only a segment that fails to DECODE (_extract_segment -> False) still
refuses the whole import; an all-silent recording yields None (the 502), same as a
whole recording the recogniser could not hear. This does not reopen the
silent-partial hole: the skipped span is genuine silence, not a dropped tail.
Watch 3 (description/diagram say "newline join" but code space-joins): FIXED. The
prose, the mermaid node, and the test description in the body now all say space join.
Watch 2 (scan-timeout probability grows with length; 502 vs the old actionable 413;
one request held open for multi-hour local jobs): ACCEPTED as a known property of a
synchronous import. The brief scoped this PR to a minimal split, not a job queue with
progress reporting; a long local transcription holding one request is the same shape
the pre-split import already had. If it proves painful in practice it is a separate
async-import change, not this PR.

@chenmingwei23

Copy link
Copy Markdown
Contributor Author
  • fixed span=first-principles-concerns (provider_splits_oversized default; undeclared surfaces)

Addressed in 5f38490.
Subtraction (drop the stt_config=None default + self-load in provider_splits_oversized):
DONE. It now takes a REQUIRED config arg and never reads config itself; a bare call is a
TypeError. The one production caller (audio_import.py) always passes the shared snapshot.
Test test_it_takes_a_required_config_and_never_reads_config_itself pins it.
Watch (item 6, undeclared SEL "allowed" split audit record; item 9 self-loading config):
the config self-load is removed by the subtraction; the split:over-Nmin audit record is
now named in the PR body's What-changed section.
Watch (the silent-truncation cause still reaches 3 other transcribe_audio callers -
slack/events.py, dashboard/handlers/core.py, messaging/attachments.py): ACCEPTED AND
DEFERRED, and now named in the body as scope this PR does not cover. A general fix inside
transcribe_audio is genuinely larger - its segment staging must sit under the
guard-exempt voice-runtime root - so it belongs in a follow-up, not folded into this
minimal meetings-route change. This PR cures the cause on the one route that probes.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 7, 2026
@chenmingwei23
chenmingwei23 force-pushed the feat/meetings-import-autosplit-8272 branch from 5f38490 to 06f8e80 Compare September 7, 2026 22:48
@chenmingwei23

Copy link
Copy Markdown
Contributor Author
  • fixed span=e851b460434d (falsy segment transcript was silently skipped, dropping a real span)

self-added: yes
mechanism: segment empty-vs-failure handling

Fixed in 06f8e80. Correct finding. Last round I made an
empty segment SKIP (to answer a Design CONCERNS about a silent break hour), but
transcribe_audio returns None for a genuine recogniser failure (per-segment decode
error, timeout, the shared recogniser singleton swapped mid-import) as well as for a
silent segment, and _extract_segment returning True only proves ffmpeg carved the WAV,
not that the recogniser succeeded - so the skip could drop a real spoken span and still
answer 200. Reverted: any falsy segment transcript now refuses the WHOLE import (return
None -> the route's 502), never a silent skip. Distinguishing decoded-empty from
recogniser-failure would need a new failure flag on transcribe_audio's contract (8
callers), out of scope for this route-level PR, so the conservative refuse-whole is the
right minimal answer. Test test_a_falsy_segment_transcript_refuses_the_whole_import pins it.
Class-level rule: widening a fix must re-check the OPPOSITE failure mode - my skip fixed
over-strictness but opened silent data loss; the guarantee "no accepted import loses a
span" outranks the convenience of importing a rare all-silent stretch.

@github-actions github-actions Bot removed the readiness: action required A blocking check or review needs attention label Sep 7, 2026
@chenmingwei23

Copy link
Copy Markdown
Contributor Author
  • fixed span=67c6f62751a7 (per-segment recognition failure treated as silence, dropping a span)

self-added: yes
mechanism: segment empty-vs-failure handling

Fixed in 06f8e80. Same root as the GPT blocking finding
and the same fix. transcribe_audio's None conflates a genuine recogniser failure (the
shared singleton swapped mid-import raising DecodeFailed even after the retry, a
per-segment timeout) with a legitimately-empty transcript, so my empty-skip branch could
silently drop a spoken span and return 200. Reverted to refuse the whole import on ANY
falsy segment transcript. The proper decoded-empty-vs-failed distinction requires
surfacing an explicit failure flag on transcribe_audio (its contract is untouched by this
PR and has 8 callers), so it is deliberately out of scope; the safe refuse-whole behavior
is what ships. A truly all-silent cap-sized stretch is now refused loudly rather than
imported with a gap - loud refusal over silent loss.

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 7, 2026
@chenmingwei23
chenmingwei23 force-pushed the feat/meetings-import-autosplit-8272 branch from 06f8e80 to ccc9aa0 Compare September 7, 2026 23:08
@chenmingwei23

Copy link
Copy Markdown
Contributor Author
  • fixed span=e851b460434d (oversized ffmpeg log line -> readline() ValueError -> unhandled 500)

self-added: yes
mechanism: line-limit-free silence-scan reader

Fixed in ccc9aa0. Correct finding. My incremental
silence-scan reader used StreamReader.readline(), whose default 64 KiB limit raises
ValueError on a single line past it; the silence-scan ffmpeg call had no -hide_banner,
so a crafted container's >64 KiB metadata tag on one line made readline() raise, and my
except BaseException re-raised it as an unhandled 500. Replaced both readline() loops in
_drain_progress_and_silence with fixed-size read(65536) chunk reads and a bounded
rolling buffer: each chunk appends to a pending fragment, complete lines (up to the last
newline) are scanned once with finditer, and the trailing partial is capped at 256 bytes
(longer than any out_time_us/silence_end token), so a giant no-newline line can neither
raise nor grow memory. Each line is scanned exactly once, so the appending silence
consumer never double-counts. Also added -hide_banner to the scan argv as defence in
depth so the metadata dump does not reach stderr at all. New test
test_an_over_64kib_stderr_line_with_no_newline_does_not_raise pins it (>200 KiB no-newline
stderr, duration still parsed, no raise).
Class-level rule: read subprocess streams with read(n) + a bounded buffer, never
readline() (which has a hidden line-length limit) nor communicate() (unbounded), on any
stream whose content an untrusted input influences. This closes the readline variant of
the same bounded-read class as the earlier communicate() fixes.

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 7, 2026
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
@chenmingwei23
chenmingwei23 force-pushed the feat/meetings-import-autosplit-8272 branch from ccc9aa0 to caf4c2a Compare September 8, 2026 02:01
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 8, 2026
@bolichen97
bolichen97 merged commit ceb3130 into main Sep 8, 2026
65 checks passed
@bolichen97
bolichen97 deleted the feat/meetings-import-autosplit-8272 branch September 8, 2026 06:39
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Meetings import: auto-split recordings longer than the STT duration cap instead of refusing with 413

3 participants