Skip to content

feat(app-server): API proposal for better thread loading performance#20532

Draft
owenlin0 wants to merge 1 commit intomainfrom
owen/performant_threads_api_proposal
Draft

feat(app-server): API proposal for better thread loading performance#20532
owenlin0 wants to merge 1 commit intomainfrom
owen/performant_threads_api_proposal

Conversation

@owenlin0
Copy link
Copy Markdown
Collaborator

External (non-OpenAI) Pull Request Requirements

External code contributions are by invitation only. Please read the dedicated "Contributing" markdown file for details:
https://github.com/openai/codex/blob/main/docs/contributing.md

If your PR conforms to our contribution guidelines, replace this text with a detailed and high quality description of your changes.

Include a link to a bug report or enhancement request.

@owenlin0 owenlin0 force-pushed the owen/performant_threads_api_proposal branch from 0364762 to 0aec5b6 Compare April 30, 2026 23:59
@@ -225,6 +225,7 @@ use codex_app_server_protocol::Turn;
use codex_app_server_protocol::TurnError;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this diff really all that's needed for codex_message_processor to implement the new APIs? Or this is a partial implementation just of the items view?

Copy link
Copy Markdown
Collaborator Author

@owenlin0 owenlin0 May 1, 2026

Choose a reason for hiding this comment

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

no, this is just purely API stubs. I found it easier to just modify the app-server code to sketch out the APIs, but there's more work to do to make this mergeable

Copy link
Copy Markdown

@rjmarsan rjmarsan left a comment

Choose a reason for hiding this comment

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

codex-on-behalf-of-rjmarsan: Since this is still draft/API-proposal work, I am not trying to block the shape yet, but I would not have clients build against this contract from the current head.

Two concrete blockers I see:

  1. The app-server crate looks non-buildable as written. common.rs adds ClientRequest::ThreadItemsList and ClientRequest::ThreadItemContentRead, but CodexMessageProcessor::process_request has no arms for those variants. Separately, large_content was added to ThreadResumeParams, ThreadForkParams, ThreadReadParams, and ThreadTurnsListParams, while the existing destructuring patterns in codex_message_processor.rs still omit that field without ... Those are Rust exhaustiveness errors before the new API can be exercised.

  2. The checked-in generated client/schema artifacts are still on the old contract. The Rust protocol source now has largeContent, itemsView, and new item/content methods, but the PR head still has schema/typescript/v2/ThreadReadParams.ts, ThreadTurnsListParams.ts, and Turn.ts exposing the pre-change shape. Since clients consume those generated artifacts, this currently gives different answers depending on whether they read the Rust source or the generated API surface.

One related contract note: the docs describe largeContent: "deferred" returning placeholders and thread/item/content/read fetching bytes later, but I do not see an executable path consuming large_content or constructing ImageGenerationContent::Deferred yet; image generation still gets built as inline content. If this PR is meant only as a sketch, I would make that explicit. Before client work depends on it, I think we need either generated artifacts for the proposed shape or one wired end-to-end path proving the deferred behavior.

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.

3 participants