fix(compression): clear all per-session state in on_session_end, not just _previous_summary#42891
Open
blut-agent wants to merge 1 commit into
Conversation
…just _previous_summary The original cross-session contamination fix (NousResearch#38788) only cleared _previous_summary in on_session_end(), but on_session_reset() clears 14+ per-session variables. When a session ends (cron exit, gateway expiry, session-id rotation) and the compressor instance is reused, the surviving stale state causes: - _ineffective_compression_count surviving → next session skips compression prematurely (anti-thrashing guard misfires) - _summary_failure_cooldown_until surviving → next session blocks summary generation for an unrelated transient error - _last_compress_aborted surviving → callers think compression is still aborted - _last_aux_model_failure_* surviving → stale error warnings shown - _last_summary_dropped_count / _last_summary_fallback_used surviving → misleading user warnings - _context_probed / _context_probe_persistable surviving → stale context-probe state Also fix on_session_reset() which was missing _last_compress_aborted clearing — a /new or /reset would inherit the aborted flag from the prior conversation. Add 6 targeted tests covering the leak vectors and a parity test ensuring on_session_end and on_session_reset always clear the same surface.
Contributor
|
Code Review — clean ✅ Reviewed the full diff (context_compressor.py + tests). The fix correctly identifies the root cause: The fix extends No issues found. 🚀 |
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
3 participants
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.
Summary
The cross-session contamination fix (#38788) only cleared
_previous_summaryinon_session_end(), buton_session_reset()clears 14+ per-session variables. When a session ends (cron exit, gateway expiry, session-id rotation) and the compressor instance is reused, the surviving stale state causes real user-visible bugs:Impact of stale state surviving across sessions
_ineffective_compression_count_summary_failure_cooldown_until_last_compress_aborted_last_aux_model_failure_*_last_summary_dropped_count_last_summary_fallback_used_context_probed/_context_probe_persistableBonus fix
on_session_reset()was also missing_last_compress_abortedclearing — a/newor/resetwould inherit the aborted flag from the prior conversation.Changes
agent/context_compressor.py:on_session_end()now clears all 16 per-session variables (matchingon_session_reset()surface).on_session_reset()now also clears_last_compress_aborted.tests/agent/test_context_compressor_session_end_clears_state.py: 6 new tests:test_on_session_end_clears_all_per_session_state— comprehensive check of every fieldtest_on_session_end_matches_on_session_reset_surface— parity test ensuring both methods clear the same surfacetest_ineffective_compression_count_does_not_leak_across_sessionstest_summary_failure_cooldown_does_not_leak_across_sessionstest_compress_aborted_flag_does_not_leak_across_sessionstest_aux_model_failure_does_not_leak_across_sessionsTest results
All 110 context compressor tests pass (including 3 existing cross-session guard tests + 6 new).
Related
_previous_summary)_previous_summaryon session end)