feat: add KRW to the preferred currency picker - #2669
Merged
Conversation
KRW was the one major Asian currency missing from the picker added in steipete#2490, so won-billed users converted USD estimates by hand. No new exchange-rate logic is needed: fetchLatestRatesIfNeeded already merges every rate in the open.er-api.com payload, which carries KRW. Listing the code in supportedCurrencies is what opens the requiresLiveRates gate, so selecting KRW triggers the same live-fetch and 24h-cache path as the existing currencies. The hardcoded fallback rate is only read before the first successful fetch. KRW is zero-decimal and currencyString pins en_US, so ICU resolves the fraction digits and won renders without a fractional separator on any host. Refs steipete#2449 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
Codex review: needs maintainer review before merge. Reviewed August 5, 2026, 9:56 PM ET / August 6, 2026, 01:56 UTC. ClawSweeper reviewWhat this changesAdds Korean won as a selectable preferred display currency, with an offline fallback rate and focused formatter/live-rate-gate coverage. Merge readiness✅ Ready for maintainer review Keep open for normal maintainer review: current main and v0.47.0 do not offer KRW, while this narrow extension follows the existing picker and exchange-rate design, has direct UI proof, and introduces no actionable defect. Priority: P3 Review scores
Verification
How this fits togetherCodexBar converts provider cost estimates into a user-selected display currency. The settings picker supplies the chosen code to the exchange service, which uses cached/live or fallback rates before formatters render menu costs. flowchart LR
A[Provider cost estimates] --> B[Preferred currency setting]
B --> C[Exchange-rate service]
C --> D[Fallback or cached live rate]
D --> E[Cost formatter]
E --> F[Menu and settings UI]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Merge the focused KRW addition through normal maintainer review, retaining the established live-rate and offline-fallback behavior. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR adds a choice to an existing preference rather than repairing a current failure; its after-fix UI proof directly demonstrates the intended flow. Is this the best way to solve the issue? Yes: adding KRW to the existing picker and rate tables reuses the established conversion, caching, and formatting path without parallel logic. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 4cdb349cbc57. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (4 earlier review cycles)
|
clawsweeper
Bot
added
proof: sufficient
Owner
|
Landed. Verified before merge: wiring matches existing currencies (membership-driven live-rate gate, pinned en_US_POSIX-style formatting via ICU so KRW renders zero decimals); tests assert both the ₩ symbol and absence of decimal separators. make check clean; UsageFormatter suite green. Partially addresses #2449 (adds the currency; billing-alignment remains open). Thanks @kes02! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
KRW (₩)to the preferred currency picker in Settings → General. KRW was the one major Asian currency missing from the list introduced in #2490 (refs #2449), so won-billed users convert USD estimates by hand to compare them against a card statement.Change
Three lines of source, no new exchange-rate logic:
CurrencyExchange.swift:18—"KRW"joinssupportedCurrencies, next to CNY/JPY rather than appended, matching the existing regional grouping.CurrencyExchange.swift:30—"KRW": 1428.90joins the hardcoded fallback table. Existing fallback values are untouched.PreferencesGeneralPane.swift:85,105—PreferredCurrencyOption.krwand its"KRW (₩)"label.fetchLatestRatesIfNeededalready merges every rate in theopen.er-api.com/v6/latest/USDpayload, and that payload carriesKRW. Listing the code insupportedCurrenciesis what opens therequiresLiveRatesgate (CurrencyExchange.swift:116), so selecting KRW triggers the same live-fetch and 24h-cache path as the existing currencies; the hardcoded rate is only read before the first successful fetch.No public API surface change:
git diff origin/main -- '*.swift'adds and removes nopublicline —supportedCurrenciesgains an element, its signature is unchanged.Formatting
KRW is a zero-decimal currency, and
UsageFormatter.currencyStringdelegates to.currency(code:), which resolves ICU's fraction digits — so no formatter change is needed: $10 renders as₩14,289, $0.42 as₩600.That path pins
Locale(identifier: "en_US"), so the rendering is host-independent and the new no-fractional-separator assertion holds on a localized machine too — the failure mode #2651 fixed for Perplexity does not apply here.Validation
make check— swiftformat clean, swiftlint 0 violations across 1809 filesmake test— 817 selections / 69 groups, 0 first-pass failures, 0 retries, 0 timeoutsswift test --filter UsageFormatter— 46 tests pass, including the new assertionsgit diff --check— cleanNew coverage in
UsageFormatterTests.swift:convert(usdAmount:to:))₩is rendered and no fractional separator is emittedrequiresLiveRates("KRW") == true, so the live fetch is actually triggered rather than the currency silently sitting on the hardcoded rateReal behavior proof
Fresh bundle built from this branch at
44393a0c(make start→ release package → relaunch), macOS 26.5 (arm64).1. Settings → General — Preferred Currency resolves to
KRW (₩)2. Menu card — provider costs render as won
Today
₩98,810, 30d cost₩1,884,614, daily-chart peak₩300,527— thousands separated, no fractional separator, matching the zero-decimal expectation asserted inUsageFormatterTests.Rate provenance. Selecting KRW opens the
requiresLiveRatesgate, so the displayed won comes from a live rate rather than the hardcoded1428.90fallback: the app fetched and cachedCodexBar.CurrencyExchangeRates["KRW"] = 1428.890254withCodexBar.CurrencyExchangeLastFetch = 2026-08-05 14:01:55 UTC. Earlier in the same session, before that fetch returned, the Today row still rendered$62.75; once the rate landed it became₩89,659— exactly62.75 × 1428.890254. The screenshot above was taken a few minutes later, after more usage had accrued.