test(bindings): remove cancellation test timing races

[jthomson04]

Summary

An already-cancelled request can return a response before its stop signal reaches the worker. The cancellation tests assume that the stop always arrives first, so they can fail under scheduling delays. In this CI failure, the stop reached the worker after 140 ms, while the mock handler waits only 100 ms before its first response.

Use an explicit server cancellation event in all four affected tests. The two pre-cancelled request tests consume early responses and wait for stream termination plus server acknowledgement under one five-second deadline. They accept only the exact expected terminal cancellation error; request errors and other stream errors still propagate. The two mid-stream cancellation tests replace their 200 ms sleeps with bounded event waits, including the metadata case.

Preserve the tests' cancellation order and existing response, metadata, and stopped/not-killed assertions. Add a 30-second module timeout to bound fixture setup and teardown, and remove the redundant event reset. Runtime behavior is unchanged.

Validation

• Full cancellation module: 14 passed.
• Four affected tests, TCP and NATS, repeated 50 times each with isolated services and forked processes: 400 passed. The four groups ran in parallel.
• The module and repeat runs used the module timeout marker without a command-line timeout override.
• Seven temporary helper checks passed: early responses and expected terminal cancellation still require acknowledgement; missing acknowledgement times out; request and unrelated stream errors propagate. The temporary checks and logs are excluded from this PR.
• Black, isort, Ruff, Flake8, and git diff --check passed.

Tests ran outside the sandbox with an existing local ai-dynamo-runtime 1.5.0 wheel built from 99224822bdbbd45a520d2ec1a4c9e38ca967e8e7 with unrelated AISimulate changes. The relevant runtime cancellation and TCP code matches the inspected source.

Related Issues

• Confirmed — no related GitHub issue.

Summary by CodeRabbit

• Tests
  • Improved coverage for request cancellation scenarios, including cancellations that occur before processing begins.
  • Tests now verify that cancellation completes within a defined timeout, improving reliability and reducing timing-related flakiness.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 740add52-8eea-4bca-a42e-96a98a258efd

📥 Commits

Reviewing files that changed from the base of the PR and between 99dae1f and 308811c.

📒 Files selected for processing (1)

• lib/bindings/python/tests/cancellation/test_cancellation.py

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

---

Walkthrough

Changes

The cancellation tests now coordinate through an asyncio.Event. The helper drains asynchronous responses and waits for cancellation completion. Two tests use the helper with a five-second timeout.

Cancellation test synchronization

Layer / File(s)	Summary
Cancellation event signaling lib/bindings/python/tests/cancellation/test_cancellation.py	MockServer.generate clears the event for each generation and sets it when cancellation occurs.
Cancellation completion checks lib/bindings/python/tests/cancellation/test_cancellation.py	_wait_for_cancelled_request replaces fixed delays and response-rejection checks in the affected tests.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 30881

No actionable merge-blocking risk remains for this cancellation-test synchronization change.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the cancellation test changes and the removal of timing races. It is concise and specific to the main change.
Description check	✅ Passed	The description provides a clear summary, implementation details, validation results, and a completed Related Issues section. It does not use the template's exact Overview, Details, or reviewer-start …

• Fix all pre-merge checks with AI

---

_{Comment @coderabbitai help to get the list of available commands.}

[jthomson04]

Automated review of the diff, traced into lib/bindings/python/rust/ to confirm which stream terminations are actually reachable. Eight inline comments above; one finding has no anchorable diff line:

server fixture still synchronizes startup with a fixed sleep (line 180)

await asyncio.sleep(0.5) in the server fixture is the same anti-pattern this PR removes from the tests, and it is both unsound and wasted. On a loaded CI runner serve_endpoint may not have registered within 500 ms -- the fixture yields anyway, and the tests only survive because client.wait_for_instances() (line 199) does the real blocking. On a fast machine it is pure dead time: 0.5 s x 7 tests x 2 transports is ~7 s per run of a module marked unit + pre_merge. Since wait_for_instances() already establishes the invariant, the sleep can simply be deleted.

---

The two highest-value changes, in my view: keep a bounded response-count assertion instead of pass (line 360), and apply the same context_cancelled fix to the two tests at lines 229 and 263 that carry the identical race.

🤖 Generated with Claude Code

[jthomson04]

Updated in 8877f16a8d. Local validation passed: 14 module cases, 400 isolated repeat cases, seven temporary helper checks, and formatting/lint checks. The PR description now covers all four affected tests and records the runtime wheel used for validation.

For the remaining review-level suggestions:

• The startup sleep suggestion is deferred. client.wait_for_instances() already establishes readiness. Removing the shared fixture delay is separate cleanup; the new module timeout bounds registration hangs.
• The CodeRabbit docstring coverage warning is left unchanged. Adding docstrings across the existing test bodies would add scope without improving this cancellation fix.

Each inline thread has a separate disposition. The handler event omission is fixed, while its proposed handler refactor remains deferred.