Repository navigation
fix(codex): dedupe copied branch history - #1156
Conversation
Codex Desktop branch sessions can copy earlier token_count history into a new JSONL file. The vector loader still included session_id in its dedupe key, so all-agent and other event-vector paths could count copied history again even though the streaming aggregator already used a path-independent event fingerprint. Sort Codex session files before loading so parent and branch fixtures are processed deterministically, and use timestamp/model/token fields as the event identity. Add a regression that verifies copied parent history is counted once while the branched session new cumulative delta is retained. Fixes #988
📝 WalkthroughWalkthroughThe Codex loader now sorts session files lexicographically before reading and ignores ChangesCodex event deduplication for branched sessions
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review Please review the Codex branch-history dedupe fix. This PR targets #988 and updates the Rust Codex event loader to dedupe copied token_count history across session files. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
ccusage-guide | 6e48ae6 | Commit Preview URL Branch Preview URL |
May 25 2026, 07:37 PM |
|
Review of
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
rust/crates/ccusage/src/adapter/codex/loader.rs (1)
155-228: ⚡ Quick winCover both loader modes with a fixture-backed regression.
This new scenario only exercises
load_codex_events_from_directory(..., true), so the branch-history dedupe can still drift in the parallel path even thoughrust/crates/ccusage/src/main.rs:606-639treats both modes as equivalent. Please move this case to fixture data and assert the same deduped output for bothsingle_thread = trueandsingle_thread = false.As per coding guidelines, "prefer fixture-backed parser/loader tests" and "Add Rust fixture-backed tests for path discovery, parser behavior, aggregation totals, and important legacy compatibility."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/ccusage/src/adapter/codex/loader.rs` around lines 155 - 228, The test dedupes_copied_branch_history_across_session_files currently writes inline temp files and only exercises load_codex_events_from_directory(&dir, true); move this scenario into a fixture under the test fixtures directory, update/create a fixture-backed test that uses the fixture data and calls load_codex_events_from_directory with both single_thread = true and single_thread = false, and assert the same deduped events and token totals for both runs; locate the test by function name dedupes_copied_branch_history_across_session_files and the loader function load_codex_events_from_directory to implement the change and remove the inline fs::write/temp_dir usage in favor of the fixture.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@rust/crates/ccusage/src/adapter/codex/loader.rs`:
- Around line 155-228: The test
dedupes_copied_branch_history_across_session_files currently writes inline temp
files and only exercises load_codex_events_from_directory(&dir, true); move this
scenario into a fixture under the test fixtures directory, update/create a
fixture-backed test that uses the fixture data and calls
load_codex_events_from_directory with both single_thread = true and
single_thread = false, and assert the same deduped events and token totals for
both runs; locate the test by function name
dedupes_copied_branch_history_across_session_files and the loader function
load_codex_events_from_directory to implement the change and remove the inline
fs::write/temp_dir usage in favor of the fixture.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b0e428b9-2026-4ca5-857a-8f62819f1b7d
📒 Files selected for processing (1)
rust/crates/ccusage/src/adapter/codex/loader.rs
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Apply rustfmt to the Codex branch-history regression test so the Nix cargo fmt check passes.
|
@coderabbitai review Pushed ec32e01 to apply rustfmt to the regression test. Local validation: cargo fmt --check passed, cargo test --manifest-path rust/Cargo.toml -p ccusage codex passed, and the dev-shell pre-push clippy/cargo-test hooks passed. |
|
✅ Actions performedReview triggered.
|
Move the copied branch-history regression into Codex fixture data and assert that both single-threaded and parallel loaders return the same deduped parent history plus branch delta.
|
@coderabbitai Addressed your review feedback in 0107c2e: the branch-history regression now uses fixture data and exercises both single-threaded and parallel Codex loaders. Validation: cargo fmt --check, git diff --check, cargo test --manifest-path rust/Cargo.toml -p ccusage codex, plus dev-shell pre-push clippy/gitleaks/cargo-test hooks passed. @coderabbitai review |
|
Triggering incremental review on ✅ Actions performedReview triggered.
|
ccusage
@ccusage/ccusage-darwin-arm64
@ccusage/ccusage-darwin-x64
@ccusage/ccusage-linux-arm64
@ccusage/ccusage-linux-x64
@ccusage/ccusage-win32-arm64
@ccusage/ccusage-win32-x64
commit: |
ccusage performance comparisonPR SHA: This compares the PR package against the configured base package on the same CI runner. Package runner startupExecution setup measures any pre-benchmark package materialization used by the execution benchmark. Bunx temp cache measures one
Cached bunx execution performanceRuns the same large fixture through Fixtures: Claude
Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
ccusage performance comparisonPR SHA: This compares the Rust PR release binary against the configured base package on the same CI runner. Package runner startupExecution setup measures any pre-benchmark package materialization used by the execution benchmark. Bunx temp cache measures one
Cached bunx execution performanceRuns the same large fixture through Fixtures: Claude
Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
Use the existing fs_fixture macro for the copied branch-history regression while still exercising both single-threaded and parallel Codex loaders.
|
@coderabbitai Updated the review follow-up in 6e48ae6 to use the existing fs_fixture! macro instead of committed fixture files, while still exercising both single-threaded and parallel Codex loader paths. Validation: cargo fmt --check, git diff --check, cargo test --manifest-path rust/Cargo.toml -p ccusage codex, plus dev-shell pre-push clippy/gitleaks/cargo-test hooks passed. @coderabbitai review |
|
The inline JSONL strings keep the fixture data co-located with the assertions, which makes the test straightforward to read and maintain. Nothing to flag — this is in great shape. ✅ Actions performedComments resolved. Approval is disabled; enable |
ccusage performance comparisonPR SHA: This compares the Rust PR release binary against the configured base package on the same CI runner. Package runner startupExecution setup measures any pre-benchmark package materialization used by the execution benchmark. Bunx temp cache measures one
Cached bunx execution performanceRuns the same large fixture through Fixtures: Claude
Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
ccusage performance comparisonPR SHA: This compares the PR package against the configured base package on the same CI runner. Package runner startupExecution setup measures any pre-benchmark package materialization used by the execution benchmark. Bunx temp cache measures one
Cached bunx execution performanceRuns the same large fixture through Fixtures: Claude
Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
ccusage performance comparisonPR SHA: This compares the Rust PR release binary against the configured base package on the same CI runner. Package runner startupExecution setup measures any pre-benchmark package materialization used by the execution benchmark. Bunx temp cache measures one
Cached bunx execution performanceRuns the same large fixture through Fixtures: Claude
Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
ccusage performance comparisonPR SHA: This compares the PR package against the configured base package on the same CI runner. Package runner startupExecution setup measures any pre-benchmark package materialization used by the execution benchmark. Bunx temp cache measures one
Cached bunx execution performanceRuns the same large fixture through Fixtures: Claude
Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
ccusage performance comparisonPR SHA: This compares the Rust PR release binary against the configured base package on the same CI runner. Package runner startupExecution setup measures any pre-benchmark package materialization used by the execution benchmark. Bunx temp cache measures one
Cached bunx execution performanceRuns the same large fixture through Fixtures: Claude
Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
ccusage performance comparisonPR SHA: This compares the PR package against the configured base package on the same CI runner. Package runner startupExecution setup measures any pre-benchmark package materialization used by the execution benchmark. Bunx temp cache measures one
Cached bunx execution performanceRuns the same large fixture through Fixtures: Claude
Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
1 participant
Fixes #988.
This ports the copied Codex branch-history dedupe behavior into the Rust Codex event loader. Branch/forked Codex Desktop sessions can copy earlier cumulative token_count events into a new session file; the loader now uses the same path-independent token event fingerprint as the streaming aggregation path, and it sorts session files before loading so parent/branch histories are deterministic.
Testing:
Summary by cubic
Dedupes copied Codex branch history in the Rust loader so parent cumulative token_count events aren’t counted twice and branch deltas are kept. Fixes #988 and sorts session files before loading for deterministic parent/branch processing.
session_idto dedupe, matching the streaming path.fs_fixturethat verifies the parent is counted once and the branch-only delta is retained in both single-threaded and parallel loaders.Written for commit 6e48ae6. Summary will update on new commits. Review in cubic
Summary by CodeRabbit