fix(evaluator): stop concurrent Evaluator workers sharing one Agent's short_term_memory - #274
Open
AmirF194 wants to merge 1 commit into
Conversation
… short_term_memory Evaluator._create_new_agent_manager() and _async_execute_workflow_graph() each build a "new" AgentManager from self.agent_manager.agents, but that list holds the same Agent instances, so every concurrent worker (thread pool or asyncio semaphore) mutates one shared Agent.short_term_memory with no lock. Messages from one benchmark example leak into another's LLM context. Add _agents_with_fresh_short_term_memory(), which clones each agent via model_copy() with a new ShortTermMemory, and use it at both call sites so every worker gets its own memory while still sharing the agent's configuration, llm, and tools. Fixes ANative-Lab#273
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause
Evaluator._create_new_agent_manager()(per-thread, used by_parallel_evaluate)and
Evaluator._async_execute_workflow_graph()(per-task, used byasync_evaluate) each build what their own comments call a "new"AgentManagerfromAgentManager(agents=self.agent_manager.agents, ...).That list holds the same
Agentinstances as the original manager, so everyconcurrent worker runs the real
Agent.short_term_memory(a 5-messagesliding-window deque, written by
_prepare_execution/_create_output_messageon every call) against one shared object with no lock. Two benchmark
examples evaluated at the same time end up reading each other's messages
back as their own LLM context.
Fix
Added
_agents_with_fresh_short_term_memory(), which clones each agent withmodel_copy(update={"short_term_memory": ShortTermMemory()})so everyworker gets its own memory while the clone still shares the agent's llm,
actions, and tools with the original (the docstring on
_create_new_agent_manageralready said "same configuration but new locks";this makes the per-agent mutable state match that intent too). Used at both
call sites.
Verification
Evaluator._create_new_agent_manager(): two threads each write 5 messagesinto their own "independent" manager's agent and read back a contaminated,
interleaved context. Fails on
main(the two managers hand back the sameAgentobject); passes on this branch._agents_with_fresh_short_term_memory()directly (the same helper the async call site uses), asserting each clone
gets its own
ShortTermMemorywhilellm/namestay shared.tests/src/evaluator/(9/9) andtests/src/agents/(69/69) pass on thebranch; ran both suites against an unmodified checkout of this repo's
requirements.txtdependency set plus a stub for the heavy optional tools(browser/vector-store backends) this code path never touches.
_async_execute_workflow_graph) end to endthrough a full
WorkFlow; it calls the same now-tested helper, but thatcall site itself is covered only by the unit test above, not a live
concurrent-workflow run.
Fixes #273