From 465bfc5202edc23497148c7945dd1d3809d1f378 Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Sun, 13 Sep 2026 16:41:21 +0200 Subject: [PATCH 01/20] Put the tutor next to the path it is tutoring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The buddy could not be reached from the one page it has most to say about. A hire stuck on a step, or on a question they had now got wrong twice, was looking at the only page in the product with nobody to ask — while a mentor sat in a dock that knew nothing about any of it. - `AskTheBuddy` on the phase, on every open step and on every unpassed question, plus the step detail page. The same mechanism the board cards use: the surface seeds a question, the mentor answers it with its own tools, so no surface needs action machinery of its own. - The question modal hands off too, and closes on the way out — the dock renders under the modal, so opening it behind would have looked like nothing happening. Loudest under a wrong answer, which is where a second guess used to be the only thing on offer. - The "no phases were generated" screen offers the conversation next to the retry. Generating again is the wrong hope when the corpus was what was thin; talking it through can actually produce something, and the mentor can offer to put the result on the path. - Openings live in `buddyDrafts.ts`, in the hire's voice, pre-filled rather than sent, quoting a long title rather than pasting it. - The path-changing actions announce themselves (`announceBuddyPathChanged`), so a step completed in the conversation is not still open on the page behind the dock. Co-Authored-By: Claude Opus 5 --- src/features/buddy/aiBuddyBus.ts | 25 ++++ .../buddy/hooks/useBuddyConversation.ts | 17 +++ src/features/buddy/types.ts | 32 +++++ src/features/onboarding/buddyDrafts.ts | 84 ++++++++++++++ .../components/OnBoardingItemPage.tsx | 11 ++ .../onboarding/components/QuestionModal.tsx | 43 ++++++- src/pages/OnBoardingPage.tsx | 67 +++++++++++ src/services/buddyService.ts | 26 ++++- .../features/onboarding/buddyDrafts.test.ts | 109 ++++++++++++++++++ 9 files changed, 412 insertions(+), 2 deletions(-) create mode 100644 src/features/onboarding/buddyDrafts.ts create mode 100644 tests/unit/features/onboarding/buddyDrafts.test.ts diff --git a/src/features/buddy/aiBuddyBus.ts b/src/features/buddy/aiBuddyBus.ts index 791dc6e86..36e64bbed 100644 --- a/src/features/buddy/aiBuddyBus.ts +++ b/src/features/buddy/aiBuddyBus.ts @@ -54,3 +54,28 @@ export function onBuddyPageReady(handler: () => void): () => void { window.addEventListener(BUDDY_PAGE_READY_EVENT, handler); return () => window.removeEventListener(BUDDY_PAGE_READY_EVENT, handler); } + +const BUDDY_PATH_CHANGED_EVENT = "sprintstart:buddy-path-changed"; + +/** + * Announced after the hire confirms a buddy action that changed their onboarding path. + * + * The buddy lives in a dock over whatever page the hire is on, and the path page is the page they + * are most likely to be on while talking about their path. Without this, confirming "mark this step + * as done" left a page behind the dock still showing it open — the hire's own click looking like it + * had done nothing. + * + * A signal rather than shared state, for the same reason `openAiBuddy` is one: the dock would + * otherwise have to know about the onboarding page's data layer, and every other surface that grows + * an interest in the path would have to be wired through it too. Whoever is showing a path listens; + * nobody has to. + */ +export function announceBuddyPathChanged(): void { + window.dispatchEvent(new Event(BUDDY_PATH_CHANGED_EVENT)); +} + +/** Subscribes to path changes the buddy made. Returns an unsubscribe function. */ +export function onBuddyPathChanged(handler: () => void): () => void { + window.addEventListener(BUDDY_PATH_CHANGED_EVENT, handler); + return () => window.removeEventListener(BUDDY_PATH_CHANGED_EVENT, handler); +} diff --git a/src/features/buddy/hooks/useBuddyConversation.ts b/src/features/buddy/hooks/useBuddyConversation.ts index 699207efc..b0f801a92 100644 --- a/src/features/buddy/hooks/useBuddyConversation.ts +++ b/src/features/buddy/hooks/useBuddyConversation.ts @@ -6,6 +6,8 @@ import { streamMessage, type BuddyOpeningAction, } from "../../../services/buddyService"; +import { announceBuddyPathChanged } from "../aiBuddyBus"; +import { BUDDY_PATH_ACTIONS } from "../types"; import type { BuddyMessageView, ProposedAction } from "../types"; /** @@ -354,6 +356,11 @@ export function useBuddyConversation() { githubLogin: proposal.githubLogin, competencyKey: proposal.competencyKey, level: proposal.level, + stepId: proposal.stepId, + questionId: proposal.questionId, + phaseId: proposal.phaseId, + answer: proposal.answer, + description: proposal.description, status: "idle", }); }, @@ -427,12 +434,22 @@ export function useBuddyConversation() { githubLogin: action.githubLogin, competencyKey: action.competencyKey, level: action.level, + stepId: action.stepId, + questionId: action.questionId, + phaseId: action.phaseId, + answer: action.answer, + description: action.description, }); patchAction(messageId, action.id, { status: "resolved", ok: result.ok, outcome: result.message, }); + // A path action just moved something on a page that may be open behind this dock. Told + // rather than polled, and only on success: a refused confirm changed nothing to refresh. + if (result.ok && BUDDY_PATH_ACTIONS.includes(action.action)) { + announceBuddyPathChanged(); + } } catch (e) { console.error(e); patchAction(messageId, action.id, { status: "error" }); diff --git a/src/features/buddy/types.ts b/src/features/buddy/types.ts index 5605c3b9f..1b150c83e 100644 --- a/src/features/buddy/types.ts +++ b/src/features/buddy/types.ts @@ -17,6 +17,20 @@ export type ProposedActionStatus = "idle" | "confirming" | "resolved" | "error" */ export const BUDDY_ACTION_OPEN_ORIENTATION = "open_orientation"; +/** + * The actions that change the hire's onboarding path. + * + * Listed once, here, because two surfaces need the same answer: confirming one of these has to tell + * whatever is showing a path that it is now stale (see `announceBuddyPathChanged`). Answering a + * question counts even when the answer was wrong — the attempt is recorded and the question's status + * moves either way. + */ +export const BUDDY_PATH_ACTIONS: readonly string[] = [ + "complete_step", + "answer_question", + "add_path_step", +]; + export type ProposedAction = { /** Local id for keying and targeting the confirm — the backend doesn't assign one. */ id: string; @@ -58,6 +72,19 @@ export type ProposedAction = { */ competencyKey?: string; level?: string; + /** + * The path-action confirm payloads: which node of the hire's own onboarding path the action is + * aimed at, the answer `answer_question` would send, and a new step's description. + * + * Echoed back verbatim for the same reason as `githubLogin`: the hire reads the step, or their own + * answer, on the button before agreeing to it, so what gets written has to be what they were + * shown — never something the client derived afterwards. + */ + stepId?: string; + questionId?: string; + phaseId?: string; + answer?: string; + description?: string; status: ProposedActionStatus; /** Whether a resolved action actually changed something (false = a handled "couldn't"). */ ok?: boolean; @@ -140,5 +167,10 @@ export type BuddyStreamHandlers = { githubLogin?: string; competencyKey?: string; level?: string; + stepId?: string; + questionId?: string; + phaseId?: string; + answer?: string; + description?: string; }) => void; }; diff --git a/src/features/onboarding/buddyDrafts.ts b/src/features/onboarding/buddyDrafts.ts new file mode 100644 index 000000000..0945a2ce6 --- /dev/null +++ b/src/features/onboarding/buddyDrafts.ts @@ -0,0 +1,84 @@ +// ============================================================ +// features/onboarding/buddyDrafts.ts +// ============================================================ +// The first sentence of a conversation about something on the +// hire's path. Pre-filled into the buddy's composer, never sent +// for them. +// ============================================================ + +import type { OnboardingPhaseEndpoint, OnboardingQuestionEndpoint, OnboardingStepEndpoint } from "./types"; + +/** + * Taking what you are looking at on your path into the conversation. + * + * The board already works this way (see `AskTheBuddy`): a surface seeds a question and the mentor + * answers it with its own tools, which is why a card needs no action machinery of its own. The path + * is the surface that wanted it most and did not have it — a hire stuck on a step, or on a question + * they have now got wrong twice, was looking at the one page in the product with nobody to ask. + * + * **Written in the hire's voice, and as an opening rather than an instruction.** The draft lands in + * the composer for them to change before it goes; a sentence that reads like a command from the page + * is one they stop trusting as their own. That is also why these say what the hire wants rather than + * what the mentor should do: the mentor has the path in front of it either way. + * + * Kept out of the components so the wording is in one place and the same subject always opens the + * same way — the mentor's replies are inconsistent enough without the questions varying too. + */ + +/** How much of somebody else's sentence is quoted into a draft before it is cut. */ +const QUOTE_LIMIT = 120; + +/** A quotable snippet of a title or a question, cut on a word boundary where there is one. */ +function snippet(text: string): string { + const trimmed = text.trim(); + if (trimmed.length <= QUOTE_LIMIT) return trimmed; + + const cut = trimmed.slice(0, QUOTE_LIMIT); + const lastSpace = cut.lastIndexOf(" "); + return `${lastSpace > QUOTE_LIMIT / 2 ? cut.slice(0, lastSpace) : cut}…`; +} + +/** Opening a conversation about the phase the hire is standing in. */ +export function askAboutPhase(phase: OnboardingPhaseEndpoint): string { + return `I'm on the "${snippet(phase.title)}" phase of my onboarding. Can you walk me through what it's for and where I should start?`; +} + +/** + * Opening a conversation about a phase that generated nothing. + * + * The case the buddy exists for on this page. An AI-enhanced phase whose project material was too + * thin is honestly left empty rather than filled with invented advice, which leaves the hire with a + * warning badge and nothing to do about it. Its title still says what it was meant to cover, so the + * conversation can — and the mentor can offer to put the result on their path. + */ +export function askAboutEmptyPhase(phaseTitle: string): string { + return `The "${snippet(phaseTitle)}" phase of my onboarding came back empty — nothing was generated for it. Can we work out together what it should contain for me?`; +} + +/** Opening a conversation about one step. */ +export function askAboutStep(step: OnboardingStepEndpoint): string { + return `I'm on the onboarding step "${snippet(step.title)}". Can you help me get going on it?`; +} + +/** + * Opening a conversation about a knowledge question. + * + * Says out loud that the hire wants to understand it rather than be told the answer. The mentor is + * not given the correct answer and will say so if asked — but a hire who opens by asking for it gets + * a refusal as their first experience of the feature, and this is the cheapest way to not start + * there. + */ +export function askAboutQuestion( + question: OnboardingQuestionEndpoint, + phaseTitle: string, +): string { + return `I'm stuck on the knowledge question "${snippet(question.question)}" in "${snippet(phaseTitle)}". Can you go through the material with me? I'd rather work the answer out than be told it.`; +} + +/** Opening a conversation about a question the hire has just got wrong. */ +export function askAboutWrongAnswer( + question: OnboardingQuestionEndpoint, + phaseTitle: string, +): string { + return `I just got the knowledge question "${snippet(question.question)}" in "${snippet(phaseTitle)}" wrong. Can you go through the material with me so I actually understand it before I try again?`; +} diff --git a/src/features/onboarding/components/OnBoardingItemPage.tsx b/src/features/onboarding/components/OnBoardingItemPage.tsx index bcc3ebad5..3d61b9588 100644 --- a/src/features/onboarding/components/OnBoardingItemPage.tsx +++ b/src/features/onboarding/components/OnBoardingItemPage.tsx @@ -38,6 +38,8 @@ import { ThumbsDown, } from "lucide-react"; import { resolveNextAction } from "../nextAction"; +import { AskTheBuddy } from "../../buddy/components/AskTheBuddy"; +import { askAboutStep } from "../buddyDrafts"; type LoadingState = "idle" | "loading" | "success" | "error"; @@ -420,6 +422,15 @@ export function OnBoardingItemPage() {

{stepDetail.description}

