fix: load full history when compacting a limited session by winklemad · Pull Request #3827 · openai/openai-agents-python

winklemad · 2026-07-13T18:34:16Z

What & why

OpenAIResponsesCompactionSession._ensure_compaction_candidates loads the underlying history with a bare underlying_session.get_items(). For a session configured with a limit (SessionSettings(limit=N)), get_items() resolves the limit to session_settings.limit and returns only the latest N items.

In input/auto compaction mode those truncated items are exactly what gets sent to responses.compact, and run_compaction then clears the underlying session and replaces it with the compacted summary. So every item older than the limit window is silently and permanently lost. Compaction runs automatically once the candidate threshold is reached (no force needed), so this bites any user who pairs a limited session with compaction.

Minimal repro:

underlying = SQLiteSession("s", session_settings=SessionSettings(limit=3))
await underlying.add_items([...8 items...])
session = OpenAIResponsesCompactionSession("s", underlying, client=mock, compaction_mode="input")
await session.run_compaction({"force": True})
# responses.compact receives only the latest 3 items; after replacement the
# session holds a single summary of those 3 — items 0..4 are gone.

Fix

The restore/replace paths already read the complete history through _get_all_underlying_session_items() (limit=_ALL_SESSION_ITEMS_LIMIT), introduced in #3117; the candidate-loading path was missed. Use the same helper so compaction always operates on the full stored history.

Tests

Added test_run_compaction_uses_full_history_when_underlying_session_has_limit: a real SQLiteSession(limit=3) holding 8 items must send all 8 to responses.compact (fails before the fix — only the latest 3 arrive). Full tests/memory/ suite (146 tests) passes; ruff and mypy clean.

Found by reading the code; no linked issue.

OpenAIResponsesCompactionSession._ensure_compaction_candidates loaded the underlying history with a bare underlying_session.get_items(), which resolves to session_settings.limit and returns only the latest N items. In input/auto mode those truncated items are what gets sent to responses.compact, and run_compaction then clears the session and replaces it with the compacted summary — so any history older than the limit window is silently and permanently lost. The restore/replace paths already read the full history through _get_all_underlying_session_items() (limit=_ALL_SESSION_ITEMS_LIMIT), added in openai#3117; the candidate-loading path was missed. Use the same helper there so compaction always operates on the complete stored history.

seratch · 2026-07-13T22:34:29Z

@codex review

chatgpt-codex-connector · 2026-07-13T22:37:48Z

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: d6c8424884

ℹ️ 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".

seratch

Using _get_all_underlying_session_items() is correct for explicit input compaction. However, the current change also loads full history for thresholding while ordinary auto with a stored response_id still resolves to previous_response_id. That request does not include the full local history, but the session is then cleared and replaced, so older items can still be silently dropped; this patch can also make that compaction trigger earlier.

Please make auto fall back to full-history input compaction whenever the stored history is not fully represented by the limited session view, while leaving explicitly requested previous_response_id behavior opt-in. Add a no-force regression test covering that auto path before replacement.

…imited In auto mode with a stored response_id, compaction resolves to previous_response_id, which does not send the local history. When the underlying session's default view is limit-bounded and does not cover the full stored history, clearing and replacing the session with the server-derived summary can still drop the unrepresented items. Auto now falls back to full-history input compaction whenever the limited view does not cover the stored history; an explicitly requested previous_response_id compaction stays opt-in. Add a no-force regression test for the auto path, and guard the test assertions against an Optional input value for pyright.

winklemad · 2026-07-13T23:07:48Z

Thanks @seratch — you're right, the previous_response_id path was still exposed. Pushed a follow-up:

Auto now falls back to full-history input compaction when the resolved mode would be previous_response_id but the underlying session's default (limit-bounded) view doesn't cover the full stored history. That way the clear+replace can't drop unrepresented items. An explicitly requested previous_response_id compaction is left opt-in and unchanged.
Detection is a small _underlying_view_covers_history() helper comparing the default get_items() view against the full view; the extra read only happens on the auto + previous_response_id branch.
Added test_run_compaction_auto_falls_back_to_input_when_history_truncated — a no-force test covering the auto path before replacement (12 candidates over a limit=3 session trigger compaction via the threshold and must send all 12 to responses.compact rather than resolving to previous_response_id).
Also fixed the typecheck (pyright reportOptionalIterable) failure by guarding the test's input assertions.

Full tests/memory/ suite passes; ruff, mypy, and pyright are clean on the changed files. PTAL.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5469581c1c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

When auto compaction falls back to input because the limited session view did not cover the full history, the stored response_id still points at a server response that omits the hidden items. Once the session later shrinks back within the limit, a forced compaction would otherwise switch back to previous_response_id and replace the full-history summary with a server-derived one that drops those items. Mark the response_id unsafe on fallback, mirroring the store=False path, and cover it with a regression test.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 10471fd7f5

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

The auto->input fallback checked the underlying session's default view, which misses a limit supplied per run (e.g. RunConfig session settings) on an otherwise-unlimited store: the response only saw the limited window, but the check reported full coverage and kept using previous_response_id. Remember the limit the Runner uses to prepare each turn's input via get_items and compare against that window instead, so the fallback triggers whenever the response did not see the full history.

winklemad · 2026-07-14T19:17:55Z

@seratch ready for another look when you get a chance — the previous_response_id path you flagged now falls back to full-history input compaction whenever the session's limit-bounded view doesn't cover the full stored history, so the clear+replace can't drop unrepresented items. The two follow-up nits since are folded in as well. Thanks!

seratch

Thanks for addressing the previous-response fallback, unsafe ID reuse, and per-run limits. The remaining blocker is ownership of the per-run limit.

_last_prepared_input_limit is one mutable field updated by every public get_items() call, rather than state tied to a specific run or response. If A prepares with limit=3, B prepares with limit=None, and A then completes, A can read B's limit and use previous_response_id even though response A omitted older history. A fresh compaction wrapper on RunState resume has the same problem because resume skips session input preparation.

Please carry the effective history-window metadata with the specific run/response into both compaction and deferred compaction instead of inferring it from shared get_items() side effects. Please also add controlled-overlap and fresh-wrapper-resume regressions that assert the surviving operation and full stored history.

…ession state

winklemad · 2026-07-15T00:34:42Z

Thanks @seratch — good catch on the ownership issue. Reworked it so the history window travels with the specific response instead of being inferred from shared get_items() side effects:

Added input_history_limit to OpenAIResponsesCompactionArgs. save_result_to_session resolves the effective window (session settings + per-run RunConfig) through a shared resolve_session_history_limit helper and passes it alongside the response into both run_compaction and _defer_compaction.
Removed _last_prepared_input_limit entirely; _underlying_view_covers_history now takes the window as an argument. So the controlled-overlap case (A prepares with limit=3, B with limit=None) can no longer let A read B's limit, and a fresh wrapper on resume — which skips input preparation — still gets the right window because the Runner carries it through save_result_to_session / save_resumed_turn_items (threaded session_settings into those paths).

Added the two regressions you asked for, both asserting the surviving operation compacts from the full stored history:

test_run_compaction_uses_per_response_limit_under_interleaved_get_items (controlled overlap)
test_save_result_to_session_carries_run_limit_to_fresh_wrapper_on_resume (fresh wrapper on resume, driven through the Runner save path)

Both fail on the previous commit and pass now. Full tests/memory/ + runner suites pass; ruff, mypy, and pyright are clean. PTAL.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d79002827f

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-07-15T00:38:53Z

+        # itself, so compaction can tell whether it saw the full stored history instead of
+        # inferring it from shared session state (which breaks under interleaved runs and
+        # fresh wrappers on resume that skip input preparation).
+        input_history_limit = resolve_session_history_limit(session, session_settings)


Thread run limits through save_turn_items_if_needed

This only preserves the per-run limit for callers that pass session_settings into save_result_to_session; the normal final-output and run-again paths still go through save_turn_items_if_needed, whose signature has no session_settings parameter and calls this function without one. With an OpenAIResponsesCompactionSession wrapping an otherwise-unlimited store and RunConfig(session_settings=SessionSettings(limit=N)), those paths resolve input_history_limit to None here even though prepare_input_with_session sent only the limited window, so auto compaction can still use previous_response_id and replace the full local history with a server summary that never saw the older items.

Useful? React with 👍 / 👎.

…ut save paths

winklemad · 2026-07-15T01:25:08Z

Follow-up: I widened the fix so the per-run window now flows through every save path that can reach compaction, not just the direct save_result_to_session calls. save_turn_items_if_needed (the final-output / run-again paths) and the streaming _save_stream_items / _save_resumed_stream_items closures now thread session_settings too, so a limited run compacts the full stored history on those paths as well.

Added regressions for each: the final-output path (test_save_turn_items_if_needed_carries_run_limit), the deferred-under-limit path (test_deferred_compaction_under_limit_uses_full_history, covering the "into both compaction and deferred compaction" point), and the streaming path (test_save_stream_items_carries_run_limit). Each fails before its threading and passes after. ruff, mypy, and pyright are clean; the full tests/memory/ + runner suites pass. PTAL @seratch.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e2ace6c9ec

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-07-15T01:30:34Z

+        prepared_view = await self.underlying_session.get_items(limit=input_history_limit)
+        full_view = await self._get_all_underlying_session_items()
+        return len(prepared_view) >= len(full_view)


Track pre-save truncation instead of post-save length

When a limited run starts with history that fits within the limit but the current turn's saved input/output pushes the stored session over that limit, this recomputes prepared_view after save_result_to_session() has already added the new items. In that case the response actually saw all pre-existing history, so previous_response_id is safe, but this returns false and forces full-history input compaction; for large sessions at the limit this can turn a safe server-side compaction into an oversized/expensive input request. Carry whether the model input was actually truncated at preparation time, or compare against the pre-save history, rather than the post-save session length.

Useful? React with 👍 / 👎.

winklemad · 2026-07-15T02:02:40Z

One more refinement on the efficiency edge: a limited run whose pre-existing history already fit within the window shouldn't be forced onto full-history input just because this turn grows the stored session past the limit — the response genuinely saw everything, so previous_response_id is still safe.

Coverage is now resolved against the pre-save store (the state the response was actually prepared from) in save_result_to_session, and carried as input_covered_full_history rather than recomputed from the post-save session length. The check is a single bounded read that never reports "covered" when it can't prove it, so it stays safe (never drops history) while avoiding the needless oversized input request at the limit boundary. Added test_compaction_reuses_previous_response_id_when_pre_save_history_fits (fails before / passes after). ruff, mypy, and pyright are clean; full tests/memory/ + runner suites pass.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2788e93e1c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-07-15T02:05:42Z

+        history_limit = resolve_session_history_limit(session, session_settings)
+        if history_limit is None:
+            input_covered_full_history = True


Honor wrapped session limits before trusting response IDs

When session is an OpenAIResponsesCompactionSession wrapping a limited store, resolve_session_history_limit() sees the wrapper's session_settings as None, even though prepare_input_with_session() later delegates get_items(None) to the underlying session and that underlying default limit truncates the model input. This branch then records input_covered_full_history=True, so normal Runner saves with an underlying SessionSettings(limit=N) keep using previous_response_id and can replace the full local store with a server summary that never saw the older hidden items. Propagate the wrapped session's effective limit here, or leave the coverage unknown so run_compaction() can compare against the full underlying history.

Useful? React with 👍 / 👎.

…ndow

winklemad · 2026-07-15T05:16:09Z

This round is a correctness fix, not just efficiency. When the compaction session wraps a store that carries its own SessionSettings(limit=N) — rather than the wrapper or a RunConfig — resolve_session_history_limit previously saw None (it only read the wrapper's settings) and recorded input_covered_full_history=True, even though get_items(None) delegates to that underlying store and its limit truncates the input. A normal save could then keep previous_response_id and drop the hidden older items.

It now walks the delegate chain (underlying_session) to find the effective limit, so the truncation is detected and compaction falls back to full-history input. Added test_compaction_honors_underlying_session_limit — a save whose limit lives only on the wrapped store must not reuse previous_response_id (fails before / passes after). ruff, mypy, and pyright are clean; the full tests/memory/ + runner suites pass.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5570d8091a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-07-15T05:20:21Z

+            # Fewer stored items than the window means the window held them all; a full
+            # window may still hide older items, so treat that as truncated.
+            prepared_view = await session.get_items(limit=history_limit)
+            input_covered_full_history = len(prepared_view) < history_limit


Treat exact-limit histories as fully covered

The new pre-save check still classifies an exact-fill window as truncated: when the stored history has exactly history_limit items before the turn, the model input contained the full history, but len(prepared_view) < history_limit is false. If the current turn then pushes the session over the limit and compaction triggers, auto mode falls back to full-history input even though previous_response_id is safe, which can turn a server-side compaction into an unnecessarily large or failing input compaction request. Consider checking whether there is an item beyond the limit, e.g. by comparing against a pre-save full count or requesting history_limit + 1.

Useful? React with 👍 / 👎.

winklemad · 2026-07-15T05:45:14Z

Tightened the pre-save coverage check to be exact. It now reads one item beyond the window (get_items(limit + 1)), so a history that exactly fills the limit counts as covered rather than truncated — the earlier len < limit form treated an exact fill as truncated and could force an unnecessary full-history input at the boundary. Since an explicit limit arg wins over a session's own default (resolve_session_limit), get_items(limit + 1) still surfaces a real overflow item, so truncation (including the wrapped-store case) is still detected and it never reports "covered" without proof. Added test_compaction_reuses_previous_response_id_at_exact_limit (fails before / passes after). ruff, mypy, and pyright are clean; the full tests/memory/ + runner suites pass.

winklemad · 2026-07-15T11:48:04Z

@seratch this is ready for another look — consolidating where it landed, since the fixes ended up spread across a few commits.

The ownership blocker is resolved. The history-window state no longer lives in a shared mutable field (_last_prepared_input_limit is gone). save_result_to_session now computes, per response, whether that response's input window covered the full stored history (input_covered_full_history), measured against the pre-save store the response was actually prepared from, and carries that boolean into run_compaction / _defer_compaction. Because it travels with the specific response, interleaved runs and fresh wrappers on resume can no longer read each other's window.

Both regressions you asked for are in:

test_run_compaction_uses_per_response_limit_under_interleaved_get_items — A prepares with a limit, B with none; asserts A still uses full-history input and the full stored history survives the replace.
test_save_result_to_session_carries_run_limit_to_fresh_wrapper_on_resume — a fresh wrapper on resume, asserting the per-run limit is honored with no prepare step.

Also hardened along the way: per-run RunConfig(session_settings=…) limits now flow through every save path that reaches compaction (final-output, run-again, streaming); an underlying store's own SessionSettings(limit=N) is honored via the delegate chain; and the pre-save coverage check reads one item beyond the window so an exact-fill window counts as covered rather than being needlessly forced onto input compaction.

Green on the current head: the compaction suite (59 tests) plus ruff, mypy, and pyright clean on the changed files.

seratch added the feature:sessions label Jul 13, 2026

seratch requested changes Jul 13, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Jul 13, 2026

View reviewed changes

Comment thread src/agents/memory/openai_responses_compaction_session.py

chatgpt-codex-connector Bot reviewed Jul 13, 2026

View reviewed changes

Comment thread src/agents/memory/openai_responses_compaction_session.py Outdated

seratch requested changes Jul 14, 2026

View reviewed changes

Carry per-response history window into compaction instead of shared s…

d790028

…ession state

chatgpt-codex-connector Bot reviewed Jul 15, 2026

View reviewed changes

Carry the per-run history window through the streaming and final-outp…

e2ace6c

…ut save paths

chatgpt-codex-connector Bot reviewed Jul 15, 2026

View reviewed changes

Measure compaction history coverage against the pre-save store

2788e93

chatgpt-codex-connector Bot reviewed Jul 15, 2026

View reviewed changes

Honor an underlying session's own limit when resolving the history wi…

5570d80

…ndow

chatgpt-codex-connector Bot reviewed Jul 15, 2026

View reviewed changes

Treat an exact-limit history window as covered, not truncated

8c41741

Uh oh!

Conversation

winklemad commented Jul 13, 2026

What & why

Fix

Tests

Uh oh!

seratch commented Jul 13, 2026

Uh oh!

chatgpt-codex-connector Bot commented Jul 13, 2026

Uh oh!

seratch left a comment

Choose a reason for hiding this comment

Uh oh!

winklemad commented Jul 13, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

winklemad commented Jul 14, 2026

Uh oh!

seratch left a comment

Choose a reason for hiding this comment

Uh oh!

winklemad commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

winklemad commented Jul 15, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

winklemad commented Jul 15, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

winklemad commented Jul 15, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

winklemad commented Jul 15, 2026

Uh oh!

winklemad commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

winklemad commented Jul 15, 2026 •

edited

Loading