fix(gateway): Responses API stores bloated context after compression - #58133
fix(gateway): Responses API stores bloated context after compression#58133LiangYang666 wants to merge 11 commits into
Conversation
Compression produces a compact transcript in result['messages'], but _build_response_conversation_history detected a prefix mismatch and concatenated the original conversation_history on front. Detect compression via _last_compaction_in_place / session_id rotation and signal through result['_compressed'] so the builder uses the compressed transcript directly.
Related: #58118 (competing fix for the same bug, same file) and #58081 (the bug this fixes). Both PRs target the /v1/responses re-compression loop where the compressed transcript isn't stored correctly in ResponseStore, but via DIFFERENT mechanisms:
|
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: LGTM
Small fix (+88/-1) placing envelope cache breakpoints on message carriers that providers actually honor.
Looks Good
- Minimal, targeted change
- No security or performance concerns
Reviewed by Hermes Agent
tonydwb
left a comment
There was a problem hiding this comment.
Supplementary review: confirms LGTM. Responses API context bloat fix is a targeted performance fix. Prior COMMENT found no issues. No additional concerns.
Reviewed by Hermes Agent
Compression produces a compact transcript in result['messages'], but _build_response_conversation_history detected a prefix mismatch and concatenated the original conversation_history on front. Detect compression via _last_compaction_in_place / session_id rotation and signal through result['_compressed'] so the builder uses the compressed transcript directly.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting the Responses compression path. The reported bloat is still present on current main: gateway/platforms/api_server.py:4001-4013 falls back to prior + current_user + agent_messages when a compacted summary no longer shares the original-history prefix.
Problems
- The added test mocks
_run_agentwith"_compressed": true, so it does not verify the new detection path for either a rotatedagent.session_idor_last_compaction_in_place. - For the legacy rotation mode described in this PR,
_run_agentalready returns the effective ID (gateway/platforms/api_server.py:4259-4265), but the Responses handler persists the original request ID (gateway/platforms/api_server.py:3628-3633) and later reloads it for chaining (:3441-3467). The rotation branch should propagate that effective ID if it is intended to be supported end-to-end.
Suggested changes
- Add focused tests for both real compression signals and a chained legacy-rotation case that asserts the stored continuation session ID.
Automated hermes-sweeper review.
| "_compressed": True, | ||
| "api_calls": 1, | ||
| }, | ||
| {"input_tokens": 0, "output_tokens": 0, "total_tokens": 0}, |
There was a problem hiding this comment.
This mocks the flag consumed by the builder, but never exercises the new _run_agent detection. Please add focused fake-agent tests for both a rotated session_id and _last_compaction_in_place=True so the production signal propagation is covered.
- Detect when history is loaded from response_store (via previous_response_id) - Add history_from_store parameter to distinguish history source - When compression occurs, persist compressed messages instead of original - Add persist_in_response_store config option (default True) - Update session_id and response headers to reflect session rotation Cherry-picked from alidev 2eb816f6b
The persist logic only checked _result_sid != session_id (rotation), missing in-place mode where session_id is unchanged but _compressed flag is set. response_store history doubled every turn (11->26->55->110->225) causing repeated re-compression. Fix: detect compression via _did_compress or _rotated, and only update _effective_session_id on actual rotation (not in-place). Note: preflight loop break (turn_context.py) from original commit eee64097a is excluded — it's an optimization, not a bug fix. Cherry-picked from alidev eee64097a (api_server.py only)
Address review feedback (PR NousResearch#58133): the original test mocked _run_agent with _compressed=True directly, bypassing the detection logic. New tests mock _create_agent instead, so _run_agent's detection path runs naturally and reads agent.session_id / _last_compaction_in_place: 1. test_rotation_compression_exercises_detection_and_persists_rotated_session_id - Fake agent with rotated session_id -> verifies _compressed is set, compressed history is stored, and rotated session_id propagates to both response_store and X-Hermes-Session-Id header. 2. test_inplace_compression_exercises_detection_and_persists_compressed_history - Fake agent with _last_compaction_in_place=True, session_id unchanged -> verifies _compressed is set, compressed history is stored, and session_id does NOT rotate. 3. test_chained_rotation_propagates_effective_session_id - Two-request chain: first request triggers rotation, second request loads history using the rotated session_id stored by the first. Asserts the compressed transcript is loaded correctly for chaining.
…ess-store-bloat-local
…o fix/responses-compress-store-bloat-local # Conflicts: # tests/gateway/test_api_server.py
|
Hi @teknium1 , first off — thank you so much for taking the time to review this PR. I'm genuinely thrilled that a maintainer on a project of this scale looked at my contribution, it means a lot! You're absolutely right on both points, and I've just pushed updates that address them: 1. Test doesn't exercise detection path Fixed in 2. Rotation mode doesn't propagate effective session ID end-to-end Also fixed. We found in production that the Responses handler was persisting the original request session ID instead of the rotated one, so
Additional production finding: While running this in production, we also discovered that the persist logic only checked for rotation ( These fixes have been running in our production environment for several days now with no recurrence of the bloat issue. I've also merged the latest Thanks again for the review — it directly led to a much more robust fix! |
…ess-store-bloat-local # Conflicts: # gateway/platforms/api_server.py
|
@teknium1 I've just pushed an update that rebases this branch on the latest The conflict was because upstream refactored Also added a small improvement based on code review: the compressed |
Address review feedback (PR #58133): the original test mocked _run_agent with _compressed=True directly, bypassing the detection logic. New tests mock _create_agent instead, so _run_agent's detection path runs naturally and reads agent.session_id / _last_compaction_in_place: 1. test_rotation_compression_exercises_detection_and_persists_rotated_session_id - Fake agent with rotated session_id -> verifies _compressed is set, compressed history is stored, and rotated session_id propagates to both response_store and X-Hermes-Session-Id header. 2. test_inplace_compression_exercises_detection_and_persists_compressed_history - Fake agent with _last_compaction_in_place=True, session_id unchanged -> verifies _compressed is set, compressed history is stored, and session_id does NOT rotate. 3. test_chained_rotation_propagates_effective_session_id - Two-request chain: first request triggers rotation, second request loads history using the rotated session_id stored by the first. Asserts the compressed transcript is loaded correctly for chaining.
…g opt-out Rework on top of the salvaged #58133 commits: - Remove the compression.persist_in_response_store config key — this is a bug fix (stored transcripts must reflect what the agent will actually replay), not behavior that should be opt-out-able. - Drop the per-request load_config() imports the handler-level persist blocks added. - Dedupe the two handler-level persist blocks: the compressed-transcript substitution already lives in _build_response_conversation_history (via result["_compressed"]), so the handlers only need to propagate the effective (possibly rotation-changed) session_id. The streaming path does this via a new session_id_snapshot arg on _persist_response_snapshot; the non-streaming path picks up result["session_id"] directly. - Rotation propagation no longer gates on history-from-store: the first request in a chain can also rotate, and its stored session_id must be the child session or the next previous_response_id request resumes the pre-rotation session and re-compresses every turn.
Address review feedback (PR NousResearch#58133): the original test mocked _run_agent with _compressed=True directly, bypassing the detection logic. New tests mock _create_agent instead, so _run_agent's detection path runs naturally and reads agent.session_id / _last_compaction_in_place: 1. test_rotation_compression_exercises_detection_and_persists_rotated_session_id - Fake agent with rotated session_id -> verifies _compressed is set, compressed history is stored, and rotated session_id propagates to both response_store and X-Hermes-Session-Id header. 2. test_inplace_compression_exercises_detection_and_persists_compressed_history - Fake agent with _last_compaction_in_place=True, session_id unchanged -> verifies _compressed is set, compressed history is stored, and session_id does NOT rotate. 3. test_chained_rotation_propagates_effective_session_id - Two-request chain: first request triggers rotation, second request loads history using the rotated session_id stored by the first. Asserts the compressed transcript is loaded correctly for chaining.
…g opt-out Rework on top of the salvaged NousResearch#58133 commits: - Remove the compression.persist_in_response_store config key — this is a bug fix (stored transcripts must reflect what the agent will actually replay), not behavior that should be opt-out-able. - Drop the per-request load_config() imports the handler-level persist blocks added. - Dedupe the two handler-level persist blocks: the compressed-transcript substitution already lives in _build_response_conversation_history (via result["_compressed"]), so the handlers only need to propagate the effective (possibly rotation-changed) session_id. The streaming path does this via a new session_id_snapshot arg on _persist_response_snapshot; the non-streaming path picks up result["session_id"] directly. - Rotation propagation no longer gates on history-from-store: the first request in a chain can also rotate, and its stored session_id must be the child session or the next previous_response_id request resumes the pre-rotation session and re-compresses every turn.
Fixes #58081
What does this PR do?
Fix a bug where
POST /v1/responsesstores a bloated conversation history in ResponseStore after context compression, causing repeated re-compression on every subsequent request viaprevious_response_idorconversationname chaining.Root Cause
After compression,
_build_response_conversation_historyreceives the original uncompressedconversation_historyAND the compressedresult["messages"]. Its prefix-matching detection (_response_messages_turn_start_index) fails for compressed transcripts (because the compressed transcript starts with a summary, not the original history prefix), falling back to concatenating both:This doubles the stored context. On the next request, the bloated history is loaded from ResponseStore, compression fires again, and the cycle repeats indefinitely.
Fix
Two changes in
gateway/platforms/api_server.py:_run_agent— After the agent completes, detect compression via the two signals thatcompress_contextalready sets:agent.session_iddiffers from the inputsession_id(a new child session was created).compression.in_place: true):agent._last_compaction_in_placeisTrue(messages archived under the same session id).Set
result["_compressed"] = Truewhen either is true._build_response_conversation_history— Whenturn_start == 0andresult["_compressed"]is set, returnresult["messages"]directly instead of concatenating it withconversation_history.Why this approach
_response_messages_turn_start_index(a shared utility also used by_extract_output_items), avoiding side effects on response output construction.compress_contextitself — the canonical source of truth for whether compression happened — rather than heuristic message-content scanning._compressedflag is onlyTruewhen compression actually occurred — zero impact on normal (non-compressed) turns.Related
Type of Change
Changes Made
gateway/platforms/api_server.py—_run_agent: detect compression via session-id rotation / in-place flag, setresult["_compressed"]gateway/platforms/api_server.py—_build_response_conversation_history: skip concatenation when_compressedis settests/gateway/test_api_server.py— addtest_previous_response_id_stores_compressed_transcript_directlyHow to Test
POST /v1/responseswithstore: trueprevious_response_idchaining until compression firesconversation_historyin ResponseStore is the compressed transcript, not original + compressedChecklist
Code
pytest tests/gateway/test_api_server.pyand all tests pass