Conversation
845d94e to
5c4c1ac
Compare
|
@codex review |
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
TonyG-FWE
left a comment
There was a problem hiding this comment.
Fresh Devin shutdown finding on 744b9370c was split by lifecycle evidence:
- Confirmed: an empty realtime-audio candidate could finish its hook after close blocked new work and then be appended as
ChatMessage(content=[""]).test_close_does_not_commit_empty_bounded_realtime_audio_turnin4dfcadf09fails on the untouched reviewed head and also verifies no generation starts and owned audio is cleared. - Not reproducible under valid ownership: the duplicate-provider-transcript half. Server-detected and manually submitted audio turns exit before this callback path. In the close race, provider audio is still unsubmitted; the existing regression verifies
commit_audio_calls == 0, so retaining non-empty externally bounded text prevents data loss rather than duplicating a provider item. - Fix:
a53327677routes every shutdown-bounded local commit through one idempotent helper that rejects empty/whitespace candidates while retaining a non-empty unsubmitted turn exactly once.
Post-fix evidence: the close module passes 9 tests; close + external-input + reply-context + AgentSession pass 189 tests; full Ruff and strict package typing pass.
Fresh Devin follow-up (
|
Final Devin ownership-boundary follow-up (
|
…sue-5408-google-realtime-external-input # Conflicts: # livekit-agents/livekit/agents/voice/audio_recognition.py
|
Integrated upstream The conflict resolution composes upstream's cancellation isolation, Gemini generation timestamps, and Validation: focused conflict/close/telemetry 47 passed; recognition/external-input 171 passed; session/fallback/close 192 passed; Google realtime 121 passed; OpenAI/xAI realtime 58 passed; Ruff and core Python 3.13/Linux typing passed. GitHub Linux unit: 2,299 passed, 5 skipped; Ruff, Python 3.10/3.13 typing, BlockGuard on all three platforms, release gate, and CLA are green. The full local Windows run ended at 4 failed, 1,886 passed, 7 skipped, 11 errors: one exact-upstream scheduling assertion, three missing optional-Rime imports (the Rime source run passed 3/3), two sandbox temp-directory errors, then the known closed-event-loop cascade. |
|
Resolved the two upstream conflicts by merging Validation: focused conflict suites 157 passed; adjacent ownership/session suites 217 passed, 44 deselected; close/fallback/provider suites 268 passed; Rime/PII assertions 7 passed; Ruff format/lint and Linux-platform Python 3.13 mypy passed. GitHub Linux unit: 2,326 passed, 5 skipped, 32 warnings; Python 3.10/3.13 typing, Ruff, BlockGuard on Ubuntu/macOS/Windows, release gate, CLA, and the automatic Devin status all passed. The Windows full-unit run reached 1,913 passed, 7 skipped before the known event-loop/environment cascade (6 failed, 9 errors); the relevant failures reproduced on exact upstream. |
|
Integrated upstream The conflict resolution composes #6962's still-generating-response cancellation with this branch's exact generation/turn ownership. On the untouched branch, upstream's six-scenario regression had 2 failures and a leaked-task teardown error; the resolved module passes 7/7, including an overlapped-response case that protects newer output. Current upstream GitHub validation is green: 2,400 passed and 5 skipped on Linux, with Ruff, Python 3.10/3.13 typing, BlockGuard on Ubuntu/macOS/Windows, release gate, and CLA all passing. @davidzhao, since you reviewed the overlapping realtime change in #6962, a maintainer review when you have bandwidth would be appreciated. |
Fixes #5408
Summary
Realtime sessions using external VAD/STT could let provider audio and finalized STT independently represent the same user turn. This caused duplicate input, dropped turns, incorrect interruption behavior, and lifecycle races.
This PR makes turn ownership explicit through
TurnHandlingOptions.realtime_input_mode:"audio"(default)"text"Text mode applies
on_user_turn_completededits before submitting the finalized message.Design
The default remains backward compatible. Provider-specific types and provider-name checks are not introduced into
AgentActivity.Review guide
Suggested review order:
turn.py,agent_activity.py, andaudio_recognition.py.llm/realtime.pyandrealtime_fallback_adapter.py.realtime_api.py.Validation
The local Windows full-unit run encountered the existing closed-event-loop teardown cascade; GitHub Linux completed the authoritative suite.