fix(device): read UIKit traits on the main thread and cache them - #497
fix(device): read UIKit traits on the main thread and cache them#497yusuftor wants to merge 7 commits into
Conversation
`interfaceStyle`, `fontSize`, `fontScale` and `preferredContentSizeCategory` read `UIScreen.traitCollection`, `UIFontMetrics` and `UIApplication.preferredContentSizeCategory` directly. Those are main-thread only, but the getters are called from background contexts such as `getTemplateDevice()`, which trips the Main Thread Checker. Reads them once on the main thread into a cached `UITraits` value, refreshed on `UIContentSizeCategory.didChangeNotification` and `UIApplication.didBecomeActiveNotification` so the values still track the user changing their text size or appearance. Observers are torn down in `deinit`. Extracts `interfaceStyleToken(for:)` alongside the existing `contentSizeCategoryToken(for:)` so both dashboard token mappings are directly testable — these strings are a backend audience-filter contract. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t stick `UIContentSizeCategory.didChangeNotification` and `didBecomeActiveNotification` cover a change made via Settings or Control Center, since both cycle the app through resign/become-active. They miss a system appearance change that lands while the app stays active — the automatic light/dark switch at sunset — leaving the cache reporting the previous token until the next foreground, which feeds the wrong value into request headers and audience filters. Reads now schedule a main-thread refresh, so the cache trails a change by one read rather than indefinitely. A test-and-set guard keeps a burst of reads to a single main-thread hop, and the read itself stays non-blocking. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Important
Moving these reads off the hot path is the right call, but swapping a live read for a cache narrows when interfaceStyle is correct. The two observed notifications don't cover a system appearance flip that happens while the app stays active, so X-Device-Interface-Style and the interfaceStyle device attribute can report the previous value until the next activation. Worth resolving before merge.
Reviewed changes — full review of the single commit afd9c05 (3 files) against develop.
- Four trait reads replaced with a cached
UITraits—interfaceStyle,fontSize,fontScaleandpreferredContentSizeCategorynow return values captured from a@DispatchQueueBackedcache instead of touchingUIScreen.traitCollection/UIFontMetrics/UIApplication.preferredContentSizeCategoryon whatever thread the getter runs on. - Cache populated in
initon the main thread —makeUITraits()uses the sameThread.isMainThread ? read() : DispatchQueue.main.sync(...)shape as the pre-existingmakeScreenMetrics()in the same initializer, and the wrapper assignment ininitcorrectly lowers to direct backing-storage construction rather than the wrapper'squeue.asyncsetter, so the initial value can't be lost. - Cache refreshed via two notification observers —
UIContentSizeCategory.didChangeNotificationandUIApplication.didBecomeActiveNotification, both delivered on.main, removed indeinit. The observer blocks captureselfweakly, so no retain cycle. interfaceStyleToken(for:)extracted as an internalstaticalongside the existingcontentSizeCategoryToken(for:), making both backend-contract token maps directly testable.- Three new tests — exhaustive
interfaceStyleTokenmapping, an off-main read of all four values, and a main-thread check that the cache holds the device's real values rather than the visionOS placeholders. - Numeric behaviour preserved —
fontSizeandfontScalecompute identically to the code they replace, from a singlescaledValue(for: 16.0)call.
ℹ️ Nothing exercises the cache-invalidation path
The new tests cover the token maps and the freshly-populated cache, but observeUITraitChanges() — the part that can silently stop working, and the reason the cached values stay correct at all — has no coverage. It also isn't reachable from a test as written: makeUITraits() is private static and the only refresh entry point is a NotificationCenter block.
Technical details
# `observeUITraitChanges()` is untested and untestable as written
## Affected sites
- `Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift:259-275` — `observeUITraitChanges()`; the refresh is only reachable by posting a real notification.
- `Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift:234` — `makeUITraits()` is `private static`, so a test can't drive a refresh directly.
- `Tests/SuperwallKitTests/Network/DeviceHelperTests.swift:83-131` — the three new tests only touch the token maps and the post-`init` cache.
## Required outcome
- A regression that breaks the refresh (observer never registered, wrong notification name, refresh reading from the wrong source) fails a test rather than shipping silently.
## Suggested approach (optional)
- Extract an internal `refreshUITraits()` that assigns `uiTraits = Self.makeUITraits()`, have `observeUITraitChanges()` call it, and assert in a test that after calling it the four values still equal the live UIKit values (the assertions `traits_matchTheCurrentTraitValues` already makes). That pins the refresh source without depending on notification delivery.
- Avoid asserting only "the value is unchanged after posting the notification" — that passes even if the refresh is a no-op.Claude Opus | 𝕏
There was a problem hiding this comment.
ℹ️ Two rough edges in the new refresh-on-read mechanism — neither blocking.
Reviewed changes — incremental review of 815dc10 only, the response to the interface-style staleness finding on afd9c05.
- Reads now trigger a refresh — the four getters go through a new private
currentUITraits, which callsrefreshUITraits()and returns the previously cached value; the notification observers are kept alongside it. - Refreshes are coalesced —
guard !$isRefreshingUITraits.testAndSetTrue()gates a singleDispatchQueue.main.asynchop per burst of reads. The latch can't be lost:testAndSetTrue()isqueue.syncand the reset isqueue.asyncon the same private serial queue, so FIFO ordering guarantees a latertestAndSetTrue()observes the reset. The UIKit read still happens on the main thread, so the original Main Thread Checker fix is intact. - One new test —
traits_stayCorrectAfterRepeatedReadsTriggerRefreshesdrives five off-main read/refresh cycles and re-checks the four values against live UIKit values. - Doc comments updated to describe the new "refreshed on notifications and on read" behaviour.
Claude Opus | 𝕏
…t they happen Refresh-on-read only narrowed the staleness window: the first read after an unobserved appearance flip still reported the old token, and with no traffic between the flip and a paywall presentation that read is the `getTemplateDevice()` call feeding audience filters. There is no notification for `userInterfaceStyle`, so this registers for trait changes on the active window (iOS 17+) and updates the cache at the moment of the flip. Registration needs a window, which may not exist when `DeviceHelper` is built, so it retries on activation and no-ops once registered. Refresh-on-read stays as the backstop for iOS 13-16 and for the window between launch and registration. It now takes an inline fast path on the main thread — the wrapper's setter and getter share one serial queue, so the write is ordered ahead of the read and main-thread callers see the live value. Also strengthens `traits_areReadableOffTheMainThread`, whose assertions passed against the `UITraits.unavailable` placeholders and so only proved the off-main read didn't hang. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Important
The trait hook is the right mechanism for closing the staleness gap, but it observes a UIWindow — an object whose lifetime it doesn't control, and whose userInterfaceStyle the host app can pin. In both of those cases it silently never fires for the sunset appearance flip it was added to catch, and the one-shot guard means it can't recover.
Reviewed changes — incremental review of 2c3ee93, the response to the two rough edges raised on 815dc10.
- Main-thread reads refresh inline —
refreshUITraits()takes aThread.isMainThreadfast path that assignsuiTraitsdirectly and returns, so main-thread callers (TestModeDeviceAttributesViewController, a main-threadSuperwall.getDeviceAttributes()) see the live value instead of trailing by one read. The latch is bypassed on that path and can't be left stuck: any block that would clear it is already enqueued on the main queue. - Added an iOS 17+ trait-change hook —
registerForTraitChanges()hops throughTask { @MainActor }toperformTraitChangeRegistration(), which registers[UITraitUserInterfaceStyle.self, UITraitPreferredContentSizeCategory.self]onUIApplication.sharedApplication?.activeWindowand refreshes the cache from the handler. This is what updates the cache at the moment of a flip rather than on the next read. - Registration is retried from the notification handlers — a window may not exist when
DeviceHelperis constructed, so the observer block callsregisterForTraitChanges()again;traitChangeRegistration == nilmakes it a no-op once registered. - Off-main read test tightened —
traits_areReadableOffTheMainThreadnow compares all four detached-task values against freshly read UIKit values instead ofisEmpty == false/> 0, so the visionOS placeholders no longer satisfy it. - Doc comments rewritten to describe the inline-vs-scheduled refresh split and to name the trait hook as the primary path with refresh-on-read as the backstop.
ℹ️ One cached value now has four refresh paths, two of them untested
init, the two notification observers, refresh-on-read, and the trait hook all write uiTraits, with overlapping coverage — UITraitPreferredContentSizeCategory in the hook duplicates UIContentSizeCategory.didChangeNotification, and refresh-on-read partly duplicates both. Nothing asserts that either the observers or the registration are wired up at all, so the two paths that can silently stop working are exactly the two with no test.
Technical details
# Refresh-path redundancy and coverage
## Affected sites
- `Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift:338-353` — `observeUITraitChanges()`; `UIContentSizeCategory.didChangeNotification` is now also covered by `UITraitPreferredContentSizeCategory` in the hook. `didBecomeActive` still earns its place (pre-iOS-17 refresh + registration retry).
- `Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift:267-292` — the registration path has no test.
- `Tests/SuperwallKitTests/Network/DeviceHelperTests.swift:83-175` — the four tests cover the token maps, the post-`init` cache and the read-triggered refresh only.
## Required outcome
- A regression that breaks a refresh path (observer never registered, registration never armed, refresh reading from the wrong source) fails a test rather than shipping silently.
- Each retained refresh path is retained deliberately, not accumulated.
## Suggested approach (optional)
- Make the refresh entry point internal and assert that after calling it the four values still equal the live UIKit values — that pins the refresh source without depending on UIKit delivery. Injecting the trait environment (default `activeWindow`) would additionally make the registration assertable.
- If the notification observers are being kept purely as the pre-iOS-17 path, say so in the doc comment so a later reader doesn't delete one as redundant.ℹ️ Nitpicks
registerForTraitChanges()'s doc comment says the retry runs "on activation", but the same handler is installed forUIContentSizeCategory.didChangeNotificationtoo, so a text-size change also retries. Both notification handlers already run on.main, so the extraTask { @MainActor }hop there only defers a call that could be made directly.
Claude Opus | 𝕏
Observing the active window was wrong in two ways. The window is a different trait environment from the one the cache reads. A window pinned via `overrideUserInterfaceStyle` reports no change in that trait when the system flips, so the handler never fired even though `UIScreen.main` — what `makeUITraits()` actually reads — had changed. Overrides propagate down the trait hierarchy, not up, so the scene's `userInterfaceStyle` follows the system the way the screen does. Adds `UIApplication.activeWindowScene` for it, mirroring `activeWindow`'s activation-state priority. `activeWindow` is also computed per call with no identity guarantee, and UIKit removes a registration when the object that created it deallocates. The `traitChangeRegistration == nil` guard then read a dead registration as a live one and never re-armed. The observed scene is now held weakly, and a non-nil token with a nil scene counts as needing re-arming, which the existing activation retry picks up. Scenes also outlive the windows they host, so this arises less often to begin with. No `deinit` unregister: UIKit cleans registrations up at end of lifecycle. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The refresh is enqueued on the main queue before the hop, so hopping to the main actor drains it without a sleep. Removes five 50ms sleeps and the timing dependency they carried. Also scopes the docstring to what the test actually pins: nothing here can move `UIScreen.main`'s trait collection, so it proves the refresh keeps the cache correct, not that a flip is picked up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
ℓ No new issues of substance — one minor suggestion inline on the new scene helper.
Reviewed changes — incremental review of dad046b, the response to the [!IMPORTANT] trait-hook finding on 2c3ee93.
- The trait hook now observes the
UIWindowScene, not aUIWindow—performTraitChangeRegistration()registers[UITraitUserInterfaceStyle.self, UITraitPreferredContentSizeCategory.self]onactiveWindowScene. This is the right target: window-leveloverrideUserInterfaceStyledoesn't propagate up to the scene, so the scene'suserInterfaceStyletracks the system the wayUIScreen.main.traitCollection— the valuemakeUITraits()actually reads — does. VerifiedUIWindowSceneconforms toUITraitChangeObservable(conforming-types list), so the registration is valid. - Registration is re-armable —
private weak var observedTraitSceneplus thetraitChangeRegistration != nil && observedTraitScene != nilguard replaces the one-shot== nilcheck, so losing the observed scene re-registers instead of disabling the hook for the process lifetime. The re-arm is reachable in the case that motivated it: Apple'ssceneDidBecomeActive(_:)docs state "UIKit posts aUIScene.didActivateNotificationand aUIApplication.didBecomeActiveNotification", so a multi-window app whose observed scene disconnects while the app stays active still drives the existing observer. - New
UIApplication.activeWindowScenehelper — mirrorsactiveWindow's three-tier activation-state priority at the scene level. Written standalone rather than asactiveWindow?.windowScene, which reads like duplication but isn't:activeWindowreturnsniloutright when the foreground-active scene has no windows yet, which is exactly the early-startup stateDeviceHelper.initruns in. - No behavioural change to the cache itself — the four getters,
makeUITraits(), the refresh-on-read latch and the notification observers are untouched, as is the test file.
ℓ Nitpicks
observeUITraitChanges()'s doc comment (DeviceHelper.swift:344-345) still says the activation retry helps because "a window is more likely to exist by then" — the registration target is a scene now.
Claude Opus | 𝕏
…le cache Caching these values regressed freshness on versions with no trait hook. Before this branch they were read live on every access, so `X-Device-Interface-Style` and the audience filters keyed on it always saw the current appearance. On iOS 13-16 nothing invalidates the cache when the appearance flips while the app stays active, so the first read after a flip reported the previous style — a regression against develop, not a pre-existing gap. Those versions now read live. `makeUITraits()` already hops to the main thread itself, so this keeps the threading fix that motivated the branch: the read is on the main thread rather than the undefined-behaviour background access it replaced. iOS 17+ keeps the cache, kept current by the scene registration, with the scheduled refresh covering the gap between launch and registration. Trade-off: a blocking main-queue hop per read before iOS 17, and with it a deadlock surface if the main thread is ever blocked waiting on the thread doing the read. Judged the better side of reporting a wrong appearance to audience filters. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Important
Reading live on iOS 13–16 does close the staleness gap, but it turns every access into a blocking DispatchQueue.main.sync — four per getTemplateDevice() and one per network request, from async code on the cooperative thread pool. Worth collapsing before merge.
Reviewed changes — incremental review of ae03842, b805a17 and b6350d9, the three commits since the dad046b review.
- Pre-iOS-17 reads bypass the cache entirely —
currentUITraitsnow returnsDeviceHelper.makeUITraits()underguard #available(iOS 17.0, *) else, so iOS 13–16 gets a live read on every access instead of the cached value plus a scheduled refresh. This does close the "one stale read after a sunset appearance flip" gap on exactly the versions that have no trait hook, and the Main Thread Checker fix is intact —makeUITraits()still performs the UIKit reads on the main thread. - iOS 17+ path unchanged — still
refreshUITraits()followed by the cacheduiTraits, with the scene trait registration keeping the cache current. - Doc comments record the iOS 13–16 rationale (
ae03842) — why atraitCollectionDidChangeview isn't injected into the host's window to reach earlier releases. - Test determinism (
b805a17) —traits_stayCorrectAfterRepeatedReadsTriggerRefreshesswaps five 50 msTask.sleeps forawait MainActor.run {}. The refresh is aDispatchQueue.main.asyncblock enqueued before the hop, so the main-actor job drains it in FIFO order; the docstring now states plainly what the test can and can't pin, and the suite drops 250 ms.
ℹ️ On iOS 13–16 the trait cache is now write-only
Once currentUITraits short-circuits to a live read, nothing below iOS 17 ever reads uiTraits. init populates it, the two notification observers keep rewriting it, refreshUITraits() and isRefreshingUITraits are unreachable, and registerForTraitChanges() spawns a Task { @MainActor } per notification only to find its #available check false. The runtime cost is negligible; the comprehension cost of five write paths, four of them inert on the older half of the supported range, is not.
Technical details
# The trait cache and three of its refresh paths are inert below iOS 17
## Affected sites
- `Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift:214-215` — `@DispatchQueueBacked uiTraits` has no reader below iOS 17; the `init` write at `:755` and the observer writes are discarded there.
- `Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift:219-220` and `:252-268` — `isRefreshingUITraits` and `refreshUITraits()` are only reachable from the iOS 17+ branch of `currentUITraits`, so the coalescing latch is never exercised below 17.
- `Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift:358-373` — `observeUITraitChanges()` installs two main-queue observers that recompute and store `uiTraits` on every `didBecomeActive` and every content-size change. Below iOS 17 both the write and the `registerForTraitChanges()` retry are no-ops.
- `Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift:208-213` — the `uiTraits` doc comment still presents the cache as *the* read path ("read on the main thread and cached, then refreshed on trait-change notifications and on read"), which has been true only from iOS 17 since `b6350d9`.
## Required outcome
- A reader can tell which mechanism is load-bearing on which OS range without deriving it from the `#available` branch in `currentUITraits`.
- Code that provably cannot affect behaviour on a supported OS version is either gated out on that version or documented as deliberately inert.
## Open questions for the human
- Is the cache meant to stay as a single code path that happens to be dead below 17, or should the two modes be separated explicitly? The answer decides whether the observers stay unconditional.Claude Opus | 𝕏
| private var currentUITraits: UITraits { | ||
| guard #available(iOS 17.0, *) else { | ||
| return DeviceHelper.makeUITraits() | ||
| } | ||
| refreshUITraits() | ||
| return uiTraits | ||
| } |
There was a problem hiding this comment.
On iOS 13–16 this serves a live read, and makeUITraits() reaches UIKit through DispatchQueue.main.sync whenever the caller isn't already on the main thread. getTemplateDevice() reads all four properties as separate accesses, so it makes four blocking main-thread round trips per call, and makeHeaders makes one per network request — both from async functions running on the cooperative thread pool, which is sized to the core count and does not grow when its threads block.
Technical details
# Pre-iOS-17 reads block the calling thread on the main queue, once per property
## Affected sites
- `Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift:245-247` — the pre-iOS-17 branch returns `DeviceHelper.makeUITraits()`, so no read is served from cache.
- `Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift:348` — `makeUITraits()` ends in `DispatchQueue.main.sync(execute: read)` for every non-main caller.
- `Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift:820-823` — `getTemplateDevice()` reads `interfaceStyle`, `fontSize`, `fontScale` and `preferredContentSizeCategory` independently: four round trips per call (three when `interfaceStyleOverride` is set).
- `Sources/SuperwallKit/Dependencies/DependencyContainer.swift:441` — `makeHeaders` reads `interfaceStyle` for `X-Device-Interface-Style`, so one round trip per network request, via `EndpointKind.swift:49` and `:72`.
- `Sources/SuperwallKit/Config/ConfigManager.swift:661-663` — `preloadPaywalls(withIdentifiers:)` adds one child task per identifier inside a `withTaskGroup`, each preparing headers. N cooperative-pool threads can sit in `main.sync` simultaneously, stalling unrelated `Task` work in the host process until the main run loop drains. Config load and refresh trigger this at launch (`ConfigManager.swift:135`), when the main thread is at its busiest.
## Required outcome
- A single `getTemplateDevice()` on iOS 13–16 performs at most one main-thread round trip, not one per property.
- The doc comment says the hop is *blocking*. "`makeUITraits()` makes the main-thread hop itself" reads as an implementation detail rather than "every off-main read waits for the main queue".
## Suggested approach (optional)
- Read `currentUITraits` once in `getTemplateDevice()` and take all four values from that snapshot, applying the `interfaceStyleOverride` short-circuit to the `interfaceStyle` field explicitly.
- Alternatively, if the one-read lag `815dc10` had is acceptable on iOS 13–16 — it self-heals on the next read, on `didBecomeActive`, and on any content-size change — keeping the cached path there removes the blocking entirely.
## Open questions for the human
- Is iOS 13–16 traffic material enough to pay a per-request main-thread round trip? The alternative is a bounded, self-healing one-read lag on `X-Device-Interface-Style`.| /// iOS 17+ only. Earlier versions deliberately fall back to the notifications and | ||
| /// refresh-on-read rather than injecting a view into the host's window to reach | ||
| /// `traitCollectionDidChange`, leaving them correct after one read. |
There was a problem hiding this comment.
This describes 815dc10's behaviour, not the current one: since b6350d9, earlier releases don't fall back to the notifications or to refresh-on-read at all — refreshUITraits() is unreachable below iOS 17, and they're correct on every read rather than "after one read".
| /// iOS 17+ only. Earlier versions deliberately fall back to the notifications and | |
| /// refresh-on-read rather than injecting a view into the host's window to reach | |
| /// `traitCollectionDidChange`, leaving them correct after one read. | |
| /// iOS 17+ only. Earlier releases read live on every access instead, rather than | |
| /// injecting a view into the host's window to reach `traitCollectionDidChange`. |

Changes in this pull request
interfaceStyle,fontSize,fontScaleandpreferredContentSizeCategoryreadUIScreen.traitCollection,UIFontMetricsandUIApplication.preferredContentSizeCategorydirectly. Those APIs are main-thread only, but the getters are called from background contexts such asgetTemplateDevice(), which trips the Main Thread Checker.UITraits, refreshed onUIContentSizeCategory.didChangeNotificationandUIApplication.didBecomeActiveNotificationso they still track the user changing their text size or appearance. Observers are removed indeinit.interfaceStyleToken(for:)next to the existingcontentSizeCategoryToken(for:), so both dashboard token mappings are directly testable — these strings are a backend audience-filter contract and must not drift.interfaceStyleTokenmapping, reading all four traits off the main thread, and asserting the cache holds the device's real values rather than the visionOS placeholders.Checklist
CHANGELOG.mdfor any breaking changes, enhancements, or bug fixes.swiftlintin the main directory and fixed any issues.Unchecked boxes are not verified rather than not applicable — only the unit tests (896 passing on iPhone 17 Pro / iOS 26.5), the added tests, the changelog entry and swiftlint were actually run. The demo projects and UI tests weren't exercised. No public API changed, so no doc updates were needed.
🤖 Generated with Claude Code
Greptile Summary
This revision completes the cached UIKit-trait implementation.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR Read["Background device-trait read"] --> Cache["Return cached UITraits"] Read --> Refresh["Schedule main-thread refresh"] Appearance["iOS 17+ scene trait change"] --> Refresh Lifecycle["Content-size or app-active notification"] --> Refresh Refresh --> UIKit["Read UIKit traits on main thread"] UIKit --> CacheReviews (3): Last reviewed commit: "fix(device): read traits live before iOS..." | Re-trigger Greptile
Context used: