Skip to content

feat(codex): detect turn starts so the Turn column counts codex turns - #646

Merged
junhoyeo merged 3 commits into
junhoyeo:mainfrom
crhan:contrib/codex-turn-detection
May 31, 2026
Merged

feat(codex): detect turn starts so the Turn column counts codex turns#646
junhoyeo merged 3 commits into
junhoyeo:mainfrom
crhan:contrib/codex-turn-detection

Conversation

@crhan

@crhan crhan commented May 31, 2026

Copy link
Copy Markdown
Contributor

Problem

Codex sessions never set is_turn_start, so the TUI/CLI Turn column is always 0 () for codex, while ClaudeCode and Kiro report real counts. turn_count is gated on msg.is_turn_start during daily/hourly/model aggregation, and the codex parser never flips the flag — turn detection currently exists only in claudecode.rs and kiro.rs.

Approach

Detect human turns from event_msg user_message events: set a deferred pending_turn_start on CodexParseState, then mark the next token_count-derived message (the assistant reply that carries the tokens) as a turn start. System-injected messages whose body begins with < (<environment_context>, <user_instructions>, …) are excluded as non-human input, mirroring claudecode::is_human_turn. The flag is #[serde(default)] so a pending turn survives incremental cache re-parses.

codex exec one-shots count too: they are headless but carry a real human prompt, so each is exactly one turn.

Verification

  • End-to-end against a real codex exec session: one user_messageturn_count = 1, including the agent_message that interleaves between the prompt and the token_count.
  • 4 new unit tests: human turn, system-injected (xml), exec one-shot (with interleaved agent_message), incremental-parse continuity.
  • cargo test -p tokscale-core green (42 codex tests); codex.rs is clippy-clean.

🤖 Generated with Claude Code


Summary by cubic

Detects human turn starts in Codex so the TUI/CLI Turn column and aggregations show correct counts. Matches ClaudeCode and Kiro, refines injected-message filtering to avoid dropping real markup prompts, and bumps the cache to keep incremental parsing accurate.

  • New Features

    • Defer a pending_turn_start from event_msg user_message to the next token-bearing token_count.
    • Treat only known system-injected tags as non-human: <environment_context>, <system-reminder>, <user_instructions>.
    • Support codex exec one-shots; an interleaved agent_message doesn’t break the turn.
  • Bug Fixes

    • Bumped message-cache schema to 16 to invalidate old caches so pending turns aren’t lost across incremental parse boundaries.

Written for commit 058110a. Summary will update on new commits.

Review in cubic

Codex sessions never set `is_turn_start`, so the TUI/CLI Turn column was
always 0 ("—") for codex while ClaudeCode and Kiro reported real counts
(turn_count is gated on msg.is_turn_start during daily/hourly/model
aggregation). The codex parser never flipped the flag — verified across all
branches and history; turn detection existed only in claudecode.rs and
kiro.rs.

Detect human turns from `event_msg` `user_message` events: set a deferred
`pending_turn_start` on CodexParseState, then mark the next
token_count-derived message (the assistant's reply, which carries the
tokens) as a turn start. System-injected messages whose body begins with
`<` (e.g. <environment_context>, <user_instructions>) are excluded as
non-human input, mirroring claudecode::is_human_turn. The flag is
`#[serde(default)]` so a pending turn survives incremental cache re-parses.

`codex exec` one-shots count too: headless but carrying a real human
prompt, so each is exactly one turn. Verified end-to-end against a real
`codex exec` session (1 user_message -> turn_count 1), including the
agent_message that interleaves between the prompt and the token_count.

Adds 4 unit tests: human turn, system-injected (xml), exec one-shot (with
interleaved agent_message), and incremental-parse continuity.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vercel

vercel Bot commented May 31, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
tokscale Ignored Ignored Preview May 31, 2026 7:17am

Request Review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bf287bc7e8

ℹ️ 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".

Comment thread crates/tokscale-core/src/sessions/codex.rs Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 1 file

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread crates/tokscale-core/src/sessions/codex.rs
Comment thread crates/tokscale-core/src/sessions/codex.rs Outdated
crhan added 2 commits May 31, 2026 15:06
The incremental Codex parse state gained a serde(default) pending_turn_start field, but old cache files written before it existed load the field as false. If the cache boundary fell between a human user_message and the token_count line that closes that turn, re-parsing the appended chunk would start with pending_turn_start=false and silently drop the turn boundary. Bumping CACHE_SCHEMA_VERSION discards stale caches so the first run after upgrade re-parses from scratch with the field present.
codex_message_is_human_turn rejected every message whose trimmed body starts with '<', which also drops legitimate human prompts that begin with markup (asking about a <div>, pasting an XML snippet, etc.). Match the specific known injected prefixes (<environment_context>, <system-reminder>, <user_instructions>) instead, and add a unit test covering both the markup-prompt and injected-context cases.
@crhan
crhan force-pushed the contrib/codex-turn-detection branch from acb9b50 to 058110a Compare May 31, 2026 07:17
@junhoyeo
junhoyeo merged commit d9f2a9b into junhoyeo:main May 31, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants