Skip to content

fix(memory): skip duplicate entities and relations within a single batch#4383

Open
JSap0914 wants to merge 1 commit into
modelcontextprotocol:mainfrom
JSap0914:fix-memory-batch-duplicates
Open

fix(memory): skip duplicate entities and relations within a single batch#4383
JSap0914 wants to merge 1 commit into
modelcontextprotocol:mainfrom
JSap0914:fix-memory-batch-duplicates

Conversation

@JSap0914

Copy link
Copy Markdown

Description

create_entities and create_relations only de-duplicated against the existing graph, never against the items in the incoming request. So passing the same entity name (or an identical relation) twice in a single call persisted duplicate records to memory.jsonl.

This contradicts the documented behavior in the README — "Ignores entities with existing names" and "Skips duplicate relations" — and breaks the implicit name-uniqueness invariant the rest of the manager relies on (addObservations/deleteEntities/deleteObservations all key on name, and find() only returns the first match).

The fix de-duplicates within the input batch as well, keeping the first occurrence. Cross-call de-duplication is unchanged.

Server Details

  • Server: memory
  • Changes to: core logic (KnowledgeGraphManager.createEntities, KnowledgeGraphManager.createRelations)

Motivation and Context

Reproduction (before this change):

// create_entities
[{ "name": "Alice", "entityType": "person", "observations": ["a"] },
 { "name": "Alice", "entityType": "person", "observations": ["b"] }]
// -> graph now contains TWO "Alice" entities

// create_relations
[{ "from": "Alice", "to": "Bob", "relationType": "knows" },
 { "from": "Alice", "to": "Bob", "relationType": "knows" }]
// -> graph now contains TWO identical relations

After the change each of the above creates a single record, matching the documented "ignores existing names" / "skips duplicate relations" contract.

How Has This Been Tested?

Added two regression tests to src/memory/__tests__/knowledge-graph.test.ts (within-batch duplicate entities, within-batch duplicate relations) and ran the existing vitest suite:

$ npx vitest run            # in src/memory
 Test Files  2 passed (2)
      Tests  47 passed (47)

Both new tests fail on main (the manager returns/persists 2 records) and pass with this change. npx tsc --noEmit is clean.

Breaking Changes

None. Behavior only changes for inputs that were already malformed (duplicate items in one request); valid de-duplicating callers are unaffected.

Types of changes

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

Checklist

  • I have read the MCP Protocol Documentation
  • My changes follows MCP security best practices
  • I have updated the server's README accordingly (no change needed — fix aligns behavior with the existing README description)
  • I have tested this with an LLM client (verified via the vitest suite instead)
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling

Prepared with AI assistance; the diff, tests, and verification were reviewed by me.

create_entities and create_relations only de-duplicated against the existing
graph, so passing the same entity name (or identical relation) twice in one
call persisted duplicate records. This contradicts the documented behavior
("Ignores entities with existing names" / "Skips duplicate relations") and
breaks the implicit name-uniqueness invariant the rest of the manager relies
on (e.g. addObservations/deleteEntities key on name).

De-duplicate within the input batch as well, keeping the first occurrence.
Copilot AI review requested due to automatic review settings June 16, 2026 09:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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.

2 participants