Codecov Report

Attention: Patch coverage is 82.92683% with 28 lines in your changes missing coverage. Please review.

Project coverage is 75.08%. Comparing base (7bbf8e0) to head (196aabd).
Report is 1 commits behind head on main.

@@            Coverage Diff             @@
##             main    #5308      +/-   ##
==========================================
+ Coverage   74.96%   75.08%   +0.11%     
==========================================
  Files         175      176       +1     
  Lines       11158    11322     +164     
==========================================
+ Hits         8365     8501     +136     
- Misses       2793     2821      +28     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rickyloynd-microsoft and I discussed this PR and PR #5227.

As a user I need:
clarity -- which memory interface to use when
consistency -- no concept/code duplication

We tried the ListMemory() interface in core and the metadata field in it is useful for storing (task, insight) tuples. That replicated some of the features in #5227 but not all. We should improve clarity about this and consistency between the two APIs.

cc: @ekzhu @jackgerrits

Here is the script we tried. cc @rickyloynd-microsoft

from autogen_agentchat.agents import AssistantAgent
from autogen_agentchat.ui import Console
from autogen_core.memory import ListMemory, MemoryContent, MemoryMimeType, MemoryQueryResult, UpdateContextResult
from autogen_ext.models.openai import OpenAIChatCompletionClient
from autogen_core.model_context import ChatCompletionContext
from autogen_core.models import SystemMessage


class CustomListMemory(ListMemory):


     async def update_context(
        self,
        model_context: ChatCompletionContext,
    ) -> UpdateContextResult:
        """Update the model context by appending memory content.

        This method mutates the provided model_context by adding all memories as a
        SystemMessage.

        Args:
            model_context: The context to update. Will be mutated if memories exist.

        Returns:
            UpdateContextResult containing the memories that were added to the context
        """

        if not self._contents:
            return UpdateContextResult(memories=MemoryQueryResult(results=[]))

        memory_strings = [f"{i}. {str(memory.content) + str(memory.metadata['plan']) if memory.metadata else ''}" for i, memory in enumerate(self._contents, 1)]

        if memory_strings:
            memory_context = "\nRelevant memory content (in chronological order):\n" + "\n".join(memory_strings) + "\n"
            await model_context.add_message(SystemMessage(content=memory_context))

        return UpdateContextResult(memories=MemoryQueryResult(results=self._contents))



async def main():
    # Initialize user memory
    # user_memory = ListMemory()

    user_memory = CustomListMemory()

    # Add user preferences to memory
    await user_memory.add(MemoryContent(content="plan for ordering peripherals from favorite website",
    metadata={"plan": "go to amazon.com > search for peripherals > order"}, 
    mime_type=MemoryMimeType.TEXT))

    await user_memory.add(MemoryContent(content="order meal from grubhub",
    metadata={"plan": "go to grubhub.com > order meal"},
    mime_type=MemoryMimeType.TEXT))

    assistant_agent = AssistantAgent(
        name="assistant_agent",
        model_client=OpenAIChatCompletionClient(
            model="gpt-4o-2024-08-06",
        ),
        memory=[user_memory],
    )

    # Run the agent with a task.
    stream = assistant_agent.run_stream(task="Create a plan to order a keyboard from my favorite website?")
    await Console(stream)

if __name__ == "__main__":
    import asyncio

    asyncio.run(main())

@gagb , thanks for the excellent example above - it indeed is in the spirit of the Memory interface. It would be great to figure out what is needed to cover functionality that is left.

Is your question on consistency related when do we use the Memory interface vs Task Centric Memory #5227 ?
I think a good way to look at it is the Memory is a base interface we provide so that anyone can inherit from it and built Memory implementations that work with the framework (and example of this being compatibility with AgentChat AssistantAgent like your example shows).

If you'd like your Memory implementation pluggable across the ecosystem, inherit from Memory

What do you think?

Yeah I agree with you.

I think it would be great if Ricky's PR was using this interface.

And if that is not possible there need to be a clear reason.

@victordibia thanks for the PR! I noticed it is still in draft, are you planning on more features or are you waiting on further review/testing? If so, I can help with the later.

I recall having some mypy/pyright errors I needed to address. Will get back to it this today/tomorrow and share where I need help, thanks!

Yeah I agree with you.

I think it would be great if Ricky's PR was using this interface.

And if that is not possible there need to be a clear reason.

As mentioned in my review, my PR #5227 now includes a Teachability(Memory) class using the same Memory interface. It's too narrow to support other features of Task-Centric Memory like self-teaching and full portability (usable by any agent or app), but a fully teachable AssistantAgent is good for users.

Thanks @rickyloynd-microsoft ,
I have made some minor updates and fixes mypy/pyright issues and updated tests.
It is now ready for review.

@husseinmozannar , @lspinheiro

LGTM.

One design question I have is if we have any thoughts about how we could let users better customise memory manipulation. I tend to think that the vectordb part is more about the memory storage, but users may want explore different ways of memory formation and retrieval such as summarising memory of assigning importance to memory events, etc. I'm curious if the interface can allow this type of customisation in the future.

Edit: I think the add/update_context methods leave room for that but we don't seem to give much user flexibility to customise what happens in those methods right now.