Skip to content

Commit a8cef79

Browse files
committed
isolate malformed async LLM batches
Signed-off-by: kigland <shuaizhicheng336@gmail.com>
1 parent 2e73550 commit a8cef79

10 files changed

Lines changed: 90 additions & 16 deletions

src/skillspector/llm_analyzer_base.py

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,9 @@ def run_batches(
374374
375375
The element type of the inner list depends on the subclass: the default
376376
:meth:`parse_response` returns :class:`Finding` objects; subclasses may
377-
return dicts or other types.
377+
return dicts or other types. Malformed structured responses are isolated
378+
per batch and omitted from the result so callers can detect partial
379+
analysis by comparing submitted and returned batches.
378380
"""
379381
results: list[tuple[Batch, list]] = []
380382
for batch in batches:
@@ -399,6 +401,9 @@ def run_batches(
399401
else:
400402
try:
401403
response = _message_text(self._llm.invoke(prompt))
404+
except ValidationError as exc:
405+
logger.warning("LLM batch failed for %s: %s", batch.file_label, exc)
406+
continue
402407
except (ValueError, NotImplementedError):
403408
raise
404409
except Exception as exc:
@@ -432,12 +437,12 @@ async def arun_batches(
432437
cross-chunk batches are parallelized in a single gather call.
433438
434439
Failures are isolated per batch: a transient error (timeout, 429,
435-
oversized-chunk 400, ...) costs only its own batch, which is logged
436-
and omitted from the result, so one bad call cannot cancel the rest
437-
of the fan-out. Callers can detect partial results by comparing the
438-
returned batches against the submitted ones. ``ValueError`` and
439-
``NotImplementedError`` signal misconfiguration rather than infra
440-
trouble and keep propagating.
440+
oversized-chunk 400, malformed structured response, ...) costs only
441+
its own batch, which is logged and omitted from the result, so one bad
442+
call cannot cancel the rest of the fan-out. Callers can detect partial
443+
results by comparing the returned batches against the submitted ones.
444+
``ValueError`` and ``NotImplementedError`` signal misconfiguration
445+
rather than infra trouble and keep propagating.
441446
442447
The return type mirrors :meth:`run_batches`.
443448
"""
@@ -462,6 +467,9 @@ async def _process(batch: Batch) -> tuple[Batch, list]:
462467
results = await asyncio.gather(*[_process(b) for b in batches], return_exceptions=True)
463468
successful: list[tuple[Batch, list]] = []
464469
for batch, result in zip(batches, results, strict=True):
470+
if isinstance(result, ValidationError):
471+
logger.warning("LLM batch failed for %s: %s", batch.file_label, result)
472+
continue
465473
if isinstance(result, (ValueError, NotImplementedError)):
466474
raise result
467475
if isinstance(result, BaseException):

src/skillspector/nodes/analyzers/semantic_developer_intent.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,12 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse:
179179
results = asyncio.run(analyzer.arun_batches(batches))
180180
findings = analyzer.collect_findings(results)
181181
logger.info("%s: %d findings", ANALYZER_ID, len(findings))
182+
if len(results) < len(batches):
183+
error = f"{len(batches) - len(results)}/{len(batches)} LLM batches failed"
184+
return {
185+
"findings": findings,
186+
"llm_call_log": [llm_call_record(ANALYZER_ID, ok=False, error=error)],
187+
}
182188
return {"findings": findings, "llm_call_log": [llm_call_record(ANALYZER_ID, ok=True)]}
183189
except ValueError:
184190
raise

src/skillspector/nodes/analyzers/semantic_quality_policy.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,12 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse:
148148
results = asyncio.run(analyzer.arun_batches(batches))
149149
findings = analyzer.collect_findings(results)
150150
logger.info("%s: %d findings", ANALYZER_ID, len(findings))
151+
if len(results) < len(batches):
152+
error = f"{len(batches) - len(results)}/{len(batches)} LLM batches failed"
153+
return {
154+
"findings": findings,
155+
"llm_call_log": [llm_call_record(ANALYZER_ID, ok=False, error=error)],
156+
}
151157
return {"findings": findings, "llm_call_log": [llm_call_record(ANALYZER_ID, ok=True)]}
152158
except ValueError:
153159
raise

src/skillspector/nodes/analyzers/semantic_security_discovery.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,12 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse:
9090
results = analyzer.run_batches(batches)
9191
findings = analyzer.collect_findings(results)
9292
logger.info("%s: %d findings", ANALYZER_ID, len(findings))
93+
if len(results) < len(batches):
94+
error = f"{len(batches) - len(results)}/{len(batches)} LLM batches failed"
95+
return {
96+
"findings": findings,
97+
"llm_call_log": [llm_call_record(ANALYZER_ID, ok=False, error=error)],
98+
}
9399
return {"findings": findings, "llm_call_log": [llm_call_record(ANALYZER_ID, ok=True)]}
94100
except ValidationError as exc:
95101
# Malformed LLM response — degrade gracefully rather than crashing the graph

src/skillspector/nodes/meta_analyzer.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,12 @@ def meta_analyzer(state: SkillspectorState) -> MetaAnalyzerResponse:
563563
len(findings),
564564
len(filtered),
565565
)
566+
if unanalysed:
567+
error = f"{len(batches) - len(batch_results)}/{len(batches)} LLM batches failed"
568+
return {
569+
"filtered_findings": filtered,
570+
"llm_call_log": [llm_call_record("meta_analyzer", ok=False, error=error)],
571+
}
566572
return {
567573
"filtered_findings": filtered,
568574
"llm_call_log": [llm_call_record("meta_analyzer", ok=True)],

tests/nodes/analyzers/test_semantic_developer_intent.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222

2323
import pytest
2424

25-
from skillspector.llm_analyzer_base import LLMAnalysisResult, LLMFinding
25+
from skillspector.llm_analyzer_base import Batch, LLMAnalysisResult, LLMFinding
2626
from skillspector.models import Finding
2727
from skillspector.nodes.analyzers.semantic_developer_intent import (
2828
ANALYZER_ID,
@@ -240,7 +240,12 @@ class TestLLMCallTelemetry:
240240
def test_success_records_ok_true(self) -> None:
241241
from skillspector.llm_analyzer_base import LLMAnalyzerBase
242242

243-
with patch.object(LLMAnalyzerBase, "arun_batches", new_callable=AsyncMock, return_value=[]):
243+
with patch.object(
244+
LLMAnalyzerBase,
245+
"arun_batches",
246+
new_callable=AsyncMock,
247+
return_value=[(Batch(file_path="main.py", content="import os"), [])],
248+
):
244249
result = node({"file_cache": {"main.py": "import os"}})
245250
assert result["llm_call_log"] == [{"node": ANALYZER_ID, "ok": True, "error": None}]
246251

tests/nodes/analyzers/test_semantic_security_discovery.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
import pytest
2424
from pydantic import ValidationError
2525

26-
from skillspector.llm_analyzer_base import LLMAnalysisResult, LLMFinding
26+
from skillspector.llm_analyzer_base import Batch, LLMAnalysisResult, LLMFinding
2727
from skillspector.models import Finding
2828
from skillspector.nodes.analyzers.semantic_security_discovery import (
2929
ANALYZER_ID,
@@ -316,7 +316,11 @@ class TestLLMCallTelemetry:
316316
def test_success_records_ok_true(self, base_state) -> None:
317317
from skillspector.llm_analyzer_base import LLMAnalyzerBase
318318

319-
with patch.object(LLMAnalyzerBase, "run_batches", return_value=[]):
319+
with patch.object(
320+
LLMAnalyzerBase,
321+
"run_batches",
322+
return_value=[(Batch(file_path="SKILL.md", content="# Skill"), [])],
323+
):
320324
result = node(base_state)
321325
assert result["llm_call_log"] == [{"node": ANALYZER_ID, "ok": True, "error": None}]
322326

@@ -374,7 +378,6 @@ def _build_file_cache(skill_dir: Path) -> dict[str, str]:
374378

375379
def _make_file_aware_run_batches(responses: dict[str, LLMAnalysisResult]):
376380
"""Return a mock run_batches that dispatches based on file_path in each batch."""
377-
from skillspector.llm_analyzer_base import Batch
378381

379382
def _run_batches(self_inner, batches: list[Batch], **_kwargs):
380383
results = []

tests/nodes/test_llm_analyzer_base.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,32 @@ async def _flaky_ainvoke(prompt: str) -> LLMAnalysisResult:
621621
results = await analyzer.arun_batches(batches)
622622
assert {batch.file_path for batch, _ in results} == {"a.py", "c.py"}
623623

624+
@patch(MOCK_PATCH_TARGET, _mock_get_chat_model)
625+
async def test_malformed_structured_batch_does_not_abort_the_others(self) -> None:
626+
"""A malformed structured response is isolated even though it is a ValueError."""
627+
628+
async def _ainvoke(prompt: str) -> LLMAnalysisResult:
629+
if "b.py" in prompt:
630+
return LLMAnalysisResult.model_validate({"findings": 'We{"findings":[]}'})
631+
return LLMAnalysisResult(
632+
findings=[
633+
LLMFinding(rule_id="T-1", message="hit", severity="LOW", start_line=1),
634+
]
635+
)
636+
637+
analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL)
638+
analyzer._structured_llm.ainvoke = _ainvoke
639+
640+
batches = [
641+
Batch(file_path="a.py", content="code a"),
642+
Batch(file_path="b.py", content="code b"),
643+
Batch(file_path="c.py", content="code c"),
644+
]
645+
results = await analyzer.arun_batches(batches)
646+
647+
assert {batch.file_path for batch, _ in results} == {"a.py", "c.py"}
648+
assert [items[0].rule_id for _, items in results] == ["T-1", "T-1"]
649+
624650
@patch(MOCK_PATCH_TARGET, _mock_get_chat_model)
625651
async def test_all_batches_failed_returns_empty(self) -> None:
626652
analyzer = LLMAnalyzerBase(base_prompt="test", model=self.MODEL)

tests/nodes/test_meta_analyzer.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -261,15 +261,18 @@ def _degr_state(**overrides: object) -> SkillspectorState:
261261

262262

263263
def test_records_ok_true_on_success() -> None:
264+
state = _degr_state()
264265
with (
265266
patch("skillspector.llm_analyzer_base.get_chat_model", return_value=MagicMock()),
266267
patch(
267268
"skillspector.nodes.meta_analyzer.LLMMetaAnalyzer.arun_batches",
268269
new_callable=AsyncMock,
269-
return_value=[],
270+
return_value=[
271+
(Batch(file_path="SKILL.md", content="# Skill", findings=state["findings"]), [])
272+
],
270273
),
271274
):
272-
result = meta_analyzer(_degr_state())
275+
result = meta_analyzer(state)
273276
assert result["llm_call_log"] == [{"node": "meta_analyzer", "ok": True, "error": None}]
274277

275278

tests/nodes/test_semantic_quality_policy.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222

2323
import pytest
2424

25-
from skillspector.llm_analyzer_base import LLMAnalysisResult, LLMFinding
25+
from skillspector.llm_analyzer_base import Batch, LLMAnalysisResult, LLMFinding
2626
from skillspector.models import Finding
2727
from skillspector.nodes.analyzers.semantic_quality_policy import (
2828
ANALYZER_ID,
@@ -269,7 +269,12 @@ class TestLLMCallTelemetry:
269269
def test_success_records_ok_true(self) -> None:
270270
from skillspector.llm_analyzer_base import LLMAnalyzerBase
271271

272-
with patch.object(LLMAnalyzerBase, "arun_batches", new_callable=AsyncMock, return_value=[]):
272+
with patch.object(
273+
LLMAnalyzerBase,
274+
"arun_batches",
275+
new_callable=AsyncMock,
276+
return_value=[(Batch(file_path="SKILL.md", content="# Skill"), [])],
277+
):
273278
result = node({"file_cache": {"SKILL.md": "# Skill"}})
274279
assert result["llm_call_log"] == [{"node": ANALYZER_ID, "ok": True, "error": None}]
275280

0 commit comments

Comments
 (0)