fix(orchestration): WorkflowDocumenter migrate chat_completion → chat (#7042)

Closes the runtime AttributeError discovered behind PR #7036 (#6983).
After the #3185 LLMInterface retirement, ``WorkflowDocumenter`` retained
its docstring-typed ``llm_interface`` slot and called
``self.llm_interface.chat_completion(...)`` — a method that doesn't
exist on LLMService. Boot succeeded (the slot is ``Optional[Any]``), but
the summary path raised AttributeError silently behind the broad
``except Exception`` guard at the call site, so workflow documentation
always shipped without the ``generated_summary`` field.

Migration applied
-----------------
- Constructor param ``llm_interface`` → ``llm_service`` (also closes
  #7092 item 2). Single external caller is ``orchestrator.py:535``,
  updated in the same commit.
- Stored attribute ``self.llm_interface`` → ``self.llm_service``.
- Method call ``self.llm_interface.chat_completion(...)`` →
  ``self.llm_service.chat(messages=...)`` per the LLMService contract.
- Response read pattern ``summary_result.get("content", "")`` →
  ``response.content`` gated by ``not response.error`` (LLMResponse
  is a Pydantic model with attribute access; the dict-style access
  would have been the same class of bug as #6940).
- Dropped the ``model="default"`` placeholder kwarg — LLMService picks
  its own default when ``model_name`` is omitted; the old
  LLMInterface.chat_completion accepted "default" as a sentinel that
  no longer exists.
- Cleaned up the transient ``# #6983: ...`` comment block at
  orchestrator.py:531-534 (the "tracked separately" follow-up landed
  in this commit; the comment is no longer accurate). Per CLAUDE.md
  style: rationale-only WHY comments, no issue-number prefix.

Verification
------------
Added ``orchestration/workflow_documentation_summary_test.py`` —
7 tests pinning the contract:

  - Constructor param renamed → only ``llm_service`` works
  - Stored attribute renamed → ``self.llm_service`` only
  - **Method-call regression pin**: stub ``llm_service.chat_completion``
    set to ``AssertionError`` side-effect; test fails if a future
    change reintroduces the legacy call
  - No-LLM path early-returns without writing a summary field
  - LLMResponse.error truthy → no partial summary written
  - chat() raises → broad except still swallows + logs warning
  - **Source-level pin**: assert ``.chat_completion(`` is absent
    from the migrated method's source

  $ python3 -m pytest \
      autobot-backend/orchestration/workflow_documentation_summary_test.py -xvs
  ============================== 7 passed in 0.75s ===============================

Verified post-fix:
  $ PYTHONPATH=autobot-backend:. python3 -c "import orchestrator"
  orchestrator OK
  $ grep -n "llm_interface" workflow_documentation.py orchestrator.py
  (empty — all renamed)

Per the [verify-return-shape-in-method-migration](https://github.com/mrveiss/AutoBot-AI/blob/Dev_new_gui/.claude/projects/feedback_verify_return_shape_in_method_migration.md) memory:
- LLMService.chat() returns LLMResponse (verified via inspect.signature)
- LLMResponse.content: str / .error: Optional[str] (verified earlier)
- chat() accepts messages= kwarg (verified)

Closes #7042
Closes #7092 item 2 (WorkflowDocumenter param rename)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: t

autobot-backend/orchestration/workflow_documentation.py

@@ -29,17 +29,17 @@ class WorkflowDocumenter:
     def __init__(
         self,
         knowledge_base: Optional[Any] = None,
-        llm_interface: Optional[Any] = None,
+        llm_service: Optional[Any] = None,
     ):
         """
         Initialize the workflow documenter.
 
         Args:
             knowledge_base: Knowledge base for storing documentation
-            llm_interface: LLM interface for generating summaries
+            llm_service: LLM service used to generate workflow summaries
         """
         self.knowledge_base = knowledge_base
-        self.llm_interface = llm_interface
+        self.llm_service = llm_service
         self._documentation: Dict[str, WorkflowDocumentation] = {}
         self._metrics = {
             "documentation_generated": 0,
@@ -157,7 +157,7 @@ async def _generate_llm_summary(
             workflow_doc: The workflow documentation to update
             execution_result: Results from workflow execution
         """
-        if not self.llm_interface:
+        if not self.llm_service:
             return
 
         try:
@@ -172,15 +172,12 @@ async def _generate_llm_summary(
             Provide a brief summary of what was accomplished and any key insights.
             """
 
-            summary_result = await self.llm_interface.chat_completion(
-                model="default",
+            response = await self.llm_service.chat(
                 messages=[{"role": "user", "content": summary_prompt}],
             )
 
-            if summary_result:
-                workflow_doc.content["generated_summary"] = summary_result.get(
-                    "content", ""
-                )
+            if response and not response.error:
+                workflow_doc.content["generated_summary"] = response.content
 
         except Exception as e:
             logger.warning("Failed to generate workflow summary: %s", e)

autobot-backend/orchestration/workflow_documentation_summary_test.py

@@ -0,0 +1,169 @@
+# AutoBot - AI-Powered Automation Platform
+# Copyright (c) 2025 mrveiss
+# Author: mrveiss
+"""Contract tests for ``WorkflowDocumenter._generate_llm_summary`` (#7042).
+
+After the #3185 LLMInterface retirement, the documenter still called
+``self.llm_interface.chat_completion(...)`` — a method that doesn't exist
+on LLMService (LLMService exposes ``chat()``). The summary path raised
+AttributeError silently behind the broad ``except Exception`` guard at
+the call site, so workflow documentation always shipped without the
+``generated_summary`` field.
+
+These tests pin the migrated shape:
+
+  - Param renamed ``llm_interface`` → ``llm_service``
+  - Method call renamed ``chat_completion`` → ``chat``
+  - Response is read via ``.content`` / ``.error`` (LLMResponse contract)
+"""
+
+from __future__ import annotations
+
+import inspect
+from datetime import datetime, timezone
+from typing import Any
+from unittest.mock import AsyncMock
+
+import pytest
+
+from orchestration.types import DocumentationType, WorkflowDocumentation
+from orchestration.workflow_documentation import WorkflowDocumenter
+
+
+@pytest.fixture
+def make_llm_response():
+    """Return a factory that builds a stub ``LLMResponse``-shaped object."""
+
+    class _StubResponse:
+        def __init__(self, *, content: str = "", error: str | None = None):
+            self.content = content
+            self.error = error
+
+    return _StubResponse
+
+
+def _make_workflow_doc(workflow_id: str = "wf-1") -> WorkflowDocumentation:
+    """Minimal WorkflowDocumentation suitable for the summary path."""
+    now = datetime.now(tz=timezone.utc)
+    return WorkflowDocumentation(
+        workflow_id=workflow_id,
+        title="Test workflow",
+        description="A test workflow for the summary path",
+        created_at=now,
+        updated_at=now,
+        documentation_type=DocumentationType.WORKFLOW_SUMMARY,
+        content={},
+    )
+
+
+# ---------------------------------------------------------------------------
+# Constructor contract — param rename + attribute rename
+# ---------------------------------------------------------------------------
+
+
+def test_constructor_accepts_llm_service_keyword() -> None:
+    """The post-#7042 keyword is ``llm_service`` (renamed from
+    ``llm_interface`` to match LLMService convention)."""
+    sig = inspect.signature(WorkflowDocumenter.__init__)
+    params = list(sig.parameters.keys())
+    assert "llm_service" in params
+    assert "llm_interface" not in params, (
+        "param must be renamed to 'llm_service' — orchestrator.py and any "
+        "future caller depend on the new name"
+    )
+
+
+def test_attribute_renamed_to_llm_service() -> None:
+    """Stored attribute must be ``self.llm_service`` (not the old
+    ``self.llm_interface``)."""
+    documenter = WorkflowDocumenter(llm_service=object())
+    assert hasattr(documenter, "llm_service")
+    assert not hasattr(documenter, "llm_interface")
+
+
+# ---------------------------------------------------------------------------
+# Method-call contract — chat() not chat_completion()
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_summary_calls_llm_service_chat_not_chat_completion(make_llm_response: Any) -> None:
+    """Regression pin for the original #7042 bug: the helper must call
+    ``llm_service.chat(...)``, never the legacy ``chat_completion``.
+    """
+    llm_service = AsyncMock()
+    llm_service.chat = AsyncMock(return_value=make_llm_response(content="Summary text", error=None))
+    # If a future change resurrects the legacy method call, fail loudly.
+    llm_service.chat_completion = AsyncMock(side_effect=AssertionError("legacy method"))
+
+    documenter = WorkflowDocumenter(llm_service=llm_service)
+    workflow_doc = _make_workflow_doc()
+
+    await documenter._generate_llm_summary(workflow_doc, {"status": "completed"})
+
+    llm_service.chat.assert_awaited_once()
+    llm_service.chat_completion.assert_not_awaited()
+    assert workflow_doc.content.get("generated_summary") == "Summary text"
+
+
+@pytest.mark.asyncio
+async def test_summary_skipped_when_no_llm_service(make_llm_response: Any) -> None:
+    """No LLM configured → early-return, no exception, no summary field."""
+    documenter = WorkflowDocumenter(llm_service=None)
+    workflow_doc = _make_workflow_doc()
+
+    await documenter._generate_llm_summary(workflow_doc, {"status": "completed"})
+
+    assert "generated_summary" not in workflow_doc.content
+
+
+@pytest.mark.asyncio
+async def test_summary_skipped_on_llm_response_error(make_llm_response: Any) -> None:
+    """LLMResponse.error truthy → skip (no partial/poisoned summary written)."""
+    llm_service = AsyncMock()
+    llm_service.chat = AsyncMock(return_value=make_llm_response(content="", error="rate limit"))
+
+    documenter = WorkflowDocumenter(llm_service=llm_service)
+    workflow_doc = _make_workflow_doc()
+
+    await documenter._generate_llm_summary(workflow_doc, {"status": "completed"})
+
+    assert "generated_summary" not in workflow_doc.content
+
+
+@pytest.mark.asyncio
+async def test_summary_swallows_exception_and_logs(make_llm_response: Any, caplog) -> None:
+    """The broad except-guard remains — ensures one bad summary call
+    doesn't break the rest of the documentation pipeline.
+    """
+    import logging
+
+    llm_service = AsyncMock()
+    llm_service.chat = AsyncMock(side_effect=RuntimeError("boom"))
+
+    documenter = WorkflowDocumenter(llm_service=llm_service)
+    workflow_doc = _make_workflow_doc()
+
+    with caplog.at_level(logging.WARNING):
+        await documenter._generate_llm_summary(workflow_doc, {"status": "completed"})
+
+    assert "generated_summary" not in workflow_doc.content
+    assert any("Failed to generate workflow summary" in r.message for r in caplog.records)
+
+
+# ---------------------------------------------------------------------------
+# Source-level pin — catches reintroduction of the bug
+# ---------------------------------------------------------------------------
+
+
+def test_source_does_not_call_chat_completion() -> None:
+    """Read the source of the migrated method and assert the legacy
+    method name is gone. Catches future regressions even if no test
+    runtime path covers them.
+    """
+    src = inspect.getsource(WorkflowDocumenter._generate_llm_summary)
+    assert ".chat_completion(" not in src, (
+        "_generate_llm_summary must call llm_service.chat(...), "
+        "not the deleted-from-LLMService chat_completion(...)"
+    )
+    assert ".chat(" in src, "expected a llm_service.chat(...) call"

autobot-backend/orchestrator.py

@@ -528,13 +528,9 @@ async def _process_simple_request(
 
     def _get_enhanced_documenter(self) -> WorkflowDocumenter:
         if not hasattr(self, "_enh_documenter") or self._enh_documenter is None:
-            # #6983: WorkflowDocumenter still names its dep `llm_interface` (legacy slot,
-            # typed Optional[Any]). Pass the LLMService instance — the rename is tracked
-            # separately. WorkflowDocumenter's internal call to .chat_completion() is
-            # broken-on-LLMService (different method name) — see follow-up tracker.
             self._enh_documenter = WorkflowDocumenter(
                 knowledge_base=self.knowledge_base,
-                llm_interface=self.llm_service,
+                llm_service=self.llm_service,
             )
         return self._enh_documenter