Skip to content

refactor(models): centralize account-trade timestamp parsing#125

Open
naruto11eth wants to merge 3 commits into
mainfrom
feature/dev-205-centralize-account-timestamp
Open

refactor(models): centralize account-trade timestamp parsing#125
naruto11eth wants to merge 3 commits into
mainfrom
feature/dev-205-centralize-account-timestamp

Conversation

@naruto11eth

@naruto11eth naruto11eth commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Removes the duplicated timestamp parser in models/clob/account.py in favor of the shared validators in models/clob/_validators.py (DEV-205).

  • Deletes the local _parse_epoch / _parse_optional_epoch / _EPOCH_MS_THRESHOLD; the five datetime fields (created_at, expires_at, matched_at, updated_at, timestamp) now use the shared RequiredEpochOrIsoTimestamp / EpochOrIsoTimestamp annotated types.
  • Moves the "naive ISO string → UTC" rule into the shared _parse_epoch_or_iso_timestamp so account behavior is preserved (it previously did replace(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:

  • Negative-string epochs (e.g. "-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 use Z-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.py and wires account CLOB models to the shared RequiredEpochOrIsoTimestamp / EpochOrIsoTimestamp validators in _validators.py.

Shared parser: _parse_epoch_or_iso_timestamp now 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, and Notification drop per-field @field_validator hooks; 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.

`_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.
type: int
payload: Any = None
timestamp: datetime
timestamp: RequiredEpochOrIsoTimestamp

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how does the type on this model looks like? As we aim at normalizing it should appear as datetime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants