Skip to content

Implement 'forget' functionality to selectively delete nodes and edges from the graph#1369

Open
dudo wants to merge 2 commits intogetzep:mainfrom
legalzoom:forget_about_it
Open

Implement 'forget' functionality to selectively delete nodes and edges from the graph#1369
dudo wants to merge 2 commits intogetzep:mainfrom
legalzoom:forget_about_it

Conversation

@dudo
Copy link
Copy Markdown

@dudo dudo commented Apr 2, 2026

Summary

Adds the ability to "forget" things, without having raw access to the data.

Addresses #864

Type of Change

  • Bug fix
  • New feature
  • Performance improvement
  • Documentation/Tests

Objective

We don't always want our agents to remember... everything.

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • All existing tests pass

Breaking Changes

  • This PR contains breaking changes

Checklist

  • Code follows project style guidelines (make lint passes)
  • Self-review completed
  • Documentation updated where necessary
  • No secrets or sensitive information committed

Related Issues

Closes #[issue number]

@danielchalef
Copy link
Copy Markdown
Member

danielchalef commented Apr 2, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@dudo
Copy link
Copy Markdown
Author

dudo commented Apr 2, 2026

I have read the CLA Document and I hereby sign the CLA

danielchalef added a commit that referenced this pull request Apr 2, 2026
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 2, 2026

PR Triage Assessment

Priority: LOW | Category: feature | Action: needs-major-rework

Summary

Adds a "forget" endpoint to the REST API and MCP server that searches for nodes/edges by semantic query and deletes them. The core logic is reasonable, but the PR bundles an unrelated company-specific Dockerfile and lacks any tests.

Quality Scores

Tests Docs Style Scope Total
0 1 2 2 5/12

Signals

Maintainer Note

Before this can be merged: (1) Remove Dockerfile.local - it contains a company-specific artifactory URL (artifactory.legalzoom.com) and is unrelated to the forget feature. (2) Add unit tests for the new forget endpoint in both server and MCP server. (3) Add integration tests that verify nodes/edges are actually deleted. The implementation itself looks sound - it reuses existing search infrastructure and follows established patterns.

Raw triage data (JSON)
{
  "pr_number": 1369,
  "title": "Implement 'forget' functionality to selectively delete nodes and edges from the graph",
  "author": "dudo",
  "category": "feature",
  "priority": "LOW",
  "action": "needs-major-rework",
  "quality_scores": {
    "tests": 0,
    "docs": 1,
    "style": 2,
    "scope": 2,
    "total": 5
  },
  "signals": {
    "follows_patterns": true,
    "focused_scope": false,
    "has_rfc_if_needed": true
  },
  "slop_signals": ["tests-missing"],
  "duplicate_of": null,
  "additions": 196,
  "deletions": 2,
  "files_changed": 5,
  "linked_issue": 864,
  "notes": "Dockerfile.local with company-specific artifactory URL (artifactory.legalzoom.com) should be removed. Core forget implementation looks reasonable but needs test coverage."
}

@claude claude bot added triage/low Low priority - backlog needs-tests PR lacks adequate test coverage labels Apr 2, 2026
@dudo
Copy link
Copy Markdown
Author

dudo commented Apr 2, 2026

Note on test coverage

Quick survey of the existing test landscape:

  • graphiti_core — 49 test files under tests/ (unit + integration). The forget logic doesn't touch core, so nothing to add here.
  • MCP server — 11 test files under mcp_server/tests/, including E2E tests for delete_episode, delete_entity_edge, and clear_graph in test_comprehensive_integration.py. These require a live Neo4j instance. A forget test case would fit naturally here once we can validate against a running DB.
  • REST API server — No server/tests/ directory exists today. This PR doesn't establish that precedent.

The forget implementation is a thin composition of existing tested primitives (search_ + node.delete + edge.delete), so the actual risk surface is the orchestration logic (edge skipping when nodes are already deleted), not the underlying operations.

@danielchalef
Copy link
Copy Markdown
Member

I see the use case here for the MCP server, but not for building this into Graphiti. See my comment in the related issue.

Can we move this functionality entirely into the MCP server?

@dudo
Copy link
Copy Markdown
Author

dudo commented Apr 2, 2026

@danielchalef Makes sense — forget is use-case dependent and better as an implementation detail than a core primitive.

Updated the PR:

  • Removed Dockerfile.local (was unrelated, my mistake)
  • Removed all server REST API changes (/forget endpoint, DTOs)
  • Kept forget exclusively as an MCP server tool, built on top of existing search_ + node.delete + edge.delete primitives

Will amend the commit once I confirm the changes look right.

@dudo dudo force-pushed the forget_about_it branch from da89524 to fcdc4e3 Compare April 2, 2026 21:47
@github-actions github-actions bot removed triage/low Low priority - backlog needs-tests PR lacks adequate test coverage labels Apr 2, 2026
@dudo
Copy link
Copy Markdown
Author

dudo commented Apr 3, 2026

I'm trying to test this against my deployment. As an http server, I'm expecting the graphiti server to expose the /mcp endpoint, but it's not there. Does the server need to be booted with an env or something? Seems like graphiti is built to be deployed as either a webserver or an mcp server?

@dudo
Copy link
Copy Markdown
Author

dudo commented Apr 3, 2026

I've tested this successfully!

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