feat(rollout): persist TurnItems for paginated thread rollouts - #30188
Conversation
4d6ed79 to
c80f9bc
Compare
ea6d2f0 to
909898e
Compare
35c6ad7 to
ee54284
Compare
909898e to
aab46a2
Compare
ee54284 to
d581108
Compare
30f1e4e to
334ca60
Compare
d581108 to
e9d6a09
Compare
334ca60 to
dbb8f8f
Compare
232e33c to
a6f98df
Compare
84244e6 to
9400737
Compare
acec956 to
f691f27
Compare
9400737 to
22e61fc
Compare
22e61fc to
5d24df7
Compare
9fd1ae2 to
9f2d003
Compare
5d24df7 to
ae5828a
Compare
d3204c7 to
3041d21
Compare
ae5828a to
f3fdff2
Compare
3041d21 to
5d4da6f
Compare
f3fdff2 to
20eca18
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b5a2836e9
ℹ️ 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".
2b0d949 to
0a7db2f
Compare
3a00fe8 to
2f22bf2
Compare
0a7db2f to
252ff86
Compare
2f22bf2 to
04d8fb3
Compare
04d8fb3 to
a8564eb
Compare
This PR depends on [#31296](#31296) for the canonical-to-legacy event mappings. ## Description This PR makes the v1 and v2 wait paths emit canonical `TurnItem::CollabAgentToolCall` lifecycle instead of `CollabWaitingBegin` / `CollabWaitingEnd` directly. Both paths already used the same legacy waiting events before this PR. The v1 item carries receiver metadata and final agent statuses for its target agents; v2 waits for mailbox activity rather than specific agents, so it keeps those fields empty, matching the existing v2 legacy payload. App-server v2 consumes the canonical item directly and ignores the mapped legacy wait events. ## Why Wait is separate from the other collab tools because it is multi-target and has distinct timeout/status behavior. Keeping it last also lets this PR remove the old helper that only existed to shape legacy wait status entries in core. ## What changed - Emit canonical collab wait items from both v1 and v2 wait handlers. - Preserve receiver metadata and agent status snapshots on completed wait items. - Remove the old core helper for building legacy wait status entries. ## Follow-up The next stack PR, [#30188](#30188), writes canonical `TurnItem` values to paginated rollout files.
e5549ae to
3932937
Compare
ec3e03c to
089106f
Compare
6e5adfb to
4beb4f0
Compare
089106f to
9aad937
Compare
wiltzius-openai
left a comment
There was a problem hiding this comment.
thread store changes look OK
we have been discussing whether we should move the metadata update back into the thread store implementations though, @miz-openai may wanna do that but this doesn't make it any easier/harder
| match event { | ||
| ResponseEvent::Created => {} | ||
| ResponseEvent::OutputItemDone(item) => { | ||
| if let Err(err) = validate_streamed_response_item_id( |
There was a problem hiding this comment.
this is a pretty brutal error to return from the agent loop
There was a problem hiding this comment.
yeah, that's fair. updated so that we generate an ID and assign it ourselves if missing
| if let Some(preview) = event_msg_preview(&ev) { | ||
| let is_user_message = match &ev { | ||
| EventMsg::UserMessage(_) => true, | ||
| EventMsg::ItemCompleted(event) => { |
There was a problem hiding this comment.
isn't EventMsg::ItemCompleted always emited? can we leave only it ?
There was a problem hiding this comment.
unfortunately not yet, it's one or the other atm (between legacy and paginated mode). will have to migrate legacy threads first before we can remove
| || matches!(event.item, TurnItem::Plan(_) | TurnItem::Sleep(_)) | ||
| } | ||
| EventMsg::TokenCount(_) | ||
| | EventMsg::ThreadGoalUpdated(_) |
There was a problem hiding this comment.
yeah... have to think about this one a bit. looks like turn_id can be none for this event
There was a problem hiding this comment.
filed a ticket to look into it
Description
This PR makes new threads with
history_mode = "paginated"persistItemCompleted(item: <turn_item>)in their rollout JSONL file.Legacy threads keep persisting the existing legacy events. Because the format is selected per thread, a rollout is either legacy or paginated; we do not need to support mixed rollouts containing both representations.
This PR depends on #31473.
Why
Paginated thread history needs stable turn/item IDs and completed item snapshots so the later SQLite projector can materialize appended rollout JSONL without rebuilding the whole thread.
Keeping the legacy persistence policy unchanged avoids changing historical rollouts or the readers that still consume them.
What changed
ItemCompletedevents and drop their redundant legacy projections; legacy threads keep the existing event set.Feature::ItemIdsis off, and reject streamed output items that arrive without server IDs.