fix: honor preferred currency in cost-history detail chart - #2887
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
P2
|
Codex review: needs maintainer review before merge. Reviewed August 12, 2026, 10:25 PM ET / August 13, 2026, 02:25 UTC. ClawSweeper reviewWhat this changesThe PR converts cost-history chart labels to the selected display currency while preserving provider-native chart geometry and refreshing hydrated charts when the exchange rate changes. Merge readinessKeep open: this is a focused, correct fix for a current-main display inconsistency, with the prior cache-invalidation concern addressed and credible production-boundary proof provided. Priority: P2 Review scores
Verification
How this fits togetherCodexBar scans provider usage snapshots and renders cost history in a hosted menu-bar chart. The chart combines native provider costs with the user’s currency preference to produce displayed labels without altering stored data or bar heights. flowchart LR
A[Provider cost snapshot] --> B[Preferred currency setting]
B --> C[Shared conversion resolver]
A --> D[Native chart geometry]
C --> E[Display currency and multiplier]
D --> F[Cost-history chart]
E --> F
F --> G[Menu-bar labels and details]
Before merge
Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Merge the display-only conversion after normal checks complete, retaining the shared formatter fallback behavior and multiplier-based cache invalidation. Do we have a high-confidence way to reproduce the issue? Yes, from source: render a cost-history snapshot with an explicit non-native preferred currency and compare the current-main raw-label path with the branch’s shared conversion path; the PR also supplies an isolated production-view run. Is this the best way to solve the issue? Yes. Reusing the existing formatter for a display-time multiplier preserves native snapshots and geometry while matching the established currency fallback contract. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against ee29794b9a1b. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
History |
![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: 09d6cfb1a7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return .costHistory(CostHistoryChartMenuView.renderFingerprint( | ||
| from: snapshot, | ||
| provider: provider, | ||
| displayCurrencyCode: self.costHistoryDisplayConversion(for: snapshot).currencyCode)) |
There was a problem hiding this comment.
When a non-USD preferred currency's rate changes while the app is running, an already-hydrated cost-history submenu can keep showing values converted with the old multiplier: refreshHostedSubviewMenu compares this fingerprint before rebuilding, but the fingerprint only records the display currency code, while the actual CostHistoryChartMenuView also depends on costHistoryDisplayConversion(...).multiplier. Reopening the submenu after CurrencyExchange refreshes rates will therefore return early as “unchanged” whenever the code is still, for example, EUR, leaving stale converted cost labels until some unrelated rebuild occurs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — addressed in 3a40760. RenderFingerprint now carries costMultiplierBitPattern alongside the display currency code, and the fingerprint call site passes the resolved multiplier, so a mid-session exchange-rate refresh invalidates an already-hydrated submenu. The added test asserts that a rate-only change (same currency code, different multiplier) produces a different fingerprint.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A hosted cost-history submenu is only rebuilt when its render fingerprint changes. The fingerprint carried the display currency code but not the conversion multiplier, so a live exchange-rate refresh mid-session left already-hydrated charts showing values converted with the stale rate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3a40760 to
d0fd6e1
Compare
|
@clawsweeper re-review The contributor branch is now rebuilt on current main at |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
proof: 📸 screenshot
|
Landed and verified. Testing and proof:
Merged as |
2 participants
Summary
The cost-history detail submenu ignored Settings → General → Currency and always rendered its labels in the snapshot's native currency. Other cost surfaces already use
UsageFormatter.convertedCost, so users selecting GBP, EUR, CNY, or another supported currency saw converted values everywhere except this submenu.Change
Display-time conversion only; stored snapshots and chart geometry stay in provider-native values.
CostHistoryChartMenuViewreceives one display currency and multiplier. Its existing formatting paths apply that multiplier to the total, day detail, model total and Standard/Fast rows, project/source rows, session rows, and Y-axis labels.StatusItemController+HostedSubmenusresolves the display conversion throughUsageFormatter.convertedCost, preserving the shared fallback contract:auto, same-currency, and unavailable exchange rates remain in the source currency with multiplier 1.Double.bitPatternof the multiplier. A rate refresh therefore invalidates an already-hydrated chart even when the selected currency code is unchanged.Deliberately unchanged: provider-native balances and other surfaces that intentionally keep native currencies separate.
Validation
swift test --filter CostHistoryChartMenuView— 30/30 passed.21.0and21.5produce different render fingerprints; the exact bit patterns are asserted.swift test --filter ProviderArchitectureGatekeeperTests— 38/38 passed.make checkpassed.Runtime and visual proof
A Developer ID-signed debug bundle (
com.steipete.codexbar.debug) was built from this head and launched with:The packaged CLI exercised the production scanner and loaded two days, two synthetic projects, three synthetic sessions, and two model rows for the latest day. The exact production
CostHistoryChartMenuViewwas then rendered offscreen with that data. The image shows CNY conversion across every affected label path while bar proportions remain source-value based.The host desktop was locked during proof capture, so this is explicitly an offscreen production-view render rather than a screenshot of the live popup. The signed app process, bundle signature, isolated scanner path, and deterministic refresh regression were verified separately.
Contributor authorship and co-author trailers are preserved in the reconstructed commits.