Skip to content

PersonalRecord.exerciseId stores a composite handle key, breaking exercise-name lookups and unscoped PR queries #69

Description

@Devasy

Summary

PRManager._checkExercise() (workout-logger/lib/services/managers/pr_manager.dart lines 72-109) computes a cache/storage key of '$exerciseId:$handle' when the logged set has a handle (e.g. a "Rope" attachment on a cable exercise), then stores that composite string directly into PersonalRecord.exerciseId:

final updated = PersonalRecord(
  exerciseId: key,   // "cable_curl:Rope", not "cable_curl"
  ...
);
_cache[key] = updated;

PersonalRecord.exerciseId is supposed to be a real exercise id everywhere else in the app. Once a handle-scoped PR is saved, that invariant breaks for that record permanently (it round-trips through storage unchanged).

How to reproduce (user-visible impact)

  1. Log a workout using an exercise that has handle/attachment options — e.g. Cable Curl, select the Rope handle — with weight/reps high enough to set a new PR.
  2. Ask the AI Coach something that triggers get_personal_records or get_exercise_performance, e.g. "What's my personal record for cable curl?"
  3. One of two broken outcomes happens:
    • The coach reports an unresolved/garbled exercise name (something like cable_curl:Rope instead of "Cable Curl"), because coach_tool_service.dart calls _wp.getExerciseName(pr.exerciseId) for every record in allRecords, and getExerciseName has no entry for the composite key.
    • Or, if you ask for the exercise's PR without mentioning the handle, _exercisePerformance calls getRecord(exerciseId) with no handle — since no exercise-wide (handle-less) entry was ever created, this returns null, and the coach says you have no PR for an exercise you've clearly set one for.

Why this isn't a quick fix

The correct fix (keep exerciseId as the real id, carry handle as its own field, and key the cache from both) requires a schema change on both storage backends this app supports mid-migration:

  • PersonalRecord model: add a handle field, update toJson/fromJson.
  • StorageService (Hive): the personal_records box is currently keyed by exercise id alone.
  • SqliteStorageService: the personal_records table has exercise_id TEXT PRIMARY KEY — a single-handle-per-exercise assumption baked into the schema. Supporting per-handle PRs needs a compound key, which means a v3 schema migration.

Given the app is mid-migration from Hive to SQLite already, this needs a deliberate schema decision rather than a patch applied while resolving PR review comments.

Pointers

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions