diff --git a/src/App.tsx b/src/App.tsx index d6b73a6d4..fd836cba0 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -22,6 +22,7 @@ import { EggEffectsLayer } from "./features/easter-eggs/components/EggEffectsLay 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"; import { GlobalShortcuts } from "./features/shortcuts"; function AppContent() { @@ -29,6 +30,7 @@ function AppContent() { 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/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 971efc90e..b2882b085 100644 --- a/src/features/board/components/BoardGrid.tsx +++ b/src/features/board/components/BoardGrid.tsx @@ -28,7 +28,6 @@ import { NoteCard } from "./NoteCard"; import { ArrivalStepsCard } from "./ArrivalStepsCard"; import { OpenPullRequestsCard } from "./OpenPullRequestsCard"; import { PathStepCard } from "./PathStepCard"; -import { PathToFirstContributionCard } from "./PathToFirstContributionCard"; import { SuggestedTasksCard } from "./SuggestedTasksCard"; import { TaskPoolCard } from "./TaskPoolCard"; import { BoardCardContext } from "./boardCardControls"; @@ -494,8 +493,6 @@ function BoardCardView({ // somebody 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/BoardPathNotes.tsx b/src/features/board/components/BoardPathNotes.tsx deleted file mode 100644 index d84bb316b..000000000 --- a/src/features/board/components/BoardPathNotes.tsx +++ /dev/null @@ -1,49 +0,0 @@ -import { AlertTriangle, CheckCircle2 } from "lucide-react"; -import { CONTRIBUTION_WORDING } from "../../../config/contributionWording"; -import { formatMoment } from "../../onboarding-metrics/format"; -import type { PathToFirstContributionContent } from "../types"; - -/** - * The two things the path card says that nothing else on the board does. - * - * The card's strip of moments went when {@link BoardPathWindow} took over the "where am I" job — - * the board answers how far along somebody is in four other places, and five labelled dots above - * it were a fifth. These two lines are not that: they are hire-facing facts with no second home. - * - * 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. Autonomy is the day onboarding - * ended, which is worth saying plainly once rather than leaving as a state the board drops out of. - * - * Both are single lines rather than panels — the header of a board somebody is working down stays - * quiet even when it has something to say. - */ -export function BoardPathNotes({ content }: { content: PathToFirstContributionContent }) { - const { autonomyReachedAt, stalledReason } = content; - - if (!stalledReason && !autonomyReachedAt) return null; - - return ( -
- {stalledReason && ( -

-

- )} - - {autonomyReachedAt && ( -

-

- )} -
- ); -} 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/board/components/CardEditHistory.tsx b/src/features/board/components/CardEditHistory.tsx index b48c74c54..75fcdaaa2 100644 --- a/src/features/board/components/CardEditHistory.tsx +++ b/src/features/board/components/CardEditHistory.tsx @@ -1,7 +1,7 @@ import { useState } from "react"; import { Check, ListChecks, Pencil, Undo2 } from "lucide-react"; import { Button } from "../../../components/ui/Button"; -import { readableTitle } from "../generation/pathToCards"; +import { readableTitle } from "../layout/cardNames"; import { useCardMarks } from "../marks/useCardMarks"; import { Marked } from "./Marked"; import type { diff --git a/src/features/board/components/ChecklistCard.tsx b/src/features/board/components/ChecklistCard.tsx index a535a7015..06b996d92 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 { CardOriginLink } from "./CardOriginLink"; @@ -152,8 +152,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 db71e99d4..969490601 100644 --- a/src/features/board/components/CurrentTaskCard.tsx +++ b/src/features/board/components/CurrentTaskCard.tsx @@ -54,15 +54,7 @@ export function CurrentTaskCard({ icon={Target} title="What you're working on" card={card} - subtitle={ - hasTask - ? closed - ? "Closed where it lives" - : content.chosen - ? "You picked this one" - : "Handed to you as a first task" - : undefined - } + subtitle={hasTask ? (closed ? "Closed where it lives" : "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/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/board/layout/boardStructure.ts b/src/features/board/layout/boardStructure.ts index d3f7dfe4f..40de638be 100644 --- a/src/features/board/layout/boardStructure.ts +++ b/src/features/board/layout/boardStructure.ts @@ -91,6 +91,10 @@ export function stageOrder(stage: BoardStage): number { * - `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. */ 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; case "PATH_STEP": // A degraded card (`reason` set) or a live step that simply has no tasks would otherwise be @@ -283,10 +286,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 }; - } case "PATH_STEP": { const total = content.tasks.length; return { done: content.tasks.filter((task) => task.finished).length, total }; diff --git a/src/features/board/layout/cardAccents.ts b/src/features/board/layout/cardAccents.ts index a4c4bf573..fbc903f34 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 0e9eb9219..b7e81caef 100644 --- a/src/features/board/layout/cardIcons.ts +++ b/src/features/board/layout/cardIcons.ts @@ -9,7 +9,6 @@ import { Network, PenLine, PlaneLanding, - Route, Sparkles, Target, type LucideIcon, @@ -30,7 +29,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 74e724a13..7b7782195 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 { PATH_STEP_FALLBACK_TITLE, 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 6e3e33312..00cd50721 100644 --- a/src/features/board/types.ts +++ b/src/features/board/types.ts @@ -11,7 +11,6 @@ import type { TaskType } from "../starter-work/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" @@ -84,32 +83,6 @@ export type BoardUndoNotice = { forReplacedAt: string | null; }; -/** 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; @@ -164,8 +137,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; /** * True once the issue behind this task is closed where it lives. False when there is no task at * all, which says nothing about anything. @@ -394,7 +365,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/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/components/BuddyActionProposals.tsx b/src/features/buddy/components/BuddyActionProposals.tsx index 9a7c6c41e..9c28b4ecc 100644 --- a/src/features/buddy/components/BuddyActionProposals.tsx +++ b/src/features/buddy/components/BuddyActionProposals.tsx @@ -2,6 +2,7 @@ import { Check, Info, Loader2, RotateCcw, TriangleAlert, Users, X } from "lucide import type { ProposedAction, ProposalRisk } from "../types"; import { BUDDY_ACTION_AMEND_CHECKLIST, + BUDDY_ACTION_FLAG_TO_PM, BUDDY_ACTION_EDIT_CHECKLIST, BUDDY_ACTION_OPEN_ORIENTATION, BUDDY_ACTION_REWORD_CHECKLIST, @@ -262,6 +263,20 @@ export function BuddyActionProposals({ )} + {/* 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. */} + {!isStored && action.reason && ( +

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

+ )} + {!isStored && 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/components/BuddyComposer.tsx b/src/features/buddy/components/BuddyComposer.tsx index 69e386b69..322705a73 100644 --- a/src/features/buddy/components/BuddyComposer.tsx +++ b/src/features/buddy/components/BuddyComposer.tsx @@ -84,6 +84,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]); + // The mirror of the blur in `submit`: once the buddy has stopped writing, the caret comes // back so a follow-up question does not need a click first. Skipped when somebody else holds // focus, so this never pulls the caret out of a field the hire moved to in the meantime. @@ -147,7 +169,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/buddy/components/BuddyMarkdown.tsx b/src/features/buddy/components/BuddyMarkdown.tsx index ca7256c01..95f2e4d1d 100644 --- a/src/features/buddy/components/BuddyMarkdown.tsx +++ b/src/features/buddy/components/BuddyMarkdown.tsx @@ -1,7 +1,23 @@ import { memo } from "react"; 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. `/\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[1] !== "/" && href[1] !== "\\"; +} + /** * 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 @@ -34,11 +50,17 @@ function BuddyMarkdownImpl({ 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 a38440876..2a32669f5 100644 --- a/src/features/buddy/hooks/useBuddyConversation.ts +++ b/src/features/buddy/hooks/useBuddyConversation.ts @@ -9,6 +9,8 @@ import { streamMessage, type BuddyOpeningAction, } from "../../../services/buddyService"; +import { announceBuddyPathChanged } from "../aiBuddyBus"; +import { BUDDY_PATH_ACTIONS } from "../types"; import { useAuth } from "../../../context/useAuth"; import { useInvalidateBoard } from "../../board/hooks/useInvalidateBoard"; import { @@ -677,6 +679,16 @@ export function useBuddyConversation( noteText: proposal.noteText, lineBefore: proposal.lineBefore, lineAfter: proposal.lineAfter, + stepId: proposal.stepId, + questionId: proposal.questionId, + phaseId: proposal.phaseId, + onboardingTaskId: proposal.onboardingTaskId, + answer: proposal.answer, + optionIds: proposal.optionIds, + description: proposal.description, + reason: proposal.reason, + waitsOnIds: proposal.waitsOnIds, + unlocksIds: proposal.unlocksIds, linkUrl: proposal.linkUrl, linkLabel: proposal.linkLabel, cardIds: proposal.cardIds, @@ -825,6 +837,16 @@ export function useBuddyConversation( noteText: action.noteText, lineBefore: action.lineBefore, lineAfter: action.lineAfter, + stepId: action.stepId, + questionId: action.questionId, + phaseId: action.phaseId, + onboardingTaskId: action.onboardingTaskId, + answer: action.answer, + optionIds: action.optionIds, + description: action.description, + reason: action.reason, + waitsOnIds: action.waitsOnIds, + unlocksIds: action.unlocksIds, linkUrl: action.linkUrl, linkLabel: action.linkLabel, cardIds: action.cardIds, @@ -834,6 +856,11 @@ export function useBuddyConversation( 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 && "action" in action && BUDDY_PATH_ACTIONS.includes(action.action)) { + announceBuddyPathChanged(); + } // `board.all()`, not a project key: the backend re-resolves the project server-side // (the caller's single onboarding project) and never tells the client which board — 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/buddy/types.ts b/src/features/buddy/types.ts index 00a5f2bf6..4c9c8978a 100644 --- a/src/features/buddy/types.ts +++ b/src/features/buddy/types.ts @@ -29,6 +29,28 @@ export type ActionPatch = { */ 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. + * + * 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", + "complete_task", + "answer_question", + "add_path_step", + "request_skip", +]; + /** * The backend's `place_checklist` action: the mentor offering to keep a list it just wrote. * @@ -90,11 +112,14 @@ export const BUDDY_ACTION_REORDER_CARDS = "reorder_cards"; export type HireActionProposal = { /** 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. */ + /** + * 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 @@ -128,6 +153,37 @@ export type HireActionProposal = { */ 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; + onboardingTaskId?: string; + answer?: string; + /** + * The options a multiple-choice `answer` stands for, as the backend resolved them when it + * proposed. Echoed back so the confirm can check the button still means what it says — the + * backend refuses it if the question changed in between. + */ + optionIds?: 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; + /** + * 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[]; /** * The `place_checklist` confirm payload: the list the buddy wrote and offered to keep. * @@ -293,7 +349,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: { @@ -306,6 +362,16 @@ export type BuddyStreamHandlers = { githubLogin?: string; competencyKey?: string; level?: string; + stepId?: string; + questionId?: string; + phaseId?: string; + onboardingTaskId?: string; + answer?: string; + optionIds?: string[]; + description?: string; + reason?: string; + waitsOnIds?: string[]; + unlocksIds?: string[]; checklistTitle?: string; checklistItems?: string[]; cardId?: string; 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/features/chatbot/components/ChatComposer.tsx b/src/features/chatbot/components/ChatComposer.tsx index b98fa6bd5..a8aef771d 100644 --- a/src/features/chatbot/components/ChatComposer.tsx +++ b/src/features/chatbot/components/ChatComposer.tsx @@ -165,6 +165,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]); + const filterButtonRef = useRef(null); const popoverRef = useRef(null); @@ -582,6 +603,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/features/onboarding-metrics/components/HireTimelineCard.tsx b/src/features/onboarding-metrics/components/HireTimelineCard.tsx index 25a94e41d..8e77277a5 100644 --- a/src/features/onboarding-metrics/components/HireTimelineCard.tsx +++ b/src/features/onboarding-metrics/components/HireTimelineCard.tsx @@ -14,7 +14,7 @@ type HireTimelineCardProps = { }; /** - * 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 cfee51ffa..9fcd9981e 100644 --- a/src/features/onboarding-metrics/components/OnboardingMetricsPage.tsx +++ b/src/features/onboarding-metrics/components/OnboardingMetricsPage.tsx @@ -121,8 +121,12 @@ function OnboardingMetricsSkeleton() { } /** - * 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 @@ -180,12 +184,12 @@ export function OnboardingMetricsPage() { pendingRefreshRef.current = false; setRefreshing(false); if (error || refetchError) { - 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.", }); @@ -268,10 +272,10 @@ export function OnboardingMetricsPage() { ); return ( -
+
@@ -283,11 +287,11 @@ export function OnboardingMetricsPage() { ) : loading ? null : error ? ( } title="Not available right now"> - The onboarding metrics couldn't be loaded. Try again in a moment. + The contribution metrics couldn't be loaded. Try again in a moment. ) : !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/buddyDrafts.ts b/src/features/onboarding/buddyDrafts.ts new file mode 100644 index 000000000..464bddfcc --- /dev/null +++ b/src/features/onboarding/buddyDrafts.ts @@ -0,0 +1,96 @@ +// ============================================================ +// 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?`; +} + +/** + * 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?`; +} + +/** + * 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/StepOriginBadge.tsx b/src/features/onboarding/components/StepOriginBadge.tsx index 59baf6ed4..248e33f8e 100644 --- a/src/features/onboarding/components/StepOriginBadge.tsx +++ b/src/features/onboarding/components/StepOriginBadge.tsx @@ -1,18 +1,61 @@ -import { UserRound } from "lucide-react"; +import { MessageCircle, PenLine, UserRound } from "lucide-react"; import { Badge } from "../../../components/ui/Badge"; 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"; }; -export function StepOriginBadge({ step }: StepOriginBadgeProps) { - if (step.isAiAssisted !== false) return null; +/** + * 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 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, viewer = "hire" }: StepOriginBadgeProps) { + if (step.origin === "BUDDY") { + return ( + + + {viewer === "hire" ? "Added with your buddy" : "Added with the buddy"} + + ); + } - return ( - - - Custom step by PM - - ); + if (step.origin === "HIRE") { + return ( + + + {viewer === "hire" ? "You added this" : "Added by the hire"} + + ); + } + + // PM, or a hand-written blueprint copy or pre-`origin` row -- see above. + if (step.origin === "PM" || step.isAiAssisted === false) { + return ( + + + Custom step by PM + + ); + } + + return null; } 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 81309e0cf..26fd8931f 100644 --- a/src/features/onboarding/components/journey/PhaseItemList.tsx +++ b/src/features/onboarding/components/journey/PhaseItemList.tsx @@ -5,11 +5,13 @@ import { Button } from "../../../../components/ui/Button"; import { formatMinutes, itemState, + linkedCardId, orderedPhaseItems, waitingOn, 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"; @@ -18,6 +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 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; @@ -37,11 +43,16 @@ export function PhaseItemList({ phase, nextItemId, expandedItemId, + linkedItemId = null, + onLinkHighlightEnd, onToggle, onPrimary, 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 ( @@ -63,10 +74,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 = linkedItemId === item.id; return (
  • { + if (event.target === event.currentTarget) onLinkHighlightEnd?.(); + } + : undefined + } >
    - ) : 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. + + )} 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); @@ -106,7 +114,27 @@ export function StepWorkspace({ onSkipAnswerSeenRef.current = onSkipAnswerSeen; }, [onSkipAnswerSeen]); + // What the last read brought, so a re-read can tell the member's own typing from the server's + // text, and the page's path refresh after a buddy change need not read the step a second time. + const loadedRef = useRef<{ + stepId: string; + buddyChanges: number; + status: string; + skipReason: string; + comment: string; + } | null>(null); + useEffect(() => { + const last = loadedRef.current; + // The buddy's change is announced here and to the page at once; the page's refresh then moves + // `stepStatus` to what the read for `buddyChanges` already brought. + if ( + last?.stepId === stepId && + last.buddyChanges === buddyChanges && + last.status === stepStatus + ) { + return; + } let cancelled = false; Promise.all([ onboardingService.fetchStep(stepId), @@ -127,12 +155,20 @@ export function StepWorkspace({ } setTasks([...fetchedTasks].sort((left, right) => left.position - right.position)); setResources(fetchedResources); - setSkipReason(detail.skip?.reason ?? ""); + // A re-read of the same step keeps what the member is typing: it runs whenever the buddy + // changes anything on the path, and used to wipe a half-written skip reason or comment. + const sameStep = last?.stepId === stepId; + const skipReason = detail.skip?.reason ?? ""; + const comment = detail.feedback?.comment ?? ""; + setSkipReason((current) => + sameStep && current !== last.skipReason ? current : skipReason, + ); if (detail.feedback) { setHelpful(detail.feedback.helpful); - setComment(detail.feedback.comment ?? ""); + setComment((current) => (sameStep && current !== last.comment ? current : comment)); setFeedbackSent(true); } + loadedRef.current = { stepId, buddyChanges, status: detail.status, skipReason, comment }; }) .catch((reason: unknown) => { if (!cancelled) setError(reason instanceof Error ? reason.message : "Unknown error"); @@ -140,7 +176,7 @@ export function StepWorkspace({ return () => { cancelled = true; }; - }, [stepId, stepStatus]); + }, [stepId, stepStatus, buddyChanges]); if (error) { return ( @@ -288,6 +324,14 @@ export function StepWorkspace({ {step.description}

    ) : 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/features/onboarding/graph/JourneyNodeCards.tsx b/src/features/onboarding/graph/JourneyNodeCards.tsx index dcb9b2629..47cea8142 100644 --- a/src/features/onboarding/graph/JourneyNodeCards.tsx +++ b/src/features/onboarding/graph/JourneyNodeCards.tsx @@ -281,12 +281,18 @@ export function ItemNodeCard({ render, isNext = false, showUpdates = false, + number, }: { item: PhaseItem; state: ItemState; render: JourneyNodeRenderState; isNext?: boolean; showUpdates?: boolean; + /** + * The number the item carries on the list and in the buddy's replies (see `itemNumbers`), so a + * node here can be talked about by the number the hire can see on it. + */ + number?: number; }) { const minutes = item.kind === "step" ? item.step.estimatedMinutes : null; const isQuestion = item.kind === "question"; @@ -316,6 +322,9 @@ export function ItemNodeCard({ : "text-app-text" }`} > + {number !== undefined ? ( + #{number} + ) : null} {item.title}

    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/features/onboarding/journey.ts b/src/features/onboarding/journey.ts index 1e8303f65..374201f48 100644 --- a/src/features/onboarding/journey.ts +++ b/src/features/onboarding/journey.ts @@ -91,6 +91,11 @@ export function phaseItems(phase: OnboardingPhaseEndpoint): PhaseItem[] { return [...steps, ...questions]; } +/** 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/features/onboarding/types.ts b/src/features/onboarding/types.ts index 78a091917..b36433944 100644 --- a/src/features/onboarding/types.ts +++ b/src/features/onboarding/types.ts @@ -47,11 +47,21 @@ export interface OnboardingStepSkip { answerSeenAt?: 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/features/pm-area/analysis/useProjectAnalysis.ts b/src/features/pm-area/analysis/useProjectAnalysis.ts index 2f43f6265..71261e660 100644 --- a/src/features/pm-area/analysis/useProjectAnalysis.ts +++ b/src/features/pm-area/analysis/useProjectAnalysis.ts @@ -131,7 +131,7 @@ export type AnalysisPhase = "idle" | "running" | "done"; const TASKS: readonly Pick[] = [ { id: "team", label: "Team & open items" }, - { id: "onboarding", label: "Onboarding metrics" }, + { id: "onboarding", label: "Contribution metrics" }, { id: "escalations", label: "Escalation inbox" }, { id: "questions", label: "Recurring questions" }, { id: "gaps", label: "Knowledge gaps" }, 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/features/team-management/components/detail/MemberJourneySection.tsx b/src/features/team-management/components/detail/MemberJourneySection.tsx index e859e688d..4fae75e70 100644 --- a/src/features/team-management/components/detail/MemberJourneySection.tsx +++ b/src/features/team-management/components/detail/MemberJourneySection.tsx @@ -653,7 +653,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/src/features/team-management/components/detail/StepDetailsPanel.tsx b/src/features/team-management/components/detail/StepDetailsPanel.tsx index 84a399d2b..af7a54d9b 100644 --- a/src/features/team-management/components/detail/StepDetailsPanel.tsx +++ b/src/features/team-management/components/detail/StepDetailsPanel.tsx @@ -125,7 +125,7 @@ export function StepDetailsPanel({ > {step.status.replace("_", " ")} - +
    } panelBackgroundClassName="bg-app-surface" diff --git a/src/pages/BoardPage.tsx b/src/pages/BoardPage.tsx index 31a9aaad8..aa1fda2c1 100644 --- a/src/pages/BoardPage.tsx +++ b/src/pages/BoardPage.tsx @@ -1,4 +1,5 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { Link } from "react-router-dom"; import { AlertCircle, Check, @@ -10,7 +11,6 @@ import { Milestone, Minimize2, RefreshCw, - Sparkles, } from "lucide-react"; import { PageHeader } from "../components/layout/PageHeader"; import { Button } from "../components/ui/Button"; @@ -21,11 +21,9 @@ import { useSwipeableTabs } from "../hooks/useHorizontalWheelNavigation"; import { useBoard } from "../features/board/hooks/useBoard"; import { useBoardStructure } from "../features/board/hooks/useBoardStructure"; import { BoardChainPanel } from "../features/board/components/BoardChainPanel"; -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 { BoardPathNotes } from "../features/board/components/BoardPathNotes"; import { BoardPathWindow } from "../features/board/components/BoardPathWindow"; import { BoardSectionTabs } from "../features/board/components/BoardSectionNav"; import { BoardFilterTriggers } from "../features/board/components/BoardFilterTriggers"; @@ -36,7 +34,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"; @@ -90,20 +87,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: * @@ -114,9 +110,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. */ /** @@ -142,21 +136,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(); @@ -179,8 +168,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); @@ -493,14 +480,6 @@ export function BoardPage() { }); } - // The path is drawn above the board -- the window for where they stand, `BoardPathNotes` for - // the two things the card says that nothing else does -- and 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. * @@ -517,22 +496,13 @@ export function BoardPage() { () => board?.cards.filter( (card) => - card !== pathCard && - !pendingRemovals.has(card.id) && - (isTaskPoolShown || card.content.kind !== "TASK_POOL"), + !pendingRemovals.has(card.id) && (isTaskPoolShown || card.content.kind !== "TASK_POOL"), ) ?? [], - [board, pathCard, pendingRemovals, isTaskPoolShown], + [board, pendingRemovals, isTaskPoolShown], ); - const { - structure, - states, - assignStage, - assignGroupStage, - toggleDone, - setPredecessor, - applyPlan, - } = useBoardStructure(boardId, allCards); + const { structure, states, assignStage, assignGroupStage, toggleDone, setPredecessor } = + useBoardStructure(boardId, allCards); /** * The card whose run is being looked at, or null. @@ -590,13 +560,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. * @@ -871,12 +834,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. @@ -897,82 +855,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. * @@ -995,7 +877,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 ? ( @@ -1008,16 +890,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. */} @@ -1055,9 +927,6 @@ export function BoardPage() { page; this is about the work, and it belongs where the work is. */} {isPathShown && } - {pathCard?.content.kind === "PATH_TO_FIRST_CONTRIBUTION" && ( - - )} {/* The page keeps a 10rem margin either side from `lg` up, and on this page it is dead space: the board is a column of cards and the margin is where a hand rests. So the offers live there — always in reach, never in the way, and out of the row above the @@ -1297,26 +1166,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/src/pages/OnBoardingPage.tsx b/src/pages/OnBoardingPage.tsx index f3eda7d8d..b677a37f8 100644 --- a/src/pages/OnBoardingPage.tsx +++ b/src/pages/OnBoardingPage.tsx @@ -22,7 +22,7 @@ import { } from "lucide-react"; import { useCallback, useEffect, useMemo, useRef, useState, type ReactNode } from "react"; import { useQueryClient } from "@tanstack/react-query"; -import { Link, useLocation, useNavigate, useParams } from "react-router-dom"; +import { Link, useLocation, useNavigate, useParams, useSearchParams } from "react-router-dom"; import { PageHeader } from "../components/layout/PageHeader"; import { PageShell } from "../components/layout/PageShell"; import { Badge } from "../components/ui/Badge"; @@ -50,12 +50,14 @@ import { ProgressRing } from "../features/onboarding/graph/JourneyNodeCards"; import { usePathRevealMoment } from "../features/onboarding/hooks/usePathRevealMoment"; import { blockingPhases, + isSkipPending, itemState, pathProgress, phaseItems, phaseProgress, phaseState, phasesUnlockedBy, + linkedCardId, sortedPhases, waitingOn, type PhaseItem, @@ -69,6 +71,13 @@ 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 { onBuddyPathChanged } from "../features/buddy/aiBuddyBus"; +import { + askAboutEmptyPhase, + askAboutEmptyPhases, + askAboutPhase, +} from "../features/onboarding/buddyDrafts"; import { ApiError } from "../services/apiClient"; import { onboardingGraphService } from "../services/onboardingGraphService"; import { onboardingService } from "../services/onboardingService"; @@ -89,6 +98,9 @@ import { type LoadingState = "loading" | "empty" | "success" | "error"; type ViewMode = "list" | "graph"; +/** How a step came to be opened: by its address, or by the member on the page. */ +type StartOptions = { byAddress?: boolean }; + const VIEW_ORDER: readonly ViewMode[] = ["list", "graph"]; type NavigationState = { @@ -165,6 +177,21 @@ export function OnBoardingPage() { const focusItemId = routeStepId ?? navigationState?.focusQuestionId; // Set by the board's "where you are" strip when a phase on it is pressed. const openPhaseId = navigationState?.openPhaseId; + /** + * Where a link from the buddy points: `?step=`, `?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 linkedStepId = searchParams.get("step"); + const linkedQuestionId = searchParams.get("question"); + const linkedPhaseId = searchParams.get("phase"); const [path, setPath] = useState(null); const [loadingState, setLoadingState] = useState("loading"); @@ -262,6 +289,11 @@ export function OnBoardingPage() { } // 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, for as long as its light plays. `key` changes per arrival, so + // following the same link again scrolls to it again. Cleared when the light has played or the + // member moves to another phase, so neither the scroll nor the light comes back on its own later. + const [linkHighlight, setLinkHighlight] = useState<{ id: string; key: string } | null>(null); + const clearLinkHighlight = useCallback(() => setLinkHighlight(null), []); usePathRevealMoment(loadingState === "success" ? path : null); @@ -318,6 +350,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. @@ -372,15 +414,30 @@ export function OnBoardingPage() { ? { kind: "question" as const, id: focusQuestionId } : wantsChooser ? { kind: "choose" as const, id: "" } - : null, - [focusQuestionId, 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], ); - 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 startStepRef = useRef<(item: PhaseItem, options: StartOptions) => 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; @@ -401,14 +458,35 @@ export function OnBoardingPage() { return; } + // A link is followed in the list, so a step left open on the graph is let go of too -- + // otherwise swiping stayed off and the graph reopened it on the way back. + if (arrival.kind.startsWith("link")) setGraphItemId(null); + + if (arrival.kind === "link-phase") { + const linkedPhase = path.phases.find((phase) => phase.id === arrival.id); + if (linkedPhase) { + setSelectedPhaseId(arrival.id); + // An item open in the linked phase stays open: it may hold a typed answer. + setExpandedItemId((current) => + phaseItems(linkedPhase).some((item) => item.id === current) ? current : 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" || 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." }, ); // Only the step has an address of its own to go back from; a question arrives in router @@ -417,6 +495,16 @@ export function OnBoardingPage() { return; } + if (arrival.kind === "link-step" || arrival.kind === "link-question") { + setSelectedPhaseId(owningPhase.id); + // "You're on #3" is most likely clicked while #3 is open -- collapsing it would throw away + // a typed answer or skip reason. + setExpandedItemId((current) => (current === arrival.id ? current : null)); + setLinkHighlight({ id: arrival.id, key: arrivalKey }); + clearLinkRef.current(); + return; + } + setSelectedPhaseId(owningPhase.id); scrollToItemRef.current = arrival.id; setExpandedItemId(arrival.id); @@ -424,10 +512,42 @@ export function OnBoardingPage() { // A step opened by its address is started like one opened by a click -- including the // second link of a visit, which the load-time version could not see. const item = phaseItems(owningPhase).find((candidate) => candidate.id === arrival.id); - if (item) startStepRef.current(item); + if (item) startStepRef.current(item, { byAddress: true }); }); }, [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 per link. A link followed from the graph switches to + // the list, which `SlidingTabPanel` mounts only after the graph has slid out -- so a card that is + // not there yet is looked for again once it can be, the same wait `scrollToChooser` makes. + const scrolledLinkRef = useRef(""); + useEffect(() => { + if (loadingState !== "success" || !linkHighlight) return; + if (scrolledLinkRef.current === linkHighlight.key) return; + const scroll = () => { + const card = document.getElementById(linkedCardId(linkHighlight.id)); + if (!card) return false; + scrolledLinkRef.current = linkHighlight.key; + card.scrollIntoView?.({ behavior: "smooth", block: "center" }); + return true; + }; + if (scroll()) return; + const timer = window.setTimeout(scroll, SLIDING_PANEL_EXIT_MS); + return () => window.clearTimeout(timer); + }, [linkHighlight, loadingState, selectedPhaseId, viewMode]); + // Scrolls to an item the page opened on the member's behalf -- a link, "up next", "continue". useEffect(() => { const target = scrollToItemRef.current; @@ -485,8 +605,12 @@ export function OnBoardingPage() { ); /** A step the member opens for the first time is started; reopening one changes nothing. */ - const beginStepIfWaiting = async (item: PhaseItem) => { + const beginStepIfWaiting = async (item: PhaseItem, { byAddress = false }: StartOptions = {}) => { if (item.kind !== "step" || item.step.status !== "WAITING" || item.step.locked) return; + // Arriving at `/onboarding/` for a step with a skip request open is arriving to change or + // withdraw it, not to begin the step; its own "Start step" button still does that. Only the + // address: the list's button says "Start", and a click on it should do what it says. + if (byAddress && isSkipPending(item.step.skip)) return; try { await onboardingService.startStep(item.step.id); // The rocket marks a step *beginning*. @@ -503,7 +627,7 @@ export function OnBoardingPage() { // Refreshed every render, so the arrival effect can call the latest of each without taking a // dependency on a function that is recreated on every render. useEffect(() => { - startStepRef.current = (item) => void beginStepIfWaiting(item); + startStepRef.current = (item, options) => void beginStepIfWaiting(item, options); scrollToChooserRef.current = scrollToChooser; }); @@ -560,6 +684,7 @@ export function OnBoardingPage() { const selectPhase = (phaseId: string) => { setSelectedPhaseId(phaseId); setExpandedItemId(null); + setLinkHighlight(null); }; const choosePhase = (phaseId: string) => { @@ -666,6 +791,7 @@ export function OnBoardingPage() { handleAnswered(item.question, result)} continueLabel={next.label} onContinue={next.run} @@ -827,6 +953,16 @@ export function OnBoardingPage() { Your project manager can rebuild your path from your page in the team area.

    )} + {/* 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 (or the pointer to the PM) rather than + instead of it. */} + {generationIssues.length > 0 ? ( + issue.title))} + label="Work it out with your buddy instead" + /> + ) : null} ); } @@ -940,6 +1076,8 @@ export function OnBoardingPage() { phase={selectedPhase} nextItemId={nextItemId} expandedItemId={expandedItemId} + linkedItemId={linkHighlight?.id ?? null} + onLinkHighlightEnd={clearLinkHighlight} onToggle={toggleItem} onPrimary={openItem} renderExpanded={(item) => renderItemBody(item, "inline")} @@ -1066,6 +1204,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 (
    @@ -1079,6 +1218,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. */} +
    { return await apiClient.fetch(`${BASE_URL}/${id}/task-zero`, { diff --git a/src/services/teamManagementService.ts b/src/services/teamManagementService.ts index 433d8b585..02f81ad96 100644 --- a/src/services/teamManagementService.ts +++ b/src/services/teamManagementService.ts @@ -429,6 +429,20 @@ export async function markOnboardingFeedbackRead(feedbackId: string): Promise { @@ -446,22 +460,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/src/styles/index.css b/src/styles/index.css index 6a5cd7989..4d3f4387c 100644 --- a/src/styles/index.css +++ b/src/styles/index.css @@ -1000,6 +1000,50 @@ body { } } +/* 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); + } +} + +/* `backwards`, not `both`: a last frame that outlived the animation beat the card's own shadow. */ +.app-link-highlight { + animation: app-link-highlight 1.6s ease-in-out 0.35s 1 backwards; +} + +/* 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) { + .app-link-highlight { + animation: app-link-highlight-still 2s step-end 1 backwards; + } +} + /* Onboarding graphs: an edge whose blocker is done and whose target is open drifts towards the target, so "this is what you can do now" reads at a glance without another colour. */ @keyframes journey-edge-flow { diff --git a/tests/unit/a11y/BoardPage.a11y.test.tsx b/tests/unit/a11y/BoardPage.a11y.test.tsx index 244fb8651..c47c3fb94 100644 --- a/tests/unit/a11y/BoardPage.a11y.test.tsx +++ b/tests/unit/a11y/BoardPage.a11y.test.tsx @@ -32,21 +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: "2026-08-04T09:00:00Z", - // Both lines of `BoardPathNotes` are on screen for this pass, so axe sees them. - stalledReason: "a review has been waiting three days", - }, - }, { id: "c2", kind: "OPEN_PULL_REQUESTS", @@ -134,11 +119,8 @@ describe("BoardPage Accessibility", () => { ); // Waits for real board content. `main` is there from first paint, so waiting for it would - // let this pass with the grid entirely broken. The header strip of moments it used to wait - // for is gone: that answered "how far through onboarding am I", which the board answers four - // other ways -- only the two lines with no second home stayed, in `BoardPathNotes`. + // let this pass with the grid entirely broken. await waitFor(() => expect(screen.getByText(/Add a health endpoint/)).toBeInTheDocument()); - expect(screen.getByText(/a review has been waiting three days/)).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 4cebe4af1..21a487537 100644 --- a/tests/unit/features/board/BoardGrid.test.tsx +++ b/tests/unit/features/board/BoardGrid.test.tsx @@ -8,7 +8,6 @@ import type { BoardCard, CurrentTaskContent, OpenPullRequestsContent, - PathToFirstContributionContent, SuggestedTasksContent, } from "../../../../src/features/board/types"; @@ -20,23 +19,6 @@ vi.mock("../../../../src/features/projects/useProjectContext", () => ({ useProjectContext: () => ({ selectedProjectId: "p1" }), })); -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 => ({ @@ -52,7 +34,6 @@ const currentTaskContent = (over: Partial = {}): CurrentTask title: "Fix the flaky login test", summary: "It fails about one run in five.", url: null, - chosen: true, closedAtSource: false, ...over, }); @@ -81,41 +62,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(); @@ -193,9 +139,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); @@ -203,20 +149,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 f7c6ecffd..6b0daf07f 100644 --- a/tests/unit/features/board/BoardSubmenus.test.tsx +++ b/tests/unit/features/board/BoardSubmenus.test.tsx @@ -61,26 +61,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("a suggested task is grabbed behind its own confirm, with the buddy one press away", () => { render( { title: "Fix the flaky login test", summary: null, url: null, - chosen: true, closedAtSource: false, }, ])} diff --git a/tests/unit/features/board/CurrentTaskCard.test.tsx b/tests/unit/features/board/CurrentTaskCard.test.tsx index fc9492a41..c28d89bb9 100644 --- a/tests/unit/features/board/CurrentTaskCard.test.tsx +++ b/tests/unit/features/board/CurrentTaskCard.test.tsx @@ -33,7 +33,6 @@ function currentTaskContent(over: Partial = {}): CurrentTask title: "Fix the flaky login test", summary: "It fails about one run in five.", url: null, - chosen: true, closedAtSource: false, ...over, }; diff --git a/tests/unit/features/board/Marked.test.tsx b/tests/unit/features/board/Marked.test.tsx index 44237e68c..023abedc3 100644 --- a/tests/unit/features/board/Marked.test.tsx +++ b/tests/unit/features/board/Marked.test.tsx @@ -89,7 +89,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, closedAtSource: false, }} card={card} diff --git a/tests/unit/features/board/cardAccents.test.ts b/tests/unit/features/board/cardAccents.test.ts index 3e32eead2..b433a8790 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 f316f061a..696673b6d 100644 --- a/tests/unit/features/board/useBoard.test.tsx +++ b/tests/unit/features/board/useBoard.test.tsx @@ -29,17 +29,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 }, })), }); @@ -132,7 +126,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 f67ad6670..50793a03c 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 = {}): HireActionProposal { 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( , @@ -140,6 +144,64 @@ 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( + , + ); + + expect( + screen.getByText("Your reason: “I already have VPN access from my last team.”"), + ).toBeInTheDocument(); + }); + /** * The one action whose confirm writes the mentor's own sentences onto the hire's board. Which is * why the lines are on the offer: "Keep this" over words somebody else wrote is not something to diff --git a/tests/unit/features/buddy/BuddyComposer.test.tsx b/tests/unit/features/buddy/BuddyComposer.test.tsx new file mode 100644 index 000000000..ef862642e --- /dev/null +++ b/tests/unit/features/buddy/BuddyComposer.test.tsx @@ -0,0 +1,52 @@ +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"; +import { BuddyDraftContext } from "../../../../src/features/buddy/buddyDraftContext"; + +/** + * 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(""); + return true; + }, + }} + > + + + + ); + } + + 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/features/buddy/BuddyMarkdown.test.tsx b/tests/unit/features/buddy/BuddyMarkdown.test.tsx new file mode 100644 index 000000000..30f7d94ff --- /dev/null +++ b/tests/unit/features/buddy/BuddyMarkdown.test.tsx @@ -0,0 +1,35 @@ +import { render, screen } from "@testing-library/react"; +import { MemoryRouter } from "react-router-dom"; +import { describe, expect, it } from "vitest"; +import { BuddyMarkdown } from "../../../../src/features/buddy/components/BuddyMarkdown"; + +function renderLink(href: string) { + render( + + + , + ); + return screen.getByRole("link", { name: "the link" }); +} + +describe("BuddyMarkdown links", () => { + it("navigates in place for an app path", () => { + const link = renderLink("/onboarding?step=s1"); + + expect(link).toHaveAttribute("href", "/onboarding?step=s1"); + expect(link).not.toHaveAttribute("target"); + }); + + it.each(["//evil.example", "https://example.com"])( + "treats %s as somebody else's site", + (href) => { + expect(renderLink(href)).toHaveAttribute("target", "_blank"); + }, + ); + + it("never hands a backslash path on as one that could leave the app", () => { + // Browsers read `/\host` as `//host`. Markdown drops the backslash before it reaches the link, + // and `isInAppPath` would refuse it if it ever did not. + expect(renderLink("/\\evil.example").getAttribute("href")).not.toMatch(/^\/[\\/]/); + }); +}); 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/buddyDrafts.test.ts b/tests/unit/features/onboarding/buddyDrafts.test.ts new file mode 100644 index 000000000..e97a5b318 --- /dev/null +++ b/tests/unit/features/onboarding/buddyDrafts.test.ts @@ -0,0 +1,128 @@ +import { describe, expect, it } from "vitest"; +import { + askAboutEmptyPhase, + askAboutEmptyPhases, + 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, + }) satisfies OnboardingPhaseEndpoint; + + const step = (over: Partial = {}) => + ({ + id: "s1", + phaseId: "p1", + 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, + }) satisfies OnboardingStepEndpoint; + + const question = (over: Partial = {}) => + ({ + id: "q1", + phaseId: "p1", + position: 0, + type: "MULTIPLE_CHOICE", + question: "Which meeting sets the sprint scope?", + status: "OPEN", + ...over, + }) satisfies 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("names every phase that came back empty, not only the first", () => { + expect(askAboutEmptyPhases(["Deployment"])).toBe(askAboutEmptyPhase("Deployment")); + const draft = askAboutEmptyPhases(["Deployment", "Testing", "Release"]); + expect(draft).toContain('The "Deployment", "Testing" and "Release" phases'); + expect(draft).toContain("?"); + }); + + 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"', + ); + }); +}); 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..46ec7e9be --- /dev/null +++ b/tests/unit/features/onboarding/components/StepOriginBadge.test.tsx @@ -0,0 +1,62 @@ +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("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(); + + 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(); + }); +}); diff --git a/tests/unit/features/onboarding/components/journey/QuestionWorkspace.test.tsx b/tests/unit/features/onboarding/components/journey/QuestionWorkspace.test.tsx new file mode 100644 index 000000000..26e08bbab --- /dev/null +++ b/tests/unit/features/onboarding/components/journey/QuestionWorkspace.test.tsx @@ -0,0 +1,91 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { QuestionWorkspace } from "../../../../../../src/features/onboarding/components/journey/QuestionWorkspace"; +import type { OnboardingQuestionEndpoint } from "../../../../../../src/features/onboarding/types"; + +const mockOpenAiBuddy = vi.hoisted(() => 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..4111d7491 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"; @@ -22,7 +22,16 @@ vi.mock("../../../../../../src/services/onboardingService", () => ({ }, })); +const mockOpenAiBuddy = vi.hoisted(() => vi.fn()); + +// 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, +})); + import { onboardingService } from "../../../../../../src/services/onboardingService"; +import { announceBuddyPathChanged } from "../../../../../../src/features/buddy/aiBuddyBus"; const step = { id: "step1", @@ -231,6 +240,64 @@ 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("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("keeps a half-written skip reason when the buddy changes the path", async () => { + const user = userEvent.setup(); + renderWorkspace(); + + await user.click(await screen.findByRole("button", { name: "Skip" })); + await user.type(screen.getByLabelText("Reason for skipping"), "Already know it"); + act(() => announceBuddyPathChanged()); + + await waitFor(() => expect(onboardingService.fetchStep).toHaveBeenCalledTimes(2)); + expect(screen.getByLabelText("Reason for skipping")).toHaveValue("Already know it"); + }); + + it("does not read the step again when the path catches up with a buddy change", async () => { + const { rerender, props } = renderWaitingWorkspace({ status: "WAITING" }); + await screen.findByText("1/2 done"); + vi.mocked(onboardingService.fetchStep).mockResolvedValue({ ...step, status: "FINISHED" }); + + act(() => announceBuddyPathChanged()); + await waitFor(() => expect(onboardingService.fetchStep).toHaveBeenCalledTimes(2)); + rerender(); + + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(onboardingService.fetchStep).toHaveBeenCalledTimes(2); + }); + + 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", diff --git a/tests/unit/features/onboarding/itemNumbers.test.ts b/tests/unit/features/onboarding/itemNumbers.test.ts new file mode 100644 index 000000000..53411a4e6 --- /dev/null +++ b/tests/unit/features/onboarding/itemNumbers.test.ts @@ -0,0 +1,63 @@ +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/features/team-management/components/detail/MemberJourneySection.test.tsx b/tests/unit/features/team-management/components/detail/MemberJourneySection.test.tsx index cd75f4f45..fc1f9f3de 100644 --- a/tests/unit/features/team-management/components/detail/MemberJourneySection.test.tsx +++ b/tests/unit/features/team-management/components/detail/MemberJourneySection.test.tsx @@ -186,6 +186,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("shows a phase asked for from the summary above, and scrolls down to it", () => { const scrollIntoView = vi.spyOn(HTMLElement.prototype, "scrollIntoView"); const ref = createRef(); diff --git a/tests/unit/pages/OnBoardingPage.test.tsx b/tests/unit/pages/OnBoardingPage.test.tsx index 40e90839a..d0daf8fd4 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, @@ -28,6 +29,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", () => ({ @@ -738,6 +751,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", () => @@ -894,3 +954,199 @@ describe("OnBoardingPage", () => { await waitFor(() => expect(screen.getByTestId("location")).toHaveTextContent(/^\/onboarding$/)); }); }); + +/** + * Following a link the buddy wrote. + * + * 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: 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(); + 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("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"))), + ); + + const { container } = render( + + + , + ); + + // 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 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("OPEN"))), + ); + + const { container } = render( + + + , + ); + + expect(await screen.findByRole("heading", { name: "Meetings", level: 2 })).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", + ); + }); + + /** + * 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(); + }); + + /** 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("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"))), + ); + + render( + + + , + ); + + expect(await screen.findByRole("heading", { name: "Meetings", level: 2 })).toBeInTheDocument(); + }); +}); diff --git a/tests/unit/services/buddyService.test.ts b/tests/unit/services/buddyService.test.ts index fb80a0d18..05abfabf0 100644 --- a/tests/unit/services/buddyService.test.ts +++ b/tests/unit/services/buddyService.test.ts @@ -254,7 +254,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( @@ -284,8 +284,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, }); @@ -515,14 +515,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 () => { @@ -575,6 +575,58 @@ describe("buddyService", () => { await performAction("reorder_cards", { cardIds: ["c-2", "c-1"] }); expect(capturedBody).toMatchObject({ action: "reorder_cards", cardIds: ["c-2", "c-1"] }); + + // The options a multiple-choice answer stands for: without them the backend cannot check the + // button still means what it said, and sends nothing. + await performAction("answer_question", { + questionId: "q-1", + answer: "Git, Docker", + optionIds: ["o-1", "o-2"], + }); + expect(capturedBody).toMatchObject({ + action: "answer_question", + questionId: "q-1", + answer: "Git, Docker", + optionIds: ["o-1", "o-2"], + }); + }); + + it("carries the resolved options of an answer proposal off the stream", async () => { + const encoder = new TextEncoder(); + const stream = new ReadableStream({ + start(controller) { + controller.enqueue( + encoder.encode( + 'data: {"type":"action_proposal","action":"answer_question","label":"Send this answer: “Git, Docker”","question_id":"q-1","answer":"Git, Docker","option_ids":["o-1","o-2"]}\n\n', + ), + ); + controller.enqueue(encoder.encode('data: {"type":"done"}\n\n')); + controller.close(); + }, + }); + server.use( + http.post( + "/api/v1/onboarding/me/buddy/messages", + () => new HttpResponse(stream, { headers: { "Content-Type": "text/event-stream" } }), + ), + ); + + const onActionProposal = vi.fn(); + await streamMessage("send both", { + onToken: vi.fn(), + onCitation: vi.fn(), + onDone: vi.fn(), + onActionProposal, + }); + + expect(onActionProposal).toHaveBeenCalledWith( + expect.objectContaining({ + action: "answer_question", + questionId: "q-1", + answer: "Git, Docker", + optionIds: ["o-1", "o-2"], + }), + ); }); });