Feat/improve clarification check (#57)

* implemented needs_clarification_improved to use LLM inference to determine if clarification is needed.

* added unit tests for needs_clarification_improved.

* minor clean up of docstring and potential await error.

multi_llm_chatbot_backend/app/api/routes/chat.py

@@ -101,7 +101,7 @@ async def chat_stream(
                     "content": message.user_input,
                 })
 
-            if chat_orchestrator.needs_clarification(session, message.user_input):
+            if await chat_orchestrator.needs_clarification_improved(session, message.user_input):
                 clar = await chat_orchestrator.generate_contextual_clarification(message.user_input)
                 yield ChatStreamLine(
                     type="clarification",

multi_llm_chatbot_backend/app/core/improved_orchestrator.py

@@ -79,6 +79,7 @@ class ImprovedChatOrchestrator:
             tool_executor=tool_executor,
         )
 
+    # TODO: Investigate if this method is still needed and remove if not.
     async def process_message(self, 
                             user_input: str, 
                             session_id: Optional[str] = None,
@@ -234,7 +235,86 @@ class ImprovedChatOrchestrator:
 
         logger.info("CLARIFICATION TRIGGERED: short input (%d words) without specific keywords", word_count)
         return True
-    
+
+    async def needs_clarification_improved(self, session: ConversationContext, user_input: str) -> bool:
+        """
+        Use an LLM call to determine whether the user's input is too vague
+        to route to the advisor panel.  Falls back to the legacy rule-based
+        method if the LLM call fails.
+        """
+        user_messages = [msg for msg in session.messages if msg.get('role') == 'user']
+        if len(user_messages) > 1:
+            logger.info("Skipping clarification: session already has %d user message(s)", len(user_messages))
+            return False
+
+        app_cfg = get_settings().app
+        orch_cfg = get_settings().orchestrator
+        advisor_descriptions = ", ".join(
+            f"{p.name} ({p.id})" for p in self.personas.values()
+        )
+        domain_keywords = ", ".join(orch_cfg.specific_keywords)
+
+        system_prompt = (
+            "You are a routing classifier for an AI advisory application.\n\n"
+            f"Application: {app_cfg.title} — {app_cfg.subtitle}\n"
+            f"Available advisors: {advisor_descriptions}\n"
+            f"Domain-relevant topics: {domain_keywords}\n\n"
+            "Your task: decide whether the user's FIRST message contains enough "
+            "substance to send to the advisors, or whether it is too vague and "
+            "requires a clarifying follow-up before the advisors can help.\n\n"
+            "A message NEEDS CLARIFICATION when it:\n"
+            "- Expresses confusion or uncertainty without a concrete topic\n"
+            "- Is a single generic request like 'help' or 'advice'\n"
+            "- Contains no identifiable subject the advisors could address\n\n"
+            "A message is CLEAR ENOUGH when it:\n"
+            "- Mentions a specific topic, question, or problem area\n"
+            "- Provides enough context for at least one advisor to respond usefully\n"
+            "- Even a short message is fine if the intent is unambiguous "
+            "(e.g. 'explain transformers' is clear)\n"
+            "- Messages mentioning domain-relevant topics are likely clear enough "
+            "to route directly, even if brief\n\n"
+            "Respond ONLY with valid JSON:\n"
+            '{"needs_clarification": true or false, "reason": "one sentence explanation"}'
+        )
+
+        user_prompt = f'User message: "{user_input}"'
+
+        raw = None
+
+        try:
+            llm = next(iter(self.personas.values())).llm
+            raw = await llm.generate(
+                system_prompt=system_prompt,
+                context=[{"role": "user", "content": user_prompt}],
+                temperature=0.0,
+                max_tokens=128,
+                response_mime_type="application/json",
+            )
+
+            parsed = json.loads(raw.strip())
+            value = parsed.get("needs_clarification")
+            if not isinstance(value, bool):
+                raise TypeError(
+                    f"needs_clarification must be a boolean, got {type(value).__name__}: {value!r}"
+                )
+            result = value
+            reason = parsed.get("reason", "")
+
+            logger.info(
+                "LLM clarification classification: needs_clarification=%s, reason=%r, input=%r",
+                result, reason, user_input,
+            )
+            return result
+
+        except (json.JSONDecodeError, KeyError, TypeError) as exc:
+            logger.error("Failed to parse LLM classification response: %s (raw=%r)", exc, raw)
+        except Exception as exc:
+            logger.error("LLM classification call failed: %s", exc)
+
+        # TODO: Evaluate if this fallback is still needed and remove if not.
+        logger.warning("Falling back to rule-based clarification check")
+        return self.needs_clarification(session, user_input)
+
     async def generate_contextual_clarification(self, user_input: str) -> Dict[str, Any]:
         """
         Use the LLM to produce a clarification question and clickable

multi_llm_chatbot_backend/app/tests/unit/test_clarification.py

@@ -0,0 +1,336 @@
+import asyncio
+import json
+import unittest
+from unittest.mock import AsyncMock, MagicMock, patch
+
+from app.core.improved_orchestrator import ImprovedChatOrchestrator
+
+
+def _make_session(user_message_count=1):
+    """Build a mock ConversationContext with N user messages."""
+    session = MagicMock()
+    session.messages = [
+        {"role": "user", "content": f"message {i}"}
+        for i in range(user_message_count)
+    ]
+    return session
+
+
+def _make_mock_settings():
+    """Build a mock settings object with the fields needs_clarification_improved reads."""
+    settings = MagicMock()
+    settings.app.title = "Test Advisory Panel"
+    settings.app.subtitle = "AI-Powered Test Guidance"
+    settings.orchestrator.specific_keywords = ["methodology", "theory", "research"]
+    settings.orchestrator.min_words_without_keywords = 6
+    settings.orchestrator.clarification_questions = ["Could you provide more details?"]
+    settings.orchestrator.clarification_suggestions = ["Ask about methodology."]
+    return settings
+
+
+def _make_orchestrator(persona_llm=None):
+    """Build an orchestrator with mocked dependencies, bypassing __init__."""
+    orch = ImprovedChatOrchestrator.__new__(ImprovedChatOrchestrator)
+    orch.llm_client = None
+    orch.session_manager = MagicMock()
+    orch.context_manager = MagicMock()
+
+    if persona_llm is not None:
+        persona = MagicMock()
+        persona.name = "Dr. Test"
+        persona.id = "tester"
+        persona.llm = persona_llm
+        orch.personas = {"tester": persona}
+    else:
+        orch.personas = {}
+
+    return orch
+
+
+@patch("app.core.improved_orchestrator.get_settings")
+class TestNeedsClarificationImproved(unittest.TestCase):
+
+    def _run(self, coro):
+        return asyncio.run(coro)
+
+    # ------------------------------------------------------------------
+    # First-message gate
+    # ------------------------------------------------------------------
+
+    def test_skips_when_session_has_multiple_user_messages(self, mock_settings):
+        mock_settings.return_value = _make_mock_settings()
+        llm = MagicMock()
+        llm.generate = AsyncMock()
+        orch = _make_orchestrator(persona_llm=llm)
+        session = _make_session(user_message_count=3)
+
+        result = self._run(
+            orch.needs_clarification_improved(session, "help")
+        )
+
+        self.assertFalse(result)
+        llm.generate.assert_not_called()
+
+    def test_proceeds_when_session_has_one_user_message(self, mock_settings):
+        mock_settings.return_value = _make_mock_settings()
+        llm = MagicMock()
+        llm.generate = AsyncMock(return_value=json.dumps({
+            "needs_clarification": False,
+            "reason": "Clear.",
+        }))
+        orch = _make_orchestrator(persona_llm=llm)
+        session = _make_session(user_message_count=1)
+
+        self._run(orch.needs_clarification_improved(session, "explain transformers"))
+
+        llm.generate.assert_called_once()
+
+    # ------------------------------------------------------------------
+    # LLM happy path — clear input
+    # ------------------------------------------------------------------
+
+    def test_returns_false_when_llm_says_clear(self, mock_settings):
+        mock_settings.return_value = _make_mock_settings()
+        llm = MagicMock()
+        llm.generate = AsyncMock(return_value=json.dumps({
+            "needs_clarification": False,
+            "reason": "The user asked about a specific topic.",
+        }))
+        orch = _make_orchestrator(persona_llm=llm)
+        session = _make_session(user_message_count=1)
+
+        result = self._run(
+            orch.needs_clarification_improved(session, "explain transformers")
+        )
+
+        self.assertFalse(result)
+
+    # ------------------------------------------------------------------
+    # LLM happy path — vague input
+    # ------------------------------------------------------------------
+
+    def test_returns_true_when_llm_says_vague(self, mock_settings):
+        mock_settings.return_value = _make_mock_settings()
+        llm = MagicMock()
+        llm.generate = AsyncMock(return_value=json.dumps({
+            "needs_clarification": True,
+            "reason": "Single generic word with no topic.",
+        }))
+        orch = _make_orchestrator(persona_llm=llm)
+        session = _make_session(user_message_count=1)
+
+        result = self._run(
+            orch.needs_clarification_improved(session, "help")
+        )
+
+        self.assertTrue(result)
+
+    # ------------------------------------------------------------------
+    # Strict boolean parsing
+    # ------------------------------------------------------------------
+
+    def test_rejects_string_false_and_falls_back(self, mock_settings):
+        """bool("false") is True in Python; ensure string values are rejected."""
+        mock_settings.return_value = _make_mock_settings()
+        llm = MagicMock()
+        llm.generate = AsyncMock(return_value=json.dumps({
+            "needs_clarification": "false",
+            "reason": "Should have been a boolean.",
+        }))
+        orch = _make_orchestrator(persona_llm=llm)
+        orch.needs_clarification = MagicMock(return_value=False)
+        session = _make_session(user_message_count=1)
+
+        result = self._run(
+            orch.needs_clarification_improved(session, "methodology")
+        )
+
+        self.assertFalse(result)
+        orch.needs_clarification.assert_called_once()
+
+    def test_rejects_string_true_and_falls_back(self, mock_settings):
+        mock_settings.return_value = _make_mock_settings()
+        llm = MagicMock()
+        llm.generate = AsyncMock(return_value=json.dumps({
+            "needs_clarification": "true",
+            "reason": "Should have been a boolean.",
+        }))
+        orch = _make_orchestrator(persona_llm=llm)
+        orch.needs_clarification = MagicMock(return_value=True)
+        session = _make_session(user_message_count=1)
+
+        result = self._run(
+            orch.needs_clarification_improved(session, "help")
+        )
+
+        self.assertTrue(result)
+        orch.needs_clarification.assert_called_once()
+
+    def test_rejects_missing_key_and_falls_back(self, mock_settings):
+        """If the LLM omits the key entirely, fall back."""
+        mock_settings.return_value = _make_mock_settings()
+        llm = MagicMock()
+        llm.generate = AsyncMock(return_value=json.dumps({
+            "reason": "Forgot the main field.",
+        }))
+        orch = _make_orchestrator(persona_llm=llm)
+        orch.needs_clarification = MagicMock(return_value=True)
+        session = _make_session(user_message_count=1)
+
+        result = self._run(
+            orch.needs_clarification_improved(session, "help")
+        )
+
+        self.assertTrue(result)
+        orch.needs_clarification.assert_called_once()
+
+    # ------------------------------------------------------------------
+    # Malformed JSON → fallback
+    # ------------------------------------------------------------------
+
+    def test_falls_back_on_malformed_json(self, mock_settings):
+        mock_settings.return_value = _make_mock_settings()
+        llm = MagicMock()
+        llm.generate = AsyncMock(return_value="this is not json at all")
+        orch = _make_orchestrator(persona_llm=llm)
+        orch.needs_clarification = MagicMock(return_value=False)
+        session = _make_session(user_message_count=1)
+
+        result = self._run(
+            orch.needs_clarification_improved(session, "something")
+        )
+
+        self.assertFalse(result)
+        orch.needs_clarification.assert_called_once()
+
+    # ------------------------------------------------------------------
+    # LLM exception → fallback
+    # ------------------------------------------------------------------
+
+    def test_falls_back_on_llm_exception(self, mock_settings):
+        mock_settings.return_value = _make_mock_settings()
+        llm = MagicMock()
+        llm.generate = AsyncMock(side_effect=RuntimeError("connection refused"))
+        orch = _make_orchestrator(persona_llm=llm)
+        orch.needs_clarification = MagicMock(return_value=True)
+        session = _make_session(user_message_count=1)
+
+        result = self._run(
+            orch.needs_clarification_improved(session, "help")
+        )
+
+        self.assertTrue(result)
+        orch.needs_clarification.assert_called_once()
+
+    # ------------------------------------------------------------------
+    # No personas registered → fallback
+    # ------------------------------------------------------------------
+
+    def test_falls_back_when_no_personas_registered(self, mock_settings):
+        mock_settings.return_value = _make_mock_settings()
+        orch = _make_orchestrator(persona_llm=None)
+        orch.needs_clarification = MagicMock(return_value=True)
+        session = _make_session(user_message_count=1)
+
+        result = self._run(
+            orch.needs_clarification_improved(session, "help")
+        )
+
+        self.assertTrue(result)
+        orch.needs_clarification.assert_called_once()
+
+    # ------------------------------------------------------------------
+    # LLM call parameters
+    # ------------------------------------------------------------------
+
+    def test_llm_called_with_json_mode_and_zero_temp(self, mock_settings):
+        mock_settings.return_value = _make_mock_settings()
+        llm = MagicMock()
+        llm.generate = AsyncMock(return_value=json.dumps({
+            "needs_clarification": False,
+            "reason": "Clear.",
+        }))
+        orch = _make_orchestrator(persona_llm=llm)
+        session = _make_session(user_message_count=1)
+
+        self._run(
+            orch.needs_clarification_improved(session, "explain transformers")
+        )
+
+        call_kwargs = llm.generate.call_args.kwargs
+        self.assertEqual(call_kwargs["temperature"], 0.0)
+        self.assertEqual(call_kwargs["max_tokens"], 128)
+        self.assertEqual(call_kwargs["response_mime_type"], "application/json")
+
+    def test_system_prompt_includes_app_context(self, mock_settings):
+        mock_settings.return_value = _make_mock_settings()
+        llm = MagicMock()
+        llm.generate = AsyncMock(return_value=json.dumps({
+            "needs_clarification": False,
+            "reason": "Clear.",
+        }))
+        orch = _make_orchestrator(persona_llm=llm)
+        session = _make_session(user_message_count=1)
+
+        self._run(
+            orch.needs_clarification_improved(session, "explain transformers")
+        )
+
+        system_prompt = llm.generate.call_args.kwargs["system_prompt"]
+        self.assertIn("Test Advisory Panel", system_prompt)
+        self.assertIn("AI-Powered Test Guidance", system_prompt)
+
+    def test_system_prompt_includes_domain_keywords(self, mock_settings):
+        mock_settings.return_value = _make_mock_settings()
+        llm = MagicMock()
+        llm.generate = AsyncMock(return_value=json.dumps({
+            "needs_clarification": False,
+            "reason": "Clear.",
+        }))
+        orch = _make_orchestrator(persona_llm=llm)
+        session = _make_session(user_message_count=1)
+
+        self._run(
+            orch.needs_clarification_improved(session, "explain transformers")
+        )
+
+        system_prompt = llm.generate.call_args.kwargs["system_prompt"]
+        self.assertIn("methodology", system_prompt)
+        self.assertIn("theory", system_prompt)
+        self.assertIn("research", system_prompt)
+
+    def test_system_prompt_includes_advisor_names(self, mock_settings):
+        mock_settings.return_value = _make_mock_settings()
+        llm = MagicMock()
+        llm.generate = AsyncMock(return_value=json.dumps({
+            "needs_clarification": False,
+            "reason": "Clear.",
+        }))
+        orch = _make_orchestrator(persona_llm=llm)
+        session = _make_session(user_message_count=1)
+
+        self._run(
+            orch.needs_clarification_improved(session, "explain transformers")
+        )
+
+        system_prompt = llm.generate.call_args.kwargs["system_prompt"]
+        self.assertIn("Dr. Test (tester)", system_prompt)
+
+    def test_user_input_passed_in_user_prompt(self, mock_settings):
+        mock_settings.return_value = _make_mock_settings()
+        llm = MagicMock()
+        llm.generate = AsyncMock(return_value=json.dumps({
+            "needs_clarification": False,
+            "reason": "Clear.",
+        }))
+        orch = _make_orchestrator(persona_llm=llm)
+        session = _make_session(user_message_count=1)
+
+        self._run(
+            orch.needs_clarification_improved(session, "How do I structure my lit review?")
+        )
+
+        context = llm.generate.call_args.kwargs["context"]
+        self.assertEqual(len(context), 1)
+        self.assertEqual(context[0]["role"], "user")
+        self.assertIn("How do I structure my lit review?", context[0]["content"])