fix(compressor): batch salvage \u2014 orphan tool_calls, cooldown abort flags, system-head summary role (#51225 #52056 #52167) - #56197
Merged
kshitijk4poor merged 4 commits intoJul 1, 2026
Conversation
…ousResearch#51218) _sanitize_tool_pairs inserted stub role="tool" results for orphaned tool_calls. The pre-API repair_message_sequence() tracks known call IDs by tc.get("id") while this sanitizer keys on call_id||id; when they disagree (Codex Responses API: id != call_id) the stubs are silently dropped by the repair pass, re-exposing the original orphans. Strip the orphaned tool_calls at the source instead (preserving any text content, adding a placeholder for an otherwise-empty assistant turn) to avoid the mismatch class entirely. Salvaged from NousResearch#51225. Co-authored-by: liuhao1024 <sunsky.lau@gmail.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>
…protected (NousResearch#52160) After the first compaction protect_first_n decays, so on a later compaction the only protected head message can be the system prompt. Adapters like Anthropic and Bedrock send the system prompt as a separate parameter, so the summary becomes the first message in messages[] — and Anthropic rejects any request whose first message is not role=user (HTTP 400). Pin the summary to role=user when the head is system-only, and stop the collision-flip logic from reverting it back to assistant. Salvaged from NousResearch#52167. Co-authored-by: liuhao1024 <sunsky.lau@gmail.com>
…overwrites text Review follow-up on the batch salvage: normalize the inter-class spacing to two blank lines (PEP8) between the three new test classes, and add an explicit assertion in test_sanitizer_strips_orphaned_preserves_text_content that the '(tool call removed)' placeholder does NOT overwrite existing assistant text. No production change.
kshitijk4poor
enabled auto-merge (rebase)
July 1, 2026 08:50
kshitijk4poor
disabled auto-merge
July 1, 2026 08:50
This was referenced Jul 1, 2026
alt-glitch
added
type/bug
Collaborator
|
1 task
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
4 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
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) #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) #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 #29559/bug(compression): failed summaries should not discard conversation context #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) #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 #51225. Closes #52056. Closes #52167.
Co-authored-by: liuhao1024 sunsky.lau@gmail.com
Co-authored-by: srojk34 286497132+srojk34@users.noreply.github.com