initial iOS 27 adjustments - #5
Open
lukaskollmer wants to merge 7 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR applies a broad set of independent correctness and concurrency fixes across Spezi modules: existential type lookups, ChangesCross-module fixes and improvements
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant HKHealthStore as "HKHealthStore+BackgroundDelivery"
participant Semaphore as "AsyncSemaphore"
participant HealthKit as "HealthKit API"
Caller->>HKHealthStore: startBackgroundDelivery(for: sampleType)
HKHealthStore->>Semaphore: wait
HKHealthStore->>HKHealthStore: enableBackgroundDelivery(for: objectType)
alt count was 0
HKHealthStore->>HealthKit: enableBackgroundDelivery
end
HKHealthStore->>Semaphore: signal
alt enable failed
HKHealthStore->>HKHealthStore: disableBackgroundDelivery(rollback)
HKHealthStore-->>Caller: throw error
else success
HKHealthStore->>HealthKit: execute observer query
HKHealthStore-->>Caller: return
end
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PSchmiedmayer
approved these changes
Jul 1, 2026
PSchmiedmayer
left a comment
PSchmiedmayer
left a comment
Contributor
There was a problem hiding this comment.
Thanks for all the improvements; good to merge as long as all the CI is passing.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
Sources/SpeziChat/MessageInputView.swift (1)
210-223: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer a structured
Loggeroverprint()for the speech-recognition error.Catching and surfacing the error is a good addition, but
print()output isn't filterable/searchable like OSLog-based logging used elsewhere in Spezi modules.♻️ Suggested direction
- } catch { - print("Speech recognition error: \(error)") - } + } catch { + logger.error("Speech recognition error: \(error)") + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/SpeziChat/MessageInputView.swift` around lines 210 - 223, Replace the speech-recognition error print in MessageInputView’s Task/do-catch block with the module’s structured Logger approach used elsewhere in Spezi. Keep the existing catch in the speechRecognizer.start() flow, but log the caught error through a Logger instance instead of print(), using a clear message that includes the error details.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/SpeziHealthKit/Health` Data
Collection/HealthKitSampleCollector.swift:
- Around line 137-141: The stop path in HealthKitSampleCollector should mirror
startBackgroundDelivery(for:) by disabling every effective sample type that was
enabled, not just sampleType.hkSampleType. Update the cleanup logic in the same
collector flow to iterate over
sampleType.hkSampleType.effectiveObjectTypesForAuthorization and call
disableBackgroundDelivery for each descriptor. Keep the existing error logging
in HealthKitSampleCollector, but make sure it runs per disabled type so expanded
types are fully turned off.
In `@Sources/SpeziStudy/Study` Manager/StudyManager.swift:
- Around line 168-177: The autosave loop in StudyManager.autosaveTask is
retaining self and never exits cleanly. Update the Swift::Task closure to
capture self weakly and guard it before each save, and add a cancellation check
inside the while loop so it stops when the task is cancelled. Also handle
Task.sleep in a way that does not swallow cancellation, using the existing
modelContext.save() path and the autosaveTask property as the main points to
adjust.
- Around line 149-157: The task launches in StudyManager.configure and the other
affected methods use invalid Swift syntax, so replace every Swift::Task
reference with Swift.Task throughout StudyManager.swift. Update the calls in
configure and the other async entry points that use the same typo so they
compile correctly, keeping the existing `@MainActor` and error-handling behavior
intact.
---
Nitpick comments:
In `@Sources/SpeziChat/MessageInputView.swift`:
- Around line 210-223: Replace the speech-recognition error print in
MessageInputView’s Task/do-catch block with the module’s structured Logger
approach used elsewhere in Spezi. Keep the existing catch in the
speechRecognizer.start() flow, but log the caught error through a Logger
instance instead of print(), using a clear message that includes the error
details.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 21766938-d010-4fae-9bbd-c48c21feb5c1
📒 Files selected for processing (24)
Sources/Spezi/Spezi/SpeziPropertyWrapper.swiftSources/SpeziAccount/AccountService/IdentityProvider/SecurityRelatedModifier.swiftSources/SpeziAccount/Mock/InMemoryAccountService.swiftSources/SpeziAccount/Mock/InMemoryAccountStorageProvider.swiftSources/SpeziAccount/ViewModifier/RequiredValidationModifier.swiftSources/SpeziAccount/Views/DataEntry/GeneralizedDataEntryView.swiftSources/SpeziAccount/Views/PasswordResetView.swiftSources/SpeziAccountPhoneNumbers/DataEntry/OTCEntryView.swiftSources/SpeziBluetooth/AccessorySetupKit/ASAccessoryEventType+Description.swiftSources/SpeziBluetooth/AccessorySetupKit/AccessorySetupKit.swiftSources/SpeziBluetooth/CoreBluetooth/Model/BluetoothManagerStorage.swiftSources/SpeziChat/MessageInputView.swiftSources/SpeziContact/Contact Views/ContactView.swiftSources/SpeziContact/Models/ContactOption.swiftSources/SpeziDevices/Testing/MockDevice.swiftSources/SpeziFoundation/LocalPreferences/LocalPreference.swiftSources/SpeziHealthKit/Health Data Collection/HealthKitSampleCollector.swiftSources/SpeziHealthKit/HealthKit Extensions/HKHealthStore+BackgroundDelivery.swiftSources/SpeziLocalStorage/LocalStorageKey.swiftSources/SpeziStudy/Study Manager/StudyManager+Other.swiftSources/SpeziStudy/Study Manager/StudyManager.swiftSources/SpeziStudyDefinition/StudyBundle/Validation/StudyBundle+QuestionnaireValidation.swiftSources/SpeziViews/Views/ManagedNavigationStack/ManagedNavigationStack.swiftSources/SpeziViews/Views/Text/MarkdownView+ImageProviders.swift
Comment on lines
+137
to
+141
| do { | ||
| try await healthStore.disableBackgroundDelivery(for: sampleType.hkSampleType) | ||
| } catch { | ||
| healthKit.logger.error("Failed to stop data collection for sample collector for \(self.sampleType.id): \(error)") | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Disable the same effective sample types that were enabled.
startBackgroundDelivery(for:) enables every descriptor from sampleType.hkSampleType.effectiveObjectTypesForAuthorization, but stop only disables sampleType.hkSampleType. Expanded types can remain enabled and keep stale observer counts.
🐛 Proposed fix
invalidator.invalidate()
- do {
- try await healthStore.disableBackgroundDelivery(for: sampleType.hkSampleType)
- } catch {
- healthKit.logger.error("Failed to stop data collection for sample collector for \(self.sampleType.id): \(error)")
+ let backgroundDeliveryTypes = sampleType.hkSampleType
+ .effectiveObjectTypesForAuthorization
+ .compactMap { $0 as? HKSampleType }
+ for backgroundDeliveryType in backgroundDeliveryTypes {
+ do {
+ try await healthStore.disableBackgroundDelivery(for: backgroundDeliveryType)
+ } catch {
+ healthKit.logger.error("Failed to stop data collection for sample collector for \(self.sampleType.id) and sample type \(backgroundDeliveryType): \(error)")
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| do { | |
| try await healthStore.disableBackgroundDelivery(for: sampleType.hkSampleType) | |
| } catch { | |
| healthKit.logger.error("Failed to stop data collection for sample collector for \(self.sampleType.id): \(error)") | |
| } | |
| invalidator.invalidate() | |
| let backgroundDeliveryTypes = sampleType.hkSampleType | |
| .effectiveObjectTypesForAuthorization | |
| .compactMap { $0 as? HKSampleType } | |
| for backgroundDeliveryType in backgroundDeliveryTypes { | |
| do { | |
| try await healthStore.disableBackgroundDelivery(for: backgroundDeliveryType) | |
| } catch { | |
| healthKit.logger.error("Failed to stop data collection for sample collector for \(self.sampleType.id) and sample type \(backgroundDeliveryType): \(error)") | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/SpeziHealthKit/Health` Data Collection/HealthKitSampleCollector.swift
around lines 137 - 141, The stop path in HealthKitSampleCollector should mirror
startBackgroundDelivery(for:) by disabling every effective sample type that was
enabled, not just sampleType.hkSampleType. Update the cleanup logic in the same
collector flow to iterate over
sampleType.hkSampleType.effectiveObjectTypesForAuthorization and call
disableBackgroundDelivery for each descriptor. Keep the existing error logging
in HealthKitSampleCollector, but make sure it runs per disabled type so expanded
types are fully turned off.
Comment on lines
+149
to
+157
| public func configure() { | ||
| Swift::Task { @MainActor in | ||
| do { | ||
| try await _configure() | ||
| } catch { | ||
| logger.error("configure failed: \(error)") | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'Swift::Task|Swift\.Task|_Concurrency\.Task|\bTask\s*\{|\bTask\.sleep' "Sources/SpeziStudy/Study Manager/StudyManager.swift"Repository: SchmiedmayerLab/Spezi
Length of output: 492
🏁 Script executed:
#!/bin/bash
sed -n '140,210p' "Sources/SpeziStudy/Study Manager/StudyManager.swift"Repository: SchmiedmayerLab/Spezi
Length of output: 2828
Replace Swift::Task with Swift.Task in Sources/SpeziStudy/Study Manager/StudyManager.swift :: is not valid Swift syntax, so these task launches won’t compile as written. The same typo appears at lines 150, 169, 174, 191, and 203.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/SpeziStudy/Study` Manager/StudyManager.swift around lines 149 - 157,
The task launches in StudyManager.configure and the other affected methods use
invalid Swift syntax, so replace every Swift::Task reference with Swift.Task
throughout StudyManager.swift. Update the calls in configure and the other async
entry points that use the same typo so they compile correctly, keeping the
existing `@MainActor` and error-handling behavior intact.
Comment on lines
+168
to
+177
| if autosaveTask == nil { | ||
| autosaveTask = Swift::Task { | ||
| while true { | ||
| await MainActor.run { | ||
| try? self.modelContext.save() | ||
| } | ||
| try? await Swift::Task.sleep(for: .seconds(0.25)) | ||
| } | ||
| } | ||
| #endif | ||
| outcomesObserverToken = scheduler.observeNewOutcomes { [weak self] outcome in | ||
| guard let self, | ||
| let studyContext = outcome.task.studyContext, | ||
| let studyBundle = self.studyEnrollments.first(where: { $0.studyId == studyContext.studyId })?.studyBundle else { | ||
| return | ||
| } | ||
| self.handleStudyLifecycleEvent( | ||
| .completedTask(componentId: studyContext.componentId), | ||
| for: studyBundle, | ||
| at: .now | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Autosave task leaks self and never stops.
The closure has no [weak self], so it strongly captures self, which is itself stored on self.autosaveTask — a retain cycle. Additionally, try? await Task.sleep(...) discards CancellationError, so the while true loop keeps running even if the task is cancelled; there's no Task.isCancelled check either. Simulator-only, but the StudyManager instance and this loop will run indefinitely for the process lifetime.
🔧 Proposed fix
if autosaveTask == nil {
- autosaveTask = Swift::Task {
- while true {
+ autosaveTask = Task { [weak self] in
+ while let self, !Task.isCancelled {
await MainActor.run {
try? self.modelContext.save()
}
- try? await Swift::Task.sleep(for: .seconds(0.25))
+ try? await Task.sleep(for: .seconds(0.25))
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if autosaveTask == nil { | |
| autosaveTask = Swift::Task { | |
| while true { | |
| await MainActor.run { | |
| try? self.modelContext.save() | |
| } | |
| try? await Swift::Task.sleep(for: .seconds(0.25)) | |
| } | |
| } | |
| #endif | |
| outcomesObserverToken = scheduler.observeNewOutcomes { [weak self] outcome in | |
| guard let self, | |
| let studyContext = outcome.task.studyContext, | |
| let studyBundle = self.studyEnrollments.first(where: { $0.studyId == studyContext.studyId })?.studyBundle else { | |
| return | |
| } | |
| self.handleStudyLifecycleEvent( | |
| .completedTask(componentId: studyContext.componentId), | |
| for: studyBundle, | |
| at: .now | |
| ) | |
| } | |
| if autosaveTask == nil { | |
| autosaveTask = Task { [weak self] in | |
| while let self, !Task.isCancelled { | |
| await MainActor.run { | |
| try? self.modelContext.save() | |
| } | |
| try? await Task.sleep(for: .seconds(0.25)) | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/SpeziStudy/Study` Manager/StudyManager.swift around lines 168 - 177,
The autosave loop in StudyManager.autosaveTask is retaining self and never exits
cleanly. Update the Swift::Task closure to capture self weakly and guard it
before each save, and add a cancellation check inside the while loop so it stops
when the task is cancelled. Also handle Task.sleep in a way that does not
swallow cancellation, using the existing modelContext.save() path and the
autosaveTask property as the main points to adjust.
lukaskollmer
added a commit
that referenced
this pull request
Jul 14, 2026
### ♻️ Current situation & Problem @PSchmiedmayer had a stab at various package-wide improvements and fixes in #17, such as addressing warnings, deprecations, etc. this work also partly overlaps with some things addressed in #5 (which probably will end up getting superseded by this PR). this PR picks up these adjustments (which initially were developed against #16), and carries them over into the current #20-based main branch state of the codebase. somewhat complete list of changes: - most of what was covered by #17 - address compiler warnings where possible - address deprecation warnings where possible - note that this PR intentionally does **not** address the firebase `updateEmail(to:)` deprecation, as there currently are some issues around the behaviour of the new API. see also #14 - some testing fixes that were left out of #20 - now that the package's effective availability is iOS 18+, we can remove a bunch of `if #available(iOS 18, *)` checks from all over the codebase - *todo* ### ⚙️ Release Notes *todo* ### 📚 Documentation *todo* ### ✅ Testing *todo* ### Code of Conduct & Contributing Guidelines By creating and submitting this pull request, you agree to follow our [Code of Conduct](https://github.com/SchmiedmayerLab/.github/blob/main/CODE_OF_CONDUCT.md) and [Contributing Guidelines](https://github.com/SchmiedmayerLab/.github/blob/main/CONTRIBUTING.md): - [x] I agree to follow the [Code of Conduct](https://github.com/SchmiedmayerLab/.github/blob/main/CODE_OF_CONDUCT.md) and [Contributing Guidelines](https://github.com/SchmiedmayerLab/.github/blob/main/CONTRIBUTING.md).
Contributor
|
@lukaskollmer I think most of that should no longer be relevant with the latest betas and setups? |
Collaborator
Author
|
@PSchmiedmayer likely yes, plus also some of the things i think we've already covered in recent other PRs update: still relevant; will revisit soon, bring in sync with main, and then merge |
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
initial iOS 27 adjustments
♻️ Current situation & Problem
Xcode 27 and the iOS/macOS/etc 27 SDKs are introducing some changes that affect various parts of Spezi.
ths purpose of this PR is to make a bunch of small changes to ensure that Spezi works well with Xcode/iOS/etc 27.
⚙️ Release Notes
ManagedNavigationStackandLocalPreferencewould not work correctly when compiled using the iOS 27 SDK📚 Documentation
n/a
✅ Testing
n/a. we will continue to test on iOS 26 for the time being, and will switch the CI to 27 down the road.
Code of Conduct & Contributing Guidelines
By creating and submitting this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by CodeRabbit