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." + )