.pr_agent_accepted_suggestions

PR 609 (2026-01-29)

[high-level] Decouple PWA logic from model number

---

✅ Decouple PWA logic from model number

Relocate the PWA banner's model number check from the InstallPromptBanner UI component to the PwaInstallService. This change centralizes all business rules for displaying the banner, improving code structure and maintainability.

Examples:

lib/page/components/pwa/install_prompt_banner.dart [26-28]

    if (!modelNumber.contains('DU-')) {
      return const SizedBox.shrink();
    }

lib/core/pwa/pwa_install_service.dart [29-49]

  Future _checkPlatformAndPersistence() async {
    // If installed (standalone), never show
    if (isStandalone) {
      state = PwaMode.none;
      return;
    }

    final dismissed = await isDismissedRecently();
    if (dismissed) {
      state = PwaMode.none;

 ... (clipped 11 lines)

Solution Walkthrough:

Before:

// In lib/page/components/pwa/install_prompt_banner.dart
class InstallPromptBanner extends ConsumerWidget {
  Widget build(BuildContext context, WidgetRef ref) {
    final modelNumber = ref.watch(globalModelNumberProvider);

    // Business logic is coupled with the UI component
    if (!modelNumber.contains('DU-')) {
      return const SizedBox.shrink();
    }

    final mode = ref.watch(pwaInstallServiceProvider);
    if (mode == PwaMode.none) {
      return const SizedBox.shrink();
    }
    // ... build banner
  }
}

After:

// In lib/page/components/pwa/install_prompt_banner.dart
class InstallPromptBanner extends ConsumerWidget {
  Widget build(BuildContext context, WidgetRef ref) {
    // UI component only depends on the service's final state
    final mode = ref.watch(pwaInstallServiceProvider);
    if (mode == PwaMode.none) {
      return const SizedBox.shrink();
    }
    // ... build banner
  }
}

// In lib/core/pwa/pwa_install_service.dart
class PwaInstallService extends Notifier {
  Future _checkPlatformAndPersistence() async {
    // Business logic is centralized in the service
    final modelNumber = ref.read(globalModelNumberProvider);
    if (!modelNumber.contains('DU-')) {
      state = PwaMode.none;
      return;
    }
    // ... other checks
  }
}

Suggestion importance[1-10]: 7

__

Why: This is a strong architectural suggestion that correctly identifies business logic (modelNumber check) misplaced in a UI component, improving maintainability by centralizing all display conditions within PwaInstallService.

---

[possible issue] Reuse PwaLogic instance for consistency

---

✅ Reuse PwaLogic instance for consistency

Refactor PwaInstallService to use a single, cached instance of PwaLogic instead of creating new instances in multiple methods.

lib/core/pwa/pwa_install_service.dart [102-114]

-Future<void> promptInstall() async {
-  if (_deferredPrompt == null) {
-    debugPrint('PWA: No deferred prompt available');
-    return;
+class PwaInstallService extends Notifier<PwaMode> {
+  final _pwaLogic = PwaLogic();
+  // ... (rest of the class)
+
+  // ... in _initListeners()
+  // _pwaLogic.initListeners(...)
+
+  // ... in promptInstall()
+  Future<void> promptInstall() async {
+    if (_deferredPrompt == null) {
+      debugPrint('PWA: No deferred prompt available');
+      return;
+    }
+
+    // Show the install prompt
+    await _pwaLogic.prompt(_deferredPrompt!);
+
+    // We've used the prompt, so we can't use it again.
+    _deferredPrompt = null;
+    state = PwaMode.none;
   }
 
-  // Show the install prompt
-  await PwaLogic().prompt(_deferredPrompt!);
-
-  // We've used the prompt, so we can't use it again.
-  _deferredPrompt = null;
-  state = PwaMode.none;
+  // ... similar changes for isIOS, isMacSafari, isStandalone
 }

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that creating new instances of PwaLogic is inefficient and proposes a good refactoring to use a single instance, improving code quality and consistency.

---

[possible issue] Fix malformed head tags

---

✅ Fix malformed head tags

Correct the malformed HTML in index.html by moving the viewport tag inside the section and removing the duplicate closing tag.

web/index.html [147-149]

-</head>
 <meta content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no" name="viewport">
 </head>

Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies and fixes malformed HTML where a tag is placed between two tags, which improves the structural correctness of the document.

---

[general] Eliminate unused helper file

---

✅ Eliminate unused helper file

Remove the unused pwa_helpers_web.dart and pwa_helpers_stub.dart files to eliminate dead code and duplication.

lib/core/pwa/pwa_helpers_web.dart [6-19]

-extension type BeforeInstallPromptEvent._(JSObject _) implements Event {
-  external JSPromise prompt();
-  external JSPromise<UserChoice> get userChoice;
-}
+// (file removed; definitions consolidated in pwa_logic_web.dart)
 
-extension type UserChoice._(JSObject _) implements JSObject {
-  external String get outcome;
-  external String get platform;
-}
-
-extension WindowExtension on Window {
-  external JSObject? get deferredBeforeInstallPromptEvent;
-}
-

Suggestion importance[1-10]: 7

__

Why: This suggestion correctly identifies that pwa_helpers_web.dart and its stub are unused and contain duplicated code, so removing them is a good cleanup that improves maintainability.

---

---

PR 607 (2026-01-28)

[possible issue] Fix incorrect null handling for server ID

---

✅ Fix incorrect null handling for server ID

Fix the runHealthCheck payload by ensuring targetServerID is null (not the string "null") when no server is selected, allowing it to be correctly removed from the request. Also, simplify the ID retrieval to avoid unnecessary type parsing.

lib/page/health_check/providers/health_check_provider.dart [51-65]

-final targetServerId = serverId ??
-    (state.selectedServer?.serverID != null
-        ? int.tryParse(state.selectedServer!.serverID)
-        : null);
+final targetServerId = serverId?.toString() ?? state.selectedServer?.serverID;
 
 final result = await repo.send(
   JNAPAction.runHealthCheck,
   data: {
     "runHealthCheckModule": module.value,
-    "targetServerID": '$targetServerId',
+    "targetServerID": targetServerId,
   }..removeWhere((key, value) => value == null),
   auth: true,
   fetchRemote: true,
   cacheLevel: CacheLevel.noCache,
 );

Suggestion importance[1-10]: 8

__

Why: This suggestion correctly identifies a bug where a null server ID is converted to the string "null" and sent in the payload, violating the requirement to omit the parameter. The fix is accurate and improves the code's correctness and robustness.

---

---