refactor(models): centralize account-trade timestamp parsing#125
Open
naruto11eth wants to merge 2 commits into
Open
refactor(models): centralize account-trade timestamp parsing#125naruto11eth wants to merge 2 commits into
naruto11eth wants to merge 2 commits into
Conversation
`_parse_epoch_or_iso_timestamp` now coerces a naive ISO string to UTC, matching how epoch inputs are parsed (`tz=UTC`). Keeps the account models' prior behavior intact when they move onto this parser, and makes `BuilderTrade` timestamps consistently tz-aware.
Drop the local `_parse_epoch`/`_parse_optional_epoch` duplicate in favor of the shared `RequiredEpochOrIsoTimestamp`/`EpochOrIsoTimestamp` validators. Behavior is preserved for all realistic inputs; negative-string epochs are now rejected (trade timestamps are never negative), locked in by a test.
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.
Removes the duplicated timestamp parser in
models/clob/account.pyin favor of the shared validators inmodels/clob/_validators.py(DEV-205)._parse_epoch/_parse_optional_epoch/_EPOCH_MS_THRESHOLD; the five datetime fields (created_at,expires_at,matched_at,updated_at,timestamp) now use the sharedRequiredEpochOrIsoTimestamp/EpochOrIsoTimestampannotated types._parse_epoch_or_iso_timestampso account behavior is preserved (it previously didreplace(tzinfo=UTC)locally).Behavior is preserved for all realistic inputs (epoch s/ms as int or string, ISO with
Z, naive ISO, offset ISO). Two intentional refinements on inputs that can't legitimately occur for these fields:"-1") are now rejected (the shared parser accepts only unsigned digit strings) — trade/order timestamps are never negative; locked in by a new test.BuilderTrade(the other consumer of the shared parser) now coerces a naive ISO timestamp to UTC instead of leaving it naive — consistent, tz-aware datetimes; its tests useZ-suffixed inputs so they're unaffected.Full unit suite green; ruff + pyright strict clean.
Refs DEV-205
Note
Low Risk
Refactor of model validation with behavior preserved for normal API inputs; minor tightening on invalid edge-case strings and naive ISO handling for other shared-parser consumers.
Overview
Removes duplicate epoch/ISO timestamp parsing from
account.pyand wires OpenOrder, ClobTrade, and Notification datetime fields through sharedRequiredEpochOrIsoTimestamp/EpochOrIsoTimestamptypes from_validators.py.The shared
_parse_epoch_or_iso_timestampnow assigns UTC to naive ISO strings (previously done only in account), so all consumers get timezone-aware datetimes. Negative numeric strings (e.g."-1") are no longer accepted as epochs—only unsigned digit strings—matching realistic API timestamps; covered by a new ClobTrade test.Reviewed by Cursor Bugbot for commit 456307c. Bugbot is set up for automated code reviews on this repo. Configure here.