fix(codex): compare the v7 millisecond timestamp, not the full UUID, in the fork-replay gate - #735
Conversation
…in the fork-replay gate forked_child_turn_starts_own_session ends a forked child's nested-parent replay-skip once the child's own turn starts, comparing the UUID v7 lexicographic order of the child's own turn_id against the child session_id. It compared the full 32-hex order key, so when the child session_meta and the child's first own turn_context are minted in the same millisecond the shared 48-bit timestamp ties and the result is decided by the random tail. Two independently-generated v7 UUIDs have a coin-flip tail order, so ~50% of same-millisecond forks get turn_key < child_key, the skip window never closes, and the child's own turn is dropped (a single-turn fast subagent loses all of its own usage). Compare only the first 12 hex (the 48-bit millisecond timestamp): the child's own turn is minted at or after its session_meta and the replayed parent turns strictly earlier, so the millisecond prefix is the causal signal, while the sub-millisecond tail is random. Different-millisecond ordering is unchanged. Add test_forked_child_same_millisecond_turn_starts_own_session, which fails before (0 messages instead of 1) and passes after.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![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: 80079613bc
ℹ️ 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".
| // UUIDs is a coin flip; comparing the full id would drop the child's | ||
| // own turn ~50% of the time whenever it starts within the same | ||
| // millisecond as the fork session_meta. | ||
| codex_uuid_v7_order_key(turn_id).is_none_or(|turn_key| turn_key[..12] >= child_key[..12]) |
There was a problem hiding this comment.
When a fork is spawned in the same millisecond as the parent turn's turn_id, a replayed parent turn_context can share the child's UUID v7 millisecond prefix while still sorting before the child session id. This prefix-only check then returns true for that replayed parent turn, clears forked_child_waiting_for_turn_context, and lets the following inherited token_count row be counted as the child's own usage; the same-ms case needs another discriminator instead of treating every equal-prefix turn as child-local.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — addressed in 13b1be9. The millisecond-only check did end the skip for any equal-prefix turn, so a replayed parent turn_context landing in the fork's millisecond would be miscounted as child-local.
The child's own turn is announced by a task_started event while replayed parent turns are not, so the fix tracks the task_started turn_ids seen while the child is still skipping its replayed parent history and, in the same-millisecond case, ends the skip only for a task-started turn_id (strictly-greater / strictly-less comparisons are unchanged). Added test_forked_child_same_millisecond_replayed_parent_turn_keeps_skipping, which fails before the change (2 messages instead of 1); full tokscale-core suite stays green (961) and clippy --all-targets is clean.
The previous millisecond-prefix comparison ended the replay-skip for any turn sharing the child's fork millisecond, so a replayed parent turn_context that coincidentally lands in the fork's millisecond (with no task_started) would end the skip early and count the inherited replayed rows as the child's own usage — the same-millisecond over-count flagged in review. The child's own turn is announced by a task_started event while replayed parent turns are not, so track the task_started turn_ids seen while a forked child is still skipping its replayed parent history and, in the same-millisecond case, end the skip only for a task-started turn_id. Strictly-greater and strictly-less millisecond comparisons are unchanged. Adds CodexParseState.forked_child_task_started_turn_ids (bumps the cache schema to 21 so cached incremental state reparses) and test_forked_child_same_millisecond_replayed_parent_turn_keeps_skipping, which fails before this change (2 messages instead of 1).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13b1be9d33
ℹ️ 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".
| std::cmp::Ordering::Equal => { | ||
| state.forked_child_task_started_turn_ids.contains(turn_id) |
There was a problem hiding this comment.
When a forked log has replayed parent metadata and the child's first real turn_context shares the UUID-v7 millisecond prefix with the child session_meta, this equal-prefix branch now only closes the replay skip if a task_started was observed. The parser already supports user fork logs that have forked_from_id/thread_source:"user" without any task_started, so in that same-ms case forked_child_waiting_for_turn_context never clears and the child's subsequent token_count rows are skipped, making the fork's own usage disappear.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 212f13b. The task_started-only discriminator did break the same-millisecond user-fork case: thread_source:"user" forks never emit a task_started, so the equal-prefix branch never closed the skip and the child's own token_count rows were dropped.
A user fork's replayed parent turns carry the parent session's millisecond prefix, not the child's, so they sort strictly earlier and never reach the equal-prefix branch; reaching it already means the turn shares the child's fork millisecond and is the child's own turn. The fix tracks whether the active fork is a user fork (CodexParseState.forked_child_is_user_fork, set from thread_source) and, in the equal-prefix case, ends the skip for a user fork even without a task_started. Subagent forks still require the task_started discriminator, so the same-millisecond over-count guard from the previous commit is unchanged. Bumped the cache schema to 22 for the new state field.
Added test_user_forked_child_same_millisecond_own_turn_counts_without_task_started, which fails before the change (0 messages instead of 1). Full tokscale-core suite stays green (962) and clippy --all-targets is clean.
…tarted The same-millisecond fork gate added in the previous commit ends the replay-skip only for a turn_id announced by a task_started, but human (thread_source:"user") forks never emit task_started. When a user fork's child session_meta and the child's own first turn_context are minted in the same millisecond, their UUID v7 ids share the 48-bit prefix, so the equal-prefix branch never closes the skip and the child's own token_count rows are dropped — the fork's own usage disappears. A user fork's replayed parent turns carry the parent session's millisecond prefix, not the child's, so they sort strictly earlier and never reach the equal-prefix branch; reaching it already means the turn shares the child's fork millisecond and is the child's own turn. Track whether the active fork is a user fork (CodexParseState.forked_child_is_user_fork, set from thread_source) and, in the equal-prefix case, end the skip for a user fork even without a task_started. Subagent forks still require the task_started discriminator, so the same-millisecond over-count guard is unchanged. Bumps the cache schema to 22 so cached incremental state reparses, and adds test_user_forked_child_same_millisecond_own_turn_counts_without_task_started, which fails before this change (0 messages instead of 1).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 212f13b88f
ℹ️ 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".
| state.forked_child_is_user_fork | ||
| || state.forked_child_task_started_turn_ids.contains(turn_id) |
There was a problem hiding this comment.
For thread_source:"user" forks this branch returns true for every equal UUID-v7 millisecond prefix, so when a replayed parent turn_context was minted in the same millisecond as the child session_meta, the parser exits the replay-skip window on the parent turn and counts the following inherited token_count as child-local usage. Fresh evidence beyond the earlier thread is the new forked_child_is_user_fork || shortcut here, which bypasses the task-started discriminator on exactly the equal-prefix replay case.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed, and this is the same-millisecond ambiguity resurfacing for the user-fork branch: when a replayed parent turn_context is minted in the exact fork millisecond it reaches the equal-prefix branch, and the forked_child_is_user_fork || shortcut ends the skip there.
The hard part is that in the equal-millisecond case a replayed parent turn_context and the child's own first turn_context are not orderable from the ids alone (the v7 tail is random), and a thread_source:"user" fork emits no task_started, so the turn-gate has no within-event signal to tell them apart. The downstream defense forked_child_should_skip_inherited_snapshot does skip replayed token_count rows independently of the gate, but only once a replayed row has established the inherited baseline — if the gate opens on the very first replayed turn_context, before any inherited token_count is seen, the baseline is still None and the next replayed row is counted.
So rather than add a third same-millisecond special case to the turn-gate, I think the robust resolution is the cumulative-total baseline @junhoyeo described in #679 (skip child rows at/below the parent's fork-time total), seeded at fork time rather than from the first replayed row so it holds regardless of the turn-gate. @junhoyeo — would you prefer I take that baseline-driven direction here, or keep this PR scoped to the millisecond-prefix + task_started fix and track the equal-ms user-fork replay separately? Happy to do either.
…behavior The fork-replay gate resolves a same-millisecond UUIDv7 tie for human (`thread_source:"user"`) forks on the millisecond prefix alone, since user forks never emit a `task_started` to harden the tie (subagent forks do). Document at the Equal branch that a replayed parent turn minted within the exact same 1ms fork window would end the skip one turn early — an accepted sub-millisecond, human-paced coincidence — and add a test that pins this current behavior so any future change to the tie-break is intentional. No fork-gate decision logic changed; comment + test only. Confidence: high Scope-risk: narrow
junhoyeo
left a comment
junhoyeo
left a comment
There was a problem hiding this comment.
Thank you, @Nanako0129! 🙏 Correct and important fix — comparing the UUIDv7 48-bit millisecond prefix instead of the full UUID is the right gate for the same-millisecond fork-replay case.
I pushed a documentation touch-up only (no logic change): a "Residual (accepted)" comment at the Equal arm noting that a user-fork tie minted within the exact same 1ms could end the skip one turn early — a sub-millisecond, human-paced coincidence, with subagent forks already hardened via task_started — plus a test pinning that current behavior. All fork tests green, clippy clean.
Approving and merging — thanks! 🎉
…in the fork-replay gate (junhoyeo#735) * fix(codex): compare the v7 millisecond timestamp, not the full UUID, in the fork-replay gate forked_child_turn_starts_own_session ends a forked child's nested-parent replay-skip once the child's own turn starts, comparing the UUID v7 lexicographic order of the child's own turn_id against the child session_id. It compared the full 32-hex order key, so when the child session_meta and the child's first own turn_context are minted in the same millisecond the shared 48-bit timestamp ties and the result is decided by the random tail. Two independently-generated v7 UUIDs have a coin-flip tail order, so ~50% of same-millisecond forks get turn_key < child_key, the skip window never closes, and the child's own turn is dropped (a single-turn fast subagent loses all of its own usage). Compare only the first 12 hex (the 48-bit millisecond timestamp): the child's own turn is minted at or after its session_meta and the replayed parent turns strictly earlier, so the millisecond prefix is the causal signal, while the sub-millisecond tail is random. Different-millisecond ordering is unchanged. Add test_forked_child_same_millisecond_turn_starts_own_session, which fails before (0 messages instead of 1) and passes after. * fix(codex): disambiguate same-millisecond fork turns via task_started The previous millisecond-prefix comparison ended the replay-skip for any turn sharing the child's fork millisecond, so a replayed parent turn_context that coincidentally lands in the fork's millisecond (with no task_started) would end the skip early and count the inherited replayed rows as the child's own usage — the same-millisecond over-count flagged in review. The child's own turn is announced by a task_started event while replayed parent turns are not, so track the task_started turn_ids seen while a forked child is still skipping its replayed parent history and, in the same-millisecond case, end the skip only for a task-started turn_id. Strictly-greater and strictly-less millisecond comparisons are unchanged. Adds CodexParseState.forked_child_task_started_turn_ids (bumps the cache schema to 21 so cached incremental state reparses) and test_forked_child_same_millisecond_replayed_parent_turn_keeps_skipping, which fails before this change (2 messages instead of 1). * fix(codex): count same-millisecond user-fork own turns without task_started The same-millisecond fork gate added in the previous commit ends the replay-skip only for a turn_id announced by a task_started, but human (thread_source:"user") forks never emit task_started. When a user fork's child session_meta and the child's own first turn_context are minted in the same millisecond, their UUID v7 ids share the 48-bit prefix, so the equal-prefix branch never closes the skip and the child's own token_count rows are dropped — the fork's own usage disappears. A user fork's replayed parent turns carry the parent session's millisecond prefix, not the child's, so they sort strictly earlier and never reach the equal-prefix branch; reaching it already means the turn shares the child's fork millisecond and is the child's own turn. Track whether the active fork is a user fork (CodexParseState.forked_child_is_user_fork, set from thread_source) and, in the equal-prefix case, end the skip for a user fork even without a task_started. Subagent forks still require the task_started discriminator, so the same-millisecond over-count guard is unchanged. Bumps the cache schema to 22 so cached incremental state reparses, and adds test_user_forked_child_same_millisecond_own_turn_counts_without_task_started, which fails before this change (0 messages instead of 1). * docs(codex): document accepted same-ms user-fork residual + lock its behavior The fork-replay gate resolves a same-millisecond UUIDv7 tie for human (`thread_source:"user"`) forks on the millisecond prefix alone, since user forks never emit a `task_started` to harden the tie (subagent forks do). Document at the Equal branch that a replayed parent turn minted within the exact same 1ms fork window would end the skip one turn early — an accepted sub-millisecond, human-paced coincidence — and add a test that pins this current behavior so any future change to the tie-break is intentional. No fork-gate decision logic changed; comment + test only. Confidence: high Scope-risk: narrow --------- Co-authored-by: Junho Yeo <i@junho.io>
ported from upstream junhoyeo#735 ported from upstream junhoyeo#737 ported from upstream junhoyeo#747 ported from upstream junhoyeo#750 ported from upstream junhoyeo#752 ported from upstream junhoyeo#760 ported from upstream junhoyeo#766
…in the fork-replay gate (junhoyeo#735) * fix(codex): compare the v7 millisecond timestamp, not the full UUID, in the fork-replay gate forked_child_turn_starts_own_session ends a forked child's nested-parent replay-skip once the child's own turn starts, comparing the UUID v7 lexicographic order of the child's own turn_id against the child session_id. It compared the full 32-hex order key, so when the child session_meta and the child's first own turn_context are minted in the same millisecond the shared 48-bit timestamp ties and the result is decided by the random tail. Two independently-generated v7 UUIDs have a coin-flip tail order, so ~50% of same-millisecond forks get turn_key < child_key, the skip window never closes, and the child's own turn is dropped (a single-turn fast subagent loses all of its own usage). Compare only the first 12 hex (the 48-bit millisecond timestamp): the child's own turn is minted at or after its session_meta and the replayed parent turns strictly earlier, so the millisecond prefix is the causal signal, while the sub-millisecond tail is random. Different-millisecond ordering is unchanged. Add test_forked_child_same_millisecond_turn_starts_own_session, which fails before (0 messages instead of 1) and passes after. * fix(codex): disambiguate same-millisecond fork turns via task_started The previous millisecond-prefix comparison ended the replay-skip for any turn sharing the child's fork millisecond, so a replayed parent turn_context that coincidentally lands in the fork's millisecond (with no task_started) would end the skip early and count the inherited replayed rows as the child's own usage — the same-millisecond over-count flagged in review. The child's own turn is announced by a task_started event while replayed parent turns are not, so track the task_started turn_ids seen while a forked child is still skipping its replayed parent history and, in the same-millisecond case, end the skip only for a task-started turn_id. Strictly-greater and strictly-less millisecond comparisons are unchanged. Adds CodexParseState.forked_child_task_started_turn_ids (bumps the cache schema to 21 so cached incremental state reparses) and test_forked_child_same_millisecond_replayed_parent_turn_keeps_skipping, which fails before this change (2 messages instead of 1). * fix(codex): count same-millisecond user-fork own turns without task_started The same-millisecond fork gate added in the previous commit ends the replay-skip only for a turn_id announced by a task_started, but human (thread_source:"user") forks never emit task_started. When a user fork's child session_meta and the child's own first turn_context are minted in the same millisecond, their UUID v7 ids share the 48-bit prefix, so the equal-prefix branch never closes the skip and the child's own token_count rows are dropped — the fork's own usage disappears. A user fork's replayed parent turns carry the parent session's millisecond prefix, not the child's, so they sort strictly earlier and never reach the equal-prefix branch; reaching it already means the turn shares the child's fork millisecond and is the child's own turn. Track whether the active fork is a user fork (CodexParseState.forked_child_is_user_fork, set from thread_source) and, in the equal-prefix case, end the skip for a user fork even without a task_started. Subagent forks still require the task_started discriminator, so the same-millisecond over-count guard is unchanged. Bumps the cache schema to 22 so cached incremental state reparses, and adds test_user_forked_child_same_millisecond_own_turn_counts_without_task_started, which fails before this change (0 messages instead of 1). * docs(codex): document accepted same-ms user-fork residual + lock its behavior The fork-replay gate resolves a same-millisecond UUIDv7 tie for human (`thread_source:"user"`) forks on the millisecond prefix alone, since user forks never emit a `task_started` to harden the tie (subagent forks do). Document at the Equal branch that a replayed parent turn minted within the exact same 1ms fork window would end the skip one turn early — an accepted sub-millisecond, human-paced coincidence — and add a test that pins this current behavior so any future change to the tie-break is intentional. No fork-gate decision logic changed; comment + test only. Confidence: high Scope-risk: narrow --------- Co-authored-by: Junho Yeo <i@junho.io>
2 participants
Fixes #734.
forked_child_turn_starts_own_sessionends a forked child's nested-parent replay-skip once the child's own turn starts, deciding that boundary by comparing the UUID v7 lexicographic order of the child's ownturn_idagainst the childsession_id. The comparison uses the full 32-hex order key, so when the childsession_metaand the child's first ownturn_contextare minted in the same millisecond the shared 48-bit timestamp ties and the result is decided by the version nibble + random tail. Two independently-generated v7 UUIDs have a coin-flip tail order, so roughly half of same-millisecond forks getturn_key < child_key: the skip window never closes and the child's own turn is dropped (a single-turn fast subagent loses all of its own usage).The millisecond timestamp is the causal signal here — the child's own turn is minted at or after its
session_meta, the replayed parent turns strictly earlier — while the sub-millisecond tail of two independently-minted v7 UUIDs is random. This compares only the first 12 hex (the 48-bit ms) of the order key, so a same-millisecond child turn now correctly starts the child's own session instead of being decided by a coin flip. Different-millisecond ordering is unchanged.Added
test_forked_child_same_millisecond_turn_starts_own_session, which fails before the change (the child's own 20/2 turn is dropped, 0 messages instead of 1) and passes after. Fulltokscale-coresuite stays green (960 passed) andclippy --all-targetsis clean.Summary by cubic
Fixes a fork replay gate bug that dropped or over-counted turns when a forked child's
session_metaand its first turn share the same millisecond, including user forks that never emittask_started. We now compare only the UUID v7 millisecond prefix and, on a tie, end the skip only for atask_startedturn or, for user forks, any child‑millisecond turn; documents and locks the accepted user‑fork residual (fixes #734).forked_child_turn_starts_own_session, compare only the 48-bit ms (first 12 hex); on same-ms ties require atask_startedturn for subagent forks, but accept withouttask_startedfor user forks viathread_source; trackforked_child_task_started_turn_idsandforked_child_is_user_fork, and clear them when the skip ends.task_started, and the documented residual where a replayed parent shares the child's ms and ends the skip early; bumped cache schema to 22 to reparse incremental state.Written for commit d1e712f. Summary will update on new commits.