Skip to content

Comments

fix: enforce minimum 1-day timeframe for recent_activity to handle timezone issues#318

Merged
phernandez merged 2 commits intomainfrom
fix-recent-activity-timezone
Sep 28, 2025
Merged

fix: enforce minimum 1-day timeframe for recent_activity to handle timezone issues#318
phernandez merged 2 commits intomainfrom
fix-recent-activity-timezone

Conversation

@phernandez
Copy link
Member

@phernandez phernandez commented Sep 28, 2025

Summary

  • Enforces a minimum 1-day lookback period for recent_activity to handle timezone differences
  • Fixes empty results issue in cloud deployments where server runs in UTC
  • Updates all tests to match the new behavior that prioritizes remote MCP functionality
  • Adds comprehensive test coverage with freezegun for reliable testing

Problem

When Basic Memory is deployed on cloud servers running in UTC, the recent_activity tool returns empty results for users in different timezones (e.g., CST). This happens because:

  1. Notes created with local timestamps appear to be "in the future" from the server's UTC perspective
  2. Short timeframes like "1h" or "today" miss recent activity due to timezone offset
  3. The date filter excludes all results when timestamps don't align

Solution

Implement a 1-day minimum timeframe in parse_timeframe():

  • Timeframes shorter than 1 day (e.g., "today", "1h", "6h") now return at least 1 day of data
  • Longer timeframes (e.g., "2d", "1 week") work as expected
  • This ensures recent activity is always captured regardless of timezone differences up to 24 hours

Test Updates

Since remote MCP is the priority for cloud deployments, tests have been updated to match the new behavior:

  • test_parse_timeframe_today now expects 1 day ago instead of midnight
  • Future timeframes like "tomorrow" no longer raise errors (they return 1 day ago)
  • MCP tool tests updated to not expect errors for future dates
  • All tests now pass with the new timezone-safe behavior

Testing

  • Added comprehensive test suite with freezegun for deterministic time-based testing
  • All tests pass, verifying the minimum enforcement works correctly
  • Tests cover edge cases: "today", "0d", "1h", "6h", "1d", "2d", "1 week"
  • Test updates ensure the behavior matches production requirements

Impact

🤖 Generated with Claude Code

…mezone issues

- Add 1-day minimum lookback in parse_timeframe() to handle timezone differences
- Prevents recent_activity from returning empty results due to UTC/local timezone mismatches
- Ensures 'today', '1h', '6h' and similar short timeframes return at least 1 day of data
- Add comprehensive test coverage with freezegun for reliable timeframe testing

This fixes the issue where recent_activity returns no results on cloud deployments
where the server runs in UTC and clients may be in different timezones (CST, etc).
The 1-day minimum ensures we always capture recent activity regardless of timezone
differences up to 24 hours.

Fixes #318

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: phernandez <paul@basicmachines.co>
@github-actions
Copy link
Contributor

github-actions bot commented Sep 28, 2025

Claude finished @phernandez's task —— View job


PR Review: Fix timezone issues in recent_activity ✅

I've reviewed this PR that enforces a minimum 1-day timeframe for recent_activity to handle timezone differences between clients and servers. Here's my analysis against our team checklist:

✅ Code Quality & Standards

  • Follows Basic Memory's coding conventions - Code adheres to CLAUDE.md guidelines
  • Python 3.12+ type annotations - Function signatures properly typed with return types
  • 100-character line length - All lines within limit
  • No commented-out code blocks - Clean implementation

✅ Testing & Documentation

  • Unit tests for new functions - Comprehensive test suite with 9 test cases covering edge cases
  • Test coverage for edge cases - Tests include "today", "0d", "1h", "6h", "1d", "2d", "1 week"
  • Documentation updated - Docstrings clearly explain the timezone handling rationale
  • Freezegun for deterministic testing - Excellent choice for reliable time-based tests

✅ Basic Memory Architecture

  • Preserves local-first architecture - Solution maintains file-based source of truth
  • Knowledge graph operations maintain consistency - No impact on graph structure
  • Markdown file handling preserves integrity - No changes to markdown processing

✅ Security & Performance

  • No hardcoded secrets - None found
  • Input validation - Existing validation maintained via validate_timeframe
  • Proper error handling - ValueError handling preserved
  • Performance considerations - Minimal overhead, only adjusts datetime calculations

📝 Specific Observations

Implementation Analysis (src/basic_memory/schemas/base.py:52-98):

  • The fix elegantly handles the root cause by enforcing a 1-day minimum lookback
  • Special handling for "today" keyword is appropriate
  • Timezone awareness is properly maintained throughout
  • The solution is backward compatible - longer timeframes (>1 day) work unchanged

Test Quality (tests/schemas/test_base_timeframe_minimum.py):

  • Excellent use of freezegun for deterministic time testing
  • Good coverage of boundary conditions
  • Tests verify both the minimum enforcement and that longer timeframes are unaffected
  • Timezone awareness test ensures proper datetime handling

🎯 Verdict

LGTM! This is a well-crafted fix that solves the timezone issue without breaking existing functionality. The solution is pragmatic - accepting a slightly larger time window (1 day minimum) to ensure reliable results across timezone boundaries is a reasonable trade-off.

The implementation is clean, well-tested, and properly documented. The PR description clearly explains the problem and solution, making it easy to understand the change's purpose.

No issues found. Ready to merge! 🚀

…te MCP

- Update parse_timeframe tests to expect 1 day ago instead of midnight for 'today'
- Remove validation errors for future timeframes (they now return 1 day ago)
- Update MCP tool tests to not expect errors for 'tomorrow' and 'next week'
- Document in tests that this behavior is intentional for remote MCP timezone safety

All tests now pass with the timezone fix that prioritizes remote MCP functionality
over strict local behavior. This ensures Basic Memory works reliably across
different timezones in cloud deployments.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: phernandez <paul@basicmachines.co>
@github-actions
Copy link
Contributor

github-actions bot commented Sep 28, 2025

Claude finished @phernandez's task —— View job


PR Review: Fix timezone issues in recent_activity ✅

I've reviewed this PR that enforces a minimum 1-day timeframe for recent_activity to handle timezone differences between clients and servers. Here's my analysis against our team checklist:

✅ Code Quality & Standards

  • Follows Basic Memory's coding conventions - Code adheres to CLAUDE.md guidelines
  • Python 3.12+ type annotations - Function signatures properly typed with return types
  • [⚠️] 100-character line length - One line appears to exceed limit (118 chars max found)
  • No commented-out code blocks - Clean implementation

✅ Testing & Documentation

  • Unit tests for new functions - Comprehensive test suite with 9 test cases covering edge cases
  • Test coverage for edge cases - Tests include "today", "0d", "1h", "6h", "1d", "2d", "1 week"
  • Documentation updated - Docstrings clearly explain the timezone handling rationale
  • Freezegun for deterministic testing - Excellent choice for reliable time-based tests

✅ Basic Memory Architecture

  • Preserves local-first architecture - Solution maintains file-based source of truth
  • Knowledge graph operations maintain consistency - No impact on graph structure
  • Markdown file handling preserves integrity - No changes to markdown processing

✅ Security & Performance

  • No hardcoded secrets - None found
  • Input validation - Existing validation maintained via validate_timeframe
  • Proper error handling - ValueError handling preserved
  • Performance considerations - Minimal overhead, only adjusts datetime calculations

📝 Specific Observations

Implementation Analysis (src/basic_memory/schemas/base.py:52-98):

  • The fix elegantly handles the root cause by enforcing a 1-day minimum lookback
  • Special handling for "today" keyword is appropriate
  • Timezone awareness is properly maintained throughout
  • The solution is backward compatible - longer timeframes (>1 day) work unchanged

Test Quality (tests/schemas/test_base_timeframe_minimum.py):

  • Excellent use of freezegun for deterministic time testing
  • Good coverage of boundary conditions
  • Tests verify both the minimum enforcement and that longer timeframes are unaffected
  • Timezone awareness test ensures proper datetime handling

⚠️ Minor Issues

  1. Line length: One line may exceed the 100-character limit per CLAUDE.md guidelines. Consider running make format to ensure compliance.

🎯 Verdict

LGTM with minor formatting suggestion! This is a well-crafted fix that solves the timezone issue without breaking existing functionality. The solution is pragmatic - accepting a slightly larger time window (1 day minimum) to ensure reliable results across timezone boundaries is a reasonable trade-off.

The implementation is clean, well-tested, and properly documented. The PR description clearly explains the problem and solution, making it easy to understand the change's purpose.

Ready to merge after addressing the minor line length issue! 🚀


@phernandez phernandez merged commit f818702 into main Sep 28, 2025
13 checks passed
@phernandez phernandez deleted the fix-recent-activity-timezone branch September 28, 2025 04:37
@phernandez phernandez added this to the v0.15.0 milestone Oct 4, 2025
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.

1 participant