fix(vars): compare timezone-aware datetimes by instant#6767
Conversation
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
There was a problem hiding this comment.
💡 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())", |
There was a problem hiding this comment.
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 SummaryThis PR fixes timezone-aware
Confidence Score: 5/5Safe 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
Reviews (2): Last reviewed commit: "fix(vars): compare timezone-aware dateti..." | Re-trigger Greptile |
Merging this PR will not alter performance
Comparing Footnotes
|
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:
equality and ordering
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.pyuv run towncrier check --config pyproject.toml --dir packages/reflex-base --compare-with origin/mainThe comparison regression tests fail against the previous implementation and pass
with this change.