feat: add AED to the preferred currency picker - #3186
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
3307310 to
b3fad01
Compare
clawsweeper
Bot
added
P3
|
Codex review: needs maintainer review before merge. Reviewed August 25, 2026, 6:34 AM ET / 10:34 UTC. ClawSweeper reviewWhat this changesThe PR makes UAE dirham (AED) selectable as CodexBar’s display currency, adds its fallback rate and regression coverage, and includes a release-note entry. Merge readiness✅ Ready for maintainer review The PR is a focused extension of the existing currency path with targeted coverage and convincing after-fix runtime proof; no blocking patch defect was found. Priority: P3 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — execution before step 1 Assertions:
How this fits togetherCodexBar converts provider cost estimates into a user-selected display currency. The Settings picker persists that choice, then shared conversion and formatting logic presents the result in menu and dashboard cost views. flowchart LR
A[Provider cost] --> B[Currency conversion]
C[Preferred currency setting] --> B
B --> D[Live or fallback rate]
D --> E[Formatted cost]
E --> F[Menu and dashboard]
C --> G[Settings picker]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Merge the narrow AED addition through the established picker, conversion, live-rate cache, and formatter path after normal check and maintainer review. Do we have a high-confidence way to reproduce the issue? Not applicable: this is a preference enhancement rather than a defect report, and the PR supplies direct after-fix picker and menu-card evidence. Is this the best way to solve the issue? Yes. Reusing the existing supported-currency list, live-rate gate, fallback table, and formatter is the narrowest maintainable implementation. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 0194d88de2d4. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (3 earlier review cycles) |
Add the UAE dirham to Settings → General → Preferred Currency so
AED-billed users see converted spend estimates instead of converting
the USD figures by hand. Follows the established shape for adding a
currency: three source/test touch points, no new exchange-rate logic.
- Sources/CodexBar/PreferencesGeneralPane.swift — PreferredCurrencyOption.aed
joins the picker with the label "AED (د.إ)". The 2025 CBUAE national
dirham symbol has no Unicode code point on shipping Apple platforms
yet (U+20C0 is SOM SIGN, U+20C1 is SAUDI RIYAL SIGN), so the label
uses the traditional Arabic abbreviation to stay renderable in system
fonts everywhere.
- Sources/CodexBarCore/CurrencyExchange.swift — "AED" joins
supportedCurrencies, which is what opens the requiresLiveRates gate;
selecting it triggers the same daily open.er-api.com live fetch with a
24h cache used by every other non-USD option. Adds the hardcoded
fallback "AED": 3.67 (USD peg), read only before the first successful
fetch.
- Tests/CodexBarTests/UsageFormatterTests.swift — assert
requiresLiveRates("AED"), proving selection actually triggers the
live-rate path rather than silently sitting on the fallback.
Formatting needs no changes: UsageFormatter.currencyString pins
Locale(identifier: "en_US"), where ICU renders two-decimal AED, so
$10 becomes "AED 10.00", host-locale independent like CZK and KRW.
b3fad01 to
c04a3ab
Compare
clawsweeper
Bot
added
rating: 🧂 unranked krab
Co-authored-by: Sami El Achi <mohamadashi7@gmail.com>
steipete
left a comment
steipete
left a comment
There was a problem hiding this comment.
The dirham option now uses its exact 3.6725 USD peg when offline, preserves native two-decimal AED formatting, supports both conversion directions and cross-currency spend, persists across settings reloads, and includes a regression ensuring every picker option matches the supported exchange-rate currencies. Contributor screenshots demonstrate the real picker and converted menu amounts. Focused formatter and settings tests plus repository lint all pass.
clawsweeper
Bot
added
rating: 🦞 diamond lobster
|
Landed in f10b605 after improving the offline fallback to the exact 3.6725 USD-to-AED peg and adding broader product coverage. The settings picker now offers the existing system-renderable Arabic dirham abbreviation, while actual amounts retain native AED code formatting with two fraction digits. Regression coverage verifies USD-to-AED, AED-to-USD and GBP-to-AED conversion, lowercase live-rate eligibility, persisted AED selection, exact formatted output, and parity between every picker option and the supported exchange currencies. Verification: 90 focused formatter and settings tests passed; make check passed with zero violations; the full local suite passed all 930 selections across 78 groups with zero failures, retries, or timeouts; and exact-head GitHub Actions run 32837620486 passed both macOS shards, Linux x64, arm64 and musl builds, lint, and security checks. Contributor credit and changelog included. |
2 participants
Summary
Adds
AED (د.إ)to the preferred currency picker in Settings → General. Dirham-billed users currently convert the USD estimates by hand to compare them against a card statement.Change
Three lines of source, no new exchange-rate logic:
CurrencyExchange.swift—"AED"joinssupportedCurrencies. Listing the code is what opens therequiresLiveRatesgate (it returns true for any supported non-USD code), so selecting AED triggers the same dailyopen.er-api.com/v6/latest/USDlive fetch + 24h cache path as every existing non-USD option; no fetch logic changes.CurrencyExchange.swift—"AED": 3.67joins the hardcoded fallback table, read only before the first successful fetch. The dirham has been pegged to the dollar at ~3.6725 since 1997, so the fallback is unusually accurate. Existing fallback values are untouched.PreferencesGeneralPane.swift—PreferredCurrencyOption.aedand its"AED (د.إ)"label.No public API surface change:
supportedCurrenciesgains an element, its signature is unchanged.Symbol choice
The Central Bank of the UAE published a national currency symbol in 2025 (a "D" with two horizontal bars), but it has no Unicode code point on shipping Apple platforms yet — U+20C0 is SOM SIGN and U+20C1 is SAUDI RIYAL SIGN; there is no DIRHAM SIGN scalar, so the glyph would render as tofu. The label therefore uses the traditional Arabic abbreviation
د.إ, which stays renderable via the system font cascade. If/when Unicode assigns a code point, the label is a one-line update.Per CBUAE guidance ("use one or the other"), rendered amounts show only the code — never symbol and code together.
Formatting
AED is a two-decimal currency and
UsageFormatter.currencyStringpinsLocale(identifier: "en_US"). ICU has noد.إsymbol under that locale, so amounts render with the code prefix and two fraction digits:(These format an amount that is already in dirhams; conversion happens upstream in
CurrencyExchange.) Because the locale is pinned, rendering is host-locale independent, and this matches how other currencies without a Latin symbol degrade under that same pinned locale —CZK 210.11. The menu-card screenshot below shows exactly this shape at runtime.Validation
./Scripts/lint.sh— swiftformat clean, swiftlint 0 violations, 0 serious in 1990 filesswift test --filter UsageFormatter— 46 tests in 2 suites passedmake test(full sharded suite) — every group that ran is green except one that is pre-existing and unrelated:CostUsagePricingTestsrecords two1e-12float-tolerance issues in Codex pricing math, atCostUsagePricingTests.swift:424and:471. Checking out cleanmain(b6a4ce968, no AED) and runningswift test --filter CostUsagePricingTestsreproduces both with identical deltas —0.5640000000000001and0.0002920000000000004— so it predates this branch and is untouched by it.UsageFormatterTests.swift:convert(usdAmount:to:)resolves the dirham instead of returningnil(which would silently drop converted amounts)convertedCostString(preferredCurrency: "AED")renders theAEDcode plus a decimal separator — the two-decimal behavior shown aboverequiresLiveRates("AED") == true, proving selection actually opens the live-rate path rather than silently sitting on the hardcoded fallbackReal behavior proof
Fresh bundle built from this branch and relaunched — CodexBar 0.55.0 (129) on macOS 26.6.2 (arm64).
1. Settings → General — Preferred Currency resolves to
AED (د.إ)The new option appears in the picker (checked, last in the list) and the row value updates to it.
2. Menu card — provider costs render as dirhams
Today
AED 988.31, 30dAED 11,961.84, daily-chart peakAED 2,060— code prefix, thousands separated, two decimals, matching the pinned-en_USexpectation asserted inUsageFormatterTests.Rate provenance. Selecting AED opens the
requiresLiveRatesgate, and the app then fetched and cached a live dirham rate rather than resting on the fallback:3.6725is the live value from the rates payload, distinct from the hardcoded3.67fallback — so the amounts above came through the live path. Because the dirham is pegged, the two rates agree closely (which is what makes the fallback a safe pre-fetch default); the cached value and fetch timestamp, not the rendered figures, are what confirm the live fetch actually ran.