Skip to content

fix(tracing): keep trace state on completed non-streamed runs - #5045

Open
BlueX888 wants to merge 1 commit into
openai:mainfrom
BlueX888:fix/prep-runstate-drops-trace-on-nonstreamed-completion
Open

BlueX888 wants to merge 1 commit into
openai:mainfrom
BlueX888:fix/prep-runstate-drops-trace-on-nonstreamed-completion

Conversation

@BlueX888

Copy link
Copy Markdown

Summary

A non-streamed run that finishes normally produces a RunResult whose to_state() carries no trace. Resuming from that state with Runner.run(agent, state) opens a new trace instead of reattaching to the original one, so a single logical run ends up split across two traces.

Root cause: AgentRunner._run_impl (src/agents/run.py:584) has several RunResult construction sites, and the completion path at src/agents/run.py:2067-2070 was the only one that did not assign result._trace_state = run_state._trace_state. RunResult has no trace attribute, so the fallback in result_to_run_state (src/agents/result.py:166-169) resolves to None and to_state() writes a state without trace information. On resume, create_trace_for_run(..., trace_state=run_state._trace_state, reattach_resumed_trace=is_resumed_state) (src/agents/run.py:2508) then has nothing to reattach to and starts a fresh trace. The other construction sites (src/agents/run.py:1426, src/agents/run.py:1607, src/agents/run.py:2590) already copy it.

This is the remaining case of #2540, whose fix (#2547) covered the interrupted-run path but not the completed-run path. The symptom is not limited to human-in-the-loop: Runner.run(agent, input=...) followed by Runner.run(agent, result.to_state()) also starts two traces on the current tree.

Changes

  • src/agents/run.py:2070 — carry run_state._trace_state onto the result in the completion path, matching the other RunResult construction sites in the module.
  • tests/test_agent_tracing.py — add test_resumed_run_after_completed_turn_reuses_original_trace.

Test plan

The new test runs an agent with a needs_approval tool, approves the interruption, lets the run finish, then resumes from approved.to_state(). It asserts exactly one trace and one trace_start/trace_end pair.

With the fix reverted (test present, src/agents/run.py at the base revision) it fails:

    traces = fetch_traces()
>       assert len(traces) == 1
E       assert 2 == 1
E        +  where 2 = len([<agents.tracing.traces.TraceImpl object at 0x10c53ff50>, <agents.tracing.traces.TraceImpl object at 0x10c53fb60>])

tests/test_agent_tracing.py:450: AssertionError
=========================== short test summary info ============================
FAILED tests/test_agent_tracing.py::test_resumed_run_after_completed_turn_reuses_original_trace
============================== 1 failed in 0.28s ==============================

With the fix applied:

  • uv run pytest tests/test_agent_tracing.py::test_resumed_run_after_completed_turn_reuses_original_trace -> 1 passed in 0.20s
  • uv run pytest tests/test_agent_tracing.py -> 34 passed in 0.52s
  • uv run pytest tests/test_agent_tracing.py tests/test_tracing.py tests/test_tracing_errors.py tests/test_tracing_errors_streamed.py tests/test_run.py tests/test_run_state.py tests/test_run_impl_resume_paths.py tests/test_responses_tracing.py -> 642 passed in 3.24s
  • bash .agents/skills/code-change-verification/scripts/run.sh -> code-change-verification: all commands passed. (make format, make lint, make typecheck; 9637 passed, 29 skipped in 47.92s parallel, 77 passed, 4 skipped, 55 deselected in 1.70s serial)

Issue number

N/A — no issue was filed for this. Related: #2540, fixed by #2547.

Checks

  • I've added new tests, if relevant
  • I've run .agents/skills/code-change-verification/scripts/run.sh
  • I've confirmed all verification steps pass
  • If using Codex, I've run /review before submitting this PR

The completion path in AgentRunner._run_impl built its RunResult without
copying run_state._trace_state, so RunResult.to_state() produced a state with
no trace. Resuming from that state opened a new trace instead of reattaching
to the original one, splitting a single logical run across two traces. The
other RunResult construction sites in this module already copy it.

@karaaslanz karaaslanz 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.

There looks to be one remaining gap in the non-HITL path described in the PR body.

The new assignment is still inside if run_state is not None, so it only copies trace state when this completion itself came from a resumed RunState. For a fresh Runner.run(agent, input=...), run_state is None, this line is skipped, and I don't see another completion/finalization assignment that puts the current trace onto the resulting RunResult. result_to_run_state() then falls back from a missing _trace_state to getattr(result, "trace", None), but RunResult does not expose that trace surface.

The regression test currently starts with an approval interruption, so approved is already a resumed run and exercises exactly the branch this patch fixes. It does not cover the separate scenario called out in the description: Runner.run(agent, input=...) -> result.to_state() -> Runner.run(agent, state).

Could we add a direct fresh-completion regression for that path and either preserve the active trace state for fresh results as well, or narrow the stated behavior if continuing a fully completed fresh result is intentionally unsupported? That would keep the implementation/test aligned with the broader claim in the PR.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants