fix(llm): retry structured calls that return no result

src/skillspector/llm_analyzer_base.py

@@ -122,6 +122,19 @@ class _StructuredResponseValidationError(Exception):
     """Signal that provider output failed structured-response validation."""
 
 
+def _require_structured_response(response: object) -> None:
+    """Reject a structured-output call that produced no result.
+
+    LangChain's tool-call parsers return ``None`` when the reply contains no
+    tool call, for example a forced tool call answered in prose. Treat that as
+    a malformed structured response so it gets the same bounded retries,
+    instead of passing ``None`` to ``parse_response``, which expects a parsed
+    schema object.
+    """
+    if response is None:
+        raise StructuredOutputParseError("model returned no structured response")
+
+
 class LLMRuntimeLimitError(RuntimeError):
     """Signal that no shared scan time remains for an LLM operation."""
 
@@ -1156,6 +1169,7 @@ def _invoke_batch(self, batch: Batch, prompt: str) -> tuple[Batch, list]:
         if structured_llm:
             try:
                 response = _invoke_with_usage(structured_llm, prompt, self._usage_collector)
+                _require_structured_response(response)
             except (StructuredOutputParseError, ValidationError) as exc:
                 raise _StructuredResponseValidationError from exc
         else:
@@ -1245,6 +1259,7 @@ async def _ainvoke_batch(self, batch: Batch, prompt: str) -> tuple[Batch, list]:
         if structured_llm:
             try:
                 response = await _ainvoke_with_usage(structured_llm, prompt, self._usage_collector)
+                _require_structured_response(response)
             except (StructuredOutputParseError, ValidationError) as exc:
                 raise _StructuredResponseValidationError from exc
         else:
@@ -1417,8 +1432,9 @@ async def arun_batches(
         retries so each coordinator retry can re-check and cap its delay
         against remaining time.
         Unrecovered errors cost only their own batch and are omitted from the result.
-        Malformed structured responses (Pydantic ``ValidationError`` or CLI
-        JSON parse failures) receive three bounded exponential-backoff retries
+        Malformed structured responses (Pydantic ``ValidationError``, CLI
+        JSON parse failures, or no structured result at all) receive three
+        bounded exponential-backoff retries
         and are then isolated to their batch. A batch makes at most seven outer
         chat-model invocations even when both
         retry policies apply; native provider retries can make additional HTTP

tests/nodes/test_llm_analyzer_base.py

@@ -1279,6 +1279,50 @@ def test_structured_validation_error_isolated_after_three_retries(
             ((2.0,), {}),
         ]
 
+    @patch(MOCK_PATCH_TARGET, _mock_get_chat_model)
+    @patch("skillspector.llm_analyzer_base.time.sleep")
+    def test_missing_structured_response_recovers_on_retry(self, sleep: MagicMock) -> None:
+        """A tool-call parser returns None when the reply has no tool call."""
+        analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL)
+        analyzer._structured_llm.invoke = MagicMock(
+            side_effect=[None, LLMAnalysisResult(findings=[])]
+        )
+
+        outcome = analyzer.run_batches_detailed([Batch(file_path="a.py", content="code")])
+
+        assert [item[0].file_path for item in outcome.successful] == ["a.py"]
+        assert outcome.failures == []
+        assert analyzer._structured_llm.invoke.call_count == 2
+        sleep.assert_called_once_with(0.5)
+
+    @patch(MOCK_PATCH_TARGET, _mock_get_chat_model)
+    @patch("skillspector.llm_analyzer_base.time.sleep")
+    def test_missing_structured_response_isolated_after_three_retries(
+        self, sleep: MagicMock
+    ) -> None:
+        analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL)
+        analyzer._structured_llm.invoke = MagicMock(
+            side_effect=[None, None, None, None, LLMAnalysisResult(findings=[])]
+        )
+        batches = [
+            Batch(file_path="missing.py", content="no tool call"),
+            Batch(file_path="clean.py", content="clean response"),
+        ]
+
+        outcome = analyzer.run_batches_detailed(batches)
+
+        assert [item[0].file_path for item in outcome.successful] == ["clean.py"]
+        assert [
+            (failure.batch.file_path, failure.error_class, failure.reason)
+            for failure in outcome.failures
+        ] == [("missing.py", "ValidationError", LedgerReason.LLM_STRUCTURED_RESPONSE_INVALID)]
+        assert analyzer._structured_llm.invoke.call_count == 5
+        assert sleep.call_args_list == [
+            ((0.5,), {}),
+            ((1.0,), {}),
+            ((2.0,), {}),
+        ]
+
     @patch(MOCK_PATCH_TARGET, _mock_get_chat_model)
     @patch("skillspector.llm_analyzer_base.time.sleep")
     def test_api_connection_error_recovers_with_bounded_backoff(self, sleep: MagicMock) -> None:
@@ -1986,6 +2030,50 @@ async def test_structured_validation_error_isolated_after_three_retries(
             ((2.0,), {}),
         ]
 
+    @patch(MOCK_PATCH_TARGET, _mock_get_chat_model)
+    @patch("skillspector.llm_analyzer_base.asyncio.sleep", new_callable=AsyncMock)
+    async def test_missing_structured_response_recovers_on_retry(self, sleep: AsyncMock) -> None:
+        """A tool-call parser returns None when the reply has no tool call."""
+        analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL)
+        analyzer._structured_llm.ainvoke = AsyncMock(
+            side_effect=[None, LLMAnalysisResult(findings=[])]
+        )
+
+        outcome = await analyzer.arun_batches_detailed([Batch(file_path="a.py", content="code")])
+
+        assert [item[0].file_path for item in outcome.successful] == ["a.py"]
+        assert outcome.failures == []
+        assert analyzer._structured_llm.ainvoke.call_count == 2
+        sleep.assert_awaited_once_with(0.5)
+
+    @patch(MOCK_PATCH_TARGET, _mock_get_chat_model)
+    @patch("skillspector.llm_analyzer_base.asyncio.sleep", new_callable=AsyncMock)
+    async def test_missing_structured_response_isolated_after_three_retries(
+        self, sleep: AsyncMock
+    ) -> None:
+        analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL)
+        analyzer._structured_llm.ainvoke = AsyncMock(
+            side_effect=[None, None, None, None, LLMAnalysisResult(findings=[])]
+        )
+        batches = [
+            Batch(file_path="missing.py", content="no tool call"),
+            Batch(file_path="clean.py", content="clean response"),
+        ]
+
+        outcome = await analyzer.arun_batches_detailed(batches, max_concurrency=1)
+
+        assert [item[0].file_path for item in outcome.successful] == ["clean.py"]
+        assert [
+            (failure.batch.file_path, failure.error_class, failure.reason)
+            for failure in outcome.failures
+        ] == [("missing.py", "ValidationError", LedgerReason.LLM_STRUCTURED_RESPONSE_INVALID)]
+        assert analyzer._structured_llm.ainvoke.call_count == 5
+        assert sleep.await_args_list == [
+            ((0.5,), {}),
+            ((1.0,), {}),
+            ((2.0,), {}),
+        ]
+
     @patch(MOCK_PATCH_TARGET, _mock_get_chat_model)
     @patch("skillspector.llm_analyzer_base.asyncio.sleep", new_callable=AsyncMock)
     async def test_api_connection_error_recovers_with_bounded_backoff(

tests/test_mcp_tool_poisoning.py

@@ -29,7 +29,7 @@
 import yaml
 from pydantic import BaseModel
 
-from skillspector.inspection_ledger import LedgerOutcome, LedgerReason
+from skillspector.inspection_ledger import LedgerOutcome, LedgerReason, finalize_ledger
 from skillspector.llm_analyzer_base import LLMAnalyzerBase
 from skillspector.llm_utils import AgentCLIChatModel
 from skillspector.nodes.analyzers import mcp_tool_poisoning
@@ -1836,14 +1836,98 @@ def test_wrapped_json_schema_response_stays_incomplete(
         )
 
     def test_tool_refusal_stays_incomplete(self, monkeypatch: pytest.MonkeyPatch) -> None:
+        """A structured call that keeps returning None ends degraded, not fatal (#843)."""
         state = _make_state("mcp_mismatched_skill", use_llm=True)
-        structured_llm = _mock_tp4_structured_llm(monkeypatch, [None])
+        monkeypatch.setattr(LLMAnalyzerBase, "_asleep_before_retry", AsyncMock())
+        structured_llm = _mock_tp4_structured_llm(monkeypatch, [None, None, None, None])
 
         result = node(state)
 
-        assert structured_llm.calls == 1
-        assert result["analyzer_status_events"][0]["status"] == "failed"
+        assert structured_llm.calls == 4
+        assert result["analyzer_status_events"][0]["status"] == "degraded"
+        semantic = [event for event in result["inspection_ledger"] if event["phase"] == "semantic"]
+        assert [
+            (event["outcome"], event["reason_code"], event["error_class"]) for event in semantic
+        ] == [
+            (
+                LedgerOutcome.SKIPPED,
+                LedgerReason.LLM_STRUCTURED_RESPONSE_INVALID,
+                "ValidationError",
+            )
+        ]
+        assert result["llm_call_log"][0]["ok"] is False
+        completeness, _ = finalize_ledger(
+            {
+                "components": state["components"],
+                "findings": result["findings"],
+                "inspection_ledger": result["inspection_ledger"],
+                "analyzer_status_events": result["analyzer_status_events"],
+            }
+        )
+        assert completeness["execution_successful"] is True
+        assert completeness["is_complete"] is False
+
+    def test_missing_structured_response_is_retried(self, monkeypatch: pytest.MonkeyPatch):
+        state = _make_state("mcp_mismatched_skill", use_llm=True)
+        sleep = AsyncMock()
+        monkeypatch.setattr(LLMAnalyzerBase, "_asleep_before_retry", sleep)
+        structured_llm = _mock_tp4_structured_llm(monkeypatch, [None, {"is_mismatch": False}])
+
+        result = node(state)
+
+        assert structured_llm.calls == 2
+        sleep.assert_called_once_with(0.5)
+        assert result["llm_call_log"] == [{"node": "mcp_tool_poisoning", "ok": True, "error": None}]
+        assert result["analyzer_status_events"][0]["status"] == "completed"
+
+    def test_bedrock_forced_tool_call_answered_in_prose_is_retried(
+        self, monkeypatch: pytest.MonkeyPatch
+    ) -> None:
+        """Bedrock's tool parser returns None for a reply with no tool call (#843)."""
+        monkeypatch.delenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", raising=False)
+        from langchain_aws import ChatBedrockConverse
+        from langchain_core.messages import AIMessage
+        from langchain_core.outputs import ChatGeneration, ChatResult
+
+        from skillspector import llm_utils
+        from skillspector.providers.bedrock import (
+            BEDROCK_DEFAULT_MODEL,
+            BEDROCK_DEFAULT_REGION,
+            BedrockProvider,
+        )
+
+        assert BedrockProvider().forced_tool_choice_supported(BEDROCK_DEFAULT_MODEL)
+        calls = []
+
+        async def answer_in_prose(*_args, **_kwargs):
+            calls.append(1)
+            return ChatResult(generations=[ChatGeneration(message=AIMessage(content="refused"))])
+
+        monkeypatch.setattr(ChatBedrockConverse, "_agenerate", answer_in_prose)
+        monkeypatch.setattr(llm_utils, "get_active_provider", BedrockProvider)
+        monkeypatch.setattr(
+            "skillspector.llm_analyzer_base.get_chat_model",
+            lambda **kwargs: ChatBedrockConverse(
+                model=kwargs["model"],
+                client=MagicMock(),
+                bedrock_client=MagicMock(),
+                region_name=BEDROCK_DEFAULT_REGION,
+            ),
+        )
+        monkeypatch.setattr(LLMAnalyzerBase, "_asleep_before_retry", AsyncMock())
+        state = _make_state("mcp_mismatched_skill", use_llm=True)
+        state["model_config"] = {"default": BEDROCK_DEFAULT_MODEL}
+
+        result = node(state)
+
+        assert len(calls) == 4
+        assert result["analyzer_status_events"][0]["status"] == "degraded"
         assert any(
+            event.get("reason_code") is LedgerReason.LLM_STRUCTURED_RESPONSE_INVALID
+            and event.get("error_class") == "ValidationError"
+            for event in result["inspection_ledger"]
+        )
+        assert not any(
             event["outcome"] is LedgerOutcome.FAILED for event in result["inspection_ledger"]
         )