Refresh time-sensitive Smart Insights and recompute on clock updates#418
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements a background refresh mechanism for the current time in SmartInsightsTab, ensuring that time-sensitive insights are updated every minute. Feedback suggests fetching a larger data buffer from the repository to maintain a full 28-day window as time elapses and hoisting the training readiness computation to the top level for better architectural consistency.
| withContext(Dispatchers.IO) { | ||
| sessionSummaries = | ||
| repository.getSessionSummariesSince(nowMs - twentyEightDaysMs, profileId) | ||
| repository.getSessionSummariesSince(fetchNowMs - twentyEightDaysMs, profileId) |
There was a problem hiding this comment.
The sessionSummaries are fetched using a fixed 28-day window relative to the initial fetch time (fetchNowMs). However, as nowMs advances via the background loop, engines like ReadinessEngine and BalanceAnalysis use a sliding 28-day window relative to the current nowMs. This causes the effective data window to shrink over time (e.g., after 1 hour, the engine will only have 27 days and 23 hours of data within its required 28-day window). To ensure insights remain accurate as the clock ticks, consider fetching a larger buffer (e.g., 35 days).
| repository.getSessionSummariesSince(fetchNowMs - twentyEightDaysMs, profileId) | |
| repository.getSessionSummariesSince(fetchNowMs - (35L * 24 * 60 * 60 * 1000), profileId) |
| val weeklyVolume = remember(sessionSummaries, nowMs) { | ||
| SmartSuggestionsEngine.computeWeeklyVolume(sessionSummaries, nowMs) | ||
| } |
There was a problem hiding this comment.
For consistency and better maintainability, consider hoisting the readiness computation to the top level of SmartInsightsContent alongside the other insights (weekly volume, balance analysis, etc.), rather than inside the LazyColumn's item block.
| val weeklyVolume = remember(sessionSummaries, nowMs) { | |
| SmartSuggestionsEngine.computeWeeklyVolume(sessionSummaries, nowMs) | |
| } | |
| val weeklyVolume = remember(sessionSummaries, nowMs) { | |
| SmartSuggestionsEngine.computeWeeklyVolume(sessionSummaries, nowMs) | |
| } | |
| val readiness = remember(sessionSummaries, nowMs) { | |
| ReadinessEngine.computeReadiness(sessionSummaries, nowMs) | |
| } |
There was a problem hiding this comment.
Pull request overview
This PR refreshes time-sensitive “Smart Insights” by turning the reference clock (nowMs) into mutable Compose state and periodically updating it so insight computations can be recomputed as time advances.
Changes:
- Make
nowMsmutable Compose state and introduce a periodic refresh interval (nowRefreshIntervalMs). - Capture a stable
fetchNowMsfor initial repository fetches to avoid time drift during loading. - Key several
remember(...)computations onnowMsso weekly volume, balance, neglected exercises, and readiness recompute on clock updates.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import com.devil.phoenixproject.ui.theme.AccessibilityTheme | ||
| import com.devil.phoenixproject.util.format | ||
| import kotlinx.coroutines.Dispatchers | ||
| import kotlinx.coroutines.IO |
| withContext(Dispatchers.IO) { | ||
| sessionSummaries = | ||
| repository.getSessionSummariesSince(nowMs - twentyEightDaysMs, profileId) | ||
| repository.getSessionSummariesSince(fetchNowMs - twentyEightDaysMs, profileId) | ||
| exerciseLastPerformed = repository.getExerciseLastPerformed(profileId) | ||
| weightHistory = repository.getExerciseWeightHistory(profileId) |
| withContext(Dispatchers.IO) { | ||
| sessionSummaries = | ||
| repository.getSessionSummariesSince(nowMs - twentyEightDaysMs, profileId) | ||
| repository.getSessionSummariesSince(fetchNowMs - twentyEightDaysMs, profileId) |
There was a problem hiding this comment.
CRITICAL: Compose state mutations inside withContext(Dispatchers.IO) violate thread-safety. sessionSummaries is updated from a background thread. Collect data into local vars inside IO, then assign state after withContext returns on main thread.
| sessionSummaries = | ||
| repository.getSessionSummariesSince(nowMs - twentyEightDaysMs, profileId) | ||
| repository.getSessionSummariesSince(fetchNowMs - twentyEightDaysMs, profileId) | ||
| exerciseLastPerformed = repository.getExerciseLastPerformed(profileId) |
There was a problem hiding this comment.
CRITICAL: Same thread-safety issue - exerciseLastPerformed mutated from background thread. Move assignment outside withContext(Dispatchers.IO) to main thread.
| repository.getSessionSummariesSince(nowMs - twentyEightDaysMs, profileId) | ||
| repository.getSessionSummariesSince(fetchNowMs - twentyEightDaysMs, profileId) | ||
| exerciseLastPerformed = repository.getExerciseLastPerformed(profileId) | ||
| weightHistory = repository.getExerciseWeightHistory(profileId) |
There was a problem hiding this comment.
CRITICAL: Same thread-safety issue - weightHistory mutated from background thread. Assign state on main thread to prevent crashes/inconsistent recompositions.
| item { | ||
| TimeOfDayCard(timeOfDay) | ||
| } | ||
|
|
There was a problem hiding this comment.
SUGGESTION: Hoist readiness computation to top level alongside other insights (weeklyVolume, balanceAnalysis, etc.) for architectural consistency. Compute it once in the body block rather than inside LazyColumn item.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Other Observations (not in diff)✅ Resolved Issues
Files Reviewed (1 files)
Fix these issues in Kilo Cloud Reviewed by ling-2.6-1t-20260423:free · 3,405,716 tokens |
* Refresh Smart Insights now timestamp on fetch (#412) * Refresh time-sensitive Smart Insights and recompute on clock updates (#418) * Align Smart Insights nowMs refresh with fetch and timer * Stabilize Smart Insights timer effect lifecycle * Remove periodic SmartInsights timer refresh loop * Harden SmartInsights loading state during refresh * Avoid mutating compose state from IO dispatcher * Align SmartInsights query and weekly-volume computation to shared nowMs (Option A) (#413) * Align insights query and weekly volume with shared nowMs * Harden insights anchor timestamp lifecycle * Fix SmartInsights loading guard for null anchor timestamp * Reset insights anchor before each profile refresh * Expand SmartSuggestions muscle-group taxonomy and add unknown-group fallback tracking (#415) * Expand muscle-group aliases and track unknown fallback usage * Refine unknown muscle-group fallback tracking * Keep SmartSuggestions fallback tracking stateless * Resolve review nits in muscle-group fallback docs/tests * Standardize inclusive cutoff policy (>=) in ReadinessEngine and add boundary tests (#416) * Define inclusive readiness cutoff policy and add boundary tests * Refine readiness boundary tests and add exclusion coverage * Address review cleanup in readiness boundary tests * Centralize inclusive readiness cutoff check * Unify insights into Snapshot→Trends→Diagnostics→Actions and add per-card metadata (#417) * Refactor analytics insights into unified hierarchy * Address review feedback on insight timeframe badges * Resolve review feedback on insights cards and cleanup * Resolve follow-up comments on readiness insight presentation * Use average intensity for Time-of-Day insights with confidence gating (#414) * Refine time-of-day insight to average intensity with confidence gating * Align time insight session-threshold copy with confidence gating * Add time-of-day tests for distinct-day gating and intensity scoring * Generalize time-window confidence copy across locales * Codex/fix high priority bug in insightstab.kt (#420) * Refactor analytics insights into unified hierarchy * Address review feedback on insight timeframe badges * Resolve review feedback on insights cards and cleanup * Resolve follow-up comments on readiness insight presentation * Fix missing RoundedCornerShape import in InsightsTab * Refresh Smart Insights now timestamp on fetch (#419) * Refresh Smart Insights now timestamp on fetch * Fix Compose state mutation in IO dispatcher and reset loading state on profile change Agent-Logs-Url: https://github.com/9thLevelSoftware/Project-Phoenix-MP/sessions/23e4c8a0-93e7-428f-8d00-b84a01429a58 Co-authored-by: 9thLevelSoftware <69057727+9thLevelSoftware@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * fix(rep-counter): ignore firmware warmup sync for variable warmup sets (#422) * Request transient duck audio focus for Android workout cues (#423) * Fix Android cue audio precedence over background music * Fix audio focus release timing for workout cues * Align audio focus attributes with playback usage * Add focusChangeListener as identity token in requestTransientDuckAudioFocus Agent-Logs-Url: https://github.com/9thLevelSoftware/Project-Phoenix-MP/sessions/1d2923b8-cdaf-4080-bc22-8839a6940e94 Co-authored-by: 9thLevelSoftware <69057727+9thLevelSoftware@users.noreply.github.com> * Add focusHoldDurationMs helper and use event-specific delay in abandonAfterDelay Agent-Logs-Url: https://github.com/9thLevelSoftware/Project-Phoenix-MP/sessions/cd565f28-356a-4acd-9102-45b2602e7e02 Co-authored-by: 9thLevelSoftware <69057727+9thLevelSoftware@users.noreply.github.com> * Tie ducking focus hold to cue duration metadata --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * build: update dependencies and add device metadata to Health Connect Update Health Connect to the stable 1.1.0 release and include device attribution in synced exercise records. - **Health**: Update `androidx-health-connect` to `1.1.0` stable and remove alpha-pinning workarounds in `shared/build.gradle.kts`. - **Health**: Implement `Metadata.activelyRecorded` for `ExerciseSessionRecord` and `TotalCaloriesBurnedRecord` to identify the "Vitruvian Trainer" device. - **Versions**: Upgrade Kotlin to `2.3.21`, AGP to `9.2.1`, and Gradle to `9.5.0`. - **Versions**: Bump Compose BOM to `2026.05.00`, Compose Navigation to `2.9.8`, and Ktor to `3.4.3`. * Revert audio focus duck attempt (#423) — restore audio output for #421 (#428) * Revert "Request transient duck audio focus for Android workout cues (#423)" This reverts commit 6371274. * fix(audio): release MediaPlayer on exception, dedupe sound name lists Address gemini-code-assist review on PR #428: - playWithMediaPlayer: hoist `mediaPlayer` declaration out of the `try` block and release it from `catch`. Previous structure leaked the native player if setVolume/setOnCompletionListener/start threw. - Extract BADGE_SOUND_NAMES and PR_SOUND_NAMES to private top-level vals. Eliminates duplication between the SoundPool-loading `remember` block and getRandomBadgeSound/getRandomPRSound, and removes per-call list allocations on the random helpers. --------- Co-authored-by: Claude <noreply@anthropic.com> * Restore Android workout cue audio routing and ignore code-review artifacts (#429) * Address PR #429 review feedback: route workout cues to STREAM_MUSIC (#430) * Address PR #429 review feedback: route cues to STREAM_MUSIC P1 (chatgpt-codex-connector / gemini-code-assist): the previous change to USAGE_ASSISTANCE_SONIFICATION routed cues to STREAM_SYSTEM, breaking the existing volumeControlStream = STREAM_MUSIC contract. Hardware volume buttons would no longer adjust workout cue loudness, leaving users with silent or unbalanced cues. - Switch STANDARD_ANDROID_CUE_USAGE to USAGE_MEDIA so SoundPool and MediaPlayer both route through STREAM_MUSIC, matching the activity's volumeControlStream and the existing media-volume warning logic. - Update warnIfMediaVolumeMuted comment/message to reflect USAGE_MEDIA. - Reword MainActivity volumeControlStream comment to reflect the actual scope (entire activity lifetime, not just the workout UI). - Update structural test assertions to match USAGE_MEDIA and add a guard preventing accidental reintroduction of USAGE_ASSISTANCE_SONIFICATION. - Replace silent fallbacks in HapticFeedbackAudioRoutingGuardTest with explicit checks so a missing project root, source file, or raw resource directory fails with an actionable message instead of an opaque FileNotFoundException or "every cue missing" report. * Address PR #430 review feedback: harden cue source parser - gemini-code-assist (high) / chatgpt-codex-connector (P2): the multi-line branch in soundNamesInConstantList searched for `\n)`, which only matches a closing paren at column 0. Replace with a simple `indexOf(")", start)`. Sound names only contain `[a-z0-9_]` so the first `)` after `start` is reliably the listOf(...) closer regardless of single-line, multi-line, or indented formatting. - gemini-code-assist (medium): extract the rep-count range (`(1..N).mapNotNull`) from the source instead of duplicating `1..25`, so the guard stays in sync if the upper bound changes. Skipped: the suggestion to skip SoundPool initialization on Fire OS (pre-existing behavior, out of scope for this PR's audio-routing focus). --------- Co-authored-by: Claude <noreply@anthropic.com> * Fix top-level superset reorder persistence and restore audio routing * Fix Android cue packaging and audio routing (#431) * Harden Compose layouts for iOS accessibility scaling (#432) * Harden iOS accessibility layouts for issue 389 * Fix cue audio resource lookup in release builds * Fix per-cable weight display and softMax handling (#433) * fix: preserve stall detection in portal sync * chore: remove stale generated artifacts * Codex/stu experiment (#436) * Fix per-cable weight display and softMax handling * Fix BLE force target to keep progression separate * Replace Just Lift auto-detection with post-set exercise tagging (#435) * Fix Just Lift retag PR tracking and completed set sync * fix: prevent Just Lift retag PR duplication * test: align AMRAP target weight expectation --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
* Refresh Smart Insights now timestamp on fetch (#412) * Refresh time-sensitive Smart Insights and recompute on clock updates (#418) * Align Smart Insights nowMs refresh with fetch and timer * Stabilize Smart Insights timer effect lifecycle * Remove periodic SmartInsights timer refresh loop * Harden SmartInsights loading state during refresh * Avoid mutating compose state from IO dispatcher * Align SmartInsights query and weekly-volume computation to shared nowMs (Option A) (#413) * Align insights query and weekly volume with shared nowMs * Harden insights anchor timestamp lifecycle * Fix SmartInsights loading guard for null anchor timestamp * Reset insights anchor before each profile refresh * Expand SmartSuggestions muscle-group taxonomy and add unknown-group fallback tracking (#415) * Expand muscle-group aliases and track unknown fallback usage * Refine unknown muscle-group fallback tracking * Keep SmartSuggestions fallback tracking stateless * Resolve review nits in muscle-group fallback docs/tests * Standardize inclusive cutoff policy (>=) in ReadinessEngine and add boundary tests (#416) * Define inclusive readiness cutoff policy and add boundary tests * Refine readiness boundary tests and add exclusion coverage * Address review cleanup in readiness boundary tests * Centralize inclusive readiness cutoff check * Unify insights into Snapshot→Trends→Diagnostics→Actions and add per-card metadata (#417) * Refactor analytics insights into unified hierarchy * Address review feedback on insight timeframe badges * Resolve review feedback on insights cards and cleanup * Resolve follow-up comments on readiness insight presentation * Use average intensity for Time-of-Day insights with confidence gating (#414) * Refine time-of-day insight to average intensity with confidence gating * Align time insight session-threshold copy with confidence gating * Add time-of-day tests for distinct-day gating and intensity scoring * Generalize time-window confidence copy across locales * Codex/fix high priority bug in insightstab.kt (#420) * Refactor analytics insights into unified hierarchy * Address review feedback on insight timeframe badges * Resolve review feedback on insights cards and cleanup * Resolve follow-up comments on readiness insight presentation * Fix missing RoundedCornerShape import in InsightsTab * Refresh Smart Insights now timestamp on fetch (#419) * Refresh Smart Insights now timestamp on fetch * Fix Compose state mutation in IO dispatcher and reset loading state on profile change Agent-Logs-Url: https://github.com/9thLevelSoftware/Project-Phoenix-MP/sessions/23e4c8a0-93e7-428f-8d00-b84a01429a58 Co-authored-by: 9thLevelSoftware <69057727+9thLevelSoftware@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * fix(rep-counter): ignore firmware warmup sync for variable warmup sets (#422) * Request transient duck audio focus for Android workout cues (#423) * Fix Android cue audio precedence over background music * Fix audio focus release timing for workout cues * Align audio focus attributes with playback usage * Add focusChangeListener as identity token in requestTransientDuckAudioFocus Agent-Logs-Url: https://github.com/9thLevelSoftware/Project-Phoenix-MP/sessions/1d2923b8-cdaf-4080-bc22-8839a6940e94 Co-authored-by: 9thLevelSoftware <69057727+9thLevelSoftware@users.noreply.github.com> * Add focusHoldDurationMs helper and use event-specific delay in abandonAfterDelay Agent-Logs-Url: https://github.com/9thLevelSoftware/Project-Phoenix-MP/sessions/cd565f28-356a-4acd-9102-45b2602e7e02 Co-authored-by: 9thLevelSoftware <69057727+9thLevelSoftware@users.noreply.github.com> * Tie ducking focus hold to cue duration metadata --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * build: update dependencies and add device metadata to Health Connect Update Health Connect to the stable 1.1.0 release and include device attribution in synced exercise records. - **Health**: Update `androidx-health-connect` to `1.1.0` stable and remove alpha-pinning workarounds in `shared/build.gradle.kts`. - **Health**: Implement `Metadata.activelyRecorded` for `ExerciseSessionRecord` and `TotalCaloriesBurnedRecord` to identify the "Vitruvian Trainer" device. - **Versions**: Upgrade Kotlin to `2.3.21`, AGP to `9.2.1`, and Gradle to `9.5.0`. - **Versions**: Bump Compose BOM to `2026.05.00`, Compose Navigation to `2.9.8`, and Ktor to `3.4.3`. * Revert audio focus duck attempt (#423) — restore audio output for #421 (#428) * Revert "Request transient duck audio focus for Android workout cues (#423)" This reverts commit 6371274. * fix(audio): release MediaPlayer on exception, dedupe sound name lists Address gemini-code-assist review on PR #428: - playWithMediaPlayer: hoist `mediaPlayer` declaration out of the `try` block and release it from `catch`. Previous structure leaked the native player if setVolume/setOnCompletionListener/start threw. - Extract BADGE_SOUND_NAMES and PR_SOUND_NAMES to private top-level vals. Eliminates duplication between the SoundPool-loading `remember` block and getRandomBadgeSound/getRandomPRSound, and removes per-call list allocations on the random helpers. --------- Co-authored-by: Claude <noreply@anthropic.com> * Restore Android workout cue audio routing and ignore code-review artifacts (#429) * Address PR #429 review feedback: route workout cues to STREAM_MUSIC (#430) * Address PR #429 review feedback: route cues to STREAM_MUSIC P1 (chatgpt-codex-connector / gemini-code-assist): the previous change to USAGE_ASSISTANCE_SONIFICATION routed cues to STREAM_SYSTEM, breaking the existing volumeControlStream = STREAM_MUSIC contract. Hardware volume buttons would no longer adjust workout cue loudness, leaving users with silent or unbalanced cues. - Switch STANDARD_ANDROID_CUE_USAGE to USAGE_MEDIA so SoundPool and MediaPlayer both route through STREAM_MUSIC, matching the activity's volumeControlStream and the existing media-volume warning logic. - Update warnIfMediaVolumeMuted comment/message to reflect USAGE_MEDIA. - Reword MainActivity volumeControlStream comment to reflect the actual scope (entire activity lifetime, not just the workout UI). - Update structural test assertions to match USAGE_MEDIA and add a guard preventing accidental reintroduction of USAGE_ASSISTANCE_SONIFICATION. - Replace silent fallbacks in HapticFeedbackAudioRoutingGuardTest with explicit checks so a missing project root, source file, or raw resource directory fails with an actionable message instead of an opaque FileNotFoundException or "every cue missing" report. * Address PR #430 review feedback: harden cue source parser - gemini-code-assist (high) / chatgpt-codex-connector (P2): the multi-line branch in soundNamesInConstantList searched for `\n)`, which only matches a closing paren at column 0. Replace with a simple `indexOf(")", start)`. Sound names only contain `[a-z0-9_]` so the first `)` after `start` is reliably the listOf(...) closer regardless of single-line, multi-line, or indented formatting. - gemini-code-assist (medium): extract the rep-count range (`(1..N).mapNotNull`) from the source instead of duplicating `1..25`, so the guard stays in sync if the upper bound changes. Skipped: the suggestion to skip SoundPool initialization on Fire OS (pre-existing behavior, out of scope for this PR's audio-routing focus). --------- Co-authored-by: Claude <noreply@anthropic.com> * Fix top-level superset reorder persistence and restore audio routing * Fix Android cue packaging and audio routing (#431) * Harden Compose layouts for iOS accessibility scaling (#432) * Harden iOS accessibility layouts for issue 389 * Fix cue audio resource lookup in release builds * Fix per-cable weight display and softMax handling * Fix per-cable weight display and softMax handling (#433) * Fix BLE force target to keep progression separate * fix: preserve stall detection in portal sync * chore: remove stale generated artifacts * Replace Just Lift auto-detection with post-set exercise tagging (#435) * Fix Just Lift retag PR tracking and completed set sync * fix: prevent Just Lift retag PR duplication * test: align AMRAP target weight expectation * fix: test legacy Kable GATT writes for Pixel starts * Codex/issue 333 ready gate test (#438) * fix: gate BLE ready state for Pixel diagnostics * fix(ble): optimize connection flow and improve Pixel device compatibility Refine BLE communication by introducing device-specific timing adjustments for Google Pixel hardware and streamlining the command observation sequence. - **DeviceInfo**: Add `isPixel()` and `isSamsung()` to the common `DeviceInfo` interface to support platform-specific hardware workarounds. - **BLE**: Implement observation for the `SAMPLE` characteristic to support raw monitor packets without opcode prefixes. - **BLE**: Update `parseMetricsPacket` in `KableBleRepository` to handle both opcoded (0x01) and raw monitor data payloads. - **BLE**: Introduce a 500ms `preDelayMs` on Pixel devices after sending program configurations to improve diagnostic read reliability. - **Session**: Remove the redundant manual `START` command (0x03) from `ActiveSessionEngine` prior to starting workout polling. - **Kable Patch**: Optimize `Connection.kt` by removing an unnecessary `async`/`await` wrapper when receiving GATT responses, reducing coroutine overhead. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
Motivation
nowMsby making recomputation triggerable when the current time changes.Description
nowMsto a mutable state (var nowMs by remember { mutableStateOf(currentTimeMillis()) }) and introducenowRefreshIntervalMsto drive periodic updates.fetchNowMsduring initial data fetch and call repository methods withfetchNowMs - twentyEightDaysMsto avoid race conditions during load.LaunchedEffectloop withdelay(nowRefreshIntervalMs)to updatenowMsperiodically and importdelay.nowMsas a key toremembercalls (e.g. weekly volume, balance analysis, neglected exercises, readiness) so insights recompute when the clock advances.Testing
Codex Task
Perform an AI-assisted review on