Skip to content

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

Merged
rng1995 merged 1 commit into
NVIDIA:mainfrom
Thanatos9404:fix/843-missing-structured-response
Oct 28, 2026
Merged

rng1995 merged 1 commit into
NVIDIA:mainfrom
Thanatos9404:fix/843-missing-structured-response

Conversation

@Thanatos9404

Copy link
Copy Markdown

Fixes #843

LangChain's tool-call parsers return None when a reply has no tool call. bind_structured_output already turns that into a StructuredOutputParseError for auto tool choice bindings (#529) and explicit function_calling (#691), so those are retried and end as llm_structured_response_invalid. Every other binding returns the None unchanged, including the default forced tool call on Bedrock (ChatBedrockConverse) and on the Anthropic API (ChatAnthropic). _invoke_batch then passes None to parse_response:

  • _TP4Analyzer.parse_response raises NotImplementedError("TP4 requires a structured assessment response"). arun_batches_detailed re-raises NotImplementedError as a misconfiguration, so _check_tp4 records failed / llm_batch_failed after one attempt and the scan ends with execution_successful: false and exit code 2, as in the issue.
  • The base parse_response, used by the three semantic analyzers, raises the same kind of NotImplementedError; the semantic node then reports the whole analyzer unavailable and drops the other batches' results.
  • LLMMetaAnalyzer.parse_response fails with AttributeError, and the batch fails as llm_batch_failed without a retry.

This PR checks for a None result inside the existing try in _invoke_batch and _ainvoke_batch and raises StructuredOutputParseError there. A missing result now takes the same path as any other malformed structured response: three bounded retries, then a skipped batch with llm_structured_response_invalid. For TP4 the analyzer ends degraded, the report shows partial analysis, and execution_successful stays true. If a retry returns a valid assessment, TP4 completes normally.

The check sits in the base class rather than in TP4's parse_response because parse_response runs outside that try, 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_calling and kept other bindings failing on purpose; this applies it to every structured binding.

On current main the 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 default us.anthropic.claude-sonnet-4-6, still hit the crash on main. The 2.13.0 release branch (#833) leaves out #806 and has the same _invoke_batch code, so the issue's exact setup still exits 2 there. The llm_analyzer_base.py change applies cleanly to yashraj/release-v2.13.0 if a backport is wanted.

Tests

tests/nodes/test_llm_analyzer_base.py, in both TestRunBatches and TestARunBatches:

  • 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: four None results. The batch fails with ValidationError / llm_structured_response_invalid after the 0.5, 1 and 2 s backoffs, and the next batch still succeeds.

tests/test_mcp_tool_poisoning.py, in TestTP4Fallbacks:

  • test_missing_structured_response_is_retried: None, then a valid assessment. TP4 ends completed after two calls.
  • test_bedrock_forced_tool_call_answered_in_prose_is_retried: a real ChatBedrockConverse for BEDROCK_DEFAULT_MODEL (forced tool choice) whose _agenerate returns prose, so LangChain's parser returns None. TP4 makes four calls, ends degraded with llm_structured_response_invalid, and records no failed rows.
  • test_tool_refusal_stays_incomplete (from fix: prefer tool output for OpenAI Opus 5 TP4 #691) pinned the old behaviour: one call, then failed. It now expects four calls, degraded, a skipped llm_structured_response_invalid row, and finalize_ledger returning execution_successful: true and is_complete: false.

All seven fail on main @ 2404212 (the base tests with NotImplementedError, 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 unmodified main; none touch the LLM code.
  • The LLM-related test files (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/ and ruff format --check src/ tests/ (what make lint and make format-check run): pass.

I also ran the TP4 node offline against a real ChatBedrockConverse whose botocore client was stubbed with Stubber to answer in prose. For us.anthropic.claude-sonnet-4-6, main makes one Converse call and ends failed / NotImplementedError with execution_successful: false; this branch makes four calls and ends degraded / llm_structured_response_invalid with execution_successful: true, and a prose reply followed by a valid tool call completes after two calls. No live provider calls were made.

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>
Copilot AI balanced review requested due to automatic review settings October 15, 2026 11:53

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

@rng1995
rng1995 merged commit 6b0ebe1 into NVIDIA:main Oct 28, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bedrock: mcp_tool_poisoning (TP4) fails with NotImplementedError when structured output returns None; run ends execution_successful=false, exit 2

3 participants