feat(lifecycle): add on_turn_start / on_turn_end hooks with TurnControl#2911
feat(lifecycle): add on_turn_start / on_turn_end hooks with TurnControl#2911adityasingh2400 wants to merge 3 commits intoopenai:mainfrom
Conversation
…ase (openai#2671) Both RunHooksBase and AgentHooksBase get two new hook methods: - on_turn_start(context, agent, turn_number): fires before each LLM call - on_turn_end(context, agent, turn_number): fires after all tool calls for the turn complete (i.e. just before the next-step decision) Turn numbers are 1-indexed and increment each time through the agent loop, regardless of handoffs. The hooks are called in both the sync and streaming code paths. Agent-level hooks on agent.hooks are also called, matching the existing on_tool_start/on_tool_end pattern. Closes openai#2671
- Remove unnecessary f-string prefixes from on_llm_start/on_llm_end (Ruff F541) - Add missing docstrings to class methods to improve docstring coverage - Add docstring to OrderTrackingHooks inner class
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05a4f15d77
ℹ️ 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".
| await asyncio.gather( | ||
| hooks.on_turn_end(context_wrapper, current_agent, current_turn), | ||
| ( | ||
| current_agent.hooks.on_turn_end( | ||
| context_wrapper, current_agent, current_turn |
There was a problem hiding this comment.
Defer turn-end hook until interrupted turn actually completes
In the sync runner, on_turn_end is fired immediately after run_single_turn returns, even when next_step is NextStepInterruption (tool approval pending). In that path the turn has not actually finished—resolve_interrupted_turn continues the same turn later—so this reports a false turn completion and the hook is never re-fired at the real end. Any hook that compacts state, records per-turn metrics, or enforces turn-level cancellation will run too early for approval-based workflows.
Useful? React with 👍 / 👎.
| await asyncio.gather( | ||
| hooks.on_turn_end(context_wrapper, current_agent, current_turn), | ||
| ( | ||
| current_agent.hooks.on_turn_end( | ||
| context_wrapper, current_agent, current_turn |
There was a problem hiding this comment.
Avoid emitting turn-end for paused streaming turns
The streaming loop has the same premature on_turn_end call before branching on turn_result.next_step. If the turn pauses with NextStepInterruption, the hook still fires even though approvals and remaining tool execution happen only after resume, so observers get an incorrect "turn complete" signal and never receive a true completion callback for that turn.
Useful? React with 👍 / 👎.
|
Adding more hooks may be worth considering, but I don't think this could be a solution for the mentioned issue. Hooks can raise an exception if something wrong but they do not affect the agent loop orchestration in a more granular way many developers wish. |
Address seratch's review feedback on openai#2911: hooks that only observe cannot affect agent loop orchestration. This commit adds a TurnControl return type ('continue' | 'stop') so on_turn_start can now halt the run before the LLM is called for that turn. Changes: - lifecycle.py: on_turn_start now returns Union[TurnControl, None] (None and 'continue' are equivalent; 'stop' halts the loop) - run.py (non-streaming path): checks return value; raises MaxTurnsExceeded with descriptive message on 'stop' - run_internal/run_loop.py (streaming path): checks return value; signals QueueCompleteSentinel on 'stop' - __init__.py: exports TurnControl, RunHooksBase, AgentHooksBase - tests: 4 new test cases covering stop-on-turn-N, stop-on-turn-1, explicit 'continue', and agent-level stop The MaxTurnsExceeded raise on 'stop' keeps behaviour consistent with the existing max_turns limit: callers can catch and inspect .run_data if needed.
05a4f15 to
f1cafcd
Compare
|
Thanks for the feedback @seratch! You're right — pure observation hooks don't solve the orchestration-control use case. I've updated this PR so that This gives callers actual control over the loop — e.g. a custom turn budget, an external kill-switch, or circuit-breaking based on |
|
Thanks for the update. However, this is still not the direction we'd like to pursue, so let us close this now. Also, all the PRs you're sending to this repo have conflicts with the latest main branch. If you're interested in sending further PRs to this project, please make sure those do not have conflicts. |
Summary
Adds turn-level lifecycle hooks to
RunHooksBaseandAgentHooksBase, addressing #2671.What's new
on_turn_start(context, agent, turn_number)— fires before the LLM is called for each turnon_turn_end(context, agent, turn_number)— fires after all tool calls for a turn completeTurnControlreturn type foron_turn_start: returning"stop"halts the run gracefully (raisesMaxTurnsExceeded) before the model is called; returningNoneor"continue"proceeds normallyAddressing reviewer feedback
on_turn_startnow returnsTurnControl("stop" | "continue" | None), giving callers direct control over the loop — not just observation. This allows patterns like:When
"stop"is returned,MaxTurnsExceededis raised — consistent with the existingmax_turnslimit, so callers handle it the same way.Files changed
src/agents/lifecycle.pyon_turn_startreturn type →Union[TurnControl, None]; docstrings updatedsrc/agents/run.pyMaxTurnsExceededon"stop"src/agents/run_internal/run_loop.pyQueueCompleteSentinelon"stop"src/agents/__init__.pyTurnControl,RunHooksBase,AgentHooksBasetests/test_turn_lifecycle_hooks.pyTurnControlcases)Closes #2671