+ {/* The step page is where a hire sits when they are stuck on one, and until now the + only things here were the task list and a Finish button. Offered while the step is + still open: there is nothing left to be stuck on once it is finished or skipped. */} + {stepDetail.status !== "FINISHED" && stepDetail.status !== "SKIPPED" && ( + + )} {stepDetail.estimatedMinutes > 0 && ( diff --git a/src/features/onboarding/components/QuestionModal.tsx b/src/features/onboarding/components/QuestionModal.tsx index 37f894804..af67d70c8 100644 --- a/src/features/onboarding/components/QuestionModal.tsx +++ b/src/features/onboarding/components/QuestionModal.tsx @@ -15,7 +15,9 @@ import type { OnboardingQuestionEndpoint, QuestionAttemptResult } from "../types import { CheckQuestionCard } from "./CheckQuestionCard"; import { emptyDraft, isAnswered, toSubmission, type DraftAnswer } from "../checkAnswers"; import { ConfettiBurst } from "./ConfettiBurst"; -import { Loader2, RotateCcw, Trophy, XCircle } from "lucide-react"; +import { Loader2, MessageCircle, RotateCcw, Trophy, XCircle } from "lucide-react"; +import { openAiBuddy } from "../../buddy/aiBuddyBus"; +import { askAboutQuestion, askAboutWrongAnswer } from "../buddyDrafts"; interface QuestionModalProps { question: OnboardingQuestionEndpoint; @@ -84,6 +86,20 @@ export function QuestionModal({ question, phaseTitle, onClose }: QuestionModalPr onboardingCompleted: result?.onboardingCompleted ?? false, }); + /** + * Hands the question over to the buddy. + * + * Closes the modal on the way out rather than opening the dock behind it: the dock renders under + * the modal, so a hire who asked for help would have watched nothing happen. Reported as an + * ordinary close — the mentor may go on to send an answer from the conversation, and the page + * re-reads the path when it does (see `announceBuddyPathChanged`), so this does not have to guess + * at what happens next. + */ + const handOffToBuddy = (opening: string) => { + openAiBuddy({ draft: opening }); + close(); + }; + const footer = result ? ( <> {!result.correct && ( @@ -146,6 +162,17 @@ export function QuestionModal({ question, phaseTitle, onClose }: QuestionModalPr
Review the answer below and try again.
+ {/* Where a second guess used to be the only thing on offer. The buddy cannot tell them + the answer -- it is not given one -- so this is help with the material, which is what + a wrong answer actually calls for. */} + )} @@ -158,6 +185,20 @@ export function QuestionModal({ question, phaseTitle, onClose }: QuestionModalPr onToggleOption={toggleOption} onTextChange={setTextAnswer} /> + + {/* Before an attempt, and deliberately quiet: guessing is allowed and costs nothing here, so + this is an offer rather than a nudge. Gone once the answer has been graded, where the + banner above carries the same offer with the reason for it. */} + {!result && ( + + )} ); } diff --git a/src/pages/OnBoardingPage.tsx b/src/pages/OnBoardingPage.tsx index 0c53cea51..5f60cd8f7 100644 --- a/src/pages/OnBoardingPage.tsx +++ b/src/pages/OnBoardingPage.tsx @@ -10,6 +10,14 @@ import type { OnboardingStepEndpoint, } from "../features/onboarding/types"; import { findActivePhaseIndex } from "../features/onboarding/activePhase"; +import { AskTheBuddy } from "../features/buddy/components/AskTheBuddy"; +import { onBuddyPathChanged } from "../features/buddy/aiBuddyBus"; +import { + askAboutEmptyPhase, + askAboutPhase, + askAboutQuestion, + askAboutStep, +} from "../features/onboarding/buddyDrafts"; import { useNavigate, useLocation } from "react-router-dom"; import { Badge } from "../components/ui/Badge"; import { Button } from "../components/ui/Button"; @@ -299,6 +307,19 @@ export function OnBoardingPage() { return () => window.removeEventListener("keydown", onKeyDown); }, [loadingState, gameActive, isUnlocked]); + /** + * Re-reads the path after the buddy changed it. + * + * The buddy lives in a dock over this page, which is the page a hire is most likely to be on while + * talking about their path. Without this, confirming "mark this step as done" in the conversation + * left the list behind it still showing the step open — their own click looking like it had done + * nothing. Told rather than polled; see `announceBuddyPathChanged`. + * + * Subscribed once: `refreshPath` only closes over the service and a setter, both stable for the + * life of the page. + */ + useEffect(() => onBuddyPathChanged(() => void refreshPath()), []); + // ── DATA FETCHING using useEffect ───────────────────────────── // Guards the initial GET against StrictMode's development-only effect replay. @@ -582,6 +603,17 @@ export function OnBoardingPage() { > Try generation again + {/* Generating again is the wrong hope when the corpus is what was thin -- it will come + back empty a second time. The conversation is the one thing here that can actually + produce something, so it is offered next to the retry rather than instead of it. */} + {generationIssues.length > 0 && ( +
+ +
+ )} ); @@ -785,6 +817,20 @@ export function OnBoardingPage() {

{currentPhase.title}

{currentPhase.description}

+ {/* The phase-level way in. A hire who does not know why a phase is here is not helped + by any of the buttons below it. */} +
{/* Locked phase notice */} @@ -851,6 +897,14 @@ export function OnBoardingPage() {

{step.description}

+ {/* Not on a finished or skipped step: there is nothing left to be + stuck on, and an invitation there is noise on a list of them. */} + {mode !== "completed" && ( + + )} {/* Action depends on the step's mode: @@ -948,6 +1002,19 @@ export function OnBoardingPage() { ? "Multiple choice" : "Short text answer"}

+ {/* The tutoring moment. Offered on a question still open -- + loudest on one already answered wrong, which is where a hire + previously had nowhere to go but another guess. */} + {mode !== "completed" && ( + + )}
diff --git a/src/services/buddyService.ts b/src/services/buddyService.ts index 0caf521b5..704590b3d 100644 --- a/src/services/buddyService.ts +++ b/src/services/buddyService.ts @@ -120,6 +120,13 @@ interface BuddyStreamChunk { github_login?: string; competency_key?: string; level?: string; + // Path-action confirm payloads: which node of the hire's own onboarding path the action names, + // the answer `answer_question` would send in their own words, and a new step's description. + step_id?: string; + question_id?: string; + phase_id?: string; + answer?: string; + description?: string; } /** The outcome of confirming a buddy-proposed action — a single line to relay in the thread. */ @@ -133,7 +140,9 @@ export interface BuddyActionResult { * changed nothing. The project is re-resolved server-side from the caller, so only the action name * and the proposal's own confirm payloads are sent: `question` for flag-to-PM, `taskId` for a * goal claim, `title` + `attesterId` for an attestation request, `githubLogin` for saving a - * username, `competencyKey` + `level` for recording where a conversation placed the hire. + * username, `competencyKey` + `level` for recording where a conversation placed the hire, and the + * path-node ids (`stepId`, `questionId`, `phaseId`, plus `answer` and `description`) for the three + * actions that move the hire along their onboarding path. */ export async function performAction( action: string, @@ -145,6 +154,11 @@ export async function performAction( githubLogin?: string; competencyKey?: string; level?: string; + stepId?: string; + questionId?: string; + phaseId?: string; + answer?: string; + description?: string; } = {}, ): Promise { return await apiClient.fetch(`/api/v1/onboarding/me/buddy/actions`, { @@ -158,6 +172,11 @@ export async function performAction( githubLogin: extras.githubLogin, competencyKey: extras.competencyKey, level: extras.level, + stepId: extras.stepId, + questionId: extras.questionId, + phaseId: extras.phaseId, + answer: extras.answer, + description: extras.description, }), }); } @@ -327,6 +346,11 @@ export async function streamMessage(content: string, handlers: BuddyStreamHandle githubLogin: event.github_login, competencyKey: event.competency_key, level: event.level, + stepId: event.step_id, + questionId: event.question_id, + phaseId: event.phase_id, + answer: event.answer, + description: event.description, }); } break; diff --git a/tests/unit/features/onboarding/buddyDrafts.test.ts b/tests/unit/features/onboarding/buddyDrafts.test.ts new file mode 100644 index 000000000..cfa1b91d8 --- /dev/null +++ b/tests/unit/features/onboarding/buddyDrafts.test.ts @@ -0,0 +1,109 @@ +import { describe, expect, it } from "vitest"; +import { + askAboutEmptyPhase, + askAboutPhase, + askAboutQuestion, + askAboutStep, + askAboutWrongAnswer, +} from "../../../../src/features/onboarding/buddyDrafts"; +import type { + OnboardingPhaseEndpoint, + OnboardingQuestionEndpoint, + OnboardingStepEndpoint, +} from "../../../../src/features/onboarding/types"; + +/** + * The opening sentence of a conversation about something on the path. + * + * Two things are worth pinning. It is written in the hire's voice, because the draft lands in their + * composer and they send it — a sentence that reads like an instruction from the page is one they + * stop trusting as their own. And somebody else's title is quoted, not pasted: a path step whose + * title runs to three lines would otherwise arrive as a composer full of it. + */ +describe("buddy drafts", () => { + const phase = (over: Partial = {}) => + ({ + id: "p1", + pathId: "path", + position: 0, + title: "Environment Setup", + description: "", + locked: false, + steps: [], + questions: [], + ...over, + }) as OnboardingPhaseEndpoint; + + const step = (over: Partial = {}) => + ({ + id: "s1", + phaseId: "p1", + position: 0, + title: "Clone the repository", + description: "", + status: "WAITING", + ...over, + }) as OnboardingStepEndpoint; + + const question = (over: Partial = {}) => + ({ + id: "q1", + phaseId: "p1", + position: 0, + type: "MULTIPLE_CHOICE", + question: "Which meeting sets the sprint scope?", + status: "OPEN", + ...over, + }) as OnboardingQuestionEndpoint; + + it("asks in the hire's own voice, not the page's", () => { + for (const draft of [ + askAboutPhase(phase()), + askAboutStep(step()), + askAboutQuestion(question(), "Meetings"), + askAboutWrongAnswer(question(), "Meetings"), + askAboutEmptyPhase("Deployment"), + ]) { + expect(draft).toMatch(/^(I|The|We)\b/); + expect(draft).toContain("?"); + } + }); + + it("names the thing it is about, so the mentor does not have to ask", () => { + expect(askAboutPhase(phase())).toContain("Environment Setup"); + expect(askAboutStep(step())).toContain("Clone the repository"); + expect(askAboutQuestion(question(), "Meetings")).toContain("Which meeting sets the sprint scope?"); + expect(askAboutQuestion(question(), "Meetings")).toContain("Meetings"); + }); + + it("says the hire wants to understand the question, not be handed the answer", () => { + // The mentor is not given the correct answer and will say so if asked. Opening this way is the + // cheapest way for a hire's first experience of the feature not to be a refusal. + expect(askAboutQuestion(question(), "Meetings")).toContain("rather work the answer out"); + expect(askAboutWrongAnswer(question(), "Meetings")).toContain("understand it"); + }); + + it("says an empty phase generated nothing, which is the fact the mentor needs", () => { + const draft = askAboutEmptyPhase("Deployment"); + + expect(draft).toContain("Deployment"); + expect(draft).toContain("empty"); + // It asks to work out what the phase should contain -- which is what `add_path_step` is for. + expect(draft).toContain("what it should contain"); + }); + + it("quotes a long title rather than pasting it", () => { + const long = "Read ".repeat(60).trim(); + + const draft = askAboutStep(step({ title: long })); + + expect(draft).toContain("…"); + expect(draft.length).toBeLessThan(long.length); + }); + + it("leaves a short title exactly as it was written", () => { + expect(askAboutStep(step({ title: "Clone the repository" }))).toContain( + '"Clone the repository"', + ); + }); +}); From e56cae08a374739ac3baedc3bc36e7368112909f Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Sun, 13 Sep 2026 18:59:04 +0200 Subject: [PATCH 02/20] Let a hire and their buddy point at the same thing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four of the six things testing turned up were on this side. - **Links are clickable and stay in the app.** The mentor is handed each item's path, so "want to take #3?" arrives as a link; `BuddyMarkdown` renders an app path through the router instead of opening a new tab, which would reload the SPA and lose the conversation. A question has no route of its own, so `?question=` opens its modal and `?phase=` selects a phase — derived from the URL rather than copied into state, so a link works whether it arrives on a fresh mount or on a click from the dock, and it stops winning as soon as the hire closes the modal or picks another phase. - **Items carry the number the buddy uses**, from one rule (`itemNumbers`) that matches the Kotlin one: steps in position order, then questions. Not `position` itself — the two kinds carry their own, so a hire counting down one visible list would have been right while the data disagreed. - **The step page refreshes itself** when the buddy changes something on it. Ticking a line off in the dock left the checklist behind it unchanged — the hire's own click looking like it had done nothing. Silent: the page is already on screen, and a spinner over it is a worse answer than a stale tick box. - `complete_task` wired through the proposal payloads. Co-Authored-By: Claude Opus 5 --- .../buddy/components/BuddyMarkdown.tsx | 31 +++++- .../buddy/hooks/useBuddyConversation.ts | 2 + src/features/buddy/types.ts | 3 + .../components/OnBoardingItemPage.tsx | 33 ++++++ src/features/onboarding/itemNumbers.ts | 34 ++++++ src/pages/OnBoardingPage.tsx | 104 ++++++++++++++++-- src/services/buddyService.ts | 4 + .../features/onboarding/buddyDrafts.test.ts | 15 ++- .../features/onboarding/itemNumbers.test.ts | 65 +++++++++++ tests/unit/pages/OnBoardingPage.test.tsx | 91 ++++++++++++++- 10 files changed, 365 insertions(+), 17 deletions(-) create mode 100644 src/features/onboarding/itemNumbers.ts create mode 100644 tests/unit/features/onboarding/itemNumbers.test.ts diff --git a/src/features/buddy/components/BuddyMarkdown.tsx b/src/features/buddy/components/BuddyMarkdown.tsx index 7f567da83..96d5ceb97 100644 --- a/src/features/buddy/components/BuddyMarkdown.tsx +++ b/src/features/buddy/components/BuddyMarkdown.tsx @@ -1,6 +1,21 @@ import ReactMarkdown from "react-markdown"; +import { Link } from "react-router-dom"; import remarkGfm from "remark-gfm"; +/** + * Whether a link the buddy wrote points inside the app. + * + * The mentor is given the paths of the things it talks about — a step, a question, a phase — so that + * "want to take the check?" can arrive as something clickable. Those are app paths, and opening one + * in a new tab would reload the whole SPA and lose the conversation the hire was having. + * + * Root-relative only, and deliberately: a protocol-relative `//evil.example` is also "relative" to a + * careless check, and the model's output is not a place to be careless. + */ +function isInAppPath(href: string | undefined): href is string { + return href !== undefined && href.startsWith("/") && !href.startsWith("//"); +} + /** * Renders the buddy's reply as Markdown (GitHub-flavoured), so lists, bold, headings, links, * code and tables come through instead of raw asterisks and backticks. Shared by both buddy @@ -27,11 +42,17 @@ export function BuddyMarkdown({ content }: { content: string }) { ( - - {children} - - ), + // An app path navigates in place; anything else is still somebody else's site in a new + // tab. The dock stays mounted across a route change, so a hire who follows a link into + // their path keeps the conversation that sent them there. + a: ({ children, href }) => + isInAppPath(href) ? ( + {children} + ) : ( + + {children} + + ), table: ({ children }) => (
diff --git a/src/features/buddy/hooks/useBuddyConversation.ts b/src/features/buddy/hooks/useBuddyConversation.ts index b0f801a92..2fcb19bf1 100644 --- a/src/features/buddy/hooks/useBuddyConversation.ts +++ b/src/features/buddy/hooks/useBuddyConversation.ts @@ -359,6 +359,7 @@ export function useBuddyConversation() { stepId: proposal.stepId, questionId: proposal.questionId, phaseId: proposal.phaseId, + onboardingTaskId: proposal.onboardingTaskId, answer: proposal.answer, description: proposal.description, status: "idle", @@ -437,6 +438,7 @@ export function useBuddyConversation() { stepId: action.stepId, questionId: action.questionId, phaseId: action.phaseId, + onboardingTaskId: action.onboardingTaskId, answer: action.answer, description: action.description, }); diff --git a/src/features/buddy/types.ts b/src/features/buddy/types.ts index 1b150c83e..804897a2d 100644 --- a/src/features/buddy/types.ts +++ b/src/features/buddy/types.ts @@ -27,6 +27,7 @@ export const BUDDY_ACTION_OPEN_ORIENTATION = "open_orientation"; */ export const BUDDY_PATH_ACTIONS: readonly string[] = [ "complete_step", + "complete_task", "answer_question", "add_path_step", ]; @@ -83,6 +84,7 @@ export type ProposedAction = { stepId?: string; questionId?: string; phaseId?: string; + onboardingTaskId?: string; answer?: string; description?: string; status: ProposedActionStatus; @@ -170,6 +172,7 @@ export type BuddyStreamHandlers = { stepId?: string; questionId?: string; phaseId?: string; + onboardingTaskId?: string; answer?: string; description?: string; }) => void; diff --git a/src/features/onboarding/components/OnBoardingItemPage.tsx b/src/features/onboarding/components/OnBoardingItemPage.tsx index 3d61b9588..a4bf6c542 100644 --- a/src/features/onboarding/components/OnBoardingItemPage.tsx +++ b/src/features/onboarding/components/OnBoardingItemPage.tsx @@ -39,6 +39,7 @@ import { } from "lucide-react"; import { resolveNextAction } from "../nextAction"; import { AskTheBuddy } from "../../buddy/components/AskTheBuddy"; +import { onBuddyPathChanged } from "../../buddy/aiBuddyBus"; import { askAboutStep } from "../buddyDrafts"; type LoadingState = "idle" | "loading" | "success" | "error"; @@ -229,6 +230,38 @@ export function OnBoardingItemPage() { } }; + /** + * Re-reads this step after the buddy changed something on it. + * + * The dock sits over this page, so ticking a line off in the conversation used to leave the + * checklist behind it unchanged — the hire's own click looking like it had done nothing. Silent on + * purpose: it re-reads the step and its tasks without going back through the loading state, because + * the page is already on screen and a spinner over it would be a worse answer than a stale tick + * box. + */ + useEffect( + () => + onBuddyPathChanged(() => { + if (!stepId) return; + void (async () => { + try { + const [step, refreshedTasks] = await Promise.all([ + onboardingService.fetchStep(stepId), + onboardingService.fetchTasks(stepId), + ]); + setStepDetail(step); + setTasks(refreshedTasks); + setLocalFinished( + new Set(refreshedTasks.filter((task) => task.finished).map((task) => task.id)), + ); + } catch (err) { + console.error("Failed to refresh the step after a buddy action:", err); + } + })(); + }), + [stepId], + ); + /** * Data Fetching Effect: Loads the full hierarchy of a step (details, tasks, resources). * It also initializes the local 'finished' state for tasks based on the fetched data. diff --git a/src/features/onboarding/itemNumbers.ts b/src/features/onboarding/itemNumbers.ts new file mode 100644 index 000000000..dbad47937 --- /dev/null +++ b/src/features/onboarding/itemNumbers.ts @@ -0,0 +1,34 @@ +// ============================================================ +// features/onboarding/itemNumbers.ts +// ============================================================ +// The number a phase's steps and questions carry on screen, so +// a hire can say "let's do 3" to their buddy and mean this one. +// ============================================================ + +import type { OnboardingPhaseEndpoint } from "./types"; + +/** + * The number each item of a phase is shown with, keyed by id. + * + * **Steps first in position order, then questions in position order.** That is the order this page + * lists them in, and the order the buddy's path tool numbers them in — the same rule stated twice, + * once per language, because the number is the whole point: a hire says "3" and the mentor has to + * land on the item they were looking at. If the page's own order ever changes, `BuddyPathTools` + * changes with it or the numbers start lying. + * + * Not `position` itself. Steps and questions carry their own positions underneath, so two items can + * share one, and a hire counting down a single visible list would be right while the data disagreed. + * + * A number is not an identity — the buddy also gets each item's id and its link, and those are what + * an action is aimed at. This exists so a person does not have to retype a title. + */ +export function itemNumbers(phase: OnboardingPhaseEndpoint): Map { + const steps = [...phase.steps].sort((a, b) => a.position - b.position); + const questions = [...phase.questions].sort((a, b) => a.position - b.position); + + return new Map( + [...steps.map((step) => step.id), ...questions.map((question) => question.id)].map( + (id, index) => [id, index + 1], + ), + ); +} diff --git a/src/pages/OnBoardingPage.tsx b/src/pages/OnBoardingPage.tsx index 5f60cd8f7..321ccdcc7 100644 --- a/src/pages/OnBoardingPage.tsx +++ b/src/pages/OnBoardingPage.tsx @@ -2,7 +2,7 @@ // OnBoardingPage.tsx // ============================================================ -import { useState, useEffect, useRef } from "react"; +import { useState, useEffect, useMemo, useRef } from "react"; import type { OnboardingPathEndpoint, OnboardingPhaseEndpoint, @@ -10,6 +10,7 @@ import type { OnboardingStepEndpoint, } from "../features/onboarding/types"; import { findActivePhaseIndex } from "../features/onboarding/activePhase"; +import { itemNumbers } from "../features/onboarding/itemNumbers"; import { AskTheBuddy } from "../features/buddy/components/AskTheBuddy"; import { onBuddyPathChanged } from "../features/buddy/aiBuddyBus"; import { @@ -18,7 +19,7 @@ import { askAboutQuestion, askAboutStep, } from "../features/onboarding/buddyDrafts"; -import { useNavigate, useLocation } from "react-router-dom"; +import { useNavigate, useLocation, useSearchParams } from "react-router-dom"; import { Badge } from "../components/ui/Badge"; import { Button } from "../components/ui/Button"; import { onboardingService } from "../services/onboardingService"; @@ -137,6 +138,21 @@ export function OnBoardingPage() { // user and the rest of their path, so this page can land on the question's phase. const focusQuestionId = (location.state as { focusQuestionId?: string } | null)?.focusQuestionId; + /** + * Where a link from the buddy points. + * + * The mentor is given each item's path so it can say "want to take [#3](...)?" and have that be + * clickable. A question has no route of its own — it is a modal on this page — so it arrives as + * `?question=`, and a phase as `?phase=`. + * + * In the URL rather than in router state, unlike `focusQuestionId`: this link is written by the + * model into text the hire can copy, keep, or open in a second tab, and state does not survive any + * of that. + */ + const [searchParams, setSearchParams] = useSearchParams(); + const linkedQuestionId = searchParams.get("question"); + const linkedPhaseId = searchParams.get("phase"); + // The question list of the focused phase, so the page can scroll to it. const questionListRef = useRef(null); const hasFocusedQuestionRef = useRef(false); @@ -195,6 +211,9 @@ export function OnBoardingPage() { }) => { const phases = OnBoardingPathEndpoint?.phases ?? []; setQuestionToAnswer(null); + // Also the link, if that is what opened it — otherwise closing the modal would reopen it on the + // next render, since the URL would still be asking for it. + if (linkedQuestionId) clearLink(); // The backend decides completion; nothing here is derived from the phase alone. if (onboardingCompleted) { @@ -320,6 +339,37 @@ export function OnBoardingPage() { */ useEffect(() => onBuddyPathChanged(() => void refreshPath()), []); + /** + * The phase and the question a buddy link names, resolved from the URL rather than copied into + * state. + * + * Derived on purpose. A link can arrive two ways — a fresh mount, or a click while the hire is + * already standing on this page — and writing state from an effect would both trip the + * set-state-in-an-effect rule and only handle the first. Deriving handles both and needs no + * clean-up: the link stops winning the moment the hire picks a different phase or closes the + * modal, because those clear the parameter. + */ + const linkedPhaseIndex = useMemo(() => { + const phases = OnBoardingPathEndpoint?.phases ?? []; + if (linkedQuestionId) { + return phases.findIndex((phase) => + phase.questions.some((question) => question.id === linkedQuestionId), + ); + } + return linkedPhaseId ? phases.findIndex((phase) => phase.id === linkedPhaseId) : -1; + }, [OnBoardingPathEndpoint, linkedQuestionId, linkedPhaseId]); + + /** Forgets the link, so the hire's own next click decides what they are looking at. */ + const clearLink = () => + setSearchParams( + (params) => { + params.delete("question"); + params.delete("phase"); + return params; + }, + { replace: true }, + ); + // ── DATA FETCHING using useEffect ───────────────────────────── // Guards the initial GET against StrictMode's development-only effect replay. @@ -363,7 +413,9 @@ export function OnBoardingPage() { // makes a re-run a no-op anyway. }, [focusQuestionId]); - const currentPhase = OnBoardingPathEndpoint?.phases[selectedPhaseIndex] ?? null; + // A link from the buddy wins while it is in the URL; the hire's own tab click clears it. + const shownPhaseIndex = linkedPhaseIndex >= 0 ? linkedPhaseIndex : selectedPhaseIndex; + const currentPhase = OnBoardingPathEndpoint?.phases[shownPhaseIndex] ?? null; const generationIssues = OnBoardingPathEndpoint?.generationIssues ?? []; const generationIssueSummary = generationIssues .map( @@ -372,6 +424,30 @@ export function OnBoardingPage() { ) .join(", "); + /** + * The question the modal is showing: the one a card opened, or the one a link names. + * + * A linked question only opens when the hire could actually answer it. A link to a locked or + * already-passed question is a dead end, and landing on its phase — which still happens — is the + * useful half of following it. + */ + const linkedQuestion = + linkedQuestionId && currentPhase + ? currentPhase.questions.find( + (question) => + question.id === linkedQuestionId && + question.status !== "LOCKED" && + question.status !== "PASSED", + ) + : undefined; + const shownQuestion = + questionToAnswer ?? + (linkedQuestion ? { question: linkedQuestion, phaseTitle: currentPhase?.title ?? "" } : null); + + // The numbers this phase's items are shown with. The buddy's path tool derives the same ones, so + // "let's do 3" means one item on both sides — see `itemNumbers`. + const numbers = currentPhase ? itemNumbers(currentPhase) : new Map(); + // Helper function for phase progress — steps and questions both count. const getPhaseProgress = (phase: OnboardingPhaseEndpoint) => { const questions = phase.questions ?? []; @@ -679,7 +755,10 @@ export function OnBoardingPage() { key={phase.id} type="button" aria-pressed={isSelected} - onClick={() => setSelectedPhaseIndex(index)} + onClick={() => { + setSelectedPhaseIndex(index); + clearLink(); + }} className={`min-w-64 flex-1 rounded-2xl border p-4 text-left transition-all duration-200 motion-reduce:hover:scale-100 ${ isSelected ? "border-app-brand bg-app-brand-soft" @@ -889,6 +968,12 @@ export function OnBoardingPage() { : "text-app-text" }`} > + {/* The number the buddy uses for this item. Quiet, and not part of + the title: it is a handle for talking about the step, not + something the step is called. */} + + #{numbers.get(step.id)} + {step.title}
@@ -995,6 +1080,9 @@ export function OnBoardingPage() { : "text-app-text" }`} > + + #{numbers.get(question.id)} + {question.question}

@@ -1049,11 +1137,11 @@ export function OnBoardingPage() { )} - {/* Per-question answer modal */} - {questionToAnswer && ( + {/* Per-question answer modal. Opened by a card, or by a link the buddy wrote. */} + {shownQuestion && ( )} diff --git a/src/services/buddyService.ts b/src/services/buddyService.ts index 704590b3d..d9bcfa450 100644 --- a/src/services/buddyService.ts +++ b/src/services/buddyService.ts @@ -125,6 +125,7 @@ interface BuddyStreamChunk { step_id?: string; question_id?: string; phase_id?: string; + onboarding_task_id?: string; answer?: string; description?: string; } @@ -157,6 +158,7 @@ export async function performAction( stepId?: string; questionId?: string; phaseId?: string; + onboardingTaskId?: string; answer?: string; description?: string; } = {}, @@ -175,6 +177,7 @@ export async function performAction( stepId: extras.stepId, questionId: extras.questionId, phaseId: extras.phaseId, + onboardingTaskId: extras.onboardingTaskId, answer: extras.answer, description: extras.description, }), @@ -349,6 +352,7 @@ export async function streamMessage(content: string, handlers: BuddyStreamHandle stepId: event.step_id, questionId: event.question_id, phaseId: event.phase_id, + onboardingTaskId: event.onboarding_task_id, answer: event.answer, description: event.description, }); diff --git a/tests/unit/features/onboarding/buddyDrafts.test.ts b/tests/unit/features/onboarding/buddyDrafts.test.ts index cfa1b91d8..5e3fd741f 100644 --- a/tests/unit/features/onboarding/buddyDrafts.test.ts +++ b/tests/unit/features/onboarding/buddyDrafts.test.ts @@ -32,7 +32,7 @@ describe("buddy drafts", () => { steps: [], questions: [], ...over, - }) as OnboardingPhaseEndpoint; + }) satisfies OnboardingPhaseEndpoint; const step = (over: Partial = {}) => ({ @@ -41,9 +41,18 @@ describe("buddy drafts", () => { position: 0, title: "Clone the repository", description: "", + type: "TASK", + estimatedMinutes: 20, + expectedOutcomes: [], + tasks: [], + resources: [], status: "WAITING", + startedAt: null, + completedAt: null, + feedback: null, + skip: null, ...over, - }) as OnboardingStepEndpoint; + }) satisfies OnboardingStepEndpoint; const question = (over: Partial = {}) => ({ @@ -54,7 +63,7 @@ describe("buddy drafts", () => { question: "Which meeting sets the sprint scope?", status: "OPEN", ...over, - }) as OnboardingQuestionEndpoint; + }) satisfies OnboardingQuestionEndpoint; it("asks in the hire's own voice, not the page's", () => { for (const draft of [ diff --git a/tests/unit/features/onboarding/itemNumbers.test.ts b/tests/unit/features/onboarding/itemNumbers.test.ts new file mode 100644 index 000000000..c5e5e4e82 --- /dev/null +++ b/tests/unit/features/onboarding/itemNumbers.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it } from "vitest"; +import { itemNumbers } from "../../../../src/features/onboarding/itemNumbers"; +import type { OnboardingPhaseEndpoint } from "../../../../src/features/onboarding/types"; + +/** + * The number an item is shown with. + * + * It exists so a hire can say "let's do 3" instead of retyping a title — which only works while this + * agrees with `BuddyPathTools`, where the same rule is written in Kotlin. Steps first in position + * order, then questions. + */ +describe("itemNumbers", () => { + function phase(over: Partial = {}): OnboardingPhaseEndpoint { + return { + id: "p1", + pathId: "path", + position: 0, + title: "Setup", + description: "", + locked: false, + steps: [], + questions: [], + ...over, + }; + } + + const step = (id: string, position: number) => ({ id, position }) as never; + const question = (id: string, position: number) => ({ id, position }) as never; + + it("numbers the steps first, then the questions", () => { + const numbers = itemNumbers( + phase({ + steps: [step("s1", 0), step("s2", 1)], + questions: [question("q1", 0)], + }), + ); + + expect(numbers.get("s1")).toBe(1); + expect(numbers.get("s2")).toBe(2); + // The question continues the same sequence rather than starting a second one, because the page + // shows one list of items and a hire counts down what they see. + expect(numbers.get("q1")).toBe(3); + }); + + it("goes by position, not by the order the payload happened to arrive in", () => { + const numbers = itemNumbers(phase({ steps: [step("late", 5), step("early", 1)] })); + + expect(numbers.get("early")).toBe(1); + expect(numbers.get("late")).toBe(2); + }); + + it("gives steps and questions distinct numbers even when their positions collide", () => { + // They carry their own positions underneath, so a shared one is ordinary. A hire counting down + // one visible list must still get one number per item. + const numbers = itemNumbers( + phase({ steps: [step("s1", 0)], questions: [question("q1", 0)] }), + ); + + expect([...numbers.values()]).toEqual([1, 2]); + }); + + it("is empty for a phase with nothing in it", () => { + expect(itemNumbers(phase()).size).toBe(0); + }); +}); diff --git a/tests/unit/pages/OnBoardingPage.test.tsx b/tests/unit/pages/OnBoardingPage.test.tsx index b2f78ccf1..dfcd622f9 100644 --- a/tests/unit/pages/OnBoardingPage.test.tsx +++ b/tests/unit/pages/OnBoardingPage.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, waitFor } from "@testing-library/react"; +import { render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { describe, it, expect, vi, beforeEach } from "vitest"; import { MemoryRouter } from "react-router-dom"; @@ -474,3 +474,92 @@ describe("OnBoardingPage", () => { expect(screen.queryByRole("heading", { name: "Phase 1" })).not.toBeInTheDocument(); }); }); + +/** + * Following a link the buddy wrote. + * + * The mentor is handed each item's path so that "want to take #3?" can be clickable. A question has + * no route of its own — it is a modal on this page — so it arrives as `?question=`, which also + * means the link survives being copied, kept or opened in a second tab. + */ +describe("OnBoardingPage: links from the buddy", () => { + beforeEach(() => { + vi.clearAllMocks(); + projectContextState.selectedProjectId = "proj1"; + }); + + function pathWithQuestion(status: "OPEN" | "LOCKED" | "PASSED") { + const phase = phaseFixture("phase-2", 1, "Meetings"); + return { + id: "path1", + userId: "user1", + createdAt: new Date().toISOString(), + generationIssues: [], + phases: [ + phaseFixture("phase-1", 0, "Overview"), + { + ...phase, + questions: [ + { + id: "q-linked", + phaseId: phase.id, + position: 1, + type: "SHORT_TEXT", + question: "Who runs the retro?", + options: [], + status, + }, + ], + }, + ], + }; + } + + it("opens the question a link names, on its own phase", async () => { + server.use(http.get("/api/v1/onboarding/me/path", () => HttpResponse.json(pathWithQuestion("OPEN")))); + + render( + + + , + ); + + // The modal, and the phase behind it: a link lands on both, because a question the hire cannot + // see the context of is a link that only half arrived. + const dialog = await screen.findByRole("dialog"); + expect(within(dialog).getByText("Who runs the retro?")).toBeInTheDocument(); + expect(screen.getByRole("heading", { name: "Meetings", level: 2 })).toBeInTheDocument(); + }); + + it("lands on the phase but opens nothing for a question that cannot be answered", async () => { + server.use( + http.get("/api/v1/onboarding/me/path", () => HttpResponse.json(pathWithQuestion("LOCKED"))), + ); + + render( + + + , + ); + + // A modal over a locked question is a link that leads to a dead end; its phase is the useful half. + expect( + await screen.findByRole("heading", { name: "Meetings", level: 2 }), + ).toBeInTheDocument(); + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); + + it("lands on the phase a link names", async () => { + server.use(http.get("/api/v1/onboarding/me/path", () => HttpResponse.json(pathWithQuestion("OPEN")))); + + render( + + + , + ); + + expect( + await screen.findByRole("heading", { name: "Meetings", level: 2 }), + ).toBeInTheDocument(); + }); +}); From ba829f664f8da2486231ee282da35f90346a9e9a Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Sun, 13 Sep 2026 20:05:42 +0200 Subject: [PATCH 03/20] Keep the team page standing, and number what the buddy numbers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - **The member's path is normalised where it comes off the wire.** A phase without `questions` took the whole team page down with a TypeError: three surfaces read it because the type promised it, and the endpoint did not deliver. The backend now sends it, and the default here means no caller downstream has to defend itself against the same shape drift again. - **The badge says who added a step**: the buddy, the hire, or a PM. It used to call all three "Custom step by PM", which is the one of the three that matters — it is what the team requires — so a step agreed to in a chat was arriving as an instruction from above. - **Numbers in the graph view and on the step page**, from the same `itemNumbers` rule as the list. A number the buddy uses and the hire cannot see on the page they are looking at is worse than no number; the step page pays one path read for it, because a step cannot know its own place among its siblings. Co-Authored-By: Claude Opus 5 --- .../components/OnBoardingItemPage.tsx | 38 ++++++++++++- .../components/OnboardingGraphViewer.tsx | 5 ++ .../onboarding/components/StepOriginBadge.tsx | 53 ++++++++++++++++--- src/features/onboarding/types.ts | 10 ++++ src/services/teamManagementService.ts | 30 +++++++---- .../components/StepOriginBadge.test.tsx | 49 +++++++++++++++++ 6 files changed, 167 insertions(+), 18 deletions(-) create mode 100644 tests/unit/features/onboarding/components/StepOriginBadge.test.tsx diff --git a/src/features/onboarding/components/OnBoardingItemPage.tsx b/src/features/onboarding/components/OnBoardingItemPage.tsx index a4bf6c542..45ab4acee 100644 --- a/src/features/onboarding/components/OnBoardingItemPage.tsx +++ b/src/features/onboarding/components/OnBoardingItemPage.tsx @@ -41,6 +41,7 @@ import { resolveNextAction } from "../nextAction"; import { AskTheBuddy } from "../../buddy/components/AskTheBuddy"; import { onBuddyPathChanged } from "../../buddy/aiBuddyBus"; import { askAboutStep } from "../buddyDrafts"; +import { itemNumbers } from "../itemNumbers"; type LoadingState = "idle" | "loading" | "success" | "error"; @@ -127,6 +128,34 @@ export function OnBoardingItemPage() { const { flyby } = useMoments(); + /** + * The number this step wears on the overview, so the two pages call it the same thing. + * + * Costs one read of the path, because a number is a fact about a step's *place among its + * siblings* and a single step cannot know it. Worth the request: the number is how a hire refers to + * this step when they ask their buddy about it, and a page that showed a different one — or none — + * would make that referring useless. + */ + const [stepNumber, setStepNumber] = useState(null); + + useEffect(() => { + if (!stepId) return; + + void (async () => { + try { + const path = await onboardingService.fetchPath(); + const phase = path.phases.find((candidate) => + candidate.steps.some((step) => step.id === stepId), + ); + setStepNumber(phase ? (itemNumbers(phase).get(stepId) ?? null) : null); + } catch { + // A number is decoration next to the title; a page that cannot fetch the path still shows + // the step. Silent on purpose -- the loader below reports anything that actually matters. + setStepNumber(null); + } + })(); + }, [stepId]); + /** * Works out what comes after this step, once the step is behind the user. * @@ -450,7 +479,14 @@ export function OnBoardingItemPage() { : "Open"}

-

{stepDetail.title}

+

+ {stepNumber !== null && ( + + #{stepNumber} + + )} + {stepDetail.title} +

diff --git a/src/features/onboarding/components/OnboardingGraphViewer.tsx b/src/features/onboarding/components/OnboardingGraphViewer.tsx index 991d8e75a..4582b7635 100644 --- a/src/features/onboarding/components/OnboardingGraphViewer.tsx +++ b/src/features/onboarding/components/OnboardingGraphViewer.tsx @@ -12,6 +12,7 @@ import type { OnboardingStepEndpoint, StepStatus, } from "../types.ts"; +import { itemNumbers } from "../itemNumbers.ts"; type OnboardingGraphNode = BlueprintGraphCanvasNode & { kind: "phase" | "step" | "question"; @@ -107,11 +108,15 @@ export function OnboardingGraphViewer({ path, selectedPhaseId, onSelectPhase }: blockerIds: question.blockerIds, })); const nodes = [...steps, ...questions]; + // The same numbers the list view prints and the buddy uses, so a node here can be talked about + // by the number the hire can see on it — which is the entire point of numbering them. + const numbers = itemNumbers(subGraphPhase); return nodes.map((node, index) => { const fallback = fallbackCoordinate(index, nodes.length); return { ...node, + title: `#${numbers.get(node.id)} ${node.title}`, graphX: node.graphX ?? fallback.x, graphY: node.graphY ?? fallback.y, blockerIds: node.blockerIds ?? (index > 0 ? [nodes[index - 1].id] : []), diff --git a/src/features/onboarding/components/StepOriginBadge.tsx b/src/features/onboarding/components/StepOriginBadge.tsx index 59baf6ed4..941acbb27 100644 --- a/src/features/onboarding/components/StepOriginBadge.tsx +++ b/src/features/onboarding/components/StepOriginBadge.tsx @@ -1,4 +1,4 @@ -import { UserRound } from "lucide-react"; +import { MessageCircle, Sparkles, UserRound } from "lucide-react"; import { Badge } from "../../../components/ui/Badge"; import type { OnboardingStepEndpoint } from "../types"; @@ -6,13 +6,50 @@ type StepOriginBadgeProps = { step: OnboardingStepEndpoint; }; +/** + * Where a step came from, said on the card. + * + * It used to read this off `isAiAssisted`, which has two values and three answers to give: anything + * not AI-generated was labelled "Custom step by PM", so a step the hire wrote themselves — and later + * a step their buddy proposed — both arrived claiming the team had prescribed it. A hire who cannot + * tell what their team requires from what they agreed to in a chat has lost the distinction this + * badge exists for. + * + * A generated step wears nothing. It is the ordinary case, and the whole path would otherwise carry + * the same badge on every card, which is a label for the page rather than for a step. + * + * **The `isAiAssisted` fallback stays** for rows written before `origin` existed: those default to + * `GENERATED` in the database while `isAiAssisted` still records that a person authored them. Their + * badge is the one they have always had. + */ export function StepOriginBadge({ step }: StepOriginBadgeProps) { - if (step.isAiAssisted !== false) return null; + if (step.origin === "BUDDY") { + return ( + + + Added with your buddy + + ); + } - return ( - - - Custom step by PM - - ); + if (step.origin === "HIRE") { + return ( + + + You added this + + ); + } + + // PM, or a pre-`origin` row that a person authored. + if (step.origin === "PM" || step.isAiAssisted === false) { + return ( + + + Custom step by PM + + ); + } + + return null; } diff --git a/src/features/onboarding/types.ts b/src/features/onboarding/types.ts index fc30e3d96..bb19a1bd5 100644 --- a/src/features/onboarding/types.ts +++ b/src/features/onboarding/types.ts @@ -45,11 +45,21 @@ export interface OnboardingStepSkip { reviewedAt: string | null; } +/** Who put a step on a path — see the backend's `StepOrigin`. */ +export type StepOrigin = "GENERATED" | "PM" | "HIRE" | "BUDDY"; + export interface OnboardingStepEndpoint { id: string; phaseId: string; position: number; isAiAssisted?: boolean; + /** + * Who put this step on the path. + * + * Optional because a row written before the column existed carries none; `StepOriginBadge` falls + * back to `isAiAssisted` for those, which is the only signal they have. + */ + origin?: StepOrigin; title: string; description: string; type: StepType; diff --git a/src/services/teamManagementService.ts b/src/services/teamManagementService.ts index 9033b5e4f..174731d59 100644 --- a/src/services/teamManagementService.ts +++ b/src/services/teamManagementService.ts @@ -337,6 +337,20 @@ export async function markOnboardingFeedbackRead(feedbackId: string): Promise { @@ -354,22 +368,20 @@ export async function getUserOnboardingPath( const hydratedPhases = await Promise.all( phases.map(async (phase) => { - if (phase.steps?.length > 0) return phase; + // Questions cannot be hydrated the way steps can: no endpoint hands out one member's + // questions with their status. An empty list is the honest stand-in, and it keeps the page + // standing instead of taking it down. + const normalised = { ...phase, questions: phase.questions ?? [] }; + if (normalised.steps?.length > 0) return normalised; try { const steps = await apiClient.fetch( `/api/v1/onboarding/phases/${phase.id}/steps`, ); - return { - ...phase, - steps, - }; + return { ...normalised, steps }; } catch { - return { - ...phase, - steps: [], - }; + return { ...normalised, steps: [] }; } }), ); diff --git a/tests/unit/features/onboarding/components/StepOriginBadge.test.tsx b/tests/unit/features/onboarding/components/StepOriginBadge.test.tsx new file mode 100644 index 000000000..0bb4fa66a --- /dev/null +++ b/tests/unit/features/onboarding/components/StepOriginBadge.test.tsx @@ -0,0 +1,49 @@ +import { render, screen } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; +import { StepOriginBadge } from "../../../../../src/features/onboarding/components/StepOriginBadge"; +import type { OnboardingStepEndpoint } from "../../../../../src/features/onboarding/types"; + +/** + * Who put a step on the path, said on the card. + * + * The badge used to read this off `isAiAssisted`, which has two values and three answers to give: + * everything not AI-generated was "Custom step by PM". So a step the hire wrote, and later a step + * their buddy proposed, both claimed their team required it — and a hire who cannot tell what the + * team requires from what they agreed to in a chat has lost the distinction the badge is for. + */ +describe("StepOriginBadge", () => { + const step = (over: Partial) => ({ id: "s1", ...over }) as never; + + it("says the buddy added it, not a PM", () => { + render(); + + expect(screen.getByText("Added with your buddy")).toBeInTheDocument(); + expect(screen.queryByText("Custom step by PM")).not.toBeInTheDocument(); + }); + + it("says the hire added it themselves", () => { + render(); + + expect(screen.getByText("You added this")).toBeInTheDocument(); + }); + + it("keeps the PM badge for what a PM prescribed", () => { + render(); + + expect(screen.getByText("Custom step by PM")).toBeInTheDocument(); + }); + + it("wears nothing on a generated step, which is most of the path", () => { + const { container } = render(); + + expect(container).toBeEmptyDOMElement(); + }); + + it("still reads a pre-origin row off isAiAssisted", () => { + // Rows written before the column default to GENERATED in the database, while `isAiAssisted` + // still records that a person authored them. Their badge is the one they always had. + render(); + + expect(screen.getByText("Custom step by PM")).toBeInTheDocument(); + }); +}); From 8bca818fc14218f6e89bc1f7eb4b3fdd4e25bf7a Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Mon, 14 Sep 2026 13:55:58 +0200 Subject: [PATCH 04/20] Name the hire, not "you", when a PM reads the step badge The origin badge also sits on the team page, where "You added this" and "Added with your buddy" read as being about the reviewer. The reviewer view now says "Added by the hire" and "Added with the buddy". Also formats the branch's onboarding files with Prettier. Co-Authored-By: Claude Opus 5 --- src/features/onboarding/buddyDrafts.ts | 11 +++++----- .../onboarding/components/StepOriginBadge.tsx | 20 ++++++++++++------- .../detail/MemberOnboardingSection.tsx | 2 +- .../components/detail/StepDetailsPanel.tsx | 2 +- .../features/onboarding/buddyDrafts.test.ts | 4 +++- .../components/StepOriginBadge.test.tsx | 13 ++++++++++++ .../features/onboarding/itemNumbers.test.ts | 4 +--- tests/unit/pages/OnBoardingPage.test.tsx | 16 +++++++-------- 8 files changed, 46 insertions(+), 26 deletions(-) diff --git a/src/features/onboarding/buddyDrafts.ts b/src/features/onboarding/buddyDrafts.ts index 0945a2ce6..27d0458da 100644 --- a/src/features/onboarding/buddyDrafts.ts +++ b/src/features/onboarding/buddyDrafts.ts @@ -6,7 +6,11 @@ // for them. // ============================================================ -import type { OnboardingPhaseEndpoint, OnboardingQuestionEndpoint, OnboardingStepEndpoint } from "./types"; +import type { + OnboardingPhaseEndpoint, + OnboardingQuestionEndpoint, + OnboardingStepEndpoint, +} from "./types"; /** * Taking what you are looking at on your path into the conversation. @@ -68,10 +72,7 @@ export function askAboutStep(step: OnboardingStepEndpoint): string { * a refusal as their first experience of the feature, and this is the cheapest way to not start * there. */ -export function askAboutQuestion( - question: OnboardingQuestionEndpoint, - phaseTitle: string, -): string { +export function askAboutQuestion(question: OnboardingQuestionEndpoint, phaseTitle: string): string { return `I'm stuck on the knowledge question "${snippet(question.question)}" in "${snippet(phaseTitle)}". Can you go through the material with me? I'd rather work the answer out than be told it.`; } diff --git a/src/features/onboarding/components/StepOriginBadge.tsx b/src/features/onboarding/components/StepOriginBadge.tsx index 941acbb27..864dab4a8 100644 --- a/src/features/onboarding/components/StepOriginBadge.tsx +++ b/src/features/onboarding/components/StepOriginBadge.tsx @@ -4,6 +4,11 @@ import type { OnboardingStepEndpoint } from "../types"; type StepOriginBadgeProps = { step: OnboardingStepEndpoint; + /** + * Who is looking. The hire reads "you" and "your buddy"; a PM reviewing somebody else's path + * would read those as being about themselves, so the reviewer view names the hire instead. + */ + viewer?: "hire" | "reviewer"; }; /** @@ -18,16 +23,17 @@ type StepOriginBadgeProps = { * A generated step wears nothing. It is the ordinary case, and the whole path would otherwise carry * the same badge on every card, which is a label for the page rather than for a step. * - * **The `isAiAssisted` fallback stays** for rows written before `origin` existed: those default to - * `GENERATED` in the database while `isAiAssisted` still records that a person authored them. Their - * badge is the one they have always had. + * **The `isAiAssisted` fallback stays**, for two kinds of step that arrive as `GENERATED` with + * `isAiAssisted` false: a step copied from a blueprint step the PM wrote by hand (the copy keeps the + * blueprint's flag), and a row written before `origin` existed. Both were authored by a person on + * the team, and both keep the badge they have always had. */ -export function StepOriginBadge({ step }: StepOriginBadgeProps) { +export function StepOriginBadge({ step, viewer = "hire" }: StepOriginBadgeProps) { if (step.origin === "BUDDY") { return ( - Added with your buddy + {viewer === "hire" ? "Added with your buddy" : "Added with the buddy"} ); } @@ -36,12 +42,12 @@ export function StepOriginBadge({ step }: StepOriginBadgeProps) { return ( - You added this + {viewer === "hire" ? "You added this" : "Added by the hire"} ); } - // PM, or a pre-`origin` row that a person authored. + // PM, or a hand-written blueprint copy or pre-`origin` row -- see above. if (step.origin === "PM" || step.isAiAssisted === false) { return ( diff --git a/src/features/team-management/components/detail/MemberOnboardingSection.tsx b/src/features/team-management/components/detail/MemberOnboardingSection.tsx index 7179b65cb..4110fede1 100644 --- a/src/features/team-management/components/detail/MemberOnboardingSection.tsx +++ b/src/features/team-management/components/detail/MemberOnboardingSection.tsx @@ -569,7 +569,7 @@ function StepCard({ {step.title}
- +
diff --git a/src/features/team-management/components/detail/StepDetailsPanel.tsx b/src/features/team-management/components/detail/StepDetailsPanel.tsx index eef6dabd0..2398d5179 100644 --- a/src/features/team-management/components/detail/StepDetailsPanel.tsx +++ b/src/features/team-management/components/detail/StepDetailsPanel.tsx @@ -111,7 +111,7 @@ export function StepDetailsPanel({ > {step.status.replace("_", " ")} - + } panelBackgroundClassName="bg-app-surface" diff --git a/tests/unit/features/onboarding/buddyDrafts.test.ts b/tests/unit/features/onboarding/buddyDrafts.test.ts index 5e3fd741f..3f55c4a54 100644 --- a/tests/unit/features/onboarding/buddyDrafts.test.ts +++ b/tests/unit/features/onboarding/buddyDrafts.test.ts @@ -81,7 +81,9 @@ describe("buddy drafts", () => { it("names the thing it is about, so the mentor does not have to ask", () => { expect(askAboutPhase(phase())).toContain("Environment Setup"); expect(askAboutStep(step())).toContain("Clone the repository"); - expect(askAboutQuestion(question(), "Meetings")).toContain("Which meeting sets the sprint scope?"); + expect(askAboutQuestion(question(), "Meetings")).toContain( + "Which meeting sets the sprint scope?", + ); expect(askAboutQuestion(question(), "Meetings")).toContain("Meetings"); }); diff --git a/tests/unit/features/onboarding/components/StepOriginBadge.test.tsx b/tests/unit/features/onboarding/components/StepOriginBadge.test.tsx index 0bb4fa66a..46ec7e9be 100644 --- a/tests/unit/features/onboarding/components/StepOriginBadge.test.tsx +++ b/tests/unit/features/onboarding/components/StepOriginBadge.test.tsx @@ -27,6 +27,19 @@ describe("StepOriginBadge", () => { expect(screen.getByText("You added this")).toBeInTheDocument(); }); + it("names the hire, not 'you', for a PM reviewing somebody else's path", () => { + // The same badge sits on the team page, where "You added this" would read as the PM's own step. + render(); + expect(screen.getByText("Added by the hire")).toBeInTheDocument(); + expect(screen.queryByText("You added this")).not.toBeInTheDocument(); + }); + + it("says the buddy without 'your' in the reviewer view", () => { + render(); + + expect(screen.getByText("Added with the buddy")).toBeInTheDocument(); + }); + it("keeps the PM badge for what a PM prescribed", () => { render(); diff --git a/tests/unit/features/onboarding/itemNumbers.test.ts b/tests/unit/features/onboarding/itemNumbers.test.ts index c5e5e4e82..53411a4e6 100644 --- a/tests/unit/features/onboarding/itemNumbers.test.ts +++ b/tests/unit/features/onboarding/itemNumbers.test.ts @@ -52,9 +52,7 @@ describe("itemNumbers", () => { it("gives steps and questions distinct numbers even when their positions collide", () => { // They carry their own positions underneath, so a shared one is ordinary. A hire counting down // one visible list must still get one number per item. - const numbers = itemNumbers( - phase({ steps: [step("s1", 0)], questions: [question("q1", 0)] }), - ); + const numbers = itemNumbers(phase({ steps: [step("s1", 0)], questions: [question("q1", 0)] })); expect([...numbers.values()]).toEqual([1, 2]); }); diff --git a/tests/unit/pages/OnBoardingPage.test.tsx b/tests/unit/pages/OnBoardingPage.test.tsx index dfcd622f9..7eb1a2225 100644 --- a/tests/unit/pages/OnBoardingPage.test.tsx +++ b/tests/unit/pages/OnBoardingPage.test.tsx @@ -516,7 +516,9 @@ describe("OnBoardingPage: links from the buddy", () => { } it("opens the question a link names, on its own phase", async () => { - server.use(http.get("/api/v1/onboarding/me/path", () => HttpResponse.json(pathWithQuestion("OPEN")))); + server.use( + http.get("/api/v1/onboarding/me/path", () => HttpResponse.json(pathWithQuestion("OPEN"))), + ); render( @@ -543,14 +545,14 @@ describe("OnBoardingPage: links from the buddy", () => { ); // A modal over a locked question is a link that leads to a dead end; its phase is the useful half. - expect( - await screen.findByRole("heading", { name: "Meetings", level: 2 }), - ).toBeInTheDocument(); + expect(await screen.findByRole("heading", { name: "Meetings", level: 2 })).toBeInTheDocument(); expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); }); it("lands on the phase a link names", async () => { - server.use(http.get("/api/v1/onboarding/me/path", () => HttpResponse.json(pathWithQuestion("OPEN")))); + server.use( + http.get("/api/v1/onboarding/me/path", () => HttpResponse.json(pathWithQuestion("OPEN"))), + ); render( @@ -558,8 +560,6 @@ describe("OnBoardingPage: links from the buddy", () => { , ); - expect( - await screen.findByRole("heading", { name: "Meetings", level: 2 }), - ).toBeInTheDocument(); + expect(await screen.findByRole("heading", { name: "Meetings", level: 2 })).toBeInTheDocument(); }); }); From 8e0ec3e3c04907a21aaa9e87fc7e9719e7c36b9e Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Mon, 14 Sep 2026 15:28:44 +0200 Subject: [PATCH 05/20] Make picked suggestions sendable, and land buddy links on the card Picking a suggestion filled the composer but left focus on the chip, so Enter sent nothing. Both composers now take the caret whenever text arrives from outside rather than from typing -- chips, hand-offs from "Ask your buddy", any of them. A step or question link from the buddy now opens the item's phase, scrolls to its card and lights it up briefly, instead of opening a question modal. Starting the step or answering the question stays the hire's own click. Co-Authored-By: Claude Opus 5 --- .../buddy/components/BuddyComposer.tsx | 27 ++++- .../chatbot/components/ChatComposer.tsx | 24 +++- src/pages/OnBoardingPage.tsx | 105 +++++++++++------- src/styles/index.css | 34 ++++++ .../features/buddy/BuddyComposer.test.tsx | 46 ++++++++ tests/unit/pages/OnBoardingPage.test.tsx | 42 ++++--- 6 files changed, 218 insertions(+), 60 deletions(-) create mode 100644 tests/unit/features/buddy/BuddyComposer.test.tsx diff --git a/src/features/buddy/components/BuddyComposer.tsx b/src/features/buddy/components/BuddyComposer.tsx index 6875a604c..de00fb6f3 100644 --- a/src/features/buddy/components/BuddyComposer.tsx +++ b/src/features/buddy/components/BuddyComposer.tsx @@ -58,6 +58,28 @@ export function BuddyComposer({ field.setSelectionRange(field.value.length, field.value.length); }, [focusOnMount]); + /** + * The last value the hire typed here, so a draft written *from outside* can be told apart. + * + * A suggestion chip, or "Ask your buddy about this step" while the dock is already open, sets the + * draft without touching the box — and focus stayed on whatever was clicked. Pressing Enter then + * re-clicked the chip instead of sending, so a filled-in question looked unsendable. Any draft + * that did not come from typing takes the caret, behind the text, which is where it has to be for + * Enter to send it and for typing to add to it. + */ + const typedRef = useRef(draft); + + useEffect(() => { + if (draft === typedRef.current) return; + typedRef.current = draft; + // Cleared after a send: nothing to hand over, and the caret is not wanted back mid-turn. + if (!draft) return; + const field = fieldRef.current; + if (!field) return; + field.focus(); + field.setSelectionRange(field.value.length, field.value.length); + }, [draft]); + const handleKeyDown = (event: KeyboardEvent) => { if (event.key !== "Enter" || event.shiftKey) return; // Enter also *commits* an IME candidate — a compose-key 'ü', or any CJK input. Sending @@ -85,7 +107,10 @@ export function BuddyComposer({ value={draft} rows={1} placeholder={placeholder} - onChange={(event) => setDraft(event.target.value)} + onChange={(event) => { + typedRef.current = event.target.value; + setDraft(event.target.value); + }} onKeyDown={handleKeyDown} className="min-w-0 flex-1 resize-none overflow-y-auto bg-transparent px-2 py-1.5 text-sm text-app-text outline-none placeholder:text-app-text-disabled" /> diff --git a/src/features/chatbot/components/ChatComposer.tsx b/src/features/chatbot/components/ChatComposer.tsx index 8d903894e..40938c5d1 100644 --- a/src/features/chatbot/components/ChatComposer.tsx +++ b/src/features/chatbot/components/ChatComposer.tsx @@ -1,5 +1,5 @@ import { Check, Filter, Send, Square, X } from "lucide-react"; -import { useState } from "react"; +import { useEffect, useRef, useState } from "react"; import type { FormEvent, RefObject } from "react"; import { SOURCE_META } from "../../data-ingestion/data"; import type { SourceSystem } from "../types"; @@ -111,6 +111,27 @@ export function ChatComposer({ }; const blocked = rangeInvalid || !hasProject; + /* + The last value typed into the box, so a value set *from outside* can be told apart. + + A suggestion chip fills the composer without touching it, and a hire who picked one and pressed + Enter found nothing sent. Owning the focus here, rather than in each page that can fill the box, + means every way of handing text over behaves the same: any value that did not come from typing + takes the caret, behind the text, so Enter sends it and typing adds to it. + */ + const typedRef = useRef(value); + + useEffect(() => { + if (value === typedRef.current) return; + typedRef.current = value; + // Cleared after a send: the page blurs the box on purpose so Space can start the game. + if (!value) return; + const element = textareaRef.current; + if (!element) return; + element.focus(); + element.setSelectionRange(element.value.length, element.value.length); + }, [value, textareaRef]); + return (
{showFilters && ( @@ -292,6 +313,7 @@ export function ChatComposer({ value={value} rows={1} onChange={(e) => { + typedRef.current = e.currentTarget.value; onChange(e.currentTarget.value); e.currentTarget.style.height = "auto"; e.currentTarget.style.height = `${e.currentTarget.scrollHeight}px`; diff --git a/src/pages/OnBoardingPage.tsx b/src/pages/OnBoardingPage.tsx index 321ccdcc7..df885c84f 100644 --- a/src/pages/OnBoardingPage.tsx +++ b/src/pages/OnBoardingPage.tsx @@ -80,6 +80,11 @@ function ProgressBar({ value, max }: ProgressBarProps) { // MAIN COMPONENT: OnBoardingPage // ───────────────────────────────────────────────────────────── +/** The DOM id of a step or question card, which a link from the buddy scrolls to. */ +function linkedCardId(itemId: string): string { + return `onboarding-item-${itemId}`; +} + /** * Displays the user's personalized onboarding path hierarchy. * Fetches and tracks progress through phases, steps and knowledge-check @@ -141,17 +146,23 @@ export function OnBoardingPage() { /** * Where a link from the buddy points. * - * The mentor is given each item's path so it can say "want to take [#3](...)?" and have that be - * clickable. A question has no route of its own — it is a modal on this page — so it arrives as - * `?question=`, and a phase as `?phase=`. + * The mentor is given each item's link so it can say "you are on [#3](...)" and have that be + * clickable: `?step=`, `?question=` or `?phase=`. + * + * A step or question link *lands* on the item rather than starting it — the right phase opens, + * the page scrolls to the card, and the card lights up briefly. Following a link in a + * conversation is a way of finding something, and starting a step (or opening a question to + * answer) is the hire's own click on the card they can now see. * * In the URL rather than in router state, unlike `focusQuestionId`: this link is written by the * model into text the hire can copy, keep, or open in a second tab, and state does not survive any * of that. */ const [searchParams, setSearchParams] = useSearchParams(); + const linkedStepId = searchParams.get("step"); const linkedQuestionId = searchParams.get("question"); const linkedPhaseId = searchParams.get("phase"); + const linkedItemId = linkedStepId ?? linkedQuestionId; // The question list of the focused phase, so the page can scroll to it. const questionListRef = useRef(null); @@ -211,9 +222,6 @@ export function OnBoardingPage() { }) => { const phases = OnBoardingPathEndpoint?.phases ?? []; setQuestionToAnswer(null); - // Also the link, if that is what opened it — otherwise closing the modal would reopen it on the - // next render, since the URL would still be asking for it. - if (linkedQuestionId) clearLink(); // The backend decides completion; nothing here is derived from the phase alone. if (onboardingCompleted) { @@ -340,29 +348,32 @@ export function OnBoardingPage() { useEffect(() => onBuddyPathChanged(() => void refreshPath()), []); /** - * The phase and the question a buddy link names, resolved from the URL rather than copied into - * state. + * The phase a buddy link names, resolved from the URL rather than copied into state. * * Derived on purpose. A link can arrive two ways — a fresh mount, or a click while the hire is * already standing on this page — and writing state from an effect would both trip the * set-state-in-an-effect rule and only handle the first. Deriving handles both and needs no - * clean-up: the link stops winning the moment the hire picks a different phase or closes the - * modal, because those clear the parameter. + * clean-up: the link stops winning the moment the hire picks a different phase or view, because + * those clear the parameter. */ const linkedPhaseIndex = useMemo(() => { const phases = OnBoardingPathEndpoint?.phases ?? []; + if (linkedStepId) { + return phases.findIndex((phase) => phase.steps.some((step) => step.id === linkedStepId)); + } if (linkedQuestionId) { return phases.findIndex((phase) => phase.questions.some((question) => question.id === linkedQuestionId), ); } return linkedPhaseId ? phases.findIndex((phase) => phase.id === linkedPhaseId) : -1; - }, [OnBoardingPathEndpoint, linkedQuestionId, linkedPhaseId]); + }, [OnBoardingPathEndpoint, linkedStepId, linkedQuestionId, linkedPhaseId]); /** Forgets the link, so the hire's own next click decides what they are looking at. */ const clearLink = () => setSearchParams( (params) => { + params.delete("step"); params.delete("question"); params.delete("phase"); return params; @@ -424,25 +435,31 @@ export function OnBoardingPage() { ) .join(", "); + // A link names a card in the list, so it shows the list even if the hire had left it on the graph. + const shownViewMode = linkedItemId ? "list" : viewMode; + /** - * The question the modal is showing: the one a card opened, or the one a link names. + * Scrolls to the card a link landed on. * - * A linked question only opens when the hire could actually answer it. A link to a locked or - * already-passed question is a dead end, and landing on its phase — which still happens — is the - * useful half of following it. + * Keyed on `location.key` as well as the id, so following the same link a second time — the hire + * scrolled away and clicked it again in the conversation — scrolls again. Only a scroll, never + * state: which phase is open and which card lights up are both derived from the URL. */ - const linkedQuestion = - linkedQuestionId && currentPhase - ? currentPhase.questions.find( - (question) => - question.id === linkedQuestionId && - question.status !== "LOCKED" && - question.status !== "PASSED", - ) - : undefined; - const shownQuestion = - questionToAnswer ?? - (linkedQuestion ? { question: linkedQuestion, phaseTitle: currentPhase?.title ?? "" } : null); + useEffect(() => { + if (loadingState !== "success" || !linkedItemId) return; + document + .getElementById(linkedCardId(linkedItemId)) + ?.scrollIntoView?.({ behavior: "smooth", block: "center" }); + }, [loadingState, linkedItemId, location.key]); + + /** + * What a card needs to be the one a link landed on: an id to scroll to, a key that restarts the + * light when the same link is followed again, and the class that plays it. + */ + const linkedCard = (id: string) => + id === linkedItemId + ? { id: linkedCardId(id), key: `${id}:${location.key}`, highlight: " app-link-highlight" } + : { id: linkedCardId(id), key: id, highlight: "" }; // The numbers this phase's items are shown with. The buddy's path tool derives the same ones, so // "let's do 3" means one item on both sides — see `itemNumbers`. @@ -798,8 +815,8 @@ export function OnBoardingPage() {
- {viewMode === "graph" ? ( + {shownViewMode === "graph" ? ( phase.id === phaseId, ); if (phaseIndex >= 0) setSelectedPhaseIndex(phaseIndex); + clearLink(); }} /> ) : ( @@ -928,11 +949,12 @@ export function OnBoardingPage() { const mode = getStepMode(step, currentPhase.locked); return (
- {/* Per-question answer modal. Opened by a card, or by a link the buddy wrote. */} - {shownQuestion && ( + {/* Per-question answer modal. Opened by the card's own button, never by a link. */} + {questionToAnswer && ( )} diff --git a/src/styles/index.css b/src/styles/index.css index 41db43ad3..8cdb307f2 100644 --- a/src/styles/index.css +++ b/src/styles/index.css @@ -924,3 +924,37 @@ body { animation: none; } } + +/* The brief light on an onboarding step or question that a link from the buddy landed on. + + The link opens the right phase and scrolls to the item, and on a list of look-alike cards + that is not quite enough to find it: this says "this one" twice and then gets out of the way. + Deliberately not a way of starting anything -- the hire presses the item's own button. + + A ring rather than a background, so it reads over any card state (open, locked, done), and + delayed a beat so it plays after the smooth scroll has arrived rather than during it. */ +@keyframes app-link-highlight { + 0%, + 100% { + box-shadow: 0 0 0 0 transparent; + } + 25%, + 65% { + box-shadow: 0 0 0 3px var(--color-app-brand); + } + 45% { + box-shadow: 0 0 0 1px var(--color-app-brand-border); + } +} + +.app-link-highlight { + animation: app-link-highlight 1.6s ease-in-out 0.35s 1 both; +} + +@media (prefers-reduced-motion: reduce) { + /* No pulse, but still marked: finding the item is the point, not the motion. */ + .app-link-highlight { + animation: none; + box-shadow: 0 0 0 2px var(--color-app-brand); + } +} diff --git a/tests/unit/features/buddy/BuddyComposer.test.tsx b/tests/unit/features/buddy/BuddyComposer.test.tsx new file mode 100644 index 000000000..9927287ae --- /dev/null +++ b/tests/unit/features/buddy/BuddyComposer.test.tsx @@ -0,0 +1,46 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { useState } from "react"; +import { describe, expect, it, vi } from "vitest"; +import { BuddyComposer } from "../../../../src/features/buddy/components/BuddyComposer"; + +/** + * A question handed to the composer from outside has to be sendable with Enter. + * + * A suggestion chip (or "Ask your buddy about this step" with the dock already open) sets the draft + * without touching the box, and focus stayed on the chip — so Enter clicked the chip again and + * nothing was sent. + */ +describe("BuddyComposer", () => { + function Harness({ onSend }: { onSend: (text: string) => void }) { + const [draft, setDraft] = useState(""); + return ( + <> + + { + event.preventDefault(); + onSend(draft); + setDraft(""); + }} + /> + + ); + } + + it("takes the caret when a chip fills it, so Enter sends the question", async () => { + const user = userEvent.setup(); + const onSend = vi.fn(); + render(); + + await user.click(screen.getByRole("button", { name: "chip" })); + + expect(screen.getByRole("textbox", { name: "Message" })).toHaveFocus(); + await user.keyboard("{Enter}"); + expect(onSend).toHaveBeenCalledWith("Where am I on my path?"); + }); +}); diff --git a/tests/unit/pages/OnBoardingPage.test.tsx b/tests/unit/pages/OnBoardingPage.test.tsx index 7eb1a2225..8413762a1 100644 --- a/tests/unit/pages/OnBoardingPage.test.tsx +++ b/tests/unit/pages/OnBoardingPage.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, waitFor, within } from "@testing-library/react"; +import { render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { describe, it, expect, vi, beforeEach } from "vitest"; import { MemoryRouter } from "react-router-dom"; @@ -478,9 +478,10 @@ describe("OnBoardingPage", () => { /** * Following a link the buddy wrote. * - * The mentor is handed each item's path so that "want to take #3?" can be clickable. A question has - * no route of its own — it is a modal on this page — so it arrives as `?question=`, which also - * means the link survives being copied, kept or opened in a second tab. + * The mentor is handed each item's link so that "you are on #3" can be clickable: `?step=`, + * `?question=` or `?phase=`, which also means the link survives being copied, kept or + * opened in a second tab. A step or question link *lands* on the card — its phase opens, the page + * scrolls to it, and it lights up — and starts nothing: that stays the hire's own click. */ describe("OnBoardingPage: links from the buddy", () => { beforeEach(() => { @@ -515,38 +516,45 @@ describe("OnBoardingPage: links from the buddy", () => { }; } - it("opens the question a link names, on its own phase", async () => { + it("lands on a linked question and lights it up, without opening it", async () => { + const scrollIntoView = vi.spyOn(HTMLElement.prototype, "scrollIntoView"); server.use( http.get("/api/v1/onboarding/me/path", () => HttpResponse.json(pathWithQuestion("OPEN"))), ); - render( + const { container } = render( , ); - // The modal, and the phase behind it: a link lands on both, because a question the hire cannot - // see the context of is a link that only half arrived. - const dialog = await screen.findByRole("dialog"); - expect(within(dialog).getByText("Who runs the retro?")).toBeInTheDocument(); - expect(screen.getByRole("heading", { name: "Meetings", level: 2 })).toBeInTheDocument(); + // Its phase, because a card the hire cannot see the context of is a link that only half arrived. + expect(await screen.findByRole("heading", { name: "Meetings", level: 2 })).toBeInTheDocument(); + const card = container.querySelector("#onboarding-item-q-linked"); + expect(card).toHaveClass("app-link-highlight"); + await waitFor(() => expect(scrollIntoView).toHaveBeenCalled()); + // Answering is the hire's click on the card, not something following a link does for them. + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); }); - it("lands on the phase but opens nothing for a question that cannot be answered", async () => { + it("lands on a linked step in a later phase and lights only that card", async () => { server.use( - http.get("/api/v1/onboarding/me/path", () => HttpResponse.json(pathWithQuestion("LOCKED"))), + http.get("/api/v1/onboarding/me/path", () => HttpResponse.json(pathWithQuestion("OPEN"))), ); - render( - + const { container } = render( + , ); - // A modal over a locked question is a link that leads to a dead end; its phase is the useful half. expect(await screen.findByRole("heading", { name: "Meetings", level: 2 })).toBeInTheDocument(); - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + expect(container.querySelector("#onboarding-item-step-phase-2")).toHaveClass( + "app-link-highlight", + ); + expect(container.querySelector("#onboarding-item-q-linked")).not.toHaveClass( + "app-link-highlight", + ); }); it("lands on the phase a link names", async () => { From cc047bbe779bbda59552542022643dcf1e877d4c Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Mon, 14 Sep 2026 15:52:12 +0200 Subject: [PATCH 06/20] Carry the skip reason through buddy proposals and show it before sending request_skip's reason rides the proposal and the confirm payload like every other action field, and the proposal shows it in full under the button, since it goes to the PM in the hire's name. Confirming refreshes the path and the step page, which now picks up the pending reason too. Co-Authored-By: Claude Opus 5 --- .../buddy/components/BuddyActionProposals.tsx | 7 ++++++ .../buddy/hooks/useBuddyConversation.ts | 2 ++ src/features/buddy/types.ts | 7 ++++++ .../components/OnBoardingItemPage.tsx | 2 ++ src/services/buddyService.ts | 9 ++++++-- .../buddy/BuddyActionProposals.test.tsx | 22 +++++++++++++++++++ 6 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/features/buddy/components/BuddyActionProposals.tsx b/src/features/buddy/components/BuddyActionProposals.tsx index 0e1a8328c..ae4a0fa66 100644 --- a/src/features/buddy/components/BuddyActionProposals.tsx +++ b/src/features/buddy/components/BuddyActionProposals.tsx @@ -100,6 +100,13 @@ export function BuddyActionProposals({ Not now
+ {/* A skip request is sent to a person in the hire's name, so the words go on screen + before the click -- the label only names the step. */} + {action.reason && ( +

+ Your reason: “{action.reason}” +

+ )} {action.status === "error" && (

Couldn't reach the server — try again. diff --git a/src/features/buddy/hooks/useBuddyConversation.ts b/src/features/buddy/hooks/useBuddyConversation.ts index 2fcb19bf1..3fb8c7259 100644 --- a/src/features/buddy/hooks/useBuddyConversation.ts +++ b/src/features/buddy/hooks/useBuddyConversation.ts @@ -362,6 +362,7 @@ export function useBuddyConversation() { onboardingTaskId: proposal.onboardingTaskId, answer: proposal.answer, description: proposal.description, + reason: proposal.reason, status: "idle", }); }, @@ -441,6 +442,7 @@ export function useBuddyConversation() { onboardingTaskId: action.onboardingTaskId, answer: action.answer, description: action.description, + reason: action.reason, }); patchAction(messageId, action.id, { status: "resolved", diff --git a/src/features/buddy/types.ts b/src/features/buddy/types.ts index 804897a2d..a4d4c2384 100644 --- a/src/features/buddy/types.ts +++ b/src/features/buddy/types.ts @@ -30,6 +30,7 @@ export const BUDDY_PATH_ACTIONS: readonly string[] = [ "complete_task", "answer_question", "add_path_step", + "request_skip", ]; export type ProposedAction = { @@ -87,6 +88,11 @@ export type ProposedAction = { onboardingTaskId?: string; answer?: string; description?: string; + /** + * The reason `request_skip` sends to the PM. Shown in full under the button, because it goes out + * in the hire's name and a label has no room for it. + */ + reason?: string; status: ProposedActionStatus; /** Whether a resolved action actually changed something (false = a handled "couldn't"). */ ok?: boolean; @@ -175,5 +181,6 @@ export type BuddyStreamHandlers = { onboardingTaskId?: string; answer?: string; description?: string; + reason?: string; }) => void; }; diff --git a/src/features/onboarding/components/OnBoardingItemPage.tsx b/src/features/onboarding/components/OnBoardingItemPage.tsx index 45ab4acee..87fc3a3cc 100644 --- a/src/features/onboarding/components/OnBoardingItemPage.tsx +++ b/src/features/onboarding/components/OnBoardingItemPage.tsx @@ -279,6 +279,8 @@ export function OnBoardingItemPage() { onboardingService.fetchTasks(stepId), ]); setStepDetail(step); + // A skip request the buddy sent shows up here as the pending reason, not an empty box. + setSkipReason(step.skip?.reason ?? ""); setTasks(refreshedTasks); setLocalFinished( new Set(refreshedTasks.filter((task) => task.finished).map((task) => task.id)), diff --git a/src/services/buddyService.ts b/src/services/buddyService.ts index d9bcfa450..1f43c409c 100644 --- a/src/services/buddyService.ts +++ b/src/services/buddyService.ts @@ -128,6 +128,8 @@ interface BuddyStreamChunk { onboarding_task_id?: string; answer?: string; description?: string; + /** `request_skip` confirm payload: the reason that goes to the PM. */ + reason?: string; } /** The outcome of confirming a buddy-proposed action — a single line to relay in the thread. */ @@ -142,8 +144,8 @@ export interface BuddyActionResult { * and the proposal's own confirm payloads are sent: `question` for flag-to-PM, `taskId` for a * goal claim, `title` + `attesterId` for an attestation request, `githubLogin` for saving a * username, `competencyKey` + `level` for recording where a conversation placed the hire, and the - * path-node ids (`stepId`, `questionId`, `phaseId`, plus `answer` and `description`) for the three - * actions that move the hire along their onboarding path. + * path-node ids (`stepId`, `questionId`, `phaseId`, plus `answer`, `description` and `reason`) for + * the actions that move the hire along their onboarding path. */ export async function performAction( action: string, @@ -161,6 +163,7 @@ export async function performAction( onboardingTaskId?: string; answer?: string; description?: string; + reason?: string; } = {}, ): Promise { return await apiClient.fetch(`/api/v1/onboarding/me/buddy/actions`, { @@ -180,6 +183,7 @@ export async function performAction( onboardingTaskId: extras.onboardingTaskId, answer: extras.answer, description: extras.description, + reason: extras.reason, }), }); } @@ -355,6 +359,7 @@ export async function streamMessage(content: string, handlers: BuddyStreamHandle onboardingTaskId: event.onboarding_task_id, answer: event.answer, description: event.description, + reason: event.reason, }); } break; diff --git a/tests/unit/features/buddy/BuddyActionProposals.test.tsx b/tests/unit/features/buddy/BuddyActionProposals.test.tsx index 61e9a3b8c..51e87c002 100644 --- a/tests/unit/features/buddy/BuddyActionProposals.test.tsx +++ b/tests/unit/features/buddy/BuddyActionProposals.test.tsx @@ -139,4 +139,26 @@ describe("BuddyActionProposals", () => { expect(screen.queryByTestId("buddy-orientation-card")).not.toBeInTheDocument(); }); + + it("shows the whole skip reason before the hire sends it in their name", () => { + render( + , + ); + + expect( + screen.getByText("Your reason: “I already have VPN access from my last team.”"), + ).toBeInTheDocument(); + }); }); From a113a54b45a622303474d52acce3222456fceb89 Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Mon, 14 Sep 2026 16:12:54 +0200 Subject: [PATCH 07/20] Carry add_path_step's graph placement through proposals waits_on_ids and unlocks_ids ride the proposal and the confirm payload like the other action fields, so a step the buddy adds lands connected in its phase graph. Co-Authored-By: Claude Opus 5 --- src/features/buddy/hooks/useBuddyConversation.ts | 4 ++++ src/features/buddy/types.ts | 8 ++++++++ src/services/buddyService.ts | 9 +++++++++ 3 files changed, 21 insertions(+) diff --git a/src/features/buddy/hooks/useBuddyConversation.ts b/src/features/buddy/hooks/useBuddyConversation.ts index 3fb8c7259..eaab20dbd 100644 --- a/src/features/buddy/hooks/useBuddyConversation.ts +++ b/src/features/buddy/hooks/useBuddyConversation.ts @@ -363,6 +363,8 @@ export function useBuddyConversation() { answer: proposal.answer, description: proposal.description, reason: proposal.reason, + waitsOnIds: proposal.waitsOnIds, + unlocksIds: proposal.unlocksIds, status: "idle", }); }, @@ -443,6 +445,8 @@ export function useBuddyConversation() { answer: action.answer, description: action.description, reason: action.reason, + waitsOnIds: action.waitsOnIds, + unlocksIds: action.unlocksIds, }); patchAction(messageId, action.id, { status: "resolved", diff --git a/src/features/buddy/types.ts b/src/features/buddy/types.ts index a4d4c2384..39f65346f 100644 --- a/src/features/buddy/types.ts +++ b/src/features/buddy/types.ts @@ -93,6 +93,12 @@ export type ProposedAction = { * in the hire's name and a label has no room for it. */ reason?: string; + /** + * Where `add_path_step` puts the new step in its phase's graph — what it waits on, and what will + * wait on it. Echoed back verbatim and re-checked against the hire's own path on confirm. + */ + waitsOnIds?: string[]; + unlocksIds?: string[]; status: ProposedActionStatus; /** Whether a resolved action actually changed something (false = a handled "couldn't"). */ ok?: boolean; @@ -182,5 +188,7 @@ export type BuddyStreamHandlers = { answer?: string; description?: string; reason?: string; + waitsOnIds?: string[]; + unlocksIds?: string[]; }) => void; }; diff --git a/src/services/buddyService.ts b/src/services/buddyService.ts index 1f43c409c..e9caf3eaf 100644 --- a/src/services/buddyService.ts +++ b/src/services/buddyService.ts @@ -130,6 +130,9 @@ interface BuddyStreamChunk { description?: string; /** `request_skip` confirm payload: the reason that goes to the PM. */ reason?: string; + /** `add_path_step` confirm payload: where the step goes in its phase's graph. */ + waits_on_ids?: string[]; + unlocks_ids?: string[]; } /** The outcome of confirming a buddy-proposed action — a single line to relay in the thread. */ @@ -164,6 +167,8 @@ export async function performAction( answer?: string; description?: string; reason?: string; + waitsOnIds?: string[]; + unlocksIds?: string[]; } = {}, ): Promise { return await apiClient.fetch(`/api/v1/onboarding/me/buddy/actions`, { @@ -184,6 +189,8 @@ export async function performAction( answer: extras.answer, description: extras.description, reason: extras.reason, + waitsOnIds: extras.waitsOnIds, + unlocksIds: extras.unlocksIds, }), }); } @@ -360,6 +367,8 @@ export async function streamMessage(content: string, handlers: BuddyStreamHandle answer: event.answer, description: event.description, reason: event.reason, + waitsOnIds: event.waits_on_ids, + unlocksIds: event.unlocks_ids, }); } break; From d926dcbcf9276d84270454d60644edba870d82aa Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Mon, 14 Sep 2026 18:09:51 +0200 Subject: [PATCH 08/20] Keep the separating space before the link highlight's interpolation The class string had the space inside the interpolated value, which the Tailwind Prettier plugin trims when it sorts, and the className test caught it. Co-Authored-By: Claude Opus 5 --- src/pages/OnBoardingPage.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pages/OnBoardingPage.tsx b/src/pages/OnBoardingPage.tsx index df885c84f..2ff233815 100644 --- a/src/pages/OnBoardingPage.tsx +++ b/src/pages/OnBoardingPage.tsx @@ -458,7 +458,7 @@ export function OnBoardingPage() { */ const linkedCard = (id: string) => id === linkedItemId - ? { id: linkedCardId(id), key: `${id}:${location.key}`, highlight: " app-link-highlight" } + ? { id: linkedCardId(id), key: `${id}:${location.key}`, highlight: "app-link-highlight" } : { id: linkedCardId(id), key: id, highlight: "" }; // The numbers this phase's items are shown with. The buddy's path tool derives the same ones, so @@ -954,7 +954,7 @@ export function OnBoardingPage() { // Completed and locked steps stay still on purpose: nothing // happens when you click them, and magnifying them would // promise an interaction that is not there. - className={`group rounded-2xl border bg-app-surface transition-all duration-200 motion-reduce:hover:scale-100${linkedCard(step.id).highlight} ${ + className={`group rounded-2xl border bg-app-surface transition-all duration-200 motion-reduce:hover:scale-100 ${linkedCard(step.id).highlight} ${ mode === "completed" ? "border-app-border opacity-60" : mode === "locked" @@ -1073,7 +1073,7 @@ export function OnBoardingPage() {

Date: Tue, 15 Sep 2026 12:53:17 +0200 Subject: [PATCH 09/20] Take the old onboarding off the board The onboarding is the path on the Onboarding page. The board carried a second one: a "Your path here" rail from joining to a first accepted contribution that announced when "onboarding ended", and a "Build my path" button that copied the path's steps into checklists once, matched by title, and then drifted from it. - Removed the path-to-first-contribution card and rail, and their moment labels. - Removed "Build my path", pathToCards, useGeneratedPathCards and applyPlan, and the card blueprints it read (a localStorage placeholder with no editor). Cards it already made stay; their invisible title markers are still stripped (layout/cardNames.ts). - The current-task card no longer tells a picked task from a handed one: nothing hands out tasks any more. - The empty board points at the Onboarding page instead. - Test fixtures moved off Task 0 and the removed card kind. Refs #311 Co-Authored-By: Claude Opus 5 --- src/features/attestation/types.ts | 2 +- src/features/board/components/BoardGrid.tsx | 3 - .../board/components/BoardPathRail.tsx | 103 -------- .../board/components/ChecklistCard.tsx | 11 +- .../board/components/CurrentTaskCard.tsx | 8 +- .../PathToFirstContributionCard.tsx | 97 -------- src/features/board/generation/pathToCards.ts | 224 ------------------ src/features/board/hooks/useBoardStructure.ts | 32 +-- .../board/hooks/useGeneratedPathCards.ts | 210 ---------------- src/features/board/layout/boardFilters.ts | 27 +-- src/features/board/layout/boardStructure.ts | 9 +- src/features/board/layout/cardAccents.ts | 3 +- src/features/board/layout/cardIcons.ts | 2 - src/features/board/layout/cardNames.ts | 16 +- src/features/board/momentLabels.ts | 37 --- src/features/board/types.ts | 30 --- src/features/buddy/types.ts | 6 +- .../card-blueprints/cardBlueprintService.ts | 154 ------------ src/features/card-blueprints/types.ts | 82 ------- src/pages/BoardPage.tsx | 183 +++----------- tests/unit/a11y/BoardPage.a11y.test.tsx | 16 +- tests/unit/features/board/BoardGrid.test.tsx | 77 +----- .../features/board/BoardSubmenus.test.tsx | 21 -- tests/unit/features/board/Marked.test.tsx | 1 - tests/unit/features/board/cardAccents.test.ts | 1 - tests/unit/features/board/pathToCards.test.ts | 223 ----------------- tests/unit/features/board/useBoard.test.tsx | 12 +- .../buddy/BuddyActionProposals.test.tsx | 20 +- tests/unit/services/buddyService.test.ts | 14 +- 29 files changed, 97 insertions(+), 1527 deletions(-) delete mode 100644 src/features/board/components/BoardPathRail.tsx delete mode 100644 src/features/board/components/PathToFirstContributionCard.tsx delete mode 100644 src/features/board/generation/pathToCards.ts delete mode 100644 src/features/board/hooks/useGeneratedPathCards.ts delete mode 100644 src/features/board/momentLabels.ts delete mode 100644 src/features/card-blueprints/cardBlueprintService.ts delete mode 100644 src/features/card-blueprints/types.ts delete mode 100644 tests/unit/features/board/pathToCards.test.ts diff --git a/src/features/attestation/types.ts b/src/features/attestation/types.ts index e833f454f..079ca0d49 100644 --- a/src/features/attestation/types.ts +++ b/src/features/attestation/types.ts @@ -5,7 +5,7 @@ export type AttestationState = "REQUESTED" | "ACCEPTED" | "WITHDRAWN"; * One request for a named colleague to confirm a hire's work. * * `returnedCount` is shown rather than hidden: work that took three passes is not the same as work - * that took none, and the autonomy milestone reads exactly this number. + * that took none. */ export interface Attestation { id: string; diff --git a/src/features/board/components/BoardGrid.tsx b/src/features/board/components/BoardGrid.tsx index 99331061b..d73b7a552 100644 --- a/src/features/board/components/BoardGrid.tsx +++ b/src/features/board/components/BoardGrid.tsx @@ -27,7 +27,6 @@ import { MemoryRecapCard } from "./MemoryRecapCard"; import { NoteCard } from "./NoteCard"; import { ArrivalStepsCard } from "./ArrivalStepsCard"; import { OpenPullRequestsCard } from "./OpenPullRequestsCard"; -import { PathToFirstContributionCard } from "./PathToFirstContributionCard"; import { SuggestedTasksCard } from "./SuggestedTasksCard"; import { BoardCardContext } from "./boardCardControls"; import { BoardStageBand } from "./BoardStageBand"; @@ -443,8 +442,6 @@ function BoardCardView({ // to wire one up later and wonder why nothing shows. const props = { card, ...shared }; switch (card.content.kind) { - case "PATH_TO_FIRST_CONTRIBUTION": - return ; case "ARRIVAL_STEPS": return ; case "OPEN_PULL_REQUESTS": diff --git a/src/features/board/components/BoardPathRail.tsx b/src/features/board/components/BoardPathRail.tsx deleted file mode 100644 index 5917018f8..000000000 --- a/src/features/board/components/BoardPathRail.tsx +++ /dev/null @@ -1,103 +0,0 @@ -import { AlertTriangle, CheckCircle2 } from "lucide-react"; -import { CONTRIBUTION_WORDING } from "../../../config/contributionWording"; -import { formatMoment } from "../../onboarding-metrics/format"; -import { momentLabel } from "../momentLabels"; -import type { PathToFirstContributionContent } from "../types"; - -type BoardPathRailProps = { - content: PathToFirstContributionContent; -}; - -/** - * The path from joining to a first accepted piece of work, as a thin rail under the page title. - * - * The same content the path card holds, drawn as page furniture instead of as one card among - * eleven. It earns that place: it is the only thing on the board that says where the hire is - * overall, and every other card is a detail of some part of it. - * - * **Deliberately quiet.** No heading, no summary line, no rule above it — the moments name - * themselves, and a title saying "Your path here" over five labelled dots is a label for a label. - * It is a strip you read on the way past, not a section that asks for attention. The accessible - * name lives on the region instead, so a screen reader still gets told what the strip is. - * - * Horizontal because the thing being drawn is a sequence in time, and a row reads as one — a - * column of five dots reads as a list of five separate facts. - * - * An unreached moment is a hollow dot and a dash — never a zero, and never a "3 of 5". A milestone - * that has not happened is not a milestone reached instantly, and a count of them is the blended - * completion figure this product does not draw. - * - * **Not dismissible.** It is part of the header now rather than a card on the board, and the one - * thing on the page that says where the hire stands is not something to lose by clicking an X next - * to it. The card behind it stays dismissible from the grid on any board that still shows it there. - * - * The stall reason is shown to the person in the stall, not only to their PM: a stall only somebody - * else can see is a stall only somebody else can fix. It is one amber line rather than a filled - * panel — the strip has to stay quiet even when it has something to say. - */ -export function BoardPathRail({ content }: BoardPathRailProps) { - const { moments, autonomyReachedAt, stalledReason } = content; - - return ( -
-
    - {moments.map((moment, index) => { - const reached = moment.reachedAt !== null; - // The connector belongs to the gap after this dot, so it lights only once the moment on - // its far side has actually happened. - const isLast = index === moments.length - 1; - const nextReached = !isLast && moments[index + 1].reachedAt !== null; - - return ( -
  1. -
    -
    - -

    - - {momentLabel(moment.key)} - - - {formatMoment(moment.reachedAt)} - -

    -
    2. - ); - })} -
- - {stalledReason && ( -

-

- )} - - {autonomyReachedAt && ( -

-

- )} -
- ); -} diff --git a/src/features/board/components/ChecklistCard.tsx b/src/features/board/components/ChecklistCard.tsx index bde836397..fae16fc85 100644 --- a/src/features/board/components/ChecklistCard.tsx +++ b/src/features/board/components/ChecklistCard.tsx @@ -4,7 +4,7 @@ import { Button } from "../../../components/ui/Button"; import { EmptyState } from "../../../components/ui/EmptyState"; import { Input } from "../../../components/ui/Input"; import { SelectionCheckbox } from "../../admin/components/SelectionCheckbox"; -import { readableTitle } from "../generation/pathToCards"; +import { readableTitle } from "../layout/cardNames"; import { useBoardCardControls } from "./boardCardControls"; import { BoardCardFrame } from "./BoardCardFrame"; import { Marked } from "./Marked"; @@ -124,8 +124,8 @@ export function ChecklistCard({ return ( !item.done).length, marks.map((mark) => mark.text), diff --git a/src/features/board/components/CurrentTaskCard.tsx b/src/features/board/components/CurrentTaskCard.tsx index e1d878ffe..decee0c59 100644 --- a/src/features/board/components/CurrentTaskCard.tsx +++ b/src/features/board/components/CurrentTaskCard.tsx @@ -36,13 +36,7 @@ export function CurrentTaskCard({ content, card, onDismiss, dismissing }: Curren icon={Target} title="What you're working on" card={card} - subtitle={ - hasTask - ? content.chosen - ? "You picked this one" - : "Handed to you as a first task" - : undefined - } + subtitle={hasTask ? "You picked this one" : undefined} onDismiss={onDismiss} dismissing={dismissing} > diff --git a/src/features/board/components/PathToFirstContributionCard.tsx b/src/features/board/components/PathToFirstContributionCard.tsx deleted file mode 100644 index 740c188d2..000000000 --- a/src/features/board/components/PathToFirstContributionCard.tsx +++ /dev/null @@ -1,97 +0,0 @@ -import { CONTRIBUTION_WORDING } from "../../../config/contributionWording"; -import { AlertTriangle, CheckCircle2, Route } from "lucide-react"; -import { formatMoment } from "../../onboarding-metrics/format"; -import { momentLabel, pathSummary } from "../momentLabels"; -import { BoardCardFrame } from "./BoardCardFrame"; -import { AskTheBuddy } from "../../buddy/components/AskTheBuddy"; -import type { BoardCard, PathToFirstContributionContent } from "../types"; - -type PathCardProps = { - content: PathToFirstContributionContent; - card: Pick; - onDismiss?: (cardId: string) => void; - dismissing?: boolean; -}; - -/** - * The path from joining to a first accepted piece of work. - * - * An unreached moment is a hollow dot and a dash — never a zero, because a milestone that has not - * happened is not a milestone reached instantly. - * - * The stall reason is shown to the person in the stall, not only to their PM: a stall only somebody - * else can see is a stall only somebody else can fix. It is framed as what is waiting, never as a - * verdict on the hire. - */ -export function PathToFirstContributionCard({ - content, - card, - onDismiss, - dismissing, -}: PathCardProps) { - const { acceptedCount, autonomyReachedAt, stalledReason } = content; - - return ( - -
    - {content.moments.map((moment) => { - const reached = moment.reachedAt !== null; - return ( -
  1. -
    2. - ); - })} -
- - {stalledReason && ( -

-

- )} - - - - {autonomyReachedAt && ( -

-

- )} - - ); -} diff --git a/src/features/board/generation/pathToCards.ts b/src/features/board/generation/pathToCards.ts deleted file mode 100644 index d58ada01e..000000000 --- a/src/features/board/generation/pathToCards.ts +++ /dev/null @@ -1,224 +0,0 @@ -import type { OnboardingPathEndpoint, OnboardingStepEndpoint } from "../../onboarding/types"; -import type { AuthoredCardRequest } from "../types"; -import type { BoardStage, DependencySource } from "../layout/boardStructure"; - -/** - * Turning the hire's personalised onboarding path into cards on their board. - * - * The path already exists and is already personalised: the AI service drafts it from the project's - * own corpus, against blueprints a PM maintains, and it comes back as phases of steps of tasks. The - * board did not know about any of it — which left the hire with a to-do list on one page and a - * working surface on another, and nothing saying which of the two was the plan. - * - * So the board becomes where the path is *worked*, and the path stays where it is generated. One - * step is one checklist card, its tasks are the lines on it, and the phase it came from is the area - * the card sits in. Nothing here writes prose: every title and every line is text the path already - * carried, so a card can be checked against the step it came from. - * - * TODO(backend): the cards this produces are posted through the ordinary authored-card endpoint, so - * they come back owned by the hire rather than attributed to the buddy. That is right for editing — - * they are the hire's list to tick, re-word and prune — and wrong for provenance, since a hire - * cannot tell a card they wrote from one their path generated. A `POST /me/board/cards/from-path` - * that mints them `AI`-owned with `placedAt` set, and still editable, is the shape this wants; - * until then {@link CARD_SOURCE_MARKERS} is the only trace, and it is deliberately never shown. - */ - -/** - * Steps whose contents are worth a card. - * - * A step already finished is a card that arrives ticked, which is a card that arrives as noise; a - * skipped one was explicitly declined. Both stay on the path page, where their history belongs. - */ -const CARD_WORTHY: readonly OnboardingStepEndpoint["status"][] = ["WAITING", "IN_PROGRESS"]; - -/** - * How a phase's position maps onto the board's two stages. - * - * A path has as many phases as it needs; the board has two coarse buckets, on purpose — see - * `boardStructure.ts`. The first phase is what the hire does now, everything after it is later. - * The finer order survives inside the areas, which keep the phases' own names and sequence, and in - * the chains between the steps of a phase. - */ -export function stageForPhase(index: number): BoardStage { - return index === 0 ? "NOW" : "LATER"; -} - -/** One card to create, and where it belongs once it exists. */ -export type PlannedCard = { - /** A key for this plan only — the real id is minted by the server on creation. */ - key: string; - request: AuthoredCardRequest; - /** - * When this card is due. - * - * Per card rather than per area, because the two are different questions and only one of them is - * a place. An area is where a card is filed — "Week one", "From your team" — and a stage is when - * it comes up; a team's blueprints are one named set of cards that deliberately spans all three - * stages. Carrying the stage on the area forced that set to be split into three areas with three - * tab stops, which is a table of contents describing the sequencing rather than the board. - */ - stage: BoardStage; - /** The key of the card that must be finished first, or null for the first of a phase. */ - afterKey: string | null; -}; - -/** One named area of cards, as a plan would file them. */ -export type PlannedArea = { - name: string; - cards: PlannedCard[]; - /** - * Who is claiming the chains in this area — the team, through a blueprint, or the buddy, through - * a generated path. - * - * Carried on the area rather than on each card because it is a property of where the plan came - * from, and one plan's areas never mix the two. It ends up on the board as the source of every - * dependency the area writes, which is what stops a hire from clearing a rule their PM wrote and - * what lets a buddy's suggestion say whose it was. - */ - source: DependencySource; -}; - -export type CardPlan = { - areas: PlannedArea[]; - /** How many cards the whole plan would create, for the confirmation the hire is shown. */ - cardCount: number; -}; - -/** - * Where a generated checklist came from, written invisibly into its title. - * - * Two jobs, both of which want a `source` column and do not have one. A second generation run has - * to recognise what the first one made so it does not create it twice; and the board's provenance - * filter has to tell a card the *team* prescribed from one the hire's own path produced — "from - * your team" is a real distinction to a new hire, and it is invisible in a checklist that looks - * exactly like every other checklist. - * - * Zero-width characters at the front of the title: they never render, never affect a comparison the - * hire can see, and survive the authored-card round trip because the title is stored verbatim. - * {@link readableTitle} takes them off everywhere a person reads one. - * - * TODO(backend): this is a channel smuggled through a text field, and it should not outlive the - * endpoint in the note above. A `source` on the card row — `HIRE` / `PATH` / `BLUEPRINT` — carries - * the same fact without encoding it in something the hire can edit. When that lands, delete every - * marker here and read the field. - */ -export const CARD_SOURCE_MARKERS = { - /** U+2063 invisible separator: a step of the hire's own personalised path. */ - PATH: "\u2063", - /** U+2060 word joiner: a card blueprint the project's PM wrote for this role. */ - TEAM: "\u2060", -} as const; - -export type GeneratedSource = keyof typeof CARD_SOURCE_MARKERS; - -/** - * The title as stored: marked with where it came from. - * - * **Trimmed, because the server trims.** A blueprint whose title a PM typed with a trailing space, - * or a path step the AI service handed over with a newline on the end, is stored without it — and a - * later run that planned the untrimmed string would find no card by that name and write a second - * one. Trimming here is the cheap half of that fix; {@link titleKey} is the half that holds when - * something else in the round trip changes the string. - */ -export function markTitle(source: GeneratedSource, title: string): string { - return `${CARD_SOURCE_MARKERS[source]}${title.trim()}`; -} - -/** Where a stored title came from, or null when the hire wrote it themselves. */ -export function sourceOfTitle(title: string | null): GeneratedSource | null { - if (title === null) return null; - - return ( - (Object.keys(CARD_SOURCE_MARKERS) as GeneratedSource[]).find((source) => - title.startsWith(CARD_SOURCE_MARKERS[source]), - ) ?? null - ); -} - -/** Whether a title was written by a generation run rather than by the hire. */ -export function isGeneratedTitle(title: string | null): boolean { - return sourceOfTitle(title) !== null; -} - -/** The title as the hire reads it, with any marker taken off. */ -export function readableTitle(title: string): string { - const source = sourceOfTitle(title); - - return source === null ? title : title.slice(CARD_SOURCE_MARKERS[source].length); -} - -/** - * What makes two cards the same card, for the purpose of not writing one twice. - * - * A generation run skips anything already on the board, and it can only recognise its own work by - * the title — so the comparison has to survive everything that happens to a title between being - * planned and being read back. It is trimmed and its inner runs of whitespace are collapsed, - * because the server stores a trimmed string and nobody can see the difference between one space - * and two. It is lowercased, because a card that differs from another only in capitalisation is a - * duplicate to the person reading the board, whatever a string comparison thinks. - * - * **The marker comes off.** A step of the hire's path and a blueprint their PM wrote can name the - * same piece of work, and the board would carry both as two cards with one visible title and no way - * to tell them apart. Keyed on what the hire reads, the second one is recognised as already there — - * and since blueprints are planned first, the version that survives is the one a person wrote. - */ -export function titleKey(title: string | null): string { - if (title === null) return ""; - - return readableTitle(title).trim().replace(/\s+/g, " ").toLowerCase(); -} - -/** - * The cards a path would put on the board, in the order they should be worked. - * - * **Steps inside a phase are chained; phases are not.** The path gives its steps an explicit - * position, so within a phase "this before that" is something the path actually claims and the - * board can honour. Across phases it is the stage that carries the order — chaining there as well - * would leave a hire with exactly one card they are allowed to open out of forty, which is a - * different kind of unusable from the one this is fixing. - * - * A step with no tasks still becomes a card, its expected outcomes as the lines: a step the path - * did not break down is still a thing to do, and an empty card at least says what it is for. - */ -export function planCardsFromPath(path: OnboardingPathEndpoint): CardPlan { - const areas: PlannedArea[] = []; - - const phases = [...path.phases].sort((a, b) => a.position - b.position); - phases.forEach((phase, phaseIndex) => { - const steps = [...phase.steps] - .sort((a, b) => a.position - b.position) - .filter((step) => CARD_WORTHY.includes(step.status)); - - const cards: PlannedCard[] = steps.map((step, stepIndex) => ({ - key: step.id, - request: { - kind: "CHECKLIST", - title: markTitle("PATH", step.title), - items: linesFor(step).map((text) => ({ text, done: false })), - }, - stage: stageForPhase(phaseIndex), - afterKey: stepIndex === 0 ? null : steps[stepIndex - 1].id, - })); - - if (cards.length > 0) areas.push({ name: phase.title, cards, source: "BUDDY" }); - }); - - return { areas, cardCount: areas.reduce((total, area) => total + area.cards.length, 0) }; -} - -/** - * What goes on the checklist for one step. - * - * Tasks first, because they are the step broken into things somebody does. Failing that, the - * expected outcomes — phrased as results rather than actions, but a hire ticking off "the project - * builds locally" is still ticking off something true. Failing both, one line naming the step, so - * the card is never a title over an empty box. - */ -function linesFor(step: OnboardingStepEndpoint): string[] { - if (step.tasks.length > 0) { - return [...step.tasks].sort((a, b) => a.position - b.position).map((task) => task.title); - } - if (step.expectedOutcomes.length > 0) return step.expectedOutcomes; - - return [step.title]; -} diff --git a/src/features/board/hooks/useBoardStructure.ts b/src/features/board/hooks/useBoardStructure.ts index 1dce818a3..4b7d2b482 100644 --- a/src/features/board/hooks/useBoardStructure.ts +++ b/src/features/board/hooks/useBoardStructure.ts @@ -15,7 +15,6 @@ import { type BoardStage, type BoardStructure, type CardState, - type DependencySource, } from "../layout/boardStructure"; export type UseBoardStructureResult = { @@ -36,18 +35,6 @@ export type UseBoardStructureResult = { * there, so the two never drift into disagreeing. */ setPredecessor: (cardId: string, blockerId: string | null) => void; - /** - * Applies a whole generated plan at once: every card's stage, and every link in every chain. - * - * One write rather than a call per card, and not for tidiness. Every other function here derives - * the next structure from the one it closed over, so calling them in a loop would have each - * iteration overwrite the last and leave only the final card sequenced — the kind of bug that - * looks like "the generator only did the last phase" and is nothing of the sort. - */ - applyPlan: ( - stages: Record, - chain: Record, - ) => void; }; /** @@ -90,16 +77,11 @@ export function useBoardStructure(boardId: string, cards: BoardCard[]): UseBoard * Pruned on every write rather than on load: a card dismissed in this session should stop * blocking things immediately, and storage should not accumulate rows for cards long gone. * - * `alsoKnown` is what keeps that from eating a freshly generated plan. Cards created a moment ago - * are not in `cards` until the board is re-read, so pruning against `cards` alone would drop - * every stage and every chain the generator just wrote — the whole plan, silently, between the - * write and the reload. - * * Plain functions rather than `useCallback`: this project compiles with the React Compiler, which * memoizes them itself and rejects hand-written dependency lists it cannot verify. */ - function save(next: BoardStructure, alsoKnown: readonly string[] = []) { - const known = new Set([...cards.map((card) => card.id), ...alsoKnown]); + function save(next: BoardStructure) { + const known = new Set(cards.map((card) => card.id)); const pruned = pruneStructure(next, known); setStructure(pruned); writeBoardStructure(boardId, pruned); @@ -121,15 +103,5 @@ export function useBoardStructure(boardId: string, cards: BoardCard[]): UseBoard save(blockerId ? setDependency(cleared, cardId, blockerId, true, "HIRE") : cleared); }, - applyPlan: (stages, chain) => { - let next = structure; - for (const [cardId, stage] of Object.entries(stages)) { - next = setCardStage(next, cardId, stage); - } - for (const [cardId, predecessor] of Object.entries(chain)) { - next = setDependency(next, cardId, predecessor.id, true, predecessor.source); - } - save(next, Object.keys(stages)); - }, }; } diff --git a/src/features/board/hooks/useGeneratedPathCards.ts b/src/features/board/hooks/useGeneratedPathCards.ts deleted file mode 100644 index 08f7ea34f..000000000 --- a/src/features/board/hooks/useGeneratedPathCards.ts +++ /dev/null @@ -1,210 +0,0 @@ -import { useState } from "react"; -import { ApiError } from "../../../services/apiClient"; -import { boardService } from "../../../services/boardService"; -import { onboardingService } from "../../../services/onboardingService"; -import { cardBlueprintService } from "../../card-blueprints/cardBlueprintService"; -import { blueprintsForRoles } from "../../card-blueprints/types"; -import { - markTitle, - planCardsFromPath, - titleKey, - type PlannedArea, -} from "../generation/pathToCards"; -import { type BoardStage, type DependencySource } from "../layout/boardStructure"; - -/** Why a generation run produced nothing, in words a page can put in a toast. */ -export type GenerationRefusal = - /** Nothing to build from: no personalised path, and no blueprints for this hire's roles. */ - | "NOTHING_TO_BUILD" - /** Everything that could be built is already a card, or already finished. */ - | "NOTHING_NEW" - /** A card could not be written. */ - | "FAILED"; - -/** What a successful run put on the board, for the page to file into areas and stages. */ -export type GenerationResult = { - cardCount: number; - /** The areas to create, in order. An area is a *place*; when a card is due is a separate answer. */ - areas: { name: string; cardIds: string[] }[]; - /** Card id to the stage it belongs in. */ - stages: Record; - /** Card id to the id of the card that must be finished first. */ - /** Each chained card's predecessor, and who is claiming the link. */ - chain: Record; -}; - -type UseGeneratedPathCardsResult = { - /** - * Builds this hire's onboarding into cards on their board. - * - * Resolves with what was created, or with the reason nothing was. Never throws: a page that has - * to try/catch around a button is a page that will forget to. - * - * @param projectId The project whose board is being filled. - * @param roleIds The hire's roles on that project, which decide the blueprints that apply. - * @param existingTitles Checklist titles already on the board, exactly as they are stored — the - * run compares them itself, by {@link titleKey}, so a caller never has to know how a title is - * marked or how the server trimmed it. - */ - generate: ( - projectId: string, - roleIds: readonly string[], - existingTitles: Iterable, - ) => Promise; - generating: boolean; -}; - -/** - * Fills the hire's board from the two things that know what they should be doing. - * - * **The team's blueprints**, which a PM wrote once for everybody of a given role, and **the - * personalised path**, which the AI service drafts for this hire from the project's own corpus. - * Both existed already and neither reached the board: blueprints had no consumer, and the path - * lived on a page a hire visited once and never worked from. A checklist they tick on the surface - * they already use is the same plan, in the place it gets used. - * - * Blueprints come first. A blueprint is what a person decided every hire of this role needs; the - * path is what the corpus suggests. When the two disagree about what comes first, the person wins. - * - * **Created one at a time, in order.** The board's own order is creation order, so posting in the - * order the plan puts them in means the board reads as the plan reads before anybody arranges - * anything. It is a handful of small writes rather than one batch because there is no batch - * endpoint; the loop stops at the first failure and reports what it managed, so a hire whose - * network dropped halfway gets the cards that landed rather than a board in an unknown state. - * - * A card whose title is already on the board is skipped, which is what makes running this twice - * safe — a hire who generates, dismisses two cards and generates again gets those two back and - * nothing else duplicated. - * - * **Already-there is judged by {@link titleKey}, not by the stored string.** The comparison is the - * only thing standing between a hire and two of every card, and an exact match is too brittle to be - * it: the server trims what it stores, a PM's blueprint title may have been typed with a trailing - * space, and the path and the blueprints can name the same work under different markers. Every one - * of those reads as "no card by that name" and writes a second copy — which is what a hire sees as - * their old card and a new one saying the same thing. Nothing about that is helped by the areas - * they are filed in, so a hire who has since dissolved an area sees the two copies side by side. - */ -export function useGeneratedPathCards(): UseGeneratedPathCardsResult { - const [generating, setGenerating] = useState(false); - - async function generate( - projectId: string, - roleIds: readonly string[], - existingTitles: Iterable, - ): Promise { - setGenerating(true); - try { - // Grows as the run goes: two blueprints named the same thing are the same card as surely as - // one blueprint and a card already on the board, and only one of them should be written. - const present = new Set([...existingTitles].map(titleKey)); - const plan = [...(await blueprintAreas(projectId, roleIds)), ...(await pathAreas())]; - if (plan.length === 0) return "NOTHING_TO_BUILD"; - - const areas: GenerationResult["areas"] = []; - const stages: Record = {}; - const chain: GenerationResult["chain"] = {}; - // Kept across every area rather than per area, because a blueprint may say it comes after one - // in a different stage — and a chain that silently dropped at an area boundary would look - // exactly like the PM never having set it. - const mintedByKey = new Map(); - let created = 0; - - for (const area of plan) { - const cardIds: string[] = []; - - for (const plannedCard of area.cards) { - // Only a titled checklist can be recognised again; anything else is written as planned. - const key = - plannedCard.request.kind === "CHECKLIST" - ? titleKey(plannedCard.request.title ?? null) - : ""; - if (key !== "") { - if (present.has(key)) continue; - present.add(key); - } - - const card = await boardService.addCard(projectId, plannedCard.request); - mintedByKey.set(plannedCard.key, card.id); - cardIds.push(card.id); - stages[card.id] = plannedCard.stage; - created += 1; - - // A predecessor that was skipped as already-present has no minted id, so this card simply - // waits on nothing rather than on a card that was never created. - const afterId = plannedCard.afterKey ? mintedByKey.get(plannedCard.afterKey) : undefined; - if (afterId) chain[card.id] = { id: afterId, source: area.source }; - } - - if (cardIds.length > 0) areas.push({ name: area.name, cardIds }); - } - - if (created === 0) return "NOTHING_NEW"; - - return { cardCount: created, areas, stages, chain }; - } catch { - return "FAILED"; - } finally { - setGenerating(false); - } - } - - return { generate, generating }; -} - -/** - * The hire's personalised path as areas of cards, or none when they have no path yet. - * - * A missing path is an ordinary state — a hire who has never generated one on the onboarding page - * simply has none — so the 404 is absorbed here rather than failing the whole run. Blueprints can - * still fill a board on their own, and often should: they are the half that does not need the hire - * to have done anything first. - */ -async function pathAreas(): Promise { - try { - return planCardsFromPath(await onboardingService.fetchPath()).areas; - } catch (error) { - if (error instanceof ApiError && error.status === 404) return []; - throw error; - } -} - -/** - * The project's card blueprints for this hire's roles, as one area of cards. - * - * **One area, whatever stages the blueprints span.** This used to be one area per stage, because a - * plan's stage lived on the area — which meant a team that had sequenced its cards across the - * stages arrived as several areas called "From your team — Now", "— Later" and so on, one tab stop - * each in the board's table of contents for one set of cards somebody wrote in one sitting. The - * stage is on the card now (see {@link PlannedArea}), so what the team prescribed is one place on - * the board and the stages fold inside it like they do everywhere else. - * - * Ordered by the PM's own ordering rather than by stage: `blueprintsForRoles` sorts by position, - * which is the order the editor's list is in and the order the cards are created in. - * - * A project with no blueprints yields nothing, which is the ordinary state on an installation where - * nobody has written any — not an error, and not an empty area. - */ -async function blueprintAreas( - projectId: string, - roleIds: readonly string[], -): Promise { - const blueprints = blueprintsForRoles(await cardBlueprintService.list(projectId), roleIds); - if (blueprints.length === 0) return []; - - return [ - { - name: "From your team", - source: "TEAM", - cards: blueprints.map((blueprint) => ({ - key: blueprint.id, - request: { - kind: "CHECKLIST" as const, - title: markTitle("TEAM", blueprint.title), - items: blueprint.items.map((text) => ({ text, done: false })), - }, - stage: blueprint.stage, - afterKey: blueprint.afterId, - })), - }, - ]; -} diff --git a/src/features/board/layout/boardFilters.ts b/src/features/board/layout/boardFilters.ts index 4a9caba78..db469a06e 100644 --- a/src/features/board/layout/boardFilters.ts +++ b/src/features/board/layout/boardFilters.ts @@ -1,21 +1,16 @@ import type { LucideIcon } from "lucide-react"; import { Bot, User } from "lucide-react"; -import { sourceOfTitle } from "../generation/pathToCards"; import type { BoardCard } from "../types"; /** * Which cards to show, by *where they came from*. * - * There used to be a fourth, "team", for the cards the project's blueprints put here. It is gone, - * and the section bar is why: the generator files those cards into an area called "From your team", - * so the filter and the bar were two controls cutting the same set — under the same words, from two - * different facts. The bar's is the better of the two. It cuts on where a card actually *is*, which - * is a thing a person can see and change; the filter's cut on an invisible marker in the card's - * title (see `generation/pathToCards.ts`, and the TODO to take it away), so the two disagreed the - * moment somebody moved a card out of the area. + * There used to be a third, "team", for the cards a PM's card blueprints put here. Card blueprints + * and the generator that wrote those cards are gone -- onboarding is the path on its own page -- so a + * checklist a hire has is theirs, whoever's idea it first was. * - * What is left has no equivalent among the sections: nobody files cards into an area by who wrote + * Neither cut has an equivalent among the sections: nobody files cards into an area by who wrote * them. */ export type BoardFilter = "all" | "buddy" | "mine"; @@ -48,23 +43,11 @@ export const FILTER_OPTIONS: { { value: "mine", label: "Yours", icon: User }, ]; -/** - * Whether a card came from the project's blueprints rather than from the hire or their buddy. - * - * Read off the invisible marker its title carries — see `generation/pathToCards.ts`, which explains - * why provenance is smuggled through a text field and what should replace it. - */ -export function isFromTeam(card: BoardCard): boolean { - return card.content.kind === "CHECKLIST" && sourceOfTitle(card.content.title) === "TEAM"; -} - export function matchesFilter(card: BoardCard, filter: BoardFilter): boolean { if (filter === "all") return true; if (filter === "buddy") return card.owner === "AI"; - // "Yours" still means yours: a blueprint card is stored as the hire's so they can edit it, but - // the team wrote it. Those are reached through their area, which is where they were put. - return card.owner === "HIRE" && !isFromTeam(card); + return card.owner === "HIRE"; } /** The label of the cut currently in force, or null when nothing is cut away. */ diff --git a/src/features/board/layout/boardStructure.ts b/src/features/board/layout/boardStructure.ts index c08da293c..3cfe7988d 100644 --- a/src/features/board/layout/boardStructure.ts +++ b/src/features/board/layout/boardStructure.ts @@ -90,6 +90,10 @@ export function stageOrder(stage: BoardStage): number { * - `TEAM` — from a card blueprint. The PM's, and the hire may not take it off. * - `BUDDY` — from a generated path. Named on the card so it does not look like the hire's own * doing, but still theirs to clear: the buddy is an assistant, not an authority. + * + * Nothing writes `TEAM` or `BUDDY` any more: card blueprints and the generator that copied the path + * onto the board were retired when the onboarding path became the one plan (#311). Boards arranged + * before then still hold such edges, and they keep their meaning. * - `HIRE` — theirs, and the only kind their own controls write. */ export type DependencySource = "TEAM" | "BUDDY" | "HIRE"; @@ -251,7 +255,6 @@ export function isSelfReporting(card: BoardCard): boolean { switch (card.content.kind) { case "CHECKLIST": case "ARRIVAL_STEPS": - case "PATH_TO_FIRST_CONTRIBUTION": return true; default: return false; @@ -276,10 +279,6 @@ export function cardProgress(card: BoardCard): { done: number; total: number } | const total = content.steps.length; return { done: total - content.outstandingCount, total }; } - case "PATH_TO_FIRST_CONTRIBUTION": { - const total = content.moments.length; - return { done: content.moments.filter((moment) => moment.reachedAt !== null).length, total }; - } default: return null; } diff --git a/src/features/board/layout/cardAccents.ts b/src/features/board/layout/cardAccents.ts index 552cd3071..8bba69cb5 100644 --- a/src/features/board/layout/cardAccents.ts +++ b/src/features/board/layout/cardAccents.ts @@ -89,8 +89,7 @@ const ORANGE: CardAccent = { }; const ACCENTS: Record = { - // What the board is steering by: where you are going, and what you are on right now. - PATH_TO_FIRST_CONTRIBUTION: BRAND, + // What the board is steering by: what you are on right now. CURRENT_TASK: BRAND, // Things that come from somewhere outside the board — the joining process, and the repository. diff --git a/src/features/board/layout/cardIcons.ts b/src/features/board/layout/cardIcons.ts index 9c5829e6c..d22e7ece6 100644 --- a/src/features/board/layout/cardIcons.ts +++ b/src/features/board/layout/cardIcons.ts @@ -7,7 +7,6 @@ import { Network, PenLine, PlaneLanding, - Route, Sparkles, Target, type LucideIcon, @@ -28,7 +27,6 @@ import type { BoardCardKind } from "../types"; * a confident wrong one reads as a different card. */ const ICONS: Record = { - PATH_TO_FIRST_CONTRIBUTION: Route, CURRENT_TASK: Target, DIAGRAM: Network, ARRIVAL_STEPS: PlaneLanding, diff --git a/src/features/board/layout/cardNames.ts b/src/features/board/layout/cardNames.ts index e5f51ae9a..b14dc69f6 100644 --- a/src/features/board/layout/cardNames.ts +++ b/src/features/board/layout/cardNames.ts @@ -1,6 +1,18 @@ -import { readableTitle } from "../generation/pathToCards"; import type { BoardCard } from "../types"; +/** + * The invisible marks the retired "Build my path" generator put at the front of a checklist title: + * U+2063 for a step of the onboarding path, U+2060 for a card blueprint. The generator is gone -- + * onboarding is the path on its own page -- but cards it made are still on boards, so the marks are + * still taken off wherever a title is read. + */ +const LEGACY_TITLE_MARKS = /^[\u2060\u2063]/; + +/** A checklist title as the hire reads it. */ +export function readableTitle(title: string): string { + return title.replace(LEGACY_TITLE_MARKS, ""); +} + /** * What to call a card when it is being talked about from somewhere else on the board. * @@ -22,8 +34,6 @@ export function cardName(card: BoardCard): string { return content.subject; case "ARRIVAL_STEPS": return "Your arrival steps"; - case "PATH_TO_FIRST_CONTRIBUTION": - return "Your path to a first contribution"; case "OPEN_PULL_REQUESTS": return "Your open pull requests"; case "CURRENT_TASK": diff --git a/src/features/board/momentLabels.ts b/src/features/board/momentLabels.ts deleted file mode 100644 index 24ac35dae..000000000 --- a/src/features/board/momentLabels.ts +++ /dev/null @@ -1,37 +0,0 @@ -import { CONTRIBUTION_WORDING } from "../../config/contributionWording"; -import type { BoardMomentKey } from "./types"; - -/** - * What each moment on the path is called. - * - * The two middle moments are built from the track's noun rather than named after git, because the - * moments themselves are not about git — somebody whose work is a facilitated ceremony still - * submits it and still waits for somebody to respond. - * - * Lives here rather than beside one of its two renderers: the path is drawn as a rail in the board - * header and as a card in the grid, and the same moment must not be able to answer to two names - * depending on which one you are looking at. - */ -export function momentLabel(key: BoardMomentKey): string { - switch (key) { - case "JOINED": - return "Joined"; - case "TASK_CLAIMED": - return "Task claimed"; - case "WORK_SUBMITTED": - return `First ${CONTRIBUTION_WORDING.noun} submitted`; - case "FIRST_RESPONSE": - return "Somebody responded"; - case "WORK_ACCEPTED": - return `First ${CONTRIBUTION_WORDING.noun} ${CONTRIBUTION_WORDING.verbPast}`; - } -} - -/** The one-line summary the path carries above it, in both of its forms. */ -export function pathSummary(acceptedCount: number): string { - if (acceptedCount === 0) { - return `Nothing ${CONTRIBUTION_WORDING.verbPast} yet — that's normal early on`; - } - const noun = acceptedCount === 1 ? CONTRIBUTION_WORDING.noun : CONTRIBUTION_WORDING.nounPlural; - return `${acceptedCount} ${noun} ${CONTRIBUTION_WORDING.verbPast}`; -} diff --git a/src/features/board/types.ts b/src/features/board/types.ts index 0f93062ed..f02001e31 100644 --- a/src/features/board/types.ts +++ b/src/features/board/types.ts @@ -9,7 +9,6 @@ import type { ArrivalStep } from "../arrival/types"; /** Every card kind the board understands. Closed set — see the module comment. */ export type BoardCardKind = - | "PATH_TO_FIRST_CONTRIBUTION" | "ARRIVAL_STEPS" | "OPEN_PULL_REQUESTS" | "CURRENT_TASK" @@ -35,32 +34,6 @@ export type AuthoredCardKind = "NOTE" | "LINK" | "CHECKLIST"; */ export type BoardCardOwner = "AI" | "HIRE"; -/** The moments a path card reports, in the order they normally happen. */ -export type BoardMomentKey = - "JOINED" | "TASK_CLAIMED" | "WORK_SUBMITTED" | "FIRST_RESPONSE" | "WORK_ACCEPTED"; - -/** One moment, and whether it has happened. `null` is "not yet", and renders as a dash, never a zero. */ -export type BoardMoment = { - key: BoardMomentKey; - reachedAt: string | null; -}; - -/** - * The path from joining to a first accepted piece of work. - * - * Composed from contributions, not pull requests, so it says something true whatever produces - * this hire's work. - */ -export type PathToFirstContributionContent = { - kind: "PATH_TO_FIRST_CONTRIBUTION"; - moments: BoardMoment[]; - acceptedCount: number; - /** When onboarding ended, dated. Null while it is still going. */ - autonomyReachedAt: string | null; - /** Why the hire currently reads as stalled, in plain words; null when they do not. */ - stalledReason: string | null; -}; - /** One open pull request. `waitingHours` is null once somebody has responded — the clock stopped. */ export type BoardPullRequest = { artifactId: string; @@ -115,8 +88,6 @@ export type CurrentTaskContent = { title: string | null; summary: string | null; url: string | null; - /** True when the hire claimed this as their goal, false when it is the Task 0 they were handed. */ - chosen: boolean; }; /** One suggested task, with the plain reasons it was suggested. Never a score. */ @@ -263,7 +234,6 @@ export type ChecklistContent = { /** The rendered content of one card, discriminated by `kind`. */ export type BoardCardContent = - | PathToFirstContributionContent | ArrivalStepsContent | OpenPullRequestsContent | CurrentTaskContent diff --git a/src/features/buddy/types.ts b/src/features/buddy/types.ts index 39f65346f..e7f56e121 100644 --- a/src/features/buddy/types.ts +++ b/src/features/buddy/types.ts @@ -36,9 +36,9 @@ export const BUDDY_PATH_ACTIONS: readonly string[] = [ export type ProposedAction = { /** Local id for keying and targeting the confirm — the backend doesn't assign one. */ id: string; - /** The action's tool name, sent back verbatim to confirm it (e.g. "claim_task_zero"). */ + /** The action's tool name, sent back verbatim to confirm it (e.g. "claim_goal"). */ action: string; - /** The button text ("Start Task 0"). */ + /** The button text ("Work toward this task"). */ label: string; /** Carried through only for flag-to-PM: the question the buddy composed. */ question?: string; @@ -168,7 +168,7 @@ export type BuddyStreamHandlers = { /** Optional: only some turns run a tool, and the surface may not show which. */ onToolUse?: (tool: string) => void; /** - * The buddy has *proposed* an action the hire must confirm (e.g. "Start Task 0"). Nothing has + * The buddy has *proposed* an action the hire must confirm (e.g. "Work toward this task"). Nothing has * changed yet — the surface renders a confirm affordance and only mutates when the hire clicks. */ onActionProposal?: (proposal: { diff --git a/src/features/card-blueprints/cardBlueprintService.ts b/src/features/card-blueprints/cardBlueprintService.ts deleted file mode 100644 index 4200a335b..000000000 --- a/src/features/card-blueprints/cardBlueprintService.ts +++ /dev/null @@ -1,154 +0,0 @@ -import type { CardBlueprint, CardBlueprintDraft } from "./types"; - -/** - * Where a project's card blueprints are kept. - * - * Local storage, per project, and this one is a *placeholder* rather than a considered trade-off — - * unlike the board's own local layers, which are one hire's preferences about one machine. These - * are a team's decisions, authored by a PM and consumed by every hire on the project, so they - * belong on the server and nowhere else. Kept here so the shape can be agreed and the screens - * exercised before the endpoints exist. - * - * TODO(backend): `GET/POST/PUT/DELETE /api/v1/onboarding/projects/{projectId}/card-blueprints`, - * authorised the way the other PM surfaces are (`@projectAuth.canAccessProject`). The functions - * below are already async and already fail loudly, so the swap is one file. Until then, blueprints - * a PM writes are visible only in the browser they were written in — which is fine for agreeing the - * model and not fine for anything else. - */ -const STORAGE_VERSION = 1; - -function storageKey(projectId: string): string { - return `sprintstart:card-blueprints:${projectId}`; -} - -type Stored = { - version: number; - blueprints: unknown; -}; - -/** One stored blueprint, with anything unrecognised dropped rather than trusted. */ -function toBlueprint(value: unknown): CardBlueprint | null { - if (typeof value !== "object" || value === null) return null; - const raw = value as Record; - if (typeof raw.id !== "string" || typeof raw.title !== "string") return null; - - return { - id: raw.id, - title: raw.title, - description: typeof raw.description === "string" ? raw.description : "", - items: Array.isArray(raw.items) - ? raw.items.filter((i): i is string => typeof i === "string") - : [], - // "NEXT" is a blueprint written when the board had three stages; it meant "not now", which is - // what "LATER" means. - stage: raw.stage === "NEXT" || raw.stage === "LATER" ? "LATER" : "NOW", - roleIds: Array.isArray(raw.roleIds) - ? raw.roleIds.filter((i): i is string => typeof i === "string") - : [], - position: typeof raw.position === "number" ? raw.position : 0, - afterId: typeof raw.afterId === "string" ? raw.afterId : null, - }; -} - -function read(projectId: string): CardBlueprint[] { - try { - const raw = window.localStorage.getItem(storageKey(projectId)); - if (!raw) return []; - - const parsed = JSON.parse(raw) as Stored; - if (parsed?.version !== STORAGE_VERSION || !Array.isArray(parsed.blueprints)) return []; - - return parsed.blueprints - .map(toBlueprint) - .filter((blueprint): blueprint is CardBlueprint => blueprint !== null) - .sort((a, b) => a.position - b.position); - } catch { - return []; - } -} - -function write(projectId: string, blueprints: CardBlueprint[]): void { - window.localStorage.setItem( - storageKey(projectId), - JSON.stringify({ version: STORAGE_VERSION, blueprints } satisfies Stored), - ); -} - -export const cardBlueprintService = { - /** - * Every card blueprint on a project, in the order the cards will be created. - * - * @param projectId The project the blueprints belong to. - */ - list(projectId: string): Promise { - return Promise.resolve(read(projectId)); - }, - - /** - * Writes a blueprint, creating it when `id` is null and replacing it otherwise. - * - * A new blueprint goes to the end: it was written last, and inserting it anywhere else would be - * this function guessing at an order the PM has a control for. - * - * @returns The blueprint as stored, with its id and position filled in. - */ - save(projectId: string, id: string | null, draft: CardBlueprintDraft): Promise { - const blueprints = read(projectId); - - if (id) { - const existing = blueprints.find((blueprint) => blueprint.id === id); - if (!existing) { - return Promise.reject(new Error(`No card blueprint ${id} on project ${projectId}`)); - } - - const updated = { ...existing, ...draft }; - write( - projectId, - blueprints.map((blueprint) => (blueprint.id === id ? updated : blueprint)), - ); - - return Promise.resolve(updated); - } - - const created: CardBlueprint = { - ...draft, - id: `bp-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`, - position: blueprints.length, - }; - write(projectId, [...blueprints, created]); - - return Promise.resolve(created); - }, - - /** - * Removes a blueprint, and unhooks anything that was waiting on it. - * - * A dangling `afterId` would leave a card blocked by a blueprint that no longer exists, which is - * a block nobody can clear — so the removal takes the edge with it. - */ - remove(projectId: string, id: string): Promise { - const blueprints = read(projectId) - .filter((blueprint) => blueprint.id !== id) - .map((blueprint) => (blueprint.afterId === id ? { ...blueprint, afterId: null } : blueprint)); - - write( - projectId, - blueprints.map((blueprint, index) => ({ ...blueprint, position: index })), - ); - - return Promise.resolve(); - }, - - /** Puts the blueprints in this order. Sends the whole order, the way the board's reorder does. */ - reorder(projectId: string, ids: string[]): Promise { - const byId = new Map(read(projectId).map((blueprint) => [blueprint.id, blueprint])); - const ordered = ids - .map((id) => byId.get(id)) - .filter((blueprint): blueprint is CardBlueprint => blueprint !== undefined) - .map((blueprint, index) => ({ ...blueprint, position: index })); - - write(projectId, ordered); - - return Promise.resolve(); - }, -}; diff --git a/src/features/card-blueprints/types.ts b/src/features/card-blueprints/types.ts deleted file mode 100644 index aed3c7260..000000000 --- a/src/features/card-blueprints/types.ts +++ /dev/null @@ -1,82 +0,0 @@ -import type { BoardStage } from "../board/layout/boardStructure"; - -/** - * A card a PM wants every new hire of some role to start with. - * - * The board fills itself from two directions. What the *system* knows goes on it automatically — - * the path, the arrival steps, the open work — and what the *buddy* judges worth keeping is placed - * in conversation. Neither covers what the team knows: that a backend hire needs the on-call rota - * explained in week one, that everybody reads the incident write-up before touching deploys. That - * lives in a PM's head, gets said once per hire, and is the first thing forgotten in a busy month. - * - * A blueprint is that knowledge written down once. It says what the card is called, what is on it, - * when it is due, which roles it applies to, and what has to be finished before it. A hire's board - * is then seeded from the blueprints their roles match — the same list every time, without anybody - * having to remember it. - * - * Deliberately *not* the same thing as the AI service's onboarding blueprints, which describe the - * phases and steps of a generated path. Those are drafted from the project's corpus and regenerated - * as it moves; these are written by a person and change only when that person changes them. The - * names collide because both are templates; nothing else about them does. - */ -export type CardBlueprint = { - id: string; - /** What the card will be called on the hire's board. */ - title: string; - /** One line under the title, saying why this card is here. Optional and often worth it. */ - description: string; - /** The lines on the checklist, in order. A blueprint with no lines is a card with a title only. */ - items: string[]; - /** When it is due — the same stages the board sorts and folds by. */ - stage: BoardStage; - /** - * Which project roles get this card. Empty means everybody on the project. - * - * Ids rather than names: a role renamed from "Backend Dev" to "Platform Engineer" should keep its - * blueprints, and matching on the name is how a rename silently empties somebody's board. - */ - roleIds: string[]; - /** Where it sits among the blueprints, which is the order the cards are created in. */ - position: number; - /** - * The blueprint that has to be finished before this one, or null. - * - * Becomes a real dependency on the hire's board: their card waits on the card the other blueprint - * produced. This is where "read the runbook before you deploy" stops being something a PM says - * and starts being something the board holds. - */ - afterId: string | null; -}; - -/** A blueprint as it is written, before it has an id or a position. */ -export type CardBlueprintDraft = Omit; - -/** An empty draft, so the editor opens on something rather than on nulls. */ -export const EMPTY_DRAFT: CardBlueprintDraft = { - title: "", - description: "", - items: [], - stage: "NOW", - roleIds: [], - afterId: null, -}; - -/** - * The blueprints that apply to a hire holding these roles, in order. - * - * A blueprint with no roles applies to everyone: "read the incident write-up" is not a backend - * thing, and forcing a PM to tick every role to say so would mean forgetting one whenever a role is - * added later. - */ -export function blueprintsForRoles( - blueprints: CardBlueprint[], - roleIds: readonly string[], -): CardBlueprint[] { - return [...blueprints] - .filter( - (blueprint) => - blueprint.roleIds.length === 0 || - blueprint.roleIds.some((roleId) => roleIds.includes(roleId)), - ) - .sort((a, b) => a.position - b.position); -} diff --git a/src/pages/BoardPage.tsx b/src/pages/BoardPage.tsx index f6d4ef415..d0d54079f 100644 --- a/src/pages/BoardPage.tsx +++ b/src/pages/BoardPage.tsx @@ -10,7 +10,6 @@ import { Maximize2, Minimize2, RefreshCw, - Sparkles, } from "lucide-react"; import { PageHeader } from "../components/layout/PageHeader"; import { Button } from "../components/ui/Button"; @@ -20,11 +19,9 @@ import { Spinner } from "../components/ui/Spinner"; import { useSwipeableTabs } from "../hooks/useHorizontalWheelNavigation"; import { useBoard } from "../features/board/hooks/useBoard"; import { useBoardStructure } from "../features/board/hooks/useBoardStructure"; -import { useGeneratedPathCards } from "../features/board/hooks/useGeneratedPathCards"; import { AddCardForm, AddCardTriggers } from "../features/board/components/AddCardForm"; import type { AuthoredCardKind } from "../features/board/types"; import { BoardGrid } from "../features/board/components/BoardGrid"; -import { BoardPathRail } from "../features/board/components/BoardPathRail"; import { BoardSectionTabs } from "../features/board/components/BoardSectionNav"; import { BoardFilterTriggers } from "../features/board/components/BoardFilterTriggers"; import { NewAreaForm } from "../features/board/components/NewAreaForm"; @@ -34,7 +31,6 @@ import { BoardNextUp } from "../features/board/components/BoardNextUp"; import { BoardLocalOnlyNotice } from "../features/board/components/BoardLocalOnlyNotice"; import { nextUp } from "../features/board/layout/nextUp"; import { useProjectContext } from "../features/projects/useProjectContext"; -import { useAuth } from "../context/useAuth"; import { useToast } from "../context/useToast"; import { useFocusMode } from "../context/useFocusMode"; import { readCollapsedCards, writeCollapsedCards } from "../features/board/layout/collapsedCards"; @@ -86,20 +82,19 @@ import { * memory and never replayed — so anything durable it showed you was gone by the next visit. This is * where those things live instead. Chat is the conversation; this is the whiteboard beside it. * - * Per project, because what belongs on it is: the path, the open work, later the current task. The - * project switcher is the same one the rest of the app uses, so the choice is remembered across - * pages rather than being a setting of this one. + * Per project, because what belongs on it is: the open work, the current task, what the buddy + * remembers. The project switcher is the same one the rest of the app uses, so the choice is + * remembered across pages rather than being a setting of this one. + * + * **Not the onboarding.** That is the path on the Onboarding page, which a PM's blueprint prescribes + * and the buddy tutors along. The board used to carry a second one -- a rail from joining to a first + * accepted contribution, and a button that copied the path's steps into checklists -- and two plans + * that drift apart are worse than one. * * The shell is the app's page shell — banner header over `app-page-frame`, `PageHeader` for the * title block, shared primitives for the actions and for every empty, loading and error state — so * the board sits at the same gutter and reads with the same weight as Starter Work beside it. * - * **The path is lifted out of the grid into the header.** It is the one card that says where the - * hire stands overall; every other card is a detail of some part of it, so it belongs above them - * rather than competing with a checklist for a slot. It keeps its place in the board's order — the - * grid renders the rest, and a reorder puts the path back at the index it came from, so lifting it - * for display never quietly rewrites what the hire arranged. - * * **The board is now a process, not a pile.** Three things carry that, and none of them changes the * board's own order: * @@ -110,9 +105,7 @@ import { * - *sections* down the side, so a board of forty cards is read one part at a time. * * The reason for all three is the same. A board that shows everything at once is fine at eight - * cards and unusable at forty, and forty is what a generated onboarding path produces. See - * `layout/boardStructure.ts` for the model and `generation/pathToCards.ts` for where the cards come - * from. + * cards and unusable at forty. See `layout/boardStructure.ts` for the model. */ /** @@ -138,21 +131,16 @@ const FOLD_THRESHOLD = 8; * Which cards the board is showing. * * Not a search and not a sort — the one cut worth making by hand is *who put this here*, which is - * the one thing a card's content never says on its own. Three sources, and they partition the - * board: + * the one thing a card's content never says on its own. Two sources, and they partition the board: * * - `buddy` — placed for the hire in conversation, contents read live. - * - `team` — a card blueprint their PM wrote for everybody in this role. - * - `mine` — everything else they own: their own notes and lists, and the steps of their own - * personalised path. The path counts as theirs because it *is*: it was drafted for them, they - * edit it, and nobody else on the project has the same one. + * - `mine` — everything they wrote themselves: their own notes, links and lists. * * Where a card sits in the process is a separate question, and the stages, the focus view and the * section tabs answer that one. */ export function BoardPage() { const { selectedProjectId, isLoading: projectsLoading } = useProjectContext(); - const { profile } = useAuth(); const toast = useToast(); const { isFocused, setFocused } = useFocusMode(); @@ -175,8 +163,6 @@ export function BoardPage() { return () => window.removeEventListener("keydown", handleKeyDown); }, [isFocused, setFocused]); - /** The hire's roles on this project, which decide which of the team's blueprints reach them. */ - const roleIds = useMemo(() => (profile?.projectRoles ?? []).map((role) => role.id), [profile]); const [isArranging, setIsArranging] = useState(false); const [filter, setFilter] = useState("all"); const [sectionId, setSectionId] = useState(null); @@ -423,13 +409,6 @@ export function BoardPage() { }); } - // The path is drawn in the header; the grid gets everything else. Its index is kept so a reorder - // of the visible cards can put it back where it was — the board's order is the hire's, and this - // is a display decision, not an edit to it. - const pathIndex = - board?.cards.findIndex((card) => card.content.kind === "PATH_TO_FIRST_CONTRIBUTION") ?? -1; - const pathCard = pathIndex === -1 ? null : (board?.cards[pathIndex] ?? null); - /** * Every card the board is holding, whatever the current view. * @@ -439,12 +418,14 @@ export function BoardPage() { * hire's attention. */ const allCards = useMemo( - () => board?.cards.filter((card) => card !== pathCard && !pendingRemovals.has(card.id)) ?? [], - [board, pathCard, pendingRemovals], + () => board?.cards.filter((card) => !pendingRemovals.has(card.id)) ?? [], + [board, pendingRemovals], ); - const { states, assignStage, assignGroupStage, toggleDone, setPredecessor, applyPlan } = - useBoardStructure(boardId, allCards); + const { states, assignStage, assignGroupStage, toggleDone, setPredecessor } = useBoardStructure( + boardId, + allCards, + ); // Lends these cards to the app shell, so the selection toolbar mounted above the router can offer // the marker pen on text that turns out to be on one of them. Taken back when this page leaves. @@ -494,13 +475,6 @@ export function BoardPage() { [allCards, states], ); - /** - * The provenance cuts worth offering on *this* board. - * - * "From your team" only appears once the team has actually prescribed something. An option that - * can only ever come back empty is a promise the board cannot keep, and on an installation where - * nobody has written a blueprint that is every board. - */ /** * Whether the board has been divided into anything worth navigating. * @@ -775,12 +749,7 @@ export function BoardPage() { return cuts; }, [allCards, filter, openStackIds, shownSectionId, sections, stacks]); - const handleReorder = (cardIds: string[]) => { - if (!pathCard || pathIndex === -1) return void reorder(cardIds); - const next = [...cardIds]; - next.splice(Math.min(pathIndex, next.length), 0, pathCard.id); - return void reorder(next); - }; + const handleReorder = (cardIds: string[]) => void reorder(cardIds); /** * Opens the planning mode, with nothing folded away. @@ -801,82 +770,6 @@ export function BoardPage() { setIsArranging(true); } - const { generate, generating } = useGeneratedPathCards(); - - /** - * Builds the hire's personalised onboarding path into cards, and files them. - * - * The cards are written server-side; the areas, stages and order between them are this client's - * to keep, so both halves are applied here rather than left for the hire to arrange by hand. A - * generated path that landed as forty loose cards would be the exact complaint this answers. - */ - async function handleGenerate() { - if (!selectedProjectId) return; - - const existingTitles = new Set( - allCards.flatMap((card) => - card.content.kind === "CHECKLIST" && card.content.title ? [card.content.title] : [], - ), - ); - - const result = await generate(selectedProjectId, roleIds, existingTitles); - - if (result === "NOTHING_TO_BUILD") { - toast.info("Nothing to build from yet", { - description: - "Generate your onboarding path on the Onboarding page, or ask your PM to set up card blueprints.", - }); - return; - } - if (result === "NOTHING_NEW") { - toast.info("Your path is already on the board", { - description: "Every step that isn't finished is already a card here.", - }); - return; - } - if (result === "FAILED") { - showErrorToast("Your path couldn't be built into cards", { - description: "Nothing was changed — try again.", - }); - return; - } - - // One area per phase, named after it — and a second run adds to the area it made the first - // time rather than making another one beside it. Without that, generating again after the PM - // added a blueprint left two areas called "From your team", one holding the old cards and one - // holding the new, which is the same card twice as far as anybody reading the board can tell. - // - // The index is in the id because a plan is applied inside a single millisecond and `Date.now()` - // alone would mint the same id for every phase. - const stamp = Date.now(); - const filed = [...groups]; - result.areas.forEach((area, index) => { - const existing = filed.findIndex((group) => group.name === area.name); - if (existing !== -1) { - filed[existing] = { - ...filed[existing], - cardIds: [...filed[existing].cardIds, ...area.cardIds], - }; - - return; - } - - filed.push({ - id: `group-path-${stamp}-${index}`, - name: area.name, - cardIds: area.cardIds, - collapsed: false, - }); - }); - saveGroups(filed); - applyPlan(result.stages, result.chain); - - refresh(); - toast.success(`${result.cardCount} cards added from your path`, { - description: "Grouped by phase, in the order the path puts them in.", - }); - } - /** * The gutters the page draws in. * @@ -899,7 +792,7 @@ export function BoardPage() { subtitle={ isArranging ? "Say when each card is due and what it waits on." - : "Where your onboarding stays put between conversations." + : "Where your work stays put between conversations." } actions={ isArranging ? ( @@ -912,16 +805,6 @@ export function BoardPage() { ) : ( <> - {/* The one switch from the rail worth a copy here, for the widths where there is no margin to put a rail in. `lg:hidden` rather than a second implementation: one state, two places it can be reached from. */} @@ -949,10 +832,6 @@ export function BoardPage() { ) } /> - - {pathCard && pathCard.content.kind === "PATH_TO_FIRST_CONTRIBUTION" && ( - - )}
)} @@ -1164,26 +1043,22 @@ export function BoardPage() { {/* A board with nothing on it is the first thing a new hire sees, and an empty page cannot say what the board is *for*. Named after what it will hold rather than after - its own emptiness — and it points at the two things that fill it: the path, and the - row of buttons directly above. */} + its own emptiness — and it says where the onboarding is, because it is not here. */} {allCards.length === 0 && ( )} diff --git a/tests/unit/a11y/BoardPage.a11y.test.tsx b/tests/unit/a11y/BoardPage.a11y.test.tsx index 0034f88d4..48f9733a0 100644 --- a/tests/unit/a11y/BoardPage.a11y.test.tsx +++ b/tests/unit/a11y/BoardPage.a11y.test.tsx @@ -32,20 +32,6 @@ const board: Board = { boardId: "b1", projectId: "p1", cards: [ - { - id: "c1", - kind: "PATH_TO_FIRST_CONTRIBUTION", - owner: "AI", - position: 0, - placedAt: null, - content: { - kind: "PATH_TO_FIRST_CONTRIBUTION", - moments: [{ key: "JOINED", reachedAt: "2026-07-20T09:00:00Z" }], - acceptedCount: 0, - autonomyReachedAt: null, - stalledReason: null, - }, - }, { id: "c2", kind: "OPEN_PULL_REQUESTS", @@ -79,7 +65,7 @@ describe("BoardPage Accessibility", () => { , ); - await waitFor(() => expect(screen.getByLabelText("Your path here")).toBeInTheDocument()); + await waitFor(() => expect(screen.getByText("Your open pull requests")).toBeInTheDocument()); expect(await axe(baseElement)).toHaveNoViolations(); }); diff --git a/tests/unit/features/board/BoardGrid.test.tsx b/tests/unit/features/board/BoardGrid.test.tsx index c277fd913..09bc6a28c 100644 --- a/tests/unit/features/board/BoardGrid.test.tsx +++ b/tests/unit/features/board/BoardGrid.test.tsx @@ -8,27 +8,9 @@ import type { BoardCard, CurrentTaskContent, OpenPullRequestsContent, - PathToFirstContributionContent, SuggestedTasksContent, } from "../../../../src/features/board/types"; -const pathContent = ( - over: Partial = {}, -): PathToFirstContributionContent => ({ - kind: "PATH_TO_FIRST_CONTRIBUTION", - moments: [ - { key: "JOINED", reachedAt: "2026-07-20T09:00:00Z" }, - { key: "TASK_CLAIMED", reachedAt: null }, - { key: "WORK_SUBMITTED", reachedAt: null }, - { key: "FIRST_RESPONSE", reachedAt: null }, - { key: "WORK_ACCEPTED", reachedAt: null }, - ], - acceptedCount: 0, - autonomyReachedAt: null, - stalledReason: null, - ...over, -}); - const pullRequestContent = ( over: Partial = {}, ): OpenPullRequestsContent => ({ @@ -44,7 +26,6 @@ const currentTaskContent = (over: Partial = {}): CurrentTask title: "Fix the flaky login test", summary: "It fails about one run in five.", url: null, - chosen: true, ...over, }); @@ -72,41 +53,6 @@ function board(cards: BoardCard["content"][], placedAt: string | null = null): B } describe("BoardGrid", () => { - it("shows an unreached moment as a dash, never as a zero", () => { - render(); - - // Four moments unreached, one (joined) reached. - expect(screen.getAllByText("—")).toHaveLength(4); - }); - - it("says nothing has been merged yet without making it sound like a failure", () => { - render(); - - expect(screen.getByText(/normal early on/i)).toBeInTheDocument(); - }); - - it("counts accepted work with the plural", () => { - render(); - - expect(screen.getByText("2 changes merged")).toBeInTheDocument(); - }); - - it("tells the hire about their own stall, and points at a person", () => { - render(); - - expect(screen.getByText(/no response in 5 days/)).toBeInTheDocument(); - // Points at a person rather than leaving the hire with a diagnosis they cannot act on. - expect(screen.getByText(/a person unblocks in a minute/i)).toBeInTheDocument(); - }); - - it("dates the end of onboarding rather than scoring it", () => { - render( - , - ); - - expect(screen.getByText(/worked unsupervised here/i)).toBeInTheDocument(); - }); - it("flags a long wait as the review being outstanding, not the hire being slow", () => { render( { it("claims the buddy added a card only when the buddy actually placed it", () => { // The mark is an icon in the header; the sentence lives in its screen-reader text, which is // what this asserts on — the fact is what matters, not how wide it is drawn. - const { rerender } = render(); + const { rerender } = render(); expect(screen.getByText("Kept up to date for you")).toBeInTheDocument(); expect(screen.queryByText("Your buddy added this card")).not.toBeInTheDocument(); - rerender(); + rerender(); // Attribution the hire cannot check is attribution they cannot trust, so the stronger // label is reserved for cards that carry a placement. expect(screen.getByText("Your buddy added this card")).toBeInTheDocument(); @@ -184,9 +130,9 @@ describe("BoardGrid", () => { it("offers to remove a card, and says the buddy will not put it back", () => { const onDismiss = vi.fn(); - render(); + render(); - const remove = screen.getByRole("button", { name: /remove the your path here card/i }); + const remove = screen.getByRole("button", { name: /remove the your open pull requests card/i }); expect(remove).toHaveAttribute("title", expect.stringMatching(/won't put it back/i)); fireEvent.click(remove); @@ -194,20 +140,17 @@ describe("BoardGrid", () => { }); it("has no remove control when removing is not offered", () => { - render(); + render(); expect(screen.queryByRole("button", { name: /remove the/i })).not.toBeInTheDocument(); }); - it("separates a task the hire picked from one they were handed", () => { - const { rerender } = render( - , - ); - expect(screen.getByText("You picked this one")).toBeInTheDocument(); + it("says the task on the card is one the hire picked", () => { + render(); - rerender(); - // Only one of the two is theirs to change their mind about. - expect(screen.getByText("Handed to you as a first task")).toBeInTheDocument(); + // Nothing hands a hire a task any more, so the card only ever holds one they claimed. + expect(screen.getByText("You picked this one")).toBeInTheDocument(); + expect(screen.queryByText(/handed to you/i)).not.toBeInTheDocument(); }); it("keeps the current-task card when there is no task, and says so", () => { diff --git a/tests/unit/features/board/BoardSubmenus.test.tsx b/tests/unit/features/board/BoardSubmenus.test.tsx index 846ba4d3a..f060c153e 100644 --- a/tests/unit/features/board/BoardSubmenus.test.tsx +++ b/tests/unit/features/board/BoardSubmenus.test.tsx @@ -53,26 +53,6 @@ describe("taking a card into the conversation", () => { expect(lastDraft()).toMatch(/where do I stand/i); }); - it("a stalled path asks about the thing that is actually stuck", () => { - render( - , - ); - - fireEvent.click(screen.getByRole("button", { name: /ask your buddy about this/i })); - - expect(lastDraft()).toContain("no response in 5 days"); - }); - it("claiming a suggested task goes through the buddy, not around the confirm gate", () => { render( { title: "Fix the flaky login test", summary: null, url: null, - chosen: true, }, ])} />, diff --git a/tests/unit/features/board/Marked.test.tsx b/tests/unit/features/board/Marked.test.tsx index a2508a7c5..0ea738a61 100644 --- a/tests/unit/features/board/Marked.test.tsx +++ b/tests/unit/features/board/Marked.test.tsx @@ -81,7 +81,6 @@ describe("highlights on a card the board re-reads", () => { title: "Ship the importer", summary: "Roll it out behind a feature flag first.", url: null, - chosen: true, }} card={card} />, diff --git a/tests/unit/features/board/cardAccents.test.ts b/tests/unit/features/board/cardAccents.test.ts index 5e0312bd0..266a06406 100644 --- a/tests/unit/features/board/cardAccents.test.ts +++ b/tests/unit/features/board/cardAccents.test.ts @@ -6,7 +6,6 @@ import { cardAccent } from "../../../../src/features/board/layout/cardAccents"; import type { BoardCardKind } from "../../../../src/features/board/types"; const KINDS: BoardCardKind[] = [ - "PATH_TO_FIRST_CONTRIBUTION", "CURRENT_TASK", "DIAGRAM", "ARRIVAL_STEPS", diff --git a/tests/unit/features/board/pathToCards.test.ts b/tests/unit/features/board/pathToCards.test.ts deleted file mode 100644 index f2d493c41..000000000 --- a/tests/unit/features/board/pathToCards.test.ts +++ /dev/null @@ -1,223 +0,0 @@ -import { describe, it, expect } from "vitest"; -import { - markTitle, - planCardsFromPath, - readableTitle, - sourceOfTitle, - titleKey, -} from "../../../../src/features/board/generation/pathToCards"; -import type { - OnboardingPathEndpoint, - OnboardingPhaseEndpoint, - OnboardingStepEndpoint, -} from "../../../../src/features/onboarding/types"; - -function step(over: Partial = {}): OnboardingStepEndpoint { - return { - id: "step-1", - phaseId: "phase-1", - position: 0, - title: "Set up your machine", - description: "", - type: "TASK", - estimatedMinutes: 30, - expectedOutcomes: [], - tasks: [], - resources: [], - status: "WAITING", - startedAt: null, - completedAt: null, - feedback: null, - skip: null, - ...over, - }; -} - -function phase(over: Partial = {}): OnboardingPhaseEndpoint { - return { - id: "phase-1", - pathId: "path-1", - position: 0, - title: "Getting set up", - description: "", - locked: false, - steps: [step()], - questions: [], - ...over, - }; -} - -function path(phases: OnboardingPhaseEndpoint[]): OnboardingPathEndpoint { - return { id: "path-1", userId: "user-1", createdAt: "2026-01-01T00:00:00Z", phases }; -} - -describe("planCardsFromPath", () => { - it("makes one card per step, with its tasks as the lines", () => { - const plan = planCardsFromPath( - path([ - phase({ - steps: [ - step({ - tasks: [ - { - id: "t2", - stepId: "step-1", - position: 1, - title: "Install Node", - description: "", - finished: false, - }, - { - id: "t1", - stepId: "step-1", - position: 0, - title: "Clone the repo", - description: "", - finished: false, - }, - ], - }), - ], - }), - ]), - ); - - expect(plan.cardCount).toBe(1); - const request = plan.areas[0].cards[0].request; - expect(request.kind).toBe("CHECKLIST"); - if (request.kind !== "CHECKLIST") throw new Error("expected a checklist"); - expect(request.items.map((item) => item.text)).toEqual(["Clone the repo", "Install Node"]); - }); - - it("names the area after the phase and stages it by position", () => { - const plan = planCardsFromPath( - path([ - phase({ id: "p1", position: 0, title: "Week one" }), - phase({ id: "p2", position: 1, title: "Week two", steps: [step({ id: "s2" })] }), - phase({ id: "p3", position: 2, title: "Later on", steps: [step({ id: "s3" })] }), - ]), - ); - - // The stage rides on the card, not on the area: an area is where a card is filed, and when it - // is due is a separate question that folds inside every area the same way. - expect(plan.areas.map((area) => [area.name, area.cards[0].stage])).toEqual([ - ["Week one", "NOW"], - ["Week two", "LATER"], - ["Later on", "LATER"], - ]); - }); - - it("chains steps inside a phase but not across phases", () => { - const plan = planCardsFromPath( - path([ - phase({ - id: "p1", - steps: [step({ id: "a", position: 0 }), step({ id: "b", position: 1 })], - }), - phase({ id: "p2", position: 1, steps: [step({ id: "c" })] }), - ]), - ); - - expect(plan.areas[0].cards.map((card) => card.afterKey)).toEqual([null, "a"]); - // Across phases the stage carries the order. Chaining here too would leave the hire with - // exactly one card they are allowed to open. - expect(plan.areas[1].cards[0].afterKey).toBeNull(); - }); - - it("leaves finished and skipped steps off the board", () => { - const plan = planCardsFromPath( - path([ - phase({ - steps: [ - step({ id: "done", status: "FINISHED" }), - step({ id: "skipped", position: 1, status: "SKIPPED" }), - step({ id: "open", position: 2 }), - ], - }), - ]), - ); - - expect(plan.cardCount).toBe(1); - expect(plan.areas[0].cards[0].key).toBe("open"); - }); - - it("falls back to expected outcomes, then to the step's own title", () => { - const plan = planCardsFromPath( - path([ - phase({ - steps: [ - step({ id: "outcomes", expectedOutcomes: ["The project builds locally"] }), - step({ id: "bare", position: 1, title: "Read the architecture doc" }), - ], - }), - ]), - ); - - const lines = plan.areas[0].cards.map((card) => - card.request.kind === "CHECKLIST" ? card.request.items.map((item) => item.text) : [], - ); - expect(lines).toEqual([["The project builds locally"], ["Read the architecture doc"]]); - }); - - it("produces no area for a phase with nothing left to do", () => { - const plan = planCardsFromPath(path([phase({ steps: [step({ status: "FINISHED" })] })])); - - expect(plan.areas).toEqual([]); - }); -}); - -describe("card source markers", () => { - it("round-trips a title through a marker without changing what a person reads", () => { - const stored = markTitle("TEAM", "Read the incident write-up"); - - expect(sourceOfTitle(stored)).toBe("TEAM"); - expect(readableTitle(stored)).toBe("Read the incident write-up"); - }); - - it("tells the two generated sources apart", () => { - expect(sourceOfTitle(markTitle("PATH", "Set up your machine"))).toBe("PATH"); - expect(sourceOfTitle(markTitle("TEAM", "Set up your machine"))).toBe("TEAM"); - }); - - it("reports a hand-written title as coming from nobody", () => { - expect(sourceOfTitle("Groceries")).toBeNull(); - expect(sourceOfTitle(null)).toBeNull(); - expect(readableTitle("Groceries")).toBe("Groceries"); - }); - - it("marks path cards as coming from the path", () => { - const plan = planCardsFromPath(path([phase()])); - const request = plan.areas[0].cards[0].request; - if (request.kind !== "CHECKLIST") throw new Error("expected a checklist"); - - expect(sourceOfTitle(request.title ?? null)).toBe("PATH"); - expect(readableTitle(request.title ?? "")).toBe("Set up your machine"); - }); -}); - -describe("recognising a card that is already there", () => { - it("stores a title the way the server will, so a second run finds it", () => { - // The server trims what it stores. A title planned with the space still on it would never - // match the card it just wrote, and every run would add another copy. - expect(markTitle("TEAM", "Read the runbook ")).toBe(markTitle("TEAM", "Read the runbook")); - }); - - it("matches a stored title against the one that was planned", () => { - expect(titleKey(markTitle("TEAM", "Read the runbook"))).toBe(titleKey("Read the runbook")); - }); - - it("ignores the differences nobody can see", () => { - expect(titleKey(" Read the runbook ")).toBe(titleKey("read the runbook")); - }); - - it("treats the same work from the path and from the team as one card", () => { - expect(titleKey(markTitle("PATH", "Read the runbook"))).toBe( - titleKey(markTitle("TEAM", "Read the runbook")), - ); - }); - - it("keeps two differently named cards apart", () => { - expect(titleKey("Read the runbook")).not.toBe(titleKey("Read the handbook")); - expect(titleKey(null)).toBe(""); - }); -}); diff --git a/tests/unit/features/board/useBoard.test.tsx b/tests/unit/features/board/useBoard.test.tsx index 63df67548..8050f2243 100644 --- a/tests/unit/features/board/useBoard.test.tsx +++ b/tests/unit/features/board/useBoard.test.tsx @@ -23,17 +23,11 @@ const board = (cardIds: string[]): Board => ({ projectId: "p1", cards: cardIds.map((id, index) => ({ id, - kind: "PATH_TO_FIRST_CONTRIBUTION", + kind: "OPEN_PULL_REQUESTS", owner: "AI", position: index, placedAt: null, - content: { - kind: "PATH_TO_FIRST_CONTRIBUTION", - moments: [], - acceptedCount: 0, - autonomyReachedAt: null, - stalledReason: null, - }, + content: { kind: "OPEN_PULL_REQUESTS", pullRequests: [], attributionMissing: false }, })), }); @@ -125,7 +119,7 @@ describe("useBoard", () => { expect(accepted).toBe(false); expect(result.current.writeError).toBe(true); - expect(result.current.board?.cards[0].content.kind).toBe("PATH_TO_FIRST_CONTRIBUTION"); + expect(result.current.board?.cards[0].content.kind).toBe("OPEN_PULL_REQUESTS"); }); it("keeps the card and surfaces the failure when removal does not go through", async () => { diff --git a/tests/unit/features/buddy/BuddyActionProposals.test.tsx b/tests/unit/features/buddy/BuddyActionProposals.test.tsx index 51e87c002..f6ae48d12 100644 --- a/tests/unit/features/buddy/BuddyActionProposals.test.tsx +++ b/tests/unit/features/buddy/BuddyActionProposals.test.tsx @@ -12,8 +12,8 @@ vi.mock("../../../../src/features/buddy/components/BuddyOrientationCard", () => function action(overrides: Partial = {}): ProposedAction { return { id: "a1", - action: "claim_task_zero", - label: "Start Task 0", + action: "claim_goal", + label: "Work toward this task", status: "idle", ...overrides, }; @@ -34,7 +34,7 @@ describe("BuddyActionProposals", () => { // Rendering the offer must not fire the action. expect(onConfirm).not.toHaveBeenCalled(); - await userEvent.click(screen.getByRole("button", { name: /Start Task 0/ })); + await userEvent.click(screen.getByRole("button", { name: /Work toward this task/ })); expect(onConfirm).toHaveBeenCalledWith("m1", expect.objectContaining({ id: "a1" })); }); @@ -61,14 +61,16 @@ describe("BuddyActionProposals", () => { render( , ); - expect(screen.getByText("Task 0 is yours.")).toBeInTheDocument(); - expect(screen.queryByRole("button", { name: /Start Task 0/ })).not.toBeInTheDocument(); + expect(screen.getByText("You are now working toward it.")).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /Work toward this task/ })).not.toBeInTheDocument(); }); it("offers a retry on a transport error", () => { @@ -83,7 +85,7 @@ describe("BuddyActionProposals", () => { expect(screen.getByText(/try again/i)).toBeInTheDocument(); // The confirm button is still there to retry. - expect(screen.getByRole("button", { name: /Start Task 0/ })).toBeInTheDocument(); + expect(screen.getByRole("button", { name: /Work toward this task/ })).toBeInTheDocument(); }); it("renders the orientation packet in the thread once open_orientation resolves", () => { @@ -112,7 +114,9 @@ describe("BuddyActionProposals", () => { const { rerender } = render( , diff --git a/tests/unit/services/buddyService.test.ts b/tests/unit/services/buddyService.test.ts index 62d4afd38..6f55f9bbc 100644 --- a/tests/unit/services/buddyService.test.ts +++ b/tests/unit/services/buddyService.test.ts @@ -247,7 +247,7 @@ describe("buddyService", () => { start(controller) { controller.enqueue( encoder.encode( - 'data: {"type":"action_proposal","action":"claim_task_zero","label":"Start Task 0"}\n\n', + 'data: {"type":"action_proposal","action":"claim_goal","label":"Work toward this task"}\n\n', ), ); controller.enqueue( @@ -277,8 +277,8 @@ describe("buddyService", () => { }); expect(onActionProposal).toHaveBeenCalledWith({ - action: "claim_task_zero", - label: "Start Task 0", + action: "claim_goal", + label: "Work toward this task", question: undefined, taskId: undefined, }); @@ -453,14 +453,14 @@ describe("buddyService", () => { server.use( http.post("/api/v1/onboarding/me/buddy/actions", async ({ request }) => { capturedBody = await request.json(); - return HttpResponse.json({ ok: true, message: "Task 0 is yours." }); + return HttpResponse.json({ ok: true, message: "You are now working toward it." }); }), ); - const result = await performAction("claim_task_zero"); + const result = await performAction("claim_goal"); - expect(result).toEqual({ ok: true, message: "Task 0 is yours." }); - expect(capturedBody).toMatchObject({ action: "claim_task_zero" }); + expect(result).toEqual({ ok: true, message: "You are now working toward it." }); + expect(capturedBody).toMatchObject({ action: "claim_goal" }); }); it("sends the composed question for a flag-to-PM confirmation", async () => { From f7f4e0667d11625608f9abb7d251b43c0863d0c7 Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Tue, 15 Sep 2026 12:53:18 +0200 Subject: [PATCH 10/20] Call the PM metrics contribution metrics They measure joining to a first accepted contribution and review waits, not onboarding progress, which is the path. Only visible wording changes; the route and file names still say onboarding. Refs #311 Co-Authored-By: Claude Opus 5 --- .../components/HireTimelineCard.tsx | 2 +- .../components/OnboardingMetricsPage.tsx | 24 +++++++++++-------- .../components/OnboardingMetricsWidget.tsx | 10 ++++---- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/features/onboarding-metrics/components/HireTimelineCard.tsx b/src/features/onboarding-metrics/components/HireTimelineCard.tsx index 688ecfb8b..48b631f62 100644 --- a/src/features/onboarding-metrics/components/HireTimelineCard.tsx +++ b/src/features/onboarding-metrics/components/HireTimelineCard.tsx @@ -33,7 +33,7 @@ function gapHours(from: string | null, to: string | null): number | null { } /** - * One hire's onboarding timeline: joined → task claimed → work submitted → first + * One hire's contribution timeline: joined → task claimed → work submitted → first * response → accepted, with the gap between each pair of moments that has actually * happened. An unreached moment is a hollow, dashed dot and a dash, never a zero. * diff --git a/src/features/onboarding-metrics/components/OnboardingMetricsPage.tsx b/src/features/onboarding-metrics/components/OnboardingMetricsPage.tsx index 5eba22812..cb18df051 100644 --- a/src/features/onboarding-metrics/components/OnboardingMetricsPage.tsx +++ b/src/features/onboarding-metrics/components/OnboardingMetricsPage.tsx @@ -69,8 +69,12 @@ function hasActivity(hires: HireTimeline[]): boolean { } /** - * The PM readout for the numbers the onboarding redesign is judged on: - * time-to-first-accepted-work, response latency, and who is stalled. The + * The PM readout for how a project's people get their work in: + * time-to-first-accepted-work, response latency, and whose work is stalled. + * + * Not onboarding progress. Onboarding is the path a PM's blueprint prescribes, and how far a hire + * is along it is on the team pages; this is the contribution side beside it. The route and the + * file names still say "onboarding" from before that split. The * aggregates lead, and the per-hire timelines follow, stalled first. * * Deliberately a measurement readout, not another dashboard: no completion @@ -129,12 +133,12 @@ export function OnboardingMetricsPage() { pendingRefreshRef.current = false; setRefreshing(false); if (error) { - toast.error("Couldn't refresh onboarding metrics", { description: "Try again shortly." }); + toast.error("Couldn't refresh contribution metrics", { description: "Try again shortly." }); return; } if (!metrics || metrics.memberCount === 0) return; if (!hasActivity(metrics.hires)) { - toast.info("No onboarding activity yet", { + toast.info("No contribution activity yet", { description: "Nothing has happened on this project yet — that's different from nobody being here.", }); @@ -152,8 +156,8 @@ export function OnboardingMetricsPage() { } if (!errorToastRef.current && !pendingRefreshRef.current) { errorToastRef.current = true; - toast.error("Couldn't load onboarding metrics", { - description: "The onboarding metrics couldn't be loaded. Try again shortly.", + toast.error("Couldn't load contribution metrics", { + description: "The contribution metrics couldn't be loaded. Try again shortly.", }); } }, [loading, error, toast]); @@ -244,8 +248,8 @@ export function OnboardingMetricsPage() {
{refreshButton}
@@ -266,11 +270,11 @@ export function OnboardingMetricsPage() { icon={} title="Couldn't load metrics" > - The onboarding metrics couldn't be loaded. Try again shortly. + The contribution metrics couldn't be loaded. Try again shortly. ) : !metrics || metrics.memberCount === 0 ? ( } title="No hires yet"> - Once people join this project, their onboarding shows up here. + Once people join this project, their contributions show up here. ) : !hasActivity(metrics.hires) ? ( } title="No data yet"> diff --git a/src/features/onboarding-metrics/components/OnboardingMetricsWidget.tsx b/src/features/onboarding-metrics/components/OnboardingMetricsWidget.tsx index 97cb75d7f..b842a2b84 100644 --- a/src/features/onboarding-metrics/components/OnboardingMetricsWidget.tsx +++ b/src/features/onboarding-metrics/components/OnboardingMetricsWidget.tsx @@ -80,10 +80,10 @@ function AttentionRow({ item }: { item: AttentionItem }) { } /** - * Compact PM-dashboard summary of onboarding health, a peer card to the other + * Compact PM-dashboard summary of how hires get their work in, a peer card to the other * Insights widgets (`FaqWidget`, `KnowledgeGapWidget`). It deliberately carries * only the two questions a PM answers at a glance — how many hires are stuck, and - * how fast onboarding reaches a first accepted piece of work — plus a short "who + * how fast a hire reaches a first accepted piece of work — plus a short "who * needs a human" preview; everything else lives on the full readout it links to. * * The metrics are derived on request, so "refresh" is a client-side refetch @@ -125,7 +125,7 @@ export function OnboardingMetricsWidget() { try { await reloadAttention(); } catch { - toast.error("Couldn't refresh onboarding metrics", { description: "Try again shortly." }); + toast.error("Couldn't refresh contribution metrics", { description: "Try again shortly." }); } finally { setRefreshing(false); } @@ -144,7 +144,7 @@ export function OnboardingMetricsWidget() {

- Onboarding metrics couldn't be loaded for this project. + Contribution metrics couldn't be loaded for this project.

- {/* A skip request is sent to a person in the hire's name, so the words go on screen - before the click -- the label only names the step. */} + {/* Anything that leaves the product in the hire's name shows its words before the + click -- the label only names the kind of thing it is. A skip request carries the + reason; a flag to the PM carries the question the buddy composed, which is the whole + of what that person will read. */} {action.reason && (

Your reason: “{action.reason}”

)} + {action.action === BUDDY_ACTION_FLAG_TO_PM && action.question && ( +

+ Sends to your PM: “{action.question}” +

+ )} {action.status === "error" && (

Couldn't reach the server — try again. diff --git a/src/features/buddy/types.ts b/src/features/buddy/types.ts index 83cca1129..bf4c10d6a 100644 --- a/src/features/buddy/types.ts +++ b/src/features/buddy/types.ts @@ -17,6 +17,12 @@ export type ProposedActionStatus = "idle" | "confirming" | "resolved" | "error" */ export const BUDDY_ACTION_OPEN_ORIENTATION = "open_orientation"; +/** + * The backend's `flag_to_pm` action. Its `question` is the message that goes to the PM, so the + * confirm shows it: this one leaves the product and arrives in somebody's inbox in the hire's name. + */ +export const BUDDY_ACTION_FLAG_TO_PM = "flag_to_pm"; + /** * The actions that change the hire's onboarding path. * @@ -40,7 +46,10 @@ export type ProposedAction = { action: string; /** The button text ("Work toward this task"). */ label: string; - /** Carried through only for flag-to-PM: the question the buddy composed. */ + /** + * Carried through only for flag-to-PM: the question the buddy composed, and the one that is + * actually sent. Shown under the button — see `BuddyActionProposals`. + */ question?: string; /** * The goal-claim confirm payload (`claim_goal`), echoed back verbatim so the action runs diff --git a/src/features/starter-work/components/StarterWorkTaskDetails.tsx b/src/features/starter-work/components/StarterWorkTaskDetails.tsx index 28a8c4645..a32c4899e 100644 --- a/src/features/starter-work/components/StarterWorkTaskDetails.tsx +++ b/src/features/starter-work/components/StarterWorkTaskDetails.tsx @@ -248,7 +248,8 @@ export function StarterWorkTaskDetails({

Use as Task 0

- Handed to a new hire as their very first task, on any project. + Marks this as a gentle first task. A hint in the pool — it is not handed to + anybody.

{canAct ? ( diff --git a/src/features/starter-work/types.ts b/src/features/starter-work/types.ts index 568f2be98..37952522c 100644 --- a/src/features/starter-work/types.ts +++ b/src/features/starter-work/types.ts @@ -28,7 +28,7 @@ export type StarterWorkTask = { status: ProposalStatus; /** Whether a person has looked at this task. Unreviewed is claimable, just ranked lower. */ reviewed: boolean; - /** Whether a PM has flagged this task as suitable for a hire's automatic first task (Task 0). */ + /** Whether a PM has flagged this task as a good first one for somebody (Task 0). A hint, not a gate. */ taskZeroEligible: boolean; /** * Whether the issue had somebody on it when reconciliation last looked. Three-valued: null diff --git a/src/services/starterWorkService.ts b/src/services/starterWorkService.ts index 909acb334..403d99943 100644 --- a/src/services/starterWorkService.ts +++ b/src/services/starterWorkService.ts @@ -125,8 +125,10 @@ export const starterWorkService = { }, /** - * A PM's decision on whether a live starter-work task is suitable as a hire's Task 0 — the - * trivial first task somebody is auto-assigned once their environment is ready. + * A PM's judgement that a live starter-work task is a good first one for somebody ("Task 0"). + * + * A label on the task, not an assignment: nothing hands a flagged task to a hire, and an + * unflagged one is claimable by anybody. Onboarding is the path their blueprint prescribes. */ async setTaskZero(id: string, eligible: boolean): Promise { return await apiClient.fetch(`${BASE_URL}/${id}/task-zero`, { diff --git a/tests/unit/features/buddy/BuddyActionProposals.test.tsx b/tests/unit/features/buddy/BuddyActionProposals.test.tsx index f6ae48d12..1c6aac750 100644 --- a/tests/unit/features/buddy/BuddyActionProposals.test.tsx +++ b/tests/unit/features/buddy/BuddyActionProposals.test.tsx @@ -144,6 +144,42 @@ describe("BuddyActionProposals", () => { expect(screen.queryByTestId("buddy-orientation-card")).not.toBeInTheDocument(); }); + it("shows the question a flag will send, not just the button", () => { + // The button only says that something will be flagged. What lands in the PM's inbox is + // the question the buddy composed, and the hire sends it in their name. + render( + , + ); + + expect( + screen.getByText("Sends to your PM: “Who owns the staging database credentials?”"), + ).toBeInTheDocument(); + }); + + it("shows no message line for an action that sends nobody anything", () => { + render( + , + ); + + expect(screen.queryByText(/Sends to your PM/)).not.toBeInTheDocument(); + }); + it("shows the whole skip reason before the hire sends it in their name", () => { render( Date: Thu, 24 Sep 2026 16:40:01 +0200 Subject: [PATCH 12/20] Land buddy links on the reworked onboarding journey The buddy writes `/onboarding?step=`, `?question=` and `?phase=` links into its replies. The journey rework (#228) only knew `/onboarding/:stepId`, so these fell through to the overview. They are now an arrival like the others, with one difference kept on purpose: a link lands instead of starting. The owning phase opens in the list, the row scrolls into view and lights up once, and nothing is unfolded or started -- following a link is finding something, starting it is the hire's own click. The handled parameter is dropped from the address, and following the same link again lands again. Refs #311 Co-Authored-By: Claude Opus 5.5 --- .../components/journey/PhaseItemList.tsx | 13 ++- src/features/onboarding/journey.ts | 5 ++ src/pages/OnBoardingPage.tsx | 88 +++++++++++++++++-- tests/unit/pages/OnBoardingPage.test.tsx | 29 ++++++ 4 files changed, 126 insertions(+), 9 deletions(-) diff --git a/src/features/onboarding/components/journey/PhaseItemList.tsx b/src/features/onboarding/components/journey/PhaseItemList.tsx index 81309e0cf..d18538607 100644 --- a/src/features/onboarding/components/journey/PhaseItemList.tsx +++ b/src/features/onboarding/components/journey/PhaseItemList.tsx @@ -5,6 +5,7 @@ import { Button } from "../../../../components/ui/Button"; import { formatMinutes, itemState, + linkedCardId, orderedPhaseItems, waitingOn, type PhaseItem, @@ -18,6 +19,11 @@ type Props = { nextItemId: string | null; /** The item unfolded in place, if any. */ expandedItemId: string | null; + /** + * The item a link from the buddy landed on, lit up once. `key` is per arrival, so the same link + * followed twice plays the light twice. + */ + linkHighlight?: { id: string; key: string } | null; onToggle: (item: PhaseItem) => void; /** Start, continue or answer: opens the item in place, starting a step that was not started. */ onPrimary: (item: PhaseItem) => void; @@ -37,6 +43,7 @@ export function PhaseItemList({ phase, nextItemId, expandedItemId, + linkHighlight = null, onToggle, onPrimary, renderExpanded, @@ -63,12 +70,14 @@ export function PhaseItemList({ const minutes = item.kind === "step" ? item.step.estimatedMinutes : null; const muted = state === "done" || state === "skipped" || state === "locked"; const canUnfold = state !== "locked"; + const isLinked = linkHighlight?.id === item.id; return (
  • `, `?question=` or `?phase=`. + * + * The mentor is handed each item's link so it can write "you are on [#3](...)" and have that be + * clickable. In the URL rather than in router state because the model writes it into text the hire + * can copy, keep or open in a second tab, and state survives none of that. + * + * A link *lands* rather than starts: the phase opens, the page scrolls to the item and lights it up + * briefly. Unlike `/onboarding/:stepId`, nothing is unfolded or started -- following a link in a + * conversation is a way of finding something, and starting it is the hire's own click. + */ + const [searchParams, setSearchParams] = useSearchParams(); + const linkedItemId = searchParams.get("step") ?? searchParams.get("question"); + const linkedPhaseId = searchParams.get("phase"); const [path, setPath] = useState(null); const [loadingState, setLoadingState] = useState("loading"); @@ -223,6 +238,9 @@ export function OnBoardingPage() { const [confirmRegenerate, setConfirmRegenerate] = useState(false); // Set when the page itself moves the member on, so the item they land on is scrolled to. const scrollToItemRef = useRef(focusItemId ?? null); + // The item a buddy link landed on. `key` changes per arrival, so following the same link again + // restarts the light instead of leaving it spent. + const [linkHighlight, setLinkHighlight] = useState<{ id: string; key: string } | null>(null); usePathRevealMoment(loadingState === "success" ? path : null); @@ -333,15 +351,28 @@ export function OnBoardingPage() { ? { kind: "question" as const, id: focusQuestionId } : wantsChooser ? { kind: "choose" as const, id: "" } - : null, - [focusQuestionId, routeStepId, wantsChooser], + : linkedItemId + ? { kind: "link-item" as const, id: linkedItemId } + : linkedPhaseId + ? { kind: "link-phase" as const, id: linkedPhaseId } + : null, + [focusQuestionId, linkedItemId, linkedPhaseId, routeStepId, wantsChooser], ); - const arrivalKey = arrival ? `${arrival.kind}:${arrival.id}` : ""; + // A link carries the navigation's key as well: the hire who scrolled away and clicks the same link + // in the conversation again should land again. + const arrivalKey = arrival + ? `${arrival.kind}:${arrival.id}${arrival.kind.startsWith("link") ? `:${location.key}` : ""}` + : ""; const handledArrivalRef = useRef(""); // Read by the arrival effect, which must not re-run every time one of these is recreated. const startStepRef = useRef<(item: PhaseItem) => void>(() => undefined); const scrollToChooserRef = useRef<() => void>(() => undefined); + /** + * Takes a handled link out of the address, so a reload or the hire's own next click decides what + * they are looking at rather than the link lighting the same card up again. + */ + const clearLinkRef = useRef<() => void>(() => undefined); useEffect(() => { if (loadingState !== "success" || !path || !arrival) return; @@ -362,14 +393,27 @@ export function OnBoardingPage() { return; } + if (arrival.kind === "link-phase") { + if (path.phases.some((phase) => phase.id === arrival.id)) { + setSelectedPhaseId(arrival.id); + setExpandedItemId(null); + } else { + toast.error("That phase is not on your path", { + description: "It may have been replaced when your path was rebuilt.", + }); + } + clearLinkRef.current(); + return; + } + const owningPhase = path.phases.find((phase) => phaseItems(phase).some((item) => item.id === arrival.id), ); if (!owningPhase) { toast.error( - arrival.kind === "step" - ? "That step is not on your path" - : "That question is not on your path", + arrival.kind === "question" + ? "That question is not on your path" + : "That step is not on your path", { description: "It may have been replaced when your path was rebuilt." }, ); // Only the step has an address of its own to go back from; a question arrives in router @@ -378,6 +422,14 @@ export function OnBoardingPage() { return; } + if (arrival.kind === "link-item") { + setSelectedPhaseId(owningPhase.id); + setExpandedItemId(null); + setLinkHighlight({ id: arrival.id, key: arrivalKey }); + clearLinkRef.current(); + return; + } + setSelectedPhaseId(owningPhase.id); scrollToItemRef.current = arrival.id; setExpandedItemId(arrival.id); @@ -389,6 +441,27 @@ export function OnBoardingPage() { }); }, [arrival, arrivalKey, loadingState, navigate, path, toast]); + useEffect(() => { + clearLinkRef.current = () => + setSearchParams( + (params) => { + params.delete("step"); + params.delete("question"); + params.delete("phase"); + return params; + }, + { replace: true }, + ); + }, [setSearchParams]); + + // Scrolls to the card a link landed on, once its phase is the one on screen. + useEffect(() => { + if (loadingState !== "success" || !linkHighlight) return; + document + .getElementById(linkedCardId(linkHighlight.id)) + ?.scrollIntoView?.({ behavior: "smooth", block: "center" }); + }, [linkHighlight, loadingState, selectedPhaseId]); + // Scrolls to an item the page opened on the member's behalf -- a link, "up next", "continue". useEffect(() => { const target = scrollToItemRef.current; @@ -882,6 +955,7 @@ export function OnBoardingPage() { phase={selectedPhase} nextItemId={nextItemId} expandedItemId={expandedItemId} + linkHighlight={linkHighlight} onToggle={toggleItem} onPrimary={openItem} renderExpanded={(item) => renderItemBody(item, "inline")} diff --git a/tests/unit/pages/OnBoardingPage.test.tsx b/tests/unit/pages/OnBoardingPage.test.tsx index 3a2a0a7d0..eaad9f3b6 100644 --- a/tests/unit/pages/OnBoardingPage.test.tsx +++ b/tests/unit/pages/OnBoardingPage.test.tsx @@ -945,6 +945,35 @@ describe("OnBoardingPage: links from the buddy", () => { ); }); + /** + * The line between a buddy link and `/onboarding/:stepId`: the address opens and starts a step, + * a link in the conversation only shows the hire where it is. + */ + it("neither unfolds nor starts a step a link lands on", async () => { + const path = pathWithQuestion("OPEN"); + path.phases[1].steps[0].status = "WAITING"; + server.use(http.get("/api/v1/onboarding/me/path", () => HttpResponse.json(path))); + const startStep = vi.spyOn(onboardingService, "startStep"); + + const { container } = render( + + + , + ); + + await waitFor(() => + expect(container.querySelector("#onboarding-item-step-phase-2")).toHaveClass( + "app-link-highlight", + ), + ); + expect(startStep).not.toHaveBeenCalled(); + expect( + within(container.querySelector("#onboarding-item-step-phase-2")!).queryByRole("button", { + expanded: true, + }), + ).not.toBeInTheDocument(); + }); + it("lands on the phase a link names", async () => { server.use( http.get("/api/v1/onboarding/me/path", () => HttpResponse.json(pathWithQuestion("OPEN"))), From 133128d78d349f87ef66648f392790fabde2d253 Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Thu, 24 Sep 2026 16:44:26 +0200 Subject: [PATCH 13/20] Number the journey's steps and questions the way the buddy does The buddy names items "#3" and a hire answers "let's do 3". The reworked list and phase graph showed no numbers, so neither side could point at anything. Both now carry `itemNumbers` -- steps first, then questions, the rule `BuddyPathTools` numbers by. The list reads in graph order, so the numbers are labels rather than a count down it. Also keeps the link highlight working: the Tailwind Prettier plugin trimmed the space after `app-link-highlight` and glued it to the next class, so it now sits in its own interpolation at the end. Refs #311 Co-Authored-By: Claude Opus 5.5 --- .../components/journey/JourneyGraph.tsx | 4 ++++ .../components/journey/PhaseItemList.tsx | 11 +++++++++-- .../onboarding/graph/JourneyNodeCards.tsx | 9 +++++++++ tests/unit/pages/OnBoardingPage.test.tsx | 17 +++++++++++++++++ 4 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/features/onboarding/components/journey/JourneyGraph.tsx b/src/features/onboarding/components/journey/JourneyGraph.tsx index de2d421e7..e6c175c4f 100644 --- a/src/features/onboarding/components/journey/JourneyGraph.tsx +++ b/src/features/onboarding/components/journey/JourneyGraph.tsx @@ -30,6 +30,7 @@ import { type JourneyEdgeTone, } from "../../graph/JourneyCanvas"; import { ItemGlyph, ItemNodeCard, PhaseNodeCard } from "../../graph/JourneyNodeCards"; +import { itemNumbers } from "../../itemNumbers"; import { ITEM_LAYOUT, ITEM_NODE_SIZE, @@ -431,6 +432,8 @@ export function JourneyGraph({ const selectedItem = items.find((item) => item.id === selectedItemId) ?? null; const focusedItem = entersItems ? (items.find((item) => item.id === openItemId) ?? null) : null; const progress = phaseProgress(openPhase); + // The numbers the list and the buddy use for these items, so a node can be named by the one on it. + const numbers = itemNumbers(openPhase); return ( )} /> diff --git a/src/features/onboarding/components/journey/PhaseItemList.tsx b/src/features/onboarding/components/journey/PhaseItemList.tsx index d18538607..02e368441 100644 --- a/src/features/onboarding/components/journey/PhaseItemList.tsx +++ b/src/features/onboarding/components/journey/PhaseItemList.tsx @@ -11,6 +11,7 @@ import { type PhaseItem, } from "../../journey"; import { ItemFlags, ItemGlyph } from "../../graph/JourneyNodeCards"; +import { itemNumbers } from "../../itemNumbers"; import { itemKindLabel, itemStateLabel, primaryActionLabel } from "../../graph/nodeLabels"; import type { OnboardingPhaseEndpoint } from "../../types"; @@ -49,6 +50,9 @@ export function PhaseItemList({ renderExpanded, }: Props) { const items = orderedPhaseItems(phase); + // The numbers the buddy uses for the same items. Labels rather than a count down this list: the + // list reads in graph order, the numbers stay put so "#3" means one item on every surface. + const numbers = itemNumbers(phase); if (items.length === 0) { return ( @@ -77,7 +81,7 @@ export function PhaseItemList({ key={isLinked ? `${item.id}:${linkHighlight.key}` : item.id} id={linkedCardId(item.id)} data-item-id={item.id} - className={`overflow-hidden rounded-2xl border transition-colors ${isLinked ? "app-link-highlight" : ""}${ + className={`overflow-hidden rounded-2xl border transition-colors ${ isExpanded ? isQuestion ? "border-app-question-border bg-app-surface shadow-lg" @@ -89,7 +93,7 @@ export function PhaseItemList({ : isQuestion ? "border-app-question-border/60 bg-app-question-bg/30 hover:bg-app-question-bg/60" : "border-app-border/70 bg-app-surface/60 hover:bg-app-surface" - }`} + } ${isLinked ? "app-link-highlight" : ""}`} >
    diff --git a/tests/unit/pages/OnBoardingPage.test.tsx b/tests/unit/pages/OnBoardingPage.test.tsx index eaad9f3b6..c1b4c69f9 100644 --- a/tests/unit/pages/OnBoardingPage.test.tsx +++ b/tests/unit/pages/OnBoardingPage.test.tsx @@ -974,6 +974,23 @@ describe("OnBoardingPage: links from the buddy", () => { ).not.toBeInTheDocument(); }); + /** Steps first, then questions -- the numbering `BuddyPathTools` gives the mentor. */ + it("numbers the rows the way the buddy numbers them", async () => { + server.use( + http.get("/api/v1/onboarding/me/path", () => HttpResponse.json(pathWithQuestion("OPEN"))), + ); + + const { container } = render( + + + , + ); + + await screen.findByRole("heading", { name: "Meetings", level: 2 }); + expect(container.querySelector("#onboarding-item-step-phase-2")).toHaveTextContent("#1"); + expect(container.querySelector("#onboarding-item-q-linked")).toHaveTextContent("#2"); + }); + it("lands on the phase a link names", async () => { server.use( http.get("/api/v1/onboarding/me/path", () => HttpResponse.json(pathWithQuestion("OPEN"))), From 4fa02007757e19794b259d51f3b4fdddab551e25 Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Thu, 24 Sep 2026 16:48:03 +0200 Subject: [PATCH 14/20] Offer the buddy on the reworked journey's steps, questions and phases The step page, the question dialog and the old phase header each had a way into the conversation, and the journey rework (#228) replaced all three. The ways in come back where the things now live: - A step still open offers "Stuck? Ask your buddy about this step". - A question offers the material before an attempt (louder on one already answered wrong), and right after a wrong answer offers to go through it -- the buddy is not given the answer, so it never promises one. - The phase header offers a walkthrough, and an empty phase offers to work out with the buddy what it should contain; so does the screen shown when no phase could be generated. Each only pre-fills the composer; the hire sends it. Refs #311 Co-Authored-By: Claude Opus 5.5 --- src/features/onboarding/buddyDrafts.ts | 2 +- .../components/journey/QuestionWorkspace.tsx | 31 ++++++- .../components/journey/StepWorkspace.tsx | 10 ++ src/pages/OnBoardingPage.tsx | 24 +++++ .../journey/QuestionWorkspace.test.tsx | 91 +++++++++++++++++++ .../components/journey/StepWorkspace.test.tsx | 28 ++++++ 6 files changed, 183 insertions(+), 3 deletions(-) create mode 100644 tests/unit/features/onboarding/components/journey/QuestionWorkspace.test.tsx diff --git a/src/features/onboarding/buddyDrafts.ts b/src/features/onboarding/buddyDrafts.ts index 27d0458da..0b3357189 100644 --- a/src/features/onboarding/buddyDrafts.ts +++ b/src/features/onboarding/buddyDrafts.ts @@ -60,7 +60,7 @@ export function askAboutEmptyPhase(phaseTitle: string): string { } /** Opening a conversation about one step. */ -export function askAboutStep(step: OnboardingStepEndpoint): string { +export function askAboutStep(step: Pick): string { return `I'm on the onboarding step "${snippet(step.title)}". Can you help me get going on it?`; } diff --git a/src/features/onboarding/components/journey/QuestionWorkspace.tsx b/src/features/onboarding/components/journey/QuestionWorkspace.tsx index ccc2bb9ed..3280bfeed 100644 --- a/src/features/onboarding/components/journey/QuestionWorkspace.tsx +++ b/src/features/onboarding/components/journey/QuestionWorkspace.tsx @@ -7,9 +7,13 @@ import { emptyDraft, isAnswered, toSubmission, type DraftAnswer } from "../../ch import type { OnboardingQuestionEndpoint, QuestionAttemptResult } from "../../types"; import { CheckQuestionCard } from "../CheckQuestionCard"; import { ConfettiBurst } from "../ConfettiBurst"; +import { AskTheBuddy } from "../../../buddy/components/AskTheBuddy"; +import { askAboutQuestion, askAboutWrongAnswer } from "../../buddyDrafts"; type Props = { question: OnboardingQuestionEndpoint; + /** The phase the question belongs to, named in what the buddy is asked about it. */ + phaseTitle: string; /** After a graded attempt; `correct` and `onboardingCompleted` come from the backend. */ onAnswered: (result: QuestionAttemptResult) => Promise | void; continueLabel: string; @@ -22,7 +26,13 @@ type Props = { * The same grading as before, without the dialog: a wrong answer is offered again right there, and a * correct one moves on the way a finished step does. */ -export function QuestionWorkspace({ question, onAnswered, continueLabel, onContinue }: Props) { +export function QuestionWorkspace({ + question, + phaseTitle, + onAnswered, + continueLabel, + onContinue, +}: Props) { const toast = useToast(); const [draft, setDraft] = useState(emptyDraft); const [submitting, setSubmitting] = useState(false); @@ -95,9 +105,26 @@ export function QuestionWorkspace({ question, onAnswered, continueLabel, onConti Look at the answer below and try again. + {/* Where another guess used to be the only thing on offer. The buddy is not given the + answer, so this is help with the material -- what a wrong answer calls for. */} +

    • - ) : null} + ) : ( + // Before an attempt, and quiet: guessing costs nothing here, so this is an offer rather + // than a nudge. Louder on a question already answered wrong on an earlier visit. + + )} ) : null} + {/* Where a hire sits when they are stuck on a step, so the way out of being stuck belongs + here. Not once it is behind them: there is nothing left to be stuck on. */} + {!isBehind ? ( + + ) : null} {/* ── Where a skip request or feedback stands -- said up front, in colour ── */} diff --git a/src/pages/OnBoardingPage.tsx b/src/pages/OnBoardingPage.tsx index dedcfec10..a54291bfa 100644 --- a/src/pages/OnBoardingPage.tsx +++ b/src/pages/OnBoardingPage.tsx @@ -70,6 +70,8 @@ import type { } from "../features/onboarding/types"; import { useMoments } from "../features/moments"; import { useProjectContext } from "../features/projects/useProjectContext"; +import { AskTheBuddy } from "../features/buddy/components/AskTheBuddy"; +import { askAboutEmptyPhase, askAboutPhase } from "../features/onboarding/buddyDrafts"; import { ApiError } from "../services/apiClient"; import { onboardingGraphService } from "../services/onboardingGraphService"; import { onboardingService } from "../services/onboardingService"; @@ -700,6 +702,7 @@ export function OnBoardingPage() { handleAnswered(item.question, result)} continueLabel={next.label} onContinue={next.run} @@ -832,6 +835,15 @@ export function OnBoardingPage() { > Try generation again + {/* Generating again is the wrong hope when the corpus is what was thin -- it comes back + empty a second time. The conversation is the one thing here that can produce + something, so it is offered beside the retry rather than instead of it. */} + {generationIssues.length > 0 ? ( + + ) : null} ); } @@ -1097,6 +1109,7 @@ function PhaseHeaderCard({ const progress = phaseProgress(phase); const waitsOn = blockingPhases(phase, phases); const unlocks = phasesUnlockedBy(phase, phases); + const isEmpty = phase.steps.length === 0 && (phase.questions ?? []).length === 0; return (
    @@ -1110,6 +1123,17 @@ function PhaseHeaderCard({ {phase.description ? (

    {phase.description}

    ) : null} + {/* The phase-level way in. A hire who does not know why a phase is here is not helped by any + of the buttons below it -- and an empty phase is the case the buddy exists for: its title + still says what it was meant to cover, and the mentor can put the result on their path. */} +
    vi.fn()); + +vi.mock("../../../../../../src/features/buddy/aiBuddyBus", () => ({ + openAiBuddy: mockOpenAiBuddy, +})); + +vi.mock("../../../../../../src/services/onboardingService", () => ({ + onboardingService: { submitQuestionAttempt: vi.fn() }, +})); + +import { onboardingService } from "../../../../../../src/services/onboardingService"; + +const question: OnboardingQuestionEndpoint = { + id: "q1", + phaseId: "phase1", + position: 1, + type: "SHORT_TEXT", + question: "Who runs the retro?", + status: "OPEN", +}; + +function renderWorkspace(over: Partial = {}) { + render( + , + ); +} + +/** The draft the last "ask the buddy" control put in the composer. */ +function lastDraft(): string { + const calls = mockOpenAiBuddy.mock.calls as [{ draft: string }][]; + return calls[calls.length - 1][0].draft; +} + +describe("QuestionWorkspace: the buddy", () => { + beforeEach(() => vi.clearAllMocks()); + + it("is offered before an attempt, asking for the material rather than the answer", async () => { + const user = userEvent.setup(); + renderWorkspace(); + + await user.click( + screen.getByRole("button", { name: "Not sure? Ask your buddy to explain the material" }), + ); + + expect(lastDraft()).toContain("Who runs the retro?"); + expect(lastDraft()).toContain("Meetings"); + expect(lastDraft()).toContain("rather work the answer out"); + }); + + it("speaks up louder on a question already answered wrong", () => { + renderWorkspace({ status: "RETRY" }); + + expect(screen.getByRole("button", { name: "Go through this with your buddy" })).toBeVisible(); + }); + + it("offers to go through the material right after a wrong answer", async () => { + vi.mocked(onboardingService.submitQuestionAttempt).mockResolvedValue({ + attemptId: "a1", + questionId: "q1", + correct: false, + createdAt: "2026-09-24T10:00:00Z", + correctOptionIds: [], + correctAnswer: null, + explanation: null, + feedback: null, + status: "RETRY", + onboardingCompleted: false, + }); + const user = userEvent.setup(); + renderWorkspace(); + + await user.type(screen.getByRole("textbox"), "The PM"); + await user.click(screen.getByRole("button", { name: "Submit answer" })); + await user.click(await screen.findByRole("button", { name: "Go through it with your buddy" })); + + expect(lastDraft()).toContain("wrong"); + expect(lastDraft()).toContain("Who runs the retro?"); + }); +}); diff --git a/tests/unit/features/onboarding/components/journey/StepWorkspace.test.tsx b/tests/unit/features/onboarding/components/journey/StepWorkspace.test.tsx index 85565c79d..11c1fdc64 100644 --- a/tests/unit/features/onboarding/components/journey/StepWorkspace.test.tsx +++ b/tests/unit/features/onboarding/components/journey/StepWorkspace.test.tsx @@ -22,6 +22,13 @@ vi.mock("../../../../../../src/services/onboardingService", () => ({ }, })); +const mockOpenAiBuddy = vi.hoisted(() => vi.fn()); + +vi.mock("../../../../../../src/features/buddy/aiBuddyBus", () => ({ + openAiBuddy: mockOpenAiBuddy, + onBuddyPathChanged: () => () => undefined, +})); + import { onboardingService } from "../../../../../../src/services/onboardingService"; const step = { @@ -231,6 +238,27 @@ describe("StepWorkspace", () => { expect(mockFlyby).toHaveBeenCalled(); }); + it("offers the buddy on a step that is still open, with the step in the draft", async () => { + const user = userEvent.setup(); + renderWorkspace(); + + await user.click( + await screen.findByRole("button", { name: "Stuck? Ask your buddy about this step" }), + ); + + expect(mockOpenAiBuddy).toHaveBeenCalledWith({ + draft: expect.stringContaining("Setup Environment") as string, + }); + }); + + it("does not offer the buddy on a step that is behind the hire", async () => { + vi.mocked(onboardingService.fetchStep).mockResolvedValue({ ...step, status: "FINISHED" }); + renderWorkspace(); + + await screen.findByText("Set up your dev environment"); + expect(screen.queryByRole("button", { name: /ask your buddy/i })).not.toBeInTheDocument(); + }); + it("sends a skip request with a reason", async () => { vi.mocked(onboardingService.skipStep).mockResolvedValue({ id: "skip1", From c1ecb0e7677b0e33880c470684e893d8ddce77d1 Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Thu, 24 Sep 2026 16:53:10 +0200 Subject: [PATCH 15/20] Refresh every path surface after the buddy changes the path The dock confirms path actions (tick a step or a line off, answer, add a step, request a skip) over whatever page is open, and announces it with `announceBuddyPathChanged`. After the journey rework nothing listened any more, so the page behind the dock kept showing the old state. - The onboarding page re-reads the path; an open step re-reads its tasks and status. - The board's "where you are" strip reads the path again. - `useBuddyPathSync`, mounted once in the app, marks the board and the onboarding status queries stale, which covers the PATH_STEP cards and the dashboard's next-step card. Refs #311 Co-Authored-By: Claude Opus 5.5 --- src/App.tsx | 2 + .../board/components/BoardPathWindow.tsx | 24 ++++++++---- src/features/buddy/hooks/useBuddyPathSync.ts | 25 +++++++++++++ .../components/journey/StepWorkspace.tsx | 8 +++- src/pages/OnBoardingPage.tsx | 11 ++++++ .../features/buddy/useBuddyPathSync.test.tsx | 28 ++++++++++++++ .../components/journey/StepWorkspace.test.tsx | 20 ++++++++-- tests/unit/pages/OnBoardingPage.test.tsx | 37 ++++++++++++++++++- 8 files changed, 142 insertions(+), 13 deletions(-) create mode 100644 src/features/buddy/hooks/useBuddyPathSync.ts create mode 100644 tests/unit/features/buddy/useBuddyPathSync.test.tsx diff --git a/src/App.tsx b/src/App.tsx index 68f5cf5c0..1d88dc394 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -21,12 +21,14 @@ import { AuroraBackground } from "./components/layout/AuroraBackground"; import { MyKnowledgeGapsProvider } from "./features/knowledge-gaps/MyKnowledgeGapsProvider"; import { KnowledgeGapOwnerAnnouncement } from "./features/knowledge-gaps/components/KnowledgeGapOwnerAnnouncement"; import { useScrollRestoration } from "./hooks/useScrollRestoration"; +import { useBuddyPathSync } from "./features/buddy/hooks/useBuddyPathSync"; function AppContent() { const { status } = useAuth(); const { showRocketPet } = useMoments(); const { isFocused } = useFocusMode(); useScrollRestoration(); + useBuddyPathSync(); // Signed in at all — the shell is drawn for anyone past the login screen, onboarding included. // `signingOut` stays out on purpose: it is the boot script's "this load is a logout return" diff --git a/src/features/board/components/BoardPathWindow.tsx b/src/features/board/components/BoardPathWindow.tsx index 106a8d446..03a333e6c 100644 --- a/src/features/board/components/BoardPathWindow.tsx +++ b/src/features/board/components/BoardPathWindow.tsx @@ -15,6 +15,7 @@ import { type PathWindowState, } from "../../onboarding/pathWindow.ts"; import { onboardingService } from "../../../services/onboardingService.ts"; +import { onBuddyPathChanged } from "../../buddy/aiBuddyBus.ts"; /** What each state is called and wears. Never colour alone — every one carries a word and a glyph. */ const STATES: Record< @@ -78,17 +79,24 @@ export function BoardPathWindow({ useEffect(() => { let cancelled = false; - void onboardingService - .fetchPath() - .then((path) => { - if (!cancelled) setWhere(pathWindow(path)); - }) - // No path, or no reaching it: the board has plenty else to show, and a strip that cannot - // say where somebody is should not say anything at all. - .catch(() => undefined); + const read = () => + void onboardingService + .fetchPath() + .then((path) => { + if (!cancelled) setWhere(pathWindow(path)); + }) + // No path, or no reaching it: the board has plenty else to show, and a strip that cannot + // say where somebody is should not say anything at all. + .catch(() => undefined); + + read(); + // Read again when the buddy moved the path on, so the strip under the dock says where the + // hire now stands. + const unsubscribe = onBuddyPathChanged(read); return () => { cancelled = true; + unsubscribe(); }; }, []); diff --git a/src/features/buddy/hooks/useBuddyPathSync.ts b/src/features/buddy/hooks/useBuddyPathSync.ts new file mode 100644 index 000000000..194bfbe6d --- /dev/null +++ b/src/features/buddy/hooks/useBuddyPathSync.ts @@ -0,0 +1,25 @@ +import { useQueryClient } from "@tanstack/react-query"; +import { useEffect } from "react"; +import { queryKeys } from "../../../services/queryKeys"; +import { onBuddyPathChanged } from "../aiBuddyBus"; + +/** + * Marks every cached read of the hire's path stale once the buddy changed it. + * + * The pages that fetch the path themselves listen for the signal on their own; this is for the + * ones that read it through the query cache -- the board, whose path-step cards and progress are + * part of the board query, and the dashboard's next-step card. Mounted once, at the top of the app, + * because the dock that confirms the change can sit over any of them. + */ +export function useBuddyPathSync(): void { + const queryClient = useQueryClient(); + + useEffect( + () => + onBuddyPathChanged(() => { + void queryClient.invalidateQueries({ queryKey: queryKeys.onboarding.myStatuses() }); + void queryClient.invalidateQueries({ queryKey: queryKeys.board.all() }); + }), + [queryClient], + ); +} diff --git a/src/features/onboarding/components/journey/StepWorkspace.tsx b/src/features/onboarding/components/journey/StepWorkspace.tsx index fe22b7c02..a0adf260f 100644 --- a/src/features/onboarding/components/journey/StepWorkspace.tsx +++ b/src/features/onboarding/components/journey/StepWorkspace.tsx @@ -28,6 +28,7 @@ import type { OnboardingTaskEndpoint, } from "../../types"; import { AskTheBuddy } from "../../../buddy/components/AskTheBuddy"; +import { onBuddyPathChanged } from "../../../buddy/aiBuddyBus"; import { askAboutStep } from "../../buddyDrafts"; import { StepOriginBadge } from "../StepOriginBadge"; import { TaskCheckItem } from "../TaskCheckItem"; @@ -96,6 +97,11 @@ export function StepWorkspace({ const [comment, setComment] = useState(""); const [feedbackSent, setFeedbackSent] = useState(false); const [now, setNow] = useState(() => Date.now()); + // Bumped when the buddy changed the path, so the open step re-reads its tasks and status: ticking + // a line off in the conversation must show on the checklist behind the dock. + const [buddyChanges, setBuddyChanges] = useState(0); + + useEffect(() => onBuddyPathChanged(() => setBuddyChanges((count) => count + 1)), []); useEffect(() => { const timer = window.setInterval(() => setNow(Date.now()), 60_000); @@ -142,7 +148,7 @@ export function StepWorkspace({ return () => { cancelled = true; }; - }, [stepId, stepStatus]); + }, [stepId, stepStatus, buddyChanges]); if (error) { return ( diff --git a/src/pages/OnBoardingPage.tsx b/src/pages/OnBoardingPage.tsx index a54291bfa..81f61376b 100644 --- a/src/pages/OnBoardingPage.tsx +++ b/src/pages/OnBoardingPage.tsx @@ -71,6 +71,7 @@ import type { import { useMoments } from "../features/moments"; import { useProjectContext } from "../features/projects/useProjectContext"; import { AskTheBuddy } from "../features/buddy/components/AskTheBuddy"; +import { onBuddyPathChanged } from "../features/buddy/aiBuddyBus"; import { askAboutEmptyPhase, askAboutPhase } from "../features/onboarding/buddyDrafts"; import { ApiError } from "../services/apiClient"; import { onboardingGraphService } from "../services/onboardingGraphService"; @@ -299,6 +300,16 @@ export function OnBoardingPage() { } }, [applyPath, toast]); + /** + * Re-reads the path after the buddy changed it. + * + * The buddy lives in a dock over this page, which is where a hire most likely is while talking + * about their path. Without this, confirming "mark this step as done" in the conversation left the + * list behind the dock still showing the step open. Told rather than polled; see + * `announceBuddyPathChanged`. + */ + useEffect(() => onBuddyPathChanged(() => void refreshPath()), [refreshPath]); + // A generation that finished -- here or while the user was elsewhere -- means there is a new path. // Read fresh rather than taken from the generation: the hire may have started working on it before // coming back here, and the stream's copy knows nothing of that. diff --git a/tests/unit/features/buddy/useBuddyPathSync.test.tsx b/tests/unit/features/buddy/useBuddyPathSync.test.tsx new file mode 100644 index 000000000..c07378d19 --- /dev/null +++ b/tests/unit/features/buddy/useBuddyPathSync.test.tsx @@ -0,0 +1,28 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { act, renderHook } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { describe, expect, it, vi } from "vitest"; +import { announceBuddyPathChanged } from "../../../../src/features/buddy/aiBuddyBus"; +import { useBuddyPathSync } from "../../../../src/features/buddy/hooks/useBuddyPathSync"; +import { queryKeys } from "../../../../src/services/queryKeys"; + +describe("useBuddyPathSync", () => { + it("marks the board and the onboarding status stale when the buddy changed the path", () => { + const client = new QueryClient(); + const invalidate = vi.spyOn(client, "invalidateQueries"); + const wrapper = ({ children }: { children: ReactNode }) => ( + {children} + ); + const { unmount } = renderHook(() => useBuddyPathSync(), { wrapper }); + + act(() => announceBuddyPathChanged()); + + expect(invalidate).toHaveBeenCalledWith({ queryKey: queryKeys.onboarding.myStatuses() }); + expect(invalidate).toHaveBeenCalledWith({ queryKey: queryKeys.board.all() }); + + unmount(); + invalidate.mockClear(); + act(() => announceBuddyPathChanged()); + expect(invalidate).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/unit/features/onboarding/components/journey/StepWorkspace.test.tsx b/tests/unit/features/onboarding/components/journey/StepWorkspace.test.tsx index 11c1fdc64..2c2fb2383 100644 --- a/tests/unit/features/onboarding/components/journey/StepWorkspace.test.tsx +++ b/tests/unit/features/onboarding/components/journey/StepWorkspace.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, waitFor } from "@testing-library/react"; +import { act, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { StepWorkspace } from "../../../../../../src/features/onboarding/components/journey/StepWorkspace"; @@ -24,12 +24,14 @@ vi.mock("../../../../../../src/services/onboardingService", () => ({ const mockOpenAiBuddy = vi.hoisted(() => vi.fn()); -vi.mock("../../../../../../src/features/buddy/aiBuddyBus", () => ({ +// The real path-changed signal, so a test can announce one the way the dock does. +vi.mock("../../../../../../src/features/buddy/aiBuddyBus", async (importOriginal) => ({ + ...(await importOriginal()), openAiBuddy: mockOpenAiBuddy, - onBuddyPathChanged: () => () => undefined, })); import { onboardingService } from "../../../../../../src/services/onboardingService"; +import { announceBuddyPathChanged } from "../../../../../../src/features/buddy/aiBuddyBus"; const step = { id: "step1", @@ -251,6 +253,18 @@ describe("StepWorkspace", () => { }); }); + it("re-reads the step after the buddy changed the path", async () => { + renderWorkspace(); + await screen.findByText("1/2 done"); + vi.mocked(onboardingService.fetchTasks).mockResolvedValue( + tasks.map((task) => ({ ...task, finished: true })), + ); + + act(() => announceBuddyPathChanged()); + + await waitFor(() => expect(screen.getByText("2/2 done")).toBeInTheDocument()); + }); + it("does not offer the buddy on a step that is behind the hire", async () => { vi.mocked(onboardingService.fetchStep).mockResolvedValue({ ...step, status: "FINISHED" }); renderWorkspace(); diff --git a/tests/unit/pages/OnBoardingPage.test.tsx b/tests/unit/pages/OnBoardingPage.test.tsx index c1b4c69f9..2e68053a4 100644 --- a/tests/unit/pages/OnBoardingPage.test.tsx +++ b/tests/unit/pages/OnBoardingPage.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, waitFor, within } from "@testing-library/react"; +import { act, render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { describe, it, expect, vi, beforeEach } from "vitest"; import { Link, MemoryRouter, Route, Routes, useLocation } from "react-router-dom"; @@ -6,6 +6,7 @@ import { OnBoardingPage } from "../../../src/pages/OnBoardingPage"; import { http, HttpResponse } from "msw"; import { server } from "../../unit/setup/vitest.setup"; import { onboardingService } from "../../../src/services/onboardingService"; +import { announceBuddyPathChanged } from "../../../src/features/buddy/aiBuddyBus"; import { OnboardingJourneyContext, type OnboardingJourneyValue, @@ -871,6 +872,40 @@ describe("OnBoardingPage", () => { * opened in a second tab. A step or question link *lands* on the card — its phase opens, the page * scrolls to it, and it lights up — and starts nothing: that stays the hire's own click. */ +describe("OnBoardingPage: changes the buddy made", () => { + beforeEach(() => { + vi.clearAllMocks(); + projectContextState.selectedProjectId = "proj1"; + }); + + it("re-reads the path when the buddy announces a change", async () => { + const fetchPath = vi.spyOn(onboardingService, "fetchPath"); + server.use( + http.get("/api/v1/onboarding/me/path", () => + HttpResponse.json({ + id: "path1", + userId: "user1", + createdAt: new Date().toISOString(), + generationIssues: [], + phases: [phaseFixture("phase-1", 0, "Overview")], + }), + ), + ); + + render( + + + , + ); + await screen.findByRole("heading", { name: "Overview", level: 2 }); + const before = fetchPath.mock.calls.length; + + act(() => announceBuddyPathChanged()); + + await waitFor(() => expect(fetchPath.mock.calls.length).toBeGreaterThan(before)); + }); +}); + describe("OnBoardingPage: links from the buddy", () => { beforeEach(() => { vi.clearAllMocks(); From c0fe1999755cdf40ae82835564df7d5045e5c935 Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Thu, 24 Sep 2026 16:54:15 +0200 Subject: [PATCH 16/20] Name the hire, not the reader, on the PM's journey view The step origin badge says "You added this" to the hire. The journey rework's member view reused it without `viewer="reviewer"`, so a PM read a hire's own step as one they had added themselves. Refs #311 Co-Authored-By: Claude Opus 5.5 --- .../detail/MemberJourneySection.tsx | 2 +- .../detail/MemberJourneySection.test.tsx | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/features/team-management/components/detail/MemberJourneySection.tsx b/src/features/team-management/components/detail/MemberJourneySection.tsx index 7a4b5574c..6e2116775 100644 --- a/src/features/team-management/components/detail/MemberJourneySection.tsx +++ b/src/features/team-management/components/detail/MemberJourneySection.tsx @@ -660,7 +660,7 @@ function StepFacts({ item, taskCount }: { item: PhaseItem; taskCount?: StepTaskC const delta = actual && item.step.estimatedMinutes ? actual - item.step.estimatedMinutes : null; return (
    - + {taskCount ? ( {taskCount.done}/{taskCount.total} tasks diff --git a/tests/unit/features/team-management/components/detail/MemberJourneySection.test.tsx b/tests/unit/features/team-management/components/detail/MemberJourneySection.test.tsx index d2d03e0b7..1e2ffcc6f 100644 --- a/tests/unit/features/team-management/components/detail/MemberJourneySection.test.tsx +++ b/tests/unit/features/team-management/components/detail/MemberJourneySection.test.tsx @@ -182,6 +182,27 @@ describe("MemberJourneySection", () => { expect(request).toMatchObject({ waitsOn: ["verify"], unlocks: [] }); }); + /** The PM reads this, so "you" in the badge would be about the PM. */ + it("names the hire, not the reader, on a step the hire added", () => { + const withHireStep: OnboardingPathEndpoint = { + ...path, + phases: [ + { + ...path.phases[0], + steps: [ + ...path.phases[0].steps, + step({ id: "own", position: 3, title: "My own step", origin: "HIRE" }), + ], + }, + path.phases[1], + ], + }; + renderSection({ path: withHireStep }); + + expect(screen.getByText("Added by the hire")).toBeInTheDocument(); + expect(screen.queryByText("You added this")).not.toBeInTheDocument(); + }); + it("says so when the member has no path yet", () => { renderSection({ path: null }); From b134578530c2b603d7e0b468720709ab691f7626 Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Thu, 24 Sep 2026 19:32:42 +0200 Subject: [PATCH 17/20] Don't start a step that is only opened to change its skip request The buddy links a step with a pending skip request to `/onboarding/` so the hire can change or withdraw the reason, which lives in the unfolded step. The journey page starts a waiting step when it unfolds it, so following that link began the very step the hire asked to skip. A step with a pending skip is now opened without being started; its own "Start" button still starts it. Refs #311 Co-Authored-By: Claude Opus 5.5 --- src/pages/OnBoardingPage.tsx | 4 ++ tests/unit/pages/OnBoardingPage.test.tsx | 47 ++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/src/pages/OnBoardingPage.tsx b/src/pages/OnBoardingPage.tsx index 81f61376b..83e2fd66e 100644 --- a/src/pages/OnBoardingPage.tsx +++ b/src/pages/OnBoardingPage.tsx @@ -50,6 +50,7 @@ import { ProgressRing } from "../features/onboarding/graph/JourneyNodeCards"; import { usePathRevealMoment } from "../features/onboarding/hooks/usePathRevealMoment"; import { blockingPhases, + isSkipPending, itemState, pathProgress, phaseItems, @@ -534,6 +535,9 @@ export function OnBoardingPage() { /** A step the member opens for the first time is started; reopening one changes nothing. */ const beginStepIfWaiting = async (item: PhaseItem) => { if (item.kind !== "step" || item.step.status !== "WAITING" || item.step.locked) return; + // Opened to change or withdraw a skip request -- which is what the buddy's link to the step is + // for -- is not beginning it. The step's own "Start" button still does that. + if (isSkipPending(item.step.skip)) return; try { await onboardingService.startStep(item.step.id); // The rocket marks a step *beginning*. diff --git a/tests/unit/pages/OnBoardingPage.test.tsx b/tests/unit/pages/OnBoardingPage.test.tsx index 2e68053a4..01d5b0866 100644 --- a/tests/unit/pages/OnBoardingPage.test.tsx +++ b/tests/unit/pages/OnBoardingPage.test.tsx @@ -707,6 +707,53 @@ describe("OnBoardingPage", () => { expect(await screen.findByRole("button", { name: "Mark as complete" })).toBeInTheDocument(); }); + /** + * The buddy links a step with a pending skip here so the hire can change or withdraw the reason. + * Opening it for that is not beginning it. + */ + it("opens a step waiting on a skip decision by its address without starting it", async () => { + const waiting = { + ...phaseFixture("phase1", 1, "Phase 1").steps[0], + status: "WAITING", + skip: { + id: "skip1", + stepId: "step-phase1", + reason: "I did this on my last team.", + accepted: null, + reviewComment: null, + reviewedAt: null, + }, + }; + server.use( + http.get("/api/v1/onboarding/me/steps/:stepId", () => HttpResponse.json(waiting)), + http.get("/api/v1/onboarding/me/steps/:stepId/tasks", () => HttpResponse.json([])), + http.get("/api/v1/onboarding/me/steps/:stepId/resources", () => HttpResponse.json([])), + http.get("/api/v1/onboarding/me/path", () => + HttpResponse.json({ + id: "path1", + userId: "user1", + createdAt: new Date().toISOString(), + phases: [{ ...phaseFixture("phase1", 1, "Phase 1"), steps: [waiting] }], + }), + ), + ); + const startStep = vi.spyOn(onboardingService, "startStep"); + + render( + + + + } /> + + + , + ); + + expect(await screen.findByText("Skip requested")).toBeInTheDocument(); + await expectUnfolded("step-phase1"); + expect(startStep).not.toHaveBeenCalled(); + }); + it("names what a locked item is waiting on", async () => { server.use( http.get("/api/v1/onboarding/me/path", () => From faac83d1406799a4d3af12af39166ab3ae265c00 Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Thu, 24 Sep 2026 20:03:01 +0200 Subject: [PATCH 18/20] Tidy the buddy-link arrival and two misplaced comments A dead question link now says it was a question, the reduced-motion link highlight goes away like the animated one instead of staying for good, and the linkedCardId and DependencySource doc comments sit where they belong. Co-Authored-By: Claude Opus 5.5 --- src/features/board/layout/boardStructure.ts | 2 +- src/features/onboarding/journey.ts | 2 +- src/pages/OnBoardingPage.tsx | 21 ++++++++------ src/styles/index.css | 15 ++++++++-- tests/unit/pages/OnBoardingPage.test.tsx | 31 +++++++++++++++++++++ 5 files changed, 57 insertions(+), 14 deletions(-) diff --git a/src/features/board/layout/boardStructure.ts b/src/features/board/layout/boardStructure.ts index 7e2b3b4fa..40de638be 100644 --- a/src/features/board/layout/boardStructure.ts +++ b/src/features/board/layout/boardStructure.ts @@ -90,11 +90,11 @@ export function stageOrder(stage: BoardStage): number { * - `TEAM` — from a card blueprint. The PM's, and the hire may not take it off. * - `BUDDY` — from a generated path. Named on the card so it does not look like the hire's own * doing, but still theirs to clear: the buddy is an assistant, not an authority. + * - `HIRE` — theirs, and the only kind their own controls write. * * Nothing writes `TEAM` or `BUDDY` any more: card blueprints and the generator that copied the path * onto the board were retired when the onboarding path became the one plan (#311). Boards arranged * before then still hold such edges, and they keep their meaning. - * - `HIRE` — theirs, and the only kind their own controls write. */ export type DependencySource = "TEAM" | "BUDDY" | "HIRE"; diff --git a/src/features/onboarding/journey.ts b/src/features/onboarding/journey.ts index 371730f82..374201f48 100644 --- a/src/features/onboarding/journey.ts +++ b/src/features/onboarding/journey.ts @@ -91,12 +91,12 @@ export function phaseItems(phase: OnboardingPhaseEndpoint): PhaseItem[] { return [...steps, ...questions]; } -/** A phase's items in the order its graph reads, top to bottom -- the list view's order. */ /** The DOM id of a step or question row, which a link from the buddy scrolls to. */ export function linkedCardId(itemId: string): string { return `onboarding-item-${itemId}`; } +/** A phase's items in the order its graph reads, top to bottom -- the list view's order. */ export function orderedPhaseItems(phase: OnboardingPhaseEndpoint): PhaseItem[] { return orderByGraph(phaseItems(phase)); } diff --git a/src/pages/OnBoardingPage.tsx b/src/pages/OnBoardingPage.tsx index 83e2fd66e..ca9a85cc2 100644 --- a/src/pages/OnBoardingPage.tsx +++ b/src/pages/OnBoardingPage.tsx @@ -182,7 +182,8 @@ export function OnBoardingPage() { * conversation is a way of finding something, and starting it is the hire's own click. */ const [searchParams, setSearchParams] = useSearchParams(); - const linkedItemId = searchParams.get("step") ?? searchParams.get("question"); + const linkedStepId = searchParams.get("step"); + const linkedQuestionId = searchParams.get("question"); const linkedPhaseId = searchParams.get("phase"); const [path, setPath] = useState(null); @@ -365,12 +366,14 @@ export function OnBoardingPage() { ? { kind: "question" as const, id: focusQuestionId } : wantsChooser ? { kind: "choose" as const, id: "" } - : linkedItemId - ? { kind: "link-item" as const, id: linkedItemId } - : linkedPhaseId - ? { kind: "link-phase" as const, id: linkedPhaseId } - : null, - [focusQuestionId, linkedItemId, linkedPhaseId, routeStepId, wantsChooser], + : linkedStepId + ? { kind: "link-step" as const, id: linkedStepId } + : linkedQuestionId + ? { kind: "link-question" as const, id: linkedQuestionId } + : linkedPhaseId + ? { kind: "link-phase" as const, id: linkedPhaseId } + : null, + [focusQuestionId, linkedPhaseId, linkedQuestionId, linkedStepId, routeStepId, wantsChooser], ); // A link carries the navigation's key as well: the hire who scrolled away and clicks the same link // in the conversation again should land again. @@ -425,7 +428,7 @@ export function OnBoardingPage() { ); if (!owningPhase) { toast.error( - arrival.kind === "question" + arrival.kind === "question" || arrival.kind === "link-question" ? "That question is not on your path" : "That step is not on your path", { description: "It may have been replaced when your path was rebuilt." }, @@ -436,7 +439,7 @@ export function OnBoardingPage() { return; } - if (arrival.kind === "link-item") { + if (arrival.kind === "link-step" || arrival.kind === "link-question") { setSelectedPhaseId(owningPhase.id); setExpandedItemId(null); setLinkHighlight({ id: arrival.id, key: arrivalKey }); diff --git a/src/styles/index.css b/src/styles/index.css index bc0208632..b9540faa8 100644 --- a/src/styles/index.css +++ b/src/styles/index.css @@ -968,11 +968,20 @@ body { animation: app-link-highlight 1.6s ease-in-out 0.35s 1 both; } +/* No pulse, but still marked for as long as the pulse would take: finding the item is the point, + not the motion. A step change rather than a fade, and gone afterwards like the pulse is. */ +@keyframes app-link-highlight-still { + 0% { + box-shadow: 0 0 0 2px var(--color-app-brand); + } + 100% { + box-shadow: 0 0 0 0 transparent; + } +} + @media (prefers-reduced-motion: reduce) { - /* No pulse, but still marked: finding the item is the point, not the motion. */ .app-link-highlight { - animation: none; - box-shadow: 0 0 0 2px var(--color-app-brand); + animation: app-link-highlight-still 2s step-end 1 both; } } diff --git a/tests/unit/pages/OnBoardingPage.test.tsx b/tests/unit/pages/OnBoardingPage.test.tsx index 01d5b0866..4057b8a1f 100644 --- a/tests/unit/pages/OnBoardingPage.test.tsx +++ b/tests/unit/pages/OnBoardingPage.test.tsx @@ -28,6 +28,18 @@ vi.mock("../../../src/context/useAuth", () => ({ useAuth: () => ({ profile: { id: signedInUserId.value } }), })); +// One stable object, as the real hook's is: the page keeps `toast` in effect dependencies. +const toastMocks = vi.hoisted(() => ({ + show: vi.fn(), + info: vi.fn(), + success: vi.fn(), + warning: vi.fn(), + error: vi.fn(), + dismiss: vi.fn(), + dismissAll: vi.fn(), +})); +vi.mock("../../../src/context/useToast", () => ({ useToast: () => toastMocks })); + // The celebratory layer is decorative and lives behind its own provider; the // page only needs a no-op `celebrate` to render. vi.mock("../../../src/features/moments", () => ({ @@ -1073,6 +1085,25 @@ describe("OnBoardingPage: links from the buddy", () => { expect(container.querySelector("#onboarding-item-q-linked")).toHaveTextContent("#2"); }); + it("names a question, not a step, when a question link points at nothing", async () => { + server.use( + http.get("/api/v1/onboarding/me/path", () => HttpResponse.json(pathWithQuestion("OPEN"))), + ); + + render( + + + , + ); + + await waitFor(() => + expect(toastMocks.error).toHaveBeenCalledWith( + "That question is not on your path", + expect.anything(), + ), + ); + }); + it("lands on the phase a link names", async () => { server.use( http.get("/api/v1/onboarding/me/path", () => HttpResponse.json(pathWithQuestion("OPEN"))), From cbbe63b7c4259448fcf6f08b4b3e95d842709297 Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Sat, 26 Sep 2026 13:14:59 +0200 Subject: [PATCH 19/20] Address review on the buddy path links and step drafts - Scroll to a linked card once the list has slid in, and only once per link - Clear the link highlight when it has played or the phase changes; key rows by item so a link no longer remounts an open step, and let the pulse end with `backwards` - Keep a half-typed skip reason or feedback comment when the buddy changes the path, and skip the second step read when the path catches up - Keep the item open when a link points at it; let go of an open graph item on a link - Hold back starting a skip-pending step only for `/onboarding/` arrivals - Reject `/\host` links; offer the buddy for every empty phase, not just the first - PenLine icon for hire-added steps, drop an unneeded `questions` fallback, fix a stale comment Co-Authored-By: Claude Opus 5.5 --- src/features/board/layout/boardGroups.ts | 2 +- .../buddy/components/BuddyMarkdown.tsx | 5 +- src/features/onboarding/buddyDrafts.ts | 11 +++ .../onboarding/components/StepOriginBadge.tsx | 4 +- .../components/journey/PhaseItemList.tsx | 25 +++++-- .../components/journey/StepWorkspace.tsx | 32 +++++++- src/pages/OnBoardingPage.tsx | 75 +++++++++++++------ src/styles/index.css | 5 +- .../features/buddy/BuddyMarkdown.test.tsx | 35 +++++++++ .../features/onboarding/buddyDrafts.test.ts | 8 ++ .../components/journey/StepWorkspace.test.tsx | 25 +++++++ 11 files changed, 189 insertions(+), 38 deletions(-) create mode 100644 tests/unit/features/buddy/BuddyMarkdown.test.tsx diff --git a/src/features/board/layout/boardGroups.ts b/src/features/board/layout/boardGroups.ts index e08947480..17bf110ac 100644 --- a/src/features/board/layout/boardGroups.ts +++ b/src/features/board/layout/boardGroups.ts @@ -79,7 +79,7 @@ export function readBoardGroups(boardId: string): BoardGroup[] { } } -/** What the generator calls the area holding a team's card blueprints. */ +/** What the since-removed generator called the area holding a team's card blueprints. */ const TEAM_AREA = "From your team"; /** diff --git a/src/features/buddy/components/BuddyMarkdown.tsx b/src/features/buddy/components/BuddyMarkdown.tsx index 96d5ceb97..307832b6a 100644 --- a/src/features/buddy/components/BuddyMarkdown.tsx +++ b/src/features/buddy/components/BuddyMarkdown.tsx @@ -10,10 +10,11 @@ import remarkGfm from "remark-gfm"; * in a new tab would reload the whole SPA and lose the conversation the hire was having. * * Root-relative only, and deliberately: a protocol-relative `//evil.example` is also "relative" to a - * careless check, and the model's output is not a place to be careless. + * careless check, and the model's output is not a place to be careless. `/\evil.example` is the same + * thing in disguise -- browsers read the backslash as a slash. */ function isInAppPath(href: string | undefined): href is string { - return href !== undefined && href.startsWith("/") && !href.startsWith("//"); + return href !== undefined && href.startsWith("/") && href[1] !== "/" && href[1] !== "\\"; } /** diff --git a/src/features/onboarding/buddyDrafts.ts b/src/features/onboarding/buddyDrafts.ts index 0b3357189..464bddfcc 100644 --- a/src/features/onboarding/buddyDrafts.ts +++ b/src/features/onboarding/buddyDrafts.ts @@ -59,6 +59,17 @@ export function askAboutEmptyPhase(phaseTitle: string): string { return `The "${snippet(phaseTitle)}" phase of my onboarding came back empty — nothing was generated for it. Can we work out together what it should contain for me?`; } +/** + * The same opening for every phase that came back empty at once, so a hire with several is not + * handed a draft about only the first of them. + */ +export function askAboutEmptyPhases(phaseTitles: readonly string[]): string { + if (phaseTitles.length === 1) return askAboutEmptyPhase(phaseTitles[0]); + const names = phaseTitles.map((title) => `"${snippet(title)}"`); + const listed = `${names.slice(0, -1).join(", ")} and ${names[names.length - 1]}`; + return `The ${listed} phases of my onboarding came back empty — nothing was generated for them. Can we work out together what they should contain for me?`; +} + /** Opening a conversation about one step. */ export function askAboutStep(step: Pick): string { return `I'm on the onboarding step "${snippet(step.title)}". Can you help me get going on it?`; diff --git a/src/features/onboarding/components/StepOriginBadge.tsx b/src/features/onboarding/components/StepOriginBadge.tsx index 864dab4a8..248e33f8e 100644 --- a/src/features/onboarding/components/StepOriginBadge.tsx +++ b/src/features/onboarding/components/StepOriginBadge.tsx @@ -1,4 +1,4 @@ -import { MessageCircle, Sparkles, UserRound } from "lucide-react"; +import { MessageCircle, PenLine, UserRound } from "lucide-react"; import { Badge } from "../../../components/ui/Badge"; import type { OnboardingStepEndpoint } from "../types"; @@ -41,7 +41,7 @@ export function StepOriginBadge({ step, viewer = "hire" }: StepOriginBadgeProps) if (step.origin === "HIRE") { return ( - + {viewer === "hire" ? "You added this" : "Added by the hire"} ); diff --git a/src/features/onboarding/components/journey/PhaseItemList.tsx b/src/features/onboarding/components/journey/PhaseItemList.tsx index 02e368441..26fd8931f 100644 --- a/src/features/onboarding/components/journey/PhaseItemList.tsx +++ b/src/features/onboarding/components/journey/PhaseItemList.tsx @@ -20,11 +20,10 @@ type Props = { nextItemId: string | null; /** The item unfolded in place, if any. */ expandedItemId: string | null; - /** - * The item a link from the buddy landed on, lit up once. `key` is per arrival, so the same link - * followed twice plays the light twice. - */ - linkHighlight?: { id: string; key: string } | null; + /** The item a link from the buddy landed on, lit up until `onLinkHighlightEnd` says it played. */ + linkedItemId?: string | null; + /** The light on `linkedItemId` has played; the page takes it away so it does not play again. */ + onLinkHighlightEnd?: () => void; onToggle: (item: PhaseItem) => void; /** Start, continue or answer: opens the item in place, starting a step that was not started. */ onPrimary: (item: PhaseItem) => void; @@ -44,7 +43,8 @@ export function PhaseItemList({ phase, nextItemId, expandedItemId, - linkHighlight = null, + linkedItemId = null, + onLinkHighlightEnd, onToggle, onPrimary, renderExpanded, @@ -74,11 +74,13 @@ export function PhaseItemList({ const minutes = item.kind === "step" ? item.step.estimatedMinutes : null; const muted = state === "done" || state === "skipped" || state === "locked"; const canUnfold = state !== "locked"; - const isLinked = linkHighlight?.id === item.id; + const isLinked = linkedItemId === item.id; return (
  • { + if (event.target === event.currentTarget) onLinkHighlightEnd?.(); + } + : undefined + } >