From f398a8ad8072084a5ee4e850bc7120eac4d94372 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Thu, 20 Aug 2026 00:24:12 +0200 Subject: [PATCH] fix(agents): AgentInterface declared a coroutine where every caller wants a generator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `generate_response` was `async def` with a `pass` body and no `yield`. An async function that never yields is a coroutine, so the declared type was Coroutine[..., AsyncGenerator[OutputItem, None]] — something a caller must await before it can be iterated. Nobody awaits it. Both implementations contain yields (TatlockAgent 5, LoremTesterAgent 3), which makes them async generators directly, and both call sites do `async for item in agent.generate_response(...)`. The abstract method's own docstring says "Yields:" and its own example iterates the call without awaiting. Implementations, consumers and prose all agreed; only the declaration dissented. Removing one word fixes it, and it is the declaration that was wrong rather than the four places reporting it. WHY NOTHING CAUGHT THIS. The abstract body is `pass` and nothing calls super().generate_response — verified across the tree — so the wrong declaration has no runtime consequence and cannot fail a test. It was invisible by construction, and it presented as four unrelated errors in four files (two override, two attr-defined), none of which named the cause. Anyone fixing them where they appeared would have annotated the implementations to match the interface and made the real defect permanent. tests/agents/test_agent_interface.py covers it going forward. The load-bearing case is not "the interface is X" or "the implementation is Y" separately — both could drift together and still pass — but that the two AGREE about what kind of callable this is. Mutation-checked: restoring `async` fails 3 of the 5 new tests, the two that still pass being the implementation checks, which are correctly unaffected. Anchor asserted unique before the mutation was written, and the fix asserted back into place afterwards. 95 errors -> 71 across this branch; this commit accounts for 4 of them. Suite 662 passed, 1 failed — that failure is the known LLM-nondeterministic calculator test, which passed on the previous run of this same branch and failed on this one, which is the clearest available evidence that it is unrelated to any of this work. Co-Authored-By: Claude --- src/agents/base.py | 2 +- tests/agents/test_agent_interface.py | 65 ++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 tests/agents/test_agent_interface.py diff --git a/src/agents/base.py b/src/agents/base.py index 38893c0..5a91896 100644 --- a/src/agents/base.py +++ b/src/agents/base.py @@ -36,7 +36,7 @@ class AgentInterface(ABC): """ @abstractmethod - async def generate_response( + def generate_response( self, messages: list[dict], reasoning: dict | None = None, diff --git a/tests/agents/test_agent_interface.py b/tests/agents/test_agent_interface.py new file mode 100644 index 0000000..dff8299 --- /dev/null +++ b/tests/agents/test_agent_interface.py @@ -0,0 +1,65 @@ +""" +The generate_response contract: an async generator, not a coroutine. + +AgentInterface.generate_response was declared `async def` with a `pass` body +and no `yield`, which makes it a coroutine that RETURNS an async generator. +Both implementations do yield, so they are async generators directly, and both +call sites `async for` over the result. The docstring on the abstract method +says "Yields:" and its own example iterates the call — so the implementations, +the consumers and the prose all agreed with each other, and only the +declaration dissented. + +mypy reported it as four separate errors in four files (two `override`, two +`attr-defined`), none of which named the cause. Nothing else caught it: the +abstract body is `pass` and no subclass delegates to it, so the wrong +declaration could never fail at runtime. It was invisible to the test suite by +construction. + +These tests fail if someone restores `async` to the abstract method. +""" + +import inspect + +import pytest + +from src.agents.base import AgentInterface +from src.agents.lorem_tester import LoremTesterAgent +from src.agents.tatlock import TatlockAgent + +IMPLEMENTATIONS = (TatlockAgent, LoremTesterAgent) + + +@pytest.mark.unit +class TestGenerateResponseContract: + """The declared shape of generate_response must match what callers do.""" + + def test_interface_does_not_declare_a_coroutine(self): + """An `async def` with no yield is a coroutine, which callers cannot + `async for` over without awaiting it first. Nobody awaits it.""" + assert not inspect.iscoroutinefunction(AgentInterface.generate_response), ( + "AgentInterface.generate_response is declared `async def` without a " + "`yield`, making it a coroutine returning an AsyncGenerator. Every " + "implementation is an async generator and every call site iterates " + "it directly. Declare it `def ... -> AsyncGenerator[OutputItem, None]`." + ) + + @pytest.mark.parametrize("impl", IMPLEMENTATIONS, ids=lambda c: c.__name__) + def test_implementations_are_async_generator_functions(self, impl): + """Each concrete agent yields, so its call returns an async generator + without being awaited. This is what the call sites depend on.""" + assert inspect.isasyncgenfunction(impl.generate_response), ( + f"{impl.__name__}.generate_response must be an async generator " + "function — callers do `async for item in agent.generate_response(...)`." + ) + + @pytest.mark.parametrize("impl", IMPLEMENTATIONS, ids=lambda c: c.__name__) + def test_implementations_agree_with_the_interface(self, impl): + """The property that actually matters, stated once: interface and + implementation are the same kind of callable. Asserting each side + separately would let both drift together and still pass.""" + assert inspect.iscoroutinefunction( + AgentInterface.generate_response + ) == inspect.iscoroutinefunction(impl.generate_response), ( + f"AgentInterface and {impl.__name__} disagree about whether " + "generate_response is a coroutine. One of them is wrong." + )