Skip to content

fix(kg-writer): persist relationships to existing chunks - #625

Open
coldbean-s wants to merge 1 commit into
neo4j:mainfrom
coldbean-s:investigate-470-writer-persistence
Open

coldbean-s wants to merge 1 commit into
neo4j:mainfrom
coldbean-s:investigate-470-writer-persistence

Conversation

@coldbean-s

Copy link
Copy Markdown

Related to #470. This is a Writer-side follow-up to the persistence gap discussed in #491.

Summary

This PR fixes a Neo4jWriter persistence issue when a relationship points from a node in the current Neo4jGraph to a Chunk that already exists in Neo4j but is not included in the current graph.nodes.

In #491, the entity-to-chunk relationships can be present in the returned Neo4jGraph, but the review noted that they may still not be persisted by Neo4jWriter.

Root cause

Neo4jWriter currently resolves both relationship endpoints using the temporary ingestion identity:

MATCH (start:__KGBuilder__ {__tmp_internal_id: row.start_node_id}),
      (end:__KGBuilder__ {__tmp_internal_id: row.end_node_id})

Nodes written in the current batch receive :__KGBuilder__ and __tmp_internal_id.

A Chunk that already exists in Neo4j and is not part of the current graph.nodes does not receive this temporary identity, so the endpoint is not matched and the relationship is silently not persisted.

Reproduction

A Writer-only reproduction, without an LLM or LLMEntityRelationExtractor, shows:

Control: new Entity -> new Chunk
Writer status: SUCCESS
Relationship persisted: yes

Experimental: new Entity -> pre-existing Chunk
Writer status: SUCCESS
Relationship persisted: no

The pre-existing Chunk itself remains in Neo4j.

Fix

The existing new-node -> new-node path remains unchanged.

For node-to-chunk relationships where:

the start node belongs to the current graph.nodes, and
the Chunk endpoint does not,

the Writer uses a separate persistence path:

start endpoint: resolved by __tmp_internal_id
existing Chunk endpoint: resolved by
LexicalGraphConfig.chunk_node_label and
LexicalGraphConfig.chunk_id_property

The configured identifiers are escaped using the project's existing Cypher identifier helper.

Tests

Added coverage for:

existing new-node -> new-Chunk behavior
new-node -> pre-existing Chunk persistence
custom LexicalGraphConfig
routing to the existing-Chunk persistence path
Cypher identifier escaping

Local results:

Writer/query unit tests:
65 passed, 27 skipped

Writer E2E:
3 passed, 2 skipped

Ruff:
All checks passed

Mypy:
Success

The Writer-only runtime reproduction after the fix reports:

Control relationship: 1
Existing-Chunk relationship: 1

The full unit suite was not run locally because the minimal development environment does not include all optional provider/vector-store extras; CI can exercise the complete matrix.

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.

1 participant