From f818702ab7f8d2d706178e7b0ed3467501c9c4a2 Mon Sep 17 00:00:00 2001 From: Paul Hernandez <60959+phernandez@users.noreply.github.com> Date: Sat, 27 Sep 2025 23:37:00 -0500 Subject: [PATCH] fix: enforce minimum 1-day timeframe for recent_activity to handle timezone issues (#318) Signed-off-by: phernandez Co-authored-by: Claude --- pyproject.toml | 1 + src/basic_memory/schemas/base.py | 29 ++++-- tests/mcp/test_tool_build_context.py | 2 +- tests/mcp/test_tool_recent_activity.py | 2 +- tests/schemas/test_base_timeframe_minimum.py | 98 ++++++++++++++++++++ tests/schemas/test_schemas.py | 60 ++++++------ uv.lock | 14 +++ 7 files changed, 171 insertions(+), 35 deletions(-) create mode 100644 tests/schemas/test_base_timeframe_minimum.py diff --git a/pyproject.toml b/pyproject.toml index a55da61b..e6227bd8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -72,6 +72,7 @@ dev-dependencies = [ "pytest-asyncio>=0.24.0", "pytest-xdist>=3.0.0", "ruff>=0.1.6", + "freezegun>=1.5.5", ] [tool.hatch.version] diff --git a/src/basic_memory/schemas/base.py b/src/basic_memory/schemas/base.py index 95ec891b..cb641e64 100644 --- a/src/basic_memory/schemas/base.py +++ b/src/basic_memory/schemas/base.py @@ -14,7 +14,7 @@ Key Concepts: import os import mimetypes import re -from datetime import datetime, time +from datetime import datetime, time, timedelta from pathlib import Path from typing import List, Optional, Annotated, Dict @@ -52,21 +52,27 @@ def to_snake_case(name: str) -> str: def parse_timeframe(timeframe: str) -> datetime: """Parse timeframe with special handling for 'today' and other natural language expressions. + Enforces a minimum 1-day lookback to handle timezone differences in distributed deployments. + Args: timeframe: Natural language timeframe like 'today', '1d', '1 week ago', etc. Returns: datetime: The parsed datetime for the start of the timeframe, timezone-aware in local system timezone + Always returns at least 1 day ago to handle timezone differences. Examples: - parse_timeframe('today') -> 2025-06-05 00:00:00-07:00 (start of today with local timezone) + parse_timeframe('today') -> 2025-06-04 14:50:00-07:00 (1 day ago, not start of today) + parse_timeframe('1h') -> 2025-06-04 14:50:00-07:00 (1 day ago, not 1 hour ago) parse_timeframe('1d') -> 2025-06-04 14:50:00-07:00 (24 hours ago with local timezone) parse_timeframe('1 week ago') -> 2025-05-29 14:50:00-07:00 (1 week ago with local timezone) """ if timeframe.lower() == "today": - # Return start of today (00:00:00) in local timezone - naive_dt = datetime.combine(datetime.now().date(), time.min) - return naive_dt.astimezone() + # For "today", return 1 day ago to ensure we capture recent activity across timezones + # This handles the case where client and server are in different timezones + now = datetime.now() + one_day_ago = now - timedelta(days=1) + return one_day_ago.astimezone() else: # Use dateparser for other formats parsed = parse(timeframe) @@ -75,7 +81,18 @@ def parse_timeframe(timeframe: str) -> datetime: # If the parsed datetime is naive, make it timezone-aware in local system timezone if parsed.tzinfo is None: - return parsed.astimezone() + parsed = parsed.astimezone() + else: + parsed = parsed + + # Enforce minimum 1-day lookback to handle timezone differences + # This ensures we don't miss recent activity due to client/server timezone mismatches + now = datetime.now().astimezone() + one_day_ago = now - timedelta(days=1) + + # If the parsed time is more recent than 1 day ago, use 1 day ago instead + if parsed > one_day_ago: + return one_day_ago else: return parsed diff --git a/tests/mcp/test_tool_build_context.py b/tests/mcp/test_tool_build_context.py index 3e9d9e3d..0cce7271 100644 --- a/tests/mcp/test_tool_build_context.py +++ b/tests/mcp/test_tool_build_context.py @@ -94,7 +94,7 @@ valid_timeframes = [ invalid_timeframes = [ "invalid", # Nonsense string - "tomorrow", # Future date + # NOTE: "tomorrow" now returns 1 day ago due to timezone safety - no longer invalid ] diff --git a/tests/mcp/test_tool_recent_activity.py b/tests/mcp/test_tool_recent_activity.py index 7c1bce8d..91dc594a 100644 --- a/tests/mcp/test_tool_recent_activity.py +++ b/tests/mcp/test_tool_recent_activity.py @@ -16,7 +16,7 @@ valid_timeframes = [ invalid_timeframes = [ "invalid", # Nonsense string - "tomorrow", # Future date + # NOTE: "tomorrow" now returns 1 day ago due to timezone safety - no longer invalid ] diff --git a/tests/schemas/test_base_timeframe_minimum.py b/tests/schemas/test_base_timeframe_minimum.py new file mode 100644 index 00000000..98be9596 --- /dev/null +++ b/tests/schemas/test_base_timeframe_minimum.py @@ -0,0 +1,98 @@ +"""Test minimum 1-day timeframe enforcement for timezone handling.""" + +from datetime import datetime, timedelta +import pytest +from freezegun import freeze_time + +from basic_memory.schemas.base import parse_timeframe + + +class TestTimeframeMinimum: + """Test that parse_timeframe enforces a minimum 1-day lookback.""" + + @freeze_time("2025-01-15 15:00:00") + def test_today_returns_one_day_ago(self): + """Test that 'today' returns 1 day ago instead of start of today.""" + result = parse_timeframe("today") + now = datetime.now() + one_day_ago = now - timedelta(days=1) + + # Should be approximately 1 day ago (within a second for test tolerance) + diff = abs((result.replace(tzinfo=None) - one_day_ago).total_seconds()) + assert diff < 1, f"Expected ~1 day ago, got {result}" + + @freeze_time("2025-01-15 15:00:00") + def test_one_hour_returns_one_day_minimum(self): + """Test that '1h' returns 1 day ago due to minimum enforcement.""" + result = parse_timeframe("1h") + now = datetime.now() + one_day_ago = now - timedelta(days=1) + + # Should be approximately 1 day ago, not 1 hour ago + diff = abs((result.replace(tzinfo=None) - one_day_ago).total_seconds()) + assert diff < 1, f"Expected ~1 day ago for '1h', got {result}" + + @freeze_time("2025-01-15 15:00:00") + def test_six_hours_returns_one_day_minimum(self): + """Test that '6h' returns 1 day ago due to minimum enforcement.""" + result = parse_timeframe("6h") + now = datetime.now() + one_day_ago = now - timedelta(days=1) + + # Should be approximately 1 day ago, not 6 hours ago + diff = abs((result.replace(tzinfo=None) - one_day_ago).total_seconds()) + assert diff < 1, f"Expected ~1 day ago for '6h', got {result}" + + @freeze_time("2025-01-15 15:00:00") + def test_one_day_returns_one_day(self): + """Test that '1d' correctly returns approximately 1 day ago.""" + result = parse_timeframe("1d") + now = datetime.now() + one_day_ago = now - timedelta(days=1) + + # Should be approximately 1 day ago (within 24 hours) + diff_hours = abs((result.replace(tzinfo=None) - one_day_ago).total_seconds()) / 3600 + assert diff_hours < 24, f"Expected ~1 day ago for '1d', got {result} (diff: {diff_hours} hours)" + + @freeze_time("2025-01-15 15:00:00") + def test_two_days_returns_two_days(self): + """Test that '2d' correctly returns approximately 2 days ago (not affected by minimum).""" + result = parse_timeframe("2d") + now = datetime.now() + two_days_ago = now - timedelta(days=2) + + # Should be approximately 2 days ago (within 24 hours) + diff_hours = abs((result.replace(tzinfo=None) - two_days_ago).total_seconds()) / 3600 + assert diff_hours < 24, f"Expected ~2 days ago for '2d', got {result} (diff: {diff_hours} hours)" + + @freeze_time("2025-01-15 15:00:00") + def test_one_week_returns_one_week(self): + """Test that '1 week' correctly returns approximately 1 week ago (not affected by minimum).""" + result = parse_timeframe("1 week") + now = datetime.now() + one_week_ago = now - timedelta(weeks=1) + + # Should be approximately 1 week ago (within 24 hours) + diff_hours = abs((result.replace(tzinfo=None) - one_week_ago).total_seconds()) / 3600 + assert diff_hours < 24, f"Expected ~1 week ago for '1 week', got {result} (diff: {diff_hours} hours)" + + @freeze_time("2025-01-15 15:00:00") + def test_zero_days_returns_one_day_minimum(self): + """Test that '0d' returns 1 day ago due to minimum enforcement.""" + result = parse_timeframe("0d") + now = datetime.now() + one_day_ago = now - timedelta(days=1) + + # Should be approximately 1 day ago, not now + diff = abs((result.replace(tzinfo=None) - one_day_ago).total_seconds()) + assert diff < 1, f"Expected ~1 day ago for '0d', got {result}" + + def test_timezone_awareness(self): + """Test that returned datetime is timezone-aware.""" + result = parse_timeframe("1d") + assert result.tzinfo is not None, "Expected timezone-aware datetime" + + def test_invalid_timeframe_raises_error(self): + """Test that invalid timeframe strings raise ValueError.""" + with pytest.raises(ValueError, match="Could not parse timeframe"): + parse_timeframe("invalid_timeframe") \ No newline at end of file diff --git a/tests/schemas/test_schemas.py b/tests/schemas/test_schemas.py index 50ad36e7..e80632dc 100644 --- a/tests/schemas/test_schemas.py +++ b/tests/schemas/test_schemas.py @@ -221,8 +221,10 @@ def test_permalink_generation(): ("last week", True), ("3 weeks ago", True), ("invalid", False), - ("tomorrow", False), - ("next week", False), + # NOTE: "tomorrow" and "next week" now return 1 day ago due to timezone safety + # They no longer raise errors - this is intentional for remote MCP + ("tomorrow", True), # Now valid - returns 1 day ago + ("next week", True), # Now valid - returns 1 day ago ("", False), ("0d", True), ("366d", False), @@ -316,25 +318,27 @@ class TestTimeframeParsing: """Test cases for parse_timeframe() and validate_timeframe() functions.""" def test_parse_timeframe_today(self): - """Test that parse_timeframe('today') returns start of current day with timezone.""" + """Test that parse_timeframe('today') returns 1 day ago for remote MCP timezone safety.""" result = parse_timeframe("today") - expected = datetime.combine(datetime.now().date(), time.min).astimezone() + now = datetime.now() + one_day_ago = now - timedelta(days=1) - assert result == expected - assert result.hour == 0 - assert result.minute == 0 - assert result.second == 0 - assert result.microsecond == 0 + # Should be approximately 1 day ago (within a second for test tolerance) + diff = abs((result.replace(tzinfo=None) - one_day_ago).total_seconds()) + assert diff < 2, f"Expected ~1 day ago for 'today', got {result}" assert result.tzinfo is not None def test_parse_timeframe_today_case_insensitive(self): """Test that parse_timeframe handles 'today' case-insensitively.""" test_cases = ["today", "TODAY", "Today", "ToDay"] - expected = datetime.combine(datetime.now().date(), time.min).astimezone() + now = datetime.now() + one_day_ago = now - timedelta(days=1) for case in test_cases: result = parse_timeframe(case) - assert result == expected + # Should be approximately 1 day ago (within a second for test tolerance) + diff = abs((result.replace(tzinfo=None) - one_day_ago).total_seconds()) + assert diff < 2, f"Expected ~1 day ago for '{case}', got {result}" def test_parse_timeframe_other_formats(self): """Test that parse_timeframe works with other dateparser formats.""" @@ -401,9 +405,9 @@ class TestTimeframeParsing: with pytest.raises(ValueError, match="Timeframe must be a string"): validate_timeframe(123) # type: ignore - # Future timeframe - with pytest.raises(ValueError, match="Timeframe cannot be in the future"): - validate_timeframe("tomorrow") + # NOTE: Future timeframes no longer raise errors due to 1-day minimum enforcement + # "tomorrow" and "next week" now return 1 day ago for timezone safety + # This is intentional for remote MCP deployments # Too far in past (>365 days) with pytest.raises(ValueError, match="Timeframe should be <= 1 year"): @@ -431,12 +435,12 @@ class TestTimeframeParsing: assert model.timeframe == "1d" def test_timeframe_integration_today_vs_1d(self): - """Test the specific bug fix: 'today' vs '1d' behavior.""" + """Test that 'today' and '1d' both return 1 day ago due to timezone safety minimum.""" class TestModel(BaseModel): timeframe: TimeFrame - # 'today' should be preserved + # 'today' should be preserved as special case in validation today_model = TestModel(timeframe="today") assert today_model.timeframe == "today" @@ -444,19 +448,21 @@ class TestTimeframeParsing: oneday_model = TestModel(timeframe="1d") assert oneday_model.timeframe == "1d" - # When parsed by parse_timeframe, they should be different + # When parsed by parse_timeframe, both should return approximately 1 day ago + # due to the 1-day minimum enforcement for remote MCP timezone safety today_parsed = parse_timeframe("today") oneday_parsed = parse_timeframe("1d") - # 'today' should be start of today (00:00:00) - assert today_parsed.hour == 0 - assert today_parsed.minute == 0 + now = datetime.now() + one_day_ago = now - timedelta(days=1) - # '1d' should be 24 hours ago (same time yesterday) - now = datetime.now().astimezone() - expected_1d = now - timedelta(days=1) - diff = abs((oneday_parsed - expected_1d).total_seconds()) - assert diff < 60 # Within 1 minute + # Both should be approximately 1 day ago + today_diff = abs((today_parsed.replace(tzinfo=None) - one_day_ago).total_seconds()) + assert today_diff < 60, f"'today' should be ~1 day ago, got {today_parsed}" - # They should be different times - assert today_parsed != oneday_parsed + oneday_diff = abs((oneday_parsed.replace(tzinfo=None) - one_day_ago).total_seconds()) + assert oneday_diff < 60, f"'1d' should be ~1 day ago, got {oneday_parsed}" + + # They should be approximately the same time (within an hour due to parsing differences) + time_diff = abs((today_parsed - oneday_parsed).total_seconds()) + assert time_diff < 3600, f"'today' and '1d' should be similar times, diff: {time_diff}s" diff --git a/uv.lock b/uv.lock index 8fd23f48..5a81b523 100644 --- a/uv.lock +++ b/uv.lock @@ -124,6 +124,7 @@ dependencies = [ [package.dev-dependencies] dev = [ + { name = "freezegun" }, { name = "gevent" }, { name = "icecream" }, { name = "pytest" }, @@ -166,6 +167,7 @@ requires-dist = [ [package.metadata.requires-dev] dev = [ + { name = "freezegun", specifier = ">=1.5.5" }, { name = "gevent", specifier = ">=24.11.1" }, { name = "icecream", specifier = ">=2.1.3" }, { name = "pytest", specifier = ">=8.3.4" }, @@ -564,6 +566,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/61/05/63f63ad5b6789a730d94b8cb3910679c5da1ed5b4e38c957140ac9edcf0e/fastmcp-2.11.3-py3-none-any.whl", hash = "sha256:28f22126c90fd36e5de9cc68b9c271b6d832dcf322256f23d220b68afb3352cc", size = 260231, upload-time = "2025-08-11T21:38:44.746Z" }, ] +[[package]] +name = "freezegun" +version = "1.5.5" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "python-dateutil" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/95/dd/23e2f4e357f8fd3bdff613c1fe4466d21bfb00a6177f238079b17f7b1c84/freezegun-1.5.5.tar.gz", hash = "sha256:ac7742a6cc6c25a2c35e9292dfd554b897b517d2dec26891a2e8debf205cb94a", size = 35914, upload-time = "2025-08-09T10:39:08.338Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/5e/2e/b41d8a1a917d6581fc27a35d05561037b048e47df50f27f8ac9c7e27a710/freezegun-1.5.5-py3-none-any.whl", hash = "sha256:cd557f4a75cf074e84bc374249b9dd491eaeacd61376b9eb3c423282211619d2", size = 19266, upload-time = "2025-08-09T10:39:06.636Z" }, +] + [[package]] name = "gevent" version = "25.5.1"