Repository navigation
Add LLM function calling for dynamic HealthKit data - #58
Open
ahmedrezik wants to merge 35 commits into
Open
ahmedrezik wants to merge 35 commits into
ahmedrezik wants to merge 35 commits into
Conversation
ahmedrezik
force-pushed
the
feature/llm-function-calling
branch
8 times, most recently
from
February 26, 2026 04:36
6876f4a to
9e35724
Compare
…tcher Introduce HealthMetric enum mapping metric names to HKQuantityTypeIdentifier, HKUnit, and HKStatisticsOptions. Add fetchQuantityData(for:unit:options:from:to:) and fetchSleepData(from:to:) methods for on-demand date-range queries.
Create three LLMFunction implementations: - GetHealthMetricFunction: fetch daily values for any metric over 1-90 days - GetAvailableMetricsFunction: list all available health metrics - ComparePeriodsFunction: compare a metric between two time periods
Add static buildToolUsePrompt() method that instructs the LLM to use function calling tools to fetch health data on demand, rather than having all data pre-loaded in the system prompt.
Add prepareLLMWithTools() for OpenAI sessions with function calling. Update resetChat() to use static tool-use prompt for OpenAI sessions and re-fetch data only for legacy (Fog/Local/Mock) sessions.
Configure LLMOpenAISchema with GetHealthMetricFunction, GetAvailableMetricsFunction, and ComparePeriodsFunction for on-demand HealthKit queries. Add overlay indicator when LLM is calling tools. Non-OpenAI paths (Fog/Local/Mock) remain unchanged.
Verify buildToolUsePrompt() contains expected instructions, tool names, and today's date.
Move @parameter attributes to same line as var declarations. Add swiftlint:disable:next for long LLM function description strings.
- Move nested enums before static properties (type_contents_order) - Extract chatContentView method after computed properties - Change @parameter Int types to String (SpeziLLM BinaryInteger init is not public), parse in execute() - Mark healthDataFetcher as nonisolated(unsafe) for Sendable conformance - Remove superfluous swiftlint:disable commands
Replace '14 days will be uploaded' with 'fetched on demand when you ask questions' to match the new tool-use architecture.
…n metadata - HealthMetricTests: verify all 6 cases, raw value round-trip, HK identifier/unit/options mappings, cumulative vs discrete options, sleep nil properties, display names and descriptions - GetAvailableMetricsFunctionTests: verify execute() returns all metrics with descriptions, and function name/description metadata - LLMFunctionMetadataTests: verify function names, MetricType enum parity with HealthMetric, unique function names, and tool-use prompt references all function names All 18 tests pass on iPhone 17 simulator.
Replace the standalone HKHealthStore with a @dependency on the shared SpeziHealthKit HealthKit module. All quantity and sleep queries now use healthKit.statisticsQuery() and healthKit.query() instead of raw HKStatisticsCollectionQueryDescriptor and HKSampleQueryDescriptor. Also adds a sampleType computed property to HealthMetric.
ahmedrezik
force-pushed
the
feature/llm-function-calling
branch
from
February 26, 2026 04:38
9e35724 to
3d508d6
Compare
1 task done
## ♻️ Current situation & Problem The app requests HealthKit read permission for `.heartRate` but queries `.restingHeartRate` in `HealthDataFetcher`. These are distinct `HKQuantityTypeIdentifier` values, so HealthKit silently returns no data for resting heart rate, and all days report 0 bpm. Resolves StanfordBDHG#56 ## ⚙️ Release Notes - Adds `.restingHeartRate` to the HealthKit `RequestReadAccess` quantity array so the fetcher can retrieve resting heart rate data ## 📚 Documentation No documentation changes needed. This is a one-line permission fix in `HealthGPTAppDelegate.swift`. ## ✅ Testing - Complete onboarding and verify the HealthKit permission dialog now includes "Resting Heart Rate" - Ask the LLM about resting heart rate and confirm it returns actual values instead of 0 or "Data doesn't exist" ### Code of Conduct & Contributing Guidelines By creating and submitting this pull request, you agree to follow our [Code of Conduct](https://github.com/StanfordBDHG/.github/blob/main/CODE_OF_CONDUCT.md) and [Contributing Guidelines](https://github.com/StanfordBDHG/.github/blob/ma in/CONTRIBUTING.md): - [x] I agree to follow the [Code of Conduct](https://github.com/StanfordBDHG/.github/blob/main/CODE_OF_CONDUCT.md) and [Contributing Guidelines](https://github.com/StanfordBDHG/.github/blob/ma in/CONTRIBUTING.md). --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Member
|
@coderabbitai Review |
✅ Actions performedReview triggered.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds LLM tool-calling for on-demand HealthKit queries: two new functions ( Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant LLM as LLM
participant GetFunc as GetHealthMetricFunction
participant CompareFunc as ComparePeriodsFunction
participant Fetcher as HealthDataFetcher
participant HealthKit as HealthKit
User->>LLM: Ask health question
LLM->>GetFunc: call get_health_metric(metric, days)
GetFunc->>Fetcher: fetchQuantityData / fetchSleepData (date range)
Fetcher->>HealthKit: Query samples (date range)
HealthKit-->>Fetcher: Samples
Fetcher-->>GetFunc: [(date, value)] or [(date, hours)]
GetFunc-->>LLM: Formatted daily results
LLM->>CompareFunc: call compare_periods(metric, p1Start, p1End, p2Start, p2End)
CompareFunc->>Fetcher: fetchAverage (period1)
Fetcher->>HealthKit: Query period1 samples
HealthKit-->>Fetcher: Samples
Fetcher-->>CompareFunc: period1 average
CompareFunc->>Fetcher: fetchAverage (period2)
Fetcher->>HealthKit: Query period2 samples
HealthKit-->>Fetcher: Samples
Fetcher-->>CompareFunc: period2 average
CompareFunc-->>LLM: Comparison string (avg, delta, percent)
LLM-->>User: Response using fetched data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds OpenAI function-calling support to fetch HealthKit metrics on demand (replacing the static 14‑day dump), including new tool prompts, metric mappings, and UI feedback while tools execute.
Changes:
- Introduces tool-use system prompt and three LLM functions (
get_health_metric,get_available_metrics,compare_periods). - Adds
HealthMetricenum mapping metrics to HealthKit identifiers/units and refactorsHealthDataFetcheronto SpeziHealthKit queries. - Updates UI to show a
.callingToolsindicator and adjusts onboarding/localization text for on-demand fetching.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| HealthGPTTests/PromptGeneratorTests.swift | Adds coverage for the new tool-use system prompt. |
| HealthGPTTests/LLMFunctionMetadataTests.swift | Verifies tool metadata and prompt references for function calling. |
| HealthGPTTests/HealthMetricTests.swift | Adds tests for HealthMetric mappings and invariants. |
| HealthGPTTests/GetAvailableMetricsFunctionTests.swift | Tests execution/metadata for get_available_metrics. |
| HealthGPT/Supporting Files/Localizable.xcstrings | Adds/updates strings for tool-calling indicator and OpenAI onboarding copy. |
| HealthGPT/HealthGPT/PromptGenerator.swift | Adds buildToolUsePrompt() for function-calling sessions. |
| HealthGPT/HealthGPT/HealthMetric.swift | Introduces metric-to-HealthKit mapping (identifiers, units, options). |
| HealthGPT/HealthGPT/HealthGPTView.swift | Wires OpenAI tools into schema + shows .callingTools overlay indicator. |
| HealthGPT/HealthGPT/HealthDataInterpreter.swift | Adds tool-based session preparation and adjusts reset behavior for OpenAI sessions. |
| HealthGPT/HealthGPT/HealthDataFetcher.swift | Switches to SpeziHealthKit APIs and adds flexible date-range fetch helpers. |
| HealthGPT/HealthGPT/Functions/GetHealthMetricFunction.swift | Adds tool to fetch metric series (1–90 days). |
| HealthGPT/HealthGPT/Functions/GetAvailableMetricsFunction.swift | Adds tool to list available metrics. |
| HealthGPT/HealthGPT/Functions/ComparePeriodsFunction.swift | Adds tool to compare two time periods for a metric. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+66
to
+72
| let percentChange = avg2 != 0 ? (diff / avg2) * 100 : 0 | ||
|
|
||
| return """ | ||
| \(healthMetric.displayName) comparison: | ||
| Period 1 (\(period1Label)): avg \(String(format: "%.1f", avg1)) | ||
| Period 2 (\(period2Label)): avg \(String(format: "%.1f", avg2)) | ||
| Difference: \(String(format: "%+.1f", diff)) (\(String(format: "%+.1f", percentChange))%) |
There was a problem hiding this comment.
When avg2 == 0 and avg1 != 0, returning 0% change is mathematically incorrect and will mislead users. Consider representing percent change as “N/A”/nil for a zero baseline (or use an alternate representation), while still reporting the absolute difference.
Suggested change
| let percentChange = avg2 != 0 ? (diff / avg2) * 100 : 0 | |
| return """ | |
| \(healthMetric.displayName) comparison: | |
| Period 1 (\(period1Label)): avg \(String(format: "%.1f", avg1)) | |
| Period 2 (\(period2Label)): avg \(String(format: "%.1f", avg2)) | |
| Difference: \(String(format: "%+.1f", diff)) (\(String(format: "%+.1f", percentChange))%) | |
| let percentChangeString: String | |
| if avg2 != 0 { | |
| let percentChange = (diff / avg2) * 100 | |
| percentChangeString = String(format: "%+.1f%%", percentChange) | |
| } else { | |
| percentChangeString = "N/A" | |
| } | |
| return """ | |
| \(healthMetric.displayName) comparison: | |
| Period 1 (\(period1Label)): avg \(String(format: "%.1f", avg1)) | |
| Period 2 (\(period2Label)): avg \(String(format: "%.1f", avg2)) | |
| Difference: \(String(format: "%+.1f", diff)) (\(percentChangeString)) |
Comment on lines
+29
to
+34
| @Parameter(description: "Number of past days to fetch (1-90)") var days: String | ||
|
|
||
| nonisolated(unsafe) let healthDataFetcher: HealthDataFetcher | ||
|
|
||
| func execute() async throws -> String? { | ||
| let clampedDays = max(1, min(Int(days) ?? 7, 90)) |
There was a problem hiding this comment.
Declaring numeric tool parameters as String will publish a string-typed schema to OpenAI function calling, which tends to decrease tool-call reliability and forces manual parsing. Prefer an integer parameter type (e.g., Int) for days (and similarly for the period parameters in ComparePeriodsFunction) and clamp/validate as needed.
Suggested change
| @Parameter(description: "Number of past days to fetch (1-90)") var days: String | |
| nonisolated(unsafe) let healthDataFetcher: HealthDataFetcher | |
| func execute() async throws -> String? { | |
| let clampedDays = max(1, min(Int(days) ?? 7, 90)) | |
| @Parameter(description: "Number of past days to fetch (1-90)") var days: Int | |
| nonisolated(unsafe) let healthDataFetcher: HealthDataFetcher | |
| func execute() async throws -> String? { | |
| let clampedDays = max(1, min(days, 90)) |
|
|
||
| @Parameter(description: "Number of past days to fetch (1-90)") var days: String | ||
|
|
||
| nonisolated(unsafe) let healthDataFetcher: HealthDataFetcher |
There was a problem hiding this comment.
nonisolated(unsafe) opts out of Swift concurrency safety and can allow cross-actor access to HealthDataFetcher from arbitrary executors, which is risky for an @Observable module that uses dependencies. A safer approach is to run the fetch on a known actor (e.g., mark execute() as @MainActor if HealthDataFetcher is main-actor bound, or wrap calls in await MainActor.run { ... }, or make the fetcher an actor/Sendable-safe wrapper).
Comment on lines
+49
to
+53
| let toolUsePrompt = PromptGenerator.buildToolUsePrompt() | ||
| let today = DateFormatter.localizedString(from: Date(), dateStyle: .full, timeStyle: .none) | ||
|
|
||
| #expect(toolUsePrompt.contains("HealthGPT")) | ||
| #expect(toolUsePrompt.contains("Today is \(today)")) |
There was a problem hiding this comment.
This test can be flaky around midnight because buildToolUsePrompt() and the test compute Date() separately (potentially yielding different “Today is …” values). Consider injecting a fixed date into buildToolUsePrompt (or checking for the "Today is " prefix rather than an exact localized date string) to make the test deterministic.
| } | ||
| }, | ||
| "Fetching your health data..." : { | ||
|
|
There was a problem hiding this comment.
The new string key has an empty definition, which makes localization behavior unclear and inconsistent with other entries that define localizations (and it’s also inconsistent with the project’s existing key style like WELCOME_TITLE). Consider adding at least an en localization entry (and optionally switching to a stable key like FETCHING_HEALTH_DATA while keeping the displayed value in the localization payload).
Suggested change
| "localizations" : { | |
| "en" : { | |
| "stringUnit" : { | |
| "state" : "translated", | |
| "value" : "Fetching your health data..." | |
| } | |
| } | |
| } |
Comment on lines
+29
to
+35
| @Parameter(description: "Start of period 1 in days ago (e.g. 7 means 7 days ago)") var period1Start: String | ||
|
|
||
| @Parameter(description: "End of period 1 in days ago (e.g. 0 means today)") var period1End: String | ||
|
|
||
| @Parameter(description: "Start of period 2 in days ago") var period2Start: String | ||
|
|
||
| @Parameter(description: "End of period 2 in days ago") var period2End: String |
There was a problem hiding this comment.
The period parsing/bounds behavior (string-to-int parsing, swapping start/end via max/min, and the “0 means today” semantics) is core tool behavior but isn’t covered by tests in this PR. Adding unit tests for common/edge inputs (e.g., non-numeric strings, reversed ranges, and ensuring “today” is excluded if that’s the intended contract) would help prevent regressions.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
HealthGPT/Supporting Files/Localizable.xcstrings (1)
80-85:⚠️ Potential issue | 🟠 MajorMake this privacy disclosure backend-specific.
Line 85 says HealthKit data is fetched on demand and sent to OpenAI, but this PR keeps Local/Fog/Mock on the legacy 14-day flow and Local/Fog do not send data to OpenAI at all. That makes the onboarding copy false in supported configurations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HealthGPT/Supporting` Files/Localizable.xcstrings around lines 80 - 85, The HealthKit permission copy in the HEALTHKIT_PERMISSIONS_DESCRIPTION string is inaccurate for Local/Fog/Mock backends; create backend-specific localized strings (e.g., HEALTHKIT_PERMISSIONS_DESCRIPTION_OPENAI and HEALTHKIT_PERMISSIONS_DESCRIPTION_LOCAL) or make the text conditional to avoid claiming data is sent to OpenAI, update the onboarding code that displays HEALTHKIT_PERMISSIONS_DESCRIPTION to instead select the appropriate key based on the current backend (Local/Fog/Mock vs OpenAI), and ensure Local/Fog/Mock variants explicitly state data is kept local or not sent to OpenAI.
🧹 Nitpick comments (4)
HealthGPT/HealthGPT/HealthDataFetcher.swift (1)
227-234: Fetch sleep samples once per range, not once per day.The
healthKit.querycall on Lines 230-234 sits inside the day loop, so a 90-day request becomes 90 sleep queries andcompare_periodscan double that. That will make sleep tool calls much slower than the quantity path.Please verify with a long-range sleep prompt that query count and latency stay acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HealthGPT/HealthGPT/HealthDataFetcher.swift` around lines 227 - 234, The sleep HK query is being executed inside the per-day loop (using asleepPredicate and healthKit.query) causing N queries for N days; instead, run a single healthKit.query for the full requested time span (use the overall HealthKitQueryTimeRange covering startOfSleep for the first day through endOfSleep for the last day), store the returned results in the results variable, then inside the day loop filter those cached results into per-day buckets by sample timestamps; ensure the same cached results are reused if compare_periods runs to avoid duplicate queries and maintain existing asleepPredicate and any downstream filtering logic that uses the sample timestamps.HealthGPT/HealthGPT/HealthDataInterpreter.swift (1)
72-84: Type-based prompt selection may be fragile for future extensibility.The check
llm is LLMOpenAISessionassumes all OpenAI sessions use tools. If someone later creates an OpenAI session viaprepareLLM(with:)(without tools),resetChat()would incorrectly use the tool-use prompt.Consider storing a flag or the prompt type explicitly during session creation, rather than inferring from the session type.
♻️ Suggested approach
`@Observable` class HealthDataInterpreter: DefaultInitializable, Module, EnvironmentAccessible { `@ObservationIgnored` `@Dependency`(LLMRunner.self) private var llmRunner `@ObservationIgnored` `@Dependency`(HealthDataFetcher.self) private var healthDataFetcher var llm: (any LLMSession)? `@ObservationIgnored` private var systemPrompt = "" + `@ObservationIgnored` private var usesToolBasedPrompt = falseThen set
usesToolBasedPrompt = trueinprepareLLMWithToolsand use that flag inresetChat().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HealthGPT/HealthGPT/HealthDataInterpreter.swift` around lines 72 - 84, The type-check in resetChat() is brittle; add a stored Bool flag (e.g., usesToolBasedPrompt) to the HealthDataInterpreter and set it to true inside prepareLLMWithTools (and false in other prepareLLM(...) paths), then update resetChat() to choose between PromptGenerator.buildToolUsePrompt() and await generateSystemPrompt() based on usesToolBasedPrompt instead of llm is LLMOpenAISession; ensure systemPrompt, llm?.context.reset(), and llm?.context.append(systemMessage:) behavior remains unchanged.HealthGPT/HealthGPT/Functions/GetHealthMetricFunction.swift (2)
63-64: Quantity metric output lacks unit labels.Sleep data output includes "hours" (line 46), but quantity metrics don't include their units. This inconsistency may confuse the LLM when interpreting values.
♻️ Suggested fix to include unit labels
- let lines = data.map { "\(dateFormatter.string(from: $0.date)): \(String(format: "%.1f", $0.value))" } + let unitLabel = healthMetric.unitLabel ?? "" + let lines = data.map { "\(dateFormatter.string(from: $0.date)): \(String(format: "%.1f", $0.value)) \(unitLabel)" }You would need to add a
unitLabelproperty toHealthMetricthat returns user-friendly strings like "steps", "calories", "minutes", "lbs", "bpm".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HealthGPT/HealthGPT/Functions/GetHealthMetricFunction.swift` around lines 63 - 64, The output for quantity metrics omits unit labels, causing inconsistency with Sleep which prints "hours"; add a unitLabel property to the HealthMetric type (e.g., returning "steps", "calories", "minutes", "lbs", "bpm") and update GetHealthMetricFunction's formatting (the block using dateFormatter, healthMetric.displayName, clampedDays, and the data.map closure) to append the metric's unitLabel to each value (e.g., format values as "12.3 bpm" or "100 steps") so all metrics consistently include units in the returned string.
14-21:MetricTypeduplicatesHealthMetriccases—code duplication persists but is verified via tests.
MetricTypemirrorsHealthMetricexactly. If a new metric is added toHealthMetric, this enum must be updated manually or the new metric won't be available via the LLM function.The duplication is already monitored by tests in
LLMFunctionMetadataTests.swiftthat verifyMetricType.allCasesmatchesHealthMetric.allCases, so drift will be caught at test time. Consider refactoring to reduce the duplication itself through protocol conformance or a shared definition.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HealthGPT/HealthGPT/Functions/GetHealthMetricFunction.swift` around lines 14 - 21, MetricType duplicates HealthMetric; remove the duplicated enum by having HealthMetric itself conform to LLMFunctionParameterEnum (and CaseIterable/RawRepresentable if required) and update GetHealthMetricFunction to use HealthMetric instead of MetricType so new metrics are automatically available; ensure LLMFunctionMetadataTests still compares HealthMetric.allCases to the expected set and adjust any references to MetricType to HealthMetric.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@HealthGPT/HealthGPT/Functions/ComparePeriodsFunction.swift`:
- Around line 65-66: The percent-change computation in ComparePeriodsFunction
(variables diff, avg1, avg2, percentChange) treats avg2 == 0 as 0% which is
misleading; change percentChange to an optional (e.g., Double?) and set it to
nil when avg2 == 0 (or otherwise return a sentinel like .none), and update the
downstream formatting/return logic that consumes percentChange to render "N/A"
(or "—") when percentChange is nil instead of printing 0.0%; keep the numeric
calculation unchanged for avg2 != 0 ((diff / avg2) * 100) and ensure any callers
of percentChange handle the optional safely.
In `@HealthGPT/HealthGPT/HealthDataFetcher.swift`:
- Around line 217-223: The loop that builds sleep buckets uses an inclusive
upper bound (while currentDay <= endDay) which creates an extra sleep window
compared to other metrics; change the loop to use an exclusive upper bound
(while currentDay < endDay) or otherwise adjust the endDay used for sleep (e.g.,
subtract one day) so sleep windows match the exclusive endDate semantics used
elsewhere; update the loop controlling symbol (currentDay, endDay) and keep the
existing calendar.date(bySettingHour:..., of: previousDay/ currentDay) logic
intact so get_health_metric/compare_periods produce consistent ranges.
- Around line 14-18: HealthDataFetcher currently hard-depends on HealthKit via
`@Dependency`(HealthKit.self) but HealthKit is registered only conditionally
(HKHealthStore.isHealthDataAvailable()), causing failures when HealthKit is
absent; change HealthDataFetcher to not require a guaranteed HealthKit
dependency by making the dependency optional or injected at init (e.g., replace
`@Dependency`(HealthKit.self) with an optional/nullable dependency or add
init(healthKit: HealthKit?) and store it), and update callers (the OpenAI tool
users GetHealthMetricFunction and ComparePeriodsFunction and the unconditional
HealthDataFetcher() instantiation) to either pass nil when HealthKit is
unavailable or guard early for healthKit == nil before using it; ensure any
HealthKit usage inside HealthDataFetcher checks for nil and fails gracefully.
In `@HealthGPT/HealthGPT/HealthGPTView.swift`:
- Around line 142-149: The onChange handler that increments messageTaskId should
also exclude the LLM's .callingTools state so tool executions don't get
interrupted: update the condition where you check llm.state (in the closure
watching llm.context and the logic that currently checks llm.state !=
.generating) to also ensure llm.state != .callingTools before bumping
messageTaskId, so the .task(id: self.messageTaskId) that calls
healthDataInterpreter.queryLLM() won't be cancelled/restarted while tools are
being invoked and their results appended to llm.context.
In `@HealthGPTTests/HealthMetricTests.swift`:
- Around line 9-12: Reorder the import statements in HealthMetricTests.swift to
satisfy SwiftLint's alphabetical import rule: place regular imports
alphabetically (Foundation then HealthKit), and move the testable import
"@testable import HealthGPT" after the regular imports; ensure the final order
is import Foundation, import HealthKit, `@testable` import HealthGPT, import
Testing so that `@testable` stays after the regular imports.
---
Outside diff comments:
In `@HealthGPT/Supporting` Files/Localizable.xcstrings:
- Around line 80-85: The HealthKit permission copy in the
HEALTHKIT_PERMISSIONS_DESCRIPTION string is inaccurate for Local/Fog/Mock
backends; create backend-specific localized strings (e.g.,
HEALTHKIT_PERMISSIONS_DESCRIPTION_OPENAI and
HEALTHKIT_PERMISSIONS_DESCRIPTION_LOCAL) or make the text conditional to avoid
claiming data is sent to OpenAI, update the onboarding code that displays
HEALTHKIT_PERMISSIONS_DESCRIPTION to instead select the appropriate key based on
the current backend (Local/Fog/Mock vs OpenAI), and ensure Local/Fog/Mock
variants explicitly state data is kept local or not sent to OpenAI.
---
Nitpick comments:
In `@HealthGPT/HealthGPT/Functions/GetHealthMetricFunction.swift`:
- Around line 63-64: The output for quantity metrics omits unit labels, causing
inconsistency with Sleep which prints "hours"; add a unitLabel property to the
HealthMetric type (e.g., returning "steps", "calories", "minutes", "lbs", "bpm")
and update GetHealthMetricFunction's formatting (the block using dateFormatter,
healthMetric.displayName, clampedDays, and the data.map closure) to append the
metric's unitLabel to each value (e.g., format values as "12.3 bpm" or "100
steps") so all metrics consistently include units in the returned string.
- Around line 14-21: MetricType duplicates HealthMetric; remove the duplicated
enum by having HealthMetric itself conform to LLMFunctionParameterEnum (and
CaseIterable/RawRepresentable if required) and update GetHealthMetricFunction to
use HealthMetric instead of MetricType so new metrics are automatically
available; ensure LLMFunctionMetadataTests still compares HealthMetric.allCases
to the expected set and adjust any references to MetricType to HealthMetric.
In `@HealthGPT/HealthGPT/HealthDataFetcher.swift`:
- Around line 227-234: The sleep HK query is being executed inside the per-day
loop (using asleepPredicate and healthKit.query) causing N queries for N days;
instead, run a single healthKit.query for the full requested time span (use the
overall HealthKitQueryTimeRange covering startOfSleep for the first day through
endOfSleep for the last day), store the returned results in the results
variable, then inside the day loop filter those cached results into per-day
buckets by sample timestamps; ensure the same cached results are reused if
compare_periods runs to avoid duplicate queries and maintain existing
asleepPredicate and any downstream filtering logic that uses the sample
timestamps.
In `@HealthGPT/HealthGPT/HealthDataInterpreter.swift`:
- Around line 72-84: The type-check in resetChat() is brittle; add a stored Bool
flag (e.g., usesToolBasedPrompt) to the HealthDataInterpreter and set it to true
inside prepareLLMWithTools (and false in other prepareLLM(...) paths), then
update resetChat() to choose between PromptGenerator.buildToolUsePrompt() and
await generateSystemPrompt() based on usesToolBasedPrompt instead of llm is
LLMOpenAISession; ensure systemPrompt, llm?.context.reset(), and
llm?.context.append(systemMessage:) behavior remains unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cc1687b5-a596-489a-8628-506e7a9534e9
📒 Files selected for processing (13)
HealthGPT/HealthGPT/Functions/ComparePeriodsFunction.swiftHealthGPT/HealthGPT/Functions/GetAvailableMetricsFunction.swiftHealthGPT/HealthGPT/Functions/GetHealthMetricFunction.swiftHealthGPT/HealthGPT/HealthDataFetcher.swiftHealthGPT/HealthGPT/HealthDataInterpreter.swiftHealthGPT/HealthGPT/HealthGPTView.swiftHealthGPT/HealthGPT/HealthMetric.swiftHealthGPT/HealthGPT/PromptGenerator.swiftHealthGPT/Supporting Files/Localizable.xcstringsHealthGPTTests/GetAvailableMetricsFunctionTests.swiftHealthGPTTests/HealthMetricTests.swiftHealthGPTTests/LLMFunctionMetadataTests.swiftHealthGPTTests/PromptGeneratorTests.swift
Comment on lines
14
to
18
| @Observable | ||
| class HealthDataFetcher: DefaultInitializable, Module, EnvironmentAccessible { | ||
| @ObservationIgnored private let healthStore = HKHealthStore() | ||
| @ObservationIgnored @Dependency(HealthKit.self) private var healthKit | ||
|
|
||
| required init() { } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expectation:
# - HealthKit registration is conditional.
# - HealthDataFetcher and OpenAI tool wiring are unconditional.
rg -n -C2 'HKHealthStore\.isHealthDataAvailable|HealthDataFetcher\(\)|@Dependency\(HealthKit\.self\)|GetHealthMetricFunction|ComparePeriodsFunction' \
"HealthGPT/HealthGPTAppDelegate.swift" \
"HealthGPT/HealthGPT/HealthDataFetcher.swift" \
"HealthGPT/HealthGPT/HealthGPTView.swift"Repository: StanfordBDHG/HealthGPT
Length of output: 1967
Don't require HealthKit here unless it is always registered.
HealthDataFetcher declares a hard dependency on HealthKit via @Dependency(HealthKit.self) at line 16, but in HealthGPTAppDelegate.swift the HealthKit module is only registered conditionally when HKHealthStore.isHealthDataAvailable() returns true (lines 24–26). However, HealthDataFetcher() is instantiated unconditionally at line 34, and the OpenAI tool functions that use it (GetHealthMetricFunction and ComparePeriodsFunction at HealthGPTView.swift lines 61, 63) are also unconditional. This creates a missing-dependency failure path on devices without HealthKit support.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@HealthGPT/HealthGPT/HealthDataFetcher.swift` around lines 14 - 18,
HealthDataFetcher currently hard-depends on HealthKit via
`@Dependency`(HealthKit.self) but HealthKit is registered only conditionally
(HKHealthStore.isHealthDataAvailable()), causing failures when HealthKit is
absent; change HealthDataFetcher to not require a guaranteed HealthKit
dependency by making the dependency optional or injected at init (e.g., replace
`@Dependency`(HealthKit.self) with an optional/nullable dependency or add
init(healthKit: HealthKit?) and store it), and update callers (the OpenAI tool
users GetHealthMetricFunction and ComparePeriodsFunction and the unconditional
HealthDataFetcher() instantiation) to either pass nil when HealthKit is
unavailable or guard early for healthKit == nil before using it; ensure any
HealthKit usage inside HealthDataFetcher checks for nil and fails gracefully.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #58 +/- ##
==========================================
- Coverage 89.00% 80.70% -8.30%
==========================================
Files 30 37 +7
Lines 200 259 +59
==========================================
+ Hits 178 209 +31
- Misses 22 50 +28
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
- Use SleepSession/splitIntoSleepSessions() for proper overlap handling - Replace duplicate MetricType enum with shared HealthMetric (LLMFunctionParameterEnum) - Use native Int @parameter types instead of String parsing - Throw errors instead of returning error strings from LLM functions - Add unitLabel property to HealthMetric for consistent output formatting - Unify prompt generation with shared basePrompt to eliminate duplication - Replace type-check (llm is LLMOpenAISession) with stored usesToolBasedPrompt flag - Replace central overlay with navigation toolbar subtitle for tool-calling state - Remove GetAvailableMetricsFunction (metrics auto-discovered from enum schema) - Extract defaultLookbackDays constant to avoid hardcoded values - Refactor HealthDataFetcher with flexible date-range queries and shared helpers - Fix test import ordering and midnight flakiness in prompt tests
ahmedrezik
added a commit
to ahmedrezik/HealthGPT
that referenced
this pull request
Apr 10, 2026
- Use SleepSession/splitIntoSleepSessions() for proper overlap handling - Replace duplicate MetricType enum with shared HealthMetric (LLMFunctionParameterEnum) - Use native Int @parameter types instead of String parsing - Throw errors instead of returning error strings from LLM functions - Add unitLabel property to HealthMetric for consistent output formatting - Unify prompt generation with shared basePrompt to eliminate duplication - Replace type-check (llm is LLMOpenAISession) with stored usesToolBasedPrompt flag - Replace central overlay with navigation toolbar subtitle for tool-calling state - Remove GetAvailableMetricsFunction (metrics auto-discovered from enum schema) - Extract defaultLookbackDays constant to avoid hardcoded values - Refactor HealthDataFetcher with flexible date-range queries and shared helpers - Fix test import ordering and midnight flakiness in prompt tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tests that just assert string literals or check every property 1:1 provide no regression value. Keep only buildMainPrompt which tests real formatting logic.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
HealthGPT/HealthGPT/Functions/GetHealthMetricFunction.swift (1)
27-30:⚠️ Potential issue | 🟠 MajorReject out-of-range
daysinstead of silently clamping it.If the model asks for 120 days, this still returns 90 days of data and the tool call looks successful. That makes bad tool arguments indistinguishable from valid ones. Fail fast here so the model can retry with a legal
1...90value.Proposed fix
func execute() async throws -> String? { - let clampedDays = max(1, min(days, 90)) + guard (1...90).contains(days) else { + throw HealthDataFetcherError.invalidDateRange + } + let endDate = Calendar.current.startOfDay(for: .now) - guard let startDate = Calendar.current.date(byAdding: .day, value: -clampedDays, to: endDate) else { + guard let startDate = Calendar.current.date(byAdding: .day, value: -days, to: endDate) else { throw HealthDataFetcherError.invalidDateRange } @@ - return "\(metric.displayName) for the last \(clampedDays) days:\n" + lines.joined(separator: "\n") + return "\(metric.displayName) for the last \(days) days:\n" + lines.joined(separator: "\n") @@ - return "\(metric.displayName) for the last \(clampedDays) days:\n" + lines.joined(separator: "\n") + return "\(metric.displayName) for the last \(days) days:\n" + lines.joined(separator: "\n") } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HealthGPT/HealthGPT/Functions/GetHealthMetricFunction.swift` around lines 27 - 30, In GetHealthMetricFunction.execute(), don't silently clamp the input days; validate that the property days is within 1...90 and if not throw a clear validation error so the caller/model can retry with a legal value. Replace the clampedDays logic with an explicit guard that checks (1...90).contains(days) and throw a descriptive error (e.g., ValidationError.invalidParameter or a new DaysOutOfRangeError) before computing endDate/startDate; keep the existing startDate calculation (Calendar.current.date(byAdding: .day, value: -days, to: endDate)) unchanged when days is valid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@HealthGPT/HealthGPT/Functions/ComparePeriodsFunction.swift`:
- Around line 35-55: Reject any period offsets that are positive (future) and
validate start<=end; compute day-aligned ranges by using
Calendar.startOfDay(for:) on .now then subtracting whole days for starts and
using startOfDay(for: start).addingTimeInterval(24*60*60) for exclusive upper
bounds passed to fetchAverage (or subtract 1 second to make inclusive if
fetchAverage expects inclusive end); replace the current
period1StartDate/period1EndDate/period2StartDate/period2EndDate calculations
with these day-aligned computations (use Calendar.startOfDay and date(byAdding:
.day,...)), and update the period1Label/period2Label to show the inclusive
human-readable end day (formatting the startOfDay and endDayMinusOneSecond as
needed) so labels match the data fetched; ensure errors throw
HealthDataFetcherError.invalidDateRange when offsets are invalid.
In `@HealthGPT/HealthGPT/HealthDataFetcher.swift`:
- Around line 16-21: Reorder the type members so the initializer comes after the
stored properties: move the required init() below the stored dependency and
static property declarations (the `@ObservationIgnored`
`@Dependency`(HealthKit.self) private var healthKit and private static let
defaultLookbackDays) to satisfy SwiftLint's type_contents_order rule; ensure no
other members are split by the initializer and run lint to confirm the violation
is resolved.
- Around line 135-153: The two-week convenience fetches use Date.now and can
include partial current-day data; update fetchSleepData and
fetchLastTwoWeeksData to anchor to full-day boundaries by computing endDate as
Calendar.current.startOfDay(for: Date.now) and then deriving startDate by
subtracting Self.defaultLookbackDays from that startOfDay; update references to
fetchQuantityData/fetchSleepData to use these adjusted startDate and endDate so
the helpers match PromptGenerator.basePrompt/GetHealthMetricFunction semantics
and avoid leaking partial current-day values.
In `@HealthGPTTests/LLMFunctionMetadataTests.swift`:
- Around line 38-42: The test toolUsePromptReferencesAllFunctionNames asserts
that buildToolUsePrompt() contains specific function names
(GetHealthMetricFunction.name, ComparePeriodsFunction.name), but
buildToolUsePrompt() intentionally returns generic tool-use instructions while
actual function names are registered in the OpenAI schema (HealthGPTView.swift);
update the test by removing or disabling the two name-specific assertions in
toolUsePromptReferencesAllFunctionNames, and instead assert only on the generic
guidance expected from PromptGenerator.buildToolUsePrompt() (or add a new test
that validates the schema registration in HealthGPTView.swift if you want to
cover function names separately).
---
Duplicate comments:
In `@HealthGPT/HealthGPT/Functions/GetHealthMetricFunction.swift`:
- Around line 27-30: In GetHealthMetricFunction.execute(), don't silently clamp
the input days; validate that the property days is within 1...90 and if not
throw a clear validation error so the caller/model can retry with a legal value.
Replace the clampedDays logic with an explicit guard that checks
(1...90).contains(days) and throw a descriptive error (e.g.,
ValidationError.invalidParameter or a new DaysOutOfRangeError) before computing
endDate/startDate; keep the existing startDate calculation
(Calendar.current.date(byAdding: .day, value: -days, to: endDate)) unchanged
when days is valid.
🪄 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
Run ID: d4eabc77-c90a-44f9-aad2-49b1aca6b494
📒 Files selected for processing (12)
HealthGPT/HealthGPT/Functions/ComparePeriodsFunction.swiftHealthGPT/HealthGPT/Functions/GetHealthMetricFunction.swiftHealthGPT/HealthGPT/HealthDataFetcher.swiftHealthGPT/HealthGPT/HealthDataFetcherError.swiftHealthGPT/HealthGPT/HealthDataInterpreter.swiftHealthGPT/HealthGPT/HealthGPTView.swiftHealthGPT/HealthGPT/HealthMetric.swiftHealthGPT/HealthGPT/PromptGenerator.swiftHealthGPT/Supporting Files/Localizable.xcstringsHealthGPTTests/HealthMetricTests.swiftHealthGPTTests/LLMFunctionMetadataTests.swiftHealthGPTTests/PromptGeneratorTests.swift
✅ Files skipped from review due to trivial changes (2)
- HealthGPT/HealthGPT/HealthDataFetcherError.swift
- HealthGPTTests/HealthMetricTests.swift
🚧 Files skipped from review as they are similar to previous changes (3)
- HealthGPTTests/PromptGeneratorTests.swift
- HealthGPT/Supporting Files/Localizable.xcstrings
- HealthGPT/HealthGPT/HealthMetric.swift
…g indicator, reuse SpeziHealthKit metric definitions - Merge buildToolUsePrompt/buildMainPrompt into single buildPrompt(usesTools:) and collapse duplicated branching in HealthDataInterpreter - Replace custom toolbar subtitle with SpeziChat's built-in messagePendingAnimation typing indicator (matching LLMonFHIR pattern) - Derive HealthMetric displayName/unitLabel from SpeziHealthKit's SampleType.displayTitle and displayUnit instead of hardcoding - Remove unused CALLING_TOOLS_SUBTITLE localization and description property
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@HealthGPT/HealthGPT/HealthGPTView.swift`:
- Around line 60-63: The OpenAI schema registration only includes
GetHealthMetricFunction and ComparePeriodsFunction but omits the
get_available_metrics tool; update the LLMOpenAISchema initialization to also
register the get_available_metrics function by adding the corresponding function
registration (e.g., GetAvailableMetricsFunction or the actual
get_available_metrics function wrapper) alongside
GetHealthMetricFunction(healthDataFetcher: healthDataFetcher) and
ComparePeriodsFunction(healthDataFetcher: healthDataFetcher) so the model can
call it.
In `@HealthGPT/HealthGPT/PromptGenerator.swift`:
- Around line 16-21: Move the type property before the initializer to satisfy
SwiftLint's type_contents_order rule: place the private static var basePrompt
declaration above init(with healthData: [HealthData] = []) so that static/type
properties (basePrompt) appear before instance initializers (init(with:)).
Ensure any related comments or attributes for basePrompt remain with that
declaration and update ordering only—no other logic changes.
- Around line 16-18: The non-tool prompt path assumes 14 entries and can index
out of range; update PromptGenerator (init(with healthData:)) and the non-tool
flow that calls buildFourteenDaysHealthDataPrompt() to first check
healthData.count >= 14 and either (a) pad/augment the array with neutral/default
HealthData objects to reach 14 entries or (b) switch to a shorter-data prompt
builder (e.g., buildShortHealthDataPrompt()) when fewer entries exist; ensure
buildFourteenDaysHealthDataPrompt() itself defensively validates indices and
returns a safe fallback so the app never crashes on partial/empty healthData.
🪄 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
Run ID: 6bcf6edc-e3d1-4585-b60a-c0d948c7a8dd
📒 Files selected for processing (6)
HealthGPT/HealthGPT/HealthDataInterpreter.swiftHealthGPT/HealthGPT/HealthGPTView.swiftHealthGPT/HealthGPT/HealthMetric.swiftHealthGPT/HealthGPT/PromptGenerator.swiftHealthGPT/Supporting Files/Localizable.xcstringsHealthGPTTests/PromptGeneratorTests.swift
✅ Files skipped from review due to trivial changes (1)
- HealthGPT/HealthGPT/HealthMetric.swift
🚧 Files skipped from review as they are similar to previous changes (3)
- HealthGPTTests/PromptGeneratorTests.swift
- HealthGPT/Supporting Files/Localizable.xcstrings
- HealthGPT/HealthGPT/HealthDataInterpreter.swift
… ordering - Use Date.now instead of startOfDay in GetHealthMetricFunction and Date() in HealthDataFetcher+Process to match the reviewed convention - Update PromptGenerator to say current-day data may be partial instead of claiming it is absent - Reorder HealthDataFetcher members so properties precede init (type_contents_order) - Validate ComparePeriodsFunction rejects negative (future) period offsets
Contributor
Author
|
@PSchmiedmayer Thank you for taking the time to review Paul, I recognize the mismatch of the repo from the ecosystem and I am spending time aligning it with the Spezi eco and other projects. |
- Move static basePrompt property above instance property and init to satisfy SwiftLint type_contents_order rule - Replace hardcoded 0...13 index loop with for-in over healthData array to prevent crash when fewer than 14 entries are available
- Make HealthDataFetcher final + @unchecked Sendable, matching the pattern used by SpeziHealthKit's own HealthKit module, and drop nonisolated(unsafe) from the LLM function structs. - Replace silent day-range clamp in GetHealthMetricFunction with an invalidDateRange throw so the model sees the error. - Replace HealthMetric.unitLabel "hours" fallback with an explicit switch and convert quantityValue's preconditionFailure(.sleep) into an unsupportedMetric throw. - Hoist the 3 PM sleep-window boundary into sleepWindowHour. - Drop unnecessary throws on GetAvailableMetricsFunction.execute. - Add ComparePeriodsFunctionTests and HealthDataFetcherTests covering resolveRange / percentChange / bucketSleepSessions; refactor PromptGeneratorTests onto a deterministic fixture.
ahmedrezik
added a commit
to ahmedrezik/HealthGPT
that referenced
this pull request
Apr 20, 2026
- Make HealthDataFetcher final + @unchecked Sendable, matching the pattern used by SpeziHealthKit's own HealthKit module, and drop nonisolated(unsafe) from the LLM function structs. - Replace silent day-range clamp in GetHealthMetricFunction with an invalidDateRange throw so the model sees the error. - Replace HealthMetric.unitLabel "hours" fallback with an explicit switch and convert quantityValue's preconditionFailure(.sleep) into an unsupportedMetric throw. - Hoist the 3 PM sleep-window boundary into sleepWindowHour. - Drop unnecessary throws on GetAvailableMetricsFunction.execute. - Add ComparePeriodsFunctionTests and HealthDataFetcherTests covering resolveRange / percentChange / bucketSleepSessions; refactor PromptGeneratorTests onto a deterministic fixture. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Re-key fetchAndProcessHealthData by date (startOfDay dictionary lookup) so a sparse statisticsQuery result no longer indexes out-of-bounds. Drops the now-unused fetchLastTwoWeeks* helpers. - Clip sleep sessions to each window in bucketSleepSessions so a session that straddles 3 PM is split proportionally instead of fully credited to both adjacent days. Add a regression test. - Drop GetAvailableMetricsFunction; the metric enum is already exposed through the parameter schema, so a separate listing tool is redundant.
ahmedrezik
added a commit
to ahmedrezik/HealthGPT
that referenced
this pull request
Apr 20, 2026
- Re-key fetchAndProcessHealthData by date (startOfDay dictionary lookup) so a sparse statisticsQuery result no longer indexes out-of-bounds. Drops the now-unused fetchLastTwoWeeks* helpers. - Clip sleep sessions to each window in bucketSleepSessions so a session that straddles 3 PM is split proportionally instead of fully credited to both adjacent days. Add a regression test. - Drop GetAvailableMetricsFunction; the metric enum is already exposed through the parameter schema, so a separate listing tool is redundant. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replaces the hardcoded `1...14` loop bound in fetchAndProcessHealthData with a named constant on HealthDataFetcher, addressing the review note about the magic number.
- Rename range tuples to `period1Range`/`period2Range` so the destructured `.start`/`.end` reads as a date range, not the raw parameter. - Rename the per-period averages to `period1Mean`/`period2Mean` to match what the helper actually returns, and rename `fetchAverage` to `averageValue` so the call site says what the value is, not how it was obtained. - Extract the duplicated "MMM d - MMM d" formatter into `Self.format(range:)`.
ahmedrezik
force-pushed
the
feature/llm-function-calling
branch
from
April 20, 2026 01:13
ba68639 to
5420c33
Compare
- Return validation errors from tool execute() as strings so the model can self-correct, rather than throwing (which aborts generation) - Conform HealthDataFetcherError to LocalizedError for readable alerts - Replace DateFormatter with Date.FormatStyle (.iso8601, .dateTime) - Distinguish "no data" from "avg 0.0" in period comparison via Double? - Rename usesToolBasedPrompt/useToolPrompt to usesTools everywhere - Collapse HealthMetric.displayName; throw on calendar-arithmetic failure in fetchSleepData instead of silently returning [] Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Move static helpers above instance methods so type_method precedes other_method per SwiftLint's type_contents_order rule. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- resolveRange params: start/end -> startDaysAgo/endDaysAgo (units in names) - switch bindings: current/baseline -> period1Value/period2Value (avoid implying period 1 is always newer) - period1Mean/period2Mean -> period1Average/period2Average (match averageValue()) - format(range:) -> formatDateRange(_:) (more specific, drops redundant label) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace the 3 PM to 3 PM windowing and proportional clipping with a wake-up-day attribution rule (session belongs to the calendar day of its end date), matching the Health app. Drops the asleep predicate so Spezi's 60-min gap rule sees awake samples when grouping; totalTimeSpentAsleep still only counts asleep phases. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 task done
Pull validation and formatting out of the LLM function execute() paths into testable statics: resolveDays, resolvePeriodOffsets, resolveRanges, buildHealthData with a DailyMetricValues struct. Add unit tests that exercise each helper directly so the branches don't require HealthKit. Replace the (offsets?, error?) tuple return on resolvePeriodOffsets with an OffsetsResolution enum so the type system proves resolved xor missing and the redundant fallback throw is gone. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Author
|
@PSchmiedmayer Happy to get your feedback and comments. This PR adds LLM function-calling support so HealthGPT can fetch Apple Health data on demand instead of relying only on a preloaded 14-day prompt. It introduces typed health metrics plus two OpenAI tools: The chat flow now uses tool-aware prompts for OpenAI sessions, keeps the existing preloaded-data path for non-tool LLM backends, and shows the pending state while tools are running. The PR also adds focused unit coverage for prompt behavior, metric helpers, date-range validation, period comparison, and sleep bucketing. |
This branch has not been deployed
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
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.
Summary
get_health_metric(fetch any metric, 1-90 days) andcompare_periods(compare two time ranges); the available metrics are exposed through themetricparameter's enum schemaHealthMetricenum mapping metric names to HealthKit identifiers/unitsHealthDataFetcherto use SpeziHealthKitstatisticsQuery()andquery()APIs instead of rawHKStatisticsCollectionQueryDescriptor/HKSampleQueryDescriptor, leveraging the sharedHealthKitmodule via@DependencyfetchAndProcessHealthDataby date so sparsestatisticsQueryresults no longer index out-of-boundsbucketSleepSessionsso a session that straddles the boundary isn't double-countedHealthDataFetcherfinal+@unchecked Sendable(matchingSpeziHealthKit.HealthKit's pattern), droppingnonisolated(unsafe)from the LLM function structs.callingToolsUI indicator while the LLM fetches health dataTest plan
PromptGeneratorTests(rewritten on a deterministic fixture),ComparePeriodsFunctionTests(coversresolveRange/percentChange),HealthDataFetcherTests(coversbucketSleepSessionsincluding the 3 PM-boundary clipping case)ComparePeriodsFunctioncalled.callingToolsindicator appears while tools executedays(e.g. 0 or 100) surfaces aninvalidDateRangetool error to the model rather than silently clamping🤖 Generated with Claude Code