fix(codex): deduplicate copied branch history#989
Conversation
|
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)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds deterministic token-usage event key and array deduplicator, applies deduplication inside loadTokenUsageEvents before sorting, and adds a Vitest case that stubs CODEX_HOME to verify duplicate events from branched sessions are removed. Token Event Deduplication
Estimated code review effort:
🚥 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 unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/ccusage/src/adapter/codex/parser.ts[baseline-browser-mapping] The data in this module is over two months old. To ensure accurate Baseline data, please update: Oops! Something went wrong! :( ESLint: 9.35.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'eslint-plugin-format' imported from /node_modules/.pnpm/@antfu+eslint-config@4.19.0_@vue+compiler-sfc@3.5.30_eslint@9.35.0_typescript@5.9.2_vit_670a2c5c75d4275eabd7bc195a173ee6/node_modules/@antfu/eslint-config/dist/index.js 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 |
|
Thanks a lot for the incredibly quick turnaround on this. From reading the PR, the fix looks aligned with the issue I reported in #988: Codex Desktop branched conversations should not cause historical usage to be counted again. From the reporter side, this looks like the right direction. Happy to see this merged once CI is green and the maintainers are comfortable with it. |
c5a2e1f to
54abc64
Compare
|
Maintainer check after rebasing this onto current main:
Validation run locally:
Decision: merge once bot checks finish. This one is directly reproduced by local real data and matches #988. |
@ccusage/amp
ccusage
@ccusage/codex
@ccusage/opencode
@ccusage/pi
commit: |
54abc64 to
a263355
Compare
|
Follow-up: replaced the initial JSON.stringify-based fingerprint with a fixed-separator string key. Local Codex data timing for loadTokenUsageEvents(), 5 runs each:
So the previous CI large-fixture slowdown was likely from JSON.stringify allocation/serialization overhead. The branch has been force-pushed and CI is running again. |
Deduplicate Codex token usage events with a session-independent fingerprint so branched or repeated session files do not count copied history more than once. Add regression coverage for copied branch history and validate against local Codex logs, where the current parser produced thousands of duplicate token events.
a263355 to
6a8c825
Compare
|
Updated after merging #1013 into main and rebasing this PR. Head SHA: 6a8c825 Local validation after rebase:
The perf workflow should now always write results to the Actions job summary and attempt the PR comment best-effort. |
|
thank you for your contribution! |
Fixes duplicated Codex Desktop usage when a branch/forked conversation copies historical JSONL token events into another session file.
What changed:
@ccusage/codexso copied historicaltoken_countevents are counted once across session files.Verification:
npx -y pnpm@10.30.1 --filter @ccusage/codex test src/data-loader.tsnpx -y pnpm@10.30.1 --filter @ccusage/codex typechecknpx -y pnpm@10.30.1 run lint --fixfromapps/codexgit diff --checkCloses #988.
Summary by CodeRabbit
Bug Fixes
Tests