Skip to content

fix(api): map entity params to filters in GET /memories (#4955) - #4960

Merged
kartik-mem0 merged 3 commits into
mem0ai:mainfrom
PratikRai0101:fix/api-get-memories-filters
Apr 24, 2026
Merged

kartik-mem0 merged 3 commits into
mem0ai:mainfrom
PratikRai0101:fix/api-get-memories-filters

Conversation

@PratikRai0101

@PratikRai0101 PratikRai0101 commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Linked Issue

Closes #4955

Description

This PR resolves a version mismatch between the OSS REST API server and the Core Memory backend (v3).

Following the v3 update, the Memory.get_all() method now requires entity-level parameters (user_id, agent_id, run_id) to be encapsulated within a filters dictionary. The API route was previously passing these as top-level keyword arguments, resulting in a RuntimeError.

Key Changes:

  • Updated the get_all_memories route in server/main.py to intercept query parameters.
  • Implemented a dictionary comprehension to bundle active entity IDs into a filters object.
  • Updated the backend call to use the filters=filters keyword argument.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactor (no functional changes)
  • Documentation update

Breaking Changes

N/A - This restores expected functionality for the /memories GET endpoint.

Test Coverage

  • I added/updated unit tests
  • I added/updated integration tests
  • I tested manually (describe below)
  • No tests needed (explain why)

Manual Validation:

  1. Booted the FastAPI server locally using uvicorn.
  2. Verified that the application successfully initializes and imports the updated routing logic.
  3. Confirmed via code execution path that query parameters are now correctly mapped to the filters dictionary before being dispatched to the core memory instance.

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have added tests that prove my fix/feature works
  • New and existing tests pass locally
  • I have updated documentation if needed

@PratikRai0101
PratikRai0101 marked this pull request as ready for review April 24, 2026 13:49
@kartik-mem0

kartik-mem0 commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

hey @PratikRai0101 thank you for your contribution can you please add some test here to validate.

@xkonjin xkonjin left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch on mapping entity params to filters. This accurately matches the get_all signature expected by the library.

Since PR #4959 contains the exact same fix, you might want to coordinate with the author or close one of them as a duplicate to avoid merge conflicts.

The fix itself looks correct.

@PratikRai0101

Copy link
Copy Markdown
Contributor Author

hey @kartik-mem0 so I just pushed a commit adding the TestGetMemories class to test_server_params.py. It explicitly tests the GET /memories?user_id=... route to ensure it returns a 200 OK and correctly maps the query parameters to the filters dict instead of passing them as top-level kwargs.

image

@kartik-mem0
kartik-mem0 merged commit 693e709 into mem0ai:main Apr 24, 2026
7 of 8 checks passed
@PratikRai0101
PratikRai0101 deleted the fix/api-get-memories-filters branch April 24, 2026 18:22
@PHclaw

PHclaw commented Apr 28, 2026

Copy link
Copy Markdown

For the timestamp not being saved with memories:

The issue is likely that the timestamp field exists in the model but is not passed to the storage layer. Check your memory creation code:

# Wrong - timestamp ignored
memory = Memory(
    content=content,
    # timestamp=datetime.now()  # Missing!
)

# Correct - timestamp passed through
memory = Memory(
    content=content,
    timestamp=datetime.now(),
)

# And in the storage layer, ensure timestamp is persisted:
def save(memory):
    db.session.add(memory)
    db.session.commit()

Also check if your database schema has a timestamp column that's not being mapped.

@PHclaw

PHclaw commented Apr 29, 2026

Copy link
Copy Markdown

For the timestamp not being preserved:

The issue is in the metadata pipeline. Here's the fix:

def add_memory(messages, metadata):
    # Ensure timestamp is in metadata
    if 'timestamp' not in metadata:
        metadata['timestamp'] = datetime.now(timezone.utc).isoformat()
    
    # Pass timestamp through to vector store
    memory_data = {
        'content': message_content,
        'metadata': metadata,  # timestamp is preserved here
    }
    
    vector_store.add(memory_data)

And in the vector store:

def add(self, data):
    # Extract timestamp from metadata and store it in the vector
    timestamp = data['metadata'].get('timestamp')
    point = PointStruct(
        id=uuid4(),
        vector=embedding,
        payload={
            'content': data['content'],
            'timestamp': timestamp,  # Stored as searchable field
        }
    )

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.

[Bug] OSS REST GET /memories forwards top-level entity params to get_all() and fails after v3 filter requirement

4 participants