Skip to content

streaming: pre-assign message_id, add block_index and seq#32440

Merged
TirmanSidhu merged 1 commit into
TirmanSidhu/streaming-message-architecturefrom
run-plan/streaming-message-architecture/pr-1
May 28, 2026
Merged

streaming: pre-assign message_id, add block_index and seq#32440
TirmanSidhu merged 1 commit into
TirmanSidhu/streaming-message-architecturefrom
run-plan/streaming-message-architecture/pr-1

Conversation

@TirmanSidhu
Copy link
Copy Markdown
Contributor

Summary

  • Assigns assistant message_id on the first delta of a turn (was: at row persist).
  • Adds per-conversation seq counter, stamped on every streaming event.
  • Adds block_index to all text/tool events.
  • Emits new message_open / block_open / block_close / message_close events alongside legacy message_complete (additive — no breaking change).

Part of plan: streaming-message-architecture.md (PR 1 of 6)

@TirmanSidhu TirmanSidhu merged commit 139f743 into TirmanSidhu/streaming-message-architecture May 28, 2026
@TirmanSidhu TirmanSidhu deleted the run-plan/streaming-message-architecture/pr-1 branch May 28, 2026 18:38
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fc67ffef94

ℹ️ 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 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 address that feedback".

Comment on lines +1395 to +1398
state.currentMessageId = undefined;
state.currentBlockIndex = undefined;
state.currentBlockType = null;
state.toolUseIdToBlockIndex.clear();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep tool-use events on the persisted assistant message

For any assistant response that includes tool calls, AgentLoop.run() dispatches message_complete before it dispatches the corresponding tool_use events (see assistant/src/agent/loop.ts around the onEvent({ type: "message_complete" ... }) call followed by the for (const toolUse of toolUseBlocks) loop). Resetting currentMessageId here therefore closes the persisted assistant row before its tool blocks are streamed; the later handleToolUse allocates a new message id, so tool_use_start/tool_result are attached to the wrong message (often the follow-up text row) and their blockIndex no longer matches the persisted content. Tool-using turns will corrupt the new streaming protocol's message/block identity even though legacy events still arrive.

Useful? React with 👍 / 👎.

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.

1 participant