fix(asr): update session handling and improve schema for GA integration

[diyuyi-agora]

openai_asr已经不支持beta api
需要更新

[github-actions]

Thanks for tackling this — the beta Realtime transcription API being retired is a real problem, and the approach of keeping the flat property.json shape and translating to the GA wire format at the boundary is the right call. It keeps the change contained and avoids breaking existing graph configs.

Below is what I found reading client.py, schemas.py, and the surrounding extension/config/test files. I could not execute Python or reach the OpenAI docs in this environment, so items marked [verify] are based on static reading and need a runtime check.

---

1. The migration is incomplete — schemas still import from the beta namespace

schemas.py:23 is unchanged by this PR:

from openai.types.beta.realtime.transcription_session_update_param import (
    SessionTurnDetection,
    SessionInputAudioTranscription,
    SessionInputAudioNoiseReduction,
)

The docstring now points at the GA guide and the !!! this is a beta api !!! warning was removed, but the types backing TranscriptionSessionUpdateParam still come from openai.types.beta.realtime. Combined with the unpinned openai>=1.99.3 in requirements.txt, the first SDK release that drops the beta.realtime namespace breaks this extension at import time — which is exactly the failure mode this PR is trying to fix, just deferred.

Two options, either is fine:

• Migrate these three to their GA equivalents under openai.types.realtime.*, or
• Keep the beta types (they are structurally just TypedDicts) but add an upper bound so the break is a deliberate upgrade rather than a surprise.

Given the repo guidance on pinning dependencies, I would lean toward an upper bound regardless of which you pick.

2. client_secret is now silently dropped

TranscriptionSessionUpdateParam.client_secret still exists in the schema, and Params (with extra="allow") will happily accept it from property.json. But build_ga_session_update() never reads it, so it is silently discarded from the payload — previously model_dump_json(exclude_none=True) included it.

Anyone relying on it gets no error, just a config that quietly stops taking effect. Either wire it into the GA payload or remove the field from the schema so misconfiguration surfaces as a validation error.

3. [verify] Nested None values are no longer stripped

This is the one I would most want confirmed on a live connection. The old path serialized the whole model with model_dump_json(exclude_none=True), which recursed into nested structures. The new path routes nested values through _to_plain_dict():

if isinstance(value, dict):
    return value          # returned as-is, nulls included
if hasattr(value, "model_dump"):
    return value.model_dump(exclude_none=True)

SessionInputAudioTranscription and friends are TypedDicts, not BaseModels — extension.py:275 calls .get("language", "en") on input_audio_transcription, which confirms it arrives as a plain dict. So the isinstance(value, dict) branch is the one that fires, and explicit nulls pass straight through.

A config like "input_audio_transcription": {"model": "whisper-1", "language": null} previously had language stripped; now it ships "language": null, which GA is likely to reject with invalid_request_error. Worth a quick test with a null-containing config. If confirmed, filtering nulls inside _to_plain_dict fixes it in one place:

if isinstance(value, dict):
    return {k: v for k, v in value.items() if v is not None}

4. Three different idioms for the same "omit if absent" check

Within one function:

if transcription:                          # truthiness — drops empty dict
if params.turn_detection is not None:      # checks the model field
if noise_reduction is not None:            # checks the converted value

These are not equivalent: an empty {} is dropped for transcription but preserved for noise_reduction. Whichever semantics you want, picking one and applying it consistently would make the function easier to reason about — and would make the intent obvious to the next person deciding whether an empty dict is meaningful.

5. Hardcoded rate: 24000 applies only to the pcm16 path

"pcm16": {"type": "audio/pcm", "rate": 24000},

This is correct and well-commented for the PCM path — extension.py:188-194 does resample to 24 kHz. But note the resampling in send_audio() is unconditional on format: it resamples to 24 kHz and emits raw PCM16 even when input_audio_format is g711_ulaw or g711_alaw, so those two map entries advertise audio/pcmu/audio/pcma while the bytes on the wire are PCM. That is pre-existing, not introduced here, but this PR is the moment the format declaration becomes explicit in the payload — so it may be worth either rejecting the g711 values at config validation or opening a follow-up.

The _AUDIO_FORMAT_TO_GA[...] direct index is safe, since the Literal type guarantees the key exists. No concern there.

