Repository navigation
fix(nodes): return the invalid JSON error from GenerateAnswerNode - #1179
Open
tanishqbaweja wants to merge 1 commit into
Open
tanishqbaweja wants to merge 1 commit into
tanishqbaweja wants to merge 1 commit into
Conversation
GenerateAnswerNode is meant to return {"error": "Invalid JSON response
format...", "raw_response": ...} when the model reply cannot be parsed, but
the three fallbacks (single chunk, chunk stage, merge) only catch Timeout and
json.JSONDecodeError. The chains end in LangChain JSON output parsers, which
raise OutputParserException for a non-JSON reply, so the fallback never ran
and graph.run() raised instead.
Catch OutputParserException in the same three places and add tests that run
the node with a fake chat model and the real parsers.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GenerateAnswerNodehas a fallback for model replies that aren't valid JSON. It's meant to put{"error": "Invalid JSON response format", "raw_response": ...}in the state instead of raising, with "in chunk processing" and "during merge" variants for the multi-chunk path. That fallback never runs for a reply that isn't JSON:graph.run()raisesOutputParserExceptioninstead. Models do reply like this: there are several closedOutputParserException: Invalid json outputreports, mostly with small models such as Llama 3 8B and Gemma 7B.Reproduction on
pre/beta, no API key needed:On
pre/betait raises:With this PR it returns:
Root cause
In 5c0bc46 the three
except Timeoutclauses (single chunk, chunk stage, merge) becameexcept (Timeout, json.JSONDecodeError), with the "Invalid JSON response format" messages. Unless the model isChatBedrock, each chain ends in an output parser:TolerantJsonOutputParserwithout a schema,JsonOutputParser(pydantic_object=...)with one. For a reply that isn't JSON, both raiselangchain_core.exceptions.OutputParserException. That exception wraps theJSONDecodeErrorbut isn't a subclass of it, so none of theexceptclauses match.The existing
test_execute_merge_json_decode_errordidn't catch this, because it replacesinvoke_with_timeoutwith a function that raisesjson.JSONDecodeErrordirectly.Fix
Import
OutputParserExceptionand add it to the threeexcepttuples. The error messages don't change,raw_responseis stillstr(e)(for this exception it includes the model's reply), andTimeoutandjson.JSONDecodeErrorare still caught.Tests
New
tests/test_generate_answer_node_invalid_json.pyruns the node withFakeListChatModeland the node's real parsers. It doesn't mockinvoke_with_timeout."Invalid JSON response format"."Invalid JSON response format in chunk processing"."Invalid JSON response format during merge".The fake model takes a lock in
_call, because the chunk chains run in parallel threads and the merge test needs the replies handed out in order.On
pre/beta(b4a992f) with only the new test file added, the four error cases fail and the three valid JSON cases pass:With the fix: 7 passed. The new file isn't on the list of test files in
.github/workflows/test-suite.yml, so CI won't run it unless it's added there.Other checks, on Windows 11 with Python 3.12.15 and dependencies from
uv sync --frozen, each compared withpre/beta:.github/workflows/test-suite.yml(first command): 95 passed, same aspre/beta.tests/test_generate_answer_node.py: 3 failed, 5 passed, same aspre/beta. These failures are already there and this PR leaves them alone:test_execute_multiple_chunksandtest_execute_merge_json_decode_error: the fakeinvoke_with_timeoutlooks for a"context"key, but the node passes"content"to the merge chain.test_init_chat_ollama_format: the test buildsChatOllamafromlangchain_community, but the node checks for thelangchain_ollamaclass.ruff checkandblack --checkpass on both changed files.isort --check-onlypasses on the new test file. It flagsgenerate_answer_node.pyfor the existing order of thelangchain_awsandlangchain_core.runnablesimports, the same as onpre/beta. The new import is where isort puts it, and the other imports aren't reordered.make lintcommands give the same results as onpre/beta: ruff reports 6 unused imports innodes/batch_generate_answer_node.pyandutils/batch_api.py, black would reformat 23 files and isort flags 26.make type-checkwasn't run. mypy on the two changed files reports nothing on the changed lines. The other errors are strict-mode errors already in the node, plus missing annotations in the new test file (the existing tests aren't annotated either).make test(the graph, integration and browser suites need API keys or network access) ormake pre-commit.