fix(compressor): batch salvage \u2014 orphan tool_calls, cooldown abort flags, system-head summary role (#51225 #52056 #52167) - #208
Merged
Conversation
…rt flags, system-head summary role (NousResearch#51225 NousResearch#52056 NousResearch#52167)
|
Review Complete Risk: 🟠 High (78/100) — no findings · 564 LOC across 2 files The compression summary is silently dropped when Files Reviewed (2 files) |
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.
Summary
User-requested batch salvage of three complementary P1 bug fixes to
agent/context_compressor.py, rebased onto currentmain. All three targeted bugs are confirmed still present on main; the three fixes touch non-overlapping regions and compose cleanly.Included PRs (authorship preserved per commit)
fix(compressor): strip orphan tool_calls instead of inserting stubs (#51218) NousResearch/hermes-agent#51225 (@liuhao1024) —
fix(compressor): strip orphan tool_calls instead of inserting stubs (#51218)_sanitize_tool_pairsinserted stubrole="tool"results for orphaned tool_calls; the pre-APIrepair_message_sequence()keys ontc.get("id")while the sanitizer keys oncall_id||id, so on the Codex Responses API format (id != call_id) the stubs were silently dropped and the orphans re-exposed. Now strips the orphaned tool_calls at the source (preserving text content, adding(tool call removed)only for an otherwise-empty assistant turn).fix(compression): preserve network/auth abort flags across cooldown re-entry (#29559) NousResearch/hermes-agent#52056 (@srojk34) —
fix(compression): preserve network/auth abort flags across cooldown re-entry (#29559)compress()eagerly reset_last_summary_auth_failure/_last_summary_network_failureat the top of every call. On a secondcompress()during the failure cooldown,_generate_summary()returnsNonefrom the cooldown early-return without re-asserting the flags, so the abort guard sawFalseand fell through to the destructive static-fallback that drops the middle window ([Bug] Context compression silently drops summary on connection error, causing total context loss for long tasks NousResearch/hermes-agent#29559/bug(compression): failed summaries should not discard conversation context NousResearch/hermes-agent#25585). The eager reset is removed; a successful summary already clears both flags, so persistence across calls is safe.fix(compressor): pin summary role to user when only system prompt is protected (#52160) NousResearch/hermes-agent#52167 (@liuhao1024) —
fix(compressor): pin summary role to user when only system prompt is protected (#52160)After the first compaction
protect_first_ndecays, so a later compaction can have only the system prompt as the protected head. Anthropic/Bedrock send the system prompt as a separate parameter, making the summary the firstmessages[]entry — and Anthropic rejects a first message that isn'trole=user(HTTP 400). Pins the summary touserfor a system-only head and blocks the collision-flip from reverting it.Follow-up commit (mine)
test(compressor): tidy blank-line spacing + assert placeholder never overwrites text— review-driven test cleanup only, no production change.Verification
pytest tests/agent/test_context_compressor.py→ 141 passed (includes 8 new tests acrossTestSanitizerStripsOrphanedToolCalls,TestCooldownReentryAbort,TestDoubleCompactionSummaryRole)._summary_failure_cooldown_untilassignments reintroduced incompress()/_generate_summary).Closes NousResearch#51225. Closes NousResearch#52056. Closes NousResearch#52167.
Co-authored-by: liuhao1024 sunsky.lau@gmail.com
Co-authored-by: srojk34 286497132+srojk34@users.noreply.github.com
Mirror-of: NousResearch#56197
NousResearch#56197