refactor(tests): extract canonical make_llm_response fixture — 7+ ad-hoc patterns across the codebase #7134

Discovery — 7+ ad-hoc patterns for mocking LLMResponse in tests

While migrating tests across PRs #7101 (chat), #7118 (workflow_documentation), and #7131 (perf benchmarks), a consistent class of duplication surfaced: every test file that needs to fake an LLMResponse from LLMService.chat() invents its own mock pattern.

$ grep -rn "make_llm_response\|class _StubResponse\|MockLLMResponse\|MagicMock(content=" \
    autobot-backend --include="*.py" | grep -v __pycache__ | head -10

Confirmed sites (each uses a different pattern):

7 patterns for the same thing. The contract is uniform — all need:

class _StubLLMResponse:
    content: str
    error: Optional[str]
    model: str = ""
    provider: str = ""

… matching LLMResponse from llm_interface_pkg/models.py:99-112.

Proposal

Extract a single canonical fixture at autobot-backend/tests/fixtures/llm.py (sibling of existing tests/fixtures/mocks.py):

def make_llm_response(
    *, content: str = "", error: str | None = None,
    model: str = "", provider: str = "",
) -> "LLMResponse":
    """Build an LLMResponse-shaped stub for tests.
    
    Returns a real LLMResponse from llm_interface_pkg.models when possible
    (preferred — pins the contract); falls back to MagicMock if importing
    the real class would pull in heavy deps in the test environment.
    """
    try:
        from llm_interface_pkg.models import LLMResponse
        return LLMResponse(content=content, error=error, model=model, provider=provider)
    except ImportError:
        from unittest.mock import MagicMock
        return MagicMock(content=content, error=error, model=model, provider=provider)


@pytest.fixture
def make_llm_response():
    """pytest fixture wrapping the factory above."""
    return make_llm_response  # noqa: F811 — function-as-fixture pattern

Then migrate the 7 sites to import from the canonical location. Existing _MockLLMResponseShim in tests/fixtures/mocks.py is the closest precedent — make it public (drop the underscore), rename to make_llm_response, and have the new fixture file re-export it OR move it.

Acceptance criteria

• Canonical make_llm_response() factory + matching pytest fixture in one file
• All 7 (and any future-discovered) ad-hoc patterns migrated to import from it
• Real LLMResponse used when importable (pins the field contract — catches drift like bug(orchestrator): is_provider_healthy returns tuple, treated as bool — Ollama failure-fast guard never fires (PR #7036 regression) #7069 introduced)
• At least one regression-pin test that asserts the factory's output is isinstance(LLMResponse, ...) when the real class is importable

Why now

• Test fixtures are the spot where field-shape drift bites first (per bug(orchestrator): is_provider_healthy returns tuple, treated as bool — Ollama failure-fast guard never fires (PR #7036 regression) #7069 lesson). Centralizing the mock makes the next field rename a one-place fix.
• Three of the 7 sites I authored myself across PRs fix(api/chat): wire chat endpoint through LLMService.chat() — closes silent canned-response regression (#7047) #7101/fix(orchestration): WorkflowDocumenter migrate chat_completion → chat (#7042) #7118/fix(test/perf): migrate performance_benchmarks.performance_test.py off LLMInterface (#7041) #7131 — each time I caught myself reinventing the wheel. The third repeat is the one that justifies extraction.
• Cost is low: ~50 LOC for the helper, ~5 lines per migrated callsite.

Found via

PR #7131 (#7041) audit sweep.

Related

• discovery(llm): add static check preventing dict-style access on LLMResponse — 7 production files had latent bugs #6940 — pre-commit hook blocking dict-style access on LLMResponse (same drift class)
• bug(orchestrator): is_provider_healthy returns tuple, treated as bool — Ollama failure-fast guard never fires (PR #7036 regression) #7069 — tuple-vs-bool regression that contract-real-LLMResponse fixtures would catch faster than MagicMock