Skip to content

Commit 2bb98d8

Browse files
committed
fix review comments
1 parent 1e8b70a commit 2bb98d8

5 files changed

Lines changed: 83 additions & 20 deletions

File tree

src/agents/memory/openai_responses_compaction_session.py

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -185,17 +185,58 @@ def _resolve_compaction_mode(
185185

186186
return resolved_mode
187187

188+
def _resolve_store_tracking(
189+
self,
190+
*,
191+
response_id: str | None,
192+
previous_response_id: str | None,
193+
store: bool | None,
194+
store_was_provided: bool,
195+
) -> bool | None:
196+
"""Resolve the effective store setting for the current response id.
197+
198+
Reuse `_last_store` only while compaction still refers to the same response. A new
199+
response id with no explicit `store` falls back to the Responses API default behavior.
200+
"""
201+
if store_was_provided:
202+
self._last_store = store
203+
return store
204+
205+
if response_id is not None and response_id != previous_response_id:
206+
self._last_store = None
207+
return None
208+
209+
return self._last_store
210+
211+
def _get_effective_store_for_response_id(
212+
self,
213+
*,
214+
response_id: str | None,
215+
store: bool | None,
216+
) -> bool | None:
217+
"""Return the effective store setting without mutating response tracking."""
218+
if store is not None:
219+
return store
220+
if response_id is not None and response_id != self._response_id:
221+
return None
222+
return self._last_store
223+
188224
async def run_compaction(self, args: OpenAIResponsesCompactionArgs | None = None) -> None:
189225
"""Run compaction using responses.compact API."""
190226
previous_response_id = self._response_id
191227
if args and args.get("response_id"):
192228
self._response_id = args["response_id"]
193229
requested_mode = args.get("compaction_mode") if args else None
194-
if args and "store" in args:
195-
store: bool | None = args["store"]
196-
self._last_store = store
197-
else:
198-
store = self._last_store
230+
store_was_provided = bool(args and "store" in args)
231+
requested_store: bool | None = (
232+
args["store"] if args is not None and "store" in args else None
233+
)
234+
store = self._resolve_store_tracking(
235+
response_id=self._response_id,
236+
previous_response_id=previous_response_id,
237+
store=requested_store,
238+
store_was_provided=store_was_provided,
239+
)
199240
turn_has_local_adds_without_new_response_id = (
200241
self._has_unacknowledged_local_session_adds
201242
and (args is None or args.get("response_id") in {None, previous_response_id})
@@ -320,7 +361,7 @@ async def _defer_compaction(self, response_id: str, store: bool | None = None) -
320361
resolved_mode = _resolve_compaction_mode(
321362
self.compaction_mode,
322363
response_id=response_id,
323-
store=store if store is not None else self._last_store,
364+
store=self._get_effective_store_for_response_id(response_id=response_id, store=store),
324365
)
325366
should_compact = self.should_trigger_compaction(
326367
{

src/agents/run.py

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
from .run_internal.agent_runner_helpers import (
4848
append_model_response_if_new,
4949
apply_resumed_conversation_settings,
50+
attach_run_state_metadata,
5051
build_interruption_result,
5152
build_resumed_stream_debug_extra,
5253
ensure_context_wrapper,
@@ -828,8 +829,6 @@ def _with_reasoning_item_id_policy(result: RunResult) -> RunResult:
828829
result._replay_from_model_input_items = list(
829830
generated_items
830831
) != list(session_items)
831-
if run_state is not None:
832-
result._trace_state = run_state._trace_state
833832
if session_persistence_enabled:
834833
input_items_for_save_1: list[TResponseInputItem] = (
835834
session_input_items_for_persistence
@@ -844,6 +843,7 @@ def _with_reasoning_item_id_policy(result: RunResult) -> RunResult:
844843
response_id=turn_result.model_response.response_id,
845844
store=store_setting,
846845
)
846+
attach_run_state_metadata(result, run_state=run_state)
847847
result._original_input = copy_input_items(original_input)
848848
return finalize_conversation_tracking(
849849
_with_reasoning_item_id_policy(result),
@@ -965,8 +965,6 @@ def _with_reasoning_item_id_policy(result: RunResult) -> RunResult:
965965
result._replay_from_model_input_items = list(generated_items) != list(
966966
session_items
967967
)
968-
if run_state is not None:
969-
result._trace_state = run_state._trace_state
970968
if session_persistence_enabled and include_in_history:
971969
handler_input_items_for_save: list[TResponseInputItem] = (
972970
session_input_items_for_persistence
@@ -981,6 +979,7 @@ def _with_reasoning_item_id_policy(result: RunResult) -> RunResult:
981979
response_id=None,
982980
store=store_setting,
983981
)
982+
attach_run_state_metadata(result, run_state=run_state)
984983
result._original_input = copy_input_items(original_input)
985984
return finalize_conversation_tracking(
986985
_with_reasoning_item_id_policy(result),
@@ -1236,10 +1235,6 @@ def _with_reasoning_item_id_policy(result: RunResult) -> RunResult:
12361235
result._replay_from_model_input_items = list(generated_items) != list(
12371236
session_items
12381237
)
1239-
if run_state is not None:
1240-
result._current_turn_persisted_item_count = (
1241-
run_state._current_turn_persisted_item_count
1242-
)
12431238
await save_turn_items_if_needed(
12441239
session=session,
12451240
run_state=run_state,
@@ -1249,6 +1244,7 @@ def _with_reasoning_item_id_policy(result: RunResult) -> RunResult:
12491244
response_id=turn_result.model_response.response_id,
12501245
store=store_setting,
12511246
)
1247+
attach_run_state_metadata(result, run_state=run_state)
12521248
result._original_input = copy_input_items(original_input)
12531249
return finalize_conversation_tracking(
12541250
_with_reasoning_item_id_policy(result),

src/agents/run_internal/agent_runner_helpers.py

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
from .tool_use_tracker import AgentToolUseTracker, serialize_tool_use_tracker
3434

3535
__all__ = [
36+
"attach_run_state_metadata",
3637
"apply_resumed_conversation_settings",
3738
"append_model_response_if_new",
3839
"build_generated_items_details",
@@ -276,6 +277,17 @@ def finalize_conversation_tracking(
276277
return result
277278

278279

280+
def attach_run_state_metadata(result: RunResult, *, run_state: RunState | None) -> RunResult:
281+
"""Copy resumable state metadata from the current RunState onto a RunResult."""
282+
if run_state is None:
283+
return result
284+
285+
result._state = run_state
286+
result._current_turn_persisted_item_count = run_state._current_turn_persisted_item_count
287+
result._trace_state = run_state._trace_state
288+
return result
289+
290+
279291
def build_interruption_result(
280292
*,
281293
result_input: str | list[TResponseInputItem],
@@ -315,10 +327,7 @@ def build_interruption_result(
315327
result._current_turn = current_turn
316328
result._model_input_items = list(generated_items)
317329
result._replay_from_model_input_items = list(generated_items) != list(session_items)
318-
if run_state is not None:
319-
result._state = run_state
320-
result._current_turn_persisted_item_count = run_state._current_turn_persisted_item_count
321-
result._trace_state = run_state._trace_state
330+
attach_run_state_metadata(result, run_state=run_state)
322331
result._original_input = copy_input_items(original_input)
323332
return result
324333

tests/memory/test_openai_responses_compaction_session.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -448,8 +448,8 @@ async def test_run_compaction_auto_uses_default_store_when_unset(self) -> None:
448448
first_kwargs = mock_client.responses.compact.call_args_list[0].kwargs
449449
second_kwargs = mock_client.responses.compact.call_args_list[1].kwargs
450450
assert "previous_response_id" not in first_kwargs
451-
assert "previous_response_id" not in second_kwargs
452-
assert second_kwargs.get("input") == []
451+
assert second_kwargs.get("previous_response_id") == "resp-stored"
452+
assert "input" not in second_kwargs
453453

454454
@pytest.mark.asyncio
455455
async def test_run_compaction_auto_uses_input_when_last_response_unstored(self) -> None:

tests/test_agent_tracing.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,23 @@ def send_email(recipient: str) -> str:
341341
assert len(after_custom_spans) == len(before_custom_spans)
342342

343343

344+
@pytest.mark.asyncio
345+
async def test_completed_result_to_state_preserves_sensitive_trace_flag() -> None:
346+
model = FakeModel()
347+
model.add_multiple_turn_outputs([[get_text_message("done")]])
348+
agent = Agent(name="trace_agent", model=model)
349+
350+
result = await Runner.run(
351+
agent,
352+
input="first_test",
353+
run_config=RunConfig(trace_include_sensitive_data=False),
354+
)
355+
356+
state = result.to_state()
357+
358+
assert state._trace_include_sensitive_data is False
359+
360+
344361
@pytest.mark.asyncio
345362
async def test_wrapped_trace_is_single_trace():
346363
model = FakeModel()

0 commit comments

Comments
 (0)