Defer initial context insertion until the first turn#14313
Conversation
Move new-session startup off the eager baseline write path so the first real turn owns initial context insertion. Update regression tests and snapshots around permissions, collaboration instructions, personality messages, and compaction layouts. Co-authored-by: Codex <noreply@openai.com>
|
@codex review this |
![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: 1170b52fb9
ℹ️ 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".
| // starts so it reflects the actual first-turn settings (permissions, etc.) and | ||
| // we do not emit model-visible "diff" updates before the first user message. | ||
| let items = self.build_initial_context(&turn_context).await; | ||
| self.record_conversation_items(&turn_context, &items).await; |
There was a problem hiding this comment.
Deleting the startup-time build_initial_context(...) block is safe because we are not removing the “inject full initial context on the first turn” behavior; we are only moving it off the eager startup path and onto the existing per-turn path.
On every real turn, run_turn calls record_context_updates_and_set_reference_context_item(...) before the user prompt is recorded. That helper explicitly checks whether reference_context_item is missing and, if so, calls build_initial_context(turn_context) instead of diffing.
So the invariant remains:
- fresh thread:
reference_context_item == None - first real turn: inject full initial context via
build_initial_context(...) - later turns: diff against the stored baseline
- after compaction clears the baseline: the same
Nonepath reinjects full initial context
In other words, we are not relying on diffs for the first turn. We are still relying on build_initial_context(...) for the first turn; we are just no longer doing that work too early at startup, which is what caused the model-visible “initial context, then first-turn diff before the first user message” artifact.
Treat local manual /compact like the remote path when the thread has no history. This avoids issuing a useless compaction request before the first user turn and updates the affected snapshot. Co-authored-by: Codex <noreply@openai.com>
|
@codex review this |
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Seed the compaction tests with a real first turn so they still exercise the actual compact path after manual /compact on an empty thread became a no-op. Also make the core token-count assertion robust to the stale prior-turn token event that can arrive ahead of the compact-specific counts. Co-authored-by: Codex <noreply@openai.com>
Keep the model visible layout snapshot test focused on the rendered request shape and leave behavior-specific counting assertions to non-snapshot coverage. Co-authored-by: Codex <noreply@openai.com>
Remove the local manual /compact empty-thread guard and revert the test reshaping that only existed to support it. Keep the current local snapshot updated to reflect the remaining first-turn compact request shape under deferred initial context. Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
2 participants
Summary
build_initial_context()until the first real turn instead of seeding model-visible context during startupreference_context_item == Noneturn-start path to inject full initial context on that first real turn (and again after baseline resets such as compaction)InitialHistory::Newand update affected deterministic tests / snapshots around developer-message layout, collaboration instructions, personality updates, and compact request shapesNotes
/compactbehavior/compactwith no prior user still skips the remote compact request; local first-turn/compactstill issues a compact request, but that request now reflects the lack of startup-seeded context