Commit 836fffb
Redesign Personal Records: multi-record movement PRs + workout/benchmark PRs (#1831)
* Fix PRs page 500 and wrong-record bug in personal records
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>
* Add implementation plan for PR view redesign
Design approved in #1829.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Add MovementRecordSet to rank/group movement logs into distinct PRs
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.
* Show one personal record per distinct rep/distance/duration test, not just one per movement
* Order multiple personal-record rows for a movement by rep/distance/duration
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.
* Add User#workout_records for best-score-per-workout tracking
* Add Workout Records page showing best score per workout
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Fix workout_records crash on logs with nil score_value
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.
* Drop RuboCop excludes and committed plan doc from PR view branch
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>
* Restructure MovementRecordSet to satisfy RuboCop without excludes
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>
* Narrow Workout Records to repeated metcons, excluding lift workouts
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>
* Merge Personal Records and Workout Records into one PersonalRecordsController
Adds a Lifts / Repeated Workouts subnav under a single Personal Records
section instead of two separate nav items.
* Ignore .superpowers/ agent scratch workspace
Matches the existing .claude/.worktrees exclusions -- prevents
skill-generated brainstorming/SDD scratch files from being committed.
* Add Barbell Lifts tab grouping rep-maxes by movement
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Wire up expand/collapse for Barbell Lifts rows
* Fix Barbell Lifts tab to handle nil load/reps gracefully
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.
* Exclude measurable_helper_test.rb from Metrics/ClassLength
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.
* Replace Lifts/Barbell Lifts tabs with one tab per movement family
Consolidates the Lifts and Barbell Lifts personal-records tabs into a
single dynamic GET /users/:user_id/personal_records/:family route,
constrained to weightlifting, gymnastics, or monostructural. The
weightlifting family keeps the existing grouped/expand-in-place view
(renamed from barbell.html.slim); gymnastics and monostructural reuse
the old flat-list view (renamed from lifts.html.slim), each filtered
to its own family. The global nav "PRs" link and the subnav partial
are updated to point at the new route.
The controller test file was split into
personal_records_controller_test.rb and
personal_records_weightlifting_test.rb (grouped by concern) to keep
Metrics/ClassLength satisfied without a new RuboCop exclude.
* Add empty-state messages to personal records family tabs
The new default landing tab (weightlifting) and other family tabs
rendered nothing but the heading and subnav when a user had no PRs
in that family, giving no indication that data exists on another
tab. Adds a text-muted empty-state line to both templates, matching
the pattern already used in logs/index.html.slim.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Fix Weightlifting tab visual issues: cards, fake links, tiny caret
Replace per-row card boxes with a plain divided list (list-group-flush)
matching the flat family-list tabs. Reset the expand toggle button so
the movement name reads as plain bold text instead of a blue underlined
link -- only the load value, which is a real navigation link, should
look like one. Bump the expand/collapse caret to fs-4 for a larger tap
target.
* Rank fixed load/reps/distance tests by elapsed time
A movement logged with load, reps, and distance all fixed (e.g. "1000
Box Step-ups @ 45lb/20in for time") was ranking by load instead of
time, so repeated attempts at the same load all survived as separate
PRs that rendered identically. Add a load_for_time candidate, checked
before the existing find-a-max-with-time-cap load_candidate, that
ranks by the fastest duration when a fixed distance is also present.
* Remove redundant family heading from the family tabs view
The subnav's active tab already names the current family; a second
"Gymnastics"/"Monostructural" heading right below it was noise.
* Unify family-tab styling and fix stray parens on reps-less records
Convert the Gymnastics/Monostructural flat list from a <table> to the
same list-group-flush row markup as the Weightlifting tab, so movement
names render with the same font-weight everywhere on the page instead
of relying on the browser's default bold <th>.
Add PersonalRecordsHelper#record_msg: measurable_additional_metrics
always wraps its output in parens, meant to trail a reps message
(e.g. "10 Rows (20 calories)"). A pure distance/duration test with no
reps (Row, Run, Air Bike) had nothing for those parens to trail,
rendering as a bare "(5000 meters)". record_msg only parenthesizes
when there's a reps message to attach to.
* Treat reps: 0 as unspecified in rep_max_label, not "0RM"
reps: 0 is this app's existing convention for uncapped/unspecified
reps (the "max reps X" prescription pattern), not a literal zero-rep
set. rep_max_label's `unless movement_log.reps` guard didn't catch it
since 0 is truthy in Ruby, so a loaded PR with reps: 0 rendered as
"0RM" instead of falling back to a bare load, like nil reps already do.
* Rename Repeated Workouts tab to Workouts, align its row styling
Convert repeated_workouts.html.slim from a <table> to the same
list-group-flush row markup as the other three tabs, and add an empty
state, matching the treatment already applied to Weightlifting and
the family tabs.
* Label reps: 0 rows "Max" instead of leaving them blank
Blank was correct in isolation but looked broken as one row among
labeled "1RM"/"2RM" siblings in the expanded list -- no indication of
what that row represented. Label it explicitly using the app's
existing "max reps" terminology instead.
* Fix INTERVAL_SCHEME backtracking into a false "1 set of 0 reps" scheme
A plain interval count with no real "sets: reps" scheme after it (e.g.
"On the Minute x 10 (5 Rounds): ...") let the first \d+ capture
backtrack, splitting "10" into captures "1" and "0" -- since the
optional "sets"/":" separator gave the regex nothing to require
between the two numbers. This fabricated a false uniform scheme that
won out over the correct single-rep fallback whenever the log
happened to have exactly as many set_details as the fabricated "set"
count (typically 1), producing reps: 0 imports like the real-world
"Front Squat 1x0" workout traced from a live PR-preview import.
Guarded with the same (?!\d) lookahead DASH_SCHEME already uses for
this exact class of problem. Moved the two existing INTERVAL_SCHEME
tests plus the new regression test into
set_scheme_extractor_uniform_scheme_test.rb, which already exists
specifically for UNIFORM_SET_SCHEMES coverage -- set_scheme_extractor_test.rb
had no headroom left under Metrics/ClassLength.
* Group Monostructural by movement, showing every distance and time
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.
* Abbreviate distance units and drop hours when under an hour
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.
* Gate Weightlifting and Gymnastics rep PRs on a verified unbroken set
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>
* Broaden set_breakdown auto-population to any set-based lifting day
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>
* Suppress the set_breakdown bracket for a trivial single-entry breakdown
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>
* Add regression test for set_breakdown auto-population via real form submission
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>
* Verify set-based lifting day rep counts structurally, no backfill needed
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>
* Trust a single max-effort achievement test (e.g. "Max Pull-up") structurally 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>
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>1 parent 428044f commit 836fffb
33 files changed
Lines changed: 1106 additions & 65 deletions
File tree
- app
- controllers
- helpers
- javascript/controllers
- models
- concerns
- services/sugarwod_import
- views
- layouts
- movement_logs
- personal_records
- config
- test
- controllers
- helpers
- models
- services/sugarwod_import
- system
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
67 | 67 | | |
68 | 68 | | |
69 | 69 | | |
| 70 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
52 | 52 | | |
53 | 53 | | |
54 | 54 | | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
55 | 58 | | |
56 | 59 | | |
57 | 60 | | |
| |||
This file was deleted.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
38 | 38 | | |
39 | 39 | | |
40 | 40 | | |
41 | | - | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
42 | 47 | | |
43 | 48 | | |
44 | 49 | | |
| |||
48 | 53 | | |
49 | 54 | | |
50 | 55 | | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
51 | 59 | | |
52 | 60 | | |
53 | 61 | | |
54 | 62 | | |
55 | | - | |
| 63 | + | |
56 | 64 | | |
57 | 65 | | |
58 | 66 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
34 | 37 | | |
35 | 38 | | |
36 | 39 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
26 | | - | |
27 | | - | |
28 | | - | |
29 | | - | |
30 | | - | |
31 | | - | |
32 | | - | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
33 | 34 | | |
34 | 35 | | |
35 | 36 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
57 | 57 | | |
58 | 58 | | |
59 | 59 | | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
60 | 79 | | |
61 | 80 | | |
62 | 81 | | |
| |||
0 commit comments