diff --git a/workout-logger/lib/screens/programs/program_designer_screen.dart b/workout-logger/lib/screens/programs/program_designer_screen.dart index ecf4c70..1a6ad5f 100644 --- a/workout-logger/lib/screens/programs/program_designer_screen.dart +++ b/workout-logger/lib/screens/programs/program_designer_screen.dart @@ -429,7 +429,7 @@ class _ProgramDesignerScreenState extends State { decoration: const InputDecoration( labelText: 'Day of Week (optional)', ), - value: dow, + initialValue: dow, items: [ const DropdownMenuItem(value: null, child: Text('Unscheduled')), ...List.generate(7, (i) => i + 1).map((d) => DropdownMenuItem( diff --git a/workout-logger/lib/screens/programs/program_detail_screen.dart b/workout-logger/lib/screens/programs/program_detail_screen.dart index 3a50687..90ee035 100644 --- a/workout-logger/lib/screens/programs/program_detail_screen.dart +++ b/workout-logger/lib/screens/programs/program_detail_screen.dart @@ -6,7 +6,6 @@ import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; import 'package:provider/provider.dart'; -import 'dart:ui' show FontFeature; import '../../models/models.dart'; import '../../services/workout_provider.dart'; diff --git a/workout-logger/lib/services/managers/analytics_manager.dart b/workout-logger/lib/services/managers/analytics_manager.dart index 5af7df2..e234bba 100644 --- a/workout-logger/lib/services/managers/analytics_manager.dart +++ b/workout-logger/lib/services/managers/analytics_manager.dart @@ -26,6 +26,13 @@ class AnalyticsManager extends ChangeNotifier { // Growth models for each exercise final Map _growthModels = {}; + // Reference to track the sessions list identity and detect stale usage + List? _lastIndexedSessions; + + // Pre-computed exerciseId → sorted-newest-first ExerciseLog and date index. + // Rebuilt via [buildSessionIndex] whenever the session list changes. + Map> _sessionIndex = {}; + // Callback to update targets with new growth models final void Function(String exerciseId, GrowthModel model)? onGrowthModelUpdated; @@ -35,6 +42,32 @@ class AnalyticsManager extends ChangeNotifier { // Getters Map get growthModels => Map.unmodifiable(_growthModels); + /// Build (or rebuild) the exerciseId → sorted log index from [sessions]. + /// + /// Callers MUST call this after any session mutation (add, delete, update). + /// The index maps each exercise id to a list of [ExerciseLog] instances + /// ordered newest-first, which lets recommendation and progression APIs + /// avoid repeated O(N log N) sorts and O(N²) scans on every render. + void buildSessionIndex(List sessions) { + _lastIndexedSessions = sessions; + + final index = >{}; + + // Sort sessions newest-first once and iterate + final sorted = List.from(sessions) + ..sort((a, b) => b.date.compareTo(a.date)); + + for (final session in sorted) { + for (final log in session.exercises) { + if (log.sets.isNotEmpty) { + index.putIfAbsent(log.exerciseId, () => []).add((log: log, date: session.date)); + } + } + } + + _sessionIndex = index; + } + /// Get growth model for a specific exercise GrowthModel? getGrowthModel(String exerciseId) => _growthModels[exerciseId]; @@ -96,12 +129,20 @@ class AnalyticsManager extends ChangeNotifier { /// Get set recommendations for an exercise. /// - /// Uses the most-recently-dated session containing this exercise. + /// Uses the most-recently-dated session containing this exercise as the + /// basis for the recommendation. Reads from the pre-built session index + /// so no sort is required at call time — O(1). List getRecommendations( String exerciseId, List sessions, ) { - final lastLog = findMostRecentExerciseLog(exerciseId, sessions); + final isFresh = identical(_lastIndexedSessions, sessions); + + // Fast path: use pre-built index if available and fresh. + final logs = isFresh ? _sessionIndex[exerciseId] : null; + final lastLog = (logs != null && logs.isNotEmpty) + ? logs.first.log // already sorted newest-first + : findMostRecentExerciseLog(exerciseId, sessions); // fallback if (lastLog == null || lastLog.sets.isEmpty) { return _mlService.getDefaultRecommendations(3); @@ -113,14 +154,34 @@ class AnalyticsManager extends ChangeNotifier { ); } - /// Get volume progression for an exercise + /// Get volume progression for an exercise. + /// + /// Reads from the pre-built session index (sorted oldest-first by + /// reversing the newest-first index list), so no allocation or sort is + /// required at call time — O(K) where K is the number of logs for this + /// exercise. List<({DateTime date, double volume})> getVolumeProgression( String exerciseId, List sessions, ) { - final data = <({DateTime date, double volume})>[]; + final isFresh = identical(_lastIndexedSessions, sessions); + + if (isFresh) { + if (_sessionIndex.containsKey(exerciseId)) { + // Fast path: use pre-built index + final logs = _sessionIndex[exerciseId]!; + // Index is newest-first; progression needs oldest-first. + return [ + for (final entry in logs.reversed) + (date: entry.date, volume: entry.log.totalVolume), + ]; + } + // Index is fresh but key is missing, meaning history is truly empty + return []; + } - // Process sessions in chronological order (oldest first) + // Fallback (index not built yet or stale) — sort + scan. + final data = <({DateTime date, double volume})>[]; final sortedSessions = List.from(sessions) ..sort((a, b) => a.date.compareTo(b.date)); @@ -132,27 +193,33 @@ class AnalyticsManager extends ChangeNotifier { } } } - return data; } - /// Get weekly volume by muscle group + + + /// Get weekly volume by muscle group. /// - /// [now] parameter allows test injection of a fixed timestamp for deterministic testing. + /// [exerciseMap] is a pre-built id → Exercise map for O(1) lookups, + /// eliminating the inner O(N) scan from the original [_findExercise]. + /// [now] parameter allows test injection of a fixed timestamp. Map getWeeklyVolumeByMuscle( List sessions, List exercises, { DateTime? now, + Map? exerciseMap, }) { final volumeByMuscle = {}; final currentTime = now ?? DateTime.now(); final weekAgo = currentTime.subtract(const Duration(days: 7)); + final Map lookup = + exerciseMap ?? {for (final e in exercises) e.id: e}; for (var session in sessions) { if (session.date.isBefore(weekAgo)) continue; for (var log in session.exercises) { - final exercise = _findExercise(log.exerciseId, exercises); + final exercise = lookup[log.exerciseId]; if (exercise == null) continue; for (var activation in exercise.muscleActivations) { @@ -167,10 +234,7 @@ class AnalyticsManager extends ChangeNotifier { return volumeByMuscle; } - Exercise? _findExercise(String id, List exercises) { - final index = exercises.indexWhere((e) => e.id == id); - return index != -1 ? exercises[index] : null; - } + /// Get quick stats for dashboard Future> getQuickStats() async { diff --git a/workout-logger/lib/services/managers/exercise_manager.dart b/workout-logger/lib/services/managers/exercise_manager.dart index 300fb8f..f7edfd7 100644 --- a/workout-logger/lib/services/managers/exercise_manager.dart +++ b/workout-logger/lib/services/managers/exercise_manager.dart @@ -24,6 +24,10 @@ class ExerciseManager extends ChangeNotifier { List _allExercises = []; + /// Memoized id → Exercise map for O(1) lookups. + /// Rebuilt whenever [_allExercises] changes. + Map _exerciseIndex = {}; + /// Allowed category values for exercises static const Set allowedCategories = {'compound', 'isolation'}; @@ -39,17 +43,15 @@ class ExerciseManager extends ChangeNotifier { /// Load all exercises from storage and database Future loadExercises() async { _allExercises = await _storage.getAllExercises(); + _exerciseIndex = {for (final e in _allExercises) e.id: e}; notifyListeners(); } - /// Get an exercise by ID + /// Get an exercise by ID in O(1) via the memoized index. /// /// Returns null if not found. Caller should ensure loadExercises() has been /// called first to populate the in-memory exercise list. - Exercise? getExercise(String id) { - final index = _allExercises.indexWhere((e) => e.id == id); - return index != -1 ? _allExercises[index] : null; - } + Exercise? getExercise(String id) => _exerciseIndex[id]; /// Get exercise name by ID String getExerciseName(String id) { @@ -109,6 +111,7 @@ class ExerciseManager extends ChangeNotifier { await _storage.saveCustomExercise(exercise); _allExercises = List.from(_allExercises)..add(exercise); + _exerciseIndex[id] = exercise; notifyListeners(); return exercise; @@ -135,6 +138,7 @@ class ExerciseManager extends ChangeNotifier { await _storage.deleteCustomExercise(exerciseId); _allExercises = List.from(_allExercises) ..removeWhere((e) => e.id == exerciseId); + _exerciseIndex.remove(exerciseId); notifyListeners(); return true; diff --git a/workout-logger/lib/services/strategies/target_calculator.dart b/workout-logger/lib/services/strategies/target_calculator.dart index b92fa71..d7d8aea 100644 --- a/workout-logger/lib/services/strategies/target_calculator.dart +++ b/workout-logger/lib/services/strategies/target_calculator.dart @@ -38,12 +38,10 @@ class RepsTargetCalculator implements TargetCalculatorStrategy { double calculate(String exerciseId, List sessions) { double bestValue = 0; - for (var log in _getExerciseLogsForExercise(exerciseId, sessions)) { - final maxReps = log.sets - .map((s) => s.reps) - .reduce((a, b) => a > b ? a : b); - if (maxReps > bestValue) { - bestValue = maxReps.toDouble(); + for (final log in _getExerciseLogsForExercise(exerciseId, sessions)) { + // In-place max scan: skips intermediate map()/reduce() allocation. + for (final set in log.sets) { + if (set.reps > bestValue) bestValue = set.reps.toDouble(); } } diff --git a/workout-logger/lib/services/workout_provider.dart b/workout-logger/lib/services/workout_provider.dart index 172c7f9..98b660c 100644 --- a/workout-logger/lib/services/workout_provider.dart +++ b/workout-logger/lib/services/workout_provider.dart @@ -208,43 +208,31 @@ class WorkoutProvider extends ChangeNotifier { return false; } - // Check for references in Sessions - for (var session in _sessions) { - if (session.exercises.any((e) => e.exerciseId == exerciseId)) { - debugPrint( - 'Cannot delete custom exercise: Used in session ${session.id}', - ); - return false; - } - } + // Build a set of referenced exercise IDs in a single pass over + // sessions, routines, targets, and the active workout. + final referencedIds = {}; + final referenceReasons = >{}; - // Check for references in Routines - for (var routine in _routines) { - if (routine.exerciseIds.contains(exerciseId)) { - debugPrint( - 'Cannot delete custom exercise: Used in routine ${routine.name}', - ); - return false; - } + void addReference(String id, String reason) { + referencedIds.add(id); + referenceReasons.putIfAbsent(id, () => {}).add(reason); } - // Check for references in Targets - for (var target in _targets) { - if (target.exerciseId == exerciseId) { - debugPrint( - 'Cannot delete custom exercise: Used in target ${target.id}', - ); - return false; - } + for (final s in _sessions) { + for (final e in s.exercises) addReference(e.exerciseId, '_sessions'); } - - // Check for references in active workout - if (_currentExerciseLogs.any((l) => l.exerciseId == exerciseId)) { - debugPrint('Cannot delete custom exercise: Used in active workout'); - return false; + for (final r in _routines) { + for (final id in r.exerciseIds) addReference(id, '_routines'); + } + for (final t in _targets) addReference(t.exerciseId, '_targets'); + for (final l in _currentExerciseLogs) addReference(l.exerciseId, '_currentExerciseLogs'); + if (_activeRoutine != null) { + for (final id in _activeRoutine!.exerciseIds) addReference(id, '_activeRoutine'); } - if (_activeRoutine?.exerciseIds.contains(exerciseId) ?? false) { - debugPrint('Cannot delete custom exercise: Used in active routine'); + + if (referencedIds.contains(exerciseId)) { + final reasons = referenceReasons[exerciseId]?.join(', ') ?? 'unknown'; + debugPrint('Cannot delete custom exercise $exerciseId: still referenced in $reasons'); return false; } @@ -693,19 +681,31 @@ class WorkoutProvider extends ChangeNotifier { return data; } - /// Get weekly volume by muscle group + /// Get weekly volume by muscle group. + /// + /// Sessions are traversed newest-first and the loop breaks once we reach + /// a session older than 7 days, so only in-range sessions are visited. + /// An exercise map is built once at the top for O(1) per-set lookup. Map getWeeklyVolumeByMuscle() { final volumeByMuscle = {}; final weekAgo = DateTime.now().subtract(const Duration(days: 7)); - for (var session in _sessions) { - if (session.date.isBefore(weekAgo)) continue; + // Pre-build exercise map for O(1) lookup + final exerciseMap = { + for (final e in _allExercises) e.id: e, + }; - for (var log in session.exercises) { - final exercise = getExercise(log.exerciseId); + // Iterate directly since _sessions is maintained newest-first. + // We use continue rather than break in case of external imports + // that might temporarily violate the newest-first invariant. + for (final session in _sessions) { + if (session.date.isBefore(weekAgo)) continue; + + for (final log in session.exercises) { + final exercise = exerciseMap[log.exerciseId]; if (exercise == null) continue; - for (var activation in exercise.muscleActivations) { + for (final activation in exercise.muscleActivations) { final muscleVolume = log.totalVolume * (activation.activationPercentage / 100); volumeByMuscle[activation.muscleGroupId] = diff --git a/workout-logger/pubspec.yaml b/workout-logger/pubspec.yaml index 905d530..982aaf9 100644 --- a/workout-logger/pubspec.yaml +++ b/workout-logger/pubspec.yaml @@ -19,7 +19,7 @@ publish_to: 'none' # Remove this line if you wish to publish to pub.dev version: 1.0.15+16 environment: - sdk: ^3.9.2 + sdk: ^3.11.4 flutter: 3.41.6 # Dependencies specify other packages that your package needs in order to work. diff --git a/workout-logger/test/performance_regression_test.dart b/workout-logger/test/performance_regression_test.dart new file mode 100644 index 0000000..857f600 --- /dev/null +++ b/workout-logger/test/performance_regression_test.dart @@ -0,0 +1,348 @@ +// Performance regression tests for PR 2 optimisations. +// +// Strategy: seed a fixture with N sessions × M exercises and assert that +// the new index/cache-based paths return results identical to the brute-force +// reference implementation. If behaviour drifts the tests will catch it, +// without needing micro-benchmark assertions. + +import 'package:flutter_test/flutter_test.dart'; +import 'package:repforge/models/models.dart'; +import 'package:repforge/services/managers/analytics_manager.dart'; +import 'package:repforge/services/managers/exercise_manager.dart'; +import 'package:repforge/services/interfaces/ml_service_interface.dart'; +import 'package:repforge/services/strategies/target_calculator.dart'; +import 'package:repforge/services/workout_provider.dart'; +import 'package:repforge/services/managers/program_manager.dart'; +import 'test_utils/mock_storage_service.dart'; +import 'test_utils/mock_ml_service.dart'; + + +// --------------------------------------------------------------------------- +// Fixture helpers +// --------------------------------------------------------------------------- + +/// Build N workout sessions, each containing logs for all [exerciseIds]. +List buildFixture({ + required List exerciseIds, + required int sessionCount, +}) { + final sessions = []; + for (var i = 0; i < sessionCount; i++) { + final date = DateTime(2025, 1, 1).add(Duration(days: i)); + final logs = exerciseIds.map((id) { + // Weight & reps increase over time so there is a recognisable maximum. + final weight = 50.0 + i.toDouble(); + final reps = 5 + (i % 5); + return ExerciseLog( + exerciseId: id, + sets: [WorkoutSet(weight: weight, reps: reps)], + ); + }).toList(); + sessions.add( + WorkoutSession( + id: 'session_$i', + date: date, + exercises: logs, + duration: 30, + ), + ); + } + // Return in reverse order so that insertion order ≠ chronological order, + // catching any code that relied on implicit ordering. + return sessions.reversed.toList(); +} + +const _exerciseIds = [ + 'ex_0', + 'ex_1', + 'ex_2', + 'ex_3', + 'ex_4', + 'ex_5', + 'ex_6', + 'ex_7', + 'ex_8', + 'ex_9', +]; + +// --------------------------------------------------------------------------- +// AnalyticsManager — session index +// --------------------------------------------------------------------------- + +void main() { + group('AnalyticsManager.buildSessionIndex — behavioral parity', () { + late MockStorageService mockStorage; + late AnalyticsManager analytics; + late List sessions; + + setUp(() { + mockStorage = MockStorageService(); + analytics = AnalyticsManager(mockStorage, MockMLService()); + sessions = buildFixture(exerciseIds: _exerciseIds, sessionCount: 10); + }); + + test('getRecommendations with index == getRecommendations without index', + () { + // Without index (cold) + final withoutIndex = analytics.getRecommendations('ex_0', sessions); + + // Build the index + analytics.buildSessionIndex(sessions); + + // With index (warm) + final withIndex = analytics.getRecommendations('ex_0', sessions); + + expect( + withIndex.length, + equals(withoutIndex.length), + reason: 'Recommendation count should be identical', + ); + for (var i = 0; i < withoutIndex.length; i++) { + expect( + withIndex[i].reps, + equals(withoutIndex[i].reps), + reason: 'reps mismatch at index $i', + ); + expect( + withIndex[i].weight, + equals(withoutIndex[i].weight), + reason: 'weight mismatch at index $i', + ); + } + }); + + test('getVolumeProgression with index is chronologically ordered', () { + analytics.buildSessionIndex(sessions); + final progression = analytics.getVolumeProgression('ex_3', sessions); + + // Should be oldest-first + expect(progression, isNotEmpty); + for (var i = 1; i < progression.length; i++) { + expect( + progression[i].date.isAfter(progression[i - 1].date) || + progression[i].date.isAtSameMomentAs(progression[i - 1].date), + isTrue, + reason: 'Progression should be in ascending date order', + ); + } + }); + + test('getVolumeProgression volumes match brute-force totals', () { + analytics.buildSessionIndex(sessions); + final indexedResult = analytics.getVolumeProgression('ex_1', sessions); + + // Brute-force reference: sort sessions oldest-first, pick first log per session + final sorted = List.from(sessions) + ..sort((a, b) => a.date.compareTo(b.date)); + final reference = []; + for (final session in sorted) { + for (final log in session.exercises) { + if (log.exerciseId == 'ex_1') { + reference.add(log.totalVolume); + break; + } + } + } + + expect(indexedResult.map((e) => e.volume).toList(), equals(reference)); + }); + + test('getWeeklyVolumeByMuscle with exerciseMap equals without', () { + // Build a minimal one-exercise list so the map has something to look up + final exercise = Exercise( + id: 'ex_0', + name: 'Test Exercise', + muscleActivations: [ + MuscleActivation(muscleGroupId: 'chest', activationPercentage: 100), + ], + category: 'compound', + ); + final exercises = [exercise]; + final exerciseMap = {exercise.id: exercise}; + final now = sessions.first.date.add(const Duration(days: 1)); + + final withMap = analytics.getWeeklyVolumeByMuscle( + sessions, + exercises, + now: now, + exerciseMap: exerciseMap, + ); + final withoutMap = analytics.getWeeklyVolumeByMuscle( + sessions, + exercises, + now: now, + ); + + expect(withMap, equals(withoutMap)); + }); + }); + + // --------------------------------------------------------------------------- + // ExerciseManager — memoized exercise index + // --------------------------------------------------------------------------- + + group('ExerciseManager.getExercise — O(1) map lookup parity', () { + late MockStorageService mockStorage; + late ExerciseManager exerciseManager; + + setUp(() async { + mockStorage = MockStorageService(); + exerciseManager = ExerciseManager(mockStorage); + await exerciseManager.loadExercises(); + }); + + test('getExercise returns same result for known exercise', () async { + // Add a custom exercise and immediately look it up + final added = await exerciseManager.addCustomExercise( + name: 'Index Lookup Test', + category: 'compound', + primaryMuscleGroupId: 'chest', + ); + + final found = exerciseManager.getExercise(added.id); + expect(found, isNotNull); + expect(found!.id, equals(added.id)); + expect(found.name, equals('Index Lookup Test')); + }); + + test('getExercise returns null for unknown id', () { + expect(exerciseManager.getExercise('phantom_id'), isNull); + }); + + test('cache is invalidated after deleteCustomExercise', () async { + final added = await exerciseManager.addCustomExercise( + name: 'To Delete Cache Test', + category: 'isolation', + primaryMuscleGroupId: 'biceps', + ); + + // Should be found before deletion + expect(exerciseManager.getExercise(added.id), isNotNull); + + await exerciseManager.deleteCustomExercise(added.id); + + // Should be gone after deletion + expect(exerciseManager.getExercise(added.id), isNull); + }); + }); + + // --------------------------------------------------------------------------- + // TargetCalculator — RepsTargetCalculator in-place max scan + // --------------------------------------------------------------------------- + + group('RepsTargetCalculator — in-place max scan parity', () { + test('returns max reps across all sessions', () { + final sessions = [ + WorkoutSession( + id: 's1', + date: DateTime(2025, 1, 1), + exercises: [ + ExerciseLog( + exerciseId: 'ex_0', + sets: [WorkoutSet(weight: 50, reps: 8), WorkoutSet(weight: 50, reps: 10)], + ), + ], + duration: 30, + ), + WorkoutSession( + id: 's2', + date: DateTime(2025, 2, 1), + exercises: [ + ExerciseLog( + exerciseId: 'ex_0', + sets: [WorkoutSet(weight: 60, reps: 12)], + ), + ], + duration: 30, + ), + ]; + + final result = RepsTargetCalculator().calculate('ex_0', sessions); + expect(result, equals(12.0)); + }); + + test('returns 0 when no sessions for the exercise', () { + final result = RepsTargetCalculator().calculate('unknown', []); + expect(result, equals(0.0)); + }); + }); + + // --------------------------------------------------------------------------- + // WorkoutProvider.getWeeklyVolumeByMuscle — early-exit sort + // --------------------------------------------------------------------------- + + group('WorkoutProvider.getWeeklyVolumeByMuscle — early exit + map', () { + late MockStorageService mockStorage; + late WorkoutProvider provider; + + setUp(() async { + mockStorage = MockStorageService(); + // Seed one session from 3 days ago and one from 30 days ago. + final recent = WorkoutSession( + id: 'recent', + date: DateTime.now().subtract(const Duration(days: 3)), + exercises: [ + ExerciseLog( + exerciseId: 'bench_press', // built-in exercise + sets: [WorkoutSet(weight: 100, reps: 5)], + ), + ], + duration: 30, + ); + final old = WorkoutSession( + id: 'old', + date: DateTime.now().subtract(const Duration(days: 30)), + exercises: [ + ExerciseLog( + exerciseId: 'bench_press', + sets: [WorkoutSet(weight: 80, reps: 5)], + ), + ], + duration: 30, + ); + mockStorage.addMockSession(recent); + mockStorage.addMockSession(old); + provider = WorkoutProvider( + mockStorage, + programManager: ProgramManager(mockStorage), + ); + await provider.init(); + }); + + test('only counts sessions within the last 7 days', () { + // Create AnalyticsManager to use getWeeklyVolumeByMuscle with deterministic 'now' + final analytics = AnalyticsManager(mockStorage, MockMLService()); + + // Build a controlled exercise with exact 100% activation + final exercises = [ + Exercise( + id: 'bench_press', + name: 'Bench Press', + muscleActivations: [ + MuscleActivation(muscleGroupId: 'chest', activationPercentage: 100), + ], + category: 'compound', + ) + ]; + final sessions = provider.sessions; + + // With current date: recent session (100kg x 5 = 500 volume) is included, + // old session (80kg x 5 = 400 volume) is excluded. + final resultWithRecent = analytics.getWeeklyVolumeByMuscle( + sessions, + exercises, + now: DateTime.now(), + ); + expect(resultWithRecent['chest'], equals(500.0)); + + // With shifted date (30+ days in future): both sessions are excluded. + final resultFuture = analytics.getWeeklyVolumeByMuscle( + sessions, + exercises, + now: DateTime.now().add(const Duration(days: 30)), + ); + expect(resultFuture['chest'], isNull); + }); + }); +} +