6. Test coverage — the new logic is never executed

build_ga_session_update() is the entire substance of this PR and has no tests. The existing suite patches out OpenAIAsrClient wholesale via patch_asr_client, so neither the new builder nor the renamed event strings are exercised in CI. The renames (transcription_session.updated to session.updated) would pass tests even if they were wrong.

This is a pure function with no I/O — about as cheap to test as code gets. A handful of cases would lock in the wire format:

• minimal params (transcription only) produce the expected nested {"type": "session.update", "session": {"type": "transcription", "audio": {"input": {...}}}}
• turn_detection / noise_reduction present vs. absent
• include=[] vs. populated
• nulls inside input_audio_transcription (which would have caught item 3)

Two smaller things in the existing fixtures while you are in there:

• tests/test_asr_result.py:143 uses "turn_detection": {"mode": "server_vad", ...} — every other config, including property.json, uses "type". Pydantic will likely strip the unknown mode key silently, so the fixture is not testing what it appears to.
• tests/mock.py:101 still emits type="conversation.item.input_audio_transcription.start", which does not correspond to any real event in either protocol version.

7. Minor: on_asr_start now reports a misleading event type

In _handle_event, on receiving session.updated the listener is handed a synthesized Session with type="session.update" (the request type) carrying the locally-held self._params — the server-echoed session config is thrown away. extension.py only logs it, so nothing breaks, but the log line now claims an event type that was never received. If the goal is observability, logging the real session.updated payload from the server would be strictly more useful for debugging GA rejections.

8. Housekeeping

• manifest.json is still at 0.3.4. This is a breaking wire-protocol change; a version bump seems warranted.
• The five docs/README.*.md files document the config surface. The flat shape is preserved so they are not wrong, but they still show "turn_detection": {"enabled": true} and "input_audio_noise_reduction": {"enabled": true}, which do not match the actual schema (pre-existing). Worth correcting while the context is fresh.
• Unrelated but adjacent: _handle_error() does nothing at all when the session is already ready — post-ready server errors never reach on_asr_server_error. Pre-existing, but GA rejection behavior may differ from beta, so it could start mattering more.

---

Summary

The core translation approach is sound and the backward-compatible config shape is the right tradeoff. The two things I would want resolved before merge are the lingering openai.types.beta.realtime import (item 1), since it undercuts the stated goal of the PR, and a unit test for build_ga_session_update (item 6), since nothing currently validates the new payload. Items 2 and 3 are silent-behavior-change risks worth a look; the rest are polish.

Happy to be wrong on item 3 — that one really needs a live connection to settle.

[github-actions]

Review: fix(asr): update session handling and improve schema for GA integration

Thanks for this — moving openai_asr_python off the retired beta Realtime API is necessary work, and the core approach is right. In particular, keeping the flat property.json shape (input_audio_format: "pcm16") as the user-facing contract and translating it to the GA session.update shape in one place (build_ga_session_update) is a good call: existing deployments keep working, and the GA-specific mapping is isolated and testable. The comment explaining why pcm16 maps to rate: 24000 is exactly the kind of note that saves the next reader a trip to the API docs.

A few things worth addressing before merge.

---

1. The core change has no test coverage

build_ga_session_update is the substance of this PR, and it is a pure function — config in, dict out — which makes it about as cheap to test as code gets. Right now nothing exercises it. The existing suite patches OpenAIAsrClient wholesale via patch_asr_client in tests/mock.py, so _update_session, the new payload builder, and the renamed session.updated event handler are all mocked out. A regression in the wire format would ship green.

This matters more than usual here because the payload shape is the only thing this PR changes, and it cannot be validated without talking to OpenAI. A handful of assertions would cover it:

def test_pcm16_maps_to_audio_pcm_24k():
    payload = build_ga_session_update(TranscriptionParam(
        input_audio_format="pcm16",
        input_audio_transcription={"model": "whisper-1", "language": "en"},
    ))
    assert payload["type"] == "session.update"
    assert payload["session"]["type"] == "transcription"
    assert payload["session"]["audio"]["input"]["format"] == {
        "type": "audio/pcm", "rate": 24000,
    }

def test_optional_fields_omitted_when_unset():
    payload = build_ga_session_update(TranscriptionParam(
        input_audio_format="pcm16",
        input_audio_transcription={"model": "whisper-1"},
    ))
    audio_input = payload["session"]["audio"]["input"]
    assert "turn_detection" not in audio_input
    assert "noise_reduction" not in audio_input
    assert "include" not in payload["session"]

Plus the two g711_* mappings. Also worth updating tests/mock.py so the mock start() emits session.updated rather than the current conversation.item.input_audio_transcription.start, so the mock reflects the real event name the client now keys on.

2. schemas.py still imports from the beta namespace

The docstring now drops the "this is a beta api" warning and points at the GA docs, but the type imports directly below it are unchanged:

from openai.types.beta.realtime.transcription_session_update_param import (
    SessionTurnDetection,
    SessionInputAudioTranscription,
    SessionInputAudioNoiseReduction,
)

So the module claims GA while its types come from the SDK path most likely to be removed. Combined with openai>=1.99.3 and no upper bound in pyproject.toml, a future SDK release that drops openai.types.beta.realtime breaks this extension at import time — which surfaces as an addon that fails to load rather than a clean error. Two options, either is fine:

• Move to the GA types under openai.types.realtime.* if the pinned SDK exposes them, or
• Keep the beta types for now but add an upper bound (openai>=1.99.3,<2) and leave a short comment saying the beta-namespaced types are deliberate.

Note requirements.txt carries the same unbounded openai>=1.99.3 and would need the same treatment.

3. manifest.json lost its trailing newline

The diff shows "No newline at end of file" on manifest.json — the only change to that file besides the version bump. Unrelated to the fix and likely to churn against formatting checks. Worth restoring.

---

Medium

Shared mutable dict leaks into the payload. build_ga_session_update inserts a reference to the module-level constant:

audio_input: dict[str, Any] = {
    "format": _AUDIO_FORMAT_TO_GA[params.input_audio_format],
}

Nothing mutates it today, so there is no live bug — but any caller that later tweaks payload["session"]["audio"]["input"]["format"] silently corrupts the constant for every subsequent session in the process, including across reconnects. Cheap to make safe: dict(_AUDIO_FORMAT_TO_GA[params.input_audio_format]).

client_secret is now silently dropped. The beta path serialised the whole TranscriptionParam, so a configured client_secret reached the wire. The new builder reads five fields explicitly and client_secret is not one of them. If it is genuinely unsupported under GA transcription sessions, please remove it from TranscriptionSessionUpdateParam so config validation rejects it loudly; silently ignoring a credential-ish field is the worst of the three outcomes.

Three different idioms for the same optional-field check. Within one function:

if transcription:                              (truthiness)
if params.turn_detection is not None:          (None check on the model field)
if noise_reduction is not None:                (None check on the converted value)

Beyond readability, these are not equivalent: a turn_detection that converts to {} gets emitted as "turn_detection": {}, while a transcription that converts to {} is dropped. Picking one form for all three would remove the inconsistency.

Related, _to_plain_dict treats its two branches differently — the model_dump branch strips None values via exclude_none=True, the isinstance(dict) branch returns the dict verbatim. Since these SDK types are TypedDicts (plain dicts at runtime), the dict branch is the one that actually fires, so an explicit {"model": "whisper-1", "prompt": null} in property.json would forward prompt: null to the GA API rather than omitting it. Stripping None in both branches would make the behaviour uniform and match what the old model_dump_json(exclude_none=True) did.

No way to explicitly disable turn detection. When params.turn_detection is None the key is omitted, which under GA means "keep the server default" (VAD on) rather than "off". Behaviour is unchanged from the beta path, so this is not a regression — but if a user wants VAD off, GA wants an explicit turn_detection: null, and there is currently no config that produces it. Possibly a follow-up.

Docs still describe the beta API. All five docs/README.*.md locales still say things like "using OpenAI beta realtime API" and "OpenAI Beta API: Uses the beta realtime transcription API for cutting-edge performance". Given the stated purpose of this PR (openai_asr 已经不支持 beta api，需要更新), these read as actively wrong now. While you are in there, the README.en-US.md example config is already stale independent of this PR — it puts api_key/organization/project/log_level at the top level instead of under params, and shows turn_detection: {"enabled": true}, which is not a field the schema accepts. property.json has the correct shape and would be a better basis for the example.

---

Minor

