Skip to content

fix(vars): compare timezone-aware datetimes by instant#6767

Open
anxkhn wants to merge 1 commit into
reflex-dev:mainfrom
anxkhn:fix/datetime-timezone-comparison
Open

fix(vars): compare timezone-aware datetimes by instant#6767
anxkhn wants to merge 1 commit into
reflex-dev:mainfrom
anxkhn:fix/datetime-timezone-comparison

Conversation

@anxkhn

@anxkhn anxkhn commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Timezone-aware datetime Vars were serialized with their UTC offsets and compared
directly as JavaScript strings. Equal instants with different offsets could compare
unequal, and string ordering could disagree with chronological ordering.

This change:

  • compares datetime operands by their JavaScript epoch-millisecond timestamps for
    equality and ordering
  • preserves the existing equality behavior for non-datetime operands
  • adds unit and Playwright regression coverage for all six comparison operators

Testing

  • uv run pytest tests/units/test_var.py -q (215 passed)
  • uv run pytest tests/units --cov --no-cov-on-fail --cov-report= (6631 passed,
    17 skipped, 78.10% coverage)
  • uv run pytest tests/integration/tests_playwright/test_datetime_operations.py -q
    (1 passed)
  • uv run ruff check .
  • uv run ruff format --check .
  • uv run pyright reflex tests (0 errors)
  • uv run pre-commit run --files packages/reflex-base/src/reflex_base/vars/datetime.py packages/reflex-base/news/6755.bugfix.md tests/units/test_var.py tests/integration/tests_playwright/test_datetime_operations.py
  • uv run towncrier check --config pyproject.toml --dir packages/reflex-base --compare-with origin/main

The comparison regression tests fail against the previous implementation and pass
with this change.

Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
@anxkhn anxkhn requested a review from a team as a code owner July 15, 2026 04:36

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 69ca079a2d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

"""
return var_operation_return(
f"({lhs} {'<' if strict else '<='} {rhs})",
f"(new Date({lhs}).getTime() {operator} new Date({rhs}).getTime())",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve microsecond precision in datetime comparisons

When either datetime has sub-millisecond precision, converting through Date.getTime() drops the microseconds that Reflex already serializes (for example, 2024-01-01 00:00:00.000001+00:00 and .000999+00:00 both become the same millisecond timestamp in JavaScript). In that scenario == now renders true and ordering comparisons render false even though the Python datetime instants differ, so apps comparing high-precision timestamps can get incorrect results.

Useful? React with 👍 / 👎.

@greptile-apps

greptile-apps Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes timezone-aware DateTimeVar comparisons by replacing direct JavaScript string comparison with new Date(x).getTime(), which normalizes both sides to epoch milliseconds so that equal instants with different UTC offsets compare correctly.

  • Rewrites date_compare_operation to emit new Date(lhs).getTime() OP new Date(rhs).getTime() for all six operators, and fixes the previously swapped lhs/rhs in date_gt_operation and date_ge_operation.
  • Adds __eq__/__ne__ overrides to DateTimeVar (with __hash__ = Var.__hash__ to preserve hashability) that use timestamp comparison for datetime operands and fall back to generic Var equality for non-datetime operands.
  • Adds unit and Playwright regression tests covering all six comparison operators across timezone-offset variants.

Confidence Score: 5/5

Safe to merge — the change is tightly scoped to datetime comparison codegen with solid unit and integration test coverage.

The fix is logically sound: wrapping both sides in getTime() correctly normalizes timezone offsets to epoch milliseconds, the previously swapped lhs/rhs in the GT/GE helpers is corrected, and hashability is preserved. Tests cover all six operators for both the new timestamp path and the non-datetime fallback path. No pre-existing behavior for non-timezone-aware datetimes is regressed.

No files require special attention.

Important Files Changed

Filename Overview
packages/reflex-base/src/reflex_base/vars/datetime.py Rewrites all six datetime comparison operations to use JS getTime() for epoch-millisecond comparison; adds eq/ne overrides with fallback to generic Var equality for non-datetime operands; fixes the previously swapped lhs/rhs in date_gt_operation and date_ge_operation; adds hash = Var.hash to preserve hashability.
tests/units/test_var.py Adds parametrized unit tests verifying all six comparison operators emit timestamp-based JS expressions, plus a test confirming the non-datetime fallback path uses the generic Var equality.
tests/integration/tests_playwright/test_datetime_operations.py Adds three timezone-aware state vars (UTC, +01:00, +02:00) and six Playwright assertions confirming equal-instant comparisons produce correct boolean results in the browser.
packages/reflex-base/news/6755.bugfix.md Towncrier changelog entry for the bugfix.

Reviews (2): Last reviewed commit: "fix(vars): compare timezone-aware dateti..." | Re-trigger Greptile

Comment thread packages/reflex-base/src/reflex_base/vars/datetime.py
@codspeed-hq

codspeed-hq Bot commented Jul 15, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 26 untouched benchmarks
⏩ 8 skipped benchmarks1


Comparing anxkhn:fix/datetime-timezone-comparison (69ca079) with main (65a2889)2

Open in CodSpeed

Footnotes

  1. 8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on main (127e4d8) during the generation of this report, so 65a2889 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@Alek99 Alek99 closed this Jul 15, 2026
@Alek99 Alek99 reopened this Jul 15, 2026
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