Make the buddy the tutor along the onboarding path

[DavidLeuter]

Summary

The onboarding page and the buddy now point at the same things: buddy links land on the right step, question or phase, items carry the numbers the buddy uses, every step, question and phase can open the buddy, and the page refreshes after a confirmed buddy action. The board's second onboarding (path card, "Build my path", card blueprints) is gone.

Refs SprintStartProject/Wiki#311

Changes

Onboarding page

• Buddy links /onboarding?step=…, ?question=…, ?phase=… open the phase, scroll to the item and highlight it briefly. They don't unfold or start anything. The highlight respects reduced motion.
• #n numbers in the list and on graph nodes (itemNumbers), the same numbers the buddy uses.
• "Ask your buddy" on steps, questions (with a wrong-answer variant), phase headers and the empty-generation state (buddyDrafts).
• A step opened via /onboarding/<id> isn't started while a skip request on it is pending.

Buddy

• Confirm payloads and proposal types for the path actions (complete_step, complete_task, answer_question, add_path_step, request_skip). The confirm shows the skip reason and the message a flag sends to the PM.
• After a confirmed path action, announceBuddyPathChanged refreshes the onboarding page, the open step and the board's path strip. useBuddyPathSync invalidates the cached statuses and the board.
• In-app links in buddy replies navigate in place instead of opening a new tab.
• A draft put into the composer from outside (a chip, "Ask your buddy") takes the focus, so Enter sends it. Same fix in ChatComposer.

Step origin

• StepOriginBadge reads the new origin: "Added with your buddy", "You added this", "Custom step by PM". Reviewers see "the hire" / "the buddy" instead of "you".

Retired from the board

• The PATH_TO_FIRST_CONTRIBUTION card, BoardPathNotes and momentLabels.
• "Build my path" and its generator (pathToCards, useGeneratedPathCards), the local-storage card blueprints and the "team" filter.
• chosen on the current-task card, since nothing hands out a Task 0 any more.

Wording

• "Onboarding metrics" reads "Contribution metrics" (texts only; routes and file names unchanged). The Task 0 toggle says it's a label, not an assignment.

Testing

• npm run lint, npm run format:check, npm run build: green
• npm run test: 2972 passed

Notes

• Needs the backend PR for the new buddy actions, origin and the moved Task 0 endpoint. Merge together.
• Replaces the draft Feature/311 buddy onboarding tutor #265.

Related PRs

• Backend: Make the buddy the tutor along the onboarding path sprintstart-backend#261
• AI: Make the buddy the tutor along the onboarding path sprintstart-ai#208

🤖 Generated with Claude Code

[daniilperkin]

PR Review — Make the buddy the tutor along the onboarding path (#267)

Head faac83d1 → dev (up to date with dev, no conflicts). Gates: npm run try green — format:check, build, lint clean; unit 2909/2909, a11y 63/63 (= the 2972 in the description). Every changed file matches Prettier as committed.
Cross-checked against backend #261: numbering (steps by position, then questions by position), the ?step=/?question=/?phase= link constants, the BuddyActionType tool names vs BUDDY_PATH_ACTIONS, answer_question returning ok: true on a wrong answer, and migration V19 deleting the PATH_TO_FIRST_CONTRIBUTION board cards. All consistent.

🔴 Blocking

None.

🟠 Should fix before merge

1. A buddy link opened from graph view never scrolls to the item. The link arrival switches the view to list (OnBoardingPage.tsx:405) and sets linkHighlight in the same batch. But the list lives inside SlidingTabPanel (AnimatePresence mode="wait"), so it only mounts after the graph's exit animation. The scroll effect (OnBoardingPage.tsx:475-480) runs on that commit, before the card exists. getElementById returns null, and nothing in the effect's deps changes afterwards to retry. The same happens on a cold load of /onboarding?step=… for a hire whose saved view is the graph. The page already has the fix for this exact case: scrollToChooser waits SLIDING_PANEL_EXIT_MS. Tests can't catch it because framer-motion is mocked globally.

2. linkHighlight is never cleared, which causes three side effects:

   • The scroll effect depends on selectedPhaseId. Leaving the linked item's phase and coming back scrolls to that item again, long after the link.
   • The <li> keeps app-link-highlight (PhaseItemList.tsx:96), so the pulse replays every time the list remounts (phase switch, graph→list).
   • The animation uses fill-mode both (index.css:968, :984), so its last frame (box-shadow: 0 0 0 0 transparent) stays applied and beats the class styles. When the hire expands the linked item, it never gets its shadow-lg.
     Fix: clear the highlight on animationend (or on a timer), or drop both and use backwards so it doesn't stick after it ends.

3. Confirming any buddy path action wipes unsaved text in the open step. StepWorkspace re-reads the step on every onBuddyPathChanged (StepWorkspace.tsx:104, :151). The re-read calls setSkipReason(detail.skip?.reason ?? "") and setComment(detail.feedback.comment ?? "") (:138, :141). So a half-typed skip reason or feedback comment is replaced by the server value, or "", when the hire confirms something unrelated in the dock (e.g. "answer_question" in another phase). Either refresh only tasks and status on a buddy change, or leave the draft fields alone when their value differs from the last server value. It also fetches twice: once from buddyChanges, once more when the page's refreshPath changes stepStatus.

🟡 Minor

4. A link to the item that's already open collapses it. The link arrival always calls setExpandedItemId(null) (OnBoardingPage.tsx:444, :416 for phases). The hire is most likely to click "you're on [#3]" while #3 is open, and that throws away a typed answer or skip reason. The page protects against exactly this elsewhere (swipe is disabled while an item is expanded). Skip the collapse when expandedItemId === arrival.id.
5. The skip-pending guard applies to every way of opening a step, not just /onboarding/<id> (OnBoardingPage.tsx:543). The list's primary button still says Start (nodeLabels.ts primaryActionLabel), so for a step with a pending skip, clicking it opens the step without starting it, and the hire has to press "Start step" again inside. Either word the button differently for that state, or limit the guard to link and route arrivals. The PR description only mentions /onboarding/<id>.
6. Following a link while a graph item is open leaves graphItemId set. Swipe stays disabled (enabled needs graphItemId === null), and switching back to the graph reopens the old item.
7. isInAppPath accepts /\evil.example (BuddyMarkdown.tsx:16). Browsers read a backslash as a slash, so this is protocol-relative. A normal click only throws in pushState, but opening it in a new tab (middle-click or context menu) goes off-site. The comment promises the model output is handled carefully; also reject \ in the second position. External links already opened in a new tab before this PR, so the exposure is small.
8. The empty-generation state offers the buddy only for the first issue: askAboutEmptyPhase(generationIssues[0].title) (OnBoardingPage.tsx:861). With several empty phases, the draft names one of them.

🔵 Nits (non-blocking)

• List numbers can appear out of order. The list is in graph order (orderedPhaseItems) but the numbers follow position order, so it can read #1, #4, #2. This is deliberate and documented, but a hire may take it for a bug.
• Wrong icon on "You added this". StepOriginBadge.tsx:44 uses Sparkles, which the app uses for AI. That's the wrong glyph for a step the hire wrote; PenLine or UserRound would fit.
• Unneeded fallback. (phase.questions ?? []) at OnBoardingPage.tsx:1130 guards a field the type marks required, and itemNumbers / phaseItems read it without the guard. Pick one approach; getUserOnboardingPath already normalises it at the service boundary.
• Stale comment. boardGroups.ts:82 still says "What the generator calls the area…" — the generator is gone. The unsplitTeamArea migration is still worth keeping; only the tense is off.
• Old localStorage data. Card blueprints saved by cardBlueprintService stay in users' browsers forever. Harmless, but a one-line cleanup is cheap.
• Focus-grabbing effect in ChatComposer. The new effect focuses the box whenever value changes without typing. That includes a restored draft after a session switch, and on touch devices that pops the keyboard. Worth a quick check.

✅ Checked and sound

• Deleting the old board path is complete. No references remain to PathToFirstContributionCard, BoardPathNotes, momentLabels, pathToCards, useGeneratedPathCards, card-blueprints, isFromTeam or applyPlan. readableTitle moved to layout/cardNames.ts and still strips legacy U+2060 and U+2063 characters from titles. DependencySource TEAM/BUDDY are kept on purpose for boards arranged before the change.
• BoardCardView still renders a visible fallback for an unknown kind, and backend V19 deletes the retired card kind anyway.
• The path-changed event: announceBuddyPathChanged fires only on result.ok for path actions. useBuddyPathSync is mounted once in AppContent and invalidates myStatuses and board.all. BoardPathWindow subscribes and unsubscribes correctly, with its cancelled guard.
• New confirm payload fields go through all four layers: stream chunk, onActionProposal, the proposal type and performAction. Skip reason and flag-to-PM text are shown before confirming, and only for non-stored proposals.
• The arrival key includes location.key for link kinds, so the same link clicked twice lands twice. clearLinkRef is assigned before the microtask reads it.
• StepOriginBadge: the isAiAssisted === false fallback covers blueprint copies and rows written before origin existed. Reviewer views pass viewer="reviewer" at both call sites.
• getUserOnboardingPath now fills in questions at the boundary, which fixes the TypeError on the team page.
• The composer focus handoff (typedRef) doesn't fire on mount (focusOnMount handles that) or on the post-send clear.

Verdict: approve after 1–3. 4 and 5 are small and worth doing in the same pass.

[daniilperkin]

Reviewed cbbe63b7 against dev (0fdb2cdd) — source diff read in full, cross-repo contract checked by hand (CI proves nothing here, it runs against current dev).

Retirement is clean. PATH_TO_FIRST_CONTRIBUTION, BoardPathNotes, momentLabels, pathToCards, useGeneratedPathCards, card-blueprints/ have zero remaining references in src/ and tests/; the kind is gone from BoardCardKind, so every exhaustive map/switch is compiler-checked. The single Build my path hit is the comment in cardNames.ts describing the retired generator.

Wire contract matches backend #261 exactly: POST /api/v1/onboarding/me/buddy/actions (hasRole('USER')), camelCase request fields (stepId, questionId, phaseId, onboardingTaskId, answer, description, reason, waitsOnIds, unlocksIds), snake_case stream fields, and the five action names byte-identical to BuddyActionType. The same DTO on dev carries a subset, i.e. the actions genuinely do not exist there yet.

Merge order: backend #261 first — merged alone against dev, the path actions would vanish silently and confirm would be rejected. AI #208 is behaviour-only (persona/prompt, step budget) and order-independent. Task-0 endpoint, reviewer path reads and phase questions are unchanged. origin on step responses is optional and StepOriginBadge falls back, so it degrades rather than crashes.

Two points worth deciding, neither a blocker:

1. useBuddyPathSync invalidates the board only for BUDDY_PATH_ACTIONS. Confirmable board-writing actions (claim_goal, the checklist actions) still don't announce, so the stale-card symptom this PR fixes for path actions remains for those — pre-existing and one-line fixable, but it is the same class of bug.
2. BuddyMarkdown is also used for GitHub issue excerpts (CorpusIssueBrowser.tsx:434), so the new root-relative in-app-link rule now renders an external text's /… link as an SPA navigation instead of a new tab.

Also: the branch conflicts with the current dev tip in exactly one file, BuddyComposer.tsx — #184's caret/dino handoff and this PR's external-draft caret rule overlap, so it needs a backmerge before merging. I can do that pass and resolve it by keeping both behaviours.

Approving on the code as reviewed; the ordering constraint above is the thing to respect at merge time.