feat: Add GraphDBBlock to LongtermAgentMemory and add GraphDBMemory #851
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Thanks @mrsbeep for the PR. There may be still something to be redeign for this PR.
An example on how to use Neo4jGraph for GraphRAG here:
https://colab.research.google.com/drive/1meBf9w8KzZvQdQU2I1bCyOg9ehoGDK1u#scrollTo=l4VANToobUzu
A couple things to be discussed:
- What is the default structure of the
LongtermAgentMemory? Should we always have bothVectorDBBlockandGraphDBBlockor allow different combinations? - Should we allow user to load a pre-built knowledge graph?
- Should we allow agent to automatically convert chat messages into knowledge graphs?
- How to score, retrieve and decide whether to use the retrieved results?
- How to combine to retrieved KGs with
vector_db_retrieveandchat_history?
| """ | ||
|
|
||
| def __init__(self, storage: Optional[BaseGraphStorage] = None) -> None: | ||
| self.storage = storage or self.default_graph_storage() |
There was a problem hiding this comment.
Should we use Neo4jGraph as the default storage?
There was a problem hiding this comment.
I'm currently using BaseGraphStorage as storage in GraphDBBlock. I will make it so that Neo4jGraph can use it too.
camel/memories/agent_memories.py
Outdated
| self, | ||
| context_creator: BaseContextCreator, | ||
| storage: Optional[BaseGraphStorage] = None, | ||
| query: str = "", |
There was a problem hiding this comment.
Should we use self._current_topic similar to VectorDBMemory for the query?
| results = self.storage.query(query, params) | ||
| score = 1.0 | ||
| return [ | ||
| ContextRecord( | ||
| memory_record=MemoryRecord.from_dict(result), score=score | ||
| ) | ||
| for result in results | ||
| ] |
| graph database. | ||
| """ | ||
| results = self.storage.query(query, params) | ||
| score = 1.0 |
There was a problem hiding this comment.
score should not be 1.0 for all ContextRecord
What is the default structure of the LongtermAgentMemory? Should we always have both VectorDBBlock and GraphDBBlock or allow different combinations? Should we allow user to load a pre-built knowledge graph? Should we allow agent to automatically convert chat messages into knowledge graphs? How to score, retrieve and decide whether to use the retrieved results? How to Combine Retrieved Knowledge Graphs with VectorDBRetrieve and Chat History? |
…memory' of https://github.com/camel-ai/camel into feature/add-graphdb-memory
…-ai/camel into feature/add-graphdb-memory
There was a problem hiding this comment.
Thanks @mrsbeep 's great contribution! Left some comments below~
| def default_graph_storage( | ||
| self, | ||
| url: Optional[str] = None, | ||
| username: Optional[str] = None, | ||
| password: Optional[str] = None, | ||
| database: Optional[str] = "neo4j", | ||
| ) -> BaseGraphStorage: | ||
| # Try to get details from environment variables first | ||
| url = os.getenv('NEO4J_URI', url) | ||
| username = os.getenv('NEO4J_USERNAME', username) | ||
| password = os.getenv('NEO4J_PASSWORD', password) | ||
| database = os.getenv('NEO4J_DATABASE', database) | ||
|
|
||
| # Log an error if any of the necessary values are missing | ||
| if not url or not username or not password: | ||
| logger.error( | ||
| "Neo4j connection detailed are missing." | ||
| "Ensure environment variables or parameters are set for" | ||
| "NEO4J_URI, NEO4J_USERNAME, and NEO4J_PASSWORD." | ||
| ) | ||
| raise ValueError("Missing Neo4j connection details.") | ||
|
|
||
| # Create and return an instance of Neo4jGraph | ||
| return Neo4jGraph(url, username, password, database or "neo4j") |
There was a problem hiding this comment.
we can just pass config information to Neo4jGraph directly since the environment variable fetching and checking would be done from class Neo4jGraph's side
There was a problem hiding this comment.
It has already been corrected.
| def _calculate_scores(self, query: str, results: List[Any]) -> List[float]: | ||
| query_graph = self._build_graph(query) | ||
| scores = [] | ||
| for result in results: | ||
| result_graph = self._build_graph(result) | ||
| score = self._graph_similarity(query_graph, result_graph) | ||
| scores.append(score) | ||
| return scores |
There was a problem hiding this comment.
for the score I think using an embedding-based vector would be more effective for calculating the score, as it aligns better with matching the user's query. A semantic-based score would be more suitable in this case. WDYT?
| else: | ||
| subj = content.get('subject') | ||
| obj = content.get('object') | ||
| rel = content.get('relation') |
There was a problem hiding this comment.
record.message.content should be str, for what case it will go to this part?
| """ | ||
| self.storage.add_triplet(subj, obj, rel) | ||
|
|
||
| def delete_triplet(self, subj: str, obj: str, rel: str) -> None: |
There was a problem hiding this comment.
I think the delete_triplet shouldn't be put into here, it will be managed from graph db's side, but we can add delete_records method
camel/memories/agent_memories.py
Outdated
| query (str, optional): | ||
| The query to retrieve data from the graph database. |
There was a problem hiding this comment.
| query (str, optional): | |
| The query to retrieve data from the graph database. | |
| query (str, optional): The query to retrieve data from the graph | |
| database. |
| def clear(self) -> None: | ||
| pass |
There was a problem hiding this comment.
we can also add code to realize this method if it's required by the memory module
camel/memories/agent_memories.py
Outdated
| def write_triplet(self, subj: str, obj: str, rel: str) -> None: | ||
| r"""Writes a triplet to the graph database. | ||
|
|
||
| Args: | ||
| subj (str): The subject of the triplet. | ||
| obj (str): The object of the triplet. | ||
| rel (str): The relationship between subject and object. | ||
| """ | ||
| self._graphdb_block.write_triplet(subj, obj, rel) |
There was a problem hiding this comment.
I think here should be write_records using self._graphdb_block.write_record()
camel/memories/agent_memories.py
Outdated
| self, | ||
| context_creator: BaseContextCreator, | ||
| storage: Optional[BaseGraphStorage] = None, | ||
| query: str = "", |
| results = self.storage.query(query, params) | ||
| scores = self._calculate_scores(query, results) |
There was a problem hiding this comment.
here the query is some query language like Cypher, calculate the score between the query and the matched result from graph database seems doesn't make sense
…-ai/camel into feature/add-graphdb-memory
Description
This pull request introduces a new feature to the LongtermAgentMemory by integrating a GraphDBBlock and adding a new GraphDBMemory. The changes aim to leverage knowledge graphs within AgentMemory, enhancing the capability to store and retrieve structured information through graph databases.
Motivation and Context
close #846Types of changes
What types of changes does your code introduce? Put an
xin all the boxes that apply:Implemented Tasks
Checklist
Go over all the following points, and put an
xin all the boxes that apply.If you are unsure about any of these, don't hesitate to ask. We are here to help!