Repository navigation
fix(coach): CoachSetup's Save, the demo Coach's failure state, and the intake line - #289
Merged
Merged
Conversation
…e intake line
Six defects across the in-app Coach's setup, intake and demo, found by writing
the first tests that render those screens (100 tests across three new files
plus one end-to-end file). Each has a test that fails against the old code.
CoachSetup's Save accepted an endpoint that "List models" had refused.
prepare() checked validateBaseUrl(...).ok; save() took .value off an unchecked
result, so a typo saved baseUrl: null, said "The Coach is on", and pointed the
phone at nothing. Save runs the same check now. And an EMPTY endpoint is
refused for a provider that has no default: validateBaseUrl('') is
{ ok, value: null } by contract, because for anthropic, openai and gemini empty
means "back to the default", but compatible has no default, so both List and
Save were letting you configure the phone to call nowhere. The admin route had
the same hole one layer down and refuses it too, with a test.
The demo Coach had no failure state. A builder that threw inside the 2.2 s
timer left the job 'running' for ever and every later request answered 409
until reload; the one reachable trigger was a workout with no `entries`, which
buildDebrief guarded twice and then handed to workoutVolume unguarded. start()
catches now and demoStatus() reports lastError in the server's shape, so
CoachChat's existing job-ended effect writes the same error line it writes for
the real Coach. The end-to-end test mounts the real CoachChat against a
throwing workout and reads that line off the thread.
Three more in the demo. demoReview ran the whole wait and answered nothing on
a profile with no two-exercise routine; it refuses up front now, the way
demoDebrief already did with no workout. buildReview picked
`reps[1] || routine.ex[1]`, which is the same entry as `first` when the only
rep-mode exercise sits at index 1, and applying both changes then aborted the
whole atomic change-set with "missing target". demoRefine built from nothing
and sent every revised plan back to Mon/Wed/Fri; it reads the saved intake
now, as the server's payload does.
CoachIntake wrote a second intake line into the thread whenever a first-timer
retried after a failed plan request, because the one-intake-line rule was
guarded by `editing`. And a daysPerWeek of 0 saved as 3 where -1 saved as 1,
because `|| 3` ate the zero before the clamp; `?? 3` lets the clamp turn it
into 1 like every other count below the floor.
Two new strings in all 14 packs.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
DuarteSantos8
added a commit
that referenced
this pull request
Sep 28, 2026
… that the phone refuses http:// ones The #289 flow tests used http://ollama.lan, which the platform batch's cleartext refusal turns down before anything is listed or saved; the placeholder is https://ollama.example.com there too.
Owner
|
Thanks @kurktchiev, all six fixes are in, and the API reference now documents the refused empty endpoint. Released in v1.3.9: https://github.com/DuarteSantos8/openGym/releases/tag/v1.3.9 |
This was referenced Sep 29, 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.
Six defects across the in-app Coach's setup, intake and demo, found by writing
the first tests that render those screens (100 tests across three new files
plus one end-to-end file). Each has a test that fails against the old code.
CoachSetup's Save accepted an endpoint that "List models" had refused.
prepare() checked validateBaseUrl(...).ok; save() took .value off an unchecked
result, so a typo saved baseUrl: null, said "The Coach is on", and pointed the
phone at nothing. Save runs the same check now. And an EMPTY endpoint is
refused for a provider that has no default: validateBaseUrl('') is
{ ok, value: null } by contract, because for anthropic, openai and gemini empty
means "back to the default", but compatible has no default, so both List and
Save were letting you configure the phone to call nowhere. The admin route had
the same hole one layer down and refuses it too, with a test.
The demo Coach had no failure state. A builder that threw inside the 2.2 s
timer left the job 'running' for ever and every later request answered 409
until reload; the one reachable trigger was a workout with no
entries, whichbuildDebrief guarded twice and then handed to workoutVolume unguarded. start()
catches now and demoStatus() reports lastError in the server's shape, so
CoachChat's existing job-ended effect writes the same error line it writes for
the real Coach. The end-to-end test mounts the real CoachChat against a
throwing workout and reads that line off the thread.
Three more in the demo. demoReview ran the whole wait and answered nothing on
a profile with no two-exercise routine; it refuses up front now, the way
demoDebrief already did with no workout. buildReview picked
reps[1] || routine.ex[1], which is the same entry asfirstwhen the onlyrep-mode exercise sits at index 1, and applying both changes then aborted the
whole atomic change-set with "missing target". demoRefine built from nothing
and sent every revised plan back to Mon/Wed/Fri; it reads the saved intake
now, as the server's payload does.
CoachIntake wrote a second intake line into the thread whenever a first-timer
retried after a failed plan request, because the one-intake-line rule was
guarded by
editing. And a daysPerWeek of 0 saved as 3 where -1 saved as 1,because
|| 3ate the zero before the clamp;?? 3lets the clamp turn itinto 1 like every other count below the floor.
Two new strings in all 14 packs.
🤖 Generated with Claude Code