Repository navigation
fix(llm): retry structured calls that return no result - #845
Conversation
LangChain's tool-call parsers return None when a reply has no tool call, for example when a forced tool call is answered in prose. Only the auto tool choice and explicit function_calling bindings turned that into a StructuredOutputParseError. On every other binding, None reached parse_response: TP4 and the semantic analyzers raised NotImplementedError, which the batch runners re-raise as a misconfiguration. For TP4 the scan then ended with execution_successful false and exit code 2. Treat a None structured result like any other malformed structured response. It now gets the existing bounded retries, and if they run out the batch is recorded as llm_structured_response_invalid (skipped), so the analysis is reported as incomplete instead of failed. Fixes NVIDIA#843 Signed-off-by: Yashvardhan Thanvi <yashvt9404@gmail.com>
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Value and readiness
A structured binding that returns None now enters the existing bounded malformed-response retry path instead of reaching a parser that expects a schema object. I found no required code change at f9a0a2b758dcd8e16214d383e694c8a50b4088c8.
Material findings
No required change found. Both sync and async invocation paths call the same narrow None guard inside the structured-validation try block; custom parser exceptions remain outside it. Exhausted retries remain isolated to the batch and produce llm_structured_response_invalid plus incomplete coverage. TP4 reports degraded execution rather than inventing successful inspection.
PIC tradeoffs
A provider prose response can cost the existing three retries. The retry/time limits and fail-closed completeness policy remain the governing tradeoff.
Verification and gaps
Read the complete base-analyzer and TP4/test changes, pinned source, description and prior automated notices. Coverage includes recovery, exhausted sync/async retries, next-batch survival, and a real Bedrock parser path mocked at transport generation. All six hosted checks succeeded. I did not execute PR code or tests.
Fixes #843
LangChain's tool-call parsers return
Nonewhen a reply has no tool call.bind_structured_outputalready turns that into aStructuredOutputParseErrorfor auto tool choice bindings (#529) and explicitfunction_calling(#691), so those are retried and end asllm_structured_response_invalid. Every other binding returns theNoneunchanged, including the default forced tool call on Bedrock (ChatBedrockConverse) and on the Anthropic API (ChatAnthropic)._invoke_batchthen passesNonetoparse_response:_TP4Analyzer.parse_responseraisesNotImplementedError("TP4 requires a structured assessment response").arun_batches_detailedre-raisesNotImplementedErroras a misconfiguration, so_check_tp4recordsfailed/llm_batch_failedafter one attempt and the scan ends withexecution_successful: falseand exit code 2, as in the issue.parse_response, used by the three semantic analyzers, raises the same kind ofNotImplementedError; the semantic node then reports the whole analyzerunavailableand drops the other batches' results.LLMMetaAnalyzer.parse_responsefails withAttributeError, and the batch fails asllm_batch_failedwithout a retry.This PR checks for a
Noneresult inside the existingtryin_invoke_batchand_ainvoke_batchand raisesStructuredOutputParseErrorthere. A missing result now takes the same path as any other malformed structured response: three bounded retries, then a skipped batch withllm_structured_response_invalid. For TP4 the analyzer endsdegraded, the report shows partial analysis, andexecution_successfulstays true. If a retry returns a valid assessment, TP4 completes normally.The check sits in the base class rather than in TP4's
parse_responsebecauseparse_responseruns outside thattry, and errors raised there deliberately propagate without a retry (test_custom_parser_validation_error_propagates_without_retry).This is the follow-up suggested in the #691 review (#691 (comment)). #691 applied it to explicit
function_callingand kept other bindings failing on purpose; this applies it to every structured binding.On current
mainthe issue's model,global.anthropic.claude-sonnet-5-5, is registered as rejecting forced tool calls (#806), so on Bedrock it already takes the auto tool choice path and is retried. Models that keep the forced tool call, such as the Bedrock defaultus.anthropic.claude-sonnet-4-6, still hit the crash onmain. The 2.13.0 release branch (#833) leaves out #806 and has the same_invoke_batchcode, so the issue's exact setup still exits 2 there. Thellm_analyzer_base.pychange applies cleanly toyashraj/release-v2.13.0if a backport is wanted.Tests
tests/nodes/test_llm_analyzer_base.py, in bothTestRunBatchesandTestARunBatches:test_missing_structured_response_recovers_on_retry:None, then a valid result. Two calls, one 0.5 s backoff, no failure.test_missing_structured_response_isolated_after_three_retries: fourNoneresults. The batch fails withValidationError/llm_structured_response_invalidafter the 0.5, 1 and 2 s backoffs, and the next batch still succeeds.tests/test_mcp_tool_poisoning.py, inTestTP4Fallbacks:test_missing_structured_response_is_retried:None, then a valid assessment. TP4 endscompletedafter two calls.test_bedrock_forced_tool_call_answered_in_prose_is_retried: a realChatBedrockConverseforBEDROCK_DEFAULT_MODEL(forced tool choice) whose_ageneratereturns prose, so LangChain's parser returnsNone. TP4 makes four calls, endsdegradedwithllm_structured_response_invalid, and records nofailedrows.test_tool_refusal_stays_incomplete(from fix: prefer tool output for OpenAI Opus 5 TP4 #691) pinned the old behaviour: one call, thenfailed. It now expects four calls,degraded, a skippedllm_structured_response_invalidrow, andfinalize_ledgerreturningexecution_successful: trueandis_complete: false.All seven fail on
main@ 2404212 (the base tests withNotImplementedError, the TP4 tests with one call where two or four are expected) and pass on this branch.Checks
On Windows, Python 3.12.5,
uv sync --frozen --all-extras, at 2404212 plus this commit:pytest -m "not integration and not provider" tests/: passes except about 40 Windows-only tests (environment size limit,os.geteuid, symlinks, paths and encodings, plus one timing-sensitive test) that fail the same way on unmodifiedmain; none touch the LLM code.tests/test_mcp_tool_poisoning.py,tests/nodes/test_llm_analyzer_base.py,tests/nodes/test_meta_analyzer.py,tests/nodes/test_meta_analyzer_fallback.py,tests/nodes/test_semantic_quality_policy.py,tests/nodes/analyzers/test_semantic_developer_intent.py,tests/nodes/analyzers/test_semantic_security_discovery.py,tests/test_semantic_runtime.py,tests/unit/test_llm_utils.py,tests/unit/test_tp4_openai_http.py,tests/unit/test_bedrock_provider.py): 770 passed.ruff check src/ tests/andruff format --check src/ tests/(whatmake lintandmake format-checkrun): pass.I also ran the TP4 node offline against a real
ChatBedrockConversewhose botocore client was stubbed withStubberto answer in prose. Forus.anthropic.claude-sonnet-4-6,mainmakes one Converse call and endsfailed/NotImplementedErrorwithexecution_successful: false; this branch makes four calls and endsdegraded/llm_structured_response_invalidwithexecution_successful: true, and a prose reply followed by a valid tool call completes after two calls. No live provider calls were made.