fix(serializer): recurse converted values through default() to prevent invalid JSON - #1877
betacatsling wants to merge 1 commit into
Conversation
|
|
| # integers are normalized | ||
| if np is not None and isinstance(obj, np.generic): | ||
| return obj.item() | ||
| return self.default(obj.item()) |
There was a problem hiding this comment.
NumPy Scalars Recurse Indefinitely
Extended-precision scalars such as np.longdouble and np.clongdouble may return themselves from .item(). Passing that result back to self.default() repeatedly re-enters this branch without reaching the later depth guard, eventually hitting Python's recursion limit instead of producing a useful serialized value.
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/_utils/serializer.py
Line: 76
Comment:
**NumPy Scalars Recurse Indefinitely**
Extended-precision scalars such as `np.longdouble` and `np.clongdouble` may return themselves from `.item()`. Passing that result back to `self.default()` repeatedly re-enters this branch without reaching the later depth guard, eventually hitting Python's recursion limit instead of producing a useful serialized value.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if is_dataclass(obj): | ||
| return asdict(obj) # type: ignore | ||
| # Recurse the dict produced by asdict() back through default() | ||
| # so nested non-finite floats and unsafe integers are normalized |
There was a problem hiding this comment.
Dataclass Keys Defeat Skipkeys
A dataclass field containing a mapping with tuple keys is now recursively processed before the standard encoder can apply skipkeys=True. Each tuple key becomes an unhashable list while rebuilding the dictionary, so the entire mapping is replaced by the serializer's error marker instead of omitting the unsupported key.
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/_utils/serializer.py
Line: 135
Comment:
**Dataclass Keys Defeat Skipkeys**
A dataclass field containing a mapping with tuple keys is now recursively processed before the standard encoder can apply `skipkeys=True`. Each tuple key becomes an unhashable list while rebuilding the dictionary, so the entire mapping is replaced by the serializer's error marker instead of omitting the unsupported key.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
What does this PR do?
EventSerializersanitizesNaN/Infinity/-Infinityfloats and JS-unsafeintegers only when a value reaches
_default_innerdirectly. Severalconversion branches returned containers or scalars without routing them back
through
self.default, so non-finite floats nested inside them were emitted asbare
NaN/Infinitytokens — invalid JSON that strict parsers reject.This routes the converted values back through
self.defaultfor:tuple/set/frozenset(per element, instead oflist(obj))np.ndarray(after.tolist())np.genericscalars (after.item(), e.g.np.float32("nan"))asdict())enum.Enumvalues (e.g. an enum wrappingfloat("nan"))Serializable.to_json()outputSame class of bug as #1811 (which covered Pydantic
model_dump()output);this covers the remaining conversion branches.
Fixes #1876
Type of change
Verification
New tests in
tests/unit/test_serializer.pyassert strict-JSON-parseableoutput (rejecting bare
NaN/Infinityconstants) and the expected stringsentinels for non-finite floats and JS-unsafe integers nested in tuples, sets,
frozensets, dataclasses, enums, numpy arrays, and numpy scalars.
Commands and results:
uv run --frozen pytest tests/unit/test_serializer.py -q1 failed, 30 passed—test_non_finite_floats_in_tuple_set_frozensetfails with a bare
NaNtoken35 passeduv run --frozen ruff check langfuse/_utils/serializer.py tests/unit/test_serializer.py: all checks passeduv run --frozen ruff format --check ...: already formatteduv run --frozen mypy langfuse --no-error-summary: cleanChecklist
The core fix appears safe to merge, with non-blocking edge cases around extended NumPy scalars, dataclass mapping keys, and CI coverage.
Summary
Reviews (1) · Last reviewed commit: "fix(serializer): recurse converted value..."