more workout types, a filter on the list, and calories that stop guessing

[abdulsaheel]

User description

Five people asked for the same three things in Discord and the discussions, so this is those three.

Types. The picker had nine, which meant boxing, rowing, hiking, skiing, stair work, pilates, racquet sports and the ball sports all had to be logged as "Other" — and reached Apple Health as an unlabelled workout. There are twenty-one now. A type only earns a tile if a pack actually draws it, so nothing ships with a stand-in icon that looks like something else. Adding a tile without adding the matching health mapping is silent — the workout still exports, just as a generic entry — so a test fails on it now.

That pushed the start sheet past the height a bottom sheet gives itself by default, which is exactly how "Log a past workout" once ended up invisible and untappable. The sheet scrolls now, and the old guard against content falling off the edge became a guard that every type can still be reached.

Filtering. The list only ever filtered by timeframe. It now filters by type, by minimum duration and by minimum strain, and sorts by longest or hardest as well as by date. Two details worth naming: a narrowed list recomputes its own summary, so the totals above it always describe what is actually on screen; and a session that is happening right now is never hidden by a floor it has not had time to clear.

Calories. The last place the app still made something up. The live tick substituted a 30-year-old 70 kg male for whatever the profile was missing, so an untouched profile produced a confident calorie total — for somebody else's body, and reading high for anyone lighter than the stand-in. The re-score path has always refused to guess. Both share one predicate now, and a session with no anchors shows a dash rather than a number, all the way out to the shareable card.

---

PR Type

Enhancement, Bug fix, Tests

---

Description

• Expanded workout types from 9 to 21, each with distinct glyph and health mapping

• Added workout list filtering by type, duration, strain, and sort order

• Fixed calorie fabrication: unanchored profiles now show "—" instead of a stand-in body's estimate

• Added tests for calorie anchors, filter logic, and health mapping coverage

---

Diagram Walkthrough

flowchart LR
  A["Profile.hasCalorieAnchors\n(age + mass + sex)"] -- "gates" --> B["Live tick\ncalorie accrual"]
  A -- "gates" --> C["Substrate re-score\nmanual session"]
  B -- "null when unanchored" --> D["caloriesOrNull\n(LiveWorkoutState)"]
  D -- "dash or count-up" --> E["Finish card / share card\n/ session sheet"]
  F["kWorkoutTypes\n(21 types)"] -- "picker tiles" --> G["workoutTypeSheet\n(scrollable)"]
  F -- "health mapping" --> H["healthActivityForType\n(iOS + Android)"]
  I["WorkoutFilter\n(type/duration/strain/sort)"] -- "apply()" --> J["Filtered list\n+ recomputed summary"]
  J --> K["_FilterBar\n(workouts_screen)"]

Loading

File Walkthrough

	Relevant files
Bug fix	5 files profile.dart Add hasCalorieAnchors predicate for Keytel formula              +13/-0    manual_session.dart Gate substrate re-score calories on hasCalorieAnchors        +5/-1      app_state.dart Remove fallback defaults from live calorie tick, use caloriesOrNull +33/-12  live_session_screen.dart Propagate nullable calories through finish card and share +22/-10  workout_share_card.dart Accept nullable calories, dash out on share card                  +6/-2
Enhancement	6 files os_icons.dart Add 12 new sport glyphs for expanded workout types              +46/-0    workout_types.dart Expand picker from 9 to 21 types, fix scrollable sheet      +54/-14  workout_filter.dart New filter/sort policy for workout list                                    +166/-0  workout_filter_sheet.dart New bottom sheet UI for filter and sort controls                  +169/-0  workouts_screen.dart Wire filter bar, recompute summary over filtered list        +164/-44 health_export.dart Map 13 new workout types to platform health activities      +39/-0
Tests	4 files workout_calorie_anchors_test.dart Tests for hasCalorieAnchors and caloriesOrNull contract    +108/-0  workout_filter_test.dart Unit tests for filter, sort, and summarize logic                  +209/-0  workout_health_mapping_test.dart Extend health mapping tests for all new types                        +63/-1    workouts_header_actions_test.dart Update sheet test to assert scrollable grid reachability  +46/-41

---

Summary by CodeRabbit

• New Features

  • Added workout filtering and sorting by type, duration, strain, date, and workout metrics.
  • Added expanded workout categories, including boxing, rowing, hiking, climbing, skiing, snowboarding, stairs, Pilates, tennis, basketball, soccer, and golf.
  • Added scrollable workout-type selection, filter controls, live result counts, and summaries.
  • Added matching activity icons and improved health-platform activity mapping.

• Bug Fixes

  • Calorie estimates now remain unavailable when required profile information is missing.
  • Live sessions, summaries, and sharing show unavailable calories as “—” instead of zero.

[coderabbitai]

Warning

Review limit reached

@abdulsaheel, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 48 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 06c0449f-81aa-4766-8cee-8443967e592a

📥 Commits

Reviewing files that changed from the base of the PR and between 14acd22 and a8eb259.

📒 Files selected for processing (11)

• lib/compute/manual_session.dart
• lib/health/health_export.dart
• lib/state/app_state.dart
• lib/ui/kit/os_icons.dart
• lib/ui/workouts/workout_filter.dart
• lib/ui/workouts/workout_types.dart
• lib/ui/workouts/workouts_screen.dart
• test/workout_calorie_anchors_test.dart
• test/workout_filter_test.dart
• test/workout_health_mapping_test.dart
• test/workout_types_label_test.dart

📝 Walkthrough

Walkthrough

The change adds profile-based calorie eligibility, preserves unavailable calories as null, expands workout type mappings and icons, and adds workout filtering, sorting, summaries, and scrollable type-selection interfaces.

Changes

Calorie availability

Layer / File(s)	Summary
Calorie anchor contract lib/compute/profile.dart, lib/compute/manual_session.dart, test/workout_calorie_anchors_test.dart	Calorie scoring now requires age, weight, and sex. Height remains optional.
Live calorie state lib/state/app_state.dart, test/workout_calorie_anchors_test.dart	Live accrual and persistence retain unavailable calories as null.
Nullable calorie surfaces lib/ui/activity/live_session_screen.dart, lib/ui/activity/workout_share_card.dart, lib/ui/workouts/workouts_screen.dart	Finish screens, sharing, live metric sheets, and workout exports display — or preserve null values when calories are unavailable.

Workout discovery

Layer / File(s)	Summary
Workout type support lib/ui/kit/os_icons.dart, lib/ui/workouts/workout_types.dart, lib/health/health_export.dart, test/workout_health_mapping_test.dart, test/workouts_header_actions_test.dart	The app adds 12 workout types with shared labels, icons, strain tinting, health mappings, and scrollable selection.
Workout filter policy lib/ui/workouts/workout_filter.dart, test/workout_filter_test.dart	Filtering supports workout type, minimum duration, minimum strain, sorting, live-session retention, and filtered summaries.
Workout filter integration lib/ui/workouts/workout_filter_sheet.dart, lib/ui/workouts/workouts_screen.dart	The workout screen connects the filter sheet, active filter bar, filtered summaries, empty states, and non-chronological grouping.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  actor User
  participant WorkoutsScreen
  participant WorkoutFilterSheet
  participant WorkoutFilter
  User->>WorkoutsScreen: Open filters
  WorkoutsScreen->>WorkoutFilterSheet: Show current filter and workouts
  User->>WorkoutFilterSheet: Edit filters and apply
  WorkoutFilterSheet-->>WorkoutsScreen: Return WorkoutFilter
  WorkoutsScreen->>WorkoutFilter: Apply filter and sort
  WorkoutFilter-->>WorkoutsScreen: Return filtered workouts and summary

Loading

Possibly related PRs

• OpenStrap/edge#189: Shares calorie and nullable metric handling across session computation and live-session surfaces.
• OpenStrap/edge#209: Also expands healthActivityForType workout mappings.
• OpenStrap/edge#162: Shares workout-sharing changes in workout_share_card.dart.

Suggested labels: Review effort 4/5

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the three main changes: more workout types, workout-list filtering, and explicit handling of unavailable calorie estimates.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit a8eb259)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Calorie Milestone Re-fire The kcal milestone dedup key is 'k$kcalStep' where kcalStep is (kcal ~/ 100) * 100. Because caloriesOrNull returns calories.round() (an integer), and kcalStep is recomputed every second from the running total, the key is stable once the total crosses a century boundary — so the dedup guard should hold. However, if _caloriesScored is reset (e.g. the LiveWorkoutState object is recreated mid-session), caloriesOrNull returns null until the first new sample, then jumps back to the accumulated value, potentially re-firing a milestone that already fired. The more concrete risk: the old code fired on w.calories (a double that started at 0.0 and only grew), while the new code fires on w.caloriesOrNull which is null until accrueCalories is called. If the profile check hasCalorieAnchors becomes true mid-session (e.g. profile updated while a workout is live), _caloriesScored starts false, then becomes true, and the milestone fires for the first century even if the session has been running for 20 minutes and the user already saw it. This is a narrow scenario but the milestone dedup key is the only guard. final kcal = w.caloriesOrNull; if (kcal != null) { final kcalStep = (kcal ~/ 100) * 100; if (kcalStep >= 100) _milestone('k$kcalStep', '$kcalStep KCAL', "Burning clean.", AppColors.coral); } Context After Await Navigator.pop(context, _draft) at line 152 is called inside onPressed of a FilledButton — that is synchronous and safe. However, showWorkoutFilterSheet is called from _openFilter in workouts_screen.dart, which does await showWorkoutFilterSheet(context, ...) and then checks mounted before calling setState. That pattern is correct. No issue here on its own, but the _FilterSheet itself calls Navigator.pop(context, _draft) without a mounted check. If the sheet is dismissed by tapping the scrim (which calls Navigator.pop via the modal route) and then the button's onPressed fires (race on a slow device or test), a double-pop could occur. This is a low-probability scenario but consistent with the repo's documented pattern of missing mounted guards after navigation. onPressed: () => Navigator.pop(context, _draft), Absent Return in workoutTypeIcon The new workoutTypeIcon function calls resolveWorkoutTypeKey(type) and then looks up the result in kWorkoutTypesByKey, returning e?.$3 ?? OsIcon.strength. However, resolveWorkoutTypeKey returns null for unrecognised types (not 'other'), so kWorkoutTypesByKey[null] returns null, and the fallback OsIcon.strength is used — which is correct. But the autodetected and workout early-return guards above it use raw.contains(...) on the lowercased string, while resolveWorkoutTypeKey also lowercases. If a type is 'autodetected_workout', both guards fire and return early before the lookup, which is correct. The subtle issue: workoutTypeIcon no longer has an explicit return after the two early-return if blocks — the function body ends with return e?.$3 ?? OsIcon.strength which is the only return for the non-early-exit path. Dart requires all paths to return; this compiles only because the last line is unconditional. Verify the function compiles without a missing-return error (it should, but the structure is fragile if someone adds another early-exit branch above the final return without noticing there is no else-return below). OsIcon workoutTypeIcon(String? type) { final raw = (type ?? '').toLowerCase(); if (raw.contains('autodetected')) return OsIcon.strength; if (raw.contains('workout')) return OsIcon.strength; final e = kWorkoutTypesByKey[resolveWorkoutTypeKey(type)]; return e?.$3 ?? OsIcon.strength; }

---

⚠️ Review coverage: The following files were not included in this review because of the token budget:

• test/workout_calorie_anchors_test.dart
• lib/ui/kit/os_icons.dart
• test/workout_types_label_test.dart
• lib/health/health_export.dart
• lib/compute/manual_session.dart
• lib/compute/profile.dart

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to a8eb259

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Fix zone accumulator truncating longer zone lists The summarizeWorkouts function skips zone_min entirely when the key is absent from a row (the uncosted test fixture omits it), but the accumulator zoneSum grows only as long as the first row's zone list. If a later row has a longer zone_min list than the first non-empty one, the extra zones are silently dropped. Rows from the repo always carry 5 zones, but the filter test's uncosted fixture omits the key entirely — the real risk is a manually-constructed row with a different zone count corrupting the sum for all subsequent rows. lib/ui/workouts/workout_filter.dart [175-183] -if (minStrain > 0) { - final s = (w['strain'] as num?)?.toDouble(); - if (s == null || s < minStrain) return false; +final zm = (w['zone_min'] as List?) ?? const []; +for (var i = 0; i < zm.length; i++) { + final v = (zm[i] as num?) ?? 0; + while (zoneSum.length <= i) zoneSum.add(0); + zoneSum[i] += v; } Suggestion importance[1-10]: 5 __ Why: The existing code in summarizeWorkouts already handles the case where a later row has more zones than zoneSum by calling zoneSum.add(v) in the else branch, so the accumulator does grow dynamically. However, the suggestion's improved_code replaces the if/else with a while pre-fill approach which is cleaner and more explicit. The bug described (truncation) doesn't actually exist in the current code, making this a minor style improvement rather than a bug fix.	Low
	Unify health export alias resolution through single seam The healthActivityForType switch handles alias strings like 'row', 'hiking', 'climbing', 'skiing', 'snowboarding', 'stair', 'squash', 'padel', 'badminton', 'football' directly in the switch. However, the canonical type key for these aliases is already resolved by resolveWorkoutTypeKey / canonicalWorkoutType in workout_types.dart. The switch is a second hand-maintained alias table that can drift from kWorkoutTypeAliases. Per AGENTS §4.7 ("capability wired into one call path but not all N"), the health export switch should resolve through the single alias seam rather than duplicating it, so that a new alias added to kWorkoutTypeAliases automatically reaches health export without a second edit. lib/health/health_export.dart [1123-1126] -case 'rowing': -case 'row': - return HealthWorkoutActivityType.ROWING; -case 'hike': -case 'hiking': - return HealthWorkoutActivityType.HIKING; +HealthWorkoutActivityType healthActivityForType( + String? type, { + required bool ios, +}) { + // Resolve aliases through the single vocabulary seam so kWorkoutTypeAliases + // is the one place to add a new spelling. + final key = resolveWorkoutTypeKey(type) ?? (type ?? '').toLowerCase().trim(); + switch (key) { + case 'run': + return HealthWorkoutActivityType.RUNNING; + // ... existing cases using canonical keys only ... + case 'rowing': + return HealthWorkoutActivityType.ROWING; + case 'hike': + return HealthWorkoutActivityType.HIKING; + // etc. + default: + return HealthWorkoutActivityType.OTHER; + } +} Suggestion importance[1-10]: 4 __ Why: The suggestion correctly identifies that healthActivityForType duplicates alias resolution already present in kWorkoutTypeAliases, creating a maintenance risk where new aliases must be added in two places. However, the improved_code is incomplete (uses placeholder comments) and the existing switch also handles platform-specific routing (e.g., soccer on Android vs iOS) that can't be trivially collapsed into a key-only lookup.	Low
	Guard Navigator.pop with mounted check Navigator.pop is called with context after the sheet may have been disposed or after an await in a parent. Inside a StatefulWidget that can be unmounted, Navigator.pop should be guarded with a mounted check. Although this is a synchronous callback, the context here belongs to _FilterSheetState and should use the widget's own context only when mounted. lib/ui/workouts/workout_filter_sheet.dart [152] -onPressed: () => Navigator.pop(context, _draft), +onPressed: () { + if (mounted) Navigator.pop(context, _draft); +}, Suggestion importance[1-10]: 2 __ Why: The onPressed callback is synchronous and fires directly from a button tap, so the widget is guaranteed to be mounted when it runs. The mounted guard is unnecessary here and adds noise without real safety benefit.	Low

---

Previous suggestions

Suggestions up to commit c0db836

Category	Suggestion	Impact
Possible issue	Health export switch must normalize case before matching The healthActivityForType switch receives the raw stored type string, which per the codebase is free-form and may arrive in any case. All other call sites in workout_types.dart normalize to lowercase before lookup, but healthActivityForType does not — a stored value of 'Tennis' or 'BOXING' falls through to OTHER instead of matching its case. The function should normalize the input at the top of the switch, consistent with every other lookup in this file's sibling functions. lib/health/health_export.dart [1147-1152] -case 'tennis': -case 'racquet': -case 'squash': -case 'padel': -case 'badminton': - return HealthWorkoutActivityType.TENNIS; +HealthWorkoutActivityType healthActivityForType( + String? type, { + required bool ios, +}) { + switch ((type ?? '').toLowerCase().trim()) { + // ... existing cases unchanged ... + case 'tennis': + case 'racquet': + case 'squash': + case 'padel': + case 'badminton': + return HealthWorkoutActivityType.TENNIS; Suggestion importance[1-10]: 6 __ Why: The healthActivityForType function does not normalize the input type string to lowercase before the switch, meaning mixed-case stored values like 'Tennis' or 'BOXING' would fall through to OTHER. This is a real correctness issue consistent with how other lookup functions in the codebase handle normalization.	Low
	Absent calorie total should be null, not zero When every session in the filtered list has a null calorie figure, total_calories is 0 — indistinguishable from a real zero-calorie total. The summary should carry a nullable calories field (or omit the key) so the screen can render "—" rather than "0 kcal" above a list of uncosted sessions, consistent with the per-session fix applied everywhere else in this PR. lib/ui/workouts/workout_filter.dart [182-188] return { 'count': count, 'total_min': totalMin, - 'total_calories': totalCal, + if (totalCal > 0 || workouts.any((w) => + w['status'] != 'live' && (w['calories'] as num?) != null)) + 'total_calories': totalCal, 'zone_min': zoneSum, }; Suggestion importance[1-10]: 5 __ Why: When all sessions have null calories, total_calories returns 0, which is ambiguous. However, the improved code uses a complex condition that may be hard to read, and the screen consuming this summary may already handle the missing key gracefully. The concern is valid but the fix is non-trivial.	Low

Suggestions up to commit b4bced4

Category	Suggestion	Impact
Possible issue	Filter chip misses aliased workout type strings The tennis entry in kWorkoutTypes uses the key 'tennis' but the health exporter also accepts aliases like 'racquet', 'squash', 'padel', and 'badminton' as the same type. However, canonicalWorkoutType only matches against e.$1 (the canonical key), so a session stored with type 'racquet' or 'squash' will collapse to 'other' instead of 'tennis', making the "Racquet" filter chip unable to catch those rows. The same multi-alias pattern used in healthActivityForType should be applied here, or the stored type should always be the canonical key. lib/ui/workouts/workout_filter.dart [36-42] String canonicalWorkoutType(String? raw) { final t = (raw ?? '').toLowerCase(); + // Multi-alias groups: the health exporter accepts these spellings for the + // same canonical key, so the filter chip must catch them too. + const _aliases = <String, String>{ + 'row': 'rowing', + 'hike': 'hike', + 'hiking': 'hike', + 'climb': 'climb', + 'climbing': 'climb', + 'ski': 'ski', + 'skiing': 'ski', + 'snowboard': 'snowboard', + 'snowboarding': 'snowboard', + 'stair': 'stairs', + 'racquet': 'tennis', + 'squash': 'tennis', + 'padel': 'tennis', + 'badminton': 'tennis', + 'soccer': 'soccer', + 'football': 'soccer', + }; + if (_aliases.containsKey(t)) return _aliases[t]!; for (final e in kWorkoutTypes) { if (e.$1 == t) return t; } return 'other'; } Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that canonicalWorkoutType only matches canonical keys, so sessions stored with alias types like 'racquet' or 'football' would collapse to 'other' instead of their canonical type. However, the PR's design intent seems to be that the stored type column always uses the canonical key (e.g., 'tennis', 'soccer'), with aliases only handled at the health export layer. The issue is real but may be a pre-existing data concern rather than a regression introduced by this PR.	Low
	Sex comparison is case-sensitive, silently using wrong formula branch The hasCalorieAnchors guard ensures w.profile.sex is non-null before this branch is reached, but the comparison only checks 'f' and 'female' in lowercase. The profile's sex field is described as free-form text from the database, and workoutTypeLabel already demonstrates that mixed-case values arrive from older rows. If sex is stored as 'F' or 'Female', the formula silently uses the male branch, producing a systematically wrong calorie figure for female users — the exact fabrication this PR is trying to fix. lib/state/app_state.dart [4423] -final female = w.profile.sex == 'f' || w.profile.sex == 'female'; +final female = w.profile.sex?.toLowerCase() == 'f' || + w.profile.sex?.toLowerCase() == 'female'; Suggestion importance[1-10]: 5 __ Why: The suggestion is valid — if sex is stored as 'F' or 'Female', the female branch would be missed and the male formula applied. However, the hasCalorieAnchors guard already uses sex != null, and the profile's sex field is likely normalized at write time. The improved_code uses ?.toLowerCase() which is safe but redundant given the guard, making this a minor defensive improvement rather than a critical fix.	Low

Suggestions up to commit 7d44eb8

Category	Suggestion	Impact
Possible issue	Alias strings collapse to wrong filter bucket The health_export.dart switch handles alias strings like 'row', 'hiking', 'climbing', 'skiing', 'football', 'racquet', 'squash', 'padel', 'badminton', and 'stair' that are not in kWorkoutTypes. When a session stored with one of these alias strings is loaded, canonicalWorkoutType collapses it to 'other', making it unreachable by its natural type filter chip (e.g. filtering by "Rowing" won't match a session stored as 'row'). The canonical lookup should mirror the same alias mapping used in healthActivityForType so that stored aliases resolve to their canonical key. lib/ui/workouts/workout_filter.dart [36-42] +// Alias map mirrors the aliases in healthActivityForType so that sessions +// stored with an older/alternate spelling still reach the right filter chip. +const _typeAliases = <String, String>{ + 'row': 'rowing', + 'hiking': 'hike', + 'climbing': 'climb', + 'skiing': 'ski', + 'snowboarding': 'snowboard', + 'stair': 'stairs', + 'racquet': 'tennis', + 'squash': 'tennis', + 'padel': 'tennis', + 'badminton': 'tennis', + 'football': 'soccer', +}; + String canonicalWorkoutType(String? raw) { final t = (raw ?? '').toLowerCase(); + final resolved = _typeAliases[t] ?? t; for (final e in kWorkoutTypes) { - if (e.$1 == t) return t; + if (e.$1 == resolved) return resolved; } return 'other'; } Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that alias strings like 'row', 'hiking', 'football' etc. used in healthActivityForType are not in kWorkoutTypes, so canonicalWorkoutType collapses them to 'other' instead of their canonical type. This is a real functional issue where filtering by "Rowing" won't match sessions stored as 'row'. The improved code is accurate and addresses the problem.	Low
General	Filter sort runs on UI isolate every build frame _draft.apply(widget.workouts) is called on every build, which runs the full filter+sort pass (including a list sort) on the UI isolate each frame the sheet rebuilds. Per AGENTS.md §4.4, heavy work must not run on the UI isolate. Cache the match count in state and recompute it only when _draft changes (i.e. inside setState), so the build method reads a pre-computed integer rather than re-sorting on every frame. lib/ui/workouts/workout_filter_sheet.dart [53] -final matches = _draft.apply(widget.workouts).length; +class _FilterSheetState extends State<_FilterSheet> { + late WorkoutFilter _draft = widget.current; + late int _matches = widget.current.apply(widget.workouts).length; + void _toggleType(String key) { + final next = Set<String>.from(_draft.types); + next.contains(key) ? next.remove(key) : next.add(key); + setState(() { + _draft = _draft.copyWith(types: next); + _matches = _draft.apply(widget.workouts).length; + }); + } + + @override + Widget build(BuildContext context) { + return SafeArea( + top: false, + child: ConstrainedBox( + constraints: BoxConstraints( + maxHeight: MediaQuery.sizeOf(context).height * 0.8, + ), + child: Padding( + padding: const EdgeInsets.all(Sp.x5), + child: Column( + mainAxisSize: MainAxisSize.min, + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Row( + children: [ + Expanded(child: Text('Filter workouts', style: AppText.h2)), + if (!_draft.isDefault) + TextButton( + onPressed: () => setState(() { + _draft = const WorkoutFilter(); + _matches = _draft.apply(widget.workouts).length; + }), + child: const Text('Clear'), + ), + ], + ), + // ... rest of build unchanged, replacing `matches` with `_matches` + Suggestion importance[1-10]: 4 __ Why: The suggestion is valid — calling _draft.apply(widget.workouts) on every build does redundant work. However, the workouts list in this context is typically small (a single timeframe's sessions), so the performance impact is minimal. The improved code correctly caches _matches in state and updates it only in setState, but the _toggleType method shown doesn't update _matches for all state changes (e.g., duration/strain/sort changes are not shown).	Low

Suggestions up to commit ab7a385

Category	Suggestion	Impact
General	Distinguish zero-calories-measured from no-calories-measured in summary When calories is present but zero (a legitimately scored session that burned nothing), it is correctly included. However, totalCal is always returned even when every session in the filtered list had a null calorie figure, making it impossible for the caller to distinguish "total is zero because nothing was measured" from "total is zero because everything was measured and burned nothing". The summary map should carry a nullable total_calories so the UI can show "—" instead of "0 kcal" for an entirely unanchored set. lib/ui/workouts/workout_filter.dart [156-157] +final cal = (w['calories'] as num?)?.toInt(); +if (cal != null) totalCal += cal; - Suggestion importance[1-10]: 3 __ Why: The suggestion raises a valid semantic concern about distinguishing "nothing measured" from "measured zero", but the existing_code and improved_code are identical, meaning no actual code change is proposed. The current behavior (returning totalCal as 0 in both cases) may be acceptable given the UI already handles null calories per-session.	Low
	Clarify live-session early-return only skips numeric floors A live session whose type does not match the active type filter is correctly excluded by the first guard. However, a live session whose type does match will pass the type guard and then immediately return true, bypassing the minMinutes and minStrain floors. That is intentional per the comment, but the comment only explains the numeric floors — the type filter is a separate decision and the code already handles it correctly before the early return. The logic is correct but the ordering makes the intent hard to verify; no code change is needed here, but the comment should clarify that the early return only applies after the type check has already passed. lib/ui/workouts/workout_filter.dart [95-102] if (types.isNotEmpty && !types.contains(canonicalWorkoutType(w['type'] as String?))) { return false; } -// The numeric floors are different: a session happening right now has no -// final duration or strain to clear them with, so holding it to them would -// hide the one thing the user is most likely looking at. +// Numeric floors only apply to finished sessions — a live session has no +// final duration or strain yet, so any floor would hide the one workout +// the user is most likely watching. The type filter above still applies. if (w['status'] == 'live') return true; Suggestion importance[1-10]: 2 __ Why: This is a comment-only suggestion that adds minor clarification to already-correct logic. The existing comment already explains the intent adequately, and this is a documentation improvement with minimal impact.	Low

[abdulsaheel]

Went through the three flagged spots.

The summary total was worth changing. Adding ?? 0 and skipping a null come to the same number, so the sum was never wrong — but writing it as a default invites someone to later swap the zero for a "reasonable" estimate, which is exactly the fabrication the rest of the change removes. It skips explicitly now, and the test says what it means: an uncosted session still counts as a session and still contributes its minutes, it just adds nothing to the kcal total.

The mid-session profile change can't happen. LiveWorkoutState.profile is final and is only ever set at construction, from the profile the session is started under — that's deliberate, a workout is scored against the profile it was performed under rather than whatever the profile says when it ends. Filling in a profile during a workout leaves the running session alone; the substrate re-score picks it up afterwards and costs the whole window properly, which is the path that was built for exactly this.

The navigator one you withdrew yourself on the second look — agreed, the mounted guard is there.

[coderabbitai]

Actionable comments posted: 2

🤖 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 `@lib/ui/workouts/workout_filter.dart`:
- Around line 91-106: Update _matches to evaluate the explicit types filter
before the live-session early return, so a live workout is retained only when
its canonical type matches; keep the live exception for duration and strain
thresholds. Add a regression test covering types {'run'} and a live cycle
session, asserting it is excluded.

In `@lib/ui/workouts/workout_types.dart`:
- Around line 69-74: Normalize the input once in workoutTypeLabel by storing
type.toLowerCase() in a local key, then compare that key with e.$1 during the
kWorkoutTypes lookup so case variants return the configured label. Add a
regression test covering an uppercase or mixed-case type such as TENNIS and
verifying it returns the mapped Racquet label.

🪄 Autofix

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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: bb60fd4e-90de-491e-b138-d7dbe5404e68

📥 Commits

Reviewing files that changed from the base of the PR and between 3b4a44b and b1f452e.

📒 Files selected for processing (15)

• lib/compute/manual_session.dart
• lib/compute/profile.dart
• lib/health/health_export.dart
• lib/state/app_state.dart
• lib/ui/activity/live_session_screen.dart
• lib/ui/activity/workout_share_card.dart
• lib/ui/kit/os_icons.dart
• lib/ui/workouts/workout_filter.dart
• lib/ui/workouts/workout_filter_sheet.dart
• lib/ui/workouts/workout_types.dart
• lib/ui/workouts/workouts_screen.dart
• test/workout_calorie_anchors_test.dart
• test/workout_filter_test.dart
• test/workout_health_mapping_test.dart
• test/workouts_header_actions_test.dart

[github-actions]

Persistent review updated to latest commit 14acd22

[abdulsaheel]

Both of these were right.

The live-session exception was too broad. Type is known the moment a session starts, so filtering to runs should never surface the ride that happens to be in progress — only the duration and strain floors need the exception, because a session two minutes old has not had a chance to clear them yet. Split accordingly, with a test for the live-cycle-under-a-run-filter case.

The case sensitivity was a real gap, and it was in all three lookups rather than just the label one — the icon lookups compared against the raw string too, so a row written as Strength would have fallen through to the generic fallback glyph. The stored type column is free-form text and older rows and imports do carry mixed case, so all three normalize once now. There is also a test asserting every key in the table is lowercase, since that is the assumption the normalization rests on.

[github-actions]

Persistent review updated to latest commit ab7a385

[abdulsaheel]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[abdulsaheel]

The strain sort one was worth taking. Null strain means the session was never scored — no profile anchor, or no heart rate for the window — and ranking that as a genuine zero puts "we could not score this" in among "you barely moved". It sorts below every scored session now, including real zeros.

The same question applies to the filter, and there the answer goes the other way: a strain floor excludes an unscored session, because it cannot be shown to clear the bar and listing it under "strain 10+" would be a claim there is no evidence for. Both are pinned by tests.

The summary total was already changed in an earlier commit on this branch — it skips a null rather than defaulting it, for exactly the reason described.

[github-actions]

Persistent review updated to latest commit 7d44eb8

[abdulsaheel]

Ran a second pass over this myself, since CodeRabbit's last run came back rate limited rather than actually reviewed. It found one thing that should not have shipped.

soccer mapped to HealthWorkoutActivityType.SOCCER, which is in the health plugin's Dart-side list but commented out of Health Connect's Kotlin write map. That combination is worse than issue #184 rather than the same as it: #184 threw before the platform channel and failed only itself, whereas this passes the Dart guard, reaches the channel, and comes back success(false). The exporter treats a false as a genuine write failure and counts it toward that day's give-up budget — so a single soccer workout would have silently paused that whole day's export, resting HR and sleep along with it. It maps to OTHER on Android now.

The test I added specifically to prevent this class of bug could not have caught it, which is the more useful half of the finding: its Android set was transcribed from the plugin's Dart list, and for Android that is the wrong source of truth. The Dart list is advisory; the Kotlin map is the gate, and here the two disagree. Both sets are now pinned against the file that actually decides, and the header says which one that is per platform.

Two smaller things from the same pass. hike now records a GPS route — it sits directly beside walk in the picker and picking it signals more outdoor intent, not less, and route points cannot be backfilled. Ski and snowboard deliberately do not: the route screen leads with distance and pace, and pace down a lift-served descent would read as a performance figure while measuring gravity. And climb was drawing mountains right next to a person hiking, where the mountains were the tile that read as hiking; it is a ladder now.

[github-actions]

Persistent review updated to latest commit b4bced4

[abdulsaheel]

Second self-review pass, this one on correctness. Six findings, five taken and one rejected — the rejected one is the interesting one.

Taken. The workout detail screen still printed 0 kcal for a null column, which is the exact failure this PR exists to remove, sitting in the most-viewed place a session appears — and the share card exported from that same screen already read "—". A live session with a complete profile but no heart rate at all (link dropped, strap off) accrued nothing and reported a confident 0; "can we score this" and "did we score this" are different questions and both have a zero-shaped answer, so calories track whether the estimate ever ran now, and agree with strain, which already called that case absent. The filter pill could overflow its row — "Basketball, Snowboard · 90m+ · strain 17+" is reachable and the header is about 350 pt wide — so it ellipsizes. A filtered summary was dropping the auto-type accuracy note, which describes the whole range and has nothing to do with a filter. And imported sessions carry the export's own spelling, so filtering by Run hid every imported run while the feed showed cards titled "Running" — there is one alias table now, shared by the filter and by every label and icon lookup, so a row cannot read as one type and filter as another.

Rejected, after trying it. The observation that a calorie figure fabricated by an older build is never cleared by a re-score is correct. The obvious fix is to let a complete substrate's null be authoritative instead of falling back to the stored value — I made that change, and session_score_reconcile_test.dart caught it immediately, which is what it was written for. That fallback is deliberate: a null max HR from a complete substrate means no worn samples survived, while the live tally actually watched the session happen, and a null strain means the profile no longer carries the anchor the score needs. Neither is grounds for destroying a real measurement taken when it could be. Healing the old fabricated values that way would also wipe legitimately scored sessions whenever the substrate happens not to be able to score them, which is the worse trade. Reverted, and the reasoning is now written down at the call site so the next person does not have to rediscover it.

So: sessions logged before this change keep whatever the old tick wrote. Everything logged after it is honest, and no migration deletes history on a guess.

[github-actions]

Persistent review updated to latest commit c0db836

[abdulsaheel]

Both taken.

The summary total was the same fabrication one level up: summarizeWorkouts started at 0 and stayed 0 when every session in the range was uncosted, so the card would have read "0 kcal" for a timeframe where nothing could be costed at all. It returns null now and the card dashes out, matching what a single session already did.

The two lookup mechanisms could not actually have diverged — kWorkoutTypesByKey is a comprehension over kWorkoutTypes, so adding a type to one adds it to the other by construction. But having one helper walk the list while its sibling used the map was pointless inconsistency, so all four now go through the map.

[github-actions]

Persistent review updated to latest commit a8eb259

[abdulsaheel]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.