Repository navigation
Python: Accept the registered audio/mpeg media type for MP3 input content - #8787
Conversation
|
Grapette.L (@Lesereingrape) please agree to the CLA. |
|
/review |
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (1 commit(s)): f65c1c35219a
Model: gpt-5.6-sol
Overview
This PR makes both OpenAI serializers recognize the standard audio/mpeg media type, with parity across the Responses and Chat Completions clients and tests preserving WAV, audio/mp3, and unsupported OGG behavior. Core detection, Telegram, and DevUI provide concrete producers of audio/mpeg. However, the unbounded substring match also reclassifies unrelated MPEG-family media types and can send incompatible payloads as MP3, so the alias check needs to be narrowed.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
1 verified finding remained after source verification (1 medium) across 1 file. Details are attached to the affected lines below.
Affected areas: python/packages/openai/agent_framework_openai/_chat_client.py
|
@microsoft-github-policy-service agree |
Normalize the media type (drop parameters, case-fold) and compare it against the registered MP3 aliases so audio/mpegurl and audio/mpeg4-generic take the unsupported-content path in both serializers.
Motivation & Context
MP3 is the only audio format besides WAV that OpenAI's
input_audiopart accepts, and both OpenAI chat clients support it — but only when the caller writes the non-standard media typeaudio/mp3. The registered type for MP3 isaudio/mpeg(RFC 3008), and because the format is chosen by substring-matching"mp3"/"wav"againstcontent.media_type,audio/mpegfalls through to the "unsupported" branch and the whole content item is silently dropped (warning in the Responses client, debug log in the Chat Completions client).The framework's own helpers produce the rejected spelling:
detect_media_type_from_base64()returns"audio/mpeg"for MP3 magic bytes (python/packages/core/agent_framework/_types.py:171), and theContent.from_data()docstring points callers at that helper to setmedia_type;"audio/mpeg"is also listed in core'sKNOWN_MEDIA_TYPES.agent-framework-hosting-telegramlabels inboundaudiomessages as"audio/mpeg"(_parsing.py:30-35), so a Telegram audio message sent to an OpenAI-based agent disappears from the request without any error reaching the caller.Description & Review Guide
audio/mpeg(and othermpeg-spelled MP3 types such asaudio/x-mpeg,audio/mpeg3) as MP3 when converting audio content, in bothOpenAIChatClient(_chat_client.py:2123) andOpenAIChatCompletionClient(_chat_completion_client.py:1318):"mp3" in content.media_type or "mpeg" in content.media_type. Two one-line changes; both clients had the identical defect, so both are fixed to keep them in parity.data/uricontent that carries the standard media type is now converted to{"type": "input_audio", "input_audio": {..., "format": "mp3"}}instead of being dropped. Nothing else in the branch changes: the WAV check still runs first, and genuinely unsupported audio types (audio/ogg,audio/flac, ...) still return{}as before —audio/oggis asserted in both test modules and stays green.mpegas a substring is the right shape for the alias, versus an explicit media type allow list. I kept the existing substring style so the diff stays minimal and the already-supportedaudio/mp3keeps working, but I am happy to switch to an explicit set if that is preferred.Local run of the package's full gate (
uv run python scripts/workspace_poe_tasks.py check --package openai):fmtandlintpass, all five typing checkers (pyright, mypy, ty, pyrefly, zuban) report 0 errors, and 623 tests pass with 94% coverage on the package. Before the source change, the two addedaudio/mpegassertions fail withKeyError: 'type'intest_openai_chat_client.py::test_prepare_content_for_openai_audio_contentandtest_openai_chat_completion_client.py::test_prepare_content_for_openai_data_content_image.Related Issue
Fixes #8786
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.