Skip to content

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

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

refactor(models): centralize account-trade timestamp parsing#125
naruto11eth wants to merge 2 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
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.py and wires OpenOrder, ClobTrade, and Notification datetime fields through shared RequiredEpochOrIsoTimestamp / EpochOrIsoTimestamp types from _validators.py.

The shared _parse_epoch_or_iso_timestamp now 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.

`_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.
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.

1 participant