fix: serialize NaN/Inf floats nested in pydantic models as JSON-safe strings - #1817
Open
uuzzrm wants to merge 1 commit into
Open
fix: serialize NaN/Inf floats nested in pydantic models as JSON-safe strings#1817uuzzrm wants to merge 1 commit into
uuzzrm wants to merge 1 commit into
Conversation
…strings The BaseModel branch of EventSerializer returned model_dump() directly, so non-finite floats nested inside a pydantic model bypassed the NaN/Inf sanitizer that the dict/list branches apply. The JSON C encoder then emitted bare NaN/Infinity tokens, which strict JSON parsers reject -- a traced pydantic object carrying a NaN/Inf float produced an event body that failed ingestion. Route the dumped dict back through default() so the same sanitization applies at every nesting depth. Fixes #16048. Signed-off-by: Ruiming Zhao <uuzzrm@gmail.com>
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.
What does this PR do?
Fixes #16048
EventSerializer'sBaseModelbranch returnedobj.model_dump()directly, so non-finite floats nested inside a pydantic model bypassed the NaN/Inf sanitizer that the dict/list branches already apply. Python's JSON C encoder then emitted bareNaN/Infinitytokens, which strict JSON parsers reject. On the tracing path, any traced pydantic object carrying a NaN/Inf float (common with ML/scoring payloads) produced an event body that failed ingestion.The fix routes the dumped dict back through
default(), so the same sanitization applies at every nesting depth:Type of change
Verification
The new regression test
test_pydantic_model_with_non_finite_float_serializes_to_valid_jsonfails before the fix (strict parse raisesValueError: NaN) and passes after.test_pathfails on this machine only (POSIX-vs-Windows path separators) and passes in CI's Linux runner; the same pre-existing mypyimport-not-foundnote forlangchain_coreis present on the unmodified file.Checklist
code_review.md..env.templateif needed.Greptile Summary
Routes Pydantic
model_dump()output back throughEventSerializerso nested non-finite floats become JSON-safe strings.NaNand infinity values consistently with existing dict/list handling.Confidence Score: 5/5
The PR appears safe to merge, with the intended Pydantic serialization behavior covered by a focused strict-JSON regression test.
The changed branch reuses the established recursive normalization path, ensuring non-finite floats nested in Pydantic models no longer produce invalid bare JSON constants, and no concrete blocking or non-blocking defect remains.
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Pydantic BaseModel] --> B[model_dump] B --> C[EventSerializer recursive normalization] C --> D[NaN and Infinity converted to strings] D --> E[Valid JSON ingestion payload]Reviews (1): Last reviewed commit: "fix: serialize NaN/Inf floats nested in ..." | Re-trigger Greptile
Context used: