Skip to content

Commit 6d977dd

Browse files
authored
Merge pull request #3297 from DukeDeSouth/fix/memory-open-nodes-relations
fix(memory): return relations connected to requested nodes in openNodes/searchNodes
2 parents 72efc42 + ca7ea22 commit 6d977dd

2 files changed

Lines changed: 49 additions & 10 deletions

File tree

src/memory/__tests__/knowledge-graph.test.ts

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -302,10 +302,20 @@ describe('KnowledgeGraphManager', () => {
302302
expect(result.entities[0].name).toBe('Alice');
303303
});
304304

305-
it('should include relations between matched entities', async () => {
305+
it('should include relations where at least one endpoint matches', async () => {
306306
const result = await manager.searchNodes('Acme');
307307
expect(result.entities).toHaveLength(2); // Alice and Acme Corp
308-
expect(result.relations).toHaveLength(1); // Only Alice -> Acme Corp relation
308+
// Both relations included: Alice → Acme Corp (Alice matched) and Bob → Acme Corp (Acme Corp matched)
309+
expect(result.relations).toHaveLength(2);
310+
});
311+
312+
it('should include outgoing relations to unmatched entities', async () => {
313+
const result = await manager.searchNodes('Alice');
314+
expect(result.entities).toHaveLength(1);
315+
// Alice → Acme Corp relation included because Alice is the source
316+
expect(result.relations).toHaveLength(1);
317+
expect(result.relations[0].from).toBe('Alice');
318+
expect(result.relations[0].to).toBe('Acme Corp');
309319
});
310320

311321
it('should return empty graph for no matches', async () => {
@@ -336,16 +346,41 @@ describe('KnowledgeGraphManager', () => {
336346
expect(result.entities.map(e => e.name)).toContain('Bob');
337347
});
338348

339-
it('should include relations between opened nodes', async () => {
349+
it('should include all relations connected to opened nodes', async () => {
340350
const result = await manager.openNodes(['Alice', 'Bob']);
351+
// Alice → Bob (both endpoints opened) and Bob → Charlie (Bob is opened)
352+
expect(result.relations).toHaveLength(2);
353+
expect(result.relations.some(r => r.from === 'Alice' && r.to === 'Bob')).toBe(true);
354+
expect(result.relations.some(r => r.from === 'Bob' && r.to === 'Charlie')).toBe(true);
355+
});
356+
357+
it('should include relations connected to opened nodes', async () => {
358+
const result = await manager.openNodes(['Bob']);
359+
// Bob has two relations: Alice → Bob and Bob → Charlie
360+
expect(result.relations).toHaveLength(2);
361+
expect(result.relations.some(r => r.from === 'Alice' && r.to === 'Bob')).toBe(true);
362+
expect(result.relations.some(r => r.from === 'Bob' && r.to === 'Charlie')).toBe(true);
363+
});
364+
365+
it('should include outgoing relations to nodes not in the open set', async () => {
366+
// This is the core bug fix for #3137: open_nodes should return
367+
// relations FROM the opened node, even if the target is not opened
368+
const result = await manager.openNodes(['Alice']);
369+
expect(result.entities).toHaveLength(1);
370+
expect(result.entities[0].name).toBe('Alice');
371+
// Alice → Bob relation is included because Alice is opened
341372
expect(result.relations).toHaveLength(1);
342373
expect(result.relations[0].from).toBe('Alice');
343374
expect(result.relations[0].to).toBe('Bob');
344375
});
345376

346-
it('should exclude relations to unopened nodes', async () => {
347-
const result = await manager.openNodes(['Bob']);
348-
expect(result.relations).toHaveLength(0);
377+
it('should include incoming relations from nodes not in the open set', async () => {
378+
const result = await manager.openNodes(['Charlie']);
379+
expect(result.entities).toHaveLength(1);
380+
// Bob → Charlie relation is included because Charlie is opened
381+
expect(result.relations).toHaveLength(1);
382+
expect(result.relations[0].from).toBe('Bob');
383+
expect(result.relations[0].to).toBe('Charlie');
349384
});
350385

351386
it('should handle opening non-existent nodes', async () => {

src/memory/index.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -197,9 +197,10 @@ export class KnowledgeGraphManager {
197197
// Create a Set of filtered entity names for quick lookup
198198
const filteredEntityNames = new Set(filteredEntities.map(e => e.name));
199199

200-
// Filter relations to only include those between filtered entities
200+
// Include relations where at least one endpoint matches the search results.
201+
// This lets callers discover connections to nodes outside the result set.
201202
const filteredRelations = graph.relations.filter(r =>
202-
filteredEntityNames.has(r.from) && filteredEntityNames.has(r.to)
203+
filteredEntityNames.has(r.from) || filteredEntityNames.has(r.to)
203204
);
204205

205206
const filteredGraph: KnowledgeGraph = {
@@ -219,9 +220,12 @@ export class KnowledgeGraphManager {
219220
// Create a Set of filtered entity names for quick lookup
220221
const filteredEntityNames = new Set(filteredEntities.map(e => e.name));
221222

222-
// Filter relations to only include those between filtered entities
223+
// Include relations where at least one endpoint is in the requested set.
224+
// Previously this required BOTH endpoints, which meant relations from a
225+
// requested node to an unrequested node were silently dropped — making it
226+
// impossible to discover a node's connections without reading the full graph.
223227
const filteredRelations = graph.relations.filter(r =>
224-
filteredEntityNames.has(r.from) && filteredEntityNames.has(r.to)
228+
filteredEntityNames.has(r.from) || filteredEntityNames.has(r.to)
225229
);
226230

227231
const filteredGraph: KnowledgeGraph = {

0 commit comments

Comments
 (0)