Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions src/agents/memory/openai_responses_compaction_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,20 @@ def _resolve_compaction_mode_for_response(
response_id: str | None,
store: bool | None,
requested_mode: OpenAIResponsesCompactionMode | None,
session_items: list[TResponseInputItem],
) -> _ResolvedCompactionMode:
mode = requested_mode or self.compaction_mode
if mode != "input":
settings = self.underlying_session.session_settings
limit = settings.limit if settings is not None else None
if limit is not None and len(session_items) > max(limit, 0):
if mode == "previous_response_id":
raise ValueError(
"OpenAIResponsesCompactionSession cannot use previous_response_id "
"compaction when the underlying session retrieval limit hides local "
"history; use compaction_mode='input' instead."
)
Comment on lines +165 to +171

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 Defer the incompatible-mode error until compaction is due

When compaction_mode="previous_response_id" wraps a session with a small retrieval limit, this condition raises as soon as stored history exceeds that limit—even if the default threshold has not been reached or a custom should_trigger_compaction hook would return False. Because the runner invokes run_compaction after persisting a successful response, such runs now fail after observable work despite no compaction being due; reject this configuration before the run starts, or raise only after the decision hook selects compaction.

AGENTS.md reference: AGENTS.md:L150-L150

Useful? React with 👍 / 👎.

