Skip to content

fix: merge same-key operator dicts in AND metadata filters#4853

Merged
kartik-mem0 merged 1 commit intomem0ai:mainfrom
CrepuscularIRIS:fix/and-filter-same-key-merge
Apr 20, 2026
Merged

fix: merge same-key operator dicts in AND metadata filters#4853
kartik-mem0 merged 1 commit intomem0ai:mainfrom
CrepuscularIRIS:fix/and-filter-same-key-merge

Conversation

@CrepuscularIRIS
Copy link
Copy Markdown
Contributor

Linked Issue

Closes #4850

Description

_process_metadata_filters silently drops filter conditions when multiple AND conditions target the same field with different operators. For example:

filters = {"AND": [{"price": {"gt": 10}}, {"price": {"lt": 20}}]}
# Before fix: {"price": {"lt": 20}}  — gt:10 silently lost
# After fix:  {"price": {"gt": 10, "lt": 20}}  — both preserved

Root cause: dict.update() replaces existing keys entirely. When two conditions share the same field name, the second condition's operator dict overwrites the first instead of merging.

Fix: Added a merge_filters() helper that deep-merges nested operator dicts when the same key already exists in the target. Applied consistently to:

  • AND block (the reported bug)
  • OR block (same pattern within per-element accumulation)
  • NOT block (same pattern)
  • Top-level else branch

Both Memory (sync) and AsyncMemory classes are fixed.

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

Test Coverage

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

Manual testing

Reproduced the bug before the fix — confirmed {"price": {"lt": 20}} with gt:10 silently dropped. After the fix, output is {"price": {"gt": 10, "lt": 20}}.

New regression tests (4 tests)

Test Description
test_and_same_key_different_operators_merged Two operators on same key in AND
test_and_same_key_three_operators_merged Three operators on same key in AND
test_and_mixed_keys_with_same_key_overlap Mixed same-key + different-key in AND
test_and_simple_equality_no_merge Simple equality values (non-dict) use last-writer-wins

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

…R/NOT blocks

When multiple conditions in an AND block target the same field with different
operators (e.g., price > 10 AND price < 20), dict.update() silently dropped
all but the last condition. Replace with a merge helper that deep-merges
nested operator dicts, preserving all constraints.

Applied the same fix to OR and NOT blocks which had the identical pattern,
and to top-level filter accumulation.

Closes mem0ai#4850
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 15, 2026

CLA assistant check
All committers have signed the CLA.

Copy link
Copy Markdown

@xkonjin xkonjin left a comment

Choose a reason for hiding this comment

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

Looks good. The change properly addresses issue #4850 by deep-merging filter dictionaries for the same key, ensuring AND conditions don't overwrite each other. Test cases are clear and correctly validate the behavior.

Copy link
Copy Markdown

@xkonjin xkonjin left a comment

Choose a reason for hiding this comment

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

Code review — merge same-key operator dicts in AND metadata filters

Fixes the bug where AND conditions with the same key would overwrite each other because of a bare .update() on the processed filters dict.

The merge_filters helper correctly deep-merges operator dicts so {"AND": [{"price": {"gt": 10}}, {"price": {"lt": 20}}]} now yields {"price": {"gt": 10, "lt": 20}} instead of the last one winning.

Bugs / edge cases spotted:

  1. Duplicate logic — the same merge_filters + _process_metadata_filters implementation is duplicated verbatim in mem0/memory/main.py (around line 1161 and again at line 2482). If only one was updated, the other would still have the overwrite bug. The diff shows both were updated, which is good, but consider extracting this to a single helper to prevent drift.

  2. Equality collision still loses — the new test test_and_simple_equality_no_merge documents that "AND": [{"status": "active"}, {"status": "pending"}] results in {"status": "pending"}. That's acceptable for now because simple equality isn't a dict of operators, but it may surprise callers who expect AND to mean both must match. Consider whether scalar equality inside AND should be combined into an $in list instead of last-write-wins.

  3. Deep merge is shallow for nested dictstarget[key].update(value) only goes one level deep. If someone somehow nests operators (unlikely today), the merge would still be shallow. Not a blocker for the current API.

Tests cover the exact bug and adjacent cases. LGTM with the caveat about deduplicating the duplicated method.

@kartik-mem0 kartik-mem0 merged commit 8ba225c into mem0ai:main Apr 20, 2026
6 of 7 checks passed
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.

Data Loss in _process_metadata_filters with AND operator

4 participants