PedCardio study definition#165
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new pediatric cardiology study (pedCardioStudy) with system prompt and tasks, registers it, supplies three synthetic FHIR patient bundles, updates app launch/config to reference the study, adds/uses a SwiftPM Algorithms dependency, adjusts localization and small UI copy, increases a bottom-sheet size offset, and replaces two OSLog warnings with print. ChangesPed Cardio Study & Data
Dependency & Package Wiring
App Config, Launch Wiring & Localization / UI Copy
Small UX / Logging
Sequence Diagram(s)sequenceDiagram
participant User as User
participant App as App
participant StudyMgr as StudyManager
participant FHIR as FHIRStore
participant Backend as FirebaseFunction
participant OpenAI as OpenAI
User->>App: start pedCardioStudy
App->>StudyMgr: request study definition & system prompt
StudyMgr->>FHIR: get_resources(filter: pedCardioStudy)
FHIR-->>StudyMgr: return FHIR resources
StudyMgr->>Backend: send prompt + resources (firebase-function:chat)
Backend->>OpenAI: forward prompt
OpenAI-->>Backend: assistant response
Backend-->>App: assistant response
App-->>User: display concise, parent‑friendly summary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #165 +/- ##
==========================================
- Coverage 18.40% 18.35% -0.05%
==========================================
Files 97 98 +1
Lines 1011 1014 +3
==========================================
Hits 186 186
- Misses 825 828 +3
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
PSchmiedmayer
approved these changes
Apr 17, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
LLMonFHIRShared/Sources/LLMonFHIRShared/ResourceBundle.swift (1)
79-82:⚠️ Potential issue | 🟡 MinorPre-existing bug nearby:
breakaborts the entire bundle scan on the first duplicate patient name.Not introduced in this PR, but worth flagging now that a new set of synthetic patients is being added under
Synthetic Patients/PedCardio/: if any two bundles share a patient'sfullName, thebreakon line 81 exits the outerfor url in enumeratorloop, so every subsequent bundle (including unrelated ones by path) is never indexed intobundlesByPathorbundlesByPatientName. This silently hides bundles from bothnamed(_:)andforPatient(named:). Likely meant to becontinue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLMonFHIRShared/Sources/LLMonFHIRShared/ResourceBundle.swift` around lines 79 - 82, The loop currently uses break inside the enumerator loop which aborts scanning all bundles when a duplicate patientName is found; in the method/initializer that populates bundlesByPatientName and bundlesByPath (look for the for url in enumerator loop in ResourceBundle.swift and the guard that checks allBundlesByPatientName.keys.contains(patientName)), change the control flow so a duplicate patientName only skips the current bundle (use continue instead of break) and ensure bundlesByPatientName[patientName] is set to nil to mark ambiguity without stopping the outer iteration, so subsequent bundles are still indexed into bundlesByPath and evaluated.
🧹 Nitpick comments (4)
LLMonFHIR/Supporting Files/UserStudyConfig.plist (1)
61-69: Config entry looks correct; minor consistency nit.The new
pedCardioStudyblock matches the existingfirebase-function:chatpattern used byspineAI. Note that thespineAIentry (lines 45–51) omitsopenAIAPIKeyentirely while the new entry includes an emptyopenAIAPIKey; either is fine but picking one convention avoids confusion about whether absence vs. empty has semantic meaning in the config reader.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLMonFHIR/Supporting` Files/UserStudyConfig.plist around lines 61 - 69, Make the config consistent by choosing one convention for absent vs empty API key: either remove the empty <key>openAIAPIKey</key> entry from the edu.stanford.LLMonFHIR.pedCardioStudy dict or add an explicit empty openAIAPIKey entry to the existing spineAI dict so both use the same pattern; locate the <key>edu.stanford.LLMonFHIR.pedCardioStudy</key> and <key>spineAI</key> blocks and update the presence/absence of the openAIAPIKey element to match.LLMonFHIRShared/Sources/LLMonFHIRShared/Resources/Synthetic Patients/PedCardio/Patient2_MateoRodriguez.json (1)
53-71: Consistency: race extension is missing (only ethnicity is set).Patient1 and Patient3 carry the
us-core-raceextension, while this patient only hasus-core-ethnicity. Under US Core these are independent profiles — having ethnicity without race is unusual and can produce asymmetric answers if the LLM is asked about demographics across the three patients. Consider adding aus-core-raceextension (or addingus-core-ethnicityto the other two bundles) so the dataset is uniformly profiled.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLMonFHIRShared/Sources/LLMonFHIRShared/Resources/Synthetic` Patients/PedCardio/Patient2_MateoRodriguez.json around lines 53 - 71, This patient JSON currently contains only the us-core-ethnicity extension under the top-level "extension" array; add a sibling extension object with "url": "http://hl7.org/fhir/us/core/StructureDefinition/us-core-race" and include the same nested structure used in Patient1/Patient3 (an "extension" array containing "ombCategory" with a valueCoding and "text" with valueString) so the bundle includes a us-core-race entry consistent with the other patients and avoids asymmetric demographics.LLMonFHIRShared/Sources/LLMonFHIRStudyDefinitions/PedCardio.swift (2)
9-9: Avoidswiftlint:disable all; disable only the specific rules you need.Disabling every rule for a ~350-line file hides real defects this PR already contains (missing
isOptionalargument consistency, long strings, grammar/typos). Prefer targeted// swiftlint:disable <rule>directives or a narrower:next/:thisscope.♻️ Proposed fix
-// swiftlint:disable all +// swiftlint:disable file_length line_length type_body_length🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLMonFHIRShared/Sources/LLMonFHIRStudyDefinitions/PedCardio.swift` at line 9, The file PedCardio.swift currently uses a blanket directive "// swiftlint:disable all"; replace that with targeted disables only for the specific rules needed (for example "line_length", "identifier_name", "string_literal_long_line", etc.) and scope them narrowly (use :next or :this where possible) to avoid hiding other issues; then run SwiftLint on PedCardio.swift, address reported problems (fix inconsistent isOptional usage across initializers/properties, break up long strings, correct grammar/typos) and only add rule-specific disables next to the offending declarations rather than disabling all rules for the entire file.
342-348: Minor: punctuation/grammar in the required opening phrase.The prompt instructs the model to start every conversation with
"Hello caregiver of (name) I understand that …". This is an awkward run‑on — without a comma after “Hello” and a separator before “I understand,” models often reproduce it verbatim in a way that reads poorly to parents. Suggest:♻️ Proposed fix
- Always start the summary by saying: "Hello caregiver of (name) I understand that …". + Always start the summary by saying: "Hello, caregiver of (name). I understand that …".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLMonFHIRShared/Sources/LLMonFHIRStudyDefinitions/PedCardio.swift` around lines 342 - 348, The opening prompt text "Hello caregiver of (name) I understand that …" is grammatically awkward; locate the prompt constant or string in PedCardio.swift (search for the exact phrase or in functions generating the summary, e.g., any method that constructs the initial greeting) and change it to a corrected form such as "Hello, caregiver of (name). I understand that …" or "Hello caregiver of (name), I understand that …" so there is a clear separator after the greeting; update any tests or usages that rely on the exact wording.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@LLMonFHIR.xcodeproj/xcshareddata/xcschemes/LLMonFHIR.xcscheme`:
- Around line 80-83: The shared scheme currently enables the CommandLineArgument
with argument="--useFirebaseEmulator"; revert it so isEnabled is "NO" in the
LLMonFHIR.xcscheme to avoid forcing the local emulator for everyone. Locate the
CommandLineArgument entry for "--useFirebaseEmulator" and set isEnabled back to
"NO", and if a developer needs this flag, advise enabling it only in their
personal/user scheme or run configuration; no changes needed to
FeatureFlags.swift or LLMonFHIRDelegate.firebaseModules(), just disable the flag
in the shared scheme.
In `@LLMonFHIR/Helper/BottomSheet.swift`:
- Line 22: The hardcoded "+100" padding in BottomSheet.swift (used when setting
height and calling presentationDetents) should be replaced with a small named
constant (e.g., PADDING) and the requested detent height should be clamped so it
never exceeds a safe maximum; compute contentHeight + PADDING and then use
min(contentHeight + PADDING, UIScreen.main.bounds.height * 0.9) (or derive max
from safeAreaInsets if available) before calling
presentationDetents([.height(...)]) so tall views like TaskInstructionView don't
get clipped and tiny sheets like uploadSheet() aren't over-inflated.
In `@LLMonFHIRShared/Sources/LLMonFHIRShared/ResourceBundle.swift`:
- Line 72: Restore structured logging by replacing the stray print calls used
when a FHIR bundle fails to decode with the module's Logger (use
Self.logger.warning or .error as appropriate) where the bundle-decoding occurs
(the same locations that currently call print for "Skipping FHIR bundle at
\(url.path) (unable to decode): \(error)"); reference the static logger declared
on the type (the Logger on the struct) and log the same message plus the error
so OSLog/Console receives level, subsystem and category metadata; if you changed
to print due to logger-init ordering, add a brief comment next to the static
logger explaining why it's safe/lazy-initialized instead of switching to print.
In `@LLMonFHIRShared/Sources/LLMonFHIRShared/Resources/Synthetic`
Patients/PedCardio/Patient1_HarryPotter.json:
- Around line 391-397: The BMI entry under the Observation with
"effectiveDateTime": "2026-02-15" has a mismatch: its valueQuantity.value is
21.34 but should be recalculated from the recorded Body weight (52.6 kg) and
Body height (158 cm) to ~21.07 kg/m2; update the BMI Observation's
valueQuantity.value to the correctly computed number (rounded consistently with
other vitals) so the Observation with "effectiveDateTime": "2026-02-15" and its
valueQuantity reflect 52.6 / 1.58^2 ≈ 21.07 and keep unit/system/code as is.
In `@LLMonFHIRShared/Sources/LLMonFHIRStudyDefinitions/PedCardio.swift`:
- Around line 33-35: Task 1 and Task 4 use inconsistent caregiver-facing wording
and Task 1's instructions end with a question mark; update the instruction
strings for both tasks so they address the caregiver about "your child's health"
(not "your health") and make Task 1's instructions an imperative sentence
(remove the trailing question mark). Locate the affected string properties (the
instructions field shown in PedCardio.swift for Task 1 and the analogous
instructions block for Task 4 around lines 58-61) and replace "your health" with
"your child's health" and change the final punctuation of Task 1 from '?' to '.'
so wording is consistent and directive.
- Around line 141-158: The instructional preamble in postInterventionQuestions
(Study.Task.Question) claims responses use "Always, Often, Sometimes, Never" and
also tells users to select "N/A", but the actual response scale
(.frequencyOptions) does not include N/A; fix by making instructions and options
consistent: either remove the "If the statement does not apply to you, select
N/A." sentence from the Study.Task.Question text or add an explicit "N/A" option
to the .frequencyOptions used for these questions so the UI matches the copy.
- Around line 75-78: The question text in the PedCardio definition uses
.init(text: "What were the most and least useful features of the LLM? Do you
have any suggestions to share", type: .freeText, isOptional: true) and is
missing terminal punctuation; update the string to end with proper punctuation
(e.g., add a question mark or period) so the prompt is grammatically
complete—locate the .init(...) call in PedCardio.swift and amend the text value
accordingly.
---
Outside diff comments:
In `@LLMonFHIRShared/Sources/LLMonFHIRShared/ResourceBundle.swift`:
- Around line 79-82: The loop currently uses break inside the enumerator loop
which aborts scanning all bundles when a duplicate patientName is found; in the
method/initializer that populates bundlesByPatientName and bundlesByPath (look
for the for url in enumerator loop in ResourceBundle.swift and the guard that
checks allBundlesByPatientName.keys.contains(patientName)), change the control
flow so a duplicate patientName only skips the current bundle (use continue
instead of break) and ensure bundlesByPatientName[patientName] is set to nil to
mark ambiguity without stopping the outer iteration, so subsequent bundles are
still indexed into bundlesByPath and evaluated.
---
Nitpick comments:
In `@LLMonFHIR/Supporting` Files/UserStudyConfig.plist:
- Around line 61-69: Make the config consistent by choosing one convention for
absent vs empty API key: either remove the empty <key>openAIAPIKey</key> entry
from the edu.stanford.LLMonFHIR.pedCardioStudy dict or add an explicit empty
openAIAPIKey entry to the existing spineAI dict so both use the same pattern;
locate the <key>edu.stanford.LLMonFHIR.pedCardioStudy</key> and
<key>spineAI</key> blocks and update the presence/absence of the openAIAPIKey
element to match.
In `@LLMonFHIRShared/Sources/LLMonFHIRShared/Resources/Synthetic`
Patients/PedCardio/Patient2_MateoRodriguez.json:
- Around line 53-71: This patient JSON currently contains only the
us-core-ethnicity extension under the top-level "extension" array; add a sibling
extension object with "url":
"http://hl7.org/fhir/us/core/StructureDefinition/us-core-race" and include the
same nested structure used in Patient1/Patient3 (an "extension" array containing
"ombCategory" with a valueCoding and "text" with valueString) so the bundle
includes a us-core-race entry consistent with the other patients and avoids
asymmetric demographics.
In `@LLMonFHIRShared/Sources/LLMonFHIRStudyDefinitions/PedCardio.swift`:
- Line 9: The file PedCardio.swift currently uses a blanket directive "//
swiftlint:disable all"; replace that with targeted disables only for the
specific rules needed (for example "line_length", "identifier_name",
"string_literal_long_line", etc.) and scope them narrowly (use :next or :this
where possible) to avoid hiding other issues; then run SwiftLint on
PedCardio.swift, address reported problems (fix inconsistent isOptional usage
across initializers/properties, break up long strings, correct grammar/typos)
and only add rule-specific disables next to the offending declarations rather
than disabling all rules for the entire file.
- Around line 342-348: The opening prompt text "Hello caregiver of (name) I
understand that …" is grammatically awkward; locate the prompt constant or
string in PedCardio.swift (search for the exact phrase or in functions
generating the summary, e.g., any method that constructs the initial greeting)
and change it to a corrected form such as "Hello, caregiver of (name). I
understand that …" or "Hello caregiver of (name), I understand that …" so there
is a clear separator after the greeting; update any tests or usages that rely on
the exact wording.
🪄 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: 4915170f-56e0-4bbf-a25b-e49a357885af
📒 Files selected for processing (12)
LLMonFHIR.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolvedLLMonFHIR.xcodeproj/xcshareddata/xcschemes/LLMonFHIR.xcschemeLLMonFHIR/FHIRInterpretation/UserStudy/UserStudyChatToolbar.swiftLLMonFHIR/Helper/BottomSheet.swiftLLMonFHIR/Resources/Localizable.xcstringsLLMonFHIR/Supporting Files/UserStudyConfig.plistLLMonFHIRShared/Sources/LLMonFHIRShared/ResourceBundle.swiftLLMonFHIRShared/Sources/LLMonFHIRShared/Resources/Synthetic Patients/PedCardio/Patient1_HarryPotter.jsonLLMonFHIRShared/Sources/LLMonFHIRShared/Resources/Synthetic Patients/PedCardio/Patient2_MateoRodriguez.jsonLLMonFHIRShared/Sources/LLMonFHIRShared/Resources/Synthetic Patients/PedCardio/Patient3_ChloeChen.jsonLLMonFHIRShared/Sources/LLMonFHIRStudyDefinitions/AllStudies.swiftLLMonFHIRShared/Sources/LLMonFHIRStudyDefinitions/PedCardio.swift
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
LLMonFHIRShared/Sources/LLMonFHIRStudyDefinitions/PedCardio.swift (2)
15-16: Doc comment is generic — doesn't identify this as the PedCardio study.
/// LLMonFHIR's usability studyis reused verbatim across study definitions. A one-line callout that this is the pediatric cardiology variant (caregiver-facing, cardiology-scoped prompt) helps disambiguate at call sites.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLMonFHIRShared/Sources/LLMonFHIRStudyDefinitions/PedCardio.swift` around lines 15 - 16, Update the generic doc comment above the public static var pedCardioStudy: Study to specifically identify this as the pediatric cardiology (PedCardio) study — e.g., note it is the caregiver-facing, cardiology-scoped variant — so callers can disambiguate this Study from other study definitions; modify the comment attached to pedCardioStudy accordingly.
9-9: Avoid blanketswiftlint:disable allat file scope.Disabling all lint rules for an entire file silently hides real issues (e.g. unused code, identifier rules, force-unwraps) in a study definition that will likely grow over time. Prefer disabling only the specific rule(s) that fail here (or use
// swiftlint:disable:next <rule>/ file-leveldisable <rule>with a short rationale).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLMonFHIRShared/Sources/LLMonFHIRStudyDefinitions/PedCardio.swift` at line 9, The file-level blanket suppression ("// swiftlint:disable all") in PedCardio.swift should be removed; instead run SwiftLint to see which specific rules are triggering, then replace the all-disable with targeted disables for only those rules (e.g. "// swiftlint:disable identifier_name" or use "// swiftlint:disable:next <rule>" immediately before the offending declaration) and add a one-line rationale comment for each file-scope disable so future contributors know why it's exempted; search for top-of-file "// swiftlint:disable all" and update it to specific rule disables or next-line disables around the offending functions/identifiers in PedCardio types and methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@LLMonFHIRShared/Sources/LLMonFHIRStudyDefinitions/PedCardio.swift`:
- Line 86: The string literal starting with text: "On a scale of 0—10 how
likely..." uses an em‑dash; replace the em‑dash (U+2014) with an en‑dash
(U+2013) or a hyphen so the range reads "0–10" or "0-10" to match numeric range
conventions and visual weight; update the text: "On a scale of 0—10 how likely
are you to recommend..." entry in PedCardio.swift accordingly.
- Around line 92-96: The instruction text for Task with id "t5" is UI-chrome
specific; update the instructions in the Task(id: "t5") definition to use
UI-agnostic or accessibility-aware wording (for example, reference the button by
its accessible label like "Continue" or "Next", or say "Use the app's
Continue/Next control to advance") so the copy does not break if the
icon/position changes and remains clear to VoiceOver users.
- Around line 343-349: Update the greeting template string used for cardiology
summaries so it includes punctuation after the salutation and marks the
recipient placeholder clearly; change “Hello caregiver of (name) I understand
that …” to include a comma (e.g., “Hello caregiver of (name), I understand that
…”) and document that (name) is a substitute placeholder (not literal text)
wherever this template is defined (search for the template string in
PedCardio.swift or the function/method that builds the prompt). Ensure any code
that injects the patient name replaces the placeholder rather than leaving the
literal parentheses.
- Line 197: Normalize the spacing around the slash in the affected string:
locate the entry where text: "I am confident that I/ my child can follow
through..." in PedCardio.swift and change "I/ my child" to "I / my child" so it
matches the other array items; ensure any other occurrences in the same array
(the fields with key text) use the same "I / my child" spacing for consistency.
---
Nitpick comments:
In `@LLMonFHIRShared/Sources/LLMonFHIRStudyDefinitions/PedCardio.swift`:
- Around line 15-16: Update the generic doc comment above the public static var
pedCardioStudy: Study to specifically identify this as the pediatric cardiology
(PedCardio) study — e.g., note it is the caregiver-facing, cardiology-scoped
variant — so callers can disambiguate this Study from other study definitions;
modify the comment attached to pedCardioStudy accordingly.
- Line 9: The file-level blanket suppression ("// swiftlint:disable all") in
PedCardio.swift should be removed; instead run SwiftLint to see which specific
rules are triggering, then replace the all-disable with targeted disables for
only those rules (e.g. "// swiftlint:disable identifier_name" or use "//
swiftlint:disable:next <rule>" immediately before the offending declaration) and
add a one-line rationale comment for each file-scope disable so future
contributors know why it's exempted; search for top-of-file "//
swiftlint:disable all" and update it to specific rule disables or next-line
disables around the offending functions/identifiers in PedCardio types and
methods.
🪄 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: 98782b24-09ca-462f-8a2c-e16a92559901
📒 Files selected for processing (2)
LLMonFHIRShared/Sources/LLMonFHIRShared/Resources/Synthetic Patients/PedCardio/Patient1_HarryPotter.jsonLLMonFHIRShared/Sources/LLMonFHIRStudyDefinitions/PedCardio.swift
✅ Files skipped from review due to trivial changes (1)
- LLMonFHIRShared/Sources/LLMonFHIRShared/Resources/Synthetic Patients/PedCardio/Patient1_HarryPotter.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
LLMonFHIRShared/Sources/LLMonFHIRShared/Study/Task/TaskQuestionType+AnswerOptions.swift (1)
71-76: Consider a stdlib alternative to avoid theswift-algorithmspackage dependency.
chain(self, CollectionOfOne("N/A"))is the only usage of theAlgorithmsmodule in this target. The same behaviour is achievable without the package:♻️ Proposed refactor — drop `Algorithms` import and use stdlib concatenation
-private import Algorithms private import Foundation private import SpeziFoundationextension Study.Task.Question.Kind.AnswerOptions { /// Creates a new `AnswerOptions`, by unconditionally appending a `N/A` option at the end. public var withNA: Self { - Self(chain(self, CollectionOfOne("N/A"))) + Self(Array(self) + ["N/A"]) } }And remove the corresponding
.product(name: "Algorithms", package: "swift-algorithms")entry inLLMonFHIRShared/Package.swift(line 44) and the package declaration itself if unused elsewhere.If
Algorithmsis expected to be used more broadly in follow-up work, keeping the dependency is fine — but please verify there are other planned usages before merging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLMonFHIRShared/Sources/LLMonFHIRShared/Study/Task/TaskQuestionType`+AnswerOptions.swift around lines 71 - 76, The withNA computed property uses chain(self, CollectionOfOne("N/A")) from the Algorithms package; replace that with stdlib concatenation (e.g., create a new AnswerOptions by appending "N/A" to the existing collection using + or by creating an array from self and adding "N/A") inside Study.Task.Question.Kind.AnswerOptions.withNA, then remove the Algorithms import and the Algorithms product entry from Package.swift if there are no other uses; update references to chain/CollectionOfOne accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@LLMonFHIR.xcodeproj/xcshareddata/xcschemes/LLMonFHIR.xcscheme`:
- Around line 113-115: The shared scheme currently has the CommandLineArgument
with argument="--mode study:edu.stanford.LLMonFHIR.pedCardioStudy" set with
isEnabled="YES"; change isEnabled to "NO" (or remove the argument entirely) so
the pedCardio study mode is not enabled by default in the shared scheme and
leave this flag enabled only in a personal/local scheme when actively developing
that study.
---
Nitpick comments:
In
`@LLMonFHIRShared/Sources/LLMonFHIRShared/Study/Task/TaskQuestionType`+AnswerOptions.swift:
- Around line 71-76: The withNA computed property uses chain(self,
CollectionOfOne("N/A")) from the Algorithms package; replace that with stdlib
concatenation (e.g., create a new AnswerOptions by appending "N/A" to the
existing collection using + or by creating an array from self and adding "N/A")
inside Study.Task.Question.Kind.AnswerOptions.withNA, then remove the Algorithms
import and the Algorithms product entry from Package.swift if there are no other
uses; update references to chain/CollectionOfOne accordingly.
🪄 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: a8cfe01b-00cf-46bd-bbc0-75666682ef43
📒 Files selected for processing (7)
LLMonFHIR.xcodeproj/xcshareddata/xcschemes/LLMonFHIR.xcschemeLLMonFHIR/FHIRInterpretation/UserStudy/QuestionnaireResponses+Summary.swiftLLMonFHIRShared/Package.swiftLLMonFHIRShared/Sources/LLMonFHIRShared/Study/Task/TaskQuestionType+AnswerOptions.swiftLLMonFHIRShared/Sources/LLMonFHIRStudyDefinitions/GynStudy.swiftLLMonFHIRShared/Sources/LLMonFHIRStudyDefinitions/PedCardio.swiftLLMonFHIRShared/Sources/LLMonFHIRStudyDefinitions/UsabilityStudy.swift
✅ Files skipped from review due to trivial changes (4)
- LLMonFHIRShared/Sources/LLMonFHIRStudyDefinitions/UsabilityStudy.swift
- LLMonFHIR/FHIRInterpretation/UserStudy/QuestionnaireResponses+Summary.swift
- LLMonFHIRShared/Sources/LLMonFHIRStudyDefinitions/GynStudy.swift
- LLMonFHIRShared/Sources/LLMonFHIRStudyDefinitions/PedCardio.swift
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.
PedCardio study definition
♻️ Current situation & Problem
this PR adds the definition of the PedCardio study into the app
⚙️ Release Notes
📚 Documentation
n/a
✅ Testing
n/a
Code of Conduct & Contributing Guidelines
By creating and submitting this pull request, you agree to follow our Code of Conduct and Contributing Guidelines: