Skip to content
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

Add RecallObservations for retrieval of prompt extensions #6909

Merged
merged 161 commits into from
Mar 15, 2025

Conversation

enyst
Copy link
Collaborator

@enyst enyst commented Feb 24, 2025

  • This change is worth documenting at https://docs.all-hands.dev/
  • Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below

Make microagents available in the event stream as recall observations (accessible from the UI, LLM)

This PR proposes to refactor prompt extensions into recalled observations:

  • take the retrieval of information out of the PromptManager, into a Memory component
  • PromptManager remains responsible with loading templates and rendering them (just a 'view manager')
  • Memory subscribes to the stream
    • on user messages, it may retrieve extensions and add them to the stream as RecallObservations
    • on the first user message, it may retrieve repo and runtime info
    • on recall actions (not yet in use, source=agent), it may retrieve e.g. library docs
  • some logic is separated from the agent (like Move PromptManager to agent controller level #6526)
  • since the information is in the stream, session restore / refresh / runtime reconnect / etc will not lose it.

Cc: @xingyaoww
I'd love your opinion about this idea. It's basically an alternative to 6526, I was curious to see roughly how it looks like, so we can maybe see if it makes sense.

Cc: @csmith49


MAIN TASKS

  • define and create RecallObservations and RecallActions
  • memory component, on_event, initialization
  • conversation_memory - move context/prompt manager; handle RecallObs, event->message
    • RecallObs should be before the user message in the messages to the LLM?
  • adapt agent controller flow
  • agent config options (enable extensions, disabled microagents)

Other expected outcomes:

  • no dependency from Runtime or Memory to agent or prompt manager (ref comment)
    • Runtime prompt manager
    • Runtime plugins
    • Memory
  • no access from agent to the stream, direct or indirect

Link of any specific issues this addresses
Fix #6535
Fix #7265
Probably fix #6191
Also fix:

  • fix first message multiple-insert in context, on session restore
  • fix microagents multi-insert on any user message, anytime

@enyst enyst marked this pull request as draft February 24, 2025 02:31
@csmith49
Copy link
Collaborator

This looks clean so far. I'm a fan of moving what we can to the event stream: much better visibility than modifying messages in-place, and I think leaning into the pub-sub approach is a good way to add extra functionality without having to tip-toe around the agent/controller/system control flow.

@xingyaoww
Copy link
Collaborator

I like this idea better than what I did in #6526! Happy to close that PR in favor of this one

Copy link
Collaborator

@csmith49 csmith49 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 🔥 🔥

This is a really cool change, I'm excited to get some extra visibility in the event stream.

Well done!

@enyst
Copy link
Collaborator Author

enyst commented Mar 13, 2025

@xingyaoww I think I addressed everything, I'd love to know what you think. There are also more tests that I separated out, that I keep an eye on, and they went well!

(I'll make another pass renaming the new events as Robert suggested on slack, but that won't change anything of substance. Done.)

I'd love to get this in, and then Calvin and I are looking into maybe doing something similar for the condenser summaries, or at least, it has become clear to us that it is a similar problem: just like repo.md gets "lost" from context sometimes after runtime errors or something, the condenser summaries can get lost, and that's why we see that strange agent behavior doing again what it did before.

Copy link
Collaborator

@xingyaoww xingyaoww left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks so much for this! (Though I kinda still like the more generalized "RecallAction" better -- because environment info fits a bit awkward in "MicroagentObservation" and can be confusing to display in the UI -- I would consider it is more like "PromptExtensionAction/Observation". But happy to go ahead if you think this is good!

isinstance(obs, MicroagentObservation)
and self.agent_config.enable_prompt_extensions
):
if obs.info_type == MicroagentInfoType.ENVIRONMENT:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we might eventually name this to other things -- MicroAgentInfo = ENVIRONMENT seems a bit confusing. But let's leave this as is for now :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll give it more thought 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regardless whether in this PR or not, we will probably want to make some change soon: because this PR doesn't include the system prompt, but you would like to do something similar with it, right?

So we still need Recall.

I think the system prompt just became more important to show to users, with these ContextWindowExceeded right at the start on OpenAI API: ultimately, users able to see and maybe edit it themselves would be good IMHO.

@enyst
Copy link
Collaborator Author

enyst commented Mar 14, 2025

(Though I kinda still like the more generalized "RecallAction" better -- because environment info fits a bit awkward in "MicroagentObservation" and can be confusing to display in the UI -- I would consider it is more like "PromptExtensionAction/Observation"

It is a bit strange! I almost stopped at half in the renaming process to think it over, because the LLM interpreted the MicroagentObservation as "the output of a MicroagentAction", which sounds like the microagent acted... That's the same confusion that existed because of the old microagents when people expected the new ones to "act" like the old ones.

These events, IMHO, are about information retrieval: we found the content of a microagent or more, and published it in the stream. Or runtime info, yes!

Microagents can be interpreted as a particular format in which a RecallObservation can find/bring information.

Or, as you said, we could look at the end result (we do this so that eventually they're added to the context), so PromptExtensions.

On the other hand, I can see maybe arguments for it:

  • we have a similar pattern with DelegateAction and DelegateObservation: the events belong to the parent. That works. So maybe it's perfectly fine, not confusing in that way
  • the events already include attributes that say "microagent", so it was RecallObservation with RecallType MICROAGENT_KNOWLEDGE, and a list of little dataclasses MicroagentKnowledge, which is maybe... half this style, half that style? I feel like, it wasn't actually generic yet, and it seems like it would affect a PromptExtension name too...
  • Robert got positive feedback on the name "microagent", maybe it will be good to double down on it for now 😄

(The 'environment' type is an enum value, maybe the UI can pick it up and do something a bit special about that type...)

Idk, the near future does seem to bring more about microagents (e.g. we can maybe handle TASK in a follow-up, custom global directory, retrieving library docs from the internet could be mapped to microagents - I suppose?), so I think maybe it's okay to test it and see where it leads us.

Cc: @rbren

@rbren
Copy link
Collaborator

rbren commented Mar 14, 2025

I trust your judgement here :)

* Revert "move out new tests"

This reverts commit cb3edc0

* move two tests

* adapt to MicroagentKnowledge dataclass

* adapt deduplication tests for reversed logic

* move tests of the new flow from old prompt manager

* adapt tests to the latest changes

* rename recall -> microagent

* add test for first user message

* adapt to serialization change
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
architecture Related to architecture backend Related to backend microagents Related to microagents
Projects
None yet
6 participants