Implementation for ai assistant integration#320
Conversation
WalkthroughAdds an AI-powered Station Support feature: new StationSupport view and view model, API/repository/use-case/model for device support, routing and bottom-sheet background color plumbing, UI bits (AI button, loader background), localization, tests, and project/resource updates. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant OverviewView
participant OverviewVM
participant Router
participant StationSupportVM
participant MeUseCase
participant MeRepository
participant API
User->>OverviewView: Tap "AI Health Check"
OverviewView->>OverviewVM: handleAISupportTap()
OverviewVM->>OverviewVM: Build StationSupportViewModel (via Factory)
OverviewVM->>Router: showBottomSheet(.stationSupport(vm), bgColor)
Router->>Router: set bottomSheetBgColor
Router->>StationSupportVM: present StationSupportView
StationSupportVM->>StationSupportVM: mode = .loading
StationSupportVM->>MeUseCase: getDeviceSupport(deviceName)
MeUseCase->>MeRepository: getDeviceSupport(deviceName)
MeRepository->>API: GET me/devices/{deviceName}/support
alt success
API-->>MeRepository: NetworkDeviceSupportResponse
MeRepository-->>MeUseCase: DataResponse(...)
MeUseCase-->>StationSupportVM: markdown result
StationSupportVM->>StationSupportVM: mode = .content(markdown)
else failure
StationSupportVM->>StationSupportVM: mode = .error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Areas to focus review on:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wxm-ios.xcodeproj/project.pbxproj (1)
5188-5214: Restore release display nameRelease now hardcodes
CFBundleDisplayNameto “WXM Dev”, so the App Store build would ship with the wrong name. Please keep the dev name only for Debug/Mock and leave Release as the production label (e.g. “WeatherXM”). Consider applying this diff in the Release config block:- INFOPLIST_KEY_CFBundleDisplayName = "WXM Dev"; + INFOPLIST_KEY_CFBundleDisplayName = WeatherXM;
🧹 Nitpick comments (4)
WeatherXMTests/PresentationLayer/Navigation/MockRouter.swift (1)
15-18: Consider storing the bgColor parameter.The
bgColorparameter is added to match the baseRoutersignature but isn't stored. If tests need to verify that the correct background color is passed, consider storing it:- override func showBottomSheet(_ route: Route, bgColor: ColorEnum = .bottomSheetBg) { + override func showBottomSheet(_ route: Route, bgColor: ColorEnum = .bottomSheetBg) { + bottomSheetBgColor = bgColor bottomSheetRoute = route showBottomSheet = true }If tests don't need to verify
bgColorbehavior, the current implementation is fine.PresentationLayer/UIComponents/Screens/WeatherStations/StationDetails/Overview/OverviewViewModel.swift (1)
23-30: Logic correctly implements the QOD threshold requirement.The computed property correctly enforces the visibility conditions: owned stations with QOD below 80. The nil-handling for both
device?.qodandfollowState?.relationis appropriate.Consider extracting the threshold value to a named constant for maintainability:
+ private static let qodThresholdForAIAssistant = 80 + var shouldShowAIAssistantButton: Bool { guard let qod = device?.qod, followState?.relation == .owned else { return false } - return qod < 80 + return qod < Self.qodThresholdForAIAssistant }wxm-ios/DataLayer/DataLayer/Networking/ApiRequestBuilders/MeApiRequestBuilder.swift (1)
195-197: Remove commented code; parameters are unused for deviceSupport.The commented block adds noise and can drift from the actual contract.
-// case let .deviceSupport(deviceName): -// return [ParameterConstants.Me.stationName: deviceName]PresentationLayer/UIComponents/Screens/WeatherStations/StationSupport/StationSupportView.swift (1)
47-58: Don’t ignore associated values in Equatable; use synthesized conformance.Custom equality treats any two
.contentvalues as equal, hiding actual content changes in tests/UI updates.Remove the manual
==and rely on synthesized Equatable:- static func == (lhs: Mode, rhs: Mode) -> Bool { - switch (lhs, rhs) { - case (.loading, .loading): return true - case (.error, .error): return true - case (.content, .content): return true - default: return false - } - }Update tests to pattern‑match the case instead of equality when you only care about the mode:
if case .content = viewModel.mode { /* ... */ }.This will fail the current test that asserts
.content(""); adjust as noted.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
PresentationLayer/Constants/FontSizeEnum.swift(2 hunks)PresentationLayer/Constants/FontsEnum.swift(1 hunks)PresentationLayer/Navigation/Router.swift(6 hunks)PresentationLayer/Navigation/RouterView.swift(1 hunks)PresentationLayer/UIComponents/BaseComponents/SpinningLoader/SpinningLoader.swift(3 hunks)PresentationLayer/UIComponents/Screens/WeatherStations/StationDetails/Overview/OverviewView.swift(2 hunks)PresentationLayer/UIComponents/Screens/WeatherStations/StationDetails/Overview/OverviewViewModel.swift(2 hunks)PresentationLayer/UIComponents/Screens/WeatherStations/StationSupport/StationSupportView.swift(1 hunks)PresentationLayer/UIComponents/Screens/WeatherStations/StationSupport/StationSupportViewModel.swift(1 hunks)PresentationLayer/UIComponents/ViewModelsFactory.swift(1 hunks)WeatherXMTests/DataLayer/RepositoryImplementations/MeRepositoryImplTests.swift(1 hunks)WeatherXMTests/DomainLayer/MockRepositories/MockMeRepositoryImpl.swift(1 hunks)WeatherXMTests/DomainLayer/UseCases/MeUseCaseTests.swift(1 hunks)WeatherXMTests/PresentationLayer/MockUseCases/MockMeUseCase.swift(1 hunks)WeatherXMTests/PresentationLayer/Navigation/MockRouter.swift(1 hunks)WeatherXMTests/PresentationLayer/ViewModels/StationSupportViewModelTests.swift(1 hunks)wxm-ios.xcodeproj/project.pbxproj(13 hunks)wxm-ios.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved(6 hunks)wxm-ios/DataLayer/DataLayer.xcodeproj/project.pbxproj(4 hunks)wxm-ios/DataLayer/DataLayer/Networking/ApiRequestBuilders/MeApiRequestBuilder.swift(5 hunks)wxm-ios/DataLayer/DataLayer/Networking/Constants/ParameterConstants.swift(1 hunks)wxm-ios/DataLayer/DataLayer/Networking/Mock/Jsons/get_station_support.json(1 hunks)wxm-ios/DataLayer/DataLayer/RepositoryImplementations/MeRepositoryImpl.swift(1 hunks)wxm-ios/DomainLayer/DomainLayer.xcodeproj/project.pbxproj(4 hunks)wxm-ios/DomainLayer/DomainLayer/DomainRepositoryInterfaces/MeRepository.swift(1 hunks)wxm-ios/DomainLayer/DomainLayer/Entities/Codables/Me/Network/NetworkDeviceSupportResponse.swift(1 hunks)wxm-ios/DomainLayer/DomainLayer/UseCases/MeUseCase.swift(1 hunks)wxm-ios/DomainLayer/DomainLayer/UseCases/UseCaseApi/MeUseCaseApi.swift(1 hunks)wxm-ios/Info.plist(0 hunks)wxm-ios/Resources/Localizable/Localizable.xcstrings(2 hunks)wxm-ios/Resources/Localizable/LocalizableString+StationDetails.swift(4 hunks)
💤 Files with no reviewable changes (1)
- wxm-ios/Info.plist
🧰 Additional context used
🧬 Code graph analysis (17)
WeatherXMTests/PresentationLayer/ViewModels/StationSupportViewModelTests.swift (1)
WeatherXMTests/DataLayer/Services/PhotosRepositoryImplTests.swift (1)
- `` (28-32)
PresentationLayer/UIComponents/ViewModelsFactory.swift (1)
wxm-ios/Swinject/SwinjectHelper.swift (1)
getContainerForSwinject(272-274)
PresentationLayer/Navigation/RouterView.swift (2)
PresentationLayer/UIComponents/BaseComponents/CustomSheet/BottomSheetModifier.swift (1)
bottomSheet(85-94)PresentationLayer/Navigation/Router.swift (1)
showBottomSheet(347-351)
WeatherXMTests/PresentationLayer/MockUseCases/MockMeUseCase.swift (3)
wxm-ios/DataLayer/DataLayer/RepositoryImplementations/MeRepositoryImpl.swift (1)
getDeviceSupport(197-202)WeatherXMTests/DomainLayer/MockRepositories/MockMeRepositoryImpl.swift (1)
getDeviceSupport(262-276)wxm-ios/DomainLayer/DomainLayer/UseCases/MeUseCase.swift (1)
getDeviceSupport(189-191)
wxm-ios/DomainLayer/DomainLayer/UseCases/MeUseCase.swift (3)
wxm-ios/DataLayer/DataLayer/RepositoryImplementations/MeRepositoryImpl.swift (1)
getDeviceSupport(197-202)WeatherXMTests/DomainLayer/MockRepositories/MockMeRepositoryImpl.swift (1)
getDeviceSupport(262-276)WeatherXMTests/PresentationLayer/MockUseCases/MockMeUseCase.swift (1)
getDeviceSupport(200-214)
wxm-ios/DomainLayer/DomainLayer/UseCases/UseCaseApi/MeUseCaseApi.swift (4)
wxm-ios/DataLayer/DataLayer/RepositoryImplementations/MeRepositoryImpl.swift (1)
getDeviceSupport(197-202)WeatherXMTests/DomainLayer/MockRepositories/MockMeRepositoryImpl.swift (1)
getDeviceSupport(262-276)WeatherXMTests/PresentationLayer/MockUseCases/MockMeUseCase.swift (1)
getDeviceSupport(200-214)wxm-ios/DomainLayer/DomainLayer/UseCases/MeUseCase.swift (1)
getDeviceSupport(189-191)
WeatherXMTests/DataLayer/RepositoryImplementations/MeRepositoryImplTests.swift (3)
wxm-ios/DataLayer/DataLayer/RepositoryImplementations/MeRepositoryImpl.swift (1)
getDeviceSupport(197-202)WeatherXMTests/DomainLayer/UseCases/MeUseCaseTests.swift (1)
getDeviceSupport(185-191)wxm-ios/Toolkit/Toolkit/Utils/Combine+.swift (1)
toAsync(12-29)
WeatherXMTests/DomainLayer/UseCases/MeUseCaseTests.swift (2)
WeatherXMTests/DataLayer/RepositoryImplementations/MeRepositoryImplTests.swift (1)
getDeviceSupport(124-129)wxm-ios/Toolkit/Toolkit/Utils/Combine+.swift (1)
toAsync(12-29)
wxm-ios/DomainLayer/DomainLayer/DomainRepositoryInterfaces/MeRepository.swift (4)
wxm-ios/DataLayer/DataLayer/RepositoryImplementations/MeRepositoryImpl.swift (1)
getDeviceSupport(197-202)WeatherXMTests/DomainLayer/MockRepositories/MockMeRepositoryImpl.swift (1)
getDeviceSupport(262-276)WeatherXMTests/PresentationLayer/MockUseCases/MockMeUseCase.swift (1)
getDeviceSupport(200-214)wxm-ios/DomainLayer/DomainLayer/UseCases/MeUseCase.swift (1)
getDeviceSupport(189-191)
PresentationLayer/Navigation/Router.swift (1)
WeatherXMTests/PresentationLayer/Navigation/MockRouter.swift (1)
showBottomSheet(15-18)
WeatherXMTests/DomainLayer/MockRepositories/MockMeRepositoryImpl.swift (3)
wxm-ios/DataLayer/DataLayer/RepositoryImplementations/MeRepositoryImpl.swift (1)
getDeviceSupport(197-202)WeatherXMTests/PresentationLayer/MockUseCases/MockMeUseCase.swift (1)
getDeviceSupport(200-214)wxm-ios/DomainLayer/DomainLayer/UseCases/MeUseCase.swift (1)
getDeviceSupport(189-191)
wxm-ios/DataLayer/DataLayer/RepositoryImplementations/MeRepositoryImpl.swift (6)
WeatherXMTests/DataLayer/RepositoryImplementations/MeRepositoryImplTests.swift (1)
getDeviceSupport(124-129)WeatherXMTests/DomainLayer/MockRepositories/MockMeRepositoryImpl.swift (1)
getDeviceSupport(262-276)WeatherXMTests/PresentationLayer/MockUseCases/MockMeUseCase.swift (1)
getDeviceSupport(200-214)wxm-ios/DomainLayer/DomainLayer/UseCases/MeUseCase.swift (1)
getDeviceSupport(189-191)wxm-ios/DataLayer/DataLayer/Networking/ApiRequestBuilders/MeApiRequestBuilder.swift (1)
asURLRequest(15-36)wxm-ios/DataLayer/DataLayer/Networking/ApiClient.swift (1)
requestCodableAuthorized(72-75)
WeatherXMTests/PresentationLayer/Navigation/MockRouter.swift (1)
PresentationLayer/Navigation/Router.swift (1)
showBottomSheet(347-351)
PresentationLayer/UIComponents/Screens/WeatherStations/StationDetails/Overview/OverviewViewModel.swift (2)
PresentationLayer/UIComponents/ViewModelsFactory.swift (1)
getStationSupportViewModel(400-403)PresentationLayer/Navigation/Router.swift (1)
showBottomSheet(347-351)
PresentationLayer/UIComponents/Screens/WeatherStations/StationDetails/Overview/OverviewView.swift (2)
PresentationLayer/UIComponents/Screens/WeatherStations/StationDetails/Overview/OverviewViewModel.swift (2)
handleStationHealthInfoTap(79-81)handleAISupportTap(95-101)PresentationLayer/Constants/FontsEnum.swift (1)
fontAwesome(22-24)
PresentationLayer/UIComponents/Screens/WeatherStations/StationSupport/StationSupportView.swift (5)
PresentationLayer/UIComponents/BaseComponents/SpinningLoader/SpinningLoader.swift (1)
body(18-58)WeatherXMTests/PresentationLayer/ViewModels/StationSupportViewModelTests.swift (1)
refresh(22-30)PresentationLayer/UIComponents/Screens/WeatherStations/StationSupport/StationSupportViewModel.swift (1)
refresh(25-43)PresentationLayer/UIComponents/ViewModelsFactory.swift (1)
getStationSupportViewModel(400-403)PresentationLayer/Constants/FontsEnum.swift (1)
fontAwesome(22-24)
PresentationLayer/UIComponents/Screens/WeatherStations/StationSupport/StationSupportViewModel.swift (1)
PresentationLayer/Navigation/Router.swift (1)
hash(19-88)
🪛 GitHub Check: swiftLint
PresentationLayer/UIComponents/Screens/WeatherStations/StationDetails/Overview/OverviewView.swift
[warning] 143-143:
Trailing Comma Violation: Collection literals should not have trailing commas (trailing_comma)
PresentationLayer/UIComponents/Screens/WeatherStations/StationSupport/StationSupportViewModel.swift
[warning] 36-36:
Empty Enum Arguments Violation: Arguments can be omitted when matching enums with associated values if they are not used (empty_enum_arguments)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: WeatherXM | Unit tests | wxm-ios - iOS
🔇 Additional comments (27)
PresentationLayer/Constants/FontsEnum.swift (1)
99-99: Code is correct—FontAwesome icon exists.The
user-robot-xmarksicon exists in FontAwesome 6 and is properly integrated into the codebase. The enum case is correctly defined and actively used in the UI. No action is required.wxm-ios.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved (1)
2-2: Dependency updates are intentional and pose no risks to this codebase.Firebase iOS SDK 11.15.0 intentionally adds AI capabilities (confirmed in release notes with
countTokens,GenerativeAIRequest.Responseimprovements) directly supporting the PR's AI assistant integration objectives. Swinject 2.10.0's Swift 6 language mode is safe—the codebase uses only standardresolver.resolve()calls without arguments and has noContainer.resolve(_:arguments:)usage, so the documented Swift 6 source-breaking change does not apply. Other updates (Google Ads 2.3.0, gRPC 1.69.1, Swift Protobuf 1.31.1) are patch/minor versions with minimal risk. All updates appear intentional and tested.wxm-ios/DomainLayer/DomainLayer.xcodeproj/project.pbxproj (1)
18-18: LGTM! Standard Xcode project configuration.The new
NetworkDeviceSupportResponse.swiftfile is correctly wired into the build system with all necessary references (build file, file reference, group placement, and sources phase).Also applies to: 129-129, 445-445, 701-701
wxm-ios/DomainLayer/DomainLayer/Entities/Codables/Me/Network/NetworkDeviceSupportResponse.swift (1)
10-44: LGTM! Clean and well-structured data model.The
NetworkDeviceSupportResponsemodel follows established patterns with:
- Proper
Sendableconformance for Swift 6 concurrency- Appropriate JSON key mapping for
stationName- Type-safe
Statusenum- Optional properties to handle partial API responses gracefully
WeatherXMTests/DataLayer/RepositoryImplementations/MeRepositoryImplTests.swift (1)
124-129: LGTM! Test follows established patterns.The test appropriately validates:
- Successful status response
- Non-empty output data
Consistent with other repository tests in this file.
wxm-ios/DataLayer/DataLayer/Networking/Constants/ParameterConstants.swift (1)
37-37: LGTM! Constant addition follows existing patterns.The
stationNameconstant is correctly placed underParameterConstants.Meand follows the established naming convention.WeatherXMTests/DomainLayer/UseCases/MeUseCaseTests.swift (1)
185-191: LGTM! Test validates the device support flow.The test appropriately verifies the use case returns device support data with successful status and non-empty output. Follows existing test patterns in this file.
wxm-ios/DomainLayer/DomainLayer/DomainRepositoryInterfaces/MeRepository.swift (1)
67-67: LGTM! Protocol method follows established patterns.The
getDeviceSupportmethod signature is consistent with other repository methods, using:
- The same
DataResponsewrapper- The same error type (
NetworkErrorResponse)- The same publisher pattern with
Neverfailure typePresentationLayer/Navigation/RouterView.swift (1)
37-39: LGTM! Properly integrates background color support.The bottom sheet now correctly passes the
bgColorfrom the router to the modifier, enabling customizable background colors for different bottom sheet presentations.wxm-ios/DomainLayer/DomainLayer/UseCases/MeUseCase.swift (1)
189-191: LGTM!The implementation correctly delegates to the repository layer and follows the established pattern used by other methods in this file.
wxm-ios/DataLayer/DataLayer/Networking/Mock/Jsons/get_station_support.json (1)
1-12: LGTM!The mock JSON fixture is well-structured and provides realistic test data for the device support feature.
PresentationLayer/UIComponents/ViewModelsFactory.swift (1)
400-403: LGTM!The factory method follows the established pattern used by other view model factories in this file and correctly resolves dependencies from the DI container.
WeatherXMTests/PresentationLayer/MockUseCases/MockMeUseCase.swift (1)
200-214: LGTM!The mock implementation follows the same pattern as other mock methods in this file and provides consistent, deterministic test data.
WeatherXMTests/DomainLayer/MockRepositories/MockMeRepositoryImpl.swift (1)
262-276: LGTM!The mock repository implementation is consistent with the corresponding mock use case and follows the established pattern in this file.
wxm-ios/DataLayer/DataLayer.xcodeproj/project.pbxproj (1)
18-18: LGTM!The project file correctly registers the new JSON mock resource for inclusion in the build.
Also applies to: 130-130, 291-291, 609-609
wxm-ios/DataLayer/DataLayer/RepositoryImplementations/MeRepositoryImpl.swift (1)
197-202: LGTM!The implementation follows the established pattern used throughout this file, properly delegates to the API client with mock support, and handles errors consistently.
PresentationLayer/UIComponents/Screens/WeatherStations/StationDetails/Overview/OverviewViewModel.swift (1)
95-101: LGTM! Handler correctly integrates with factory and router.The implementation properly guards against nil device name and follows the established patterns in this file. The use of
ViewModelsFactory.getStationSupportViewModelandRouter.shared.showBottomSheetaligns with the relevant code snippets provided.wxm-ios/DomainLayer/DomainLayer/UseCases/UseCaseApi/MeUseCaseApi.swift (1)
42-42: Clarify identifier semantics and error contract for getDeviceSupport.
- Using deviceName (vs deviceId) is a departure from most Me APIs; confirm it’s a unique, stable identifier and aligns with backend contracts.
- Signature is throws + Never publisher; confirm throws is strictly for request construction (as with other APIs) and not for network/domain errors.
Would you like me to scan call sites and docs to ensure consistent naming and expectations?
PresentationLayer/UIComponents/Screens/WeatherStations/StationDetails/Overview/OverviewView.swift (1)
78-96: Verify header layout under long locales and Dynamic Type.Text + info button + AI button in one HStack with smallSpacing may compress/clip on narrow screens or longer translations. Consider:
- Give the title a higher layoutPriority and lineLimit(1) with truncation, or move AI button below on compact widths.
I can provide a quick preview variant to test Dynamic Type and DE/FR localizations.
PresentationLayer/UIComponents/BaseComponents/SpinningLoader/SpinningLoader.swift (1)
15-35: No changes required—code is backward compatible.The
showLoaderBgparameter defaults tofalse(line 63), so all 27 existing call sites automatically preserve current behavior without modification. The background circle logic is correctly gated and only renders when explicitly enabled. The change is safe and introduces no visual regressions.PresentationLayer/Navigation/Router.swift (5)
83-84: LGTM!The hash implementation for the new
stationSupportcase correctly follows the established pattern for view-model-based routes.
154-155: LGTM!The string representation correctly follows the naming convention used by other routes.
191-191: LGTM!The route case declaration correctly follows the pattern established by other view-model-based routes.
297-298: LGTM!The view mapping correctly instantiates
StationSupportViewwith the associated view model, following the pattern used by other routes.
329-329: No issues found—code is correct as-is.The verification confirms RouterView properly reads
bottomSheetBgColorvia line 37:.bottomSheet(show: $router.showBottomSheet, bgColor: router.bottomSheetBgColor). Since the color is always modified alongside the@Published showBottomSheet(see Router.swift lines 329 and 349), changes toshowBottomSheettrigger the view update, andbottomSheetBgColoris read during that render cycle. The lack of@PublishedonbottomSheetBgColoris not a concern because the property is never mutated independently. No reactivity issues will occur.wxm-ios/Resources/Localizable/LocalizableString+StationDetails.swift (2)
62-62: LGTM: Enum cases properly declared.The new localization cases are correctly declared and follow the existing naming conventions and structure.
Also applies to: 85-88
224-225: All localization keys are properly defined.Verification confirms that all five new keys are present in
Localizable.xcstrings. The code mappings are correct and will display localized text as intended.
| case XXLTitleFontSize | ||
| case XXXLTitleFontSize | ||
| case maxFontSize | ||
| case hugeFontSize |
There was a problem hiding this comment.
Semantic inconsistency: maxFontSize is no longer the maximum.
The name maxFontSize suggests it's the largest font size available, but hugeFontSize (80.0) is now larger than maxFontSize (64.0). This creates confusion and could lead to incorrect usage if developers assume maxFontSize is actually the maximum.
Consider renaming for clarity:
case XXXLTitleFontSize
- case maxFontSize
+ case extraHugeFontSize // or XXXXLTitleFontSize for consistency
case hugeFontSizeOr alternatively, if maxFontSize has existing usage that's difficult to change, consider renaming the new case:
case maxFontSize
- case hugeFontSize
+ case superMaxFontSize // though this perpetuates the naming issueAlso applies to: 57-58
🤖 Prompt for AI Agents
In PresentationLayer/Constants/FontSizeEnum.swift around line 25 (and also lines
57-58), the semantic meaning of maxFontSize is broken because hugeFontSize
(80.0) is larger than maxFontSize (64.0); either rename maxFontSize to something
that reflects it is not the absolute maximum (e.g., defaultMaxFontSize or
standardMaxFontSize) or rename the new case (hugeFontSize) to indicate it
exceeds the previous maximum (e.g., extraHugeFontSize or overflowFontSize) and
update any references/usages accordingly so the enum names accurately reflect
their relative sizes.
There was a problem hiding this comment.
Fix SwiftLint trailing comma and add basic accessibility to AI button.
- Lint: trailing comma in gradient stops.
- A11y: ensure clear label and minimum hit target.
Apply:
@@
- LinearGradient(
- stops: [
- Gradient.Stop(color: Color(red: 0.91, green: 0.59, blue: 0.69), location: 0.00),
- Gradient.Stop(color: Color(red: 0.73, green: 0.53, blue: 0.89), location: 0.25),
- Gradient.Stop(color: Color(red: 0.51, green: 0.71, blue: 0.86), location: 0.63),
- Gradient.Stop(color: Color(red: 0.41, green: 0.46, blue: 0.84), location: 1.00),
- ],
- startPoint: UnitPoint(x: 0, y: 0.5),
- endPoint: UnitPoint(x: 1, y: 0.5))
+ LinearGradient(
+ stops: [
+ Gradient.Stop(color: Color(red: 0.91, green: 0.59, blue: 0.69), location: 0.00),
+ Gradient.Stop(color: Color(red: 0.73, green: 0.53, blue: 0.89), location: 0.25),
+ Gradient.Stop(color: Color(red: 0.51, green: 0.71, blue: 0.86), location: 0.63),
+ Gradient.Stop(color: Color(red: 0.41, green: 0.46, blue: 0.84), location: 1.00)
+ ],
+ startPoint: UnitPoint(x: 0, y: 0.5),
+ endPoint: UnitPoint(x: 1, y: 0.5))
@@
- .cornerRadius(CGFloat(.cardCornerRadius))
+ .cornerRadius(CGFloat(.cardCornerRadius))
+ .frame(minHeight: 44) // a11y minimum target
+ .accessibilityLabel(Text(LocalizableString.StationDetails.aiHealthCheck.localized))This should also address the SwiftLint warning.
🧰 Tools
🪛 GitHub Check: swiftLint
[warning] 143-143:
Trailing Comma Violation: Collection literals should not have trailing commas (trailing_comma)
| func refresh() { | ||
| mode = .loading | ||
| do { | ||
| try useCase.getDeviceSupport(deviceName: stationName).receive(on: DispatchQueue.main).sink { [weak self] response in | ||
| switch response.result { | ||
| case .success(let support): | ||
| if let markdownString = support.outputs?.result { | ||
| self?.mode = .content(markdownString: markdownString) | ||
| } else { | ||
| self?.mode = .error | ||
| } | ||
| case .failure(_): | ||
| self?.mode = .error | ||
| } | ||
| }.store(in: &cancellables) | ||
| } catch { | ||
| print(error) | ||
| } | ||
| } |
There was a problem hiding this comment.
Ensure error states are surfaced; fix lint; guard empty markdown.
- If
getDeviceSupportthrows, the UI remains in.loading. Set.errorincatch. - SwiftLint: prefer
case .failure:overcase .failure(_). - Treat empty/whitespace result as error for better UX.
func refresh() {
mode = .loading
do {
- try useCase.getDeviceSupport(deviceName: stationName).receive(on: DispatchQueue.main).sink { [weak self] response in
+ try useCase.getDeviceSupport(deviceName: stationName)
+ .receive(on: DispatchQueue.main)
+ .sink { [weak self] response in
switch response.result {
case .success(let support):
- if let markdownString = support.outputs?.result {
- self?.mode = .content(markdownString: markdownString)
+ if let markdown = support.outputs?.result,
+ !markdown.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty {
+ self?.mode = .content(markdownString: markdown)
} else {
self?.mode = .error
}
- case .failure(_):
+ case .failure:
self?.mode = .error
}
}.store(in: &cancellables)
} catch {
- print(error)
+ print(error)
+ mode = .error
}
}Optional: clear previous subscription before new fetch to avoid stacking sinks:
cancellables.removeAll() at the start of refresh().
🧰 Tools
🪛 GitHub Check: swiftLint
[warning] 36-36:
Empty Enum Arguments Violation: Arguments can be omitted when matching enums with associated values if they are not used (empty_enum_arguments)
🤖 Prompt for AI Agents
In
PresentationLayer/UIComponents/Screens/WeatherStations/StationSupport/StationSupportViewModel.swift
around lines 25 to 43, the refresh() method leaves the UI in .loading if
useCase.getDeviceSupport throws, uses a lint-unfriendly pattern for failure, and
treats empty/whitespace markdown as content; update the catch block to set
self?.mode = .error, replace "case .failure(_):" with "case .failure:" per
SwiftLint, guard the fetched markdown by trimming whitespace/newlines and treat
an empty trimmed string as an error (set .error) instead of content, and
optionally clear previous subscriptions at the start of refresh() by calling
cancellables.removeAll() to avoid stacking sinks.
| @Test func refresh() async throws { | ||
| #expect(viewModel.mode == .loading) | ||
| try await confirmation { confirm in | ||
| viewModel.refresh() | ||
| try await Task.sleep(for: .seconds(1)) | ||
| #expect(viewModel.mode == .content(markdownString: "")) | ||
| confirm() | ||
| } | ||
| } |
There was a problem hiding this comment.
Verify the expected value in the test assertion.
Line 27 expects markdownString: "" (empty string), but the MockMeUseCase.getDeviceSupport implementation at lines 200-214 in WeatherXMTests/PresentationLayer/MockUseCases/MockMeUseCase.swift returns "This is the result". This mismatch suggests either:
- The test assertion is incorrect and should expect
"This is the result" - The ViewModel transforms the result in a way not evident from the provided context
Apply this diff if the test should verify the actual mock response:
- #expect(viewModel.mode == .content(markdownString: ""))
+ #expect(viewModel.mode == .content(markdownString: "This is the result"))📝 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.
| @Test func refresh() async throws { | |
| #expect(viewModel.mode == .loading) | |
| try await confirmation { confirm in | |
| viewModel.refresh() | |
| try await Task.sleep(for: .seconds(1)) | |
| #expect(viewModel.mode == .content(markdownString: "")) | |
| confirm() | |
| } | |
| } | |
| @Test func refresh() async throws { | |
| #expect(viewModel.mode == .loading) | |
| try await confirmation { confirm in | |
| viewModel.refresh() | |
| try await Task.sleep(for: .seconds(1)) | |
| #expect(viewModel.mode == .content(markdownString: "This is the result")) | |
| confirm() | |
| } | |
| } |
🤖 Prompt for AI Agents
In
WeatherXMTests/PresentationLayer/ViewModels/StationSupportViewModelTests.swift
around lines 22 to 30, the test asserts viewModel.mode ==
.content(markdownString: "") but the MockMeUseCase.getDeviceSupport returns
"This is the result"; update the expectation to assert .content(markdownString:
"This is the result") (or adjust the mock/transform if the ViewModel should
alter the text) so the test verifies the actual mock response.
| return "me/devices/\(deviceName)/support" | ||
| } | ||
| } |
There was a problem hiding this comment.
Encode deviceName as a path component to prevent path injection and 404s.
Interpolating deviceName directly into the URL path is unsafe (spaces, “/”, “%”, etc.). Percent‑encode as a single path component.
Apply:
- case let .deviceSupport(deviceName):
- return "me/devices/\(deviceName)/support"
+ case let .deviceSupport(deviceName):
+ let allowed = CharacterSet.alphanumerics.union(CharacterSet(charactersIn: "-._~"))
+ let safe = deviceName.addingPercentEncoding(withAllowedCharacters: allowed) ?? deviceName
+ return "me/devices/\(safe)/support"Also consider using a small helper for path components to keep this consistent across builders.
| "station_details_ai_health_check" : { | ||
| "extractionState" : "manual", | ||
| "localizations" : { | ||
| "en" : { | ||
| "stringUnit" : { | ||
| "state" : "translated", | ||
| "value" : "AI Health Check" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "station_details_analyzing_your_station" : { | ||
| "extractionState" : "manual", | ||
| "localizations" : { | ||
| "en" : { | ||
| "stringUnit" : { | ||
| "state" : "translated", | ||
| "value" : "Analyzing your station..." | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "station_details_analyzing_your_station_description" : { | ||
| "extractionState" : "manual", | ||
| "localizations" : { | ||
| "en" : { | ||
| "stringUnit" : { | ||
| "state" : "translated", | ||
| "value" : "Our AI is checking every sensor for you.\nYour station’s story is loading, give it some time!" | ||
| } | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
Remove duplicate AI health-check strings
The four keys introduced for the AI health-check flow are defined twice in this catalog. JSON objects require unique keys; keeping duplicates risks parser crashes or silently discarded entries when Xcode ingests the catalog.(gist.github.com) Please drop the second copy so each key is present only once.
- "station_details_storm_in_system" : {
- "extractionState" : "manual",
- "localizations" : {
- "en" : {
- "stringUnit" : {
- "state" : "translated",
- "value" : "Oops, storm in the system!"
- }
- }
- }
- },
- "station_details_storm_in_system_description" : {
- "extractionState" : "manual",
- "localizations" : {
- "en" : {
- "stringUnit" : {
- "state" : "translated",
- "value" : "Something went wrong while checking your station.\nTry again in a moment."
- }
- }
- }
- },Also applies to: 10893-10914
🤖 Prompt for AI Agents
In wxm-ios/Resources/Localizable/Localizable.xcstrings around lines 10156-10188
and also 10893-10914, there are duplicate definitions of the four AI
health-check keys (station_details_ai_health_check,
station_details_analyzing_your_station,
station_details_analyzing_your_station_description, etc.); remove the second
copy of these keys so each key appears exactly once in the file, ensuring the
remaining entry preserves the correct extractionState/localizations structure
and values and that no other keys are accidentally removed.
Code Coverage Summary
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
PresentationLayer/UIComponents/Screens/WeatherStations/StationSupport/StationSupportViewModel.swift (1)
25-46: Address remaining issues from previous review.While the catch block now correctly sets
mode = .error(good!), two critical items from the previous review remain unaddressed:
- SwiftLint violation (line 38): Use
case .failure:instead ofcase .failure(_):per the empty enum arguments rule.- Empty markdown validation (line 33): Treat empty or whitespace-only markdown as an error state for better UX.
- Error logging (line 43): Replace
print(error)with proper error logging or structured logging.Apply this diff to address all issues:
func refresh() { mode = .loading do { try useCase.getDeviceSupport(deviceName: stationName) .receive(on: DispatchQueue.main) .sink { [weak self] response in switch response.result { case .success(let support): - if let markdownString = support.outputs?.result { + if let markdownString = support.outputs?.result, + !markdownString.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty { self?.mode = .content(markdownString: markdownString) } else { self?.mode = .error } - case .failure(_): + case .failure: self?.mode = .error } }.store(in: &cancellables) } catch { - print(error) + // TODO: Use proper error logging instead of print mode = .error } }Optional: Consider clearing previous subscriptions at the start of
refresh()to avoid stacking sinks if called multiple times:cancellables.removeAll().
🧹 Nitpick comments (1)
PresentationLayer/UIComponents/Screens/WeatherStations/StationSupport/StationSupportView.swift (1)
83-103: Consider using the existingspinningLoadermodifier to reduce duplication.The
loadingViewduplicates functionality already available in theSpinningLoadermodifier (shown in relevant snippets), which supports spinner, background circle, title, and subtitle. Using the existing modifier would reduce code duplication and maintain consistency.Replace the custom loading view with the modifier:
-case .loading: - loadingView +case .loading: + Color.clear + .spinningLoader(show: true, + showLoaderBg: true, + title: LocalizableString.StationDetails.analyzingYourStation.localized, + subtitle: LocalizableString.StationDetails.analyzingYourStationDescription.localized)Then remove the
loadingViewcomputed property (lines 83-103).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
PresentationLayer/UIComponents/Screens/WeatherStations/StationDetails/Overview/OverviewViewModel.swift(2 hunks)PresentationLayer/UIComponents/Screens/WeatherStations/StationSupport/StationSupportView.swift(1 hunks)PresentationLayer/UIComponents/Screens/WeatherStations/StationSupport/StationSupportViewModel.swift(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- PresentationLayer/UIComponents/Screens/WeatherStations/StationDetails/Overview/OverviewViewModel.swift
🧰 Additional context used
🧬 Code graph analysis (2)
PresentationLayer/UIComponents/Screens/WeatherStations/StationSupport/StationSupportView.swift (4)
PresentationLayer/UIComponents/BaseComponents/SpinningLoader/SpinningLoader.swift (1)
body(18-58)PresentationLayer/UIComponents/Screens/WeatherStations/StationSupport/StationSupportViewModel.swift (1)
refresh(25-46)PresentationLayer/UIComponents/ViewModelsFactory.swift (1)
getStationSupportViewModel(400-403)PresentationLayer/Constants/FontsEnum.swift (1)
fontAwesome(22-24)
PresentationLayer/UIComponents/Screens/WeatherStations/StationSupport/StationSupportViewModel.swift (1)
PresentationLayer/Navigation/Router.swift (1)
hash(19-88)
🪛 GitHub Check: swiftLint
PresentationLayer/UIComponents/Screens/WeatherStations/StationSupport/StationSupportViewModel.swift
[warning] 38-38:
Empty Enum Arguments Violation: Arguments can be omitted when matching enums with associated values if they are not used (empty_enum_arguments)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: WeatherXM | Unit tests | wxm-ios - iOS
🔇 Additional comments (6)
PresentationLayer/UIComponents/Screens/WeatherStations/StationSupport/StationSupportView.swift (4)
11-39: LGTM!The view structure is clean and follows SwiftUI best practices. The state-driven mode switching and automatic refresh on appearance are well implemented.
62-81: LGTM!The markdown rendering with proper title and styling is well implemented.
105-128: LGTM!The error view with icon and messaging is well structured and provides clear feedback to users.
131-133: LGTM!Preview setup correctly uses the factory pattern.
PresentationLayer/UIComponents/Screens/WeatherStations/StationSupport/StationSupportViewModel.swift (2)
12-23: LGTM!The class structure is well designed with appropriate use of
@MainActorfor UI state management and proper encapsulation of dependencies.
49-53: LGTM!The
HashableViewModelconformance correctly hashes bystationNameand is consistent with the router pattern.
| static func == (lhs: Mode, rhs: Mode) -> Bool { | ||
| switch (lhs, rhs) { | ||
| case (.loading, .loading): | ||
| return true | ||
| case (.error, .error): | ||
| return true | ||
| case (.content, .content): | ||
| return true | ||
| default: | ||
| return false | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix Equatable to compare associated markdown value.
The custom Equatable implementation only compares enum cases, ignoring the markdownString associated value. This means two .content cases with different markdown are considered equal, which can prevent SwiftUI from updating the view when the markdown changes.
Apply this diff to properly compare the associated value:
static func == (lhs: Mode, rhs: Mode) -> Bool {
switch (lhs, rhs) {
case (.loading, .loading):
return true
case (.error, .error):
return true
- case (.content, .content):
- return true
+ case (.content(let lhsMarkdown), .content(let rhsMarkdown)):
+ return lhsMarkdown == rhsMarkdown
default:
return false
}
}🤖 Prompt for AI Agents
In
PresentationLayer/UIComponents/Screens/WeatherStations/StationSupport/StationSupportView.swift
around lines 47 to 58, the custom Equatable only compares enum cases and ignores
the markdownString associated value for the .content case; update the ==
implementation so that for .content it returns true only when both are .content
and their associated markdownString values are equal (e.g., compare
lhs.markdownString == rhs.markdownString), keeping the existing comparisons for
.loading and .error unchanged.
Code Coverage Summary
|
pantelisss
deleted the
feature/fe-1995-implementation-for-ai-assistant-integration
branch
3 participants
Why?
AI support troubleshooting for problematic stations
How?
MeApiRequestBuilderStationSupportViewshows the bottom sheet with the answerAI Health Checkbutton is rended in the overview tab of an owned stationTesting
Ensure the button is visible only in owned stations with
QOD < 80and everything else looks as expectedAdditional context
fixes fe-1995
Summary by CodeRabbit
New Features
Style
Localization