Repository navigation
fix(coach): batch 2 — post-review P2 perf + test - #28
Merged
Merged
Conversation
…son open The first milestone access runs solveCube on the fixed scramble (~420ms) on the main thread, freezing the UI on the first lesson's first demo. Export warmMilestones and call it from the Coach route via requestIdleCallback (with a setTimeout fallback), so the solve happens while the learner reads the first screen and the demo opens against a warm cache. Keeps the real-solve doctrine — no hand-built hardcoded state — so the milestones stay engine-derived. Refs #17
The last step of a chapter renders either a 'Chapitre suivant' link to the next chapter in journey order or a 'Retour au Coach' link on the final chapter — neither branch was exercised. Add a test for each: an intermediate chapter links to the next lesson id, and the final chapter links back to /coach. Refs #18
There was a problem hiding this comment.
Pull request overview
This PR addresses two remaining Coach v1 post-review P2 findings by (1) moving the expensive first-time milestone computation off the “open first demo” click path and (2) adding missing tests for the final-step navigation fork in LessonPlayer.
Changes:
- Prewarms the solver-derived milestone cache from the Coach route using
requestIdleCallback(with asetTimeoutfallback) to avoid a first-demo UI freeze. - Exposes a small
warmMilestones()helper and adds a unit test to ensure it’s safe/idempotent. - Adds tests covering both “next chapter” and “back to /coach” branches on the last step of a lesson.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| apps/web/src/pages/Coach.tsx | Schedules idle-time milestone cache warming on Coach route mount to avoid first-demo click freezes. |
| apps/web/src/features/coach/index.ts | Re-exports warmMilestones from the coach feature entrypoint. |
| apps/web/src/features/coach/data/illustrative.ts | Adds warmMilestones() wrapper to eagerly populate the memoized milestones cache. |
| apps/web/src/features/coach/data/illustrative.spec.ts | Adds coverage ensuring warmMilestones is safe/idempotent and milestones remain available. |
| apps/web/src/features/coach/components/LessonPlayer.spec.tsx | Adds tests for last-step navigation: intermediate chapter links to the next; final chapter links back to /coach. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
What
Batch 2 of the Coach v1 post-review issues — the two remaining P2 findings from the
ce-code-reviewon PR #11. Stacked conceptually on batch 1 (PR #27, already merged todevelop).solveCubeon the fixed scramble (~420ms) on the main thread, freezing the first lesson's first demo.warmMilestonesis now prewarmed from the Coach route viarequestIdleCallback(setTimeout fallback) so the solve happens during idle while the learner reads the first screen.Chapitre suivantvsRetour au Coach) had no test. Added one per branch: an intermediate chapter links to the next lesson in journey order; the final chapter links back to/coach.Design note — #17
Chose the idle prewarm over hardcoding the white-cross state. Hardcoding would eliminate
solveCubeentirely but contradicts the module's doctrine — every milestone comes from a real solve, no hand-built state. Prewarming keeps milestones engine-derived and only moves the cost off the click path. (The latentfindIndex-1 edge noted in #18 is unreachable: an unknownlessonIdrendersLessonNotFoundbefore that lookup, so this is test-only as filed.)Verification
format:check,lint:check,typecheck,test(299 pass),build— all green locally.Closes #17
Closes #18