Repository navigation
fix(codex): dedupe goal rollout token events - #1237
Conversation
Align the streaming Codex group aggregation dedupe key with the event loader so copied token_count rows from goal or rollout session files are counted once instead of once per session file. This prevents daily and monthly table reports from double-counting copied Codex history while preserving the same token tuple based duplicate detection already used by JSON/all-agent loading paths.
|
@coderabbitai review\n\nPlease review the latest push. This fixes Codex goal/rollout token_count dedupe in the streaming aggregation path. |
|
@cubic-dev-ai review\n\nPlease review the latest push as well. |
@ryoppippi I have started the AI code review. It will take a few minutes to complete. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
ccusage-guide | c7ffd05 | Commit Preview URL Branch Preview URL |
Jun 08 2026, 10:28 PM |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Fetch PR details and diff
gh pr view 1237 --repo ryoppippi/ccusage --json title,body,files,commits,headRefName 2>/dev/null | jq .
echo "=== PR DIFF ==="
gh pr diff 1237 --repo ryoppippi/ccusage 2>/dev/nullLength of output: 6051
Review of
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughMake Codex deduplication key kind-aware by passing ChangesCodex Event Deduplication by Report Kind
Sequence DiagramsequenceDiagram
participant SessionFile
participant Aggregator
participant insert_event_key
participant codex_event_key
SessionFile->>Aggregator: read token-count event
Aggregator->>insert_event_key: insert_event_key(event, kind)
insert_event_key->>codex_event_key: codex_event_key(event, kind)
codex_event_key->>insert_event_key: CodexEventKey(kind-aware)
insert_event_key->>Aggregator: returns whether key is new (dedupe decision)
Aggregator->>Aggregator: group/sum tokens based on date + CodexEventKey
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 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 |
ryoppippi
changed the title
![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/aggregate.rs (1)
477-493: ⚡ Quick winCover both aggregation threading paths in this regression test.
Line 477 currently validates dedupe in a single runtime mode. Looping over
single_threadin this test would lock the contract for both local and parallel aggregation paths.♻️ Suggested test update
- let shared = SharedArgs { - timezone: Some("UTC".to_string()), - ..SharedArgs::default() - }; - - let groups = - load_groups_from_directory(&fixture.path("sessions"), &shared, AgentReportKind::Daily) - .unwrap(); - - assert_eq!(groups.len(), 1); - let group = groups.get("2026-05-29").unwrap(); - assert_eq!(group.input_tokens, 1_000); - assert_eq!(group.cached_input_tokens, 100); - assert_eq!(group.output_tokens, 200); - assert_eq!(group.reasoning_output_tokens, 20); - assert_eq!(group.total_tokens, 1_200); + for single_thread in [true, false] { + let shared = SharedArgs { + single_thread, + timezone: Some("UTC".to_string()), + ..SharedArgs::default() + }; + + let groups = load_groups_from_directory( + &fixture.path("sessions"), + &shared, + AgentReportKind::Daily, + ) + .unwrap(); + + assert_eq!(groups.len(), 1); + let group = groups.get("2026-05-29").unwrap(); + assert_eq!(group.input_tokens, 1_000); + assert_eq!(group.cached_input_tokens, 100); + assert_eq!(group.output_tokens, 200); + assert_eq!(group.reasoning_output_tokens, 20); + assert_eq!(group.total_tokens, 1_200); + }As per coding guidelines, "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/aggregate.rs` around lines 477 - 493, The test currently only exercises one aggregation threading mode; update it to loop over both local and parallel aggregation paths by iterating over SharedArgs { single_thread: true/false, timezone: Some("UTC".into()), ..SharedArgs::default() } (or setting single_thread on the existing `shared`), and for each iteration call `load_groups_from_directory(&fixture.path("sessions"), &shared, AgentReportKind::Daily)` and run the same assertions on the resulting `groups` and `group` (checks for `input_tokens`, `cached_input_tokens`, `output_tokens`, `reasoning_output_tokens`, and `total_tokens`) so both aggregation threads are validated.Source: Coding guidelines
🤖 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/aggregate.rs`:
- Around line 477-493: The test currently only exercises one aggregation
threading mode; update it to loop over both local and parallel aggregation paths
by iterating over SharedArgs { single_thread: true/false, timezone:
Some("UTC".into()), ..SharedArgs::default() } (or setting single_thread on the
existing `shared`), and for each iteration call
`load_groups_from_directory(&fixture.path("sessions"), &shared,
AgentReportKind::Daily)` and run the same assertions on the resulting `groups`
and `group` (checks for `input_tokens`, `cached_input_tokens`, `output_tokens`,
`reasoning_output_tokens`, and `total_tokens`) so both aggregation threads are
validated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 66fe19dd-9d96-4609-964e-72d5ba2dbfc6
📒 Files selected for processing (2)
rust/crates/ccusage/src/adapter/codex/aggregate.rsrust/crates/ccusage/src/adapter/codex/mod.rs
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — removes session_id from the streaming aggregation dedupe key so identical token_count rows copied across root and goal Codex session files are counted once instead of once per file.
-
Align
codex_event_keywith loader dedupe shape — the streamingCodexEventKeytuple inaggregate.rsnow starts with(timestamp, hash_text(model), ...)instead of(hash_text(&event.session_id), event.session_id.len(), timestamp, ...). The loader inloader.rsalready deduped on(timestamp, model, input_tokens, ...)withoutsession_id; the streaming path was the outlier. -
Update existing test and add root/goal dedupe test —
keeps_matching_grouped_codex_usage_events_from_distinct_sessionsinmod.rsis renamed todedupes_...and its assertions updated from 2x to 1x totals. New testdedupes_copied_token_usage_across_session_filesinaggregate.rsplaces the same event inroot.jsonlandgoal.jsonland asserts deduped single-event totals.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
Extend the copied rollout token regression test across both local and parallel Codex aggregation paths. This addresses CodeRabbit feedback and keeps the dedupe contract covered for each SharedArgs single_thread mode.
|
@coderabbitai Addressed your test coverage feedback in d319394 by exercising the copied rollout token dedupe regression across both single-threaded and parallel aggregation paths. Validation: direnv exec . cargo test -p ccusage adapter::codex -- --nocapture; pre-push clippy/treefmt/gitleaks/cargo test passed. Please review the latest push. |
|
@cubic-dev-ai Please review the latest push d319394 as well. |
@ryoppippi I have started the AI code review. It will take a few minutes to complete. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Fetch the latest commit details
gh pr view 1237 --repo ryoppippi/ccusage --json commits | jq '.commits[-3:]'
echo "=== LATEST COMMIT DIFF ==="
gh api repos/ryoppippi/ccusage/commits/d3193940 --jq '.files[] | "--- \(.filename) ---\n\(.patch)"'Length of output: 3513
|
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 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. |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — wraps the new rollout dedupe test in for single_thread in [true, false] so both sequential and parallel aggregation paths are validated.
- Cover both threading modes in rollout dedupe test —
dedupes_copied_token_usage_across_session_filesnow iterates oversingle_thread: trueandsingle_thread: false, passingsingle_threadintoSharedArgsso the dedupe contract is validated for both the local and parallel aggregation paths.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
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. |
Keep Codex rollout token dedupe across files for daily, weekly, and monthly reports while restoring session-scoped dedupe keys for session reports. The previous dedupe key intentionally removed session_id to collapse copied root/goal rollout token_count rows, but that same global key ran before session grouping and could drop a distinct session bucket when two session files had identical token usage rows. Session reports now include the session hash in the key, while aggregate period reports use a zero session component to retain cross-session dedupe. Adds a regression test that identical token usage rows in two session files remain visible as two session groups across both single-threaded and parallel aggregation.
|
@cubic-dev-ai Please re-review c7ffd05. I addressed your session-report undercount finding by preserving session_id only for AgentReportKind::Session dedupe keys and added coverage for identical token rows in distinct session groups across both aggregation threading modes. |
@ryoppippi I have started the AI code review. It will take a few minutes to complete. |
|
@coderabbitai review Pushed c7ffd05 to address the session-report undercount finding: session reports now keep session-scoped dedupe keys, while daily/weekly/monthly reports still dedupe copied rollout token events across session files. Validation passed locally: direnv exec . cargo test --manifest-path rust/Cargo.toml -p ccusage adapter::codex -- --nocapture; direnv exec . pnpm run format; direnv exec . pnpm run test; and pre-push clippy/treefmt/gitleaks/cargo test. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Fetch the latest commit details
gh pr view 1237 --repo ryoppippi/ccusage --json commits | jq '.commits[-3:]'
echo "=== LATEST COMMIT DIFF ==="
gh api repos/ryoppippi/ccusage/commits/c7ffd05 --jq '.files[] | "--- \(.filename) ---\n\(.patch)"'Length of output: 6865
|
| Report kind | session_hash / session_len in key |
Effect |
|---|---|---|
Session |
real hash_text(session_id) / session_id.len() |
Distinct session buckets are preserved even with identical token rows |
Daily / Weekly / Monthly |
0 / 0 (sentinel) |
Cross-file dedup from the previous fix is retained |
No collision risk from the zero-sentinel: a real session always has len > 0, the sentinel always has len == 0. ✅
Both aggregation paths updated consistently:
- Parallel path:
insert_event_key(event, timestamp, model, kind, seen)✅ - Local path:
codex_event_key(event, timestamp, model, kind)✅
New regression test keeps_matching_token_usage_in_distinct_session_groups:
- Two session files with the same token-usage line, loaded with
AgentReportKind::Session - Asserts
groups.len() == 2and each group has the correctinput_tokens == 1_000(not collapsed) - Covers both
single_thread = trueandfalse✅
The fix is minimal, the sentinel approach is elegant, and the test suite now covers both the cross-session-dedup path (non-session reports) and the session-preservation path (session reports). No blockers — ready to merge.
(˶˃ᵕ˂˶) ✨
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the new commit preserves session-report granularity by adding session_id back to the dedupe key for AgentReportKind::Session, while Daily/Weekly/Monthly reports continue deduplicating identical token events across session files.
- Conditional session-scoped dedupe key —
codex_event_key()now takesAgentReportKindand prepends(session_hash, session_len)only forSessionkind; non-session kinds use(0, 0)sentinel values.insert_event_key(),add_event_to_groups(), andadd_event_to_groups_local()threadkindthrough to preserve both behaviors. - New session-granularity test —
keeps_matching_token_usage_in_distinct_session_groupsplaces identical events inroot.jsonlandgoal.jsonlwithAgentReportKind::Session, asserts 2 distinct groups each with full token totals, and loops over both threading modes.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
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
Deduplicates copied Codex token_count rows when rollout or goal session files contain the same usage event as another session file.
The streaming group aggregation path now uses the same token tuple based dedupe shape as the event loader, so table reports do not count copied session history once per rollout file.
Testing:
Summary by cubic
Fixes double-counting of Codex token usage when rollout or goal session files copy the same events. The streaming aggregator now uses the event loader’s token-tuple key and scopes it by report kind, so daily/weekly/monthly dedupe across files while session reports keep distinct groups.
session_idfor daily/weekly/monthly to dedupe across session files, but include it for session reports to avoid collapsing separate sessions. Tests cover both single-thread and parallel paths.Written for commit c7ffd05. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests