fix(moim): freeze turn-context timestamp at turn start to preserve prefix cache - #10734
Conversation
michaelneale
left a comment
michaelneale
left a comment
There was a problem hiding this comment.
I think it is right - this means the latest Moim gets the latest real time though doesnit it @Abhijay007 ?
oh yea it gets the real time captured once at turn start when the user sends their message, the only thing removed is the mid-turn refresh between tool calls, which was cache-busting with no benefit to the model |
|
Freezing the time stamp fixes the most common cause of this, but when you cross 50% of the compaction threshold, |
thanks, good catch, will add that in this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 977fac29ca
ℹ️ 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".
|
@Abhijay007 please merge after you complete #10734 (comment) |
Done in 977fac2 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2cdab76041
ℹ️ 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".
| let turn_start_compaction_info = | ||
| super::moim::compute_compaction_info(&session_config.id, &self.extension_manager) | ||
| .await; |
There was a problem hiding this comment.
In sessions where a platform extension emits changing MOIM text during the same reply loop, this snapshot still does not stabilize the cached prefix: inject_moim continues to call collect_moim_parts for every provider request, and the summon extension's MOIM includes running/idle durations, so OpenAI-compatible cache-control paths still hash different bytes in the last user message after a tool loop. Snapshot the extension MOIM alongside the timestamp/compaction data, or keep it outside the cached prefix as well.
Useful? React with 👍 / 👎.
…x cache OpenAI-compatible providers cache implicitly on the longest byte-stable request prefix. #10734 froze the turn-context block within a turn, but it still migrates to the new user message at every turn boundary, re-prefilling the entire previous turn. Strip it at format time and re-emit it at the end of the formatted request, merging into a trailing user message when one exists, mirroring the Anthropic-side fix (#10030). Covers the remaining turn-boundary half of #10706 for every provider that builds requests through formats/openai.rs.
…x cache OpenAI-compatible providers cache implicitly on the longest byte-stable request prefix. #10734 froze the turn-context block within a turn, but it still migrates to the new user message at every turn boundary, re-prefilling the entire previous turn. Strip it at format time and re-emit it at the end of the formatted request, merging into a trailing user message when one exists, mirroring the Anthropic-side fix (#10030). Covers the remaining turn-boundary half of #10706 for every provider that builds requests through formats/openai.rs.
4 participants
Fixes #10706
Summary
timestamp was regenerated on every LLM call inside a turn, busting the prefix cache for OpenAI-compatible providers. Now captured once before the agent loop and passed frozen into inject_moim.
Testing
manual