mlx-swift - #7
Open
NIkhil-cmd-cmd wants to merge 13 commits into
Open
Conversation
|
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 HealthKit ECG ingestion and an ElectrocardiogramData model, builds ECG-aware system prompts, implements local model staging and download UI, introduces an OpenTSLM encoder/projector/pipeline with LoRA and interleaver support, adds a Swift package and runner plus tests, and wires inference into the interpreter flow. ChangesECG + OpenTSLM integration
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (6)
HealthyLLM/HealthyLLM/Fetcher/HealthDataFetcher.swift-20-21 (1)
20-21:⚠️ Potential issue | 🟡 MinorUse
HKObjectType.workoutType()instead ofHKSeriesType.workoutType()for correct HealthKit API usage.Line 20 uses an incorrect type accessor. According to Apple's HealthKit API,
workoutType()is a class method onHKObjectType, notHKSeriesType. Also verify that your app's minimum iOS deployment target supportselectrocardiogramType()(iOS 14.0+).Proposed fix
- HKSeriesType.workoutType(), + HKObjectType.workoutType(), HKObjectType.electrocardiogramType()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HealthyLLM/HealthyLLM/Fetcher/HealthDataFetcher.swift` around lines 20 - 21, Replace the incorrect HKSeriesType.workoutType() call with HKObjectType.workoutType() in the HealthDataFetcher where the types array is constructed (replace the HKSeriesType reference with HKObjectType); also ensure the electrocardiogramType() usage is guarded by or documented with an iOS version availability check (iOS 14.0+) so calls to HKObjectType.electrocardiogramType() are only used on supported deployments or wrapped with an availability check.HealthyLLM/HealthyLLM/HealthyLLMChatView.swift-82-95 (1)
82-95:⚠️ Potential issue | 🟡 MinorReset/failure leaves the auto-bootstrap prompt permanently disabled.
Line 86 flips
didSendInitialPromptbefore the initial query succeeds, and the reset action never clears it. After a transient startup failure—or after tapping reset in the same view instance—the ECG bootstrap prompt will not run again.🔁 Possible fix
- didSendInitialPrompt = true await healthDataInterpreter.resetChat() do { let initialContext: Chat = [.init(role: .user, content: firstPrompt)] try await healthDataInterpreter.queryLLM(with: initialContext, healthKit: healthKit) + didSendInitialPrompt = true } catch { + didSendInitialPrompt = false showErrorAlert = true errorMessage = "Error querying LLM: \(error.localizedDescription)" }Task { await healthDataInterpreter.resetChat() + didSendInitialPrompt = false + lastSubmittedUserMessageID = nil }Also applies to: 106-111
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HealthyLLM/HealthyLLM/HealthyLLMChatView.swift` around lines 82 - 95, The code sets didSendInitialPrompt = true before the initial LLM query and never clears it on failure or reset, preventing the bootstrap prompt from retrying; change the flow so didSendInitialPrompt is only set to true after try await healthDataInterpreter.queryLLM(...) completes successfully, and ensure the reset action (healthDataInterpreter.resetChat()) also resets didSendInitialPrompt to false so subsequent resets can trigger the bootstrap; update both the initial bootstrap block and the analogous block at lines 106-111 to follow this pattern and preserve existing error handling (showErrorAlert / errorMessage) when queryLLM throws.HealthyLLM/HealthyLLM/HealthContextGenerator.swift-27-31 (1)
27-31:⚠️ Potential issue | 🟡 MinorFix the current SwiftLint failures before merge.
The chain on Lines 29-31 violates
multiline_function_chains, and Line 61 exceeds the configured max line length. CI will stay red until these are reformatted.Also applies to: 57-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HealthyLLM/HealthyLLM/HealthContextGenerator.swift` around lines 27 - 31, The multiline function chain on the prompt construction using electrocardiograms.enumerated().map { … }.joined(...) and the long line later violate SwiftLint; to fix, break the chain into intermediate lets (e.g., let ecgEntries = electrocardiograms.enumerated().map { index, sample in formatECGSample(sample, index: index + 1) } and then prompt += ecgEntries.joined(separator: "\n\n")), and wrap/line-break any long string concatenations so no line (including the call to formatECGSample or the prompt += lines around the ECG block and the similar block at lines 57-64) exceeds the configured max length; keep references to electrocardiograms, formatECGSample, and prompt when refactoring so behavior is unchanged.src-swift/Tests/OpenTSLMKitTests/TransformerCNNEncoderTests.swift-128-131 (1)
128-131:⚠️ Potential issue | 🟡 MinorError message references wrong file extension.
The error message says
"encoder_io.npz"but the code loads"encoder_io.safetensors"(line 113). This could cause confusion during debugging.📝 Proposed fix
guard let refInput = io["input"], let refOutput = io["output"] else { - XCTFail("encoder_io.npz missing 'input' or 'output' key") + XCTFail("encoder_io.safetensors missing 'input' or 'output' key") return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-swift/Tests/OpenTSLMKitTests/TransformerCNNEncoderTests.swift` around lines 128 - 131, The XCTFail message in the guard (guard let refInput = io["input"], let refOutput = io["output"]) incorrectly references "encoder_io.npz" while the test loads "encoder_io.safetensors"; update the failure string to match the actual resource name (or use the same filename variable/constant used when loading the file) so the error message correctly says "encoder_io.safetensors" (or the loaded filename) in TransformerCNNEncoderTests.swift.src-swift/Sources/OpenTSLMRunner/main.swift-76-93 (1)
76-93:⚠️ Potential issue | 🟡 MinorInconsistent indentation in help text.
Lines 87-89 have excessive leading whitespace compared to other options, which will display poorly in the terminal.
📝 Proposed fix
--hidden-size <int> Projector output dimension - --healthkit-ecg-json <path> HealthKit ECG JSON file for direct inference - --hardcoded-ecg Use deterministic hardcoded ECG sample - --hardcoded-ecg-length <int> Hardcoded ECG sample length + --healthkit-ecg-json <path> HealthKit ECG JSON file for direct inference + --hardcoded-ecg Use deterministic hardcoded ECG sample + --hardcoded-ecg-length <int> Hardcoded ECG sample length --help Show this help🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-swift/Sources/OpenTSLMRunner/main.swift` around lines 76 - 93, The help text in printHelpAndExit contains inconsistent indentation for the HealthKit/hardcoded ECG options (the lines starting with "--healthkit-ecg-json", "--hardcoded-ecg", "--hardcoded-ecg-length") causing misaligned display; update the multiline string in printHelpAndExit to remove the excess leading spaces so those options align with the other "--..." entries (match the same two-space indentation used for "--encoder", "--projector", etc.), preserve existing wording and line breaks, and keep Foundation.exit(0) unchanged.HealthyLLM/HealthyLLM/HealthyLLMView.swift-121-135 (1)
121-135:⚠️ Potential issue | 🟡 Minor
localModelSourceExists()only checks directory existence, not contents.Unlike
HealthDataInterpreter.resolveLocalModelSourceDirectory()(context snippet 2, lines 124-144), this method doesn't verify that the required model files exist within the directory. A directory could exist but be empty or incomplete.Additionally, the
NSStringusage on line 125 is flagged by SwiftLint as a legacy Objective-C type.♻️ Proposed improvement
private func localModelSourceExists() -> Bool { let fileManager = FileManager.default if let overridePath = Constants.localModelSourcePathOverride, - fileManager.fileExists(atPath: NSString(string: overridePath).expandingTildeInPath) { - return true + fileManager.fileExists(atPath: (overridePath as NSString).expandingTildeInPath) { + // Consider also checking for required files within this directory + return true } if let bundledLocalModelURL = Bundle.main.resourceURL? .appendingPathComponent(Constants.localModelBundleSubdirectory, isDirectory: true) { return fileManager.fileExists(atPath: bundledLocalModelURL.path) } return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HealthyLLM/HealthyLLM/HealthyLLMView.swift` around lines 121 - 135, localModelSourceExists() only checks that a directory exists and uses a SwiftLint-flagged NSString initializer; update it to validate that the directory actually contains the required model files (reuse the same file-name checks/logic used by HealthDataInterpreter.resolveLocalModelSourceDirectory() to verify presence of required model artifacts) and replace the NSString(string: overridePath).expandingTildeInPath call with the Swift-idiomatic (overridePath as NSString).expandingTildeInPath (or equivalent URL-based expansion) to satisfy SwiftLint.
🧹 Nitpick comments (8)
HealthyLLM/Onboarding/DownloadLLM.swift (1)
19-19: Consider localizing this user-facing string.Line 19 is hardcoded English text; using localized resources will avoid blocking future localization work.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HealthyLLM/Onboarding/DownloadLLM.swift` at line 19, The hardcoded user-facing string assigned to downloadDescription should be localized: replace the literal "Download the \(Constants.llmModelName) model from Hugging Face." with a localized format lookup (e.g., NSLocalizedString or SwiftUI LocalizedStringKey) and use Constants.llmModelName as the format argument; add a corresponding key like "download_llm_description" = "Download the %@ model from Hugging Face." to Localizable.strings so DownloadLLM's downloadDescription returns the localized, formatted string.HealthyLLM/Supporting Files/Localizable.xcstrings (1)
44-50: Consolidate to one canonical download-description key.You now have a new extracted key plus a stale legacy key for the same user-facing sentence. Keeping both long-term can cause localization drift and duplicate translator work.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HealthyLLM/Supporting` Files/Localizable.xcstrings around lines 44 - 50, Consolidate the duplicate localization entries by keeping a single canonical key for the download description and removing the stale legacy key: choose the human-readable key "Download the %@ model from Hugging Face." (or vice‑versa if you prefer the constantized form) and delete "DOWNLOAD_MODEL_DESCRIPTION `%@`"; then update all code references that use DOWNLOAD_MODEL_DESCRIPTION to use the chosen canonical key (search for occurrences of DOWNLOAD_MODEL_DESCRIPTION and replace them), and preserve the existing "comment" field and isCommentAutoGenerated or move its content to the remaining entry so translators retain context.src-swift/Sources/OpenTSLMKit/SleepEDFDataset.swift (2)
137-161: Dictionary iteration order may affect reproducibility.
groups.valuesiteration order is not guaranteed to be stable across Swift versions or platforms. While each label group is independently shuffled and split, the order in which groups are appended toselectedcould vary, affecting the final array order. If deterministic cross-platform reproducibility is required, consider sorting the dictionary keys before iteration.♻️ Proposed fix for reproducible iteration
- for labelRows in groups.values { + for key in groups.keys.sorted() { + let labelRows = groups[key]! var shuffled = labelRows🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-swift/Sources/OpenTSLMKit/SleepEDFDataset.swift` around lines 137 - 161, The iteration over groups.values is nondeterministic and can change the final selected order; update the loop to iterate over a stable ordering by sorting the dictionary keys (e.g., let sortedKeys = groups.keys.sorted()) and then for each key use groups[key]! to get labelRows before shuffling/splitting. Keep the existing shuffling/splitting logic (shuffled, train/validation/test, selected.append(contentsOf: ...)) but perform it in the deterministic loop over sortedKeys so selected order is reproducible across runs/platforms.
77-98: Redundant mean/std computation.
convertRowcomputes mean and variance for the prompt text (lines 81-86), butzNormalize(called on line 79) also computes them internally. Consider refactoringzNormalizeto return both the normalized values and the statistics, avoiding the double computation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-swift/Sources/OpenTSLMKit/SleepEDFDataset.swift` around lines 77 - 98, convertRow currently calls zNormalize(rawSeries) and then recomputes mean/variance/std redundantly; refactor zNormalize to return both the normalized series and the computed statistics (mean and std), update convertRow to call the new zNormalize signature (e.g. let (normalized, mean, std) = zNormalize(rawSeries)), remove the local mean/variance/std calculation, and use the returned mean/std when constructing the OpenTSLMSPSample.timeSeriesText so the prompt shows the exact stats used for normalization; also update any other call sites of zNormalize to the new return type or provide a compatibility wrapper if needed.HealthyLLM/HealthyLLM/HealthyLLMView.swift (1)
10-12: Fix import sort order (SwiftLint).SwiftLint flags that imports should be sorted alphabetically.
📝 Proposed fix
import SwiftUI -import SpeziHealthKit import Hub +import SpeziHealthKit import SpeziLLMLocalDownload🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HealthyLLM/HealthyLLM/HealthyLLMView.swift` around lines 10 - 12, The import statements at the top of HealthyLLMView.swift are not alphabetized; reorder the imports so they follow SwiftLint's alphabetical rule (e.g., place import Hub, import SpeziHealthKit, import SpeziLLMLocalDownload in alphabetical order) to satisfy SwiftLint in the file where the imports are declared.src-swift/Sources/OpenTSLMRunner/main.swift (1)
7-9: Default relative paths may break depending on working directory.The default paths (
"../checkpoints/...","../data/...") assume the runner is executed from a specific subdirectory. Consider documenting the expected working directory in the help text, or detecting/warning when files don't exist at the default locations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-swift/Sources/OpenTSLMRunner/main.swift` around lines 7 - 9, The default relative paths (encoderPath, projectorPath, csvPath in main.swift) can break if the process is started from a different working directory; update main.swift to (1) add a short help/usage message that documents the expected working directory or that absolute paths are accepted, and (2) validate the three paths at startup using FileManager.default.fileExists(atPath:) and emit a clear warning/error (including the missing path) if any file is not found, returning a non-zero exit code or prompting the user to supply correct paths; reference encoderPath, projectorPath, and csvPath when implementing these checks and messages.src-swift/Sources/OpenTSLMKit/OpenTSLMSPPipeline.swift (1)
51-80: Consider handling edge case where all series are empty.If
seriesBatchcontains only empty arrays (e.g.,[[], []]),paddedLengthreturns 0, and the encoder receives an input with shape[N, 0]. This could cause unexpected behavior depending on the encoder implementation.🛡️ Optional guard for empty series
public func projectTimeSeries(_ seriesBatch: [[Float]]) -> [MLXArray] { guard !seriesBatch.isEmpty else { return [] } let maxLength = paddedLength(for: seriesBatch) + guard maxLength > 0 else { return [] } + var paddedRows: [[Float]] = []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-swift/Sources/OpenTSLMKit/OpenTSLMSPPipeline.swift` around lines 51 - 80, projectTimeSeries doesn't handle the case where every series is empty (paddedLength == 0) and will call encoder with a [N, 0] input; add an early guard after computing maxLength that checks if maxLength == 0 and in that case return an array of empty MLXArray instances (one per input series) instead of proceeding to build the flat input and calling encoder/projector. Reference the functions/values: projectTimeSeries, paddedLength, MLXArray(converting:), encoder, and projector when locating where to add the guard.HealthyLLM/HealthyLLM/OpenTSLM/TransformerCNNEncoder.swift (1)
11-123: Avoid maintaining a second encoder implementation.This file mirrors
src-swift/Sources/OpenTSLMKit/TransformerCNNEncoder.swift. Keeping both copies in sync will make every shape or weight-loading fix a two-file change. If the app target can depend onOpenTSLMKit, prefer importing the package type here instead of duplicating the implementation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HealthyLLM/HealthyLLM/OpenTSLM/TransformerCNNEncoder.swift` around lines 11 - 123, This file duplicates the TransformerCNNEncoder/TransformerEncoderLayer implementation already present in OpenTSLMKit; remove the local classes (TransformerCNNEncoder and TransformerEncoderLayer and related loadWeights extension) and instead import and reuse the shared implementation from OpenTSLMKit (or add a small wrapper/typealias that forwards to OpenTSLMKit.TransformerCNNEncoder) so there’s a single source of truth for class names like TransformerCNNEncoder, TransformerEncoderLayer and the loadWeights API; update any local references to use the imported type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@HealthyLLM.xcodeproj/project.pbxproj`:
- Line 571: The HealthyLLMStudy target is using the same
PRODUCT_BUNDLE_IDENTIFIER value as HealthyLLM; update the
PRODUCT_BUNDLE_IDENTIFIER for the HealthyLLMStudy target (both its Debug and
Release build configurations) to a distinct reverse-DNS string (e.g., change
edu.maxrosenblattl.bdhg.healthyllm.co to something like
edu.maxrosenblattl.bdhg.healthyllm.study) so the HealthyLLMStudy entries no
longer collide with HealthyLLM in install/signing/distribution flows; ensure you
update each occurrence of PRODUCT_BUNDLE_IDENTIFIER for the HealthyLLMStudy
target in the project.pbxproj so all build configurations reflect the new
identifier.
In `@HealthyLLM/HealthyLLM/HealthContextGenerator.swift`:
- Around line 22-34: The prompt currently embeds full userInfo JSON and detailed
ECG samples (userInfo.asJSONRepresentation and electrocardiograms.map {
formatECGSample(...) }) which turns logs into PHI/PII when HealthDataInterpreter
logs the assembled context; change the assembly to include only a
redacted/sanitized summary (e.g., userInfo.redactedSummary() containing
non-identifiable fields or hashed IDs and allowed demographics) and replace full
ECG sample serialization with an aggregated/summary representation (e.g.,
ecgSummary = summarizeECGSamples(electrocardiograms) returning count, timestamps
range, and flagged abnormalities) or call a new formatECGSampleSummary instead
of formatECGSample, and ensure the code that logs the context (the variable
named context in HealthDataInterpreter.swift) either logs the redacted context
or stops logging sensitive fields (remove or downgrade to debug and redact PHI)
so no raw JSON/metadata is written to logs.
- Around line 48-64: The prompt currently formats mean/min/max and
normalizedPreview using 0 when sample.voltages is empty, fabricating a flat
trace; update the code that builds normalizedVoltages/normalizedPreview and the
sleep_cot_style_sample block to treat empty or nil sample.voltages as missing
data: use optional checks on average/minimum/maximum (and normalizedVoltages)
and emit "No Data" (or an explicit "missing ECG data" token) instead of
String(format: "%.6f", 0), and ensure normalizedPreview is either an explicit
"No Data" string when voltages.isEmpty or constructed only from existing values;
change references to voltageMeanText/voltageMinText/voltageMaxText (and
time_series_normalized_preview) so the prompt interpolates those safe "No Data"
placeholders rather than zeros.
In `@HealthyLLM/HealthyLLM/HealthDataInterpreter.swift`:
- Around line 222-225: The current branch in HealthDataInterpreter where
Constants.includeHardcodedECGSample is true inserts hardcodedECGSample() at
index 0 (making it appear as the "latest" ECG); change this so the synthetic
sample is appended to the fetched array (e.g.,
merged.append(hardcodedECGSample())) or gate the insertion behind a debug-only
flag so real HealthKit data remains first; update the logic around the merged
variable in the same conditional that currently calls hardcodedECGSample() and
ensure HealthContextGenerator.buildSystemPrompt() continues to label the list
accurately as "Latest ECG samples".
- Around line 393-401: The system prompt is being appended after the user
message, weakening its precedence; when no .system exists, build the
systemPrompt (via healthDataFetcher.fetchUser, ecgSamplesForPrompt,
healthContextGenerator.buildSystemPrompt) and insert/prepend it at the front
(index 0) of both context and advancedContext instead of using append so the
.system role precedes user content for the initial queryLLM call.
In `@HealthyLLM/HealthyLLM/HealthyLLMChatView.swift`:
- Around line 39-45: The setter currently drops a completed user prompt when
isSubmittingPrompt is true; fix it by introducing a pending prompt buffer (e.g.,
pendingUserPrompt or pendingUserPromptQueue) and, instead of silently returning
in the guard that checks userPrompt, lastSubmittedUserMessageID, and
isSubmittingPrompt, enqueue the userPrompt when isSubmittingPrompt is true.
Ensure lastSubmittedUserMessageID is still checked/updated when you actually
start processing a prompt, and update the submission-complete path (the code
that sets isSubmittingPrompt = false or calls finishSubmission) to check the
pending buffer/queue and dequeue the next prompt to process immediately. Use the
existing symbols userPrompt, lastSubmittedUserMessageID, isSubmittingPrompt, and
the setter that inspects newValue.last to locate where to add the buffer/enqueue
logic and where to trigger processing after completion.
In `@HealthyLLM/HealthyLLM/HealthyLLMView.swift`:
- Around line 115-119: localModelExists() currently only checks for
"model.safetensors" which can produce false positives because
HealthDataInterpreter.requiredModelFiles expects five files; update
localModelExists() to verify all requiredModelFiles exist in the repo directory
(use HubApi().localRepoLocation(.init(id: Constants.llmModelName)) and check
each filename from HealthDataInterpreter.requiredModelFiles) so the view only
skips download when every required file is present and
healthDataInterpreter.setup() won’t fail.
In `@HealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMInferenceService.swift`:
- Around line 88-104: runECGSampleInference currently only formats ECG data and
never touches the model stack so the /opentslm-ecg-sample route can report
success even when encoder/projector assets are missing; update
runECGSampleInference to follow the same projection path as the sleep sample:
after makeOpenTSLMSample(from:), construct OpenTSLMSPPipeline (load checkpoints
/ projector assets), invoke the pipeline’s projection method on the sample (or
the same function used by the sleep sample), and include any projection/output
in the formatted report while propagating errors when pipeline construction or
projection fails; alternatively, if you intend this to be purely a preview,
rename runECGSampleInference to indicate it is a preview/formatter and ensure
the route does not claim full model inference success.
In `@HealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMSPPipeline.swift`:
- Around line 1-94: The file duplicates OpenTSLMSPPipeline/OpenTSLMSPSample code
across packages; remove the copy and have HealthyLLM reuse the implementation
from OpenTSLMKit by adding a dependency on OpenTSLMKit and importing it where
OpenTSLMSPPipeline, OpenTSLMSPSample, projectTimeSeries, and projectSample are
referenced; delete the duplicated OpenTSLMSPPipeline.swift in HealthyLLM, update
HealthyLLM's Package.swift to depend on OpenTSLMKit, and adjust any call sites
to import OpenTSLMKit instead of the local types (ensuring public visibility of
OpenTSLMSPPipeline/OpenTSLMSPSample in OpenTSLMKit).
In `@HealthyLLM/HealthyLLM/OpenTSLM/SleepEDFDataset.swift`:
- Around line 1-239: This file duplicates the SleepEDFDataset implementation;
remove duplication by deleting this copy and make HealthyLLM depend on the
existing implementation in OpenTSLMKit: remove
HealthyLLM/HealthyLLM/OpenTSLM/SleepEDFDataset.swift, add OpenTSLMKit as a local
package dependency in HealthyLLM's Package.swift, import OpenTSLMKit where
SleepEDFDataset is used, and update any references (e.g., SleepEDFDataset,
OpenTSLMSPSample, SeededGenerator, parseCSV, parseTimeSeries, zNormalize,
stratifiedSplit) to use the single canonical implementation ensuring the
original symbols remain public if needed.
In `@HealthyLLM/Shared` Context/Constants.swift:
- Line 24: The includeHardcodedECGSample default is currently enabled by falling
back to "1"; change the fallback to "0" so synthetic ECGs are opt-in. Update the
Constants.swift static let includeHardcodedECGSample to compare the environment
variable
(ProcessInfo.processInfo.environment["HEALTHYLLM_INCLUDE_HARDCODED_ECG"])
against "1" but use "0" as the default fallback value, ensuring
includeHardcodedECGSample is false unless the env var is explicitly set to "1".
- Around line 21-22: The constant hostHuggingFaceCacheRoot currently falls back
to a hard-coded user path; change it to derive the home directory dynamically or
return nil by default so resolveLocalModelSourceDirectory() doesn't rely on a
workstation-specific path—e.g., build the fallback using
FileManager.default.homeDirectoryForCurrentUser.path (or NSHomeDirectory()) +
"/.cache/huggingface/hub", or make hostHuggingFaceCacheRoot optional and handle
the nil case in resolveLocalModelSourceDirectory() to avoid shipping a
user-specific default.
In `@HealthyLLM/Supporting` Files/LocalLLM/generation_config.json:
- Around line 1-9: The runtime is ignoring generation_config.json and using
hardcoded values in HealthyLLM/HealthyLLM/HealthDataInterpreter.swift
(temperature=0.001, topP=1.0) causing config drift; update HealthDataInterpreter
to read and parse Supporting Files/LocalLLM/generation_config.json at startup
(or add a loader function) and use the parsed "temperature" and "top_p" values
when constructing the generation request (replace the hardcoded temperature/topP
usage), with sensible fallbacks if keys are missing; alternatively, centralize
generation settings into a single config accessor used by the code that prepares
the model request so Temperature and TopP are sourced from the parsed config
rather than hardcoded constants.
In `@src-swift/Package.swift`:
- Around line 11-13: The Package.swift dependency declaration currently uses
.package(..., from: "0.21.2") which can float across 0.x minors; change the
constraint to .upToNextMinor(from: "0.21.2") for the mlx-swift package so the
resolver will only pick versions >=0.21.2 and <0.22.0; update the entry in the
dependencies array where mlx-swift is declared to use .upToNextMinor instead of
.package(..., from:).
In `@src-swift/Sources/OpenTSLMKit/HealthKitECGSample.swift`:
- Around line 82-84: The interpolation inside timeSeriesText uses escaped quotes
(\"%.6f\") which is invalid inside the \(...) expression; update the
String(format:) calls in HealthKitECGSample.timeSeriesText to use normal quotes
(e.g. "%.6f") for the format strings so the expressions compile (locate the
String(format: \"%.6f\", mean) and String(format: \"%.6f\", std) usages and
remove the backslashes).
In `@src-swift/Sources/OpenTSLMKit/SoftPromptInterleaver.swift`:
- Around line 17-19: The computed property length in SoftPromptInterleaver
currently converts attentionMask.sum() into an Int and treats that as a prefix
length, which wrongly assumes masks are contiguous prefixes (see usage in the
interleave logic around attentionMask and the slice at lines where 0..<length is
used); instead either validate the attentionMask is a prefix-style mask inside
SoftPromptInterleaver (e.g., assert or throw if any 0 appears before a 1) or
change the interleaving logic to preserve and use the original attentionMask
vector values rather than converting to a scalar length — locate the length
property and the code that slices 0..<length and replace the scalar-length
approach with mask-preserving logic or add explicit validation of prefix-binary
masks before slicing so masked positions cannot leak back into the batch.
In `@src-swift/Sources/OpenTSLMKit/TransformerCNNEncoder.swift`:
- Around line 96-98: The positional-embedding slice can index past the learned
table when x.dim(1) (n) is larger than posEmbed.dim(1); modify the forward logic
in TransformerCNNEncoder (where x and posEmbed are used) to guard against this
by computing let n = x.dim(1); let m = min(n, posEmbed.dim(1)); use
posEmbed[0..., 0 ..< m, 0...] for the slice and if m < n pad the remaining (n -
m) positions (e.g., zeros or by repeating/tiling posEmbed slices) so x +
posEmbed works for all n, or alternatively add a preconditionFailure with a
clear message referencing x and posEmbed to fail early; apply the same
guard/strategy to the app copy too.
In `@src-swift/Tests/OpenTSLMKitTests/TransformerCNNEncoderTests.swift`:
- Around line 16-21: Replace the source-path-based fixturesURL with a
bundle-based lookup so tests use SPM-copied resources: update the fixturesURL
computed property in TransformerCNNEncoderTests to use Bundle.module (e.g.
Bundle.module.resourceURL?.appendingPathComponent("Fixtures") or
Bundle.module.url(forResource: "Fixtures", withExtension: nil)) instead of
constructing a path from `#filePath`, ensuring the test loads resources from the
test bundle at runtime.
---
Minor comments:
In `@HealthyLLM/HealthyLLM/Fetcher/HealthDataFetcher.swift`:
- Around line 20-21: Replace the incorrect HKSeriesType.workoutType() call with
HKObjectType.workoutType() in the HealthDataFetcher where the types array is
constructed (replace the HKSeriesType reference with HKObjectType); also ensure
the electrocardiogramType() usage is guarded by or documented with an iOS
version availability check (iOS 14.0+) so calls to
HKObjectType.electrocardiogramType() are only used on supported deployments or
wrapped with an availability check.
In `@HealthyLLM/HealthyLLM/HealthContextGenerator.swift`:
- Around line 27-31: The multiline function chain on the prompt construction
using electrocardiograms.enumerated().map { … }.joined(...) and the long line
later violate SwiftLint; to fix, break the chain into intermediate lets (e.g.,
let ecgEntries = electrocardiograms.enumerated().map { index, sample in
formatECGSample(sample, index: index + 1) } and then prompt +=
ecgEntries.joined(separator: "\n\n")), and wrap/line-break any long string
concatenations so no line (including the call to formatECGSample or the prompt
+= lines around the ECG block and the similar block at lines 57-64) exceeds the
configured max length; keep references to electrocardiograms, formatECGSample,
and prompt when refactoring so behavior is unchanged.
In `@HealthyLLM/HealthyLLM/HealthyLLMChatView.swift`:
- Around line 82-95: The code sets didSendInitialPrompt = true before the
initial LLM query and never clears it on failure or reset, preventing the
bootstrap prompt from retrying; change the flow so didSendInitialPrompt is only
set to true after try await healthDataInterpreter.queryLLM(...) completes
successfully, and ensure the reset action (healthDataInterpreter.resetChat())
also resets didSendInitialPrompt to false so subsequent resets can trigger the
bootstrap; update both the initial bootstrap block and the analogous block at
lines 106-111 to follow this pattern and preserve existing error handling
(showErrorAlert / errorMessage) when queryLLM throws.
In `@HealthyLLM/HealthyLLM/HealthyLLMView.swift`:
- Around line 121-135: localModelSourceExists() only checks that a directory
exists and uses a SwiftLint-flagged NSString initializer; update it to validate
that the directory actually contains the required model files (reuse the same
file-name checks/logic used by
HealthDataInterpreter.resolveLocalModelSourceDirectory() to verify presence of
required model artifacts) and replace the NSString(string:
overridePath).expandingTildeInPath call with the Swift-idiomatic (overridePath
as NSString).expandingTildeInPath (or equivalent URL-based expansion) to satisfy
SwiftLint.
In `@src-swift/Sources/OpenTSLMRunner/main.swift`:
- Around line 76-93: The help text in printHelpAndExit contains inconsistent
indentation for the HealthKit/hardcoded ECG options (the lines starting with
"--healthkit-ecg-json", "--hardcoded-ecg", "--hardcoded-ecg-length") causing
misaligned display; update the multiline string in printHelpAndExit to remove
the excess leading spaces so those options align with the other "--..." entries
(match the same two-space indentation used for "--encoder", "--projector",
etc.), preserve existing wording and line breaks, and keep Foundation.exit(0)
unchanged.
In `@src-swift/Tests/OpenTSLMKitTests/TransformerCNNEncoderTests.swift`:
- Around line 128-131: The XCTFail message in the guard (guard let refInput =
io["input"], let refOutput = io["output"]) incorrectly references
"encoder_io.npz" while the test loads "encoder_io.safetensors"; update the
failure string to match the actual resource name (or use the same filename
variable/constant used when loading the file) so the error message correctly
says "encoder_io.safetensors" (or the loaded filename) in
TransformerCNNEncoderTests.swift.
---
Nitpick comments:
In `@HealthyLLM/HealthyLLM/HealthyLLMView.swift`:
- Around line 10-12: The import statements at the top of HealthyLLMView.swift
are not alphabetized; reorder the imports so they follow SwiftLint's
alphabetical rule (e.g., place import Hub, import SpeziHealthKit, import
SpeziLLMLocalDownload in alphabetical order) to satisfy SwiftLint in the file
where the imports are declared.
In `@HealthyLLM/HealthyLLM/OpenTSLM/TransformerCNNEncoder.swift`:
- Around line 11-123: This file duplicates the
TransformerCNNEncoder/TransformerEncoderLayer implementation already present in
OpenTSLMKit; remove the local classes (TransformerCNNEncoder and
TransformerEncoderLayer and related loadWeights extension) and instead import
and reuse the shared implementation from OpenTSLMKit (or add a small
wrapper/typealias that forwards to OpenTSLMKit.TransformerCNNEncoder) so there’s
a single source of truth for class names like TransformerCNNEncoder,
TransformerEncoderLayer and the loadWeights API; update any local references to
use the imported type.
In `@HealthyLLM/Onboarding/DownloadLLM.swift`:
- Line 19: The hardcoded user-facing string assigned to downloadDescription
should be localized: replace the literal "Download the \(Constants.llmModelName)
model from Hugging Face." with a localized format lookup (e.g.,
NSLocalizedString or SwiftUI LocalizedStringKey) and use Constants.llmModelName
as the format argument; add a corresponding key like "download_llm_description"
= "Download the %@ model from Hugging Face." to Localizable.strings so
DownloadLLM's downloadDescription returns the localized, formatted string.
In `@HealthyLLM/Supporting` Files/Localizable.xcstrings:
- Around line 44-50: Consolidate the duplicate localization entries by keeping a
single canonical key for the download description and removing the stale legacy
key: choose the human-readable key "Download the %@ model from Hugging Face."
(or vice‑versa if you prefer the constantized form) and delete
"DOWNLOAD_MODEL_DESCRIPTION `%@`"; then update all code references that use
DOWNLOAD_MODEL_DESCRIPTION to use the chosen canonical key (search for
occurrences of DOWNLOAD_MODEL_DESCRIPTION and replace them), and preserve the
existing "comment" field and isCommentAutoGenerated or move its content to the
remaining entry so translators retain context.
In `@src-swift/Sources/OpenTSLMKit/OpenTSLMSPPipeline.swift`:
- Around line 51-80: projectTimeSeries doesn't handle the case where every
series is empty (paddedLength == 0) and will call encoder with a [N, 0] input;
add an early guard after computing maxLength that checks if maxLength == 0 and
in that case return an array of empty MLXArray instances (one per input series)
instead of proceeding to build the flat input and calling encoder/projector.
Reference the functions/values: projectTimeSeries, paddedLength,
MLXArray(converting:), encoder, and projector when locating where to add the
guard.
In `@src-swift/Sources/OpenTSLMKit/SleepEDFDataset.swift`:
- Around line 137-161: The iteration over groups.values is nondeterministic and
can change the final selected order; update the loop to iterate over a stable
ordering by sorting the dictionary keys (e.g., let sortedKeys =
groups.keys.sorted()) and then for each key use groups[key]! to get labelRows
before shuffling/splitting. Keep the existing shuffling/splitting logic
(shuffled, train/validation/test, selected.append(contentsOf: ...)) but perform
it in the deterministic loop over sortedKeys so selected order is reproducible
across runs/platforms.
- Around line 77-98: convertRow currently calls zNormalize(rawSeries) and then
recomputes mean/variance/std redundantly; refactor zNormalize to return both the
normalized series and the computed statistics (mean and std), update convertRow
to call the new zNormalize signature (e.g. let (normalized, mean, std) =
zNormalize(rawSeries)), remove the local mean/variance/std calculation, and use
the returned mean/std when constructing the OpenTSLMSPSample.timeSeriesText so
the prompt shows the exact stats used for normalization; also update any other
call sites of zNormalize to the new return type or provide a compatibility
wrapper if needed.
In `@src-swift/Sources/OpenTSLMRunner/main.swift`:
- Around line 7-9: The default relative paths (encoderPath, projectorPath,
csvPath in main.swift) can break if the process is started from a different
working directory; update main.swift to (1) add a short help/usage message that
documents the expected working directory or that absolute paths are accepted,
and (2) validate the three paths at startup using
FileManager.default.fileExists(atPath:) and emit a clear warning/error
(including the missing path) if any file is not found, returning a non-zero exit
code or prompting the user to supply correct paths; reference encoderPath,
projectorPath, and csvPath when implementing these checks and messages.
🪄 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: 82a2f265-e349-48b7-b105-775620d80e8e
📒 Files selected for processing (47)
.gitignoreHealthyLLM.xcodeproj/project.pbxprojHealthyLLM/HealthyLLM/Fetcher/HealthDataFetcher+Electrocardiogram.swiftHealthyLLM/HealthyLLM/Fetcher/HealthDataFetcher.swiftHealthyLLM/HealthyLLM/HealthContextGenerator.swiftHealthyLLM/HealthyLLM/HealthDataInterpreter.swiftHealthyLLM/HealthyLLM/HealthyLLMChatView.swiftHealthyLLM/HealthyLLM/HealthyLLMView.swiftHealthyLLM/HealthyLLM/Models.swiftHealthyLLM/HealthyLLM/OpenTSLM/MLPProjector.swiftHealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMInferenceService.swiftHealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMSPPipeline.swiftHealthyLLM/HealthyLLM/OpenTSLM/SleepEDFDataset.swiftHealthyLLM/HealthyLLM/OpenTSLM/TransformerCNNEncoder.swiftHealthyLLM/HealthyLLMAppDelegate.swiftHealthyLLM/Onboarding/DownloadLLM.swiftHealthyLLM/Shared Context/Constants.swiftHealthyLLM/Supporting Files/LocalLLM/config.jsonHealthyLLM/Supporting Files/LocalLLM/generation_config.jsonHealthyLLM/Supporting Files/LocalLLM/special_tokens_map.jsonHealthyLLM/Supporting Files/LocalLLM/tokenizer.jsonHealthyLLM/Supporting Files/LocalLLM/tokenizer_config.jsonHealthyLLM/Supporting Files/Localizable.xcstringsHealthyLLM/Supporting Files/OpenTSLM/mlx-checkpoint.encoder.safetensorsHealthyLLM/Supporting Files/OpenTSLM/mlx-checkpoint.projector.safetensorsREADME.mdjsons/config.jsonjsons/generation_config.jsonjsons/special_tokens_map.jsonjsons/tokenizer.jsonjsons/tokenizer_config.jsonsrc-swift/Package.resolvedsrc-swift/Package.swiftsrc-swift/Sources/OpenTSLMKit/HealthKitECGSample.swiftsrc-swift/Sources/OpenTSLMKit/MLPProjector.swiftsrc-swift/Sources/OpenTSLMKit/OpenTSLMSPPipeline.swiftsrc-swift/Sources/OpenTSLMKit/SleepEDFDataset.swiftsrc-swift/Sources/OpenTSLMKit/SoftPromptInterleaver.swiftsrc-swift/Sources/OpenTSLMKit/TransformerCNNEncoder.swiftsrc-swift/Sources/OpenTSLMRunner/main.swiftsrc-swift/Tests/OpenTSLMKitTests/Fixtures/encoder_io.npzsrc-swift/Tests/OpenTSLMKitTests/Fixtures/encoder_io.safetensorssrc-swift/Tests/OpenTSLMKitTests/Fixtures/encoder_weights.safetensorssrc-swift/Tests/OpenTSLMKitTests/MLPProjectorTests.swiftsrc-swift/Tests/OpenTSLMKitTests/SoftPromptInterleaverTests.swiftsrc-swift/Tests/OpenTSLMKitTests/TransformerCNNEncoderTests.swiftsrc-swift/default.metallib
Comment on lines
+48
to
+64
| let normalizedVoltages = zNormalize(voltages) | ||
| let normalizedPreview = normalizedVoltages.prefix(256).map { String(format: "%.6f", $0) }.joined(separator: ", ") | ||
|
|
||
| let averageHeartRateText = sample.averageHeartRate.map(String.init(describing:)) ?? "No Data" | ||
| let samplingFrequencyText = sample.samplingFrequency.map(String.init(describing:)) ?? "No Data" | ||
| let voltageMeanText = average.map(String.init(describing:)) ?? "No Data" | ||
| let voltageMinText = minimum.map(String.init(describing:)) ?? "No Data" | ||
| let voltageMaxText = maximum.map(String.init(describing:)) ?? "No Data" | ||
|
|
||
| let sleepCotStylePrompt = """ | ||
| sleep_cot_style_sample: | ||
| pre_prompt: You are given a short single-lead ECG time series segment. Analyze rhythm, signal quality, and notable concerns conservatively. | ||
| time_series_text: | ||
| - The following is the ECG time series with mean \(String(format: "%.6f", average ?? 0)) and min/max \(String(format: "%.6f", minimum ?? 0))/\(String(format: "%.6f", maximum ?? 0)). | ||
| time_series_normalized_preview: [\(normalizedPreview)] | ||
| post_prompt: First summarize waveform quality and rhythm regularity, then provide brief safety guidance and when to seek care. | ||
| """ |
There was a problem hiding this comment.
Treat missing voltage arrays as missing data, not zeros.
When sample.voltages is empty, Line 61 still emits mean/min/max as 0.000000 with an empty preview. That fabricates a flat trace and can push the model toward a bogus safety interpretation instead of recognizing that the ECG payload is missing.
🩺 Safer handling for empty ECG samples
- let sleepCotStylePrompt = """
- sleep_cot_style_sample:
- pre_prompt: You are given a short single-lead ECG time series segment. Analyze rhythm, signal quality, and notable concerns conservatively.
- time_series_text:
- - The following is the ECG time series with mean \(String(format: "%.6f", average ?? 0)) and min/max \(String(format: "%.6f", minimum ?? 0))/\(String(format: "%.6f", maximum ?? 0)).
- time_series_normalized_preview: [\(normalizedPreview)]
- post_prompt: First summarize waveform quality and rhythm regularity, then provide brief safety guidance and when to seek care.
- """
+ let sleepCotStylePrompt: String
+ if voltages.isEmpty {
+ sleepCotStylePrompt = """
+ sleep_cot_style_sample:
+ time_series_text:
+ - No voltage samples available.
+ """
+ } else {
+ sleepCotStylePrompt = """
+ sleep_cot_style_sample:
+ pre_prompt: You are given a short single-lead ECG time series segment. Analyze rhythm, signal quality, and notable concerns conservatively.
+ time_series_text:
+ - The following is the ECG time series with mean \(String(format: "%.6f", average!)) and min/max \(String(format: "%.6f", minimum!))/\(String(format: "%.6f", maximum!)).
+ time_series_normalized_preview: [\(normalizedPreview)]
+ post_prompt: First summarize waveform quality and rhythm regularity, then provide brief safety guidance and when to seek care.
+ """
+ }🧰 Tools
🪛 GitHub Check: SwiftLint / SwiftLint
[failure] 59-59:
Line Length Violation: Line should be 150 characters or less; currently it has 159 characters (line_length)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@HealthyLLM/HealthyLLM/HealthContextGenerator.swift` around lines 48 - 64, The
prompt currently formats mean/min/max and normalizedPreview using 0 when
sample.voltages is empty, fabricating a flat trace; update the code that builds
normalizedVoltages/normalizedPreview and the sleep_cot_style_sample block to
treat empty or nil sample.voltages as missing data: use optional checks on
average/minimum/maximum (and normalizedVoltages) and emit "No Data" (or an
explicit "missing ECG data" token) instead of String(format: "%.6f", 0), and
ensure normalizedPreview is either an explicit "No Data" string when
voltages.isEmpty or constructed only from existing values; change references to
voltageMeanText/voltageMinText/voltageMaxText (and
time_series_normalized_preview) so the prompt interpolates those safe "No Data"
placeholders rather than zeros.
Comment on lines
+11
to
+13
| dependencies: [ | ||
| .package(url: "https://github.com/ml-explore/mlx-swift", from: "0.21.2"), | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Swift Package Manager, what versions can .package(url: "...", from: "0.21.2")resolve to when the dependency is still on major version 0, and doesmlx-swift document compatibility guarantees before 1.0?
💡 Result:
For Swift Package Manager, .package(url: "...", from: "0.21.2") resolves to any tagged version >= 0.21.2 with the same major version 0 (i.e., 0.y.z where y.z >= 0.21.2), such as 0.21.2, 0.21.3, 0.22.0, up to but not including 1.0.0. This follows Semantic Versioning (SemVer) rules where "from" is equivalent to up-to-next-major, preventing automatic updates across major versions to avoid breaking changes. mlx-swift is still on major version 0 (latest releases include 0.31.3 as of 2026-04-01), so it would resolve to the highest satisfying version like 0.31.3. It does not explicitly document compatibility guarantees before 1.0 beyond following MLX Python versioning and standard tagging practices; pre-1.0 changes may include breaking updates within major 0 as per SemVer conventions for initial development.
Citations:
- 1: https://docs.swift.org/package-manager/PackageDescription/PackageDescription.html
- 2: https://developer.apple.com/documentation/packagedescription/package/dependency/package(url:from:traits:)?changes=_3_3
- 3: https://stackoverflow.com/questions/77330024/swift-package-manager-dependency-rules-from-vs-uptonextmajor
- 4: https://blog.eidinger.info/all-options-to-specify-a-swift-package-version
- 5: https://github.com/apple/swift-package-manager
- 6: https://github.com/ml-explore/mlx-swift
- 7: https://github.com/ml-explore/mlx-swift/tags
Constrain mlx-swift more tightly while it is still 0.x.
.package(..., from: "0.21.2") can float to later pre-1.0 releases (up to 0.31.3 as of April 2026) on a fresh resolve. Pre-1.0 packages do not document stable compatibility guarantees and may include breaking changes within major 0. Use .upToNextMinor(from: "0.21.2") for predictable dependency resolution.
📦 Safer constraint
- .package(url: "https://github.com/ml-explore/mlx-swift", from: "0.21.2"),
+ .package(url: "https://github.com/ml-explore/mlx-swift", .upToNextMinor(from: "0.21.2")),📝 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
| dependencies: [ | |
| .package(url: "https://github.com/ml-explore/mlx-swift", from: "0.21.2"), | |
| ], | |
| dependencies: [ | |
| .package(url: "https://github.com/ml-explore/mlx-swift", .upToNextMinor(from: "0.21.2")), | |
| ], |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-swift/Package.swift` around lines 11 - 13, The Package.swift dependency
declaration currently uses .package(..., from: "0.21.2") which can float across
0.x minors; change the constraint to .upToNextMinor(from: "0.21.2") for the
mlx-swift package so the resolver will only pick versions >=0.21.2 and <0.22.0;
update the entry in the dependencies array where mlx-swift is declared to use
.upToNextMinor instead of .package(..., from:).
Comment on lines
+82
to
+84
| timeSeriesText: [ | ||
| "The following is the ECG time series sampled at \(ecg.samplingFrequency)Hz with mean \(String(format: \"%.6f\", mean)) and std \(String(format: \"%.6f\", std))." | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
from pathlib import Path
path = Path("src-swift/Sources/OpenTSLMKit/HealthKitECGSample.swift")
for i, line in enumerate(path.read_text().splitlines(), start=1):
if 80 <= i <= 84:
print(f"{i}: {line!r}")
PYRepository: StanfordBDHG/HealthyLLM
Length of output: 1812
🏁 Script executed:
cd /tmp && git clone --depth 1 https://github.com/StanfordBDHG/HealthyLLM.git 2>&1 | head -20Repository: StanfordBDHG/HealthyLLM
Length of output: 1897
Fix the escaped quotes in the interpolation.
Line 83 uses \"%.6f\" inside Swift string interpolation. Inside \(...) expressions, you are in expression context, not string literal context, so escaped quotes are invalid. This code will not compile.
Proposed fix
- "The following is the ECG time series sampled at \(ecg.samplingFrequency)Hz with mean \(String(format: \"%.6f\", mean)) and std \(String(format: \"%.6f\", std))."
+ "The following is the ECG time series sampled at \(ecg.samplingFrequency)Hz with mean \(String(format: "%.6f", mean)) and std \(String(format: "%.6f", std))."📝 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
| timeSeriesText: [ | |
| "The following is the ECG time series sampled at \(ecg.samplingFrequency)Hz with mean \(String(format: \"%.6f\", mean)) and std \(String(format: \"%.6f\", std))." | |
| ], | |
| timeSeriesText: [ | |
| "The following is the ECG time series sampled at \(ecg.samplingFrequency)Hz with mean \(String(format: "%.6f", mean)) and std \(String(format: "%.6f", std))." | |
| ], |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-swift/Sources/OpenTSLMKit/HealthKitECGSample.swift` around lines 82 - 84,
The interpolation inside timeSeriesText uses escaped quotes (\"%.6f\") which is
invalid inside the \(...) expression; update the String(format:) calls in
HealthKitECGSample.timeSeriesText to use normal quotes (e.g. "%.6f") for the
format strings so the expressions compile (locate the String(format: \"%.6f\",
mean) and String(format: \"%.6f\", std) usages and remove the backslashes).
Comment on lines
+17
to
+19
| var length: Int { | ||
| Int(attentionMask.sum().item(Float.self)) | ||
| } |
There was a problem hiding this comment.
Don't turn arbitrary masks into prefix lengths.
length is derived from sum(), then Line 64 slices 0 ..< length and Line 65 replaces the original mask with all ones. A valid mask like [1, 0, 1] becomes two active tokens, so masked positions can leak back into the batch. Either validate that masks are prefix-binary, or preserve the real mask values when interleaving.
Also applies to: 62-65
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-swift/Sources/OpenTSLMKit/SoftPromptInterleaver.swift` around lines 17 -
19, The computed property length in SoftPromptInterleaver currently converts
attentionMask.sum() into an Int and treats that as a prefix length, which
wrongly assumes masks are contiguous prefixes (see usage in the interleave logic
around attentionMask and the slice at lines where 0..<length is used); instead
either validate the attentionMask is a prefix-style mask inside
SoftPromptInterleaver (e.g., assert or throw if any 0 appears before a 1) or
change the interleaving logic to preserve and use the original attentionMask
vector values rather than converting to a scalar length — locate the length
property and the code that slices 0..<length and replace the scalar-length
approach with mask-preserving logic or add explicit validation of prefix-binary
masks before slicing so masked positions cannot leak back into the batch.
Comment on lines
+96
to
+98
| let n = x.dim(1) | ||
| // Add positional embeddings sliced to actual patch count | ||
| x = x + posEmbed[0..., 0 ..< n, 0...] |
There was a problem hiding this comment.
Guard the positional-embedding slice against long inputs.
If x.dim(1) exceeds posEmbed.dim(1), Line 98 indexes past the learned table and the forward pass fails at runtime. Add a precondition or resize strategy before the slice. The same guard is needed in the app copy too.
🛡️ Minimal guard
x = patchEmbed(x)
let n = x.dim(1)
+ precondition(n <= posEmbed.dim(1), "Input produced \(n) patches, but maxPatches is only \(posEmbed.dim(1))")
// Add positional embeddings sliced to actual patch count
x = x + posEmbed[0..., 0 ..< n, 0...]📝 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
| let n = x.dim(1) | |
| // Add positional embeddings sliced to actual patch count | |
| x = x + posEmbed[0..., 0 ..< n, 0...] | |
| let n = x.dim(1) | |
| precondition(n <= posEmbed.dim(1), "Input produced \(n) patches, but maxPatches is only \(posEmbed.dim(1))") | |
| // Add positional embeddings sliced to actual patch count | |
| x = x + posEmbed[0..., 0 ..< n, 0...] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-swift/Sources/OpenTSLMKit/TransformerCNNEncoder.swift` around lines 96 -
98, The positional-embedding slice can index past the learned table when
x.dim(1) (n) is larger than posEmbed.dim(1); modify the forward logic in
TransformerCNNEncoder (where x and posEmbed are used) to guard against this by
computing let n = x.dim(1); let m = min(n, posEmbed.dim(1)); use posEmbed[0...,
0 ..< m, 0...] for the slice and if m < n pad the remaining (n - m) positions
(e.g., zeros or by repeating/tiling posEmbed slices) so x + posEmbed works for
all n, or alternatively add a preconditionFailure with a clear message
referencing x and posEmbed to fail early; apply the same guard/strategy to the
app copy too.
Comment on lines
+16
to
+21
| // Fixtures directory sits next to this file. | ||
| private var fixturesURL: URL { | ||
| URL(fileURLWithPath: #filePath) | ||
| .deletingLastPathComponent() | ||
| .appendingPathComponent("Fixtures") | ||
| } |
There was a problem hiding this comment.
Use Bundle.module for SPM test resources instead of #filePath.
The fixturesURL computed property resolves the path relative to the source file location using #filePath. However, the test resources are declared with .copy() in Package.swift (lines 37-41), which copies them into the test bundle. At runtime in an SPM test context, #filePath points to the source directory, not the bundle where resources are actually located.
🐛 Proposed fix using Bundle.module
- // Fixtures directory sits next to this file.
- private var fixturesURL: URL {
- URL(fileURLWithPath: `#filePath`)
- .deletingLastPathComponent()
- .appendingPathComponent("Fixtures")
- }
+ // Fixtures are copied into the test bundle via Package.swift resources.
+ private var fixturesURL: URL {
+ Bundle.module.bundleURL.appendingPathComponent("Fixtures")
+ }📝 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
| // Fixtures directory sits next to this file. | |
| private var fixturesURL: URL { | |
| URL(fileURLWithPath: #filePath) | |
| .deletingLastPathComponent() | |
| .appendingPathComponent("Fixtures") | |
| } | |
| // Fixtures are copied into the test bundle via Package.swift resources. | |
| private var fixturesURL: URL { | |
| Bundle.module.bundleURL.appendingPathComponent("Fixtures") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-swift/Tests/OpenTSLMKitTests/TransformerCNNEncoderTests.swift` around
lines 16 - 21, Replace the source-path-based fixturesURL with a bundle-based
lookup so tests use SPM-copied resources: update the fixturesURL computed
property in TransformerCNNEncoderTests to use Bundle.module (e.g.
Bundle.module.resourceURL?.appendingPathComponent("Fixtures") or
Bundle.module.url(forResource: "Fixtures", withExtension: nil)) instead of
constructing a path from `#filePath`, ensuring the test loads resources from the
test bundle at runtime.
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@HealthyLLM/HealthyLLM/HealthDataInterpreter.swift`:
- Around line 257-265: The current logic returns hardcodedECGSample() when
fetched is empty even if Constants.includeHardcodedECGSample is false; update
the control flow in the code that uses fetched and hardcodedECGSample() so that
hardcodedECGSample() is only ever returned/inserted when
Constants.includeHardcodedECGSample is true (i.e., inside the if
Constants.includeHardcodedECGSample { ... } block), and when
includeHardcodedECGSample is false and fetched.isEmpty simply return an empty
array (or fetched) instead of hardcodedECGSample(); adjust the branch that
checks fetched.isEmpty to avoid fabricating data so callers of this
HealthDataInterpreter logic won’t see synthetic ECG samples.
- Around line 84-117: The staging path in stageLocalModelIfNeeded() silently
returns on directory creation failure and also only logs when required files are
still missing after copying; change both to fail fast by throwing an error so
setup() won't continue with an incomplete model: when
FileManager.createDirectory(at: Constants.llmLocalModelDirectory) fails, rethrow
or wrap and throw a descriptive error instead of returning; after
copyDirectoryContents(from: sourceURL, to: destinationURL) and
stageLoRACheckpointIfAvailable(...) if hasRequiredModelFiles(in: destinationURL,
fileManager:) is false, throw a descriptive staging error (including
destination/source paths) rather than just logging; keep existing logger calls
but ensure stageLocalModelIfNeeded() surfaces errors to callers.
In `@HealthyLLM/HealthyLLM/HealthyLLMView.swift`:
- Around line 121-132: The localModelSourceExists() helper currently returns
true if the override path or bundle directory exists even when required model
files are missing; update localModelSourceExists() to validate the source
payload by checking for the presence of the expected model files (e.g., model
binary, tokenizer/config) inside the path pointed to by
Constants.localModelSourcePathOverride and inside
Bundle.main.resourceURL.appendingPathComponent(Constants.localModelBundleSubdirectory),
and only return true when all required filenames (use the exact expected
filenames/asset names used elsewhere in setup) are present and readable; ensure
you reference and use localModelSourceExists(),
Constants.localModelSourcePathOverride, and
Constants.localModelBundleSubdirectory when implementing these checks.
In `@HealthyLLM/HealthyLLM/OpenTSLM/MLXEmbeddingGenerator.swift`:
- Around line 48-69: In generate(inputsEmbeds:maxNewTokens:temperature:)
validate the temperature before it is used: add a guard that temperature > 0 and
otherwise throw a clear MLXEmbeddingGeneratorError (e.g., generationFailed with
a message like "Invalid temperature: must be > 0") so that the division of
lastLogits by Float(temperature) and the subsequent call to
tokenSampler(lastLogits) never receives an invalid scaling factor.
- Around line 22-23: The protocol currently declares makeCache() -> Cache and
func callAsFunction(_ inputs: MLXArray?, cache: Cache, inputEmbedding:
MLXArray?) throws -> MLXArray but generate() reuses the same cache instance
across iterations (autoregressive decoding), so either make the mutation
contract explicit by changing callAsFunction to accept an inout Cache (func
callAsFunction(_ inputs: MLXArray?, cache: inout Cache, inputEmbedding:
MLXArray?) throws -> MLXArray) or constrain Cache to reference semantics
(require Cache: AnyObject / class-bound) so mutations persist; update all
conformers and usages (including makeCache and generate) accordingly. Also add a
precondition/guard in generate() to validate temperature > 0 before any logits
division or sampling to avoid invalid math when temperature is non-positive.
Ensure references to makeCache, callAsFunction, Cache, and generate are updated
consistently.
In `@HealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMInferenceService.swift`:
- Around line 256-264: The override path passed into resolveAssetURL (and
similarly resolveECGJSONURL) isn't having a leading "~" expanded, so URLs like
"~/checkpoints/encoder.safetensors" never resolve; update both functions to
expand the overridePath before creating a URL and checking the filesystem (e.g.,
call NSString(string: overridePath).expandingTildeInPath or similar to get
expandedPath, then use URL(fileURLWithPath: expandedPath) and
fileManager.fileExists(atPath: expandedURL.path)); mirror the same
tilde-expansion logic used in existingDirectoryURL to ensure local-debug
overrides resolve correctly.
- Around line 506-511: sourceDescription currently infers provenance from
optional metadata (classification, symptomsStatus, averageHeartRate) which
classifies minimal JSON overrides (that only include voltages) as "hardcoded";
update the logic in the sourceDescription computed property to detect whether
the sample was loaded from an override JSON by checking the actual
voltages/override indicator (e.g., a voltages property or a loadedFromJSON flag)
and return a distinct provenance string (for example "json_override" or
"healthkit_json") when voltages came from an override file, otherwise fall back
to the existing checks and "hardcoded".
- Around line 354-383: createSoftPromptSample is synthesizing zero tensors and
ignoring the tokenizer and dynamic hidden size; update it to use the provided
tokenizer to tokenize prePrompt and postPrompt (e.g., call tokenizer.tokenize or
tokenizer.encode to get token ids) to determine exact token lengths, derive
hiddenSize from the tokenizer/model (not a hardcoded 2048), and produce real
embeddings for prePrompt/postPrompt/timeSeriesText by calling the
model/tokenizer embedding routine (e.g., tokenizer.embedTokens or model.embed)
instead of MLXArray.zeros; build corresponding attention masks from the actual
token lengths and preserve projectedEmbeddings for timeSeriesEmbeddings when
constructing the SoftPromptSample (refer to createSoftPromptSample,
SoftPromptSegment, SoftPromptSample, MLXArray, and the tokenizer parameter).
- Around line 545-553: The guard in OpenTSLMInferenceService that parses
"voltages" should reject empty arrays immediately: update the parsing guard that
binds let voltages = object["voltages"] as? [Double] to also require
!voltages.isEmpty so an empty "voltages": [] throws the same NSError (domain
"OpenTSLMInferenceService", code 6, NSLocalizedDescriptionKey "Unsupported ECG
JSON format"); this ensures makeOpenTSLMSample and downstream encoder/projector
code never receive an empty voltage series.
In `@HealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMLLM.swift`:
- Around line 45-57: This code mutates the shared LLMLocalSession
(session.customContext and session parameters via session.update(parameters:))
and never restores them, causing state leakage and races; fix by either creating
and using a dedicated LLMLocalSession copy for this request or by saving the
original session state (e.g., oldCustomContext = session.customContext and
oldParameters = current parameters) before updating, performing generation via
session.generate(), and restoring session.customContext and parameters in a
defer/finally block (or ensure restoration on MainActor) so concurrent calls or
subsequent requests don't inherit the temporary prompt or maxTokens.
- Around line 30-42: The prompt currently only serializes inputsEmbeds.shape and
mean into embeddingDescription and thus fullPrompt, discarding the actual
embedding values; instead, flatten inputsEmbeds and produce a deterministic,
compact serialization (e.g., formatted float list, comma-separated sample, or a
base64/hex of the raw bytes, or a short quantized/hash signature if size is a
concern) and include that serialization in embeddingDescription so the LLM is
conditioned on the learned representation; update the code that builds
embeddingDescription (referencing inputsEmbeds, embeddingDescription, and
fullPrompt) to serialize the full embedding (or an explicit sampled/quantized
representation with clear labeling) before composing fullPrompt.
In `@HealthyLLM/HealthyLLM/OpenTSLM/SoftPromptInterleaver.swift`:
- Around line 37-47: The initializer currently only checks counts but should
also validate that each MLXArray in timeSeriesEmbeddings has a 2-D shape and
that its second dimension (hidden size) matches the hidden size used by
prePrompt/postPrompt; add upfront shape checks in init (before assigning
properties) to iterate timeSeriesEmbeddings, verify dimensionality == 2 and
shape[1] == prePrompt.hiddenSize (or an appropriate property on
prePrompt/postPrompt), and throw or precondition-fail with a clear message
identifying the offending index; this prevents
padAndInterleaveBatch()/concatenated/stacked from crashing later due to a
mismatched projected tensor.
🪄 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: 52e20944-4782-47ee-af55-9bcde9f09041
📒 Files selected for processing (9)
HealthyLLM/HealthyLLM/HealthDataInterpreter.swiftHealthyLLM/HealthyLLM/HealthyLLMView.swiftHealthyLLM/HealthyLLM/OpenTSLM/MLXEmbeddingGenerator.swiftHealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMInferenceService.swiftHealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMLLM.swiftHealthyLLM/HealthyLLM/OpenTSLM/SoftPromptInterleaver.swiftHealthyLLM/Shared Context/Constants.swiftHealthyLLMStudy/Shared Context/Constants.swiftHealthyLLMStudy/Study/DebugView.swift
Comment on lines
+22
to
+23
| func makeCache() -> Cache | ||
| func callAsFunction(_ inputs: MLXArray?, cache: Cache, inputEmbedding: MLXArray?) throws -> MLXArray |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "MLXEmbeddingGenerator.swift" | head -20Repository: StanfordBDHG/HealthyLLM
Length of output: 128
🏁 Script executed:
cat -n ./HealthyLLM/HealthyLLM/OpenTSLM/MLXEmbeddingGenerator.swiftRepository: StanfordBDHG/HealthyLLM
Length of output: 3648
🏁 Script executed:
rg "EmbeddingPrimedLanguageModel" --type swiftRepository: StanfordBDHG/HealthyLLM
Length of output: 312
🏁 Script executed:
rg "extension.*EmbeddingPrimedLanguageModel|conform.*EmbeddingPrimedLanguageModel" --type swiftRepository: StanfordBDHG/HealthyLLM
Length of output: 49
🏁 Script executed:
rg "Cache" ./HealthyLLM/HealthyLLM/OpenTSLM/ --type swift -A 2 -B 2Repository: StanfordBDHG/HealthyLLM
Length of output: 2081
Clarify cache mutation contract in the protocol.
The cache parameter is passed by value in callAsFunction, but generate() reuses the same cache instance across loop iterations (line 77), assuming it accumulates state. If a conformer implements Cache as a value type (struct), mutations inside callAsFunction won't persist to the next call, breaking autoregressive decoding. Either use inout Cache to explicitly model the mutation contract, or add a protocol constraint requiring Cache to provide reference semantics.
Also add validation for temperature > 0 before line 68, since dividing logits by a non-positive temperature will corrupt sampling.
Suggested contract change
public protocol EmbeddingPrimedLanguageModel {
associatedtype Cache
func makeCache() -> Cache
- func callAsFunction(_ inputs: MLXArray?, cache: Cache, inputEmbedding: MLXArray?) throws -> MLXArray
+ func callAsFunction(_ inputs: MLXArray?, cache: inout Cache, inputEmbedding: MLXArray?) throws -> MLXArray
}
@@
- let cache = model.makeCache()
- var logits = try model.callAsFunction(nil, cache: cache, inputEmbedding: inputsEmbeds)
+ var cache = model.makeCache()
+ var logits = try model.callAsFunction(nil, cache: &cache, inputEmbedding: inputsEmbeds)
@@
- logits = try model.callAsFunction(inputIds, cache: cache, inputEmbedding: nil)
+ logits = try model.callAsFunction(inputIds, cache: &cache, inputEmbedding: nil)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@HealthyLLM/HealthyLLM/OpenTSLM/MLXEmbeddingGenerator.swift` around lines 22 -
23, The protocol currently declares makeCache() -> Cache and func
callAsFunction(_ inputs: MLXArray?, cache: Cache, inputEmbedding: MLXArray?)
throws -> MLXArray but generate() reuses the same cache instance across
iterations (autoregressive decoding), so either make the mutation contract
explicit by changing callAsFunction to accept an inout Cache (func
callAsFunction(_ inputs: MLXArray?, cache: inout Cache, inputEmbedding:
MLXArray?) throws -> MLXArray) or constrain Cache to reference semantics
(require Cache: AnyObject / class-bound) so mutations persist; update all
conformers and usages (including makeCache and generate) accordingly. Also add a
precondition/guard in generate() to validate temperature > 0 before any logits
division or sampling to avoid invalid math when temperature is non-positive.
Ensure references to makeCache, callAsFunction, Cache, and generate are updated
consistently.
Comment on lines
+48
to
+69
| public func generate( | ||
| inputsEmbeds: MLXArray, | ||
| maxNewTokens: Int = 128, | ||
| temperature: Float = 1.0 | ||
| ) throws -> String { | ||
| let cache = model.makeCache() | ||
| var logits = try model.callAsFunction(nil, cache: cache, inputEmbedding: inputsEmbeds) | ||
| var generatedTokenIds: [Int] = [] | ||
|
|
||
| for _ in 0..<maxNewTokens { | ||
| guard logits.ndim >= 2 else { | ||
| throw MLXEmbeddingGeneratorError.generationFailed("Unexpected logits ndim: \(logits.ndim)") | ||
| } | ||
|
|
||
| let seqLen = Int(logits.dim(1)) | ||
| guard seqLen > 0 else { | ||
| throw MLXEmbeddingGeneratorError.generationFailed("Logits sequence length is zero") | ||
| } | ||
|
|
||
| let lastLogitsSlice = logits[0 ..< 1, seqLen - 1 ..< seqLen] | ||
| let lastLogits = lastLogitsSlice / Float(temperature) | ||
| let nextToken = try tokenSampler(lastLogits) |
There was a problem hiding this comment.
Reject non-positive temperatures before scaling logits.
Line 68 divides by temperature without validating it. 0 or negative values will feed invalid logits into tokenSampler, which turns a caller error into a much harder-to-diagnose generation failure.
Suggested guard
public func generate(
inputsEmbeds: MLXArray,
maxNewTokens: Int = 128,
temperature: Float = 1.0
) throws -> String {
+ guard temperature > 0 else {
+ throw MLXEmbeddingGeneratorError.generationFailed("temperature must be greater than 0")
+ }
+
let cache = model.makeCache()
var logits = try model.callAsFunction(nil, cache: cache, inputEmbedding: inputsEmbeds)
var generatedTokenIds: [Int] = []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@HealthyLLM/HealthyLLM/OpenTSLM/MLXEmbeddingGenerator.swift` around lines 48 -
69, In generate(inputsEmbeds:maxNewTokens:temperature:) validate the temperature
before it is used: add a guard that temperature > 0 and otherwise throw a clear
MLXEmbeddingGeneratorError (e.g., generationFailed with a message like "Invalid
temperature: must be > 0") so that the division of lastLogits by
Float(temperature) and the subsequent call to tokenSampler(lastLogits) never
receives an invalid scaling factor.
Comment on lines
+37
to
+47
| public init( | ||
| prePrompt: SoftPromptSegment, | ||
| timeSeriesText: [SoftPromptSegment], | ||
| timeSeriesEmbeddings: [MLXArray], | ||
| postPrompt: SoftPromptSegment | ||
| ) { | ||
| precondition(timeSeriesText.count == timeSeriesEmbeddings.count, "time series text/embedding count mismatch") | ||
| self.prePrompt = prePrompt | ||
| self.timeSeriesText = timeSeriesText | ||
| self.timeSeriesEmbeddings = timeSeriesEmbeddings | ||
| self.postPrompt = postPrompt |
There was a problem hiding this comment.
Validate projected embedding shapes up front.
padAndInterleaveBatch() assumes every projected tensor is 2-D and uses the same hidden size as prePrompt / postPrompt, but the initializer only checks array counts. One mismatched projector output will crash later inside concatenated or stacked with a much less actionable error.
Suggested validation
public init(
prePrompt: SoftPromptSegment,
timeSeriesText: [SoftPromptSegment],
timeSeriesEmbeddings: [MLXArray],
postPrompt: SoftPromptSegment
) {
precondition(timeSeriesText.count == timeSeriesEmbeddings.count, "time series text/embedding count mismatch")
+ let hiddenSize = prePrompt.embeddings.dim(1)
+ precondition(postPrompt.embeddings.dim(1) == hiddenSize, "postPrompt hidden size mismatch")
+ precondition(
+ timeSeriesText.allSatisfy { $0.embeddings.dim(1) == hiddenSize },
+ "time series text hidden size mismatch"
+ )
+ precondition(
+ timeSeriesEmbeddings.allSatisfy { $0.ndim == 2 && $0.dim(1) == hiddenSize },
+ "time series embeddings must have shape [T, H] and match prompt hidden size"
+ )
self.prePrompt = prePrompt
self.timeSeriesText = timeSeriesText
self.timeSeriesEmbeddings = timeSeriesEmbeddings
self.postPrompt = postPrompt
}📝 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
| public init( | |
| prePrompt: SoftPromptSegment, | |
| timeSeriesText: [SoftPromptSegment], | |
| timeSeriesEmbeddings: [MLXArray], | |
| postPrompt: SoftPromptSegment | |
| ) { | |
| precondition(timeSeriesText.count == timeSeriesEmbeddings.count, "time series text/embedding count mismatch") | |
| self.prePrompt = prePrompt | |
| self.timeSeriesText = timeSeriesText | |
| self.timeSeriesEmbeddings = timeSeriesEmbeddings | |
| self.postPrompt = postPrompt | |
| public init( | |
| prePrompt: SoftPromptSegment, | |
| timeSeriesText: [SoftPromptSegment], | |
| timeSeriesEmbeddings: [MLXArray], | |
| postPrompt: SoftPromptSegment | |
| ) { | |
| precondition(timeSeriesText.count == timeSeriesEmbeddings.count, "time series text/embedding count mismatch") | |
| let hiddenSize = prePrompt.embeddings.dim(1) | |
| precondition(postPrompt.embeddings.dim(1) == hiddenSize, "postPrompt hidden size mismatch") | |
| precondition( | |
| timeSeriesText.allSatisfy { $0.embeddings.dim(1) == hiddenSize }, | |
| "time series text hidden size mismatch" | |
| ) | |
| precondition( | |
| timeSeriesEmbeddings.allSatisfy { $0.ndim == 2 && $0.dim(1) == hiddenSize }, | |
| "time series embeddings must have shape [T, H] and match prompt hidden size" | |
| ) | |
| self.prePrompt = prePrompt | |
| self.timeSeriesText = timeSeriesText | |
| self.timeSeriesEmbeddings = timeSeriesEmbeddings | |
| self.postPrompt = postPrompt | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@HealthyLLM/HealthyLLM/OpenTSLM/SoftPromptInterleaver.swift` around lines 37 -
47, The initializer currently only checks counts but should also validate that
each MLXArray in timeSeriesEmbeddings has a 2-D shape and that its second
dimension (hidden size) matches the hidden size used by prePrompt/postPrompt;
add upfront shape checks in init (before assigning properties) to iterate
timeSeriesEmbeddings, verify dimensionality == 2 and shape[1] ==
prePrompt.hiddenSize (or an appropriate property on prePrompt/postPrompt), and
throw or precondition-fail with a clear message identifying the offending index;
this prevents padAndInterleaveBatch()/concatenated/stacked from crashing later
due to a mismatched projected tensor.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
HealthyLLM/HealthyLLM/HealthDataInterpreter.swift (1)
290-300: ⚡ Quick winReplace full sort with max-selection to satisfy lint and reduce work.
Line 290-300 sorts all directories and then takes
.first; a direct max-by-date selection is clearer and avoids unnecessary sorting.♻️ Proposed refactor
- return directoryContents? - .filter { url in - var isDirectory: ObjCBool = false - return fileManager.fileExists(atPath: url.path, isDirectory: &isDirectory) && isDirectory.boolValue - } - .sorted(by: { lhs, rhs in - let lhsDate = (try? lhs.resourceValues(forKeys: [.contentModificationDateKey]).contentModificationDate) ?? .distantPast - let rhsDate = (try? rhs.resourceValues(forKeys: [.contentModificationDateKey]).contentModificationDate) ?? .distantPast - return lhsDate > rhsDate - }) - .first + return directoryContents? + .filter { url in + var isDirectory: ObjCBool = false + return fileManager.fileExists(atPath: url.path, isDirectory: &isDirectory) && isDirectory.boolValue + } + .max(by: { lhs, rhs in + let lhsDate = (try? lhs.resourceValues(forKeys: [.contentModificationDateKey]).contentModificationDate) ?? .distantPast + let rhsDate = (try? rhs.resourceValues(forKeys: [.contentModificationDateKey]).contentModificationDate) ?? .distantPast + return lhsDate < rhsDate + })🤖 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 `@HealthyLLM/HealthyLLM/HealthDataInterpreter.swift` around lines 290 - 300, Current code sorts all entries then takes `.first`; replace that with a single max-selection to avoid full sort and satisfy lint. In the block using `directoryContents?.filter { ... }.sorted(by: { lhs, rhs in ... }).first`, swap the `.sorted(...).first` chain for `.max(by:)` where you compare `lhs.resourceValues(forKeys: [.contentModificationDateKey]).contentModificationDate` and `rhs.resourceValues(forKeys: [.contentModificationDateKey]).contentModificationDate` (falling back to `.distantPast` when the try? returns nil) so only the newest directory is selected; keep the existing `fileManager.fileExists(atPath:isDirectory:)` filter and error-safe date extraction logic.
🤖 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 `@HealthyLLM/HealthyLLM/HealthDataInterpreter.swift`:
- Around line 556-558: The logger.debug call that prints raw model output chunks
inside the async loop over sharedSession.generate() must be removed or replaced
with a non-sensitive placeholder; specifically, in HealthDataInterpreter.swift
within the loop that iterates "for try await stringPiece in try await
sharedSession.generate()", stop logging stringPiece (which may contain PHI) and
instead log a redacted/sanitized indicator or omit logging entirely while still
appending to assistantOutput; update any tests or callers that expect the debug
output accordingly.
---
Nitpick comments:
In `@HealthyLLM/HealthyLLM/HealthDataInterpreter.swift`:
- Around line 290-300: Current code sorts all entries then takes `.first`;
replace that with a single max-selection to avoid full sort and satisfy lint. In
the block using `directoryContents?.filter { ... }.sorted(by: { lhs, rhs in ...
}).first`, swap the `.sorted(...).first` chain for `.max(by:)` where you compare
`lhs.resourceValues(forKeys:
[.contentModificationDateKey]).contentModificationDate` and
`rhs.resourceValues(forKeys:
[.contentModificationDateKey]).contentModificationDate` (falling back to
`.distantPast` when the try? returns nil) so only the newest directory is
selected; keep the existing `fileManager.fileExists(atPath:isDirectory:)` filter
and error-safe date extraction logic.
🪄 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: 691538b2-3a95-4a38-841c-bcf6e71a5124
📒 Files selected for processing (8)
HealthyLLM.xcodeproj/project.pbxprojHealthyLLM.xcodeproj/xcshareddata/xcschemes/HealthyLLM.xcschemeHealthyLLM/HealthyLLM/HealthDataInterpreter.swiftHealthyLLM/HealthyLLM/HealthyLLMContext.swiftHealthyLLM/HealthyLLM/HealthyLLMView.swiftHealthyLLM/Shared Context/Constants.swiftHealthyLLM/Supporting Files/Localizable.xcstringsREADME.md
✅ Files skipped from review due to trivial changes (1)
- README.md
NIkhil-cmd-cmd
force-pushed
the
swift-port-mlx-opentslm
branch
from
May 26, 2026 05:16
5ffa94a to
4a13549
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (8)
HealthyLLM/Shared Context/Constants.swift (1)
41-41:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDefault hardcoded ECG injection to off.
Defaulting to
"1"enables synthetic ECG data in normal runs unless someone explicitly disables it.Suggested minimal fix
-static let includeHardcodedECGSample = (ProcessInfo.processInfo.environment["HEALTHYLLM_INCLUDE_HARDCODED_ECG"] ?? "1") == "1" +static let includeHardcodedECGSample = (ProcessInfo.processInfo.environment["HEALTHYLLM_INCLUDE_HARDCODED_ECG"] ?? "0") == "1"🤖 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 `@HealthyLLM/Shared` Context/Constants.swift at line 41, The constant includeHardcodedECGSample currently defaults to "1" causing hardcoded ECGs to be enabled by default; change the fallback value in the environment lookup from "1" to "0" so ProcessInfo.processInfo.environment["HEALTHYLLM_INCLUDE_HARDCODED_ECG"] ?? "0" == "1", ensuring includeHardcodedECGSample is false unless explicitly enabled; update the literal string "HEALTHYLLM_INCLUDE_HARDCODED_ECG" and the boolean comparison only, leaving the rest of the expression and uses of includeHardcodedECGSample unchanged.HealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMLLM.swift (2)
46-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore
LLMLocalSessionstate after temporary prompt/parameter overrides.This mutates shared session state (
customContext, output length) and leaves it changed for subsequent requests.🤖 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 `@HealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMLLM.swift` around lines 46 - 55, Save the existing session state, apply the temporary prompt and parameters, then restore the originals after generation; specifically, capture the current session.customContext and current parameters (from LLMLocalSession/ session) before overwriting with the temporary customContext and LLMLocalParameters(maxOutputLength: maxTokens), then perform the generation, and use a defer or finally-style restore to set session.customContext back to the saved value and call session.update(parameters: savedParameters) to restore prior parameters so the shared LLMLocalSession state is not permanently mutated.
30-33:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftCurrent embedding conditioning is too lossy to be meaningful.
Only shape and mean are injected, so very different embedding tensors can produce the same prompt-conditioning text.
🤖 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 `@HealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMLLM.swift` around lines 30 - 33, The textual embedding conditioning in embeddingDescription (using inputsEmbeds.shape and inputsEmbeds.mean()) is too lossy — update the prompt representation to include richer, deterministic summary statistics and a compact fingerprint: add min, max, variance/stddev, a few leading/trailing (or top-k) normalized component values, and a short hash/fingerprint of inputsEmbeds to preserve identity; compute these from the inputsEmbeds tensor (use inputsEmbeds.min(), inputsEmbeds.max(), inputsEmbeds.variance()/std(), slice or topk for sample components, and a stable hash like SHA256 over a serialized byte slice) and include them in the multi-line string built in embeddingDescription so different tensors produce distinct conditioning strings.HealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMInferenceService.swift (3)
318-323:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winExpand
~in override paths before filesystem checks.
resolveAssetURLandresolveECGJSONURLstill use raw paths, so common overrides like~/...won’t resolve.Suggested minimal fix
- let overrideURL = URL(fileURLWithPath: overridePath) + let expandedPath = NSString(string: overridePath).expandingTildeInPath + let overrideURL = URL(fileURLWithPath: expandedPath)Also applies to: 506-514
🤖 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 `@HealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMInferenceService.swift` around lines 318 - 323, The overridePath values are not expanding a leading '~', so calls in resolveAssetURL (and similarly resolveECGJSONURL) use raw paths and fail to find user-home-based overrides; update both functions to expand tildes before creating URLs and checking FileManager by transforming overridePath via NSString.expandingTildeInPath (or equivalent) into expandedPath and then use expandedPath for URL(fileURLWithPath:) and fileExists(atPath:), ensuring any further path normalization (e.g., standardizingPath) is applied the same way.
646-648:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject empty ECG voltage arrays during parse.
"voltages": []currently passes parsing and fails downstream later. Fail fast here with the format error.Suggested minimal fix
guard let object = decoded as? [String: Any], - let voltages = object["voltages"] as? [Double] + let voltages = object["voltages"] as? [Double], + !voltages.isEmpty else {🤖 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 `@HealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMInferenceService.swift` around lines 646 - 648, When parsing the decoded object in OpenTSLMInferenceService (the guard that currently binds decoded as [String: Any] and voltages as [Double]), also validate that voltages is not empty and fail fast with the existing format error path when voltages.isEmpty is true; update the guard (or add an immediate check after it) to reject empty arrays so `"voltages": []` triggers the same format error handling used for other malformed inputs.
607-612:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTrack ECG source provenance explicitly instead of inferring from optional fields.
Minimal JSON overrides containing only
voltagesare currently reported ashardcoded, which is misleading in reports.🤖 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 `@HealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMInferenceService.swift` around lines 607 - 612, The computed property sourceDescription in OpenTSLMInferenceService.swift currently infers "healthkit_json" by checking optional fields (classification, symptomsStatus, averageHeartRate) which mislabels minimal ECG-only payloads; instead add an explicit provenance flag or enum (e.g., ecgSource / provenance or isFromHealthKit) set at parse/initialization time when the payload originated from HealthKit, then change sourceDescription to return "healthkit_json" only when that explicit provenance indicates HealthKit and "hardcoded" otherwise; ensure the parser/initializer that consumes incoming JSON sets this new provenance flag for ECG-only payloads that came from HealthKit so minimal voltages-only overrides are reported correctly.HealthyLLM/HealthyLLM/HealthDataInterpreter.swift (2)
657-658:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInsert the system prompt at the start of context.
On first turn, the user message is already present. Appending the
.systemmessage after it weakens instruction precedence. Use insertion at index0for bothcontextandadvancedContext.Suggested minimal fix
- context.append(systemPrompt) - advancedContext.append(systemPrompt) + context.insert(systemPrompt, at: 0) + advancedContext.insert(systemPrompt, at: 0)🤖 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 `@HealthyLLM/HealthyLLM/HealthDataInterpreter.swift` around lines 657 - 658, The system prompt is being appended after the user message which reduces its precedence; instead insert systemPrompt at index 0 for both context and advancedContext to ensure it is the first message. Locate the lines where context.append(systemPrompt) and advancedContext.append(systemPrompt) are used and change them to insert at the start (using context.insert(systemPrompt, at: 0) and advancedContext.insert(systemPrompt, at: 0) or the Swift equivalent) so the system prompt precedes the existing user message.
452-460:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not present synthetic ECG as the latest real sample.
Line 454 inserts the hardcoded sample first, and Line 459 returns a hardcoded sample even when
HEALTHYLLM_INCLUDE_HARDCODED_ECGis off. That makes the “Latest ECG samples” section misleading.Suggested minimal fix
if Constants.includeHardcodedECGSample { var merged = fetched - merged.insert(hardcodedECGSample(), at: 0) + merged.append(hardcodedECGSample()) return merged } if fetched.isEmpty { - return [hardcodedECGSample()] + return [] } return fetched🤖 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 `@HealthyLLM/HealthyLLM/HealthDataInterpreter.swift` around lines 452 - 460, The code currently inserts or returns hardcodedECGSample() at the front regardless of intent, making synthetic data appear as the latest real sample; change the logic so Constants.includeHardcodedECGSample gates any use of hardcodedECGSample(), never insert it at index 0, and only supply it when there are no fetched samples and the flag is true (or alternatively append it at the end if you want it included but not shown as the latest). Update the block using Constants.includeHardcodedECGSample, fetched, and hardcodedECGSample() to: do not insert at position 0, only return [hardcodedECGSample()] when fetched.isEmpty AND the flag is true, otherwise return fetched unchanged.
🤖 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 `@HealthyLLM/HealthyLLM/HealthDataInterpreter.swift`:
- Around line 17-18: SwiftLint flags unsorted imports; reorder the two import
statements so they are alphabetically sorted (adjust the import lines for
MLXLMCommon and MLXLLM) and remove any extra blank lines so the import block is
in a single, sorted group.
In `@HealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMInferenceService.swift`:
- Around line 222-224: The OpenTSLMLoRA.applyIfNeeded(on: llmSession) call
currently throws and aborts ECG flow but sleep flow only logs and continues;
change both occurrences (the block containing OpenTSLMLoRA.applyIfNeeded(on:
llmSession) and GPU.clearCache()) to mirror the sleep inference path by wrapping
the applyIfNeeded call in a do-catch, logging the caught error (use the same
logger used elsewhere in this file) and continuing execution so GPU.clearCache()
still runs; ensure you do not rethrow the error so optional LoRA failures become
non-fatal.
In `@HealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMLoRA.swift`:
- Around line 36-38: The code is appending a raw URL from
Constants.openTSLMLoRACheckpointPath which won't expand a leading tilde; update
the logic in OpenTSLMLoRA to expand any ~ in
Constants.openTSLMLoRACheckpointPath (for example via NSString(string:
path).expandingTildeInPath or similar) before creating the URL and appending to
candidates so overrides like HEALTHYLLM_OPEN_TSLM_LORA_CHECKPOINT with ~/...
resolve correctly.
In `@HealthyLLM/HealthyLLM/OpenTSLM/SleepEDFDataset.swift`:
- Around line 27-28: Validate the incoming maxRows at the start of
SleepEDFDataset.init (the initializer shown) and any other places where maxRows
is used (e.g., before calling Array.reserveCapacity around line that currently
uses reserveCapacity), by checking it's non-negative and throwing an appropriate
error (or returning) if it's negative; move or add this guard before any call to
reserveCapacity or other array sizing logic and reference the initializer
SleepEDFDataset.init and the reserveCapacity usage so the check runs first.
---
Duplicate comments:
In `@HealthyLLM/HealthyLLM/HealthDataInterpreter.swift`:
- Around line 657-658: The system prompt is being appended after the user
message which reduces its precedence; instead insert systemPrompt at index 0 for
both context and advancedContext to ensure it is the first message. Locate the
lines where context.append(systemPrompt) and
advancedContext.append(systemPrompt) are used and change them to insert at the
start (using context.insert(systemPrompt, at: 0) and
advancedContext.insert(systemPrompt, at: 0) or the Swift equivalent) so the
system prompt precedes the existing user message.
- Around line 452-460: The code currently inserts or returns
hardcodedECGSample() at the front regardless of intent, making synthetic data
appear as the latest real sample; change the logic so
Constants.includeHardcodedECGSample gates any use of hardcodedECGSample(), never
insert it at index 0, and only supply it when there are no fetched samples and
the flag is true (or alternatively append it at the end if you want it included
but not shown as the latest). Update the block using
Constants.includeHardcodedECGSample, fetched, and hardcodedECGSample() to: do
not insert at position 0, only return [hardcodedECGSample()] when
fetched.isEmpty AND the flag is true, otherwise return fetched unchanged.
In `@HealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMInferenceService.swift`:
- Around line 318-323: The overridePath values are not expanding a leading '~',
so calls in resolveAssetURL (and similarly resolveECGJSONURL) use raw paths and
fail to find user-home-based overrides; update both functions to expand tildes
before creating URLs and checking FileManager by transforming overridePath via
NSString.expandingTildeInPath (or equivalent) into expandedPath and then use
expandedPath for URL(fileURLWithPath:) and fileExists(atPath:), ensuring any
further path normalization (e.g., standardizingPath) is applied the same way.
- Around line 646-648: When parsing the decoded object in
OpenTSLMInferenceService (the guard that currently binds decoded as [String:
Any] and voltages as [Double]), also validate that voltages is not empty and
fail fast with the existing format error path when voltages.isEmpty is true;
update the guard (or add an immediate check after it) to reject empty arrays so
`"voltages": []` triggers the same format error handling used for other
malformed inputs.
- Around line 607-612: The computed property sourceDescription in
OpenTSLMInferenceService.swift currently infers "healthkit_json" by checking
optional fields (classification, symptomsStatus, averageHeartRate) which
mislabels minimal ECG-only payloads; instead add an explicit provenance flag or
enum (e.g., ecgSource / provenance or isFromHealthKit) set at
parse/initialization time when the payload originated from HealthKit, then
change sourceDescription to return "healthkit_json" only when that explicit
provenance indicates HealthKit and "hardcoded" otherwise; ensure the
parser/initializer that consumes incoming JSON sets this new provenance flag for
ECG-only payloads that came from HealthKit so minimal voltages-only overrides
are reported correctly.
In `@HealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMLLM.swift`:
- Around line 46-55: Save the existing session state, apply the temporary prompt
and parameters, then restore the originals after generation; specifically,
capture the current session.customContext and current parameters (from
LLMLocalSession/ session) before overwriting with the temporary customContext
and LLMLocalParameters(maxOutputLength: maxTokens), then perform the generation,
and use a defer or finally-style restore to set session.customContext back to
the saved value and call session.update(parameters: savedParameters) to restore
prior parameters so the shared LLMLocalSession state is not permanently mutated.
- Around line 30-33: The textual embedding conditioning in embeddingDescription
(using inputsEmbeds.shape and inputsEmbeds.mean()) is too lossy — update the
prompt representation to include richer, deterministic summary statistics and a
compact fingerprint: add min, max, variance/stddev, a few leading/trailing (or
top-k) normalized component values, and a short hash/fingerprint of inputsEmbeds
to preserve identity; compute these from the inputsEmbeds tensor (use
inputsEmbeds.min(), inputsEmbeds.max(), inputsEmbeds.variance()/std(), slice or
topk for sample components, and a stable hash like SHA256 over a serialized byte
slice) and include them in the multi-line string built in embeddingDescription
so different tensors produce distinct conditioning strings.
In `@HealthyLLM/Shared` Context/Constants.swift:
- Line 41: The constant includeHardcodedECGSample currently defaults to "1"
causing hardcoded ECGs to be enabled by default; change the fallback value in
the environment lookup from "1" to "0" so
ProcessInfo.processInfo.environment["HEALTHYLLM_INCLUDE_HARDCODED_ECG"] ?? "0"
== "1", ensuring includeHardcodedECGSample is false unless explicitly enabled;
update the literal string "HEALTHYLLM_INCLUDE_HARDCODED_ECG" and the boolean
comparison only, leaving the rest of the expression and uses of
includeHardcodedECGSample unchanged.
🪄 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: 86899701-0716-4a08-95be-b3537b5b7a89
📒 Files selected for processing (8)
HealthyLLM.xcodeproj/xcshareddata/xcschemes/HealthyLLM.xcschemeHealthyLLM/HealthyLLM/HealthDataInterpreter.swiftHealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMInferenceService.swiftHealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMLLM.swiftHealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMLoRA.swiftHealthyLLM/HealthyLLM/OpenTSLM/SleepEDFDataset.swiftHealthyLLM/Shared Context/Constants.swiftHealthyLLM/Supporting Files/OpenTSLM/mlx-checkpoint.lora.safetensors
Load Llama-3.2-1B from Documents, apply OpenTSLM LoRA on sample commands only, keep LoRA out of the HF model dir, and run /opentslm-sleep-sample without OOM. Cap chat generation and reduce repetitive decode loops. Co-authored-by: Cursor <cursoragent@cursor.com>
NIkhil-cmd-cmd
force-pushed
the
swift-port-mlx-opentslm
branch
from
May 26, 2026 05:23
4a13549 to
0efbddd
Compare
Skip function-call pass for auto ECG prompt, clear MLX cache between generations, restore session.generate for defaultResponse, and shrink ECG context to avoid Metal address faults on device. Co-authored-by: Cursor <cursoragent@cursor.com>
Default hardcoded ECG off; enable via scheme. Fix system prompt order, LoRA path tilde expansion, non-fatal LoRA apply, session state restore, ECG provenance, empty voltage guard, and snapshot selection without full sort. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
HealthyLLM/HealthyLLM/HealthDataInterpreter.swift (1)
206-211:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDirectory creation failure silently returns instead of failing fast.
When
createDirectoryfails, the method logs and returns without throwing, allowingsetup()to continue with an incomplete model directory. This contradicts the fail-fast pattern and can cause opaque failures later during model loading.🛠️ Proposed fix
do { try fileManager.createDirectory(at: destinationURL, withIntermediateDirectories: true) } catch { logger.error("Failed creating local model destination directory: \(error.localizedDescription, privacy: .public)") - return + throw 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 `@HealthyLLM/HealthyLLM/HealthDataInterpreter.swift` around lines 206 - 211, The catch block for fileManager.createDirectory(at: destinationURL, ...) in HealthDataInterpreter currently logs and returns, which masks failures; change it to propagate the error so setup() fails fast: either remove the local catch and let try propagate, or rethrow the caught error (e.g., throw error) after logging, and update the enclosing method signature (e.g., setup() or the surrounding initializer in HealthDataInterpreter) to be throws so callers must handle the failure; ensure logger.error still records error.localizedDescription with privacy flags before rethrowing.
🧹 Nitpick comments (1)
HealthyLLM/HealthyLLM/HealthDataInterpreter.swift (1)
604-612: 💤 Low valueRedundant condition — always true at this point.
The
userPrompt.content != Constants.ecgAutoPromptcheck is always true here because the ECG auto-prompt case returns early at line 601. This condition adds confusion without providing any guard.♻️ Proposed simplification
- if userPrompt.content != Constants.ecgAutoPrompt { - do { - try await checkForFunctionCall(prompt: userPrompt.content, healthKit: healthKit) - } catch { - logger.error("queryLLM: checkForFunctionCall threw \(String(reflecting: error), privacy: .public) — localizedDescription=\(error.localizedDescription, privacy: .public)") - throw error - } - releaseLLMSessionBetweenGenerations() + do { + try await checkForFunctionCall(prompt: userPrompt.content, healthKit: healthKit) + } catch { + logger.error("queryLLM: checkForFunctionCall threw \(String(reflecting: error), privacy: .public) — localizedDescription=\(error.localizedDescription, privacy: .public)") + throw error } + releaseLLMSessionBetweenGenerations()🤖 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 `@HealthyLLM/HealthyLLM/HealthDataInterpreter.swift` around lines 604 - 612, The if-check comparing userPrompt.content to Constants.ecgAutoPrompt is redundant because the ECG auto-prompt path returns earlier; remove the conditional and always execute the checkForFunctionCall call and subsequent releaseLLMSessionBetweenGenerations() for non-early-returning flows—i.e., replace the if-block with a direct try await checkForFunctionCall(prompt: userPrompt.content, healthKit: healthKit) inside the existing do/catch and then call releaseLLMSessionBetweenGenerations() afterward, keeping the existing logger.error behavior in the catch.
🤖 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 `@HealthyLLM/HealthyLLM/HealthDataInterpreter.swift`:
- Around line 155-176: The direct-local-load branch currently sets
sharedSession.modelContainer and then sets sharedSession.state = .ready without
running the session initialization; instead, after successful
LLMModelFactory.shared.loadContainer(...) returns, call try await
sharedSession.setup() (which should be idempotent if modelContainer is already
set) rather than manually toggling sharedSession.state, so the internal
LLMLocalSession initialization always runs; adjust hasRequiredModelFiles branch
to await sharedSession.setup() after assigning the container (or prefer
assigning container inside setup) and avoid directly setting state.
---
Outside diff comments:
In `@HealthyLLM/HealthyLLM/HealthDataInterpreter.swift`:
- Around line 206-211: The catch block for fileManager.createDirectory(at:
destinationURL, ...) in HealthDataInterpreter currently logs and returns, which
masks failures; change it to propagate the error so setup() fails fast: either
remove the local catch and let try propagate, or rethrow the caught error (e.g.,
throw error) after logging, and update the enclosing method signature (e.g.,
setup() or the surrounding initializer in HealthDataInterpreter) to be throws so
callers must handle the failure; ensure logger.error still records
error.localizedDescription with privacy flags before rethrowing.
---
Nitpick comments:
In `@HealthyLLM/HealthyLLM/HealthDataInterpreter.swift`:
- Around line 604-612: The if-check comparing userPrompt.content to
Constants.ecgAutoPrompt is redundant because the ECG auto-prompt path returns
earlier; remove the conditional and always execute the checkForFunctionCall call
and subsequent releaseLLMSessionBetweenGenerations() for non-early-returning
flows—i.e., replace the if-block with a direct try await
checkForFunctionCall(prompt: userPrompt.content, healthKit: healthKit) inside
the existing do/catch and then call releaseLLMSessionBetweenGenerations()
afterward, keeping the existing logger.error behavior in the catch.
🪄 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: b569172b-2f24-4252-a6d8-cb8f3a4d9e19
📒 Files selected for processing (12)
HealthyLLM.xcodeproj/xcshareddata/xcschemes/HealthyLLM.xcschemeHealthyLLM/HealthyLLM/HealthContextGenerator.swiftHealthyLLM/HealthyLLM/HealthDataInterpreter.swiftHealthyLLM/HealthyLLM/OpenTSLM/EmbeddingLlamaModel.swiftHealthyLLM/HealthyLLM/OpenTSLM/MLXEmbeddingGenerator.swiftHealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMInferenceService.swiftHealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMLLM.swiftHealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMLoRA.swiftHealthyLLM/HealthyLLM/OpenTSLM/SleepEDFDataset.swiftHealthyLLM/Helper/LLMRunner+onShot.swiftHealthyLLM/Shared Context/Constants.swiftHealthyLLM/Supporting Files/OpenTSLM/mlx-checkpoint.lora.safetensors
🚧 Files skipped from review as they are similar to previous changes (5)
- HealthyLLM/HealthyLLM/OpenTSLM/MLXEmbeddingGenerator.swift
- HealthyLLM/HealthyLLM/HealthContextGenerator.swift
- HealthyLLM/HealthyLLM/OpenTSLM/SleepEDFDataset.swift
- HealthyLLM/Shared Context/Constants.swift
- HealthyLLM/HealthyLLM/OpenTSLM/OpenTSLMLoRA.swift
Comment on lines
+155
to
+176
| if hasRequiredModelFiles(in: modelDirectory, fileManager: fileManager) { | ||
| logger.info("setup(): loading MLX container from local directory (bf16 base weights only)") | ||
| do { | ||
| let container = try await LLMModelFactory.shared.loadContainer( | ||
| configuration: ModelConfiguration(directory: modelDirectory) | ||
| ) | ||
| await MainActor.run { | ||
| sharedSession.modelContainer = container | ||
| sharedSession.state = .ready | ||
| } | ||
| let setupDuration = Date().timeIntervalSince(setupStart) | ||
| logger.info("setup(): direct loadContainer succeeded in \(setupDuration, privacy: .public)s") | ||
| await MainActor.run { | ||
| loaded = true | ||
| loadingStage = .ready | ||
| loadingDetail = String(format: "Loaded in %.1fs", setupDuration) | ||
| } | ||
| return | ||
| } catch { | ||
| logger.error("setup(): direct loadContainer failed: \(String(reflecting: error), privacy: .public)") | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if LLMLocalSession.setup() does more than just loading the container
rg -n -A 10 'func setup\(\)' --type swift -g '!*Test*' | head -60Repository: StanfordBDHG/HealthyLLM
Length of output: 1290
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find where sharedSession.setup() is called
rg -n "sharedSession\.setup\(" -S --glob='*.swift'
# 2) Find the declaration/type of sharedSession in this file
rg -n "sharedSession" HealthyLLM/HealthyLLM/HealthDataInterpreter.swift
# 3) Search for the session type that implements setup() (focus on LLMLocalSession / Local session equivalents)
rg -n "class .*Session|struct .*Session|typealias .*Session|LLMLocalSession" --glob='*.swift'
# 4) Locate other setup() implementations (beyond HealthDataInterpreter.setup) that match a "local session" concept
rg -n "func setup\(\) async throws" --glob='*.swift' | head -200Repository: StanfordBDHG/HealthyLLM
Length of output: 3179
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find any definition of LLMLocalSession within the repo
rg -n "typealias LLMLocalSession|struct LLMLocalSession|class LLMLocalSession|actor LLMLocalSession" --glob='*.swift'
# If not found, locate package/module sources that might be vendored locally (rare)
fd -a "LLMLocalSession*" -t f . 2>/dev/null | head -50
# Search for any func setup() that mentions LLMLocalSession
rg -n "func setup\(\)[^{]*\{|setup\(\) async throws" --glob='*.swift' --context 3Repository: StanfordBDHG/HealthyLLM
Length of output: 49
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Show all occurrences of LLMLocalSession to see if it's defined locally or imported from a dependency
rg -n "LLMLocalSession" --glob='*.swift' . || true
# 2) Find where SpeziLLMLocal / related modules are referenced
rg -n "SpeziLLMLocal|LLMLocal|LLMModelFactory|LocalSession" --glob='*.swift' . || true
# 3) Inspect Package.swift / manifest(s) to identify dependencies that define LLMLocalSession
ls -1
find . -maxdepth 3 -name "Package.swift" -o -name "*.podspec" -o -name "Package.resolved" -o -name "Cartfile.resolved" 2>/dev/nullRepository: StanfordBDHG/HealthyLLM
Length of output: 8233
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the SwiftPM manifest in src-swift (this is where dependencies are declared)
ls -1 src-swift
sed -n '1,200p' src-swift/Package.swift
# Locate SpeziLLMLocal dependency lines if present
rg -n "SpeziLLMLocal" -n src-swift/Package.swift src-swift/Package.resolved || trueRepository: StanfordBDHG/HealthyLLM
Length of output: 1549
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for how SpeziLLMLocal is referenced/pinned (Xcode project / manifests / resolved files)
rg -n "SpeziLLMLocal" -S . || true
# Show likely SPM resolution file(s) and try to find SpeziLLMLocal version info
find . -maxdepth 4 -name "Package.resolved" -o -name "Package.swift" -o -name "*.xcodeproj" -o -name "*.xcworkspace" 2>/dev/null | sed 's|^\./||'
# Try to extract SpeziLLMLocal pin from any project files (may be large; limit output)
rg -n "SpeziLLMLocal|LLMLocalSession" HealthyLLM.xcodeproj src-swift/Package.resolved 2>/dev/null || trueRepository: StanfordBDHG/HealthyLLM
Length of output: 6386
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Extract the SpeziLLMLocal package reference metadata from the Xcode project
rg -n "SpeziLLMLocal[^\\n]*\\{|productName = SpeziLLMLocal;|SpeziLLMLocal\\s*=\\s*\\{|name = SpeziLLMLocal" HealthyLLM.xcodeproj/project.pbxproj
# Print the blocks around the PBXFileReference/product entries for SpeziLLMLocal
# (use surrounding context to capture URL/revision fields)
rg -n "440072002CFD20ED00DC7C5A|44E72E6F2D2C62B000520745" HealthyLLM.xcodeproj/project.pbxproj -n -C 6Repository: StanfordBDHG/HealthyLLM
Length of output: 5614
🌐 Web query:
SpeziLLMLocal LLMLocalSession setup() implementation
💡 Result:
In the Stanford SpeziLLM framework, LLMLocalSession.setup is an asynchronous method provided to explicitly initialize and prepare a local large language model for inference [1]. The implementation details are as follows: 1. Purpose: Calling setup in advance preloads the model into memory, which helps reduce response latency for the initial prompt [1]. 2. Implementation: The public method setup is defined as follows: public func setup async throws { guard await _setup(continuation: nil) else { throw LLMLocalError.modelNotReadyYet } } 3. Key Considerations: - Lifecycle Management: The LLMLocalSession should not be created manually; it is instantiated by the LLMLocalPlatform through the LLMRunner [2][1]. - Automatic Loading: While setup allows for proactive preparation, the model is also automatically reloaded into memory when calling generate if it is not currently active [3]. - Resource Offloading: To manage memory and performance, you can use LLMLocalSession.offload to release the model from memory when it is no longer actively needed [3]. For a complete SpeziLLM setup, the LLMRunner must be configured within the application's SpeziAppDelegate to use the LLMLocalPlatform [3][4]: class TestAppDelegate: SpeziAppDelegate { override var configuration: Configuration { Configuration { LLMRunner { LLMLocalPlatform } } } }
Citations:
- 1: https://github.com/StanfordSpezi/SpeziLLM/blob/main/Sources/SpeziLLMLocal/LLMLocalSession.swift
- 2: https://github.com/StanfordSpezi/SpeziLLM/blob/main/Sources/SpeziLLMLocal/LLMLocalPlatform.swift
- 3: https://github.com/StanfordSpezi/SpeziLLM
- 4: https://swiftpackageregistry.com/StanfordSpezi/SpeziLLM
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find Xcode SwiftPM resolution file(s) that might include SpeziLLM/SpeziLLMLocal.
find HealthyLLM.xcodeproj HealthyLLM.xcodeproj/project.xcworkspace -type f -name "Package.resolved" -o -name "Package.swift" 2>/dev/null || true
# Search those resolution files for SpeziLLM.
find HealthyLLM.xcodeproj HealthyLLM.xcodeproj/project.xcworkspace -type f -name "Package.resolved" 2>/dev/null -exec rg -n "SpeziLLM|SpeziLLMLocal" {} + || true
# Also extract the XCRemoteSwiftPackageReference entry for SpeziLLM to see minVersion / exact revision info.
rg -n "XCRemoteSwiftPackageReference \"SpeziLLM\"|SpeziLLM\\\";" HealthyLLM.xcodeproj/project.pbxproj -n -C 4 || trueRepository: StanfordBDHG/HealthyLLM
Length of output: 707
🏁 Script executed:
#!/bin/bash
set -euo pipefail
RES="HealthyLLM.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved"
# Print the SpeziLLM package entry (near "location": "https://github.com/StanfordSpezi/SpeziLLM/")
rg -n "\"location\" *: *\"https://github.com/StanfordSpezi/SpeziLLM/\"" "$RES" -n
LINE=$(rg -n "\"location\" *: *\"https://github.com/StanfordSpezi/SpeziLLM/\"" "$RES" | head -1 | cut -d: -f1)
start=$((LINE-25)); end=$((LINE+60))
sed -n "${start},${end}p" "$RES"
# Also try to extract any version/revision fields for SpeziLLM
rg -n "SpeziLLM|StanfordSpezi/SpeziLLM" "$RES" | head -50Repository: StanfordBDHG/HealthyLLM
Length of output: 2811
🌐 Web query:
https://github.com/StanfordSpezi/SpeziLLM/blob/9f4e8a4f78e3796173863b45e835aa12adc8fcc1/Sources/SpeziLLMLocal/LLMLocalSession.swift
💡 Result:
The file you linked (SpeziLLM/Sources/SpeziLLMLocal/LLMLocalSession.swift at commit 9f4e8a4f78e3796173863b45e835aa12adc8fcc1) defines the Spezi “local LLM session” abstraction: an executable session that holds LLM context/state and performs on-device inference for an LLMLocalSchema via MLX/MLXLLM. What it is - LLMLocalSession is described as “the executable version of the local LLM containing context and state as defined by the LLMLocalSchema.”[1] - The file states it “utilizes [MLX … mlx-swift] to locally execute a large language model on-device.”[1] How it fits into the SpeziLLM architecture - SpeziLLMLocalSession is instantiated by the local platform: LLMLocalPlatform “turns a received LLMLocalSchema to an executable LLMLocalSession” and shouldn’t be used directly; it’s delegated to via SpeziLLM’s LLMRunner.[2] - SpeziLLM’s LLMRunner is responsible for turning an LLMSchema into an executable LLMSession using the configured LLMPlatform(s).[3] Key API behavior in LLMLocalSession - generate: Produces the model output as an AsyncThrowingStream of String “pieces” (streaming partial responses).[1][4] - cancel: The session supports cancellation via LLMLocalSession/cancel.[1] - setup / generate will (re)load the model as needed, and the package docs mention an offload method to unload the model to free resources when not in use.[4] Why you’d care - This session layer is what your app would subscribe to for streaming local inference results, while the SpeziLLM runner/platform handle schema-to-session creation and lifecycle/concurrency boundaries.[3][2][1] If you want, paste the rest of the file contents (or tell me what specific function/type inside LLMLocalSession you’re interested in—e.g., how it builds prompts, manages KV-cache/context, or handles errors), and I’ll map each part precisely to the control flow and concurrency model used in SpeziLLM.
Citations:
- 1: https://github.com/StanfordSpezi/SpeziLLM/blob/main/Sources/SpeziLLMLocal/LLMLocalSession.swift
- 2: https://github.com/StanfordSpezi/SpeziLLM/blob/main/Sources/SpeziLLMLocal/LLMLocalPlatform.swift
- 3: https://github.com/StanfordSpezi/SpeziLLM/blob/main/Sources/SpeziLLM/LLMRunner.swift
- 4: https://github.com/StanfordSpezi/SpeziLLM/
🌐 Web query:
SpeziLLM LLMLocalSession generate() behavior "modelNotReadyYet" or auto-reload when not active
💡 Result:
In SpeziLLM’s local implementation (LLMLocalSession), generate fails with LLMLocalError.modelNotReadyYet when the session’s model hasn’t been initialized/loaded yet (state.uninitialized). It does not “auto-reload” and continue the same generate call; instead, it attempts setup and, if setup can’t proceed, it transitions the session into an error state and finishes the generation stream with modelNotReadyYet. 1) What LLMLocalSession.generate does when not ready - LLMLocalSession.setup is the explicit method that initializes the model in advance; it throws LLMLocalError.modelNotReadyYet if its internal setup guard fails [1]. - In LLMLocalSession.generate, the code checks whether the session state is.uninitialized; if so, it calls an internal _setup(...). If that guard fails, generate sets state to.error(error: LLMLocalError.modelNotReadyYet) and finishes the generation stream with that error [1]. So the behavior is: calling generate before the model is ready results in either (a) model initialization occurring via the internal setup path, or (b) if that internal setup can’t succeed, the stream terminates with modelNotReadyYet rather than automatically reloading later during the same call [1]. 2) Where auto-reload / lazy loading applies - The SpeziLLM documentation for the local session states that calling LLMLocalSession.setup or LLMLocalSession.generate “will automatically reload the model into memory as needed” [2]. This describes lazy (on-demand) loading when a model was previously offloaded/unloaded, not a retry loop when a session setup immediately fails. - Another doc snippet recommends using LLMLocalSession.offload to unload the model when not actively in use, and says that generate/setup will reload “as needed” when interaction is required again [2]. Therefore, auto-reload is intended for the “model not actively in use because it was offloaded” scenario, not for the “generation invoked while the session cannot be initialized” scenario where modelNotReadyYet is surfaced [1][2]. Practical guidance - Call LLMLocalSession.setup before starting generation if you want to avoid first-prompt latency and avoid modelNotReadyYet from a failed internal setup path [1]. - If you use offload to free resources, it’s fine to rely on LLMLocalSession.generate to reload the model “as needed” for the next interaction [2]. - If you see modelNotReadyYet during generate, treat it as an initialization failure for that session and handle it (e.g., call setup explicitly, verify configuration/model availability) rather than expecting the existing generate stream to self-retry/reload and continue [1].
Citations:
- 1: https://github.com/StanfordSpezi/SpeziLLM/blob/main/Sources/SpeziLLMLocal/LLMLocalSession.swift
- 2: https://github.com/StanfordSpezi/SpeziLLM
🌐 Web query:
SpeziLLM LLMLocalSessionState enum cases .ready .setup
💡 Result:
In the SpeziLLM framework, there is no enum named LLMLocalSessionState. Instead, the session state is represented by the LLMState enum [1][2]. The LLMState enum, which defines the current status of an LLMSession, includes the following cases [1]:.uninitialized: The LLMSession is allocated, but the underlying model has not yet been initialized [1]..loading: The LLMSession is in the process of being initialized [1]..ready: The LLMSession is initialized and ready for use [1]..generating: The LLMSession is currently in the process of generating an output [1]..callingTools: The LLMSession is currently executing function calls requested by the LLM [1]..error(error: any LLMError): The LLMSession is in an error state [1]. While there is no.setup case within LLMState, the LLMLocalSession class does contain a public setup method used to initialize the model in advance [2]. Additionally, the LLMLocalSession.offload method is available to unload models from memory, which can then be reloaded by calling either setup or generate [3].
Citations:
- 1: https://github.com/StanfordSpezi/SpeziLLM/blob/main/Sources/SpeziLLM/Models/LLMState.swift
- 2: https://github.com/StanfordSpezi/SpeziLLM/blob/main/Sources/SpeziLLMLocal/LLMLocalSession.swift
- 3: https://github.com/StanfordSpezi/SpeziLLM/
Avoid marking sharedSession as .ready without running sharedSession.setup() (can bypass required LLMLocalSession initialization).
In SpeziLLM, LLMLocalSession.generate() only invokes the internal setup path when the session is .uninitialized; if it’s already .ready, setup is skipped. Here the direct loadContainer(...) path sets sharedSession.modelContainer and then flips sharedSession.state = .ready without calling try await sharedSession.setup(), while the fallback path does call sharedSession.setup(). This makes correctness depend on setup() doing nothing beyond those two assignments.
After loadContainer(...) succeeds, call try await sharedSession.setup() (and ideally make the direct path idempotent/no-op when modelContainer is already set) instead of manually setting .ready.
🤖 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 `@HealthyLLM/HealthyLLM/HealthDataInterpreter.swift` around lines 155 - 176,
The direct-local-load branch currently sets sharedSession.modelContainer and
then sets sharedSession.state = .ready without running the session
initialization; instead, after successful
LLMModelFactory.shared.loadContainer(...) returns, call try await
sharedSession.setup() (which should be idempotent if modelContainer is already
set) rather than manually toggling sharedSession.state, so the internal
LLMLocalSession initialization always runs; adjust hasRequiredModelFiles branch
to await sharedSession.setup() after assigning the container (or prefer
assigning container inside setup) and avoid directly setting state.
Auto-run /opentslm-ecg-sample on launch, clear chat context before sample inference, and tune scheme env vars to reduce memory pressure during LLM decode tests. Co-authored-by: Cursor <cursoragent@cursor.com>
Mirror SleepEDFDataset: read CoT CSV rows, index one sample, and lazy-load PTB-XL waveforms exported from OpenTSLM instead of a monolithic formatted JSON. Co-authored-by: Cursor <cursoragent@cursor.com>
Physical iPhones were killed when Llama (~2GB) loaded alongside the 12-lead ECG encoder even with RUN_LLM=0. Add SKIP_LLM_LOAD for a true encoder-only path and tune the scheme for capped series length. Co-authored-by: Cursor <cursoragent@cursor.com>
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.
♻️ Current situation & Problem
This PR ports key OpenTSLM components to Swift and integrates them into HealthyLLM’s local inference flow.
Adds an in-app OpenTSLM encoder/projector pipeline.
Adds ECG fetching from HealthKit and ECG-first prompt behavior.
Improves local model staging/startup so the app can run from bundled/local assets without manual setup.
Includes large model/tokenizer/checkpoint assets required for local execution.
Related issue(s): #
⚙️ Release Notes
Added Swift OpenTSLM modules (encoder, projector, sample pipeline, inference service).
Added ECG data fetch + voltage extraction and automatic ECG starter prompt.
Added model setup flow:
Local model detection/staging
Download fallback UI when assets are missing
Environment-variable overrides for local debugging
Added OpenTSLM Swift package, CLI runner, and test fixtures.
Updated ignore rules for local large artifacts/datasets used during development.
📚 Documentation
README updated with ECG inference behavior.
Added/updated inline docs around model/runtime configuration and OpenTSLM flow.
Added Swift package metadata and runner usage/help text.
✅ Testing
Added unit tests for:
TransformerCNNEncoder
MLPProjector
SoftPromptInterleaver
Added fixture-based numerical parity checks (Python vs Swift encoder output).
CI/CodeCov should validate overall coverage and integration behavior.
Code of Conduct & Contributing Guidelines
By creating and submitting this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:
I agree to follow the Code of Conduct and Contributing Guidelines.