Fix chatbot: session_id validation, init failure guard, blank message check

backend/api/routes/chatbot.py

@@ -5,9 +5,8 @@ from __future__ import annotations
 import logging
 import threading
 
-from pydantic import BaseModel, Field
-from fastapi import APIRouter, Depends, Request
-from fastapi.responses import JSONResponse
+from pydantic import BaseModel, Field, field_validator
+from fastapi import APIRouter, Depends, HTTPException, Request
 from slowapi import Limiter
 from slowapi.util import get_remote_address
 
@@ -21,12 +20,19 @@ router = APIRouter()
 
 class ChatRequest(BaseModel):
     message: str = Field(..., min_length=1, max_length=4000)
-    session_id: str = "default"
+    session_id: str = Field("default", min_length=1, max_length=100, pattern=r"^[a-zA-Z0-9_\-]+$")
+
+    @field_validator("message")
+    @classmethod
+    def message_not_blank(cls, v: str) -> str:
+        if not v.strip():
+            raise ValueError("Message must not be blank")
+        return v.strip()
 
 
 class FeedbackRequest(BaseModel):
-    session_id: str
-    message_id: str
+    session_id: str = Field(..., min_length=1, max_length=100)
+    message_id: str = Field(..., min_length=1, max_length=100)
     rating: str = Field(..., pattern=r"^(up|down|flag)$")
     comment: str = Field("", max_length=2000)
 
@@ -34,16 +40,25 @@ class FeedbackRequest(BaseModel):
 # Thread-safe lazy chatbot init
 _chatbot = None
 _chatbot_lock = threading.Lock()
+_chatbot_init_failed = False
 
 
 def _get_chatbot(hub: DataHub):
-    global _chatbot
+    global _chatbot, _chatbot_init_failed
     if _chatbot is not None:
         return _chatbot
+    if _chatbot_init_failed:
+        return None
     with _chatbot_lock:
-        if _chatbot is None:
+        if _chatbot is not None:
+            return _chatbot
+        try:
             from src.chatbot.vineyard_chatbot import VineyardChatbot
             _chatbot = VineyardChatbot(hub=hub)
+        except Exception as exc:
+            _chatbot_init_failed = True
+            log.error("Chatbot init failed (will not retry): %s", exc)
+            return None
     return _chatbot
 
 
@@ -51,15 +66,21 @@ def _get_chatbot(hub: DataHub):
 @limiter.limit("10/minute")
 async def chat_message(request: Request, req: ChatRequest, hub: DataHub = Depends(get_datahub)):
     bot = _get_chatbot(hub)
-    response = bot.chat(req.message)
-    return {
-        "message": response.message,
-        "confidence": getattr(response, "confidence", None),
-        "sources": getattr(response, "sources", []),
-        "caveats": getattr(response, "caveats", []),
-        "rule_violations": getattr(response, "rule_violations", []),
-        "response_mode": getattr(response, "response_mode", "info"),
-    }
+    if bot is None:
+        raise HTTPException(status_code=503, detail="Chatbot unavailable")
+    try:
+        response = bot.chat(req.message)
+        return {
+            "message": response.message,
+            "confidence": getattr(response, "confidence", None),
+            "sources": getattr(response, "sources", []),
+            "caveats": getattr(response, "caveats", []),
+            "rule_violations": getattr(response, "rule_violations", []),
+            "response_mode": getattr(response, "response_mode", "info"),
+        }
+    except Exception as exc:
+        log.error("Chatbot error: %s", exc, exc_info=True)
+        raise HTTPException(status_code=500, detail="Chatbot processing failed")
 
 
 @router.post("/feedback")
@@ -76,4 +97,4 @@ async def chat_feedback(request: Request, req: FeedbackRequest):
         return {"status": "ok"}
     except Exception as exc:
         log.error("Feedback logging failed: %s", exc)
-        return JSONResponse(status_code=500, content={"error": "Feedback logging failed"})
+        raise HTTPException(status_code=500, detail="Feedback logging failed")