fix: catch pydantic ValidationError in VectorStoreQueryOutputParser#20450
Conversation
Wrap pydantic ValidationError as OutputParserException to ensure consistent exception handling in VectorIndexAutoRetriever. Fixes run-llama#19410 Signed-off-by: majiayu000 <1835304752@qq.com>
| except ValidationError as e: | ||
| raise OutputParserException( | ||
| f"Failed to validate query spec. Error: {e}. Got JSON dict: {json_dict}" | ||
| ) from e |
There was a problem hiding this comment.
Did you notice a better UX here by throwing the OutputParserException instead of the ValidationError that would be normally thrown without this extra step?
There was a problem hiding this comment.
Farther up the stack the OutputParserException is caught
There was a problem hiding this comment.
(see the linked issue for more details)
Signed-off-by: majiayu000 <1835304752@qq.com>
Signed-off-by: majiayu000 <1835304752@qq.com>
There was a problem hiding this comment.
Looks like these changes are unrelated
Signed-off-by: majiayu000 <1835304752@qq.com>
|
@AstraBert Thanks for the suggestion. The parser now wraps the ValidationError and raises OutputParserException with context (see llama-index-core/llama_index/core/indices/vector_store/retrievers/auto_retriever/output_parser.py). @logan-markewich The schema changes are part of the MediaResource hash fix; happy to split if you prefer. |
|
@majiayu000 I see you have another PR open for the MediaResource hashing changes, we can follow up there with those and I suggest you remove them from here |
|
Thanks @AstraBert! You are right. I have updated this PR and removed the This PR is now focused solely on catching the |
Description
Wrap pydantic
ValidationErrorasOutputParserExceptioninVectorStoreQueryOutputParser.parse()to ensure consistent exception handling inVectorIndexAutoRetriever._parse_generated_spec.Previously, when the LLM returned malformed JSON that passed JSON parsing but failed Pydantic validation, an uncaught
ValidationErrorwould be raised instead of the expectedOutputParserException.Fixes #19410
New Package?
Version Bump?
Type of Change
How Has This Been Tested?
Added two test cases:
test_output_parser_invalid_schema_raises_output_parser_exception: Tests that missing required fields raiseOutputParserExceptiontest_output_parser_invalid_type_raises_output_parser_exception: Tests that invalid types raiseOutputParserExceptionSuggested Checklist:
uv run make format; uv run make lintto appease the lint gods