Skip to content

fix(evaluator): stop concurrent Evaluator workers sharing one Agent's short_term_memory - #274

Open
AmirF194 wants to merge 1 commit into
ANative-Lab:mainfrom
AmirF194:fix/273-evaluator-concurrent-short-term-memory-sharing
Open

fix(evaluator): stop concurrent Evaluator workers sharing one Agent's short_term_memory#274
AmirF194 wants to merge 1 commit into
ANative-Lab:mainfrom
AmirF194:fix/273-evaluator-concurrent-short-term-memory-sharing

Conversation

@AmirF194

Copy link
Copy Markdown

Root cause

Evaluator._create_new_agent_manager() (per-thread, used by _parallel_evaluate)
and Evaluator._async_execute_workflow_graph() (per-task, used by
async_evaluate) each build what their own comments call a "new"
AgentManager from AgentManager(agents=self.agent_manager.agents, ...).
That list holds the same Agent instances as the original manager, so every
concurrent worker runs the real Agent.short_term_memory (a 5-message
sliding-window deque, written by _prepare_execution/_create_output_message
on 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 with
model_copy(update={"short_term_memory": ShortTermMemory()}) so every
worker 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_manager already said "same configuration but new locks";
this makes the per-agent mutable state match that intent too). Used at both
call sites.

Verification

  • New test reproduces the bug through the real, unmodified
    Evaluator._create_new_agent_manager(): two threads each write 5 messages
    into their own "independent" manager's agent and read back a contaminated,
    interleaved context. Fails on main (the two managers hand back the same
    Agent object); passes on this branch.
  • A second unit test exercises _agents_with_fresh_short_term_memory()
    directly (the same helper the async call site uses), asserting each clone
    gets its own ShortTermMemory while llm/name stay shared.
  • tests/src/evaluator/ (9/9) and tests/src/agents/ (69/69) pass on the
    branch; ran both suites against an unmodified checkout of this repo's
    requirements.txt dependency set plus a stub for the heavy optional tools
    (browser/vector-store backends) this code path never touches.
  • Did not run the async path (_async_execute_workflow_graph) end to end
    through a full WorkFlow; it calls the same now-tested helper, but that
    call site itself is covered only by the unit test above, not a live
    concurrent-workflow run.

Fixes #273

… 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] concurrent Evaluator workers share the same Agent.short_term_memory across benchmark examples

1 participant