From 508f1fb8014c5778eb3fb573641756b4b874e287 Mon Sep 17 00:00:00 2001 From: Boris 'B' Kurktchiev Date: Tue, 29 Sep 2026 10:06:36 -0400 Subject: [PATCH] fix(coach): the consent screen names session notes, and they arrive whole The Coach's payload carries each recent session's note (cleanWorkout in api/coach/core/payload.js), but the consent screen's "Your logged training" line listed weights, reps, times, effort ratings and session lengths and never mentioned notes. It now ends "and your session notes", and the AI_COACH.md table says the same. CONSENT_VERSION goes from 1 to 2 with it, so everyone who agreed to the old wording is asked again the next time they open the Coach: the chat sends them to the intake while hasConsent() is false. Before, that redirect replayed all seven intake questions and ended in a new plan request. An athlete who already has a profile now sees only the consent step and goes back to the Coach with their answers untouched. Someone who never agreed still gets the full intake. The note itself was cut at 300 characters while the app lets it run to 500 (NOTE_MAX in frontend/src/lib/history.js). payload.js has its own NOTE_MAX = 500 now, and payload-parity.test.js pins it to the app's. Tests: CoachIntake.test.jsx gains two (35), payload.test.js one (28), payload-parity.test.js one (4); each fails against the old code. The three CoachChat test fixtures that hard-coded consent version 1 read CONSENT_VERSION instead. Co-Authored-By: Claude Opus 5.5 (1M context) --- api/coach/core/categories.js | 2 +- api/coach/core/payload.js | 7 +++++- api/test/payload-parity.test.js | 7 ++++++ api/test/payload.test.js | 18 +++++++++++++++ docs/AI_COACH.md | 2 +- frontend/src/lib/coach.js | 6 +++-- .../src/views/CoachChat.demo-failure.test.jsx | 3 ++- .../src/views/CoachChat.fast-failure.test.jsx | 3 ++- frontend/src/views/CoachChat.test.jsx | 3 ++- frontend/src/views/CoachIntake.jsx | 9 ++++++-- frontend/src/views/CoachIntake.test.jsx | 22 +++++++++++++++++++ 11 files changed, 72 insertions(+), 10 deletions(-) diff --git a/api/coach/core/categories.js b/api/coach/core/categories.js index c7600697a7..004c72ea35 100644 --- a/api/coach/core/categories.js +++ b/api/coach/core/categories.js @@ -6,7 +6,7 @@ * builds from it, so the screen cannot drift from the payload. */ export const DATA_CATEGORIES = Object.freeze([ 'plan', // routines, exercises, sets/reps, schedule, progression settings - 'training', // logged sets, targets, effort ratings, durations, PRs in the review window + 'training', // logged sets, targets, effort ratings, durations, PRs, session notes in the review window 'bodyweight', // weigh-ins in the window and your goal weight 'profile', // the intake answers you gave the Coach, including any limitations 'prefs' // unit, language, effort scale diff --git a/api/coach/core/payload.js b/api/coach/core/payload.js index 3830a7cadd..cc6db8e1d9 100644 --- a/api/coach/core/payload.js +++ b/api/coach/core/payload.js @@ -18,6 +18,11 @@ export const CONTRACT = 1; // makes the payload bigger and the reading vaguer, not better. export const MAX_WEEKS = 12; export const MAX_SESSIONS = 60; +// The most a session note can be: the app's own cap (frontend/src/lib/history.js NOTE_MAX, pinned +// by api/test/payload-parity.test.js), so a note the athlete wrote arrives whole and the bound only +// holds for a document edited outside the app. It was 300 here, and a 500-character note reached +// the model cut off mid-sentence. +export const NOTE_MAX = 500; // Last-resort ceiling for a free-text note/refine (issue #267). The real limit is the admin's // `maxMessageLen`, enforced in jobs.js before a message ever reaches this module — this module // stays a pure allowlist with no config import of its own, so it keeps its own constant instead. @@ -389,7 +394,7 @@ function cleanWorkout(w) { name: word(w.name, NAME_MAX), minutes: w.end && w.start ? Math.round((w.end - w.start) / 60000) : null, ...(w.rating ? { rating: typeof w.rating === 'number' ? w.rating : text(String(w.rating), 20) } : {}), - ...(w.note ? { note: String(w.note).slice(0, 300) } : {}), + ...(w.note ? { note: String(w.note).slice(0, NOTE_MAX) } : {}), prs: list(w.prs).length, entries: entriesOf(w).map(en => ({ id: ident(en.id), diff --git a/api/test/payload-parity.test.js b/api/test/payload-parity.test.js index 572429f25f..5658d59ee4 100644 --- a/api/test/payload-parity.test.js +++ b/api/test/payload-parity.test.js @@ -14,11 +14,18 @@ import assert from 'node:assert/strict'; import { tempData, sampleState } from './helpers.mjs'; import { isWarmupRow } from '../../frontend/src/lib/workout-model.js'; import { readSession as frontendReadSession } from '../../frontend/src/lib/progression.js'; +import { NOTE_MAX as frontendNoteMax } from '../../frontend/src/lib/history.js'; tempData(); const payload = await import('../coach/core/payload.js'); const { handleFor } = await import('../coach/handle.js'); +/* The payload bounds a session note at the app's own cap, so a note the app let the athlete write + is never cut on its way to the model. */ +test('the payload\'s NOTE_MAX is the app\'s NOTE_MAX', () => { + assert.equal(payload.NOTE_MAX, frontendNoteMax); +}); + test('isWarmupSet agrees with the frontend isWarmupRow', () => { const cases = [ {}, null, undefined, diff --git a/api/test/payload.test.js b/api/test/payload.test.js index babd6832fd..e71ac41fc8 100644 --- a/api/test/payload.test.js +++ b/api/test/payload.test.js @@ -375,3 +375,21 @@ test('a workout\'s photos and videos never reach the payload', () => { } } }); + +/* The session note is the athlete's own explanation of the numbers, and the app lets it run to + NOTE_MAX (500) characters. It was cut at 300 here, so a note explaining how a dumbbell weight + had been counted reached the model mid-sentence. */ +test('a session note reaches the model whole, bounded only at the app\'s own cap', () => { + const S = sampleState(); + const whole = 'It is confusing whether the weight is total or per dumbbell. '.repeat(8).trim(); // 487 chars: past the old 300, within the cap + const mk = (d, note) => ({ + id: 'w' + d, d, name: 'A', start: 0, end: 60000, prs: [], note, + entries: [{ id: '0001', target: { sets: 3, reps: 10, weight: 20 }, sets: [{ w: 20, r: 10, done: true }] }] + }); + S.workouts = [mk('2026-09-02', whole), mk('2026-09-03', 'y'.repeat(payload.NOTE_MAX + 100))]; // only a document edited outside the app carries the second + const p = payload.build(S, { handle: handleFor('u1'), kind: 'review' }); + const [full, edited] = p.window.workouts; + assert.equal(full.compact, undefined, 'both sessions are in full detail'); + assert.equal(full.note, whole, 'the session note arrives whole (was cut at 300)'); + assert.equal(edited.note.length, payload.NOTE_MAX, 'a note no app could have written is bounded at the cap'); +}); diff --git a/docs/AI_COACH.md b/docs/AI_COACH.md index fed0ccbc32..442210f330 100644 --- a/docs/AI_COACH.md +++ b/docs/AI_COACH.md @@ -236,7 +236,7 @@ renders from, so the screen cannot drift from the payload — are: | Category | What it covers | | --- | --- | | `plan` | routines, exercises, sets/reps, schedule, progression settings | -| `training` | logged sets, targets, effort ratings, durations, PRs in the review window | +| `training` | logged sets, targets, effort ratings, durations, PRs and session notes in the review window | | `bodyweight` | weigh-ins in the window and your goal weight | | `profile` | the intake answers you gave the Coach, including any limitations | | `cohort` (optional) | only with comparison on and your own opt-in: anonymous medians from the other people sharing — never their data, and never yours to them beyond the same medians | diff --git a/frontend/src/lib/coach.js b/frontend/src/lib/coach.js index 5b13ab95d6..8a1ccb8f0b 100644 --- a/frontend/src/lib/coach.js +++ b/frontend/src/lib/coach.js @@ -21,7 +21,9 @@ import { POLICIES } from './progression.js' import { t } from './i18n.js' // Bumping this re-prompts everyone: it means what we share, or who we share it with, changed. -export const CONSENT_VERSION = 1 +// 2: the training line names the session notes, which the payload carries and version 1 never +// mentioned. +export const CONSENT_VERSION = 2 // Bounds. The whole state has to stay inside the server's 5 MB body limit, and a Coach log // that grows forever is exactly the kind of thing that eats it invisibly. Worst case here is @@ -64,7 +66,7 @@ export const coachAvailable = (config, user, { demo, mobile, coachMode } = {}) = // builder uses (api/coach/core/categories.js), so the screen cannot promise less than leaves. export const CATEGORY_TEXT = { plan: ['Your plan', 'Routines, exercises, sets and reps, your weekly schedule and progression settings.'], - training: ['Your logged training', 'Sets you logged in the review window — weights, reps, times, effort ratings and how long sessions took.'], + training: ['Your logged training', 'Sets you logged in the review window — weights, reps, times, effort ratings, how long sessions took, and your session notes.'], bodyweight: ['Body weight', 'Weigh-ins from the same window, and your goal weight if you set one.'], profile: ['What you tell the Coach', 'Your intake answers, including any limitations or injuries you describe.'], prefs: ['A few preferences', 'Your unit, your language and which effort scale you log.'] diff --git a/frontend/src/views/CoachChat.demo-failure.test.jsx b/frontend/src/views/CoachChat.demo-failure.test.jsx index 02ca5121db..4a18616472 100644 --- a/frontend/src/views/CoachChat.demo-failure.test.jsx +++ b/frontend/src/views/CoachChat.demo-failure.test.jsx @@ -3,6 +3,7 @@ import { createRoot } from 'react-dom/client' import { parseHTML } from 'linkedom' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import CoachChat from './CoachChat.jsx' +import { CONSENT_VERSION } from '../lib/coach.js' // the consent a fixture grants is the current one, whatever its number // The demo Coach's failure state, end to end. Every other CoachChat test mocks lib/coach-api.js; // this one runs the real thing with DEMO forced on, so the chain under test is exactly what the @@ -74,7 +75,7 @@ const state = () => ({ unit: 'kg', lang: 'en', customEx: [], workouts: [brokenWorkout()], bodyweight: [], exWeights: {}, dayPlan: {}, routines: [], week: {}, coach: { - consent: { agreedAt: '2026-07-01T00:00:00Z', version: 1 }, + consent: { agreedAt: '2026-07-01T00:00:00Z', version: CONSENT_VERSION }, profile: { goal: 'muscle', experience: 'new', daysPerWeek: 3, sessionMin: 60, preferredDays: [1, 3, 5], equipment: [] }, log: [], snapshots: [], chat: [{ id: 'c1', role: 'user', kind: 'intake', at: 1 }], timings: [] }, diff --git a/frontend/src/views/CoachChat.fast-failure.test.jsx b/frontend/src/views/CoachChat.fast-failure.test.jsx index 49e07869f2..8838dcb1e8 100644 --- a/frontend/src/views/CoachChat.fast-failure.test.jsx +++ b/frontend/src/views/CoachChat.fast-failure.test.jsx @@ -3,6 +3,7 @@ import React, { act } from 'react' import { createRoot } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import CoachChat from './CoachChat.jsx' +import { CONSENT_VERSION } from '../lib/coach.js' // the consent a fixture grants is the current one, whatever its number import { requestPlan, settleAwaited } from '../lib/coach-api.js' // A run that fails before the first status poll. A provider that refuses the connection fails @@ -81,7 +82,7 @@ const state = routines => ({ unit: 'kg', lang: 'en', customEx: [], workouts: [{ id: 'w1', name: 'Push Day', d: '2026-09-01', entries: [] }], bodyweight: [], exWeights: {}, dayPlan: {}, routines, week: {}, coach: { - consent: { agreedAt: '2026-07-01T00:00:00Z', version: 1 }, + consent: { agreedAt: '2026-07-01T00:00:00Z', version: CONSENT_VERSION }, profile: { goal: 'muscle', experience: 'new', daysPerWeek: 3, sessionMin: 60, preferredDays: [1, 3, 5], equipment: [] }, log: [], snapshots: [], chat: [{ id: 'c1', role: 'user', kind: 'intake', at: 1 }], timings: [] }, diff --git a/frontend/src/views/CoachChat.test.jsx b/frontend/src/views/CoachChat.test.jsx index 0a19d47431..12560025bb 100644 --- a/frontend/src/views/CoachChat.test.jsx +++ b/frontend/src/views/CoachChat.test.jsx @@ -3,6 +3,7 @@ import { createRoot } from 'react-dom/client' import { parseHTML } from 'linkedom' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import CoachChat from './CoachChat.jsx' +import { CONSENT_VERSION } from '../lib/coach.js' // the consent a fixture grants is the current one, whatever its number import { effectiveRoutine } from '../lib/history.js' import { todayISO } from '../lib/format.js' @@ -70,7 +71,7 @@ const state = () => ({ unit: 'kg', lang: 'en', customEx: [], workouts: [], bodyweight: [], exWeights: {}, dayPlan: {}, routines: [], week: {}, coach: { - consent: { agreedAt: '2026-07-01T00:00:00Z', version: 1 }, + consent: { agreedAt: '2026-07-01T00:00:00Z', version: CONSENT_VERSION }, profile: { goal: 'muscle', experience: 'new', daysPerWeek: 3, sessionMin: 60, preferredDays: [1, 3, 5], equipment: [] }, log: [], snapshots: [], chat: [{ id: 'c1', role: 'user', kind: 'intake', at: 1 }], timings: [] }, diff --git a/frontend/src/views/CoachIntake.jsx b/frontend/src/views/CoachIntake.jsx index a5f8597527..39ba2e4093 100644 --- a/frontend/src/views/CoachIntake.jsx +++ b/frontend/src/views/CoachIntake.jsx @@ -59,7 +59,11 @@ export default function CoachIntake() { const update = useStore(s => s.update) const toast = useUI(s => s.toast) const [needConsent] = useState(() => !editing && !hasConsent(S)) - const STEPS = [...(needConsent ? ['consent'] : []), 'goal', 'experience', 'days', 'length', 'equipment', 'limits', 'extras'] + // An athlete who already answered the intake and only lacks the CURRENT consent (the wording + // changed and CONSENT_VERSION moved) reads the new terms and goes back to the Coach. Without + // this the redirect from the chat replayed all seven questions and ended in a new plan request. + const [reconsentOnly] = useState(() => !editing && !hasConsent(S) && !!S.coach?.profile) + const STEPS = reconsentOnly ? ['consent'] : [...(needConsent ? ['consent'] : []), 'goal', 'experience', 'days', 'length', 'equipment', 'limits', 'extras'] const [step, setStep] = useState(0) const [busy, setBusy] = useState(false) const [p, setP] = useState(() => ({ @@ -81,7 +85,8 @@ export default function CoachIntake() { const agree = () => { update(s => { s.coach = { ...(s.coach || emptyCoach()), consent: { agreedAt: new Date().toISOString(), version: CONSENT_VERSION } } }) - setStep(step + 1) + if (reconsentOnly) nav('/coach', { replace: true }) + else setStep(step + 1) } const finish = async () => { diff --git a/frontend/src/views/CoachIntake.test.jsx b/frontend/src/views/CoachIntake.test.jsx index 9cdf5aefa6..ed9e1815af 100644 --- a/frontend/src/views/CoachIntake.test.jsx +++ b/frontend/src/views/CoachIntake.test.jsx @@ -147,6 +147,28 @@ describe('CoachIntake — the consent screen', () => { expect(mocks.nav).toHaveBeenCalledWith('/plan') }) + it('a returning athlete whose consent is out of date reads the new terms only, then goes back to the Coach', () => { + // CONSENT_VERSION moved (the training line now names the session notes). The chat's redirect + // used to replay all seven questions and end in a new plan request. + mocks.S.coach = { consent: { agreedAt: '2026-09-01T00:00:00Z', version: CONSENT_VERSION - 1 }, profile: { goal: 'strength', experience: 'intermediate', daysPerWeek: 3 }, chat: [] } + mount() + expect(eyebrow()).toBe('Before we start') + expect(all('.ob-dot')).toHaveLength(0) + tap('button', 'I understand') + expect(mocks.S.coach.consent.version).toBe(CONSENT_VERSION) + expect(mocks.S.coach.profile.goal).toBe('strength') // the answers stay as they were + expect(mocks.nav).toHaveBeenCalledWith('/coach', { replace: true }) + expect(vi.mocked(requestPlan)).not.toHaveBeenCalled() + }) + + it('the training line names the session notes, which the Coach reads', () => { + // api/coach/core/payload.js sends each recent session's note (cleanWorkout). The consent + // screen listed weights, reps, times, effort and durations, and never said so. + mount() + const training = all('.ob-consent-row').find(r => r.querySelector('b').textContent === 'Your logged training') + expect(training.textContent).toContain('your session notes') + }) + it('lists the built-in categories first, then whatever the server says actually goes', async () => { vi.mocked(disclosure).mockResolvedValue({ categories: ['plan', 'prefs'], providerLabel: 'Anthropic', payer: 'you', host: 'api.anthropic.com' }) mount()