fix(agents): AgentInterface declared a coroutine where every caller wants a generator

`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 <noreply@anthropic.com>
This commit is contained in:
2026-08-20 00:24:12 +02:00
co-authored by Claude
parent eabf0f4a11
commit f398a8ad80
2 changed files with 66 additions and 1 deletions
+1 -1
View File
@@ -36,7 +36,7 @@ class AgentInterface(ABC):
"""
@abstractmethod
async def generate_response(
def generate_response(
self,
messages: list[dict],
reasoning: dict | None = None,
+65
View File
@@ -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."
)