From ba00ac92f6ac853fff3e0c6abb651f4ed4008ea0 Mon Sep 17 00:00:00 2001 From: kigland Date: Tue, 7 Jul 2026 10:42:29 +0800 Subject: [PATCH 1/3] isolate malformed LLM batch responses Signed-off-by: kigland --- src/skillspector/llm_analyzer_base.py | 5 ++- tests/nodes/test_llm_analyzer_base.py | 44 +++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/skillspector/llm_analyzer_base.py b/src/skillspector/llm_analyzer_base.py index 4ad6c558..8edfbf19 100644 --- a/src/skillspector/llm_analyzer_base.py +++ b/src/skillspector/llm_analyzer_base.py @@ -34,7 +34,7 @@ from typing import Any, Literal, cast from langchain_core.messages import BaseMessage -from pydantic import BaseModel, Field, field_validator +from pydantic import BaseModel, Field, ValidationError, field_validator from skillspector.inspection_ledger import ( AnalyzerStatusEvent, @@ -567,6 +567,9 @@ def run_batches_detailed( response = _message_text(self._llm.invoke(prompt)) logger.debug("LLM response for %s", batch.file_label) outcome.successful.append((batch, self.parse_response(response, batch))) + except ValidationError as exc: + logger.warning("LLM batch failed for %s: %s", batch.file_label, exc) + outcome.failures.append(BatchFailure(batch=batch, error_class=type(exc).__name__)) except (ValueError, NotImplementedError): raise except Exception as exc: diff --git a/tests/nodes/test_llm_analyzer_base.py b/tests/nodes/test_llm_analyzer_base.py index 1d9b6a23..4e574e5b 100644 --- a/tests/nodes/test_llm_analyzer_base.py +++ b/tests/nodes/test_llm_analyzer_base.py @@ -416,6 +416,50 @@ async def test_arun_batches_uses_message_text_for_content_blocks(self) -> None: assert results[0][1] == ["async chunk"] +# --------------------------------------------------------------------------- +# LLMAnalyzerBase.run_batches (sync execution) +# --------------------------------------------------------------------------- + + +class TestRunBatches: + MODEL = "nvidia/openai/gpt-oss-120b" + + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) + def test_malformed_structured_batch_does_not_abort_the_others(self) -> None: + """A malformed structured response costs only its own batch.""" + + def _invoke(prompt: str) -> LLMAnalysisResult: + if "b.py" in prompt: + return LLMAnalysisResult.model_validate({"findings": 'We{"findings":[]}'}) + return LLMAnalysisResult( + findings=[ + LLMFinding(rule_id="T-1", message="hit", severity="LOW", start_line=1), + ] + ) + + analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) + analyzer._structured_llm.invoke.side_effect = _invoke + + batches = [ + Batch(file_path="a.py", content="code a"), + Batch(file_path="b.py", content="code b"), + Batch(file_path="c.py", content="code c"), + ] + results = analyzer.run_batches(batches) + + assert {batch.file_path for batch, _ in results} == {"a.py", "c.py"} + assert [items[0].rule_id for _, items in results] == ["T-1", "T-1"] + + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) + def test_value_error_still_propagates(self) -> None: + """ValueError signals misconfiguration, not a malformed model response.""" + analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) + analyzer._structured_llm.invoke.side_effect = ValueError("no API key") + + with pytest.raises(ValueError, match="no API key"): + analyzer.run_batches([Batch(file_path="a.py", content="code")]) + + # --------------------------------------------------------------------------- # LLMAnalyzerBase.arun_batches (async parallel execution) # --------------------------------------------------------------------------- From 3e85882f018c00702949ba8b93c753504dce363b Mon Sep 17 00:00:00 2001 From: kigland Date: Thu, 16 Jul 2026 10:36:26 +0800 Subject: [PATCH 2/3] isolate malformed async LLM batches Signed-off-by: kigland --- src/skillspector/llm_analyzer_base.py | 6 +++++ .../test_semantic_security_discovery.py | 9 ++++--- tests/nodes/test_llm_analyzer_base.py | 26 +++++++++++++++++++ tests/nodes/test_semantic_quality_policy.py | 9 +++++-- 4 files changed, 45 insertions(+), 5 deletions(-) diff --git a/src/skillspector/llm_analyzer_base.py b/src/skillspector/llm_analyzer_base.py index 8edfbf19..2d55e2ab 100644 --- a/src/skillspector/llm_analyzer_base.py +++ b/src/skillspector/llm_analyzer_base.py @@ -642,6 +642,12 @@ async def _process(batch: Batch) -> tuple[Batch, list]: results = await asyncio.gather(*[_process(b) for b in batches], return_exceptions=True) outcome = BatchExecutionResult() for batch, result in zip(batches, results, strict=True): + if isinstance(result, ValidationError): + logger.warning("LLM batch failed for %s: %s", batch.file_label, result) + outcome.failures.append( + BatchFailure(batch=batch, error_class=type(result).__name__) + ) + continue if isinstance(result, (ValueError, NotImplementedError)): raise result if isinstance(result, BaseException): diff --git a/tests/nodes/analyzers/test_semantic_security_discovery.py b/tests/nodes/analyzers/test_semantic_security_discovery.py index 4ce12117..dba29db3 100644 --- a/tests/nodes/analyzers/test_semantic_security_discovery.py +++ b/tests/nodes/analyzers/test_semantic_security_discovery.py @@ -23,7 +23,7 @@ import pytest from pydantic import ValidationError -from skillspector.llm_analyzer_base import LLMAnalysisResult, LLMFinding +from skillspector.llm_analyzer_base import Batch, LLMAnalysisResult, LLMFinding from skillspector.models import Finding from skillspector.nodes.analyzers.semantic_security_discovery import ( ANALYZER_ID, @@ -388,7 +388,11 @@ class TestLLMCallTelemetry: def test_success_records_ok_true(self, base_state) -> None: from skillspector.llm_analyzer_base import LLMAnalyzerBase - with patch.object(LLMAnalyzerBase, "run_batches", return_value=[]): + with patch.object( + LLMAnalyzerBase, + "run_batches", + return_value=[(Batch(file_path="SKILL.md", content="# Skill"), [])], + ): result = node(base_state) assert result["llm_call_log"] == [{"node": ANALYZER_ID, "ok": True, "error": None}] @@ -446,7 +450,6 @@ def _build_file_cache(skill_dir: Path) -> dict[str, str]: def _make_file_aware_run_batches(responses: dict[str, LLMAnalysisResult]): """Return a mock run_batches that dispatches based on file_path in each batch.""" - from skillspector.llm_analyzer_base import Batch def _run_batches(self_inner, batches: list[Batch], **_kwargs): results = [] diff --git a/tests/nodes/test_llm_analyzer_base.py b/tests/nodes/test_llm_analyzer_base.py index 4e574e5b..2c47ecf0 100644 --- a/tests/nodes/test_llm_analyzer_base.py +++ b/tests/nodes/test_llm_analyzer_base.py @@ -666,6 +666,32 @@ async def _flaky_ainvoke(prompt: str) -> LLMAnalysisResult: results = await analyzer.arun_batches(batches) assert {batch.file_path for batch, _ in results} == {"a.py", "c.py"} + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) + async def test_malformed_structured_batch_does_not_abort_the_others(self) -> None: + """A malformed structured response is isolated even though it is a ValueError.""" + + async def _ainvoke(prompt: str) -> LLMAnalysisResult: + if "b.py" in prompt: + return LLMAnalysisResult.model_validate({"findings": 'We{"findings":[]}'}) + return LLMAnalysisResult( + findings=[ + LLMFinding(rule_id="T-1", message="hit", severity="LOW", start_line=1), + ] + ) + + analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) + analyzer._structured_llm.ainvoke = _ainvoke + + batches = [ + Batch(file_path="a.py", content="code a"), + Batch(file_path="b.py", content="code b"), + Batch(file_path="c.py", content="code c"), + ] + results = await analyzer.arun_batches(batches) + + assert {batch.file_path for batch, _ in results} == {"a.py", "c.py"} + assert [items[0].rule_id for _, items in results] == ["T-1", "T-1"] + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) async def test_all_batches_failed_returns_empty(self) -> None: analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) diff --git a/tests/nodes/test_semantic_quality_policy.py b/tests/nodes/test_semantic_quality_policy.py index e8ba916c..6b6f8fc6 100644 --- a/tests/nodes/test_semantic_quality_policy.py +++ b/tests/nodes/test_semantic_quality_policy.py @@ -22,7 +22,7 @@ import pytest -from skillspector.llm_analyzer_base import LLMAnalysisResult, LLMFinding +from skillspector.llm_analyzer_base import Batch, LLMAnalysisResult, LLMFinding from skillspector.models import Finding from skillspector.nodes.analyzers.semantic_quality_policy import ( ANALYZER_ID, @@ -272,7 +272,12 @@ class TestLLMCallTelemetry: def test_success_records_ok_true(self) -> None: from skillspector.llm_analyzer_base import LLMAnalyzerBase - with patch.object(LLMAnalyzerBase, "arun_batches", new_callable=AsyncMock, return_value=[]): + with patch.object( + LLMAnalyzerBase, + "arun_batches", + new_callable=AsyncMock, + return_value=[(Batch(file_path="SKILL.md", content="# Skill"), [])], + ): result = node({"file_cache": {"SKILL.md": "# Skill"}}) assert result["llm_call_log"] == [{"node": ANALYZER_ID, "ok": True, "error": None}] From 734a47182ee7ab73bf0dcb29b8d8fa4bef509ef9 Mon Sep 17 00:00:00 2001 From: kigland Date: Tue, 21 Jul 2026 13:34:32 +0800 Subject: [PATCH 3/3] test malformed response isolation paths Signed-off-by: kigland --- tests/nodes/test_llm_analyzer_base.py | 43 +++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/nodes/test_llm_analyzer_base.py b/tests/nodes/test_llm_analyzer_base.py index 2c47ecf0..683fcb21 100644 --- a/tests/nodes/test_llm_analyzer_base.py +++ b/tests/nodes/test_llm_analyzer_base.py @@ -404,6 +404,27 @@ def test_run_batches_uses_message_text_for_content_blocks(self) -> None: assert results[0][1] == ["chunk"] + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) + def test_run_batches_isolates_raw_invoke_validation_error(self) -> None: + analyzer = _RawTextAnalyzer(base_prompt="test", model=self.MODEL) + + def _invoke(prompt: str) -> AIMessage: + if "b.py" in prompt: + LLMAnalysisResult.model_validate({"findings": 'We{"findings":[]}'}) + return AIMessage(content="ok") + + analyzer._llm.invoke.side_effect = _invoke + batches = [ + Batch(file_path="a.py", content="code a"), + Batch(file_path="b.py", content="code b"), + Batch(file_path="c.py", content="code c"), + ] + + results = analyzer.run_batches(batches) + + assert {batch.file_path for batch, _ in results} == {"a.py", "c.py"} + assert [items for _, items in results] == [["ok"], ["ok"]] + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) async def test_arun_batches_uses_message_text_for_content_blocks(self) -> None: analyzer = _RawTextAnalyzer(base_prompt="test", model=self.MODEL) @@ -459,6 +480,28 @@ def test_value_error_still_propagates(self) -> None: with pytest.raises(ValueError, match="no API key"): analyzer.run_batches([Batch(file_path="a.py", content="code")]) + @patch(MOCK_PATCH_TARGET, _mock_get_chat_model) + def test_parse_validation_error_does_not_abort_the_others(self) -> None: + analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL) + analyzer._structured_llm.invoke.return_value = LLMAnalysisResult(findings=[]) + original_parse = analyzer.parse_response + + def _parse(response: object, batch: Batch) -> list[Finding]: + if batch.file_path == "b.py": + LLMAnalysisResult.model_validate({"findings": 'We{"findings":[]}'}) + return original_parse(response, batch) + + analyzer.parse_response = _parse + batches = [ + Batch(file_path="a.py", content="code a"), + Batch(file_path="b.py", content="code b"), + Batch(file_path="c.py", content="code c"), + ] + + results = analyzer.run_batches(batches) + + assert {batch.file_path for batch, _ in results} == {"a.py", "c.py"} + # --------------------------------------------------------------------------- # LLMAnalyzerBase.arun_batches (async parallel execution)