Skip to content

fix(memory): compact the full stored history of a limited session - #4628

Open
hsusul wants to merge 1 commit into
openai:mainfrom
hsusul:fix/compaction-load-full-limited-session-history
Open

fix(memory): compact the full stored history of a limited session#4628
hsusul wants to merge 1 commit into
openai:mainfrom
hsusul:fix/compaction-load-full-limited-session-history

Conversation

@hsusul

@hsusul hsusul commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

OpenAIResponsesCompactionSession loads compaction input with underlying_session.get_items(), which applies SessionSettings.limit. After responses.compact, it clears the entire underlying store and writes the compact result.

For a SQLite (or similar) session that stores full history but retrieves only the last N items, successful compaction therefore deletes everything older than N. The restore path already uses _get_all_underlying_session_items(); candidate loading did not.

Reproduction

from agents.memory import SQLiteSession, SessionSettings, OpenAIResponsesCompactionSession

underlying = SQLiteSession("s", session_settings=SessionSettings(limit=2))
# add 3+ items, then force input-mode compaction
# responses.compact receives only the last 2 items
# the session is then replaced with that compact output

Solution

Load candidates via the existing _get_all_underlying_session_items() helper (limit=2_147_483_647), matching the replacement/restore path.

This is a smaller version of the candidate-loading fix from closed stale #3827, without that PR's broader previous_response_id rewrite.

Test plan

  • Added a SQLiteSession regression that sets SessionSettings(limit=2), stores 3 items, and asserts responses.compact receives all 3
  • uv run pytest tests/memory/test_openai_responses_compaction_session.py (56 passed)
  • uv run ruff format / ruff check on the changed files
  • uv run pyright on the changed files
  • git diff --check clean

Issue number

N/A. Related closed stale PR: #3827.

Checks

  • I've added new tests, if relevant
  • I've run targeted format, lint, typecheck, and tests on the changed files
  • I've run .agents/skills/code-change-verification/scripts/run.sh (full make format currently fails on an unrelated pre-existing E501 in .agents/skills/implementation-final-review/scripts/test_skill_contract.py on main)
  • If using Codex, I've run /review before submitting this PR

OpenAIResponsesCompactionSession loaded candidates with get_items(),
which applies SessionSettings.limit. Compaction then replaced the entire
store with a summary of that truncated window, permanently dropping older
items. Load history through the existing unbounded helper instead.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 76381b5c04

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +447 to +449
history = _normalize_compaction_session_items(
await self._get_all_underlying_session_items()
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid triggering response-ID compaction from hidden rows

When the underlying session has SessionSettings(limit=N) with N below the threshold but more than ten candidates stored, this full read now triggers default "auto" compaction; however, _resolve_compaction_mode() selects previous_response_id for a normally stored response, so the compaction request does not include these full session_items. Because that response was created using only the limited session window, its compacted output cannot represent the older rows, yet the replacement clears the entire local store. This therefore newly deletes the hidden history in the default mode; either switch to input-mode compaction when the full store differs from the retrieval window or keep hidden rows from triggering response-ID compaction.

AGENTS.md reference: AGENTS.md:L201-L203

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants