Skip to content

Commit d94ad2d

Browse files
committed
fix review comments
1 parent 9e0f7af commit d94ad2d

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
@@ -186,17 +186,58 @@ def _resolve_compaction_mode(
186186

187187
return resolved_mode
188188

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

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
@@ -551,8 +551,8 @@ async def test_run_compaction_auto_uses_default_store_when_unset(self) -> None:
551551
first_kwargs = mock_client.responses.compact.call_args_list[0].kwargs
552552
second_kwargs = mock_client.responses.compact.call_args_list[1].kwargs
553553
assert "previous_response_id" not in first_kwargs
554-
assert "previous_response_id" not in second_kwargs
555-
assert second_kwargs.get("input") == []
554+
assert second_kwargs.get("previous_response_id") == "resp-stored"
555+
assert "input" not in second_kwargs
556556

557557
@pytest.mark.asyncio
558558
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)