feat(app-server): API proposal for better thread loading performance#20532
feat(app-server): API proposal for better thread loading performance#20532
Conversation
0364762 to
0aec5b6
Compare
| @@ -225,6 +225,7 @@ use codex_app_server_protocol::Turn; | |||
| use codex_app_server_protocol::TurnError; | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
rjmarsan
left a comment
There was a problem hiding this comment.
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:
-
The app-server crate looks non-buildable as written.
common.rsaddsClientRequest::ThreadItemsListandClientRequest::ThreadItemContentRead, butCodexMessageProcessor::process_requesthas no arms for those variants. Separately,large_contentwas added toThreadResumeParams,ThreadForkParams,ThreadReadParams, andThreadTurnsListParams, while the existing destructuring patterns incodex_message_processor.rsstill omit that field without... Those are Rust exhaustiveness errors before the new API can be exercised. -
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 hasschema/typescript/v2/ThreadReadParams.ts,ThreadTurnsListParams.ts, andTurn.tsexposing 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.
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.