You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(serializers): refuse AutoSerializer datetimes that read back as another value (LAB-12972) - #723
AutoSerializer now refuses at write two datetime values that would read back as another value:
A pandas Timestamp with non-zero nanoseconds raises TypeError, bare or inside any container. Its ISO text kept all nine digits, but datetime.fromisoformat keeps six, so a lone value came back truncated, Timestamps 1 ns apart in a set collapsed into one element with no error, and nine of them, or two frozensets of them, failed every read under the set bound from #721. pd.NaT is a datetime but not a Timestamp, so it is never refused.
A set or frozenset holding an aware datetime whose UTC offset depends on fold, a wall time its zone repeats or skips, at either fold, raises TypeError, whether the datetime is an element or sits at any depth inside one of the set's tuples. PEP 495 makes such a datetime unequal to every datetime in another zone, but it is stored with its offset and without fold, so it reads back as a fixed-offset datetime that can equal another element. With d the first 01:30 of New York's repeated hour, {d, d.astimezone(UTC)} wrote two elements and read back one; at fold=1, {frozenset({d1}), frozenset({utc})} failed every read. Naive and fixed-offset (UTC included) datetimes are never refused. Outside a set the same datetime writes the same bytes as before.
No stored format changes. Every value that is not refused writes byte-identical payloads, and tests pin the bytes for a zero-nanosecond Timestamp, a plain datetime, and a fold=1datetime bare, in a list and as a dict value. Keeping nanoseconds or fold would need a new stored field on the __datetime__ marker the protocol shares, so it is out of this change.
The refusal message says what to do: ts.floor("us") or ts.value for a Timestamp, dt.astimezone(timezone.utc) for a fold-dependent datetime.
Docs
docs/serializers/auto.md: the "What It Preserves" row says datetime round-trips to the microsecond, and a new paragraph states the Timestamp behaviour (exact to the microsecond, sub-microsecond refused at write, pd.NaT written but unreadable). The "Set Size Bound" residual paragraph now says the writer refuses these sets, and names them only as entries an earlier version wrote.
docs/error-codes.md: "Serialization unsupported type" lists the two new TypeErrors; the set-bound paragraph of "Deserialization failed" names these sets as written by an earlier version.
Cost
The fold check walks a set's elements and its tuples a level at a time in C passes; only datetimes outside a fixed-offset zone reach Python, which calls utcoffset() at both folds. It is skipped for a set of only str and bytes. Measured on serialize() of one set, guard on against guard off: 1,024 UTC datetimes +5%, 1,024 America/New_York datetimes +31% (about 0.4 ms), 1,024 nested 3-tuples +14%, 100k strings no change. Reads are unchanged.
Testing
tests/unit/test_auto_serializer_new_types.py: the old test that asserted the 8-Timestamp collapse and the read-side refusal now asserts TypeError at write for the set of 8, the set of 9 and the frozenset pair. A new TestAutoSerializerDatetimeRoundTrip pins the nanosecond refusal in each container, pd.NaT, the byte-identical payloads, eight fold-dependent set shapes (fold=1 and fold=0 in a repeated hour, both folds in a skipped hour, bare, in tuples and in frozensets), and that naive, fixed-offset and unambiguous zoned datetimes still round-trip.
Against the base commit, the 10 new refusal tests fail and the 4 byte-identity and non-refusal tests pass.
pytest tests --ignore=tests/fuzzing -m "not slow" -n auto: 8624 passed, 109 skipped. ruff check, ruff format --check and basedpyright pass.
…nother value
A pandas Timestamp with nanoseconds was stored as ISO text that keeps them
and read back with datetime.fromisoformat, which drops them: a lone value
came back truncated, Timestamps 1 ns apart in a set collapsed into one
element, and nine of them, or two frozensets of them, failed every read.
Refuse such a Timestamp at write with TypeError, as the serializer already
refuses sets its reader would refuse. pd.NaT is not a Timestamp and is not
refused.
An aware datetime at fold=1 is stored with its offset but without fold, so
inside a set it can read back equal to another element. Refuse a set or
frozenset that holds one, directly or at any depth inside its tuples, when
fold changes its UTC offset. Outside a set it writes the same bytes as
before.
No stored format changes: every value that is not refused writes the same
bytes.
No actionable comments were generated in the recent review.
Summary by CodeRabbit
Bug Fixes
Auto serialization now rejects pandas timestamps with sub-microsecond precision and certain folded datetimes in sets when they cannot be safely restored. This prevents values from losing precision or merging during a round trip.
Clarified that NaT can be written but fails on read, and that affected entries from older versions may be refused or restored with fewer set elements.
Documentation
Updated serializer guidance to explain datetime precision, set handling and compatibility with earlier cache entries.
Walkthrough
AutoSerializer now rejects values that can lose precision or collide during decoding. Tests and documentation cover pandas timestamp precision, folded datetimes in sets, and entries written by earlier cachekit versions.
AutoSerializer rejects pandas Timestamp values with non-zero nanoseconds. It also rejects sets or frozensets containing an aware fold=1 datetime when its UTC offset changes at fold=0. Tests and documentation cover these rules and describe how earlier-version entries may be handled.
cachekit-io/cachekit-py#721: Both PRs add write-time refusal rules for set and frozenset encoding in auto_serializer.py and update related tests and documentation.
Suggested reviewers: kodus-27b
Merge Risk: Minimal · up to 0b16f
The reviewed refusal behavior has no identified issue that needs fixing before merge.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title names the main change: AutoSerializer now refuses datetime values that would read back as a different value. It matches the changeset, which adds write-time refusals for sub-microsecond pand…
Docstring Coverage
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Description check
The description covers the required template areas in substance. It gives a clear Description of the change (refusing AutoSerializer datetimes that read back as another value, with the two refusal cla…
Full details: Title check
Explanation
The title names the main change: AutoSerializer now refuses datetime values that would read back as a different value. It matches the changeset, which adds write-time refusals for sub-microsecond pandas Timestamps and for fold=1 aware datetimes in sets. The conventional-commit prefix and the LAB-12972 ticket reference are acceptable. The title is 93 characters, slightly longer than ideal, but it stays clear and specific.
Full details: Docstring Coverage
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 5 functions across 2 files. (2 skipped: 2 unsupported.)
Full details: Description check
Explanation
The description covers the required template areas in substance. It gives a clear Description of the change (refusing AutoSerializer datetimes that read back as another value, with the two refusal classes and the byte-identical storage guarantee). It explains the Motivation through concrete failure cases (Timestamp collapse in sets, fold-dependent datetimes equal to other elements) and links the issue LAB-12972. It documents the docs changes, cost measurements, and testing results with commands and counts. The template's Type of Change checklist is not ticked, and the Security Checklist, Documentation Validation Checklist, and Backward Compatibility sections are not explicitly marked. The Backward Compatibility topic is partly covered: the description states that no stored format changes and that older entries may be refused or read back with fewer elements, but it does not tick the box or state the compatibility status in template terms. The Security section is also not addressed, although the change touches src/cachekit/serializers/, which the template lists as security-critical and which requires review by the security team. The description is mostly complete, so the check passes, but the gaps are worth noting.
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Reviewing files that changed from the base of the PR and between 9e3a3e7 and 0b16ff9.
Files selected for processing: 4
docs/error-codes.md
docs/serializers/auto.md
src/cachekit/serializers/auto_serializer.py
tests/unit/test_auto_serializer_new_types.py
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
Kody Code Review — 1 suggested fix.
Paste the prompt below to your agent and all review fixed at once!
🛠️ Open Agent Prompt
A code review identified the following issues in this pull request.
Each section describes what was found and includes a reference implementation where available.
Files involved:
- src/cachekit/serializers/auto_serializer.py:413
---
### [1/1] src/cachekit/serializers/auto_serializer.py:413
Issue identified during code review:
Incomplete check in _fold_shifts_offset(): it inspects only datetimes with fold=1, but CPython's PEP 495 inter-zone equality exception applies to any aware datetime whose UTC offset changes when fold is flipped, including fold=0 in an ambiguous hour and any time in a DST gap. The set {datetime(2025,11,2,1,30,tzinfo=ZoneInfo('America/New_York')), datetime(2025,11,2,5,30,tzinfo=UTC)} has 2 elements because PEP 495 makes them compare unequal, but it is written as '01:30-04:00', reads back as a fixed-offset datetime equal to the UTC twin, and silently returns as a 1-element set, which is the 'read back with fewer elements than it held' case the PR says the writer now prevents. Fix: check every aware datetime by flipping its fold (fold=1 - d.fold), not only those with fold=1.
Reference implementation (from code review):
// src/cachekit/serializers/auto_serializer.py:413
stamps = compress(values, map(issubclass, types, repeat(datetime)))
if any(
d.tzinfo is not None and d.utcoffset() != d.replace(fold=1 - d.fold).utcoffset() for d in stamps
):
return True
---
Review each issue in context, use the reference implementations as guidance, and apply fixes that are consistent with the surrounding codebase.
…fold
PEP 495 makes an aware datetime whose UTC offset depends on fold unequal to
every datetime in another zone, at fold=0 as well as fold=1, and in a
skipped hour as well as a repeated one. The stored text keeps the offset but
not fold, so such a datetime can read back equal to another element of its
set: {d, d.astimezone(UTC)} for the first 01:30 of a repeated hour wrote two
elements and read back one. Check every aware datetime by flipping its fold,
skipping naive and fixed-offset ones in C.
Head 1cfd44d21223942192f834a4b2abafa1b904ef0f measured against base 9e3a3e783b6953d18dee822270074470173cc731 on the same runner, on cpython-3.12-x86_64 (3.12.16). Workflow run
Verdict
Path
Head Ir/op
Base Ir/op
Delta
ok
byte_storage_store_40b
2,468
2,468
+0.00%
ok
byte_storage_store_64k
724,199
724,200
-0.00%
ok
file_set
77,119
77,090
+0.04%
ok
l1_hit
2,991
2,991
+0.00%
ok
l1_hit_backed
83,798
83,739
+0.07%
ok
l1_hit_backed_auto
83,671
83,660
+0.01%
ok
l1_hit_interop
109,608
109,640
-0.03%
ok
l2_hit
168,485
168,604
-0.07%
ok
l2_hit_async_metrics
168,728
168,647
+0.05%
ok
local_hit
3,142
3,142
+0.00%
ok
minimal_l1_hit
2,991
2,991
+0.00%
ok
miss
220,400
221,027
-0.28%
ok
object_cache_put_full
172,722
172,724
-0.00%
ok
secure_l1_hit
128,097
128,052
+0.04%
ok
serializer_arrow
1,927,817
1,929,613
-0.09%
ok
serializer_auto
53,408
53,389
+0.04%
ok
serializer_default
48,515
48,496
+0.04%
ok
serializer_default_records
1,026,166
1,026,309
-0.01%
ok
serializer_encrypted
90,546
90,538
+0.01%
ok
serializer_orjson
22,022
22,024
-0.01%
A path fails at +1% or more against the base. This check is not required: a deliberate cost increase is accepted at merge. Method: docs/performance.md, Instruction budgets.
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
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.
![kodus-27b[bot]](https://avatars.githubusercontent.com/u/223823690?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Description
AutoSerializernow refuses at write twodatetimevalues that would read back as another value:Timestampwith non-zero nanoseconds raisesTypeError, bare or inside any container. Its ISO text kept all nine digits, butdatetime.fromisoformatkeeps six, so a lone value came back truncated, Timestamps 1 ns apart in a set collapsed into one element with no error, and nine of them, or two frozensets of them, failed every read under the set bound from #721.pd.NaTis adatetimebut not aTimestamp, so it is never refused.datetimewhose UTC offset depends onfold, a wall time its zone repeats or skips, at eitherfold, raisesTypeError, whether thedatetimeis an element or sits at any depth inside one of the set's tuples. PEP 495 makes such adatetimeunequal to everydatetimein another zone, but it is stored with its offset and withoutfold, so it reads back as a fixed-offsetdatetimethat can equal another element. Withdthe first 01:30 of New York's repeated hour,{d, d.astimezone(UTC)}wrote two elements and read back one; atfold=1,{frozenset({d1}), frozenset({utc})}failed every read. Naive and fixed-offset (UTC included)datetimes are never refused. Outside a set the samedatetimewrites the same bytes as before.No stored format changes. Every value that is not refused writes byte-identical payloads, and tests pin the bytes for a zero-nanosecond
Timestamp, a plaindatetime, and afold=1datetimebare, in a list and as a dict value. Keeping nanoseconds orfoldwould need a new stored field on the__datetime__marker the protocol shares, so it is out of this change.The refusal message says what to do:
ts.floor("us")orts.valuefor aTimestamp,dt.astimezone(timezone.utc)for a fold-dependentdatetime.Docs
docs/serializers/auto.md: the "What It Preserves" row saysdatetimeround-trips to the microsecond, and a new paragraph states theTimestampbehaviour (exact to the microsecond, sub-microsecond refused at write,pd.NaTwritten but unreadable). The "Set Size Bound" residual paragraph now says the writer refuses these sets, and names them only as entries an earlier version wrote.docs/error-codes.md: "Serialization unsupported type" lists the two newTypeErrors; the set-bound paragraph of "Deserialization failed" names these sets as written by an earlier version.Cost
The
foldcheck walks a set's elements and its tuples a level at a time in C passes; onlydatetimes outside a fixed-offset zone reach Python, which callsutcoffset()at both folds. It is skipped for a set of onlystrandbytes. Measured onserialize()of one set, guard on against guard off: 1,024 UTC datetimes +5%, 1,024America/New_Yorkdatetimes +31% (about 0.4 ms), 1,024 nested 3-tuples +14%, 100k strings no change. Reads are unchanged.Testing
tests/unit/test_auto_serializer_new_types.py: the old test that asserted the 8-Timestamp collapse and the read-side refusal now assertsTypeErrorat write for the set of 8, the set of 9 and the frozenset pair. A newTestAutoSerializerDatetimeRoundTrippins the nanosecond refusal in each container,pd.NaT, the byte-identical payloads, eight fold-dependent set shapes (fold=1andfold=0in a repeated hour, both folds in a skipped hour, bare, in tuples and in frozensets), and that naive, fixed-offset and unambiguous zoneddatetimes still round-trip.pytest tests --ignore=tests/fuzzing -m "not slow" -n auto: 8624 passed, 109 skipped.ruff check,ruff format --checkandbasedpyrightpass.Closes LAB-12972