Skip to content

Fix datetime conversion to normalize naive datetimes to UTC#8

Draft
Copilot wants to merge 6 commits into
mainfrom
copilot/fix-chrono-conversion-issues
Draft

Fix datetime conversion to normalize naive datetimes to UTC#8
Copilot wants to merge 6 commits into
mainfrom
copilot/fix-chrono-conversion-issues

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Oct 30, 2025

Naive Python datetimes were being reconstructed from Y/M/D/h/m/s/μs components without timezone context, causing them to be interpreted as local time. This triggered "End time cannot be before the start time" errors in CI when system timezone != UTC.

Changes

include/hgraph/python/chrono.h:

  • from_python: Use datetime.timestamp() for POSIX seconds conversion; attach timezone.utc to naive datetimes before calling timestamp()
  • from_cpp: Return timezone-aware datetimes via datetime.fromtimestamp(ts, tz=timezone.utc)
  • Both methods fall back to original component-based logic if Python API calls fail

Tests:

  • test_timestamp_logic.py: Validates UTC normalization, timezone handling, microsecond precision
  • test_chrono_utc.py: Integration tests for round-trip conversion

Memory management: All PyObject* references properly DECREFed in all code paths.

Example

# Before: naive datetime treated as local time (EST = UTC-5)
naive_dt = datetime(2020, 1, 1, 0, 0, 0)  
# -> 2020-01-01 05:00:00 UTC (wrong)

# After: naive datetime explicitly treated as UTC
naive_dt = datetime(2020, 1, 1, 0, 0, 0)  
# -> 2020-01-01 00:00:00 UTC (correct)
Original prompt

Summary:

Many unit tests fail in CI with ValueError: "End time cannot be before the start time" due to inconsistent conversion of Python datetime objects to C++ std::chrono::system_clock::time_point values. The existing nanobind type_caster in include/hgraph/python/chrono.h reconstructs time_point from unpacked Y/M/D/h/m/s/μs components and ignores Python tzinfo, causing naive datetimes to be interpreted inconsistently across environments and timezone shifts on CI.

Goal:

Create a PR that updates the C++ <-> Python chrono conversions to explicitly normalize datetimes to UTC:

  • Use Python's datetime.timestamp() to obtain a POSIX seconds value in UTC (which respects tz-aware datetimes).
  • For naive datetimes (tzinfo is None), explicitly attach datetime.timezone.utc before calling timestamp(), because the project expects all times to be UTC.
  • Convert the resulting POSIX seconds (double) to the target std::chrono::time_point Duration.
  • Update the from_cpp conversion to return a timezone-aware Python datetime in UTC via datetime.datetime.fromtimestamp(ts, tz=datetime.timezone.utc) to make round-trip explicit.
  • Preserve fallbacks: if timestamp() is not available or the conversion fails, fall back to the existing unpack_datetime packing/unpacking logic.

Files to change:

  • include/hgraph/python/chrono.h
    • Replace the from_python and from_cpp implementations in the type_caster specialization for std::chrono::time_point<std::chrono::system_clock, Duration> with the timestamp-based logic and the UTC-aware from_cpp logic.

Patch details (high level):

  • In from_python:

    • Import Python's datetime module using nanobind (nb::module_ datetime = nb::module_::import("datetime");).
    • If src.tzinfo is None, call src.replace(tzinfo=datetime.timezone.utc).
    • Call src.timestamp() and cast to double.
    • Convert to std::chrono::duration and then to the target Duration, assign to value.
    • If any exception occurs, fall back to unpack_datetime() as before.
  • In from_cpp:

    • Compute seconds since epoch as double: ch::duration(src.time_since_epoch()).count().
    • Build a Python datetime via datetime.datetime.fromtimestamp(ts, datetime.timezone.utc) and return that handle.
    • If this fails, fall back to existing component-based pack_datetime.

Why this fixes the CI failures:

  • Using Python's timestamp() ensures proper handling of tz-aware datetimes (converts to UTC correctly), and explicitly attaching UTC to naive datetimes makes conversions deterministic across environments. This prevents timezone-induced shifts that previously caused end < start comparisons to fail in CI.

Testing and validation:

  • Rebuild the C++ extension and run the Python unit test suite (especially tests under tests/python/unit/engine/). The failing tests in the CI run are primarily in that folder and should stop failing once conversions are normalized.
  • Add unit tests for the type caster (or a small echo helper) to confirm round-trip behavior for:
    • tz-aware UTC datetimes
    • tz-aware non-UTC datetimes (e.g., UTC+2)
    • naive datetimes (should be treated as UTC)

Additional notes:

  • Keep string_util.cpp usage of std::gmtime (UTC) as-is; no changes required.
  • Consider setting TZ=UTC in CI temporarily as a stop-gap, but the primary fix is to make the conversion deterministic in code.

References:

Please implement the change by creating a branch and opening a PR with these edits. The code snippet and exact replacement have been prepared and should be included in the PR commit. No assignee is required.

This pull request was created as a result of the following prompt from Copilot chat.

Summary:

Many unit tests fail in CI with ValueError: "End time cannot be before the start time" due to inconsistent conversion of Python datetime objects to C++ std::chrono::system_clock::time_point values. The existing nanobind type_caster in include/hgraph/python/chrono.h reconstructs time_point from unpacked Y/M/D/h/m/s/μs components and ignores Python tzinfo, causing naive datetimes to be interpreted inconsistently across environments and timezone shifts on CI.

Goal:

Create a PR that updates the C++ <-> Python chrono conversions to explicitly normalize datetimes to UTC:

  • Use Python's datetime.timestamp() to obtain a POSIX seconds value in UTC (which respects tz-aware datetimes).
  • For naive datetimes (tzinfo is None), explicitly attach datetime.timezone.utc before calling timestamp(), because the project expects all times to be UTC.
  • Convert the resulting POSIX seconds (double) to the target std::chrono::time_point Duration.
  • Update the from_cpp conversion to return a timezone-aware Python datetime in UTC via datetime.datetime.fromtimestamp(ts, tz=datetime.timezone.utc) to make round-trip explicit.
  • Preserve fallbacks: if timestamp() is not available or the conversion fails, fall back to the existing unpack_datetime packing/unpacking logic.

Files to change:

  • include/hgraph/python/chrono.h
    • Replace the from_python and from_cpp implementations in the type_caster specialization for std::chrono::time_point<std::chrono::system_clock, Duration> with the timestamp-based logic and the UTC-aware from_cpp logic.

Patch details (high level):

  • In from_python:

    • Import Python's datetime module using nanobind (nb::module_ datetime = nb::module_::import("datetime");).
    • If src.tzinfo is None, call src.replace(tzinfo=datetime.timezone.utc).
    • Call src.timestamp() and cast to double.
    • Convert to std::chrono::duration and then to the target Duration, assign to value.
    • If any exception occurs, fall back to unpack_datetime() as before.
  • In from_cpp:

    • Compute seconds since epoch as double: ch::duration(src.time_since_epoch()).count().
    • Build a Python datetime via datetime.datetime.fromtimestamp(ts, datetime.timezone.utc) and return that handle.
    • If this fails, fall back to existing component-based pack_datetime.

Why this fixes the CI failures:

  • Using Python's timestamp() ensures proper handling of tz-aware datetimes (converts to UTC correctly), and explicitly attaching UTC to naive datetimes makes conversions deterministic across environments. This prevents timezone-induced shifts that previously caused end < start comparisons to fail in CI.

Testing and validation:

  • Rebuild the C++ extension and run the Python unit test suite (especially tests under tests/python/unit/engine/). The failing tests in the CI run are primarily in that folder and should stop failing once conversions are normalized.
  • Add unit tests for the type caster (or a small echo helper) to confirm round-trip behavior for:
    • tz-aware UTC datetimes
    • tz-aware non-UTC datetimes (e.g., UTC+2)
    • naive datetimes (should be treated as UTC)

Additional notes:

  • Keep string_util.cpp usage of std::gmtime (UTC) as-is; no changes required.
  • Consider setting TZ=UTC in CI temporarily as a stop-gap, but the primary fix is to make the conversion deterministic in code.

References:

Please implement the change by creating a branch and opening a PR with these edits. The code snippet and exact replacement have been prepared and should be included in the PR commit. No assignee is required.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 5 commits October 30, 2025 02:55
Co-authored-by: hhenson <2552364+hhenson@users.noreply.github.com>
Co-authored-by: hhenson <2552364+hhenson@users.noreply.github.com>
Co-authored-by: hhenson <2552364+hhenson@users.noreply.github.com>
Co-authored-by: hhenson <2552364+hhenson@users.noreply.github.com>
Co-authored-by: hhenson <2552364+hhenson@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix datetime conversion issues between Python and C++ Fix datetime conversion to normalize naive datetimes to UTC Oct 30, 2025
Copilot AI requested a review from hhenson October 30, 2025 03:09
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