Skip to content

update dependencies, some small restructuring #106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 4, 2025
Merged

Conversation

lukaskollmer
Copy link
Member

@lukaskollmer lukaskollmer commented May 28, 2025

update dependencies, some small restructuring

♻️ Current situation & Problem

this PR updates the HealthKitOnFHIR and SpeziHealthKit dependencies to their 1.0 releases. it also makes some other small adjustments.

note: as part of this PR, LLMonFHIR loses the ability to detect clinical records being deleted from HealthKit while the app is running

⚙️ Release Notes

  • updated some internals

📚 Documentation

n/a

✅ Testing

n/a

Code of Conduct & Contributing Guidelines

By creating and submitting this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

@lukaskollmer lukaskollmer requested a review from jdisho May 28, 2025 21:23
@lukaskollmer lukaskollmer self-assigned this May 28, 2025
@lukaskollmer lukaskollmer added the enhancement New feature or request label May 28, 2025
@@ -983,8 +986,8 @@
isa = XCRemoteSwiftPackageReference;
repositoryURL = "https://github.com/StanfordSpezi/SpeziFHIR";
requirement = {
kind = upToNextMinorVersion;
minimumVersion = 0.7.6;
branch = "lukas/update-deps";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: switch to a properly tagged version before merging

Copy link
Member

@jdisho jdisho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking care of this @lukaskollmer 🌟

But I need some more time to fully understand the new implementation details in the LLMonFHIRStandard. I'll continue tomorrow with a fresh mind.

Copy link

codecov bot commented May 28, 2025

Codecov Report

Attention: Patch coverage is 17.64706% with 14 lines in your changes missing coverage. Please review.

Project coverage is 23.06%. Comparing base (189abac) to head (503e041).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
LLMonFHIR/LLMonFHIRStandard.swift 23.08% 10 Missing ⚠️
LLMonFHIR/Settings/ResourceSelection.swift 0.00% 2 Missing ⚠️
LLMonFHIR/Onboarding/HealthKitPermissions.swift 0.00% 1 Missing ⚠️
LLMonFHIR/Onboarding/OnboardingFlow.swift 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #106      +/-   ##
==========================================
+ Coverage   22.95%   23.06%   +0.11%     
==========================================
  Files          57       57              
  Lines         632      642      +10     
==========================================
+ Hits          145      148       +3     
- Misses        487      494       +7     
Files with missing lines Coverage Δ
...nterpretation/UserStudy/UserStudyWelcomeView.swift 2.50% <ø> (ø)
LLMonFHIR/LLMonFHIRDelegate.swift 100.00% <ø> (ø)
LLMonFHIR/Onboarding/HealthKitPermissions.swift 85.72% <0.00%> (-14.28%) ⬇️
LLMonFHIR/Onboarding/OnboardingFlow.swift 83.34% <0.00%> (ø)
LLMonFHIR/Settings/ResourceSelection.swift 0.00% <0.00%> (ø)
LLMonFHIR/LLMonFHIRStandard.swift 35.00% <23.08%> (-1.36%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 189abac...503e041. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lukaskollmer
Copy link
Member Author

lukaskollmer commented May 28, 2025

@jdisho to explain the changes in a nutshell: in the past, LLMonFHIR used SpeziHealthKit's CollectSample API to fetch the clinical records from HealthKit (this worked bc there was an option to tell SpeziHealthKit not to save the anchor for its continuous queries, thereby effectively rerunning the initial query on every app launch, and as a result getting all samples every time). CollectSample never really was intended to be used this way; hence why this option was removed.

since this API has been removed, we now instead perform a custom fetch (using the healthKit.query(_:timeRange:) function), and get the samples from there instead.

depending on the authorization status, we either perform this fetch directly on launch (as part of the standard's configure() method), or after the user has been prompted to grant us access permissions.

Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great; thank you for the quick fix and improvement 🚀

@PSchmiedmayer PSchmiedmayer merged commit 2c012cc into main Jun 4, 2025
8 of 9 checks passed
@PSchmiedmayer PSchmiedmayer deleted the lukas/update-deps branch June 4, 2025 18:47
@jdisho jdisho mentioned this pull request Jun 5, 2025
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants