Repository navigation
Add feature-gated session segmentation - #27249
friel-openai wants to merge 1 commit into
Conversation
6d63707 to
d6de718
Compare
d6de718 to
4f22825
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f22825f9f
ℹ️ 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".
47046e0 to
b1573f3
Compare
b1573f3 to
349965b
Compare
e8a1aea to
d1bf0db
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1bf0db5f6
ℹ️ 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".
jif-oai
left a comment
There was a problem hiding this comment.
I don’t think we should land this as one PR. The goal—bounding live rollout files—is valid, but this introduces a graph of mutable file references before defining the core contracts: immutable snapshot identity, ownership/lifetime, deletion and GC, atomic rotation, failure behavior, and the distinction between bounded model replay and complete transcript history.
This would make the contract explicit and easier to reason about the PRs while reviewing.
My comments are still valid though and some probably need a refactor
ffbd02a to
0b4829e
Compare
0b4829e to
ad31e8b
Compare
ad31e8b to
848575d
Compare
|
Closing this pull request because it has had no updates for more than 14 days. If you plan to continue working on it, feel free to reopen or open a new PR. |
Summary
session_segmentationfeature.This is the writer follow-up to #27968. That prerequisite owns the unchanged
RolloutReferenceItemwire format and all reference-aware readers. This PR adds no database migration and does not change the wire format.Write contracts
nth_user_message: Noneand consumes the bounded model-replay segment budget. A fork snapshot usesnth_user_message: Some(usize::MAX)and does not consume that budget.segment_id. A legacy rollout without one is copied to a dedicated immutableinitialpath before publication, so its live canonical path cannot later resolve as the captured predecessor.If a failure occurs before atomic replacement, the old canonical file and writer remain usable. If reopening the successor fails after replacement, rotation atomically restores the predecessor. A retry also removes an immutable collision left by a process crash only after confirming that no canonical rollout references it.
Test plan
cargo test -p codex-thread-store(88 passed)cargo test -p codex-rollout(85 passed)cargo test -p codex-features(51 passed)codex-corecompaction, alternate context-window, full-history subagent, inactive mid-turn fork, and repeated explicit-fork tests passed withRUST_MIN_STACK=33554432bazel --batch test //codex-rs/thread-store:thread-store-unit-testsjust fixforcodex-thread-store,codex-rollout,codex-features, andcodex-corejust write-config-schemabazel --batch mod deps --lockfile_mode=errorjust fmtgit diff --checkThe full
codex-core --librun passed 1,914 tests. Five unrelated tests failed because this sandbox exposes/tmpas a synthetic Git root; every changed core path passed independently.