diff --git a/packages/skills/diffity-tour/SKILL.md b/packages/skills/diffity-tour/SKILL.md index d4f0edf..659faa5 100644 --- a/packages/skills/diffity-tour/SKILL.md +++ b/packages/skills/diffity-tour/SKILL.md @@ -150,7 +150,7 @@ The tour UI has a dedicated explanation panel. The intro (from `tour-start --bod - `--file`: Path relative to repo root (e.g. `src/server.ts`) - `--line` / `--end-line`: The exact line range to highlight. Keep it focused on the relevant section. - - `--annotation`: A short label (3-6 words) shown as the step title. Think of it as a chapter heading. + - `--annotation`: A short label (3-6 words) shown as the step title. Think of it as a chapter heading. In a review tour it is also the one-line label beside the file in the reordered file list, so make it say why this file is read at this point ("the primitive", "first consumer", "the P1 lives here") rather than restating the file name. - `--body`: The narrative shown in the explanation panel. This has generous space — use it to write thorough explanations using markdown: **Verify targets before committing to a step.** Before calling `tour-step`, verify that the `--file` path is readable from the repo root and that the function or block you're describing actually lives at the advertised `--line`/`--end-line` range — read the range, don't trust memory from earlier in the session. The same applies to every `goto:` link in the body: a broken goto link is worse than no link because the reader trusts it and gets dropped in the wrong place without any signal that the link is wrong. If you're not sure of the exact line, use plain backtick code instead of a goto link. @@ -235,8 +235,10 @@ The tour UI has a dedicated explanation panel. The intro (from `tour-start --bod ### Phase 3: Open in browser -1. Get the running instance port from `{{binary}} list --json`. -2. Open the tour: `open "http://localhost:/tour/"` (or the appropriate command for the user's OS). +1. Get the running instance port and `ref` from `{{binary}} list --json`. +2. Open it, which differs by mode: + - **Review mode**: `open "http://localhost:/diff?ref="`. The walkthrough drives the diff itself — the file list is reordered into your reading order, each file shows its `--annotation` as a one-line label, and a stepper walks the stops. Do **not** send the reader to `/tour/` for a review; that route browses the repository, not the change. + - **Every other mode**: `open "http://localhost:/tour/"`. 3. Tell the user the tour is ready: > Your tour is ready — check your browser. diff --git a/packages/ui/src/components/diff/diff-page.tsx b/packages/ui/src/components/diff/diff-page.tsx index 5ab7de8..0325f5a 100644 --- a/packages/ui/src/components/diff/diff-page.tsx +++ b/packages/ui/src/components/diff/diff-page.tsx @@ -7,6 +7,9 @@ import { useTheme } from '../../hooks/use-theme'; import { useWrapLines } from '../../hooks/use-wrap-lines'; import { useKeyboard } from '../../hooks/use-keyboard'; import { useReviewThreads } from '../../hooks/use-review-threads'; +import { useTours } from '../../hooks/use-tours'; +import { pickActiveTour, orderPathsByTour, stopsByPath } from '../../lib/tour-order'; +import { TourStepper } from './tour-stepper'; import { useCommentActions } from '../../hooks/use-comment-actions'; import { Toolbar } from '../layout/toolbar'; import { DiffView, type DiffViewHandle } from './diff-view'; @@ -74,6 +77,23 @@ export function DiffPage() { const commentActions = useCommentActions(sessionId, reviewsEnabled); const commentCountsByFile = useMemo(() => buildThreadCountsByFile(threads), [threads]); + const { data: tours } = useTours(reviewsEnabled ? sessionId : null); + const activeTour = useMemo(() => pickActiveTour(tours), [tours]); + const [reviewOrderEnabled, setReviewOrderEnabled] = useState(true); + const [tourStepIndex, setTourStepIndex] = useState(0); + + const diffPaths = useMemo(() => (diff ? diff.files.map(file => getFilePath(file)) : []), [diff]); + const tourStops = useMemo(() => stopsByPath(activeTour, diffPaths), [activeTour, diffPaths]); + + const orderedDiff = useMemo(() => { + if (!diff || !activeTour || !reviewOrderEnabled || activeTour.steps.length === 0) { + return diff; + } + const order = orderPathsByTour(diffPaths, activeTour.steps.map(step => step.filePath)); + const byPath = new Map(diff.files.map(file => [getFilePath(file), file])); + return { ...diff, files: order.map(path => byPath.get(path)!) }; + }, [diff, activeTour, reviewOrderEnabled, diffPaths]); + const filesWithComments = useMemo(() => { return new Set(commentCountsByFile.keys()); }, [commentCountsByFile]); @@ -190,21 +210,31 @@ export function DiffPage() { }, []); const getCurrentFilePath = useCallback((): string | null => { - if (!diff) { + if (!orderedDiff) { return null; } - return getFilePath(diff.files[currentFileIdx.current]); - }, [diff]); + return getFilePath(orderedDiff.files[currentFileIdx.current]); + }, [orderedDiff]); const navigateFile = useCallback((direction: number) => { - if (!diff) { + if (!orderedDiff) { return; } - const nextIdx = Math.max(0, Math.min(diff.files.length - 1, currentFileIdx.current + direction)); + const nextIdx = Math.max(0, Math.min(orderedDiff.files.length - 1, currentFileIdx.current + direction)); currentFileIdx.current = nextIdx; - const path = getFilePath(diff.files[nextIdx]); + const path = getFilePath(orderedDiff.files[nextIdx]); diffViewRef.current?.scrollToFile(path); - }, [diff]); + }, [orderedDiff]); + + const handleTourStepChange = useCallback((index: number) => { + const steps = activeTour ? [...activeTour.steps].sort((a, b) => a.sortOrder - b.sortOrder) : []; + if (steps.length === 0) { + return; + } + const clamped = Math.max(0, Math.min(steps.length - 1, index)); + setTourStepIndex(clamped); + diffViewRef.current?.scrollToFile(steps[clamped].filePath); + }, [activeTour]); const navigateHunk = useCallback((direction: number) => { const hunks = getHunkHeaders(); @@ -239,10 +269,10 @@ export function DiffPage() { } }, onCollapseAll: () => { - if (!diff) { + if (!orderedDiff) { return; } - const allPaths = diff.files.map((f) => getFilePath(f)); + const allPaths = orderedDiff.files.map((f) => getFilePath(f)); const anyExpanded = allPaths.some((p) => !collapsedFiles.has(p)); manuallyToggledRef.current = new Set(); if (anyExpanded) { @@ -380,18 +410,34 @@ export function DiffPage() { onGitHubPulled={() => queryClient.invalidateQueries({ queryKey: ['threads'] })} /> {isStale && } + {activeTour && ( + + )}
0 + ? { + stops: tourStops, + enabled: reviewOrderEnabled, + onToggle: () => setReviewOrderEnabled(prev => !prev), + } + : undefined + } /> - {diff ? ( + {orderedDiff ? ( ; + activeFile: string | null; + reviewedFiles: Set; + commentCountsByFile: Map; + onFileClick: (path: string) => void; +} + +function splitPath(path: string): { dir: string; name: string } { + const idx = path.lastIndexOf('/'); + if (idx === -1) { + return { dir: '', name: path }; + } + return { dir: path.slice(0, idx), name: path.slice(idx + 1) }; +} + +export function ReviewOrderList(props: ReviewOrderListProps) { + const { files, stops, activeFile, reviewedFiles, commentCountsByFile, onFileClick } = props; + + const firstUnvisited = files.findIndex(file => !stops.has(getFilePath(file))); + + return ( +
    + {files.map((file, index) => { + const path = getFilePath(file); + const stop = stops.get(path); + const { dir, name } = splitPath(path); + const comments = commentCountsByFile.get(path); + const isActive = activeFile === path; + + return ( +
  • + {index === firstUnvisited && firstUnvisited > 0 && ( +
    + + Not in the walkthrough + +
    + )} + +
    • + ); + })} +
+ ); +} diff --git a/packages/ui/src/components/diff/tour-stepper.tsx b/packages/ui/src/components/diff/tour-stepper.tsx new file mode 100644 index 0000000..d2d0cd1 --- /dev/null +++ b/packages/ui/src/components/diff/tour-stepper.tsx @@ -0,0 +1,76 @@ +import { useMemo } from 'react'; +import type { Tour } from '../../lib/api'; +import { CompassIcon } from '../icons/compass-icon'; +import { ChevronUpIcon } from '../icons/chevron-up-icon'; +import { ChevronDownIcon } from '../icons/chevron-down-icon'; +import { Spinner } from '../icons/spinner'; +import { MarkdownContent } from '../layout/markdown-content'; + +interface TourStepperProps { + tour: Tour; + stepIndex: number; + onStepChange: (index: number) => void; +} + +export function TourStepper(props: TourStepperProps) { + const { tour, stepIndex, onStepChange } = props; + + const steps = useMemo( + () => [...tour.steps].sort((a, b) => a.sortOrder - b.sortOrder), + [tour.steps], + ); + const isBuilding = tour.status === 'building'; + + if (steps.length === 0) { + return ( +
+ {isBuilding ? : } + {isBuilding ? 'Working out a reading order…' : tour.topic} +
+ ); + } + + const index = Math.min(stepIndex, steps.length - 1); + const step = steps[index]; + + return ( +
+ + + {index + 1}/{steps.length} + {isBuilding && } + +
+
+ {step.filePath} + + {step.startLine === step.endLine ? step.startLine : `${step.startLine}-${step.endLine}`} + +
+ {step.body && ( +
+ +
+ )} +
+
+ + +
+
+ ); +} diff --git a/packages/ui/src/components/icons/list-ordered-icon.tsx b/packages/ui/src/components/icons/list-ordered-icon.tsx new file mode 100644 index 0000000..f9e7f00 --- /dev/null +++ b/packages/ui/src/components/icons/list-ordered-icon.tsx @@ -0,0 +1,18 @@ +export function ListOrderedIcon(props: { className?: string }) { + return ( + + ); +} diff --git a/packages/ui/src/components/layout/sidebar.tsx b/packages/ui/src/components/layout/sidebar.tsx index 6c87db5..25f63c4 100644 --- a/packages/ui/src/components/layout/sidebar.tsx +++ b/packages/ui/src/components/layout/sidebar.tsx @@ -1,5 +1,6 @@ import { useEffect, useMemo, useRef, useState } from 'react'; import type { DiffFile } from '@diffity/parser'; +import { getFilePath } from '../../lib/diff-utils'; import { FileTree } from '../tree/file-tree'; import type { FileTreeHandle } from '../tree/file-tree'; import { SidebarIcon } from '../icons/sidebar-icon'; @@ -8,6 +9,9 @@ import { XIcon } from '../icons/x-icon'; import { CommentIcon } from '../icons/comment-icon'; import { CollapseAllIcon } from '../icons/collapse-all-icon'; import { ExpandAllIcon } from '../icons/expand-all-icon'; +import { ListOrderedIcon } from '../icons/list-ordered-icon'; +import { ReviewOrderList } from '../diff/review-order-list'; +import type { TourFileStop } from '../../lib/tour-order'; interface SidebarProps { files: DiffFile[]; @@ -16,6 +20,12 @@ interface SidebarProps { commentCountsByFile: Map; onFileClick: (path: string) => void; onCommentedFileClick: (path: string) => void; + /** Absent when no walkthrough exists for this diff, which hides the ordering toggle. */ + reviewOrder?: { + stops: Map; + enabled: boolean; + onToggle: () => void; + }; } export function Sidebar(props: SidebarProps) { @@ -26,6 +36,7 @@ export function Sidebar(props: SidebarProps) { commentCountsByFile, onFileClick, onCommentedFileClick, + reviewOrder, } = props; const fileTreeRef = useRef(null); const [search, setSearch] = useState(''); @@ -33,6 +44,7 @@ export function Sidebar(props: SidebarProps) { const [commentedFilesOnly, setCommentedFilesOnly] = useState(false); const [allExpanded, setAllExpanded] = useState(true); + const inReviewOrder = !!reviewOrder?.enabled; const commentedFileCount = commentCountsByFile.size; const commentedFileCountLabel = commentedFileCount > 99 ? '99+' : String(commentedFileCount); const countLabel = useMemo(() => { @@ -51,6 +63,17 @@ export function Sidebar(props: SidebarProps) { } }, [commentedFileCount, commentedFilesOnly]); + const visibleFiles = useMemo(() => { + const needle = search.trim().toLowerCase(); + return files.filter(file => { + const path = getFilePath(file); + if (commentedFilesOnly && !commentCountsByFile.has(path)) { + return false; + } + return !needle || path.toLowerCase().includes(needle); + }); + }, [files, search, commentedFilesOnly, commentCountsByFile]); + const handleTreeFileClick = (path: string) => { if (commentedFilesOnly && commentCountsByFile.has(path)) { onCommentedFileClick(path); @@ -77,29 +100,43 @@ export function Sidebar(props: SidebarProps) { ); } diff --git a/packages/ui/src/hooks/use-tours.ts b/packages/ui/src/hooks/use-tours.ts new file mode 100644 index 0000000..772ea59 --- /dev/null +++ b/packages/ui/src/hooks/use-tours.ts @@ -0,0 +1,12 @@ +import { useQuery } from '@tanstack/react-query'; +import { fetchTours, type Tour } from '../lib/api'; + +export function useTours(sessionId: string | null | undefined) { + return useQuery({ + queryKey: ['tours', sessionId], + queryFn: () => fetchTours(sessionId!), + enabled: !!sessionId, + // Matches the comment poll, so a walkthrough appears step by step while it is written. + refetchInterval: 2000, + }); +} diff --git a/packages/ui/src/lib/api.ts b/packages/ui/src/lib/api.ts index 9a6daca..78f3390 100644 --- a/packages/ui/src/lib/api.ts +++ b/packages/ui/src/lib/api.ts @@ -278,6 +278,10 @@ export function fetchTour(tourId: string): Promise { return apiFetch(`/api/tours/${tourId}`); } +export function fetchTours(sessionId: string): Promise { + return apiFetch(`/api/tours?session=${encodeURIComponent(sessionId)}`); +} + export interface TreeEntryResponse { type: 'blob' | 'tree'; path: string; diff --git a/packages/ui/src/lib/tour-order.ts b/packages/ui/src/lib/tour-order.ts new file mode 100644 index 0000000..082300e --- /dev/null +++ b/packages/ui/src/lib/tour-order.ts @@ -0,0 +1,68 @@ +import type { Tour } from './api'; + +export interface TourFileStop { + /** 1-based position of the file in the reading order. */ + position: number; + annotation: string; + body: string; +} + +export function pickActiveTour(tours: Tour[] | undefined | null): Tour | null { + if (!tours || tours.length === 0) { + return null; + } + return tours.reduce((latest, tour) => (tour.createdAt > latest.createdAt ? tour : latest)); +} + +/** + * Files the walkthrough visits come first, in the order it visits them; everything else keeps + * the order the diff already had. A file the walkthrough mentions but the diff does not contain + * is dropped rather than inventing a row for it. + */ +export function orderPathsByTour(paths: string[], tourPaths: string[]): string[] { + const available = new Set(paths); + const ordered: string[] = []; + const placed = new Set(); + + for (const path of tourPaths) { + if (available.has(path) && !placed.has(path)) { + ordered.push(path); + placed.add(path); + } + } + + for (const path of paths) { + if (!placed.has(path)) { + ordered.push(path); + } + } + + return ordered; +} + +/** + * The first stop at each file, keyed by path. A walkthrough that returns to a file later still + * numbers it by where it was first read. + */ +export function stopsByPath(tour: Tour | null, availablePaths: string[]): Map { + const stops = new Map(); + if (!tour) { + return stops; + } + + const available = new Set(availablePaths); + const steps = [...tour.steps].sort((a, b) => a.sortOrder - b.sortOrder); + + for (const step of steps) { + if (!available.has(step.filePath) || stops.has(step.filePath)) { + continue; + } + stops.set(step.filePath, { + position: stops.size + 1, + annotation: step.annotation, + body: step.body, + }); + } + + return stops; +} diff --git a/packages/ui/tests/tour-order.test.ts b/packages/ui/tests/tour-order.test.ts new file mode 100644 index 0000000..5016655 --- /dev/null +++ b/packages/ui/tests/tour-order.test.ts @@ -0,0 +1,108 @@ +import { describe, it, expect } from 'vitest'; +import { pickActiveTour, orderPathsByTour, stopsByPath } from '../src/lib/tour-order'; +import type { Tour, TourStep } from '../src/lib/api'; + +function step(filePath: string, sortOrder: number, annotation = ''): TourStep { + return { + id: `s${sortOrder}`, + tourId: 't', + sortOrder, + filePath, + startLine: 1, + endLine: 1, + body: `read ${filePath}`, + annotation, + createdAt: '2026-08-21T10:00:00.000Z', + }; +} + +function tour(steps: TourStep[], overrides: Partial = {}): Tour { + return { + id: 't', + sessionId: 'sess', + topic: 'Reading order', + body: '', + status: 'ready', + createdAt: '2026-08-21T10:00:00.000Z', + steps, + ...overrides, + }; +} + +describe('orderPathsByTour', () => { + const paths = ['a.ts', 'b.ts', 'c.ts', 'd.ts']; + + it('puts the walkthrough first and keeps the rest in their original order', () => { + expect(orderPathsByTour(paths, ['c.ts', 'a.ts'])).toEqual(['c.ts', 'a.ts', 'b.ts', 'd.ts']); + }); + + it('places a file where it is first visited when the walkthrough returns to it', () => { + expect(orderPathsByTour(paths, ['d.ts', 'b.ts', 'd.ts'])).toEqual(['d.ts', 'b.ts', 'a.ts', 'c.ts']); + }); + + it('ignores files the walkthrough names but the diff does not contain', () => { + expect(orderPathsByTour(paths, ['gone.ts', 'b.ts'])).toEqual(['b.ts', 'a.ts', 'c.ts', 'd.ts']); + }); + + it('is the identity when the walkthrough is empty', () => { + expect(orderPathsByTour(paths, [])).toEqual(paths); + }); + + it('loses no files and duplicates none', () => { + const ordered = orderPathsByTour(paths, ['c.ts', 'c.ts', 'a.ts']); + + expect([...ordered].sort()).toEqual([...paths].sort()); + }); +}); + +describe('stopsByPath', () => { + it('numbers files by first visit, not by step', () => { + const stops = stopsByPath( + tour([step('a.ts', 1, 'entry point'), step('b.ts', 2), step('a.ts', 3)]), + ['a.ts', 'b.ts'], + ); + + expect(stops.get('a.ts')).toEqual({ position: 1, annotation: 'entry point', body: 'read a.ts' }); + expect(stops.get('b.ts')?.position).toBe(2); + expect(stops.size).toBe(2); + }); + + it('orders by sortOrder rather than arrival', () => { + const stops = stopsByPath(tour([step('b.ts', 2), step('a.ts', 1)]), ['a.ts', 'b.ts']); + + expect(stops.get('a.ts')?.position).toBe(1); + expect(stops.get('b.ts')?.position).toBe(2); + }); + + it('skips files that are not in the diff', () => { + const stops = stopsByPath(tour([step('gone.ts', 1), step('a.ts', 2)]), ['a.ts']); + + expect(stops.has('gone.ts')).toBe(false); + expect(stops.get('a.ts')?.position).toBe(1); + }); + + it('is empty without a walkthrough', () => { + expect(stopsByPath(null, ['a.ts']).size).toBe(0); + }); +}); + +describe('pickActiveTour', () => { + it('takes the most recent walkthrough', () => { + const older = tour([], { id: 'old', createdAt: '2026-08-21T09:00:00.000Z' }); + const newer = tour([], { id: 'new', createdAt: '2026-08-21T11:00:00.000Z' }); + + expect(pickActiveTour([older, newer])?.id).toBe('new'); + expect(pickActiveTour([newer, older])?.id).toBe('new'); + }); + + it('includes one that is still being written', () => { + const building = tour([], { id: 'building', status: 'building' }); + + expect(pickActiveTour([building])?.id).toBe('building'); + }); + + it('returns null when there is none', () => { + expect(pickActiveTour([])).toBeNull(); + expect(pickActiveTour(undefined)).toBeNull(); + }); +}); diff --git a/skills/diffity-tour/SKILL.md b/skills/diffity-tour/SKILL.md index 12d8bd5..0855452 100644 --- a/skills/diffity-tour/SKILL.md +++ b/skills/diffity-tour/SKILL.md @@ -153,7 +153,7 @@ The tour UI has a dedicated explanation panel. The intro (from `tour-start --bod - `--file`: Path relative to repo root (e.g. `src/server.ts`) - `--line` / `--end-line`: The exact line range to highlight. Keep it focused on the relevant section. - - `--annotation`: A short label (3-6 words) shown as the step title. Think of it as a chapter heading. + - `--annotation`: A short label (3-6 words) shown as the step title. Think of it as a chapter heading. In a review tour it is also the one-line label beside the file in the reordered file list, so make it say why this file is read at this point ("the primitive", "first consumer", "the P1 lives here") rather than restating the file name. - `--body`: The narrative shown in the explanation panel. This has generous space — use it to write thorough explanations using markdown: **Verify targets before committing to a step.** Before calling `tour-step`, verify that the `--file` path is readable from the repo root and that the function or block you're describing actually lives at the advertised `--line`/`--end-line` range — read the range, don't trust memory from earlier in the session. The same applies to every `goto:` link in the body: a broken goto link is worse than no link because the reader trusts it and gets dropped in the wrong place without any signal that the link is wrong. If you're not sure of the exact line, use plain backtick code instead of a goto link. @@ -238,8 +238,10 @@ The tour UI has a dedicated explanation panel. The intro (from `tour-start --bod ### Phase 3: Open in browser -1. Get the running instance port from `diffity list --json`. -2. Open the tour: `open "http://localhost:/tour/"` (or the appropriate command for the user's OS). +1. Get the running instance port and `ref` from `diffity list --json`. +2. Open it, which differs by mode: + - **Review mode**: `open "http://localhost:/diff?ref="`. The walkthrough drives the diff itself — the file list is reordered into your reading order, each file shows its `--annotation` as a one-line label, and a stepper walks the stops. Do **not** send the reader to `/tour/` for a review; that route browses the repository, not the change. + - **Every other mode**: `open "http://localhost:/tour/"`. 3. Tell the user the tour is ready: > Your tour is ready — check your browser.