fix codex first turn model - #30
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe7c705b5a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "Codex (@codex) address that feedback".
| if let Some(turn) = model_turn_id.as_deref().and_then(|turn_id| { | ||
| scope.open_turns.iter().find(|turn| turn.turn_id == turn_id) | ||
| }) { |
There was a problem hiding this comment.
Backfill the active turn when
turn_id is absent
For rollout variants whose turn_context payload contains model but no turn_id—the format still used by the other Codex translator and replay fixtures—this expression always returns None. If such a context follows task_started, the already-inserted turn therefore retains a null model, so the change does not fix the first-turn trace for those sessions. Fall back to the sole/latest open turn when the context lacks an explicit ID.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
hmmm I think I'd rather not fill this in the ambiguous case - seems like that could get you into a weird state if there are somehow overlapping turns? From what I can see turn_id is never missing from these spans, and it looks like it should always get populated on the existing code path.
Codex writes task_started before turn_context, so the first turn span is created before its model is known. The daemon already uses the later turn_context event to backfill the session root; this change also backfills the matching turn span using its turn_id.