From 1fe3bef6630b04f14b347e5d5ebb317e0c5be71d Mon Sep 17 00:00:00 2001 From: James Veitch <1722315+darth-veitcher@users.noreply.github.com> Date: Sun, 12 Jul 2026 09:39:55 +0100 Subject: [PATCH] docs(tests): stop overclaiming what the _run_async regression tests prove MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit beacon-reviewer caught it: reconstructing the old buggy _run_async and running it against test_run_async_works_when_called_from_within_a_running_loop and test_run_async_propagates_exceptions_from_within_a_running_loop shows both pass against the old code too. asyncio.get_running_loop() never spuriously raises under plain CPython/pytest, so the old conditional also takes the safe worker-thread branch there — the actual reported bug is not reproducible outside real ComfyUI's execution engine, and only the live end-to-end run against it proved the fix. Corrects the block comment to say what the tests actually do: lock in the documented contract so a regression to a no-thread asyncio.run(coro) call is caught immediately, without claiming they reproduce the specific environment-dependent failure. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_0132ojafeazQ3ephcBejEWFj --- tests/test_ollama_provider.py | 36 ++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/tests/test_ollama_provider.py b/tests/test_ollama_provider.py index e97e84f..f14ee46 100644 --- a/tests/test_ollama_provider.py +++ b/tests/test_ollama_provider.py @@ -331,19 +331,29 @@ def test_chat_structured_caches_after_successful_validation(monkeypatch): # --------------------------------------------------------------------------- -# _run_async — live-verified critical bug (found running the real -# docker-compose ComfyUI harness, not caught by any prior test): ComfyUI's -# actual async execution engine runs node functions synchronously *inside* -# an already-running event loop. The old implementation tried -# `asyncio.get_running_loop()` first and only spun up an isolated worker -# thread if that succeeded — under real ComfyUI (Python 3.13), that check -# sometimes raised anyway, falling through to a direct `asyncio.run(coro)` -# call on the *current* thread — the one thread guaranteed to already have -# a loop running — reproducing "asyncio.run() cannot be called from a -# running event loop" exactly, every time chat_structured() was called. -# Every existing test called _run_async from a plain synchronous pytest -# function (no ambient loop), which never exercised this path — pytest -# alone could not have caught this. +# _run_async — regression coverage for a bug found running the real +# docker-compose ComfyUI harness (not caught by any prior test, and not +# reproducible under pytest — see below). ComfyUI's actual async execution +# engine runs node functions synchronously *inside* an already-running event +# loop. The old implementation tried `asyncio.get_running_loop()` first and +# only spun up an isolated worker thread if that succeeded — under real +# ComfyUI (Python 3.13), that check sometimes raised anyway, falling through +# to a direct `asyncio.run(coro)` call on the *current* thread — the one +# thread guaranteed to already have a loop running — reproducing "asyncio.run() +# cannot be called from a running event loop" exactly, every time +# chat_structured() was called. +# +# Honest caveat (raised by beacon-reviewer, confirmed by reconstructing the +# old code and running it against these tests): under CPython/pytest, +# asyncio.get_running_loop() inside asyncio.run(outer()) never spuriously +# raises, so the *old, buggy* implementation also takes the safe +# worker-thread branch here and these tests pass against it too. They do not +# reproduce the actual reported bug — only the live end-to-end run against +# real ComfyUI did that. What these tests do lock in: the documented +# contract ("calling _run_async from within a running loop must work and +# must propagate exceptions"), so a regression to a naive +# `asyncio.run(coro)`-with-no-thread implementation (which *does* fail here) +# gets caught immediately. # ---------------------------------------------------------------------------