Redesign Personal Records: multi-record movement PRs + workout/benchmark PRs - #1831
Conversation
measurable_reps_msg crashed on movement logs with no reps recorded (load/distance/duration/calorie-only PRs). User#personal_records also showed whichever movement log was logged first, not the best one, because the inherited Log#movement_logs order(:id) scope silently won over the intended value-based ordering. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Design approved in #1829. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Excludes MovementRecordSet's rule-table dispatch method from the complexity/AbcSize cops (same pattern already used for Ability), since its branch count tracks the PR rule table rows, not incidental complexity.
… just one per movement
…ration Task 2 made User#personal_records return several MovementLog rows per movement (e.g. Deadlift at 1, 5, and 52 reps). The controller's sort only grouped by movement name, so multi-record rows for one movement appeared in insertion order instead of a readable progression.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
User#workout_records raised NoMethodError (time-scored nil) or ArgumentError (mixed nil/real scores) for any user with an unscored log, since score_value is nullable but only score_type is required. Guard nil so an unscored log always loses the comparison instead of crashing or winning by accident.
…_01WTLwi4FRhKYhikPnfneBCV
candidate_for was restructured into guard-clause methods along the rule table's own groupings (load / distance-or-duration / bare metric), so it satisfies AbcSize, CyclomaticComplexity, and PerceivedComplexity on its own merits instead of carving out per-file excludes in .rubocop.yml. The implementation plan now lives as a comment on the design issue (#1829 (comment)) rather than committed to the repo, matching how the design spec itself is tracked. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
candidate_for was flagged for AbcSize/CyclomaticComplexity/PerceivedComplexity. Split into guard-clause methods along the rule table's own groupings (load / distance-or-duration / bare metric) instead of carving out per-file excludes in .rubocop.yml -- behavior and test coverage are unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
🚅 Deployed to the pr-cf53cd-1831 environment in wod-tracker
|
Every workout the user had ever logged was showing up as a "record," including one-off attempts and weight-scored lift workouts -- the latter are already represented on the movement PR page (grouped by rep count), so showing them again here was redundant. A workout now only appears once it's been logged more than once, and only if it isn't weight-scored. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Yanchek99
left a comment
There was a problem hiding this comment.
Thanks for the detailed PR description and test coverage. The movement record grouping PORO is nicely isolated, and the controller/view shape for the new page matches the existing user-scoped PR page. I found a few requirement mismatches against issue #1829 that should be fixed before merge: workout records currently exclude single-attempt workouts and weight-scored workouts even though the design says both qualify, the tests encode those exclusions, and movement PR display ordering does not match the duration-based grouping rule for fixed-duration records.
…ntroller Adds a Lifts / Repeated Workouts subnav under a single Personal Records section instead of two separate nav items.
…_01WTLwi4FRhKYhikPnfneBCV
Matches the existing .claude/.worktrees exclusions -- prevents skill-generated brainstorming/SDD scratch files from being committed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The barbell action grouped weightlifting PRs without checking that load was present, so a reps-only record (no load) could render as a fabricated "0 lbs" PR and even outrank the real headline due to the reps-based sort. Filter the barbell tab to loaded records only, and guard rep_max_label so a loaded record with no rep count renders the load without a bare "RM" label instead of dropping the row.
…_01WTLwi4FRhKYhikPnfneBCV
Grew past the 100-line limit purely from merging two unrelated branches' independent test additions to this shared file -- not from any single change growing it. Matches the existing precedent for other oversized test files in this same cop.
Movements whose PRs are uniformly distance+time (no load) now match the Weightlifting tab's grouped/expand-in-place pattern: one row per movement, headlined by the most recently logged distance, expanding to show every distance in ascending order with its time. Reuses the existing generic rep-max-row Stimulus controller unchanged. Records that aren't distance+time shaped (calorie-based tests, reps-in-a-time-cap tests) stay in the existing flat list below the grouped movements -- nothing disappears, nothing gets forced into a row format that doesn't fit its shape.
distance_label now renders "500m" instead of "500 meters". duration_msg reuses MeasurableHelper's existing duration_metric_msg, which already drops the leading "00:" when there are no hours (e.g. "20:00" instead of "00:20:00") -- an established format elsewhere in the app, not a new one.
|
Blocked by #1860 — the Weightlifting tab's rep-max ("NRM") grouping in this PR labels any weightlifting record with reps+load present, without verifying the reps were performed as one continuous, unbroken set (e.g. an interval-ladder total like Diane's 45-rep aggregated deadlift entry would render as "45RM"). #1860 tracks adding that verification. This PR can merge as-is, but #1860 should land soon after to avoid shipping misleading rep-max claims. |
…_01WTLwi4FRhKYhikPnfneBCV # Conflicts: # app/helpers/measurable_helper.rb # app/javascript/controllers/index.js # test/helpers/measurable_helper_test.rb
Resolves #1860: a rep count > 1 (e.g. Diane's aggregated 45-rep Deadlift, Murph's 100 pull-ups) is only shown as a rep-max PR once MovementLog#set_breakdown verifies a genuine unbroken effort, via the new MovementLog#verified_unbroken_reps. The Weightlifting tab labels off the verified count instead of the raw total, and the same exclusion now applies to the Gymnastics flat list. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
#1860 is resolved (merged as #1861, plus follow-up work on this branch to actually wire the Weightlifting and Gymnastics tabs into it). No longer blocked. A rep count > 1 (e.g. Diane's aggregated 45-rep Deadlift, Murph's 100 pull-ups) is now only shown as a rep-max PR once |
single_weightlifting_exercise_day? only recognized a workout with exactly one exercise total, missing the far more common variable set-based lifting structure (e.g. "Back Squat: 5-5-3-3-1-1", multiple sets of the same movement, scored by weight). Reuse the existing Workout#set_based_lifting? concept, which already covers both the fixed-round and variable-set shapes, instead of the narrower ad-hoc check. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A single recorded set (e.g. one line of a "5x5 Back Squat", set_breakdown: [5]) isn't a "breakdown" -- there's nothing split across multiple sets to reveal, so showing "[5]" next to "5 Back Squats" was just noise. The verification data itself (needed for the Weightlifting rep-max gate) is unchanged; only the display is suppressed when there's one set to show. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…_01WTLwi4FRhKYhikPnfneBCV
…ubmission Verifies the full LogsController#create path -- not just Log#build_movement_logs -- correctly auto-populates set_breakdown for a set-based lifting day even when the recording form's hidden set_breakdown_text field submits blank. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
MovementLog#verified_unbroken_reps required a captured set_breakdown for any reps > 1, which meant historical set-based lifting logs (e.g. old "5x5 Back Squat" entries) stayed excluded from the Weightlifting tab even though set_based_lifting? already proves them unbroken by construction. That proof comes from workout structure (segments, exercises, score type) that already exists for every log, old or new -- trust reps directly in that case instead of requiring set_breakdown to have been written first. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…turally too verified_unbroken_reps only recognized set_based_lifting? (weighted sets), so a bare "how many can you do" bodyweight test -- one exercise, prescribed reps: 0, no time cap -- still required a captured set_breakdown despite being unbroken by definition: going until failure in one continuous attempt is what the test is. Reuse the existing Workout#single_achievement_test? concept, which already identifies exactly this shape, regardless of movement family. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…_01WTLwi4FRhKYhikPnfneBCV # Conflicts: # app/models/concerns/log_set_breakdown.rb
Summary
Design: #1829
Plan:
docs/superpowers/plans/2026-07-28-pr-view-redesign.mdBuilt with subagent-driven development: one implementer + one task-scoped reviewer per plan task, plus a final whole-branch review that caught and fixed one more bug (
User#workout_recordscrashing on a log with noscore_value).Test plan
🤖 Generated with Claude Code