-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Agentic memory #5227
base: main
Are you sure you want to change the base?
Agentic memory #5227
Conversation
Make memory optional. Filter out insights with negative scores.
Refactor memory paths. Enrich page logging.
Seed messages with random int for variability.
Save sessions as yaml for readability.
Eval simplifications.
…ified in settings.
python/packages/autogen-ext/src/autogen_ext/agentic_memory/README.md
Outdated
Show resolved
Hide resolved
from .page_logger import PageLogger | ||
|
||
|
||
class AgentWrapper: |
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.
Implement the autogen_agentchat.base.TaskRunner
protocol. It has two methods, run and run_stream.
Instead of subclassing this, we can pass in an autogen_agentchat.base.TaskRunner
to the constructor and use that as the inner runner.
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 might have to rethink this for component config, but for now it's important to make sure the concepts are aligned rather than diverging.
raise AssertionError("Invalid base agent") | ||
|
||
self.logger.leave_function() | ||
return response, work_history |
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.
If we implement the run
method, the response should be the last message, and the work history should be the full message history.
from .page_logger import PageLogger | ||
|
||
|
||
class AgenticMemoryController: |
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 think we should try to subclass autogen_agentchat.agents.BaseChatAgent
to make sure wee can play well with the rest of the framework.
from .agentic_memory_controller import AgenticMemoryController | ||
|
||
|
||
class Apprentice: |
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.
What's the difference between Apprentice and AgentController. It seems to me they are basically the same except the former has memory hidden as part of its implementation.
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 really need some standard logging here. The presentation layer of the logs cannot be the log itself -- they must be decoupled. Otherwise the logs cannot be consumed and used by observability tools.
- add_task_with_solution: Adds a task-insight pair to the memory bank, to be retrieved together later. | ||
- get_relevant_insights: Returns any insights from the memory bank that appear sufficiently relevant to the given | ||
""" | ||
def __init__(self, settings: Dict, reset: bool, logger: PageLogger) -> None: |
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.
Rather than using dictionary settings, we should either flatten the settings in the constructor, or use a config class that is a Pydantic basemodel for validation and serializable configs. See existing example in autogen_agentchat.agents.AssistantAgent
. If this class (and others in this PR) implements the ComponentConfig, you can easily load the configurations from a file to create an object of the class.
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.
Please take a look at the serializable configuration example in the latest release note: https://github.com/microsoft/autogen/releases/tag/v0.4.4. We already support this type of configuration you are doing here, so I suggest we keep this aligned with the rest of the framework.
I haven't gone deep into the implementation logics, but I believe from just API level, we need to work on it to make sure it aligns with the rest of the framework. E.g., serializable configuration is already supported, and we should be using that. |
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.
This PR is simply too large. Please propose your changes progressively and iteratively using a separate sequence of PRs. We cannot effectively review these changes as is.
@jackgerrits we will be refactoring this in the current branch first |
Adds a baseline implementation of agentic memory to the autogen-ext package.
For technical details, see the Agentic Memory README.
To see it in action, view the page logs generated while running the code samples.
This PR is a draft pending a few remaining items in-progress:
autogen-core/memory