From a9103f8f8798d43e89c0796af5ab0e5d15d6b7b6 Mon Sep 17 00:00:00 2001 From: Ryanmello07 <67509637+Ryanmello07@users.noreply.github.com> Date: Mon, 7 Sep 2026 21:54:57 -0700 Subject: [PATCH 1/3] stop the widget chart drawing past its data, and add a refresh button Three defects hid behind one report of "the widgets don't update". The chart was the worst of them, and it was not staleness but a false statement. Every entry in a timeline carries the SAME snapshot at a later date, and the throughput window was anchored to the entry's date, so the plot walked past the newest bucket the tunnel had published. Out there `byStart[bucket] ?? 0` stops meaning "no traffic" and starts meaning "not measured yet" -- drawn as a flat zero line across up to a quarter of the plot while traffic was flowing. Anchor the window to the snapshot's own clock instead, clamped so a future change to the entry count degrades to frozen-but-true rather than collapsing again. The freshness label under it was derived from the entry date too, so it stepped in five-minute jumps and then froze for whatever remained of the interval, under-reporting the age of what was on screen. It becomes a relative Text, the one element WidgetKit advances without a reload, bound to the snapshot's age rather than the render's. The cadence was over-subscribed rather than too slow. The timeline asked every 20 minutes (72 a day) while the extension asked every 15 (96 a day) against a budget of roughly 40-70 -- and the per-reason throttles let one kind be asked for far more often still: 720 a day for the globe, 480 for contracts. The two clocks do not add up, because every reload re-arms the timeline's `.after(...)`; the faster one wins and the slower one's budget is spent for nothing. Collapse every number into one WidgetRefreshPolicy with the arithmetic written down, make the throttles per-kind, and make the extension's a backstop that is deliberately slower than the policy. That buys honesty, not speed. The freshness a user feels comes from the two paths that are not charged: a reload from an in-widget intent, and one requested while the app is in the foreground. So add both -- a refresh button on all three widgets that asks the tunnel to publish and waits briefly for the write, and a reload on the app's foreground transition, which nothing on any app lifecycle path was doing. The request crosses as a file plus a Darwin notification, because notifications are not queued for a suspended process and this extension is expected to be suspended; the writer serves a dropped one on its next tick. The button cannot fake success: with the tunnel down there is no writer, so the label keeps counting up. A live graph remains impossible. WidgetKit archives the views at timeline-build time and only date-style Texts move between reloads. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01AWV4MaF9JBcQppSX9UxATa --- app/extension/WidgetSnapshotWriter.swift | 126 +++++++++++++----- app/network/NetworkApp.swift | 12 ++ .../Shared/ViewModels/VPNManager.swift | 13 +- .../Shared/Widgets/WidgetRefresh.swift | 60 +++++++++ .../Shared/Widgets/WidgetSnapshots.swift | 67 ++++++++++ .../WidgetRefreshPolicyTests.swift | 74 ++++++++++ .../WidgetReloadThrottleTests.swift | 116 ++++++++++++++++ app/widgets/Contracts/ContractsWidget.swift | 4 + app/widgets/Dashboard/DashboardView.swift | 51 ++++--- app/widgets/Globe/ProviderGlobeWidget.swift | 1 + app/widgets/Shared/RefreshWidgetsIntent.swift | 112 ++++++++++++++++ .../Shared/WidgetSnapshotTimeline.swift | 32 ++--- 12 files changed, 598 insertions(+), 70 deletions(-) create mode 100644 app/networkTests/WidgetRefreshPolicyTests.swift create mode 100644 app/networkTests/WidgetReloadThrottleTests.swift create mode 100644 app/widgets/Shared/RefreshWidgetsIntent.swift diff --git a/app/extension/WidgetSnapshotWriter.swift b/app/extension/WidgetSnapshotWriter.swift index dbbe8025..0314316e 100644 --- a/app/extension/WidgetSnapshotWriter.swift +++ b/app/extension/WidgetSnapshotWriter.swift @@ -27,16 +27,22 @@ final class WidgetSnapshotWriter { /// they move like the real widgets would if WidgetKit re-rendered that /// often. Back to `writeInterval` when the mark clears or expires. static let previewWriteInterval: TimeInterval = 2 - /// Routine widget reload cadence while the tunnel is up. WidgetKit - /// budgets roughly 40-70 reloads a day per widget instance. - static let routineReloadInterval: TimeInterval = 15 * 60 - /// Minimum spacing for globe reloads driven by providers joining or - /// leaving, so a churning window cannot burn the budget. - static let providerReloadInterval: TimeInterval = 2 * 60 - /// Minimum spacing for contracts-widget reloads driven by peers or - /// contracts appearing and closing; byte counts and rates ride the - /// routine cadence. - static let contractReloadInterval: TimeInterval = 3 * 60 + /// The floor between reloads of any ONE widget kind, whatever asked for + /// it. Deliberately slower than the timeline's own policy + /// (`WidgetRefreshPolicy`): this is a backstop filling a gap the policy + /// left, not a second clock racing it -- every reload re-arms the + /// timeline's `.after(...)`, so a faster second requester buys nothing and + /// spends the same budget. + /// + /// These used to be per-REASON rather than per-kind, which let one kind be + /// asked for far more often than the budget allows: providers joining and + /// leaving drove the globe every 2 minutes (720 a day) and contract + /// churn drove the contracts widget every 3 (480 a day), against the + /// roughly 40-70 a day the comments alongside them cited. + static let reloadBackstopInterval: TimeInterval = WidgetRefreshPolicy.extensionBackstopInterval + /// A refresh request from a widget publishes at most this often, so a + /// user tapping repeatedly cannot drive the write path. + static let refreshRequestFloor: TimeInterval = 2 /// Contract change events arrive per contract, about once a second while /// bytes move; the two lists are re-read at most this often. static let contractRefreshInterval: TimeInterval = 2 @@ -65,17 +71,22 @@ final class WidgetSnapshotWriter { private var lastWritten: WidgetTunnelSnapshot? /// Mirrors WidgetPreviewVisibility; owned by `queue`. private var previewVisible = false - /// The writer the process-wide Darwin observer forwards to. + /// The writer the process-wide Darwin observers forward to. private static weak var current: WidgetSnapshotWriter? private static var previewObserverRegistered = false + private static var refreshObserverRegistered = false + /// When a widget's refresh request was last served; owned by `queue`. + private var lastRefreshWriteAt: Date? private var contracts = ContractTracker() private var contractRefreshPending = false private var lastContractRefreshAt: Date? - private let routineReload: WidgetReloadThrottle - private let providerReload: WidgetReloadThrottle - private let contractReload: WidgetReloadThrottle + // one throttle per widget kind, so every reason to reload shares that + // kind's budget instead of each keeping its own + private let dashboardReload: WidgetReloadThrottle + private let globeReload: WidgetReloadThrottle + private let contractsReload: WidgetReloadThrottle init(device: SdkDeviceLocal, logger: Logger) { self.device = device @@ -85,15 +96,13 @@ final class WidgetSnapshotWriter { self.accumulator = WidgetThroughputAccumulator( resuming: WidgetSnapshotStore.loadTunnel()?.throughput ?? .empty ) - self.routineReload = WidgetReloadThrottle(interval: Self.routineReloadInterval) { + self.dashboardReload = WidgetReloadThrottle(interval: Self.reloadBackstopInterval) { WidgetRefresh.reloadDashboard() - WidgetRefresh.reloadProviderGlobe() - WidgetRefresh.reloadContracts() } - self.providerReload = WidgetReloadThrottle(interval: Self.providerReloadInterval) { + self.globeReload = WidgetReloadThrottle(interval: Self.reloadBackstopInterval) { WidgetRefresh.reloadProviderGlobe() } - self.contractReload = WidgetReloadThrottle(interval: Self.contractReloadInterval) { + self.contractsReload = WidgetReloadThrottle(interval: Self.reloadBackstopInterval) { WidgetRefresh.reloadContracts() } } @@ -132,8 +141,8 @@ final class WidgetSnapshotWriter { guard snapshot != self.location else { return } self.location = snapshot self.write() - WidgetRefresh.reloadDashboard() - self.providerReload.request(urgent: true) + self.dashboardReload.request(urgent: true) + self.globeReload.request(urgent: true) } }) { subs.append(sub) @@ -143,7 +152,7 @@ final class WidgetSnapshotWriter { guard let self, self.active, self.providing != provideEnabled else { return } self.providing = provideEnabled self.write() - self.routineReload.request(urgent: true) + self.dashboardReload.request(urgent: true) } }) { subs.append(sub) @@ -153,7 +162,7 @@ final class WidgetSnapshotWriter { guard let self, self.active, self.provideMode != mode else { return } self.provideMode = mode self.write() - self.routineReload.request(urgent: true) + self.dashboardReload.request(urgent: true) } }) { subs.append(sub) @@ -167,7 +176,7 @@ final class WidgetSnapshotWriter { self.write() // the globe follows providers joining and leaving, like the // app's provider details view, within the reload budget - self.providerReload.request() + self.globeReload.request() } }) { subs.append(sub) @@ -228,8 +237,17 @@ final class WidgetSnapshotWriter { // the app died with the previews open: the mark expired self.applyPreviewVisibility() } + // a darwin notification is not queued for a suspended + // process, so a tap made while this one was frozen is served + // here instead of being lost + if WidgetSnapshotRefreshRequest.consume() { + self.serveRefreshRequest() + return + } if self.write() { - self.routineReload.request() + self.dashboardReload.request() + self.globeReload.request() + self.contractsReload.request() } } writeTimer.resume() @@ -237,6 +255,7 @@ final class WidgetSnapshotWriter { Self.current = self Self.registerPreviewObserver() + Self.registerRefreshObserver() applyPreviewVisibility() let balanceTimer = DispatchSource.makeTimerSource(queue: queue) @@ -256,7 +275,10 @@ final class WidgetSnapshotWriter { queue.async { [self] in guard active else { return } write() - WidgetRefresh.reloadAll() + WidgetRefresh.reloadControl() + dashboardReload.request(urgent: true) + globeReload.request(urgent: true) + contractsReload.request(urgent: true) } } @@ -294,9 +316,9 @@ final class WidgetSnapshotWriter { writeTimer = nil balanceTimer?.cancel() balanceTimer = nil - routineReload.cancel() - providerReload.cancel() - contractReload.cancel() + dashboardReload.cancel() + globeReload.cancel() + contractsReload.cancel() } // MARK: Preview visibility @@ -318,6 +340,47 @@ final class WidgetSnapshotWriter { ) } + // MARK: Refresh requests + + /// One Darwin observer per process, the same shape as the preview one. + private static func registerRefreshObserver() { + guard !refreshObserverRegistered else { return } + refreshObserverRegistered = true + CFNotificationCenterAddObserver( + CFNotificationCenterGetDarwinNotifyCenter(), + nil, + { _, _, _, _, _ in + WidgetSnapshotWriter.current?.snapshotRefreshRequested() + }, + WidgetSnapshotRefreshRequest.darwinNotificationName as CFString, + nil, + .deliverImmediately + ) + } + + private func snapshotRefreshRequested() { + queue.async { [weak self] in + guard let self, self.active else { return } + guard WidgetSnapshotRefreshRequest.consume() else { return } + self.serveRefreshRequest() + } + } + + /// Publish now, for a widget that is waiting on it. + /// + /// Deliberately asks for NO reload: the widget process already requested + /// one, and a reload caused by an in-widget intent is not charged against + /// the budget while one requested from here is. Contracts are re-read + /// first so the one write carries a current membership rather than the + /// last cached one. + private func serveRefreshRequest() { + let elapsed = lastRefreshWriteAt.map { Date().timeIntervalSince($0) } ?? .infinity + guard Self.refreshRequestFloor <= elapsed else { return } + lastRefreshWriteAt = Date() + refreshContracts() + write() + } + private func previewVisibilityChanged() { queue.async { [weak self] in self?.applyPreviewVisibility() @@ -370,7 +433,7 @@ final class WidgetSnapshotWriter { write() // peers and contracts coming and going is what the contracts // widget shows; rates and byte counts ride the routine cadence - contractReload.request() + contractsReload.request() } } @@ -413,7 +476,8 @@ final class WidgetSnapshotWriter { isPro: result.currentSubscription != nil ) if WidgetSnapshotStore.save(snapshot) { - self.routineReload.request() + // the balance bar is the dashboard's alone + self.dashboardReload.request() } } } diff --git a/app/network/NetworkApp.swift b/app/network/NetworkApp.swift index 637465da..19f96b9b 100644 --- a/app/network/NetworkApp.swift +++ b/app/network/NetworkApp.swift @@ -325,6 +325,12 @@ struct NetworkApp: App { .onChange(of: scenePhase) { phase in setPresentationActive(phase == .active) if phase == .active { + // a reload requested while the app is in the + // foreground is not charged against the widget budget, + // and nothing else on any app lifecycle path reloads + // them -- so opening the app is the reliable way to + // un-stick a widget the system has been deferring + WidgetRefresh.reloadAll() refreshJwtOnForeground() } } @@ -381,6 +387,12 @@ struct NetworkApp: App { .onChange(of: scenePhase) { phase in setMacPresentationActive(sceneActive: phase == .active) if phase == .active { + // a reload requested while the app is in the + // foreground is not charged against the widget budget, + // and nothing else on any app lifecycle path reloads + // them -- so opening the app is the reliable way to + // un-stick a widget the system has been deferring + WidgetRefresh.reloadAll() refreshJwtOnForeground() } } diff --git a/app/network/Shared/ViewModels/VPNManager.swift b/app/network/Shared/ViewModels/VPNManager.swift index d9542514..ee40ce85 100644 --- a/app/network/Shared/ViewModels/VPNManager.swift +++ b/app/network/Shared/ViewModels/VPNManager.swift @@ -175,6 +175,9 @@ class VPNManager: ObservableObject { // Retain the live manager objects so their NEVPNConnection status updates // can be observed. The desired-state cache is not a health signal. private var observedVpnManagers: [NETunnelProviderManager] = [] + /// The last connection state the widgets were reloaded for, so the + /// several transitions one connect delivers cost one reload. + private var lastWidgetReloadState: VPNTunnelConnectionState? private var vpnStatusObservers: [NSObjectProtocol] = [] private var healthAuditWork: DispatchWorkItem? private var healthAuditDeadline: Date? @@ -961,8 +964,14 @@ class VPNManager: ObservableObject { let state = Self.tunnelConnectionState(connection.status) // the quick connect control and the widgets read NEVPNStatus when // rendered but are not told when it changes; every transition seen - // here re-renders them - WidgetRefresh.reloadAll() + // here re-renders them -- but only when the state they DRAW changed. + // One connect delivers connecting, connected and sometimes + // reasserting, and the widgets render those identically, so reloading + // per transition spent several of a budget of tens per day on one tap. + if state != lastWidgetReloadState { + lastWidgetReloadState = state + WidgetRefresh.reloadAll() + } // a toggle made from Control Center while the app is running arrives // here as a status change; fold the shared intent in before the // desired state is compared to it (only a pending outside intent diff --git a/app/network/Shared/Widgets/WidgetRefresh.swift b/app/network/Shared/Widgets/WidgetRefresh.swift index ee5bc4fd..4f71f8b2 100644 --- a/app/network/Shared/Widgets/WidgetRefresh.swift +++ b/app/network/Shared/Widgets/WidgetRefresh.swift @@ -23,6 +23,66 @@ import Foundation import WidgetKit +/// Every widget refresh cadence, in one place. +/// +/// These numbers used to live in three files that each restated the same +/// budget and then picked a number in isolation: the timeline asked for a +/// reload every 20 minutes (72 a day) while the tunnel extension's routine +/// throttle asked every 15 (96 a day), against the roughly 40-70 a day +/// WidgetKit allows one widget instance. The two clocks do not add up -- +/// every reload re-arms the timeline's `.after(...)`, so the faster one wins +/// and the slower one's budget is spent for nothing. Over-requesting is not +/// free: the system answers an over-subscribed budget with deferrals, which +/// is how a design asking twice per hour ended up refreshing less often than +/// either of its own numbers. +/// +/// 25 minutes is ~58 requests a day, inside the band with headroom for the +/// event-driven reloads a real day contains (connect, disconnect, location +/// change). The extension's backstop is deliberately SLOWER than the +/// timeline policy so it fills a gap the policy left rather than racing it. +/// +/// The freshness a user actually feels does not come from this clock. It +/// comes from the two paths that are not charged against the budget: a +/// reload caused by an in-widget intent (the refresh button), and a reload +/// requested while the app is in the foreground. +enum WidgetRefreshPolicy { + + /// Requested spacing between timeline reloads while the tunnel is up. + static let refreshIntervalWhileUp: TimeInterval = 25 * 60 + + /// While the tunnel is down there is no writer at all -- the snapshot + /// writer lives in the packet tunnel process and its timers are cancelled + /// on stop -- so no new snapshot can appear however often the widget + /// asks. The only thing that can change is NEVPNStatus, and every + /// transition already reloads from `VPNManager`. + static let refreshIntervalWhileDown: TimeInterval = 60 * 60 + + /// Entries re-render the same snapshot at later dates. Five minutes is + /// the spacing WidgetKit expects; it is the one constant here that is not + /// free to lower. + static let entrySpacing: TimeInterval = 5 * 60 + + /// WidgetKit archives every entry's rendered view up front, so entries + /// are not free -- the globe archives a full render each. Six covers the + /// tunnel-up policy exactly; the hour-long down policy is capped by it, + /// which costs nothing because with the tunnel down there is no writer, + /// nothing on screen is a function of the entry's date any more, and the + /// freshness label advances itself. + static let maxEntryCount = 6 + + /// Enough entries that the last one lands on the policy date. Four + /// entries five minutes apart covered only 15 minutes of a 20-minute + /// policy, so the final stretch of every cycle rendered an entry whose + /// date had already passed. + static func entryCount(covering interval: TimeInterval) -> Int { + min(maxEntryCount, max(1, Int((interval / entrySpacing).rounded(.down)) + 1)) + } + + /// The tunnel extension's routine reload floor. Slower than + /// `refreshIntervalWhileUp` on purpose: a backstop, not a second clock. + static let extensionBackstopInterval: TimeInterval = 30 * 60 +} + enum WidgetRefresh { /// The Control Center / Lock Screen / Action button toggle (iOS 18, diff --git a/app/network/Shared/Widgets/WidgetSnapshots.swift b/app/network/Shared/Widgets/WidgetSnapshots.swift index 470a8ebb..ef13f331 100644 --- a/app/network/Shared/Widgets/WidgetSnapshots.swift +++ b/app/network/Shared/Widgets/WidgetSnapshots.swift @@ -400,6 +400,73 @@ enum WidgetPreviewVisibility { } } +/// A widget asked for a fresh snapshot, now. +/// +/// Only the packet tunnel process holds the live counters, so the widget +/// process -- which can read the published file but cannot produce a newer +/// one -- signals across and waits briefly for the write to land. +/// +/// Deliberately NOT `WidgetPreviewVisibility`, which is the right shape and +/// the wrong channel: that mark carries an expiry because the extension needs +/// to know how long to keep writing fast, its handler returns early unless +/// the flag actually flipped (so a second tap inside the mark window would be +/// a silent no-op), and it asks for no reload at all. This asks for exactly +/// one write. +/// +/// The request is a file as well as a notification because Darwin +/// notifications are not queued for a suspended process, and this extension +/// is expected to be suspended. The file lets the writer serve a dropped +/// notification on its next timer tick instead of losing the tap; the window +/// keeps a tap made while the tunnel was down from causing a surprise write +/// when it next starts. +enum WidgetSnapshotRefreshRequest { + + static let darwinNotificationName = "network.ur.widgets.refresh-request" + static let fileName = "refresh-request.json" + /// How long a request stays worth serving. + static let requestWindow: TimeInterval = 30 + + private struct Request: Codable { + var at: Date + } + + /// Ask the tunnel to publish. Writes the request before posting, so a + /// notification that arrives first still finds it. + static func post(at date: Date = Date()) { + if let directory = WidgetSnapshotStore.directoryURL { + do { + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + let data = try WidgetSnapshotStore.encoder.encode(Request(at: date)) + try data.write(to: directory.appendingPathComponent(fileName), options: .atomic) + } catch { + // the notification below is still worth posting: a live + // extension serves it without reading the file + } + } + CFNotificationCenterPostNotification( + CFNotificationCenterGetDarwinNotifyCenter(), + CFNotificationName(darwinNotificationName as CFString), + nil, nil, true + ) + } + + /// True when a request inside the window is pending. The file is removed + /// either way, so a stale request cannot be served twice. + @discardableResult + static func consume(now: Date = Date()) -> Bool { + guard let url = WidgetSnapshotStore.directoryURL?.appendingPathComponent(fileName) else { + return false + } + let data = try? Data(contentsOf: url) + try? FileManager.default.removeItem(at: url) + guard let data, + let request = try? WidgetSnapshotStore.decoder.decode(Request.self, from: data) else { + return false + } + return now.timeIntervalSince(request.at) <= requestWindow + } +} + enum WidgetSnapshotStore { static let directoryName = "Widgets" diff --git a/app/networkTests/WidgetRefreshPolicyTests.swift b/app/networkTests/WidgetRefreshPolicyTests.swift new file mode 100644 index 00000000..386cafe5 --- /dev/null +++ b/app/networkTests/WidgetRefreshPolicyTests.swift @@ -0,0 +1,74 @@ +// +// WidgetRefreshPolicyTests.swift +// networkTests +// +// The widget refresh cadences, which until now were three numbers in three +// files with no assertion on any of them. +// +// Two of these encode decisions that are invisible at the call site and were +// each wrong in the shipped code: a timeline has to carry enough entries to +// reach its own policy date (four entries five minutes apart did not cover a +// twenty-minute policy), and the extension's backstop has to be SLOWER than +// the timeline policy or the two become competing clocks, which is how the +// widgets ended up asking for ~96 reloads a day against a budget of 40-70 +// and refreshing less often than either number alone would have given. +// + +import Testing +import Foundation +@testable import URnetwork + +struct WidgetRefreshPolicyTests { + + /// A day's worth of routine requests for one widget instance. + private func requestsPerDay(_ interval: TimeInterval) -> Double { + (24 * 60 * 60) / interval + } + + @Test func entriesReachThePolicyDate() { + let interval = WidgetRefreshPolicy.refreshIntervalWhileUp + let count = WidgetRefreshPolicy.entryCount(covering: interval) + // entries run 0, spacing, 2*spacing ... and the last one must land on + // (or past) the moment the next timeline is asked for, or the widget + // holds one render for the remainder of every cycle + let lastEntryOffset = Double(count - 1) * WidgetRefreshPolicy.entrySpacing + #expect(interval <= lastEntryOffset + WidgetRefreshPolicy.entrySpacing) + #expect(lastEntryOffset <= interval) + } + + /// The hour-long down policy is capped rather than covered: nothing on + /// screen is a function of the entry's date once the tunnel is down, and + /// every entry costs an archived render. + @Test func entriesAreCapped() { + let count = WidgetRefreshPolicy.entryCount(covering: WidgetRefreshPolicy.refreshIntervalWhileDown) + #expect(count == WidgetRefreshPolicy.maxEntryCount) + #expect(WidgetRefreshPolicy.entryCount(covering: WidgetRefreshPolicy.refreshIntervalWhileUp) + <= WidgetRefreshPolicy.maxEntryCount) + } + + /// The band the code's own comments cite for one widget instance. The + /// shipped 20-minute policy was 72 a day and failed this. + @Test func routineRequestsStayInsideTheBudget() { + let perDay = requestsPerDay(WidgetRefreshPolicy.refreshIntervalWhileUp) + #expect(40 <= perDay) + #expect(perDay <= 70) + } + + /// A backstop, not a second clock. If the extension asks faster than the + /// timeline policy it pre-empts it, spends the same budget and gains + /// nothing, because every reload re-arms the timeline's `.after(...)`. + @Test func theExtensionBackstopIsSlowerThanTheTimelinePolicy() { + #expect(WidgetRefreshPolicy.refreshIntervalWhileUp < WidgetRefreshPolicy.extensionBackstopInterval) + } + + /// WidgetKit's expected entry spacing: the one constant here that is not + /// free to lower. + @Test func entrySpacingIsAtLeastFiveMinutes() { + #expect(5 * 60 <= WidgetRefreshPolicy.entrySpacing) + } + + @Test func entryCountIsNeverZero() { + #expect(1 <= WidgetRefreshPolicy.entryCount(covering: 0)) + #expect(1 <= WidgetRefreshPolicy.entryCount(covering: 1)) + } +} diff --git a/app/networkTests/WidgetReloadThrottleTests.swift b/app/networkTests/WidgetReloadThrottleTests.swift new file mode 100644 index 00000000..0271750d --- /dev/null +++ b/app/networkTests/WidgetReloadThrottleTests.swift @@ -0,0 +1,116 @@ +// +// WidgetReloadThrottleTests.swift +// networkTests +// +// The coalescer every widget reload passes through. It is what keeps a +// chatty source -- providers joining and leaving, a counter ticking every +// second -- from spending a daily reload budget in a minute, and it had no +// coverage at all. +// +// The throttle fires on its own queue against the wall clock, so these wait +// on the counter reaching a value rather than on a fixed sleep having been +// long enough: a fixed sleep passes on an idle machine and fails on a busy +// one, which is the worst kind of test to leave in a suite. +// + +import Testing +import Foundation +@testable import URnetwork + +struct WidgetReloadThrottleTests { + + /// Short enough to keep the suite quick, long enough that a loaded + /// machine cannot cross it while a test is between two statements. + private static let interval: TimeInterval = 0.5 + + @Test func aBurstOfRoutineRequestsFiresOnce() async { + let counter = Counter() + let throttle = WidgetReloadThrottle(interval: Self.interval) { counter.increment() } + + // the first lands immediately (nothing has fired yet); the rest fall + // inside the window and collapse into one trailing fire + for _ in 0..<10 { + throttle.request() + } + #expect(await counter.reaches(1)) + await Self.settle() + + #expect(counter.value <= 2) + } + + @Test func urgentRequestsAreNotDeferred() async { + let counter = Counter() + let throttle = WidgetReloadThrottle(interval: 60) { counter.increment() } + + throttle.request(urgent: true) + throttle.request(urgent: true) + + #expect(await counter.reaches(2)) + } + + /// A routine request made right after an urgent one waits: `fire` stamps + /// the same clock either way, so an urgent reload still spaces the next. + @Test func anUrgentRequestSpacesTheNextRoutineOne() async { + let counter = Counter() + let throttle = WidgetReloadThrottle(interval: 60) { counter.increment() } + + throttle.request(urgent: true) + #expect(await counter.reaches(1)) + throttle.request() + await Self.settle() + + #expect(counter.value == 1) + } + + @Test func cancelDropsAPendingReload() async { + let counter = Counter() + let throttle = WidgetReloadThrottle(interval: Self.interval) { counter.increment() } + + // wait for the first fire before making the one that must be dropped, + // so "pending" is not a race with the first request still queued + throttle.request(urgent: true) + #expect(await counter.reaches(1)) + + throttle.request() + throttle.cancel() + await Self.settle() + + #expect(counter.value == 1) + } + + /// Long enough for a pending work item scheduled at `interval` to have + /// run if it was going to. + private static func settle() async { + try? await Task.sleep(nanoseconds: UInt64(interval * 4 * 1_000_000_000)) + } + + /// The throttle fires on its own queue, so the count needs its own lock. + private final class Counter: @unchecked Sendable { + private let lock = NSLock() + private var count = 0 + + func increment() { + lock.lock() + count += 1 + lock.unlock() + } + + var value: Int { + lock.lock() + defer { lock.unlock() } + return count + } + + /// True once the count reaches `target`; false if it never does. + func reaches(_ target: Int, timeout: TimeInterval = 5) async -> Bool { + let deadline = Date().addingTimeInterval(timeout) + while Date() < deadline { + if target <= value { + return true + } + try? await Task.sleep(nanoseconds: 20_000_000) + } + return false + } + } +} diff --git a/app/widgets/Contracts/ContractsWidget.swift b/app/widgets/Contracts/ContractsWidget.swift index 550555ac..c62071e4 100644 --- a/app/widgets/Contracts/ContractsWidget.swift +++ b/app/widgets/Contracts/ContractsWidget.swift @@ -83,6 +83,10 @@ struct ContractsView: View { .foregroundStyle(WidgetTheme.textMuted) } } + // unlike the globe, small uses this header too and has no room + if family != .systemSmall { + WidgetRefreshIcon() + } } } diff --git a/app/widgets/Dashboard/DashboardView.swift b/app/widgets/Dashboard/DashboardView.swift index 1abcb57e..b281c0b7 100644 --- a/app/widgets/Dashboard/DashboardView.swift +++ b/app/widgets/Dashboard/DashboardView.swift @@ -33,8 +33,9 @@ struct DashboardView: View { VStack(alignment: .leading, spacing: 10) { header BalanceBarView(balance: entry.balance) + Spacer(minLength: 0) + footer } - .frame(maxHeight: .infinity, alignment: .center) } } // a tap anywhere else opens the app on the connect tab @@ -111,7 +112,7 @@ struct DashboardView: View { ) }, bucketSeconds: throughput.bucketSeconds, - now: entry.date, + now: chartHorizon, placeholder: nil ) ThroughputChartView( @@ -125,12 +126,28 @@ struct DashboardView: View { ) }, bucketSeconds: throughput.bucketSeconds, - now: entry.date, + now: chartHorizon, placeholder: entry.tunnel.providing ? nil : "Provider stats will appear when the provider is enabled." ) } } + /// How far right the chart is allowed to plot. + /// + /// Every entry in a timeline carries the SAME snapshot at a later date, so + /// a window anchored to the entry walks past the newest bucket the tunnel + /// published. Out there `byStart[bucket] ?? 0` stops meaning "no traffic" + /// and starts meaning "not measured yet" -- and the chart drew it as a + /// flat zero line across up to a quarter of the plot while traffic was + /// flowing. Reporting staleness is the footer's job; the chart's job is to + /// be true. + /// + /// Clamped rather than simply swapped so that a future change to the entry + /// count degrades to frozen-but-true instead of collapsing again. + private var chartHorizon: Date { + min(entry.date, entry.tunnel.updatedAt) + } + /// "Provider ยท Auto": the chart name with the current provide mode. private var providerTitle: LocalizedStringKey { guard let mode = entry.tunnel.provideMode, let label = Self.provideModeLabel(mode) else { @@ -159,9 +176,17 @@ struct DashboardView: View { .foregroundStyle(WidgetTheme.textFaint) } else if entry.tunnel.tunnelActive || entry.balance != nil { let updatedAt = max(entry.tunnel.updatedAt, entry.balance?.updatedAt ?? .distantPast) - Text(updatedLabel(updatedAt)) + WidgetRefreshButton { + HStack(spacing: 4) { + Image(systemName: "arrow.clockwise") + // the age of the DATA, not of this render: a tap that + // reaches no tunnel leaves this counting up, so the button + // cannot report a refresh that did not happen + Text("Updated \(Text(updatedAt, style: .relative))") + } .font(WidgetTheme.caption) .foregroundStyle(WidgetTheme.textFaint) + } } else { Text("Connect once to see your traffic here") .font(WidgetTheme.caption) @@ -170,24 +195,6 @@ struct DashboardView: View { } } -extension DashboardView { - - /// "Updated 3 min. ago", formatted for the timeline entry's date (entries - /// are five minutes apart, so this advances at that cadence). Not a - /// relative-time Text: that reserves the width of its widest possible - /// value inside the sentence. - private func updatedLabel(_ updatedAt: Date) -> String { - let elapsed = entry.date.timeIntervalSince(updatedAt) - if elapsed < 60 { - return String(localized: "Updated just now") - } - let formatter = RelativeDateTimeFormatter() - formatter.unitsStyle = .abbreviated - let relative = formatter.localizedString(for: updatedAt, relativeTo: entry.date) - return String(format: String(localized: "Updated %@"), relative) - } -} - /// The in-widget quick connect: the same intent as the Control Center /// toggle, drawn as a bordered button carrying the connector mark. WidgetKit /// flips a toggle optimistically before the timeline is re-rendered, and the diff --git a/app/widgets/Globe/ProviderGlobeWidget.swift b/app/widgets/Globe/ProviderGlobeWidget.swift index 67487bab..e5d7a9a8 100644 --- a/app/widgets/Globe/ProviderGlobeWidget.swift +++ b/app/widgets/Globe/ProviderGlobeWidget.swift @@ -95,6 +95,7 @@ struct ProviderGlobeView: View { .font(WidgetTheme.label) .foregroundStyle(WidgetTheme.textMuted) } + WidgetRefreshIcon() } if providers.isEmpty { Text(emptyMessage) diff --git a/app/widgets/Shared/RefreshWidgetsIntent.swift b/app/widgets/Shared/RefreshWidgetsIntent.swift new file mode 100644 index 00000000..d54f20d4 --- /dev/null +++ b/app/widgets/Shared/RefreshWidgetsIntent.swift @@ -0,0 +1,112 @@ +// +// RefreshWidgetsIntent.swift +// URnetworkWidgets +// +// The refresh button behind every widget's freshness line. +// +// A widget cannot make its own data: only the packet tunnel process holds +// the live counters, and it publishes them to the App Group. So a tap asks +// the tunnel to publish now, waits a bounded time for that write to land, +// and then re-renders. With the tunnel down there is nothing to wait for -- +// the writer does not exist -- so the tap re-reads live NEVPNStatus and the +// last published snapshot instead, which is still worth something: a tunnel +// brought up from Settings with the app force-quit shows as connected. +// +// This is the freshness path that matters, because a reload caused by an +// in-widget intent is not charged against the reload budget that keeps the +// automatic cadence at tens of minutes (WidgetRefreshPolicy). +// + +import AppIntents +import SwiftUI +import WidgetKit + +struct RefreshWidgetsIntent: AppIntent { + + static let title: LocalizedStringResource = "Refresh URnetwork widgets" + + /// Not a standalone Shortcuts action: it only means anything as the + /// button on a widget that is about to re-render. + static let isDiscoverable: Bool = false + + /// Readable from the Lock Screen without authentication, like the quick + /// connect toggle it sits beside -- it publishes nothing new, it only + /// re-reads what the device already shows. + static let authenticationPolicy: IntentAuthenticationPolicy = .alwaysAllowed + + /// How long to wait for the tunnel's write. An intent has far longer, but + /// the user is watching a button: past a couple of seconds a stale render + /// is better than a spinner. + static let writeTimeout: TimeInterval = 2 + static let pollInterval: TimeInterval = 0.1 + + init() {} + + func perform() async throws -> some IntentResult { + // read the baseline BEFORE signalling, or the wait can miss its own + // answer when the tunnel writes faster than this task resumes + let baseline = WidgetSnapshotStore.loadTunnel()?.updatedAt + WidgetSnapshotRefreshRequest.post() + + if await TunnelControlSupport.currentState().isOn { + await Self.waitForWrite(after: baseline) + } + + // the surface that ran this intent is re-rendered by the system when + // perform() returns; the others are not, and the tunnel extension's + // own reload requests are best-effort (see ToggleTunnelIntent) + WidgetRefresh.reloadAll() + return .result() + } + + private static func waitForWrite(after baseline: Date?) async { + let deadline = Date().addingTimeInterval(writeTimeout) + while Date() < deadline { + try? await Task.sleep(nanoseconds: UInt64(pollInterval * 1_000_000_000)) + guard let updatedAt = WidgetSnapshotStore.loadTunnel()?.updatedAt else { + continue + } + if let baseline { + if baseline < updatedAt { + return + } + } else { + return + } + } + } +} + +/// The refresh affordance, defined once so the glyph, hit area and +/// accessibility label are the same on every widget. +/// +/// `.invalidatableContent()` marks what the tap is about to replace, so the +/// system dims it while the intent runs -- the only in-flight feedback a +/// widget has. The real confirmation is the freshness label itself, which is +/// bound to the snapshot's age rather than to the render, so the button +/// cannot report a success that did not happen. +struct WidgetRefreshButton: View { + + @ViewBuilder var label: () -> Label + + var body: some View { + Button(intent: RefreshWidgetsIntent()) { + label() + } + .buttonStyle(.plain) + .invalidatableContent() + .accessibilityLabel(Text("Refresh")) + } +} + +/// The icon-only form, for headers with no freshness line of their own. +struct WidgetRefreshIcon: View { + + var body: some View { + WidgetRefreshButton { + Image(systemName: "arrow.clockwise") + .font(WidgetTheme.label) + .foregroundStyle(WidgetTheme.textMuted) + } + } +} diff --git a/app/widgets/Shared/WidgetSnapshotTimeline.swift b/app/widgets/Shared/WidgetSnapshotTimeline.swift index 6174790d..126c1db9 100644 --- a/app/widgets/Shared/WidgetSnapshotTimeline.swift +++ b/app/widgets/Shared/WidgetSnapshotTimeline.swift @@ -9,11 +9,14 @@ // the snapshot, so a toggle flipped from Control Center reads correctly even // before the tunnel has written anything. // -// Reload policy: WidgetKit budgets reloads (roughly 40-70 a day per widget -// instance) and the tunnel extension's own reload requests are best-effort, -// so the timeline asks for a refresh every 20 minutes while the tunnel is -// up and hourly while it is down. State changes arrive sooner through the -// reloads the app and the tunnel request. +// Reload policy lives in WidgetRefreshPolicy, which carries the budget +// arithmetic. Entries do NOT keep the data moving -- they all render the one +// snapshot this timeline read, so what they advance is the elements that are +// a function of the entry's own date (the globe's provider durations). The +// dashboard's chart is anchored to the snapshot's clock rather than the +// entry's, and its freshness label ticks on its own, so neither depends on +// the entry cadence. State changes arrive sooner through the reloads the app +// and the tunnel request, and on demand through the refresh button. // import Foundation @@ -36,13 +39,6 @@ struct SnapshotEntry: TimelineEntry { struct SnapshotTimelineProvider: TimelineProvider { - static let refreshIntervalWhileUp: TimeInterval = 20 * 60 - static let refreshIntervalWhileDown: TimeInterval = 60 * 60 - /// Entries per timeline; each re-renders the same snapshot at a later - /// date so relative times and the chart axis keep moving. - static let entrySpacing: TimeInterval = 5 * 60 - static let entryCount = 4 - func placeholder(in context: Context) -> SnapshotEntry { SnapshotEntry.sample(at: Date()) } @@ -61,11 +57,17 @@ struct SnapshotTimelineProvider: TimelineProvider { Task { let now = Date() let current = await Self.currentEntry(at: now) + // the interval is needed before the entries: the timeline is + // sized so its last entry lands on the policy date, rather than + // running out partway through and holding one render until the + // reload arrives + let interval = current.isOn + ? WidgetRefreshPolicy.refreshIntervalWhileUp + : WidgetRefreshPolicy.refreshIntervalWhileDown var entries: [SnapshotEntry] = [] - for i in 0.. Date: Tue, 8 Sep 2026 17:12:35 -0700 Subject: [PATCH 2/3] scale the throughput chart to what it draws The chart took its vertical scale from every bucket in the snapshot, not from the hour it draws. The accumulator records a bucket only for a minute that carried traffic, so its sixty buckets can span many hours -- which means a burst from outside the window routinely set the scale for a curve it was not part of. Real recent traffic was squashed onto the axis and the peak label reported a rate from hours ago that never changed. That is the "the lines don't move and don't look right" report, and it is sticky: the history is restored on resume, and buckets are evicted by array length, so a poisoned bucket only leaves after sixty further traffic-bearing minutes. Scope peak and peakPackets to the drawn window. The labels read the same plot as the curve, so the header can no longer report a rate that is nowhere on the chart -- and the provider placeholder, which is shown only when both peaks are zero, stops being suppressed forever by one old providing session. Stop plotting the minute in progress. Each point is drawn at its bucket's END because the traffic is only known once the minute has elapsed, but the loop ran to `now`, so the newest point was a partial minute drawn at full weight and then held flat to the right edge. The end of the curve dived toward zero for reasons that had nothing to do with the network. Re-baseline a counter that goes backwards instead of taking it as a delta. These counters are cumulative for the session, and the drop is not always a restart: a reconnect can publish one tick carrying the retiring client's total on top of the new base, and the next tick then reads lower. Taking that as a delta wrote the ENTIRE session's byte count into a single minute -- a bucket orders of magnitude above anything real, which then owned the scale until sixty traffic-bearing minutes evicted it. Re-baselining drops at most one sample interval, which is one second. The alternative is unbounded. The geometry moves into a pure `plot` so it can be asserted at all; `draw` runs inside a Canvas closure where nothing could observe what it decided, which is why none of this had ever been caught. Also: the refresh request no longer consumes the tap and then drops it when it lands inside the write floor, and the intent waits four seconds rather than two. The extension serves the request on a .utility queue, which is exactly the work the system defers under Low Power Mode, so the old bound turned a working refresh into a visible no-op. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01AWV4MaF9JBcQppSX9UxATa --- app/extension/WidgetSnapshotWriter.swift | 13 ++- .../Shared/Widgets/ThroughputChartView.swift | 64 +++++++++++-- .../Shared/Widgets/WidgetSnapshots.swift | 23 ++++- .../ThroughputChartPlotTests.swift | 83 ++++++++++++++++ .../WidgetThroughputAccumulatorTests.swift | 95 +++++++++++++++++++ app/widgets/Shared/RefreshWidgetsIntent.swift | 12 ++- 6 files changed, 269 insertions(+), 21 deletions(-) create mode 100644 app/networkTests/ThroughputChartPlotTests.swift create mode 100644 app/networkTests/WidgetThroughputAccumulatorTests.swift diff --git a/app/extension/WidgetSnapshotWriter.swift b/app/extension/WidgetSnapshotWriter.swift index 0314316e..5f63b691 100644 --- a/app/extension/WidgetSnapshotWriter.swift +++ b/app/extension/WidgetSnapshotWriter.swift @@ -41,8 +41,9 @@ final class WidgetSnapshotWriter { /// roughly 40-70 a day the comments alongside them cited. static let reloadBackstopInterval: TimeInterval = WidgetRefreshPolicy.extensionBackstopInterval /// A refresh request from a widget publishes at most this often, so a - /// user tapping repeatedly cannot drive the write path. - static let refreshRequestFloor: TimeInterval = 2 + /// user tapping repeatedly cannot drive the write path. Under the intent's + /// own wait, so a second tap is served rather than timing out. + static let refreshRequestFloor: TimeInterval = 1 /// Contract change events arrive per contract, about once a second while /// bytes move; the two lists are re-read at most this often. static let contractRefreshInterval: TimeInterval = 2 @@ -361,6 +362,12 @@ final class WidgetSnapshotWriter { private func snapshotRefreshRequested() { queue.async { [weak self] in guard let self, self.active else { return } + // the floor is checked BEFORE consuming: a request left on disk is + // served by the next write-timer tick, where consuming and then + // bailing would swallow the tap and leave the widget waiting out + // its timeout for a write that was never going to come + let elapsed = self.lastRefreshWriteAt.map { Date().timeIntervalSince($0) } ?? .infinity + guard Self.refreshRequestFloor <= elapsed else { return } guard WidgetSnapshotRefreshRequest.consume() else { return } self.serveRefreshRequest() } @@ -374,8 +381,6 @@ final class WidgetSnapshotWriter { /// first so the one write carries a current membership rather than the /// last cached one. private func serveRefreshRequest() { - let elapsed = lastRefreshWriteAt.map { Date().timeIntervalSince($0) } ?? .infinity - guard Self.refreshRequestFloor <= elapsed else { return } lastRefreshWriteAt = Date() refreshContracts() write() diff --git a/app/network/Shared/Widgets/ThroughputChartView.swift b/app/network/Shared/Widgets/ThroughputChartView.swift index cb5522f8..ca56186c 100644 --- a/app/network/Shared/Widgets/ThroughputChartView.swift +++ b/app/network/Shared/Widgets/ThroughputChartView.swift @@ -88,12 +88,16 @@ struct ThroughputChartView: View { TimeInterval(bucketSeconds * Int64(Self.windowBuckets)) } + private var plot: Plot { + Self.plot(points: points, bucketSeconds: bucketSeconds, now: now) + } + private var peak: Int64 { - points.map { max($0.egress, $0.ingress) }.max() ?? 0 + plot.peak } private var peakPackets: Int64 { - points.map { max($0.egressPackets, $0.ingressPackets) }.max() ?? 0 + plot.peakPackets } /// Bytes per bucket as a rate, formatted like the app's chart labels. @@ -106,15 +110,60 @@ struct ThroughputChartView: View { formatPacketRate(peakPackets / max(1, bucketSeconds)) } + /// What the chart plots, computed apart from the drawing. + /// + /// `draw` runs inside a `Canvas` closure, so nothing can observe what it + /// decided. Every judgement that can be wrong -- which buckets fall in + /// the window, and what the curve is scaled against -- lives here so it + /// can be asserted directly. + struct Plot: Equatable { + /// Bucket start times, oldest first, one per bucket across the whole + /// window whether or not the snapshot carries that bucket. + var bucketStarts: [Int64] + /// The largest byte and packet values the scale is taken from. + var peak: Int64 + var peakPackets: Int64 + } + + static func plot(points: [Point], bucketSeconds: Int64, now: Date) -> Plot { + let window = TimeInterval(bucketSeconds * Int64(windowBuckets)) + let nowSeconds = now.timeIntervalSince1970 + let windowStart = nowSeconds - window + var starts: [Int64] = [] + var bucket = (Int64(windowStart) / bucketSeconds) * bucketSeconds + // up to the last COMPLETE bucket: a bucket's traffic is only known + // once the minute has elapsed, and the one in progress holds a + // fraction of its eventual total. Drawn at full weight -- and then + // held flat to the right edge -- it dived the end of the curve toward + // zero for reasons that had nothing to do with the network + while bucket + bucketSeconds <= Int64(nowSeconds) { + starts.append(bucket) + bucket += bucketSeconds + } + // the scale comes only from what is on screen. The accumulator + // records a bucket only for a minute that carried traffic, so its 60 + // buckets can span many hours -- and a burst from outside this window + // used to set the scale for a curve it was not part of, squashing + // real recent traffic flat onto the axis and freezing the peak label + // on a rate from hours ago + let drawn = Set(starts) + let visible = points.filter { drawn.contains($0.start) } + return Plot( + bucketStarts: starts, + peak: visible.map { max($0.egress, $0.ingress) }.max() ?? 0, + peakPackets: visible.map { max($0.egressPackets, $0.ingressPackets) }.max() ?? 0 + ) + } + private func draw(_ context: inout GraphicsContext, size: CGSize) { let centerY = size.height / 2 let plotHalf = max(1, centerY - 1) + let plot = self.plot // each series pair on its own scale: the peak of either reaches the // plot edge, so both are readable whatever their ratio - let scale = Double(max(peak, Self.minimumScale)) - let packetScale = Double(max(peakPackets, Self.minimumPacketScale)) + let scale = Double(max(plot.peak, Self.minimumScale)) + let packetScale = Double(max(plot.peakPackets, Self.minimumPacketScale)) let nowSeconds = now.timeIntervalSince1970 - let windowStart = nowSeconds - window // one sample per bucket across the whole window, zero where nothing // was recorded, so the spline is evenly spaced and reaches both edges @@ -132,9 +181,7 @@ struct ThroughputChartView: View { func offset(_ value: Int64, _ scale: Double) -> CGFloat { plotHalf * CGFloat(min(1, Double(value) / scale)) } - let firstBucket = (Int64(windowStart) / bucketSeconds) * bucketSeconds - var bucket = firstBucket - while bucket <= Int64(nowSeconds) { + for bucket in plot.bucketStarts { let point = byStart[bucket] // plot at the bucket's end: the bucket's traffic is known once it // has elapsed @@ -144,7 +191,6 @@ struct ThroughputChartView: View { ingress.append(CGPoint(x: px, y: centerY + offset(point?.ingress ?? 0, scale))) egressPackets.append(CGPoint(x: px, y: centerY - offset(point?.egressPackets ?? 0, packetScale))) ingressPackets.append(CGPoint(x: px, y: centerY + offset(point?.ingressPackets ?? 0, packetScale))) - bucket += bucketSeconds } func holdToEdge(_ series: inout [CGPoint]) { if let last = series.last, last.x < size.width { diff --git a/app/network/Shared/Widgets/WidgetSnapshots.swift b/app/network/Shared/Widgets/WidgetSnapshots.swift index ef13f331..3066547a 100644 --- a/app/network/Shared/Widgets/WidgetSnapshots.swift +++ b/app/network/Shared/Widgets/WidgetSnapshots.swift @@ -295,12 +295,25 @@ struct WidgetThroughputAccumulator: Codable, Equatable { buckets[buckets.count - 1] = bucket } - /// A counter that went backwards is a restarted session: count the new - /// value as the delta rather than dropping it. The first observation - /// after a resume is a baseline only. + /// A counter that went backwards re-baselines: the sample is treated as + /// a new starting point and contributes nothing. + /// + /// Taking `value` as the delta instead -- on the reasoning that a + /// restarted session counts from zero -- is unsafe here, because these + /// counters are cumulative for the whole session and the drop is not + /// always a restart. A reconnect can publish one tick carrying the + /// retiring client's total on top of the new base, and the next tick then + /// reads lower; taking that lower value as a delta writes the ENTIRE + /// session's byte count into a single minute. One such bucket is a rate + /// orders of magnitude above anything real, and it sets the chart's scale + /// until sixty further traffic-bearing minutes evict it. + /// + /// The cost of re-baselining is bounded by the sample interval, which is + /// one second, so at most a second of traffic is dropped. The cost of the + /// alternative is unbounded. private static func delta(from last: Int64?, to value: Int64) -> Int64 { - guard let last else { return 0 } - return value < last ? value : value - last + guard let last, last <= value else { return 0 } + return value - last } private mutating func currentBucket(at date: Date) -> WidgetThroughputBucket { diff --git a/app/networkTests/ThroughputChartPlotTests.swift b/app/networkTests/ThroughputChartPlotTests.swift new file mode 100644 index 00000000..de781ffc --- /dev/null +++ b/app/networkTests/ThroughputChartPlotTests.swift @@ -0,0 +1,83 @@ +// +// ThroughputChartPlotTests.swift +// networkTests +// +// What the widget's throughput chart decides to draw. +// +// `draw` runs inside a Canvas closure, so none of this was observable and +// none of it had ever been asserted. Both cases below are things a reader +// of the chart cannot tell are happening: the curve is flattened by a value +// that is not on screen, and its right-hand end is a minute that has not +// finished yet. +// + +import Testing +import Foundation +@testable import URnetwork + +struct ThroughputChartPlotTests { + + private typealias Point = ThroughputChartView.Point + + private static let bucketSeconds: Int64 = 60 + /// An arbitrary fixed instant, 30 seconds into the bucket starting at it. + private static let bucketStart: Int64 = 1_800_000_000 + private static var now: Date { Date(timeIntervalSince1970: TimeInterval(bucketStart + 30)) } + + private func plot(_ points: [Point], now: Date = ThroughputChartPlotTests.now) -> ThroughputChartView.Plot { + ThroughputChartView.plot(points: points, bucketSeconds: Self.bucketSeconds, now: now) + } + + /// The accumulator only appends a bucket for a minute that carried + /// traffic, so 60 buckets can span many hours. A burst from outside the + /// drawn hour must not set the scale the visible curve is drawn against + /// -- if it does, real recent traffic is squashed onto the axis and the + /// chart reads as flat. + @Test func theScaleComesOnlyFromWhatIsDrawn() { + let threeHoursAgo = Self.bucketStart - 3 * 3600 + let points = [ + Point(start: threeHoursAgo, egress: 10 * 1024 * 1024, ingress: 10 * 1024 * 1024, + egressPackets: 40_000, ingressPackets: 40_000), + Point(start: Self.bucketStart - 120, egress: 100 * 1024, ingress: 100 * 1024, + egressPackets: 400, ingressPackets: 400), + ] + let plot = self.plot(points) + + // the old burst is genuinely off screen + #expect(!plot.bucketStarts.contains(threeHoursAgo)) + // ... so it must not be what the curve is measured against + #expect(plot.peak == 100 * 1024) + #expect(plot.peakPackets == 400) + } + + /// A bucket's traffic is only known once the minute has elapsed -- the + /// file says so where it plots each point at the bucket's END. The + /// in-progress minute holds a fraction of its eventual traffic, so + /// drawing it at full weight dives the right-hand end of the curve + /// toward zero for reasons that have nothing to do with the network. + @Test func theInProgressMinuteIsNotPlotted() { + let plot = self.plot([]) + // now is 30s into `bucketStart`, so the newest COMPLETE bucket is the + // one before it + #expect(plot.bucketStarts.last == Self.bucketStart - Self.bucketSeconds) + } + + /// Nothing is plotted from outside the window on either side. + @Test func everyPlottedBucketIsInsideTheWindow() { + let plot = self.plot([]) + let window = Self.bucketSeconds * 60 + let oldest = try! #require(plot.bucketStarts.first) + let newest = try! #require(plot.bucketStarts.last) + #expect(Self.bucketStart - window <= oldest) + #expect(newest + Self.bucketSeconds <= Self.bucketStart + 30) + } + + /// A complete bucket that has just closed IS plotted -- the fix must not + /// drop a whole minute of real data to avoid the partial one. + @Test func theMostRecentCompleteMinuteIsPlotted() { + let justClosed = Self.bucketStart - Self.bucketSeconds + let plot = self.plot([Point(start: justClosed, egress: 5, ingress: 5)]) + #expect(plot.bucketStarts.contains(justClosed)) + #expect(plot.peak == 5) + } +} diff --git a/app/networkTests/WidgetThroughputAccumulatorTests.swift b/app/networkTests/WidgetThroughputAccumulatorTests.swift new file mode 100644 index 00000000..a0ba9971 --- /dev/null +++ b/app/networkTests/WidgetThroughputAccumulatorTests.swift @@ -0,0 +1,95 @@ +// +// WidgetThroughputAccumulatorTests.swift +// networkTests +// +// The per-minute history behind the widget's throughput chart. +// +// The counters it is fed are cumulative for the whole tunnel session, and it +// stores differences. That makes one case dangerous out of proportion to how +// often it happens: a counter that reads LOWER than the last sample. Treating +// that reading as a delta writes an entire session's byte count into a single +// minute -- a rate orders of magnitude above anything real, which then owns +// the chart's vertical scale until sixty further traffic-bearing minutes +// evict it. A tunnel restart does not clear it, because the history is +// restored on resume. +// + +import Testing +import Foundation +@testable import URnetwork + +struct WidgetThroughputAccumulatorTests { + + private static let minute: TimeInterval = 60 + private static func at(_ offset: TimeInterval) -> Date { + Date(timeIntervalSince1970: 1_800_000_000 + offset) + } + + /// The first reading has nothing to difference against, so it establishes + /// the baseline and contributes no traffic. + @Test func theFirstSampleIsABaselineOnly() { + var accumulator = WidgetThroughputAccumulator() + accumulator.recordClient(egress: 5_000, ingress: 9_000, egressPackets: 10, ingressPackets: 20, at: Self.at(0)) + + #expect(accumulator.buckets.isEmpty) + } + + @Test func subsequentSamplesRecordTheDifference() { + var accumulator = WidgetThroughputAccumulator() + accumulator.recordClient(egress: 1_000, ingress: 2_000, egressPackets: 1, ingressPackets: 2, at: Self.at(0)) + accumulator.recordClient(egress: 4_000, ingress: 6_000, egressPackets: 4, ingressPackets: 7, at: Self.at(1)) + + #expect(accumulator.buckets.count == 1) + let bucket = accumulator.buckets[0] + #expect(bucket.clientEgress == 3_000) + #expect(bucket.clientIngress == 4_000) + #expect(bucket.clientEgressPackets == 3) + #expect(bucket.clientIngressPackets == 5) + } + + /// The one that matters. A reconnect can publish a tick carrying the + /// retiring client's total on top of the new base, so the next tick reads + /// lower. That drop must re-baseline, not be banked as a minute's traffic. + @Test func aCounterThatWentBackwardsDoesNotBecomeAMinuteOfTraffic() { + var accumulator = WidgetThroughputAccumulator() + // a long session: five gigabytes moved + accumulator.recordClient( + egress: 5_000_000_000, ingress: 5_000_000_000, + egressPackets: 4_000_000, ingressPackets: 4_000_000, at: Self.at(0) + ) + // ... then the counter reads lower, as it does after a reconnect + accumulator.recordClient( + egress: 1_000, ingress: 1_000, + egressPackets: 2, ingressPackets: 2, at: Self.at(1) + ) + + #expect(accumulator.buckets.isEmpty) + + // and it carries on from the new baseline + accumulator.recordClient( + egress: 4_000, ingress: 5_000, + egressPackets: 6, ingressPackets: 8, at: Self.at(2) + ) + #expect(accumulator.buckets.count == 1) + #expect(accumulator.buckets[0].clientEgress == 3_000) + #expect(accumulator.buckets[0].clientIngress == 4_000) + } + + /// A minute that carried no traffic gets no bucket at all, which is why 60 + /// buckets can span many hours and why the chart must scope its scale to + /// the window it draws. + @Test func idleMinutesAreNotRecorded() { + var accumulator = WidgetThroughputAccumulator() + accumulator.recordClient(egress: 0, ingress: 0, egressPackets: 0, ingressPackets: 0, at: Self.at(0)) + accumulator.recordClient(egress: 100, ingress: 100, egressPackets: 1, ingressPackets: 1, at: Self.at(1)) + // three hours later, with nothing in between + accumulator.recordClient( + egress: 200, ingress: 200, egressPackets: 2, ingressPackets: 2, + at: Self.at(3 * 3600) + ) + + #expect(accumulator.buckets.count == 2) + let span = accumulator.buckets[1].start - accumulator.buckets[0].start + #expect(span == 3 * 3600) + } +} diff --git a/app/widgets/Shared/RefreshWidgetsIntent.swift b/app/widgets/Shared/RefreshWidgetsIntent.swift index d54f20d4..73cfb07c 100644 --- a/app/widgets/Shared/RefreshWidgetsIntent.swift +++ b/app/widgets/Shared/RefreshWidgetsIntent.swift @@ -35,9 +35,15 @@ struct RefreshWidgetsIntent: AppIntent { static let authenticationPolicy: IntentAuthenticationPolicy = .alwaysAllowed /// How long to wait for the tunnel's write. An intent has far longer, but - /// the user is watching a button: past a couple of seconds a stale render - /// is better than a spinner. - static let writeTimeout: TimeInterval = 2 + /// the user is watching a button: past a few seconds a stale render is + /// better than a spinner. + /// + /// The extension serves this on a `.utility` queue, which is exactly the + /// work the system defers under Low Power Mode and thermal pressure, so + /// too tight a bound turns a working refresh into a visible no-op -- the + /// write lands just after the wait gives up and is not read until the next + /// timeline reload, up to the policy interval later. + static let writeTimeout: TimeInterval = 4 static let pollInterval: TimeInterval = 0.1 init() {} From f0bc7fa2d9e564e07be7c3550ebbe09093d29d76 Mon Sep 17 00:00:00 2001 From: Ryanmello07 <67509637+Ryanmello07@users.noreply.github.com> Date: Tue, 8 Sep 2026 18:06:54 -0700 Subject: [PATCH 3/3] record the widget's throughput at the resolution the app draws The widget kept one bucket per minute over an hour while the app's chart draws one point per second over a minute. The two were never going to look alike, and at that resolution a minute of real traffic was a single point -- which is why the curve read as dead even once it was correct. The reasoning for the hour confused two kinds of staleness. The snapshot is written by the tunnel on its own timer whatever the widget is doing, so whenever WidgetKit rebuilds a timeline it reads a file at most one write interval old. What goes stale between reloads is the RENDERED view, and no window size changes that. The hour bought nothing for it and cost the chart all of its detail. So record one second per bucket over a minute. The chart derives its window from the snapshot's own bucketSeconds, so this needed no change there and a snapshot written by an older build still renders as the hour it was recorded as. The scale floor moves with the bucket size, staying the same rate rather than becoming sixty times stricter. Halve the write interval to 30s. The newest bucket a reload can find is one write old, and a whole window of lag would leave the widget drawing the minute BEFORE the last one. Writes are not the budgeted resource -- WidgetKit reloads are -- so this is paid in a few KB, not in refreshes. The chart tests move to bucket and window multiples rather than literal seconds, so they keep their meaning if the resolution changes again, and one pins that a legacy coarse snapshot still renders. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01AWV4MaF9JBcQppSX9UxATa --- app/extension/WidgetSnapshotWriter.swift | 8 +- .../Shared/Widgets/ThroughputChartView.swift | 16 +++- .../Shared/Widgets/WidgetSnapshots.swift | 16 +++- .../ThroughputChartPlotTests.swift | 93 +++++++++++++------ .../WidgetThroughputAccumulatorTests.swift | 20 +++- 5 files changed, 111 insertions(+), 42 deletions(-) diff --git a/app/extension/WidgetSnapshotWriter.swift b/app/extension/WidgetSnapshotWriter.swift index 5f63b691..7979ebd4 100644 --- a/app/extension/WidgetSnapshotWriter.swift +++ b/app/extension/WidgetSnapshotWriter.swift @@ -21,7 +21,13 @@ final class WidgetSnapshotWriter { /// How often the snapshot file is rewritten while the tunnel is up. Cheap /// (a few KB, atomic), so the next reload always finds fresh buckets. - static let writeInterval: TimeInterval = 60 + /// + /// Half the chart's window: the newest bucket a reload can find is one + /// write old, and a whole window of lag would leave the widget drawing + /// the minute BEFORE the last one. Writes are not the budgeted resource + /// -- WidgetKit reloads are -- so this is paid in a few KB, not in + /// refreshes. + static let writeInterval: TimeInterval = 30 /// The write cadence while the app's Account > Widgets previews are on /// screen (WidgetPreviewVisibility): the previews read every write, so /// they move like the real widgets would if WidgetKit re-rendered that diff --git a/app/network/Shared/Widgets/ThroughputChartView.swift b/app/network/Shared/Widgets/ThroughputChartView.swift index ca56186c..2d50030e 100644 --- a/app/network/Shared/Widgets/ThroughputChartView.swift +++ b/app/network/Shared/Widgets/ThroughputChartView.swift @@ -7,10 +7,14 @@ // // A static version of the app's TransferChart for one route: bytes (green, // filled) and packets (pink, a line) sent above the axis and received -// below, each pair on its own scale, Catmull-Rom smoothed. The app draws -// one point per second over a 60 s window; the widget draws one bucket per -// minute over the last hour, because that is the cadence a Home Screen -// widget can honestly show. +// below, each pair on its own scale, Catmull-Rom smoothed. Both draw one +// point per second over a 60 s window, so the widget shows the same curve +// the app does -- it simply holds still between reloads, where the app's +// redraws every second. +// +// The window is derived from the snapshot's own `bucketSeconds`, so a +// snapshot written by an older build (a minute per bucket) still renders, +// as the hour it was recorded as. // import SwiftUI @@ -39,7 +43,9 @@ struct ThroughputChartView: View { private static let labelBand: CGFloat = 14 private static let windowBuckets = WidgetThroughputAccumulator.bucketCount /// Floor for the byte scale so an idle chart is flat rather than noisy. - private static let minimumScale: Int64 = 64 * 1024 + /// Expressed per bucket, so it stays the same RATE whatever the bucket + /// size is: 8 KiB/s. + private static let minimumScale: Int64 = 8 * 1024 * WidgetThroughputAccumulator.bucketSeconds /// Floor for the packet scale: the app's 8 packets/s over one bucket. private static let minimumPacketScale: Int64 = 8 * WidgetThroughputAccumulator.bucketSeconds private static let packetColor = WidgetTheme.packetSeries diff --git a/app/network/Shared/Widgets/WidgetSnapshots.swift b/app/network/Shared/Widgets/WidgetSnapshots.swift index 3066547a..d3e1619f 100644 --- a/app/network/Shared/Widgets/WidgetSnapshots.swift +++ b/app/network/Shared/Widgets/WidgetSnapshots.swift @@ -222,8 +222,20 @@ struct WidgetBalanceSnapshot: Codable, Equatable { /// restart instead of resetting to flat). struct WidgetThroughputAccumulator: Codable, Equatable { - static let bucketSeconds: Int64 = 60 - /// One hour of history. + /// One second per bucket, one minute of history -- the same shape the + /// app's own TransferChart draws, so the two show the same curve. + /// + /// This used to be a minute per bucket over an hour, on the reasoning + /// that an hour is the cadence a widget can honestly show. That reasoning + /// confused two different kinds of staleness. The snapshot on disk is + /// written by the tunnel on its own timer whatever the widget is doing, + /// so whenever WidgetKit rebuilds a timeline it reads a file that is at + /// most one write interval old; what goes stale between reloads is the + /// RENDERED view, and no window size changes that. An hour-wide window + /// bought nothing for it and cost the chart all of its detail -- a minute + /// of real traffic became one point, which is why the curve read as dead. + static let bucketSeconds: Int64 = 1 + /// One minute of history. static let bucketCount = 60 private(set) var buckets: [WidgetThroughputBucket] = [] diff --git a/app/networkTests/ThroughputChartPlotTests.swift b/app/networkTests/ThroughputChartPlotTests.swift index de781ffc..4758931f 100644 --- a/app/networkTests/ThroughputChartPlotTests.swift +++ b/app/networkTests/ThroughputChartPlotTests.swift @@ -7,9 +7,13 @@ // `draw` runs inside a Canvas closure, so none of this was observable and // none of it had ever been asserted. Both cases below are things a reader // of the chart cannot tell are happening: the curve is flattened by a value -// that is not on screen, and its right-hand end is a minute that has not +// that is not on screen, and its right-hand end is a bucket that has not // finished yet. // +// Everything here is expressed in bucket and window multiples rather than in +// literal seconds, so the assertions keep their meaning if the recorded +// resolution changes again. +// import Testing import Foundation @@ -19,65 +23,96 @@ struct ThroughputChartPlotTests { private typealias Point = ThroughputChartView.Point - private static let bucketSeconds: Int64 = 60 - /// An arbitrary fixed instant, 30 seconds into the bucket starting at it. + private static let bucketSeconds: Int64 = WidgetThroughputAccumulator.bucketSeconds + private static let window: Int64 = bucketSeconds * Int64(WidgetThroughputAccumulator.bucketCount) + + /// An arbitrary instant on a bucket boundary, and a `now` part-way into + /// the bucket that starts there -- so that bucket is in progress. private static let bucketStart: Int64 = 1_800_000_000 - private static var now: Date { Date(timeIntervalSince1970: TimeInterval(bucketStart + 30)) } + private static var now: Date { + Date(timeIntervalSince1970: TimeInterval(bucketStart) + Double(bucketSeconds) / 2) + } - private func plot(_ points: [Point], now: Date = ThroughputChartPlotTests.now) -> ThroughputChartView.Plot { - ThroughputChartView.plot(points: points, bucketSeconds: Self.bucketSeconds, now: now) + private func plot(_ points: [Point]) -> ThroughputChartView.Plot { + ThroughputChartView.plot( + points: points, + bucketSeconds: ThroughputChartPlotTests.bucketSeconds, + now: ThroughputChartPlotTests.now + ) } - /// The accumulator only appends a bucket for a minute that carried - /// traffic, so 60 buckets can span many hours. A burst from outside the - /// drawn hour must not set the scale the visible curve is drawn against - /// -- if it does, real recent traffic is squashed onto the axis and the - /// chart reads as flat. + /// The accumulator records a bucket only for an interval that carried + /// traffic, so its buckets can span far more time than the window. A burst + /// from outside the drawn window must not set the scale the visible curve + /// is drawn against -- if it does, real recent traffic is squashed onto + /// the axis and the chart reads as flat. @Test func theScaleComesOnlyFromWhatIsDrawn() { - let threeHoursAgo = Self.bucketStart - 3 * 3600 + let longAgo = Self.bucketStart - 3 * Self.window + let recent = Self.bucketStart - 2 * Self.bucketSeconds let points = [ - Point(start: threeHoursAgo, egress: 10 * 1024 * 1024, ingress: 10 * 1024 * 1024, + Point(start: longAgo, egress: 10 * 1024 * 1024, ingress: 10 * 1024 * 1024, egressPackets: 40_000, ingressPackets: 40_000), - Point(start: Self.bucketStart - 120, egress: 100 * 1024, ingress: 100 * 1024, + Point(start: recent, egress: 100 * 1024, ingress: 100 * 1024, egressPackets: 400, ingressPackets: 400), ] let plot = self.plot(points) // the old burst is genuinely off screen - #expect(!plot.bucketStarts.contains(threeHoursAgo)) + #expect(!plot.bucketStarts.contains(longAgo)) + #expect(plot.bucketStarts.contains(recent)) // ... so it must not be what the curve is measured against #expect(plot.peak == 100 * 1024) #expect(plot.peakPackets == 400) } - /// A bucket's traffic is only known once the minute has elapsed -- the - /// file says so where it plots each point at the bucket's END. The - /// in-progress minute holds a fraction of its eventual traffic, so - /// drawing it at full weight dives the right-hand end of the curve - /// toward zero for reasons that have nothing to do with the network. - @Test func theInProgressMinuteIsNotPlotted() { + /// A bucket's traffic is only known once it has elapsed -- the chart plots + /// each point at the bucket's END for exactly that reason. The bucket in + /// progress holds a fraction of its eventual traffic, so drawing it at + /// full weight dives the right-hand end of the curve toward zero for + /// reasons that have nothing to do with the network. + @Test func theInProgressBucketIsNotPlotted() { let plot = self.plot([]) - // now is 30s into `bucketStart`, so the newest COMPLETE bucket is the - // one before it #expect(plot.bucketStarts.last == Self.bucketStart - Self.bucketSeconds) + #expect(!plot.bucketStarts.contains(Self.bucketStart)) } /// Nothing is plotted from outside the window on either side. @Test func everyPlottedBucketIsInsideTheWindow() { let plot = self.plot([]) - let window = Self.bucketSeconds * 60 + let nowSeconds = Self.now.timeIntervalSince1970 let oldest = try! #require(plot.bucketStarts.first) let newest = try! #require(plot.bucketStarts.last) - #expect(Self.bucketStart - window <= oldest) - #expect(newest + Self.bucketSeconds <= Self.bucketStart + 30) + #expect(Double(oldest) >= nowSeconds - Double(Self.window) - Double(Self.bucketSeconds)) + #expect(Double(newest + Self.bucketSeconds) <= nowSeconds) } - /// A complete bucket that has just closed IS plotted -- the fix must not - /// drop a whole minute of real data to avoid the partial one. - @Test func theMostRecentCompleteMinuteIsPlotted() { + /// A bucket that has just closed IS plotted -- the fix must not drop real + /// data to avoid the partial bucket. + @Test func theMostRecentCompleteBucketIsPlotted() { let justClosed = Self.bucketStart - Self.bucketSeconds let plot = self.plot([Point(start: justClosed, egress: 5, ingress: 5)]) #expect(plot.bucketStarts.contains(justClosed)) #expect(plot.peak == 5) } + + /// The drawn window spans the recorded history, so the chart shows what + /// the accumulator kept -- neither a slice of it nor more than exists. + @Test func theWindowSpansTheRecordedHistory() { + let plot = self.plot([]) + #expect(plot.bucketStarts.count == WidgetThroughputAccumulator.bucketCount) + } + + /// A snapshot written by an older build recorded a minute per bucket; it + /// must still render, as the hour it was recorded as, rather than being + /// squeezed into the new window. + @Test func aLegacyCoarseSnapshotStillRenders() { + let legacyBucketSeconds: Int64 = 60 + let plot = ThroughputChartView.plot( + points: [], + bucketSeconds: legacyBucketSeconds, + now: Self.now + ) + let span = Double(plot.bucketStarts.count) * Double(legacyBucketSeconds) + #expect(3000 <= span) + } } diff --git a/app/networkTests/WidgetThroughputAccumulatorTests.swift b/app/networkTests/WidgetThroughputAccumulatorTests.swift index a0ba9971..ffa34ac5 100644 --- a/app/networkTests/WidgetThroughputAccumulatorTests.swift +++ b/app/networkTests/WidgetThroughputAccumulatorTests.swift @@ -75,10 +75,10 @@ struct WidgetThroughputAccumulatorTests { #expect(accumulator.buckets[0].clientIngress == 4_000) } - /// A minute that carried no traffic gets no bucket at all, which is why 60 - /// buckets can span many hours and why the chart must scope its scale to - /// the window it draws. - @Test func idleMinutesAreNotRecorded() { + /// An interval that carried no traffic gets no bucket at all, which is why + /// the buckets can span far more time than the window and why the chart + /// must scope its scale to the window it draws. + @Test func idleTimeIsNotRecorded() { var accumulator = WidgetThroughputAccumulator() accumulator.recordClient(egress: 0, ingress: 0, egressPackets: 0, ingressPackets: 0, at: Self.at(0)) accumulator.recordClient(egress: 100, ingress: 100, egressPackets: 1, ingressPackets: 1, at: Self.at(1)) @@ -90,6 +90,16 @@ struct WidgetThroughputAccumulatorTests { #expect(accumulator.buckets.count == 2) let span = accumulator.buckets[1].start - accumulator.buckets[0].start - #expect(span == 3 * 3600) + #expect(3 * 3600 - WidgetThroughputAccumulator.bucketSeconds <= span) + } + + /// The widget records the same shape the app's chart draws, so the two + /// show the same curve rather than a minute and an hour of the same data. + @Test func theHistoryIsOneMinuteAtOneSecondResolution() { + #expect(WidgetThroughputAccumulator.bucketSeconds == 1) + let window = WidgetThroughputAccumulator.bucketSeconds + * Int64(WidgetThroughputAccumulator.bucketCount) + #expect(30 <= window) + #expect(window <= 60) } }