Phase 2 Plan 02-02: audio I/O hardening + WAV-export correctness - #96
Merged
Conversation
Centralizes WAV/audio writes through a single audited path that defends
against the four documented torchaudio.save failure modes (CUDA/MPS
tensor, non-contiguous, out-of-range, wrong dtype) AND the torchaudio
2.9+ TorchCodec-delegation behavior drift.
* services/audio_io.py:_safe_torchaudio_save now performs:
- .cpu() move (torchaudio cannot serialize CUDA/MPS)
- dtype coercion to torch.float32
- .clamp(-1.0, 1.0) (out-of-range = silent clipping on some backends)
- .unsqueeze(0) for 1D (mono) inputs
- .contiguous() (torch.cat of slices = non-contig = silent corruption)
- explicit encoding="PCM_S/PCM_F" + bits_per_sample so future
torchaudio backend selection cannot drift the on-disk format
- format passthrough for wav/flac/mp3/ogg with encoding-kwarg fallback
for older codec builds
* services/audio_io.py:_safe_soundfile_write — sibling helper for the
one sf.write call site (dub_core.py). Applies the same dtype/contig/
range checks before delegating to soundfile.write.
* services/audio_io.py:atomic_save_wav (existing P0 helper) now
delegates the actual encode to _safe_torchaudio_save so atomicity
and correctness compose: every byte that lands at the target path
was produced by the audited helper.
* tests/backend/services/test_audio_io.py — 29 tests (25 pass + 4
skipped for MPS dtype incompatibility): parametric round-trip across
dtype x device x contiguity, plus out-of-range clamp, format
passthrough, in-memory buffer, empty-tensor rejection, 1D auto-
unsqueeze, and a smoke check that atomic_save_wav inherits the
safety guarantees.
No new Python dependencies. SoniTranslate untouched (D1 locked).
Refs BUG-01 / #48.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Migrates all 12 grep-audit bare audio-write call sites in backend/api/routers/ to route through services.audio_io. Closes the last surface area of BUG-01 / #48 that the P0 atomic-write commit (fb52140) did not cover. Sites migrated (grep before → after): generation.py:148 torchaudio.save → _safe_torchaudio_save generation.py:162 torchaudio.save → _safe_torchaudio_save openai_compat.py:155 torchaudio.save → _safe_torchaudio_save openai_compat.py:160 torchaudio.save → _safe_torchaudio_save openai_compat.py:168 torchaudio.save → _safe_torchaudio_save openai_compat.py:172 torchaudio.save → _safe_torchaudio_save openai_compat.py:178 torchaudio.save → _safe_torchaudio_save openai_compat.py:182 torchaudio.save → _safe_torchaudio_save openai_compat.py:193 torchaudio.save → _safe_torchaudio_save dub_generate.py:509 torchaudio.save → _safe_torchaudio_save batch.py:341 torchaudio.save → atomic_save_wav (track assembly) dub_core.py:438 sf.write → _safe_soundfile_write batch.py:341 specifically swapped to atomic_save_wav (not just the safe helper) because it writes the final track to disk — same shape as dub_generate.py:390 — and needs atomic publication, not only audited encoding. atomic_save_wav already delegates internally to _safe_torchaudio_save (per the Task 1 commit) so it inherits both guarantees. openai_compat.py:185 pcm branch produces raw int16 bytes (no container), so it can't go through _safe_torchaudio_save; it now inlines the same .cpu/.float32/.clamp/.contiguous sanity steps the helper enforces. tests/backend/test_dub_pipeline_wav.py: - test_no_bare_audio_writes_in_routers (in-process grep gate) - test_no_bare_audio_writes_via_subprocess_grep (CI-shell parity gate) - test_track_assembly_handles_non_contig_after_torch_cat (the #48 smoking-gun reproduction — torch.cat of out-of-range non-contig slices saved through the helper) - test_atomic_save_wav_assembly_pattern (same shape, via atomic_save_wav) - test_safe_soundfile_write_dub_core_pattern (ASR transcribe-chunk pattern from dub_core.py) - test_dub_pipeline_produces_valid_wav (xfailed — Phase 0 fixture sample_5s.mp4 not present; structural reproduction tests above already cover the helper code path #48 went through) Grep gate is green: grep -nE '(torchaudio\.save|soundfile\.write|sf\.write)\(' \ backend/api/routers/ -r --include='*.py' \ | grep -v '_safe_torchaudio_save\|_safe_soundfile_write' \ | grep -v '^[^:]*:[[:space:]]*#' \ returns 0 lines. Full suite green: 348 passed, 10 skipped, 13 xfailed, 1 xpassed. SoniTranslate untouched (D1 locked). Closes BUG-01 / #48. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis PR hardens audio I/O by centralizing write operations through audited helpers ( ChangesAudio I/O Hardening & Router Migration
Sequence DiagramsequenceDiagram
participant Router as Route Handler
participant SafeWrite as _safe_torchaudio_save
participant TorchAudio as torchaudio.save
Router->>SafeWrite: tensor (raw float/int/non-contiguous)
SafeWrite->>SafeWrite: CPU move & float32 coerce
SafeWrite->>SafeWrite: clamp values to [-1, 1]
SafeWrite->>SafeWrite: reshape to 2D (channels, samples)
SafeWrite->>SafeWrite: enforce contiguity
SafeWrite->>TorchAudio: call with explicit bits_per_sample=16
TorchAudio-->>SafeWrite: WAV file (PCM_16 on-disk format)
SafeWrite-->>Router: return or fallback on non-WAV format error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/backend/services/test_audio_io.py (1)
80-90: ⚡ Quick winAdd an int16 anti-clipping assertion to catch normalization regressions.
Current checks can pass even if int16 input is saturated to rails. Add a simple distribution/fidelity assertion for the int16 branch.
💡 Proposed test addition
samples, _ = sf.read(str(target)) assert samples.size > 0 assert abs(samples).max() > 0.1, ( f"samples too quiet: max={abs(samples).max()} — silent-corruption mode" ) + if dtype == torch.int16: + unclipped_ratio = float(np.mean(np.abs(samples) < 0.98)) + assert unclipped_ratio > 0.5, ( + "int16 path appears heavily clipped; expected normalized waveform, " + f"got unclipped_ratio={unclipped_ratio:.3f}" + )Also applies to: 115-119
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/backend/services/test_audio_io.py` around lines 80 - 90, Add an anti-clipping assertion for the int16 branch in tests/backend/services/test_audio_io.py: when dtype == torch.int16 (where wave_f32 is created via _sine_tensor and wave = (wave_f32 * 32767).to(torch.int16)), assert that the int16 wave is not saturated to rails (e.g., ensure not all values equal int16 max/min and that a reasonable fraction lie between rails) to catch normalization regressions; add the same kind of distribution/fidelity check for the other int16 case around the second int16 branch so both int16 test paths validate non-clipped signal content.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/services/audio_io.py`:
- Around line 124-131: The code currently casts integer PCM tensors to float32
then immediately clamps, which collapses integer ranges to ±1.0; modify the
logic in the audio conversion routine (the block handling tensor dtype in
backend/services/audio_io.py) to first detect integer dtypes (e.g., torch.int16,
torch.int32, torch.int8, torch.uint8), convert them to float32 and normalize by
their full-scale value (e.g., divide by 32768 for int16, 2147483648 for int32,
128 for int8, and 255/128 handling for uint8) to map samples into the -1.0..1.0
range, then apply tensor.clamp(-1.0, 1.0); leave non-integer floats unchanged
except for the existing dtype cast and clamping. Ensure you update the branch
that references tensor.dtype and tensor.clamp so integer inputs are normalized
before clamping.
- Around line 248-263: The function _safe_soundfile_write currently allows
NaN/Inf in samples to reach sf.write; add a finite-value check after
casting/contiguity (e.g., check np.isfinite(samples).all()) and if any
non-finite values are present raise a clear ValueError (include
sample_rate/path/subtype context) instead of calling sf.write; ensure this
validation runs for both float and integer branches before calling sf.write to
prevent invalid files.
---
Nitpick comments:
In `@tests/backend/services/test_audio_io.py`:
- Around line 80-90: Add an anti-clipping assertion for the int16 branch in
tests/backend/services/test_audio_io.py: when dtype == torch.int16 (where
wave_f32 is created via _sine_tensor and wave = (wave_f32 *
32767).to(torch.int16)), assert that the int16 wave is not saturated to rails
(e.g., ensure not all values equal int16 max/min and that a reasonable fraction
lie between rails) to catch normalization regressions; add the same kind of
distribution/fidelity check for the other int16 case around the second int16
branch so both int16 test paths validate non-clipped signal content.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f19cf185-d7ec-40e8-97e6-f65d10c47e45
📒 Files selected for processing (9)
.planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-02-SUMMARY.mdbackend/api/routers/batch.pybackend/api/routers/dub_core.pybackend/api/routers/dub_generate.pybackend/api/routers/generation.pybackend/api/routers/openai_compat.pybackend/services/audio_io.pytests/backend/services/test_audio_io.pytests/backend/test_dub_pipeline_wav.py
Comment on lines
+124
to
+131
| if tensor.dtype != torch.float32: | ||
| tensor = tensor.to(torch.float32) | ||
|
|
||
| # ── Failure mode 3: out-of-range values. apply_mastering produces | ||
| # transient peaks > 1.0 on dynamic input; the soundfile backend | ||
| # wraps these around (int16 overflow) on some platforms instead of | ||
| # clipping, producing audible pops. | ||
| tensor = tensor.clamp(-1.0, 1.0) |
Contributor
There was a problem hiding this comment.
Normalize integer PCM before clamping to avoid hard clipping.
On Line 124, integer tensors are cast to float32 and then clamped. For int16/int32 inputs this collapses most samples to ±1.0 and audibly distorts output.
💡 Proposed fix
- if tensor.dtype != torch.float32:
- tensor = tensor.to(torch.float32)
+ if tensor.is_floating_point():
+ if tensor.dtype != torch.float32:
+ tensor = tensor.to(torch.float32)
+ elif tensor.dtype in (torch.int8, torch.int16, torch.int32, torch.int64):
+ info = torch.iinfo(tensor.dtype)
+ scale = float(max(abs(info.min), info.max))
+ tensor = tensor.to(torch.float32) / scale
+ else:
+ raise TypeError(
+ f"_safe_torchaudio_save unsupported dtype {tensor.dtype}; expected float or signed int"
+ )
# ── Failure mode 3: out-of-range values...
tensor = tensor.clamp(-1.0, 1.0)
+ if not torch.isfinite(tensor).all():
+ raise ValueError("_safe_torchaudio_save expects finite samples (no NaN/Inf)")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/audio_io.py` around lines 124 - 131, The code currently
casts integer PCM tensors to float32 then immediately clamps, which collapses
integer ranges to ±1.0; modify the logic in the audio conversion routine (the
block handling tensor dtype in backend/services/audio_io.py) to first detect
integer dtypes (e.g., torch.int16, torch.int32, torch.int8, torch.uint8),
convert them to float32 and normalize by their full-scale value (e.g., divide by
32768 for int16, 2147483648 for int32, 128 for int8, and 255/128 handling for
uint8) to map samples into the -1.0..1.0 range, then apply tensor.clamp(-1.0,
1.0); leave non-integer floats unchanged except for the existing dtype cast and
clamping. Ensure you update the branch that references tensor.dtype and
tensor.clamp so integer inputs are normalized before clamping.
Comment on lines
+248
to
+263
| if samples.dtype not in (np.float32, np.float64, np.int16, np.int32): | ||
| samples = samples.astype(np.float32) | ||
|
|
||
| # Out-of-range protection for float inputs. | ||
| if samples.dtype in (np.float32, np.float64): | ||
| # ``np.clip`` with ``out=`` requires the out array to be | ||
| # writable + same dtype. ``np.ascontiguousarray`` may return | ||
| # the original array (writable) or a copy (also writable), so | ||
| # clipping in place is safe after it. | ||
| samples = np.ascontiguousarray(samples) | ||
| np.clip(samples, -1.0, 1.0, out=samples) | ||
| else: | ||
| samples = np.ascontiguousarray(samples) | ||
|
|
||
| sf.write(path, samples, sample_rate, subtype=subtype) | ||
|
|
Contributor
There was a problem hiding this comment.
Reject NaN/Inf samples before writing with soundfile.
_safe_soundfile_write documents finite-sample invariants but currently allows NaN/Inf through to sf.write, which can produce invalid output behavior.
💡 Proposed fix
if samples.dtype in (np.float32, np.float64):
# ``np.clip`` with ``out=`` requires the out array to be
# writable + same dtype. ``np.ascontiguousarray`` may return
# the original array (writable) or a copy (also writable), so
# clipping in place is safe after it.
samples = np.ascontiguousarray(samples)
+ if not np.isfinite(samples).all():
+ raise ValueError("_safe_soundfile_write expects finite samples (no NaN/Inf)")
np.clip(samples, -1.0, 1.0, out=samples)
else:
samples = np.ascontiguousarray(samples)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/audio_io.py` around lines 248 - 263, The function
_safe_soundfile_write currently allows NaN/Inf in samples to reach sf.write; add
a finite-value check after casting/contiguity (e.g., check
np.isfinite(samples).all()) and if any non-finite values are present raise a
clear ValueError (include sample_rate/path/subtype context) instead of calling
sf.write; ensure this validation runs for both float and integer branches before
calling sf.write to prevent invalid files.
3 tasks
This was referenced Jun 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes BUG-01 / closes #48.
Summary
backend/api/routers/on a single audited helper path inbackend/services/audio_io.py. The two new helpers (_safe_torchaudio_save,_safe_soundfile_write) enforce CPU device, float32 dtype,[-1, 1]clamp, contiguous memory, and explicitencoding=PCM_S/PCM_F+bits_per_samplebefore delegating — defending against all four documented torchaudio.save silent-corruption modes plus the torchaudio 2.9+ TorchCodec-backend behavior drift.atomic_save_wav, commit fb52140) is preserved — it now delegates the actual encode to_safe_torchaudio_save, so atomicity and audited normalization compose. Every byte that lands at a dub-output path was produced by the audited helper..planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-02-SUMMARY.md). A CI grep gate in the new test file prevents future drift.Coverage of the P0 commit
Commit fb52140 covered three dub-pipeline disk-write sites (
dub_generate.py:289/328/390) with atomicos.replacesemantics. This PR does not regress that coverage:atomic_save_wavunchanged.atomic_save_wavnow delegates internally to_safe_torchaudio_save, so the on-disk WAV at those three paths additionally inherits the new audit checks (clamp, contig, encoding lock).backend/tests/test_atomic_wav.py(the P0 regression suite) still passes — 7/7 green.New artifacts
backend/services/audio_io.py— extended (_safe_torchaudio_save,_safe_soundfile_write,atomic_save_wavnow delegates to the safe helper)tests/backend/services/test_audio_io.py— 29 parametric tests (25 pass + 4 skipped for MPS dtype incompatibility)tests/backend/test_dub_pipeline_wav.py— 6 tests (5 pass + 1 xfail pending Phase 0 video fixture); includes in-process and subprocess grep gates, thetorch.cat-of-slices [Bug] Exported Wav file is corrupted #48 smoking-gun reproduction, theatomic_save_wavassembly-pattern test, and the_safe_soundfile_writedub_core ASR-chunk test.planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-02-SUMMARY.md— execution summaryTest plan
uv run pytest tests/backend/services/test_audio_io.py -v— 25 passed, 4 skipped (MPS dtype)uv run pytest tests/backend/test_dub_pipeline_wav.py -v— 5 passed, 1 xfail (missing fixture)uv run pytest tests/ -q --ignore=tests/manual— 348 passed, 10 skipped, 13 xfailed, 1 xpasseduv run pytest tests/smoke/ -q— 4 passedcd backend && uv run pytest tests/test_atomic_wav.py -v— 7 passed (P0 not regressed)torchaudio.save/soundfile.write/sf.writeinbackend/api/routers/git diff backend/services/sonitranslate.pyempty (D1 locked decision)🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests