fix(compression): preserve network/auth abort flags across cooldown re-entry (#29559) - #52056
fix(compression): preserve network/auth abort flags across cooldown re-entry (#29559)#52056srojk34 wants to merge 1 commit into
Conversation
…e-entry (NousResearch#29559) compress() unconditionally reset _last_summary_network_failure and _last_summary_auth_failure at the top of every call. When a second auto-compress fired during the 30s failure cooldown, _generate_summary() returned None from the cooldown early-return without re-asserting these flags — so the abort guard saw False and fell through to the destructive static-fallback path, dropping the middle context window. This re-introduced the exact data-loss scenario that PR NousResearch#51881 / issue NousResearch#29559 fixed. Both flags are already cleared on success (_generate_summary lines 1697-1698), so the eager reset was redundant. Remove it so the flags persist during cooldown and the abort guard fires correctly on re-entry. Adds two regression tests (network + auth) that trigger compress() twice — once to fail and once during cooldown — asserting the second call also returns messages unchanged with _last_compress_aborted=True.
|
Thanks for the focused fix. I verified the premise against current main and did not find blocking problems. Current main still resets The PR’s shape matches the bug class: it removes the eager resets and adds network plus auth regression tests for second-call cooldown re-entry. It also preserves the success path, where No design-fit issues found: no new tools, env vars, schema surface, or cache-breaking prompt changes. Salvageability looks high; this should be mostly mechanical to cherry-pick. Automated hermes-sweeper review. |
…e-entry (#29559) compress() eagerly reset _last_summary_auth_failure and _last_summary_network_failure at the top of every call. On a second compress() during the failure cooldown, _generate_summary() returns None from the cooldown early-return WITHOUT re-asserting those flags, so the abort guard saw False and fell through to the destructive static-fallback that drops the middle window — the data-loss #29559/#25585 describe. Stop resetting them eagerly; a successful summary already clears both, so letting them persist across calls is safe and keeps the cooldown abort protection intact. Salvaged from #52056. Co-authored-by: srojk34 <286497132+srojk34@users.noreply.github.com>
…rt flags, system-head summary role (NousResearch#51225 NousResearch#52056 NousResearch#52167) (#208) Co-authored-by: qbit-mirror-bot <qbit-mirror-bot@users.noreply.github.com>
…e-entry (NousResearch#29559) compress() eagerly reset _last_summary_auth_failure and _last_summary_network_failure at the top of every call. On a second compress() during the failure cooldown, _generate_summary() returns None from the cooldown early-return WITHOUT re-asserting those flags, so the abort guard saw False and fell through to the destructive static-fallback that drops the middle window — the data-loss NousResearch#29559/NousResearch#25585 describe. Stop resetting them eagerly; a successful summary already clears both, so letting them persist across calls is safe and keeps the cooldown abort protection intact. Salvaged from NousResearch#52056. Co-authored-by: srojk34 <286497132+srojk34@users.noreply.github.com>
…e-entry (NousResearch#29559) compress() eagerly reset _last_summary_auth_failure and _last_summary_network_failure at the top of every call. On a second compress() during the failure cooldown, _generate_summary() returns None from the cooldown early-return WITHOUT re-asserting those flags, so the abort guard saw False and fell through to the destructive static-fallback that drops the middle window — the data-loss NousResearch#29559/NousResearch#25585 describe. Stop resetting them eagerly; a successful summary already clears both, so letting them persist across calls is safe and keeps the cooldown abort protection intact. Salvaged from NousResearch#52056. Co-authored-by: srojk34 <286497132+srojk34@users.noreply.github.com> (cherry picked from commit 82ac7e1)
…e-entry (NousResearch#29559) compress() eagerly reset _last_summary_auth_failure and _last_summary_network_failure at the top of every call. On a second compress() during the failure cooldown, _generate_summary() returns None from the cooldown early-return WITHOUT re-asserting those flags, so the abort guard saw False and fell through to the destructive static-fallback that drops the middle window — the data-loss NousResearch#29559/NousResearch#25585 describe. Stop resetting them eagerly; a successful summary already clears both, so letting them persist across calls is safe and keeps the cooldown abort protection intact. Salvaged from NousResearch#52056. Co-authored-by: srojk34 <286497132+srojk34@users.noreply.github.com>
…e-entry (NousResearch#29559) compress() eagerly reset _last_summary_auth_failure and _last_summary_network_failure at the top of every call. On a second compress() during the failure cooldown, _generate_summary() returns None from the cooldown early-return WITHOUT re-asserting those flags, so the abort guard saw False and fell through to the destructive static-fallback that drops the middle window — the data-loss NousResearch#29559/NousResearch#25585 describe. Stop resetting them eagerly; a successful summary already clears both, so letting them persist across calls is safe and keeps the cooldown abort protection intact. Salvaged from NousResearch#52056. Co-authored-by: srojk34 <286497132+srojk34@users.noreply.github.com>
…e-entry (NousResearch#29559) compress() eagerly reset _last_summary_auth_failure and _last_summary_network_failure at the top of every call. On a second compress() during the failure cooldown, _generate_summary() returns None from the cooldown early-return WITHOUT re-asserting those flags, so the abort guard saw False and fell through to the destructive static-fallback that drops the middle window — the data-loss NousResearch#29559/NousResearch#25585 describe. Stop resetting them eagerly; a successful summary already clears both, so letting them persist across calls is safe and keeps the cooldown abort protection intact. Salvaged from NousResearch#52056. Co-authored-by: srojk34 <286497132+srojk34@users.noreply.github.com>
…e-entry (NousResearch#29559) compress() eagerly reset _last_summary_auth_failure and _last_summary_network_failure at the top of every call. On a second compress() during the failure cooldown, _generate_summary() returns None from the cooldown early-return WITHOUT re-asserting those flags, so the abort guard saw False and fell through to the destructive static-fallback that drops the middle window — the data-loss NousResearch#29559/NousResearch#25585 describe. Stop resetting them eagerly; a successful summary already clears both, so letting them persist across calls is safe and keeps the cooldown abort protection intact. Salvaged from NousResearch#52056. Co-authored-by: srojk34 <286497132+srojk34@users.noreply.github.com>
…e-entry (NousResearch#29559) compress() eagerly reset _last_summary_auth_failure and _last_summary_network_failure at the top of every call. On a second compress() during the failure cooldown, _generate_summary() returns None from the cooldown early-return WITHOUT re-asserting those flags, so the abort guard saw False and fell through to the destructive static-fallback that drops the middle window — the data-loss NousResearch#29559/NousResearch#25585 describe. Stop resetting them eagerly; a successful summary already clears both, so letting them persist across calls is safe and keeps the cooldown abort protection intact. Salvaged from NousResearch#52056. Co-authored-by: srojk34 <286497132+srojk34@users.noreply.github.com>
Summary
compress()unconditionally resets_last_summary_network_failureand_last_summary_auth_failureat the top of every call. When a second auto-compress fires during the 30-second failure cooldown,_generate_summary()returnsNonefrom the cooldown early-return without re-asserting these flags — so the abort guard seesFalseand falls through to the destructive static-fallback path, dropping the middle context window._generate_summarylines 1697-1698), so the eager reset was redundant and harmful. This PR removes it so the flags persist during cooldown and the abort guard fires correctly on re-entry.Repro scenario
ConnectionErrorduring auto-compress → firstcompress()correctly aborts (PR fix(compression): abort + preserve context on transient network summary failure (#29559, #25585) #51881) ✅compress()triggers within 30s cooldowncompress()resets_last_summary_network_failure = False→_generate_summary()hits cooldown guard, returnsNonewithout re-setting flag → abort guard seesFalse→ middle window dropped ❌Fix
Remove the eager
_last_summary_auth_failure = False/_last_summary_network_failure = Falsereset at the top ofcompress(). The flags are already cleared on success, and persisting them during cooldown is safe and correct.Test plan
TestCooldownReentryAbort::test_network_failure_cooldown_reentry_still_aborts— ConnectionError → first compress aborts → second compress during cooldown also aborts (not fallback)TestCooldownReentryAbort::test_auth_failure_cooldown_reentry_still_aborts— same for 401 auth failurestest_context_compressor.pytests pass (zero regression)