fix: negate the day range in Timestamp != date filters - #655
Merged
Conversation
Timestamp.__ne__'s date branch called self.between(start, end) — the same
positive range construction as __eq__ — so `Timestamp("f") != date(...)`
matched the entire day its docstring says it excludes, silently returning
exactly the rows the caller asked to filter out. The non-date path already
negates via FilterOperator.NE, as do the NE forms of Tag, Text, Num and Geo.
Compute the day bounds exactly as __eq__ does (including the astimezone
conversion the date branch of __ne__ was also missing) and emit them through
the existing NE operator template, making `!= date` the exact string negation
of `== date` on any machine. Date-only ISO strings take the same path.
vishal-bala
self-requested a review
August 6, 2026 08:24
vishal-bala
approved these changes
Aug 6, 2026
vishal-bala
left a comment
vishal-bala
left a comment
Collaborator
There was a problem hiding this comment.
Looks good to me! Thanks for the contribution 🙌
vishal-bala
added a commit
that referenced
this pull request
Aug 7, 2026
## What this fixes
`Timestamp`'s class docstring states that "All timestamps are converted
to Unix timestamps in UTC for consistency." That holds for datetimes,
ISO strings, and Unix timestamps — but not for bare `date` objects (or
date-only ISO strings). Those were anchored to the **host's local
timezone**, so the same query produced different day boundaries
depending on where it ran:
```python
d = date(2023, 3, 17)
Timestamp("f") == d # TZ-dependent
Timestamp("f") == datetime(2023, 3, 17, 0, 0) # always UTC
```
| `TZ` | `== date(2023,3,17)` starts at | naive `datetime` / `between()`
starts at |
| --- | --- | --- |
| `UTC` | `1679011200.0` | `1679011200.0` |
| `America/New_York` | `1679025600.0` (+4h) | `1679011200.0` |
| `Asia/Tokyo` | `1678978800.0` (−9h) | `1679011200.0` |
A `date` and its equivalent naive `datetime` resolved to different
instants unless the machine ran on UTC.
## Root cause
Two code paths converted naive datetimes with opposite conventions.
`_convert_to_timestamp` treats a naive datetime as **already UTC**
(`filter.py:751-756`):
```python
if value.tzinfo is None:
value = value.replace(tzinfo=datetime.timezone.utc)
```
The `__eq__`/`__ne__` date branches instead called `.astimezone(utc)` on
a naive datetime, which Python interprets as **local time** before
converting:
```python
start = datetime.datetime.combine(other, datetime.time.min).astimezone(
datetime.timezone.utc
)
```
## The fix
`_convert_to_timestamp` already derives day bounds from a bare `date` —
that's what the `end_date` flag is for, combining with
`time.min`/`time.max` — and `between()` already calls it correctly for
both bounds. So the manual combine was duplicated logic with divergent
semantics, and the fix is to delete it and pass the date straight
through:
```python
if self._is_date(other):
if isinstance(other, str):
other = datetime.datetime.strptime(other, "%Y-%m-%d").date()
assert isinstance(other, datetime.date)
return self.between(other, other)
```
Net −13 lines in `filter.py`.
One subtlety worth flagging for review: the `strptime` normalization has
to stay. `_convert_to_timestamp` parses `"2023-03-17"` with
`fromisoformat`, which returns a *datetime*, not a `date` — so the
`end_date` branch would never fire and the upper bound would collapse to
midnight. Normalizing to a `date` first keeps date-only strings on the
same path as `date` objects.
`==` and `!=` remain exact negations of each other, preserving the
invariant established in #655.
## Verification
Timezone-stable where it previously was not:
| `TZ` | before | after |
| --- | --- | --- |
| `UTC` | `[1679011200.0 1679097599.999999]` | `[1679011200.0
1679097599.999999]` |
| `America/New_York` | `[1679025600.0 1679111999.999999]` |
`[1679011200.0 1679097599.999999]` |
| `Asia/Tokyo` | `[1678978800.0 1679065199.999999]` | `[1679011200.0
1679097599.999999]` |
## Tests
Three existing tests (`test_timestamp_date`,
`test_timestamp_not_equal_date`, `test_timestamp_iso_string`) computed
their expected values with the same `.astimezone(utc)` idiom as the
implementation, so they mirrored the bug and passed regardless. They now
specify the intended UTC semantics directly, and `test_timestamp_date`
additionally asserts a hard-coded epoch literal (`1679011200.0` /
`1679097599.999999`) as ground truth independent of any formula.
Added `test_timestamp_date_bounds_are_utc_regardless_of_local_timezone`.
This is the test that actually pins the fix — CI sets no `TZ` and GitHub
runners default to UTC, where the correct and the buggy conversions
agree, so nothing else in the suite would catch a regression to
local-day bounds. It parametrizes over five dates (an ordinary date,
both US DST transitions, the Unix epoch, and a post-2038 date) and
asserts identical output under `UTC`, `America/New_York`, `Asia/Tokyo`,
`Asia/Kathmandu` (+05:45), and `Pacific/Chatham` (+12:45/+13:45) — the
non-hour offsets would catch an hour-granularity mistake that whole-hour
zones alone would not. Expected values come from `calendar.timegm`, an
oracle that shares no code with the conversion path under test. It
restores `TZ` and calls `tzset()` in a `finally` block so a mid-loop
failure cannot leak a changed zone into later tests, and is skipped
where `tzset()` is unavailable.
All of these fail on `main` and pass here.
`tests/unit/test_filter.py`: 68/68 pass. Full `tests/unit` (excluding
`test_mcp`): 991 passed, 1 skipped, with 2 failures and 28 errors
identical on unmodified `main` in my environment (missing optional
`pydantic_settings` / `google-genai` deps). `black`, `isort`, `mypy`,
and `codespell` clean via `pre-commit`.
## Documentation
The `__eq__`/`__ne__` docstrings previously said date inputs match "the
entire day" — the same ambiguity that let this bug exist. They now name
the UTC calendar day explicitly, as does the class docstring.
`_convert_to_timestamp` now documents its `end_date` parameter, which
the fix relies on to derive the upper bound, and records that naive
datetimes are read as UTC.
Note that `Timestamp` is not autodocumented in `docs/api/filter.rst`
(only `FilterExpression`, `Tag`, `Text`, `Num`, `Geo`, `GeoRadius` are),
so these docstrings currently reach IDE and source readers but not the
published API docs. That gap predates this PR and is worth a separate
follow-up.
## Behavior change
This changes results for non-UTC clients using `date` filters — the
window moves by the host's offset. Output is unchanged for clients
already on UTC. Worth a release-note callout; anyone depending on
local-day semantics can pass explicit tz-aware `datetime` bounds to
`between()`.
No docs or integration tests exercise the `date` path (all use
`datetime`), so nothing else needed updating.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Changes query semantics for date-based `Timestamp` filters on non-UTC
hosts (intentional fix), which can alter production search results until
callers adapt or use explicit tz-aware `between()` bounds.
>
> **Overview**
> **Fixes inconsistent day boundaries** when filtering on bare `date`
values or date-only ISO strings (`YYYY-MM-DD`). Those paths previously
built bounds with naive `datetime.combine` + `.astimezone(UTC)`, which
Python treats as **local** time, while other `Timestamp` inputs already
convert as **UTC**.
>
> `==` / `!=` on dates now delegate to the same logic as
`between()`—passing the `date` through `_convert_to_timestamp` (with
`end_date` for the upper bound on `!=`) so the window is always
**00:00:00–23:59:59.999999 UTC** for that calendar day. Docstrings now
state UTC-day semantics explicitly.
>
> Tests were updated to expect UTC bounds (including fixed epoch
literals) and a new POSIX-only test varies `TZ` across several zones to
ensure output does not depend on the host timezone.
>
> **Behavior change:** clients not on UTC will see different query
ranges for date filters; UTC hosts are unchanged.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
a260840. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
|
🚀 PR was released in |
applied-ai-release-bot
Bot
added
the
released
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
Timestamp.__ne__'s docstring promises: "For date objects (without time), this excludes the entire day." The implementation calledself.between(start, end)— the same positive range construction as__eq__— so the filter matched the entire day instead:Silent inverted filtering:
!=with a date returns exactly the rows the caller asked to exclude, with no error or warning. The non-date path of the same method already negates correctly viaFilterOperator.NE((-@created_at:[v v])), as do theNEforms ofTag,Text,Num, andGeo— the date branch was the one exception.There was a second, smaller asymmetry hiding in the same branch:
__eq__computes its day bounds with.astimezone(timezone.utc)while__ne__used baredatetime.combine, so the two methods disagreed about where the day starts on machines whose local timezone isn't UTC.The fix
Compute the day bounds exactly as
__eq__does (including theastimezoneconversion), convert with_convert_to_timestampexactly asbetween()does, and emit through the existingOPERATOR_MAP[FilterOperator.NE]template. The invariant this establishes:str(ts != d)is byte-for-bytef"(-{ts == d})"on any machine, for bothdateobjects and date-only ISO strings ("2023-03-17"), which take the same branch. The datetime/unix path is untouched.Tests
Added
test_timestamp_not_equal_date, which asserts the negated-range form, the exact-negation-of-__eq__invariant, and the ISO-string path. The existing suite covered!=only with adatetime(the non-date path), which is why this never failed. The new test fails on currentmainand passes with the fix.tests/unit/test_filter.py: 63/63 pass. Fulltests/unit: 1168 passed, 1 skipped — with 6 errors identical on unmodifiedmainin my environment (Docker-dependent CLI fixtures; no daemon locally).black,isort, andmypyclean.Notes
auto:patchlabel per CONTRIBUTING — I can't set labels as an outside contributor.filter.pywhile working on docs: add exception reference and error handling guide #654 (exception docs, still open) — happy to rebase either PR if they land in either order; they don't overlap.Note
Medium Risk
Changes query semantics for a previously broken filter path; callers relying on the old (incorrect) behavior would see different results, but the fix aligns with documented behavior and other filter types.
Overview
Fixes
Timestamp != date(and date-only ISO strings) so they exclude the full day instead of matching it. The date branch no longer callsbetween(); it builds the same UTC day bounds as==, converts them with_convert_to_timestamp, and emits a negated Redis range viaFilterOperator.NE—sostr(ts != d)matches(-{ts == d}).Adds
test_timestamp_not_equal_dateto lock in the negated query shape, the==/!=pairing, and the ISO-string path.Reviewed by Cursor Bugbot for commit 1157c08. Bugbot is set up for automated code reviews on this repo. Configure here.