feat: add stepwise executor contract#46
Conversation
sarvesh1327
commented
Apr 24, 2026
- Adds per-turn executor request/result models for upcoming multi-turn rollouts.
- Adds run_executor_step(...) to call step-native executors or adapt legacy executors as terminal one-step runs.
- Covers both step-capable and legacy fallback paths with RED/GREEN tests.
- Verified with targeted orchestration/integration tests, ruff, and full Advisor suite: 209 passed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 173fc77dec
ℹ️ 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".
| execute_step = getattr(executor, "execute_step", None) | ||
| if callable(execute_step): | ||
| result = execute_step(request) | ||
| return result if isinstance(result, ExecutorStepResult) else ExecutorStepResult(**dict(result)) |
There was a problem hiding this comment.
Handle legacy-shaped results from
execute_step
If a step-capable executor implements execute_step() by reusing existing logic and returns an ExecutorRunResult, this branch coerces it with ExecutorStepResult(**dict(result)) instead of using the legacy adapter. Because ExecutorStepResult defaults done=False and ignores extra fields like retries/metadata, a completed one-step run can be treated as non-terminal and continue looping until budget exhaustion. Please detect ExecutorRunResult here and route it through _executor_run_result_to_step_result(...) so terminal semantics are preserved.
Useful? React with 👍 / 👎.