App-wide refactor and feature expansion - #48
Merged
Merged
Conversation
- Implemented WorkoutSummaryScreen to display post-workout details including duration, volume, sets, and exercises. - Added visual elements such as trophy header and muscle groups trained section. - Refactored AppTheme to centralize color management through AppColors, improving maintainability and consistency across the app. - Updated theme properties for better visual coherence and modern styling.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughLarge refactor spanning shared UI, workout flow, analytics, AI screens, models, startup wiring, and platform/config updates. ChangesApp-wide product and UI refresh
|
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 35
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
workout-logger/lib/screens/edit_workout_session_screen.dart (1)
41-58:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAliasing of
s.dropsshares mutable state with the original session.
EditableSet(... drops: s.drops, ...)copies the reference, not the contents._EditableSetRowState's callbacks always replace the list (good), so today this works, but any future code that doeseditableSet.drops!.add(...)/removeAt(...)will silently mutate the persistedWorkoutSessionbefore_saveruns (and even if the user discards changes). Defensive copy keeps the snapshot semantics intact:(s) => EditableSet( weight: s.weight, reps: s.reps, isDropset: s.isDropset, - drops: s.drops, + drops: s.drops == null + ? null + : List<DropsetEntry>.from(s.drops!), timeTaken: s.timeTaken, timestamp: s.timestamp, ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/screens/edit_workout_session_screen.dart` around lines 41 - 58, The EditableSet construction in the _exercises mapping aliases the drops list (drops: s.drops), which can cause shared-mutation with widget.session.exercises; change it to create a defensive copy of the drops list (e.g., a new List from s.drops) when constructing EditableSet inside the EditableExerciseLog map so subsequent mutations to EditableSet.drops (via _EditableSetRowState or elsewhere) do not mutate the original WorkoutSession before _save is called. Ensure the copy handles nullability (preserve null vs empty-list semantics) consistent with existing code.
🤖 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 `@workout-logger/lib/screens/analytics_screen.dart`:
- Around line 318-324: The week-bucketing logic in build uses w =
now.difference(s.date).inDays ~/ 7 which can be negative for future dates and
thus creates negative keys in weeks; update the guard to only increment when w
is within 0..3 (e.g., check w >= 0 && w < 4) before doing weeks[w] = (weeks[w]
?? 0) + 1 so negative w values are ignored and the weeks map isn’t mutated with
negative keys (refer to build, provider.sessions, weeks, and variable w).
- Around line 252-274: The chart code computes maxVol from top.first.value and
then divides by it, which can produce NaN when the top value is 0; update the
logic in the block that builds the muscle chart (symbols: byMuscle, sorted, top,
maxVol, and the mapping that computes pct for RFProgressBar) to guard against
maxVol == 0 — either early-return the _ChartCard with isEmpty: true (like the
byMuscle.isEmpty case) or set pct = 0 when maxVol == 0 so RFProgressBar receives
a valid numeric value; ensure the chosen approach is applied before computing
pct and volStr.
- Around line 244-310: The muscle/volume charts hardcode unit labels and unsafe
calculations; in _MuscleVolumeChart and _VolumeChart grab SettingsProvider via
context.watch<SettingsProvider>() in build(), convert displayed values with
settings.toDisplay(value) and append settings.unitLabel instead of hardcoded
"kg"/"t" or string concatenation, update the subtitle in _VolumeChart to use
settings.unitLabel and converted totals, and ensure you guard against division
by zero in _MuscleVolumeChart (use pct = maxVol > 0 ? entry.value / maxVol : 0)
and filter week offsets in _FrequencyGrid (only include w when w >= 0 && w < 4)
so future-dated sessions don’t corrupt the weeks map.
In `@workout-logger/lib/screens/edit_workout_session_screen.dart`:
- Around line 419-439: The helper _sectionLabel currently uses four positional
parameters (BuildContext context, IconData icon, Color color, String label)
which is error-prone; change its signature to drop the unused BuildContext and
use named parameters (e.g. Widget _sectionLabel({required IconData icon,
required Color color, required String label})) and update all call sites to pass
arguments by name (icon:, color:, label:); ensure any imports/types remain and
run the analyzer to fix any resulting errors.
In `@workout-logger/lib/screens/exercise_library_screen.dart`:
- Around line 410-437: The inline TextField duplicates the existing widget
_SearchBar and omits its clear-button and styling; refactor to reuse _SearchBar
by updating _SearchBar to accept required String query and required
ValueChanged<String> onChanged, then replace the inline Container/TextField
block with a call to _SearchBar(query: _query, onChanged: (v) => setState(() =>
_query = v)); ensure the _SearchBar preserves the original decoration (prefix
icon, contentPadding, border) and clear-button affordance so both screens share
identical behavior.
In `@workout-logger/lib/screens/history_screen.dart`:
- Around line 414-425: The _menuItem function uses four positional parameters
which violates the guideline to use named parameters for functions with three or
more arguments; change its signature to use named parameters (e.g.,
_menuItem({required String value, required IconData icon, required String label,
required Color color})) and update every call site in the itemBuilder to pass
arguments by name (value:, icon:, label:, color:), keeping the returned
PopupMenuItem construction unchanged and preserving types and requiredness.
In `@workout-logger/lib/screens/home_screen.dart`:
- Around line 210-220: The bottom-nav index logic wrongly subtracts 1 from the
raw _items index (causing Analytics/Profile to target the wrong tabs); fix by
computing the visible nav item index (the Nth non-placeholder in _items) and use
that for selected and onTap instead of (i < 2 ? i : i - 1). Locate the loop
iterating _items and replace the ternary expressions for selected and onTap with
a computed visibleIndex (e.g., visibleIndex = _items.sublist(0, i +
1).where((elem) => elem.$1 != null).length - 1) and use selected: currentIndex
== visibleIndex and onTap: () => onTap(visibleIndex) so indexes align with the
IndexedStack/switchTab and _buildQuickActions.
In `@workout-logger/lib/screens/routines_screen.dart`:
- Around line 26-30: Both _RoutinesHeader and _RoutinesTab are immutable
StatelessWidget classes but lack const constructors, preventing const
instantiation; add explicit const constructors to the classes (e.g., const
_RoutinesHeader(); const _RoutinesTab();) and then update their usages (the
instantiations of _RoutinesHeader() and _RoutinesTab() at the TabBar/TabBarView
and the other occurrences mentioned) to use const so they comply with
prefer_const_constructors_in_immutables and flutter_lints.
In `@workout-logger/lib/screens/widgets/dashboard_widgets.dart`:
- Around line 117-125: The Volume card currently hardcodes "Volume (kg)" and
formats weeklyVolume via _formatVolume without respecting user units; update the
caller that renders StatGridCard to read SettingsProvider.unitLabel (and units
type) and pass a unit-aware label and a converted value: convert weeklyVolume to
imperial when needed before calling _formatVolume, then call StatGridCard(icon:
..., value: _formatVolume(convertedWeeklyVolume), label: 'Volume
(${settings.unitLabel})', ...). Reference symbols to change: StatGridCard usage
in this widget, the weeklyVolume variable, and the _formatVolume call; follow
the same approach used in rf_cards.dart and exercise_details_sheet.dart where
the caller supplies unit-aware label/formatter from SettingsProvider.
- Around line 18-24: The startOfWeek calculation preserves the current time
causing late-Sunday sessions from the previous week to be counted; fix by
truncating to local midnight before subtracting weekdays: create todayMidnight
from DateTime(today.year, today.month, today.day), compute startOfWeekMidnight =
todayMidnight.subtract(Duration(days: today.weekday - 1)), then update the
filter on sessions (used in trainedDays) to include sessions where
!s.date.isBefore(startOfWeekMidnight) (or use isAfter/isAtSameMomentAs against
startOfWeekMidnight) so comparisons use date-only boundaries.
In `@workout-logger/lib/screens/widgets/editable_exercise_card.dart`:
- Around line 122-142: The keys for rows use list indexes
(ValueKey('set_${exerciseName}_$i') and
ValueKey('drop_${widget.setNumber}_$i')), which causes state loss on
reorder/delete; add a stable id to the model (e.g., add a final String id on
EditableSet and DropsetEntry populated once when created — use UUID or
timestamp) and change the key usage in EditableExerciseCard and EditableSetRow
to ValueKey(set.id) / ValueKey(drops.id) (or
ValueKey('${exerciseName}_${set.id}')) so keys remain stable across list
mutations and TextEditingController/FocusNode state is preserved. Ensure new id
is assigned when sets/drops are created/copied and used wherever keys are
currently derived from the index.
In `@workout-logger/lib/screens/widgets/exercise_details_sheet.dart`:
- Around line 288-305: The delete path currently only shows a success SnackBar
when provider.deleteCustomExercise(exercise.id) returns true, leaving failures
silent; update the block in exercise_details_sheet.dart (the code using
confirmed, provider.deleteCustomExercise, ScaffoldMessenger.of(context) and
Navigator.of(context)) to add an else branch that leaves the sheet open and
calls messenger.showSnackBar with an error message (e.g., '"${exercise.name}"
could not be deleted') and an error-appropriate backgroundColor so users get
visible feedback when deleteCustomExercise returns false.
- Line 197: The Last Session chips hardcode "kg" and raw numbers (e.g.,
'${s.weight}kg × ${s.reps}' and '+${...} kg volume/session'), so read the app
SettingsProvider in the widget (e.g., via final settings =
context.watch<SettingsProvider>() or Provider.of<SettingsProvider>(context)) and
replace direct interpolations with unit-aware formatting: use
settings.formatWeight(s.weight) (or settings.formatWeight(...) for the computed
volume) and settings.unitLabel for any unit text; update the chip rendering and
the '+... volume/session' text accordingly to call settings.formatWeight(...)
instead of embedding "kg" and raw values so values and unit labels follow user
settings (refer to existing usage in _OneRMCard and _SessionHistory for
examples).
In `@workout-logger/lib/screens/widgets/exercise_input_section.dart`:
- Around line 191-192: The AI-suggestion Icon in exercise_input_section.dart is
fully const-evaluable but not declared const; update the widget by adding the
const keyword before the Icon constructor (the child: Icon(...) expression) so
it becomes a const Icon — this satisfies prefer_const_constructors and uses the
existing static const AppColors.primary.
- Around line 141-145: The increment-label formatting in _RecommendationCard
uses inc.toStringAsFixed(2).replaceAll(RegExp(r'0+$'), '') which can produce a
trailing dot (e.g., "1."); update the formatting logic for the variable handling
the increment label (the code referencing inc inside _RecommendationCard) to
also strip a trailing '.' after removing zeros (e.g., apply an additional
replace/removal for a trailing dot or use a single regex that removes the
fractional part including the dot when only zeros remain) so labels render as "1
kg" not "1. kg".
In `@workout-logger/lib/screens/widgets/exercise_progress_view.dart`:
- Around line 26-30: The code recomputes the performed set on every provider
notification because it uses context.watch<WorkoutProvider>() and iterates
sessions→exercises; change this to use a scoped selector so the widget only
rebuilds when the underlying exercise IDs change—for example replace
context.watch<WorkoutProvider>() with context.select<WorkoutProvider,
Set<String>>((p) => p.sessions.fold(<String>{}, (acc, s) { for (final e in
s.exercises) acc.add(e.exerciseId); return acc; })) to compute performed (or add
a WorkoutProvider getter like performedExerciseIds and select that) so the heavy
nested loop is only executed when the sessions/exercise IDs actually change.
- Around line 102-119: The DropdownButton can throw when its value
(selected/_selectedId) is not present in the items (ids); update the widget to
guard/reset the selection: ensure the DropdownButton's value is either null or
one of ids (e.g., use a conditional like ids.contains(selected) ? selected :
null) or reset _selectedId in the parent build before passing it down; reference
DropdownButton, selected, ids, and the parent state variable _selectedId and
onChanged when making the change so the value passed into DropdownButton always
matches one of the items or is null.
In `@workout-logger/lib/screens/widgets/profile_sections.dart`:
- Around line 568-573: Add const constructors to the two stateless widgets by
declaring const _SectionDivider() and const _ComingSoonBadge() (they have no
fields), then update all places that instantiate them (the existing calls to
_SectionDivider() and _ComingSoonBadge()) to use const _SectionDivider() and
const _ComingSoonBadge() so those widgets can be canonicalized and avoid
unnecessary rebuild allocations; ensure you update the class declarations for
_SectionDivider and _ComingSoonBadge and replace each call site accordingly.
- Line 200: Both screens define duplicate private constants named _hcColor with
different values; centralize the Health Connect brand color inside AppColors and
update callers. Add a single public/static color constant on AppColors (e.g.,
AppColors.healthConnect = Color(0xFF4ECDC4) using the desired teal), remove the
private _hcColor declarations from profile_sections.dart and
session_details_sheet.dart, and replace all uses of _hcColor in those files with
AppColors.healthConnect so both surfaces render the same brand color.
In `@workout-logger/lib/screens/widgets/rf_cards.dart`:
- Around line 30-36: SessionCard and RecentSessionTile currently append a
hardcoded "kg" to session volume (see variables day/month/weekday/volume/volStr)
which breaks non-metric users; update the components to accept a caller-provided
preformatted volumeLabel (preferred) or an optional unit/formatter param (e.g.,
volumeLabel String or formatVolume/ unitLabel) and use that instead of appending
' kg', or else call SettingsProvider.formatWeight()/unitLabel inside
SessionCard/RecentSessionTile to format/label the volume; update callers to pass
the correctly formatted volumeLabel (using SettingsProvider) where these widgets
are instantiated.
- Around line 184-217: The current regex parsing of value (creating numericValue
via RegExp(r'[^0-9.]') and extracting suffix with RegExp(r'[0-9.]')) is fragile
(e.g., "1.5k", "1,000", "-5", "$100"); instead, change the widget API to accept
an explicit double? animatedValue and String? displaySuffix provided by the
caller (the caller which already has the raw numeric data), update the rendering
logic around AnimatedCounter to use animatedValue directly
(AnimatedCounter(value: animatedValue, suffix: displaySuffix)) and if
animatedValue is null, render plain Text(value) — remove the regex parsing and
numericValue variable and update any constructors/uses of this RF card widget
and references to AnimatedCounter accordingly.
In `@workout-logger/lib/screens/widgets/rf_inputs.dart`:
- Around line 537-548: The long-press handlers create Timer.periodic instances
that are never cancelled; add a Timer field (e.g. _holdTimer) to the State that
owns the step buttons, set _holdTimer = Timer.periodic(...) in the long-press
start handler and call _holdTimer?.cancel() in onLongPressEnd, onTap handlers
and State.dispose(), update the decrement/increment button usages to use
onLongPressStart/onLongPressEnd (or pass both callbacks into _StepButton) and
mirror the same pattern used by RFNumberField to ensure timers are always
cancelled.
- Around line 220-262: The inline TextField can remain stuck in edit mode
because losing focus (tapping outside) isn't handled; modify the TextField
inside the GestureDetector to handle focus loss by calling the existing
_commitEdit() (or cancel behavior) — either add the onTapOutside handler on the
TextField to call _commitEdit(), or wrap the TextField with a Focus widget and
use onFocusChange to invoke _commitEdit() when focus becomes false; ensure this
integrates with the existing _editing and _ctrl state so the UI exits edit mode
correctly.
In `@workout-logger/lib/screens/widgets/rf_widgets.dart`:
- Around line 60-68: The AnimatedScale around content is a no-op because scale
is hard-coded to 1.0; either remove the AnimatedScale and return
GestureDetector(onTap: onTap, child: content) directly, or make the widget
stateful and drive the scale from a press state: add a private bool _pressed,
update it in GestureDetector handlers (onTapDown/onTapUp/onTapCancel) and set
AnimatedScale.scale to (_pressed ? 0.95 : 1.0) with the existing duration so the
tap press animates. Update the code around AnimatedScale, GestureDetector,
onTap, and content accordingly.
- Around line 121-133: Move the UI callback invocation after restoring the scale
and guard awaits against disposal: in _onTapUp and _onTapCancel, await
_ctrl.forward() (and in _onTapDown await _ctrl.reverse()) before calling
widget.onPressed, wrapping the await in a try/catch that ignores TickerCanceled
(or catches Object) so a disposed AnimationController won’t throw; after the
await, check if (mounted) then call widget.onPressed?.call(); reference:
_onTapUp, _onTapCancel, _onTapDown, _ctrl, widget.onPressed.
In `@workout-logger/lib/screens/widgets/session_details_sheet.dart`:
- Line 11: Replace the hard-coded color literal used by the _hcColor constant
with a centralized AppColors entry: add a static const (e.g., healthConnect) to
the AppColors class and change references to use AppColors.healthConnect instead
of the local _hcColor (and remove the duplicate _hcColor in
profile_sections.dart). Update any widget files that reference _hcColor (notably
session_details_sheet.dart and profile_sections.dart) to import AppColors and
use AppColors.healthConnect so the same brand color is shared project-wide.
- Around line 145-151: The volume and weight labels are hardcoded as "kg" and
raw kg values are shown; update the widget to use the user's unit preference by
obtaining SettingsProvider (either via context.read<SettingsProvider>() in build
or adding it as a constructor param to match WorkoutSummaryScreen) and route all
weight/volume displays through settings.toDisplay(value) and append
settings.unitLabel(), replacing occurrences such as volStr/ _StatBannerBox
value, per-exercise total label, the set row string ('${set.weight} kg ×
${set.reps} reps') and per-set volume calculations so they show converted values
and the correct unit label.
In `@workout-logger/lib/screens/widgets/targets_tab.dart`:
- Around line 287-326: The submit flow allows multiple concurrent submissions;
add a private boolean state _isSubmitting, guard the start of _submit to return
early if _isSubmitting is true, setState(_isSubmitting = true) before awaiting
context.read<WorkoutProvider>().createTarget(...) and use try/finally to
setState(_isSubmitting = false) after the await, and change the GlowButton's
onPressed to onPressed: _isSubmitting ? null : _submit so the button is disabled
while the async createTarget is in-flight; keep the existing mounted check
before Navigator.of(context).pop() inside the try/finally to avoid popping when
the widget is unmounted.
- Around line 110-114: The _types array and target submission must use
SettingsProvider to show and store units correctly: replace the hardcoded labels
in the static _types definition with dynamic labels using
SettingsProvider.unitLabel (e.g., build the display strings for 'weight' and
'volume' by reading SettingsProvider.unitLabel at widget build time rather than
hardcoding "kg"), and in the _submit path (the function that calls
WorkoutProvider.createTarget) convert the user-entered targetValue via
SettingsProvider.toStorage(value) before passing it to createTarget so the
stored value is in the canonical unit; update references to
_targetType/_selectedExerciseId as needed but keep createTarget parameters the
same.
In `@workout-logger/lib/screens/widgets/workout_header.dart`:
- Around line 190-196: The _OptionsMenu call is hardcoding restSeconds to 90 so
the menu always shows 90s selected; change the call site to pass the actual
current rest value (e.g., the header widget's restSeconds field or the workout
model's restSeconds) instead of 90, ensure _OptionsMenu's constructor parameter
restSeconds is used unchanged inside its build logic (the for loop over
[30,60,90,120,180] should compare against that parameter), and propagate the
real restSeconds from the parent screen (thread it from workout_flow_screen.dart
into this header widget and then into the _OptionsMenu invocation) so user
changes persistently reflect in the checkmark.
- Around line 22-23: The constructor parameters onPrevious and onNext on the
WorkoutHeader widget are marked required but never used; either wire them to
visible controls or remove them from the API. To fix, locate the WorkoutHeader
class/constructor and either (A) add previous/next buttons in the build method
(e.g., IconButton or GestureDetector near the header or in the AppBar area) and
call widget.onPrevious()/widget.onNext() (or the corresponding final fields) on
tap, ensuring accessibility and tests are updated, or (B) remove onPrevious and
onNext from the constructor and all callers (and update tests) so they are no
longer required. Make the change consistently for both parameters referenced in
the constructor signature.
In `@workout-logger/lib/screens/workout_flow_screen.dart`:
- Around line 420-450: In _toggleDropset and _addDrop, convert weights to
display units before assigning controller text: instead of using raw kg values
for _mainWeightCtrl.text and the TextEditingController for _dropWeightCtrls,
call settings.toDisplay(value) and format with
toStringAsFixed(samePrecisionUsedElsewhere) (matching the pattern used in
_loadLastSessionData and onApplyRecommendation) so controllers show rounded
display units; update _mainWeightCtrl, each new TextEditingController(text:
...), and any place that sets drop controller text to use
settings.toDisplay(...) formatted as a string.
In `@workout-logger/lib/screens/workout_summary_screen.dart`:
- Around line 25-28: The volume string is built from raw session.totalVolume and
hardcodes "kg"; update the screen to obtain SettingsProvider (like the existing
WorkoutProvider usage) and replace raw formatting with
settings.toDisplay(session.totalVolume) and use settings.unitLabel for labels
instead of appending "kg"; also update any usage in _ExerciseSummaryRow so it
doesn't add "kg" but uses the provided formatted value and settings.unitLabel;
apply the same change to the other similar blocks (the other volStr
computations) so all volume displays use settings.toDisplay(...) and
settings.unitLabel consistently.
- Around line 138-143: Change _buildStatGrid to accept named parameters instead
of four positional ones (e.g., declare parameters as required int duration,
required String volume, required int sets, required int exercises) and update
the call site that currently invokes _buildStatGrid(...) to use the matching
named arguments (duration: ..., volume: ..., sets: ..., exercises: ...); keep
parameter types the same and preserve any null-safety/required annotations
already used in the project.
In `@workout-logger/lib/theme/app_theme.dart`:
- Around line 195-201: The hardcoded non-const color literal in chipTheme
(selectedColor: Color(0x4D6C5CE7)) violates the AppColors contract and allocates
a new Color each read; replace it with a named token on AppColors (e.g.,
AppColors.primaryVariant or AppColors.chipSelected) and make it const so the
theme uses a single shared constant value; update the AppColors definition to
include the new const Color and then reference that constant from
chipTheme.selectedColor.
---
Outside diff comments:
In `@workout-logger/lib/screens/edit_workout_session_screen.dart`:
- Around line 41-58: The EditableSet construction in the _exercises mapping
aliases the drops list (drops: s.drops), which can cause shared-mutation with
widget.session.exercises; change it to create a defensive copy of the drops list
(e.g., a new List from s.drops) when constructing EditableSet inside the
EditableExerciseLog map so subsequent mutations to EditableSet.drops (via
_EditableSetRowState or elsewhere) do not mutate the original WorkoutSession
before _save is called. Ensure the copy handles nullability (preserve null vs
empty-list semantics) consistent with existing code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e1272135-4ad5-4499-bb34-232dbfe4ad7c
📒 Files selected for processing (25)
workout-logger/lib/screens/add_custom_exercise_screen.dartworkout-logger/lib/screens/analytics_screen.dartworkout-logger/lib/screens/edit_workout_session_screen.dartworkout-logger/lib/screens/exercise_library_screen.dartworkout-logger/lib/screens/history_screen.dartworkout-logger/lib/screens/home_screen.dartworkout-logger/lib/screens/profile_screen.dartworkout-logger/lib/screens/routines_screen.dartworkout-logger/lib/screens/widgets/dashboard_widgets.dartworkout-logger/lib/screens/widgets/editable_exercise_card.dartworkout-logger/lib/screens/widgets/exercise_details_sheet.dartworkout-logger/lib/screens/widgets/exercise_input_section.dartworkout-logger/lib/screens/widgets/exercise_progress_view.dartworkout-logger/lib/screens/widgets/profile_sections.dartworkout-logger/lib/screens/widgets/rest_timer_view.dartworkout-logger/lib/screens/widgets/rf_cards.dartworkout-logger/lib/screens/widgets/rf_inputs.dartworkout-logger/lib/screens/widgets/rf_widgets.dartworkout-logger/lib/screens/widgets/routine_creator.dartworkout-logger/lib/screens/widgets/session_details_sheet.dartworkout-logger/lib/screens/widgets/targets_tab.dartworkout-logger/lib/screens/widgets/workout_header.dartworkout-logger/lib/screens/workout_flow_screen.dartworkout-logger/lib/screens/workout_summary_screen.dartworkout-logger/lib/theme/app_theme.dart
Comment on lines
410
to
437
| Padding( | ||
| padding: const EdgeInsets.all(AppSpacing.md), | ||
| child: TextField( | ||
| decoration: const InputDecoration( | ||
| hintText: 'Search exercises...', | ||
| prefixIcon: Icon(Icons.search), | ||
| child: Container( | ||
| height: 44, | ||
| decoration: BoxDecoration( | ||
| color: AppColors.card, | ||
| borderRadius: BorderRadius.circular(AppRadius.full), | ||
| border: Border.all(color: AppColors.glassBorder), | ||
| ), | ||
| child: TextField( | ||
| onChanged: (v) => setState(() => _query = v), | ||
| style: | ||
| const TextStyle(color: AppColors.textPrimary, fontSize: 14), | ||
| decoration: const InputDecoration( | ||
| hintText: 'Search exercises…', | ||
| hintStyle: | ||
| TextStyle(color: AppColors.textMuted, fontSize: 14), | ||
| prefixIcon: Icon( | ||
| Icons.search_rounded, | ||
| color: AppColors.textMuted, | ||
| size: 18, | ||
| ), | ||
| border: InputBorder.none, | ||
| contentPadding: EdgeInsets.symmetric(vertical: 12), | ||
| ), | ||
| ), | ||
| onChanged: (val) => setState(() => _searchQuery = val), | ||
| ), | ||
| ), |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Reuse _SearchBar instead of re-implementing it inline.
The search field built here duplicates _SearchBar (Lines 177-227) but loses the clear-button affordance and drifts the styling. Hoist _SearchBar (or a shared component) and reuse it in both screens to keep behavior consistent and eliminate the copy.
♻️ Proposed refactor
If _SearchBar is generalized (required ValueChanged<String> onChanged, required String query), the selector body collapses to:
- Padding(
- padding: const EdgeInsets.all(AppSpacing.md),
- child: Container(
- height: 44,
- decoration: BoxDecoration(...),
- child: TextField(
- onChanged: (v) => setState(() => _query = v),
- style: const TextStyle(...),
- decoration: const InputDecoration(...),
- ),
- ),
- ),
+ Padding(
+ padding: const EdgeInsets.all(AppSpacing.md),
+ child: _SearchBar(
+ query: _query,
+ onChanged: (v) => setState(() => _query = v),
+ ),
+ ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/lib/screens/exercise_library_screen.dart` around lines 410 -
437, The inline TextField duplicates the existing widget _SearchBar and omits
its clear-button and styling; refactor to reuse _SearchBar by updating
_SearchBar to accept required String query and required ValueChanged<String>
onChanged, then replace the inline Container/TextField block with a call to
_SearchBar(query: _query, onChanged: (v) => setState(() => _query = v)); ensure
the _SearchBar preserves the original decoration (prefix icon, contentPadding,
border) and clear-button affordance so both screens share identical behavior.
Comment on lines
+138
to
+143
| Widget _buildStatGrid( | ||
| int duration, | ||
| String volume, | ||
| int sets, | ||
| int exercises, | ||
| ) { |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use named parameters for _buildStatGrid.
_buildStatGrid(int duration, String volume, int sets, int exercises) takes four positional parameters of similar primitive types — easy to swap sets and exercises accidentally.
♻️ Proposed signature
- Widget _buildStatGrid(
- int duration,
- String volume,
- int sets,
- int exercises,
- ) {
+ Widget _buildStatGrid({
+ required int duration,
+ required String volume,
+ required int sets,
+ required int exercises,
+ }) {Update the call site at line 56 accordingly.
As per coding guidelines, "Use named parameters for functions with 3 or more arguments".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/lib/screens/workout_summary_screen.dart` around lines 138 -
143, Change _buildStatGrid to accept named parameters instead of four positional
ones (e.g., declare parameters as required int duration, required String volume,
required int sets, required int exercises) and update the call site that
currently invokes _buildStatGrid(...) to use the matching named arguments
(duration: ..., volume: ..., sets: ..., exercises: ...); keep parameter types
the same and preserve any null-safety/required annotations already used in the
project.
Comment on lines
195
to
201
| chipTheme: ChipThemeData( | ||
| backgroundColor: cardColor, | ||
| selectedColor: primaryColor.withOpacity(0.3), | ||
| labelStyle: const TextStyle(color: textPrimary), | ||
| backgroundColor: AppColors.card, | ||
| selectedColor: Color(0x4D6C5CE7), | ||
| labelStyle: const TextStyle(color: AppColors.textPrimary), | ||
| side: BorderSide.none, | ||
| shape: RoundedRectangleBorder( | ||
| borderRadius: BorderRadius.circular(8), | ||
| ), | ||
| shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(8)), | ||
| ), |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Hex literal violates this file's own contract; also missing const.
selectedColor: Color(0x4D6C5CE7) is a hardcoded primary color with an alpha — exactly the pattern the AppColors header tells callers to avoid. It also isn't const, so a fresh Color is allocated on every theme read. Promote it to a named token (or use const Color(0x4D6C5CE7) at minimum) so the source of the value is clear and maintained alongside AppColors.primary:
♻️ Proposed fix
class AppColors {
...
static const primary = Color(0xFF6C5CE7);
+ // Primary @ 30% alpha – used for selected chip backgrounds.
+ static const primarySelected = Color(0x4D6C5CE7);
...
}
...
chipTheme: ChipThemeData(
backgroundColor: AppColors.card,
- selectedColor: Color(0x4D6C5CE7),
+ selectedColor: AppColors.primarySelected,
labelStyle: const TextStyle(color: AppColors.textPrimary),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/lib/theme/app_theme.dart` around lines 195 - 201, The
hardcoded non-const color literal in chipTheme (selectedColor:
Color(0x4D6C5CE7)) violates the AppColors contract and allocates a new Color
each read; replace it with a named token on AppColors (e.g.,
AppColors.primaryVariant or AppColors.chipSelected) and make it const so the
theme uses a single shared constant value; update the AppColors definition to
include the new const Color and then reference that constant from
chipTheme.selectedColor.
- Updated ProgramsScreen to improve UI elements and replace AppTheme with AppColors. - Enhanced FloatingActionButton styles and added new RFWidgets for better consistency. - Implemented a new ProgramWeekEditorStep widget for editing week structures in training programs. - Introduced ProgramWeekTile for displaying collapsible week cards in ProgramDetailScreen. - Added functionality for managing deload weeks and intensity factors within the week editor. - Improved overall code organization and readability across the modified files.
- Introduced `GlassCard` with gradient background and accent border option. - Added `AmbientGlow` for decorative ambient effects. - Implemented `RFNavBar` for a custom bottom navigation bar with glassmorphism. - Created `Sparkline` and `VolumeChart` widgets for data visualization. - Developed `WheelPickerField` for weight and reps input with haptic feedback. - Updated `WorkoutHeader` with gradient background and improved text styles. - Refined `AppTheme` with new color definitions and text styles using Google Fonts. - Added Google Fonts dependency for enhanced typography.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
♻️ Duplicate comments (5)
workout-logger/lib/screens/history_screen.dart (1)
701-706: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
_menuItemstill uses four positional arguments.This is the same readability/guideline issue raised earlier: the helper and its call sites are still positional even though they carry four distinct meanings.
As per coding guidelines, "Use named parameters for functions with 3 or more arguments".
Also applies to: 715-726
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/screens/history_screen.dart` around lines 701 - 706, The helper _menuItem currently takes four positional parameters; change its signature to use named parameters (e.g., {required String key, required IconData icon, required String label, required Color color}) and update all call sites (the places building the popup menu entries including the ones that currently call _menuItem('edit', ...), _menuItem('sync', ...), _menuItem('delete', ...) and the other similar calls around the 715-726 region) to invoke it with named arguments (key: ..., icon: ..., label: ..., color: ...); ensure imports/types remain correct and run analyzer to fix any now-unreferenced positional assumptions.workout-logger/lib/screens/analytics_screen.dart (3)
187-238:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHardcoded unit labels still ignore
SettingsProvider.Subtitle (line 187), Y-axis tick (line 228) and the
/ 1000conversion in spots (line 238) all assume kg→tonnes regardless of user unit. PullSettingsProviderviacontext.watchand applysettings.toDisplay()/settings.unitLabel, mirroring the pattern used inexercise_progress_view.dart.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/screens/analytics_screen.dart` around lines 187 - 238, The chart currently hardcodes kg→tonnes in the subtitle, Y-axis tick labels, and spot values (the '/ 1000' conversion) instead of using the user's settings; fix by pulling SettingsProvider via context.watch<SettingsProvider>() (as used in exercise_progress_view.dart), use settings.toDisplay(value) when building each FlSpot value and the leftTitles getTitlesWidget, and replace the hardcoded subtitle text with '${settings.unitLabel} Last ${sessions.length} workouts' or similar using settings.unitLabel so units reflect user preference; update references in the LineChart/LineChartData (subtitle, leftTitles.getTitlesWidget, and spots mapping that currently divides by 1000) and ensure you use settings.toDisplay for conversion logic.
295-303:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard against
maxVol == 0to avoid NaN bars.
byMuscle.isEmptyis the only early return; a non-empty map whose top entry is0still flows through and producespct = 0 / 0 = NaN, whichRFProgressBarwon’t render correctly. Either short-circuit to the empty-card path or computepct = maxVol > 0 ? entry.value / maxVol : 0.🛡️ Proposed guard
final top = sorted.take(8).toList(); - final maxVol = top.first.value; + final maxVol = top.first.value; + if (maxVol <= 0) { + return _ChartCard( + title: 'Weekly Muscle Volume', + isEmpty: true, + child: const SizedBox.shrink(), + ); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/screens/analytics_screen.dart` around lines 295 - 303, The code computes pct = entry.value / maxVol where maxVol = top.first.value, which can be 0 and yield NaN; update the logic in the widget building (around variables top and maxVol used to build _ChartCard and progress bars) to guard against division by zero: either short-circuit to the empty-card path when maxVol == 0 or compute pct as maxVol > 0 ? entry.value / maxVol : 0 before passing it to RFProgressBar (and ensure any place using maxVol checks for zero).
341-344:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winBound week bucket against future-dated sessions.
now.difference(s.date).inDays ~/ 7is negative whens.date > now(clock skew or manually edited dates), andif (w < 4)admits negatives, mutatingweekswith new keys that render as labels like--1w. Add a non-negative bound.🛡️ Proposed guard
for (final s in provider.sessions) { final w = now.difference(s.date).inDays ~/ 7; - if (w < 4) weeks[w] = (weeks[w] ?? 0) + 1; + if (w >= 0 && w < 4) weeks[w] = (weeks[w] ?? 0) + 1; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/screens/analytics_screen.dart` around lines 341 - 344, The loop that computes week bucket w from now.difference(s.date).inDays ~/ 7 can produce negative values for future-dated sessions and currently allows negatives into weeks; update the loop over provider.sessions so it only updates weeks when w is within 0..3 (e.g., check w >= 0 && w < 4) before mutating weeks, using the existing variables now, s.date, w and weeks to locate the change.workout-logger/lib/screens/widgets/rf_widgets.dart (1)
322-330:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRace risk on
_onTapUp/_onTapCancelstill present.
widget.onPressed?.call()runs beforeawait _ctrl.forward(), so anonPressedthat pops/pushes a route can dispose this widget while the controller is still awaited, throwingAnimationController.forward() called after dispose(). Restore the scale first andmounted-guard the callback (and guard_onTapCancel's forward similarly).🛡️ Proposed fix
Future<void> _onTapUp(TapUpDetails _) async { HapticFeedback.heavyImpact(); - widget.onPressed?.call(); - await _ctrl.forward(); + if (mounted) { + await _ctrl.forward(); + } + if (mounted) widget.onPressed?.call(); } Future<void> _onTapCancel() async { - await _ctrl.forward(); + if (mounted) await _ctrl.forward(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/screens/widgets/rf_widgets.dart` around lines 322 - 330, The race occurs because widget.onPressed is invoked before awaiting the scale animation, so popping a route can dispose the widget while _ctrl.forward() is still awaited; change _onTapUp to first await _ctrl.forward(), then check mounted and call widget.onPressed (e.g., if (!mounted) return; widget.onPressed?.call()), and update _onTapCancel to await _ctrl.forward() and guard with mounted after the await as well; reference the methods _onTapUp, _onTapCancel, the AnimationController _ctrl, and the widget.onPressed callback when making these changes.
🤖 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 `@workout-logger/lib/screens/history_screen.dart`:
- Around line 68-71: The loop over monthSessions currently does last-write-wins
when multiple sessions share the same day (map[s.date.day] =
CalendarDayData(...)); change it to merge by computing the day's max intensity
(or aggregate totalVolume per day then compute intensity) before assigning so
duplicate days keep the highest badge. Locate the for loop iterating
monthSessions and update it to check map[s.date.day] (or an intermediate
dayVolumes accumulator keyed by s.date.day), compute newIntensity from
s.totalVolume, and set map[...] = CalendarDayData(intensity:
max(existingIntensity, newIntensity)) (or compute intensity after summing
volumes) to preserve the strongest badge for that date.
- Line 176: Change the positional-parameter signature of _buildHeader to use
named parameters (e.g., Widget _buildHeader(BuildContext context, {required bool
hasUnsynced, required HistoryManager historyManager}) ) and update all callers
(the call near where _buildHeader is invoked) to pass hasUnsynced: value and
historyManager: myManager; ensure required is used so calls remain explicit and
update any imports/exports if needed to reflect the new signature.
- Around line 578-585: _confirmDelete currently calls
Navigator.of(context).pop(), which causes the page route to be popped when
deletion is triggered from the overflow menu; remove the route dismissal from
_confirmDelete so it only performs the deletion and shows the SnackBar.
Specifically, in _confirmDelete (where you call
provider.deleteWorkoutSession(session.id) and then
nav.pop()/messenger.showSnackBar), drop the nav.pop() call (retain the
context.mounted checks and the messenger.showSnackBar call), and ensure any
callers that present a sheet (the details sheet) explicitly dismiss their sheet
after awaiting _confirmDelete instead of relying on _confirmDelete to pop the
route.
In `@workout-logger/lib/screens/home_screen.dart`:
- Around line 155-159: Normalize date comparisons by stripping time components
before computing differences and use inclusive-start/exclusive-end week windows:
create a local "today" without time (e.g., DateTime today = DateTime(now.year,
now.month, now.day)) and for each session build a date-only "sessionDate" from
s.date (DateTime(s.date.year, s.date.month, s.date.day)); then compute diff =
today.difference(sessionDate).inDays. For week checks compute startOfWeek =
today.subtract(Duration(days: today.weekday - 1)) and endOfWeek =
startOfWeek.add(Duration(days: 7)) and test inclusion with
!sessionDate.isBefore(startOfWeek) && sessionDate.isBefore(endOfWeek). Apply the
same pattern wherever similar logic appears (the loop using now/data/sessions,
and the other occurrences noted).
- Around line 275-279: Both helper functions _buildStreakHero and
_buildRecentWorkouts use positional parameters for three arguments; change their
signatures to use named parameters (e.g., Widget _buildStreakHero({required
BuildContext context, required WorkoutProvider provider, _HomeScreenState?
homeState}) and similarly for _buildRecentWorkouts) and then update every call
site to pass arguments by name (context: ..., provider: ..., homeState: ...),
preserving nullability and requiredness; ensure imports/types remain unchanged
and run analyzer to fix any cascaded call-site errors.
In `@workout-logger/lib/screens/programs/programs_screen.dart`:
- Around line 69-73: The RFEmptyState instance here should be promoted to a
const construction: change the instantiation of RFEmptyState (the widget named
RFEmptyState with icon, title, subtitle) so it uses the const constructor (i.e.,
replace RFEmptyState(...) with const RFEmptyState(...)), since RFEmptyState's
constructor is const and all provided arguments (Icons.calendar_month_rounded,
the title and subtitle strings) are compile-time constants.
In `@workout-logger/lib/screens/routines_screen.dart`:
- Line 34: Change the helper _buildQuickStartCard to accept named parameters
instead of three positional ones (e.g. {_buildQuickStartCard(BuildContext
context, Routine routine, RoutinesProvider provider)} ->
{_buildQuickStartCard({required BuildContext context, required Routine routine,
required RoutinesProvider provider})}) and update the call site inside
SliverToBoxAdapter to pass those parameters by name (context: ..., routine: ...,
provider: ...); update any other usages and adjust any internal references to
the parameters accordingly.
In `@workout-logger/lib/screens/widgets/activity_heatmap.dart`:
- Around line 43-68: The "Less" and "More" legend Text widgets and their
TextStyle should be made const to follow the "use const constructors" guideline;
update the two Text(...) instances to use const Text('Less', style: const
TextStyle(...)) and const Text('More', style: const TextStyle(...)) (reference
the Text widgets rendering the legend and the TextStyle that uses
AppColors.textFaint) so the TextStyle and Text are promoted to compile-time
constants.
In `@workout-logger/lib/screens/widgets/body_heatmap.dart`:
- Around line 127-145: The three helper functions _drawHeat, _ellipse, and
_circle use many positional parameters which makes call sites unreadable; change
their signatures to use named, required parameters (e.g., _drawHeat({required
Canvas canvas, required double sx, required double sy, required String muscle,
required Path path, required Color color, required double baseOpacity}),
_ellipse({required double cx, required double cy, required double rx, required
double ry, required double sx, required double sy}), and _circle({required
double cx, required double cy, required double r, required double sx, required
double sy})), then update every call site that invokes _drawHeat, _ellipse, and
_circle to pass arguments by name accordingly; preserve return types and
behavior and keep Paint/color logic unchanged.
In `@workout-logger/lib/screens/widgets/calendar_grid.dart`:
- Around line 5-9: The doc comment for CalendarDayData.intensity is incorrect:
update the comment on class CalendarDayData (and its intensity field) to state
the valid range is 0–3 and clarify semantics (0 = no workout, 1–3 = intensity
levels) so it matches how _DayCell handles intensity == 0 (transparent
background).
- Around line 94-138: Make the PR marker Container and the three legend Text
widgets const: change the small circular Container (the one with
BoxDecoration(shape: BoxShape.circle, color: AppColors.success)) to use a const
BoxDecoration and const Container, and prefix the three Text widgets (Text('PR',
style: TextStyle(...)), Text('Less', style: ...), Text('More', style: ...)) with
const and use const TextStyle where applicable; leave the List.generate-derived
Containers unchanged since they depend on runtime values. This ensures the
PR-marker BoxDecoration and the legend Text/TextStyle constructors are compiled
as const.
In `@workout-logger/lib/screens/widgets/program_week_editor.dart`:
- Around line 43-45: The onPressed callbacks for the stepper decrement/increment
use (value - step).clamp(min, max) and (value + step).clamp(min, max) which
return num, causing a type error when passed to the ValueChanged<int> onChanged;
update both callback expressions in the stepper widget (the onPressed branches
referencing value, step, min, max, and onChanged) to convert the clamped result
to int by appending .toInt() before calling onChanged so the callbacks pass an
int.
In `@workout-logger/lib/screens/widgets/program_week_tile.dart`:
- Line 69: _buildHeader currently takes three positional arguments (week, phase,
phaseColor) which makes calls hard to scan and error-prone; change its
definition to use named parameters (e.g., {required WeekType week, required
PhaseType phase, required Color phaseColor} or the actual types used) and update
all call sites (including the occurrences around the current call and those at
lines ~108-112) to pass arguments by name (_buildHeader(week: week, phase:
phase, phaseColor: phaseColor)); ensure any imports/types referenced by the
signature remain correct and run analyzer/formatter to catch any remaining call
sites.
---
Duplicate comments:
In `@workout-logger/lib/screens/analytics_screen.dart`:
- Around line 187-238: The chart currently hardcodes kg→tonnes in the subtitle,
Y-axis tick labels, and spot values (the '/ 1000' conversion) instead of using
the user's settings; fix by pulling SettingsProvider via
context.watch<SettingsProvider>() (as used in exercise_progress_view.dart), use
settings.toDisplay(value) when building each FlSpot value and the leftTitles
getTitlesWidget, and replace the hardcoded subtitle text with
'${settings.unitLabel} Last ${sessions.length} workouts' or similar using
settings.unitLabel so units reflect user preference; update references in the
LineChart/LineChartData (subtitle, leftTitles.getTitlesWidget, and spots mapping
that currently divides by 1000) and ensure you use settings.toDisplay for
conversion logic.
- Around line 295-303: The code computes pct = entry.value / maxVol where maxVol
= top.first.value, which can be 0 and yield NaN; update the logic in the widget
building (around variables top and maxVol used to build _ChartCard and progress
bars) to guard against division by zero: either short-circuit to the empty-card
path when maxVol == 0 or compute pct as maxVol > 0 ? entry.value / maxVol : 0
before passing it to RFProgressBar (and ensure any place using maxVol checks for
zero).
- Around line 341-344: The loop that computes week bucket w from
now.difference(s.date).inDays ~/ 7 can produce negative values for future-dated
sessions and currently allows negatives into weeks; update the loop over
provider.sessions so it only updates weeks when w is within 0..3 (e.g., check w
>= 0 && w < 4) before mutating weeks, using the existing variables now, s.date,
w and weeks to locate the change.
In `@workout-logger/lib/screens/history_screen.dart`:
- Around line 701-706: The helper _menuItem currently takes four positional
parameters; change its signature to use named parameters (e.g., {required String
key, required IconData icon, required String label, required Color color}) and
update all call sites (the places building the popup menu entries including the
ones that currently call _menuItem('edit', ...), _menuItem('sync', ...),
_menuItem('delete', ...) and the other similar calls around the 715-726 region)
to invoke it with named arguments (key: ..., icon: ..., label: ..., color: ...);
ensure imports/types remain correct and run analyzer to fix any now-unreferenced
positional assumptions.
In `@workout-logger/lib/screens/widgets/rf_widgets.dart`:
- Around line 322-330: The race occurs because widget.onPressed is invoked
before awaiting the scale animation, so popping a route can dispose the widget
while _ctrl.forward() is still awaited; change _onTapUp to first await
_ctrl.forward(), then check mounted and call widget.onPressed (e.g., if
(!mounted) return; widget.onPressed?.call()), and update _onTapCancel to await
_ctrl.forward() and guard with mounted after the await as well; reference the
methods _onTapUp, _onTapCancel, the AnimationController _ctrl, and the
widget.onPressed callback when making these changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 665df341-5701-4f8a-beee-62feb4c6b16b
📒 Files selected for processing (18)
workout-logger/lib/screens/analytics_screen.dartworkout-logger/lib/screens/history_screen.dartworkout-logger/lib/screens/home_screen.dartworkout-logger/lib/screens/programs/program_detail_screen.dartworkout-logger/lib/screens/programs/programs_screen.dartworkout-logger/lib/screens/routines_screen.dartworkout-logger/lib/screens/widgets/activity_heatmap.dartworkout-logger/lib/screens/widgets/body_heatmap.dartworkout-logger/lib/screens/widgets/calendar_grid.dartworkout-logger/lib/screens/widgets/program_week_editor.dartworkout-logger/lib/screens/widgets/program_week_tile.dartworkout-logger/lib/screens/widgets/rf_widgets.dartworkout-logger/lib/screens/widgets/sparkline_painter.dartworkout-logger/lib/screens/widgets/volume_chart.dartworkout-logger/lib/screens/widgets/wheel_picker.dartworkout-logger/lib/screens/widgets/workout_header.dartworkout-logger/lib/theme/app_theme.dartworkout-logger/pubspec.yaml
Comment on lines
+43
to
+45
| onPressed: value > min | ||
| ? () => onChanged((value - step).clamp(min, max)) | ||
| : null, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Dart, does int.clamp(min, max)returnintornum, and does it need .toInt()before passing it to aValueChanged callback?
💡 Result:
int.clamp(min, max) is typed as returning num (not int). The num base signature is num clamp(num lowerLimit, num upperLimit) [1], and int’s clamp return type is shown as → num as well [2]. Because the static type is num, if you pass the result to a ValueChanged<int> callback, you generally need to convert it to int (e.g., onChanged(value.clamp(… , …).toInt())). The .toInt() is required whenever the callback parameter is statically int, since num is not assignable to int. Note: there is discussion that the runtime returns an int when all arguments are ints, but the static signature historically remained num clamp(num, num) [3]. So for type safety, use .toInt() when the receiving API expects int.
Citations:
- 1: https://api.dart.dev/stable/2.17.0/dart-core/num/clamp.html
- 2: https://api.dart.dev/stable/3.4.0/dart-core/int-class.html
- 3: int.clamp() should statically return int if the arguments are ints dart-lang/sdk#28249
Add .toInt() to both stepper value callbacks.
The clamp() method returns num, not int. Passing it directly to ValueChanged<int> causes a type error. Convert the result with .toInt() at both callback sites.
Proposed fix
onPressed: value > min
- ? () => onChanged((value - step).clamp(min, max))
+ ? () => onChanged((value - step).clamp(min, max).toInt())
: null,
@@
onPressed: value < max
- ? () => onChanged((value + step).clamp(min, max))
+ ? () => onChanged((value + step).clamp(min, max).toInt())
: null,Also applies to: 64-66
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/lib/screens/widgets/program_week_editor.dart` around lines 43
- 45, The onPressed callbacks for the stepper decrement/increment use (value -
step).clamp(min, max) and (value + step).clamp(min, max) which return num,
causing a type error when passed to the ValueChanged<int> onChanged; update both
callback expressions in the stepper widget (the onPressed branches referencing
value, step, min, max, and onChanged) to convert the clamped result to int by
appending .toInt() before calling onChanged so the callbacks pass an int.
- Introduced PersonalRecord model to track best weight, reps, and volume for exercises. - Implemented PRManager to manage personal records, including checking and updating records after workouts. - Added methods in storage service for saving and retrieving personal records. - Updated UI to display personal records in the Analytics screen and Workout Summary screen. - Enhanced onboarding process to prompt for user name and handle version updates. - Refactored various screens to improve layout and user experience.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
♻️ Duplicate comments (1)
workout-logger/lib/screens/widgets/editable_exercise_card.dart (1)
126-127:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftUse stable row keys to prevent input/focus state loss during list mutations.
Line 126 and Line 385 key rows by index-derived values, so deleting/reordering rows will remount stateful children and can drop in-progress input/focus.
Suggested direction
class EditableSet { + final String id; double weight; int reps; bool isDropset; List<DropsetEntry>? drops; int? timeTaken; DateTime timestamp; EditableSet({ + required this.id, required this.weight, required this.reps, required this.timestamp, this.isDropset = false, this.drops, this.timeTaken, }); } ... - key: ValueKey('set_${exerciseName}_$i'), + key: ValueKey('set_${set.id}'), ... - key: ValueKey('drop_${widget.setNumber}_$i'), + key: ValueKey(drop.id),Also applies to: 385-386
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/screens/widgets/editable_exercise_card.dart` around lines 126 - 127, The rows are currently keyed by an index-derived string ValueKey('set_${exerciseName}_$i') which causes stateful children to remount and lose input/focus when rows are deleted or reordered; update the keying in EditableExerciseCard (and the other occurrence at lines ~385-386) to use a stable, intrinsic identifier from the model (e.g., set.id or a persistent unique field on the set object) or a stable composite key like '$exerciseName_${set.id}' instead of the loop index so row widgets preserve their state across list mutations.
🤖 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 `@workout-logger/lib/screens/analytics_screen.dart`:
- Around line 411-412: The code attempts to call sort() on prManager.allRecords
(used to populate the local final records variable), but prManager.allRecords
returns an unmodifiable list; create a mutable copy before sorting by replacing
the direct reference with a copied list (e.g., final records =
[...prManager.allRecords] or prManager.allRecords.toList()) and then call
records.sort((a, b) => b.achievedAt.compareTo(a.achievedAt)) so the sort
operates on a mutable list.
- Around line 497-505: The `_PRCard` currently hardcodes "kg" and raw values
(e.g., in the `_PRStat` calls for `record.bestWeight` and `record.bestVolume`),
ignoring user unit settings; update `_RecordsTab.build()` to obtain `final
settings = context.watch<SettingsProvider>();` (or pass `settings` down into
`_PRCard`) and replace the hardcoded formatting with `settings.toDisplay(...)`
for weight/volume values and append `settings.unitLabel` for the unit label
(keep `bestReps` as-is); ensure you use `record.bestWeight` and
`record.bestVolume` as inputs to `toDisplay` and update the related `_PRStat`
value strings accordingly so imperial users see converted values and the correct
unit label.
In `@workout-logger/lib/screens/history_screen.dart`:
- Around line 411-416: Add an explicit const constructor to the StatelessWidget
class _VertDivider by declaring const _VertDivider(); and update all places that
instantiate it (the two uses of _VertDivider() in the HistoryScreen build) to
use const _VertDivider() so the widget can be created as a compile-time constant
and satisfy prefer_const_constructors_in_immutables.
In `@workout-logger/lib/screens/home_screen.dart`:
- Around line 351-355: The Icon instantiation using
Icons.local_fire_department_rounded with size 14 and color AppColors.primary
should be a const constructor; update the Icon(...) to const Icon(...) in the
HomeScreen widget (the Icon instance in the home_screen.dart build tree) so it
uses a compile-time constant and satisfies
prefer_const_constructors/flutter_lints.
- Around line 478-492: The BoxDecoration color ternary inside home_screen.dart
has a dead inner branch: the isFuture ? const Color(0x0FFFFFFF) : const
Color(0x0FFFFFFF) does nothing; update the color logic in the decoration (where
variables done, isToday, isFuture are used) to either remove the isFuture check
and return a single default color, or give future days a distinct color/alpha
(e.g., a lower-alpha variant) so the isFuture branch has effect; adjust the
BoxDecoration.color expression in the widget building code (the decoration block
using BorderRadius.circular(3) and border) accordingly.
- Around line 567-577: The sparkline data for "Sets" and "Avg time" is synthetic
(built from weekSets and avgDuration) and therefore misleading; update the
_StatItem usage so spark is either removed for these cards or replaced with true
per-week series computed from sessions (same approach as
weeklyWorkouts/weeklyVolumes). Locate the variables weekSets and avgDuration and
either (A) compute a List<double> of seven entries by grouping sessions by week
and summing sets / averaging durations (mirror the logic used in weeklyWorkouts
and weeklyVolumes) and pass that list into the spark parameter of the _StatItem,
or (B) omit the spark argument for those _StatItem instances so no fake trend is
rendered. Ensure any helper used to derive weekly series is reused or extracted
for clarity.
In `@workout-logger/lib/screens/onboarding_screen.dart`:
- Around line 258-270: The two _WhatsNewItem instantiations (the ones with icon:
Icons.emoji_events_rounded / Icons.bar_chart_rounded, color: AppColors.warning /
AppColors.secondary, and string title/description) should be prefixed with const
because _WhatsNewItem has a const constructor and all arguments are compile-time
constants; update those two widget creations to use const _WhatsNewItem(...) so
the analyzer rule prefer_const_constructors is satisfied.
- Around line 157-164: The GlowButton is still enabled when _saving because
onPressed uses an empty closure; update the onPressed assignment in the
SizedBox/GlowButton block so it passes null when _saving (i.e., onPressed:
_saving ? null : _submit) allowing GlowButton to render disabled and ignore
taps, preventing double submits during the async _submit flow.
In `@workout-logger/lib/screens/widgets/editable_exercise_card.dart`:
- Around line 54-60: Change the onSetChanged callback from a 5-argument
positional function to a single function that takes named parameters (e.g. void
Function({required int setIndex, required double weight, required int reps,
required bool isDropset, List<DropsetEntry>? drops})); update the
EditableExerciseCard constructor/property declaration for onSetChanged to that
named-params shape and then update every invocation (the call sites in this file
where onSetChanged(...) is called) to pass arguments by name (setIndex:,
weight:, reps:, isDropset:, drops:). Ensure any external callers constructing
EditableExerciseCard are updated to use the new named arguments as well and
adjust any typedefs or tests referencing the old signature.
- Around line 97-111: Replace the raw GestureDetector interactive controls with
proper semantic button widgets: use IconButton for icon-only actions (the shown
delete control that calls _confirmDelete(context) and the other icon-only
occurrences) and use TextButton for any text+icon combos; wire the onPressed to
the same handlers (e.g., call _confirmDelete(context)), move the visual styling
into the button's style (background color, padding, shape via ButtonStyle or
IconButton.styleFrom) and add a tooltip/semanticLabel for accessibility; update
the three occurrences referenced (the delete IconButton around _confirmDelete,
and the other icon-only and text+icon locations) so they gain built-in focus,
keyboard activation and semantics instead of GestureDetector.
In `@workout-logger/lib/screens/widgets/exercise_input_section.dart`:
- Around line 452-456: The dropset UI contains widgets that are fully
const-evaluable but not declared const — for example the Icon call shown as
Icon(Icons.trending_down_rounded, color: AppColors.warning, size: 18) and other
static widgets in the same section — so update those widget constructors to be
const (e.g., const Icon(...)) and, where applicable, mark parent
collections/children as const (const [...]) so all immutable widgets in the
dropset section (including the other Icon/Text/SizedBox instances flagged) use
const constructors.
- Around line 575-577: The dropset row contains widgets that are fully
const-evaluable but not marked const; inside the ExerciseInputSection widget
update the child Text('×', style: TextStyle(...)) to use a const constructor
(i.e., const Text(...)) and likewise mark the other analogous widget at the
second occurrence (the similar widget around line with the second '×') as const
so both the Padding and its Text children are fully const; ensure any nested
TextStyle or EdgeInsets used there are also const where needed.
- Around line 794-798: The Padding and its Icon child in the
ExerciseInputSection widget are fully const-evaluable but the Icon is not marked
const; update the widget tree by making the Icon constructor const (i.e., change
child: Icon(...) to child: const Icon(...)) so both Padding and Icon are const;
this affects the Padding and Icon expressions inside the exercise input section
UI.
- Around line 709-720: The "First time doing this exercise!" Row and its
children should be fully const-evaluable: ensure the Row and each child widget
use const so the whole subtree is compile-time constant—add const before
Icon(Icons.star_outline_rounded...), SizedBox(width: 8) and Text('First time
doing this exercise!', ...) (the widgets referenced are Row, Icon, SizedBox, and
Text) so the entire child list is const.
- Around line 711-717: The Row's children (Icon, SizedBox, Text and nested
TextStyle) are implicitly const but need explicit const keywords to satisfy
prefer_const_constructors; update the children in the Row inside
exercise_input_section.dart by prefixing const to the
Icon(Icons.star_outline_rounded, ...), const SizedBox(width: 8), and const
Text(..., style: const TextStyle(...)) (ensure the nested TextStyle is also
marked const) so each const-constructible widget is explicitly const.
In `@workout-logger/lib/screens/widgets/routine_creator.dart`:
- Around line 410-440: Wrap the await calls to provider.updateRoutine and
provider.createRoutine inside a try/catch in _save so storage failures are
handled: call provider.updateRoutine(updated) or provider.createRoutine(...)
inside try, on success pop via Navigator.of(context).pop() (only if mounted),
and in the catch block show a SnackBar with an error message (and optionally log
the error) instead of popping; ensure you catch generic exceptions/errors so an
unhandled Future error does not surface and use the same
ScaffoldMessenger.of(context).showSnackBar pattern as the history-screen delete
flow.
---
Duplicate comments:
In `@workout-logger/lib/screens/widgets/editable_exercise_card.dart`:
- Around line 126-127: The rows are currently keyed by an index-derived string
ValueKey('set_${exerciseName}_$i') which causes stateful children to remount and
lose input/focus when rows are deleted or reordered; update the keying in
EditableExerciseCard (and the other occurrence at lines ~385-386) to use a
stable, intrinsic identifier from the model (e.g., set.id or a persistent unique
field on the set object) or a stable composite key like
'$exerciseName_${set.id}' instead of the loop index so row widgets preserve
their state across list mutations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 46035bd0-d30e-488d-a7f0-d5af2e16e059
📒 Files selected for processing (18)
workout-logger/lib/main.dartworkout-logger/lib/models/models.dartworkout-logger/lib/screens/analytics_screen.dartworkout-logger/lib/screens/history_screen.dartworkout-logger/lib/screens/home_screen.dartworkout-logger/lib/screens/onboarding_screen.dartworkout-logger/lib/screens/routines_screen.dartworkout-logger/lib/screens/widgets/editable_exercise_card.dartworkout-logger/lib/screens/widgets/exercise_input_section.dartworkout-logger/lib/screens/widgets/routine_creator.dartworkout-logger/lib/screens/workout_flow_screen.dartworkout-logger/lib/screens/workout_summary_screen.dartworkout-logger/lib/services/interfaces/storage_service_interface.dartworkout-logger/lib/services/managers/managers.dartworkout-logger/lib/services/managers/pr_manager.dartworkout-logger/lib/services/settings_provider.dartworkout-logger/lib/services/storage_service.dartworkout-logger/test/test_utils/mock_storage_service.dart
Comment on lines
+575
to
+577
| const Padding( | ||
| padding: EdgeInsets.symmetric(horizontal: 6), | ||
| child: Text('×', style: TextStyle(color: AppColors.textMuted)), |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Use const for static widgets in dropset row.
Two widgets are fully const-evaluable.
♻️ Proposed fixes
- const Padding(
+ const Padding(
padding: EdgeInsets.symmetric(horizontal: 6),
child: Text('×', style: TextStyle(color: AppColors.textMuted)),
),- icon: const Icon(Icons.close_rounded, size: 16),
+ icon: const Icon(Icons.close_rounded, size: 16),As per coding guidelines, "Use const constructors wherever possible".
Also applies to: 599-599
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/lib/screens/widgets/exercise_input_section.dart` around lines
575 - 577, The dropset row contains widgets that are fully const-evaluable but
not marked const; inside the ExerciseInputSection widget update the child
Text('×', style: TextStyle(...)) to use a const constructor (i.e., const
Text(...)) and likewise mark the other analogous widget at the second occurrence
(the similar widget around line with the second '×') as const so both the
Padding and its Text children are fully const; ensure any nested TextStyle or
EdgeInsets used there are also const where needed.
Comment on lines
+709
to
+720
| child: const Row( | ||
| children: [ | ||
| Icon(Icons.star_outline_rounded, | ||
| color: AppColors.textMuted, size: 16), | ||
| SizedBox(width: 8), | ||
| Text( | ||
| 'First time doing this exercise!', | ||
| style: TextStyle(color: AppColors.textMuted, fontSize: 13), | ||
| ), | ||
| ], | ||
| ), | ||
| ); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Use const for the "first time" message Row.
The Row and its children are fully const-evaluable since all properties are constant.
♻️ Proposed fix
- child: const Row(
+ child: const Row(
children: [
- Icon(Icons.star_outline_rounded,
+ const Icon(Icons.star_outline_rounded,
color: AppColors.textMuted, size: 16),
- SizedBox(width: 8),
- Text(
+ const SizedBox(width: 8),
+ const Text(
'First time doing this exercise!',
style: TextStyle(color: AppColors.textMuted, fontSize: 13),
),
],
),As per coding guidelines, "Use const constructors wherever possible".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| child: const Row( | |
| children: [ | |
| Icon(Icons.star_outline_rounded, | |
| color: AppColors.textMuted, size: 16), | |
| SizedBox(width: 8), | |
| Text( | |
| 'First time doing this exercise!', | |
| style: TextStyle(color: AppColors.textMuted, fontSize: 13), | |
| ), | |
| ], | |
| ), | |
| ); | |
| child: const Row( | |
| children: [ | |
| const Icon(Icons.star_outline_rounded, | |
| color: AppColors.textMuted, size: 16), | |
| const SizedBox(width: 8), | |
| const Text( | |
| 'First time doing this exercise!', | |
| style: const TextStyle(color: AppColors.textMuted, fontSize: 13), | |
| ), | |
| ], | |
| ), | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/lib/screens/widgets/exercise_input_section.dart` around lines
709 - 720, The "First time doing this exercise!" Row and its children should be
fully const-evaluable: ensure the Row and each child widget use const so the
whole subtree is compile-time constant—add const before
Icon(Icons.star_outline_rounded...), SizedBox(width: 8) and Text('First time
doing this exercise!', ...) (the widgets referenced are Row, Icon, SizedBox, and
Text) so the entire child list is const.
Comment on lines
+711
to
+717
| Icon(Icons.star_outline_rounded, | ||
| color: AppColors.textMuted, size: 16), | ||
| SizedBox(width: 8), | ||
| Text( | ||
| 'First time doing this exercise!', | ||
| style: TextStyle(color: AppColors.textMuted, fontSize: 13), | ||
| ), |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add explicit const to Row children.
While the children are implicitly const due to the parent const Row, flutter_lints' prefer_const_constructors expects explicit const keywords on each const-constructible child.
♻️ Proposed fix
child: const Row(
children: [
- Icon(Icons.star_outline_rounded,
+ const Icon(Icons.star_outline_rounded,
color: AppColors.textMuted, size: 16),
- SizedBox(width: 8),
- Text(
+ const SizedBox(width: 8),
+ const Text(
'First time doing this exercise!',
style: TextStyle(color: AppColors.textMuted, fontSize: 13),
),
],
),As per coding guidelines, "Use const constructors wherever possible".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/lib/screens/widgets/exercise_input_section.dart` around lines
711 - 717, The Row's children (Icon, SizedBox, Text and nested TextStyle) are
implicitly const but need explicit const keywords to satisfy
prefer_const_constructors; update the children in the Row inside
exercise_input_section.dart by prefixing const to the
Icon(Icons.star_outline_rounded, ...), const SizedBox(width: 8), and const
Text(..., style: const TextStyle(...)) (ensure the nested TextStyle is also
marked const) so each const-constructible widget is explicitly const.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #48 +/- ##
==========================================
+ Coverage 32.18% 38.54% +6.36%
==========================================
Files 36 85 +49
Lines 6187 14088 +7901
==========================================
+ Hits 1991 5430 +3439
- Misses 4196 8658 +4462 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…ject guidelines; remove MainActivity.kt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@CLAUDE.md`:
- Around line 297-301: The rule in CLAUDE.md that mandates always reading
graphify-out/GRAPH_REPORT.md (the “Read `graphify-out/GRAPH_REPORT.md` before
reading source files...” bullet under Rules) should be made conditional: change
the text to check for the existence of graphify-out/GRAPH_REPORT.md and only
require it when the graphify-out/ directory exists, otherwise provide fallback
behavior (e.g., proceed without the graph or advise running `graphify update .`
or `graphify generate` to create the graph). Update the Rules section to
explicitly state the gated condition and the fallback instructions so fresh
clones aren’t blocked by a missing graphify-out/ directory.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 372c1943-e4a5-4fdd-a5a6-f0506cf76598
📒 Files selected for processing (5)
.gitignoreCLAUDE.mdworkout-logger/android/app/src/main/kotlin/com/workoutlogger/workout_logger/MainActivity.ktworkout-logger/test/add_custom_exercise_screen_test.dartworkout-logger/test/exercise_library_screen_test.dart
💤 Files with no reviewable changes (1)
- workout-logger/android/app/src/main/kotlin/com/workoutlogger/workout_logger/MainActivity.kt
_FrequencyGrid: added w >= 0 && guard so future-dated sessions can't insert negative map keys
_MuscleVolumeChart: early-return isEmpty when maxVol == 0 (prevents NaN); converted volume display via settings.toDisplay + settings.unitLabel
dashboard_widgets.dart
startOfWeek now truncated to midnight (DateTime(y,m,d)) before subtracting weekdays — fixes the bug where sessions earlier in the day than "now" were excluded
Volume StatGridCard now uses settings.toDisplay(weeklyVolume) and 'Volume (${settings.unitLabel})'
edit_workout_session_screen.dart
drops: s.drops?.toList() — defensive copy prevents shared-mutation with the original WorkoutSession
exercise_details_sheet.dart
Added else branch on delete failure — shows error SnackBar instead of silently doing nothing
Set chips now use settings.toDisplay(s.weight) + settings.unitLabel
Growth trend label uses settings.toDisplay(growthModel.slope) + settings.unitLabel
exercise_input_section.dart
Icon(Icons.auto_awesome_rounded, …) → const Icon(…)
exercise_progress_view.dart
DropdownButton.value guarded with ids.contains(selected) ? selected : null — prevents assert/crash when the selected exercise id is no longer in the performed set
profile_sections.dart
const _SectionDivider() and const _ComingSoonBadge() constructors + all call sites updated
rf_inputs.dart
Added onLongPressEnd to _StepButton (wired to GestureDetector.onLongPressEnd)
_NumberPickerSheetState now holds Timer? _holdTimer; onLongPress assigns _holdTimer = Timer.periodic(…), onLongPressEnd cancels it, dispose() also cancels it — no more leaked timers
session_details_sheet.dart
Volume banner: settings.toDisplay(session.totalVolume) + 'Volume ${settings.unitLabel}'
Per-exercise total: settings.toDisplay(log.totalVolume) + settings.unitLabel
Per-set row: settings.toDisplay(set.weight) formatted + settings.unitLabel
targets_tab.dart
Added _isSubmitting bool; _submit guards against re-entry and wraps provider call in try/finally; GlowButton.onPressed is null while submitting
workout_flow_screen.dart
_toggleDropset: weight controller text now uses settings.toDisplay(_currentWeight) with proper decimal formatting (matching _loadLastSessionData)
_addDrop: new drop controller text also uses settings.toDisplay(newWeight)
WorkoutHeader now receives restSeconds: _restSeconds
workout_header.dart
Added optional restSeconds field (defaults to 90 for backwards compatibility); _OptionsMenu receives widget.restSeconds instead of the hardcoded literal
workout_summary_screen.dart
volStr built from settings.toDisplay(session.totalVolume); label updated to 'Volume (${settings.unitLabel})'
…HealthConnectService
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
workout-logger/lib/screens/widgets/rf_inputs.dart (1)
242-262:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle focus loss for inline numeric editing.
The inline editor still lacks a focus-loss path, so users can get stuck in
_editingwhen they tap outside without submitting. Add an outside-tap/focus-change handler that calls_commitEdit().Proposed fix
child: _editing ? TextField( controller: _ctrl, autofocus: true, textAlign: TextAlign.center, keyboardType: const TextInputType.numberWithOptions( decimal: true, ), + onTapOutside: (_) => _commitEdit(), style: TextStyle( color: c, fontSize: 22, fontWeight: FontWeight.w800, fontFeatures: const [FontFeature.tabularFigures()], ),#!/bin/bash # Verify the inline RFNumberField editor still has no explicit outside-tap/focus-loss commit handler. rg -n -C3 "child:\s*_editing|TextField\(|onTapOutside|onFocusChange|onEditingComplete|onSubmitted" workout-logger/lib/screens/widgets/rf_inputs.dart🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/screens/widgets/rf_inputs.dart` around lines 242 - 262, The inline numeric TextField can leave _editing true when the user taps outside; add a FocusNode (or use onTapOutside if available) and attach a focus-change listener that calls _commitEdit() when focus is lost; update the TextField to use this focusNode (and keep existing _ctrl, onSubmitted and onEditingComplete) and ensure the FocusNode is created and disposed in the same scope or state class that owns _ctrl and _editing so you don’t leak resources.
🤖 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 `@workout-logger/lib/screens/history_screen.dart`:
- Around line 619-697: The history card is hardcoding "kg" and not applying user
settings; update _HistoryCard to obtain the weight unit/formatter from
SettingsProvider (either via BuildContext or by threading a settings object from
_MonthGroup into _HistoryCard) and use that to both convert/format the total
volume (replace the vol/volStr computation) and to render the unit label
(replace the Text('kg', ...) widget) so displayed volume respects the user's
weight unit preference; reference the _HistoryCard and _MonthGroup widgets and
the SettingsProvider when making these changes.
- Around line 266-290: The summary card currently uses raw totalVolume and a
hardcoded 'kg' unit in _buildSummaryCard and _SummaryCell; obtain the user's
unit settings from SettingsProvider (via context.watch<SettingsProvider>() or by
passing the provider/value into _buildSummaryCard), convert totalVolume using
settings.toDisplay(totalVolume) before computing volStr, and set the unit label
for the VOLUME _SummaryCell to the settings' display unit (e.g.
settings.displayUnitLabel or equivalent) so both the formatted value and unit
reflect the user's preference.
In `@workout-logger/lib/screens/home_screen.dart`:
- Around line 784-927: The "kg vol" label is hardcoded in _buildRecentWorkouts;
update the volume formatting and unit label to use the app settings: compute
volStr via settings.toDisplay(s.totalVolume) (instead of raw k conversion) and
replace the Text('kg vol') with Text('${settings.unitLabel} vol'); locate the
volStr calculation and the trailing Text widget (both inside
_buildRecentWorkouts) and use provider.settings (or the existing settings
object) to call toDisplay(...) and unitLabel so the UI respects user units.
- Around line 644-782: The card hardcodes 'kg' and uses raw volumes; fetch the
SettingsProvider inside _buildMuscleVolumeCard (e.g., final settings =
Provider.of<SettingsProvider>(context, listen: false)) and use
settings.toDisplay(...) when computing muscleVols (replace uses of
el.totalVolume with the converted value) so maxVol, normalizedVols, topMuscles
and pct reflect display units, and replace the literal 'kg' Text with
settings.unitLabel; ensure BodyHeatmapWidget receives normalizedVols based on
the converted volumes and keep references to _buildMuscleVolumeCard, provider,
settings.toDisplay, and settings.unitLabel to locate changes.
In `@workout-logger/lib/screens/widgets/body_heatmap.dart`:
- Around line 161-162: The current shouldRepaint implementation in _BodyPainter
only checks reference inequality (old.muscleVolumes != muscleVolumes) which
misses in-place updates to the same Map instance; change it to perform deep
equality on the maps (e.g., use package:collection's DeepCollectionEquality or
Flutter's mapEquals) and return true when the contents differ so the heatmap
repaints correctly; update the _BodyPainter.shouldRepaint method to compare map
keys/values with deep equality against muscleVolumes instead of a simple
reference check.
In `@workout-logger/lib/screens/widgets/exercise_input_section.dart`:
- Line 353: The onTap handlers call onChanged with the result of (value -
step).clamp(0, 999) and (value + step).clamp(0, 999), but clamp returns num
while onChanged is ValueChanged<double>; update those calls in the widget (the
closures using onChanged) to convert the clamped result to double by calling
.toDouble() on the clamp result so the types match.
In `@workout-logger/lib/screens/widgets/exercise_progress_view.dart`:
- Around line 291-293: The trend label currently hardcodes "kg/session" instead
of using user units; update _GrowthCard to accept a SettingsProvider (or
settings) and replace the hardcoded string by formatting model.slope with
settings.toDisplay(...) and appending settings.unitLabel (and keep "/session" as
needed or derive from settings if available). Locate the build where _GrowthCard
is instantiated and pass the SettingsProvider instance, then inside _GrowthCard
use settings.toDisplay(model.slope.abs()) + ' ' + settings.unitLabel +
'/session' (or equivalent) to render the trend text so it matches the rest of
the view.
In `@workout-logger/lib/screens/widgets/rf_widgets.dart`:
- Line 889: The computed ring progress currently uses final progress = total > 0
? remaining / total : 0.0 which can produce values <0 or >1; change the
computation to clamp the ratio into [0.0, 1.0] (e.g., when total > 0, compute
the ratio and apply clamp(0.0, 1.0) and cast to double) so the progress variable
(progress) is always a valid double between 0.0 and 1.0; update the expression
that uses remaining and total accordingly.
---
Duplicate comments:
In `@workout-logger/lib/screens/widgets/rf_inputs.dart`:
- Around line 242-262: The inline numeric TextField can leave _editing true when
the user taps outside; add a FocusNode (or use onTapOutside if available) and
attach a focus-change listener that calls _commitEdit() when focus is lost;
update the TextField to use this focusNode (and keep existing _ctrl, onSubmitted
and onEditingComplete) and ensure the FocusNode is created and disposed in the
same scope or state class that owns _ctrl and _editing so you don’t leak
resources.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9c0ed329-c335-4f5f-ab62-95a5093557ab
📒 Files selected for processing (28)
workout-logger/lib/screens/analytics_screen.dartworkout-logger/lib/screens/edit_workout_session_screen.dartworkout-logger/lib/screens/history_screen.dartworkout-logger/lib/screens/home_screen.dartworkout-logger/lib/screens/onboarding_screen.dartworkout-logger/lib/screens/programs/programs_screen.dartworkout-logger/lib/screens/routines_screen.dartworkout-logger/lib/screens/widgets/activity_heatmap.dartworkout-logger/lib/screens/widgets/body_heatmap.dartworkout-logger/lib/screens/widgets/calendar_grid.dartworkout-logger/lib/screens/widgets/dashboard_widgets.dartworkout-logger/lib/screens/widgets/editable_exercise_card.dartworkout-logger/lib/screens/widgets/exercise_details_sheet.dartworkout-logger/lib/screens/widgets/exercise_input_section.dartworkout-logger/lib/screens/widgets/exercise_progress_view.dartworkout-logger/lib/screens/widgets/profile_sections.dartworkout-logger/lib/screens/widgets/program_week_tile.dartworkout-logger/lib/screens/widgets/rf_inputs.dartworkout-logger/lib/screens/widgets/rf_widgets.dartworkout-logger/lib/screens/widgets/routine_creator.dartworkout-logger/lib/screens/widgets/session_details_sheet.dartworkout-logger/lib/screens/widgets/targets_tab.dartworkout-logger/lib/screens/widgets/workout_header.dartworkout-logger/lib/screens/workout_flow_screen.dartworkout-logger/lib/screens/workout_summary_screen.dartworkout-logger/lib/services/health_connect_service.dartworkout-logger/pubspec.yamlworkout-logger/test/exercise_library_screen_test.dart
…nd improve UI components
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
workout-logger/lib/screens/programs/programs_screen.dart (1)
265-268:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard mini-timeline math when
totalWeeks <= 0.If
program.totalWeeksis zero (e.g., malformed imported payload),fractionbecomes non-finite and.round()will throw. Add a safe denominator guard.Proposed fix
- final fraction = - (phase.endWeek - phase.startWeek + 1) / program.totalWeeks; + final totalWeeks = program.totalWeeks <= 0 ? 1 : program.totalWeeks; + final fraction = + (phase.endWeek - phase.startWeek + 1) / totalWeeks;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/screens/programs/programs_screen.dart` around lines 265 - 268, Guard the mini-timeline math by using a safe denominator when computing fraction to avoid division by zero or non-finite values: replace the calculation of fraction in the code that references phase and program.totalWeeks with a safeDenominator (e.g., final safeDenominator = program.totalWeeks <= 0 ? 1 : program.totalWeeks) and compute fraction = (phase.endWeek - phase.startWeek + 1) / safeDenominator; then keep the Expanded(flex: ((fraction * 100).round()).clamp(1, 100)) logic (or alternatively set flex = 1 if fraction.isNaN or !fraction.isFinite) so the flex value is always a valid int between 1 and 100.
♻️ Duplicate comments (6)
workout-logger/lib/screens/widgets/exercise_progress_view.dart (1)
28-32: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winScope provider rebuilds for performed-ID computation.
This recomputes
sessions × exerciseson everyWorkoutProvidernotification. Prefer a scopedcontext.select(or provider getter) so this widget rebuilds only when exercise IDs change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/screens/widgets/exercise_progress_view.dart` around lines 28 - 32, The current code uses context.watch<WorkoutProvider>() and recomputes the performed set by iterating provider.sessions and their exercises on every provider change; change this to a scoped listener so the widget only rebuilds when exercise IDs change—either add a getter on WorkoutProvider (e.g., performedExerciseIds) that returns the Set<String> of exerciseId, or replace context.watch with context.select((WorkoutProvider p) => p.sessions.fold/flatten to the Set of exerciseId) so the widget listens only to that set (refer to provider, sessions, exercises, exerciseId, and the performed variable).workout-logger/lib/screens/widgets/profile_sections.dart (1)
200-207: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winMove Health Connect color into
AppColorsand consume the token here.A local
_hcColorworks against the color centralization in this PR. Promote it toAppColors.healthConnect(or equivalent) and reference that token in this file.Also applies to: 239-255
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/screens/widgets/profile_sections.dart` around lines 200 - 207, Replace the local color constant _hcColor with the centralized token in AppColors (e.g., AppColors.healthConnect) and update usages in this file (the _hcColor declaration and any places consuming it, such as the _ProfileSection invocation that uses iconColor) to reference AppColors.healthConnect instead; also make the same replacement for the other occurrence later in the file that currently uses _hcColor so all Health Connect color usage is centralized.workout-logger/lib/screens/widgets/editable_exercise_card.dart (2)
54-60: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftConvert
onSetChangedto named parameters.This callback is still carrying 5 positional arguments, which keeps every call site hard to read and easy to misorder.
#!/bin/bash sed -n '52,60p' workout-logger/lib/screens/widgets/editable_exercise_card.dart rg -nP --type=dart 'EditableExerciseCard\s*\(' workout-logger/lib -C2 rg -nP --type=dart '\bonSetChanged\s*:' workout-logger/lib -C2As per coding guidelines: "Use named parameters for functions with 3 or more arguments in Dart".
Also applies to: 135-142
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/screens/widgets/editable_exercise_card.dart` around lines 54 - 60, The onSetChanged callback on EditableExerciseCard currently uses five positional parameters; change its typedef/signature to accept a single set of named parameters (e.g. {required int setIndex, required double weight, required int reps, required bool isDropset, List<DropsetEntry>? drops}) and update the field declaration in EditableExerciseCard and any constructor parameters accordingly; then update every call site that invokes onSetChanged(...) to call onSetChanged(setIndex: ..., weight: ..., reps: ..., isDropset: ..., drops: ...) to preserve behavior and improve readability. Ensure you update all occurrences (including the ones around lines referenced) and run analyzer/tests to catch missed call sites.
384-405:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftUse stable keys for
EditableDropRow.
ValueKey('drop_${widget.setNumber}_$i')still changes for every remaining row after a delete, so Flutter will dispose and recreate the survivingEditableDropRowstates. That drops focus and any in-flight text in the remaining drop editors.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/screens/widgets/editable_exercise_card.dart` around lines 384 - 405, The current ValueKey('drop_${widget.setNumber}_$i') is index-based and shifts after removals causing EditableDropRow widgets to be recreated; add a stable identifier to each DropsetEntry (e.g., an id field) and use that as the key (e.g., ValueKey(drop.id)) in the EditableDropRow constructor instead of the index-based key; update any code that constructs new DropsetEntry instances (in onWeightChanged/onRepsChanged) to preserve or copy the id so the identifier remains stable across edits and deletes.workout-logger/lib/screens/home_screen.dart (1)
521-560:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize all dashboard date comparisons to calendar boundaries.
These ranges still bucket sessions with hour/minute data attached, so late Sunday sessions can leak into “this week” and recent items can render
Today/Yesterdayincorrectly. Strip both sides to date-only values and use inclusive-start/exclusive-end week windows everywhere this dashboard groups by day or week.Also applies to: 644-650, 919-925
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/screens/home_screen.dart` around lines 521 - 560, Normalize all date comparisons to calendar boundaries by converting DateTime instances to date-only values (e.g., DateTime(y, m, d)) before building weekStart/weekEnd and before comparing session.date; update the logic around weekStart, weekSessions, weeklyWorkouts, weeklyVolumes, weeklySets, and weeklyAvgDurations to compute an inclusive-start/exclusive-end window (startDate <= session.date < endDate) using date-only start and end (end = start.add(Duration(days: 7))). Apply the same normalization/compare approach to the other dashboard groupings referenced (the other weekly/day buckets) so comparisons consistently use date-only boundaries and inclusive-start/exclusive-end semantics.workout-logger/lib/screens/edit_workout_session_screen.dart (1)
419-423: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
_sectionLabelstill uses four positional parameters.This helper still carries an unused
BuildContextplus three more positional args, so the earlier named-parameter refactor is still needed.As per coding guidelines, "Use named parameters for functions with 3 or more arguments".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/screens/edit_workout_session_screen.dart` around lines 419 - 423, The helper _sectionLabel currently declares four positional parameters (including an unused BuildContext) which violates the named-parameter guideline; change its signature to use named parameters (e.g. Widget _sectionLabel({required IconData icon, required Color color, required String label}) and remove the unused BuildContext parameter), update all call sites to pass icon:, color:, label:, and adjust any internal references accordingly so compilation and behavior remain the same.
🤖 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 `@workout-logger/lib/screens/history_screen.dart`:
- Around line 552-555: The bottom sheet is being popped immediately in the
onDelete callback (Navigator.of(ctx).pop()) before the confirmation and delete
result are known; change _confirmDelete to return a Future<bool> indicating
success/cancellation, call await _confirmDelete(context) in onDelete, and only
call Navigator.of(ctx).pop() if that returned true (successful delete). Update
all similar handlers (the block around lines 561-599) to follow the same pattern
so the sheet remains open if the user cancels or deletion fails.
- Line 120: The helper _buildSummaryCard currently uses three positional
parameters; change its signature to use named parameters (e.g.,
{_buildSummaryCard({required Type all, required Type totalVolume, required Type
settings})}) and update all call sites (including the one that currently passes
_buildSummaryCard(all, totalVolume, settings) and the other occurrences around
the other helper uses) to call with named arguments like _buildSummaryCard(all:
all, totalVolume: totalVolume, settings: settings); ensure any imports/uses of
the function match the new signature and adjust any tests or references
accordingly.
In `@workout-logger/lib/screens/home_screen.dart`:
- Around line 527-579: The volume stats use raw kg values (weekVol and
weeklyVolumes) and a hardcoded 'kg' unit; update the tile to respect user
settings by passing numeric volumes through SettingsProvider (use
settings.toDisplay(weekVol) for the displayed value and map weeklyVolumes with
settings.toDisplay(...)) and replace the hardcoded 'kg' unit with
settings.unitLabel; locate the _StatItem instantiation for the 'Volume' card and
the weeklyVolumes generator to apply these changes so values and unit labels
match the rest of the dashboard.
In `@workout-logger/lib/screens/onboarding_screen.dart`:
- Around line 36-48: Prevent re-entry by returning early if _saving is already
true, and ensure _saving is always cleared in a finally block so an exception
doesn't leave the UI locked; inside _submit check if (_saving) return; then
setState(()=>_saving = true) before the async work, wrap the awaits to
SettingsProvider.setUserName, getCurrentVersion and markVersionSeen in try { ...
} finally { if (mounted) setState(()=>_saving = false); } and only call
widget.onComplete() after the awaits and when mounted; apply the same pattern
(re-entry guard + try/finally reset) to the other similar handlers that call
SettingsProvider methods.
In `@workout-logger/lib/screens/widgets/body_heatmap.dart`:
- Around line 137-139: The code currently defaults missing muscle volume to 0.5
which produces a visible heat; update the fallback to 0.0 so absent data doesn't
show heat. Locate the usage of muscleVolumes (where vol is assigned from
muscleVolumes[muscle] ?? 0.5) and change the default to 0.0, then ensure the
subsequent opacity calculation using baseOpacity and opacity clamping (opacity =
(baseOpacity * (0.5 + vol * 0.5)).clamp(0.0, 1.0)) and the canvas.drawPath call
with color.withValues(alpha: opacity) continue to work with the new zero
fallback.
In `@workout-logger/lib/screens/widgets/dashboard_widgets.dart`:
- Around line 23-26: The current trainedDays computation only filters sessions
by startOfWeek and thus includes sessions beyond this week; compute a
startOfNextWeek (e.g., startOfWeek.add(Duration(days: 7))) and constrain the
predicate to include sessions s where !s.date.isBefore(startOfWeek) &&
s.date.isBefore(startOfNextWeek), using date-truncated/midnight versions of
s.date and startOfWeek to avoid time-of-day issues; update the trainedDays chain
(the sessions.where(...).map(...)) to use that two-sided window.
In `@workout-logger/lib/screens/widgets/exercise_input_section.dart`:
- Around line 814-823: The calls to the helper function _metaChip currently pass
three positional arguments (icon, label, color); update _metaChip's signature to
use named parameters (e.g., {required IconData icon, required String label,
required Color color}) and change all call sites (including the uses that render
rest/tempo/superset chips) to call _metaChip(icon: ..., label: ..., color: ...);
ensure the parameters are required in the function signature so callers must use
the named args and update any other occurrences (the other chip calls around the
same widget) to match.
In `@workout-logger/lib/screens/widgets/exercise_progress_view.dart`:
- Around line 46-55: Create an effective selected id derived from the current
ids list and _selectedId (e.g. final effectiveSelectedId =
ids.contains(_selectedId) ? _selectedId : null) and use that single variable for
the dropdown's selected value and for the conditional that decides whether to
render _ExerciseStats; replace both uses of _selectedId in the snippet with
effectiveSelectedId (and pass effectiveSelectedId! as exerciseId when non-null).
Keep onChanged as setState(() => _selectedId = id) so selections update the
canonical _selectedId, but always normalize via effectiveSelectedId before
rendering.
In `@workout-logger/lib/screens/widgets/rf_widgets.dart`:
- Around line 69-73: Several shared tappable primitives (GlassCard, _NavItem,
GlowButton) use raw GestureDetector which strips button semantics; wrap the
GestureDetector (the block that returns GestureDetector when onTap != null) with
a Semantics widget set button: true and provide an accessible label (use an
existing label prop if present or add a new String semanticsLabel property to
the widget and fall back to a sensible default like the visible text or role).
Update each widget to place Semantics(...) as the direct parent of
GestureDetector (or replace GestureDetector with an appropriate Flutter button
if design allows) and ensure the semanticsLabel is passed through from callers
where necessary so screen readers and focus/firing semantics are restored.
---
Outside diff comments:
In `@workout-logger/lib/screens/programs/programs_screen.dart`:
- Around line 265-268: Guard the mini-timeline math by using a safe denominator
when computing fraction to avoid division by zero or non-finite values: replace
the calculation of fraction in the code that references phase and
program.totalWeeks with a safeDenominator (e.g., final safeDenominator =
program.totalWeeks <= 0 ? 1 : program.totalWeeks) and compute fraction =
(phase.endWeek - phase.startWeek + 1) / safeDenominator; then keep the
Expanded(flex: ((fraction * 100).round()).clamp(1, 100)) logic (or alternatively
set flex = 1 if fraction.isNaN or !fraction.isFinite) so the flex value is
always a valid int between 1 and 100.
---
Duplicate comments:
In `@workout-logger/lib/screens/edit_workout_session_screen.dart`:
- Around line 419-423: The helper _sectionLabel currently declares four
positional parameters (including an unused BuildContext) which violates the
named-parameter guideline; change its signature to use named parameters (e.g.
Widget _sectionLabel({required IconData icon, required Color color, required
String label}) and remove the unused BuildContext parameter), update all call
sites to pass icon:, color:, label:, and adjust any internal references
accordingly so compilation and behavior remain the same.
In `@workout-logger/lib/screens/home_screen.dart`:
- Around line 521-560: Normalize all date comparisons to calendar boundaries by
converting DateTime instances to date-only values (e.g., DateTime(y, m, d))
before building weekStart/weekEnd and before comparing session.date; update the
logic around weekStart, weekSessions, weeklyWorkouts, weeklyVolumes, weeklySets,
and weeklyAvgDurations to compute an inclusive-start/exclusive-end window
(startDate <= session.date < endDate) using date-only start and end (end =
start.add(Duration(days: 7))). Apply the same normalization/compare approach to
the other dashboard groupings referenced (the other weekly/day buckets) so
comparisons consistently use date-only boundaries and
inclusive-start/exclusive-end semantics.
In `@workout-logger/lib/screens/widgets/editable_exercise_card.dart`:
- Around line 54-60: The onSetChanged callback on EditableExerciseCard currently
uses five positional parameters; change its typedef/signature to accept a single
set of named parameters (e.g. {required int setIndex, required double weight,
required int reps, required bool isDropset, List<DropsetEntry>? drops}) and
update the field declaration in EditableExerciseCard and any constructor
parameters accordingly; then update every call site that invokes
onSetChanged(...) to call onSetChanged(setIndex: ..., weight: ..., reps: ...,
isDropset: ..., drops: ...) to preserve behavior and improve readability. Ensure
you update all occurrences (including the ones around lines referenced) and run
analyzer/tests to catch missed call sites.
- Around line 384-405: The current ValueKey('drop_${widget.setNumber}_$i') is
index-based and shifts after removals causing EditableDropRow widgets to be
recreated; add a stable identifier to each DropsetEntry (e.g., an id field) and
use that as the key (e.g., ValueKey(drop.id)) in the EditableDropRow constructor
instead of the index-based key; update any code that constructs new DropsetEntry
instances (in onWeightChanged/onRepsChanged) to preserve or copy the id so the
identifier remains stable across edits and deletes.
In `@workout-logger/lib/screens/widgets/exercise_progress_view.dart`:
- Around line 28-32: The current code uses context.watch<WorkoutProvider>() and
recomputes the performed set by iterating provider.sessions and their exercises
on every provider change; change this to a scoped listener so the widget only
rebuilds when exercise IDs change—either add a getter on WorkoutProvider (e.g.,
performedExerciseIds) that returns the Set<String> of exerciseId, or replace
context.watch with context.select((WorkoutProvider p) => p.sessions.fold/flatten
to the Set of exerciseId) so the widget listens only to that set (refer to
provider, sessions, exercises, exerciseId, and the performed variable).
In `@workout-logger/lib/screens/widgets/profile_sections.dart`:
- Around line 200-207: Replace the local color constant _hcColor with the
centralized token in AppColors (e.g., AppColors.healthConnect) and update usages
in this file (the _hcColor declaration and any places consuming it, such as the
_ProfileSection invocation that uses iconColor) to reference
AppColors.healthConnect instead; also make the same replacement for the other
occurrence later in the file that currently uses _hcColor so all Health Connect
color usage is centralized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 58ba5abd-be31-445f-99f3-48761c1a50d6
📒 Files selected for processing (30)
workout-logger/android/app/build.gradle.ktsworkout-logger/android/app/src/main/AndroidManifest.xmlworkout-logger/lib/screens/analytics_screen.dartworkout-logger/lib/screens/edit_workout_session_screen.dartworkout-logger/lib/screens/history_screen.dartworkout-logger/lib/screens/home_screen.dartworkout-logger/lib/screens/onboarding_screen.dartworkout-logger/lib/screens/programs/programs_screen.dartworkout-logger/lib/screens/routines_screen.dartworkout-logger/lib/screens/widgets/activity_heatmap.dartworkout-logger/lib/screens/widgets/body_heatmap.dartworkout-logger/lib/screens/widgets/calendar_grid.dartworkout-logger/lib/screens/widgets/dashboard_widgets.dartworkout-logger/lib/screens/widgets/editable_exercise_card.dartworkout-logger/lib/screens/widgets/exercise_details_sheet.dartworkout-logger/lib/screens/widgets/exercise_input_section.dartworkout-logger/lib/screens/widgets/exercise_progress_view.dartworkout-logger/lib/screens/widgets/profile_sections.dartworkout-logger/lib/screens/widgets/program_week_tile.dartworkout-logger/lib/screens/widgets/rf_inputs.dartworkout-logger/lib/screens/widgets/rf_widgets.dartworkout-logger/lib/screens/widgets/routine_creator.dartworkout-logger/lib/screens/widgets/session_details_sheet.dartworkout-logger/lib/screens/widgets/targets_tab.dartworkout-logger/lib/screens/widgets/workout_header.dartworkout-logger/lib/screens/workout_flow_screen.dartworkout-logger/lib/screens/workout_summary_screen.dartworkout-logger/lib/services/health_connect_service.dartworkout-logger/pubspec.yamlworkout-logger/test/exercise_library_screen_test.dart
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@workout-logger/lib/screens/widgets/exercise_input_section.dart`:
- Around line 419-424: onEditingComplete currently only resets the field when
parse fails, which leaves the visible text out-of-sync when onChanged clamped an
out-of-range value; update the onEditingComplete handler in
exercise_input_section.dart to always normalize the visible text to the
source-of-truth by setting _controller.text = _format() (or only when
_controller.text != _format()) before calling _focusNode.unfocus(), referencing
the existing onEditingComplete, _controller, _format(), and _focusNode symbols.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 20bb73de-9b20-4764-8f3a-d92c635d524e
📒 Files selected for processing (10)
workout-logger/lib/screens/analytics_screen.dartworkout-logger/lib/screens/profile_screen.dartworkout-logger/lib/screens/widgets/exercise_input_section.dartworkout-logger/lib/screens/widgets/profile_sections.dartworkout-logger/lib/screens/widgets/rf_widgets.dartworkout-logger/lib/screens/workout_flow_screen.dartworkout-logger/lib/services/health_connect_service.dartworkout-logger/lib/services/interfaces/ml_service_interface.dartworkout-logger/lib/services/ml_service.dartworkout-logger/lib/services/workout_provider.dart
…ompt Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…M with ask_user_questions pausing Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…d + history inbox) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ineOptimizerScreen
…een from routine card
Owner
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
workout-logger/lib/models/models.dart (2)
327-332:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
Routine.copyWithstill leaks mutableexerciseIds.The past review flagged this issue and marked it addressed, but the current code at Line 330 still reuses the mutable list reference (
exerciseIds ?? this.exerciseIds). The constructor (Lines 306-311) doesn't wrap the incoming list, andcopyWithdoesn't create a defensive copy, so external mutations can silently affect the Routine instance.♻️ Proposed fix
Routine({ required this.id, required this.name, - required this.exerciseIds, + required List<String> exerciseIds, DateTime? createdAt, - }) : createdAt = createdAt ?? DateTime.now(); + }) : exerciseIds = List.unmodifiable(exerciseIds), + createdAt = createdAt ?? DateTime.now();Then ensure
copyWithalso produces an unmodifiable copy:Routine copyWith({String? name, List<String>? exerciseIds}) => Routine( id: id, name: name ?? this.name, exerciseIds: exerciseIds ?? this.exerciseIds, createdAt: createdAt, );As per coding guidelines:
All model mutations must go through copyWith() — never mutate state directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/models/models.dart` around lines 327 - 332, Routine.copyWith currently returns the same mutable exerciseIds list reference (exerciseIds ?? this.exerciseIds) which allows external mutation; update Routine.copyWith (and where the Routine constructor accepts exerciseIds if applicable) to produce and return a defensive, unmodifiable copy of the list (e.g., create a new List from the provided exerciseIds or this.exerciseIds and wrap it as unmodifiable) so the Routine instance never exposes a mutable list; reference the Routine.copyWith method and the Routine constructor (the fields id, name, exerciseIds, createdAt) when making this change.Source: Coding guidelines
866-925:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
Conversation.messagesremains mutable despite past review.The past review recommended wrapping
messagesinList.unmodifiable(...)at Lines 885, 904–906, and 921–923 to enforce copyWith-only mutations. The current code still exposes a mutable list, allowing direct modification viaconversation.messages.add(...)without going throughcopyWith().♻️ Proposed fix
Conversation({ String? id, required this.title, this.kind = 'coach', DateTime? createdAt, DateTime? updatedAt, List<ChatMessage>? messages, }) : id = id ?? _uuid.v4(), createdAt = createdAt ?? DateTime.now(), updatedAt = updatedAt ?? createdAt ?? DateTime.now(), - messages = messages ?? const []; + messages = List.unmodifiable(messages ?? const []);factory Conversation.fromJson(Map<String, dynamic> json) => Conversation( id: json['id'] as String?, title: json['title'] as String, kind: json['kind'] as String? ?? 'coach', createdAt: DateTime.parse(json['createdAt'] as String), updatedAt: json['updatedAt'] != null ? DateTime.parse(json['updatedAt'] as String) : DateTime.parse(json['createdAt'] as String), - messages: (json['messages'] as List) - .map((m) => ChatMessage.fromJson(m as Map<String, dynamic>)) - .toList(), + messages: List.unmodifiable( + (json['messages'] as List) + .map((m) => ChatMessage.fromJson(m as Map<String, dynamic>)), + ), );Conversation copyWith({ Object? title = _sentinel, Object? kind = _sentinel, Object? createdAt = _sentinel, Object? updatedAt = _sentinel, Object? messages = _sentinel, }) => Conversation( id: id, title: title == _sentinel ? this.title : title as String, kind: kind == _sentinel ? this.kind : kind as String, createdAt: createdAt == _sentinel ? this.createdAt : createdAt as DateTime, updatedAt: updatedAt == _sentinel ? this.updatedAt : updatedAt as DateTime, - messages: messages == _sentinel - ? this.messages - : messages as List<ChatMessage>, + messages: messages == _sentinel + ? this.messages + : List.unmodifiable(messages as List<ChatMessage>), );As per coding guidelines:
All model mutations must go through copyWith() — never mutate state directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/models/models.dart` around lines 866 - 925, The Conversation model still exposes a mutable messages list; wrap messages with List.unmodifiable(...) in all construction paths so callers cannot mutate it directly: in the Conversation constructor initializer (assign messages = List.unmodifiable(messages ?? const [])), in factory Conversation.fromJson (set messages = List.unmodifiable((json['messages'] as List).map(...).toList())), and in copyWith when a new messages value is provided (use List.unmodifiable(messages as List<ChatMessage>)); keep the field final and ensure toJson continues to serialize from the unmodifiable list.Source: Coding guidelines
🤖 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.
Duplicate comments:
In `@workout-logger/lib/models/models.dart`:
- Around line 327-332: Routine.copyWith currently returns the same mutable
exerciseIds list reference (exerciseIds ?? this.exerciseIds) which allows
external mutation; update Routine.copyWith (and where the Routine constructor
accepts exerciseIds if applicable) to produce and return a defensive,
unmodifiable copy of the list (e.g., create a new List from the provided
exerciseIds or this.exerciseIds and wrap it as unmodifiable) so the Routine
instance never exposes a mutable list; reference the Routine.copyWith method and
the Routine constructor (the fields id, name, exerciseIds, createdAt) when
making this change.
- Around line 866-925: The Conversation model still exposes a mutable messages
list; wrap messages with List.unmodifiable(...) in all construction paths so
callers cannot mutate it directly: in the Conversation constructor initializer
(assign messages = List.unmodifiable(messages ?? const [])), in factory
Conversation.fromJson (set messages = List.unmodifiable((json['messages'] as
List).map(...).toList())), and in copyWith when a new messages value is provided
(use List.unmodifiable(messages as List<ChatMessage>)); keep the field final and
ensure toJson continues to serialize from the unmodifiable list.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5692a17d-2812-4a95-938e-5a2da1ce3734
📒 Files selected for processing (15)
workout-logger/lib/models/models.dartworkout-logger/lib/screens/routine_optimizer_screen.dartworkout-logger/lib/screens/routines_screen.dartworkout-logger/lib/screens/widgets/rf_question_card.dartworkout-logger/lib/services/ai/coach_tool_service.dartworkout-logger/lib/services/ai/gemini_ai_service.dartworkout-logger/lib/services/gemini_context_builder.dartworkout-logger/lib/services/interfaces/ai_service_interface.dartworkout-logger/lib/services/managers/conversation_manager.dartworkout-logger/lib/viewmodels/routine_optimizer_view_model.dartworkout-logger/test/ai_coach_view_model_test.dartworkout-logger/test/conversation_manager_test.dartworkout-logger/test/model_serialization_test.dartworkout-logger/test/rf_question_card_test.dartworkout-logger/test/routine_optimizer_view_model_test.dart
feat: add workout summary screen, health analytics, and production APK signing - Workout summary screen with session details - Sleep HR chart and debug log buffer - HealthHistoryManager for sleep/heart rate data - Workout heart rate analysis and recovery metrics - EC P-256 production keystore signing via GitHub Secrets - Split-per-ABI APK builds for arm64, armeabi-v7a, x86_64 - FUTURE_IMPROVEMENTS.md documenting Options B & C (F-Droid, fastlane)
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 34035904 | Triggered | Generic Password | ff1761c | workout-logger/android/app/build.gradle.kts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
…51) - Remove google_fonts dependency; replace with bundled variable font files (Geist-Variable.ttf + GeistMono-Variable.ttf from Vercel v1.7.2, MIT licensed) - Replace all 377 GoogleFonts.geist/geistMono() calls with TextStyle(fontFamily:) across 31 dart files — no runtime Google CDN fetch, F-Droid build-compatible - Declare fonts in pubspec.yaml flutter.fonts section - Add fdroid/metadata/com.devasy.repforge.yml with anti-features (NonFreeNet) and auto-update config for F-Droid submission Co-authored-by: Devasy Patel <110348311+Devasy23@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Owner
Author
|
@coderabbitai review |
Contributor
|
Your plan includes 1 PR review per hour. More reviews will be available in 22 minutes and 45 seconds. |
Owner
Author
|
@coderabbitai review |
Contributor
|
Your plan includes 1 PR review per hour. Reviews are available now. |
Owner
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Devasy
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
.github/workflows/release.yml (1)
40-43:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGate release publishing on test execution.
This workflow can ship APKs without running the test suite, so regressions can be released directly.
✅ Minimal gate to add
- name: Install dependencies working-directory: ./workout-logger run: flutter pub get + - name: Run tests + working-directory: ./workout-logger + run: flutter test + - name: Bump version if: github.event_name == 'push'Based on learnings: "CI builds APK but does not run tests".
Also applies to: 122-130, 151-172
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml around lines 40 - 43, Add a test execution step to the release workflow that runs the Flutter test suite before any APK publishing occurs. After the "Install dependencies" step at lines 40-43, insert a step to execute flutter test and configure it to fail the workflow if tests do not pass. Apply the same test execution gate at the other affected locations in the workflow (lines 122-130 and 151-172) to ensure tests run and pass before any release artifacts are published, preventing regressions from being shipped.Source: Learnings
workout-logger/lib/screens/ai_program_generator_screen.dart (1)
480-490:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winConstrain generated phase names before the trailing week label.
phase.nameis AI-generated, so a long value can overflow thisRowbecause theTextsits before an unconstrainedSpacer. Wrap it inExpanded/Flexibleand ellipsize it.🐛 Proposed fix
- Text( - phase.name, - style: TextStyle(fontFamily: 'Geist', - color: AppColors.textPrimary, - fontSize: 13, - fontWeight: FontWeight.w500, - ), - ), - const Spacer(), + Expanded( + child: Text( + phase.name, + overflow: TextOverflow.ellipsis, + style: TextStyle(fontFamily: 'Geist', + color: AppColors.textPrimary, + fontSize: 13, + fontWeight: FontWeight.w500, + ), + ), + ), + const SizedBox(width: 8),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/screens/ai_program_generator_screen.dart` around lines 480 - 490, The Text widget displaying phase.name is positioned before a Spacer in a Row, which allows the AI-generated name to overflow when it is long. Wrap the Text widget containing phase.name in an Expanded or Flexible widget to constrain its width, and add overflow: TextOverflow.ellipsis to the TextStyle to ensure long names are truncated with an ellipsis rather than overflowing the layout.workout-logger/lib/screens/home_screen.dart (2)
118-122:⚠️ Potential issue | 🟠 Major | ⚡ Quick winResume the active workout instead of opening the tapped routine.
When conflict resolution returns
resume, no new routine workout was started, but this still pushesWorkoutFlowScreen(routine: routine). That can show/initialize the wrong routine for the existing active workout.🐛 Proposed fix
- if (started || conflictAction == StartWorkoutConflictAction.resume) { + if (started) { HapticFeedback.mediumImpact(); Navigator.push(context, _slide(WorkoutFlowScreen(routine: routine))); + } else if (conflictAction == StartWorkoutConflictAction.resume) { + HapticFeedback.mediumImpact(); + _resumeWorkout(context); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/screens/home_screen.dart` around lines 118 - 122, The condition in the WorkoutFlowScreen navigation is incorrectly using OR logic between the started flag and the resume conflict action. When resuming an active workout, a new routine was not started, so pushing WorkoutFlowScreen with the tapped routine will display the wrong routine. Fix this by changing the condition to only push the WorkoutFlowScreen when a new workout was actually started (when started is true), and remove StartWorkoutConflictAction.resume from the OR condition, since resuming should not navigate to a new workout screen with the tapped routine.
1235-1241:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSurface weekly-insight generation failures.
A user can tap Generate/Refresh, hit a network/API error, and see the card silently return to its previous state. Show a small error SnackBar or inline message in the catch block.
🐛 Proposed fix
- } catch (_) { - // silently ignore network/API errors + } catch (_) { + if (mounted) { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + content: const Text('Could not generate weekly insights.'), + backgroundColor: AppColors.error, + behavior: SnackBarBehavior.floating, + shape: RoundedRectangleBorder( + borderRadius: BorderRadius.circular(AppRadius.md), + ), + ), + ); + } } finally {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/screens/home_screen.dart` around lines 1235 - 1241, The catch block in the weekly insights generation try-catch statement silently ignores network and API errors by doing nothing. Replace the empty catch block that catches the exception with code that displays an error message to the user, such as a SnackBar or inline error message. Check the mounted state before showing the error to ensure the widget is still in the tree, and provide a clear user-facing error message so they understand that the generation failed and can retry if needed.workout-logger/lib/models/models.dart (1)
978-990:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFreeze newly added list fields at model boundaries.
QuestionSpec.options,AnswerSpec.selected,PendingQuestions.questions, andSleepPeriod.stageTimelineretain caller-provided mutable lists. Wrap these withList.unmodifiable(...)in constructors/fromJson so model state cannot be mutated behind the notifier/persistence flow. As per coding guidelines, “All model mutations in Dart must go throughcopyWith()— never mutate state directly”.🛡️ Proposed pattern
class QuestionSpec { final String question; final List<String> options; final bool multiSelect; final bool allowCustom; - const QuestionSpec({ + QuestionSpec({ required this.question, - required this.options, + required List<String> options, this.multiSelect = false, this.allowCustom = true, - }); + }) : options = List.unmodifiable(options);Apply the same boundary-copy pattern to
AnswerSpec.selected,PendingQuestions.questions, andSleepPeriod.stageTimeline.Also applies to: 999-1009, 1017-1024, 1081-1089
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/models/models.dart` around lines 978 - 990, Wrap the mutable list fields with List.unmodifiable(...) to enforce immutability at model boundaries. In QuestionSpec.fromJson, wrap the options list assignment with List.unmodifiable() to prevent external mutation of the model state. Apply the same pattern to AnswerSpec.selected, PendingQuestions.questions, and SleepPeriod.stageTimeline in their respective constructors and fromJson factory methods, ensuring all list fields are made unmodifiable when assigned from external input to comply with the guideline that all model mutations must go through copyWith().Source: Coding guidelines
workout-logger/lib/screens/widgets/exercise_details_sheet.dart (1)
213-234:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGate the success trend to positive growth.
growthModel.slopecan be negative, so this currently renders values like+-5.0 lb volume/weekwhile still showing a green “trending up” banner. Hide this banner for non-positive slopes or render a separate decline state.🐛 Proposed fix
- if (growthModel != null && growthModel.r2 > 0.2) ...[ + if (growthModel != null && + growthModel.r2 > 0.2 && + growthModel.slope > 0) ...[🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/screens/widgets/exercise_details_sheet.dart` around lines 213 - 234, The success trend banner is displayed whenever growthModel has a sufficient r2 value, but it does not validate that the slope is positive. This causes negative slopes to display with a green "trending up" icon and confusing messages like "+-5.0 lb volume/week". Add an additional condition to check that growthModel.slope is greater than 0 in the conditional that guards the Container widget with the success styling, Icon(Icons.trending_up_rounded), and the Text showing the slope calculation. This ensures the success banner only appears for positive growth trends, not negative ones.workout-logger/lib/screens/profile_screen.dart (1)
211-214:⚠️ Potential issue | 🟠 MajorRemove
ShareResultStatus.dismissedfrom the success condition—it indicates user cancellation, not a successful backup.When the user dismisses the share sheet, no file is durably saved anywhere.
ShareResultStatus.dismissedexplicitly means the user closed the dialog without completing a share action. Showing "Backup exported successfully!" misleads users into thinking a backup exists when none was created. Display a cancellation message for dismissed status instead.Proposed fix
- if (result.status == ShareResultStatus.success || - result.status == ShareResultStatus.dismissed) { + if (result.status == ShareResultStatus.success) { _showSnack('Backup exported successfully!', AppColors.success); + } else if (result.status == ShareResultStatus.dismissed) { + _showSnack('Export cancelled.', AppColors.warning); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workout-logger/lib/screens/profile_screen.dart` around lines 211 - 214, The condition checking the share result status in the profile_screen.dart file incorrectly treats ShareResultStatus.dismissed as a success case. When a user dismisses the share dialog, no backup is actually created, so showing "Backup exported successfully!" is misleading. Refactor the if statement to only show the success message when the result status equals ShareResultStatus.success. For the ShareResultStatus.dismissed case, handle it separately with a distinct message (e.g., indicating the user cancelled the backup operation) to accurately communicate what happened to the user.
🤖 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 `@docs/superpowers/specs/2026-06-11-sleep-hr-chart-design.md`:
- Line 120: The markdown linting violations in the spec file need to be fixed at
two locations. At line 120, add the language identifier `text` to the opening
code fence (change ``` to ```text) and ensure proper blank-line separation
before and after the fenced code block. At line 162, add a blank line before the
opening ```dart fence to provide proper separation between the preceding text
and the fenced code block. These changes will resolve the markdownlint warnings
for missing language specifiers and improper fence spacing.
In `@workout-logger/android/app/build.gradle.kts`:
- Around line 63-68: The release build signing configuration currently falls
back to the debug key when the release keystore is missing, which can silently
produce incorrectly-signed production artifacts. Remove the fallback logic that
assigns signingConfigs.getByName("debug") when releaseConfig.storeFile is null.
Instead, explicitly throw an error or fail the build when the release signing
configuration is not properly set up, ensuring that release artifacts are never
accidentally signed with the debug key in CI or release pipelines.
In `@workout-logger/lib/main.dart`:
- Around line 193-195: The fire-and-forget calls to ReadinessManager.refresh()
lack error handling, which can cause unhandled Flutter errors if the async
operation fails. At workout-logger/lib/main.dart#L193-195, modify the
readiness.refresh() call to attach .catchError(...) and log any failure so
startup remains non-blocking. At
workout-logger/lib/screens/profile_screen.dart#L177-179, similarly attach
.catchError(...) inside the unawaited(...) call that wraps readiness.refresh()
before showing the success snack, ensuring any async errors are caught and
logged without blocking the UI.
In `@workout-logger/lib/models/models.dart`:
- Around line 1091-1098: The hasStages getter only checks for lightMinutes,
deepMinutes, and remMinutes, but does not account for cases where Health Connect
returns a stage timeline containing only awake intervals. This causes the
minutes getter to incorrectly fall back to the raw session span calculation,
overstating actual sleep time. Modify the hasStages getter to also check for
awakeMinutes or whether stageTimeline is not empty, so that stage data including
only awake minutes is properly recognized as staged data and the minutes getter
uses the accurate stage-based calculation instead of falling back to the raw
span.
In `@workout-logger/lib/models/sleep_hr_models.dart`:
- Around line 72-79: The SleepHrSnapshot constructor at lines 72-79 and a
similar constructor at lines 148-155 are storing mutable List inputs directly
(segments, stageStats, and buckets respectively) without defensive copying. This
allows callers to mutate these lists after construction, violating immutability
expectations. At each constructor location, defensively copy each list field by
wrapping it with List.from() or using the spread operator to create a new list
instance. This ensures that external mutations to the original list cannot
affect the model's internal state, and all modifications must go through
copyWith() as per coding guidelines.
In `@workout-logger/lib/models/workout_hr_models.dart`:
- Around line 68-92: The WorkoutHrAnalysis class exposes mutable List references
for curve, rests, and exercises fields, allowing callers to mutate them directly
after construction which bypasses controlled updates. To fix this, wrap each of
these three list fields with List.unmodifiable() in the constructor to enforce
immutability boundaries. Additionally, implement a copyWith() method on the
WorkoutHrAnalysis class that accepts optional parameters for all fields (start,
end, avgBpm, peakBpm, minBpm, curve, rests, exercises, and hasRestAnalysis) and
returns a new WorkoutHrAnalysis instance with the provided values or the
existing ones if not provided, ensuring all model mutations go through this
controlled method rather than direct mutation.
In `@workout-logger/lib/screens/sleep_detail_screen.dart`:
- Around line 126-142: The `_ist()` method hardcodes the IST timezone offset
(UTC+05:30) which causes incorrect sleep times for users outside that timezone.
Replace the hardcoded duration conversion in the `_ist()` method with
`dt.toLocal()` to use the device's actual local timezone. Additionally, update
the display text in the build method to remove the hardcoded "IST" label since
the time is now shown in device-local time rather than a fixed timezone.
---
Outside diff comments:
In @.github/workflows/release.yml:
- Around line 40-43: Add a test execution step to the release workflow that runs
the Flutter test suite before any APK publishing occurs. After the "Install
dependencies" step at lines 40-43, insert a step to execute flutter test and
configure it to fail the workflow if tests do not pass. Apply the same test
execution gate at the other affected locations in the workflow (lines 122-130
and 151-172) to ensure tests run and pass before any release artifacts are
published, preventing regressions from being shipped.
In `@workout-logger/lib/models/models.dart`:
- Around line 978-990: Wrap the mutable list fields with List.unmodifiable(...)
to enforce immutability at model boundaries. In QuestionSpec.fromJson, wrap the
options list assignment with List.unmodifiable() to prevent external mutation of
the model state. Apply the same pattern to AnswerSpec.selected,
PendingQuestions.questions, and SleepPeriod.stageTimeline in their respective
constructors and fromJson factory methods, ensuring all list fields are made
unmodifiable when assigned from external input to comply with the guideline that
all model mutations must go through copyWith().
In `@workout-logger/lib/screens/ai_program_generator_screen.dart`:
- Around line 480-490: The Text widget displaying phase.name is positioned
before a Spacer in a Row, which allows the AI-generated name to overflow when it
is long. Wrap the Text widget containing phase.name in an Expanded or Flexible
widget to constrain its width, and add overflow: TextOverflow.ellipsis to the
TextStyle to ensure long names are truncated with an ellipsis rather than
overflowing the layout.
In `@workout-logger/lib/screens/home_screen.dart`:
- Around line 118-122: The condition in the WorkoutFlowScreen navigation is
incorrectly using OR logic between the started flag and the resume conflict
action. When resuming an active workout, a new routine was not started, so
pushing WorkoutFlowScreen with the tapped routine will display the wrong
routine. Fix this by changing the condition to only push the WorkoutFlowScreen
when a new workout was actually started (when started is true), and remove
StartWorkoutConflictAction.resume from the OR condition, since resuming should
not navigate to a new workout screen with the tapped routine.
- Around line 1235-1241: The catch block in the weekly insights generation
try-catch statement silently ignores network and API errors by doing nothing.
Replace the empty catch block that catches the exception with code that displays
an error message to the user, such as a SnackBar or inline error message. Check
the mounted state before showing the error to ensure the widget is still in the
tree, and provide a clear user-facing error message so they understand that the
generation failed and can retry if needed.
In `@workout-logger/lib/screens/profile_screen.dart`:
- Around line 211-214: The condition checking the share result status in the
profile_screen.dart file incorrectly treats ShareResultStatus.dismissed as a
success case. When a user dismisses the share dialog, no backup is actually
created, so showing "Backup exported successfully!" is misleading. Refactor the
if statement to only show the success message when the result status equals
ShareResultStatus.success. For the ShareResultStatus.dismissed case, handle it
separately with a distinct message (e.g., indicating the user cancelled the
backup operation) to accurately communicate what happened to the user.
In `@workout-logger/lib/screens/widgets/exercise_details_sheet.dart`:
- Around line 213-234: The success trend banner is displayed whenever
growthModel has a sufficient r2 value, but it does not validate that the slope
is positive. This causes negative slopes to display with a green "trending up"
icon and confusing messages like "+-5.0 lb volume/week". Add an additional
condition to check that growthModel.slope is greater than 0 in the conditional
that guards the Container widget with the success styling,
Icon(Icons.trending_up_rounded), and the Text showing the slope calculation.
This ensures the success banner only appears for positive growth trends, not
negative ones.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: be3b75f8-0157-41d3-83b9-1489e1ab96a1
⛔ Files ignored due to path filters (2)
workout-logger/assets/fonts/Geist-Variable.ttfis excluded by!**/*.ttfworkout-logger/assets/fonts/GeistMono-Variable.ttfis excluded by!**/*.ttf
📒 Files selected for processing (68)
.github/workflows/release.ymldocs/FUTURE_IMPROVEMENTS.mddocs/superpowers/specs/2026-06-11-sleep-hr-chart-design.mdfdroid/metadata/com.devasy.repforge.ymlworkout-logger/android/app/build.gradle.ktsworkout-logger/android/app/src/main/AndroidManifest.xmlworkout-logger/lib/main.dartworkout-logger/lib/models/models.dartworkout-logger/lib/models/sleep_hr_models.dartworkout-logger/lib/models/workout_hr_models.dartworkout-logger/lib/screens/ai_coach_screen.dartworkout-logger/lib/screens/ai_program_generator_screen.dartworkout-logger/lib/screens/analytics_screen.dartworkout-logger/lib/screens/heart_rate_detail_screen.dartworkout-logger/lib/screens/history_screen.dartworkout-logger/lib/screens/home_screen.dartworkout-logger/lib/screens/onboarding_screen.dartworkout-logger/lib/screens/profile_screen.dartworkout-logger/lib/screens/routine_optimizer_screen.dartworkout-logger/lib/screens/routines_screen.dartworkout-logger/lib/screens/sleep_detail_screen.dartworkout-logger/lib/screens/widgets/analytics_overview.dartworkout-logger/lib/screens/widgets/calendar_grid.dartworkout-logger/lib/screens/widgets/exercise_details_sheet.dartworkout-logger/lib/screens/widgets/exercise_input_section.dartworkout-logger/lib/screens/widgets/exercise_progress_view.dartworkout-logger/lib/screens/widgets/health_bar_chart.dartworkout-logger/lib/screens/widgets/health_detail_shell.dartworkout-logger/lib/screens/widgets/heart_rate_card.dartworkout-logger/lib/screens/widgets/muscle_detail_sheet.dartworkout-logger/lib/screens/widgets/profile_sections.dartworkout-logger/lib/screens/widgets/readiness_card.dartworkout-logger/lib/screens/widgets/rf_question_card.dartworkout-logger/lib/screens/widgets/rf_widgets.dartworkout-logger/lib/screens/widgets/session_details_sheet.dartworkout-logger/lib/screens/widgets/sleep_hr_card.dartworkout-logger/lib/screens/widgets/sleep_hr_charts.dartworkout-logger/lib/screens/widgets/targets_tab.dartworkout-logger/lib/screens/widgets/volume_chart.dartworkout-logger/lib/screens/widgets/wheel_picker.dartworkout-logger/lib/screens/widgets/workout_header.dartworkout-logger/lib/screens/widgets/workout_hr_section.dartworkout-logger/lib/screens/workout_flow_screen.dartworkout-logger/lib/services/ai/coach_tool_service.dartworkout-logger/lib/services/ai/gemini_ai_service.dartworkout-logger/lib/services/debug_log_buffer.dartworkout-logger/lib/services/gemini_context_builder.dartworkout-logger/lib/services/health_connect_service.dartworkout-logger/lib/services/interfaces/health_connect_service_interface.dartworkout-logger/lib/services/interfaces/interfaces.dartworkout-logger/lib/services/interfaces/readiness_manager_interface.dartworkout-logger/lib/services/managers/health_history_manager.dartworkout-logger/lib/services/managers/managers.dartworkout-logger/lib/services/managers/readiness_manager.dartworkout-logger/lib/services/ml_service.dartworkout-logger/lib/services/settings_provider.dartworkout-logger/lib/services/utils/readiness_calculator.dartworkout-logger/lib/services/utils/sleep_hr_builder.dartworkout-logger/lib/services/utils/workout_hr_builder.dartworkout-logger/lib/theme/app_theme.dartworkout-logger/pubspec.yamlworkout-logger/test/health_history_manager_test.dartworkout-logger/test/health_sync_manager_test.dartworkout-logger/test/ml_service_test.dartworkout-logger/test/readiness_calculator_test.dartworkout-logger/test/readiness_manager_test.dartworkout-logger/test/routine_optimizer_screen_test.dartworkout-logger/test/workout_hr_builder_test.dart
| **File:** `lib/screens/widgets/sleep_hr_card.dart` | ||
|
|
||
| Layout: | ||
| ``` |
Contributor
There was a problem hiding this comment.
Fix fenced code-block lint violations in the spec.
Line 120 needs a fenced language and blank-line separation; Line 162 needs blank-line separation around the fence. This will clear the markdownlint warnings cleanly.
💡 Suggested fix
Layout:
+
-```
+```text
┌─────────────────────────────────┐
│ Sleep heart rate 1:24–8:17 │ ← header row
│ P95 67bpm REM 64bpm Deep 52bpm│ ← three mini-stats
│ [sparkline bar chart] │ ← canvas, 38dp tall
└─────────────────────────────────┘@@
Stage color map:
+
const _stageColors = {
'deep': Color(0xFF4C8EFF),
'rem': Color(0xFFA78BFA),
'light': Color(0xFF34D399),
'awake': Color(0xFFF59E0B),
};</details>
Also applies to: 162-162
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 120-120: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
---
[warning] 120-120: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @docs/superpowers/specs/2026-06-11-sleep-hr-chart-design.md at line 120, The
markdown linting violations in the spec file need to be fixed at two locations.
At line 120, add the language identifier text to the opening code fence
(change totext) and ensure proper blank-line separation before and after
the fenced code block. At line 162, add a blank line before the opening ```dart
fence to provide proper separation between the preceding text and the fenced
code block. These changes will resolve the markdownlint warnings for missing
language specifiers and improper fence spacing.
</details>
<!-- fingerprinting:phantom:poseidon:hawk -->
<!-- cr-comment:v1:0492749857b299db9a84a2f2 -->
_Source: Linters/SAST tools_
<!-- This is an auto-generated comment by CodeRabbit -->
Comment on lines
+63
to
+68
| // Uses the production EC P-256 keystore when KEYSTORE_PATH env var is set | ||
| // (CI injects it via GitHub Secrets). Falls back to debug key for local | ||
| // flutter run --release without env vars configured. | ||
| val releaseConfig = signingConfigs.getByName("release") | ||
| signingConfig = if (releaseConfig.storeFile != null) releaseConfig | ||
| else signingConfigs.getByName("debug") |
Contributor
There was a problem hiding this comment.
Avoid debug-key fallback for release artifacts in CI/release pipelines.
Line 67-68 silently signs release with the debug key when secrets are missing. That can leak unsigned/incorrectly-signed production artifacts if CI/env config regresses.
💡 Suggested fix
release {
@@
val releaseConfig = signingConfigs.getByName("release")
- signingConfig = if (releaseConfig.storeFile != null) releaseConfig
- else signingConfigs.getByName("debug")
+ val isCi = System.getenv("CI") == "true"
+ if (releaseConfig.storeFile == null && isCi) {
+ throw GradleException(
+ "Release signing is not configured (missing keystore env vars)."
+ )
+ }
+ signingConfig = if (releaseConfig.storeFile != null) {
+ releaseConfig
+ } else {
+ signingConfigs.getByName("debug")
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Uses the production EC P-256 keystore when KEYSTORE_PATH env var is set | |
| // (CI injects it via GitHub Secrets). Falls back to debug key for local | |
| // flutter run --release without env vars configured. | |
| val releaseConfig = signingConfigs.getByName("release") | |
| signingConfig = if (releaseConfig.storeFile != null) releaseConfig | |
| else signingConfigs.getByName("debug") | |
| // Uses the production EC P-256 keystore when KEYSTORE_PATH env var is set | |
| // (CI injects it via GitHub Secrets). Falls back to debug key for local | |
| // flutter run --release without env vars configured. | |
| val releaseConfig = signingConfigs.getByName("release") | |
| val isCi = System.getenv("CI") == "true" | |
| if (releaseConfig.storeFile == null && isCi) { | |
| throw GradleException( | |
| "Release signing is not configured (missing keystore env vars)." | |
| ) | |
| } | |
| signingConfig = if (releaseConfig.storeFile != null) { | |
| releaseConfig | |
| } else { | |
| signingConfigs.getByName("debug") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/android/app/build.gradle.kts` around lines 63 - 68, The
release build signing configuration currently falls back to the debug key when
the release keystore is missing, which can silently produce incorrectly-signed
production artifacts. Remove the fallback logic that assigns
signingConfigs.getByName("debug") when releaseConfig.storeFile is null. Instead,
explicitly throw an error or fail the build when the release signing
configuration is not properly set up, ensuring that release artifacts are never
accidentally signed with the debug key in CI or release pipelines.
Comment on lines
+193
to
+195
| // Fire-and-forget readiness refresh — must run after settings.init() | ||
| // so the opt-in flag is loaded; never blocks or fails app init. | ||
| readiness.refresh(); |
Contributor
There was a problem hiding this comment.
Handle errors from fire-and-forget readiness refreshes. Both call sites intentionally avoid blocking the UI, but neither attaches an error handler; any async failure from ReadinessManager.refresh() can surface as an unhandled Flutter error.
workout-logger/lib/main.dart#L193-L195: callreadiness.refresh().catchError(...)and log the failure so startup remains non-blocking and non-fatal.workout-logger/lib/screens/profile_screen.dart#L177-L179: attach.catchError(...)inside theunawaited(...)call before showing the success snack.
🛡️ Proposed pattern
- readiness.refresh();
+ readiness.refresh().catchError(
+ (Object e, StackTrace st) =>
+ debugPrint('readiness.refresh failed: $e\n$st'),
+ );- unawaited(context.read<ReadinessManager>().refresh(force: true));
+ unawaited(
+ context.read<ReadinessManager>().refresh(force: true).catchError(
+ (Object e, StackTrace st) =>
+ debugPrint('readiness.refresh failed: $e\n$st'),
+ ),
+ );📍 Affects 2 files
workout-logger/lib/main.dart#L193-L195(this comment)workout-logger/lib/screens/profile_screen.dart#L177-L179
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/lib/main.dart` around lines 193 - 195, The fire-and-forget
calls to ReadinessManager.refresh() lack error handling, which can cause
unhandled Flutter errors if the async operation fails. At
workout-logger/lib/main.dart#L193-195, modify the readiness.refresh() call to
attach .catchError(...) and log any failure so startup remains non-blocking. At
workout-logger/lib/screens/profile_screen.dart#L177-179, similarly attach
.catchError(...) inside the unawaited(...) call that wraps readiness.refresh()
before showing the success snack, ensuring any async errors are caught and
logged without blocking the UI.
Comment on lines
+1091
to
+1098
| bool get hasStages => | ||
| lightMinutes != null || deepMinutes != null || remMinutes != null; | ||
|
|
||
| /// Actual sleep minutes: light + deep + rem when stage data exists, | ||
| /// otherwise the raw session span (start → end). | ||
| int get minutes => hasStages | ||
| ? (lightMinutes ?? 0) + (deepMinutes ?? 0) + (remMinutes ?? 0) | ||
| : end.difference(start).inMinutes; |
Contributor
There was a problem hiding this comment.
Treat awake-only stage data as staged sleep.
If Health Connect returns a stage timeline with only awake/out-of-bed intervals, hasStages is false and minutes falls back to the raw session span, overstating actual sleep. Include awakeMinutes or stageTimeline.isNotEmpty in the staged-data check.
🐛 Proposed fix
bool get hasStages =>
- lightMinutes != null || deepMinutes != null || remMinutes != null;
+ lightMinutes != null ||
+ deepMinutes != null ||
+ remMinutes != null ||
+ awakeMinutes != null ||
+ stageTimeline.isNotEmpty;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| bool get hasStages => | |
| lightMinutes != null || deepMinutes != null || remMinutes != null; | |
| /// Actual sleep minutes: light + deep + rem when stage data exists, | |
| /// otherwise the raw session span (start → end). | |
| int get minutes => hasStages | |
| ? (lightMinutes ?? 0) + (deepMinutes ?? 0) + (remMinutes ?? 0) | |
| : end.difference(start).inMinutes; | |
| bool get hasStages => | |
| lightMinutes != null || | |
| deepMinutes != null || | |
| remMinutes != null || | |
| awakeMinutes != null || | |
| stageTimeline.isNotEmpty; | |
| /// Actual sleep minutes: light + deep + rem when stage data exists, | |
| /// otherwise the raw session span (start → end). | |
| int get minutes => hasStages | |
| ? (lightMinutes ?? 0) + (deepMinutes ?? 0) + (remMinutes ?? 0) | |
| : end.difference(start).inMinutes; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/lib/models/models.dart` around lines 1091 - 1098, The
hasStages getter only checks for lightMinutes, deepMinutes, and remMinutes, but
does not account for cases where Health Connect returns a stage timeline
containing only awake intervals. This causes the minutes getter to incorrectly
fall back to the raw session span calculation, overstating actual sleep time.
Modify the hasStages getter to also check for awakeMinutes or whether
stageTimeline is not empty, so that stage data including only awake minutes is
properly recognized as staged data and the minutes getter uses the accurate
stage-based calculation instead of falling back to the raw span.
Comment on lines
+72
to
+79
| const SleepHrSnapshot({ | ||
| required this.sleepStart, | ||
| required this.sleepEnd, | ||
| required this.p5Bpm, | ||
| required this.p95Bpm, | ||
| required this.segments, | ||
| required this.stageStats, | ||
| }); |
Contributor
There was a problem hiding this comment.
Defensively freeze list fields in model constructors.
Line 72-79 and Line 148-155 store mutable List inputs directly. Callers can mutate segments, stageStats, or buckets after construction, bypassing immutable-model expectations.
As per coding guidelines: "All model mutations in Dart must go through copyWith() — never mutate state directly".
💡 Suggested fix
class SleepHrSnapshot {
@@
- const SleepHrSnapshot({
+ const SleepHrSnapshot({
required this.sleepStart,
required this.sleepEnd,
required this.p5Bpm,
required this.p95Bpm,
- required this.segments,
- required this.stageStats,
- });
+ required List<SleepHrSegment> segments,
+ required List<SleepStageStats> stageStats,
+ }) : segments = List.unmodifiable(segments),
+ stageStats = List.unmodifiable(stageStats);
@@
class HrDaySnapshot {
@@
- const HrDaySnapshot({
+ const HrDaySnapshot({
required this.day,
required this.restingBpm,
required this.minBpm,
required this.maxBpm,
required this.avgBpm,
- required this.buckets,
- });
+ required List<HrBucket> buckets,
+ }) : buckets = List.unmodifiable(buckets);Also applies to: 148-155
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/lib/models/sleep_hr_models.dart` around lines 72 - 79, The
SleepHrSnapshot constructor at lines 72-79 and a similar constructor at lines
148-155 are storing mutable List inputs directly (segments, stageStats, and
buckets respectively) without defensive copying. This allows callers to mutate
these lists after construction, violating immutability expectations. At each
constructor location, defensively copy each list field by wrapping it with
List.from() or using the spread operator to create a new list instance. This
ensures that external mutations to the original list cannot affect the model's
internal state, and all modifications must go through copyWith() as per coding
guidelines.
Source: Coding guidelines
Comment on lines
+68
to
+92
| final List<HrCurvePoint> curve; | ||
|
|
||
| /// Per-rest recovery. Empty when set timestamps aren't trustworthy | ||
| /// ([hasRestAnalysis] is false) — the curve still renders. | ||
| final List<RestRecovery> rests; | ||
|
|
||
| /// Exercise sections across the session, ordered in time. Empty when set | ||
| /// timestamps aren't trustworthy. | ||
| final List<ExerciseHrSpan> exercises; | ||
|
|
||
| /// Whether rest/section analysis was computed (set timestamps spanned the | ||
| /// session). | ||
| final bool hasRestAnalysis; | ||
|
|
||
| const WorkoutHrAnalysis({ | ||
| required this.start, | ||
| required this.end, | ||
| required this.avgBpm, | ||
| required this.peakBpm, | ||
| required this.minBpm, | ||
| required this.curve, | ||
| required this.rests, | ||
| required this.exercises, | ||
| required this.hasRestAnalysis, | ||
| }); |
Contributor
There was a problem hiding this comment.
Enforce immutable list boundaries and add copyWith() on WorkoutHrAnalysis.
WorkoutHrAnalysis stores raw List references, so callers can mutate curve/rests/exercises after construction. That bypasses controlled model updates and can desync derived getters.
♻️ Suggested fix
+import 'dart:collection';
+
class WorkoutHrAnalysis {
@@
- final List<HrCurvePoint> curve;
+ final UnmodifiableListView<HrCurvePoint> curve;
@@
- final List<RestRecovery> rests;
+ final UnmodifiableListView<RestRecovery> rests;
@@
- final List<ExerciseHrSpan> exercises;
+ final UnmodifiableListView<ExerciseHrSpan> exercises;
@@
- const WorkoutHrAnalysis({
+ WorkoutHrAnalysis({
required this.start,
required this.end,
required this.avgBpm,
required this.peakBpm,
required this.minBpm,
- required this.curve,
- required this.rests,
- required this.exercises,
+ required List<HrCurvePoint> curve,
+ required List<RestRecovery> rests,
+ required List<ExerciseHrSpan> exercises,
required this.hasRestAnalysis,
- });
+ }) : curve = UnmodifiableListView(curve),
+ rests = UnmodifiableListView(rests),
+ exercises = UnmodifiableListView(exercises);
+
+ WorkoutHrAnalysis copyWith({
+ DateTime? start,
+ DateTime? end,
+ int? avgBpm,
+ int? peakBpm,
+ int? minBpm,
+ List<HrCurvePoint>? curve,
+ List<RestRecovery>? rests,
+ List<ExerciseHrSpan>? exercises,
+ bool? hasRestAnalysis,
+ }) {
+ return WorkoutHrAnalysis(
+ start: start ?? this.start,
+ end: end ?? this.end,
+ avgBpm: avgBpm ?? this.avgBpm,
+ peakBpm: peakBpm ?? this.peakBpm,
+ minBpm: minBpm ?? this.minBpm,
+ curve: curve ?? this.curve,
+ rests: rests ?? this.rests,
+ exercises: exercises ?? this.exercises,
+ hasRestAnalysis: hasRestAnalysis ?? this.hasRestAnalysis,
+ );
+ }As per coding guidelines: "All model mutations in Dart must go through copyWith() — never mutate state directly".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| final List<HrCurvePoint> curve; | |
| /// Per-rest recovery. Empty when set timestamps aren't trustworthy | |
| /// ([hasRestAnalysis] is false) — the curve still renders. | |
| final List<RestRecovery> rests; | |
| /// Exercise sections across the session, ordered in time. Empty when set | |
| /// timestamps aren't trustworthy. | |
| final List<ExerciseHrSpan> exercises; | |
| /// Whether rest/section analysis was computed (set timestamps spanned the | |
| /// session). | |
| final bool hasRestAnalysis; | |
| const WorkoutHrAnalysis({ | |
| required this.start, | |
| required this.end, | |
| required this.avgBpm, | |
| required this.peakBpm, | |
| required this.minBpm, | |
| required this.curve, | |
| required this.rests, | |
| required this.exercises, | |
| required this.hasRestAnalysis, | |
| }); | |
| import 'dart:collection'; | |
| final UnmodifiableListView<HrCurvePoint> curve; | |
| /// Per-rest recovery. Empty when set timestamps aren't trustworthy | |
| /// ([hasRestAnalysis] is false) — the curve still renders. | |
| final UnmodifiableListView<RestRecovery> rests; | |
| /// Exercise sections across the session, ordered in time. Empty when set | |
| /// timestamps aren't trustworthy. | |
| final UnmodifiableListView<ExerciseHrSpan> exercises; | |
| /// Whether rest/section analysis was computed (set timestamps spanned the | |
| /// session). | |
| final bool hasRestAnalysis; | |
| WorkoutHrAnalysis({ | |
| required this.start, | |
| required this.end, | |
| required this.avgBpm, | |
| required this.peakBpm, | |
| required this.minBpm, | |
| required List<HrCurvePoint> curve, | |
| required List<RestRecovery> rests, | |
| required List<ExerciseHrSpan> exercises, | |
| required this.hasRestAnalysis, | |
| }) : curve = UnmodifiableListView(curve), | |
| rests = UnmodifiableListView(rests), | |
| exercises = UnmodifiableListView(exercises); | |
| WorkoutHrAnalysis copyWith({ | |
| DateTime? start, | |
| DateTime? end, | |
| int? avgBpm, | |
| int? peakBpm, | |
| int? minBpm, | |
| List<HrCurvePoint>? curve, | |
| List<RestRecovery>? rests, | |
| List<ExerciseHrSpan>? exercises, | |
| bool? hasRestAnalysis, | |
| }) { | |
| return WorkoutHrAnalysis( | |
| start: start ?? this.start, | |
| end: end ?? this.end, | |
| avgBpm: avgBpm ?? this.avgBpm, | |
| peakBpm: peakBpm ?? this.peakBpm, | |
| minBpm: minBpm ?? this.minBpm, | |
| curve: curve ?? this.curve, | |
| rests: rests ?? this.rests, | |
| exercises: exercises ?? this.exercises, | |
| hasRestAnalysis: hasRestAnalysis ?? this.hasRestAnalysis, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/lib/models/workout_hr_models.dart` around lines 68 - 92, The
WorkoutHrAnalysis class exposes mutable List references for curve, rests, and
exercises fields, allowing callers to mutate them directly after construction
which bypasses controlled updates. To fix this, wrap each of these three list
fields with List.unmodifiable() in the constructor to enforce immutability
boundaries. Additionally, implement a copyWith() method on the WorkoutHrAnalysis
class that accepts optional parameters for all fields (start, end, avgBpm,
peakBpm, minBpm, curve, rests, exercises, and hasRestAnalysis) and returns a new
WorkoutHrAnalysis instance with the provided values or the existing ones if not
provided, ensuring all model mutations go through this controlled method rather
than direct mutation.
Source: Coding guidelines
Comment on lines
+126
to
+142
| static DateTime _ist(DateTime dt) => dt.toUtc().add(const Duration(hours: 5, minutes: 30)); | ||
| static String _fmt(DateTime dt) { | ||
| final h = dt.hour == 0 ? 12 : dt.hour > 12 ? dt.hour - 12 : dt.hour; | ||
| final m = dt.minute.toString().padLeft(2, '0'); | ||
| return '$h:$m ${dt.hour < 12 ? 'AM' : 'PM'}'; | ||
| } | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| return GlassCard( | ||
| padding: const EdgeInsets.all(16), | ||
| child: Column( | ||
| crossAxisAlignment: CrossAxisAlignment.start, | ||
| children: [ | ||
| Text( | ||
| 'Asleep · ${_fmt(_ist(snapshot.sleepStart))} – ${_fmt(_ist(snapshot.sleepEnd))} IST', | ||
| style: TextStyle(fontFamily: 'Geist', color: AppColors.textMuted, fontSize: 12), |
Contributor
There was a problem hiding this comment.
Use device-local time instead of hardcoded IST.
Sleep data should be shown in the user’s local timezone. Converting every timestamp to UTC+05:30 makes sleep start/end times wrong for users outside IST.
🐛 Proposed fix
- static DateTime _ist(DateTime dt) => dt.toUtc().add(const Duration(hours: 5, minutes: 30));
- static String _fmt(DateTime dt) {
- final h = dt.hour == 0 ? 12 : dt.hour > 12 ? dt.hour - 12 : dt.hour;
- final m = dt.minute.toString().padLeft(2, '0');
- return '$h:$m ${dt.hour < 12 ? 'AM' : 'PM'}';
- }
+ static String _fmt(DateTime dt) => DateFormat.jm().format(dt.toLocal());
@@
- 'Asleep · ${_fmt(_ist(snapshot.sleepStart))} – ${_fmt(_ist(snapshot.sleepEnd))} IST',
+ 'Asleep · ${_fmt(snapshot.sleepStart)} – ${_fmt(snapshot.sleepEnd)}',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| static DateTime _ist(DateTime dt) => dt.toUtc().add(const Duration(hours: 5, minutes: 30)); | |
| static String _fmt(DateTime dt) { | |
| final h = dt.hour == 0 ? 12 : dt.hour > 12 ? dt.hour - 12 : dt.hour; | |
| final m = dt.minute.toString().padLeft(2, '0'); | |
| return '$h:$m ${dt.hour < 12 ? 'AM' : 'PM'}'; | |
| } | |
| @override | |
| Widget build(BuildContext context) { | |
| return GlassCard( | |
| padding: const EdgeInsets.all(16), | |
| child: Column( | |
| crossAxisAlignment: CrossAxisAlignment.start, | |
| children: [ | |
| Text( | |
| 'Asleep · ${_fmt(_ist(snapshot.sleepStart))} – ${_fmt(_ist(snapshot.sleepEnd))} IST', | |
| style: TextStyle(fontFamily: 'Geist', color: AppColors.textMuted, fontSize: 12), | |
| static String _fmt(DateTime dt) => DateFormat.jm().format(dt.toLocal()); | |
| `@override` | |
| Widget build(BuildContext context) { | |
| return GlassCard( | |
| padding: const EdgeInsets.all(16), | |
| child: Column( | |
| crossAxisAlignment: CrossAxisAlignment.start, | |
| children: [ | |
| Text( | |
| 'Asleep · ${_fmt(snapshot.sleepStart)} – ${_fmt(snapshot.sleepEnd)}', | |
| style: TextStyle(fontFamily: 'Geist', color: AppColors.textMuted, fontSize: 12), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workout-logger/lib/screens/sleep_detail_screen.dart` around lines 126 - 142,
The `_ist()` method hardcodes the IST timezone offset (UTC+05:30) which causes
incorrect sleep times for users outside that timezone. Replace the hardcoded
duration conversion in the `_ist()` method with `dt.toLocal()` to use the
device's actual local timezone. Additionally, update the display text in the
build method to remove the hardcoded "IST" label since the time is now shown in
device-local time rather than a fixed timezone.
Owner
Author
|
@coderabbitai make a consolidated issue for all the open unaddressed review comments |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="Devasy/Workout-logger"
owner="${repo%%/*}"
name="${repo#*/}"
pr_number="48"
requester="Devasy"
pr_url="https://github.com/${repo}/pull/${pr_number}"
tmp_json="$(mktemp)"
: > "$tmp_json"
after="null"
while :; do
if [[ "$after" == "null" ]]; then
response="$(gh api graphql \
-F owner="$owner" \
-F name="$name" \
-F prNumber="$pr_number" \
-f query='
query($owner: String!, $name: String!, $prNumber: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $prNumber) {
reviewThreads(first: 100) {
pageInfo { hasNextPage endCursor }
nodes {
id
isResolved
isOutdated
path
line
startLine
originalLine
diffSide
comments(first: 50) {
nodes {
id
author { login }
body
url
createdAt
}
}
}
}
}
}
}')"
else
response="$(gh api graphql \
-F owner="$owner" \
-F name="$name" \
-F prNumber="$pr_number" \
-F after="$after" \
-f query='
query($owner: String!, $name: String!, $prNumber: Int!, $after: String!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $prNumber) {
reviewThreads(first: 100, after: $after) {
pageInfo { hasNextPage endCursor }
nodes {
id
isResolved
isOutdated
path
line
startLine
originalLine
diffSide
comments(first: 50) {
nodes {
id
author { login }
body
url
createdAt
}
}
}
}
}
}
}')"
fi
echo "$response" | jq -c '.data.repository.pullRequest.reviewThreads.nodes[]' >> "$tmp_json"
has_next="$(echo "$response" | jq -r '.data.repository.pullRequest.reviewThreads.pageInfo.hasNextPage')"
after="$(echo "$response" | jq -r '.data.repository.pullRequest.reviewThreads.pageInfo.endCursor')"
[[ "$has_next" == "true" && "$after" != "null" ]] || break
done
threads_json="$(mktemp)"
jq -s '
map(select(.isResolved == false))
| map(select([.comments.nodes[].author.login // ""] | any(test("coderabbit"; "i"))))
| map({
path,
line: (.line // .originalLine // .startLine),
isOutdated,
url: ((.comments.nodes | map(select((.author.login // "") | test("coderabbit"; "i"))) | .[0].url) // (.comments.nodes[0].url // "")),
createdAt: ((.comments.nodes | map(select((.author.login // "") | test("coderabbit"; "i"))) | .[0].createdAt) // (.comments.nodes[0].createdAt // "")),
body: ((.comments.nodes | map(select((.author.login // "") | test("coderabbit"; "i"))) | .[0].body) // "")
})
' "$tmp_json" > "$threads_json"
count="$(jq 'length' "$threads_json")"
if [[ "$count" -eq 0 ]]; then
echo "NO_UNRESOLVED_CODERABBIT_THREADS"
exit 0
fi
body_file="$(mktemp)"
{
echo "## Consolidated follow-up for unresolved PR review comments"
echo
echo "Requested by: @${requester}"
echo
echo "Source PR: ${pr_url}"
echo
echo "This issue tracks the open, unresolved CodeRabbit review threads remaining on PR #${pr_number} so they can be handled after merge without losing context."
echo
echo "Total unresolved CodeRabbit review threads found: **${count}**"
echo
echo "### Open review threads"
echo
} > "$body_file"
jq -r '
to_entries[]
| .key as $i
| .value
| "#### " + (($i + 1) | tostring) + ". `" + (.path // "Unknown path") + "`" +
(if .line then " — line " + (.line | tostring) else "" end) + "\n\n" +
"- Review thread: " + (.url // "") + "\n" +
"- Outdated thread: " + (if .isOutdated then "yes" else "no" end) + "\n" +
"- Created: " + (.createdAt // "unknown") + "\n" +
"- Concern excerpt:\n\n" +
" > " + (
(.body // "")
| gsub("\r"; "")
| split("\n")
| map(select(length > 0))
| .[0:8]
| join("\n > ")
) + "\n"
' "$threads_json" >> "$body_file"
{
echo
echo "### Notes"
echo
echo "- This issue intentionally links back to the original review threads for full context."
echo "- No implementation details are prescribed here; use the linked review comments as the source of truth."
} >> "$body_file"
title="Consolidate unresolved CodeRabbit review comments from PR #${pr_number}"
issue_url="$(gh issue create \
--repo "$repo" \
--title "$title" \
--body-file "$body_file" \
--assignee "$requester")"
echo "CREATED_ISSUE_URL=$issue_url"
echo "UNRESOLVED_THREAD_COUNT=$count"
echo "ISSUE_BODY_PREVIEW_START"
sed -n '1,220p' "$body_file"
echo "ISSUE_BODY_PREVIEW_END"Length of output: 15920
Issue: #52 It includes:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements