Fix token cost estimation for non-Anthropic models in Claude logs (#2393) - #2413
Fix token cost estimation for non-Anthropic models in Claude logs (#2393)#2413Yuxin-Qiao wants to merge 2 commits into
Conversation
![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: 40c26b9f2d
ℹ️ 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".
| inputTokens: totalPromptTokens, | ||
| cachedInputTokens: cacheReadInputTokens, | ||
| outputTokens: outputTokens, | ||
| cacheWriteInputTokens: 0, |
There was a problem hiding this comment.
When a Claude Code log row for a GPT/OpenAI model includes nonzero cache_creation_input_tokens (for example gpt-5.6, which has cache-write rates in the bundled Codex table), this fallback folds those tokens into totalPromptTokens but then passes cacheWriteInputTokens: 0. codexCostUSD therefore bills those cache writes as ordinary uncached input instead of the cache-write rate, so cost totals are wrong for those proxy logs. Pass cacheCreationInputTokens through as cacheWriteInputTokens rather than dropping it.
Useful? React with 👍 / 👎.
|
|
||
| publish-release-assets: | ||
| runs-on: ubuntu-24.04 | ||
| needs: build-cli |
There was a problem hiding this comment.
In release-cli.yml release runs, publish-release-assets and update-homebrew-tap both only depend on build-cli, so after the matrix finishes they can run in parallel. Since the tap workflow is dispatched with an artifact_template that points at GitHub release assets, it can start resolving or publishing the formula before this new publisher job has completed gh release upload, causing the Homebrew update to fail or reference assets that are not available yet. Make the tap job depend on publish-release-assets (or keep uploading before dispatch).
Useful? React with 👍 / 👎.
40c26b9 to
6d22c2e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d22c2ea7f
ℹ️ 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".
| if let lookup = self.modelsDevLookupAnyProvider( | ||
| model: key, | ||
| catalog: modelsDevCatalog, | ||
| cacheRoot: modelsDevCacheRoot) |
There was a problem hiding this comment.
When this new any-provider fallback prices a Claude Pi-compatible row via models.dev for a Google or DeepSeek model, PiSessionCostScanner.pricingContext still fingerprints only anthropic and openai, while buildReport reuses complete cached costNanos without recomputing. After a models.dev price change for one of the providers added here, the Pi cache remains valid and continues showing stale costs for those rows; include every provider this fallback can read in the pricing key before caching those computed costs.
Useful? React with 👍 / 👎.
6d22c2e to
3fce373
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3fce373039
ℹ️ 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".
| L("≈%d full 5h window of weekly left"), | ||
| 1) | ||
| } else { | ||
| String.localizedStringWithFormat( | ||
| L("≈%d full 5h windows of weekly left"), |
There was a problem hiding this comment.
By splitting the existing .stringsdict lookup into new standalone L(...) keys, these strings no longer match the localized plural entries, which are still keyed by the original combined sentence (and the verdict key has the same issue below). In any non-English locale this falls back to the English key text for the session-equivalent details instead of using the translated resources; keep formatting through the existing stringsdict key or add localized entries for every new key.
Useful? React with 👍 / 👎.
| if let lookup = self.modelsDevLookupAnyProvider( | ||
| model: model, | ||
| catalog: modelsDevCatalog, | ||
| cacheRoot: modelsDevCacheRoot) |
There was a problem hiding this comment.
This expands Codex-session pricing to models.dev providers beyond OpenAI, but the regular Codex daily cache is still keyed through CostUsageScanner.codexPricingKey, which fingerprints only OpenAI pricing. For a Codex history row priced via this fallback, such as a deepseek-r1 model, a later DeepSeek/Google/Anthropic models.dev price change will leave the cached costNanos valid and reports will continue showing the stale cost until users force-rescan or clear the cache; include all providers reachable from this fallback in the Codex pricing key or bump the cache formula.
Useful? React with 👍 / 👎.
|
I built the latest version of the PR and the calculations might be off. GPT 5.6 sol in Codex GPT 5.6 sol in claude cli The estimated token cost is way higher in Claude cli. It should also really be counted towards Codex usage, no Claude |
|
I tested with CLIProxyAPI correctly separates uncached input and cache-read tokens. However, Claude CLI writes multiple transcript rows for a single response—typically CodexBar currently deduplicates only when both IDs exist. Without In my sample:
The final row often reclassifies interim uncached input as cache reads, so summing the rows inflates cost significantly. Suggested fix: when Relevant code:
A regression test with interim and final rows sharing a |
clawsweeper
Bot
added
proof: 📸 screenshot
|
Codex review: needs real behavior proof before merge. Reviewed July 30, 2026, 11:32 AM ET / 15:32 UTC. ClawSweeper reviewWhat this changesThe branch deduplicates Claude log rows that have only a message ID or request ID, and separately adds retry logic around Swift toolchain installation in CI. Merge readiness⛔ Blocked until real behavior proof from a real setup is added - 10 items remain Keep this PR open: the new single-ID deduplication can correct file-local Claude log rows, but incremental refreshes still use different identity rules and can retain both cumulative snapshots, inflating token and cost totals. The branch also lacks after-fix real behavior proof; the available user report instead describes an incorrect built-PR result. Priority: P2 Review scores
Verification
How this fits togetherCodexBar scans Claude Code JSONL transcripts, turns assistant usage rows into cached usage records, and then aggregates those records into menu-bar token and cost estimates. Row identity must stay consistent between an initial file scan, incremental cache merge, and cache invalidation so the displayed cost remains accurate. flowchart LR
A[Claude Code JSONL logs] --> B[Claude usage scanner]
B --> C[Row identity and deduplication]
C --> D[Incremental usage cache]
D --> E[Cost aggregation]
E --> F[CodexBar menu estimates]
G[CI Swift installation] --> H[Build workflow]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk options
Maintainer options:
Technical reviewBest possible solution: Use one validated, nonempty row-identity helper for both parsing and incremental merging, rotate or migrate affected cached Claude/Vertex rows, add focused regressions for those upgrade paths, and provide redacted after-fix output from a real proxy-backed Claude log before merge. Do we have a high-confidence way to reproduce the issue? Yes, at source level: scan a Claude JSONL file with cumulative rows sharing only a nonempty message ID or request ID, cache it, append another cumulative row, then perform an incremental refresh. The initial scan and merge currently take different identity paths, giving a focused regression route. Is this the best way to solve the issue? No. The in-file fallback alone is not the narrowest correct fix because the incremental merge and existing cache state do not use the same identity contract; a shared helper plus cache compatibility coverage is safer. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 8ef86077e70a. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
9d93fe0 to
90bf63f
Compare
…eipete#2393) and add swiftly retry
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90bf63f306
ℹ️ 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".
| if let messageId, let requestId { | ||
| return "\(messageId):\(requestId)" | ||
| } | ||
| return messageId ?? requestId |
There was a problem hiding this comment.
When a stream with only messageId or requestId appends after its file has already been cached, this parser deduplicates the newly read chunk by that lone ID, but mergeClaudeRows still calls claudeInFileKey, which returns a key only when both IDs exist. The cached cumulative snapshot and the appended cumulative snapshot therefore both survive and are summed, inflating tokens and cost during normal incremental refreshes; use the same single-ID key semantics in the merge path.
Useful? React with 👍 / 👎.
| if let messageId, let requestId { | ||
| return "\(messageId):\(requestId)" | ||
| } | ||
| return messageId ?? requestId |
There was a problem hiding this comment.
When a proxy omits one identifier and reuses the other—especially when it writes an empty-string placeholder—this turns every such row into the same dictionary entry and retains only the last one. The existing claude counts different request ids separately test already establishes that a message ID alone is not sufficient to distinguish requests, so logs with that request field omitted will now undercount unrelated assistant calls; validate that a lone ID is nonempty and uniquely identifies a stream before using it as a deduplication key.
Useful? React with 👍 / 👎.
| let key: String? = { | ||
| if let messageId, let requestId { | ||
| return "\(messageId):\(requestId)" | ||
| } | ||
| return messageId ?? requestId |
There was a problem hiding this comment.
For users upgrading with affected rows already stored in the Claude/Vertex v6 cache, unchanged JSONL files are skipped by processClaudeFile, and these providers have neither a producer key nor an artifact-version change to invalidate their cached rows. Consequently the new deduplication behavior is not applied to existing logs until a force/full rescan happens, leaving previously overcounted usage visible after the upgrade; rotate the Claude/Vertex cache version or add an equivalent migration trigger.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🦪 silver shellfish
2 participants
Summary
Fixes #2393.
When users run non-Anthropic models (such as GPT-4o, GPT-5.4, or DeepSeek) inside Claude Code via proxies (like
CLIProxyApi), Claude Code logs store assistant entries with model names like"gpt-5.4". CodexBar was previously hardcoding model pricing lookup to theanthropicprovider for Claude Code logs, causing cost calculations to returnnil($0.00 / unpriced).This PR:
ModelsDevCatalog.pricing(modelID:)to search across all catalog providers when provider ID is unknown or mismatched.CostUsagePricing.claudeCostUSDandCostUsagePricing.codexCostUSDwith recursion protection (allowFallback).CodexParserHash.generated.swift.Testing
swift test --filter CostUsagePricingTests(47/47 passed)swift test --filter CostUsageScannerClaudeRegressionTests(13/13 passed)make check(passed)