Skip to content

fix(sdk/python): generate_music no longer hardcodes stream=true for non-pcm16 formats - #963

Open
AbirAbbas wants to merge 1 commit into
mainfrom
triage/music
Open

fix(sdk/python): generate_music no longer hardcodes stream=true for non-pcm16 formats#963
AbirAbbas wants to merge 1 commit into
mainfrom
triage/music

Conversation

@AbirAbbas

Copy link
Copy Markdown
Contributor

Summary

OpenRouterProvider.generate_music still hardcoded "stream": True with a default format of wav, so the plain generate_music("...") call reproduced issue #584's 400: 'audio.format' does not support 'wav' when stream=true. Supported values are: 'pcm16'. PR #962 fixed exactly this for generate_audio but did not touch generate_music. This applies the same format-driven transport routing to generate_music, reusing the helpers #962 added (_nonstream_openrouter_audio, _wrap_pcm16_as_wav_b64) rather than duplicating them: wav → wire pcm16 over SSE then re-wrapped client-side as RIFF/WAVE, pcm16 → SSE unchanged, mp3/flac/opusstream=False via the non-streaming JSON path with label="music".

Everything else about generate_music is unchanged — prompt/duration-hint message shape, model resolution and openrouter/ prefix stripping, duration validation, and the returned MultimodalResponse fields — and there is a test pinning that.

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Docs only
  • Tests only
  • CI / tooling
  • Breaking change

Test plan

All commands run from sdk/python/.

  • uvx ruff@0.15.22 check .All checks passed!
  • uvx ruff@0.15.22 format agentfield/media_providers.py tests/test_openrouter_audio.py tests/test_media_integration.py then uvx ruff@0.15.22 format --check <same files>3 files already formatted
  • uv run --extra dev ./scripts/run_pytest.sh -p no:cacheprovider2009 passed, 4 skipped, 38 deselected in 112.36s, exit 0; coverage TOTAL 2002 stmts / 116 miss / 94%
  • uv run --python 3.10 --extra dev ./scripts/run_pytest.sh -p no:cacheprovider --no-cov -q → exit 0 (tests=2013, failures=0, errors=0, skipped=4 from the JUnit summary; this project's -q run does not print the trailing counts line under 3.10)
  • Targeted: uv run --extra dev python -m pytest tests/test_openrouter_audio.py tests/test_media_integration.py -p no:cacheprovider --no-cov -q68 passed

Red-before-green check: with the media_providers.py change reverted and only the new tests in place, pytest tests/test_openrouter_audio.py::TestOpenRouterMusicFormatRouting fails 4 of 7 — test_default_format_streams_pcm16_and_returns_wav, test_compressed_formats_use_nonstream_json[mp3], test_compressed_formats_use_nonstream_json[flac], test_request_shape_is_identical_across_transports[mp3-False] — i.e. every contract item that describes new behaviour genuinely fails against the old code.

Validation contract

Behaviours this change must exhibit, and the test each maps to (all in sdk/python/tests/test_openrouter_audio.py, class TestOpenRouterMusicFormatRouting, asserting on the payload actually POSTed and on the returned audio via the same aiohttp fake pattern the generate_audio tests use):

# Behaviour Test
C1 A default generate_music(prompt=...) call (no format) sends stream: True with audio.format == "pcm16" on the wire, and returns audio whose format is "wav" and whose base64 decodes to a RIFF/WAVE container still containing the streamed pcm bytes. test_default_format_streams_pcm16_and_returns_wav
C2 format="pcm16" sends stream: True with wire format pcm16 and returns the concatenated base64 verbatim, format "pcm16". test_pcm16_streams_and_returns_data_untouched
C3 format="mp3" and format="flac" send stream: False to /chat/completions with the requested wire format, and the result is parsed from choices[0].message.audio.{data,transcript}. test_compressed_formats_use_nonstream_json[mp3], [flac]
C4 A non-200 HTTP response on the non-streaming music path raises RuntimeError whose message contains "music" and the status code. test_nonstream_http_error_mentions_music_and_status (402 + opus)
C5 The request shape is unchanged versus the current code for the same inputs, and identical across both transports: default model google/lyria-3-pro, openrouter/ prefix stripped, messages == [{"role": "user", "content": "<prompt> (duration: N seconds)"}], modalities == ["text", "audio"]; duration validation still raises ValueError before any HTTP call. test_request_shape_is_identical_across_transports[wav/pcm16/mp3], test_duration_validation_runs_before_any_request

Two existing tests were re-pinned, not reshaped

test_openrouter_generate_music_streams and test_media_integration.py::test_music_generation_returns_audio requested the default wav format while asserting the raw streamed base64 came back. Only the pcm16 wire path can return that, so both are now pinned to format="pcm16" and additionally assert stream is True — the same treatment #962 gave the two generate_audio SSE tests. Their intent (SSE parsing → AudioOutput) is preserved; the new C1 test covers what the default wav call now does.

Note on the diff

ruff format reflowed two pre-existing f-strings inside _nonstream_openrouter_audio (added by #962, which left them not format-clean). Those two hunks are mechanical and are required for ruff format --check on this file to pass; no other unrelated formatting was touched.

The WAV re-wrap uses sample_rate=24000, matching generate_audio exactly so the two paths stay consistent. The generate_music docstring's "48kHz stereo" line describes the upstream model output, not the client-side mono RIFF header; if OpenRouter's music pcm16 stream turns out to be 48 kHz, that constant should change in both call sites together.

Test coverage

  • I ran tests for the surface(s) I changed locally.
  • New code paths are covered by tests in this PR (no bare additions). Both new branches in generate_music (non-stream dispatch, wav re-wrap) are exercised by C1/C3/C4.
  • If I removed code, I updated coverage-baseline.json in this PR only if the removal caused a legitimate regression and I called it out in the summary above. No code removed; no baseline change.
  • The coverage gate check is green in CI before requesting review. Not yet observable — nothing has been pushed.

Checklist

Related issues / PRs

Refs #584
Refs #962

…eaming OpenRouter path (#584)

OpenRouterProvider.generate_music hardcoded "stream": True with a default
format of "wav", so the default call reproduced #584's 400:

    'audio.format' does not support 'wav' when stream=true.
    Supported values are: 'pcm16'

PR #962 fixed exactly this for generate_audio but left generate_music
untouched. Apply the same routing here, reusing the helpers #962 added
rather than duplicating them:

- wav  -> request pcm16 over the wire, stream over SSE, then re-wrap the
  pcm16 payload as a RIFF/WAVE container via _wrap_pcm16_as_wav_b64
- pcm16 -> stream over SSE, returned base64 untouched
- mp3 / flac / opus -> stream=False via _nonstream_openrouter_audio with
  label="music"

Everything else about generate_music is unchanged: prompt/duration-hint
message shape, model resolution and openrouter/ prefix stripping,
duration validation, and the returned MultimodalResponse fields.

Two existing SSE tests requested the default wav format while asserting
the raw streamed base64 — only the pcm16 wire path can return that, so
they are pinned to format="pcm16" and now assert stream is True, the same
treatment #962 gave the generate_audio tests.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AbirAbbas
AbirAbbas requested a review from a team as a code owner August 26, 2026 15:06
@AbirAbbas
AbirAbbas enabled auto-merge August 26, 2026 15:06
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.

1 participant