Skip to content

fix(multimodal): raise explicit errors in Audio instead of assert#2361

Closed
IgnazioDS wants to merge 1 commit into
567-labs:mainfrom
IgnazioDS:fix-audio-validation-errors
Closed

fix(multimodal): raise explicit errors in Audio instead of assert#2361
IgnazioDS wants to merge 1 commit into
567-labs:mainfrom
IgnazioDS:fix-audio-validation-errors

Conversation

@IgnazioDS

Copy link
Copy Markdown
Contributor

What & why

Audio.from_url and Audio.from_path validate inputs with bare assert statements:

assert content_type in VALID_AUDIO_MIME_TYPES, "..."
assert path.is_file(), "..."
assert mime_type in VALID_AUDIO_MIME_TYPES, "..."

Two problems:

  1. assert is stripped under python -O (PEP 230), so these validation checks silently vanish in optimized deployments — invalid audio then flows downstream instead of being rejected at the boundary.
  2. When they do fire, they raise AssertionError rather than the ValueError callers expect.

This also leaves Audio inconsistent with its sibling classes: Image.from_path and PDF.from_path already raise FileNotFoundError for a missing file, ValueError("... file is empty") for an empty file, and ValueError("Unsupported ... format: ...") for an unsupported type.

Change

Replace the asserts in Audio with the same explicit errors Image/PDF use:

  • missing file → FileNotFoundError
  • empty file → ValueError("Audio file is empty") (new check, matching Image/PDF)
  • unsupported format (URL or path) → ValueError("Unsupported audio format: ...")

No behavior change for valid inputs.

Tests

Adds 4 regression tests in tests/v2/test_core_multimodal_runtime.py — each fails on main and passes here:

  • from_url with an unsupported content-type → ValueError
  • from_path with an unsupported format → ValueError
  • from_path with an empty file → ValueError
  • from_path with a missing file → FileNotFoundError

Verification: pytest tests/v2/test_core_multimodal_runtime.py → 18 passed; tests/multimodal/test_multimodal.py (non-LLM) → 57 passed; ruff check / ruff format --check / ty check clean. CHANGELOG.md updated under [Unreleased].

`Audio.from_url` and `Audio.from_path` validated inputs with bare
`assert` statements. Asserts are silently stripped under `python -O`
(PEP 230), so the checks vanish in optimized deployments, and when they
do fire they raise `AssertionError` rather than the `ValueError` used
everywhere else. This left `Audio` inconsistent with `Image` and `PDF`,
which already raise `FileNotFoundError` / `ValueError` and reject empty
files.

Replace the asserts with explicit `FileNotFoundError` (missing file)
and `ValueError` (empty file, unsupported format), mirroring the
`Image`/`PDF` pattern. Add regression tests covering all four cases.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

jxnl commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Consolidated this Audio validation fix into draft PR #2400. Closing this PR to keep the queue focused on the consolidated patch; the new PR carries the explicit ValueError / FileNotFoundError behavior and focused regression tests.

@jxnl jxnl closed this Jun 29, 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.

2 participants