Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions src/agents/voice/result.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,19 @@ def _transform_audio_buffer(

np_array = np.frombuffer(combined_buffer, dtype=np.int16)

if output_dtype == np.int16:
# Resolve the configured dtype the way NumPy does so that every spelling of a supported
# dtype is accepted, including the strings that dictionary-based settings carry. NumPy
# reports an unparseable dtype as either TypeError or ValueError. Both are answered with
# the SDK-owned error, keeping the NumPy cause attached because it names the spelling
# that failed to parse.
try:
resolved_dtype = np.dtype(output_dtype)
except (TypeError, ValueError) as error:
raise UserError("Invalid output dtype") from error

if resolved_dtype == np.int16:
return np_array
Comment on lines +119 to 120

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
if output_dtype == np.int16:
    return np_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.

elif output_dtype == np.float32:
elif resolved_dtype == np.float32:
return (np_array.astype(np.float32) / 32767.0).reshape(-1, 1)
else:
raise UserError("Invalid output dtype")
Expand Down
100 changes: 100 additions & 0 deletions tests/voice/test_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import agents._debug as _debug
from agents import trace
from agents.exceptions import UserError
from tests.testing_processor import fetch_events, fetch_ordered_spans, fetch_span_errors

try:
Expand Down Expand Up @@ -1325,6 +1326,105 @@ async def test_voicepipeline_float32() -> None:
await fake_tts.verify_audio("out_1", audio_chunks[0], dtype=np.float32)


@pytest.mark.asyncio
@pytest.mark.parametrize(
("dtype_spelling", "expected_dtype"),
[
("float32", np.float32),
("int16", np.int16),
("f4", np.float32),
(np.dtype("float32"), np.float32),
],
ids=["float32-string", "int16-string", "alias-spelling", "already-supported-spelling"],
)
async def test_voicepipeline_accepts_numpy_dtype_spellings(
dtype_spelling: npt.DTypeLike, expected_dtype: type[np.int16] | type[np.float32]
) -> None:
"""Dictionary settings carry ``dtype`` as the spelling NumPy resolves, not the type object.

The string cases are the ones that fail before this change, and the alias holds the
property the fix rests on: the value is resolved the way NumPy resolves it rather than
matched against a fixed set of names. The resolved-dtype case is a pin on the spelling
that already worked rather than new coverage, since every accepted spelling now resolves
to the same dtype and takes the same branch.
"""
fake_stt = QueuedSTTModel(["first"])
workflow = QueuedVoiceWorkflow([["out_1"]])
fake_tts = ZeroPcmTTSModel()
pipeline = VoicePipeline(
workflow=workflow,
stt_model=fake_stt,
tts_model=fake_tts,
config={"tts_settings": {"buffer_size": 1, "dtype": dtype_spelling}},
)
result = await pipeline.run(AudioInput(buffer=np.zeros(2, dtype=np.int16)))

events: list[str] = []
audio_dtypes: list[np.dtype[Any]] = []
async for event in result.stream():
if isinstance(event, VoiceStreamEventAudio):
assert event.data is not None
audio_dtypes.append(event.data.dtype)
events.append("audio")
elif isinstance(event, VoiceStreamEventLifecycle):
events.append(event.event)

assert events == ["turn_started", "audio", "turn_ended", "session_ended"]
assert audio_dtypes == [np.dtype(expected_dtype)]


@pytest.mark.asyncio
@pytest.mark.parametrize(
("dtype_spelling", "expected_cause"),
[
("int32", None),
({"names": ["x"], "formats": []}, ValueError),
("not-a-dtype", TypeError),
(np.dtype(np.int16).newbyteorder("S"), None),
],
ids=[
"resolvable-but-unsupported",
"unresolvable-structured-dtype",
"unparseable-string",
"non-native-byte-order",
],
)
async def test_voicepipeline_rejects_unsupported_output_dtype(
dtype_spelling: npt.DTypeLike, expected_cause: type[Exception] | None
) -> None:
"""An unsupported dtype keeps the SDK error, whether or not NumPy can parse it.

A non-native byte order is rejected on purpose. The emitted samples are read from the
PCM stream in native order, so honoring a byte-swapped request would need the samples
converted rather than relabeled, and returning them as they are would hand back
different values than the caller asked to read. That case is swapped from the running
host's own order so the expectation holds on a big-endian machine too.

A value NumPy cannot parse keeps the parse failure attached as the cause, since it names
the spelling that failed. A value NumPy resolves to an unsupported dtype has no cause,
because nothing was raised on the way to rejecting it.
"""
fake_stt = QueuedSTTModel(["first"])
workflow = QueuedVoiceWorkflow([["out_1"]])
fake_tts = ZeroPcmTTSModel()
pipeline = VoicePipeline(
workflow=workflow,
stt_model=fake_stt,
tts_model=fake_tts,
config={"tts_settings": {"buffer_size": 1, "dtype": dtype_spelling}},
)
result = await pipeline.run(AudioInput(buffer=np.zeros(2, dtype=np.int16)))

with pytest.raises(UserError, match="Invalid output dtype") as raised:
async for _ in result.stream():
pass

if expected_cause is None:
assert raised.value.__cause__ is None
else:
assert isinstance(raised.value.__cause__, expected_cause)


@pytest.mark.asyncio
async def test_voicepipeline_transform_data() -> None:
# Single turn. Should produce a single audio output, which is the TTS output for "out_1".
Expand Down