Skip to content

docs(telemetry): document the seconds-vs-ms epoch boundary assumption (#121) - #126

Merged
KRoperUK merged 2 commits into
mainfrom
fix/121-timestamp-boundary
Oct 17, 2026
Merged

KRoperUK merged 2 commits into
mainfrom
fix/121-timestamp-boundary

Conversation

@KRoperUK

@KRoperUK KRoperUK commented Oct 9, 2026

Copy link
Copy Markdown
Owner

Summary

_coerce_timestamp disambiguates seconds from milliseconds purely by magnitude (> 1e12 → divide by 1000). That's safe for the data it parses — a Unix seconds epoch doesn't reach 1e12 until the year ~33658, and the cloud emits second-resolution TS — but the assumption was undocumented, so a future reader could mistake the one-sided heuristic for a bug. Closes #121 (low priority, as filed).

Per the issue's accepted resolution ("a comment documenting the boundary assumption, or a test pinning the split"), this does both:

  • A comment explaining why 1e12 is safe for second-epochs, and that the converse edge (a pre-2001 millisecond epoch < 1e12, which the cloud has never produced) is intentionally not defended.
  • A test pinning each side of the threshold. It also captures a non-obvious truth surfaced while writing it: a value just below 1e12, read as seconds, lands in year ~33658 which datetime cannot represent, so the existing OverflowError/ValueError guard drops it (returns None) rather than silently mis-parsing.

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation / chore (comment + characterization test; no behaviour change)

Checklist

  • ruff check + ruff format pass
  • mypy strict passes (9 source files)
  • pytest passes (201); the new test pins the boundary behaviour
  • No behaviour change — this documents and guards existing behaviour

Behaviour changes worth calling out

None. Comment + test only.

_coerce_timestamp disambiguates seconds from milliseconds purely by magnitude
(> 1e12 is treated as ms). This is safe for the data it parses -- a seconds
epoch does not reach 1e12 until the year ~33658 and the cloud emits
second-resolution TS -- but the assumption was undocumented, so a future reader
could mistake the one-sided heuristic for a bug.

Add a comment explaining why the threshold is safe and that the converse edge
(a pre-2001 ms epoch) is intentionally not defended, and a test pinning each
side of the 1e12 boundary (including that a near-boundary seconds value
overflows datetime and is dropped rather than mis-parsed).

Closes #121
@KRoperUK
KRoperUK merged commit 4a13e64 into main Oct 17, 2026
10 checks passed
@KRoperUK
KRoperUK deleted the fix/121-timestamp-boundary branch October 17, 2026 22:34
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.

_coerce_timestamp seconds-vs-ms heuristic is magnitude-only (latent edge, low priority)

1 participant