diff --git a/src/memory/README.md b/src/memory/README.md index 0f294231a9..6c7683bd92 100644 --- a/src/memory/README.md +++ b/src/memory/README.md @@ -86,7 +86,7 @@ Example: - Remove entities and their relations - Input: `entityNames` (string[]) - Cascading deletion of associated relations - - Silent operation if entity doesn't exist + - No error if an entity doesn't exist; the response reports which names were not found - **delete_observations** - Remove specific observations from entities @@ -94,7 +94,7 @@ Example: - Each object contains: - `entityName` (string): Target entity - `observations` (string[]): Observations to remove - - Silent operation if observation doesn't exist + - No error if an observation doesn't exist; the response reports how many were deleted - **delete_relations** - Remove specific relations from the graph @@ -103,7 +103,7 @@ Example: - `from` (string): Source entity name - `to` (string): Target entity name - `relationType` (string): Relationship type - - Silent operation if relation doesn't exist + - No error if a relation doesn't exist; the response reports how many were deleted - **read_graph** - Read the entire knowledge graph diff --git a/src/memory/__tests__/delete-reporting.test.ts b/src/memory/__tests__/delete-reporting.test.ts new file mode 100644 index 0000000000..944c30204f --- /dev/null +++ b/src/memory/__tests__/delete-reporting.test.ts @@ -0,0 +1,99 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { promises as fs } from 'fs'; +import path from 'path'; +import { fileURLToPath } from 'url'; +import { KnowledgeGraphManager, Entity, Relation } from '../index.js'; + +/** + * The delete tools stay silent when a target is absent, which the README + * documents. What they must not do is report a deletion that did not happen: + * an agent that mistypes a name is told its memory is clean while the data is + * still on disk, and nothing in the response contradicts that. + */ +describe('delete reporting', () => { + let manager: KnowledgeGraphManager; + let testFilePath: string; + + const entities: Entity[] = [ + { name: 'Alice', entityType: 'person', observations: ['works at Acme Corp', 'likes tea'] }, + { name: 'Bob', entityType: 'person', observations: ['likes programming'] }, + ]; + const relations: Relation[] = [{ from: 'Alice', to: 'Bob', relationType: 'works_with' }]; + + beforeEach(async () => { + testFilePath = path.join( + path.dirname(fileURLToPath(import.meta.url)), + `test-delete-reporting-${Date.now()}-${Math.random().toString(16).slice(2)}.jsonl` + ); + manager = new KnowledgeGraphManager(testFilePath); + await manager.createEntities(entities); + await manager.createRelations(relations); + }); + + afterEach(async () => { + try { + await fs.unlink(testFilePath); + } catch { + // the file is gone already + } + }); + + describe('deleteEntities', () => { + it('reports which names matched and which did not', async () => { + const result = await manager.deleteEntities(['Alice', 'Alise']); + expect(result).toEqual({ deleted: ['Alice'], notFound: ['Alise'] }); + }); + + it('reports nothing deleted when no name matches', async () => { + const result = await manager.deleteEntities(['Nobody']); + expect(result).toEqual({ deleted: [], notFound: ['Nobody'] }); + + const graph = await manager.readGraph(); + expect(graph.entities).toHaveLength(2); + }); + + it('still deletes the entity and its relations', async () => { + await manager.deleteEntities(['Alice']); + + const graph = await manager.readGraph(); + expect(graph.entities.map(e => e.name)).toEqual(['Bob']); + expect(graph.relations).toHaveLength(0); + }); + }); + + describe('deleteObservations', () => { + it('counts only the observations that were present', async () => { + const result = await manager.deleteObservations([ + { entityName: 'Alice', observations: ['likes tea', 'never said this'] }, + ]); + expect(result).toEqual({ deletedCount: 1, missingEntities: [] }); + }); + + it('names an entity that does not exist', async () => { + const result = await manager.deleteObservations([ + { entityName: 'Carol', observations: ['anything'] }, + ]); + expect(result).toEqual({ deletedCount: 0, missingEntities: ['Carol'] }); + }); + }); + + describe('deleteRelations', () => { + it('counts only the relations that matched', async () => { + const result = await manager.deleteRelations([ + { from: 'Alice', to: 'Bob', relationType: 'works_with' }, + { from: 'Alice', to: 'Bob', relationType: 'never_existed' }, + ]); + expect(result).toEqual({ deletedCount: 1 }); + }); + + it('reports nothing deleted when the relation type is wrong', async () => { + const result = await manager.deleteRelations([ + { from: 'Alice', to: 'Bob', relationType: 'manages' }, + ]); + expect(result).toEqual({ deletedCount: 0 }); + + const graph = await manager.readGraph(); + expect(graph.relations).toHaveLength(1); + }); + }); +}); diff --git a/src/memory/index.ts b/src/memory/index.ts index b8704a9515..f938b5a3a7 100644 --- a/src/memory/index.ts +++ b/src/memory/index.ts @@ -175,32 +175,45 @@ export class KnowledgeGraphManager { return results; } - async deleteEntities(entityNames: string[]): Promise { + async deleteEntities(entityNames: string[]): Promise<{ deleted: string[]; notFound: string[] }> { const graph = await this.loadGraph(); + const present = new Set(graph.entities.map(e => e.name)); + const deleted = entityNames.filter(name => present.has(name)); + const notFound = entityNames.filter(name => !present.has(name)); graph.entities = graph.entities.filter(e => !entityNames.includes(e.name)); graph.relations = graph.relations.filter(r => !entityNames.includes(r.from) && !entityNames.includes(r.to)); await this.saveGraph(graph); + return { deleted, notFound }; } - async deleteObservations(deletions: { entityName: string; observations: string[] }[]): Promise { + async deleteObservations(deletions: { entityName: string; observations: string[] }[]): Promise<{ deletedCount: number; missingEntities: string[] }> { const graph = await this.loadGraph(); + let deletedCount = 0; + const missingEntities: string[] = []; deletions.forEach(d => { const entity = graph.entities.find(e => e.name === d.entityName); if (entity) { + const before = entity.observations.length; entity.observations = entity.observations.filter(o => !d.observations.includes(o)); + deletedCount += before - entity.observations.length; + } else { + missingEntities.push(d.entityName); } }); await this.saveGraph(graph); + return { deletedCount, missingEntities }; } - async deleteRelations(relations: Relation[]): Promise { + async deleteRelations(relations: Relation[]): Promise<{ deletedCount: number }> { const graph = await this.loadGraph(); + const before = graph.relations.length; graph.relations = graph.relations.filter(r => !relations.some(delRelation => r.from === delRelation.from && r.to === delRelation.to && r.relationType === delRelation.relationType )); await this.saveGraph(graph); + return { deletedCount: before - graph.relations.length }; } async readGraph(): Promise { @@ -410,11 +423,14 @@ server.registerTool( } }, async ({ entityNames }) => { - await knowledgeGraphManager.deleteEntities(entityNames); + const { deleted, notFound } = await knowledgeGraphManager.deleteEntities(entityNames); notifyGraphUpdated(); + const message = notFound.length === 0 + ? "Entities deleted successfully" + : `Deleted ${deleted.length} of ${entityNames.length} entities. Not found: ${notFound.join(", ")}`; return { - content: [{ type: "text" as const, text: "Entities deleted successfully" }], - structuredContent: { success: true, message: "Entities deleted successfully" } + content: [{ type: "text" as const, text: message }], + structuredContent: { success: true, message } }; } ); @@ -443,11 +459,16 @@ server.registerTool( } }, async ({ deletions }) => { - await knowledgeGraphManager.deleteObservations(deletions); + const { deletedCount, missingEntities } = await knowledgeGraphManager.deleteObservations(deletions); notifyGraphUpdated(); + const requested = deletions.reduce((total, d) => total + d.observations.length, 0); + const message = deletedCount === requested + ? "Observations deleted successfully" + : `Deleted ${deletedCount} of ${requested} observations.` + + (missingEntities.length ? ` Entities not found: ${missingEntities.join(", ")}` : ""); return { - content: [{ type: "text" as const, text: "Observations deleted successfully" }], - structuredContent: { success: true, message: "Observations deleted successfully" } + content: [{ type: "text" as const, text: message }], + structuredContent: { success: true, message } }; } ); @@ -473,11 +494,14 @@ server.registerTool( } }, async ({ relations }) => { - await knowledgeGraphManager.deleteRelations(relations); + const { deletedCount } = await knowledgeGraphManager.deleteRelations(relations); notifyGraphUpdated(); + const message = deletedCount === relations.length + ? "Relations deleted successfully" + : `Deleted ${deletedCount} of ${relations.length} relations. The rest matched nothing.`; return { - content: [{ type: "text" as const, text: "Relations deleted successfully" }], - structuredContent: { success: true, message: "Relations deleted successfully" } + content: [{ type: "text" as const, text: message }], + structuredContent: { success: true, message } }; } );