Fix STLTMemory consolidation discarding the oldest memory entry before LLM summarization#187
Fix STLTMemory consolidation discarding the oldest memory entry before LLM summarization#187BEASTSHRIRAM wants to merge 2 commits intomesa:mainfrom
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
for more information, see https://pre-commit.ci
|
Already a fix for this issue here, #166. Please look into it. |
Summary
Fixes a silent data loss bug in
STLTMemory.process_step()andaprocess_step()where the oldest short-term memory entry was permanently dropped during consolidation becausepopleft()was called before_update_long_term_memory()ran. The fix movespopleft()to after the LLM summarization completes, ensuring every evicted entry is captured in long-term memory before being removed.Bug / Issue
Fixes #186
In a multi-round negotiation simulation using the
negotiationexample (or any model with long-running agents), eachLLMAgentusesSTLTMemory(capacity=5, consolidation_capacity=2)by default. After step 7 — when the deque first exceedscapacity + consolidation_capacity— consolidation triggers for the first time.At that moment, the agent's oldest entry (e.g., step 1: "Counterparty opened with $500k, non-negotiable on delivery timeline") is evicted from the deque before the LLM runs. The consolidation prompt only includes steps 2–7. The agent's long-term memory never records the opening offer.
By step 20, the agent has completely forgotten the counterparty's original constraints and begins making proposals that violate the terms it had agreed to never challenge — causing the negotiation to loop indefinitely or reach a logically inconsistent outcome. No error is raised; the model simply behaves incorrectly.
The bug is in
_process_step_core():Implementation
Removed
popleft()from the consolidation branch in_process_step_core()so the entry remains in the deque when_build_consolidation_prompt()callsformat_short_term(). The eviction is then performed in bothprocess_step()andaprocess_step()after the LLM call completes:The no-consolidation branch (
not self.consolidation_capacity) intentionally discards without summarizing — that path is unchanged.Testing
Added
test_consolidation_includes_evicted_entry()in test_STLT_memory.py. The test:STLTMemorywithcapacity=2, consolidation_capacity=1All 11 tests pass.
Additional Notes
This is a correctness bug in the default memory type used by every
LLMAgent. Any simulation longer thancapacity + consolidation_capacitysteps is affected. The failure is completely silent — the agent continues running with a corrupted long-term memory, making it very hard to diagnose in practice.No API changes. No new dependencies. The fix is 3 lines moved to a later point in the call sequence.