fix: land SmartChatAgent (PR #7 merged into the wrong base)

agents/smart_chat.py

@@ -0,0 +1,113 @@
+# agents/smart_chat.py
+"""
+SmartChatAgent
+===============
+The conversational assistant behind POST /assistant/start, /assistant/chat,
+/assistant/end (backend/routes_ai.py).
+
+This did not exist anywhere in the repository. `backend/routes_ai.py` did
+`from app.smart_chat import SmartChatAgent` against a module that was never
+committed -- there is no `app/` package at all in this codebase -- and that
+missing import took the entire FastAPI app down at boot, over these three
+endpoints out of several dozen (see the app-boot fix in the previous PR).
+
+It is not built from nothing. `orchestration/chat_workflow.py`'s
+`ChatManager` already implements the real conversational pipeline this needs:
+multi-turn history, follow-up detection, intent classification with context,
+clarification prompts, routing through the super-graph, and follow-up
+suggestions. `SmartChatAgent` is a thin adapter over it, in the shape
+`routes_ai.py` already expects (`agent.context.session_id`,
+`agent.chat_sync(message)` returning `{"content", "metadata": {...}}`) --
+built to match a real call site, not invented against no spec.
+
+Placed in `agents/`, not `app/`: every other specialized agent already lives
+here, and creating a one-file `app/` package just to preserve a broken import
+path that was never real would be structure for its own sake.
+"""
+
+from __future__ import annotations
+
+from dataclasses import dataclass
+from typing import Any, Dict, List, Optional
+
+from orchestration.chat_workflow import ChatManager
+
+
+@dataclass
+class ChatContext:
+    """Minimal session context. `routes_ai.py` reads `.session_id` off this."""
+
+    session_id: str
+    user_email: Optional[str] = None
+
+
+class SmartChatAgent:
+    """Conversational assistant for one chat session.
+
+    One instance per session (routes_ai.py caches instances in `_chat_agents`
+    keyed by session_id) -- state (conversation history) lives in the
+    `ChatManager` it wraps, scoped to this instance's session.
+    """
+
+    def __init__(self, repo=None, user_email: Optional[str] = None, *, use_llm: bool = True):
+        # `repo` is accepted for interface compatibility with how every other
+        # call site in this codebase constructs agents (TasksAgent(repo),
+        # MeetingAgent(repo), ...). It isn't threaded through further: the
+        # underlying ChatManager / process_user_request pipeline constructs
+        # its own DataRepo() internally, the same pattern every other agent
+        # in this codebase already uses. Passing a different repo instance
+        # here would not currently change what data the chat pipeline reads.
+        self.repo = repo
+
+        # use_llm=False has no caller anywhere in this codebase (routes_ai.py
+        # always passes True) and the underlying ChatManager has no
+        # non-LLM mode to fall back to -- intent classification and response
+        # generation both go through the gateway. Rather than silently
+        # ignoring the flag, an unsupported request for it fails clearly.
+        if not use_llm:
+            raise NotImplementedError(
+                "SmartChatAgent has no non-LLM mode; the underlying chat "
+                "pipeline requires a configured LLM gateway for intent "
+                "classification and response generation."
+            )
+
+        self._manager = ChatManager()
+        session_id = self._manager.start_session(user_email=user_email or "")
+        self.context = ChatContext(session_id=session_id, user_email=user_email)
+
+    def chat_sync(self, message: str) -> Dict[str, Any]:
+        """Process one message and return a response in the shape
+        routes_ai.py expects.
+
+        Synchronous: matches the existing call site
+        (`result = agent.chat_sync(message)`, no `await`). The underlying
+        gateway call is itself synchronous (see governance/litellm_gateway.py),
+        so there is no event loop to bridge here, unlike the Graph API calls
+        in repos/data_repo.py.
+        """
+        result = self._manager.process_message(
+            session_id=self.context.session_id,
+            user_email=self.context.user_email or "",
+            user_input=message,
+        )
+
+        return {
+            "content": result.get("response", "I couldn't process that request."),
+            "metadata": {
+                "intent": result.get("intent", "unknown"),
+                "confidence": result.get("confidence"),
+                "reasoning_trace": result.get("reasoning_trace", []),
+                "agents_invoked": result.get("agents_invoked", []),
+                "is_followup": result.get("is_followup", False),
+                "needs_clarification": result.get("needs_clarification", False),
+                "followup_suggestions": result.get("followup_suggestions", []),
+            },
+        }
+
+    def get_history(self) -> List[Dict[str, Any]]:
+        """Full conversation history for this session."""
+        return self._manager.get_conversation_history(self.context.session_id)
+
+    def end(self) -> None:
+        """End the session -- flushes a summary to episodic memory."""
+        self._manager.end_session(self.context.session_id)

backend/routes_ai.py

@@ -13,39 +13,16 @@
 
 router = APIRouter()
 
-# SmartChatAgent (backing the /assistant/* endpoints below) was never
-# committed to this repo -- `app.smart_chat` does not exist anywhere in the
-# tree. That used to take the entire router down at import time, which took
-# down the whole FastAPI app with it, over three endpoints out of the
-# thirteen in this file.
-#
-# The other ten endpoints (plan_today, nudges, weekly reports, wellness
-# score, email/meeting analysis, EOD reports, burnout check) don't touch
-# SmartChatAgent at all and have no reason to be unavailable because of it.
-# The import is optional now; only /assistant/* is affected, and it fails
-# with a clear 501 rather than either fabricating a chat agent or refusing to
-# boot the app.
-try:
-    from app.smart_chat import SmartChatAgent
-    _SMART_CHAT_AVAILABLE = True
-except ImportError:
-    SmartChatAgent = None  # type: ignore[assignment]
-    _SMART_CHAT_AVAILABLE = False
-
-# Keep a cache of SmartChatAgent instances per user
-_chat_agents: Dict[str, Any] = {}
-
-
-def _require_smart_chat() -> None:
-    if not _SMART_CHAT_AVAILABLE:
-        raise HTTPException(
-            status_code=501,
-            detail=(
-                "The conversational assistant is not available: app.smart_chat "
-                "was never implemented in this codebase. The other endpoints "
-                "in this router are unaffected."
-            ),
-        )
+# SmartChatAgent used to be imported as `from app.smart_chat import
+# SmartChatAgent` -- a package that never existed anywhere in this repo, and
+# that missing import took the entire FastAPI app down at boot. It's now
+# implemented in agents/smart_chat.py (alongside every other specialized
+# agent) as a real adapter over orchestration/chat_workflow.py's already-
+# working ChatManager. See that module's docstring for the full story.
+from agents.smart_chat import SmartChatAgent
+
+# Keep a cache of SmartChatAgent instances per session
+_chat_agents: Dict[str, SmartChatAgent] = {}
 
 
 @router.post('/ai/plan_today')
@@ -92,56 +69,58 @@ async def wellness_score(payload: dict):
 
 @router.post('/assistant/start')
 async def assistant_start(payload: dict):
-    _require_smart_chat()
     user_email = payload.get('user_email')
     if not user_email:
         raise HTTPException(status_code=400, detail='user_email required')
     # Create a new SmartChatAgent for this user/session
     repo = DataRepo()
-    agent = SmartChatAgent(repo, use_llm=True)
+    agent = SmartChatAgent(repo, user_email, use_llm=True)
     session_id = agent.context.session_id
     _chat_agents[session_id] = agent
     return {"session_id": session_id}
 
 
 @router.post('/assistant/chat')
 async def assistant_chat(payload: dict):
-    _require_smart_chat()
     session_id = payload.get('session_id')
     user_email = payload.get('user_email')
     message = payload.get('message')
     if not user_email or not message:
         raise HTTPException(status_code=400, detail='user_email and message required')
-    
+
     # Get or create SmartChatAgent
     if session_id and session_id in _chat_agents:
         agent = _chat_agents[session_id]
     else:
         # Create new agent
         repo = DataRepo()
-        agent = SmartChatAgent(repo, use_llm=True)
+        agent = SmartChatAgent(repo, user_email, use_llm=True)
         session_id = agent.context.session_id
         _chat_agents[session_id] = agent
-    
+
     # Use SmartChatAgent's chat_sync for comprehensive response
     try:
         result = agent.chat_sync(message)
         return {
             "response": result.get("content", "I couldn't process that request."),
             "intent": result.get("metadata", {}).get("intent", "unknown"),
-            "confidence": result.get("metadata", {}).get("confidence", 0),
+            "confidence": result.get("metadata", {}).get("confidence"),
             "reasoning_trace": result.get("metadata", {}).get("reasoning_trace", []),
             "session_id": session_id
         }
     except Exception as e:
+        # A chat UI showing "something went wrong" as a reply is normal,
+        # expected degradation -- unlike fabricating business data, telling
+        # the user their own request failed is honest. What was wrong before
+        # was the hardcoded confidence: 0.8 on an error path, which claimed
+        # 80% confidence in a response that was, by definition, a failure.
         import traceback
         print(f"[ERROR] SmartChatAgent failed: {e}")
         traceback.print_exc()
-        # Fallback to simple response
         return {
             "response": f"I encountered an issue processing your request. Error: {str(e)}",
             "intent": "error",
-            "confidence": 0.8,
+            "confidence": None,
             "session_id": session_id
         }
 
@@ -153,6 +132,7 @@ async def assistant_end(payload: dict):
         raise HTTPException(status_code=400, detail='session_id required')
     # Clean up agent
     if session_id in _chat_agents:
+        _chat_agents[session_id].end()
         del _chat_agents[session_id]
     return {"status": "ended"}
 

orchestration/chat_workflow.py

@@ -125,6 +125,9 @@ def process_message(
             user_input, intent, history
         )
 
+        confidence: Optional[float] = None
+        reasoning_trace: List[Any] = []
+
         if needs_clarification:
             # Return clarification question
             response = clarification
@@ -136,9 +139,14 @@ def process_message(
                 user_email=user_email,
                 session_id=session_id
             )
-            
+
             response = result.get("response", "I'm here to help!")
             agents_invoked = result.get("agents_used", [])
+            # process_user_request already computes these; they were being
+            # discarded here rather than surfaced to callers that need them
+            # (e.g. SmartChatAgent.chat_sync's metadata).
+            confidence = result.get("confidence")
+            reasoning_trace = result.get("reasoning_trace", [])
 
         # Create turn record
         turn = ConversationTurn(
@@ -164,6 +172,8 @@ def process_message(
         return {
             "response": response,
             "intent": intent,
+            "confidence": confidence,
+            "reasoning_trace": reasoning_trace,
             "agents_invoked": agents_invoked,
             "is_followup": is_followup,
             "needs_clarification": needs_clarification,

tests/test_app_boots.py

@@ -5,6 +5,10 @@
 backend/routes_ai.py importing `app.smart_chat.SmartChatAgent`, a module
 that was never committed anywhere in this repository -- took the entire
 FastAPI app down at import time over three endpoints out of several dozen.
+
+SmartChatAgent is real now (agents/smart_chat.py, a follow-up PR) -- the
+assistant endpoints are tested with a fake gateway in test_smart_chat.py,
+not here. This file stays focused on "does the app boot at all."
 """
 
 from __future__ import annotations
@@ -21,23 +25,27 @@ def client():
     # gateway eagerly and this test environment has no LLM configured, which
     # correctly raises LLMNotConfiguredError (see test_gateway.py -- that's
     # the fail-closed behavior working as intended, not a bug). This suite is
-    # about the SEPARATE SmartChatAgent import regression, so a 500 from an
-    # unrelated, expected cause should come back as a normal response to
-    # assert against, not propagate and fail these tests for the wrong reason.
+    # about "does the app boot," so an unrelated 500 should come back as a
+    # normal response to assert against, not propagate and fail these tests
+    # for the wrong reason.
     with TestClient(app_module.app, raise_server_exceptions=False) as c:
         yield c
 
 
 class TestAppImports:
-    def test_app_module_imports_without_smart_chat(self):
+    def test_app_module_imports_without_app_dot_smart_chat(self):
         """The regression this test exists for: importing backend.app must
-        not require app.smart_chat, which does not exist in this repo."""
+        not require the never-existed `app.smart_chat` package."""
         import sys
 
         import backend.app  # noqa: F401
 
         assert "app.smart_chat" not in sys.modules
 
+    def test_smart_chat_agent_is_importable_from_agents(self):
+        """Where it actually lives now."""
+        from agents.smart_chat import SmartChatAgent  # noqa: F401
+
     def test_app_registers_a_substantial_number_of_routes(self):
         """`len(app.routes)` is not a stable thing to assert on: newer FastAPI
         represents each include_router() call as a single lazy
@@ -57,8 +65,8 @@ def test_app_registers_a_substantial_number_of_routes(self):
 
 
 class TestUnaffectedEndpointsStillWork:
-    """The ten /ai/* endpoints that don't touch SmartChatAgent must be
-    completely unaffected by its absence."""
+    """The /ai/* endpoints that don't touch SmartChatAgent must be completely
+    unaffected by whatever state it's in."""
 
     def test_nudges_endpoint_responds(self, client):
         r = client.get("/api/v1/ai/nudges")
@@ -71,23 +79,22 @@ def test_weekly_reports_endpoint_responds(self, client):
         assert r.status_code != 404
 
 
-class TestAssistantEndpointsFailClearly:
-    """The three endpoints that DO need SmartChatAgent must answer 501 --
-    not fabricate a response, not 500, not silently do nothing."""
+class TestAssistantEndpointsAreRegistered:
+    """The real conversational behavior is tested with a fake gateway in
+    test_smart_chat.py. Here: just confirm the routes exist and fail for the
+    RIGHT reason in an unconfigured test environment (no LLM), not a 404."""
 
-    def test_assistant_start_returns_501(self, client):
+    def test_assistant_start_route_exists(self, client):
         r = client.post("/api/v1/assistant/start", json={"user_email": "a@example.com"})
-        assert r.status_code == 501
-        assert "smart_chat" in r.json()["detail"].lower() or "assistant" in r.json()["detail"].lower()
+        assert r.status_code != 404
 
-    def test_assistant_chat_returns_501(self, client):
+    def test_assistant_chat_route_exists(self, client):
         r = client.post(
             "/api/v1/assistant/chat",
             json={"session_id": "x", "user_email": "a@example.com", "message": "hi"},
         )
-        assert r.status_code == 501
+        assert r.status_code != 404
 
-    def test_error_names_the_missing_capability_not_a_generic_failure(self, client):
-        r = client.post("/api/v1/assistant/start", json={"user_email": "a@example.com"})
-        detail = r.json()["detail"]
-        assert "never implemented" in detail or "not available" in detail
+    def test_assistant_end_route_exists(self, client):
+        r = client.post("/api/v1/assistant/end", json={"session_id": "x"})
+        assert r.status_code != 404