refactor(models): centralize account-trade timestamp parsing#125
Open
naruto11eth wants to merge 3 commits into
Open
refactor(models): centralize account-trade timestamp parsing#125naruto11eth wants to merge 3 commits into
naruto11eth wants to merge 3 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.
cesarenaldi
reviewed
Jun 30, 2026
| type: int | ||
| payload: Any = None | ||
| timestamp: datetime | ||
| timestamp: RequiredEpochOrIsoTimestamp |
Collaborator
There was a problem hiding this comment.
how does the type on this model looks like? As we aim at normalizing it should appear as datetime.
…alize-account-timestamp
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
Validation-only refactor in API response models; behavior changes are limited to invalid or edge-case inputs and are covered by existing and new unit tests.
Overview
Removes duplicated epoch/ISO timestamp parsing from
models/clob/account.pyand wires account CLOB models to the sharedRequiredEpochOrIsoTimestamp/EpochOrIsoTimestampvalidators in_validators.py.Shared parser:
_parse_epoch_or_iso_timestampnow treats naive ISO strings as UTC (aligned with epoch parsing and the old account-local behavior). Other consumers of the same helper (e.g. builder trade models) get UTC-aware datetimes for naive ISO inputs.Account models:
OpenOrder,ClobTrade, andNotificationdrop per-field@field_validatorhooks; timestamp fields are declared with the annotated types only.Stricter edge case: unsigned digit strings only for string epochs—values like
"-1"are rejected; covered by a new unit test.Reviewed by Cursor Bugbot for commit 15258c6. Bugbot is set up for automated code reviews on this repo. Configure here.