Add usage stats and backup backend - #20
Conversation
- Created backend/ directory with FastAPI server. - Added MongoDB integration and Pydantic models. - Configured Railway deployment with railway.toml. - Added http dependency to Flutter app. - Implemented ApiService for reporting usage and backing up data. - Added exportAllData method to WorkoutProvider. - Integrated usage reporting in app initialization. - Added Settings screen with Backup functionality. Co-authored-by: Devasy23 <110348311+Devasy23@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
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:
WalkthroughAdds a FastAPI analytics backend with MongoDB access and Pydantic models, a Streamlit dashboard, and client/server integration: new Dart ApiService, Flutter SettingsScreen and startup reporting, provider export API, storage/service adjustments, dependency additions, and a revised .gitignore and top-level app import. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Fix all issues with AI agents
In `@backend/main.py`:
- Around line 7-16: The endpoints (report_usage and the /backup handler) accept
user data with no auth; add an API-key dependency to block unauthenticated
callers by implementing a get_api_key dependency that reads a header (e.g.,
X-API-Key) and validates it against a configured secret (env var or config) and
raises HTTPException(401) on mismatch, then add that dependency to the route
signatures (e.g., report_usage(..., api_key: str = Depends(get_api_key)) and the
backup handler) so db.reports.insert_one and the backup logic only run when the
API key is valid.
- Around line 12-13: UsageStats.report_date and BackupData.backup_received_at
currently use datetime.now() as a default (evaluated at import time), causing
all instances to share the same timestamp; change their declarations to use
pydantic's Field with a default_factory (e.g., report_date: datetime =
Field(default_factory=datetime.now) and backup_received_at: datetime =
Field(default_factory=datetime.now)) and add the Field import from pydantic so
each model instance gets the current time when created.
- Around line 11-16: Update the exception handling in the report handler (the
block that calls stats.model_dump() and db.reports.insert_one) to catch a
narrower error (use pymongo.errors.PyMongoError instead of bare Exception), move
the success return into an else: block so the try only contains the risky calls,
and re-raise the HTTPException using "raise HTTPException(... ) from e" to
preserve the exception chain; apply the identical pattern to the /backup handler
that also calls db.reports.insert_one or similar DB operations.
In `@backend/models.py`:
- Line 10: The default datetime values are being evaluated at import time;
update the UsageStats.report_date and BackupData.backup_received_at definitions
to use Field(default_factory=...) so a fresh timestamp is produced per instance
(e.g., change report_date: datetime = datetime.now() to report_date: datetime =
Field(default_factory=datetime.now) and do the same for backup_received_at), and
add the necessary import for Field where models.py defines those classes.
- Around line 1-3: The file imports deprecated typing generics and an unused
name: remove List, Dict, and Optional from the typing import and replace them
with built-in generics; update import to only import Any if needed (keep from
typing import Any) or drop entirely if unused, and change all model annotations
that use List[Dict[str, Any]] (or similar) to use list[dict[str, Any]]; ensure
BaseModel subclasses (models) reflect the new annotations and remove the unused
Optional import.
In `@backend/requirements.txt`:
- Around line 1-5: The requirements.txt currently lists unpinned packages
(fastapi, uvicorn, motor, dnspython, pydantic) and uses the deprecated Motor
driver; update requirements.txt to pin or constrain versions (e.g.,
fastapi==0.128.5 or fastapi~=0.128, uvicorn==0.22.0, pydantic==1.10.9,
dnspython==2.3.0) and replace motor with a pinned PyMongo async-capable version
(e.g., pymongo==4.5.0 or pymongo>=4.5,<5); then search the codebase for Motor
symbols (imports like motor.motor_asyncio.AsyncIOMotorClient or names
AsyncIOMotorClient, MotorClient, motor) and replace those usages with PyMongo’s
async API equivalents (update connection creation functions and any code that
calls AsyncIOMotorClient.connect/close/query to use pymongo’s asyncio support or
the recommended async wrapper), adjusting imports and types accordingly so the
app uses the pinned pymongo driver instead of motor.
In `@workout-logger/lib/main.dart`:
- Around line 97-103: The call to ApiService().reportUsage(stats) inside
provider.getQuickStats().then(...) isn't returned, so its errors escape the
surrounding .catchError and can cause unhandled async exceptions; update the
then callback to return the Future from reportUsage (or attach its own
.catchError) so errors are propagated to the outer catchError, and refactor the
concrete ApiService() instantiation to use the same DI/abstraction pattern as
IStorageService and IMLService (inject an IApiService or pass an ApiService from
the provider/container) to keep the code testable and consistent.
In `@workout-logger/lib/screens/settings_screen.dart`:
- Around line 21-26: The code is doing an unnecessary JSON round-trip:
exportAllData() returns a JSON string but you decode it to a Map only to have
ApiService.backupData re-encode it; fix by passing the JSON string directly to
backupData (call ApiService.backupData(jsonString)) and update
ApiService.backupData signature/implementation to accept a pre-serialized String
(remove its internal jsonEncode), or alternatively change exportAllData() to
return a Map<String, dynamic> and keep backupData as-is — choose one approach
and update references to exportAllData and ApiService.backupData accordingly.
- Around line 45-52: Replace the SnackBar content that currently exposes the raw
exception (Text('Error: $e')) with a generic, user-friendly message (e.g.,
'Something went wrong. Please try again.') while keeping the same styling
(SnackBar, backgroundColor: AppTheme.error) and mounted check; separately log
the actual exception and stack trace using the app's logging mechanism (or
debugPrint) inside the catch block so the error details are available for
debugging but not shown to the user—update the code around the
ScaffoldMessenger.of(context).showSnackBar call and the catch block to implement
this change.
- Line 26: The call instantiates ApiService inline (final success = await
ApiService().backupData(data);) which breaks DI and testability; modify the
SettingsScreen (or its State, e.g., _SettingsScreenState) to accept an
ApiService dependency (via constructor or obtain from Provider/InheritedWidget)
and replace the inline new ApiService() with the injected instance when calling
backupData; ensure the injected field (e.g., apiService) is used everywhere the
widget calls apiService.backupData(...) so tests can supply a mock.
In `@workout-logger/lib/services/api_service.dart`:
- Around line 14-36: reportUsage currently returns void and swallows all
exceptions; change its signature to Future<bool> reportUsage(Map<String,
dynamic> stats) and implement success/failure semantics: build the same payload,
await _client.post (same call site), return true when response.statusCode ==
200, log and return false when non-200, and catch exceptions to debugPrint the
error and return false (do not rethrow). Ensure callers that expect
backupData-style bool results can use the returned value to detect failures.
- Around line 25-29: The POST calls using _client.post to '$_baseUrl/report' and
'$_baseUrl/backup' lack timeouts and can hang; wrap each await _client.post(...)
call with .timeout(Duration(seconds: <n>)) (e.g., 10s) and surround them with
try/catch that explicitly catches TimeoutException (from dart:async) to return
or throw a clear timeout-specific error, while keeping a general catch for other
exceptions—update both occurrences of _client.post (the '/report' and '/backup'
calls) to use this pattern.
- Around line 5-12: The ApiService currently hardcodes _baseUrl; make it
configurable by adding a final String _baseUrl field and accepting an optional
String baseUrl parameter in the ApiService constructor (keep the current
emulator default 'http://10.0.2.2:8000' when baseUrl is null) so callers/tests
can inject alternate URLs; update the constructor signature
(ApiService({http.Client? client, String? baseUrl})) to set _baseUrl = baseUrl
?? 'http://10.0.2.2:8000' and remove the inline TODO comment so
deployment/configuration no longer requires code edits.
| @app.post("/report") | ||
| async def report_usage(stats: UsageStats): | ||
| if db is None: | ||
| raise HTTPException(status_code=503, detail="Database not configured") | ||
| try: | ||
| report_data = stats.model_dump() | ||
| await db.reports.insert_one(report_data) | ||
| return {"status": "success", "message": "Usage stats reported"} | ||
| except Exception as e: | ||
| raise HTTPException(status_code=500, detail=str(e)) |
There was a problem hiding this comment.
No authentication or authorization on endpoints receiving user data.
Both /report and /backup accept arbitrary data from any caller without authentication. The /backup endpoint is particularly sensitive as it receives complete workout history. At minimum, add an API key check (e.g., via a Depends header dependency) before accepting data.
🧰 Tools
🪛 Ruff (0.15.0)
[warning] 14-14: Consider moving this statement to an else block
(TRY300)
[warning] 15-15: Do not catch blind exception: Exception
(BLE001)
[warning] 16-16: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🤖 Prompt for AI Agents
In `@backend/main.py` around lines 7 - 16, The endpoints (report_usage and the
/backup handler) accept user data with no auth; add an API-key dependency to
block unauthenticated callers by implementing a get_api_key dependency that
reads a header (e.g., X-API-Key) and validates it against a configured secret
(env var or config) and raises HTTPException(401) on mismatch, then add that
dependency to the route signatures (e.g., report_usage(..., api_key: str =
Depends(get_api_key)) and the backup handler) so db.reports.insert_one and the
backup logic only run when the API key is valid.
| from pydantic import BaseModel | ||
| from typing import List, Dict, Any, Optional | ||
| from datetime import datetime |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use modern built-in generics and drop unused import.
Per Ruff UP035, List and Dict from typing are deprecated since Python 3.9 — use list and dict directly. Optional is also imported but unused.
♻️ Proposed fix
-from typing import List, Dict, Any, Optional
+from typing import AnyThen replace List[Dict[str, Any]] with list[dict[str, Any]] in the model fields.
🧰 Tools
🪛 Ruff (0.15.0)
[warning] 2-2: typing.List is deprecated, use list instead
(UP035)
[warning] 2-2: typing.Dict is deprecated, use dict instead
(UP035)
🤖 Prompt for AI Agents
In `@backend/models.py` around lines 1 - 3, The file imports deprecated typing
generics and an unused name: remove List, Dict, and Optional from the typing
import and replace them with built-in generics; update import to only import Any
if needed (keep from typing import Any) or drop entirely if unused, and change
all model annotations that use List[Dict[str, Any]] (or similar) to use
list[dict[str, Any]]; ensure BaseModel subclasses (models) reflect the new
annotations and remove the unused Optional import.
| try { | ||
| final provider = context.read<WorkoutProvider>(); | ||
| final jsonString = await provider.exportAllData(); | ||
| final data = jsonDecode(jsonString) as Map<String, dynamic>; | ||
|
|
||
| final success = await ApiService().backupData(data); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Unnecessary double serialization: JSON string → Map → JSON string.
exportAllData() returns a JSON string, which is decoded to a Map on Line 24, only to be re-encoded by ApiService.backupData (via jsonEncode). Either have exportAllData return a Map directly, or have backupData accept a pre-serialized JSON string to avoid the redundant round-trip.
🤖 Prompt for AI Agents
In `@workout-logger/lib/screens/settings_screen.dart` around lines 21 - 26, The
code is doing an unnecessary JSON round-trip: exportAllData() returns a JSON
string but you decode it to a Map only to have ApiService.backupData re-encode
it; fix by passing the JSON string directly to backupData (call
ApiService.backupData(jsonString)) and update ApiService.backupData
signature/implementation to accept a pre-serialized String (remove its internal
jsonEncode), or alternatively change exportAllData() to return a Map<String,
dynamic> and keep backupData as-is — choose one approach and update references
to exportAllData and ApiService.backupData accordingly.
| final jsonString = await provider.exportAllData(); | ||
| final data = jsonDecode(jsonString) as Map<String, dynamic>; | ||
|
|
||
| final success = await ApiService().backupData(data); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
ApiService instantiated inline on every backup — inject it instead.
Creating a new ApiService() per call bypasses dependency injection and makes testing harder. Inject it via the constructor or provide it through the widget tree (e.g., via Provider), consistent with the project's DI roadmap.
Based on learnings, new code should align with the refactoring roadmap that prioritizes dependency injection.
Proposed approach
class _SettingsScreenState extends State<SettingsScreen> {
bool _isBackingUp = false;
+ late final ApiService _apiService;
+
+ `@override`
+ void initState() {
+ super.initState();
+ _apiService = ApiService();
+ }
Future<void> _performBackup() async {
// ...
- final success = await ApiService().backupData(data);
+ final success = await _apiService.backupData(data);📝 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.
| final success = await ApiService().backupData(data); | |
| class _SettingsScreenState extends State<SettingsScreen> { | |
| bool _isBackingUp = false; | |
| late final ApiService _apiService; | |
| `@override` | |
| void initState() { | |
| super.initState(); | |
| _apiService = ApiService(); | |
| } | |
| Future<void> _performBackup() async { | |
| // ... | |
| final success = await _apiService.backupData(data); | |
| // ... | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@workout-logger/lib/screens/settings_screen.dart` at line 26, The call
instantiates ApiService inline (final success = await
ApiService().backupData(data);) which breaks DI and testability; modify the
SettingsScreen (or its State, e.g., _SettingsScreenState) to accept an
ApiService dependency (via constructor or obtain from Provider/InheritedWidget)
and replace the inline new ApiService() with the injected instance when calling
backupData; ensure the injected field (e.g., apiService) is used everywhere the
widget calls apiService.backupData(...) so tests can supply a mock.
| } catch (e) { | ||
| if (!mounted) return; | ||
| ScaffoldMessenger.of(context).showSnackBar( | ||
| SnackBar( | ||
| content: Text('Error: $e'), | ||
| backgroundColor: AppTheme.error, | ||
| ), | ||
| ); |
There was a problem hiding this comment.
Avoid exposing raw exception details to users.
Text('Error: $e') in the SnackBar could display internal stack traces or system details. Show a user-friendly message and log the exception separately.
Proposed fix
- content: Text('Error: $e'),
+ content: const Text('Backup failed due to an unexpected error.'),📝 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.
| } catch (e) { | |
| if (!mounted) return; | |
| ScaffoldMessenger.of(context).showSnackBar( | |
| SnackBar( | |
| content: Text('Error: $e'), | |
| backgroundColor: AppTheme.error, | |
| ), | |
| ); | |
| } catch (e) { | |
| if (!mounted) return; | |
| ScaffoldMessenger.of(context).showSnackBar( | |
| SnackBar( | |
| content: const Text('Backup failed due to an unexpected error.'), | |
| backgroundColor: AppTheme.error, | |
| ), | |
| ); |
🤖 Prompt for AI Agents
In `@workout-logger/lib/screens/settings_screen.dart` around lines 45 - 52,
Replace the SnackBar content that currently exposes the raw exception
(Text('Error: $e')) with a generic, user-friendly message (e.g., 'Something went
wrong. Please try again.') while keeping the same styling (SnackBar,
backgroundColor: AppTheme.error) and mounted check; separately log the actual
exception and stack trace using the app's logging mechanism (or debugPrint)
inside the catch block so the error details are available for debugging but not
shown to the user—update the code around the
ScaffoldMessenger.of(context).showSnackBar call and the catch block to implement
this change.
| Future<void> reportUsage(Map<String, dynamic> stats) async { | ||
| try { | ||
| // Map Dart camelCase stats to Python snake_case model | ||
| final payload = { | ||
| 'total_workouts': stats['totalWorkouts'], | ||
| 'weekly_workouts': stats['weeklyWorkouts'], | ||
| 'weekly_volume': stats['weeklyVolume'], | ||
| 'exercises_this_week': stats['exercisesThisWeek'], | ||
| 'report_date': DateTime.now().toIso8601String(), | ||
| }; | ||
|
|
||
| final response = await _client.post( | ||
| Uri.parse('$_baseUrl/report'), | ||
| headers: {'Content-Type': 'application/json'}, | ||
| body: jsonEncode(payload), | ||
| ); | ||
|
|
||
| if (response.statusCode != 200) { | ||
| debugPrint('Failed to report usage: ${response.body}'); | ||
| } | ||
| } catch (e) { | ||
| debugPrint('Error reporting usage: $e'); | ||
| } |
There was a problem hiding this comment.
reportUsage silently swallows all errors — caller has no way to know if reporting failed.
The method returns void and catches all exceptions. While this is acceptable for fire-and-forget telemetry, it means the caller's debugPrint('Failed to report usage') (per the summary) will never fire since the exception is caught here. Consider returning a bool like backupData does, or at minimum re-log with enough context to diagnose issues.
🤖 Prompt for AI Agents
In `@workout-logger/lib/services/api_service.dart` around lines 14 - 36,
reportUsage currently returns void and swallows all exceptions; change its
signature to Future<bool> reportUsage(Map<String, dynamic> stats) and implement
success/failure semantics: build the same payload, await _client.post (same call
site), return true when response.statusCode == 200, log and return false when
non-200, and catch exceptions to debugPrint the error and return false (do not
rethrow). Ensure callers that expect backupData-style bool results can use the
returned value to detect failures.
| final response = await _client.post( | ||
| Uri.parse('$_baseUrl/report'), | ||
| headers: {'Content-Type': 'application/json'}, | ||
| body: jsonEncode(payload), | ||
| ); |
There was a problem hiding this comment.
HTTP requests have no timeout — can hang indefinitely on network issues.
Both _client.post calls lack a timeout. On mobile, this can freeze the UI or hold resources indefinitely when the backend is unreachable.
Proposed fix
- final response = await _client.post(
- Uri.parse('$_baseUrl/report'),
- headers: {'Content-Type': 'application/json'},
- body: jsonEncode(payload),
- );
+ final response = await _client.post(
+ Uri.parse('$_baseUrl/report'),
+ headers: {'Content-Type': 'application/json'},
+ body: jsonEncode(payload),
+ ).timeout(const Duration(seconds: 15));Apply the same pattern to the /backup call. Catch TimeoutException explicitly if you want distinct error messaging.
Also applies to: 41-45
🤖 Prompt for AI Agents
In `@workout-logger/lib/services/api_service.dart` around lines 25 - 29, The POST
calls using _client.post to '$_baseUrl/report' and '$_baseUrl/backup' lack
timeouts and can hang; wrap each await _client.post(...) call with
.timeout(Duration(seconds: <n>)) (e.g., 10s) and surround them with try/catch
that explicitly catches TimeoutException (from dart:async) to return or throw a
clear timeout-specific error, while keeping a general catch for other
exceptions—update both occurrences of _client.post (the '/report' and '/backup'
calls) to use this pattern.
- Create backend/ directory with FastAPI and MongoDB logic. - Move requirements.txt to root for Railpack detection. - Configure railway.toml for Railpack deployment. - Implement ApiService in Flutter for reporting and backups. - Update WorkoutProvider to expose exportAllData. - Add SettingsScreen to UI for manual backups. - Fix Pydantic models to use default_factory. - Make API URL configurable via environment variable. Co-authored-by: Devasy23 <110348311+Devasy23@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @.gitignore:
- Line 18: Remove the ".git" entry from the .gitignore because ignoring ".git"
is unusual and can hide nested repositories; locate the ".git" pattern in the
.gitignore and delete that line (or replace it with a documented, targeted
pattern if you truly intend to ignore specific nested repo directories), and
commit the updated .gitignore so nested .git directories (e.g., submodules or
accidental clones) are not silently suppressed.
- Line 21: Replace the misspelled ignore entry ".hypothesise" in .gitignore with
the correct Hypothesis cache directory name ".hypothesis/" so the intended
directory is actually ignored; update the string ".hypothesise" to
".hypothesis/".
In `@railway.toml`:
- Line 2: Update the startCommand value so the uvicorn invocation uses
production-ready options: include a workers flag to run multiple worker
processes, set sensible timeout and keep-alive values, and add logging/config
flags as needed; modify the string assigned to startCommand (the existing
"uvicorn backend.main:app --host 0.0.0.0 --port $PORT") to include these options
(e.g., --workers, --timeout-keep-alive, --log-level) and tune them for your
Railway plan.
In `@workout-logger/lib/services/api_service.dart`:
- Around line 42-60: The backupData method currently jsonEncodes the entire Map
and posts it to '$_baseUrl/backup' which can create oversized payloads; update
backupData to first compute the serialized size (jsonEncode(data)) and if it
exceeds a configurable threshold (e.g., MAX_BACKUP_SIZE_BYTES) either reject
early with a meaningful error or split the data into smaller chunks and upload
them sequentially (e.g., POST to '$_baseUrl/backup' for each chunk or use a
multipart/append API), ensuring each chunk respects the size limit and
handling/retrying failed chunk uploads; keep references to the same _client.post
call and ensure error handling/logging for partial failures remains in place.
- Around line 20-26: The payload construction reads values directly from the
stats Map, which will silently insert nulls if keys like 'totalWorkouts',
'weeklyWorkouts', 'weeklyVolume', or 'exercisesThisWeek' are missing; update the
code in api_service.dart where payload is created to defensively handle missing
keys by either (a) validating that the required keys exist and are non-null (and
early-return or log/throw a clear error) or (b) using safe fallbacks like
stats['totalWorkouts'] ?? 0, stats['weeklyWorkouts'] ?? 0, stats['weeklyVolume']
?? 0, stats['exercisesThisWeek'] ?? 0 before building the payload, ensuring
report_date remains DateTime.now().toIso8601String(); pick validation if missing
keys should be treated as an error, otherwise apply the fallback defaults.
- Around line 5-15: ApiService currently creates a persistent http.Client in the
constructor (the _client field) that is never closed, causing a resource leak;
fix this by adding a public dispose() method on ApiService that calls
_client.close(), update callers (e.g., where ApiService() is constructed in
main.dart) to call dispose() when the service is no longer needed, or
alternatively replace the persistent _client with one-shot top-level helpers
(e.g., use http.post/get directly instead of _client.post/_client.get) if you
prefer not to manage lifecycle—choose one approach and apply it consistently
across all methods that use _client.
| .git | ||
| .mypy_cache | ||
| .pytest_cache | ||
| .hypothesise |
There was a problem hiding this comment.
Typo: .hypothesise → .hypothesis
The Hypothesis testing library uses .hypothesis/ as its cache directory.
-.hypothesise
+.hypothesis/📝 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.
| .hypothesise | |
| .hypothesis/ |
🤖 Prompt for AI Agents
In @.gitignore at line 21, Replace the misspelled ignore entry ".hypothesise" in
.gitignore with the correct Hypothesis cache directory name ".hypothesis/" so
the intended directory is actually ignored; update the string ".hypothesise" to
".hypothesis/".
| @@ -0,0 +1,2 @@ | |||
| [deploy] | |||
| startCommand = "uvicorn backend.main:app --host 0.0.0.0 --port $PORT" | |||
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding production-ready uvicorn options.
For production deployments, you may want to enhance the uvicorn configuration with additional options such as worker processes and timeouts for better performance and reliability.
⚙️ Optional production configuration enhancement
-startCommand = "uvicorn backend.main:app --host 0.0.0.0 --port $PORT"
+startCommand = "uvicorn backend.main:app --host 0.0.0.0 --port $PORT --workers 2 --timeout-keep-alive 30"Note: Adjust the number of workers based on your Railway plan's available resources.
📝 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.
| startCommand = "uvicorn backend.main:app --host 0.0.0.0 --port $PORT" | |
| startCommand = "uvicorn backend.main:app --host 0.0.0.0 --port $PORT --workers 2 --timeout-keep-alive 30" |
🤖 Prompt for AI Agents
In `@railway.toml` at line 2, Update the startCommand value so the uvicorn
invocation uses production-ready options: include a workers flag to run multiple
worker processes, set sensible timeout and keep-alive values, and add
logging/config flags as needed; modify the string assigned to startCommand (the
existing "uvicorn backend.main:app --host 0.0.0.0 --port $PORT") to include
these options (e.g., --workers, --timeout-keep-alive, --log-level) and tune them
for your Railway plan.
| class ApiService { | ||
| // TODO: Update this URL with your Railway deployment URL | ||
| // Example: https://repforge-backend-production.up.railway.app | ||
| static const String _baseUrl = String.fromEnvironment( | ||
| 'API_URL', | ||
| defaultValue: 'http://10.0.2.2:8000', // Default to Android Emulator for dev | ||
| ); | ||
|
|
||
| final http.Client _client; | ||
|
|
||
| ApiService({http.Client? client}) : _client = client ?? http.Client(); |
There was a problem hiding this comment.
http.Client created internally is never closed — resource leak.
When callers instantiate ApiService() without injecting a client (which is the common path per main.dart), a new http.Client is created but never closed. In Dart, http.Client holds an underlying connection pool that should be closed when no longer needed.
Consider either:
- Adding a
dispose()method and managing the lifecycle, or - Using a one-shot helper (e.g.,
http.post) instead of a persistentClientfor the fire-and-forget cases.
Option 2: use top-level http functions for simple cases
- final http.Client _client;
-
- ApiService({http.Client? client}) : _client = client ?? http.Client();
+ final http.Client? _client;
+
+ ApiService({http.Client? client}) : _client = client;
+
+ Future<http.Response> _post(Uri url, {Map<String, String>? headers, Object? body}) {
+ final c = _client;
+ if (c != null) {
+ return c.post(url, headers: headers, body: body);
+ }
+ // Uses a one-shot request that cleans up automatically
+ return http.post(url, headers: headers, body: body);
+ }Then replace _client.post(...) calls with _post(...).
🤖 Prompt for AI Agents
In `@workout-logger/lib/services/api_service.dart` around lines 5 - 15, ApiService
currently creates a persistent http.Client in the constructor (the _client
field) that is never closed, causing a resource leak; fix this by adding a
public dispose() method on ApiService that calls _client.close(), update callers
(e.g., where ApiService() is constructed in main.dart) to call dispose() when
the service is no longer needed, or alternatively replace the persistent _client
with one-shot top-level helpers (e.g., use http.post/get directly instead of
_client.post/_client.get) if you prefer not to manage lifecycle—choose one
approach and apply it consistently across all methods that use _client.
| final payload = { | ||
| 'total_workouts': stats['totalWorkouts'], | ||
| 'weekly_workouts': stats['weeklyWorkouts'], | ||
| 'weekly_volume': stats['weeklyVolume'], | ||
| 'exercises_this_week': stats['exercisesThisWeek'], | ||
| 'report_date': DateTime.now().toIso8601String(), | ||
| }; |
There was a problem hiding this comment.
Fragile map access — missing keys silently produce null values in the payload.
If stats is missing any expected key (e.g., totalWorkouts), the payload will contain null for that field without any indication. This could cause silent data quality issues on the backend or 422 validation errors that are only caught by the generic debugPrint.
Consider defensive access with fallback defaults or an early validation check.
Example: validate required keys
+ static const _requiredKeys = [
+ 'totalWorkouts', 'weeklyWorkouts', 'weeklyVolume', 'exercisesThisWeek',
+ ];
+
Future<void> reportUsage(Map<String, dynamic> stats) async {
try {
+ final missing = _requiredKeys.where((k) => !stats.containsKey(k)).toList();
+ if (missing.isNotEmpty) {
+ debugPrint('reportUsage: missing keys $missing');
+ return;
+ }
final payload = {🤖 Prompt for AI Agents
In `@workout-logger/lib/services/api_service.dart` around lines 20 - 26, The
payload construction reads values directly from the stats Map, which will
silently insert nulls if keys like 'totalWorkouts', 'weeklyWorkouts',
'weeklyVolume', or 'exercisesThisWeek' are missing; update the code in
api_service.dart where payload is created to defensively handle missing keys by
either (a) validating that the required keys exist and are non-null (and
early-return or log/throw a clear error) or (b) using safe fallbacks like
stats['totalWorkouts'] ?? 0, stats['weeklyWorkouts'] ?? 0, stats['weeklyVolume']
?? 0, stats['exercisesThisWeek'] ?? 0 before building the payload, ensuring
report_date remains DateTime.now().toIso8601String(); pick validation if missing
keys should be treated as an error, otherwise apply the fallback defaults.
| Future<bool> backupData(Map<String, dynamic> data) async { | ||
| try { | ||
| final response = await _client.post( | ||
| Uri.parse('$_baseUrl/backup'), | ||
| headers: {'Content-Type': 'application/json'}, | ||
| body: jsonEncode(data), | ||
| ); | ||
|
|
||
| if (response.statusCode == 200) { | ||
| return true; | ||
| } else { | ||
| debugPrint('Failed to backup data: ${response.body}'); | ||
| return false; | ||
| } | ||
| } catch (e) { | ||
| debugPrint('Error backing up data: $e'); | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider guarding against excessively large backup payloads.
backupData serialises the entire data map and sends it in a single POST. For users with extensive workout history, this could produce a very large payload that may time out or be rejected by the server. A size check or chunked upload strategy would improve reliability.
🤖 Prompt for AI Agents
In `@workout-logger/lib/services/api_service.dart` around lines 42 - 60, The
backupData method currently jsonEncodes the entire Map and posts it to
'$_baseUrl/backup' which can create oversized payloads; update backupData to
first compute the serialized size (jsonEncode(data)) and if it exceeds a
configurable threshold (e.g., MAX_BACKUP_SIZE_BYTES) either reject early with a
meaningful error or split the data into smaller chunks and upload them
sequentially (e.g., POST to '$_baseUrl/backup' for each chunk or use a
multipart/append API), ensuring each chunk respects the size limit and
handling/retrying failed chunk uploads; keep references to the same _client.post
call and ensure error handling/logging for partial failures remains in place.
- Create backend/ directory with FastAPI and MongoDB logic. - Move requirements.txt to root for Railpack detection. - Add main.py to root for Railpack detection. - Remove railway.toml to use default Railpack configuration. - Implement ApiService in Flutter for reporting and backups. - Update WorkoutProvider to expose exportAllData. - Add SettingsScreen to UI for manual backups. - Fix Pydantic models to use default_factory. - Make API URL configurable via environment variable. - Update .gitignore to include Python and Flutter artifacts. Co-authored-by: Devasy23 <110348311+Devasy23@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.gitignore:
- Around line 30-36: The .gitignore contains duplicate entries for
android/gradlew.bat and android/local.properties; remove the extra occurrences
so each of those patterns appears only once (leave a single entry for
android/gradlew.bat and a single entry for android/local.properties), and tidy
any accidental repeated lines to avoid redundancy.
- Line 4: The .gitignore currently contains a broad "*.lock" pattern that will
exclude important dependency lock files (e.g., package-lock.json, pubspec.lock,
Pipfile.lock); update the .gitignore to remove or replace the global "*.lock"
entry with more targeted ignore patterns for editor or OS lock/swap files (e.g.,
specific editor temp/lock filenames) so dependency lockfiles (package-lock.json,
pubspec.lock, poetry.lock, Pipfile.lock) remain tracked; locate the "*.lock"
entry and either delete it or narrow it to the intended filenames.
In `@main.py`:
- Line 1: This file re-exports app from backend.main but lacks a local-run
entrypoint; add an if __name__ == "__main__" guard that imports the exported app
symbol and launches a dev server (for example, call uvicorn.run(app,
host="127.0.0.1", port=8000, reload=True) or app.run(...) depending on
framework) so developers can run main.py directly for local development;
reference the exported app symbol from backend.main and the __main__ guard to
locate where to add the change.
|
|
||
| # vuepress v2.x temp and cache directory | ||
| .temp | ||
| *.lock |
There was a problem hiding this comment.
*.lock will ignore dependency lock files that should be committed.
This pattern will exclude pubspec.lock (Flutter), package-lock.json, Pipfile.lock, poetry.lock, etc. Dependency lock files are essential for reproducible builds and should generally be tracked in version control. This likely intended to ignore only editor swap/lock files.
🐛 Proposed fix
-*.lockIf you need to ignore specific lock files (e.g., editor locks), use a more targeted pattern instead.
📝 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.
| *.lock |
🤖 Prompt for AI Agents
In @.gitignore at line 4, The .gitignore currently contains a broad "*.lock"
pattern that will exclude important dependency lock files (e.g.,
package-lock.json, pubspec.lock, Pipfile.lock); update the .gitignore to remove
or replace the global "*.lock" entry with more targeted ignore patterns for
editor or OS lock/swap files (e.g., specific editor temp/lock filenames) so
dependency lockfiles (package-lock.json, pubspec.lock, poetry.lock,
Pipfile.lock) remain tracked; locate the "*.lock" entry and either delete it or
narrow it to the intended filenames.
| android/gradlew | ||
| android/gradlew.bat | ||
| android/local.properties | ||
| android/.gradle | ||
| android/captures/ | ||
| android/gradlew.bat | ||
| android/local.properties |
There was a problem hiding this comment.
Duplicate entries: android/gradlew.bat and android/local.properties appear twice.
Lines 31 & 35 both list android/gradlew.bat, and lines 32 & 36 both list android/local.properties.
Proposed fix
android/gradle/
android/gradlew
android/gradlew.bat
android/local.properties
android/.gradle
android/captures/
-android/gradlew.bat
-android/local.properties📝 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.
| android/gradlew | |
| android/gradlew.bat | |
| android/local.properties | |
| android/.gradle | |
| android/captures/ | |
| android/gradlew.bat | |
| android/local.properties | |
| android/gradlew | |
| android/gradlew.bat | |
| android/local.properties | |
| android/.gradle | |
| android/captures/ |
🤖 Prompt for AI Agents
In @.gitignore around lines 30 - 36, The .gitignore contains duplicate entries
for android/gradlew.bat and android/local.properties; remove the extra
occurrences so each of those patterns appears only once (leave a single entry
for android/gradlew.bat and a single entry for android/local.properties), and
tidy any accidental repeated lines to avoid redundancy.
| @@ -0,0 +1 @@ | |||
| from backend.main import app | |||
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minimal entry point — consider adding a local dev runner.
The bare re-export works for Railpack/Railway auto-detection, but there's no way to run this file directly during local development. Adding a __main__ guard would improve developer ergonomics:
💡 Suggested improvement
from backend.main import app
+
+if __name__ == "__main__":
+ import uvicorn
+ uvicorn.run("backend.main:app", host="0.0.0.0", port=8000, reload=True)📝 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.
| from backend.main import app | |
| from backend.main import app | |
| if __name__ == "__main__": | |
| import uvicorn | |
| uvicorn.run("backend.main:app", host="0.0.0.0", port=8000, reload=True) |
🤖 Prompt for AI Agents
In `@main.py` at line 1, This file re-exports app from backend.main but lacks a
local-run entrypoint; add an if __name__ == "__main__" guard that imports the
exported app symbol and launches a dev server (for example, call
uvicorn.run(app, host="127.0.0.1", port=8000, reload=True) or app.run(...)
depending on framework) so developers can run main.py directly for local
development; reference the exported app symbol from backend.main and the
__main__ guard to locate where to add the change.
- Create backend/ directory with FastAPI and MongoDB logic. - Move requirements.txt to root for Railpack detection. - Add main.py to root for Railpack detection. - Remove railway.toml to use default Railpack configuration. - Implement ApiService in Flutter for reporting and backups. - Update WorkoutProvider to expose exportAllData. - Add SettingsScreen to UI for manual backups. - Fix Pydantic models to use default_factory. - Make API URL configurable via environment variable. - Update .gitignore to include Python and Flutter artifacts. - Add CORS middleware to backend for cross-origin requests. Co-authored-by: Devasy23 <110348311+Devasy23@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@backend/main.py`:
- Around line 27-36: The /backup endpoint accepts a BackupData payload with
unbounded list fields which can exceed MongoDB's 16MB limit; update the Pydantic
BackupData model (in backend/models.py) to add max_items (or max_length)
constraints on sessions, routines, targets, muscleGroups, and customExercises
and/or validators that enforce a total-item or approximate-bytes limit, then
update the backup_data handler (backup_data and the db.backups.insert_one call)
to explicitly reject requests that violate those constraints with a 413/400
response; additionally ensure the deployment/webserver has a request body size
limit configured to guard against oversized uploads.
- Around line 8-14: The current app.add_middleware(CORSMiddleware,
allow_origins=["*"], allow_credentials=True, allow_methods=["*"],
allow_headers=["*"]) is invalid and too permissive; update the CORSMiddleware
config used in backend/main.py by either removing allow_credentials=True or
replacing allow_origins=["*"] with a concrete list of allowed origins (e.g.,
your Flutter web origin) and set allow_methods and allow_headers to only the
HTTP methods and headers your client uses (instead of ["*"]) so credentialed
requests work and CORS surface area is minimized; locate the CORSMiddleware call
and change allow_origins, allow_credentials, allow_methods, and allow_headers
accordingly.
| app.add_middleware( | ||
| CORSMiddleware, | ||
| allow_origins=["*"], | ||
| allow_credentials=True, | ||
| allow_methods=["*"], | ||
| allow_headers=["*"], | ||
| ) |
There was a problem hiding this comment.
allow_credentials=True with a wildcard origin is invalid per the CORS specification.
When allow_credentials is True, browsers reject responses whose Access-Control-Allow-Origin is *. This means credentialed requests will silently fail. Either drop allow_credentials=True or replace the wildcard with explicit allowed origins.
Additionally, allow_origins=["*"] with allow_methods=["*"] and allow_headers=["*"] is overly permissive for an API that handles user workout data and backups. Restrict these to the actual Flutter web origin and the methods/headers your client uses.
Proposed fix
app.add_middleware(
CORSMiddleware,
- allow_origins=["*"],
- allow_credentials=True,
- allow_methods=["*"],
- allow_headers=["*"],
+ allow_origins=[os.getenv("ALLOWED_ORIGINS", "http://localhost:3000").split(",")],
+ allow_credentials=False,
+ allow_methods=["GET", "POST"],
+ allow_headers=["Content-Type"],
)🤖 Prompt for AI Agents
In `@backend/main.py` around lines 8 - 14, The current
app.add_middleware(CORSMiddleware, allow_origins=["*"], allow_credentials=True,
allow_methods=["*"], allow_headers=["*"]) is invalid and too permissive; update
the CORSMiddleware config used in backend/main.py by either removing
allow_credentials=True or replacing allow_origins=["*"] with a concrete list of
allowed origins (e.g., your Flutter web origin) and set allow_methods and
allow_headers to only the HTTP methods and headers your client uses (instead of
["*"]) so credentialed requests work and CORS surface area is minimized; locate
the CORSMiddleware call and change allow_origins, allow_credentials,
allow_methods, and allow_headers accordingly.
| @app.post("/backup") | ||
| async def backup_data(data: BackupData): | ||
| if db is None: | ||
| raise HTTPException(status_code=503, detail="Database not configured") | ||
| try: | ||
| backup_doc = data.model_dump() | ||
| await db.backups.insert_one(backup_doc) | ||
| return {"status": "success", "message": "Backup received"} | ||
| except Exception as e: | ||
| raise HTTPException(status_code=500, detail=str(e)) |
There was a problem hiding this comment.
No payload size constraint on /backup — risk of oversized documents.
BackupData contains five unbounded lists (sessions, routines, targets, muscleGroups, customExercises). A single request could exceed MongoDB's 16 MB BSON document limit or consume excessive server memory before insertion fails. Consider:
- Enforcing a max request body size at the web-server/reverse-proxy level.
- Adding
max_length/max_itemsconstraints on the Pydantic model fields inbackend/models.py.
🧰 Tools
🪛 Ruff (0.15.0)
[warning] 34-34: Consider moving this statement to an else block
(TRY300)
[warning] 35-35: Do not catch blind exception: Exception
(BLE001)
[warning] 36-36: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🤖 Prompt for AI Agents
In `@backend/main.py` around lines 27 - 36, The /backup endpoint accepts a
BackupData payload with unbounded list fields which can exceed MongoDB's 16MB
limit; update the Pydantic BackupData model (in backend/models.py) to add
max_items (or max_length) constraints on sessions, routines, targets,
muscleGroups, and customExercises and/or validators that enforce a total-item or
approximate-bytes limit, then update the backup_data handler (backup_data and
the db.backups.insert_one call) to explicitly reject requests that violate those
constraints with a 413/400 response; additionally ensure the
deployment/webserver has a request body size limit configured to guard against
oversized uploads.
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
workout-logger/lib/services/storage_service.dart (2)
291-318:⚠️ Potential issue | 🟠 Major
importDatasilently dropsmuscleGroupsandcustomExerciseson restore.
exportAllData(Line 279) serializes five data categories (sessions,routines,targets,muscleGroups,customExercises), butimportDataonly restores three. Any custom exercises and muscle-group growth-rate customizations will be permanently lost after a backup-restore cycle — a critical gap for the new backup feature.Proposed fix to restore the missing data types
// Import targets if (data['targets'] != null) { for (var json in data['targets']) { final target = Target.fromJson(jsonDecode(json)); await saveTarget(target); } } + + // Import muscle groups + if (data['muscleGroups'] != null) { + for (var json in data['muscleGroups']) { + final mg = MuscleGroup.fromJson(jsonDecode(json)); + await _muscleGroupsBoxInstance.put(mg.id, jsonEncode(mg.toJson())); + } + } + + // Import custom exercises + if (data['customExercises'] != null) { + for (var json in data['customExercises']) { + final exercise = Exercise.fromJson(jsonDecode(json)); + await saveCustomExercise(exercise); + } + } }
95-117: 🧹 Nitpick | 🔵 Trivial
getSessionsForExerciseandgetSessionsInDateRangepay for an unnecessary sort.Both methods delegate to
getAllWorkoutSessions(), which now sorts results by date. The sort is wasted here since these callers only filter — they don't depend on ordering (andgetSessionsForExerciseinherited its order from the already-sorted list anyway). Consider extracting a private unsorted_getAllSessionsRaw()helper and havinggetAllWorkoutSessionscall that + sort, while filter methods use the raw list directly.
🤖 Fix all issues with AI agents
In `@backend/main.py`:
- Around line 77-101: The handler heartbeat performs two separate writes
(db.users.update_one and db.heartbeats.insert_one) that must be atomic; wrap
both operations in a single MongoDB transaction/session so either both succeed
or both roll back. Modify the heartbeat function to start an async client
session (e.g., with db.client.start_session()) and run the update_one and
insert_one inside the session (or use session.with_transaction) and pass the
session object to both db.users.update_one(...) and
db.heartbeats.insert_one(...); ensure proper exception/error handling to abort
the transaction and surface a 500 if the transaction fails.
- Around line 106-139: The analytics read endpoints (e.g., the
analytics_overview function) currently have no access control; require
authenticated + authorized access (admin role) before executing DB queries by
reusing your existing auth mechanism (e.g., the current_user/get_current_user
dependency or token validation helper) or adding an authorization decorator;
update analytics_overview to call the auth check (and _ensure_db after auth) and
return HTTP 401/403 for unauthenticated/unauthorized requests, and apply the
same guard to all /analytics/* handlers to ensure only admins can fetch
aggregate or per-user data.
- Around line 34-46: The dual-write in the /report handler uses
stats.model_dump(), then calls db.reports.update_one(...) followed by
db.report_log.insert_one(...), risking partial failure; change the order to
insert the time-series log first via db.report_log.insert_one(doc) and then
perform the upsert to db.reports.update_one, or wrap both operations in a
MongoDB session/transaction; when inserting first, remove the generated "_id"
from the doc before using it in the {"$set": ...} for db.reports.update_one to
avoid writing _id into the upserted document; apply the same fix to the
/heartbeat handler where the same pattern appears.
In `@dashboard/app.py`:
- Around line 279-286: The Raw backup JSON expander currently displays the
entire backup document from db.backups.find_one (variable doc) via st.json,
which can expose large or sensitive workout/health data; change the UI to either
require an explicit user confirmation (e.g., a checkbox or "Show raw JSON"
button) before rendering the raw doc, or render a redacted/summary view instead
(limit sessions/routines arrays and omit sensitive fields) and only call
st.json(doc) when the user has explicitly opted in; update the logic around sel,
doc, st.expander and st.json to enforce this guard.
- Around line 1-30: The app exposes sensitive user data because there is no
authentication around access to the Streamlit UI or the MongoDB connection;
modify the app to require login before any DB access by implementing an
authentication gate (e.g., using streamlit-authenticator or a simple password
check against st.secrets) and only call get_db() or render analytics after
successful auth; update get_db() to use least-privilege DB credentials stored in
st.secrets["mongo"] and ensure any admin/backups pages are additionally
restricted to an admin role check so viewers cannot access raw user/events data
without proper credentials.
- Around line 94-109: The dashboard currently duplicates backend aggregation
logic (see hb_pipeline, hb_data, df_hb and the Plotly chart creation) — replace
the direct MongoDB aggregation and DataFrame construction with HTTP calls to the
existing backend analytics endpoints (e.g., /analytics/overview and
/analytics/retention), parse the JSON response into the same shape the current
code expects, and use that data to build the Plotly figure; ensure you handle
HTTP errors and empty responses the same way the current hb_data check does and
remove the hb_pipeline/db.heartbeats.aggregate usage so the dashboard relies on
the single backend source of truth.
- Around line 53-54: The DAU calculation currently uses
db.heartbeats.count_documents which counts heartbeat documents rather than
unique users; change the calculation that sets total_heartbeats_today to use
db.heartbeats.distinct on the user identifier (e.g., distinct("user_id",
{"timestamp": {"$gte": day_ago}})) and update the metric label (replace "DAU
(heartbeats today)" with a clearer label like "DAU (unique users today)" or "DAU
(users today)") so it matches WAU/MAU semantics; adjust any variable names or
usages of total_heartbeats_today if needed to reflect it's a unique-user count.
In `@dashboard/requirements.txt`:
- Around line 1-5: The dependencies in requirements.txt (streamlit,
pymongo[srv], dnspython, pandas, plotly) are unpinned; update the file to pin
each package to a specific, tested version to ensure reproducible builds—choose
and set exact versions (e.g., streamlit==x.y.z, pymongo[srv]==a.b.c,
dnspython==d.e.f, pandas==g.h.i, plotly==j.k.l) or use a constraints file
generated from a known-good environment, and commit the updated requirements.txt
so installs are deterministic.
In `@workout-logger/lib/main.dart`:
- Around line 99-101: The calls to ApiService.sendHeartbeat() and
ApiService.trackEvent('app_open') drop their returned Futures; import
dart:async's unawaited and wrap each call with unawaited(...) to make the intent
explicit and silence lint warnings (i.e., add an import for unawaited and change
the invocations of ApiService.sendHeartbeat and ApiService.trackEvent to use
unawaited).
In `@workout-logger/lib/services/api_service.dart`:
- Around line 122-131: The backupData method currently mutates the caller's Map
by doing data['user_app_id'] = id; instead create a new Map copy inside
backupData, e.g. copy the incoming Map (preserving its entries), add the
'user_app_id' to that new map, and use the new map for jsonEncode and the POST
call so the original Map passed to backupData (e.g. from
SettingsScreen._performBackup) is not modified; update references in backupData
and keep userAppId lookup logic the same.
- Around line 1-2: Remove the dart:io import that breaks web builds and stop
using Platform; instead use existing package:flutter/foundation.dart exports:
guard with kIsWeb and use defaultTargetPlatform/TargetPlatform in the _platform
getter (replace uses of Platform.* with logic that returns 'web' when kIsWeb is
true, then switch on defaultTargetPlatform to return 'android', 'ios', or
'unknown'); update the import list to remove "dart:io show Platform" and ensure
_platform references kIsWeb and defaultTargetPlatform/TargetPlatform.
| try: | ||
| doc = stats.model_dump() | ||
| # upsert: latest report per user_app_id replaces older one | ||
| await db.reports.update_one( | ||
| {"user_app_id": stats.user_app_id}, | ||
| {"$set": doc}, | ||
| upsert=True, | ||
| ) | ||
| # also keep a time-series log for trend analysis | ||
| await db.report_log.insert_one(doc) | ||
| return {"status": "success", "message": "Usage stats reported"} | ||
| except Exception as e: | ||
| raise HTTPException(status_code=500, detail=str(e)) |
There was a problem hiding this comment.
Non-atomic dual writes in /report — partial failure risk.
Lines 37-41 upsert the latest report, then line 43 appends to the time-series log. If the second write fails, the latest report is updated but the log entry is lost, creating data inconsistency. The same pattern exists in /heartbeat (lines 84-98). Consider wrapping both operations in a MongoDB transaction, or at minimum, inserting the log entry first (append-only is safer to retry).
♻️ Proposed fix (insert log first)
try:
doc = stats.model_dump()
- # upsert: latest report per user_app_id replaces older one
- await db.reports.update_one(
- {"user_app_id": stats.user_app_id},
- {"$set": doc},
- upsert=True,
- )
- # also keep a time-series log for trend analysis
+ # append to time-series log first (idempotent append is safer to retry)
await db.report_log.insert_one(doc)
+ # then upsert latest report
+ await db.reports.update_one(
+ {"user_app_id": stats.user_app_id},
+ {"$set": {k: v for k, v in doc.items() if k != '_id'}},
+ upsert=True,
+ )Note: after insert_one, MongoDB mutates doc by adding _id. The dict comprehension strips it before the $set to avoid writing _id into the upserted document's fields.
🧰 Tools
🪛 Ruff (0.15.0)
[warning] 44-44: Consider moving this statement to an else block
(TRY300)
[warning] 45-45: Do not catch blind exception: Exception
(BLE001)
[warning] 46-46: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🤖 Prompt for AI Agents
In `@backend/main.py` around lines 34 - 46, The dual-write in the /report handler
uses stats.model_dump(), then calls db.reports.update_one(...) followed by
db.report_log.insert_one(...), risking partial failure; change the order to
insert the time-series log first via db.report_log.insert_one(doc) and then
perform the upsert to db.reports.update_one, or wrap both operations in a
MongoDB session/transaction; when inserting first, remove the generated "_id"
from the doc before using it in the {"$set": ...} for db.reports.update_one to
avoid writing _id into the upserted document; apply the same fix to the
/heartbeat handler where the same pattern appears.
| @app.post("/heartbeat") | ||
| async def heartbeat(payload: HeartbeatPayload): | ||
| """Minimal ping on every app-open for DAU/MAU tracking.""" | ||
| _ensure_db() | ||
| try: | ||
| doc = payload.model_dump() | ||
| # upsert user record | ||
| await db.users.update_one( | ||
| {"user_app_id": payload.user_app_id}, | ||
| { | ||
| "$set": { | ||
| "last_seen": doc["timestamp"], | ||
| "app_version": doc.get("app_version"), | ||
| "platform": doc.get("platform"), | ||
| }, | ||
| "$setOnInsert": {"first_seen": doc["timestamp"]}, | ||
| "$inc": {"total_opens": 1}, | ||
| }, | ||
| upsert=True, | ||
| ) | ||
| # also append to heartbeat log for DAU/MAU queries | ||
| await db.heartbeats.insert_one(doc) | ||
| return {"status": "success"} | ||
| except Exception as e: | ||
| raise HTTPException(status_code=500, detail=str(e)) |
There was a problem hiding this comment.
heartbeat handler has the same non-atomic dual-write concern as /report.
Lines 84-96 upsert the user record, then line 98 appends to the heartbeat log. A failure on the second write would update the user's last_seen/total_opens without recording the corresponding heartbeat, skewing DAU/MAU queries.
🧰 Tools
🪛 Ruff (0.15.0)
[warning] 99-99: Consider moving this statement to an else block
(TRY300)
[warning] 100-100: Do not catch blind exception: Exception
(BLE001)
[warning] 101-101: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🤖 Prompt for AI Agents
In `@backend/main.py` around lines 77 - 101, The handler heartbeat performs two
separate writes (db.users.update_one and db.heartbeats.insert_one) that must be
atomic; wrap both operations in a single MongoDB transaction/session so either
both succeed or both roll back. Modify the heartbeat function to start an async
client session (e.g., with db.client.start_session()) and run the update_one and
insert_one inside the session (or use session.with_transaction) and pass the
session object to both db.users.update_one(...) and
db.heartbeats.insert_one(...); ensure proper exception/error handling to abort
the transaction and surface a 500 if the transaction fails.
| @app.get("/analytics/overview") | ||
| async def analytics_overview(): | ||
| """High-level numbers: total users, DAU, WAU, MAU, total workouts.""" | ||
| _ensure_db() | ||
| now = datetime.now(timezone.utc) | ||
| day_ago = now - timedelta(days=1) | ||
| week_ago = now - timedelta(days=7) | ||
| month_ago = now - timedelta(days=30) | ||
|
|
||
| total_users = await db.users.count_documents({}) | ||
| dau = await db.heartbeats.count_documents( | ||
| {"timestamp": {"$gte": day_ago}}, | ||
| ) | ||
| # distinct user_app_ids in last 7 / 30 days | ||
| wau_ids = await db.heartbeats.distinct( | ||
| "user_app_id", {"timestamp": {"$gte": week_ago}} | ||
| ) | ||
| mau_ids = await db.heartbeats.distinct( | ||
| "user_app_id", {"timestamp": {"$gte": month_ago}} | ||
| ) | ||
|
|
||
| # aggregate total workouts across latest reports | ||
| pipeline = [{"$group": {"_id": None, "total": {"$sum": "$total_workouts"}}}] | ||
| cursor = db.reports.aggregate(pipeline) | ||
| agg = await cursor.to_list(1) | ||
| total_workouts = agg[0]["total"] if agg else 0 | ||
|
|
||
| return { | ||
| "total_users": total_users, | ||
| "dau": dau, | ||
| "wau": len(wau_ids), | ||
| "mau": len(mau_ids), | ||
| "total_workouts_all_users": total_workouts, | ||
| } |
There was a problem hiding this comment.
Analytics read endpoints lack any access control.
The /analytics/* endpoints expose user counts, workout totals, and per-user drill-downs to any unauthenticated caller. While the ingest endpoint auth was flagged previously, these read endpoints deserve explicit mention since they return aggregated and per-user PII.
🤖 Prompt for AI Agents
In `@backend/main.py` around lines 106 - 139, The analytics read endpoints (e.g.,
the analytics_overview function) currently have no access control; require
authenticated + authorized access (admin role) before executing DB queries by
reusing your existing auth mechanism (e.g., the current_user/get_current_user
dependency or token validation helper) or adding an authorization decorator;
update analytics_overview to call the auth check (and _ensure_db after auth) and
return HTTP 401/403 for unauthenticated/unauthorized requests, and apply the
same guard to all /analytics/* handlers to ensure only admins can fetch
aggregate or per-user data.
| """ | ||
| RepForge Analytics Dashboard | ||
| ──────────────────────────── | ||
| Connects to the same MongoDB Atlas used by the FastAPI backend | ||
| and visualises usage stats, retention, events, and user drill-downs. | ||
| """ | ||
|
|
||
| import streamlit as st | ||
| import pandas as pd | ||
| import plotly.express as px | ||
| import plotly.graph_objects as go | ||
| from pymongo import MongoClient | ||
| from datetime import datetime, timedelta, timezone | ||
|
|
||
| # ─── page config ─── | ||
| st.set_page_config( | ||
| page_title="RepForge Analytics", | ||
| page_icon="🏋️", | ||
| layout="wide", | ||
| ) | ||
|
|
||
| # ─── MongoDB connection (cached) ─── | ||
| @st.cache_resource | ||
| def get_db(): | ||
| uri = st.secrets["mongo"]["uri"] | ||
| client = MongoClient(uri) | ||
| return client.get_database("workout_logger") | ||
|
|
||
|
|
||
| db = get_db() |
There was a problem hiding this comment.
Dashboard has no authentication — all user data is publicly accessible.
Anyone who can reach this Streamlit app can browse user records, events, and raw backup data. Streamlit supports basic password protection via st.secrets or you can use streamlit-authenticator. This is critical for a dashboard exposing user analytics and backup payloads.
🤖 Prompt for AI Agents
In `@dashboard/app.py` around lines 1 - 30, The app exposes sensitive user data
because there is no authentication around access to the Streamlit UI or the
MongoDB connection; modify the app to require login before any DB access by
implementing an authentication gate (e.g., using streamlit-authenticator or a
simple password check against st.secrets) and only call get_db() or render
analytics after successful auth; update get_db() to use least-privilege DB
credentials stored in st.secrets["mongo"] and ensure any admin/backups pages are
additionally restricted to an admin role check so viewers cannot access raw
user/events data without proper credentials.
| total_users = db.users.count_documents({}) | ||
| total_heartbeats_today = db.heartbeats.count_documents({"timestamp": {"$gte": day_ago}}) |
There was a problem hiding this comment.
DAU metric counts heartbeat documents, not distinct users.
count_documents({"timestamp": {"$gte": day_ago}}) counts total heartbeat pings in the last 24 hours, not unique users. The label says "DAU (heartbeats today)" which is ambiguous. For consistency with WAU/MAU (which use distinct), use distinct here too.
♻️ Proposed fix
- total_heartbeats_today = db.heartbeats.count_documents({"timestamp": {"$gte": day_ago}})
+ dau = len(db.heartbeats.distinct("user_app_id", {"timestamp": {"$gte": day_ago}}))Then update the metric label:
- c2.metric("DAU (heartbeats today)", total_heartbeats_today)
+ c2.metric("DAU", dau)🤖 Prompt for AI Agents
In `@dashboard/app.py` around lines 53 - 54, The DAU calculation currently uses
db.heartbeats.count_documents which counts heartbeat documents rather than
unique users; change the calculation that sets total_heartbeats_today to use
db.heartbeats.distinct on the user identifier (e.g., distinct("user_id",
{"timestamp": {"$gte": day_ago}})) and update the metric label (replace "DAU
(heartbeats today)" with a clearer label like "DAU (unique users today)" or "DAU
(users today)") so it matches WAU/MAU semantics; adjust any variable names or
usages of total_heartbeats_today if needed to reflect it's a unique-user count.
| if sel: | ||
| doc = db.backups.find_one({"user_app_id": sel}, {"_id": 0}) | ||
| if doc: | ||
| st.metric("Sessions", len(doc.get("sessions", []))) | ||
| st.metric("Routines", len(doc.get("routines", []))) | ||
| st.metric("Custom Exercises", len(doc.get("customExercises", []))) | ||
| with st.expander("Raw backup JSON"): | ||
| st.json(doc) |
There was a problem hiding this comment.
Raw backup JSON exposed in dashboard without redaction.
The backup detail view renders the full backup document including all sessions, routines, and custom exercises. For a user with extensive history this could be very large and may contain sensitive workout/health data. Consider limiting the raw view or requiring an explicit confirmation.
🤖 Prompt for AI Agents
In `@dashboard/app.py` around lines 279 - 286, The Raw backup JSON expander
currently displays the entire backup document from db.backups.find_one (variable
doc) via st.json, which can expose large or sensitive workout/health data;
change the UI to either require an explicit user confirmation (e.g., a checkbox
or "Show raw JSON" button) before rendering the raw doc, or render a
redacted/summary view instead (limit sessions/routines arrays and omit sensitive
fields) and only call st.json(doc) when the user has explicitly opted in; update
the logic around sel, doc, st.expander and st.json to enforce this guard.
| streamlit | ||
| pymongo[srv] | ||
| dnspython | ||
| pandas | ||
| plotly |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
streamlit latest version 2026
💡 Result:
As of February 11, 2026, the latest Streamlit release is Streamlit 1.54.0, published February 4, 2026. [1][2]
To upgrade:
pip install --upgrade streamlit[2]
Sources: PyPI “streamlit” release history [1], Streamlit official release notes (v1.54.0 latest) [2].
🌐 Web query:
pymongo latest version 2026
💡 Result:
As of February 11, 2026, the latest stable PyMongo release on PyPI is PyMongo 4.16.0, released January 7, 2026. [1]
Release notes for v4.16 are available in the official MongoDB docs. [2] The release announcement is also posted on MongoDB Community Hub. [3]
Sources: [1] [2] [3]
🌐 Web query:
pandas latest version 2026
💡 Result:
As of February 11, 2026, the latest stable release of pandas on PyPI is pandas 3.0.0, released January 21, 2026. [1][2]
The development release notes indicate 3.0.1 is planned for February 2026 (date not finalized there), but it is not listed as released on PyPI yet. [3]
Sources: [1] PyPI release history, [2] pandas docs “What’s new in 3.0.0”, [3] pandas dev “Release notes” page.
🌐 Web query:
plotly latest version 2026
💡 Result:
- Plotly for Python (
plotly/ plotly.py) latest release on PyPI: 6.5.2 (released Jan 14, 2026). [1] - Plotly.js latest release on GitHub: v3.3.1 (released Dec 12, 2025). [2]
Sources: [1] (pypi.org) [2] (github.com)
Citations:
- 1: https://pypi.org/project/plotly/?utm_source=openai
- 2: https://github.com/plotly/plotly.js/releases
🌐 Web query:
dnspython latest version 2026
💡 Result:
As of February 11, 2026, the latest released version of dnspython is 2.8.0, published September 7, 2025. [1][2]
It requires Python 3.10+. [2]
Sources: PyPI release history and the dnspython official news post. [1][2]
Pin dependency versions for reproducible builds.
All five dependencies lack version pins, making builds non-deterministic and vulnerable to silent breakage from upstream changes.
♻️ Proposed fix
-streamlit
-pymongo[srv]
-dnspython
-pandas
-plotly
+streamlit>=1.30,<2
+pymongo[srv]>=4.6,<5
+dnspython>=2.5,<3
+pandas>=2.1,<4
+plotly>=5.18,<7📝 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.
| streamlit | |
| pymongo[srv] | |
| dnspython | |
| pandas | |
| plotly | |
| streamlit>=1.30,<2 | |
| pymongo[srv]>=4.6,<5 | |
| dnspython>=2.5,<3 | |
| pandas>=2.1,<4 | |
| plotly>=5.18,<7 |
🤖 Prompt for AI Agents
In `@dashboard/requirements.txt` around lines 1 - 5, The dependencies in
requirements.txt (streamlit, pymongo[srv], dnspython, pandas, plotly) are
unpinned; update the file to pin each package to a specific, tested version to
ensure reproducible builds—choose and set exact versions (e.g.,
streamlit==x.y.z, pymongo[srv]==a.b.c, dnspython==d.e.f, pandas==g.h.i,
plotly==j.k.l) or use a constraints file generated from a known-good
environment, and commit the updated requirements.txt so installs are
deterministic.
| final api = ApiService(); | ||
| api.sendHeartbeat(); | ||
| api.trackEvent('app_open'); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Dropped futures from sendHeartbeat() and trackEvent().
Lines 100-101 invoke async methods without await or .catchError. While these methods internally swallow errors, the Future objects are silently dropped. Dart's unawaited() from dart:async makes the intent explicit and silences lint warnings.
♻️ Proposed fix
+import 'dart:async' show unawaited;
...
final api = ApiService();
- api.sendHeartbeat();
- api.trackEvent('app_open');
+ unawaited(api.sendHeartbeat());
+ unawaited(api.trackEvent('app_open'));📝 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.
| final api = ApiService(); | |
| api.sendHeartbeat(); | |
| api.trackEvent('app_open'); | |
| import 'dart:async' show unawaited; | |
| final api = ApiService(); | |
| unawaited(api.sendHeartbeat()); | |
| unawaited(api.trackEvent('app_open')); |
🤖 Prompt for AI Agents
In `@workout-logger/lib/main.dart` around lines 99 - 101, The calls to
ApiService.sendHeartbeat() and ApiService.trackEvent('app_open') drop their
returned Futures; import dart:async's unawaited and wrap each call with
unawaited(...) to make the intent explicit and silence lint warnings (i.e., add
an import for unawaited and change the invocations of ApiService.sendHeartbeat
and ApiService.trackEvent to use unawaited).
| import 'dart:convert'; | ||
| import 'dart:io' show Platform; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check all dart:io imports in the codebase
rg "dart:io" --type=dart -nRepository: Devasy/Workout-logger
Length of output: 189
🏁 Script executed:
# Check if web is a target platform by looking at pubspec.yaml
fd "pubspec.yaml" -x cat {}Repository: Devasy/Workout-logger
Length of output: 4402
🏁 Script executed:
# Examine the api_service.dart file to see the full context
fd "api_service.dart" -x wc -l {}Repository: Devasy/Workout-logger
Length of output: 115
🏁 Script executed:
# Read api_service.dart to see the full context and verify line 40 has kIsWeb guard
cat -n workout-logger/lib/services/api_service.dart | head -50Repository: Devasy/Workout-logger
Length of output: 1819
🏁 Script executed:
# Check how Platform is actually used in the file
rg "Platform\." workout-logger/lib/services/api_service.dart -A 2 -B 2Repository: Devasy/Workout-logger
Length of output: 229
🏁 Script executed:
# Check if kIsWeb is imported and used
rg "kIsWeb|package:flutter/foundation" workout-logger/lib/services/api_service.dart -nRepository: Devasy/Workout-logger
Length of output: 141
dart:io import breaks web builds.
import 'dart:io' show Platform; fails at compile time on Flutter web targets because dart:io is not available for web. The kIsWeb guard on line 40 prevents calling Platform, but the import itself is the problem.
Replace with package:flutter/foundation.dart (already imported on line 4):
♻️ Proposed fix
-import 'dart:io' show Platform;
+import 'package:flutter/foundation.dart' show defaultTargetPlatform, TargetPlatform;Then update _platform (lines 39-44):
String get _platform {
if (kIsWeb) return 'web';
switch (defaultTargetPlatform) {
case TargetPlatform.android: return 'android';
case TargetPlatform.iOS: return 'ios';
default: return 'unknown';
}
}🤖 Prompt for AI Agents
In `@workout-logger/lib/services/api_service.dart` around lines 1 - 2, Remove the
dart:io import that breaks web builds and stop using Platform; instead use
existing package:flutter/foundation.dart exports: guard with kIsWeb and use
defaultTargetPlatform/TargetPlatform in the _platform getter (replace uses of
Platform.* with logic that returns 'web' when kIsWeb is true, then switch on
defaultTargetPlatform to return 'android', 'ios', or 'unknown'); update the
import list to remove "dart:io show Platform" and ensure _platform references
kIsWeb and defaultTargetPlatform/TargetPlatform.
| Future<bool> backupData(Map<String, dynamic> data) async { | ||
| try { | ||
| final id = await userAppId; | ||
| data['user_app_id'] = id; | ||
|
|
||
| final response = await _client.post( | ||
| Uri.parse('$_baseUrl/backup'), | ||
| headers: {'Content-Type': 'application/json'}, | ||
| body: jsonEncode(data), | ||
| ); |
There was a problem hiding this comment.
backupData mutates the caller's map in-place.
Line 125 (data['user_app_id'] = id) modifies the Map passed by the caller, which is a side effect. In SettingsScreen._performBackup, this silently injects user_app_id into the decoded export data. If that map is ever reused or inspected after the call, it will contain unexpected keys.
Create a new map instead:
🐛 Proposed fix
Future<bool> backupData(Map<String, dynamic> data) async {
try {
final id = await userAppId;
- data['user_app_id'] = id;
+ final payload = {...data, 'user_app_id': id};
final response = await _client.post(
Uri.parse('$_baseUrl/backup'),
headers: {'Content-Type': 'application/json'},
- body: jsonEncode(data),
+ body: jsonEncode(payload),
);📝 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.
| Future<bool> backupData(Map<String, dynamic> data) async { | |
| try { | |
| final id = await userAppId; | |
| data['user_app_id'] = id; | |
| final response = await _client.post( | |
| Uri.parse('$_baseUrl/backup'), | |
| headers: {'Content-Type': 'application/json'}, | |
| body: jsonEncode(data), | |
| ); | |
| Future<bool> backupData(Map<String, dynamic> data) async { | |
| try { | |
| final id = await userAppId; | |
| final payload = {...data, 'user_app_id': id}; | |
| final response = await _client.post( | |
| Uri.parse('$_baseUrl/backup'), | |
| headers: {'Content-Type': 'application/json'}, | |
| body: jsonEncode(payload), | |
| ); |
🤖 Prompt for AI Agents
In `@workout-logger/lib/services/api_service.dart` around lines 122 - 131, The
backupData method currently mutates the caller's Map by doing
data['user_app_id'] = id; instead create a new Map copy inside backupData, e.g.
copy the incoming Map (preserving its entries), add the 'user_app_id' to that
new map, and use the new map for jsonEncode and the POST call so the original
Map passed to backupData (e.g. from SettingsScreen._performBackup) is not
modified; update references in backupData and keep userAppId lookup logic the
same.
… backend - Flutter: decode JSON-string list items to Maps before sending to /backup - Backend: BackupData model accepts List[Any] with parsed_backup() helper - Fixes 'Input should be a valid dictionary' validation error
1 participant
Implemented a backend service using FastAPI and MongoDB to collect usage statistics and provide remote backup functionality. Integrated the Flutter application to report usage on startup and allow users to trigger a manual backup from a new Settings screen. Configured Railway deployment.
PR created automatically by Jules for task 9244323662212404022 started by @Devasy23
Summary by CodeRabbit
New Features
Backend
Dashboard