Skip to content

TEST: Coordinate realtime callback delivery explicitly - #3453

Merged
Roman Lutz (romanlutz) merged 3 commits into
microsoft:mainfrom
romanlutz:romanlutz-realtime-callback-coordination
Oct 24, 2026
Merged

Roman Lutz (romanlutz) merged 3 commits into
microsoft:mainfrom
romanlutz:romanlutz-realtime-callback-coordination

Conversation

@romanlutz

Copy link
Copy Markdown
Contributor

Description

Replace fixed sleeps and callback-delivery polling in realtime tests with explicit delivery futures and dispatcher completion signals, using the existing test watchdog helper.

The concurrency regression now proves that both callbacks enter while the release gate is closed and neither has finished. It then releases and awaits both callbacks. Failure cleanup releases the gate and stops the dispatcher so owned tasks do not leak. Legitimate scheduling yields and actual deadline coverage are unchanged. No production behavior changes.

Related reliability work in #3282 and #3443 changes other tests and shared helpers, not these realtime tests. Checked current main and actual file lists of all 138 PRs updated since October 19 for duplication before opening this PR. Merged main at ddaa6af996ac6fc1debf2828d0eb050ea509b7b8 without conflicts; shared async helpers are unchanged by this branch.

Tests and Documentation

Validation after merging main:

  • uv run pytest tests\unit\prompt_target\target\test_realtime_audio.py tests\unit\prompt_target\target\test_realtime_target.py -q --disable-warnings --maxfail=1: 117 passed in 31.55s.
  • uv run ruff check tests\unit\prompt_target\target\test_realtime_audio.py tests\unit\prompt_target\target\test_realtime_target.py: passed.
  • uv run ruff format --check tests\unit\prompt_target\target\test_realtime_audio.py tests\unit\prompt_target\target\test_realtime_target.py: passed, both files already formatted.
  • uv run ty check tests\unit\prompt_target\target\test_realtime_audio.py tests\unit\prompt_target\target\test_realtime_target.py: passed.
  • git diff origin/main...HEAD --check: passed.

An earlier in-memory mutation experiment serialized callbacks behind a lock: the concurrency test rejected serialization with the expected watchdog failure, and cleanup left both owned tasks done with no pending tasks. An earlier three-repeat run of both realtime modules passed all 351 cases.

Full repository tests, live-provider integration, and JupyText were not run. Documentation changes are not applicable to this test-only change.

Replace fixed sleeps and polling with callback and dispatcher completion signals. Prove callbacks overlap before release and always clean up owned tasks.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 4d7642e0-787d-45bf-9615-a83fce946f63
@richlundeen

Copy link
Copy Markdown
Contributor

Review authored by GitHub Copilot, posted on my behalf.

The new callback gates preserve the concurrency check and remove fixed sleeps. To complete the realtime reliability work, please address these related gaps in this PR. These are existing gaps, not regressions introduced here:

  • tests/unit/prompt_target/target/test_realtime_audio.py:131-132,170-171: the adjacent delivery and router-error tests still use one-second correctness deadlines and skip stop_async() if the wait fails. Use the shared watchdog and stop the dispatcher in finally, as the changed tests do.
  • tests/unit/prompt_target/target/test_openai_realtime_streaming_session.py:440-467,657-668: replace the fixed counts of sleep(0) with readiness signals. Signal failure-callback registration from the mock, and signal chunk delivery from _push_audio_chunk_async (the producer fills _pending_chunks before that call). Inject the failure or commit only after the matching signal. In the shared _run_session_with_events helper at lines 147-167, retain both tasks, set finish in finally, and cancel/join unfinished tasks: gather() alone does not stop the consumer when the event driver raises.
  • tests/unit/prompt_target/target/test_realtime_target.py:1704-1726: the reset-cancellation test still has an unbounded close_started.wait() and no failure-safe release/join. Use the same watchdog and release the close gate in finally.

This keeps the existing behavior checks, adds no success-path delay, and applies the same readiness and cleanup rules across the related realtime tests.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved. The callback coordination changes are sound. The adjacent-test scope comments are non-blocking follow-up work.

Review prepared by GitHub Copilot and submitted at my request.

Use shared watchdogs and failure-safe dispatcher shutdown. Coordinate streaming failure registration and chunk delivery explicitly, release cancellation gates in finally, and retain and drain session helper tasks on errors.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 4d7642e0-787d-45bf-9615-a83fce946f63
@romanlutz

Copy link
Copy Markdown
Contributor Author

Addressed all three follow-up items in 4aaf363:

  • The adjacent delivery and router-error tests now use the shared watchdog and stop the dispatcher in finally.
  • Streaming tests now wait for failure-callback registration and chunk-delivery signals before injecting failures or commits. The shared session helper retains both tasks, releases finish in finally, and cancels/joins unfinished tasks. Two regression cases cover consumer and event-driver errors and verify that no tasks remain running.
  • The reset-cancellation test observes close readiness alongside the cleanup task, uses the watchdog, and releases the close gate and joins cleanup in finally. The deliberate cancellation scheduling yield remains unchanged.

Validation: uv run pytest tests\unit\prompt_target\target\test_realtime_audio.py tests\unit\prompt_target\target\test_realtime_target.py tests\unit\prompt_target\target\test_openai_realtime_streaming_session.py -q --disable-warnings --maxfail=1 passed all 152 tests. Repeating all three modules twice in one run with --keep-duplicates passed all 304 tests. Ruff, formatting, type checks, diff checks, and commit hooks passed. Full-suite and live-provider tests were not run.

@romanlutz
Roman Lutz (romanlutz) added this pull request to the merge queue Oct 24, 2026
Merged via the queue into microsoft:main with commit 96de757 Oct 24, 2026
61 checks passed
@romanlutz
Roman Lutz (romanlutz) deleted the romanlutz-realtime-callback-coordination branch October 24, 2026 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants