[codex] Add native audio input support - #2
Conversation
|
@claude can you review this PR for any bugs, security issues, or areas for improvement? we dont want to over engineer here, but we want to make sure we're solid supporting transcription through the subscription model provider when the model provider supports it natively. |
|
Claude finished @Schwartz10's task in 6m 29s —— View job Claude finished reviewing this PRTodo list:
SummaryThis is a small, well-scoped PR — validation logic is centralized in One gap stands out given your stated goal of "solid support... when the model provider supports it natively" — see #1 below. Findings1. No model-capability gating for native audio input (the main thing worth fixing before merge) Unlike vision, which has a real fallback gate — Practically: if a client sends Given you don't want to over-engineer this: I wouldn't suggest building an STT fallback path here. But wiring the already-existing 2. Overly broad exception swallowing in
if ptype in {"input_audio", "audio"} and role != "assistant":
try:
from agent.audio_routing import normalize_input_audio_part
converted.append(normalize_input_audio_part(part, validate_data=False))
except Exception:
continueThis catches any exception — including bugs like 3. Minor:
4. Minor: inconsistent recognition of the
5. Nit: Test coverageGood coverage of the validation/normalization core ( Note: I wasn't able to execute the test suite in this sandboxed review environment (command execution required approval that wasn't available), so I relied on static review — the commands listed in the PR description look correct and should be run in CI. |
|
Pushed update 1095fee for the native audio honesty/safety work. What changed:
Validation:
@claude please review this PR again, especially the native audio capability resolution and gateway preflight ordering. |
|
Claude finished @Schwartz10's task in 4m 45s —— View job Claude finished re-reviewing this PRTodo list:
SummaryThis update lands the fix I flagged as the main gap last round, and does it well. Capability resolution (
|
|
@claude please review the latest commit ( I changed this PR from native-audio chat support to an STT utility endpoint only. What changed:
Why: William should transcribe audio first, then send transcript text into normal Hermes chat. That keeps Hermes chat text/image-based and keeps ChatGPT subscription credentials behind the Codex auth boundary. Verification: focused pytest suite passed ( |
|
Claude finished @Schwartz10's task in 5m 19s —— View job Reviewing latest commit
|
|
@claude Final review request for follow-up commit I addressed the three findings from your review:
Self-review against your original comment:
Validation run:
|
|
Claude finished @Schwartz10's task in 2m 55s —— View job Reviewing latest commit
|
|
@claude please do a final review of the new Codex transcription fix in commit f63da06. What changed:
Verification:
Focused self-review found one issue before push: structured Codex JSON errors could have echoed asset_pointer metadata. I fixed that and added coverage. |
|
Claude finished @Schwartz10's task in 2m 45s —— View job Reviewing latest commit
|
… fail on '(empty)' sentinel Two related bugs caused subagent delegation to silently return empty summaries with 0 tokens when the user configured delegation.provider=bedrock alongside delegation.base_url=https://bedrock-runtime.<region>.amazonaws.com. Root cause #1 — misrouting in _resolve_delegation_credentials(): The configured_base_url branch unconditionally forced provider='custom' and api_mode='chat_completions', only specializing for chatgpt.com, anthropic, and kimi hosts. Bedrock (and other native-SDK providers) fell through as 'custom' + chat_completions, which then POSTed OpenAI-shaped JSON at Bedrock's native API. Bedrock rejected the payload and returned nothing, which looked like an empty LLM response to the child agent. Fix: when provider is one of {bedrock, vertex, google, google-genai}, skip the base_url short-circuit and fall through to resolve_runtime_provider(), which knows how to construct the proper SDK client. base_url can still be forwarded through that path for regional overrides. Root cause #2 — '(empty)' sentinel accepted as success: After N retries of empty LLM responses, run_agent.py emits the literal string '(empty)' as final_response. _run_single_child then hit `elif summary:` — '(empty)' is truthy, so status became 'completed' and the parent surfaced a blank result with no error. Users saw api_calls=4, tokens=0, duration~0.4s, status=completed. Fix: treat final_response.strip() == '(empty)' as a failure so the parent surfaces it instead of silently accepting zero-content 'success'. Both paths were reproduced in a live Hermes TUI session on us-west-2 Bedrock (provider=bedrock, model=us.anthropic.claude-sonnet-4-6) and are covered by new tests in tests/tools/test_delegate.py.
Summary
Adds the smallest native-audio path through Hermes agent/API code:
input_audio/audiocontent parts in the API serverThis intentionally leaves CLI, Telegram/gateway voice behavior, desktop microphone UX, and STT fallback routing untouched.
Validation
.venv/bin/python -m compileall agent/audio_routing.py agent/codex_responses_adapter.py gateway/platforms/api_server.py run_agent.py.venv/bin/python -m pytest tests/agent/test_audio_routing.py tests/run_agent/test_run_agent_multimodal_prologue.py -q.venv/bin/python -m pytest tests/gateway/test_api_server_multimodal.py tests/gateway/test_api_server.py -q -k 'audio or multimodal or capabilities_advertises_plugin_safe_contract or capabilities_requires_auth_when_key_configured or capabilities_advertises_session_key_header'