refactor(training): one lane load reading for Training Load, Cardio and Strength - #27
Merged
Merged
Conversation
Move the detached computation in TrainingLoadModel.load into a static prepare function without changing a line of its arithmetic, so a fixture can run it. The pinned figures cover rated and unrated sets, an unknown cardio day inside and outside the window, a duplicate awaiting review and a session rated twice. Analysis migration required: no
…s lanes The Cardio and Strength screens computed "load vs your usual" their own way: Cardio could fall back to non-TRIMP effort and knew no unknown days, and both compared only through today. Tapping from Training Load to either screen could therefore show a different percentage for the same week. TrainingLoadLanes now holds the one strength and cardio lane computation, read through any day. Training Load reads it through today. Cardio and Strength read it through the selected week's Sunday, over their history window plus the 84-day lookback a reading needs, and their load tiles and coach context use it. Analysis migration required: no
There was a problem hiding this comment.
🟡 Changes recommended
Address duplicated fusion work and preserve as-of correctness for historical cardio readings.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Refactors Training Load, Cardio, and Strength to share consistent lane-load calculations and weekly comparisons.
Changes:
- Adds shared lane, ratio, coverage, and status calculations.
- Updates Cardio and Strength models and views to consume shared lanes.
- Adds oracle, lookback, and week-boundary regression tests.
File summaries
| File | Summary |
|---|---|
StrandTests/TrainingLoadLanesTests.swift |
Adds shared lane and lookback behavior tests. |
Strand/Screens/TrainingLoadView.swift |
Uses the extracted lane calculations. |
Strand/Screens/TrainingLoadLanes.swift |
Implements shared lane computation. |
Strand/Screens/StrengthView.swift |
Displays shared strength lane values. |
Strand/Screens/StrengthModel.swift |
Loads and exposes strength lanes. Moderate concern: duplicated history reads and session fusion (2 votes). |
Strand/Screens/CardioView.swift |
Displays shared cardio lane values. |
Strand/Screens/CardioModel.swift |
Loads and exposes cardio lanes. Moderate concerns: duplicated fusion work (2 votes) and non-as-of deduplication affecting past readings (1 vote). |
Review details
Suppressed comments (1)
Strand/Screens/CardioModel.swift:127
- This resolution is computed over the entire loaded window, but
cardioLoadsresolves overlaps newest-first. If a past session has a later overlapping twin, the later record is kept and the past record is marked as a duplicate beforecardioLane(... through: laneDay)truncates the series. The past week's reading can therefore change when data after its Sunday is added, contrary to the as-of semantics described byreadingDay; resolve/deduplicate the input as of each reading day (or preserve an as-of result per session) before building the cached lane.
let laneResolution = await repo.cardioLoads(for: laneFusion.sessions)
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+126
to
+127
| let laneFusion = await repo.trainingSessions(days: range.days + TrainingLoadLanes.lookbackDays) | ||
| let laneResolution = await repo.cardioLoads(for: laneFusion.sessions) |
|
|
||
| async let historyRead = repo.resolvedStrengthHistory(days: historyDays) | ||
| async let fusedRead = repo.trainingSessions(days: historyDays) | ||
| async let laneHistoryRead = repo.resolvedStrengthHistory(days: historyDays + TrainingLoadLanes.lookbackDays) |
This was referenced Sep 17, 2026
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
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
Second package of the Training Load / Cardio / Strength redesign. The redesigned screens will show a large "+x % vs usual" on all three, and tapping a lane on Training Load opens Cardio or Strength. So the figure has to be identical, and before this PR it was not:
relativeLoadreading.CardioSession.cardioLoadTrend. That could fall back to a non-TRIMP effort figure, knew no unknown days, and read only the week's fused non-strength sessions.StrengthSession.strengthLoadTrend, a plain trend rather than the staged reading.What changed:
Strand/Screens/TrainingLoadLanes.swiftnow holds the single strength and cardio lane computation, read through any day. It covers weighted sets, the TRIMP series with unknown days, relative load, coverage counts, lower bound, distribution, week over week, status, and the 56 daily ratios.TrainingLoadModel.preparereads it through today. The session lane, the provisional ring, history, adaptation and recovery stay where they were.CardioModel/StrengthModelexposelaneandlaneRatiosper selected week:TrainingLoadLanes.lookbackDays(84 days: 56 for the comparison, 28 for the RPE median that weights unrated sets). Data older than that cannot move a reading.CardioSession.cardioLoadTrendandStrengthSession.strengthLoadTrendhave no app caller any more. They stay for now because their package tests still coveradditiveLoadand the effort axis.Behaviour change to be aware of
Verification
TrainingLoadModel.prepareand its outputs were pinned as literals: rated/unrated sets, an unknown cardio day inside and outside the window, a duplicate awaiting review, a session rated twice. Commit 2 refactors, and the same literals still pass.readingDayreturns Sunday, or today while the week runs.xcodebuild … -scheme Strand test -only-testing:StrandTests: 3,371 tests, 0 failures.xcodebuild … -scheme NOOPiOS -destination 'generic/platform=iOS Simulator' build: succeeded.python3 Tools/doc_comment_lint.py: OK.Analysis migration required: no. No formula, window or stored value changes; Cardio and Strength now display the reading Training Load already showed.