Codex: distinguish API estimates from subscription bills - #2181
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: 966c444d28
ℹ️ 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("Estimated from local Codex logs for the selected account."), | ||
| L("cost_estimate_hint"), | ||
| ].joined(separator: "\n") |
There was a problem hiding this comment.
When the user selects the Inline summary cost display style, Codex has no separate token-usage section, so this hint is only shown through costHistoryInlineDashboard, which appends the entire hint as one detailLines entry and InlineUsageDashboardContent renders each entry with .lineLimit(1). Because the new cost_estimate_hint is placed after a newline in the same string, the “may differ from your bill” disclaimer is clipped in that mode; split the Codex hints into separate dashboard detail rows or otherwise render this hint multiline.
Useful? React with 👍 / 👎.
Yuxin-Qiao
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 943b3d1e35
ℹ️ 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".
| /* Cost estimation */ | ||
| "cost_header_estimated" = "Cost (estimated)"; | ||
| "cost_estimate_hint" = "Estimated from local logs · may differ from your bill"; | ||
| "cost_header_estimated" = "API-equivalent estimate"; |
There was a problem hiding this comment.
This key is rendered unconditionally by the SwiftUI token-cost sections (Sources/CodexBar/MenuCardView.swift lines 216 and 761) for every provider with token-cost data, not just Codex. After changing the shared header to “API-equivalent estimate”, non-Codex cards such as OpenAI, Mistral, and Bedrock also get that label even though their hints describe Admin API, Mistral billing, or AWS Cost Explorer data; this contradicts the stated scope of keeping non-Codex labels unchanged. Please make the header provider-specific like the status-menu title or keep this shared key generic.
Useful? React with 👍 / 👎.
|
Addressed the two inline review findings in 3e6e63c.
Verification: focused menu/inline/CLI/widget Swift regression suite and |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e6e63c638
ℹ️ 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".
| let sessionLabel = if provider == .bedrock || provider == .mistral { | ||
| "Latest billing day" | ||
| } else if provider == .codex { | ||
| "Today API est. · not billed" |
There was a problem hiding this comment.
For Codex widget snapshots this new label is later used by CompactMetricFormatter for the compact .todayCost metric, which builds the title as "\($0.sessionLabel) cost" in Sources/CodexBarWidget/CodexBarWidgetViews.swift:211-213. With this value the compact widget renders Today API est. · not billed cost, reintroducing the cost wording immediately after the not-billed disclaimer and making the Codex estimate disclosure confusing; please either make the compact widget avoid appending cost for Codex estimates or store a separate compact label.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| static func costMenuTitleForProvider(_ provider: UsageProvider) -> String { | ||
| provider == .codex ? L("codex_api_estimate_header") : self.costMenuTitle |
There was a problem hiding this comment.
When a Codex cost history submenu is available, costMenuVisibleDetailLines(..., hasSubmenu: true) returns no detail rows and native items omit the tooltip, so this title is the only changed top-level text before opening a submenu whose CostHistoryChartMenuView still labels the chart/axis and total as Cost/Est. total without the not-billed hint. In the submenu style, the dollar totals are still presented as cost, so users can miss the API-equivalent/not-billed boundary; pass provider-specific copy into the chart or add the disclaimer there.
Useful? React with 👍 / 👎.
| static func tokenUsageHeader(provider: UsageProvider) -> String { | ||
| provider == .codex ? L("codex_api_estimate_header") : L("cost_header_estimated") |
There was a problem hiding this comment.
This adds a provider-specific token-usage header, but the Settings provider detail still renders Codex token USD as ProviderMetricInlineTextRow(title: L("Cost"), value: tokenUsage.sessionLine) and does not show tokenUsage.hintLine, so Settings continues to show the local Codex API-rate total as Cost with no not-billed disclosure. Please route that provider-detail row through the same provider-specific title/disclaimer path so the in-app Settings surface does not contradict the new Codex wording.
Useful? React with 👍 / 👎.
| let hintLines = Self.tokenUsageHintLines(provider: provider) | ||
| if hintLines.isEmpty == false { | ||
| details.append(contentsOf: hintLines) |
There was a problem hiding this comment.
When the user selects the Inline summary display, adding the Codex hint lines here still leaves the KPI title built above as 30d cost or Last N days Cost, so the visible dollar total in that summary is still labeled as cost rather than an API-equivalent estimate. Please make the historyTitle provider-specific for Codex in this inline dashboard, otherwise this surface still contradicts the new estimate/not-billed wording.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32693b4442
ℹ️ 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".
| } | ||
|
|
||
| static func costMetricLabel(_ label: String, provider: UsageProvider) -> String { | ||
| provider == .codex ? label : "\(label) cost" |
There was a problem hiding this comment.
When entry.tokenUsage does not come from the new UsageStore labels—for example WidgetPreviewData.snapshot() and any persisted pre-upgrade WidgetSnapshot, whose TokenUsageSummary defaults are still Today/30d—this Codex branch returns the bare label unchanged. A compact Codex metric can then show a dollar value labelled only Today or 30d, with no API-estimate/not-billed disclosure until the main app rewrites the snapshot; please normalize bare Codex labels here or migrate all producers before suppressing the cost suffix.
Useful? React with 👍 / 👎.
| .init( | ||
| title: usesLatestPrimary ? L("Latest") : L("Today"), | ||
| title: provider == .codex | ||
| ? "\(L("Today")) · \(L("codex_api_estimate_header"))" |
There was a problem hiding this comment.
When provider == .codex and the inline dashboard is rendered, this branch relabels the visible KPI as an API-equivalent estimate, but the same model still sets the mini chart accessibility label to a string ending in cost trend, which MiniUsageBars exposes via .accessibilityLabel(...). VoiceOver users therefore still hear the Codex dollar chart described as a cost trend even though the visible UI says it is not a bill; make the accessibility label provider-specific too.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
Codex review: needs real behavior proof before merge. Reviewed July 16, 2026, 10:29 AM ET / 14:29 UTC. Summary Reproducibility: yes. at the source level: construct a Codex widget entry using a legacy TokenUsageSummary label such as Today, 7d, or 30d and render a non-compact switcher or history layout; those paths bypass the new CompactMetricFormatter normalization. A live widget reproduction was not supplied. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Normalize legacy Codex TokenUsageSummary labels at a shared snapshot/model or formatting boundary used by every widget layout, retain unchanged behavior for other providers, add coverage for legacy numeric windows across compact and non-compact layouts, and provide redacted native app and widget screenshots or a short recording. Do we have a high-confidence way to reproduce the issue? Yes at the source level: construct a Codex widget entry using a legacy TokenUsageSummary label such as Today, 7d, or 30d and render a non-compact switcher or history layout; those paths bypass the new CompactMetricFormatter normalization. A live widget reproduction was not supplied. Is this the best way to solve the issue? No, not yet: provider-specific copy and compact compatibility handling are appropriate, but normalization belongs in a shared widget boundary so every layout handles persisted snapshots consistently. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 6d71af30b84d. Label changesLabel changes:
Label 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 (3 earlier review cycles)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f335ddc68e
ℹ️ 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".
| var model = InlineUsageDashboardModel( | ||
| accessibilityLabel: "\(providerName) \(periodLabel) cost trend", | ||
| accessibilityLabel: provider == .codex | ||
| ? "\(providerName) \(periodLabel) \(L(\"codex_api_estimate_header\")) trend" |
There was a problem hiding this comment.
This string literal does not compile because the escaped quotes are inside a Swift string interpolation expression; Swift parses L(\"...\") as an unterminated interpolation/string. Use normal quotes inside the interpolation (for example L("codex_api_estimate_header")), otherwise any target that builds InlineUsageDashboardContent.swift fails before tests can run.
Useful? React with 👍 / 👎.
| let fallbackTitle = StatusItemController.costMenuFallbackAttributedTitle( | ||
| title: "API-equivalent estimate", | ||
| visibleDetailLines: visibleLines) | ||
| #expect(fallbackTitle.string == "API-equivalent estimate") |
There was a problem hiding this comment.
With the new hasSubmenu branch, a token section that has a non-empty hintLine now makes visibleLines contain that hint, so the fallback title built here becomes API-equivalent estimate <hint> instead of the bare title. This assertion still expects the old empty-detail result and will fail for exactly the fixture set up above; either update the expected string or make the fixture match the no-detail case.
Useful? React with 👍 / 👎.
| #expect(todayMetric.label.hasPrefix("Today · ")) | ||
| #expect(historyMetric.label.hasPrefix("30d · ")) |
There was a problem hiding this comment.
For this .codex entry, staleToken uses the default Today/30d labels, and CompactMetricFormatter.costMetricLabel now rewrites those to Today API est. · not billed and 30d API est. · not billed before tokenRowTitle appends the stale-age suffix. These expectations still look for the old bare prefixes, so this widget test will fail once the production formatter exercises the new normalization.
Useful? React with 👍 / 👎.
5ba5044 to
288e823
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 288e82360d
ℹ️ 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".
| return switch label { | ||
| case "Today": "Today API est. · not billed" | ||
| case "30d": "30d API est. · not billed" | ||
| default: label |
There was a problem hiding this comment.
Fresh evidence in this revision is that the new fallback only rewrites Today and 30d, but pre-upgrade widget snapshots were produced from the configurable costUsageHistoryDays value, so Codex users who had a 7/90/etc. day window can have persisted labels like 7d or 90d. In that case the compact .last30DaysCost metric still renders a bare dollar value labelled only 7d until the main app republishes the snapshot, with no API-estimate/not-billed disclosure; please normalize other bare Nd window labels (or migrate decoded snapshots) as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09f0fbec0b
ℹ️ 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".
| static func costMetricLabel(_ label: String, provider: UsageProvider) -> String { | ||
| guard provider == .codex else { return "\(label) cost" } |
There was a problem hiding this comment.
Fresh evidence after the compact fix is that this normalization is confined to CompactMetricFormatter; the regular provider/switcher/history widget layouts still pass token.sessionLabel and token.last30DaysLabel directly to WidgetFormat.tokenRowTitle (for example SwitcherSmallUsageView and HistoryView). When the widget extension renders WidgetPreviewData.snapshot() or a persisted pre-upgrade Codex WidgetSnapshot whose TokenUsageSummary defaults are still Today/30d, those non-compact layouts continue to show dollar values with bare labels and no API-estimate/not-billed disclosure until the app rewrites the snapshot. Please normalize Codex token labels at decode/format time or in all widget render paths.
Useful? React with 👍 / 👎.
|
Added redacted live |
clawsweeper
Bot
added
rating: 🦐 gold shrimp
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96363a7f57
ℹ️ 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".
| guard !hasSubmenu else { | ||
| guard provider == .codex else { return [] } | ||
| return tokenUsage?.hintLine? | ||
| .split(separator: "\n") | ||
| .map(String.init) | ||
| .filter { !$0.isEmpty } ?? [] |
There was a problem hiding this comment.
When Codex has token-cost data but no history submenu (for example the cost-menu display is enabled while tokenSnapshot.daily is empty, so webItems.hasCostHistory is false), this submenu-only branch is skipped and the method falls through to the primary session/month lines, which omit hintLine whenever those lines exist. That leaves the visible menu row showing Codex dollar amounts under API-equivalent estimate without the new not a subscription bill or plan value disclosure unless the user discovers the tooltip; include the Codex hint in the no-submenu visible rows too.
Useful? React with 👍 / 👎.
…e-disclosure # Conflicts: # Sources/CodexBar/InlineUsageDashboardContent.swift
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5d2b76c9c
ℹ️ 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".
| ?? (historyDays == 1 | ||
| ? L("Today") | ||
| : historyDays == 30 | ||
| ? "30d" |
There was a problem hiding this comment.
When the app is running in a non-English locale and the Codex inline dashboard uses the default 30-day window, this literal bypasses the existing localized 30d key, so the second KPI becomes mixed-language like 30d · API-equivalent estimate even though the previous 30-day cost label was localized through L("30d cost"). Use L("30d") for this period before appending the Codex-specific header.
Useful? React with 👍 / 👎.
|
Merged as c613397. Verification:
|
2 participants
Summary
Why
CodexBar derives this number from local Codex JSONL token usage and public API prices. It is not the user's ChatGPT/Codex subscription bill and cannot measure the value of a Plus, Pro, or other flat-rate plan.
Scope
Live CLI proof (redacted)
Ran the PR head against real local Codex and Claude history. Amounts and token totals are redacted; provider names and the changed disclosure text are preserved:
This demonstrates that the Codex section carries the new not-billed subscription disclosure while the non-Codex provider keeps its existing provider-specific API-rate wording. No project path, account identifier, session content, prompt, or credential is included.
Validation
CodexBarCLI cost --provider both --days 1output abovegit diff --check upstream/main...HEAD