From bed92bc321d203acf41dcb4be4bb8178c4430fc4 Mon Sep 17 00:00:00 2001 From: Boris 'B' Kurktchiev Date: Tue, 29 Sep 2026 10:09:11 -0400 Subject: [PATCH] history: one workoutAt(w), replacing the copies that had drifted "When did this workout happen" was written by hand ten times in eight files, each as w.start || new Date(w.d).getTime() or a close variant: lib/coach-insights.js, lib/effort.js, lib/muscles.js, lib/recovery.js, lib/strength-exercises.js (two copies), lib/coach-demo.js, views/Stats.jsx (two copies), and the MCP server's muscle_balance in mcp/src/tools.js new Date('2026-09-22') is UTC midnight, which any negative UTC offset drags back into the day before. Every other date helper in the app parses local noon precisely so no timezone or DST shift can move the day. And `||` discards a legitimate start of 0. lastWorkSetAt and the effort/recovery windows feed the retained-strength and fatigue decay off this, so imported history with no start was dated at UTC midnight instead of local noon: up to a day early west of UTC (17 h at UTC-5), and nothing reported it. muscleBalanceWindow and the Stats volume/last- trained numbers dated their own cutoffs the same way, moving a workout in or out of a window at the edge. One exported workoutAt in history.js now answers it, and every call site routes through it, the MCP server included (it already imports history.js), so there is one rule to get right. exercise-history.js already had this right in its own local startOf; it now shares the same function instead of keeping a correct copy of its own. entriesWithId in strength-exercises.js keeps -Infinity for the undated case on purpose, because isNewerOccurrence compares with !== then >, and a NaN pair would answer "not equal, not greater" instead of falling through to insertion order. frontend 9 new tests in workout-at.test.js, including a source-tree grep that fails if any file outside history.js, in frontend/src or mcp/src, hand-rolls the pattern again. 7 of the 9 fail against the old code (checked by stashing the fix and re-running); the grep test names all eight offending files. Co-Authored-By: Claude Opus 5.5 (1M context) --- frontend/src/lib/coach-demo.js | 4 +- frontend/src/lib/coach-insights.js | 5 +- frontend/src/lib/effort.js | 4 +- frontend/src/lib/exercise-history.js | 10 ++- frontend/src/lib/history.js | 15 ++++ frontend/src/lib/muscles.js | 3 +- frontend/src/lib/recovery.js | 12 ++-- frontend/src/lib/strength-exercises.js | 15 ++-- frontend/src/lib/workout-at.test.js | 96 ++++++++++++++++++++++++++ frontend/src/views/Stats.jsx | 6 +- mcp/src/tools.js | 4 +- 11 files changed, 139 insertions(+), 35 deletions(-) create mode 100644 frontend/src/lib/workout-at.test.js diff --git a/frontend/src/lib/coach-demo.js b/frontend/src/lib/coach-demo.js index 9d4fc8c12..6e5a740b5 100644 --- a/frontend/src/lib/coach-demo.js +++ b/frontend/src/lib/coach-demo.js @@ -13,7 +13,7 @@ // replaces at build time. import { EXIDX, EXDB } from './exercises.js' -import { modeOf, workoutVolume } from './history.js' +import { modeOf, workoutVolume, workoutAt } from './history.js' import { isWarmupRow } from './workout-model.js' import { best1RM } from './onerm.js' import { fmtNum } from './format.js' @@ -156,7 +156,7 @@ function buildDebrief(S, workoutId) { /** What "the room" would say on a busy instance — five people, plausible medians, your real bests. */ export function demoCohort(S) { const since = Date.now() - 56 * 864e5 - const you = Math.round((S.workouts || []).filter(w => (w.start || new Date(w.d).getTime()) > since).length / 8 * 10) / 10 + const you = Math.round((S.workouts || []).filter(w => workoutAt(w) > since).length / 8 * 10) / 10 const ids = [...new Set((S.routines || []).flatMap(r => (r.ex || []).map(e => e.id)))].slice(0, 5) const exercises = ids.map(id => { const b = best1RM(S, id) diff --git a/frontend/src/lib/coach-insights.js b/frontend/src/lib/coach-insights.js index db42187a2..cc57daedf 100644 --- a/frontend/src/lib/coach-insights.js +++ b/frontend/src/lib/coach-insights.js @@ -6,7 +6,7 @@ // window it names, which is why an old proposal in the thread still draws its chart months // later, against the data as it was then. import { EXIDX } from './exercises.js' -import { workoutVolume } from './history.js' +import { workoutVolume, workoutAt } from './history.js' import { isWarmupRow } from './workout-model.js' import { bestSetOf } from './onerm.js' @@ -16,7 +16,6 @@ const median = arr => { } const bpOf = (S, id) => EXIDX[id]?.bp || (S.customEx || []).find(c => c.id === id)?.bp || null const nameOf = (S, id) => EXIDX[id]?.n || (S.customEx || []).find(c => c.id === id)?.n || id -const tsOf = w => w.start || new Date(w.d + 'T12:00:00').getTime() /** Workouts inside an inclusive ISO-date window; either bound may be missing. */ export function windowWorkouts(S, { from, to } = {}) { @@ -61,7 +60,7 @@ export function insightsFor(S, win = {}, { topN = 3 } = {}) { const best = bestSetOf(en) if (!best) return if (!series.has(en.id)) series.set(en.id, []) - series.get(en.id).push({ t: tsOf(w), d: w.d, y: Math.round(best.est * 10) / 10 }) + series.get(en.id).push({ t: workoutAt(w), d: w.d, y: Math.round(best.est * 10) / 10 }) })) const strength = [...series.entries()] .filter(([, pts]) => pts.length >= 2) diff --git a/frontend/src/lib/effort.js b/frontend/src/lib/effort.js index 09b2982c3..e09ea7922 100644 --- a/frontend/src/lib/effort.js +++ b/frontend/src/lib/effort.js @@ -6,7 +6,7 @@ // half-empty series. So everything aggregates in RIR and is converted back for display. // RIR is the internal unit because it has a real zero — a set taken to failure — where RPE's // floor of 6 is only a convention about which sets are worth rating. RPE 8 == RIR 2. -import { EFFORT, effortOf } from './history.js' +import { EFFORT, effortOf, workoutAt } from './history.js' import { weekKey, weekStartOf, startOfWeek } from './format.js' import { isWarmupRow } from './workout-model.js' @@ -51,7 +51,7 @@ function eachDoneSet(S, fn) { // A window in days, counted back from now. 0 = everything, which is also what an empty // history means for every caller here. const inWindow = (w, days) => - !days || (w.start || new Date(w.d).getTime()) > Date.now() - days * 86400000 + !days || workoutAt(w) > Date.now() - days * 86400000 export const avgRir = sets => { const vs = (sets || []).map(rirOf).filter(v => v != null) diff --git a/frontend/src/lib/exercise-history.js b/frontend/src/lib/exercise-history.js index 0063d3fb2..40272fc4f 100644 --- a/frontend/src/lib/exercise-history.js +++ b/frontend/src/lib/exercise-history.js @@ -1,4 +1,4 @@ -import { metricEntriesForExercise, metricRowsForEntry, bestWeightForEntry, completedRepsOf, modeOf } from './history.js' +import { metricEntriesForExercise, metricRowsForEntry, bestWeightForEntry, completedRepsOf, modeOf, workoutAt } from './history.js' import { completedVolumeOf } from './workout-model.js' import { bestSetOf } from './onerm.js' import { beatsWeight } from './exercises.js' @@ -21,8 +21,6 @@ import { beatsWeight } from './exercises.js' export const HISTORY_SESSIONS = 10 -const startOf = w => (Number.isFinite(w.start) ? w.start : new Date(w.d + 'T12:00:00').getTime()) - // Volume of the exercise in one session: main set plus its drops/bursts, reps mode only — // there is no honest tonnage for a hold or a run. const entryVolume = rows => rows.reduce((v, s) => v + completedVolumeOf(s), 0) @@ -43,7 +41,7 @@ export function exerciseHistory(S, exId, { limit = HISTORY_SESSIONS } = {}) { const en = { ...sameMode.at(-1).entry, sets: rows } if (rows.length) logged.push({ w, en, mode, rows }) }) - logged.sort((a, b) => startOf(a.w) - startOf(b.w)) + logged.sort((a, b) => workoutAt(a.w) - workoutAt(b.w)) const empty = { mode: modeOf({ id: exId }), metric: 'weight', best: 0, prId: null, total: 0, sessions: [], points: [], e1rmPoints: [] } if (!logged.length) return empty @@ -64,7 +62,7 @@ export function exerciseHistory(S, exId, { limit = HISTORY_SESSIONS } = {}) { const same = m === mode const value = same ? valueOf({ en, rows }) : null const e1rm = m === 'reps' ? (bestSetOf(en)?.est ?? null) : null - const t = startOf(w) + const t = workoutAt(w) // "PR" goes on the session that first reached the all-time best, not on every session // that later matched it — one marker says where the record was set. if (value != null && value > best) { best = value; prId = w.id } @@ -120,7 +118,7 @@ export function bestSetFor(S, exId, mode = modeOf({ id: exId })) { for (const en of w.entries || []) { if (en.id !== exId) continue for (const set of metricRowsForEntry(en, mode)) { - t ??= startOf(w) + t ??= workoutAt(w) if (!best || better(set, best.set) || (!better(best.set, set) && t < best.t)) best = { d: w.d, set, target: en.target || null, t } } } diff --git a/frontend/src/lib/history.js b/frontend/src/lib/history.js index 7247ac061..39f634733 100644 --- a/frontend/src/lib/history.js +++ b/frontend/src/lib/history.js @@ -20,6 +20,21 @@ import { t } from './i18n-core.js' import { queueNext, pinState, queueLiveOn } from './queue.js' import { isPyramid, pyramidLabel, pyramidTargetAt, PYRAMID_MAX } from './pyramid.js' +// When a workout happened, as epoch ms: its own recorded start, else noon on its calendar day. +// Noon rather than midnight because `new Date('2026-09-22')` parses as UTC midnight, which any +// negative UTC offset drags back into the day before; noon survives every zone and DST shift. +// Several copies of this rule had drifted apart — some onto UTC midnight, some onto `||`, which +// throws away a legitimate start of 0 — and readers that feed the strength/recovery decay off it +// dated start-less history up to 14 h out, in a direction set by the reader's timezone. NaN when +// the workout carries neither a start nor a usable day. +export const workoutAt = w => { + if (Number.isFinite(w?.start)) return w.start + const day = w?.d + return typeof day === 'string' && /^\d{4}-\d{2}-\d{2}$/.test(day) + ? new Date(day + 'T12:00:00').getTime() + : NaN +} + // How an exercise is logged (issue #16). This used to be derived from the body part alone, // which meant a plank or a farmer's carry could only be timed by filing it under cardio. // A routine entry can now say so explicitly: diff --git a/frontend/src/lib/muscles.js b/frontend/src/lib/muscles.js index 6d4603d87..62240b0e7 100644 --- a/frontend/src/lib/muscles.js +++ b/frontend/src/lib/muscles.js @@ -11,6 +11,7 @@ import { isWarmupRow } from './workout-model.js' import { EXIDX, smOf } from './exercises.js' import { todayISO, weekKey, MONDAY } from './format.js' import { queueOf } from './queue.js' +import { workoutAt } from './history.js' // The muscles a map can shade, in head-to-toe order — also the order of any list // built from them, so "what am I neglecting" reads top-down like a body. @@ -286,7 +287,7 @@ export function muscleBalanceWindow(workouts, win, now = Date.now(), today = tod ? true : win === 7 ? weekKey(workout.d, ws) === weekKey(today, ws) - : (workout.start || new Date(workout.d).getTime()) > now - win * 86400000) + : workoutAt(workout) > now - win * 86400000) } /** Load a routine *would* produce, from its planned set counts. */ diff --git a/frontend/src/lib/recovery.js b/frontend/src/lib/recovery.js index 5412f42ca..3fa557f7f 100644 --- a/frontend/src/lib/recovery.js +++ b/frontend/src/lib/recovery.js @@ -1,4 +1,5 @@ import { EXIDX } from './exercises.js' +import { workoutAt } from './history.js' import { MUSCLES, musclesOf } from './muscles.js' import { isWarmupRow, dropsOf } from './workout-model.js' @@ -50,12 +51,11 @@ export function halfLifeDecay(ageMs, halfLifeMs) { return 0.5 ** (ageMs / halfLifeMs) } -// The v2 data contract has one timestamp per workout, not per set. Keep this fallback in one -// place so fatigue and strength use exactly the same stimulus time as effort.js. -function workoutTimestamp(workout) { - const timestamp = workout?.start || new Date(workout?.d).getTime() - return Number.isFinite(timestamp) ? timestamp : Number(timestamp) -} +// The v2 data contract has one timestamp per workout, not per set. history.js owns the rule +// (workoutAt) so fatigue, strength, effort.js and Stats all read the same stimulus time: a +// start of 0 is a real timestamp, and a date-only workout is local noon rather than UTC +// midnight, which is the previous day west of Greenwich. +const workoutTimestamp = workoutAt function emptyMuscleMap(value) { return Object.fromEntries(MUSCLES.map(slug => [slug, value])) diff --git a/frontend/src/lib/strength-exercises.js b/frontend/src/lib/strength-exercises.js index ecdeb8c39..104bb77dc 100644 --- a/frontend/src/lib/strength-exercises.js +++ b/frontend/src/lib/strength-exercises.js @@ -7,7 +7,7 @@ // catalogue-first (EXIDX), exactly like the fatigue/strength maps, falling back to the // logged snapshot (muscleWeights) for exercises no longer in the catalogue. import { best1RM } from './onerm.js' -import { entriesForExercise } from './history.js' +import { entriesForExercise, workoutAt } from './history.js' import { STRENGTH_FULL_MS, STRENGTH_HALF_LIFE_MS, STRENGTH_FLOOR, halfLifeDecay } from './recovery.js' import { musclesOf } from './muscles.js' import { EXIDX } from './exercises.js' @@ -28,7 +28,7 @@ function strengthFromAge(ageMs) { function lastWorkSetAt(S, id) { let latest = -Infinity for (const workout of S?.workouts || []) { - const ts = workout.start || new Date(workout.d).getTime() + const ts = workoutAt(workout) if (!Number.isFinite(ts) || ts <= latest) continue const entries = entriesForExercise(workout, id) if (entries.some(entry => (entry.sets || []).some(s => hasCompletedWork(s) && !isWarmupRow(s)))) latest = ts @@ -76,19 +76,14 @@ function workoutDay(workout) { return typeof day === 'string' && /^\d{4}-\d{2}-\d{2}$/.test(day) ? day : null } -function workoutTimestamp(workout) { - if (Number.isFinite(workout?.start)) return workout.start - const day = workoutDay(workout) - const timestamp = day ? new Date(day + 'T12:00:00').getTime() : NaN - return Number.isFinite(timestamp) ? timestamp : -Infinity -} - function entriesWithId(S, id) { let order = 0 return (S?.workouts || []).flatMap(workout => entriesForExercise(workout, id).map(entry => ({ entry, day: workoutDay(workout), - timestamp: workoutTimestamp(workout), + // -Infinity, not NaN: isNewerOccurrence compares these with `!==` then `>`, and a NaN pair + // would answer "not equal, not greater" instead of falling through to insertion order. + timestamp: Number.isFinite(workoutAt(workout)) ? workoutAt(workout) : -Infinity, order: order++, }))) } diff --git a/frontend/src/lib/workout-at.test.js b/frontend/src/lib/workout-at.test.js new file mode 100644 index 000000000..6f7dba828 --- /dev/null +++ b/frontend/src/lib/workout-at.test.js @@ -0,0 +1,96 @@ +import { describe, it, expect } from 'vitest' +import { workoutAt } from './history.js' +import { muscleBalanceWindow } from './muscles.js' + +// Several copies of "when did this workout happen" had drifted apart, and the two ways they were +// wrong are the two things worth pinning: the UTC-midnight parse, and `||` swallowing a start +// of 0. Everything else in the app dates a day at local noon so that no timezone or DST shift +// can move it, and this is the one function that decides it now. +describe('workoutAt', () => { + it('prefers the recorded start', () => { + expect(workoutAt({ start: 1_700_000_000_000, d: '2026-09-22' })).toBe(1_700_000_000_000) + }) + + it('keeps a start of 0 instead of falling through to the day', () => { + // Several of the copies wrote `w.start || ...`, so the first instant of 1970 read as + // "no start recorded" and silently became noon on the workout's calendar day. + expect(workoutAt({ start: 0, d: '2026-09-22' })).toBe(0) + }) + + it('dates a start-less workout at noon on its own calendar day', () => { + const at = new Date(workoutAt({ d: '2026-09-22' })) + expect([at.getFullYear(), at.getMonth() + 1, at.getDate()]).toEqual([2026, 9, 22]) + expect(at.getHours()).toBe(12) + }) + + it('does not land on the day before, the way a bare Date(iso) does west of UTC', () => { + const iso = '2026-09-22' + const utcMidnight = new Date(iso).getTime() + const ours = workoutAt({ d: iso }) + expect(new Date(ours).getDate()).toBe(22) + // Only west of UTC does the old parse actually cross the boundary, so assert the bug + // itself only where it bites. The noon rule above holds in every zone either way. + if (new Date(iso + 'T12:00:00').getTimezoneOffset() > 0) { + expect(new Date(utcMidnight).getDate()).toBe(21) + expect(ours).toBeGreaterThan(utcMidnight) + } + }) + + it('answers NaN when the workout carries neither a start nor a usable day', () => { + for (const w of [{}, null, undefined, { d: '' }, { d: 'yesterday' }, { d: '2026-9-2' }, { start: 'x' }]) { + expect(Number.isNaN(workoutAt(w))).toBe(true) + } + }) +}) + +describe('muscleBalanceWindow dates its cutoff the same way', () => { + it('keeps a workout logged today inside a 30-day window', () => { + // The UTC-midnight parse put a start-less workout up to 12 h earlier than it happened, so + // a workout right on the edge of the window fell out of it depending on the reader's zone. + const now = new Date('2026-09-22T12:00:00').getTime() + const kept = muscleBalanceWindow([{ d: '2026-09-22' }], 30, now) + expect(kept).toHaveLength(1) + }) + + it('still drops one that predates the window', () => { + const now = new Date('2026-09-22T12:00:00').getTime() + expect(muscleBalanceWindow([{ d: '2026-07-01' }], 30, now)).toHaveLength(0) + }) +}) + +/* Several call sites had their own copy of the rule: Stats.jsx twice, recovery.js, effort.js, + coach-demo.js, coach-insights.js, muscles.js, strength-exercises.js and the MCP server's + muscle_balance. A grep is the only guard that would have caught that, so here it is. */ +describe('nothing re-implements the rule', () => { + it('no source file dates a workout by hand', async () => { + const fs = await import('node:fs') + const path = await import('node:path') + const root = path.resolve(import.meta.dirname, '..') + const mcp = path.resolve(root, '../../mcp/src') // imports these helpers too + const walk = dir => fs.readdirSync(dir, { withFileTypes: true }).flatMap(e => { + const p = path.join(dir, e.name) + if (e.isDirectory()) return e.name === 'locales' ? [] : walk(p) + return /\.(js|jsx)$/.test(e.name) && !/\.test\.(js|jsx)$/.test(e.name) ? [p] : [] + }) + // `w.start || new Date(w.d)` and its spellings. history.js itself is where the rule lives. + const handRolled = /\bstart\s*(\|\||\?\?)\s*(Number\()?new Date\s*\(/ + const offenders = [...walk(root), ...walk(mcp)] + .filter(p => !p.endsWith(path.join('lib', 'history.js'))) + .filter(p => handRolled.test(fs.readFileSync(p, 'utf8'))) + .map(p => path.relative(root, p)) + expect(offenders).toEqual([]) + }) +}) + +/* What the sites were getting wrong in practice. `new Date('2026-09-22')` is UTC midnight, + which is 20:00 the previous day in New York, so a date-only workout logged today was already + four hours old to every window filter and could fall out of one early. */ +describe('a date-only workout against a window edge', () => { + it('stays inside a window that a UTC-midnight parse would push it out of', () => { + const iso = '2026-09-22' + // A window that opened at 06:00 local on the workout's own day. + const opened = new Date('2026-09-22T06:00:00').getTime() + expect(workoutAt({ d: iso })).toBeGreaterThan(opened) + expect(new Date(iso).getTime()).toBeLessThan(opened) // what the hand-rolled copies computed + }) +}) diff --git a/frontend/src/views/Stats.jsx b/frontend/src/views/Stats.jsx index 57ea85e53..fd6b29c61 100644 --- a/frontend/src/views/Stats.jsx +++ b/frontend/src/views/Stats.jsx @@ -2,7 +2,7 @@ import { useEffect, useMemo, useState } from 'react' import { useNavigate } from 'react-router-dom' import { useStore } from '../store/useStore.js' import { EXIDX, matchExercise, betterWeight } from '../lib/exercises.js' -import { lastBW, streakWeeks, setLabel, modeOf, effortOf, entriesForExercise, metricEntriesForExercise, metricModeForEntry, bestWeightForEntry, completedRepsOf, workoutDay } from '../lib/history.js' +import { lastBW, streakWeeks, setLabel, modeOf, effortOf, entriesForExercise, metricEntriesForExercise, metricModeForEntry, bestWeightForEntry, completedRepsOf, workoutDay, workoutAt } from '../lib/history.js' import { fmtNum, fmtDate, fmtVol, todayISO, isoOf, weekKey, weekStartOf, exerciseNameText } from '../lib/format.js' import { speedUnitOf, speedLabel, toSpeed } from '../lib/speed.js' import { t, exerciseNameFor, exerciseNameClass, getLang } from '../lib/i18n.js' @@ -30,7 +30,7 @@ import { isWarmupRow } from '../lib/workout-model.js' function latestMuscleTraining(workouts) { const latest = {} for (const workout of workouts || []) { - const timestamp = Number(workout?.start || new Date(workout?.d).getTime()) + const timestamp = workoutAt(workout) if (!Number.isFinite(timestamp)) continue for (const entry of workout.entries || []) { if (!(entry.sets || []).some(set => set?.done === true && !isWarmupRow(set))) continue @@ -147,7 +147,7 @@ function MuscleBalance({ S }) { const comparisonMuscles = MUSCLES .filter(muscle => (planned[muscle] || 0) > 0 || (load[muscle] || 0) > 0) .sort((a, b) => (load[b] || 0) - (load[a] || 0) || (planned[b] || 0) - (planned[a] || 0) || MUSCLES.indexOf(a) - MUSCLES.indexOf(b)) - const volWin = S.workouts.filter(w => (w.start || new Date(w.d).getTime()) > now - 90 * 86400000) + const volWin = S.workouts.filter(w => workoutAt(w) > now - 90 * 86400000) const vol90 = loadOfWorkouts(volWin, null) const { worked, missed } = rankOf(load) const { worked: strengthOrder } = rankOf(strength) diff --git a/mcp/src/tools.js b/mcp/src/tools.js index 6b014b03c..ee5853ddc 100644 --- a/mcp/src/tools.js +++ b/mcp/src/tools.js @@ -7,7 +7,7 @@ import { fmt, setLabel, exLine, muscleName, policyName, friendlyDuration, ratio, muscleOrder } from './labels.js' import { - modeOf, workoutVolume, setsDone, effectiveRoutine, effectiveRoutineIds, lastEntryFor + modeOf, workoutVolume, setsDone, effectiveRoutine, effectiveRoutineIds, lastEntryFor, workoutAt } from '../../frontend/src/lib/history.js' import { queueView, queueNext, pinState } from '../../frontend/src/lib/queue.js' import { exOr } from '../../frontend/src/lib/exercises.js' @@ -466,7 +466,7 @@ export const muscleBalance = { const cutoff = period === 'week' ? midnightDaysBack(6) : period === 'month' ? midnightDaysBack(29) : Number.NEGATIVE_INFINITY - const workouts = (S.workouts || []).filter(w => (w.start || new Date(w.d + 'T12:00:00').getTime()) >= cutoff) + const workouts = (S.workouts || []).filter(w => workoutAt(w) >= cutoff) // loadOf() resolves each entry through EXIDX, which holds the catalogue only, so a // custom exercise's sets score zero here. Attaching the custom itself lets loadOf's own // `historical` branch resolve it. Only a *found* custom: exOr's miss placeholder carries