Migrate neo4j retriever #109

vga91 · 2025-03-21T13:21:12Z

Issue

Closes #

Change

Migration of Neo4jContentRetriever

General checklist

There are no breaking changes
I have added unit and integration tests for my change
I have manually run all the unit tests in all modules, and they are all green
I have manually run all integration tests in the module I have added/changed, and they are all green

I have added/updated the documentation
I have added an example in the examples repo (only for "big" features)
I have added/updated Spring Boot starter(s) (if applicable)

Checklist for adding new maven module

I have added my new module in the root pom.xml and langchain4j-community-bom/pom.xml

Checklist for adding new embedding store integration

I have added a {NameOfIntegration}EmbeddingStoreIT that extends from either EmbeddingStoreIT or EmbeddingStoreWithFilteringIT
I have added a {NameOfIntegration}EmbeddingStoreRemovalIT that extends from EmbeddingStoreWithRemovalIT

Checklist for changing existing embedding store integration

I have manually verified that the {NameOfIntegration}EmbeddingStore works correctly with the data persisted using the latest released version of LangChain4j

Martin7-1

@vga91 Thank you!

Martin7-1 · 2025-03-22T12:30:49Z

content-retrievers/langchain4j-community-neo4j-retriever/pom.xml

+        <relativePath>../../pom.xml</relativePath>
+    </parent>
+
+    <artifactId>langchain4j-community-neo4j-retriever</artifactId>


What about naming it langchain4j-community-content-retriever-neo4j?

is that the naming pattern? otherwise, what other kinds of retrievers are there? structure retrievers?

Actually no naming pattern about it... Just to tell users that what component in this module

Martin7-1 · 2025-03-22T12:31:50Z

.../src/main/java/dev/langchain4j/rag/content/retriever/neo4j/Neo4jContentRetrieverBuilder.java

+import dev.langchain4j.model.chat.ChatLanguageModel;
+import dev.langchain4j.model.input.PromptTemplate;
+
+public class Neo4jContentRetrieverBuilder {


What about moving it to Neo4jContentRetriever and make it a inner static class?

Martin7-1 · 2025-03-22T12:33:33Z

...iever/src/test/java/dev/langchain4j/rag/content/retriever/neo4j/Neo4jContentRetrieverIT.java

+    void shouldRetrieveContentWhenQueryIsValidAndOpenAiChatModelIsUsed() {
+
+        // With
+        ChatLanguageModel openAiChatModel = OpenAiChatModel.builder()


I'm curious why you need to test with OpenAiChatModel in particular, is it different from mock's ChatLanguageModel?

I mean for an IT you want to use a real model/infra not just a mock.
I don't know if LC4J has also local models as part of the IT infrastructure that could be run on the GH action server?

LC4J only have embedding local model for now.

Maybe this test should move to a new Neo4jContentRetrieverIT to do integration test and add @EnabledIfEnvironmentVariable. And this old one should rename to Neo4jContentRetrieverTest and use mock model. WDYT?

jexp · 2025-03-24T08:51:23Z

Some format violations:

Error:  Failed to execute goal com.diffplug.spotless:spotless-maven-plugin:2.43.0:check (default) on project langchain4j-community-neo4j-retriever: The following files had format violations:
Error:      src/test/java/dev/langchain4j/rag/content/retriever/neo4j/Neo4jContentRetrieverIT.java
Error:          @@ -28,10 +28,11 @@
Error:           @ExtendWith(MockitoExtension.class)
Error:           class·Neo4jContentRetrieverIT·{
Error:           ····private·static·final·String·NEO4J_VERSION·=·System.getProperty("neo4jVersion",·"5.26");
Error:          -····
Error:          +
Error:           ····@Container
Error:          -····private·static·final·Neo4jContainer<?>·neo4jContainer·=
Error:          -············new·Neo4jContainer<>("neo4j:"·+·NEO4J_VERSION).withoutAuthentication().withPlugins("apoc");
Error:          +····private·static·final·Neo4jContainer<?>·neo4jContainer·=·new·Neo4jContainer<>("neo4j:"·+·NEO4J_VERSION)
Error:          +············.withoutAuthentication()
Error:          +············.withPlugins("apoc");
Error:           
Error:           ····private·Driver·driver;
Error:           ····private·Neo4jGraph·graph;
Error:  Run 'mvn spotless:apply' to fix these violations.

jexp · 2025-03-24T09:19:41Z

...triever/src/main/java/dev/langchain4j/rag/content/retriever/neo4j/Neo4jContentRetriever.java

+import org.neo4j.driver.types.Type;
+import org.neo4j.driver.types.TypeSystem;
+
+public class Neo4jContentRetriever implements ContentRetriever {


We should probably name this something like Neo4jText2CypherRetriever ?

jexp · 2025-03-24T09:21:37Z

...ty-neo4j-retriever/src/main/java/dev/langchain4j/rag/content/retriever/neo4j/Neo4jGraph.java

+        }
+    }
+
+    private static final String NODE_PROPERTIES_QUERY =


we could probably do this in one query, but that's an improvement for the future

jexp · 2025-03-24T09:22:03Z

...ty-neo4j-retriever/src/main/java/dev/langchain4j/rag/content/retriever/neo4j/Neo4jGraph.java

+
+    public List<Record> executeRead(String queryString) {
+
+        try (Session session = this.driver.session()) {


we can move this to driver.executeQuery which automatically does retries

jexp · 2025-03-24T09:24:12Z

...triever/src/main/java/dev/langchain4j/rag/content/retriever/neo4j/Neo4jContentRetriever.java

+        List<Record> records = graph.executeRead(cypherQuery);
+        return records.stream()
+                .flatMap(r -> r.values().stream())
+                .map(value -> NODE.isTypeOf(value) ? value.asMap().toString() : value.toString())


should probably also handle RELATIONSHIP and PATH?

jexp · 2025-03-24T09:25:00Z

...ty-neo4j-retriever/src/main/java/dev/langchain4j/rag/content/retriever/neo4j/Neo4jGraph.java

+
+    public void refreshSchema() {
+
+        List<String> nodeProperties = formatNodeProperties(executeRead(NODE_PROPERTIES_QUERY));


in the future we should also add the enhanced schema with sample property values

jexp · 2025-03-24T09:26:43Z

...iever/src/test/java/dev/langchain4j/rag/content/retriever/neo4j/Neo4jContentRetrieverIT.java

+    }
+
+    @Test
+    void shouldReturnEmptyListWhenQueryIsInvalid() {


is the query invalid or does it just return empty list?

we should probably also add a test for a truly invalid query generated that fails during execution

jexp · 2025-03-24T09:27:42Z

...triever/src/main/java/dev/langchain4j/rag/content/retriever/neo4j/Neo4jContentRetriever.java

+
+        String question = query.text();
+        String schema = graph.getSchema();
+        String cypherQuery = generateCypherQuery(schema, question);


in the future we should also add the capability to re-generate and re-run failed queries, best with the question, previous query and error message from the database, for the model to fix (up to N retries)

jexp

LGTM most of my comments are future improvements

Martin7-1

@vga91 Thank you!

Martin7-1 · 2025-03-24T10:48:16Z

...ver/src/main/java/dev/langchain4j/rag/content/retriever/neo4j/Neo4jText2CypherRetriever.java

+import org.neo4j.driver.types.Type;
+import org.neo4j.driver.types.TypeSystem;
+
+public class Neo4jText2CypherRetriever implements ContentRetriever {


This is a breaking change from Neo4jContentRetriever to Neo4jText2CypherRetriever... Any idea to keep compatibility (marked Neo4jContentRetriever as @Deprecated and removed it in the future?)

Added the @Deprecated class, lemme know if it's ok this way

Martin7-1 · 2025-03-24T10:48:59Z

content-retrievers/langchain4j-community-neo4j-retriever/pom.xml

+        <relativePath>../../pom.xml</relativePath>
+    </parent>
+
+    <artifactId>langchain4j-community-neo4j-retriever</artifactId>


Actually no naming pattern about it... Just to tell users that what component in this module

Martin7-1 · 2025-03-24T14:44:16Z

Maybe the IT should split into two parts:

Neo4JText2CypherRetrieverIT: The real IT contains OpenAiChatModel and should annotated with @EnabledIfEnvironmentVariable(named = "OPENAI_API_KEY", matches = ".+")
Neo4JText2CypherRetrieverTest: The test uses mock model to test functionality.

@vga91 WDYT?

vga91 · 2025-03-24T16:03:58Z

Maybe the IT should split into two parts:

Neo4JText2CypherRetrieverIT: The real IT contains OpenAiChatModel and should annotated with @EnabledIfEnvironmentVariable(named = "OPENAI_API_KEY", matches = ".+")

Neo4JText2CypherRetrieverTest: The test uses mock model to test functionality.

@vga91 WDYT?

Yes, great idea. I just split them

Martin7-1

@vga91 Thank you! Could you please remove the langchain4j-neo4j in langchain4j repo? Also, the embedding store and content retriever need a detailed migration docs.

@jexp Thanks for the review!

Migrating neo4j-content-retriever

ff8e656

vga91 marked this pull request as draft March 21, 2025 13:21

changed module name

4d2a7b4

vga91 force-pushed the migrate-neo4j-retriever branch from d2cfa4f to 4d2a7b4 Compare March 21, 2025 13:22

vga91 mentioned this pull request Mar 21, 2025

Migrating neo4j-embedding-store #103

Merged

11 tasks

Martin7-1 reviewed Mar 22, 2025

View reviewed changes

jexp reviewed Mar 24, 2025

View reviewed changes

jexp approved these changes Mar 24, 2025

View reviewed changes

changes reviews

3e12612

vga91 force-pushed the migrate-neo4j-retriever branch from cd690dd to 3e12612 Compare March 24, 2025 10:24

Martin7-1 reviewed Mar 24, 2025

View reviewed changes

vga91 marked this pull request as ready for review March 24, 2025 14:02

added Neo4jContentRetriever as a deprecated class

44d7829

split mock and real chatModel tests

810e28d

Martin7-1 approved these changes Mar 25, 2025

View reviewed changes

Martin7-1 added 2 commits March 25, 2025 15:29

Merge branch 'main' into migrate-neo4j-retriever

c663179

format

207b4c7

Martin7-1 merged commit 43eff19 into langchain4j:main Mar 25, 2025
2 of 4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate neo4j retriever #109

Migrate neo4j retriever #109

vga91 commented Mar 21, 2025 •

edited

Loading

Martin7-1 left a comment

Martin7-1 Mar 22, 2025

jexp Mar 24, 2025

Martin7-1 Mar 24, 2025

Martin7-1 Mar 22, 2025

Martin7-1 Mar 22, 2025

jexp Mar 24, 2025

Martin7-1 Mar 24, 2025

jexp commented Mar 24, 2025

jexp Mar 24, 2025

jexp Mar 24, 2025

jexp Mar 24, 2025

jexp Mar 24, 2025

jexp Mar 24, 2025

jexp Mar 24, 2025

jexp Mar 24, 2025

jexp left a comment

Martin7-1 left a comment

Martin7-1 Mar 24, 2025

vga91 Mar 24, 2025

Martin7-1 Mar 24, 2025

Martin7-1 commented Mar 24, 2025 •

edited

Loading

vga91 commented Mar 24, 2025

Martin7-1 left a comment •

edited

Loading


		public List<Record> executeRead(String queryString) {

		try (Session session = this.driver.session()) {


		public void refreshSchema() {

		List<String> nodeProperties = formatNodeProperties(executeRead(NODE_PROPERTIES_QUERY));

Migrate neo4j retriever #109

Migrate neo4j retriever #109

Conversation

vga91 commented Mar 21, 2025 • edited Loading

Issue

Change

General checklist

Checklist for adding new maven module

Checklist for adding new embedding store integration

Checklist for changing existing embedding store integration

Martin7-1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jexp commented Mar 24, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jexp left a comment

Choose a reason for hiding this comment

Martin7-1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Martin7-1 commented Mar 24, 2025 • edited Loading

vga91 commented Mar 24, 2025

Martin7-1 left a comment • edited Loading

Choose a reason for hiding this comment

vga91 commented Mar 21, 2025 •

edited

Loading

Martin7-1 commented Mar 24, 2025 •

edited

Loading

Martin7-1 left a comment •

edited

Loading