Merged
Conversation
Add equipmentType to exercises, replace prettifyWeight with snapWeight. Barbell/machine: 2.5kg/5lbs, dumbbell: 1kg<10/2kg>=10/5lbs. Domain-level snap validation, slider step per equipment, custom exercise picker. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
tomkis
commented
Mar 6, 2026
Owner
Author
tomkis
left a comment
tomkis
left a comment
There was a problem hiding this comment.
Code Review
Overall
Clean, well-structured feature. Domain-level snapping prevents inconsistencies vs UI-only approach. Good test coverage.
Issues
1. Dumbbell metric getSliderStep vs snapWeight mismatch (bug)
getSliderStep returns 2 for dumbbell metric, but snapWeight uses 1 for weights under 10kg. Slider step of 2 means users can't reach odd values (7kg, 9kg) that snapWeight would allow. Consider returning 1 for dumbbell metric slider step.
2. getSliderStep and getIncrement duplicate logic
Near-copy of each other minus the weight-dependent branch. Consider having getSliderStep delegate to getIncrement with a sensible default, or co-locate with a note about why they differ.
3. Repeated await getUnit() — N+1 concern
In workout-context.local.ts, every method hits DB for unit. If multiple methods run in sequence (e.g., finishWorkout → initializeMesocycle), unit is fetched repeatedly. Minor perf, not blocking.
4. estimatedOneRepMax lost snapping
In exercise.aggregate-root.ts, prettifyWeight replaced with Math.round. 1RM estimate in exercise library no longer snaps to equipment increments. Arguably correct (estimate, not prescribed weight) — confirm intentional.
5. Diamond Pushups classified as machine
Bodyweight exercise — machine is closest bucket but slightly misleading. Not blocking.
Nits
- Imperial
getIncrementswitch could collapse all three cases (all return5) Dumbellmisspelled throughout seed data (pre-existing)
Looks Good
- Migration with backfill approach
snapWeightrounding logic- Full-stack threading of
equipmentType+unit - Test coverage
- UI equipment type picker
…er 10kg getSliderStep hardcoded 2kg for all dumbbell metric weights, but getIncrement correctly uses 1kg under 10kg. This prevented users from selecting odd dumbbell weights (3, 5, 7, 9kg) via the slider. Fix by making getSliderStep delegate to getIncrement, eliminating the logic duplication and ensuring slider step matches snapping behavior. https://claude.ai/code/session_014yWJPLx34yjcKDFzUeAMpD
The previous fix still had a problem: getSliderStep was weight-dependent but the Slider component takes a single fixed step prop. Starting at 8kg (step=1) and dragging past 10kg kept the step at 1 instead of 2, and starting at 12kg (step=2) made odd values below 10kg unreachable. Fix: getSliderStep now returns the smallest valid increment for the equipment/unit combo (1kg for dumbbell metric), and the slider's onValueChange callback applies snapWeight to ensure the displayed and committed values always land on real dumbbell weights. https://claude.ai/code/session_014yWJPLx34yjcKDFzUeAMpD
getSliderStep now delegates to getIncrement(…, 0) instead of duplicating the same unit/equipment branching logic. Also simplified the imperial branch in getIncrement since all equipment types share the same 5lb increment. https://claude.ai/code/session_013y997tgR5DYJPBkhuNz18Y
Adds a property-based test across all equipment/unit combos ensuring every snapped weight is a multiple of the slider step, guarding against future increments that the slider couldn't reach. https://claude.ai/code/session_013y997tgR5DYJPBkhuNz18Y
Dumbbells now always snap to 1kg increments instead of the previous 1kg-under-10/2kg-over-10 split. This removes getSliderStep (redundant with getIncrement) and simplifies the weight-snapping module. https://claude.ai/code/session_013y997tgR5DYJPBkhuNz18Y
Unit is now stored as part of the mesocycle rather than being fetched separately from onboarding data on every aggregate root instantiation. This eliminates the redundant getUnit() helper and the extra DB query per operation, and ensures the unit is persisted with the mesocycle. https://claude.ai/code/session_01Q3WfY4UmxVH4FH8EAvGJhb
Instead of adding a unit column to the mesocycle table, join the onboarding_data table in getMesocycleById to get the user's unit. This avoids a schema migration and keeps unit as a single source of truth on the user's onboarding data. https://claude.ai/code/session_01Q3WfY4UmxVH4FH8EAvGJhb
…om CLAUDE.md Add "No Silent Defaults" section to code-standards.md with examples. Update CLAUDE.md to require reading docs (architecture, code-standards, and relevant domain docs) before writing any code, instead of inlining rules directly in CLAUDE.md. https://claude.ai/code/session_01Q3WfY4UmxVH4FH8EAvGJhb
Rework exercise classification from equipment-based (Barbell/Dumbbell/Machine) to loading-based (DoublePlates/Dumbbell/Plates/Stack). This better describes how weight is added: double plates for barbells and plate-loaded machines, plates for bodyweight+added plates (dips, pushups), and stack for cable/ selectorized machines. Reclassify all 100 curated exercises accordingly. https://claude.ai/code/session_0145iqbUZpnMa1impMJEMpGu
Remove the silent `.default('stack')` from the schema so new exercises
fail without an explicit loading type. Add migration 0003 to rename
equipment_type → loading_type and backfill existing values (machine → stack,
barbell → double_plates).
https://claude.ai/code/session_0145iqbUZpnMa1impMJEMpGu
…e labels Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #37
equipmentType(barbell/dumbbell/machine) to exercise schema with DB migration + backfillprettifyWeightwith domain-levelsnapWeight— barbell/machine: 2.5kg/5lbs, dumbbell: 1kg<10/2kg≥10/5lbsequipmentType+unitthrough entire chain: DB → DAO → domain → aggregate → UITest plan
🤖 Generated with Claude Code