Fix unknown Codex model attribution - #2061
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes Codex cost attribution when token events lack a model by removing the prior implicit fallback to gpt-5, preferring the model present on each token event over stale turn_context, and ensuring unresolved usage is tracked under an explicit unknown model bucket. It also bumps the Codex cost cache artifact version to invalidate previously mis-attributed cached data.
Changes:
- Update Codex session scanning to select
record.model/modelFromInfobeforeturn_context, and to fall back to an explicitunknownkey instead ofgpt-5. - Bump Codex cost cache artifact from
codex-v8.jsontocodex-v9.jsonto force a clean rebuild. - Add/update tests and formatting to display the
unknownbucket as “Unknown model”.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/CodexBarTests/UsageFormatterTests.swift | Adds coverage for displaying the unknown model bucket as “Unknown model”. |
| Tests/CodexBarTests/CostUsageScannerBreakdownTests.swift | Adds regression tests for missing model handling and model precedence; updates expected Codex cache artifact version. |
| Tests/CodexBarTests/CostUsageCacheTests.swift | Updates expected Codex cache filename to codex-v9.json. |
| Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift | Removes gpt-5 fallback, prefers event model over turn_context, and introduces the unknown model key. |
| Sources/CodexBarCore/Vendored/CostUsage/CostUsageCache.swift | Bumps Codex cache artifact version to 9 (filename changes to codex-v9.json). |
| Sources/CodexBarCore/UsageFormatter.swift | Renders model key unknown as “Unknown model”. |
| Sources/CodexBarCore/Generated/CodexParserHash.generated.swift | Updates generated parser hash value. |
| CHANGELOG.md | Documents the Codex cost attribution fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| else { return } | ||
|
|
||
| let model = currentModel ?? record.model ?? "gpt-5" | ||
| let model = record.model ?? currentModel ?? Self.codexUnknownModel |
There was a problem hiding this comment.
Fixed in 7373e30. Model candidates are now trimmed and blank strings are treated as missing before event/context precedence is applied. Added regression coverage for a whitespace-only event model falling back to the valid turn context.
| ?? payload["model"] as? String | ||
| ?? obj["model"] as? String | ||
| let model = currentModel ?? modelFromInfo ?? "gpt-5" | ||
| let model = modelFromInfo ?? currentModel ?? Self.codexUnknownModel |
There was a problem hiding this comment.
Fixed in 7373e30. Each candidate field is normalized before the nil-coalescing chain, so a blank model can no longer hide a valid model_name. Added an end-to-end regression test for that exact shape.
|
Codex review: needs maintainer review before merge. Reviewed July 11, 2026, 1:32 PM ET / 17:32 UTC. Summary Reproducibility: yes. Current main has a concrete model-less token-event path that falls through to GPT-5, and the PR supplies both a real affected-cache rebuild and an isolated end-to-end scanner/CLI reproduction. Review metrics: 2 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Next step before merge
Security Review detailsBest possible solution: Merge the focused correction once required checks pass so CodexBar preserves observed model attribution, reports unresolved tokens explicitly, and never assigns fabricated GPT-5 cost. Do we have a high-confidence way to reproduce the issue? Yes. Current main has a concrete model-less token-event path that falls through to GPT-5, and the PR supplies both a real affected-cache rebuild and an isolated end-to-end scanner/CLI reproduction. Is this the best way to solve the issue? Yes. Preserving current turn context, filtering blank evidence, retaining unresolved token counts under an unpriced sentinel, and invalidating stale caches is the narrowest maintainable correction. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 060ffe13012d. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (5 earlier review cycles)
|
|
@clawsweeper re-review Added after-fix proof to the PR body: a real cache rebuild screenshot with the false GPT-5 row removed, plus isolated |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
proof: sufficient
clawsweeper
Bot
added
rating: 🐚 platinum hermit
clawsweeper
Bot
added
rating: 🦞 diamond lobster
Summary
gpt-5.turn_contextmodel when available, matching the scanner's established precedence.unknownkey, rendered asUnknown model, without applying GPT-5 pricing.codex-v9.jsonso existing false attribution is removed.Refs #1013. This is a follow-up to #1014: that patch recovered model state from oversized
turn_contextrows, while this patch removes the remaining assumption that an unresolved model must be GPT-5.Evidence
Before
CodexBar briefly showed 211M tokens and $34.90 attributed to
gpt-5:The underlying Codex data for that day contained no
model:"gpt-5"records. The local session JSONL contained 112gpt-5.6-solmodel records and 352codex-auto-reviewrecords;logs_2.sqlitealso contained no GPT-5 completion. A later cache refresh removed the GPT-5 row, confirming that this was transient scanner attribution rather than a backend model fallback.The remaining cause was the scanner's final fallback:
That fallback turned missing evidence into a specific model and a priced cost. This patch keeps the usage visible but explicitly unattributed. It preserves current turn-context precedence and uses normalized token-event model evidence when context is unavailable.
After: real cache rebuild
The same Jul 11 history after rebuilding with the patched scanner has no GPT-5 row. The resolved usage is attributed to
gpt-5.6-solandcodex-auto-review:After: unresolved-model path
I also ran the freshly built
CodexBarCLI costagainst an isolated Codex home containing one model-lesstoken_countevent with 55M tokens. This exercises the complete JSONL scanner, cache, pricing, and JSON output path without reading a real account:{ "date": "2026-07-11", "modelBreakdowns": [ { "modelName": "unknown", "totalTokens": 55000000 } ], "modelsUsed": ["unknown"], "totalTokens": 55000000 }There is no
costortotalCostfield for the unknown breakdown, and no GPT-5 row. The fixture used a redacted project path and no credentials, endpoints, browser data, or Keychain access.Validation
make checkDYLD_FRAMEWORK_PATH="$PWD/.build/out/Products/Debug" swift test --filter CostUsageScannerBreakdownTests --filter CostUsageCacheTests(65 tests passed)DYLD_FRAMEWORK_PATH="$PWD/.build/out/Products/Debug" swift test --filter UsageFormatterTests(42 tests passed)The full sharded suite reached an unrelated order-sensitive Claude test group that failed in the combined run and passed when rerun by itself. The Codex scanner/cache and formatter suites pass cleanly.