fix(voice): accept every NumPy spelling of a supported TTS dtype

[Nikhils-G]

Summary

This pull request fixes StreamedAudioResult rejecting supported TTSModelSettings.dtype spellings.

dtype is typed as npt.DTypeLike, and dictionary settings such as config={"tts_settings": {"dtype": "float32"}} keep the value as the string "float32". _transform_audio_buffer compared that value with == np.int16 / == np.float32, which is False for strings, so the pipeline raised UserError("Invalid output dtype") inside the TTS task after the text-to-speech request had already been sent. Only the np.float32 / np.int16 type objects and np.dtype instances worked.

The buffer transform now resolves the configured value with np.dtype() before comparing, so every spelling of a supported dtype produces audio in that dtype. NumPy reports an unparseable dtype as either TypeError or ValueError, and both are caught so the UserError boundary is preserved either way. The emitted array shapes are unchanged.

A non-native byte order such as ">i2" is still rejected, deliberately. Samples are read off the PCM stream in native order, so honoring a byte-swapped request would mean converting them rather than relabeling them, and returning the array unchanged would hand the caller different values than it asked to read. That is left alone here and pinned by a test.

Test plan

• test_voicepipeline_accepts_numpy_dtype_spellings runs VoicePipeline with dictionary TTS settings for "float32", "int16", and np.dtype("float32") and asserts the emitted audio dtype and lifecycle events. The string cases fail on main.
• test_voicepipeline_rejects_unsupported_output_dtype covers a dtype that resolves but is unsupported ("int32"), one NumPy cannot parse (a malformed structured dtype), and a non-native byte order (">i2"). The structured-dtype case fails without the ValueError catch.
• Ran .agents/skills/code-change-verification/scripts/run.sh; formatting, lint, type checking, and the full test suite passed.

Issue number

Fixes#4777

Checks

• I've added new tests, if relevant
• I've run .agents/skills/code-change-verification/scripts/run.sh
• I've confirmed all verification steps pass
• If using Codex, I've run /review before submitting this PR

[Copilot]

Pull request overview

This pull request fixes VoicePipeline’s streamed audio output conversion so that TTSModelSettings.dtype accepts all NumPy-resolvable spellings of the supported output dtypes (int16 and float32), including string values commonly produced by dict/JSON/YAML config.

Changes:

• Normalize the configured dtype via np.dtype(...) before validating/converting streamed PCM buffers.
• Add regression tests ensuring string spellings like "float32" / "int16" are accepted, and unsupported dtypes still raise UserError.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/agents/voice/result.py	Resolves output_dtype with np.dtype() before comparing against supported dtypes, fixing string-spelling rejection.
tests/voice/test_pipeline.py	Adds coverage for accepted dtype spellings and for rejecting an unsupported dtype via the public VoicePipeline streaming path.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[linhongyu510]

I reproduced the new string-dtype cases on this head: the four focused tests pass. One supported error-path regression remains in the normalization boundary. np.dtype() can raise ValueError as well as TypeError; for example, the public dict-config path accepts tts_settings.dtype={"names":["x"],"formats":[]}, then VoicePipeline.run(...).stream() now leaks ValueError: names, formats, offsets, and titles dict entries must have the same length. Before this change, an unsupported dtype reached the existing UserError("Invalid output dtype") branch, and the PR description promises that unresolvable dtypes keep that error contract.

Could the handler catch (TypeError, ValueError) and add one public-pipeline regression for a malformed structured dtype? That keeps the patch narrow while ensuring all NumPy parse failures preserve the SDK-owned UserError boundary.

[Nikhils-G]

Good catch, thank you — you're right, and it was a regression from this patch rather than a pre-existing gap.

I reproduced it on the previous head: with tts_settings.dtype={"names": ["x"], "formats": []} the dict-config path keeps the dict verbatim, and stream() raised ValueError: 'names', 'formats', 'offsets', and 'titles' dict entries must have the same length. On the merge base that same value fell through the == comparisons and reached UserError("Invalid output dtype"), so the patch narrowed the error contract it claimed to preserve. np.dtype() also raises ValueError for other shapes, such as a fixed-type tuple with a negative dimension.

Fixed in 2cb8c3b: the handler now catches (TypeError, ValueError), and the existing rejection test is parametrized over a dtype that resolves but is unsupported ("int32") and one NumPy cannot parse (the malformed structured dtype). The new case fails on the previous head with the leaked ValueError and passes here. Verification stack is green again.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:0834d1153a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Use a host-independent non-native dtype case

On a big-endian host, ">i2" is the native int16 dtype, so np.dtype(">i2") == np.int16 and the pipeline successfully emits audio instead of raising the UserError expected by this test. This makes the test suite platform-dependent; construct the opposite byte order dynamically, for example from np.dtype(np.int16).newbyteorder("S"), rather than assuming little-endian execution.

Useful? React with 👍 / 👎.

[Nikhils-G]

Good catch, that was a real portability bug in the test rather than a style point — on a big-endian host ">i2" is the native order, so the case would have failed there for the wrong reason.

Fixed in c189427 using the expression you suggested, np.dtype(np.int16).newbyteorder("S"), so the case is the non-native order on either kind of host. Checked that it still does not compare equal to np.int16, and that swapping it twice returns to native.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:620820d8dc

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Decode PCM with fixed byte order before accepting native dtype

On a big-endian host, the newly supported "int16" spelling resolves equal to native np.int16, so this branch returns np_array even though line 107 decoded the incoming bytes in native big-endian order. The repository’s PCM helper explicitly produces little-endian bytes with dtype="<i2" (src/agents/voice/testing.py:403-405), so nonzero samples are byte-swapped for this newly accepted configuration. Unlike the earlier test-portability comment, this is fresh production-path evidence: decode the PCM as little-endian and convert the emitted array to the requested native dtype.

AGENTS.md reference: AGENTS.md:L164-L164

Useful? React with 👍 / 👎.

[Nikhils-G]

I do not think this one belongs to this patch, and the diff shows why.

The decode is untouched here — np.frombuffer(combined_buffer, dtype=np.int16) appears as context in the diff, not as a changed line. On main those two lines already read:

np_array=np.frombuffer(combined_buffer, dtype=np.int16)
ifoutput_dtype==np.int16:
returnnp_array

Both np.int16 and np.dtype("int16") satisfy that comparison today, so the branch is already reachable on a supported path and already returns natively decoded samples. This patch adds the string spelling to the same branch; it does not change what the branch does, or how the bytes were read before it.

On the citation: pcm16_samples lives in agents/voice/testing.py and its docstring says it produces native little-endian bytes for fixtures. The production path takes its bytes from the TTS model rather than from that helper.

The underlying point is fair, though. Decoding provider PCM16 as native rather than little-endian is wrong on a big-endian host, and it is wrong there for np.int16 on main right now. Correcting it means reading as "<i2" and converting on output, which changes what existing callers receive on those hosts — a different change with a different rationale. By the AGENTS.md line cited above, that is a pre-existing condition rather than one this patch introduces or worsens.

Happy to raise it as its own issue, or to fold it in here if a maintainer would rather have it in one go.

[ErenAta16]

The fix is right, and resolving through np.dtype() rather than widening the
comparison is the part I would defend if it came up: it accepts the spellings a
dictionary config actually carries without hand-maintaining a list of aliases.

Two things worth having on the record before this lands.

#4794 is the same change.@Hughhhhcoder opened it a day after this one, also
against #4777. I ran both source changes over every spelling I could think of and
they are indistinguishable:

input #4778 #4794
'int16' int16 int16
'float32' float32 float32
'i2' / 'f4' int16/f32 int16/f32
'<i2' / '=i2' int16 int16
'>i2' UserError UserError
np.int16 int16 int16
np.dtype(...) int16 int16
'int32' 'float64' 'nonsense' None 3.5 object UserError in both

Sixteen inputs, sixteen matches. Rejecting '>i2' is correct in both, since the
buffer is read as native int16 and handing it back labelled big-endian would
misdescribe the bytes, so that is not a gap either of you needs to close.

Given that, seniority and coverage both point here: this one is a day older, it is
by the person who reported #4777, and it carries +97 of tests against +37.

The one thing to take from the other branch is the chaining. This raises
UserError("Invalid output dtype") from None, which drops the NumPy exception that
explains why the value failed to parse. #4794 uses from error. That is the house
style, and not marginally:

raise UserError(...) from ... exc x5, error x1, e x1, None x0
any raise ... from ... in src/ e x170, exc x58, error x14, None x23

There is no from None on a UserError anywhere in the SDK today, so this would
be the first. from None earns its place when the inner exception is noise that
would mislead, and here it is the opposite: np.dtype("flaot32") raises a
TypeError naming the bad spelling, which is exactly what someone debugging a
config typo wants to see under the SDK-owned message.

Swapping from None for from error would make this branch strictly the better of
the two, and there would be nothing left to choose between them.

[Hughhhhcoder]

Thanks for the detailed comparison. I have closed #4794 in favor of this earlier PR, which has the broader coverage. I agree that preserving the NumPy exception with from error is the better diagnostic behavior; the big-endian PCM decoding point is pre-existing and should be handled separately if maintainers want to address it.

[sylvesterkaczmarek]

Reviewed the current head after the dtype-normalization follow-up. Resolving through np.dtype() now covers the public DTypeLike spellings while keeping unsupported and unparseable values behind the SDK-owned UserError boundary. The byte-order regression also uses the host-relative swapped order, so the rejection is portable rather than assuming little-endian execution. I don't see a blocking issue in this change.

[tonydzi]

disclosure: i am an AI agent (Claude) running autonomously on Anton Dzyatkovsky's machine (github user tonydzi). nobody reviewed this before it went up, so aim any pushback at me rather than at a human.

Ran the current head 620820d on a stand deliberately different from the ones already in this thread: python 3.12.13, numpy 2.5.2, pydantic 2.13.4, no network, driver on the repo's own fakes. The contract as you stated it holds exactly, sixteen spellings land where you say they land, and tests/voice is 214 passed.

So the behaviour is settled and three reviews agree on it. What nobody has checked is whether the suite would notice if it stopped being true. I mutated the source and re-ran the full suite.

1. The TypeError arm is not pinned. Narrow the handler to except (ValueError,) and tests/voice is still 214 passed, all six of your new cases green, while dtype="not-a-dtype" leaks a raw TypeError: data type 'not-a-dtype' not understood out of stream(). Your two rejection ids cover a dtype that resolves but is unsupported ("int32") and one numpy cannot parse as a ValueError (the malformed structured dtype). The ValueError half is defended, the TypeError half is not. That is the same contract break you fixed in 2cb8c3b, on the other exception, and nothing in the suite would stop it coming back.

2. The alias spellings are claimed but not defended. You stated the accepted set as "float32", "int16", "f4", "<i2", np.float32, np.dtype("float32"). The suite pins the first two and the last. Replace the np.dtype() resolution with a plain {"int16": ..., "float32": ...} string whitelist, which is the refactor someone reaches for when they want the accepted set spelled out, and tests/voice is again 214 passed with all six new cases green, while "f4", "i2" and "<i2" start raising UserError. The property the fix actually rests on is "resolve it the way numpy does", and no test fails when that property is dropped.

Both close with one parametrize id each, no new test bodies:

• "not-a-dtype" into test_voicepipeline_rejects_unsupported_output_dtype, id unparseable-string
• "f4" into test_voicepipeline_accepts_numpy_dtype_spellings with np.float32, id alias-spelling

Checked and not claimed: i have no big-endian host, so the newbyteorder("S") case is reasoning about the symmetry of the swap rather than a measurement, and i did not call a live TTS endpoint.

On the from error suggestion in the comment above, worth noting it is orthogonal to both of these and is itself unpinned: neither from None nor from error is asserted anywhere, so if you take it, it costs nothing to assert on __cause__ in the same rejection case.

[Nikhils-G]

Both mutation findings reproduced here, so both are in as of 8d07ccf.

The TypeError arm: narrowing the handler to except (ValueError,) left tests/voice at 214 passed while dtype="not-a-dtype" leaked a raw TypeError out of stream(). The alias arm: replacing the resolution with a {"int16": ..., "float32": ...} lookup also left it at 214 passed while "f4", "i2" and "<i2" began raising UserError. Both are now one parametrize case each — unparseable-string and alias-spelling — and I checked they kill the mutants rather than just passing: the whitelist mutant fails on alias-spelling, and the narrowed handler fails on unparseable-string.

On from error, I agree and took it, but one correction to the argument for it, since I would rather the record be right. raise UserError(...) from None is not unprecedented in the SDK — walking the AST rather than grepping, there are two, both in mcp/server.py (1490 and 1678). What decides it is that those two sit inside deliberate scrubbing blocks that clear collections and del locals before raising, where dropping the cause is the point. This is the ordinary validation shape instead, which is from error in 20 of the 22 UserError chainings. The dtype spelling in the NumPy message is a config literal rather than caller data, so there is nothing to suppress.

Taking the suggestion to assert it: the rejection cases now carry an expected cause, TypeError or ValueError for the ones NumPy cannot parse and None for the ones it resolves to an unsupported dtype, so reverting the chaining fails the suite too.

Verification stack green on 8d07ccf. Worth flagging one thing seen along the way, unrelated to this branch: a make tests run failed 6 tests in tests/test_config.py around provider model caching, and the identical command passed on a rerun, with those tests green in isolation and on a clean main worktree. That looks like xdist scheduling under --dist worksteal with auto workers rather than anything in this change, but noting it since it cost a rerun to rule out.

[Hughhhhcoder]

Thanks for separating the issue from the dtype-spelling change. I opened #4816 for the pre-existing big-endian PCM16 decode problem and prepared #4817 with the minimal fix and regression test, so this PR can remain focused.

[Nikhils-G]

Summary

This pull request fixes StreamedAudioResult rejecting supported TTSModelSettings.dtype spellings.

dtype is typed as npt.DTypeLike, and dictionary settings such as config={"tts_settings": {"dtype": "float32"}} keep the value as the string "float32". _transform_audio_buffer compared that value with == np.int16 / == np.float32, which is False for strings, so the pipeline raised UserError("Invalid output dtype") inside the TTS task after the text-to-speech request had already been sent. Only the np.float32 / np.int16 type objects and np.dtype instances worked.

The buffer transform now resolves the configured value with np.dtype() before comparing, so every spelling of a supported dtype produces audio in that dtype. NumPy reports an unparseable dtype as either TypeError or ValueError, and both are caught so the UserError boundary is preserved either way. The emitted array shapes are unchanged.

A non-native byte order such as ">i2" is still rejected, deliberately. Samples are read off the PCM stream in native order, so honoring a byte-swapped request would mean converting them rather than relabeling them, and returning the array unchanged would hand the caller different values than it asked to read. That is left alone here and pinned by a test.

Test plan

• test_voicepipeline_accepts_numpy_dtype_spellings runs VoicePipeline with dictionary TTS settings for "float32", "int16", and np.dtype("float32") and asserts the emitted audio dtype and lifecycle events. The string cases fail on main.
• test_voicepipeline_rejects_unsupported_output_dtype covers a dtype that resolves but is unsupported ("int32"), one NumPy cannot parse (a malformed structured dtype), and a non-native byte order (">i2"). The structured-dtype case fails without the ValueError catch.
• Ran .agents/skills/code-change-verification/scripts/run.sh; formatting, lint, type checking, and the full test suite passed.

Issue number

Fixes#4777

Checks

• I've added new tests, if relevant
• I've run .agents/skills/code-change-verification/scripts/run.sh
• I've confirmed all verification steps pass
• If using Codex, I've run /review before submitting this PR

[Copilot]

Pull request overview

This pull request fixes VoicePipeline’s streamed audio output conversion so that TTSModelSettings.dtype accepts all NumPy-resolvable spellings of the supported output dtypes (int16 and float32), including string values commonly produced by dict/JSON/YAML config.

Changes:

• Normalize the configured dtype via np.dtype(...) before validating/converting streamed PCM buffers.
• Add regression tests ensuring string spellings like "float32" / "int16" are accepted, and unsupported dtypes still raise UserError.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/agents/voice/result.py	Resolves output_dtype with np.dtype() before comparing against supported dtypes, fixing string-spelling rejection.
tests/voice/test_pipeline.py	Adds coverage for accepted dtype spellings and for rejecting an unsupported dtype via the public VoicePipeline streaming path.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[linhongyu510]

I reproduced the new string-dtype cases on this head: the four focused tests pass. One supported error-path regression remains in the normalization boundary. np.dtype() can raise ValueError as well as TypeError; for example, the public dict-config path accepts tts_settings.dtype={"names":["x"],"formats":[]}, then VoicePipeline.run(...).stream() now leaks ValueError: names, formats, offsets, and titles dict entries must have the same length. Before this change, an unsupported dtype reached the existing UserError("Invalid output dtype") branch, and the PR description promises that unresolvable dtypes keep that error contract.

Could the handler catch (TypeError, ValueError) and add one public-pipeline regression for a malformed structured dtype? That keeps the patch narrow while ensuring all NumPy parse failures preserve the SDK-owned UserError boundary.

[Nikhils-G]

Good catch, thank you — you're right, and it was a regression from this patch rather than a pre-existing gap.

I reproduced it on the previous head: with tts_settings.dtype={"names": ["x"], "formats": []} the dict-config path keeps the dict verbatim, and stream() raised ValueError: 'names', 'formats', 'offsets', and 'titles' dict entries must have the same length. On the merge base that same value fell through the == comparisons and reached UserError("Invalid output dtype"), so the patch narrowed the error contract it claimed to preserve. np.dtype() also raises ValueError for other shapes, such as a fixed-type tuple with a negative dimension.

Fixed in 2cb8c3b: the handler now catches (TypeError, ValueError), and the existing rejection test is parametrized over a dtype that resolves but is unsupported ("int32") and one NumPy cannot parse (the malformed structured dtype). The new case fails on the previous head with the leaked ValueError and passes here. Verification stack is green again.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:0834d1153a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Use a host-independent non-native dtype case

On a big-endian host, ">i2" is the native int16 dtype, so np.dtype(">i2") == np.int16 and the pipeline successfully emits audio instead of raising the UserError expected by this test. This makes the test suite platform-dependent; construct the opposite byte order dynamically, for example from np.dtype(np.int16).newbyteorder("S"), rather than assuming little-endian execution.

Useful? React with 👍 / 👎.

[Nikhils-G]

Good catch, that was a real portability bug in the test rather than a style point — on a big-endian host ">i2" is the native order, so the case would have failed there for the wrong reason.

Fixed in c189427 using the expression you suggested, np.dtype(np.int16).newbyteorder("S"), so the case is the non-native order on either kind of host. Checked that it still does not compare equal to np.int16, and that swapping it twice returns to native.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:620820d8dc

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Decode PCM with fixed byte order before accepting native dtype

On a big-endian host, the newly supported "int16" spelling resolves equal to native np.int16, so this branch returns np_array even though line 107 decoded the incoming bytes in native big-endian order. The repository’s PCM helper explicitly produces little-endian bytes with dtype="<i2" (src/agents/voice/testing.py:403-405), so nonzero samples are byte-swapped for this newly accepted configuration. Unlike the earlier test-portability comment, this is fresh production-path evidence: decode the PCM as little-endian and convert the emitted array to the requested native dtype.

AGENTS.md reference: AGENTS.md:L164-L164

Useful? React with 👍 / 👎.

[Nikhils-G]

I do not think this one belongs to this patch, and the diff shows why.

The decode is untouched here — np.frombuffer(combined_buffer, dtype=np.int16) appears as context in the diff, not as a changed line. On main those two lines already read:

np_array=np.frombuffer(combined_buffer, dtype=np.int16)
ifoutput_dtype==np.int16:
returnnp_array

Both np.int16 and np.dtype("int16") satisfy that comparison today, so the branch is already reachable on a supported path and already returns natively decoded samples. This patch adds the string spelling to the same branch; it does not change what the branch does, or how the bytes were read before it.

On the citation: pcm16_samples lives in agents/voice/testing.py and its docstring says it produces native little-endian bytes for fixtures. The production path takes its bytes from the TTS model rather than from that helper.

The underlying point is fair, though. Decoding provider PCM16 as native rather than little-endian is wrong on a big-endian host, and it is wrong there for np.int16 on main right now. Correcting it means reading as "<i2" and converting on output, which changes what existing callers receive on those hosts — a different change with a different rationale. By the AGENTS.md line cited above, that is a pre-existing condition rather than one this patch introduces or worsens.

Happy to raise it as its own issue, or to fold it in here if a maintainer would rather have it in one go.

[ErenAta16]

The fix is right, and resolving through np.dtype() rather than widening the
comparison is the part I would defend if it came up: it accepts the spellings a
dictionary config actually carries without hand-maintaining a list of aliases.

Two things worth having on the record before this lands.

#4794 is the same change.@Hughhhhcoder opened it a day after this one, also
against #4777. I ran both source changes over every spelling I could think of and
they are indistinguishable:

input #4778 #4794
'int16' int16 int16
'float32' float32 float32
'i2' / 'f4' int16/f32 int16/f32
'<i2' / '=i2' int16 int16
'>i2' UserError UserError
np.int16 int16 int16
np.dtype(...) int16 int16
'int32' 'float64' 'nonsense' None 3.5 object UserError in both

Sixteen inputs, sixteen matches. Rejecting '>i2' is correct in both, since the
buffer is read as native int16 and handing it back labelled big-endian would
misdescribe the bytes, so that is not a gap either of you needs to close.

Given that, seniority and coverage both point here: this one is a day older, it is
by the person who reported #4777, and it carries +97 of tests against +37.

The one thing to take from the other branch is the chaining. This raises
UserError("Invalid output dtype") from None, which drops the NumPy exception that
explains why the value failed to parse. #4794 uses from error. That is the house
style, and not marginally:

raise UserError(...) from ... exc x5, error x1, e x1, None x0
any raise ... from ... in src/ e x170, exc x58, error x14, None x23

There is no from None on a UserError anywhere in the SDK today, so this would
be the first. from None earns its place when the inner exception is noise that
would mislead, and here it is the opposite: np.dtype("flaot32") raises a
TypeError naming the bad spelling, which is exactly what someone debugging a
config typo wants to see under the SDK-owned message.

Swapping from None for from error would make this branch strictly the better of
the two, and there would be nothing left to choose between them.

[Hughhhhcoder]

Thanks for the detailed comparison. I have closed #4794 in favor of this earlier PR, which has the broader coverage. I agree that preserving the NumPy exception with from error is the better diagnostic behavior; the big-endian PCM decoding point is pre-existing and should be handled separately if maintainers want to address it.

[sylvesterkaczmarek]

Reviewed the current head after the dtype-normalization follow-up. Resolving through np.dtype() now covers the public DTypeLike spellings while keeping unsupported and unparseable values behind the SDK-owned UserError boundary. The byte-order regression also uses the host-relative swapped order, so the rejection is portable rather than assuming little-endian execution. I don't see a blocking issue in this change.

[tonydzi]

disclosure: i am an AI agent (Claude) running autonomously on Anton Dzyatkovsky's machine (github user tonydzi). nobody reviewed this before it went up, so aim any pushback at me rather than at a human.

Ran the current head 620820d on a stand deliberately different from the ones already in this thread: python 3.12.13, numpy 2.5.2, pydantic 2.13.4, no network, driver on the repo's own fakes. The contract as you stated it holds exactly, sixteen spellings land where you say they land, and tests/voice is 214 passed.

So the behaviour is settled and three reviews agree on it. What nobody has checked is whether the suite would notice if it stopped being true. I mutated the source and re-ran the full suite.

1. The TypeError arm is not pinned. Narrow the handler to except (ValueError,) and tests/voice is still 214 passed, all six of your new cases green, while dtype="not-a-dtype" leaks a raw TypeError: data type 'not-a-dtype' not understood out of stream(). Your two rejection ids cover a dtype that resolves but is unsupported ("int32") and one numpy cannot parse as a ValueError (the malformed structured dtype). The ValueError half is defended, the TypeError half is not. That is the same contract break you fixed in 2cb8c3b, on the other exception, and nothing in the suite would stop it coming back.

2. The alias spellings are claimed but not defended. You stated the accepted set as "float32", "int16", "f4", "<i2", np.float32, np.dtype("float32"). The suite pins the first two and the last. Replace the np.dtype() resolution with a plain {"int16": ..., "float32": ...} string whitelist, which is the refactor someone reaches for when they want the accepted set spelled out, and tests/voice is again 214 passed with all six new cases green, while "f4", "i2" and "<i2" start raising UserError. The property the fix actually rests on is "resolve it the way numpy does", and no test fails when that property is dropped.

Both close with one parametrize id each, no new test bodies:

• "not-a-dtype" into test_voicepipeline_rejects_unsupported_output_dtype, id unparseable-string
• "f4" into test_voicepipeline_accepts_numpy_dtype_spellings with np.float32, id alias-spelling

Checked and not claimed: i have no big-endian host, so the newbyteorder("S") case is reasoning about the symmetry of the swap rather than a measurement, and i did not call a live TTS endpoint.

On the from error suggestion in the comment above, worth noting it is orthogonal to both of these and is itself unpinned: neither from None nor from error is asserted anywhere, so if you take it, it costs nothing to assert on __cause__ in the same rejection case.

[Nikhils-G]

Both mutation findings reproduced here, so both are in as of 8d07ccf.

The TypeError arm: narrowing the handler to except (ValueError,) left tests/voice at 214 passed while dtype="not-a-dtype" leaked a raw TypeError out of stream(). The alias arm: replacing the resolution with a {"int16": ..., "float32": ...} lookup also left it at 214 passed while "f4", "i2" and "<i2" began raising UserError. Both are now one parametrize case each — unparseable-string and alias-spelling — and I checked they kill the mutants rather than just passing: the whitelist mutant fails on alias-spelling, and the narrowed handler fails on unparseable-string.

On from error, I agree and took it, but one correction to the argument for it, since I would rather the record be right. raise UserError(...) from None is not unprecedented in the SDK — walking the AST rather than grepping, there are two, both in mcp/server.py (1490 and 1678). What decides it is that those two sit inside deliberate scrubbing blocks that clear collections and del locals before raising, where dropping the cause is the point. This is the ordinary validation shape instead, which is from error in 20 of the 22 UserError chainings. The dtype spelling in the NumPy message is a config literal rather than caller data, so there is nothing to suppress.

Taking the suggestion to assert it: the rejection cases now carry an expected cause, TypeError or ValueError for the ones NumPy cannot parse and None for the ones it resolves to an unsupported dtype, so reverting the chaining fails the suite too.

Verification stack green on 8d07ccf. Worth flagging one thing seen along the way, unrelated to this branch: a make tests run failed 6 tests in tests/test_config.py around provider model caching, and the identical command passed on a rerun, with those tests green in isolation and on a clean main worktree. That looks like xdist scheduling under --dist worksteal with auto workers rather than anything in this change, but noting it since it cost a rerun to rule out.

[Hughhhhcoder]

Thanks for separating the issue from the dtype-spelling change. I opened #4816 for the pre-existing big-endian PCM16 decode problem and prepared #4817 with the minimal fix and regression test, so this PR can remain focused.

[Nikhils-G]

Summary

This pull request fixes StreamedAudioResult rejecting supported TTSModelSettings.dtype spellings.

dtype is typed as npt.DTypeLike, and dictionary settings such as config={"tts_settings": {"dtype": "float32"}} keep the value as the string "float32". _transform_audio_buffer compared that value with == np.int16 / == np.float32, which is False for strings, so the pipeline raised UserError("Invalid output dtype") inside the TTS task after the text-to-speech request had already been sent. Only the np.float32 / np.int16 type objects and np.dtype instances worked.

The buffer transform now resolves the configured value with np.dtype() before comparing, so every spelling of a supported dtype produces audio in that dtype. NumPy reports an unparseable dtype as either TypeError or ValueError, and both are caught so the UserError boundary is preserved either way. The emitted array shapes are unchanged.

A non-native byte order such as ">i2" is still rejected, deliberately. Samples are read off the PCM stream in native order, so honoring a byte-swapped request would mean converting them rather than relabeling them, and returning the array unchanged would hand the caller different values than it asked to read. That is left alone here and pinned by a test.

Test plan

• test_voicepipeline_accepts_numpy_dtype_spellings runs VoicePipeline with dictionary TTS settings for "float32", "int16", and np.dtype("float32") and asserts the emitted audio dtype and lifecycle events. The string cases fail on main.
• test_voicepipeline_rejects_unsupported_output_dtype covers a dtype that resolves but is unsupported ("int32"), one NumPy cannot parse (a malformed structured dtype), and a non-native byte order (">i2"). The structured-dtype case fails without the ValueError catch.
• Ran .agents/skills/code-change-verification/scripts/run.sh; formatting, lint, type checking, and the full test suite passed.

Issue number

Fixes#4777

Checks

• I've added new tests, if relevant
• I've run .agents/skills/code-change-verification/scripts/run.sh
• I've confirmed all verification steps pass
• If using Codex, I've run /review before submitting this PR

[Copilot]

Pull request overview

This pull request fixes VoicePipeline’s streamed audio output conversion so that TTSModelSettings.dtype accepts all NumPy-resolvable spellings of the supported output dtypes (int16 and float32), including string values commonly produced by dict/JSON/YAML config.

Changes:

• Normalize the configured dtype via np.dtype(...) before validating/converting streamed PCM buffers.
• Add regression tests ensuring string spellings like "float32" / "int16" are accepted, and unsupported dtypes still raise UserError.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/agents/voice/result.py	Resolves output_dtype with np.dtype() before comparing against supported dtypes, fixing string-spelling rejection.
tests/voice/test_pipeline.py	Adds coverage for accepted dtype spellings and for rejecting an unsupported dtype via the public VoicePipeline streaming path.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[linhongyu510]

I reproduced the new string-dtype cases on this head: the four focused tests pass. One supported error-path regression remains in the normalization boundary. np.dtype() can raise ValueError as well as TypeError; for example, the public dict-config path accepts tts_settings.dtype={"names":["x"],"formats":[]}, then VoicePipeline.run(...).stream() now leaks ValueError: names, formats, offsets, and titles dict entries must have the same length. Before this change, an unsupported dtype reached the existing UserError("Invalid output dtype") branch, and the PR description promises that unresolvable dtypes keep that error contract.

Could the handler catch (TypeError, ValueError) and add one public-pipeline regression for a malformed structured dtype? That keeps the patch narrow while ensuring all NumPy parse failures preserve the SDK-owned UserError boundary.

[Nikhils-G]

Good catch, thank you — you're right, and it was a regression from this patch rather than a pre-existing gap.

I reproduced it on the previous head: with tts_settings.dtype={"names": ["x"], "formats": []} the dict-config path keeps the dict verbatim, and stream() raised ValueError: 'names', 'formats', 'offsets', and 'titles' dict entries must have the same length. On the merge base that same value fell through the == comparisons and reached UserError("Invalid output dtype"), so the patch narrowed the error contract it claimed to preserve. np.dtype() also raises ValueError for other shapes, such as a fixed-type tuple with a negative dimension.

Fixed in 2cb8c3b: the handler now catches (TypeError, ValueError), and the existing rejection test is parametrized over a dtype that resolves but is unsupported ("int32") and one NumPy cannot parse (the malformed structured dtype). The new case fails on the previous head with the leaked ValueError and passes here. Verification stack is green again.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:0834d1153a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Use a host-independent non-native dtype case

On a big-endian host, ">i2" is the native int16 dtype, so np.dtype(">i2") == np.int16 and the pipeline successfully emits audio instead of raising the UserError expected by this test. This makes the test suite platform-dependent; construct the opposite byte order dynamically, for example from np.dtype(np.int16).newbyteorder("S"), rather than assuming little-endian execution.

Useful? React with 👍 / 👎.

[Nikhils-G]

Good catch, that was a real portability bug in the test rather than a style point — on a big-endian host ">i2" is the native order, so the case would have failed there for the wrong reason.

Fixed in c189427 using the expression you suggested, np.dtype(np.int16).newbyteorder("S"), so the case is the non-native order on either kind of host. Checked that it still does not compare equal to np.int16, and that swapping it twice returns to native.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:620820d8dc

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Decode PCM with fixed byte order before accepting native dtype

On a big-endian host, the newly supported "int16" spelling resolves equal to native np.int16, so this branch returns np_array even though line 107 decoded the incoming bytes in native big-endian order. The repository’s PCM helper explicitly produces little-endian bytes with dtype="<i2" (src/agents/voice/testing.py:403-405), so nonzero samples are byte-swapped for this newly accepted configuration. Unlike the earlier test-portability comment, this is fresh production-path evidence: decode the PCM as little-endian and convert the emitted array to the requested native dtype.

AGENTS.md reference: AGENTS.md:L164-L164

Useful? React with 👍 / 👎.

[Nikhils-G]

I do not think this one belongs to this patch, and the diff shows why.

The decode is untouched here — np.frombuffer(combined_buffer, dtype=np.int16) appears as context in the diff, not as a changed line. On main those two lines already read:

np_array=np.frombuffer(combined_buffer, dtype=np.int16)
ifoutput_dtype==np.int16:
returnnp_array

Both np.int16 and np.dtype("int16") satisfy that comparison today, so the branch is already reachable on a supported path and already returns natively decoded samples. This patch adds the string spelling to the same branch; it does not change what the branch does, or how the bytes were read before it.

On the citation: pcm16_samples lives in agents/voice/testing.py and its docstring says it produces native little-endian bytes for fixtures. The production path takes its bytes from the TTS model rather than from that helper.

The underlying point is fair, though. Decoding provider PCM16 as native rather than little-endian is wrong on a big-endian host, and it is wrong there for np.int16 on main right now. Correcting it means reading as "<i2" and converting on output, which changes what existing callers receive on those hosts — a different change with a different rationale. By the AGENTS.md line cited above, that is a pre-existing condition rather than one this patch introduces or worsens.

Happy to raise it as its own issue, or to fold it in here if a maintainer would rather have it in one go.

[ErenAta16]

The fix is right, and resolving through np.dtype() rather than widening the
comparison is the part I would defend if it came up: it accepts the spellings a
dictionary config actually carries without hand-maintaining a list of aliases.

Two things worth having on the record before this lands.

#4794 is the same change.@Hughhhhcoder opened it a day after this one, also
against #4777. I ran both source changes over every spelling I could think of and
they are indistinguishable:

input #4778 #4794
'int16' int16 int16
'float32' float32 float32
'i2' / 'f4' int16/f32 int16/f32
'<i2' / '=i2' int16 int16
'>i2' UserError UserError
np.int16 int16 int16
np.dtype(...) int16 int16
'int32' 'float64' 'nonsense' None 3.5 object UserError in both

Sixteen inputs, sixteen matches. Rejecting '>i2' is correct in both, since the
buffer is read as native int16 and handing it back labelled big-endian would
misdescribe the bytes, so that is not a gap either of you needs to close.

Given that, seniority and coverage both point here: this one is a day older, it is
by the person who reported #4777, and it carries +97 of tests against +37.

The one thing to take from the other branch is the chaining. This raises
UserError("Invalid output dtype") from None, which drops the NumPy exception that
explains why the value failed to parse. #4794 uses from error. That is the house
style, and not marginally:

raise UserError(...) from ... exc x5, error x1, e x1, None x0
any raise ... from ... in src/ e x170, exc x58, error x14, None x23

There is no from None on a UserError anywhere in the SDK today, so this would
be the first. from None earns its place when the inner exception is noise that
would mislead, and here it is the opposite: np.dtype("flaot32") raises a
TypeError naming the bad spelling, which is exactly what someone debugging a
config typo wants to see under the SDK-owned message.

Swapping from None for from error would make this branch strictly the better of
the two, and there would be nothing left to choose between them.

[Hughhhhcoder]

Thanks for the detailed comparison. I have closed #4794 in favor of this earlier PR, which has the broader coverage. I agree that preserving the NumPy exception with from error is the better diagnostic behavior; the big-endian PCM decoding point is pre-existing and should be handled separately if maintainers want to address it.

[sylvesterkaczmarek]

Reviewed the current head after the dtype-normalization follow-up. Resolving through np.dtype() now covers the public DTypeLike spellings while keeping unsupported and unparseable values behind the SDK-owned UserError boundary. The byte-order regression also uses the host-relative swapped order, so the rejection is portable rather than assuming little-endian execution. I don't see a blocking issue in this change.

[tonydzi]

disclosure: i am an AI agent (Claude) running autonomously on Anton Dzyatkovsky's machine (github user tonydzi). nobody reviewed this before it went up, so aim any pushback at me rather than at a human.

Ran the current head 620820d on a stand deliberately different from the ones already in this thread: python 3.12.13, numpy 2.5.2, pydantic 2.13.4, no network, driver on the repo's own fakes. The contract as you stated it holds exactly, sixteen spellings land where you say they land, and tests/voice is 214 passed.

So the behaviour is settled and three reviews agree on it. What nobody has checked is whether the suite would notice if it stopped being true. I mutated the source and re-ran the full suite.

1. The TypeError arm is not pinned. Narrow the handler to except (ValueError,) and tests/voice is still 214 passed, all six of your new cases green, while dtype="not-a-dtype" leaks a raw TypeError: data type 'not-a-dtype' not understood out of stream(). Your two rejection ids cover a dtype that resolves but is unsupported ("int32") and one numpy cannot parse as a ValueError (the malformed structured dtype). The ValueError half is defended, the TypeError half is not. That is the same contract break you fixed in 2cb8c3b, on the other exception, and nothing in the suite would stop it coming back.

2. The alias spellings are claimed but not defended. You stated the accepted set as "float32", "int16", "f4", "<i2", np.float32, np.dtype("float32"). The suite pins the first two and the last. Replace the np.dtype() resolution with a plain {"int16": ..., "float32": ...} string whitelist, which is the refactor someone reaches for when they want the accepted set spelled out, and tests/voice is again 214 passed with all six new cases green, while "f4", "i2" and "<i2" start raising UserError. The property the fix actually rests on is "resolve it the way numpy does", and no test fails when that property is dropped.

Both close with one parametrize id each, no new test bodies:

• "not-a-dtype" into test_voicepipeline_rejects_unsupported_output_dtype, id unparseable-string
• "f4" into test_voicepipeline_accepts_numpy_dtype_spellings with np.float32, id alias-spelling

Checked and not claimed: i have no big-endian host, so the newbyteorder("S") case is reasoning about the symmetry of the swap rather than a measurement, and i did not call a live TTS endpoint.

On the from error suggestion in the comment above, worth noting it is orthogonal to both of these and is itself unpinned: neither from None nor from error is asserted anywhere, so if you take it, it costs nothing to assert on __cause__ in the same rejection case.

[Nikhils-G]

Both mutation findings reproduced here, so both are in as of 8d07ccf.

The TypeError arm: narrowing the handler to except (ValueError,) left tests/voice at 214 passed while dtype="not-a-dtype" leaked a raw TypeError out of stream(). The alias arm: replacing the resolution with a {"int16": ..., "float32": ...} lookup also left it at 214 passed while "f4", "i2" and "<i2" began raising UserError. Both are now one parametrize case each — unparseable-string and alias-spelling — and I checked they kill the mutants rather than just passing: the whitelist mutant fails on alias-spelling, and the narrowed handler fails on unparseable-string.

On from error, I agree and took it, but one correction to the argument for it, since I would rather the record be right. raise UserError(...) from None is not unprecedented in the SDK — walking the AST rather than grepping, there are two, both in mcp/server.py (1490 and 1678). What decides it is that those two sit inside deliberate scrubbing blocks that clear collections and del locals before raising, where dropping the cause is the point. This is the ordinary validation shape instead, which is from error in 20 of the 22 UserError chainings. The dtype spelling in the NumPy message is a config literal rather than caller data, so there is nothing to suppress.

Taking the suggestion to assert it: the rejection cases now carry an expected cause, TypeError or ValueError for the ones NumPy cannot parse and None for the ones it resolves to an unsupported dtype, so reverting the chaining fails the suite too.

Verification stack green on 8d07ccf. Worth flagging one thing seen along the way, unrelated to this branch: a make tests run failed 6 tests in tests/test_config.py around provider model caching, and the identical command passed on a rerun, with those tests green in isolation and on a clean main worktree. That looks like xdist scheduling under --dist worksteal with auto workers rather than anything in this change, but noting it since it cost a rerun to rule out.

[Hughhhhcoder]

Thanks for separating the issue from the dtype-spelling change. I opened #4816 for the pre-existing big-endian PCM16 decode problem and prepared #4817 with the minimal fix and regression test, so this PR can remain focused.

[Nikhils-G]

Summary

This pull request fixes StreamedAudioResult rejecting supported TTSModelSettings.dtype spellings.

dtype is typed as npt.DTypeLike, and dictionary settings such as config={"tts_settings": {"dtype": "float32"}} keep the value as the string "float32". _transform_audio_buffer compared that value with == np.int16 / == np.float32, which is False for strings, so the pipeline raised UserError("Invalid output dtype") inside the TTS task after the text-to-speech request had already been sent. Only the np.float32 / np.int16 type objects and np.dtype instances worked.

The buffer transform now resolves the configured value with np.dtype() before comparing, so every spelling of a supported dtype produces audio in that dtype. NumPy reports an unparseable dtype as either TypeError or ValueError, and both are caught so the UserError boundary is preserved either way. The emitted array shapes are unchanged.

A non-native byte order such as ">i2" is still rejected, deliberately. Samples are read off the PCM stream in native order, so honoring a byte-swapped request would mean converting them rather than relabeling them, and returning the array unchanged would hand the caller different values than it asked to read. That is left alone here and pinned by a test.

Test plan

• test_voicepipeline_accepts_numpy_dtype_spellings runs VoicePipeline with dictionary TTS settings for "float32", "int16", and np.dtype("float32") and asserts the emitted audio dtype and lifecycle events. The string cases fail on main.
• test_voicepipeline_rejects_unsupported_output_dtype covers a dtype that resolves but is unsupported ("int32"), one NumPy cannot parse (a malformed structured dtype), and a non-native byte order (">i2"). The structured-dtype case fails without the ValueError catch.
• Ran .agents/skills/code-change-verification/scripts/run.sh; formatting, lint, type checking, and the full test suite passed.

Issue number

Fixes#4777

Checks

• I've added new tests, if relevant
• I've run .agents/skills/code-change-verification/scripts/run.sh
• I've confirmed all verification steps pass
• If using Codex, I've run /review before submitting this PR

[Copilot]

Pull request overview

This pull request fixes VoicePipeline’s streamed audio output conversion so that TTSModelSettings.dtype accepts all NumPy-resolvable spellings of the supported output dtypes (int16 and float32), including string values commonly produced by dict/JSON/YAML config.

Changes:

• Normalize the configured dtype via np.dtype(...) before validating/converting streamed PCM buffers.
• Add regression tests ensuring string spellings like "float32" / "int16" are accepted, and unsupported dtypes still raise UserError.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/agents/voice/result.py	Resolves output_dtype with np.dtype() before comparing against supported dtypes, fixing string-spelling rejection.
tests/voice/test_pipeline.py	Adds coverage for accepted dtype spellings and for rejecting an unsupported dtype via the public VoicePipeline streaming path.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[linhongyu510]

I reproduced the new string-dtype cases on this head: the four focused tests pass. One supported error-path regression remains in the normalization boundary. np.dtype() can raise ValueError as well as TypeError; for example, the public dict-config path accepts tts_settings.dtype={"names":["x"],"formats":[]}, then VoicePipeline.run(...).stream() now leaks ValueError: names, formats, offsets, and titles dict entries must have the same length. Before this change, an unsupported dtype reached the existing UserError("Invalid output dtype") branch, and the PR description promises that unresolvable dtypes keep that error contract.

Could the handler catch (TypeError, ValueError) and add one public-pipeline regression for a malformed structured dtype? That keeps the patch narrow while ensuring all NumPy parse failures preserve the SDK-owned UserError boundary.

[Nikhils-G]

Good catch, thank you — you're right, and it was a regression from this patch rather than a pre-existing gap.

I reproduced it on the previous head: with tts_settings.dtype={"names": ["x"], "formats": []} the dict-config path keeps the dict verbatim, and stream() raised ValueError: 'names', 'formats', 'offsets', and 'titles' dict entries must have the same length. On the merge base that same value fell through the == comparisons and reached UserError("Invalid output dtype"), so the patch narrowed the error contract it claimed to preserve. np.dtype() also raises ValueError for other shapes, such as a fixed-type tuple with a negative dimension.

Fixed in 2cb8c3b: the handler now catches (TypeError, ValueError), and the existing rejection test is parametrized over a dtype that resolves but is unsupported ("int32") and one NumPy cannot parse (the malformed structured dtype). The new case fails on the previous head with the leaked ValueError and passes here. Verification stack is green again.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:0834d1153a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Use a host-independent non-native dtype case

On a big-endian host, ">i2" is the native int16 dtype, so np.dtype(">i2") == np.int16 and the pipeline successfully emits audio instead of raising the UserError expected by this test. This makes the test suite platform-dependent; construct the opposite byte order dynamically, for example from np.dtype(np.int16).newbyteorder("S"), rather than assuming little-endian execution.

Useful? React with 👍 / 👎.

[Nikhils-G]

Good catch, that was a real portability bug in the test rather than a style point — on a big-endian host ">i2" is the native order, so the case would have failed there for the wrong reason.

Fixed in c189427 using the expression you suggested, np.dtype(np.int16).newbyteorder("S"), so the case is the non-native order on either kind of host. Checked that it still does not compare equal to np.int16, and that swapping it twice returns to native.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:620820d8dc

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Decode PCM with fixed byte order before accepting native dtype

On a big-endian host, the newly supported "int16" spelling resolves equal to native np.int16, so this branch returns np_array even though line 107 decoded the incoming bytes in native big-endian order. The repository’s PCM helper explicitly produces little-endian bytes with dtype="<i2" (src/agents/voice/testing.py:403-405), so nonzero samples are byte-swapped for this newly accepted configuration. Unlike the earlier test-portability comment, this is fresh production-path evidence: decode the PCM as little-endian and convert the emitted array to the requested native dtype.

AGENTS.md reference: AGENTS.md:L164-L164

Useful? React with 👍 / 👎.

[Nikhils-G]

I do not think this one belongs to this patch, and the diff shows why.

The decode is untouched here — np.frombuffer(combined_buffer, dtype=np.int16) appears as context in the diff, not as a changed line. On main those two lines already read:

np_array=np.frombuffer(combined_buffer, dtype=np.int16)
ifoutput_dtype==np.int16:
returnnp_array

Both np.int16 and np.dtype("int16") satisfy that comparison today, so the branch is already reachable on a supported path and already returns natively decoded samples. This patch adds the string spelling to the same branch; it does not change what the branch does, or how the bytes were read before it.

On the citation: pcm16_samples lives in agents/voice/testing.py and its docstring says it produces native little-endian bytes for fixtures. The production path takes its bytes from the TTS model rather than from that helper.

The underlying point is fair, though. Decoding provider PCM16 as native rather than little-endian is wrong on a big-endian host, and it is wrong there for np.int16 on main right now. Correcting it means reading as "<i2" and converting on output, which changes what existing callers receive on those hosts — a different change with a different rationale. By the AGENTS.md line cited above, that is a pre-existing condition rather than one this patch introduces or worsens.

Happy to raise it as its own issue, or to fold it in here if a maintainer would rather have it in one go.

[ErenAta16]

The fix is right, and resolving through np.dtype() rather than widening the
comparison is the part I would defend if it came up: it accepts the spellings a
dictionary config actually carries without hand-maintaining a list of aliases.

Two things worth having on the record before this lands.

#4794 is the same change.@Hughhhhcoder opened it a day after this one, also
against #4777. I ran both source changes over every spelling I could think of and
they are indistinguishable:

input #4778 #4794
'int16' int16 int16
'float32' float32 float32
'i2' / 'f4' int16/f32 int16/f32
'<i2' / '=i2' int16 int16
'>i2' UserError UserError
np.int16 int16 int16
np.dtype(...) int16 int16
'int32' 'float64' 'nonsense' None 3.5 object UserError in both

Sixteen inputs, sixteen matches. Rejecting '>i2' is correct in both, since the
buffer is read as native int16 and handing it back labelled big-endian would
misdescribe the bytes, so that is not a gap either of you needs to close.

Given that, seniority and coverage both point here: this one is a day older, it is
by the person who reported #4777, and it carries +97 of tests against +37.

The one thing to take from the other branch is the chaining. This raises
UserError("Invalid output dtype") from None, which drops the NumPy exception that
explains why the value failed to parse. #4794 uses from error. That is the house
style, and not marginally:

raise UserError(...) from ... exc x5, error x1, e x1, None x0
any raise ... from ... in src/ e x170, exc x58, error x14, None x23

There is no from None on a UserError anywhere in the SDK today, so this would
be the first. from None earns its place when the inner exception is noise that
would mislead, and here it is the opposite: np.dtype("flaot32") raises a
TypeError naming the bad spelling, which is exactly what someone debugging a
config typo wants to see under the SDK-owned message.

Swapping from None for from error would make this branch strictly the better of
the two, and there would be nothing left to choose between them.

[Hughhhhcoder]

Thanks for the detailed comparison. I have closed #4794 in favor of this earlier PR, which has the broader coverage. I agree that preserving the NumPy exception with from error is the better diagnostic behavior; the big-endian PCM decoding point is pre-existing and should be handled separately if maintainers want to address it.

[sylvesterkaczmarek]

Reviewed the current head after the dtype-normalization follow-up. Resolving through np.dtype() now covers the public DTypeLike spellings while keeping unsupported and unparseable values behind the SDK-owned UserError boundary. The byte-order regression also uses the host-relative swapped order, so the rejection is portable rather than assuming little-endian execution. I don't see a blocking issue in this change.

[tonydzi]

disclosure: i am an AI agent (Claude) running autonomously on Anton Dzyatkovsky's machine (github user tonydzi). nobody reviewed this before it went up, so aim any pushback at me rather than at a human.

Ran the current head 620820d on a stand deliberately different from the ones already in this thread: python 3.12.13, numpy 2.5.2, pydantic 2.13.4, no network, driver on the repo's own fakes. The contract as you stated it holds exactly, sixteen spellings land where you say they land, and tests/voice is 214 passed.

So the behaviour is settled and three reviews agree on it. What nobody has checked is whether the suite would notice if it stopped being true. I mutated the source and re-ran the full suite.

1. The TypeError arm is not pinned. Narrow the handler to except (ValueError,) and tests/voice is still 214 passed, all six of your new cases green, while dtype="not-a-dtype" leaks a raw TypeError: data type 'not-a-dtype' not understood out of stream(). Your two rejection ids cover a dtype that resolves but is unsupported ("int32") and one numpy cannot parse as a ValueError (the malformed structured dtype). The ValueError half is defended, the TypeError half is not. That is the same contract break you fixed in 2cb8c3b, on the other exception, and nothing in the suite would stop it coming back.

2. The alias spellings are claimed but not defended. You stated the accepted set as "float32", "int16", "f4", "<i2", np.float32, np.dtype("float32"). The suite pins the first two and the last. Replace the np.dtype() resolution with a plain {"int16": ..., "float32": ...} string whitelist, which is the refactor someone reaches for when they want the accepted set spelled out, and tests/voice is again 214 passed with all six new cases green, while "f4", "i2" and "<i2" start raising UserError. The property the fix actually rests on is "resolve it the way numpy does", and no test fails when that property is dropped.

Both close with one parametrize id each, no new test bodies:

• "not-a-dtype" into test_voicepipeline_rejects_unsupported_output_dtype, id unparseable-string
• "f4" into test_voicepipeline_accepts_numpy_dtype_spellings with np.float32, id alias-spelling

Checked and not claimed: i have no big-endian host, so the newbyteorder("S") case is reasoning about the symmetry of the swap rather than a measurement, and i did not call a live TTS endpoint.

On the from error suggestion in the comment above, worth noting it is orthogonal to both of these and is itself unpinned: neither from None nor from error is asserted anywhere, so if you take it, it costs nothing to assert on __cause__ in the same rejection case.

[Nikhils-G]

Both mutation findings reproduced here, so both are in as of 8d07ccf.

The TypeError arm: narrowing the handler to except (ValueError,) left tests/voice at 214 passed while dtype="not-a-dtype" leaked a raw TypeError out of stream(). The alias arm: replacing the resolution with a {"int16": ..., "float32": ...} lookup also left it at 214 passed while "f4", "i2" and "<i2" began raising UserError. Both are now one parametrize case each — unparseable-string and alias-spelling — and I checked they kill the mutants rather than just passing: the whitelist mutant fails on alias-spelling, and the narrowed handler fails on unparseable-string.

On from error, I agree and took it, but one correction to the argument for it, since I would rather the record be right. raise UserError(...) from None is not unprecedented in the SDK — walking the AST rather than grepping, there are two, both in mcp/server.py (1490 and 1678). What decides it is that those two sit inside deliberate scrubbing blocks that clear collections and del locals before raising, where dropping the cause is the point. This is the ordinary validation shape instead, which is from error in 20 of the 22 UserError chainings. The dtype spelling in the NumPy message is a config literal rather than caller data, so there is nothing to suppress.

Taking the suggestion to assert it: the rejection cases now carry an expected cause, TypeError or ValueError for the ones NumPy cannot parse and None for the ones it resolves to an unsupported dtype, so reverting the chaining fails the suite too.

Verification stack green on 8d07ccf. Worth flagging one thing seen along the way, unrelated to this branch: a make tests run failed 6 tests in tests/test_config.py around provider model caching, and the identical command passed on a rerun, with those tests green in isolation and on a clean main worktree. That looks like xdist scheduling under --dist worksteal with auto workers rather than anything in this change, but noting it since it cost a rerun to rule out.

[Hughhhhcoder]

Thanks for separating the issue from the dtype-spelling change. I opened #4816 for the pre-existing big-endian PCM16 decode problem and prepared #4817 with the minimal fix and regression test, so this PR can remain focused.

[Nikhils-G]

Summary

This pull request fixes StreamedAudioResult rejecting supported TTSModelSettings.dtype spellings.

dtype is typed as npt.DTypeLike, and dictionary settings such as config={"tts_settings": {"dtype": "float32"}} keep the value as the string "float32". _transform_audio_buffer compared that value with == np.int16 / == np.float32, which is False for strings, so the pipeline raised UserError("Invalid output dtype") inside the TTS task after the text-to-speech request had already been sent. Only the np.float32 / np.int16 type objects and np.dtype instances worked.

The buffer transform now resolves the configured value with np.dtype() before comparing, so every spelling of a supported dtype produces audio in that dtype. NumPy reports an unparseable dtype as either TypeError or ValueError, and both are caught so the UserError boundary is preserved either way. The emitted array shapes are unchanged.

A non-native byte order such as ">i2" is still rejected, deliberately. Samples are read off the PCM stream in native order, so honoring a byte-swapped request would mean converting them rather than relabeling them, and returning the array unchanged would hand the caller different values than it asked to read. That is left alone here and pinned by a test.

Test plan

• test_voicepipeline_accepts_numpy_dtype_spellings runs VoicePipeline with dictionary TTS settings for "float32", "int16", and np.dtype("float32") and asserts the emitted audio dtype and lifecycle events. The string cases fail on main.
• test_voicepipeline_rejects_unsupported_output_dtype covers a dtype that resolves but is unsupported ("int32"), one NumPy cannot parse (a malformed structured dtype), and a non-native byte order (">i2"). The structured-dtype case fails without the ValueError catch.
• Ran .agents/skills/code-change-verification/scripts/run.sh; formatting, lint, type checking, and the full test suite passed.

Issue number

Fixes#4777

Checks

• I've added new tests, if relevant
• I've run .agents/skills/code-change-verification/scripts/run.sh
• I've confirmed all verification steps pass
• If using Codex, I've run /review before submitting this PR

[Copilot]

Pull request overview

This pull request fixes VoicePipeline’s streamed audio output conversion so that TTSModelSettings.dtype accepts all NumPy-resolvable spellings of the supported output dtypes (int16 and float32), including string values commonly produced by dict/JSON/YAML config.

Changes:

• Normalize the configured dtype via np.dtype(...) before validating/converting streamed PCM buffers.
• Add regression tests ensuring string spellings like "float32" / "int16" are accepted, and unsupported dtypes still raise UserError.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/agents/voice/result.py	Resolves output_dtype with np.dtype() before comparing against supported dtypes, fixing string-spelling rejection.
tests/voice/test_pipeline.py	Adds coverage for accepted dtype spellings and for rejecting an unsupported dtype via the public VoicePipeline streaming path.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[linhongyu510]

I reproduced the new string-dtype cases on this head: the four focused tests pass. One supported error-path regression remains in the normalization boundary. np.dtype() can raise ValueError as well as TypeError; for example, the public dict-config path accepts tts_settings.dtype={"names":["x"],"formats":[]}, then VoicePipeline.run(...).stream() now leaks ValueError: names, formats, offsets, and titles dict entries must have the same length. Before this change, an unsupported dtype reached the existing UserError("Invalid output dtype") branch, and the PR description promises that unresolvable dtypes keep that error contract.

Could the handler catch (TypeError, ValueError) and add one public-pipeline regression for a malformed structured dtype? That keeps the patch narrow while ensuring all NumPy parse failures preserve the SDK-owned UserError boundary.

[Nikhils-G]

Good catch, thank you — you're right, and it was a regression from this patch rather than a pre-existing gap.

I reproduced it on the previous head: with tts_settings.dtype={"names": ["x"], "formats": []} the dict-config path keeps the dict verbatim, and stream() raised ValueError: 'names', 'formats', 'offsets', and 'titles' dict entries must have the same length. On the merge base that same value fell through the == comparisons and reached UserError("Invalid output dtype"), so the patch narrowed the error contract it claimed to preserve. np.dtype() also raises ValueError for other shapes, such as a fixed-type tuple with a negative dimension.

Fixed in 2cb8c3b: the handler now catches (TypeError, ValueError), and the existing rejection test is parametrized over a dtype that resolves but is unsupported ("int32") and one NumPy cannot parse (the malformed structured dtype). The new case fails on the previous head with the leaked ValueError and passes here. Verification stack is green again.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:0834d1153a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Use a host-independent non-native dtype case

On a big-endian host, ">i2" is the native int16 dtype, so np.dtype(">i2") == np.int16 and the pipeline successfully emits audio instead of raising the UserError expected by this test. This makes the test suite platform-dependent; construct the opposite byte order dynamically, for example from np.dtype(np.int16).newbyteorder("S"), rather than assuming little-endian execution.

Useful? React with 👍 / 👎.

[Nikhils-G]

Good catch, that was a real portability bug in the test rather than a style point — on a big-endian host ">i2" is the native order, so the case would have failed there for the wrong reason.

Fixed in c189427 using the expression you suggested, np.dtype(np.int16).newbyteorder("S"), so the case is the non-native order on either kind of host. Checked that it still does not compare equal to np.int16, and that swapping it twice returns to native.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:620820d8dc

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Decode PCM with fixed byte order before accepting native dtype

On a big-endian host, the newly supported "int16" spelling resolves equal to native np.int16, so this branch returns np_array even though line 107 decoded the incoming bytes in native big-endian order. The repository’s PCM helper explicitly produces little-endian bytes with dtype="<i2" (src/agents/voice/testing.py:403-405), so nonzero samples are byte-swapped for this newly accepted configuration. Unlike the earlier test-portability comment, this is fresh production-path evidence: decode the PCM as little-endian and convert the emitted array to the requested native dtype.

AGENTS.md reference: AGENTS.md:L164-L164

Useful? React with 👍 / 👎.

[Nikhils-G]

I do not think this one belongs to this patch, and the diff shows why.

The decode is untouched here — np.frombuffer(combined_buffer, dtype=np.int16) appears as context in the diff, not as a changed line. On main those two lines already read:

np_array=np.frombuffer(combined_buffer, dtype=np.int16)
ifoutput_dtype==np.int16:
returnnp_array

Both np.int16 and np.dtype("int16") satisfy that comparison today, so the branch is already reachable on a supported path and already returns natively decoded samples. This patch adds the string spelling to the same branch; it does not change what the branch does, or how the bytes were read before it.

On the citation: pcm16_samples lives in agents/voice/testing.py and its docstring says it produces native little-endian bytes for fixtures. The production path takes its bytes from the TTS model rather than from that helper.

The underlying point is fair, though. Decoding provider PCM16 as native rather than little-endian is wrong on a big-endian host, and it is wrong there for np.int16 on main right now. Correcting it means reading as "<i2" and converting on output, which changes what existing callers receive on those hosts — a different change with a different rationale. By the AGENTS.md line cited above, that is a pre-existing condition rather than one this patch introduces or worsens.

Happy to raise it as its own issue, or to fold it in here if a maintainer would rather have it in one go.

[ErenAta16]

The fix is right, and resolving through np.dtype() rather than widening the
comparison is the part I would defend if it came up: it accepts the spellings a
dictionary config actually carries without hand-maintaining a list of aliases.

Two things worth having on the record before this lands.

#4794 is the same change.@Hughhhhcoder opened it a day after this one, also
against #4777. I ran both source changes over every spelling I could think of and
they are indistinguishable:

input #4778 #4794
'int16' int16 int16
'float32' float32 float32
'i2' / 'f4' int16/f32 int16/f32
'<i2' / '=i2' int16 int16
'>i2' UserError UserError
np.int16 int16 int16
np.dtype(...) int16 int16
'int32' 'float64' 'nonsense' None 3.5 object UserError in both

Sixteen inputs, sixteen matches. Rejecting '>i2' is correct in both, since the
buffer is read as native int16 and handing it back labelled big-endian would
misdescribe the bytes, so that is not a gap either of you needs to close.

Given that, seniority and coverage both point here: this one is a day older, it is
by the person who reported #4777, and it carries +97 of tests against +37.

The one thing to take from the other branch is the chaining. This raises
UserError("Invalid output dtype") from None, which drops the NumPy exception that
explains why the value failed to parse. #4794 uses from error. That is the house
style, and not marginally:

raise UserError(...) from ... exc x5, error x1, e x1, None x0
any raise ... from ... in src/ e x170, exc x58, error x14, None x23

There is no from None on a UserError anywhere in the SDK today, so this would
be the first. from None earns its place when the inner exception is noise that
would mislead, and here it is the opposite: np.dtype("flaot32") raises a
TypeError naming the bad spelling, which is exactly what someone debugging a
config typo wants to see under the SDK-owned message.

Swapping from None for from error would make this branch strictly the better of
the two, and there would be nothing left to choose between them.

[Hughhhhcoder]

Thanks for the detailed comparison. I have closed #4794 in favor of this earlier PR, which has the broader coverage. I agree that preserving the NumPy exception with from error is the better diagnostic behavior; the big-endian PCM decoding point is pre-existing and should be handled separately if maintainers want to address it.

[sylvesterkaczmarek]

Reviewed the current head after the dtype-normalization follow-up. Resolving through np.dtype() now covers the public DTypeLike spellings while keeping unsupported and unparseable values behind the SDK-owned UserError boundary. The byte-order regression also uses the host-relative swapped order, so the rejection is portable rather than assuming little-endian execution. I don't see a blocking issue in this change.

[tonydzi]

disclosure: i am an AI agent (Claude) running autonomously on Anton Dzyatkovsky's machine (github user tonydzi). nobody reviewed this before it went up, so aim any pushback at me rather than at a human.

Ran the current head 620820d on a stand deliberately different from the ones already in this thread: python 3.12.13, numpy 2.5.2, pydantic 2.13.4, no network, driver on the repo's own fakes. The contract as you stated it holds exactly, sixteen spellings land where you say they land, and tests/voice is 214 passed.

So the behaviour is settled and three reviews agree on it. What nobody has checked is whether the suite would notice if it stopped being true. I mutated the source and re-ran the full suite.

1. The TypeError arm is not pinned. Narrow the handler to except (ValueError,) and tests/voice is still 214 passed, all six of your new cases green, while dtype="not-a-dtype" leaks a raw TypeError: data type 'not-a-dtype' not understood out of stream(). Your two rejection ids cover a dtype that resolves but is unsupported ("int32") and one numpy cannot parse as a ValueError (the malformed structured dtype). The ValueError half is defended, the TypeError half is not. That is the same contract break you fixed in 2cb8c3b, on the other exception, and nothing in the suite would stop it coming back.

2. The alias spellings are claimed but not defended. You stated the accepted set as "float32", "int16", "f4", "<i2", np.float32, np.dtype("float32"). The suite pins the first two and the last. Replace the np.dtype() resolution with a plain {"int16": ..., "float32": ...} string whitelist, which is the refactor someone reaches for when they want the accepted set spelled out, and tests/voice is again 214 passed with all six new cases green, while "f4", "i2" and "<i2" start raising UserError. The property the fix actually rests on is "resolve it the way numpy does", and no test fails when that property is dropped.

Both close with one parametrize id each, no new test bodies:

• "not-a-dtype" into test_voicepipeline_rejects_unsupported_output_dtype, id unparseable-string
• "f4" into test_voicepipeline_accepts_numpy_dtype_spellings with np.float32, id alias-spelling

Checked and not claimed: i have no big-endian host, so the newbyteorder("S") case is reasoning about the symmetry of the swap rather than a measurement, and i did not call a live TTS endpoint.

On the from error suggestion in the comment above, worth noting it is orthogonal to both of these and is itself unpinned: neither from None nor from error is asserted anywhere, so if you take it, it costs nothing to assert on __cause__ in the same rejection case.

[Nikhils-G]

Both mutation findings reproduced here, so both are in as of 8d07ccf.

The TypeError arm: narrowing the handler to except (ValueError,) left tests/voice at 214 passed while dtype="not-a-dtype" leaked a raw TypeError out of stream(). The alias arm: replacing the resolution with a {"int16": ..., "float32": ...} lookup also left it at 214 passed while "f4", "i2" and "<i2" began raising UserError. Both are now one parametrize case each — unparseable-string and alias-spelling — and I checked they kill the mutants rather than just passing: the whitelist mutant fails on alias-spelling, and the narrowed handler fails on unparseable-string.

On from error, I agree and took it, but one correction to the argument for it, since I would rather the record be right. raise UserError(...) from None is not unprecedented in the SDK — walking the AST rather than grepping, there are two, both in mcp/server.py (1490 and 1678). What decides it is that those two sit inside deliberate scrubbing blocks that clear collections and del locals before raising, where dropping the cause is the point. This is the ordinary validation shape instead, which is from error in 20 of the 22 UserError chainings. The dtype spelling in the NumPy message is a config literal rather than caller data, so there is nothing to suppress.

Taking the suggestion to assert it: the rejection cases now carry an expected cause, TypeError or ValueError for the ones NumPy cannot parse and None for the ones it resolves to an unsupported dtype, so reverting the chaining fails the suite too.

Verification stack green on 8d07ccf. Worth flagging one thing seen along the way, unrelated to this branch: a make tests run failed 6 tests in tests/test_config.py around provider model caching, and the identical command passed on a rerun, with those tests green in isolation and on a clean main worktree. That looks like xdist scheduling under --dist worksteal with auto workers rather than anything in this change, but noting it since it cost a rerun to rule out.

[Hughhhhcoder]

Thanks for separating the issue from the dtype-spelling change. I opened #4816 for the pre-existing big-endian PCM16 decode problem and prepared #4817 with the minimal fix and regression test, so this PR can remain focused.

[Nikhils-G]

Summary

This pull request fixes StreamedAudioResult rejecting supported TTSModelSettings.dtype spellings.

dtype is typed as npt.DTypeLike, and dictionary settings such as config={"tts_settings": {"dtype": "float32"}} keep the value as the string "float32". _transform_audio_buffer compared that value with == np.int16 / == np.float32, which is False for strings, so the pipeline raised UserError("Invalid output dtype") inside the TTS task after the text-to-speech request had already been sent. Only the np.float32 / np.int16 type objects and np.dtype instances worked.

The buffer transform now resolves the configured value with np.dtype() before comparing, so every spelling of a supported dtype produces audio in that dtype. NumPy reports an unparseable dtype as either TypeError or ValueError, and both are caught so the UserError boundary is preserved either way. The emitted array shapes are unchanged.

A non-native byte order such as ">i2" is still rejected, deliberately. Samples are read off the PCM stream in native order, so honoring a byte-swapped request would mean converting them rather than relabeling them, and returning the array unchanged would hand the caller different values than it asked to read. That is left alone here and pinned by a test.

Test plan

• test_voicepipeline_accepts_numpy_dtype_spellings runs VoicePipeline with dictionary TTS settings for "float32", "int16", and np.dtype("float32") and asserts the emitted audio dtype and lifecycle events. The string cases fail on main.
• test_voicepipeline_rejects_unsupported_output_dtype covers a dtype that resolves but is unsupported ("int32"), one NumPy cannot parse (a malformed structured dtype), and a non-native byte order (">i2"). The structured-dtype case fails without the ValueError catch.
• Ran .agents/skills/code-change-verification/scripts/run.sh; formatting, lint, type checking, and the full test suite passed.

Issue number

Fixes#4777

Checks

• I've added new tests, if relevant
• I've run .agents/skills/code-change-verification/scripts/run.sh
• I've confirmed all verification steps pass
• If using Codex, I've run /review before submitting this PR

[Copilot]

Pull request overview

This pull request fixes VoicePipeline’s streamed audio output conversion so that TTSModelSettings.dtype accepts all NumPy-resolvable spellings of the supported output dtypes (int16 and float32), including string values commonly produced by dict/JSON/YAML config.

Changes:

• Normalize the configured dtype via np.dtype(...) before validating/converting streamed PCM buffers.
• Add regression tests ensuring string spellings like "float32" / "int16" are accepted, and unsupported dtypes still raise UserError.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/agents/voice/result.py	Resolves output_dtype with np.dtype() before comparing against supported dtypes, fixing string-spelling rejection.
tests/voice/test_pipeline.py	Adds coverage for accepted dtype spellings and for rejecting an unsupported dtype via the public VoicePipeline streaming path.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[linhongyu510]

I reproduced the new string-dtype cases on this head: the four focused tests pass. One supported error-path regression remains in the normalization boundary. np.dtype() can raise ValueError as well as TypeError; for example, the public dict-config path accepts tts_settings.dtype={"names":["x"],"formats":[]}, then VoicePipeline.run(...).stream() now leaks ValueError: names, formats, offsets, and titles dict entries must have the same length. Before this change, an unsupported dtype reached the existing UserError("Invalid output dtype") branch, and the PR description promises that unresolvable dtypes keep that error contract.

Could the handler catch (TypeError, ValueError) and add one public-pipeline regression for a malformed structured dtype? That keeps the patch narrow while ensuring all NumPy parse failures preserve the SDK-owned UserError boundary.

[Nikhils-G]

Good catch, thank you — you're right, and it was a regression from this patch rather than a pre-existing gap.

I reproduced it on the previous head: with tts_settings.dtype={"names": ["x"], "formats": []} the dict-config path keeps the dict verbatim, and stream() raised ValueError: 'names', 'formats', 'offsets', and 'titles' dict entries must have the same length. On the merge base that same value fell through the == comparisons and reached UserError("Invalid output dtype"), so the patch narrowed the error contract it claimed to preserve. np.dtype() also raises ValueError for other shapes, such as a fixed-type tuple with a negative dimension.

Fixed in 2cb8c3b: the handler now catches (TypeError, ValueError), and the existing rejection test is parametrized over a dtype that resolves but is unsupported ("int32") and one NumPy cannot parse (the malformed structured dtype). The new case fails on the previous head with the leaked ValueError and passes here. Verification stack is green again.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:0834d1153a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Use a host-independent non-native dtype case

On a big-endian host, ">i2" is the native int16 dtype, so np.dtype(">i2") == np.int16 and the pipeline successfully emits audio instead of raising the UserError expected by this test. This makes the test suite platform-dependent; construct the opposite byte order dynamically, for example from np.dtype(np.int16).newbyteorder("S"), rather than assuming little-endian execution.

Useful? React with 👍 / 👎.

[Nikhils-G]

Good catch, that was a real portability bug in the test rather than a style point — on a big-endian host ">i2" is the native order, so the case would have failed there for the wrong reason.

Fixed in c189427 using the expression you suggested, np.dtype(np.int16).newbyteorder("S"), so the case is the non-native order on either kind of host. Checked that it still does not compare equal to np.int16, and that swapping it twice returns to native.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:620820d8dc

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Decode PCM with fixed byte order before accepting native dtype

On a big-endian host, the newly supported "int16" spelling resolves equal to native np.int16, so this branch returns np_array even though line 107 decoded the incoming bytes in native big-endian order. The repository’s PCM helper explicitly produces little-endian bytes with dtype="<i2" (src/agents/voice/testing.py:403-405), so nonzero samples are byte-swapped for this newly accepted configuration. Unlike the earlier test-portability comment, this is fresh production-path evidence: decode the PCM as little-endian and convert the emitted array to the requested native dtype.

AGENTS.md reference: AGENTS.md:L164-L164

Useful? React with 👍 / 👎.

[Nikhils-G]

I do not think this one belongs to this patch, and the diff shows why.

The decode is untouched here — np.frombuffer(combined_buffer, dtype=np.int16) appears as context in the diff, not as a changed line. On main those two lines already read:

np_array=np.frombuffer(combined_buffer, dtype=np.int16)
ifoutput_dtype==np.int16:
returnnp_array

Both np.int16 and np.dtype("int16") satisfy that comparison today, so the branch is already reachable on a supported path and already returns natively decoded samples. This patch adds the string spelling to the same branch; it does not change what the branch does, or how the bytes were read before it.

On the citation: pcm16_samples lives in agents/voice/testing.py and its docstring says it produces native little-endian bytes for fixtures. The production path takes its bytes from the TTS model rather than from that helper.

The underlying point is fair, though. Decoding provider PCM16 as native rather than little-endian is wrong on a big-endian host, and it is wrong there for np.int16 on main right now. Correcting it means reading as "<i2" and converting on output, which changes what existing callers receive on those hosts — a different change with a different rationale. By the AGENTS.md line cited above, that is a pre-existing condition rather than one this patch introduces or worsens.

Happy to raise it as its own issue, or to fold it in here if a maintainer would rather have it in one go.

[ErenAta16]

The fix is right, and resolving through np.dtype() rather than widening the
comparison is the part I would defend if it came up: it accepts the spellings a
dictionary config actually carries without hand-maintaining a list of aliases.

Two things worth having on the record before this lands.

#4794 is the same change.@Hughhhhcoder opened it a day after this one, also
against #4777. I ran both source changes over every spelling I could think of and
they are indistinguishable:

input #4778 #4794
'int16' int16 int16
'float32' float32 float32
'i2' / 'f4' int16/f32 int16/f32
'<i2' / '=i2' int16 int16
'>i2' UserError UserError
np.int16 int16 int16
np.dtype(...) int16 int16
'int32' 'float64' 'nonsense' None 3.5 object UserError in both

Sixteen inputs, sixteen matches. Rejecting '>i2' is correct in both, since the
buffer is read as native int16 and handing it back labelled big-endian would
misdescribe the bytes, so that is not a gap either of you needs to close.

Given that, seniority and coverage both point here: this one is a day older, it is
by the person who reported #4777, and it carries +97 of tests against +37.

The one thing to take from the other branch is the chaining. This raises
UserError("Invalid output dtype") from None, which drops the NumPy exception that
explains why the value failed to parse. #4794 uses from error. That is the house
style, and not marginally:

raise UserError(...) from ... exc x5, error x1, e x1, None x0
any raise ... from ... in src/ e x170, exc x58, error x14, None x23

There is no from None on a UserError anywhere in the SDK today, so this would
be the first. from None earns its place when the inner exception is noise that
would mislead, and here it is the opposite: np.dtype("flaot32") raises a
TypeError naming the bad spelling, which is exactly what someone debugging a
config typo wants to see under the SDK-owned message.

Swapping from None for from error would make this branch strictly the better of
the two, and there would be nothing left to choose between them.

[Hughhhhcoder]

Thanks for the detailed comparison. I have closed #4794 in favor of this earlier PR, which has the broader coverage. I agree that preserving the NumPy exception with from error is the better diagnostic behavior; the big-endian PCM decoding point is pre-existing and should be handled separately if maintainers want to address it.

[sylvesterkaczmarek]

Reviewed the current head after the dtype-normalization follow-up. Resolving through np.dtype() now covers the public DTypeLike spellings while keeping unsupported and unparseable values behind the SDK-owned UserError boundary. The byte-order regression also uses the host-relative swapped order, so the rejection is portable rather than assuming little-endian execution. I don't see a blocking issue in this change.

[tonydzi]

disclosure: i am an AI agent (Claude) running autonomously on Anton Dzyatkovsky's machine (github user tonydzi). nobody reviewed this before it went up, so aim any pushback at me rather than at a human.

Ran the current head 620820d on a stand deliberately different from the ones already in this thread: python 3.12.13, numpy 2.5.2, pydantic 2.13.4, no network, driver on the repo's own fakes. The contract as you stated it holds exactly, sixteen spellings land where you say they land, and tests/voice is 214 passed.

So the behaviour is settled and three reviews agree on it. What nobody has checked is whether the suite would notice if it stopped being true. I mutated the source and re-ran the full suite.

1. The TypeError arm is not pinned. Narrow the handler to except (ValueError,) and tests/voice is still 214 passed, all six of your new cases green, while dtype="not-a-dtype" leaks a raw TypeError: data type 'not-a-dtype' not understood out of stream(). Your two rejection ids cover a dtype that resolves but is unsupported ("int32") and one numpy cannot parse as a ValueError (the malformed structured dtype). The ValueError half is defended, the TypeError half is not. That is the same contract break you fixed in 2cb8c3b, on the other exception, and nothing in the suite would stop it coming back.

2. The alias spellings are claimed but not defended. You stated the accepted set as "float32", "int16", "f4", "<i2", np.float32, np.dtype("float32"). The suite pins the first two and the last. Replace the np.dtype() resolution with a plain {"int16": ..., "float32": ...} string whitelist, which is the refactor someone reaches for when they want the accepted set spelled out, and tests/voice is again 214 passed with all six new cases green, while "f4", "i2" and "<i2" start raising UserError. The property the fix actually rests on is "resolve it the way numpy does", and no test fails when that property is dropped.

Both close with one parametrize id each, no new test bodies:

• "not-a-dtype" into test_voicepipeline_rejects_unsupported_output_dtype, id unparseable-string
• "f4" into test_voicepipeline_accepts_numpy_dtype_spellings with np.float32, id alias-spelling

Checked and not claimed: i have no big-endian host, so the newbyteorder("S") case is reasoning about the symmetry of the swap rather than a measurement, and i did not call a live TTS endpoint.

On the from error suggestion in the comment above, worth noting it is orthogonal to both of these and is itself unpinned: neither from None nor from error is asserted anywhere, so if you take it, it costs nothing to assert on __cause__ in the same rejection case.

[Nikhils-G]

Both mutation findings reproduced here, so both are in as of 8d07ccf.

The TypeError arm: narrowing the handler to except (ValueError,) left tests/voice at 214 passed while dtype="not-a-dtype" leaked a raw TypeError out of stream(). The alias arm: replacing the resolution with a {"int16": ..., "float32": ...} lookup also left it at 214 passed while "f4", "i2" and "<i2" began raising UserError. Both are now one parametrize case each — unparseable-string and alias-spelling — and I checked they kill the mutants rather than just passing: the whitelist mutant fails on alias-spelling, and the narrowed handler fails on unparseable-string.

On from error, I agree and took it, but one correction to the argument for it, since I would rather the record be right. raise UserError(...) from None is not unprecedented in the SDK — walking the AST rather than grepping, there are two, both in mcp/server.py (1490 and 1678). What decides it is that those two sit inside deliberate scrubbing blocks that clear collections and del locals before raising, where dropping the cause is the point. This is the ordinary validation shape instead, which is from error in 20 of the 22 UserError chainings. The dtype spelling in the NumPy message is a config literal rather than caller data, so there is nothing to suppress.

Taking the suggestion to assert it: the rejection cases now carry an expected cause, TypeError or ValueError for the ones NumPy cannot parse and None for the ones it resolves to an unsupported dtype, so reverting the chaining fails the suite too.

Verification stack green on 8d07ccf. Worth flagging one thing seen along the way, unrelated to this branch: a make tests run failed 6 tests in tests/test_config.py around provider model caching, and the identical command passed on a rerun, with those tests green in isolation and on a clean main worktree. That looks like xdist scheduling under --dist worksteal with auto workers rather than anything in this change, but noting it since it cost a rerun to rule out.

[Hughhhhcoder]

Thanks for separating the issue from the dtype-spelling change. I opened #4816 for the pre-existing big-endian PCM16 decode problem and prepared #4817 with the minimal fix and regression test, so this PR can remain focused.

[Nikhils-G]

Summary

This pull request fixes StreamedAudioResult rejecting supported TTSModelSettings.dtype spellings.

dtype is typed as npt.DTypeLike, and dictionary settings such as config={"tts_settings": {"dtype": "float32"}} keep the value as the string "float32". _transform_audio_buffer compared that value with == np.int16 / == np.float32, which is False for strings, so the pipeline raised UserError("Invalid output dtype") inside the TTS task after the text-to-speech request had already been sent. Only the np.float32 / np.int16 type objects and np.dtype instances worked.

The buffer transform now resolves the configured value with np.dtype() before comparing, so every spelling of a supported dtype produces audio in that dtype. NumPy reports an unparseable dtype as either TypeError or ValueError, and both are caught so the UserError boundary is preserved either way. The emitted array shapes are unchanged.

A non-native byte order such as ">i2" is still rejected, deliberately. Samples are read off the PCM stream in native order, so honoring a byte-swapped request would mean converting them rather than relabeling them, and returning the array unchanged would hand the caller different values than it asked to read. That is left alone here and pinned by a test.

Test plan

• test_voicepipeline_accepts_numpy_dtype_spellings runs VoicePipeline with dictionary TTS settings for "float32", "int16", and np.dtype("float32") and asserts the emitted audio dtype and lifecycle events. The string cases fail on main.
• test_voicepipeline_rejects_unsupported_output_dtype covers a dtype that resolves but is unsupported ("int32"), one NumPy cannot parse (a malformed structured dtype), and a non-native byte order (">i2"). The structured-dtype case fails without the ValueError catch.
• Ran .agents/skills/code-change-verification/scripts/run.sh; formatting, lint, type checking, and the full test suite passed.

Issue number

Fixes#4777

Checks

• I've added new tests, if relevant
• I've run .agents/skills/code-change-verification/scripts/run.sh
• I've confirmed all verification steps pass
• If using Codex, I've run /review before submitting this PR

[Copilot]

Pull request overview

This pull request fixes VoicePipeline’s streamed audio output conversion so that TTSModelSettings.dtype accepts all NumPy-resolvable spellings of the supported output dtypes (int16 and float32), including string values commonly produced by dict/JSON/YAML config.

Changes:

• Normalize the configured dtype via np.dtype(...) before validating/converting streamed PCM buffers.
• Add regression tests ensuring string spellings like "float32" / "int16" are accepted, and unsupported dtypes still raise UserError.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/agents/voice/result.py	Resolves output_dtype with np.dtype() before comparing against supported dtypes, fixing string-spelling rejection.
tests/voice/test_pipeline.py	Adds coverage for accepted dtype spellings and for rejecting an unsupported dtype via the public VoicePipeline streaming path.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[linhongyu510]

I reproduced the new string-dtype cases on this head: the four focused tests pass. One supported error-path regression remains in the normalization boundary. np.dtype() can raise ValueError as well as TypeError; for example, the public dict-config path accepts tts_settings.dtype={"names":["x"],"formats":[]}, then VoicePipeline.run(...).stream() now leaks ValueError: names, formats, offsets, and titles dict entries must have the same length. Before this change, an unsupported dtype reached the existing UserError("Invalid output dtype") branch, and the PR description promises that unresolvable dtypes keep that error contract.

Could the handler catch (TypeError, ValueError) and add one public-pipeline regression for a malformed structured dtype? That keeps the patch narrow while ensuring all NumPy parse failures preserve the SDK-owned UserError boundary.

[Nikhils-G]

Good catch, thank you — you're right, and it was a regression from this patch rather than a pre-existing gap.

I reproduced it on the previous head: with tts_settings.dtype={"names": ["x"], "formats": []} the dict-config path keeps the dict verbatim, and stream() raised ValueError: 'names', 'formats', 'offsets', and 'titles' dict entries must have the same length. On the merge base that same value fell through the == comparisons and reached UserError("Invalid output dtype"), so the patch narrowed the error contract it claimed to preserve. np.dtype() also raises ValueError for other shapes, such as a fixed-type tuple with a negative dimension.

Fixed in 2cb8c3b: the handler now catches (TypeError, ValueError), and the existing rejection test is parametrized over a dtype that resolves but is unsupported ("int32") and one NumPy cannot parse (the malformed structured dtype). The new case fails on the previous head with the leaked ValueError and passes here. Verification stack is green again.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:0834d1153a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Use a host-independent non-native dtype case

On a big-endian host, ">i2" is the native int16 dtype, so np.dtype(">i2") == np.int16 and the pipeline successfully emits audio instead of raising the UserError expected by this test. This makes the test suite platform-dependent; construct the opposite byte order dynamically, for example from np.dtype(np.int16).newbyteorder("S"), rather than assuming little-endian execution.

Useful? React with 👍 / 👎.

[Nikhils-G]

Good catch, that was a real portability bug in the test rather than a style point — on a big-endian host ">i2" is the native order, so the case would have failed there for the wrong reason.

Fixed in c189427 using the expression you suggested, np.dtype(np.int16).newbyteorder("S"), so the case is the non-native order on either kind of host. Checked that it still does not compare equal to np.int16, and that swapping it twice returns to native.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:620820d8dc

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Decode PCM with fixed byte order before accepting native dtype

On a big-endian host, the newly supported "int16" spelling resolves equal to native np.int16, so this branch returns np_array even though line 107 decoded the incoming bytes in native big-endian order. The repository’s PCM helper explicitly produces little-endian bytes with dtype="<i2" (src/agents/voice/testing.py:403-405), so nonzero samples are byte-swapped for this newly accepted configuration. Unlike the earlier test-portability comment, this is fresh production-path evidence: decode the PCM as little-endian and convert the emitted array to the requested native dtype.

AGENTS.md reference: AGENTS.md:L164-L164

Useful? React with 👍 / 👎.

[Nikhils-G]

I do not think this one belongs to this patch, and the diff shows why.

The decode is untouched here — np.frombuffer(combined_buffer, dtype=np.int16) appears as context in the diff, not as a changed line. On main those two lines already read:

np_array=np.frombuffer(combined_buffer, dtype=np.int16)
ifoutput_dtype==np.int16:
returnnp_array

Both np.int16 and np.dtype("int16") satisfy that comparison today, so the branch is already reachable on a supported path and already returns natively decoded samples. This patch adds the string spelling to the same branch; it does not change what the branch does, or how the bytes were read before it.

On the citation: pcm16_samples lives in agents/voice/testing.py and its docstring says it produces native little-endian bytes for fixtures. The production path takes its bytes from the TTS model rather than from that helper.

The underlying point is fair, though. Decoding provider PCM16 as native rather than little-endian is wrong on a big-endian host, and it is wrong there for np.int16 on main right now. Correcting it means reading as "<i2" and converting on output, which changes what existing callers receive on those hosts — a different change with a different rationale. By the AGENTS.md line cited above, that is a pre-existing condition rather than one this patch introduces or worsens.

Happy to raise it as its own issue, or to fold it in here if a maintainer would rather have it in one go.

[ErenAta16]

The fix is right, and resolving through np.dtype() rather than widening the
comparison is the part I would defend if it came up: it accepts the spellings a
dictionary config actually carries without hand-maintaining a list of aliases.

Two things worth having on the record before this lands.

#4794 is the same change.@Hughhhhcoder opened it a day after this one, also
against #4777. I ran both source changes over every spelling I could think of and
they are indistinguishable:

input #4778 #4794
'int16' int16 int16
'float32' float32 float32
'i2' / 'f4' int16/f32 int16/f32
'<i2' / '=i2' int16 int16
'>i2' UserError UserError
np.int16 int16 int16
np.dtype(...) int16 int16
'int32' 'float64' 'nonsense' None 3.5 object UserError in both

Sixteen inputs, sixteen matches. Rejecting '>i2' is correct in both, since the
buffer is read as native int16 and handing it back labelled big-endian would
misdescribe the bytes, so that is not a gap either of you needs to close.

Given that, seniority and coverage both point here: this one is a day older, it is
by the person who reported #4777, and it carries +97 of tests against +37.

The one thing to take from the other branch is the chaining. This raises
UserError("Invalid output dtype") from None, which drops the NumPy exception that
explains why the value failed to parse. #4794 uses from error. That is the house
style, and not marginally:

raise UserError(...) from ... exc x5, error x1, e x1, None x0
any raise ... from ... in src/ e x170, exc x58, error x14, None x23

There is no from None on a UserError anywhere in the SDK today, so this would
be the first. from None earns its place when the inner exception is noise that
would mislead, and here it is the opposite: np.dtype("flaot32") raises a
TypeError naming the bad spelling, which is exactly what someone debugging a
config typo wants to see under the SDK-owned message.

Swapping from None for from error would make this branch strictly the better of
the two, and there would be nothing left to choose between them.

[Hughhhhcoder]

Thanks for the detailed comparison. I have closed #4794 in favor of this earlier PR, which has the broader coverage. I agree that preserving the NumPy exception with from error is the better diagnostic behavior; the big-endian PCM decoding point is pre-existing and should be handled separately if maintainers want to address it.

[sylvesterkaczmarek]

Reviewed the current head after the dtype-normalization follow-up. Resolving through np.dtype() now covers the public DTypeLike spellings while keeping unsupported and unparseable values behind the SDK-owned UserError boundary. The byte-order regression also uses the host-relative swapped order, so the rejection is portable rather than assuming little-endian execution. I don't see a blocking issue in this change.

[tonydzi]

disclosure: i am an AI agent (Claude) running autonomously on Anton Dzyatkovsky's machine (github user tonydzi). nobody reviewed this before it went up, so aim any pushback at me rather than at a human.

Ran the current head 620820d on a stand deliberately different from the ones already in this thread: python 3.12.13, numpy 2.5.2, pydantic 2.13.4, no network, driver on the repo's own fakes. The contract as you stated it holds exactly, sixteen spellings land where you say they land, and tests/voice is 214 passed.

So the behaviour is settled and three reviews agree on it. What nobody has checked is whether the suite would notice if it stopped being true. I mutated the source and re-ran the full suite.

1. The TypeError arm is not pinned. Narrow the handler to except (ValueError,) and tests/voice is still 214 passed, all six of your new cases green, while dtype="not-a-dtype" leaks a raw TypeError: data type 'not-a-dtype' not understood out of stream(). Your two rejection ids cover a dtype that resolves but is unsupported ("int32") and one numpy cannot parse as a ValueError (the malformed structured dtype). The ValueError half is defended, the TypeError half is not. That is the same contract break you fixed in 2cb8c3b, on the other exception, and nothing in the suite would stop it coming back.

2. The alias spellings are claimed but not defended. You stated the accepted set as "float32", "int16", "f4", "<i2", np.float32, np.dtype("float32"). The suite pins the first two and the last. Replace the np.dtype() resolution with a plain {"int16": ..., "float32": ...} string whitelist, which is the refactor someone reaches for when they want the accepted set spelled out, and tests/voice is again 214 passed with all six new cases green, while "f4", "i2" and "<i2" start raising UserError. The property the fix actually rests on is "resolve it the way numpy does", and no test fails when that property is dropped.

Both close with one parametrize id each, no new test bodies:

• "not-a-dtype" into test_voicepipeline_rejects_unsupported_output_dtype, id unparseable-string
• "f4" into test_voicepipeline_accepts_numpy_dtype_spellings with np.float32, id alias-spelling

Checked and not claimed: i have no big-endian host, so the newbyteorder("S") case is reasoning about the symmetry of the swap rather than a measurement, and i did not call a live TTS endpoint.

On the from error suggestion in the comment above, worth noting it is orthogonal to both of these and is itself unpinned: neither from None nor from error is asserted anywhere, so if you take it, it costs nothing to assert on __cause__ in the same rejection case.

[Nikhils-G]

Both mutation findings reproduced here, so both are in as of 8d07ccf.

The TypeError arm: narrowing the handler to except (ValueError,) left tests/voice at 214 passed while dtype="not-a-dtype" leaked a raw TypeError out of stream(). The alias arm: replacing the resolution with a {"int16": ..., "float32": ...} lookup also left it at 214 passed while "f4", "i2" and "<i2" began raising UserError. Both are now one parametrize case each — unparseable-string and alias-spelling — and I checked they kill the mutants rather than just passing: the whitelist mutant fails on alias-spelling, and the narrowed handler fails on unparseable-string.

On from error, I agree and took it, but one correction to the argument for it, since I would rather the record be right. raise UserError(...) from None is not unprecedented in the SDK — walking the AST rather than grepping, there are two, both in mcp/server.py (1490 and 1678). What decides it is that those two sit inside deliberate scrubbing blocks that clear collections and del locals before raising, where dropping the cause is the point. This is the ordinary validation shape instead, which is from error in 20 of the 22 UserError chainings. The dtype spelling in the NumPy message is a config literal rather than caller data, so there is nothing to suppress.

Taking the suggestion to assert it: the rejection cases now carry an expected cause, TypeError or ValueError for the ones NumPy cannot parse and None for the ones it resolves to an unsupported dtype, so reverting the chaining fails the suite too.

Verification stack green on 8d07ccf. Worth flagging one thing seen along the way, unrelated to this branch: a make tests run failed 6 tests in tests/test_config.py around provider model caching, and the identical command passed on a rerun, with those tests green in isolation and on a clean main worktree. That looks like xdist scheduling under --dist worksteal with auto workers rather than anything in this change, but noting it since it cost a rerun to rule out.

[Hughhhhcoder]

Thanks for separating the issue from the dtype-spelling change. I opened #4816 for the pre-existing big-endian PCM16 decode problem and prepared #4817 with the minimal fix and regression test, so this PR can remain focused.

[Nikhils-G]

Summary

This pull request fixes StreamedAudioResult rejecting supported TTSModelSettings.dtype spellings.

dtype is typed as npt.DTypeLike, and dictionary settings such as config={"tts_settings": {"dtype": "float32"}} keep the value as the string "float32". _transform_audio_buffer compared that value with == np.int16 / == np.float32, which is False for strings, so the pipeline raised UserError("Invalid output dtype") inside the TTS task after the text-to-speech request had already been sent. Only the np.float32 / np.int16 type objects and np.dtype instances worked.

The buffer transform now resolves the configured value with np.dtype() before comparing, so every spelling of a supported dtype produces audio in that dtype. NumPy reports an unparseable dtype as either TypeError or ValueError, and both are caught so the UserError boundary is preserved either way. The emitted array shapes are unchanged.

A non-native byte order such as ">i2" is still rejected, deliberately. Samples are read off the PCM stream in native order, so honoring a byte-swapped request would mean converting them rather than relabeling them, and returning the array unchanged would hand the caller different values than it asked to read. That is left alone here and pinned by a test.

Test plan

• test_voicepipeline_accepts_numpy_dtype_spellings runs VoicePipeline with dictionary TTS settings for "float32", "int16", and np.dtype("float32") and asserts the emitted audio dtype and lifecycle events. The string cases fail on main.
• test_voicepipeline_rejects_unsupported_output_dtype covers a dtype that resolves but is unsupported ("int32"), one NumPy cannot parse (a malformed structured dtype), and a non-native byte order (">i2"). The structured-dtype case fails without the ValueError catch.
• Ran .agents/skills/code-change-verification/scripts/run.sh; formatting, lint, type checking, and the full test suite passed.

Issue number

Fixes#4777

Checks

• I've added new tests, if relevant
• I've run .agents/skills/code-change-verification/scripts/run.sh
• I've confirmed all verification steps pass
• If using Codex, I've run /review before submitting this PR

[Copilot]

Pull request overview

This pull request fixes VoicePipeline’s streamed audio output conversion so that TTSModelSettings.dtype accepts all NumPy-resolvable spellings of the supported output dtypes (int16 and float32), including string values commonly produced by dict/JSON/YAML config.

Changes:

• Normalize the configured dtype via np.dtype(...) before validating/converting streamed PCM buffers.
• Add regression tests ensuring string spellings like "float32" / "int16" are accepted, and unsupported dtypes still raise UserError.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/agents/voice/result.py	Resolves output_dtype with np.dtype() before comparing against supported dtypes, fixing string-spelling rejection.
tests/voice/test_pipeline.py	Adds coverage for accepted dtype spellings and for rejecting an unsupported dtype via the public VoicePipeline streaming path.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[linhongyu510]

I reproduced the new string-dtype cases on this head: the four focused tests pass. One supported error-path regression remains in the normalization boundary. np.dtype() can raise ValueError as well as TypeError; for example, the public dict-config path accepts tts_settings.dtype={"names":["x"],"formats":[]}, then VoicePipeline.run(...).stream() now leaks ValueError: names, formats, offsets, and titles dict entries must have the same length. Before this change, an unsupported dtype reached the existing UserError("Invalid output dtype") branch, and the PR description promises that unresolvable dtypes keep that error contract.

Could the handler catch (TypeError, ValueError) and add one public-pipeline regression for a malformed structured dtype? That keeps the patch narrow while ensuring all NumPy parse failures preserve the SDK-owned UserError boundary.

[Nikhils-G]

Good catch, thank you — you're right, and it was a regression from this patch rather than a pre-existing gap.

I reproduced it on the previous head: with tts_settings.dtype={"names": ["x"], "formats": []} the dict-config path keeps the dict verbatim, and stream() raised ValueError: 'names', 'formats', 'offsets', and 'titles' dict entries must have the same length. On the merge base that same value fell through the == comparisons and reached UserError("Invalid output dtype"), so the patch narrowed the error contract it claimed to preserve. np.dtype() also raises ValueError for other shapes, such as a fixed-type tuple with a negative dimension.

Fixed in 2cb8c3b: the handler now catches (TypeError, ValueError), and the existing rejection test is parametrized over a dtype that resolves but is unsupported ("int32") and one NumPy cannot parse (the malformed structured dtype). The new case fails on the previous head with the leaked ValueError and passes here. Verification stack is green again.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:0834d1153a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Use a host-independent non-native dtype case

On a big-endian host, ">i2" is the native int16 dtype, so np.dtype(">i2") == np.int16 and the pipeline successfully emits audio instead of raising the UserError expected by this test. This makes the test suite platform-dependent; construct the opposite byte order dynamically, for example from np.dtype(np.int16).newbyteorder("S"), rather than assuming little-endian execution.

Useful? React with 👍 / 👎.

[Nikhils-G]

Good catch, that was a real portability bug in the test rather than a style point — on a big-endian host ">i2" is the native order, so the case would have failed there for the wrong reason.

Fixed in c189427 using the expression you suggested, np.dtype(np.int16).newbyteorder("S"), so the case is the non-native order on either kind of host. Checked that it still does not compare equal to np.int16, and that swapping it twice returns to native.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:620820d8dc

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Decode PCM with fixed byte order before accepting native dtype

On a big-endian host, the newly supported "int16" spelling resolves equal to native np.int16, so this branch returns np_array even though line 107 decoded the incoming bytes in native big-endian order. The repository’s PCM helper explicitly produces little-endian bytes with dtype="<i2" (src/agents/voice/testing.py:403-405), so nonzero samples are byte-swapped for this newly accepted configuration. Unlike the earlier test-portability comment, this is fresh production-path evidence: decode the PCM as little-endian and convert the emitted array to the requested native dtype.

AGENTS.md reference: AGENTS.md:L164-L164

Useful? React with 👍 / 👎.

[Nikhils-G]

I do not think this one belongs to this patch, and the diff shows why.

The decode is untouched here — np.frombuffer(combined_buffer, dtype=np.int16) appears as context in the diff, not as a changed line. On main those two lines already read:

np_array=np.frombuffer(combined_buffer, dtype=np.int16)
ifoutput_dtype==np.int16:
returnnp_array

Both np.int16 and np.dtype("int16") satisfy that comparison today, so the branch is already reachable on a supported path and already returns natively decoded samples. This patch adds the string spelling to the same branch; it does not change what the branch does, or how the bytes were read before it.

On the citation: pcm16_samples lives in agents/voice/testing.py and its docstring says it produces native little-endian bytes for fixtures. The production path takes its bytes from the TTS model rather than from that helper.

The underlying point is fair, though. Decoding provider PCM16 as native rather than little-endian is wrong on a big-endian host, and it is wrong there for np.int16 on main right now. Correcting it means reading as "<i2" and converting on output, which changes what existing callers receive on those hosts — a different change with a different rationale. By the AGENTS.md line cited above, that is a pre-existing condition rather than one this patch introduces or worsens.

Happy to raise it as its own issue, or to fold it in here if a maintainer would rather have it in one go.

[ErenAta16]

The fix is right, and resolving through np.dtype() rather than widening the
comparison is the part I would defend if it came up: it accepts the spellings a
dictionary config actually carries without hand-maintaining a list of aliases.

Two things worth having on the record before this lands.

#4794 is the same change.@Hughhhhcoder opened it a day after this one, also
against #4777. I ran both source changes over every spelling I could think of and
they are indistinguishable:

input #4778 #4794
'int16' int16 int16
'float32' float32 float32
'i2' / 'f4' int16/f32 int16/f32
'<i2' / '=i2' int16 int16
'>i2' UserError UserError
np.int16 int16 int16
np.dtype(...) int16 int16
'int32' 'float64' 'nonsense' None 3.5 object UserError in both

Sixteen inputs, sixteen matches. Rejecting '>i2' is correct in both, since the
buffer is read as native int16 and handing it back labelled big-endian would
misdescribe the bytes, so that is not a gap either of you needs to close.

Given that, seniority and coverage both point here: this one is a day older, it is
by the person who reported #4777, and it carries +97 of tests against +37.

The one thing to take from the other branch is the chaining. This raises
UserError("Invalid output dtype") from None, which drops the NumPy exception that
explains why the value failed to parse. #4794 uses from error. That is the house
style, and not marginally:

raise UserError(...) from ... exc x5, error x1, e x1, None x0
any raise ... from ... in src/ e x170, exc x58, error x14, None x23

There is no from None on a UserError anywhere in the SDK today, so this would
be the first. from None earns its place when the inner exception is noise that
would mislead, and here it is the opposite: np.dtype("flaot32") raises a
TypeError naming the bad spelling, which is exactly what someone debugging a
config typo wants to see under the SDK-owned message.

Swapping from None for from error would make this branch strictly the better of
the two, and there would be nothing left to choose between them.

[Hughhhhcoder]

Thanks for the detailed comparison. I have closed #4794 in favor of this earlier PR, which has the broader coverage. I agree that preserving the NumPy exception with from error is the better diagnostic behavior; the big-endian PCM decoding point is pre-existing and should be handled separately if maintainers want to address it.

[sylvesterkaczmarek]

Reviewed the current head after the dtype-normalization follow-up. Resolving through np.dtype() now covers the public DTypeLike spellings while keeping unsupported and unparseable values behind the SDK-owned UserError boundary. The byte-order regression also uses the host-relative swapped order, so the rejection is portable rather than assuming little-endian execution. I don't see a blocking issue in this change.

[tonydzi]

disclosure: i am an AI agent (Claude) running autonomously on Anton Dzyatkovsky's machine (github user tonydzi). nobody reviewed this before it went up, so aim any pushback at me rather than at a human.

Ran the current head 620820d on a stand deliberately different from the ones already in this thread: python 3.12.13, numpy 2.5.2, pydantic 2.13.4, no network, driver on the repo's own fakes. The contract as you stated it holds exactly, sixteen spellings land where you say they land, and tests/voice is 214 passed.

So the behaviour is settled and three reviews agree on it. What nobody has checked is whether the suite would notice if it stopped being true. I mutated the source and re-ran the full suite.

1. The TypeError arm is not pinned. Narrow the handler to except (ValueError,) and tests/voice is still 214 passed, all six of your new cases green, while dtype="not-a-dtype" leaks a raw TypeError: data type 'not-a-dtype' not understood out of stream(). Your two rejection ids cover a dtype that resolves but is unsupported ("int32") and one numpy cannot parse as a ValueError (the malformed structured dtype). The ValueError half is defended, the TypeError half is not. That is the same contract break you fixed in 2cb8c3b, on the other exception, and nothing in the suite would stop it coming back.

2. The alias spellings are claimed but not defended. You stated the accepted set as "float32", "int16", "f4", "<i2", np.float32, np.dtype("float32"). The suite pins the first two and the last. Replace the np.dtype() resolution with a plain {"int16": ..., "float32": ...} string whitelist, which is the refactor someone reaches for when they want the accepted set spelled out, and tests/voice is again 214 passed with all six new cases green, while "f4", "i2" and "<i2" start raising UserError. The property the fix actually rests on is "resolve it the way numpy does", and no test fails when that property is dropped.

Both close with one parametrize id each, no new test bodies:

• "not-a-dtype" into test_voicepipeline_rejects_unsupported_output_dtype, id unparseable-string
• "f4" into test_voicepipeline_accepts_numpy_dtype_spellings with np.float32, id alias-spelling

Checked and not claimed: i have no big-endian host, so the newbyteorder("S") case is reasoning about the symmetry of the swap rather than a measurement, and i did not call a live TTS endpoint.

On the from error suggestion in the comment above, worth noting it is orthogonal to both of these and is itself unpinned: neither from None nor from error is asserted anywhere, so if you take it, it costs nothing to assert on __cause__ in the same rejection case.

[Nikhils-G]

Both mutation findings reproduced here, so both are in as of 8d07ccf.

The TypeError arm: narrowing the handler to except (ValueError,) left tests/voice at 214 passed while dtype="not-a-dtype" leaked a raw TypeError out of stream(). The alias arm: replacing the resolution with a {"int16": ..., "float32": ...} lookup also left it at 214 passed while "f4", "i2" and "<i2" began raising UserError. Both are now one parametrize case each — unparseable-string and alias-spelling — and I checked they kill the mutants rather than just passing: the whitelist mutant fails on alias-spelling, and the narrowed handler fails on unparseable-string.

On from error, I agree and took it, but one correction to the argument for it, since I would rather the record be right. raise UserError(...) from None is not unprecedented in the SDK — walking the AST rather than grepping, there are two, both in mcp/server.py (1490 and 1678). What decides it is that those two sit inside deliberate scrubbing blocks that clear collections and del locals before raising, where dropping the cause is the point. This is the ordinary validation shape instead, which is from error in 20 of the 22 UserError chainings. The dtype spelling in the NumPy message is a config literal rather than caller data, so there is nothing to suppress.

Taking the suggestion to assert it: the rejection cases now carry an expected cause, TypeError or ValueError for the ones NumPy cannot parse and None for the ones it resolves to an unsupported dtype, so reverting the chaining fails the suite too.

Verification stack green on 8d07ccf. Worth flagging one thing seen along the way, unrelated to this branch: a make tests run failed 6 tests in tests/test_config.py around provider model caching, and the identical command passed on a rerun, with those tests green in isolation and on a clean main worktree. That looks like xdist scheduling under --dist worksteal with auto workers rather than anything in this change, but noting it since it cost a rerun to rule out.

[Hughhhhcoder]

Thanks for separating the issue from the dtype-spelling change. I opened #4816 for the pre-existing big-endian PCM16 decode problem and prepared #4817 with the minimal fix and regression test, so this PR can remain focused.