• The Session[TranscriptionParam] echo handed to on_asr_start is now built with type="session.update", but it is reporting an event that already happened — session.updated would be more accurate. It is log-only (this object never hits the wire), so cosmetic.
• That same echo still serialises the old flat _params shape, so vendor_status_changed: on_asr_start {...} in the logs no longer matches the payload actually sent. Logging the built GA payload instead would make debugging session setup against real traffic easier.
• _AUDIO_FORMAT_TO_GA accepts g711_ulaw/g711_alaw, but send_audio() in extension.py unconditionally resamples to 24 kHz PCM16 regardless of the configured input_audio_format. So those two mappings would advertise a format the extension never actually sends. Pre-existing, not introduced here — but since the new map makes it look supported, either wiring it up or dropping the entries would avoid a confusing failure mode.
• The explicit five-field whitelist means any GA option not enumerated (e.g. a future audio.input.format.rate override) cannot be passed through at all. Fine under YAGNI for now; just flagging that each new GA knob will need a code change.

---

Things I could not verify

I reviewed this statically and did not run the extension against the live API, so I cannot confirm the two facts the fix hinges on: that the GA endpoint still accepts ?intent=transcription on wss://api.openai.com/v1/realtime without an OpenAI-Beta header, and that GA replies to session.update with session.updated for a transcription-type session. Both look right from the docs, but if you have a successful end-to-end log against GA it would be worth noting in the PR description — that is the evidence a unit test cannot provide.

Version bumps in manifest.json and pyproject.toml are consistent at 0.3.5.

[github-actions]

Review: fix(asr): update session handling and improve schema for GA integration

Thanks for this — migrating off the beta realtime API is necessary and the overall shape is right: dropping the OpenAI-Beta header, renaming transcription_session.update(d) to session.update(d), and translating the flat property.json config into the GA nested session.audio.input payload while keeping the user-facing schema backward compatible. Converting at the boundary rather than breaking existing configs is a good call. The 5-language README updates are consistent with each other and with property.json, and moving api_key under params fixes docs that were already wrong relative to config.py.

A few things I think need attention before merge, most important first.

1. The openai pin looks self-contradictory (likely blocking)

pyproject.toml / requirements.txt change to openai>=1.99.3,<2, but schemas.py now imports from the GA module tree:

from openai.types.realtime.audio_transcription_param import AudioTranscriptionParam
from openai.types.realtime.realtime_transcription_session_audio_input_param import NoiseReduction
from openai.types.realtime.realtime_transcription_session_audio_input_turn_detection_param import (
    RealtimeTranscriptionSessionAudioInputTurnDetectionParam,
)

openai.types.realtime.* (as opposed to the old openai.types.beta.realtime.*) is the GA layout that landed with the 2.x line. If that is right, then a resolver that picks any 1.99.x — which >=1.99.3,<2 explicitly permits, and which is what a fresh install will do — gets an ImportError at addon load. Please double-check which release first ships openai.types.realtime and set the lower bound to that; if it is a 2.x, the <2 cap has to go. I could not verify against PyPI from this environment (no network access), so I may be wrong about the exact boundary, but the combination of "cap below 2" and "import GA-only paths" cannot both be correct.

Second, related concern: <2 is not local to this extension. openai_llm2_python, computer_tool_python, grok_python, and openai_image_generate_tool all declare openai>=2.44.0 in their pyproject.toml, their requirements.txt files all say a bare openai, and install_python_deps.sh runs pip install -r per extension directory in traversal order into one shared site-packages. Several shipped examples (voice-assistant, doodler, http-control, voice-assistant-live2d) include both openai_asr_python and openai_llm2_python. So a <2 cap here makes the final resolved version depend on install order, and in the unlucky order it silently downgrades openai underneath the LLM extensions. If the cap is not strictly required, please drop it.

2. Event rename can stall the stream silently, with unbounded buffering

_handle_event now only recognizes session.updated. If the peer ever replies with the old transcription_session.updated — a proxied endpoint, a gateway, a custom base_url, a vendor lagging the GA rollout — then params_ready_event is never set. The failure mode is not an error; it is:

• send_pcm_data appends to self._pending_audio_messages forever (no cap, no warning), so a live mic stream grows the list without bound;
• no transcript is ever produced;
• _handle_error only forwards errors while not params_ready_event.is_set() and type == "invalid_request_error", so nothing surfaces to the user either.

The unbounded list is pre-existing, but this rename makes it materially easier to hit. Two cheap mitigations: accept both session.updated and transcription_session.updated during the transition, and put a cap (or a duration/size watchdog) on _pending_audio_messages that logs a warning and raises a NON_FATAL_ERROR instead of growing quietly.

3. G711 formats are now silently wrong rather than unsupported

_AUDIO_FORMAT_TO_GA maps g711_ulaw to audio/pcmu and g711_alaw to audio/pcma, but extension.py unconditionally resamples to 24 kHz PCM16 before sending. So configuring G711 declares one encoding to OpenAI and sends another — garbage transcripts, no error anywhere. The READMEs document this honestly ("accepted for forward compatibility"), which is better than nothing, but per docs/ai/L1/04_conventions.md invalid required config should be a FATAL_ERROR. I would rather reject a non-pcm16 input_audio_format at config validation time with a clear message than ship a config knob whose only effect is to corrupt output. That also lets you delete two thirds of the mapping table and two of the new tests.

4. manifest.json lost its trailing newline

The diff ends with a "No newline at end of file" marker. Unrelated to the change and worth restoring.

5. Observability regression in on_asr_start

ga_payload = build_ga_session_update(response.session)
self.ten_env.log_info(f"vendor_status_changed: on_asr_start {json.dumps(ga_payload)}", ...)

The client now populates real values on the response (type="session.updated", event_id=message.get("event_id")), and this log throws both away — it re-serializes the request we would have sent rather than reporting what the server actually acknowledged. For a vendor_status_changed line that is the less useful of the two. Logging the server's actual session.updated payload, or at minimum keeping event_id and type, would debug better and avoids rebuilding the payload purely to log it.

Relatedly, in client.py:

session_update = build_ga_session_update(self._params)
self.logger.debug("Session updated: %s", json.dumps(session_update))

The json.dumps runs even when debug logging is off, and this is the second construction of the same payload on that code path. Minor, but it is on connection setup.

6. include=[] is forwarded, {} is dropped

_to_plain_dict collapses an all-None/empty dict to None so the key is omitted, but include is gated on is not None, so an empty list is sent as "include": []. The __main__ demo in client.py passes exactly include=[]. Inconsistent, and an empty include is meaningless to send — suggest if params.include:.

7. Test coverage

The new test_ga_session_update.py is a good addition — the format mapping, optional-field omission, and especially test_format_dict_is_not_shared_with_module_constant (nice catch on the shared-mutable hazard) are all worth having. Gaps I would close:

• The riskiest part of this PR is untested. Nothing asserts that the client sends type session.update, or that it reacts to session.updated. mock.py replaces the client wholesale, so the rename is covered by zero tests. A small unit test that feeds {"type": "session.updated", "event_id": "..."} into _handle_event and asserts params_ready_event is set and pending audio was flushed would directly cover the regression class in point 2.
• _to_plain_dict is tested only through its dict branch; the model_dump and dict(value) fallback branches are unexercised. It is also private — testing it through build_ga_session_update would let you drop the underscore import.
• No test that a bad input_audio_format is rejected (relevant if you take point 3).
• The new file is missing the Apache license header that the other files in this extension carry.

Nits

• The realtime_transcription_session_audio_input_turn_detection_param import line is 99 chars. Black will not split it, and pylint line-too-long is a convention, which pylint-exit --warn-fail --error-fail does not treat as fatal, so CI should still pass — but it exceeds the repo 80-col standard. Importing the module and aliasing the symbol would read better.
• _set_optional_field checks value is None and then calls _to_plain_dict, which checks it again. Harmless, slightly redundant.
• Given the removal of client_secret, the OpenAI-Beta header, and the event renames, this is a breaking change for anyone on a beta-only endpoint. 0.3.4 to 0.4.0 is defensible for a 0.x package, but a one-line README note about the minimum openai version and the dropped client_secret field would help upgraders.

No security concerns from my read: api_key still flows through params and stays covered by encrypting_serializer, and the new build_ga_session_update payload contains no credentials, so the new log lines do not leak the key.

Happy to re-review once the version pin question is settled — that is the one I would not merge without resolving.