Skip to content

Commit f818702

Browse files
phernandezclaude
andauthored
fix: enforce minimum 1-day timeframe for recent_activity to handle timezone issues (#318)
Signed-off-by: phernandez <[email protected]> Co-authored-by: Claude <[email protected]>
1 parent 2efd8f4 commit f818702

File tree

7 files changed

+171
-35
lines changed

7 files changed

+171
-35
lines changed

pyproject.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ dev-dependencies = [
7272
"pytest-asyncio>=0.24.0",
7373
"pytest-xdist>=3.0.0",
7474
"ruff>=0.1.6",
75+
"freezegun>=1.5.5",
7576
]
7677

7778
[tool.hatch.version]

src/basic_memory/schemas/base.py

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
import os
1515
import mimetypes
1616
import re
17-
from datetime import datetime, time
17+
from datetime import datetime, time, timedelta
1818
from pathlib import Path
1919
from typing import List, Optional, Annotated, Dict
2020

@@ -52,21 +52,27 @@ def to_snake_case(name: str) -> str:
5252
def parse_timeframe(timeframe: str) -> datetime:
5353
"""Parse timeframe with special handling for 'today' and other natural language expressions.
5454
55+
Enforces a minimum 1-day lookback to handle timezone differences in distributed deployments.
56+
5557
Args:
5658
timeframe: Natural language timeframe like 'today', '1d', '1 week ago', etc.
5759
5860
Returns:
5961
datetime: The parsed datetime for the start of the timeframe, timezone-aware in local system timezone
62+
Always returns at least 1 day ago to handle timezone differences.
6063
6164
Examples:
62-
parse_timeframe('today') -> 2025-06-05 00:00:00-07:00 (start of today with local timezone)
65+
parse_timeframe('today') -> 2025-06-04 14:50:00-07:00 (1 day ago, not start of today)
66+
parse_timeframe('1h') -> 2025-06-04 14:50:00-07:00 (1 day ago, not 1 hour ago)
6367
parse_timeframe('1d') -> 2025-06-04 14:50:00-07:00 (24 hours ago with local timezone)
6468
parse_timeframe('1 week ago') -> 2025-05-29 14:50:00-07:00 (1 week ago with local timezone)
6569
"""
6670
if timeframe.lower() == "today":
67-
# Return start of today (00:00:00) in local timezone
68-
naive_dt = datetime.combine(datetime.now().date(), time.min)
69-
return naive_dt.astimezone()
71+
# For "today", return 1 day ago to ensure we capture recent activity across timezones
72+
# This handles the case where client and server are in different timezones
73+
now = datetime.now()
74+
one_day_ago = now - timedelta(days=1)
75+
return one_day_ago.astimezone()
7076
else:
7177
# Use dateparser for other formats
7278
parsed = parse(timeframe)
@@ -75,7 +81,18 @@ def parse_timeframe(timeframe: str) -> datetime:
7581

7682
# If the parsed datetime is naive, make it timezone-aware in local system timezone
7783
if parsed.tzinfo is None:
78-
return parsed.astimezone()
84+
parsed = parsed.astimezone()
85+
else:
86+
parsed = parsed
87+
88+
# Enforce minimum 1-day lookback to handle timezone differences
89+
# This ensures we don't miss recent activity due to client/server timezone mismatches
90+
now = datetime.now().astimezone()
91+
one_day_ago = now - timedelta(days=1)
92+
93+
# If the parsed time is more recent than 1 day ago, use 1 day ago instead
94+
if parsed > one_day_ago:
95+
return one_day_ago
7996
else:
8097
return parsed
8198

tests/mcp/test_tool_build_context.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ async def test_get_discussion_context_not_found(client, test_project):
9494

9595
invalid_timeframes = [
9696
"invalid", # Nonsense string
97-
"tomorrow", # Future date
97+
# NOTE: "tomorrow" now returns 1 day ago due to timezone safety - no longer invalid
9898
]
9999

100100

tests/mcp/test_tool_recent_activity.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
invalid_timeframes = [
1818
"invalid", # Nonsense string
19-
"tomorrow", # Future date
19+
# NOTE: "tomorrow" now returns 1 day ago due to timezone safety - no longer invalid
2020
]
2121

2222

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
"""Test minimum 1-day timeframe enforcement for timezone handling."""
2+
3+
from datetime import datetime, timedelta
4+
import pytest
5+
from freezegun import freeze_time
6+
7+
from basic_memory.schemas.base import parse_timeframe
8+
9+
10+
class TestTimeframeMinimum:
11+
"""Test that parse_timeframe enforces a minimum 1-day lookback."""
12+
13+
@freeze_time("2025-01-15 15:00:00")
14+
def test_today_returns_one_day_ago(self):
15+
"""Test that 'today' returns 1 day ago instead of start of today."""
16+
result = parse_timeframe("today")
17+
now = datetime.now()
18+
one_day_ago = now - timedelta(days=1)
19+
20+
# Should be approximately 1 day ago (within a second for test tolerance)
21+
diff = abs((result.replace(tzinfo=None) - one_day_ago).total_seconds())
22+
assert diff < 1, f"Expected ~1 day ago, got {result}"
23+
24+
@freeze_time("2025-01-15 15:00:00")
25+
def test_one_hour_returns_one_day_minimum(self):
26+
"""Test that '1h' returns 1 day ago due to minimum enforcement."""
27+
result = parse_timeframe("1h")
28+
now = datetime.now()
29+
one_day_ago = now - timedelta(days=1)
30+
31+
# Should be approximately 1 day ago, not 1 hour ago
32+
diff = abs((result.replace(tzinfo=None) - one_day_ago).total_seconds())
33+
assert diff < 1, f"Expected ~1 day ago for '1h', got {result}"
34+
35+
@freeze_time("2025-01-15 15:00:00")
36+
def test_six_hours_returns_one_day_minimum(self):
37+
"""Test that '6h' returns 1 day ago due to minimum enforcement."""
38+
result = parse_timeframe("6h")
39+
now = datetime.now()
40+
one_day_ago = now - timedelta(days=1)
41+
42+
# Should be approximately 1 day ago, not 6 hours ago
43+
diff = abs((result.replace(tzinfo=None) - one_day_ago).total_seconds())
44+
assert diff < 1, f"Expected ~1 day ago for '6h', got {result}"
45+
46+
@freeze_time("2025-01-15 15:00:00")
47+
def test_one_day_returns_one_day(self):
48+
"""Test that '1d' correctly returns approximately 1 day ago."""
49+
result = parse_timeframe("1d")
50+
now = datetime.now()
51+
one_day_ago = now - timedelta(days=1)
52+
53+
# Should be approximately 1 day ago (within 24 hours)
54+
diff_hours = abs((result.replace(tzinfo=None) - one_day_ago).total_seconds()) / 3600
55+
assert diff_hours < 24, f"Expected ~1 day ago for '1d', got {result} (diff: {diff_hours} hours)"
56+
57+
@freeze_time("2025-01-15 15:00:00")
58+
def test_two_days_returns_two_days(self):
59+
"""Test that '2d' correctly returns approximately 2 days ago (not affected by minimum)."""
60+
result = parse_timeframe("2d")
61+
now = datetime.now()
62+
two_days_ago = now - timedelta(days=2)
63+
64+
# Should be approximately 2 days ago (within 24 hours)
65+
diff_hours = abs((result.replace(tzinfo=None) - two_days_ago).total_seconds()) / 3600
66+
assert diff_hours < 24, f"Expected ~2 days ago for '2d', got {result} (diff: {diff_hours} hours)"
67+
68+
@freeze_time("2025-01-15 15:00:00")
69+
def test_one_week_returns_one_week(self):
70+
"""Test that '1 week' correctly returns approximately 1 week ago (not affected by minimum)."""
71+
result = parse_timeframe("1 week")
72+
now = datetime.now()
73+
one_week_ago = now - timedelta(weeks=1)
74+
75+
# Should be approximately 1 week ago (within 24 hours)
76+
diff_hours = abs((result.replace(tzinfo=None) - one_week_ago).total_seconds()) / 3600
77+
assert diff_hours < 24, f"Expected ~1 week ago for '1 week', got {result} (diff: {diff_hours} hours)"
78+
79+
@freeze_time("2025-01-15 15:00:00")
80+
def test_zero_days_returns_one_day_minimum(self):
81+
"""Test that '0d' returns 1 day ago due to minimum enforcement."""
82+
result = parse_timeframe("0d")
83+
now = datetime.now()
84+
one_day_ago = now - timedelta(days=1)
85+
86+
# Should be approximately 1 day ago, not now
87+
diff = abs((result.replace(tzinfo=None) - one_day_ago).total_seconds())
88+
assert diff < 1, f"Expected ~1 day ago for '0d', got {result}"
89+
90+
def test_timezone_awareness(self):
91+
"""Test that returned datetime is timezone-aware."""
92+
result = parse_timeframe("1d")
93+
assert result.tzinfo is not None, "Expected timezone-aware datetime"
94+
95+
def test_invalid_timeframe_raises_error(self):
96+
"""Test that invalid timeframe strings raise ValueError."""
97+
with pytest.raises(ValueError, match="Could not parse timeframe"):
98+
parse_timeframe("invalid_timeframe")

tests/schemas/test_schemas.py

Lines changed: 33 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -221,8 +221,10 @@ def test_permalink_generation():
221221
("last week", True),
222222
("3 weeks ago", True),
223223
("invalid", False),
224-
("tomorrow", False),
225-
("next week", False),
224+
# NOTE: "tomorrow" and "next week" now return 1 day ago due to timezone safety
225+
# They no longer raise errors - this is intentional for remote MCP
226+
("tomorrow", True), # Now valid - returns 1 day ago
227+
("next week", True), # Now valid - returns 1 day ago
226228
("", False),
227229
("0d", True),
228230
("366d", False),
@@ -316,25 +318,27 @@ class TestTimeframeParsing:
316318
"""Test cases for parse_timeframe() and validate_timeframe() functions."""
317319

318320
def test_parse_timeframe_today(self):
319-
"""Test that parse_timeframe('today') returns start of current day with timezone."""
321+
"""Test that parse_timeframe('today') returns 1 day ago for remote MCP timezone safety."""
320322
result = parse_timeframe("today")
321-
expected = datetime.combine(datetime.now().date(), time.min).astimezone()
323+
now = datetime.now()
324+
one_day_ago = now - timedelta(days=1)
322325

323-
assert result == expected
324-
assert result.hour == 0
325-
assert result.minute == 0
326-
assert result.second == 0
327-
assert result.microsecond == 0
326+
# Should be approximately 1 day ago (within a second for test tolerance)
327+
diff = abs((result.replace(tzinfo=None) - one_day_ago).total_seconds())
328+
assert diff < 2, f"Expected ~1 day ago for 'today', got {result}"
328329
assert result.tzinfo is not None
329330

330331
def test_parse_timeframe_today_case_insensitive(self):
331332
"""Test that parse_timeframe handles 'today' case-insensitively."""
332333
test_cases = ["today", "TODAY", "Today", "ToDay"]
333-
expected = datetime.combine(datetime.now().date(), time.min).astimezone()
334+
now = datetime.now()
335+
one_day_ago = now - timedelta(days=1)
334336

335337
for case in test_cases:
336338
result = parse_timeframe(case)
337-
assert result == expected
339+
# Should be approximately 1 day ago (within a second for test tolerance)
340+
diff = abs((result.replace(tzinfo=None) - one_day_ago).total_seconds())
341+
assert diff < 2, f"Expected ~1 day ago for '{case}', got {result}"
338342

339343
def test_parse_timeframe_other_formats(self):
340344
"""Test that parse_timeframe works with other dateparser formats."""
@@ -401,9 +405,9 @@ def test_validate_timeframe_error_cases(self):
401405
with pytest.raises(ValueError, match="Timeframe must be a string"):
402406
validate_timeframe(123) # type: ignore
403407

404-
# Future timeframe
405-
with pytest.raises(ValueError, match="Timeframe cannot be in the future"):
406-
validate_timeframe("tomorrow")
408+
# NOTE: Future timeframes no longer raise errors due to 1-day minimum enforcement
409+
# "tomorrow" and "next week" now return 1 day ago for timezone safety
410+
# This is intentional for remote MCP deployments
407411

408412
# Too far in past (>365 days)
409413
with pytest.raises(ValueError, match="Timeframe should be <= 1 year"):
@@ -431,32 +435,34 @@ class TestModel(BaseModel):
431435
assert model.timeframe == "1d"
432436

433437
def test_timeframe_integration_today_vs_1d(self):
434-
"""Test the specific bug fix: 'today' vs '1d' behavior."""
438+
"""Test that 'today' and '1d' both return 1 day ago due to timezone safety minimum."""
435439

436440
class TestModel(BaseModel):
437441
timeframe: TimeFrame
438442

439-
# 'today' should be preserved
443+
# 'today' should be preserved as special case in validation
440444
today_model = TestModel(timeframe="today")
441445
assert today_model.timeframe == "today"
442446

443447
# '1d' should also be preserved (it's already in standard format)
444448
oneday_model = TestModel(timeframe="1d")
445449
assert oneday_model.timeframe == "1d"
446450

447-
# When parsed by parse_timeframe, they should be different
451+
# When parsed by parse_timeframe, both should return approximately 1 day ago
452+
# due to the 1-day minimum enforcement for remote MCP timezone safety
448453
today_parsed = parse_timeframe("today")
449454
oneday_parsed = parse_timeframe("1d")
450455

451-
# 'today' should be start of today (00:00:00)
452-
assert today_parsed.hour == 0
453-
assert today_parsed.minute == 0
456+
now = datetime.now()
457+
one_day_ago = now - timedelta(days=1)
454458

455-
# '1d' should be 24 hours ago (same time yesterday)
456-
now = datetime.now().astimezone()
457-
expected_1d = now - timedelta(days=1)
458-
diff = abs((oneday_parsed - expected_1d).total_seconds())
459-
assert diff < 60 # Within 1 minute
459+
# Both should be approximately 1 day ago
460+
today_diff = abs((today_parsed.replace(tzinfo=None) - one_day_ago).total_seconds())
461+
assert today_diff < 60, f"'today' should be ~1 day ago, got {today_parsed}"
462+
463+
oneday_diff = abs((oneday_parsed.replace(tzinfo=None) - one_day_ago).total_seconds())
464+
assert oneday_diff < 60, f"'1d' should be ~1 day ago, got {oneday_parsed}"
460465

461-
# They should be different times
462-
assert today_parsed != oneday_parsed
466+
# They should be approximately the same time (within an hour due to parsing differences)
467+
time_diff = abs((today_parsed - oneday_parsed).total_seconds())
468+
assert time_diff < 3600, f"'today' and '1d' should be similar times, diff: {time_diff}s"

uv.lock

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)