From 623faa87e82078d2f0f0b86ee4d6e56915f7bc05 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 16:13:33 +0000 Subject: [PATCH] fix: observe published workouts instead of scanning, and translate durations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review comments on #4129. **The dashboard now observes the cache.** publishedWorkoutsOf walked every note in LocalCache on a suspend call, so it was both O(cache) per refresh and a snapshot: a workout arriving from a relay — or the one the user had just posted — only appeared on the next resume. It is now LocalCache.observeEvents(Filter(kinds = [1301], authors = [me])), which is indexed by kind and author, so it neither walks the cache nor needs a timer, and the screen updates itself. That turned the ViewModel inside out for the better. Health Connect stays a push source — the platform has no change feed to observe, so refresh() still re-reads it on resume and on a permission grant — but the published side is a flow, and state is now a combine() of the two, keyed on the account via flatMapLatest so an account switch re-subscribes. Loading is expressed as "the Health Connect status is not resolved yet" rather than a separate flag. The carousel takes .first() from the same flow: it reloads on resume anyway, and one definition of "what have I already posted" is the point — two would let it offer something the dashboard knows is a duplicate. This does not close the REQ gap documented in the PR: observing the cache still only sees what something else fetched. It does mean that once the REQ exists, the dashboard will pick its results up without further work. **Durations are translatable.** formatDuration hardcoded "7h 12m" / "42m" / "45s". The suffixes are English abbreviations and the order of the two parts is not universal either, so both move into string resources and the function becomes @Composable. The zero case reuses the minutes form rather than a second literal. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019egdJyBHnrATZHjs86up8f --- .../workouts/health/PublishedWorkouts.kt | 32 +++---- .../workouts/fitness/MyFitnessFormat.kt | 20 +++-- .../workouts/fitness/MyFitnessViewModel.kt | 87 +++++++++++++------ .../suggestion/DetectedWorkoutCarousel.kt | 3 +- .../composeResources/values/strings.xml | 5 ++ 5 files changed, 97 insertions(+), 50 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/workouts/health/PublishedWorkouts.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/workouts/health/PublishedWorkouts.kt index ad2d5fdfe5..7f0947f50a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/workouts/health/PublishedWorkouts.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/workouts/health/PublishedWorkouts.kt @@ -24,30 +24,26 @@ import com.vitorpamplona.amethyst.commons.fitness.DetectedWorkout import com.vitorpamplona.amethyst.commons.fitness.toDetectedWorkout import com.vitorpamplona.amethyst.model.LocalCache import com.vitorpamplona.quartz.experimental.fitness.workout.WorkoutRecordEvent -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.withContext +import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.map /** - * The workouts [pubkeyHex] has published, from [sinceEpochSeconds] onwards, as the cache currently - * holds them. + * The workouts [pubkeyHex] has published, as a live view of the cache. * * Shared by the My Fitness dashboard, which counts them, and the New Workout carousel, which uses * them to avoid offering a workout the user already shared. Both need the same view of "what have * I already posted", and two answers to that question would mean the carousel offering something * the dashboard knows is a duplicate. * - * Scans off the main thread: LocalCache holds every event the session has seen and this walks all - * of them. It reports only what is already cached — it issues no REQ of its own. + * [LocalCache.observeEvents] rather than a scan: it is indexed by kind and author, so it neither + * walks every note in the cache nor needs re-running on a timer — the dashboard updates itself + * when a relay delivers a workout, including the one the user just published. + * + * Reports only what the cache holds; it issues no REQ of its own. */ -suspend fun publishedWorkoutsOf( - pubkeyHex: String, - sinceEpochSeconds: Long, -): List = - withContext(Dispatchers.Default) { - LocalCache.notes - .filterIntoSet { _, note -> - val event = note.event - event is WorkoutRecordEvent && event.pubKey == pubkeyHex - }.mapNotNull { (it.event as WorkoutRecordEvent).toDetectedWorkout() } - .filter { it.startTimeEpochSeconds >= sinceEpochSeconds } - } +fun publishedWorkoutsOf(pubkeyHex: String): Flow> = + LocalCache + .observeEvents( + Filter(kinds = listOf(WorkoutRecordEvent.KIND), authors = listOf(pubkeyHex)), + ).map { events -> events.mapNotNull { it.toDetectedWorkout() } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/MyFitnessFormat.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/MyFitnessFormat.kt index 576b2c7d38..9599e2c129 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/MyFitnessFormat.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/MyFitnessFormat.kt @@ -22,6 +22,9 @@ package com.vitorpamplona.amethyst.ui.screen.loggedIn.workouts.fitness import androidx.compose.runtime.Composable import com.vitorpamplona.amethyst.commons.resources.Res +import com.vitorpamplona.amethyst.commons.resources.my_fitness_duration_hours_minutes +import com.vitorpamplona.amethyst.commons.resources.my_fitness_duration_minutes +import com.vitorpamplona.amethyst.commons.resources.my_fitness_duration_seconds import com.vitorpamplona.amethyst.commons.resources.my_fitness_unit_ft import com.vitorpamplona.amethyst.commons.resources.my_fitness_unit_km import com.vitorpamplona.amethyst.commons.resources.my_fitness_unit_m @@ -40,17 +43,24 @@ import kotlin.math.roundToLong */ internal fun prefersMiles(): Boolean = phonePrefersMiles() -/** `7h 12m` / `42m` / `45s` — a total, so hours run past 24 rather than wrapping. */ +/** + * `7h 12m` / `42m` / `45s` — a total, so hours run past 24 rather than wrapping. + * + * The unit suffixes come from string resources: "h"/"m"/"s" are English abbreviations, and the + * order of the two parts is not universal either, so both belong to the translator rather than + * to this function. + */ +@Composable internal fun formatDuration(totalSeconds: Long): String { - if (totalSeconds <= 0) return "0m" + if (totalSeconds <= 0) return stringRes(Res.string.my_fitness_duration_minutes, 0) val hours = totalSeconds / 3600 val minutes = (totalSeconds % 3600) / 60 return when { - hours > 0 -> "${hours}h ${minutes}m" - minutes > 0 -> "${minutes}m" - else -> "${totalSeconds}s" + hours > 0 -> stringRes(Res.string.my_fitness_duration_hours_minutes, hours, minutes) + minutes > 0 -> stringRes(Res.string.my_fitness_duration_minutes, minutes) + else -> stringRes(Res.string.my_fitness_duration_seconds, totalSeconds) } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/MyFitnessViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/MyFitnessViewModel.kt index 3f829aed08..ad8324944f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/MyFitnessViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/fitness/MyFitnessViewModel.kt @@ -25,13 +25,22 @@ import androidx.compose.runtime.Immutable import androidx.compose.runtime.Stable import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import com.vitorpamplona.amethyst.commons.fitness.DetectedWorkout import com.vitorpamplona.amethyst.commons.fitness.TrainingLog import com.vitorpamplona.amethyst.commons.fitness.WorkoutStats import com.vitorpamplona.amethyst.service.workouts.health.HealthConnectManager import com.vitorpamplona.amethyst.service.workouts.health.publishedWorkoutsOf +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.flowOn +import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch import java.time.Duration import java.time.Instant @@ -64,7 +73,7 @@ class MyFitnessViewModel : ViewModel() { @Immutable sealed interface State { - /** First load, or a reload after a permission change. */ + /** First load, before Health Connect has been checked. */ data object Loading : State /** @@ -78,46 +87,72 @@ class MyFitnessViewModel : ViewModel() { ) : State } - private val _state = MutableStateFlow(State.Loading) - val state: StateFlow = _state.asStateFlow() + private val pubkeyHex = MutableStateFlow(null) + + /** + * Health Connect's contribution. A push source: the platform has no change feed we can + * observe, so [refresh] re-reads it when the screen resumes or a permission is granted. + */ + private val fromHealthConnect = MutableStateFlow>(emptyList()) + + /** Null until the first [refresh] resolves, which is what keeps the screen on [State.Loading]. */ + private val healthConnectStatus = MutableStateFlow(null) private var manager: HealthConnectManager? = null - /** The pubkey whose workouts this dashboard summarises. Set by the screen before refreshing. */ - private var pubkeyHex: String? = null - - fun init(pubkeyHex: String) { - if (this.pubkeyHex != pubkeyHex) { - this.pubkeyHex = pubkeyHex - _state.value = State.Loading + /** + * The user's published workouts, live. Re-subscribes on an account switch; a workout arriving + * from a relay — or the one the user just posted — lands here without a refresh. + */ + @OptIn(ExperimentalCoroutinesApi::class) + private val fromRelays: Flow> = + pubkeyHex.flatMapLatest { me -> + if (me == null) flowOf(emptyList()) else publishedWorkoutsOf(me) } + + val state: StateFlow = + combine(fromHealthConnect, fromRelays, healthConnectStatus) { healthConnect, published, status -> + if (status == null) { + State.Loading + } else { + val now = Instant.now() + val since = now.minus(Duration.ofDays(WorkoutStats.WINDOW_DAYS)).epochSecond + + State.Ready( + report = + WorkoutStats.report( + TrainingLog.merge(healthConnect, published.filter { it.startTimeEpochSeconds >= since }), + now, + ), + healthConnect = status, + ) + } + }.flowOn(Dispatchers.Default) + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), State.Loading) + + /** The account whose workouts this dashboard summarises. */ + fun init(pubkeyHex: String) { + this.pubkeyHex.value = pubkeyHex } /** - * Rebuilds the dashboard. Safe to call on every resume: it re-checks permissions as well as - * data, so revoking access in Health Connect drops those workouts out of the log rather than - * leaving stale numbers on display. + * Re-reads Health Connect. Safe to call on every resume: it re-checks permissions as well as + * data, so revoking access drops those workouts out of the log rather than leaving stale + * numbers on display. The published side needs no refresh — it is observed. */ fun refresh(context: Context) { viewModelScope.launch { - val now = Instant.now() - val since = now.minus(Duration.ofDays(WorkoutStats.WINDOW_DAYS)) - val status = healthConnectStatus(context) - val fromHealthConnect = + + fromHealthConnect.value = if (status == HealthConnectStatus.CONNECTED) { - manager?.readWorkouts(since, now).orEmpty() + val now = Instant.now() + manager?.readWorkouts(now.minus(Duration.ofDays(WorkoutStats.WINDOW_DAYS)), now).orEmpty() } else { emptyList() } - val fromRelays = publishedWorkoutsOf(pubkeyHex ?: return@launch, since.epochSecond) - - _state.value = - State.Ready( - report = WorkoutStats.report(TrainingLog.merge(fromHealthConnect, fromRelays), now), - healthConnect = status, - ) + healthConnectStatus.value = status } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/suggestion/DetectedWorkoutCarousel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/suggestion/DetectedWorkoutCarousel.kt index 1f29d64ecd..0f2322ca1d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/suggestion/DetectedWorkoutCarousel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/workouts/suggestion/DetectedWorkoutCarousel.kt @@ -78,6 +78,7 @@ import com.vitorpamplona.amethyst.ui.screen.loggedIn.workouts.health.HealthConne import com.vitorpamplona.amethyst.ui.screen.loggedIn.workouts.labelRes import com.vitorpamplona.amethyst.ui.screen.loggedIn.workouts.symbol import com.vitorpamplona.amethyst.ui.stringRes +import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import java.time.Duration import java.time.Instant @@ -123,7 +124,7 @@ fun DetectedWorkoutCarousel( // the same effort. merge() flags the Health Connect copies that match something // already posted, so drop those; what is left is genuinely unshared. TrainingLog - .merge(manager.readWorkouts(since), publishedWorkoutsOf(myPubkey, since.epochSecond)) + .merge(manager.readWorkouts(since), publishedWorkoutsOf(myPubkey).first()) .filter { it.origin == WorkoutOrigin.HEALTH_CONNECT && !it.alreadyPublished } .sortedByDescending { it.startTimeEpochSeconds } } else { diff --git a/commonsUI/src/commonMain/composeResources/values/strings.xml b/commonsUI/src/commonMain/composeResources/values/strings.xml index ae6a40eb98..053702457f 100644 --- a/commonsUI/src/commonMain/composeResources/values/strings.xml +++ b/commonsUI/src/commonMain/composeResources/values/strings.xml @@ -694,6 +694,11 @@ Active days Day streak Time + + %1$dh %2$dm + %1$dm + %1$ds Distance Calories Steps