Gemma 4 Local LLM, Create Alarms And Timer from ZSWatch#4
Merged
Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds on-device LiteRT-LM inference and Flutter bridge, end-to-end watch↔phone voice chat with TTS and persisted chat history (DB migration), watchface image processing and upload, file-system probing/downloads, lifecycle diagnostics and background-recovery wiring, Android boot receiver/permissions, platform plugin additions, and multiple UI screens/providers. ChangesOn‑Device LLM & End-to-End Chat
Watchface Background Image Processing & Upload
File System Diagnostics, Lifecycle Logging & Background Recovery
Voice Memo / Timer/Alarm Extraction & Processing
DFU / Filesystem Uploads, Firmware Manager
Platform plugins, dependencies, navigation & UI updates
Sequence Diagram(s)sequenceDiagram
participant Watch
participant Phone as Phone:WatchChatService
participant Trans as TranscriptionEngine
participant LLM as LlmService
participant TTS as TtsService
participant DB as AppDatabase
Watch->>Phone: question_ready (sessionId, audioPath, codec)
Phone->>Phone: validate + dedupe
Phone->>Phone: phase=receivingAudio
Phone->>Phone: download audio via FsManager
Phone->>Trans: transcribe(audioWav)
Trans-->>Phone: transcript
Phone->>Phone: phase=transcribing
Phone->>LLM: generateChatReply(transcript, history, answerLength)
LLM-->>Phone: reply text (+latency)
Phone->>Phone: phase=thinking
Phone->>TTS: synthesizeToFile(reply)
TTS-->>Phone: replyWav (48kHz PCM)
Phone->>Phone: uploadWav to watch (FsManager)
Phone->>Watch: reply_ready
Watch->>Phone: playback_done
Phone->>DB: insertChatHistory(success/failed)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Pull request overview
This PR expands the companion app’s AI + tooling capabilities by adding (1) timer/alarm extracted actions that can be created via OS clock intents/URLs, (2) Android local LiteRT-LM (Gemma) inference plumbing, and (3) new developer/watch utilities (filesystem log viewer, watchface background picker + crop/upload) while also hardening SMP enable/disable handling and improving AI pipeline robustness against “no speech” transcripts.
Changes:
- Add timer/alarm intent routing & extraction (schema + DB + UI) and OS-level creation support (Android intents, iOS Clock deep-links).
- Add Android LiteRT-LM bridge/service for local on-device inference.
- Add watchface background selection (built-ins + custom crop/upload) and a developer filesystem/log viewer; improve SMP safety + crash recovery for AI processing.
Reviewed changes
Copilot reviewed 64 out of 130 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| zswatch_app/windows/flutter/generated_plugins.cmake | Register new desktop plugins needed by added packages. |
| zswatch_app/windows/flutter/generated_plugin_registrant.cc | Register new Windows plugins (file selector, share_plus). |
| zswatch_app/test/no_speech_filter_test.dart | Adds unit tests for no-speech transcript filtering. |
| zswatch_app/pubspec.yaml | Adds sharing + image picking/cropping deps and backgrounds assets. |
| zswatch_app/pubspec.lock | Locks new dependencies and updates Flutter SDK constraint. |
| zswatch_app/macos/Flutter/GeneratedPluginRegistrant.swift | Registers file selector + share_plus on macOS. |
| zswatch_app/linux/flutter/generated_plugins.cmake | Registers file_selector plugin on Linux. |
| zswatch_app/linux/flutter/generated_plugin_registrant.cc | Registers file_selector plugin on Linux. |
| zswatch_app/lib/ui/widgets/voice_memos/memo_list_item.dart | Updates voice memo list item UI; adds archive gesture + better tags/title extraction. |
| zswatch_app/lib/ui/widgets/ai_debug_widgets.dart | Improves debug formatting for timer durations vs datetime. |
| zswatch_app/lib/ui/screens/watchface/watchface_crop_screen.dart | New interactive crop screen returning cropped bytes for background upload. |
| zswatch_app/lib/ui/screens/watchface/watchface_background_screen.dart | New background picker screen (built-ins + custom image flow + upload progress). |
| zswatch_app/lib/ui/screens/voice_memos/alarms_timers_screen.dart | New screen to manage extracted timer/alarm actions (esp. for iOS). |
| zswatch_app/lib/ui/screens/settings/ai_models_settings_screen.dart | Adds a manual “Timer & Alarm Test” section for OS creation flows. |
| zswatch_app/lib/ui/screens/firmware/firmware_update_screen.dart | Minor UI tweaks (branch ellipsis; retry logic adjustment). |
| zswatch_app/lib/ui/screens/developer/file_system_screen.dart | New developer UI to browse/download/share watch filesystem logs. |
| zswatch_app/lib/ui/screens/developer/developer_screen.dart | Adds navigation tile to the new File System screen. |
| zswatch_app/lib/ui/screens/dashboard/dashboard_screen.dart | Makes Device Information collapsible; adds Watchface shortcut tile. |
| zswatch_app/lib/ui/navigation/app_router.dart | Adds routes for file system, alarms/timers, and watchface background screens. |
| zswatch_app/lib/services/watch_service.dart | Adds connection generation tracking for safer SMP cleanup after reconnects. |
| zswatch_app/lib/services/voice_memo/voice_memo_sync_service.dart | Uses generation-aware SMP disable to avoid disabling SMP on a new connection. |
| zswatch_app/lib/services/filesystem/file_system_service.dart | New service for probing and downloading files over MCUmgr FS. |
| zswatch_app/lib/services/dfu/firmware_manager.dart | De-duplicates CI runs by commit SHA rather than run id. |
| zswatch_app/lib/services/dfu/filesystem_upload_service.dart | Cleans up stale managers on retry/failure; ensures timers stop and cleanup runs. |
| zswatch_app/lib/services/coredump/coredump_service.dart | Uses generation-aware SMP disable for reconnect safety. |
| zswatch_app/lib/services/ai/voice_note_ai_pipeline.dart | Adds no-speech filtering, queued status, crash recovery for stuck memos, and timer/alarm action support. |
| zswatch_app/lib/services/ai/litert_lm_service.dart | New Dart wrapper for Android LiteRT-LM via method/event channels. |
| zswatch_app/lib/services/ai/extracted_action_creation_service.dart | Adds duration/skipUi to drafts; supports timer/alarm creation; adjusts permission logic. |
| zswatch_app/lib/services/ai/ai_debug_info.dart | Extends debug model to include timer duration seconds. |
| zswatch_app/lib/providers/watchface_background_provider.dart | New notifier/provider orchestrating SMP + FS upload + shell apply for watchface backgrounds. |
| zswatch_app/lib/providers/voice_memo_providers.dart | Skips auto-create for “notes”; adds memo archiving support. |
| zswatch_app/lib/providers/file_system_provider.dart | New notifier/provider for filesystem probing and downloads (with SMP lifecycle). |
| zswatch_app/lib/providers/ai_providers.dart | Adds delete op + streams for alarm/timer actions and memo action-types map. |
| zswatch_app/lib/data/repositories/voice_memo_repository.dart | Adds archived flag + stuck-processing reset method. |
| zswatch_app/lib/data/repositories/extracted_action_repository.dart | Persists durationSeconds; adds delete + alarm/timer watchers and memo action-type map. |
| zswatch_app/lib/data/models/voice_memo.freezed.dart | Regenerated model to include archived. |
| zswatch_app/lib/data/models/voice_memo.dart | Adds queued status and archived flag; updates processing-state helpers. |
| zswatch_app/lib/data/models/fs_file_entry.dart | New model for filesystem entries with formatted size helper. |
| zswatch_app/lib/data/models/extracted_action.freezed.dart | Regenerated model to include durationSeconds and new action types. |
| zswatch_app/lib/data/models/extracted_action.dart | Adds timer/alarm types and durationSeconds field with mapping helpers. |
| zswatch_app/lib/data/database/tables/voice_memos_table.dart | Adds archived column to voice memos table. |
| zswatch_app/lib/data/database/tables/extracted_actions_table.dart | Adds duration_seconds column to extracted actions table. |
| zswatch_app/lib/data/database/app_database.dart | Bumps schema to v5; adds migrations + startup crash-recovery reset for stuck memos. |
| zswatch_app/lib/core/utils/watchface_image_processor.dart | New image pipeline (crop/resize/mask/convert) for watchface backgrounds. |
| zswatch_app/lib/core/utils/lvgl_image_converter.dart | New LVGL RGB565 binary converter with header generation. |
| zswatch_app/lib/app.dart | Adjusts app bootstrap imports (AI providers added). |
| zswatch_app/ios/Runner/AppDelegate.swift | Adds timer/alarm handling via Clock URL schemes for iOS. |
| zswatch_app/assets/backgrounds/bg_starfield.png | Adds built-in background preview asset. |
| zswatch_app/assets/backgrounds/bg_matrix_rain.png | Adds built-in background preview asset. |
| zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/MainActivity.kt | Adds timer/alarm creation via AlarmClock intents; wires LiteRT-LM bridge. |
| zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/LiteRtLmBridge.kt | New Kotlin bridge for LiteRT-LM model load/generate/cancel + token streaming. |
| zswatch_app/android/app/src/main/AndroidManifest.xml | Adds SET_ALARM permission + package visibility queries for timer/alarm intents. |
| zswatch_app/android/app/build.gradle.kts | Adds LiteRT-LM Android dependency. |
| packages/chrono_ai_flow/lib/src/time_expression_resolver.dart | Formatting adjustments in time resolution logic. |
| packages/chrono_ai_flow/lib/src/prompt_template.dart | Adds “no speech” rules; introduces router + timer/alarm extraction prompts. |
| packages/chrono_ai_flow/lib/src/parser.dart | Adds duration_seconds parsing + timer/alarm intent normalization and validation. |
| packages/chrono_ai_flow/lib/src/models.dart | Extends extraction model with durationSeconds. |
| packages/chrono_ai_flow/lib/src/correction_prompt_template.dart | Minor formatting refactor. |
| ai_testbench/lib/services/model_benchmark_service.dart | Adds timer/alarm cases and duration validation to benchmarks; prompt selection support. |
| ai_testbench/lib/router_benchmark_main.dart | New headless router benchmark for two-stage routing approach. |
| ai_testbench/lib/main.dart | Adds CLI entry for router benchmark mode. |
| ai_testbench/lib/benchmark_main.dart | Serializes new duration validation fields in benchmark output. |
| ai_testbench/README.md | Documents new timer/alarm benchmark flags. |
| ai_testbench/.gitignore | Ignores benchmark_results output directory. |
Comment on lines
+66
to
+70
| case OnDownloadCompleted(): | ||
| completer?.complete(callback.data); | ||
| completer = null; | ||
| case OnDownloadFailed(): | ||
| _log('Download failed: ${callback.cause}'); |
There was a problem hiding this comment.
In downloadFile, the completer is set to null after completion (OnDownloadCompleted/OnDownloadFailed/etc), but later the code force-unwraps completer!.future. If the callback fires quickly (before the await completer!.future line), this will throw a null dereference. Keep a non-null local completer (don’t set it to null), or store the Future in a local variable before nulling, and guard with isCompleted to avoid double-completes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
zswatch_app/lib/services/dfu/firmware_manager.dart (1)
239-249:⚠️ Potential issue | 🟡 MinorEmpty-SHA fallback can collapse unrelated runs.
Using
''as the fallback whenhead_shais null means the first run with a missing SHA is kept and any subsequent run with a missing SHA is silently dropped as a "duplicate". In practice GitHub Actions runs almost always includehead_sha, so this is low-risk, but if the field is ever absent you'll under-fill thenumRunswindow rather than treating each such run as distinct.Proposed fix
- .where((run) { - final sha = run['head_sha'] as String? ?? ''; - if (seenShas.contains(sha)) return false; - seenShas.add(sha); - return true; - }) + .where((run) { + final sha = run['head_sha'] as String?; + // Treat missing SHAs as unique so we don't collapse unrelated runs. + if (sha == null || sha.isEmpty) return true; + if (seenShas.contains(sha)) return false; + seenShas.add(sha); + return true; + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zswatch_app/lib/services/dfu/firmware_manager.dart` around lines 239 - 249, The current de-dup logic uses head_sha with a '' fallback which causes distinct runs missing head_sha to collapse; update the uniqueness key in the seenShas/uniqueRuns logic to use a stable fallback (for example combine head_sha with run['id'] or use run['id'].toString() when head_sha is null) so each run without head_sha is treated as distinct; locate the block that builds seenShas and uniqueRuns (variables seenShas, uniqueRuns, runsWithMain, numRuns, and the use of 'head_sha') and change the key computation to a composite/key expression that includes run['id'] (or another unique run field) instead of an empty string.
🟡 Minor comments (20)
zswatch_app/lib/ui/screens/firmware/firmware_update_screen.dart-1610-1620 (1)
1610-1620:⚠️ Potential issue | 🟡 MinorRetry prioritization when both DFU and FS failed.
The outer guard on Line 1582 enters this block when
dfuState.status == DfuStatus.failed || isFsFailed. The new inner conditionisFsFailed && dfuState.status != DfuStatus.failedintentionally routes to the filesystem retry only when DFU has not also failed; if both have failed, the firmware retry path is taken and the user has no direct way to re-trigger the FS upload from this screen until the firmware retry is handled/reset. If that’s the intended UX (firmware-first recovery), consider a brief helper note in the error text; otherwise you may want to surface both retry options.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zswatch_app/lib/ui/screens/firmware/firmware_update_screen.dart` around lines 1610 - 1620, Currently the Retry FilledButton logic (FilledButton, dfuState.status, DfuStatus.failed, isFsFailed, operationState.canStartFilesystemUpload, onStartFilesystem, operationState.canStartFirmwareUpdate, onStartFirmware) hides the filesystem retry when both DFU and FS have failed; change the UI so that when both dfuState.status == DfuStatus.failed and isFsFailed are true you render two distinct retry controls (e.g., separate buttons or a split button): one that enables/disables based on operationState.canStartFirmwareUpdate and calls onStartFirmware, and another that enables/disables based on operationState.canStartFilesystemUpload and calls onStartFilesystem; otherwise keep the current single-button behavior. Ensure accessibility/labels differentiate the two retries.zswatch_app/lib/providers/file_system_provider.dart-104-106 (1)
104-106:⚠️ Potential issue | 🟡 Minor
refreshFiles()is effectively a no-op.It just resets
isLoadingFilesto false and emptiesotherFiles. If this is a placeholder for a follow-up, aTODOcomment referencing the intended behavior would help — otherwise consumers may call it expecting a real probe.Happy to stub the corresponding
probeFiles(recordingsDir, …)call if you want this wired up in the same PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zswatch_app/lib/providers/file_system_provider.dart` around lines 104 - 106, refreshFiles() currently just clears state by setting isLoadingFiles to false and otherFiles to an empty list, making it effectively a no-op; either implement the intended file-probing logic or clearly mark it as a placeholder. Replace the body of refreshFiles() to call the actual probe method (e.g., probeFiles(recordingsDir, ...)) and update state before/after probing (use state.copyWith to set isLoadingFiles true/false and populate otherFiles with probe results), or add a TODO comment inside refreshFiles() referencing probeFiles(recordingsDir, ...) so callers know it’s a stub.zswatch_app/lib/providers/file_system_provider.dart-151-157 (1)
151-157:⚠️ Potential issue | 🟡 Minor
downloadFilerethrows but leaves no error in state — inconsistent withrefreshLogs.
refreshLogsstoresloadErroron the notifier so the UI can render it from the state stream, but the download path clears the UI state and rethrows. Any caller thatawaitsdownloadFile()without a try/catch will bubble an unhandled exception; callers that do catch must render errors via a separate channel. Consider storing the failure on state (e.g., a newdownloadError, or reusingloadError) for consistency with the rest of the notifier.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zswatch_app/lib/providers/file_system_provider.dart` around lines 151 - 157, The catch block in downloadFile currently clears downloadingPath and rethrows without recording the error in state, causing inconsistency with refreshLogs; update the error handling in the downloadFile catch to set an error field on the notifier's state (either add a new downloadError to the state model or reuse loadError) via state.copyWith(downloadingPath: null, downloadProgress: 0.0, downloadError: e) (or loadError: e) before rethrowing so the UI can observe the failure; ensure you update the state class and copyWith signature (and any consumers) to include the chosen error field.zswatch_app/lib/services/filesystem/file_system_service.dart-27-34 (1)
27-34:⚠️ Potential issue | 🟡 Minor
probeFilessilently swallows everystatus()error as "file not found".If SMP/BLE drops mid-probe, every remaining iteration hits
catch (_)and the function returns a partial list with no indication that probing was truncated. The caller cannot distinguish "log slot empty" from "link failed". Consider at least logging distinct errors, or breaking out when repeated non-404-style errors occur so the caller can surface a real failure.🛠️ Suggested refinement
try { final sizeBytes = await fsManager.status(path); entries.add(FsFileEntry(path: path, name: name, sizeBytes: sizeBytes)); _log('Found file: $path ($sizeBytes bytes)'); - } catch (_) { - // File not found — skip + } catch (e) { + // MCUmgr returns a not-found error for absent slots — expected and skipped. + // Log at debug so transport-level failures aren't invisible. + _log('status($path) failed: $e'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zswatch_app/lib/services/filesystem/file_system_service.dart` around lines 27 - 34, The probeFiles loop currently swallows all errors in the catch (_) after calling fsManager.status(path) making link failures indistinguishable from missing files; update the catch to inspect the thrown error from fsManager.status (in probeFiles) and: if it clearly indicates "not found" (404/NotFound exception), continue as before; otherwise log the error with context (path, iteration) using the existing logger and either break/stop probing or rethrow the error so the caller can surface a truncated probe; avoid using a bare catch (_) and reference the fsManager.status call and probeFiles function to locate where to implement the conditional handling and logging.packages/chrono_ai_flow/lib/src/prompt_template.dart-264-264 (1)
264-264:⚠️ Potential issue | 🟡 MinorWrapping
$promptPlaceholderTranscriptin quotes can break the new templates.Both
routerTemplate(Line 264) andtimerAlarmTemplate(Line 314) embed the transcript asMemo: "$promptPlaceholderTranscript". If the transcript contains a literal"character (which transcribers occasionally produce), the example structure is broken and may confuse the model — e.g. the apparent string ends mid-transcript. Note thatdefaultTemplate/compactTemplatedeliberately use an un-quoted form (Line 135, 221).Suggest matching the existing convention to keep the templates robust:
♻️ Proposed fix
-Memo: "$promptPlaceholderTranscript" +Voice memo: + +$promptPlaceholderTranscriptOr, at minimum, escape
"in the transcript before substitution inrender(...).Also applies to: 314-314
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chrono_ai_flow/lib/src/prompt_template.dart` at line 264, routerTemplate and timerAlarmTemplate embed the transcript as Memo: "$promptPlaceholderTranscript" which breaks when the transcript contains a " character; update those templates (routerTemplate and timerAlarmTemplate) to match defaultTemplate/compactTemplate by removing the surrounding quotes (use Memo: $promptPlaceholderTranscript) OR alternatively modify the render(...) path that substitutes promptPlaceholderTranscript to escape embedded double quotes before interpolation; make the change for both occurrences to ensure template robustness.packages/chrono_ai_flow/lib/src/parser.dart-79-81 (1)
79-81:⚠️ Potential issue | 🟡 Minor
duration_secondsas a stringified float will be silently dropped.
int.tryParse('5400.0')returnsnull, so a model that emits"duration_seconds": "5400.0"will fail the timer-completeness check at lines 85-87 and the whole extraction will be discarded. Given the prompt explicitly tells the model to output integers, this is unlikely but cheap to harden against.♻️ Proposed fix
} else if (rawDuration is String) { - durationSeconds = int.tryParse(rawDuration); + durationSeconds = + int.tryParse(rawDuration) ?? double.tryParse(rawDuration)?.round(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chrono_ai_flow/lib/src/parser.dart` around lines 79 - 81, The parser currently sets durationSeconds = int.tryParse(rawDuration) when rawDuration is a String (in the parsing logic around rawDuration and durationSeconds), which drops stringified floats like "5400.0"; update the logic to, if int.tryParse returns null, attempt double.tryParse(rawDuration) and, if that succeeds and is finite, convert to an integer (e.g., .toInt() or Math.round as appropriate for your semantics) and assign that to durationSeconds so stringified floats are accepted; keep the original int.parse path for pure integer strings and ensure you still handle null safely so the later timer-completeness check behaves correctly.zswatch_app/lib/ui/screens/settings/ai_models_settings_screen.dart-1679-1681 (1)
1679-1681:⚠️ Potential issue | 🟡 MinorStale doc comment placement.
The doc
/// Compact tile shown in the benchmark section after a completed run.was previously attached to_LastResultTile, but it now sits directly above_TimerAlarmTestSection(which is unrelated to the benchmark "last result" tile). Either move it back above_LastResultTile(Line 1770) or delete it.♻️ Proposed fix
-/// Compact tile shown in the benchmark section after a completed run. class _TimerAlarmTestSection extends ConsumerStatefulWidget {…and re-attach above
_LastResultTileif still desired.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zswatch_app/lib/ui/screens/settings/ai_models_settings_screen.dart` around lines 1679 - 1681, The doc comment "Compact tile shown in the benchmark section after a completed run." is placed above _TimerAlarmTestSection but belongs to _LastResultTile; move that triple-slash doc comment from above the _TimerAlarmTestSection class declaration and re-attach it directly above the _LastResultTile class declaration (or remove it entirely if no longer needed) so comments match the correct widget (_TimerAlarmTestSection remains without the stale benchmark comment and _LastResultTile gets the intended documentation).zswatch_app/lib/services/dfu/filesystem_upload_service.dart-99-99 (1)
99-99:⚠️ Potential issue | 🟡 MinorUnawaited
_cleanup()inonErrorcallback.
_cleanup()returns aFuture<void>but is fired-and-forgotten here (and similarly at lines 143/154/160 inside_handleUploadCallback). If a newstartUpload(...)is triggered immediately after a failure, theawait _cleanup()at the top ofstartUploadmay complete before the in-flight cleanup from this error path has actually killed_fsManager/cancelled the subscription, leading to a race. It also violates the repo'sunawaited_futureslint rule.♻️ Proposed fix
_uploadSubscription = _fsManager!.uploadCallbacks.listen( (callback) => _handleUploadCallback(callback, startTime), - onError: (Object error) { + onError: (Object error) async { _log('Upload error: $error'); _stopSpeedTimer(); _updateState( FilesystemUploadState.failed( error.toString(), startedAt: startTime, ), ); - _cleanup(); + await _cleanup(); }, );Note: If you don't want to make
onErrorasync (Stream errors will still be delivered, just not back-pressured), wrap withunawaited(_cleanup())and addimport 'dart:async'(already present). As per coding guidelines: "Enforce rules: ...unawaited_futures...".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zswatch_app/lib/services/dfu/filesystem_upload_service.dart` at line 99, The onError callback calls _cleanup() without awaiting which violates the unawaited_futures lint and can race with startUpload's awaited cleanup; fix by either making the onError / the enclosing callback async and awaiting await _cleanup(), or explicitly mark the fire-and-forget call as unawaited(_cleanup()) (ensure import 'dart:async' is present), and apply the same change for other occurrences noted in _handleUploadCallback (lines where _cleanup() is invoked on error) so the cleanup semantics are deterministic relative to startUpload and lint-compliant.zswatch_app/lib/ui/screens/watchface/watchface_background_screen.dart-136-136 (1)
136-136:⚠️ Potential issue | 🟡 MinorCopy mismatch with the crop screen.
The hint says "240×240 circle", but
WatchfaceCropScreenonly enforcesaspectRatio: 1with a circle UI — the final 240×240 sizing must happen in the upload pipeline. If that pipeline ever changes the target resolution, this string will silently lie. Consider sourcing the dimension from the same constant used by the uploader (or just say "1:1 circle").🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zswatch_app/lib/ui/screens/watchface/watchface_background_screen.dart` at line 136, The hint text is hardcoded to "240×240 circle" but WatchfaceCropScreen only enforces a 1:1 (square) aspect with a circular UI and the actual 240px sizing happens later in the uploader; update the hint to avoid mismatching the uploader by either referencing the uploader's size constant (use the same constant used by the upload pipeline instead of hardcoding) or change the copy to a generic, accurate string like "1:1 circle" so it cannot drift; locate the text near WatchfaceCropScreen and replace the literal "240×240 circle" with a reference to the shared size constant or the new "1:1 circle" copy.specs/ai-tasks/alarms-and-timers.md-114-120 (1)
114-120:⚠️ Potential issue | 🟡 MinorSpec drifts from what this PR actually ships.
Per the commit log on this PR, the implementation went with Option B (two-stage router) and uses Android
AlarmClock.ACTION_SET_*intents withEXTRA_SKIP_UI+ iOS Clock URL scheme — not the watch-side confirm-popup loop with confirm-before-set described in §5.1, §5.3, and the Phase 3 plan. A few touch-ups would prevent the spec from misleading future maintainers:
- §3 Recommendation (line 120): currently says "Start with Option A". Update to reflect that Option B was adopted, ideally with a one-liner on why.
- §5.1 / §5.3: the watch-confirm round-trip is V2 / unimplemented. Either move it under a clearly labeled "Future" subsection or note that V1 fires the platform intent directly from the phone.
- §7 phases: Phase 2 / Phase 3 ordering implies the confirm popup ships in the same release; mark Phase 3 as deferred so the spec matches reality.
Also worth marking the doc as "Status: Implemented (V1, partial — iOS not fully tested)" rather than "Draft" if this is the source of truth going forward.
Also applies to: 283-322, 497-545
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/ai-tasks/alarms-and-timers.md` around lines 114 - 120, Update the spec to reflect the shipped two-stage router: change the Recommendation in §3 to state Option B (two-stage routing) was adopted with a one-line rationale; mark §5.1 and §5.3 to indicate the watch-confirm round-trip is V2/unimplemented and either move that content into a clearly labeled "Future" subsection or add a note that V1 fires platform intents directly from the phone using Android AlarmClock.EXTRA_SKIP_UI and the iOS Clock URL scheme; update the §7 phases to mark Phase 3 (confirm popup) as deferred and ensure Phase 2/Phase 3 ordering reflects that; and change the document Status to "Implemented (V1, partial — iOS not fully tested)".zswatch_app/lib/ui/screens/watchface/watchface_crop_screen.dart-36-46 (1)
36-46:⚠️ Potential issue | 🟡 MinorMissing
mountedcheck after async I/O in_loadImage.
File.readAsBytes()is awaited and thensetStateis called unconditionally. If the user pops the route while the file read is in flight, this will throwsetState() called after dispose().🛡️ Proposed fix
Future<void> _loadImage() async { try { final bytes = await File(widget.imagePath).readAsBytes(); + if (!mounted) return; // Try using the raw bytes first (works for JPEG/PNG). // If crop_your_image can't parse them, the re-encode path below handles it. setState(() => _imageBytes = bytes); } catch (e) { + if (!mounted) return; setState(() => _error = 'Could not load image file'); debugPrint('[WatchfaceCropScreen] Image load error: $e'); } }Also: the doc comment on lines 33–35 claims a re-encode fallback via Flutter's native codec exists, but there is no such code path here — only the raw
readAsBytes()route. Either implement the fallback or trim the comment so it doesn't mislead future readers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zswatch_app/lib/ui/screens/watchface/watchface_crop_screen.dart` around lines 36 - 46, The _loadImage() method awaits File(widget.imagePath).readAsBytes() and calls setState unconditionally, which can throw if the widget is disposed; add a mounted check before any setState calls (i.e., after the await, verify if (!mounted) return) and only then update _imageBytes or _error via setState; also either implement the promised re-encode fallback path (use Flutter's image codec to decode/encode into PNG/JPEG and assign bytes to _imageBytes) or remove/clarify the misleading comment about a re-encode fallback so the comment matches the actual behavior in _loadImage.specs/ai-tasks/alarms-and-timers.md-548-555 (1)
548-555:⚠️ Potential issue | 🟡 MinorRemove
SCHEDULE_EXACT_ALARMpermission row — not required forAlarmClock.ACTION_SET_ALARM/ACTION_SET_TIMERintents.The table incorrectly lists
SCHEDULE_EXACT_ALARMas required for system timer/alarm intents on Android 12+. This permission only gatesAlarmManager.setExact*()andsetAlarmClock(). The implementation usesAlarmClock.ACTION_SET_TIMERandACTION_SET_ALARMintents (as seen inMainActivity.kt), which only requirecom.android.alarm.permission.SET_ALARMin the manifest—already declared inAndroidManifest.xml. Drop the row to prevent misleading future developers about permission requirements this PR doesn't actually use.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/ai-tasks/alarms-and-timers.md` around lines 548 - 555, The permissions table under "## 8. Permissions Required" incorrectly lists SCHEDULE_EXACT_ALARM; remove the table row that references SCHEDULE_EXACT_ALARM so the table only documents the actual intents used (AlarmClock.ACTION_SET_TIMER and AlarmClock.ACTION_SET_ALARM) and the manifest-declared com.android.alarm.permission.SET_ALARM. Locate the table (the header "## 8. Permissions Required") and delete the row containing `SCHEDULE_EXACT_ALARM`, leaving the existing `ACTION_SET_ALARM`/`ACTION_SET_TIMER` and manifest permission references intact; verify MainActivity.kt uses the AlarmClock intents and AndroidManifest.xml retains com.android.alarm.permission.SET_ALARM.zswatch_app/lib/ui/widgets/voice_memos/memo_list_item.dart-60-69 (1)
60-69:⚠️ Potential issue | 🟡 MinorSilent failure path in archive swipe.
If
setArchived(...)throws (e.g., the DB write fails), the swipe gesture still returnsfalse(no error visible to the user) and the card remains in the old state. The equivalent delete path usesonDismissedwhich has no such failure surface, but archive is user-triggered and should report failures like the other mutations do. Wrap the call in a try/catch and surface viaScaffoldMessenger.🩹 Proposed fix
- confirmDismiss: (direction) async { - if (direction == DismissDirection.endToStart) { - return confirmDeleteMemo(context, memo); - } - // Archive/unarchive — no confirmation needed - await ref - .read(voiceMemoActionsProvider.notifier) - .setArchived(memo.filename, archived: !memo.archived); - return false; // Don't remove the widget, the stream will update - }, + confirmDismiss: (direction) async { + if (direction == DismissDirection.endToStart) { + return confirmDeleteMemo(context, memo); + } + try { + await ref + .read(voiceMemoActionsProvider.notifier) + .setArchived(memo.filename, archived: !memo.archived); + } catch (e) { + if (context.mounted) { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar(content: Text('Failed to archive: $e')), + ); + } + } + return false; // state-driven update, stream will rebuild + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zswatch_app/lib/ui/widgets/voice_memos/memo_list_item.dart` around lines 60 - 69, The archive swipe currently calls setArchived(...) inside the confirmDismiss closure and ignores errors, causing silent failures; wrap the await ref.read(voiceMemoActionsProvider.notifier).setArchived(memo.filename, archived: !memo.archived) call in a try/catch, and on catch call ScaffoldMessenger.of(context).showSnackBar(...) with a user-friendly message that includes error.toString(), then return false so the Dismissible remains visible; keep the successful path unchanged (await setArchived then return false) and optionally log the error for diagnostics.zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/MainActivity.kt-680-698 (1)
680-698:⚠️ Potential issue | 🟡 MinorValidate timer duration before launching intent.
If
durationSecondsis null or 0,AlarmClock.ACTION_SET_TIMERwithEXTRA_LENGTH=0andEXTRA_SKIP_UI=trueproduces inconsistent OEM behavior (silent no-op on some stock clock apps, UI shown on others). Given the LLM can emit a timer intent without a parsed duration, surface this as an explicit error so the UI can retry/prompt instead of appearing to succeed while nothing happens.AlarmClockrequiresEXTRA_LENGTHto be in[1, 86400].🛡️ Proposed fix
- private fun createTimerViaIntent(durationSeconds: Int, label: String, skipUi: Boolean = true): Map<String, Any?> { + private fun createTimerViaIntent(durationSeconds: Int, label: String, skipUi: Boolean = true): Map<String, Any?> { + require(durationSeconds in 1..86400) { + "Timer duration must be between 1 and 86400 seconds (got $durationSeconds)." + } val intent = android.content.Intent(android.provider.AlarmClock.ACTION_SET_TIMER).apply {Alternatively, reject early in
handleCreateActionso the error code/message is a more descriptiveINVALID_ARGUMENTrather than a genericCREATE_ACTION_FAILED.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/MainActivity.kt` around lines 680 - 698, The createTimerViaIntent function must validate durationSeconds before building/starting the AlarmClock intent: ensure durationSeconds is non-null and within 1..86400 (inclusive); if it falls outside this range, do not launch the intent and instead throw a clear IllegalArgumentException (or return/propagate an INVALID_ARGUMENT-style error) with a message like "durationSeconds must be between 1 and 86400"; update callers such as handleCreateAction to catch/translate that specific error so the client receives a descriptive invalid-argument response rather than a generic CREATE_ACTION_FAILED.zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/LiteRtLmBridge.kt-161-270 (1)
161-270:⚠️ Potential issue | 🟡 Minor
currentJobonly tracks the most recent coroutine, socancel()can miss queued generations.Each call to
generate()overwritescurrentJob. If a secondgenerate()is invoked while the first holdsgenerateMutex, the second coroutine is whatcurrentJobpoints to — callingcancel()won't cancel the in-flight inference holding the mutex (nor the underlyingConversation.cancelProcess()targeting the active conversation, since a newcreateConversationwill run when the second coroutine acquires the lock). Consider tracking the currently-executing job (set under the mutex, not at launch time) or a list of live jobs, socancel()reliably stops what the user sees.🩹 Proposed fix
currentJob = scope.launch { generateMutex.withLock { + currentJob = coroutineContext[Job] try { val conv = createConversation(temperature, topK, topP)(And remove the earlier
currentJob = scope.launch { ... }assignment.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/LiteRtLmBridge.kt` around lines 161 - 270, The issue is that currentJob is set to the outer coroutine launched in generate(), so cancel() can miss the coroutine actually executing the inference inside generateMutex; move responsibility for tracking the active inference job inside the locked section (or maintain a synchronized list of live jobs) so cancel() can reliably cancel the running inference and call conversation?.cancelProcess(). Specifically, remove or stop setting currentJob = scope.launch at the top of generate(), and instead assign currentJob (or add to liveJobs) while inside generateMutex.withLock after createConversation(...) is called (and clear it before exiting the lock), and update cancel() to iterate liveJobs (or cancel the currentJob tracked under the mutex) and call conversation?.cancelProcess() to ensure the in-flight inference is stopped.zswatch_app/lib/services/ai/llm_service.dart-375-379 (1)
375-379:⚠️ Potential issue | 🟡 Minor
_isCurrentModelLiteRtLmis unused and ignores imported models.Two issues:
- The getter only checks
catalogModelsand won't detect imported.litertlmmodels (whichimportModel()andavailableModels()support). Compare with_resolveModelById(), which correctly scans both catalog and imported models.- No call sites found — remove the getter if unused, or fix it to check imported models alongside
catalogModels.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zswatch_app/lib/services/ai/llm_service.dart` around lines 375 - 379, The _isCurrentModelLiteRtLm getter currently only checks catalogModels and is unused; either remove it or update it to mirror _resolveModelById() by searching both catalog and imported models (the same sources used by importModel() and availableModels()) and return the target model's isLiteRtLm flag; if you keep it, replace the catalog-only lookup with a call to _resolveModelById(_selectedModelId) (or equivalent combined search) so imported .litertlm models are detected.zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/MainActivity.kt-688-716 (1)
688-716:⚠️ Potential issue | 🟡 MinorChain the caught
ActivityNotFoundExceptionas the cause.detekt flagged both
catchblocks as swallowing the original exception. Passeas the cause so logs retain the underlying stack trace when something other than "no handler" triggers the throw (Android sometimes raisesActivityNotFoundExceptionfor permission/component-disabled cases).🩹 Proposed fix
try { startActivity(intent) } catch (e: android.content.ActivityNotFoundException) { - throw IllegalStateException("No app found that can handle timers. Please install a clock app.") + throw IllegalStateException("No app found that can handle timers. Please install a clock app.", e) }Apply the same change in
createAlarmViaIntentat line 715.As per the static analysis hint:
The caught exception is swallowed. The original exception could be lost.(detekt.exceptions.SwallowedException).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/MainActivity.kt` around lines 688 - 716, The catch block in createAlarmViaIntent is currently throwing a new IllegalStateException without chaining the caught android.content.ActivityNotFoundException, which loses the original stack trace; modify the catch to throw IllegalStateException("No app found that can handle alarms. Please install a clock app.", e) (i.e., pass the caught exception `e` as the cause) so the original exception is preserved (mirror the same change already made in the timer-handling catch).ai_testbench/lib/services/model_benchmark_service.dart-1140-1142 (1)
1140-1142:⚠️ Potential issue | 🟡 Minor
_containsEmptyJsonArrayis too permissive.After stripping whitespace, any output containing the substring
[]anywhere is treated as valid — e.g.{"items":[],"note":"hi"},"text [] more", or a partial/broken output with a stray pair of brackets will all pass forexpectedCount == 0cases. This can mask genuine parser failures on no-speech cases.Anchor the check to a standalone empty array (optionally wrapped in code fences/sanitized output):
♻️ Proposed fix
- /// Check if the raw output contains an empty JSON array `[]`. - bool _containsEmptyJsonArray(String raw) { - final cleaned = raw.replaceAll(RegExp(r'\s+'), ''); - return cleaned.contains('[]'); - } + /// Check if the raw output is effectively a top-level empty JSON array `[]`. + bool _containsEmptyJsonArray(String raw) { + final cleaned = _parser.sanitizeModelOutput(raw).trim(); + return cleaned == '[]'; + }Also applies to: 1343-1347
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ai_testbench/lib/services/model_benchmark_service.dart` around lines 1140 - 1142, _current logic in _containsEmptyJsonArray is too permissive because it treats any occurrence of "[]" as a valid standalone empty array; update _containsEmptyJsonArray to only return true when the entire sanitized output is a standalone empty JSON array optionally wrapped in code fences or inline code (allow surrounding whitespace and optional surrounding triple-backtick or single-backtick fences), by using a anchored regex or strict parsing rather than a plain substring check; apply the same stricter check to the other occurrence referenced around lines 1343-1347 (replace the substring check with this anchored fenced-array check).ai_testbench/lib/router_benchmark_main.dart-130-132 (1)
130-132:⚠️ Potential issue | 🟡 MinorLatent division-by-zero in averages and overhead percentage.
Three integer
~/divisions and one floating-point/rely on non-empty lists:
- Line 131:
routerTimes.length(cases list)- Line 200:
totalTimes.length(timer_alarm filtered cases)- Line 237:
singlePassTimes.length(voiceMemoCases.take(5))- Line 242:
avgSingleMsin overhead ratioWhile hardcoded today, any future filter/case change (or
take(5)returning fewer than expected) will crash the benchmark at the aggregation step rather than reporting gracefully. A small guard keeps the tool resilient:♻️ Proposed fix
- final avgRouterMs = - routerTimes.fold<int>(0, (s, d) => s + d.inMilliseconds) ~/ - routerTimes.length; + final avgRouterMs = routerTimes.isEmpty + ? 0 + : routerTimes.fold<int>(0, (s, d) => s + d.inMilliseconds) ~/ + routerTimes.length; @@ - stdout.writeln( - '[RouterBench] Overhead: ${avgTotalMs - avgSingleMs}ms (${((avgTotalMs - avgSingleMs) / avgSingleMs * 100).toStringAsFixed(0)}%)'); + final overheadPct = avgSingleMs == 0 + ? 'n/a' + : '${((avgTotalMs - avgSingleMs) / avgSingleMs * 100).toStringAsFixed(0)}%'; + stdout.writeln( + '[RouterBench] Overhead: ${avgTotalMs - avgSingleMs}ms ($overheadPct)');(Apply the same
isEmpty ? 0 : ...pattern for the other two averages.)Also applies to: 199-201, 236-242
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ai_testbench/lib/router_benchmark_main.dart` around lines 130 - 132, The average and ratio calculations can divide by zero when lists are empty (e.g., routerTimes, totalTimes, singlePassTimes) — update each computation (notably avgRouterMs, the average computed for totalTimes around the timer_alarm filtered cases, the average over singlePassTimes for voiceMemoCases.take(5), and avgSingleMs used in the overhead ratio) to guard against empty lists by returning 0 (or a safe default) when the collection isEmpty before performing the integer ~/ or floating-point / division; use the pattern "isEmpty ? 0 : (existing calculation)" to make the benchmark resilient.ai_testbench/lib/services/model_benchmark_service.dart-910-916 (1)
910-916:⚠️ Potential issue | 🟡 MinorAlarm test expectations look off-by-one-day for past times.
referenceTimeis2026-03-11 10:15. These two cases expect the alarm to resolve to the same day at an earlier time:
- Line 915:
en_alarm_morning— "Set an alarm for 7:30 AM" → expects2026-03-11 07:30(3 hours before the reference).- Line 952:
sv_alarm_labeled— "Alarm klockan halv 8, dags att gå" → expects2026-03-11 07:30.Sibling cases in the same file roll past times forward — e.g.
en_alarm_no_action(line 974) expects2026-03-12 07:00for bare"7 AM", andsv_alarm_simple(line 944) expects2026-03-12 07:00. The expected behavior for an alarm set "today at a past time" is almost universally "next occurrence", so a well-behaved model will fail these benchmarks.Consider changing the expected dates to
2026-03-12(next day) to match the other alarm cases and real-world alarm semantics.Also applies to: 946-953
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ai_testbench/lib/services/model_benchmark_service.dart` around lines 910 - 916, The alarm benchmark expectations for past times are off-by-one-day: update the BenchmarkCase.single entries named en_alarm_morning and sv_alarm_labeled (and the nearby cases in the 946-953 range) so their expectedDateTime advances from DateTime(2026, 3, 11, ...) to DateTime(2026, 3, 12, ...) to match the "next occurrence" alarm semantics used by other cases (e.g., en_alarm_no_action and sv_alarm_simple) and keep consistency across BenchmarkCase.single definitions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f7d1b5bc-684b-439f-9325-f078ba10072e
⛔ Files ignored due to path filters (65)
zswatch_app/assets/backgrounds/bg_aurora.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_aurora.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_brushed_titanium.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_brushed_titanium.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_copper_mesh.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_copper_mesh.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_crt_neon_grid.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_crt_neon_grid.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_deep_space.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_deep_space.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_ember.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_ember.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_flow_field_silk.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_flow_field_silk.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_flow_ink_parchment.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_flow_ink_parchment.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_forest_canopy.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_forest_canopy.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_fume_guilloche.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_fume_guilloche.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_guilloche_dial.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_guilloche_dial.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_halftone_poster.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_halftone_poster.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_ink_wash.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_ink_wash.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_lava_flow.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_lava_flow.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_low_poly_shards.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_low_poly_shards.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_matrix_rain.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_matrix_rain.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_midnight_frost.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_midnight_frost.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_moire_interference.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_moire_interference.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_mosaic_gem.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_mosaic_gem.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_ocean.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_ocean.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_paper_cut.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_paper_cut.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_pixel_neon.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_pixel_neon.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_reaction_coral.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_reaction_coral.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_retro_sunburst.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_retro_sunburst.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_sdf_rings.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_sdf_rings.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_solar_flare.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_solar_flare.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_starfield.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_starfield.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_topo_metal_arc.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_topo_metal_arc.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_topographic.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_topographic.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_voronoi_glass.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_voronoi_glass.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_vortex.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_vortex.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_warp_marble.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_warp_marble.pngis excluded by!**/*.pngzswatch_app/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (65)
ai_testbench/.gitignoreai_testbench/README.mdai_testbench/lib/benchmark_main.dartai_testbench/lib/main.dartai_testbench/lib/router_benchmark_main.dartai_testbench/lib/services/model_benchmark_service.dartpackages/chrono_ai_flow/lib/src/correction_prompt_template.dartpackages/chrono_ai_flow/lib/src/models.dartpackages/chrono_ai_flow/lib/src/parser.dartpackages/chrono_ai_flow/lib/src/prompt_template.dartpackages/chrono_ai_flow/lib/src/time_expression_resolver.dartspecs/ai-tasks/alarms-and-timers.mdzswatch_app/android/app/build.gradle.ktszswatch_app/android/app/src/main/AndroidManifest.xmlzswatch_app/android/app/src/main/kotlin/dev/zswatch/app/LiteRtLmBridge.ktzswatch_app/android/app/src/main/kotlin/dev/zswatch/app/MainActivity.ktzswatch_app/ios/Runner/AppDelegate.swiftzswatch_app/lib/app.dartzswatch_app/lib/core/utils/lvgl_image_converter.dartzswatch_app/lib/core/utils/watchface_image_processor.dartzswatch_app/lib/data/database/app_database.dartzswatch_app/lib/data/database/app_database.g.dartzswatch_app/lib/data/database/tables/extracted_actions_table.dartzswatch_app/lib/data/database/tables/voice_memos_table.dartzswatch_app/lib/data/models/extracted_action.dartzswatch_app/lib/data/models/extracted_action.freezed.dartzswatch_app/lib/data/models/fs_file_entry.dartzswatch_app/lib/data/models/voice_memo.dartzswatch_app/lib/data/models/voice_memo.freezed.dartzswatch_app/lib/data/repositories/extracted_action_repository.dartzswatch_app/lib/data/repositories/voice_memo_repository.dartzswatch_app/lib/providers/ai_providers.dartzswatch_app/lib/providers/file_system_provider.dartzswatch_app/lib/providers/voice_memo_providers.dartzswatch_app/lib/providers/watchface_background_provider.dartzswatch_app/lib/services/ai/ai_debug_info.dartzswatch_app/lib/services/ai/extracted_action_creation_service.dartzswatch_app/lib/services/ai/litert_lm_service.dartzswatch_app/lib/services/ai/llm_service.dartzswatch_app/lib/services/ai/voice_note_ai_pipeline.dartzswatch_app/lib/services/coredump/coredump_service.dartzswatch_app/lib/services/dfu/filesystem_upload_service.dartzswatch_app/lib/services/dfu/firmware_manager.dartzswatch_app/lib/services/filesystem/file_system_service.dartzswatch_app/lib/services/voice_memo/voice_memo_sync_service.dartzswatch_app/lib/services/watch_service.dartzswatch_app/lib/ui/navigation/app_router.dartzswatch_app/lib/ui/screens/dashboard/dashboard_screen.dartzswatch_app/lib/ui/screens/developer/developer_screen.dartzswatch_app/lib/ui/screens/developer/file_system_screen.dartzswatch_app/lib/ui/screens/firmware/firmware_update_screen.dartzswatch_app/lib/ui/screens/settings/ai_models_settings_screen.dartzswatch_app/lib/ui/screens/voice_memos/alarms_timers_screen.dartzswatch_app/lib/ui/screens/voice_memos/voice_memos_screen.dartzswatch_app/lib/ui/screens/watchface/watchface_background_screen.dartzswatch_app/lib/ui/screens/watchface/watchface_crop_screen.dartzswatch_app/lib/ui/widgets/ai_debug_widgets.dartzswatch_app/lib/ui/widgets/voice_memos/memo_list_item.dartzswatch_app/linux/flutter/generated_plugin_registrant.cczswatch_app/linux/flutter/generated_plugins.cmakezswatch_app/macos/Flutter/GeneratedPluginRegistrant.swiftzswatch_app/pubspec.yamlzswatch_app/test/no_speech_filter_test.dartzswatch_app/windows/flutter/generated_plugin_registrant.cczswatch_app/windows/flutter/generated_plugins.cmake
Comment on lines
+54
to
+67
| TextButton( | ||
| onPressed: _isCropping | ||
| ? null | ||
| : () { | ||
| setState(() => _isCropping = true); | ||
| _cropController.crop(); | ||
| }, | ||
| child: _isCropping | ||
| ? const SizedBox( | ||
| width: 20, | ||
| height: 20, | ||
| child: CircularProgressIndicator(strokeWidth: 2), | ||
| ) | ||
| : const Text('Done'), |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
crop_your_image 1.1.0 onCropped CropResult error handling onStatusChanged
💡 Result:
For crop_your_image 1.1.0, you handle cropping errors via the onCropped callback (not via onStatusChanged). 1) onCropped is where you get success vs error - In the newer docs for this package, onCropped is documented as void Function(CropResult) and the result can be either success or error. - The recommended pattern is to switch on the CropResult: onCropped: (result) { switch (result) { case CropResult.success(:final croppedImage): // use croppedImage case CropResult.error(:final error): // handle error } } 2) onStatusChanged is for UI state, not for error payloads - The Crop widget also exposes onStatusChanged: (status) { ... } as a callback when the CropStatus changes. - CropStatus has values: loading, ready, cropping, and nothing. So, implement: - onStatusChanged to react to lifecycle/progress (e.g., disable buttons while cropping/loading). - onCropped to handle the actual error object coming from the crop operation. If you’re seeing “errors” in 1.1.0, the package’s intended mechanism is to surface them through onCropped’s CropResult.error (and you should branch there).
Citations:
- 1: https://pub.dev/documentation/crop_your_image/1.1.0/
- 2: https://pub.dev/documentation/crop_your_image/latest/index.html
- 3: https://pub.dev/documentation/crop_your_image/latest/crop_your_image/CropStatus.html
- 4: https://pub.dev/documentation/crop_your_image/latest
🏁 Script executed:
# Find pubspec files and check crop_your_image version
find zswatch_app -maxdepth 2 -name "pubspec.*" | head -5Repository: ZSWatch/ZSWatch-App
Length of output: 112
🏁 Script executed:
# Read the watchface_crop_screen.dart file to see the actual implementation
cat -n zswatch_app/lib/ui/screens/watchface/watchface_crop_screen.dartRepository: ZSWatch/ZSWatch-App
Length of output: 4349
🏁 Script executed:
# Check the class definition and onCropped callback
sed -n '1,100p' zswatch_app/lib/ui/screens/watchface/watchface_crop_screen.dartRepository: ZSWatch/ZSWatch-App
Length of output: 3515
🏁 Script executed:
# Check crop_your_image version in pubspec.yaml
grep -A 2 "crop_your_image" zswatch_app/pubspec.yamlRepository: ZSWatch/ZSWatch-App
Length of output: 113
🏁 Script executed:
# Also check pubspec.lock for the resolved version
grep -A 5 "crop_your_image:" zswatch_app/pubspec.lock | head -20Repository: ZSWatch/ZSWatch-App
Length of output: 266
Use ConsumerStatefulWidget and reset _isCropping in the onCropped callback to prevent the button from latching.
This screen is in ui/screens/ and must use ConsumerStatefulWidget per project guidelines (currently StatefulWidget).
The _isCropping flag is set to true at line 58 but never reset. If cropping succeeds, fails, or the user navigates away before onCropped runs, the button remains disabled with a spinner forever.
Additionally, crop_your_image 1.1.0+ requires handling both success and error paths via the onCropped callback's CropResult type. The current code does not handle crop errors at all.
🛡️ Proposed fix
onCropped: (result) {
+ if (!mounted) return;
+ switch (result) {
+ case CropResult.success(:final croppedImage):
+ Navigator.of(context).pop(croppedImage);
+ case CropResult.error(:final error):
+ setState(() => _isCropping = false);
+ ScaffoldMessenger.of(context).showSnackBar(
+ SnackBar(content: Text('Crop failed: $error')),
+ );
+ }
},Also refactor the class to extend ConsumerStatefulWidget instead of StatefulWidget to align with project architecture.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@zswatch_app/lib/ui/screens/watchface/watchface_crop_screen.dart` around lines
54 - 67, The widget currently uses StatefulWidget and sets _isCropping=true when
calling _cropController.crop() but never resets it and does not handle
CropResult; change the widget class to extend ConsumerStatefulWidget (and its
State to ConsumerState) per project conventions, and implement the onCropped
callback for _cropController to handle both success and error branches of
CropResult by resetting _isCropping = false inside setState(), processing the
cropped result on success, and handling/logging error on failure; also ensure
any early exits (e.g., navigation) clear/reset _isCropping so the Done button
never stays disabled.
- Add filesystem browser (file_system_service, file_system_screen) - Add watchface background upload with crop/preview UI - Add bundled background assets (30+ designs as .bin + .png) - Add LVGL image converter and watchface image processor utils - Fix filesystem upload cleanup on error/retry - Add connection generation tracking to prevent stale SMP disable - Wire new screens into navigation and developer tools - Update pubspec deps and generated platform registrants
Add Google LiteRT-LM SDK (v0.10.0) as an alternative inference backend alongside the existing fllama/llama.cpp path. This enables running Gemma 4 E2B (.litertlm format) with XNNPack/NEON CPU acceleration. - LiteRtLmBridge.kt: Kotlin native bridge with MethodChannel/EventChannel and Mutex-based serialization to prevent concurrent native calls (SIGSEGV) - litert_lm_service.dart: Dart wrapper for the platform channel bridge - llm_service.dart: dual-path inference (LiteRT-LM vs fllama), model catalog entry for Gemma 4 E2B, import support for .litertlm files, Dart-level locking for concurrent call safety - Gemma 4 benchmarks: 46/49 extraction (93.9%), 22/25 correction (88.0%)
When processAllUnprocessed() runs, mark all pending memos as 'queued' upfront so the UI immediately shows them as waiting for AI instead of displaying the manual 'Process with AI' button (which could trigger duplicate concurrent inference calls). - Add 'queued' to VoiceNoteProcessingStatus enum and isAiProcessing - Show 'Queued for AI processing...' in detail view, 'queued' tag in list - Include 'queued' in crash-recovery reset query
Restore Flutter desktop generated plugin registrant files to their main branch versions so they are not included in the feature PR. Also add explicit ignore rules documenting that these generated desktop registrant files should not be committed from local regeneration.
- Deduplicate workflow runs by branch (not SHA) so feature branches aren't crowded out by multiple main commits filling the numRuns limit - Increase per_page from numRuns*2 to 50 so non-firmware runs (security scans etc.) don't exhaust the fetch budget - Raise default numRuns from 5 to 10 - Add zswatch@1 to _artifactFilters so those artifacts are shown
- Add watch chat service and TTS service - Add chat history DB table, model, and provider - Add AI models settings screen - Improve LLM/LiteRT service integration - Add watchface background selection improvements (horizon sunset asset) - Add TTS flutter_tts dependency - Extend watch_service and watch_service with chat/TTS support - Update generated plugin registrants for new dependencies
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
zswatch_app/lib/data/database/app_database.dart (1)
775-780:⚠️ Potential issue | 🟠 Major | ⚡ Quick winApply the same retention policy to chat history.
cleanupOldData()still skips the newChatHistoryrows, so transcripts and model replies will accumulate indefinitely even thoughdeleteOldChatHistory()already exists. That bypasses the app's 60-day cleanup path for the most privacy-sensitive data in this PR.Suggested fix
Future<void> cleanupOldData() async { final cutoff = DateTime.now().subtract(const Duration(days: 60)); await deleteOldHealthSamples(cutoff); await deleteOldBatteryReadings(cutoff); await deleteOldConnectionEvents(cutoff); + await deleteOldChatHistory(cutoff); }Also applies to: 821-827
🤖 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 `@zswatch_app/lib/data/database/app_database.dart` around lines 775 - 780, cleanupOldData() currently omits chat rows; call deleteOldChatHistory(cutoff) alongside deleteOldHealthSamples, deleteOldBatteryReadings, and deleteOldConnectionEvents inside the Future<void> cleanupOldData() method (use the same cutoff variable), and make the same addition to the other cleanup routine that mirrors these calls (the method that currently invokes deleteOldHealthSamples/deleteOldBatteryReadings/deleteOldConnectionEvents) so chat transcripts and model replies are pruned on the same 60-day retention policy.zswatch_app/lib/services/ai/voice_note_ai_pipeline.dart (1)
288-303:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftClaiming queued memos is still race-prone.
getUnprocessedMemos()runs before any row is claimed, and the laterqueuedwrites happen one memo at a time. If two callers enterprocessAllUnprocessed()together, both can read the same batch before either finishes queueing it, so the same memo can be sent through the LLM twice and re-create actions twice.Please move this to an atomic claim in the repository/database layer, e.g. “mark eligible rows queued in one transaction and return only the rows this caller claimed.”
🤖 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 `@zswatch_app/lib/services/ai/voice_note_ai_pipeline.dart` around lines 288 - 303, The current race: processAllUnprocessed() calls _memoRepository.getUnprocessedMemos(), then loops and calls updateProcessingStatus(filename, 'queued') per memo, allowing concurrent callers to read the same rows; fix by moving the claim into the repository layer as an atomic DB transaction—add or change a method like _memoRepository.claimUnprocessedMemos(batchSize?) (or modify getUnprocessedMemos to perform the claim) that in one transaction selects eligible rows, updates their status to "queued" and returns only those rows this caller claimed; then have processAllUnprocessed() use that new claim method instead of separate read+per-row updates.zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/MainActivity.kt (2)
358-370:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOnly bind the foreground service after a successful start, and avoid double-binding.
bindService(..., BIND_AUTO_CREATE)still runs whenBleConnectionForegroundService.start(...)returnsfalse, which can create/bind the service anyway and leave Dart believing startup failed. Repeatedstartcalls also rebind without checkingforegroundServiceBound.Suggested fix
"start" -> { val watchName = call.argument<String>("watchName") ?: "ZSWatch" val connectionState = call.argument<String>("connectionState") ?: BleConnectionForegroundService.STATE_CONNECTED LifecycleLogger.log("ForegroundChannel", "start watchName=$watchName state=$connectionState") val started = BleConnectionForegroundService.start(this, watchName, connectionState) - // Bind to service to receive disconnect callbacks - val intent = Intent(this, BleConnectionForegroundService::class.java) - bindService(intent, foregroundServiceConnection, Context.BIND_AUTO_CREATE) + if (started && !foregroundServiceBound) { + // Bind to service to receive disconnect callbacks + val intent = Intent(this, BleConnectionForegroundService::class.java) + bindService(intent, foregroundServiceConnection, Context.BIND_AUTO_CREATE) + } result.success(started) }🤖 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 `@zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/MainActivity.kt` around lines 358 - 370, The code currently calls bindService(...) unconditionally after calling BleConnectionForegroundService.start(...), which can create/bind the service even when start(...) returns false and also rebinding on repeated starts; change the logic so you only call bindService(intent, foregroundServiceConnection, Context.BIND_AUTO_CREATE) when started == true and foregroundServiceBound is false (i.e., check the existing binding flag before binding), leaving result.success(started) as before; use the existing symbols BleConnectionForegroundService.start, foregroundServiceConnection and foregroundServiceBound to locate and implement this conditional bind.
661-689:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate timer duration and alarm time before reporting success.
The timer path converts a missing duration to
0and always providesEXTRA_LENGTHto the intent, which prevents Android's graceful fallback (showing UI when duration is missing). A timer with 0 seconds is invalid and should be rejected. The alarm path correctly omits time extras when null, allowing Android to show UI, but still reports success before the user commits—inconsistent with defensive programming principles.Suggested fix
if (actionType == "timer") { + if (durationSeconds == null || durationSeconds <= 0) { + result.error("INVALID_ARGUMENT", "durationSeconds must be > 0 for timers.", null) + return + } try { val response = createTimerViaIntent( - durationSeconds = durationSeconds ?: 0, + durationSeconds = durationSeconds, label = title, skipUi = skipUi ) Log.d(TAG, "createAction (timer) succeeded with response=$response") result.success(response) @@ if (actionType == "alarm") { + if (scheduledAtMillis == null) { + result.error("INVALID_ARGUMENT", "scheduledAtMillis is required for alarms.", null) + return + } try { val response = createAlarmViaIntent( - triggerAtMillis = scheduledAtMillis, + triggerAtMillis = scheduledAtMillis, label = title, skipUi = skipUi )🤖 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 `@zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/MainActivity.kt` around lines 661 - 689, The timer and alarm branches prematurely report success and pass invalid/missing time values: in the actionType == "timer" branch validate durationSeconds is non-null and > 0 (do not convert null to 0 and do not include the EXTRA_LENGTH intent extra when duration is missing), and return result.error for invalid or missing durations before calling createTimerViaIntent; in the actionType == "alarm" branch require scheduledAtMillis to be non-null and a sensible future timestamp (or otherwise return result.error) and only call result.success after createAlarmViaIntent completes successfully; use the symbols actionType, durationSeconds, createTimerViaIntent, scheduledAtMillis, and createAlarmViaIntent to locate and update the checks and error handling.
♻️ Duplicate comments (1)
zswatch_app/lib/ui/screens/watchface/watchface_crop_screen.dart (1)
89-93:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
onCroppedstill passesCropResultasUint8List— runtime type error for callers.
crop_your_image1.1.0 changed "the argument type exposed byonCroppedcallback intoCropResultfromUint8List." The result is exposed as aCropResultobject —CropResult.success()contains cropped image data, andCropResult.error()contains an error object.The current
(croppedBytes)parameter is therefore typed asCropResult, notUint8List. CallingNavigator.of(context).pop(croppedBytes)passes aCropResulttowatchface_background_screen.dart, which expectsUint8List— this is a runtime type error on a successful crop.🐛 Proposed fix — handle both `CropResult` arms
- onCropped: (croppedBytes) { - if (!mounted) return; - setState(() => _isCropping = false); - Navigator.of(context).pop(croppedBytes); - }, + onCropped: (result) { + if (!mounted) return; + switch (result) { + case CropResult.success(:final croppedImage): + setState(() => _isCropping = false); + Navigator.of(context).pop(croppedImage); + case CropResult.error(:final error): + setState(() => _isCropping = false); + debugPrint('[WatchfaceCropScreen] Crop error: $error'); + ScaffoldMessenger.of(context).showSnackBar( + const SnackBar(content: Text('Cropping failed, please try again.')), + ); + } + },crop_your_image 1.1.0 CropResult sealed class dart pattern matching🤖 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 `@zswatch_app/lib/ui/screens/watchface/watchface_crop_screen.dart` around lines 89 - 93, The onCropped callback in WatchfaceCropScreen is receiving a CropResult (not Uint8List) and must handle both success and error arms: update the onCropped handler in watchface_crop_screen.dart (the closure that sets _isCropping and calls Navigator.of(context).pop) to pattern-match or inspect CropResult (e.g., CropResult.success() vs CropResult.error()), extract the Uint8List bytes from CropResult.success() and pass those bytes to Navigator.of(context).pop, and handle CropResult.error() by logging/propagating an error or popping null/empty as the app expects (so callers in watchface_background_screen.dart continue to receive a Uint8List or null).
🧹 Nitpick comments (4)
zswatch_app/lib/services/dfu/firmware_manager.dart (1)
173-181: 💤 Low value
per_page=50doesn't scale withnumRuns— consider deriving it dynamically.The hardcoded
per_page=50is a fixed fetch window regardless of thenumRunsargument. For the current default of 10 the 5× buffer is fine, but a caller passingnumRuns: 20could silently receive fewer results than requested once the success-filtered, branch-deduplicated pool shrinks below 20.♻️ Suggested fix
- Uri.parse('$_apiBase/repos/$_owner/$_repo/actions/runs?per_page=50'), + Uri.parse('$_apiBase/repos/$_owner/$_repo/actions/runs?per_page=${(numRuns * 5).clamp(20, 100)}'),🤖 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 `@zswatch_app/lib/services/dfu/firmware_manager.dart` around lines 173 - 181, fetchWorkflowRuns currently hardcodes per_page=50 which won't scale when callers pass a larger numRuns; update fetchWorkflowRuns to compute per_page dynamically (e.g. perPage = max(numRuns * N, minValue) and clamp to GitHub max 100) and use that computed perPage when building the request URI (the http.get call that uses '$_apiBase/repos/$_owner/$_repo/actions/runs?per_page=50'); ensure the computed perPage replaces the literal 50 in the Future.wait request(s) so the returned pool reliably covers the requested numRuns after filtering/deduplication.zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/NativeBackgroundPreferences.kt (1)
83-94: ⚡ Quick win
getStringSet()returns a live internal reference — add a defensive copy.Per Android docs, the
Setreturned bygetStringSet()is internal toSharedPreferencesand must not be modified or later passed back toputStringSet(). Storing it directly inSnapshot.blockedNotificationAppsis safe today (callers only read it), but a single line of.toSet()eliminates the risk of future code accidentally mutating SharedPreferences' internal state.♻️ Proposed fix
- blockedNotificationApps = prefs.getStringSet(KEY_BLOCKED_NOTIFICATION_APPS, emptySet()) ?: emptySet(), + blockedNotificationApps = prefs.getStringSet(KEY_BLOCKED_NOTIFICATION_APPS, emptySet())?.toSet() ?: emptySet(),🤖 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 `@zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/NativeBackgroundPreferences.kt` around lines 83 - 94, In getSnapshot, prefs.getStringSet(KEY_BLOCKED_NOTIFICATION_APPS, emptySet()) returns a live internal SharedPreferences reference; change the assignment to Snapshot.blockedNotificationApps to store an immutable defensive copy (e.g., call .toSet() or create a new HashSet) so you don't keep or later pass back the internal set—update the line that builds Snapshot.blockedNotificationApps in the getSnapshot function accordingly.zswatch_app/lib/providers/settings_providers.dart (1)
529-584: 🏗️ Heavy liftMove the voice-chat providers into a chat-specific provider module.
This block adds a new chat domain to a catch-all settings file. Keeping it under
chat_providers.dartor a dedicatedchat_settings_providers.dartwould match the repo’s provider boundary rule and keep future chat changes localized.As per coding guidelines, "Organize providers in
providers/directory with one file per domain (e.g.,health_providers.dart), exporting relevant services and state."🤖 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 `@zswatch_app/lib/providers/settings_providers.dart` around lines 529 - 584, The new chat-related providers (chatAnswerLengthProvider / ChatAnswerLengthNotifier, chatMemoryModeProvider / ChatMemoryModeNotifier, and chatMaxHistoryProvider / ChatMaxHistoryNotifier) should be moved out of the generic settings_providers file into a chat-specific provider module; create a new file (e.g., chat_settings_providers.dart) under providers/, copy these three provider declarations and their notifier classes there, update any exports/imports to expose them from the providers package, and remove the moved code from settings_providers.dart so chat domain logic is localized to the chat providers file.zswatch_app/lib/services/chat/watch_chat_service.dart (1)
296-304: ⚡ Quick winIsolate async call to avoid losing history entry on model-info failure.
The
await _llmService.currentModelInfo()inside the insert call means if model info retrieval throws, the entire chat history entry is lost—even though the actual chat succeeded. Consider extracting beforehand with a fallback:+ // Capture model info safely before insert + String? modelName; + try { + modelName = (await _llmService.currentModelInfo()).displayName; + } catch (_) { + modelName = 'unknown'; + } + await _database.insertChatHistoryEntry( ChatHistoryCompanion.insert( timestampUtc: DateTime.now().toUtc().millisecondsSinceEpoch ~/ 1000, transcript: transcript, answer: Value(answer), - modelUsed: Value((await _llmService.currentModelInfo()).displayName), + modelUsed: Value(modelName), latencyMs: Value(latencyMs), ), );🤖 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 `@zswatch_app/lib/services/chat/watch_chat_service.dart` around lines 296 - 304, Extract the model info before building the ChatHistoryCompanion so a failure in _llmService.currentModelInfo() doesn't abort the insert: call await _llmService.currentModelInfo() in a try/catch, set a fallback string (e.g., "unknown" or previousModel) on exception, then pass that Value into ChatHistoryCompanion.insert for the modelUsed field and call _database.insertChatHistoryEntry; reference _llmService.currentModelInfo(), ChatHistoryCompanion.insert, and _database.insertChatHistoryEntry when making the change.
🤖 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 `@zswatch_app/android/app/src/main/AndroidManifest.xml`:
- Line 20: Remove the unused battery exemption permission by deleting the
<uses-permission> entry for
android.permission.REQUEST_IGNORE_BATTERY_OPTIMIZATIONS from the Android
manifest (the element containing
android:name="android.permission.REQUEST_IGNORE_BATTERY_OPTIMIZATIONS"); ensure
no code calls ACTION_REQUEST_IGNORE_BATTERY_OPTIMIZATIONS before removing, and
run a build to verify no references remain to that permission string.
In `@zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/BootReceiver.kt`:
- Around line 10-11: Remove the synchronous disk I/O call from the
BroadcastReceiver by deleting the call to
LifecycleLogger.recordHistoricalExitReasons(...) from BootReceiver.onReceive and
leave only LifecycleLogger.initialize(context) there; rely on
BleConnectionForegroundService.onCreate() (which already calls
LifecycleLogger.initialize() and recordHistoricalExitReasons()) to perform the
historical exit reason recording off the main thread.
In `@zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/LifecycleLogger.kt`:
- Around line 87-92: The NotificationService branch in LifecycleLogger.kt
currently only allows messages containing "oncreate", "ondestroy",
"listenerconnected", or "listenerdisconnected", so new logs from
NotificationListenerServiceImpl.kt like "drop posted notification ..." and
"forward removed notification ..." are not persisted; update the predicate in
the lowerSource.contains("notificationservice") branch to also accept
notification post/remove events by adding checks such as
lowerMessage.contains("posted") || lowerMessage.contains("removed") (or the
specific tokens "post"/"remove" you prefer) against lowerMessage, or
alternatively route those events under a dedicated source name that
shouldPersist(...) recognizes; modify the condition around lowerMessage to
include these new clauses so posted/removed diagnostics are persisted.
In `@zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/LiteRtLmBridge.kt`:
- Around line 264-271: The cancel() implementation must not call
currentJob?.cancel() because generate() converts the SDK cancellation callback
into a `{ cancelled: true }` result (via conversation.cancelProcess()), and
cancelling the coroutine can cause deferred.await() to fail into the
GENERATE_FAILED path before the callback finishes; remove or disable
currentJob?.cancel() from cancel() so only conversation.cancelProcess() is
invoked, and instead ensure the generation coroutine clears currentJob when it
exits (e.g., in generate()’s finally/cleanup path set currentJob = null) so the
job reference is cleared after completion.
In `@zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/MainActivity.kt`:
- Around line 130-131: MainActivity currently creates liteRtLmBridge via
liteRtLmBridge = LiteRtLmBridge(this) and calls setup(flutterEngine) but never
disposes it; update MainActivity's lifecycle teardown (override onDestroy()) to
call liteRtLmBridge?.dispose() (or the bridge's appropriate cleanup method) and
then set liteRtLmBridge = null, similar to how mediaBridge is disposed, to
release native model/session state and prevent leaks; apply the same cleanup in
the other activity lifecycle usages referenced around the 1187-1199 region.
In `@zswatch_app/lib/data/database/app_database.g.dart`:
- Around line 5607-5680: Persisting full transcript/answer/errorMessage
indefinitely is a privacy risk; modify the schema and migration to add
retention/minimization controls: add a nullable expiration column (e.g.,
GeneratedColumn<DateTime> expiresAt or int expires_at_utc) to the table
alongside transcript/answer/errorMessage in the table definition (so update the
GeneratedColumn list that includes transcript, answer, modelUsed, latencyMs,
success, errorMessage and $columns), implement a migration that creates the new
column and backfills or sets sensible default retention for existing rows, and
update insertion/DAO logic to either store minimized payloads (e.g.,
redactedTranscript or hashedTranscript and a trimmed answer field) or enforce
max length before persisting; finally add a periodic purge routine (or DB
TTL/index-based delete) that deletes rows where expiresAt is past to ensure data
is removed.
In `@zswatch_app/lib/data/models/chat_history_entry.dart`:
- Around line 25-44: copyWith currently uses `??` so passing null for nullable
properties (answer, modelUsed, latencyMs, errorMessage) cannot clear them;
change copyWith on ChatHistoryEntry to use a sentinel-based API: add a private
sentinel (e.g. Object _unset = Object()) and change the parameters for the
nullable fields to accept Object? (or use Object sentinel-typed params) instead
of their concrete nullable types, then in the return expression use conditional
checks like `param == _unset ? this.field : param as T?` to preserve existing
value only when the sentinel is provided and allow explicit null to clear the
field; update parameters for answer, modelUsed, latencyMs, and errorMessage and
the return mapping in copyWith accordingly.
In `@zswatch_app/lib/providers/analytics_providers.dart`:
- Around line 230-235: The family parameter of lifecycleLogEntriesProvider is
never used so every watch gets the same global lifecycle list; update the
provider to actually use the family key (e.g., rename the second param to
watchId) and filter the returned entries by that id (either call a filtered API
like service.getLifecycleEvents(watchId) or post-filter the List from
service.getLifecycleEvents() with .where((e) => e.watchId == watchId)), or if
you intend a global diagnostics source, remove .family and the unused parameter
and make it a non-family provider so it is clearly app-scoped; locate
lifecycleLogEntriesProvider, foregroundServiceProvider and getLifecycleEvents()
to implement the appropriate change.
In `@zswatch_app/lib/providers/file_system_provider.dart`:
- Around line 76-90: After calling _watchService.enableSmp(), awaiting the delay
and calling _watchService.rediscoverServices(), re-read the watch device to
avoid using a stale device.remoteId: capture the latest device from
_watchService.getDevice(...) or the same accessor used originally (so the
probeFiles and subsequent download call use the refreshed device.remoteId), and
keep the existing smpGen comparison logic in the finally block; apply the same
re-read/guard pattern in the second occurrence around lines 119–135 so both
probeFiles and any download operations target the current connection.
In `@zswatch_app/lib/providers/foreground_service_providers.dart`:
- Around line 49-60: scheduleSync currently only sends non-null fields to
syncBackgroundPreferences so Android never learns to clear a stale lastWatch
snapshot; update the payload/API to include an explicit clear indicator for the
last watch (e.g. add clearLastWatch boolean or allow empty-string values for
lastWatchId/lastWatchName) and make scheduleSync send that when the provider
detects there is no valid/paired watch (i.e. when lastWatchId/lastWatchName are
null or disconnected); also update the syncBackgroundPreferences caller and
implementation to honor the clear flag or empty values and delete/clear the
native stored last-watch values accordingly (apply same change for the other
sync call used around lines 86-99).
In `@zswatch_app/lib/services/dfu/filesystem_upload_service.dart`:
- Around line 248-283: The _completeUpload method can publish completed/failed
after a concurrent cancel() clears _fsManager/_expectedUploadPath, so capture a
short-lived upload token at the start of _completeUpload (or read a unique
_currentUploadId/_uploadGeneration) and re-check it before calling _updateState
or running _cleanup; specifically, read the generation/id at the top of
_completeUpload, use the same token for all async checks (status/size) and
before each terminal _updateState(FilesystemUploadState.completed/failed...)
bail out if the current upload id/generation no longer matches (meaning cancel()
or a new upload replaced it), ensuring _cleanup and state transitions only run
for the matching upload instance.
- Around line 94-104: In the onError handler the call to _cleanup() is dropped,
which triggers the unawaited_futures lint; wrap that call as a fire-and-forget
by invoking unawaited(_cleanup()) so the Future is intentionally ignored
(keeping the rest of the handler that logs the error, stops the timer, and
updates state via FilesystemUploadState.failed unchanged); ensure the unawaited
symbol is imported where appropriate.
In `@zswatch_app/lib/services/tts/tts_service.dart`:
- Around line 198-202: The stop() method currently calls _tts.stop() but does
not resolve the in-progress synthesis future, so synthesizeToFile() can hang;
inside stop() detect if _synthesisCompleter exists and is not completed,
complete it (e.g., completeError with a cancellation error or complete with a
cancellation value consistent with synthesizeToFile()'s contract), then
null/clear _synthesisCompleter and finally add TtsStatus.idle to
_statusController; ensure you reference the existing stop(), _tts.stop(),
_synthesisCompleter, and _statusController.add(TtsStatus.idle) symbols when
making the change.
In `@zswatch_app/lib/services/watch_service.dart`:
- Around line 487-502: The early returns in _setupNus when nusService or rxChar
are missing should not silently return: replace those returns with code that
surfaces a setup failure so the connection enters the error/reconnect path (i.e.
invoke your setup-failure handler instead of returning). Specifically, in
_setupNus (when nusService == null or rxChar == null) call the existing
setup-failure flow used by _onSetupRequired() (for example invoke the module's
_onSetupFailed/_handleSetupFailure/other failure handler) and ensure _sendNus()
will no longer be treated as usable; this makes the missing NUS
service/characteristic a real setup failure and triggers reconnect/error
handling.
In `@zswatch_app/lib/ui/screens/watchface/watchface_crop_screen.dart`:
- Around line 94-101: The onStatusChanged callback for the crop controller
currently resets _isCropping to false when status == CropStatus.ready without
surfacing an error; update the branch inside onStatusChanged to also notify the
user or developer when this reset happens by emitting a visible message (e.g.,
show a SnackBar via ScaffoldMessenger.of(context).showSnackBar(...) or call
debugPrint) indicating the crop failed, while still guarding with if (!mounted)
return and setState(() => _isCropping = false); reference the onStatusChanged
closure, CropStatus.ready, and the _isCropping state so you add the feedback
next to the existing setState call.
---
Outside diff comments:
In `@zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/MainActivity.kt`:
- Around line 358-370: The code currently calls bindService(...) unconditionally
after calling BleConnectionForegroundService.start(...), which can create/bind
the service even when start(...) returns false and also rebinding on repeated
starts; change the logic so you only call bindService(intent,
foregroundServiceConnection, Context.BIND_AUTO_CREATE) when started == true and
foregroundServiceBound is false (i.e., check the existing binding flag before
binding), leaving result.success(started) as before; use the existing symbols
BleConnectionForegroundService.start, foregroundServiceConnection and
foregroundServiceBound to locate and implement this conditional bind.
- Around line 661-689: The timer and alarm branches prematurely report success
and pass invalid/missing time values: in the actionType == "timer" branch
validate durationSeconds is non-null and > 0 (do not convert null to 0 and do
not include the EXTRA_LENGTH intent extra when duration is missing), and return
result.error for invalid or missing durations before calling
createTimerViaIntent; in the actionType == "alarm" branch require
scheduledAtMillis to be non-null and a sensible future timestamp (or otherwise
return result.error) and only call result.success after createAlarmViaIntent
completes successfully; use the symbols actionType, durationSeconds,
createTimerViaIntent, scheduledAtMillis, and createAlarmViaIntent to locate and
update the checks and error handling.
In `@zswatch_app/lib/data/database/app_database.dart`:
- Around line 775-780: cleanupOldData() currently omits chat rows; call
deleteOldChatHistory(cutoff) alongside deleteOldHealthSamples,
deleteOldBatteryReadings, and deleteOldConnectionEvents inside the Future<void>
cleanupOldData() method (use the same cutoff variable), and make the same
addition to the other cleanup routine that mirrors these calls (the method that
currently invokes
deleteOldHealthSamples/deleteOldBatteryReadings/deleteOldConnectionEvents) so
chat transcripts and model replies are pruned on the same 60-day retention
policy.
In `@zswatch_app/lib/services/ai/voice_note_ai_pipeline.dart`:
- Around line 288-303: The current race: processAllUnprocessed() calls
_memoRepository.getUnprocessedMemos(), then loops and calls
updateProcessingStatus(filename, 'queued') per memo, allowing concurrent callers
to read the same rows; fix by moving the claim into the repository layer as an
atomic DB transaction—add or change a method like
_memoRepository.claimUnprocessedMemos(batchSize?) (or modify getUnprocessedMemos
to perform the claim) that in one transaction selects eligible rows, updates
their status to "queued" and returns only those rows this caller claimed; then
have processAllUnprocessed() use that new claim method instead of separate
read+per-row updates.
---
Duplicate comments:
In `@zswatch_app/lib/ui/screens/watchface/watchface_crop_screen.dart`:
- Around line 89-93: The onCropped callback in WatchfaceCropScreen is receiving
a CropResult (not Uint8List) and must handle both success and error arms: update
the onCropped handler in watchface_crop_screen.dart (the closure that sets
_isCropping and calls Navigator.of(context).pop) to pattern-match or inspect
CropResult (e.g., CropResult.success() vs CropResult.error()), extract the
Uint8List bytes from CropResult.success() and pass those bytes to
Navigator.of(context).pop, and handle CropResult.error() by logging/propagating
an error or popping null/empty as the app expects (so callers in
watchface_background_screen.dart continue to receive a Uint8List or null).
---
Nitpick comments:
In
`@zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/NativeBackgroundPreferences.kt`:
- Around line 83-94: In getSnapshot,
prefs.getStringSet(KEY_BLOCKED_NOTIFICATION_APPS, emptySet()) returns a live
internal SharedPreferences reference; change the assignment to
Snapshot.blockedNotificationApps to store an immutable defensive copy (e.g.,
call .toSet() or create a new HashSet) so you don't keep or later pass back the
internal set—update the line that builds Snapshot.blockedNotificationApps in the
getSnapshot function accordingly.
In `@zswatch_app/lib/providers/settings_providers.dart`:
- Around line 529-584: The new chat-related providers (chatAnswerLengthProvider
/ ChatAnswerLengthNotifier, chatMemoryModeProvider / ChatMemoryModeNotifier, and
chatMaxHistoryProvider / ChatMaxHistoryNotifier) should be moved out of the
generic settings_providers file into a chat-specific provider module; create a
new file (e.g., chat_settings_providers.dart) under providers/, copy these three
provider declarations and their notifier classes there, update any
exports/imports to expose them from the providers package, and remove the moved
code from settings_providers.dart so chat domain logic is localized to the chat
providers file.
In `@zswatch_app/lib/services/chat/watch_chat_service.dart`:
- Around line 296-304: Extract the model info before building the
ChatHistoryCompanion so a failure in _llmService.currentModelInfo() doesn't
abort the insert: call await _llmService.currentModelInfo() in a try/catch, set
a fallback string (e.g., "unknown" or previousModel) on exception, then pass
that Value into ChatHistoryCompanion.insert for the modelUsed field and call
_database.insertChatHistoryEntry; reference _llmService.currentModelInfo(),
ChatHistoryCompanion.insert, and _database.insertChatHistoryEntry when making
the change.
In `@zswatch_app/lib/services/dfu/firmware_manager.dart`:
- Around line 173-181: fetchWorkflowRuns currently hardcodes per_page=50 which
won't scale when callers pass a larger numRuns; update fetchWorkflowRuns to
compute per_page dynamically (e.g. perPage = max(numRuns * N, minValue) and
clamp to GitHub max 100) and use that computed perPage when building the request
URI (the http.get call that uses
'$_apiBase/repos/$_owner/$_repo/actions/runs?per_page=50'); ensure the computed
perPage replaces the literal 50 in the Future.wait request(s) so the returned
pool reliably covers the requested numRuns after filtering/deduplication.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9eebd499-f059-41b0-9275-2019aff485ea
⛔ Files ignored due to path filters (67)
zswatch_app/assets/backgrounds/bg_aurora.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_aurora.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_brushed_titanium.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_brushed_titanium.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_copper_mesh.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_copper_mesh.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_crt_neon_grid.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_crt_neon_grid.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_deep_space.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_deep_space.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_ember.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_ember.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_flow_field_silk.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_flow_field_silk.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_flow_ink_parchment.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_flow_ink_parchment.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_forest_canopy.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_forest_canopy.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_fume_guilloche.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_fume_guilloche.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_guilloche_dial.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_guilloche_dial.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_halftone_poster.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_halftone_poster.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_horizon_sunset.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_horizon_sunset.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_ink_wash.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_ink_wash.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_lava_flow.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_lava_flow.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_low_poly_shards.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_low_poly_shards.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_matrix_rain.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_matrix_rain.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_midnight_frost.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_midnight_frost.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_moire_interference.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_moire_interference.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_mosaic_gem.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_mosaic_gem.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_ocean.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_ocean.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_paper_cut.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_paper_cut.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_pixel_neon.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_pixel_neon.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_reaction_coral.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_reaction_coral.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_retro_sunburst.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_retro_sunburst.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_sdf_rings.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_sdf_rings.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_solar_flare.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_solar_flare.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_starfield.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_starfield.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_topo_metal_arc.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_topo_metal_arc.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_topographic.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_topographic.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_voronoi_glass.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_voronoi_glass.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_vortex.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_vortex.pngis excluded by!**/*.pngzswatch_app/assets/backgrounds/bg_warp_marble.binis excluded by!**/*.binzswatch_app/assets/backgrounds/bg_warp_marble.pngis excluded by!**/*.pngzswatch_app/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (58)
zswatch_app/.gitignorezswatch_app/android/app/build.gradle.ktszswatch_app/android/app/src/main/AndroidManifest.xmlzswatch_app/android/app/src/main/kotlin/dev/zswatch/app/BleConnectionForegroundService.ktzswatch_app/android/app/src/main/kotlin/dev/zswatch/app/BootReceiver.ktzswatch_app/android/app/src/main/kotlin/dev/zswatch/app/LifecycleLogger.ktzswatch_app/android/app/src/main/kotlin/dev/zswatch/app/LiteRtLmBridge.ktzswatch_app/android/app/src/main/kotlin/dev/zswatch/app/MainActivity.ktzswatch_app/android/app/src/main/kotlin/dev/zswatch/app/MediaSessionBridge.ktzswatch_app/android/app/src/main/kotlin/dev/zswatch/app/NativeBackgroundPreferences.ktzswatch_app/android/app/src/main/kotlin/dev/zswatch/app/NotificationListenerServiceImpl.ktzswatch_app/ios/Runner/AppDelegate.swiftzswatch_app/lib/app.dartzswatch_app/lib/core/utils/lifecycle_logger.dartzswatch_app/lib/core/utils/lvgl_image_converter.dartzswatch_app/lib/core/utils/watchface_image_processor.dartzswatch_app/lib/data/database/app_database.dartzswatch_app/lib/data/database/app_database.g.dartzswatch_app/lib/data/database/tables/chat_history_table.dartzswatch_app/lib/data/models/chat_history_entry.dartzswatch_app/lib/data/models/fs_file_entry.dartzswatch_app/lib/data/models/voice_memo.dartzswatch_app/lib/providers/analytics_providers.dartzswatch_app/lib/providers/chat_providers.dartzswatch_app/lib/providers/file_system_provider.dartzswatch_app/lib/providers/foreground_service_providers.dartzswatch_app/lib/providers/settings_providers.dartzswatch_app/lib/providers/watchface_background_provider.dartzswatch_app/lib/services/ai/litert_lm_service.dartzswatch_app/lib/services/ai/llm_service.dartzswatch_app/lib/services/ai/voice_note_ai_pipeline.dartzswatch_app/lib/services/background/foreground_service.dartzswatch_app/lib/services/ble/ble_connection_service.dartzswatch_app/lib/services/chat/watch_chat_service.dartzswatch_app/lib/services/coredump/coredump_service.dartzswatch_app/lib/services/dfu/filesystem_upload_service.dartzswatch_app/lib/services/dfu/firmware_manager.dartzswatch_app/lib/services/filesystem/file_system_service.dartzswatch_app/lib/services/tts/tts_service.dartzswatch_app/lib/services/voice_memo/voice_memo_sync_service.dartzswatch_app/lib/services/watch_service.dartzswatch_app/lib/ui/navigation/app_router.dartzswatch_app/lib/ui/screens/analytics/analytics_screen.dartzswatch_app/lib/ui/screens/dashboard/dashboard_screen.dartzswatch_app/lib/ui/screens/developer/developer_screen.dartzswatch_app/lib/ui/screens/developer/file_system_screen.dartzswatch_app/lib/ui/screens/firmware/firmware_update_screen.dartzswatch_app/lib/ui/screens/settings/ai_models_settings_screen.dartzswatch_app/lib/ui/screens/voice_memos/voice_memos_screen.dartzswatch_app/lib/ui/screens/watchface/watchface_background_screen.dartzswatch_app/lib/ui/screens/watchface/watchface_crop_screen.dartzswatch_app/lib/ui/widgets/voice_memos/memo_list_item.dartzswatch_app/linux/flutter/generated_plugin_registrant.cczswatch_app/linux/flutter/generated_plugins.cmakezswatch_app/macos/Flutter/GeneratedPluginRegistrant.swiftzswatch_app/pubspec.yamlzswatch_app/windows/flutter/generated_plugin_registrant.cczswatch_app/windows/flutter/generated_plugins.cmake
✅ Files skipped from review due to trivial changes (14)
- zswatch_app/android/app/build.gradle.kts
- zswatch_app/linux/flutter/generated_plugins.cmake
- zswatch_app/.gitignore
- zswatch_app/lib/core/utils/lvgl_image_converter.dart
- zswatch_app/lib/data/database/tables/chat_history_table.dart
- zswatch_app/lib/ui/screens/voice_memos/voice_memos_screen.dart
- zswatch_app/macos/Flutter/GeneratedPluginRegistrant.swift
- zswatch_app/lib/services/coredump/coredump_service.dart
- zswatch_app/lib/services/ble/ble_connection_service.dart
- zswatch_app/windows/flutter/generated_plugin_registrant.cc
- zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/MediaSessionBridge.kt
- zswatch_app/windows/flutter/generated_plugins.cmake
- zswatch_app/lib/ui/screens/developer/file_system_screen.dart
- zswatch_app/lib/data/models/voice_memo.dart
🚧 Files skipped from review as they are similar to previous changes (10)
- zswatch_app/lib/services/voice_memo/voice_memo_sync_service.dart
- zswatch_app/lib/ui/screens/firmware/firmware_update_screen.dart
- zswatch_app/lib/ui/screens/developer/developer_screen.dart
- zswatch_app/pubspec.yaml
- zswatch_app/lib/ui/screens/dashboard/dashboard_screen.dart
- zswatch_app/lib/ui/screens/watchface/watchface_background_screen.dart
- zswatch_app/lib/services/ai/litert_lm_service.dart
- zswatch_app/lib/ui/navigation/app_router.dart
- zswatch_app/lib/services/filesystem/file_system_service.dart
- zswatch_app/lib/services/ai/llm_service.dart
| <uses-permission android:name="android.permission.FOREGROUND_SERVICE_CONNECTED_DEVICE" /> | ||
| <uses-permission android:name="android.permission.FOREGROUND_SERVICE_SHORT_SERVICE" /> | ||
| <uses-permission android:name="android.permission.WAKE_LOCK" /> | ||
| <uses-permission android:name="android.permission.REQUEST_IGNORE_BATTERY_OPTIMIZATIONS" /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether ACTION_REQUEST_IGNORE_BATTERY_OPTIMIZATIONS or isIgnoringBatteryOptimizations
# are referenced anywhere in the Kotlin/Dart sources.
rg -n "REQUEST_IGNORE_BATTERY_OPTIMIZATIONS\|isIgnoringBatteryOptimizations\|ACTION_REQUEST_IGNORE" \
--type kotlin --type dart -C 3Repository: ZSWatch/ZSWatch-App
Length of output: 45
Remove REQUEST_IGNORE_BATTERY_OPTIMIZATIONS permission — not actively used in codebase.
This permission is declared but no code references it (ACTION_REQUEST_IGNORE_BATTERY_OPTIMIZATIONS or equivalent calls are absent). Google Play rejects apps that request battery optimization exemptions without demonstrating that core functionality requires it. Remove this permission unless you add functionality that explicitly requests battery optimization exemption.
🤖 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 `@zswatch_app/android/app/src/main/AndroidManifest.xml` at line 20, Remove the
unused battery exemption permission by deleting the <uses-permission> entry for
android.permission.REQUEST_IGNORE_BATTERY_OPTIMIZATIONS from the Android
manifest (the element containing
android:name="android.permission.REQUEST_IGNORE_BATTERY_OPTIMIZATIONS"); ensure
no code calls ACTION_REQUEST_IGNORE_BATTERY_OPTIMIZATIONS before removing, and
run a build to verify no references remain to that permission string.
Comment on lines
+87
to
+92
| if (lowerSource.contains("notificationservice")) { | ||
| return lowerMessage.contains("oncreate") || | ||
| lowerMessage.contains("ondestroy") || | ||
| lowerMessage.contains("listenerconnected") || | ||
| lowerMessage.contains("listenerdisconnected") | ||
| } |
There was a problem hiding this comment.
These notification forward/drop events are filtered out.
NotificationListenerServiceImpl.kt now logs messages like drop posted notification ... and forward removed notification ..., but this branch only persists oncreate, ondestroy, listenerconnected, and listenerdisconnected. The new posted/removed diagnostics never make it into SharedPreferences.
Either broaden this predicate or log those events under a dedicated source that shouldPersist(...) accepts.
🤖 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 `@zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/LifecycleLogger.kt`
around lines 87 - 92, The NotificationService branch in LifecycleLogger.kt
currently only allows messages containing "oncreate", "ondestroy",
"listenerconnected", or "listenerdisconnected", so new logs from
NotificationListenerServiceImpl.kt like "drop posted notification ..." and
"forward removed notification ..." are not persisted; update the predicate in
the lowerSource.contains("notificationservice") branch to also accept
notification post/remove events by adding checks such as
lowerMessage.contains("posted") || lowerMessage.contains("removed") (or the
specific tokens "post"/"remove" you prefer) against lowerMessage, or
alternatively route those events under a dedicated source name that
shouldPersist(...) recognizes; modify the condition around lowerMessage to
include these new clauses so posted/removed diagnostics are persisted.
Comment on lines
+264
to
+271
| private fun cancel() { | ||
| try { | ||
| conversation?.cancelProcess() | ||
| } catch (e: Exception) { | ||
| Log.w(TAG, "cancel failed: ${e.message}") | ||
| } | ||
| currentJob?.cancel() | ||
| } |
There was a problem hiding this comment.
Don’t cancel the coroutine that is supposed to report cancellation.
generate() already converts the SDK’s cancellation callback into a { cancelled: true } result, but cancel() also does currentJob?.cancel(). That can abort the coroutine while it is waiting on deferred.await(), sending the request down the GENERATE_FAILED path before the callback has a chance to complete normally.
Let conversation.cancelProcess() drive the cancelled result, and clear currentJob after the generation coroutine exits.
🤖 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 `@zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/LiteRtLmBridge.kt`
around lines 264 - 271, The cancel() implementation must not call
currentJob?.cancel() because generate() converts the SDK cancellation callback
into a `{ cancelled: true }` result (via conversation.cancelProcess()), and
cancelling the coroutine can cause deferred.await() to fail into the
GENERATE_FAILED path before the callback finishes; remove or disable
currentJob?.cancel() from cancel() so only conversation.cancelProcess() is
invoked, and instead ensure the generation coroutine clears currentJob when it
exits (e.g., in generate()’s finally/cleanup path set currentJob = null) so the
job reference is cleared after completion.
Comment on lines
+130
to
+131
| liteRtLmBridge = LiteRtLmBridge(this) | ||
| liteRtLmBridge!!.setup(flutterEngine) |
There was a problem hiding this comment.
Dispose LiteRtLmBridge when the activity is torn down.
MainActivity now owns a bridge that holds native model/session state, but onDestroy() only disposes mediaBridge. Recreated activities can leak the old LLM backend until process death.
Suggested fix
override fun onDestroy() {
LifecycleLogger.log(
"MainActivity",
"onDestroy foregroundBound=$foregroundServiceBound notificationSink=${notificationEventSink != null} mediaSink=${mediaEventSink != null}",
)
if (foregroundServiceBound) {
unbindService(foregroundServiceConnection)
foregroundServiceBound = false
}
+ liteRtLmBridge?.dispose()
+ liteRtLmBridge = null
mediaBridge?.dispose()
mediaBridge = null
NotificationListenerServiceImpl.notificationCallback = null
super.onDestroy()
}Also applies to: 1187-1199
🤖 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 `@zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/MainActivity.kt`
around lines 130 - 131, MainActivity currently creates liteRtLmBridge via
liteRtLmBridge = LiteRtLmBridge(this) and calls setup(flutterEngine) but never
disposes it; update MainActivity's lifecycle teardown (override onDestroy()) to
call liteRtLmBridge?.dispose() (or the bridge's appropriate cleanup method) and
then set liteRtLmBridge = null, similar to how mediaBridge is disposed, to
release native model/session state and prevent leaks; apply the same cleanup in
the other activity lifecycle usages referenced around the 1187-1199 region.
Comment on lines
94
to
104
| onError: (Object error) { | ||
| _log('Upload error: $error'); | ||
| _stopSpeedTimer(); | ||
| _updateState( | ||
| FilesystemUploadState.failed( | ||
| error.toString(), | ||
| startedAt: startTime, | ||
| ), | ||
| ); | ||
| _cleanup(); | ||
| }, |
There was a problem hiding this comment.
Mark _cleanup() as fire-and-forget here.
Line 103 drops the Future from _cleanup() even though the sibling failure/cancel branches already use unawaited(...). That leaves cleanup failures unobserved and will trip the repo’s unawaited_futures rule.
♻️ Suggested fix
- _cleanup();
+ unawaited(_cleanup());As per coding guidelines "Enforce rules: cancel_subscriptions, close_sinks, unawaited_futures, prefer_single_quotes, prefer_const_constructors."
🤖 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 `@zswatch_app/lib/services/dfu/filesystem_upload_service.dart` around lines 94
- 104, In the onError handler the call to _cleanup() is dropped, which triggers
the unawaited_futures lint; wrap that call as a fire-and-forget by invoking
unawaited(_cleanup()) so the Future is intentionally ignored (keeping the rest
of the handler that logs the error, stops the timer, and updates state via
FilesystemUploadState.failed unchanged); ensure the unawaited symbol is imported
where appropriate.
Comment on lines
+248
to
+283
| Future<void> _completeUpload(DateTime startTime) async { | ||
| _stopSpeedTimer(); | ||
| _log('Upload transfer completed, verifying remote file...'); | ||
|
|
||
| try { | ||
| final fsManager = _fsManager; | ||
| final expectedPath = _expectedUploadPath; | ||
| final expectedSize = _expectedUploadSize; | ||
|
|
||
| if (fsManager == null || expectedPath == null || expectedSize == null) { | ||
| throw FilesystemUploadException('Upload verification state missing'); | ||
| } | ||
|
|
||
| final remoteSize = await fsManager.status(expectedPath); | ||
| if (remoteSize != expectedSize) { | ||
| throw FilesystemUploadException( | ||
| 'Uploaded file size mismatch for $expectedPath: ' | ||
| 'expected $expectedSize bytes, got $remoteSize bytes', | ||
| ); | ||
| } | ||
|
|
||
| _log('Verified $expectedPath ($remoteSize bytes)'); | ||
| await _cleanup(); | ||
| _updateState(FilesystemUploadState.completed(startedAt: startTime)); | ||
| } catch (e) { | ||
| _log('Upload verification failed: $e'); | ||
| _updateState( | ||
| FilesystemUploadState.failed(e.toString(), startedAt: startTime), | ||
| ); | ||
|
|
||
| try { | ||
| await _cleanup(); | ||
| } catch (cleanupError) { | ||
| _log('Cleanup after upload verification failed: $cleanupError'); | ||
| } | ||
| } |
There was a problem hiding this comment.
Don’t let verification overwrite a cancelled upload.
After OnUploadCompleted, the service still looks uploading while fsManager.status(...) runs. If cancel() happens in that window, _cleanup() clears the manager/path, but _completeUpload() can still publish completed or failed afterwards. That can flip a cancelled upload back to a terminal state and trigger downstream flows that listen for completion.
Guard the final _updateState(...) calls with an upload generation token, or bail out when the current upload has already been cancelled/replaced.
🤖 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 `@zswatch_app/lib/services/dfu/filesystem_upload_service.dart` around lines 248
- 283, The _completeUpload method can publish completed/failed after a
concurrent cancel() clears _fsManager/_expectedUploadPath, so capture a
short-lived upload token at the start of _completeUpload (or read a unique
_currentUploadId/_uploadGeneration) and re-check it before calling _updateState
or running _cleanup; specifically, read the generation/id at the top of
_completeUpload, use the same token for all async checks (status/size) and
before each terminal _updateState(FilesystemUploadState.completed/failed...)
bail out if the current upload id/generation no longer matches (meaning cancel()
or a new upload replaced it), ensuring _cleanup and state transitions only run
for the matching upload instance.
Comment on lines
+198
to
+202
| /// Stop any in-progress synthesis or speech. | ||
| Future<void> stop() async { | ||
| await _tts.stop(); | ||
| _statusController.add(TtsStatus.idle); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
When flutter_tts synthesizeToFile(...)is interrupted withstop(), are completion or error handlers guaranteed to fire on Android and iOS?
💡 Result:
No, completion and error handlers are not guaranteed to fire when flutter_tts synthesizeToFile(...) is interrupted with stop() on Android and iOS. The official documentation lists separate handlers including setCancelHandler alongside setCompletionHandler and setErrorHandler, suggesting distinct behavior for cancellation events like stop [1][2][3]. For speak(), issues confirm stop triggers completion unexpectedly [4], but for synthesizeToFile, native APIs indicate interruption: Android's TextToSpeech.stop interrupts synthesis to file and can return STOPPED via callbacks [5][6], while iOS AVSpeechSynthesizer.stopSpeaking prevents normal completion delegate calls [7]. The plugin likely propagates this via cancel handler rather than completion/error, though source code lacks explicit confirmation of guarantees [8]. Use setCancelHandler to handle interruptions reliably.
Citations:
- 1: https://pub.dev/documentation/flutter_tts/latest
- 2: https://pub.dev/packages/flutter_tts
- 3: https://github.com/dlutton/flutter_tts
- 4: tts.stop() without triggering completionHandler dlutton/flutter_tts#107
- 5: https://developer.android.com/reference/android/speech/tts/TextToSpeech
- 6: https://developer.android.com/reference/android/speech/tts/SynthesisCallback
- 7: https://developer.apple.com/documentation/avfaudio/avspeechsynthesizer
- 8: https://github.com/dlutton/flutter_tts/blob/master/lib/flutter_tts.dart
🏁 Script executed:
# Find and examine the TTS service implementation
find zswatch_app -name "tts_service.dart" -type fRepository: ZSWatch/ZSWatch-App
Length of output: 109
🏁 Script executed:
# Also check for watch_chat_service.dart to see the call context
find zswatch_app -name "watch_chat_service.dart" -type fRepository: ZSWatch/ZSWatch-App
Length of output: 117
🏁 Script executed:
# Examine tts_service.dart - get line count and critical sections
wc -l zswatch_app/lib/services/tts/tts_service.dartRepository: ZSWatch/ZSWatch-App
Length of output: 113
🏁 Script executed:
# Check the _synthesisCompleter initialization and the synthesizeToFile method
cat -n zswatch_app/lib/services/tts/tts_service.dart | head -150Repository: ZSWatch/ZSWatch-App
Length of output: 6416
🏁 Script executed:
# Check the stop() method and surrounding context (lines 190-210)
cat -n zswatch_app/lib/services/tts/tts_service.dart | sed -n '190,210p'Repository: ZSWatch/ZSWatch-App
Length of output: 607
🏁 Script executed:
# Check watch_chat_service.dart around the cited lines for stop() usage
cat -n zswatch_app/lib/services/chat/watch_chat_service.dart | sed -n '310,345p'Repository: ZSWatch/ZSWatch-App
Length of output: 1607
🏁 Script executed:
# Check if setCancelHandler is registered anywhere in the service
rg "setCancelHandler" zswatch_app/lib/services/tts/Repository: ZSWatch/ZSWatch-App
Length of output: 45
🏁 Script executed:
# Check the complete synthesizeToFile method to see full flow
cat -n zswatch_app/lib/services/tts/tts_service.dart | sed -n '115,196p'Repository: ZSWatch/ZSWatch-App
Length of output: 3561
Resolve pending synthesis in stop() to prevent 30-second hangs on cancellation.
synthesizeToFile() blocks awaiting _synthesisCompleter.future.timeout(30s), but stop() only calls _tts.stop() without resolving the completer. When flutter_tts synthesizeToFile() is interrupted, native callbacks (setCompletionHandler / setErrorHandler) are not guaranteed to fire on Android or iOS—the plugin has a separate setCancelHandler for interruptions. This means a cancelled synthesis can hang for the full 30-second timeout while awaiting completion that never comes.
Resolve and clear _synthesisCompleter inside stop():
Fix
Future<void> stop() async {
+ _synthesisCompleter?.complete();
+ _synthesisCompleter = null;
await _tts.stop();
_statusController.add(TtsStatus.idle);
}🤖 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 `@zswatch_app/lib/services/tts/tts_service.dart` around lines 198 - 202, The
stop() method currently calls _tts.stop() but does not resolve the in-progress
synthesis future, so synthesizeToFile() can hang; inside stop() detect if
_synthesisCompleter exists and is not completed, complete it (e.g.,
completeError with a cancellation error or complete with a cancellation value
consistent with synthesizeToFile()'s contract), then null/clear
_synthesisCompleter and finally add TtsStatus.idle to _statusController; ensure
you reference the existing stop(), _tts.stop(), _synthesisCompleter, and
_statusController.add(TtsStatus.idle) symbols when making the change.
Comment on lines
+487
to
+502
| if (nusService == null) { | ||
| LifecycleLogger.log('WatchService', '_setupNus missing service'); | ||
| return; | ||
| } | ||
|
|
||
| final rxChar = _findCharacteristic( | ||
| nusService, | ||
| _guid(NusUuids.rxCharacteristic), | ||
| ); | ||
| if (rxChar == null) return; | ||
| if (rxChar == null) { | ||
| LifecycleLogger.log( | ||
| 'WatchService', | ||
| '_setupNus missing RX characteristic', | ||
| ); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Do not treat a missing NUS channel as a successful setup.
These early returns let _onSetupRequired() keep going and later log completion, but every protocol send will then just no-op because _sendNus() cannot find the service/characteristic. Surface this as a setup failure so reconnect/error handling can run instead of leaving the app in a zombie “connected” state.
Suggested fix
final nusService = _findServiceIn(services, _guid(NusUuids.service));
if (nusService == null) {
- LifecycleLogger.log('WatchService', '_setupNus missing service');
- return;
+ LifecycleLogger.log('WatchService', '_setupNus missing service');
+ throw StateError('Missing NUS service');
}
@@
if (rxChar == null) {
LifecycleLogger.log(
'WatchService',
'_setupNus missing RX characteristic',
);
- return;
+ throw StateError('Missing NUS RX characteristic');
}🤖 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 `@zswatch_app/lib/services/watch_service.dart` around lines 487 - 502, The
early returns in _setupNus when nusService or rxChar are missing should not
silently return: replace those returns with code that surfaces a setup failure
so the connection enters the error/reconnect path (i.e. invoke your
setup-failure handler instead of returning). Specifically, in _setupNus (when
nusService == null or rxChar == null) call the existing setup-failure flow used
by _onSetupRequired() (for example invoke the module's
_onSetupFailed/_handleSetupFailure/other failure handler) and ensure _sendNus()
will no longer be treated as usable; this makes the missing NUS
service/characteristic a real setup failure and triggers reconnect/error
handling.
Comment on lines
+94
to
+101
| onStatusChanged: (status) { | ||
| // If the controller returns to ready while _isCropping is | ||
| // set (e.g. internal crop failure), reset the button state. | ||
| if (status == CropStatus.ready && _isCropping) { | ||
| if (!mounted) return; | ||
| setState(() => _isCropping = false); | ||
| } | ||
| }, |
There was a problem hiding this comment.
onStatusChanged resets the button silently on internal crop failure.
When an internal crop failure causes CropStatus.ready while _isCropping is true, the "Done" button simply re-enables with no indication to the user that cropping failed. At minimum, a SnackBar or debugPrint should surface the failure.
💬 Proposed fix — add user feedback
onStatusChanged: (status) {
if (status == CropStatus.ready && _isCropping) {
if (!mounted) return;
setState(() => _isCropping = false);
+ debugPrint('[WatchfaceCropScreen] Crop returned to ready unexpectedly');
+ ScaffoldMessenger.of(context).showSnackBar(
+ const SnackBar(content: Text('Cropping failed, please try again.')),
+ );
}
},Note: once the
onCroppedfix above is in place andCropResult.erroris handled there, this branch may no longer trigger — but keeping it as a defensive backstop is still worthwhile.
📝 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
| onStatusChanged: (status) { | |
| // If the controller returns to ready while _isCropping is | |
| // set (e.g. internal crop failure), reset the button state. | |
| if (status == CropStatus.ready && _isCropping) { | |
| if (!mounted) return; | |
| setState(() => _isCropping = false); | |
| } | |
| }, | |
| onStatusChanged: (status) { | |
| // If the controller returns to ready while _isCropping is | |
| // set (e.g. internal crop failure), reset the button state. | |
| if (status == CropStatus.ready && _isCropping) { | |
| if (!mounted) return; | |
| setState(() => _isCropping = false); | |
| debugPrint('[WatchfaceCropScreen] Crop returned to ready unexpectedly'); | |
| ScaffoldMessenger.of(context).showSnackBar( | |
| const SnackBar(content: Text('Cropping failed, please try again.')), | |
| ); | |
| } | |
| }, |
🤖 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 `@zswatch_app/lib/ui/screens/watchface/watchface_crop_screen.dart` around lines
94 - 101, The onStatusChanged callback for the crop controller currently resets
_isCropping to false when status == CropStatus.ready without surfacing an error;
update the branch inside onStatusChanged to also notify the user or developer
when this reset happens by emitting a visible message (e.g., show a SnackBar via
ScaffoldMessenger.of(context).showSnackBar(...) or call debugPrint) indicating
the crop failed, while still guarding with if (!mounted) return and setState(()
=> _isCropping = false); reference the onStatusChanged closure,
CropStatus.ready, and the _isCropping state so you add the feedback next to the
existing setState call.
There was a problem hiding this comment.
Actionable comments posted: 14
♻️ Duplicate comments (3)
zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/MainActivity.kt (1)
1204-1217:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPast concern still applies:
liteRtLmBridgeis not disposed inonDestroy.
liteRtLmBridge(initialized at line 147) holds native LLM model/session state but is never released when the activity is torn down. SinceMainActivitycan be recreated (configuration changes, process re-attach), the previous bridge will leak until process death.mediaBridgeis correctly disposed here — apply the same pattern toliteRtLmBridge.🛠 Proposed fix
if (foregroundServiceBound) { unbindService(foregroundServiceConnection) foregroundServiceBound = false } + liteRtLmBridge?.dispose() + liteRtLmBridge = null mediaBridge?.dispose() mediaBridge = null NotificationListenerServiceImpl.notificationCallback = null super.onDestroy()(Confirm
LiteRtLmBridgeexposes adispose()/ cleanup method; if named differently, swap accordingly.)#!/bin/bash # Verify LiteRtLmBridge has a dispose/teardown method and what it's named. fd -t f 'LiteRtLmBridge.kt' --exec cat {}🤖 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 `@zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/MainActivity.kt` around lines 1204 - 1217, MainActivity.onDestroy currently disposes mediaBridge but never releases liteRtLmBridge, causing native LLM resources to leak; update onDestroy to check liteRtLmBridge != null and call its cleanup API (e.g., dispose(), close(), or teardown()—use the actual method on LiteRtLmBridge) then set liteRtLmBridge = null, mirroring the mediaBridge disposal pattern to ensure native model/session state is released when the activity is destroyed.zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/BootReceiver.kt (1)
10-11:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPast concern still applies: synchronous disk I/O on the main thread inside
onReceive.
LifecycleLogger.recordHistoricalExitReasons()performsSharedPreferences.commit()(seeLifecycleLogger.ktline 215), which blocks the main thread inside a BroadcastReceiver's ~10s ANR window.BleConnectionForegroundService.onCreate()already calls bothinitialize()andrecordHistoricalExitReasons()shortly after, so this call here is redundant.🛠 Proposed fix
override fun onReceive(context: Context, intent: Intent?) { LifecycleLogger.initialize(context) - LifecycleLogger.recordHistoricalExitReasons(context) val action = intent?.action🤖 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 `@zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/BootReceiver.kt` around lines 10 - 11, Remove the synchronous disk I/O call by deleting the call to LifecycleLogger.recordHistoricalExitReasons() from BootReceiver.onReceive and rely on the existing invocation inside BleConnectionForegroundService.onCreate(); the issue is that LifecycleLogger.recordHistoricalExitReasons() uses SharedPreferences.commit() (sync), so keep LifecycleLogger.initialize(context) in BootReceiver if needed but remove the redundant recordHistoricalExitReasons() call (or alternatively replace that call with an async approach inside LifecycleLogger, e.g., use apply() or perform I/O off the main thread) to avoid blocking the BroadcastReceiver.zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/LifecycleLogger.kt (1)
105-112:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPast concern still applies: notification post/remove diagnostics are dropped by this filter.
NotificationListenerServiceImplnow emits messages such asdrop posted notification …,forward posted notification …,drop removed notification …, andforward removed notification …(seeNotificationListenerServiceImpl.ktlines 456-465 and 516-526). None of these containoncreate/ondestroy/onlowmemory/ontrimmemory/listenerconnected/listenerdisconnected, so they are silently filtered out and never persisted — only emitted viaLog.d.If these diagnostics are intended to be persisted, broaden this branch (or route them under a dedicated source name that already passes
shouldPersist). If they are intentionally Log.d-only, consider dropping the redundantLifecycleLogger.logcalls at the post/remove sites to avoid the implicit "logged but not persisted" footgun.🛠 Suggested predicate update
if (lowerSource.contains("notificationservice")) { return lowerMessage.contains("oncreate") || lowerMessage.contains("ondestroy") || lowerMessage.contains("onlowmemory") || lowerMessage.contains("ontrimmemory") || lowerMessage.contains("listenerconnected") || - lowerMessage.contains("listenerdisconnected") + lowerMessage.contains("listenerdisconnected") || + lowerMessage.contains("drop posted") || + lowerMessage.contains("forward posted") || + lowerMessage.contains("drop removed") || + lowerMessage.contains("forward removed") }🤖 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 `@zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/LifecycleLogger.kt` around lines 105 - 112, The current filter in LifecycleLogger (the branch testing lowerSource.contains("notificationservice")) excludes NotificationListenerServiceImpl's "drop/forward posted/removed notification" messages so they never persist; update that predicate in LifecycleLogger.kt to also allow message substrings like "posted", "removed", "drop", and "forward" (or specifically "posted notification" / "removed notification") so these diagnostics pass shouldPersist, or alternatively route those logs from NotificationListenerServiceImpl into a dedicated source name that already persists; if those notifications are intentionally transient, remove the redundant LifecycleLogger.log calls in NotificationListenerServiceImpl to avoid logging-but-not-persisting confusion.
🧹 Nitpick comments (2)
zswatch_app/lib/providers/dfu_providers.dart (1)
223-235: 💤 Low valueConsider polling
rediscoverServicesinstead of a fixed 2s wait.Every DFU, FS upload, and combined update path now pays a flat 2-second penalty before any work begins, even when the watch registers the SMP transport in a few hundred ms. A short poll loop with a total timeout would shorten the happy path while also being more tolerant of slower watches/firmwares (where 2s is occasionally not enough and the user gets the misleading "older firmware" error message).
♻️ Sketch of a poll-based wait
debugPrint('[DfuNotifier] Ensuring SMP is enabled...'); await watchService.enableSmp(); - await Future<void>.delayed(const Duration(seconds: 2)); - final ready = await watchService.rediscoverServices(); - if (!ready) { + bool ready = false; + final deadline = DateTime.now().add(const Duration(seconds: 5)); + while (DateTime.now().isBefore(deadline)) { + await Future<void>.delayed(const Duration(milliseconds: 300)); + ready = await watchService.rediscoverServices(); + if (ready) break; + } + if (!ready) { throw Exception( 'SMP service did not become available. ' 'If your watch firmware is older, you may need to manually enable it: ' 'on the watch go to Apps → Update → Enable FW Update.', ); }🤖 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 `@zswatch_app/lib/providers/dfu_providers.dart` around lines 223 - 235, Replace the fixed 2s delay in the SMP enable sequence with a poll loop: after await watchService.enableSmp() call rediscoverServices() repeatedly at a short interval (e.g., 200–500ms) until it returns true or a total timeout elapses (e.g., 5–10s); if rediscoverServices() becomes true proceed to _ref.invalidate(hasSmpServiceProvider) and the debugPrint, otherwise throw the same exception on timeout. Implement the loop around the existing watchService.rediscoverServices() call inside the same method that calls watchService.enableSmp() (the DfuNotifier/enableSmp flow shown) and ensure you await between polls so you don’t busy-wait.zswatch_app/lib/app.dart (1)
42-46: ⚡ Quick winUse the state class name in these debug logs.
[app]breaks the repo's logging format, which makes startup/lifecycle traces harder to grep consistently.Suggested tweak
- debugPrint('[app] watchChatServiceProvider initialized in initState'); + debugPrint( + '[_ZSWatchAppState] watchChatServiceProvider initialized in initState', + ); } catch (e, st) { - debugPrint('[app] early watchChatServiceProvider init failed: $e\n$st'); + debugPrint( + '[_ZSWatchAppState] early watchChatServiceProvider init failed: $e\n$st', + ); }As per coding guidelines, "Use
debugPrint('[ClassName] message')with class name prefix for logging — do NOT useprint()."🤖 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 `@zswatch_app/lib/app.dart` around lines 42 - 46, The logs in initState use a hardcoded '[app]' tag which violates the repo logging convention; update both debugPrint calls in the initState that reference watchChatServiceProvider to use this State class's actual name as the prefix (e.g., debugPrint('[<YourStateClassName>] ...')) instead of '[app]'; ensure the same replacement is applied to both the success log and the catch log (the try block that reads watchChatServiceProvider and the catch block that prints the error/stack).
🤖 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 `@ai_testbench/bin/test_time_extraction.dart`:
- Around line 307-309: The cleanup currently only removes closed think blocks
for the variable raw in the replaceAll call; update the regex used when cleaning
raw (the replaceAll on raw) to also match unclosed <think> blocks by allowing
the match to end at either the closing tag or the end of the string (e.g.,
change the pattern to accept (</think>|$) with dotAll enabled), so any '<think>'
without a corresponding '</think>' is removed as well.
- Around line 312-314: The log printing of token/s in the print call that uses
secs and tokenCount can divide by zero when secs is zero; update the expression
used in that print (the string interpolation in the print call) to guard the
division by checking secs > 0 and computing a safe rate (e.g., compute rate =
secs > 0 ? tokenCount / secs : 0) and use that rate in the (~${...} tok/s)
portion so you never produce Infinity/NaN while keeping the rest of the
formatting (toStringAsFixed) intact.
In `@ai_testbench/lib/correction_main.dart`:
- Around line 24-35: The code ignores the documented --model flag and will crash
if --model-dir does not exist; update the logic around readValue('--model') and
readValue('--model-dir') so: if readValue('--model') is provided use that path
(validate the file exists and endsWith('.gguf')); otherwise take modelDir (from
readValue('--model-dir') or 'models'), check Directory(modelDir).existsSync()
and isDirectory before calling listSync(), emit a friendly error via outputPath
handling function (or throw with clear message) if the directory is missing or
empty, and keep the existing filtering/sorting of .gguf paths (reference
symbols: readValue, modelDir, outputPath, and the Directory(...).listSync()
block) so single-file flow works and missing directories are handled gracefully.
In `@ai_testbench/lib/router_benchmark_main.dart`:
- Around line 147-153: The reported "Overhead" mixes different case sets —
avgRouterMs is computed from routerTimes for all cases while avgTotalMs and
avgSingleMs use timerCases and five voice_memo cases respectively; update the
measurement logic (references: avgRouterMs, avgTotalMs, avgSingleMs,
routerTimes, timerCases, voice_memo) to compute comparisons over the same
transcript subset: either (a) track per-case timings keyed by case id when
recording routerTimes/totalTimes/singleTimes and then compute
avgRouterMs/avgTotalMs/avgSingleMs by filtering that map to the identical set of
case ids, or (b) restrict routerTimes to only the timerCases (or to the
voice_memo cases) before averaging; ensure the overhead calculation uses
matching averaged sets so it becomes a valid apples-to-apples comparison.
- Around line 180-189: The router result is currently timed but not used to gate
Stage 2, so extraction and extractionCorrect are counted even when route !=
'timer_alarm'; update the flow around routerResult (from
llm.generate(routerPrompt)) to parse routerResult.output JSON and only run the
extraction routine (the code that parses timer/alarm intents and updates
extractionCorrect) when route == 'timer_alarm' (apply same gating for the other
block at 198-233). Additionally, change the extractionCorrect check so it
validates the parsed structured output against the expected structured values
(e.g., expected duration or alarm time) instead of only checking intent
tokens—compare the extracted fields to the expected fields and only increment
extractionCorrect on exact match.
- Around line 20-32: The code calls Directory(modelDir).listSync() directly when
computing modelPaths, which throws if modelDir doesn't exist; update the logic
in the modelPaths construction (the expression that uses
Directory(modelDir).listSync()) to first check existence (e.g.,
Directory(modelDir).existsSync()) or wrap listSync() in a try/catch and treat
missing/non-readable directory as an empty list so the downstream "No .gguf
models found" path runs; reference the modelDir variable and the modelFilter
logic so the filtering/sorting of .gguf files (the modelPaths computation)
remains the same when the directory is absent.
In `@ai_testbench/lib/screens/testbench_screen.dart`:
- Around line 247-254: Remove the raw model-output preview log to avoid leaking
user content: stop printing _streamBuffer contents in the Testbench stream
completion path (the debugPrint that references _streamBuffer.substring(...)
inside the stream done block). Instead log only metadata (event count, buffer
length, elapsed ms) or, if absolutely needed for dev debugging, gate any content
preview behind a strict debug-only and redaction path (e.g., ensure it runs only
in non-production builds and redacts sensitive substrings before logging).
Update the block that references _streamBuffer to eliminate direct content
output and keep only non-sensitive metadata.
- Line 253: The clamp calls return num but are used where int/double are
required; update the two call sites to convert the clamped num to the correct
numeric type: for the substring call using _streamBuffer.substring(0,
_streamBuffer.length.clamp(0, 300)), append .toInt() to the clamp result so
substring receives an int, and for the progress indicator using
LinearProgressIndicator(value: fraction.clamp(0, 1)), append .toDouble() (or
wrap with ( ... as num).toDouble()) so value receives a double; adjust the
expressions where _streamBuffer and fraction are used accordingly.
In `@ai_testbench/lib/services/correction_benchmark_service.dart`:
- Around line 595-598: The current assignment of modificationMatch forces true
when testCase.expectModification is false, letting unintended rewrites pass;
change the logic so modificationMatch reflects whether a modification actually
matches the expectation: set modificationMatch to wasModified when
testCase.expectModification is true, and to !wasModified when
testCase.expectModification is false (i.e., require no change for clean-input
cases). Update the code around the modificationMatch variable (referencing
modificationMatch, testCase.expectModification, and wasModified) accordingly.
In `@ai_testbench/lib/services/llm_service.dart`:
- Around line 202-214: fllamaChat's returned Future isn't handling startup
failures so when it throws the stream remains open and
_requestInFlight/_runningRequestId stay stale; in generateStream() attach an
error handler (e.g. catchError or onError in the Future chain) to the fllamaChat
call that: sets _requestInFlight = false, sets _runningRequestId = -1, and
either calls controller.addError(error) and/or controller.close() (checking
controller.isClosed first) so the stream is errored/closed on startup failure
and state is reset for next requests.
In `@ai_testbench/lib/services/model_benchmark_service.dart`:
- Around line 1303-1306: The final progress metadata is using benchmarkCases for
currentCaseName/currentCaseIndex which is wrong when selectedCases/casesToRun
differ; update the metadata to derive current case info from the executed case
list (casesToRun) instead of benchmarkCases/totalCases: use casesToRun.isEmpty ?
'' : casesToRun.last.name for currentCaseName and currentCaseIndex:
casesToRun.isEmpty ? 0 : casesToRun.length - 1 (keep
currentModelIndex/currentModelPath logic as-is using modelPaths). Ensure you
reference the variables currentCaseName and currentCaseIndex when making the
change so the progress reflects the actual cases run.
- Around line 1115-1118: The current validJson check in
model_benchmark_service.dart that relies on _containsEmptyJsonArray is too
permissive; change the logic to parse result.output as JSON and validate the
top-level structure: in the branch where testCase.expectedCount == 0, parse
result.output and confirm it is a JSON array of length 0 (not just containing
"[]"); otherwise ensure the parsed JSON is a non-empty array or object as
appropriate. Add strict schema validation for items using the chrono_ai_flow
fields (intent, title, datetime) — e.g., implement or call a validator function
(new or existing) to assert presence/types for intent, title and a parsable
datetime — and replace the _containsEmptyJsonArray heuristic with this
parsing+validation in the code that computes validJson.
In `@zswatch_app/lib/app.dart`:
- Around line 54-58: The unawaited call from didChangeAppLifecycleState to
_syncForegroundServiceLifecycleState can throw uncaught async errors; wrap the
body of _syncForegroundServiceLifecycleState (and the other lifecycle-sync
helper used later) in a local try/catch so failures are handled best-effort:
catch any exception coming from resolving foregroundServiceProvider or calling
updateNotification() and log it (e.g., via LifecycleLogger.error) but do not
rethrow. Ensure you reference and protect the same symbols:
_syncForegroundServiceLifecycleState, foregroundServiceProvider, and
updateNotification (and mirror the same try/catch pattern for the other
lifecycle-sync function mentioned later).
- Around line 92-94: The call to
ref.read(nativeBackgroundPreferencesSyncProvider) can throw and currently lives
inside the broad try that wraps _initializeBle(), which lets it short-circuit
subsequent startup steps (watchInfoPersistenceProvider,
voiceMemoSyncServiceProvider, analytics, crash persistence); isolate this by
moving the nativeBackgroundPreferencesSyncProvider init into its own try/catch
block (or surrounding only that ref.read call with try/catch) and catch/log the
error (do not rethrow) so failures in nativeBackgroundPreferencesSyncProvider do
not prevent initializing _initializeBle(), watchInfoPersistenceProvider,
voiceMemoSyncServiceProvider, analytics, or crash persistence.
---
Duplicate comments:
In `@zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/BootReceiver.kt`:
- Around line 10-11: Remove the synchronous disk I/O call by deleting the call
to LifecycleLogger.recordHistoricalExitReasons() from BootReceiver.onReceive and
rely on the existing invocation inside
BleConnectionForegroundService.onCreate(); the issue is that
LifecycleLogger.recordHistoricalExitReasons() uses SharedPreferences.commit()
(sync), so keep LifecycleLogger.initialize(context) in BootReceiver if needed
but remove the redundant recordHistoricalExitReasons() call (or alternatively
replace that call with an async approach inside LifecycleLogger, e.g., use
apply() or perform I/O off the main thread) to avoid blocking the
BroadcastReceiver.
In `@zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/LifecycleLogger.kt`:
- Around line 105-112: The current filter in LifecycleLogger (the branch testing
lowerSource.contains("notificationservice")) excludes
NotificationListenerServiceImpl's "drop/forward posted/removed notification"
messages so they never persist; update that predicate in LifecycleLogger.kt to
also allow message substrings like "posted", "removed", "drop", and "forward"
(or specifically "posted notification" / "removed notification") so these
diagnostics pass shouldPersist, or alternatively route those logs from
NotificationListenerServiceImpl into a dedicated source name that already
persists; if those notifications are intentionally transient, remove the
redundant LifecycleLogger.log calls in NotificationListenerServiceImpl to avoid
logging-but-not-persisting confusion.
In `@zswatch_app/android/app/src/main/kotlin/dev/zswatch/app/MainActivity.kt`:
- Around line 1204-1217: MainActivity.onDestroy currently disposes mediaBridge
but never releases liteRtLmBridge, causing native LLM resources to leak; update
onDestroy to check liteRtLmBridge != null and call its cleanup API (e.g.,
dispose(), close(), or teardown()—use the actual method on LiteRtLmBridge) then
set liteRtLmBridge = null, mirroring the mediaBridge disposal pattern to ensure
native model/session state is released when the activity is destroyed.
---
Nitpick comments:
In `@zswatch_app/lib/app.dart`:
- Around line 42-46: The logs in initState use a hardcoded '[app]' tag which
violates the repo logging convention; update both debugPrint calls in the
initState that reference watchChatServiceProvider to use this State class's
actual name as the prefix (e.g., debugPrint('[<YourStateClassName>] ...'))
instead of '[app]'; ensure the same replacement is applied to both the success
log and the catch log (the try block that reads watchChatServiceProvider and the
catch block that prints the error/stack).
In `@zswatch_app/lib/providers/dfu_providers.dart`:
- Around line 223-235: Replace the fixed 2s delay in the SMP enable sequence
with a poll loop: after await watchService.enableSmp() call rediscoverServices()
repeatedly at a short interval (e.g., 200–500ms) until it returns true or a
total timeout elapses (e.g., 5–10s); if rediscoverServices() becomes true
proceed to _ref.invalidate(hasSmpServiceProvider) and the debugPrint, otherwise
throw the same exception on timeout. Implement the loop around the existing
watchService.rediscoverServices() call inside the same method that calls
watchService.enableSmp() (the DfuNotifier/enableSmp flow shown) and ensure you
await between polls so you don’t busy-wait.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f8f0fd12-65fd-4464-bf61-35c74f4e957c
📒 Files selected for processing (22)
ai_testbench/bin/test_time_extraction.dartai_testbench/lib/benchmark_main.dartai_testbench/lib/correction_main.dartai_testbench/lib/main.dartai_testbench/lib/router_benchmark_main.dartai_testbench/lib/screens/testbench_screen.dartai_testbench/lib/screens/time_extraction_screen.dartai_testbench/lib/services/correction_benchmark_service.dartai_testbench/lib/services/llm_service.dartai_testbench/lib/services/model_benchmark_service.dartai_testbench/lib/services/time_extraction_benchmark_service.dartai_testbench/lib/time_extraction_main.dartai_testbench/lib/widgets/memo_card.dartpackages/chrono_ai_flow/scripts/time_resolver_debug_manual.dartpackages/chrono_ai_flow/test/parser_test.dartzswatch_app/android/app/src/main/kotlin/dev/zswatch/app/BleConnectionForegroundService.ktzswatch_app/android/app/src/main/kotlin/dev/zswatch/app/BootReceiver.ktzswatch_app/android/app/src/main/kotlin/dev/zswatch/app/LifecycleLogger.ktzswatch_app/android/app/src/main/kotlin/dev/zswatch/app/MainActivity.ktzswatch_app/android/app/src/main/kotlin/dev/zswatch/app/NotificationListenerServiceImpl.ktzswatch_app/lib/app.dartzswatch_app/lib/providers/dfu_providers.dart
✅ Files skipped from review due to trivial changes (1)
- packages/chrono_ai_flow/test/parser_test.dart
Comment on lines
+307
to
+309
| raw = raw | ||
| .replaceAll(RegExp(r'<think>.*?</think>', dotAll: true), '') | ||
| .trim(); |
There was a problem hiding this comment.
Strip unclosed <think> blocks too.
Current cleanup only removes closed think blocks. Unclosed blocks can still poison JSON extraction.
Suggested fix
raw = raw
.replaceAll(RegExp(r'<think>.*?</think>', dotAll: true), '')
+ .replaceAll(RegExp(r'<think>.*', dotAll: true), '')
.trim();📝 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
| raw = raw | |
| .replaceAll(RegExp(r'<think>.*?</think>', dotAll: true), '') | |
| .trim(); | |
| raw = raw | |
| .replaceAll(RegExp(r'<think>.*?</think>', dotAll: true), '') | |
| .replaceAll(RegExp(r'<think>.*', dotAll: true), '') | |
| .trim(); |
🤖 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 `@ai_testbench/bin/test_time_extraction.dart` around lines 307 - 309, The
cleanup currently only removes closed think blocks for the variable raw in the
replaceAll call; update the regex used when cleaning raw (the replaceAll on raw)
to also match unclosed <think> blocks by allowing the match to end at either the
closing tag or the end of the string (e.g., change the pattern to accept
(</think>|$) with dotAll enabled), so any '<think>' without a corresponding
'</think>' is removed as well.
Comment on lines
+312
to
+314
| print( | ||
| ' LLM time: ${secs.toStringAsFixed(2)}s (~${(tokenCount / secs).toStringAsFixed(1)} tok/s)', | ||
| ); |
There was a problem hiding this comment.
Guard tok/s calculation against zero elapsed time.
Line 313 can divide by zero when generation finishes within the same millisecond, producing Infinity/NaN in logs.
Suggested fix
- final secs = genSw.elapsed.inMilliseconds / 1000;
+ final secs = genSw.elapsed.inMilliseconds / 1000;
+ final tokPerSec = secs > 0 ? (tokenCount / secs) : 0.0;
print(
- ' LLM time: ${secs.toStringAsFixed(2)}s (~${(tokenCount / secs).toStringAsFixed(1)} tok/s)',
+ ' LLM time: ${secs.toStringAsFixed(2)}s (~${tokPerSec.toStringAsFixed(1)} tok/s)',
);🤖 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 `@ai_testbench/bin/test_time_extraction.dart` around lines 312 - 314, The log
printing of token/s in the print call that uses secs and tokenCount can divide
by zero when secs is zero; update the expression used in that print (the string
interpolation in the print call) to guard the division by checking secs > 0 and
computing a safe rate (e.g., compute rate = secs > 0 ? tokenCount / secs : 0)
and use that rate in the (~${...} tok/s) portion so you never produce
Infinity/NaN while keeping the rest of the formatting (toStringAsFixed) intact.
Comment on lines
+24
to
+35
| final modelDir = | ||
| readValue('--model-dir') ?? Directory('models').absolute.path; | ||
| final outputPath = readValue('--output'); | ||
|
|
||
| final modelPaths = Directory(modelDir) | ||
| .listSync() | ||
| .whereType<File>() | ||
| .map((f) => f.path) | ||
| .where((p) => p.toLowerCase().endsWith('.gguf')) | ||
| .toList() | ||
| ..sort(); | ||
| final modelPaths = | ||
| Directory(modelDir) | ||
| .listSync() | ||
| .whereType<File>() | ||
| .map((f) => f.path) | ||
| .where((p) => p.toLowerCase().endsWith('.gguf')) | ||
| .toList() | ||
| ..sort(); |
There was a problem hiding this comment.
--model is documented but ignored, and invalid --model-dir can crash
The code only supports directory scanning, so the documented single-model flow won’t work. Also, listSync() on a missing directory throws before you can print a friendly error.
Suggested fix
- final modelDir =
- readValue('--model-dir') ?? Directory('models').absolute.path;
+ final singleModel = readValue('--model');
+ final modelDir =
+ readValue('--model-dir') ?? Directory('models').absolute.path;
final outputPath = readValue('--output');
- final modelPaths =
- Directory(modelDir)
- .listSync()
- .whereType<File>()
- .map((f) => f.path)
- .where((p) => p.toLowerCase().endsWith('.gguf'))
- .toList()
- ..sort();
+ late final List<String> modelPaths;
+ if (singleModel != null) {
+ modelPaths = [File(singleModel).absolute.path];
+ } else {
+ final dir = Directory(modelDir);
+ if (!dir.existsSync()) {
+ stdout.writeln('[CorrectionBench] Model directory not found: $modelDir');
+ exitCode = 1;
+ return;
+ }
+ modelPaths = dir
+ .listSync()
+ .whereType<File>()
+ .map((f) => f.path)
+ .where((p) => p.toLowerCase().endsWith('.gguf'))
+ .toList()
+ ..sort();
+ }📝 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
| final modelDir = | |
| readValue('--model-dir') ?? Directory('models').absolute.path; | |
| final outputPath = readValue('--output'); | |
| final modelPaths = Directory(modelDir) | |
| .listSync() | |
| .whereType<File>() | |
| .map((f) => f.path) | |
| .where((p) => p.toLowerCase().endsWith('.gguf')) | |
| .toList() | |
| ..sort(); | |
| final modelPaths = | |
| Directory(modelDir) | |
| .listSync() | |
| .whereType<File>() | |
| .map((f) => f.path) | |
| .where((p) => p.toLowerCase().endsWith('.gguf')) | |
| .toList() | |
| ..sort(); | |
| final singleModel = readValue('--model'); | |
| final modelDir = | |
| readValue('--model-dir') ?? Directory('models').absolute.path; | |
| final outputPath = readValue('--output'); | |
| late final List<String> modelPaths; | |
| if (singleModel != null) { | |
| modelPaths = [File(singleModel).absolute.path]; | |
| } else { | |
| final dir = Directory(modelDir); | |
| if (!dir.existsSync()) { | |
| stdout.writeln('[CorrectionBench] Model directory not found: $modelDir'); | |
| exitCode = 1; | |
| return; | |
| } | |
| modelPaths = dir | |
| .listSync() | |
| .whereType<File>() | |
| .map((f) => f.path) | |
| .where((p) => p.toLowerCase().endsWith('.gguf')) | |
| .toList() | |
| ..sort(); | |
| } |
🤖 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 `@ai_testbench/lib/correction_main.dart` around lines 24 - 35, The code ignores
the documented --model flag and will crash if --model-dir does not exist; update
the logic around readValue('--model') and readValue('--model-dir') so: if
readValue('--model') is provided use that path (validate the file exists and
endsWith('.gguf')); otherwise take modelDir (from readValue('--model-dir') or
'models'), check Directory(modelDir).existsSync() and isDirectory before calling
listSync(), emit a friendly error via outputPath handling function (or throw
with clear message) if the directory is missing or empty, and keep the existing
filtering/sorting of .gguf paths (reference symbols: readValue, modelDir,
outputPath, and the Directory(...).listSync() block) so single-file flow works
and missing directories are handled gracefully.
Comment on lines
+20
to
+32
| final modelPaths = | ||
| Directory(modelDir) | ||
| .listSync() | ||
| .whereType<File>() | ||
| .map((f) => f.path) | ||
| .where((p) => p.toLowerCase().endsWith('.gguf')) | ||
| .where( | ||
| (p) => | ||
| modelFilter == null || | ||
| p.toLowerCase().contains(modelFilter.toLowerCase()), | ||
| ) | ||
| .toList() | ||
| ..sort(); |
There was a problem hiding this comment.
Handle a missing --model-dir before calling listSync().
Directory(modelDir).listSync() throws if the directory does not exist, so this benchmark crashes with a FileSystemException instead of reaching the friendly "No .gguf models found" path below.
Suggested fix
- final modelPaths =
- Directory(modelDir)
+ final modelRoot = Directory(modelDir);
+ if (!modelRoot.existsSync()) {
+ stdout.writeln('[RouterBench] Model directory not found: $modelDir');
+ exitCode = 1;
+ return;
+ }
+
+ final modelPaths =
+ modelRoot
.listSync()
.whereType<File>()
.map((f) => f.path)📝 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
| final modelPaths = | |
| Directory(modelDir) | |
| .listSync() | |
| .whereType<File>() | |
| .map((f) => f.path) | |
| .where((p) => p.toLowerCase().endsWith('.gguf')) | |
| .where( | |
| (p) => | |
| modelFilter == null || | |
| p.toLowerCase().contains(modelFilter.toLowerCase()), | |
| ) | |
| .toList() | |
| ..sort(); | |
| final modelRoot = Directory(modelDir); | |
| if (!modelRoot.existsSync()) { | |
| stdout.writeln('[RouterBench] Model directory not found: $modelDir'); | |
| exitCode = 1; | |
| return; | |
| } | |
| final modelPaths = | |
| modelRoot | |
| .listSync() | |
| .whereType<File>() | |
| .map((f) => f.path) | |
| .where((p) => p.toLowerCase().endsWith('.gguf')) | |
| .where( | |
| (p) => | |
| modelFilter == null || | |
| p.toLowerCase().contains(modelFilter.toLowerCase()), | |
| ) | |
| .toList() | |
| ..sort(); |
🤖 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 `@ai_testbench/lib/router_benchmark_main.dart` around lines 20 - 32, The code
calls Directory(modelDir).listSync() directly when computing modelPaths, which
throws if modelDir doesn't exist; update the logic in the modelPaths
construction (the expression that uses Directory(modelDir).listSync()) to first
check existence (e.g., Directory(modelDir).existsSync()) or wrap listSync() in a
try/catch and treat missing/non-readable directory as an empty list so the
downstream "No .gguf models found" path runs; reference the modelDir variable
and the modelFilter logic so the filtering/sorting of .gguf files (the
modelPaths computation) remains the same when the directory is absent.
Comment on lines
147
to
153
| final avgRouterMs = | ||
| routerTimes.fold<int>(0, (s, d) => s + d.inMilliseconds) ~/ | ||
| routerTimes.length; | ||
| routerTimes.length; | ||
| stdout.writeln( | ||
| '\n[RouterBench] Router accuracy: $routerCorrect/${cases.length}'); | ||
| '\n[RouterBench] Router accuracy: $routerCorrect/${cases.length}', | ||
| ); | ||
| stdout.writeln('[RouterBench] Router avg latency: ${avgRouterMs}ms'); |
There was a problem hiding this comment.
These latency deltas mix different case sets.
avgRouterMs is averaged over all router cases, avgTotalMs only over timerCases, and avgSingleMs only over five voice_memo cases. So avgTotalMs - avgRouterMs and the reported "Overhead" are apples-to-oranges numbers. Collect the compared timings from the same transcripts before deriving extract-only or overhead metrics.
Also applies to: 228-285
🤖 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 `@ai_testbench/lib/router_benchmark_main.dart` around lines 147 - 153, The
reported "Overhead" mixes different case sets — avgRouterMs is computed from
routerTimes for all cases while avgTotalMs and avgSingleMs use timerCases and
five voice_memo cases respectively; update the measurement logic (references:
avgRouterMs, avgTotalMs, avgSingleMs, routerTimes, timerCases, voice_memo) to
compute comparisons over the same transcript subset: either (a) track per-case
timings keyed by case id when recording routerTimes/totalTimes/singleTimes and
then compute avgRouterMs/avgTotalMs/avgSingleMs by filtering that map to the
identical set of case ids, or (b) restrict routerTimes to only the timerCases
(or to the voice_memo cases) before averaging; ensure the overhead calculation
uses matching averaged sets so it becomes a valid apples-to-apples comparison.
Comment on lines
+202
to
214
| fllamaChat(request, (String response, String responseJson, bool done) { | ||
| debugPrint('[LlmService] stream cb: done=$done, len=${response.length}'); | ||
| if (!controller.isClosed) { | ||
| controller.add(response); | ||
| if (done) { | ||
| _requestInFlight = false; | ||
| _runningRequestId = -1; | ||
| controller.close(); | ||
| } | ||
| }, | ||
| ).then((id) { | ||
| } | ||
| }).then((id) { | ||
| _runningRequestId = id; | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and explore the repository structure
find . -type f -name "llm_service.dart" | head -20Repository: ZSWatch/ZSWatch-App
Length of output: 155
🏁 Script executed:
# Read the llm_service.dart file from ai_testbench to verify the code snippet
wc -l ./ai_testbench/lib/services/llm_service.dartRepository: ZSWatch/ZSWatch-App
Length of output: 112
🏁 Script executed:
# Read the relevant section of the file (lines around 202-214)
sed -n '190,225p' ./ai_testbench/lib/services/llm_service.dartRepository: ZSWatch/ZSWatch-App
Length of output: 1069
🏁 Script executed:
# Get broader context of the generateStream method
sed -n '170,220p' ./ai_testbench/lib/services/llm_service.dartRepository: ZSWatch/ZSWatch-App
Length of output: 1477
🏁 Script executed:
# Search for fllamaChat definition and imports
head -50 ./ai_testbench/lib/services/llm_service.dartRepository: ZSWatch/ZSWatch-App
Length of output: 1283
🏁 Script executed:
# Search for other Stream creation patterns and error handling in the file
rg -n "StreamController|addError|catchError" ./ai_testbench/lib/services/llm_service.dartRepository: ZSWatch/ZSWatch-App
Length of output: 118
🏁 Script executed:
# Search for all fllamaChat calls in the file and other methods that return Stream
rg -n "fllamaChat|Stream<" ./ai_testbench/lib/services/llm_service.dartRepository: ZSWatch/ZSWatch-App
Length of output: 293
🏁 Script executed:
# Check the other fllamaChat call around line 121 to see error handling pattern
sed -n '110,135p' ./ai_testbench/lib/services/llm_service.dartRepository: ZSWatch/ZSWatch-App
Length of output: 712
🏁 Script executed:
# View the full first method to see error handling
sed -n '85,140p' ./ai_testbench/lib/services/llm_service.dartRepository: ZSWatch/ZSWatch-App
Length of output: 1551
🏁 Script executed:
# Look for controller.close() and verify error handling is the issue
rg -n "controller\." ./ai_testbench/lib/services/llm_service.dartRepository: ZSWatch/ZSWatch-App
Length of output: 207
Handle fllamaChat startup failures in generateStream().
If fllamaChat fails, the stream is never errored/closed and _requestInFlight remains stale, preventing subsequent requests. Add an error path that resets state and closes the controller.
Suggested fix
fllamaChat(request, (String response, String responseJson, bool done) {
debugPrint('[LlmService] stream cb: done=$done, len=${response.length}');
if (!controller.isClosed) {
controller.add(response);
if (done) {
_requestInFlight = false;
_runningRequestId = -1;
controller.close();
}
}
}).then((id) {
_runningRequestId = id;
- });
+ }).catchError((Object e, StackTrace st) {
+ _requestInFlight = false;
+ _runningRequestId = -1;
+ if (!controller.isClosed) {
+ controller.addError(e, st);
+ controller.close();
+ }
+ });📝 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
| fllamaChat(request, (String response, String responseJson, bool done) { | |
| debugPrint('[LlmService] stream cb: done=$done, len=${response.length}'); | |
| if (!controller.isClosed) { | |
| controller.add(response); | |
| if (done) { | |
| _requestInFlight = false; | |
| _runningRequestId = -1; | |
| controller.close(); | |
| } | |
| }, | |
| ).then((id) { | |
| } | |
| }).then((id) { | |
| _runningRequestId = id; | |
| }); | |
| fllamaChat(request, (String response, String responseJson, bool done) { | |
| debugPrint('[LlmService] stream cb: done=$done, len=${response.length}'); | |
| if (!controller.isClosed) { | |
| controller.add(response); | |
| if (done) { | |
| _requestInFlight = false; | |
| _runningRequestId = -1; | |
| controller.close(); | |
| } | |
| } | |
| }).then((id) { | |
| _runningRequestId = id; | |
| }).catchError((Object e, StackTrace st) { | |
| _requestInFlight = false; | |
| _runningRequestId = -1; | |
| if (!controller.isClosed) { | |
| controller.addError(e, st); | |
| controller.close(); | |
| } | |
| }); |
🤖 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 `@ai_testbench/lib/services/llm_service.dart` around lines 202 - 214,
fllamaChat's returned Future isn't handling startup failures so when it throws
the stream remains open and _requestInFlight/_runningRequestId stay stale; in
generateStream() attach an error handler (e.g. catchError or onError in the
Future chain) to the fllamaChat call that: sets _requestInFlight = false, sets
_runningRequestId = -1, and either calls controller.addError(error) and/or
controller.close() (checking controller.isClosed first) so the stream is
errored/closed on startup failure and state is reset for next requests.
Comment on lines
+1115
to
1118
| final validJson = | ||
| extractions.isNotEmpty || | ||
| (testCase.expectedCount == 0 && | ||
| _containsEmptyJsonArray(result.output)); |
There was a problem hiding this comment.
Harden empty-output JSON validation to avoid false positives.
Line 1115 currently treats any output containing [] as valid JSON for zero-item cases, which can pass noisy/non-JSON text and skew benchmark pass rates. Parse and validate the top-level JSON structure strictly.
Suggested fix
+import 'dart:convert';
import 'dart:async';
import 'dart:io';
@@
- final validJson =
- extractions.isNotEmpty ||
- (testCase.expectedCount == 0 &&
- _containsEmptyJsonArray(result.output));
+ final validJson =
+ extractions.isNotEmpty ||
+ (testCase.expectedCount == 0 &&
+ _isStrictEmptyJsonArray(result.output));
@@
- bool _containsEmptyJsonArray(String raw) {
- final cleaned = raw.replaceAll(RegExp(r'\s+'), '');
- return cleaned.contains('[]');
- }
+ bool _isStrictEmptyJsonArray(String raw) {
+ try {
+ final decoded = jsonDecode(raw.trim());
+ return decoded is List && decoded.isEmpty;
+ } catch (_) {
+ return false;
+ }
+ }📝 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
| final validJson = | |
| extractions.isNotEmpty || | |
| (testCase.expectedCount == 0 && | |
| _containsEmptyJsonArray(result.output)); | |
| final validJson = | |
| extractions.isNotEmpty || | |
| (testCase.expectedCount == 0 && | |
| _isStrictEmptyJsonArray(result.output)); |
🤖 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 `@ai_testbench/lib/services/model_benchmark_service.dart` around lines 1115 -
1118, The current validJson check in model_benchmark_service.dart that relies on
_containsEmptyJsonArray is too permissive; change the logic to parse
result.output as JSON and validate the top-level structure: in the branch where
testCase.expectedCount == 0, parse result.output and confirm it is a JSON array
of length 0 (not just containing "[]"); otherwise ensure the parsed JSON is a
non-empty array or object as appropriate. Add strict schema validation for items
using the chrono_ai_flow fields (intent, title, datetime) — e.g., implement or
call a validator function (new or existing) to assert presence/types for intent,
title and a parsable datetime — and replace the _containsEmptyJsonArray
heuristic with this parsing+validation in the code that computes validJson.
Comment on lines
+1303
to
+1306
| currentModelIndex: modelPaths.isEmpty ? 0 : modelPaths.length - 1, | ||
| currentCaseIndex: totalCases == 0 ? 0 : totalCases - 1, | ||
| currentModelPath: modelPaths.isEmpty ? '' : modelPaths.last, | ||
| currentCaseName: | ||
| benchmarkCases.isEmpty ? '' : benchmarkCases.last.name, | ||
| currentCaseName: benchmarkCases.isEmpty ? '' : benchmarkCases.last.name, |
There was a problem hiding this comment.
Use the executed case list for final progress metadata.
Line 1306 pulls currentCaseName from benchmarkCases.last.name, which is incorrect when selectedCases is provided (or empty). It should mirror casesToRun.
Suggested fix
- currentCaseName: benchmarkCases.isEmpty ? '' : benchmarkCases.last.name,
+ currentCaseName: casesToRun.isEmpty ? '' : casesToRun.last.name,📝 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
| currentModelIndex: modelPaths.isEmpty ? 0 : modelPaths.length - 1, | |
| currentCaseIndex: totalCases == 0 ? 0 : totalCases - 1, | |
| currentModelPath: modelPaths.isEmpty ? '' : modelPaths.last, | |
| currentCaseName: | |
| benchmarkCases.isEmpty ? '' : benchmarkCases.last.name, | |
| currentCaseName: benchmarkCases.isEmpty ? '' : benchmarkCases.last.name, | |
| currentModelIndex: modelPaths.isEmpty ? 0 : modelPaths.length - 1, | |
| currentCaseIndex: totalCases == 0 ? 0 : totalCases - 1, | |
| currentModelPath: modelPaths.isEmpty ? '' : modelPaths.last, | |
| currentCaseName: casesToRun.isEmpty ? '' : casesToRun.last.name, |
🤖 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 `@ai_testbench/lib/services/model_benchmark_service.dart` around lines 1303 -
1306, The final progress metadata is using benchmarkCases for
currentCaseName/currentCaseIndex which is wrong when selectedCases/casesToRun
differ; update the metadata to derive current case info from the executed case
list (casesToRun) instead of benchmarkCases/totalCases: use casesToRun.isEmpty ?
'' : casesToRun.last.name for currentCaseName and currentCaseIndex:
casesToRun.isEmpty ? 0 : casesToRun.length - 1 (keep
currentModelIndex/currentModelPath logic as-is using modelPaths). Ensure you
reference the variables currentCaseName and currentCaseIndex when making the
change so the progress reflects the actual cases run.
Comment on lines
+54
to
+58
| @override | ||
| void didChangeAppLifecycleState(AppLifecycleState state) { | ||
| super.didChangeAppLifecycleState(state); | ||
| LifecycleLogger.log('AppLifecycle', state.name); | ||
| unawaited(_syncForegroundServiceLifecycleState(state)); |
There was a problem hiding this comment.
Catch failures inside the lifecycle sync path.
This runs via unawaited(...). If foregroundServiceProvider or updateNotification() throws during a lifecycle transition, the error escapes as an uncaught async exception. Make this path best-effort with a local try/catch.
Suggested guard
Future<void> _syncForegroundServiceLifecycleState(
AppLifecycleState state,
) async {
- if (!Platform.isAndroid) return;
-
- final foregroundService = ref.read(foregroundServiceProvider);
- if (!foregroundService.isRunning) return;
-
- final connection = ref.read(watchConnectionProvider);
- final watchName = connection.watchName ?? 'ZSWatch';
-
- if (state == AppLifecycleState.resumed) {
- await foregroundService.updateNotification(
- watchName: watchName,
- state: _foregroundStateForConnection(connection.state),
- );
- return;
- }
-
- if (state == AppLifecycleState.inactive ||
- state == AppLifecycleState.hidden ||
- state == AppLifecycleState.paused) {
- await foregroundService.updateNotification(
- watchName: watchName,
- state: ForegroundConnectionState.watcher,
- );
- }
+ try {
+ if (!Platform.isAndroid) return;
+
+ final foregroundService = ref.read(foregroundServiceProvider);
+ if (!foregroundService.isRunning) return;
+
+ final connection = ref.read(watchConnectionProvider);
+ final watchName = connection.watchName ?? 'ZSWatch';
+
+ if (state == AppLifecycleState.resumed) {
+ await foregroundService.updateNotification(
+ watchName: watchName,
+ state: _foregroundStateForConnection(connection.state),
+ );
+ return;
+ }
+
+ if (state == AppLifecycleState.inactive ||
+ state == AppLifecycleState.hidden ||
+ state == AppLifecycleState.paused) {
+ await foregroundService.updateNotification(
+ watchName: watchName,
+ state: ForegroundConnectionState.watcher,
+ );
+ }
+ } catch (e, st) {
+ debugPrint(
+ '[_ZSWatchAppState] foreground lifecycle sync failed: $e\n$st',
+ );
+ }
}Also applies to: 111-137
🤖 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 `@zswatch_app/lib/app.dart` around lines 54 - 58, The unawaited call from
didChangeAppLifecycleState to _syncForegroundServiceLifecycleState can throw
uncaught async errors; wrap the body of _syncForegroundServiceLifecycleState
(and the other lifecycle-sync helper used later) in a local try/catch so
failures are handled best-effort: catch any exception coming from resolving
foregroundServiceProvider or calling updateNotification() and log it (e.g., via
LifecycleLogger.error) but do not rethrow. Ensure you reference and protect the
same symbols: _syncForegroundServiceLifecycleState, foregroundServiceProvider,
and updateNotification (and mirror the same try/catch pattern for the other
lifecycle-sync function mentioned later).
Comment on lines
+92
to
+94
| // Keep Android-owned background recovery preferences synchronized for | ||
| // boot/package-replaced receiver scaffolding. | ||
| ref.read(nativeBackgroundPreferencesSyncProvider); |
There was a problem hiding this comment.
Keep this provider init from short-circuiting the rest of startup.
_initializeBle() is wrapped in one broad try. If nativeBackgroundPreferencesSyncProvider throws here, everything below Line 94 is skipped, including watchInfoPersistenceProvider, voiceMemoSyncServiceProvider, analytics, and crash persistence. That makes this best-effort sync able to block later app features.
Suggested containment
ref.read(foregroundServiceNotifierProvider);
// Keep Android-owned background recovery preferences synchronized for
// boot/package-replaced receiver scaffolding.
- ref.read(nativeBackgroundPreferencesSyncProvider);
+ try {
+ ref.read(nativeBackgroundPreferencesSyncProvider);
+ } catch (e, st) {
+ debugPrint(
+ '[_ZSWatchAppState] nativeBackgroundPreferencesSyncProvider init failed: $e\n$st',
+ );
+ }
// Initialize watch info persistence to sync firmware version and lastConnectedAt to database
// This listens to watch info and connection state changes and persists them
ref.read(watchInfoPersistenceProvider);🤖 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 `@zswatch_app/lib/app.dart` around lines 92 - 94, The call to
ref.read(nativeBackgroundPreferencesSyncProvider) can throw and currently lives
inside the broad try that wraps _initializeBle(), which lets it short-circuit
subsequent startup steps (watchInfoPersistenceProvider,
voiceMemoSyncServiceProvider, analytics, crash persistence); isolate this by
moving the nativeBackgroundPreferencesSyncProvider init into its own try/catch
block (or surrounding only that ref.read call with try/catch) and catch/log the
error (do not rethrow) so failures in nativeBackgroundPreferencesSyncProvider do
not prevent initializing _initializeBle(), watchInfoPersistenceProvider,
voiceMemoSyncServiceProvider, analytics, or crash persistence.
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
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.
iOS Alarms and Timers are not fully tested.
Summary by CodeRabbit
New Features
Improvements