return "input"
if (
mode == "auto"
and store is None
Expand Down Expand Up @@ -189,10 +201,13 @@ async def run_compaction(
self._last_unstored_response_id = None
else:
store = None

compaction_candidate_items, session_items = await self._ensure_compaction_candidates()
resolved_mode = self._resolve_compaction_mode_for_response(
response_id=self._response_id,
store=store,
requested_mode=requested_mode,
session_items=session_items,
)

if resolved_mode == "previous_response_id" and not self._response_id:
Expand All @@ -201,8 +216,6 @@ async def run_compaction(
"when using previous_response_id compaction."
)

compaction_candidate_items, session_items = await self._ensure_compaction_candidates()

force = args.get("force", False) if args else False
should_compact = force or self.should_trigger_compaction(
{
Expand Down Expand Up @@ -391,6 +404,7 @@ async def _defer_compaction(self, response_id: str, store: bool | None = None) -
response_id=response_id,
store=store,
requested_mode=None,
session_items=session_items,
)
should_compact = self.should_trigger_compaction(
{
Expand Down Expand Up @@ -442,7 +456,11 @@ async def _ensure_compaction_candidates(
if self._compaction_candidate_items is not None and self._session_items is not None:
return (self._compaction_candidate_items[:], self._session_items[:])

history = _normalize_compaction_session_items(await self.underlying_session.get_items())
# Bypass SessionSettings.limit so compaction sees stored history, not just the
# retrieval window. Replacement still writes over the full store.
history = _normalize_compaction_session_items(
await self._get_all_underlying_session_items()
)
Comment on lines +461 to +463

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 👍 / 👎.

candidates = select_compaction_candidate_items(history)
self._compaction_candidate_items = candidates
self._session_items = history
Expand Down
159 changes: 158 additions & 1 deletion tests/memory/test_openai_responses_compaction_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ def test_client_preserves_falsy_default_client(self) -> None:
def create_mock_session(self) -> MagicMock:
mock = MagicMock(spec=Session)
mock.session_id = "test-session"
mock.session_settings = None
mock.get_items = AsyncMock(return_value=[])
mock.add_items = AsyncMock()
mock.pop_item = AsyncMock(return_value=None)
Expand Down Expand Up @@ -424,7 +425,7 @@ async def test_run_compaction_auto_uses_default_store_when_unset(self) -> None:
mock_session.get_items.return_value = items

mock_compact_response = MagicMock()
mock_compact_response.output = []
mock_compact_response.output = items

mock_client = MagicMock()
mock_client.responses.compact = AsyncMock(return_value=mock_compact_response)
Expand Down Expand Up @@ -489,6 +490,100 @@ async def test_run_compaction_auto_uses_input_when_last_response_unstored(self)
assert "previous_response_id" not in second_kwargs
assert second_kwargs.get("input") == mock_compact_response.output

@pytest.mark.asyncio
async def test_run_compaction_auto_uses_full_input_when_session_limit_hides_history(
self, tmp_path
) -> None:
history = [
cast(
TResponseInputItem,
{"type": "message", "role": "assistant", "content": f"msg{i}"},
)
for i in range(DEFAULT_COMPACTION_THRESHOLD + 1)
]
underlying = SQLiteSession(
"limited-auto-compact",
str(tmp_path / "limited_auto_compact.db"),
session_settings=SessionSettings(limit=DEFAULT_COMPACTION_THRESHOLD - 1),
)
await underlying.add_items(history)

mock_compact_response = MagicMock()
mock_compact_response.output = [
{"type": "message", "role": "assistant", "content": "compacted"}
]
mock_client = MagicMock()
mock_client.responses.compact = AsyncMock(return_value=mock_compact_response)
session = OpenAIResponsesCompactionSession(
session_id="test",
underlying_session=underlying,
client=mock_client,
)

await session.run_compaction({"response_id": "resp-limited"})

mock_client.responses.compact.assert_awaited_once_with(model="gpt-4.1", input=history)
assert await underlying.get_items(limit=len(history)) == mock_compact_response.output

@pytest.mark.asyncio
async def test_run_compaction_rejects_previous_response_id_when_session_limit_hides_history(
self, tmp_path
) -> None:
history = [
cast(
TResponseInputItem,
{"type": "message", "role": "assistant", "content": f"msg{i}"},
)
for i in range(DEFAULT_COMPACTION_THRESHOLD + 1)
]
underlying = SQLiteSession(
"limited-response-compact",
str(tmp_path / "limited_response_compact.db"),
session_settings=SessionSettings(limit=DEFAULT_COMPACTION_THRESHOLD - 1),
)
await underlying.add_items(history)

mock_client = MagicMock()
mock_client.responses.compact = AsyncMock()
session = OpenAIResponsesCompactionSession(
session_id="test",
underlying_session=underlying,
client=mock_client,
compaction_mode="previous_response_id",
)

with pytest.raises(ValueError, match="use compaction_mode='input'"):
await session.run_compaction({"response_id": "resp-limited"})

mock_client.responses.compact.assert_not_awaited()
assert await underlying.get_items(limit=len(history)) == history

@pytest.mark.asyncio
async def test_run_compaction_previous_response_id_allows_empty_negative_limit(
self, tmp_path
) -> None:
underlying = SQLiteSession(
"empty-negative-limit-compact",
str(tmp_path / "empty_negative_limit_compact.db"),
session_settings=SessionSettings(limit=-1),
)
mock_compact_response = MagicMock()
mock_compact_response.output = []
mock_client = MagicMock()
mock_client.responses.compact = AsyncMock(return_value=mock_compact_response)
session = OpenAIResponsesCompactionSession(
session_id="test",
underlying_session=underlying,
client=mock_client,
compaction_mode="previous_response_id",
)

await session.run_compaction({"response_id": "resp-empty", "force": True})

mock_client.responses.compact.assert_awaited_once_with(
model="gpt-4.1", previous_response_id="resp-empty"
)

@pytest.mark.asyncio
async def test_run_compaction_skips_when_below_threshold(self) -> None:
mock_session = self.create_mock_session()
Expand All @@ -510,6 +605,23 @@ async def test_run_compaction_skips_when_below_threshold(self) -> None:
# Should not have called the compact API
mock_client.responses.compact.assert_not_called()

@pytest.mark.asyncio
async def test_defer_compaction_reuses_cached_full_history_for_mode_resolution(self) -> None:
mock_session = self.create_mock_session()
mock_session.get_items.return_value = [
cast(TResponseInputItem, {"type": "message", "role": "assistant", "content": "hi"})
]
session = OpenAIResponsesCompactionSession(
session_id="test",
underlying_session=mock_session,
)

await session._defer_compaction("resp-123")
await session._defer_compaction("resp-456")

assert mock_session.get_items.await_count == 1
assert mock_session.get_items.await_args_list[0].kwargs == {"limit": 2_147_483_647}

@pytest.mark.asyncio
async def test_run_compaction_executes_when_threshold_met(self) -> None:
mock_session = self.create_mock_session()
Expand Down Expand Up @@ -1026,6 +1138,50 @@ async def clear_session(self) -> None:
assert failing_session.clear_calls == 2
assert failing_session.add_calls == 2

@pytest.mark.asyncio
async def test_run_compaction_input_uses_full_history_when_session_limit_applies(
self, tmp_path
) -> None:
history: list[TResponseInputItem] = [
cast(TResponseInputItem, {"type": "message", "role": "user", "content": "oldest"}),
cast(
TResponseInputItem,
{"type": "message", "role": "assistant", "content": "middle"},
),
cast(TResponseInputItem, {"type": "message", "role": "user", "content": "newest"}),
]
compacted_items: list[TResponseInputItem] = [
cast(
TResponseInputItem,
{"type": "message", "role": "assistant", "content": "compacted"},
)
]

underlying = SQLiteSession(
"limited-compact",
str(tmp_path / "limited_compact.db"),
session_settings=SessionSettings(limit=2),
)
await underlying.add_items(history)
assert len(await underlying.get_items()) == 2

mock_compact_response = MagicMock()
mock_compact_response.output = compacted_items
mock_client = MagicMock()
mock_client.responses.compact = AsyncMock(return_value=mock_compact_response)

session = OpenAIResponsesCompactionSession(
session_id="test",
underlying_session=underlying,
client=mock_client,
compaction_mode="input",
)

await session.run_compaction({"force": True})

compact_input = mock_client.responses.compact.call_args.kwargs["input"]
assert compact_input == history

@pytest.mark.asyncio
async def test_run_compaction_does_not_restore_when_clear_fails_without_mutation(
self,
Expand Down Expand Up @@ -1709,6 +1865,7 @@ class TestCompactionStripsOrphanedIds:
def create_mock_session(self) -> MagicMock:
mock = MagicMock(spec=Session)
mock.session_id = "test-session"
mock.session_settings = None
mock.get_items = AsyncMock(return_value=[])
mock.add_items = AsyncMock()
mock.pop_item = AsyncMock(return_value=None)
Expand Down