refactor: fetch speed test servers on-demand instead of polling

[AustinChangLinksys]

Summary

• Remove GetCloseHealthCheckServers from core transaction to reduce polling payload overhead
• updateServers() now fetches directly from router (fetchRemote: true) on-demand when entering the speed test view
• Added loading spinner while fetching server list, with GO button disabled during fetch
• Differentiate between fetch error and empty server list: error shows retry button, empty list falls back to legacy flow silently

Closes #713

Test plan

• Verify speed test page shows spinner then dropdown when servers load successfully
• Verify GO button is disabled during server fetch
• Verify error message + retry icon appears when server fetch fails (e.g. network issue)
• Verify retry button re-triggers fetch and recovers correctly
• Verify empty server list from router falls back to legacy flow (GO enabled, no dropdown)
• Verify Dashboard / Instant Verify speed test dialog still works correctly
• Verify polling payload no longer includes GetCloseHealthCheckServers

🤖 Generated with Claude Code

[qodo-code-review]

Review Summary by Qodo

Fetch speed test servers on-demand with loading and error states

✨ Enhancement 🐞 Bug fix

Walkthroughs

Description

• Remove polling overhead by fetching speed test servers on-demand
• Add loading spinner and error handling with retry capability
• Differentiate server fetch errors from empty lists for proper fallback
• Disable GO button during server fetch to prevent premature test start

Diagram

flowchart LR
  A["Polling Provider"] -->|Remove GetCloseHealthCheckServers| B["Reduced Polling Payload"]
  C["Speed Test View"] -->|On-demand fetch| D["Health Check Provider"]
  D -->|fetchRemote: true| E["Router Repository"]
  E -->|Success| F["Display Servers Dropdown"]
  E -->|Error| G["Show Error + Retry Button"]
  E -->|Empty| H["Legacy Fallback"]
  I["Loading State"] -->|Disable GO Button| J["Prevent Premature Test"]

Loading

File Changes

1. lib/core/jnap/providers/polling_provider.dart ✨ Enhancement +0/-3

Remove server polling from core transaction

• Removed GetCloseHealthCheckServers JNAP action from polling commands
• Reduces core transaction payload by eliminating unnecessary polling of server list
• Server list now fetched on-demand instead of continuously polled

lib/core/jnap/providers/polling_provider.dart

---

2. lib/page/health_check/providers/health_check_provider.dart ✨ Enhancement +7/-8

Implement on-demand server fetching with error tracking

• Changed updateServers() to fetch directly from router with fetchRemote: true
• Removed reliance on polling cache and CacheLevel.localCached
• Added serversError state tracking to differentiate fetch errors from empty lists
• Updated error handling to set error flag and return empty list on failure

lib/page/health_check/providers/health_check_provider.dart

---

3. lib/page/health_check/providers/health_check_state.dart ✨ Enhancement +9/-1

Add server error state tracking to health check state

• Added serversError boolean field to track server fetch failures
• Updated constructor, factory methods, and copyWith() to include new field
• Added serversError to equality props and serialization methods
• Initialized serversError to false by default

lib/page/health_check/providers/health_check_state.dart

---

View more (2)

4. lib/page/health_check/views/speed_test_view.dart ✨ Enhancement +36/-8

Add loading spinner and error UI for server fetch

• Added _loadingServers state variable to track server fetch progress
• Implemented loading spinner display during server fetch
• Added error state UI with retry button when server fetch fails
• Disabled GO button during server loading to prevent premature test start
• Reordered conditional rendering to show loading, error, then dropdown states

lib/page/health_check/views/speed_test_view.dart

---

5. lib/l10n/app_en.arb 📝 Documentation +1/-0

Add localization for server load error message

• Added new localization string speedTestFailedToLoadServers for error message
• Provides user-facing text when server list fails to load

lib/l10n/app_en.arb

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

1. Cannot clear selected server ☑ 🐞 Bug ✓ Correctness

Description

HealthCheckState.copyWith() prevents setting selectedServer to null, so setSelectedServer(null)
never clears an existing selection. SpeedTestView’s “empty” dropdown option attempts to clear the
selection but the state retains the previous server, making the UI behavior inconsistent.

Code

lib/page/health_check/providers/health_check_state.dart[R80-81]

     selectedServer: selectedServer ?? this.selectedServer,
+      serversError: serversError ?? this.serversError,

Evidence

HealthCheckProvider.setSelectedServer(null) passes null into copyWith(), but copyWith uses the
null-coalescing operator for selectedServer, so null is replaced by the existing value.
SpeedTestView explicitly calls setSelectedServer(null) when the empty dropdown item is chosen, so
the intent to clear selection cannot work.

lib/page/health_check/providers/health_check_provider.dart[274-276]
lib/page/health_check/providers/health_check_state.dart[59-82]
lib/page/health_check/views/speed_test_view.dart[248-262]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`HealthCheckState.copyWith()` cannot clear `selectedServer` because it uses `selectedServer ?? this.selectedServer`. Calls like `setSelectedServer(null)` therefore keep the old value, which breaks the Speed Test dropdown's “empty” option behavior.
### Issue Context
`SpeedTestView` includes an empty dropdown option that calls `setSelectedServer(null)`, implying the UX supports clearing the selection.
### Fix Focus Areas
- lib/page/health_check/providers/health_check_state.dart[59-82]
- lib/page/health_check/providers/health_check_provider.dart[274-276]
- lib/page/health_check/views/speed_test_view.dart[248-262]
### Implementation notes
Update `copyWith` to support *explicitly* setting `selectedServer` to null (e.g., via a sentinel/`Object?` parameter pattern or an additional boolean like `clearSelectedServer`). Then keep `setSelectedServer(null)` working as intended.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Missing locale translations ☑ 🐞 Bug ⚙ Maintainability

Description

The new speedTestFailedToLoadServers string is only added to app_en.arb, but the UI now references
it for all locales. Other locale ARB files do not define this key, leading to untranslated output
being generated/tracked (and potentially incorrect UI text for non-English users).

Code

lib/l10n/app_en.arb[910]

+  "speedTestFailedToLoadServers": "Failed to load server list",

Evidence

The new key is present in the English template ARB but absent from at least one other locale file
(e.g., Spanish). The project’s l10n.yaml uses app_en.arb as the template and tracks missing
translations via untranslated-messages-file, so missing keys are an expected maintenance concern.

lib/l10n/app_en.arb[900-914]
lib/l10n/app_es.arb[710-721]
l10n.yaml[1-6]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new localization key is used in UI but is only defined in `app_en.arb`. Other locales lack the key, causing missing/untranslated strings for non-English users.
### Issue Context
`l10n.yaml` uses `app_en.arb` as the template, and the UI references `speedTestFailedToLoadServers`.
### Fix Focus Areas
- lib/l10n/app_en.arb[900-914]
- lib/l10n/app_es.arb[710-721]
- l10n.yaml[1-6]
### Implementation notes
Add `"speedTestFailedToLoadServers": "..."` to each `lib/l10n/app_*.arb` (with appropriate translations, or at minimum an English placeholder to unblock builds/automation if translations are handled later).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Loading flag may stick 🐞 Bug ⛯ Reliability

Description

_fetchServers() sets _loadingServers=true before awaiting updateServers(), but does not guarantee
resetting it via try/finally. If an unexpected exception occurs during the fetch path, the UI can
remain stuck in a loading state with GO disabled.

Code

lib/page/health_check/views/speed_test_view.dart[R49-62]

 Future<void> _fetchServers() async {
   final serviceHelper = getIt<ServiceHelper>();
   if (serviceHelper.isSupportHealthCheckManager2()) {
+      setState(() {
+        _loadingServers = true;
+      });
     final notifier = ref.read(healthCheckProvider.notifier);
     await notifier.updateServers();
-      // State is updated in provider, use ref.read to check
     if (mounted) {
-        // final state = ref.read(healthCheckProvider);
-        // if (state.servers.isNotEmpty && state.selectedServer == null) {
-        //   notifier.setSelectedServer(state.servers.first);
-        // }
+        setState(() {
+          _loadingServers = false;
+        });
     }
   }

Evidence

The only reset of _loadingServers occurs after the await and inside a mounted-check; there is no
catch/finally to ensure the flag is cleared on error paths.

lib/page/health_check/views/speed_test_view.dart[49-63]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`_fetchServers()` can leave `_loadingServers` stuck as `true` if an exception happens after setting it and before the reset `setState`, resulting in a permanently disabled GO button/spinner.
### Issue Context
Even if `updateServers()` currently catches common failures, this method should still be exception-safe to avoid future regressions (e.g., parsing changes, DI failures, unexpected throws).
### Fix Focus Areas
- lib/page/health_check/views/speed_test_view.dart[49-63]
### Implementation notes
Use `try { await ... } finally { if (mounted) setState(() => _loadingServers = false); }` (and optionally guard against re-entrancy by early-returning when `_loadingServers` is already true).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[AustinChangLinksys]

Addressed Qodo Review (6b939e0)

1. Cannot clear selected server (Bug)

Added clearSelectedServer flag to copyWith() to explicitly support setting selectedServer to null. Updated setSelectedServer(null) to pass clearSelectedServer: true, so the dropdown "empty" option now correctly clears the selection.

2. Missing locale translations (Bug)

Added speedTestFailedToLoadServers with proper translations to all 25 locale .arb files (ar, da, de, el, es, es_ar, fi, fr, fr_ca, id, it, ja, ko, nb, nl, pl, pt, pt_pt, ru, sv, th, tr, vi, zh, zh_TW).