-
-
Notifications
You must be signed in to change notification settings - Fork 17.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/Add Neo4j GraphRag support #3686
Conversation
…sage of the GraphCypherQaChain node and modifies the FewShotPromptTemplate node to handle variables from the prefix field.
thank you @ghondar , will probably take some time (1-2 weeks) for me to review and play around with it! |
Thanks for the update, @HenryHengZJ Let me know if you have any questions during your review. |
also feel free to bring up any ideas/integrations or PR like this, love the work! |
type: 'Neo4j' | ||
}, | ||
{ | ||
label: 'Memory', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont think we can have memory here, since the chain itself is not able to accept memory like conversation chain
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
description: 'If true, return the raw query results instead of using the QA chain' | ||
}, | ||
{ | ||
label: 'Return Intermediate Steps', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turning on this doesn't have any difference though, still seeing the same streaming output, prob can remove this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, intermediate steps generates a new array type field intermediateSteps in which the query and the data returned from neo4j come for the specific case of flowise it would not be necessary, I have removed it
baseClasses: [this.type, ...getBaseClasses(GraphCypherQAChain)] | ||
}, | ||
{ | ||
label: 'Output Prediction', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here we allow this chain to output a string or json, but we dont have any function that checks for the output
You can reference LLMChain
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took RetrievalQAChain as a reference and removed Output Prediction
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added it back. Works now!
} | ||
|
||
// Validate required variables in prompts | ||
if (!cypherPromptTemplate.inputVariables.includes('schema') || !cypherPromptTemplate.inputVariables.includes('question')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cypherPromptTemplate
might be undefined, then it will throw error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add optional operator to prevent undefined
try { | ||
let response | ||
if (shouldStreamResponse) { | ||
const handler = new CustomChainHandler(sseStreamer, chatId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use CustomChainHandler
to stream the LLM output back to UI. However, this chain has 2 chains under the hood, this will then stream both LLM outputs back to UI, but we are only interested in the latter one.
To prevent this from happening, CustomChainHandler
takes in a parameter called skipK
. This will skip streaming for first K numbers of handleLLMStart event. In this scenario, you can specify 2 for that. And you will be able to see only the last LLM response will be streamed back.
By setting skipK to 2, it works without returnDirect
. But when returnDirect
is turned on, only Cypher Chain LLM is executed, not the QA Chain. Since we set it to 2, the LLM output from Cypher Chain will be skipped, causing nothing to stream back to UI.
To solve this, we can just directly get the final answer let result = response?.result
, and stream the whole response back if (result) streamResponse(sseStreamer, chatId, result)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the explication, the response has been fixed
description: 'Memory to store chat history for context' | ||
}, | ||
{ | ||
label: 'Cypher Generation Prompt', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can have all of these prompts as optional
.
in future, we will be adding conditional input
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, Cypher Generation Prompt now will be optional
constructor(fields?: { sessionId?: string }) { | ||
this.label = 'Graph Cypher QA Chain' | ||
this.name = 'graphCypherQAChain' | ||
this.version = 2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
version 1.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you right, fix version number
cypherLLM: cypherModel, | ||
graph, | ||
cypherPrompt: cypherPromptTemplate, | ||
qaPrompt: omitQaPrompt ? undefined : qaPromptTemplate, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need omitQaPrompt
? cant we just pass in qaPrompt: qaPromptTemplate
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
omitQaPrompt variable removed
…ut, and enhance prompt handling - Changed version from 2.0 to 1.0. - Removed the 'Memory' input parameter from the GraphCypherQAChain. - Made 'cypherPrompt' optional and improved error handling for prompt validation. - Updated the 'init' and 'run' methods to streamline input processing and response handling. - Enhanced streaming response logic based on the 'returnDirect' flag.
…signature - Consolidated import statements for better readability. - Removed the 'input' and 'options' parameters from the 'init' method, streamlining its signature to only accept 'nodeData'.
@HenryHengZJ thanks for review |
…Flowise into feature/graphragsupport
qaLLM: qaModel, | ||
cypherLLM: cypherModel, | ||
graph, | ||
cypherPrompt: cypherPromptTemplate, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
// Validate required variables in prompts | ||
if ( | ||
cypherPromptTemplate && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here we check if cypherPromptTemplate
is not undefined first
@ghondar I've fixed all the remaining issues, can you give it a final test? |
I've already done all the tests and it works correctly, thanks for the support @HenryHengZJ It has helped me a lot to understand the flow of the project 🤙🏿 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome, thank you so much @ghondar !
Summary:
This pull request introduces a Neo4j database connectivity, provides configuration settings for Neo4j credentials, and supports the usage of the GraphCypherQaChain node. Additionally, it modifies the FewShotPromptTemplate node to handle variables from the prefix field.
Description:
A new node has been created to allow seamless connection to a Neo4j database. This node streamlines the process of querying and managing graph data.
A dedicated configuration section for Neo4j credentials has been introduced, enabling management of authentication details.
A new node has been implemented for integrating GraphCypherQaChain, allowing you to leverage Cypher queries directly within LangChain workflows.
The FewShotPromptTemplate node has been updated to support variables derived from the prefix field.
References:
For additional guidance and best practices on graph-related integrations, please see the LangChain documentation on graph prompting: LangChain Graph Prompting.
Points to Consider:
Screenshots: