Refresh Smart Insights timestamp at data fetch time#412
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates SmartInsightsTab so time-windowed insight calculations use a timestamp captured when data is fetched, aligning Smart Suggestions and readiness calculations with the latest refresh cycle.
Changes:
- Replaced the fixed remembered timestamp with mutable Compose state.
- Refreshed
nowMsat the start of the profile-based fetch flow. - Added
nowMsto memoization keys for time-sensitive insight computations.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -117,6 +117,7 @@ private fun SmartInsightsContent(modifier: Modifier = Modifier) { | |||
| var weightHistory by remember { mutableStateOf<List<SessionSummary>>(emptyList()) } | |||
|
|
|||
| LaunchedEffect(profileId) { | |||
There was a problem hiding this comment.
Code Review
This pull request updates the SmartInsightsTab to refresh the current timestamp whenever the profile changes and ensures that dependent insights are recomputed by adding nowMs as a key to their respective remember blocks. A review comment suggests resetting the isLoading state when the profile changes to avoid displaying stale data and recommends using a local variable for the timestamp within the LaunchedEffect to ensure consistency during the data fetch.
| nowMs = currentTimeMillis() | ||
| withContext(Dispatchers.IO) { | ||
| sessionSummaries = | ||
| repository.getSessionSummariesSince(nowMs - twentyEightDaysMs, profileId) |
There was a problem hiding this comment.
When profileId changes, isLoading should be reset to true at the start of the LaunchedEffect. Currently, if the user switches profiles, isLoading remains false (from the previous profile's state). The update to nowMs at line 120 will trigger a recomposition, causing insights to be recomputed using the new timestamp but stale sessionSummaries from the previous profile before the new data fetch completes. Using a local variable for the fetch also ensures consistency within the effect.
| nowMs = currentTimeMillis() | |
| withContext(Dispatchers.IO) { | |
| sessionSummaries = | |
| repository.getSessionSummariesSince(nowMs - twentyEightDaysMs, profileId) | |
| isLoading = true | |
| val freshNow = currentTimeMillis() | |
| nowMs = freshNow | |
| withContext(Dispatchers.IO) { | |
| sessionSummaries = | |
| repository.getSessionSummariesSince(freshNow - twentyEightDaysMs, profileId) |
Code Review SummaryStatus: 1 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
| Files Reviewed (1 files)
Reviewed by nemotron-3-super-120b-a12b-20230311:free · 156,023 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
nowMsvalues from causing inconsistent insight calculations after background refreshes or profile changes.Description
var nowMs by remember { mutableStateOf(currentTimeMillis()) }inSmartInsightsContent.nowMs = currentTimeMillis()at the start of theLaunchedEffect(profileId)fetch flow.nowMswhen calling repository functions (fetch window) and includenowMsin memoization keys forrememberonweeklyVolume,balanceAnalysis,neglectedExercises, andreadinessso downstreamSmartSuggestionsEngineandReadinessEnginecomputations align with the latest refresh.Testing
./gradlew :shared:compileKotlinMetadata, which failed in this environment due to missing Supabase credentials required by the build configuration.Codex Task
Perform an AI-assisted review on