From 53d2246b16bcbf4a0c11ebf4469042daf3133735 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 20 Apr 2026 20:57:57 -0700 Subject: [PATCH] =?UTF-8?q?feat:=20Code=20Tour=20=E2=80=94=20guided=20PR?= =?UTF-8?q?=20walkthrough=20as=20a=20third=20agent=20provider=20(#569)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(server): add Code Tour agent as third review provider Adds a "tour" provider alongside "claude" and "codex" that generates a guided walkthrough of a changeset from a product-minded colleague's perspective. Reuses the entire agent-jobs infrastructure (process lifecycle, SSE broadcasting, live logs, kill support, capability detection) so the only new server plumbing is the prompt/schema module, the GET endpoint for the result, and checklist persistence. - tour-review.ts: prompt, JSON schema, Claude (stream-json) and Codex (file output) command builders, and parsers for both. Prompt frames the agent as a colleague giving a casual tour, orders stops by reading flow (not impact), chunks by logical change (not per-file), and writes QA questions a human can answer by reading code or using the product. - review.ts: tour buildCommand with per-engine model defaults (fixes a bug where Codex was defaulting to "sonnet", a Claude model), an onJobComplete handler that parses and stores the tour, plus GET /api/tour/:jobId and PUT /api/tour/:jobId/checklist endpoints. - agent-jobs.ts: tour capability detection and config threading from the POST body through to the provider's buildCommand. - shared/agent-jobs.ts: engine/model fields on AgentJobInfo so the UI can render tour jobs with their chosen provider + model. For provenance purposes, this commit was AI assisted. * feat(review-editor): Code Tour dialog with animated walkthrough Adds the tour overlay surface: a full-screen dialog with three animated pages (Overview, Walkthrough, Checklist) that renders the CodeTourOutput from the server-side agent. Opens automatically when a tour job completes and can be dismissed with Escape or backdrop click. - components/tour/TourDialog.tsx: three-page animated dialog. Intro page uses a composition cascade (greeting, Intent/Before/After cards with one-shot color-dot pulse, key takeaways table) with a pinned Start Tour button and a bottom fade so content scrolls cleanly under it. Page slides between Overview/Walkthrough/Checklist are ease-out springs with direction auto-derived from tab index. - components/tour/TourStopCard.tsx: per-stop accordion using motion's AnimatePresence for height + opacity springs with staggered children for detail text and anchor blocks. Anchors lazy-mount DiffHunkPreview on first open to keep mount costs low. Callout labels (Important, Warning, Note) are deterministic typography, no emoji. - components/tour/QAChecklist.tsx: always-open list of verification questions with per-item spring entrance and persistent checkbox state that PUTs to the server. - hooks/useTourData.ts: fetch + checklist persistence with a dev-mode short-circuit when jobId === DEMO_TOUR_ID so UI iteration doesn't require a live agent run. - demoTour.ts: realistic demo data (multi-stop, multiple takeaways, real diff hunks) for dev iteration. - App.tsx: tour dialog state, auto-open on job completion, and a dev-only "Demo tour" floating button + Cmd/Ctrl+Shift+T shortcut guarded by import.meta.env.DEV. - index.css: all tour animations (dialog enter/exit, page slides, stop reveal cascade, intro exit) with reduced-motion fallbacks. Dark-mode contrast tuning across structural surfaces (borders, surface tints, callout backgrounds) so the design works in both themes. - package.json: adds motion@12.38.0 for spring-driven accordion physics and the intro composition cascade. For provenance purposes, this commit was AI assisted. * feat(review-editor): wire Code Tour into agent jobs UI + polish DiffHunkPreview Connects the tour provider to the existing agent jobs sidebar and detail panel so the user can launch a tour the same way they launch Claude or Codex reviews. Also tightens DiffHunkPreview (used inside tour anchors) to render correctly on first mount and handle every diff hunk shape the agent might produce. - ui/components/AgentsTab.tsx: tour engine (Claude/Codex) + model select when launching a tour job. Resets model to blank when falling back to Codex-only so Codex uses its own default instead of inheriting "sonnet" (a Claude model). - ui/hooks/useAgentJobs.ts: forward engine/model config from the UI through to the POST /api/agents/jobs body so the server's tour provider can pick the right command shape. - dock/panels/ReviewAgentJobDetailPanel.tsx: tour-aware detail panel. Replaces the "Findings" tab with a "Status" card for tour jobs and surfaces an "Open Tour" button that opens the dialog overlay when the tab is already in the dock. - components/DiffHunkPreview.tsx: synchronous pierre theme init via a useState lazy initializer so the first render (inside a tooltip) is already themed; robust hunk parser that handles bare @@ hunks, file- level --- hunks, and full git diffs; "Diff not available" fallback for broken hunks. - components/ReviewSidebar.tsx, dock/ReviewStateContext.tsx: small wiring changes to expose openTourPanel from the review state context. Also removes two dead components (TourHeader, DiffAnchorChip) that were superseded by the inline title bar in TourDialog and the inline AnchorBlock in TourStopCard. For provenance purposes, this commit was AI assisted. * refactor(server): extract Code Tour lifecycle into shared createTourSession factory + Pi parity The route-parity test suite requires the Pi server (apps/pi-extension/) to expose the same routes as the Bun server (packages/server/). After Code Tour shipped in the prior 3 commits, Pi was missing /api/tour/:jobId (GET) and /api/tour/:jobId/checklist (PUT). A naive mirror would duplicate ~100 lines of provider-branch logic (buildCommand, onJobComplete, in-memory maps) into Pi's serverReview.ts, perpetuating the existing claude/codex duplication problem. Instead, extract the pure runtime-agnostic tour lifecycle into a createTourSession() factory that both servers consume. Route handlers stay per-server (different HTTP primitives) but are ~5 lines each. Net effect: Pi port is ~25 lines instead of ~100. Future providers that adopt the same pattern cost ~15 lines per server. - tour-review.ts: new createTourSession() at the bottom of the module. Encapsulates tourResults + tourChecklists maps, buildCommand (with the Claude-vs-Codex model-default fix baked in), onJobComplete (parse/store/summarize), plus getTour/saveChecklist lookup helpers for route handlers. - review.ts (Bun): tour branch in buildCommand, tour branch in onJobComplete, and both route handlers collapse to one-line calls into the factory. Drops ~70 lines. - vendor.sh: add tour-review to the review-agent loop so Pi regenerates generated/tour-review.ts on every build:pi. - serverReview.ts (Pi): import createTourSession from ../generated/tour-review.js; add tour branch to buildCommand (one line), tour branch to onJobComplete (three lines), and GET/PUT route handlers using Pi's json() helper. ~25 lines added. - agent-jobs.ts (Pi): extend buildCommand interface to accept config and return engine/model; thread config from POST body; extend spawnJob to persist engine/model on AgentJobInfo; add tour to capability list. Claude and Codex branches are intentionally left in the old pattern; they can migrate to the factory approach when next touched to keep this change's blast radius contained. Tests: 518/518 passing (previous 3 route-parity failures resolved, plus 2 extra assertions passing since tour is now in both servers' route tables). For provenance purposes, this commit was AI assisted. * refactor(tour): self-review cleanup — Pi route match parity + remove setOpen shim Two small cleanups surfaced by a self-review pass: - Pi's GET /api/tour/:jobId route used `endsWith("/checklist")` to block the checklist sub-route, while Bun uses `includes("/", ...)`. The two are not equivalent: a URL like /api/tour/abc/extra would be accepted by Pi (jobId becomes "abc/extra") but correctly rejected by Bun. Align Pi to Bun's pattern. - TourStopCard had a leftover `setOpen` shim from when open state was local to the card. State is now lifted to TourDialog, so the shim just aliases onToggle and ignores its argument. Replace with a direct `onClick={onToggle}` on the trigger button. 520/0 tests still pass. For provenance purposes, this commit was AI assisted. * feat(review): per-provider model + effort controls across all review agents Exposes model, effort/reasoning, and fast-mode (Codex) as per-job knobs for Claude, Codex, and Code Tour via the Agents tab. Threads the config from the UI through the POST body into each provider's buildCommand, which emits the corresponding CLI flags (--model/--effort for Claude; -m / -c model_reasoning_effort / -c service_tier for Codex). Bun and Pi servers stay in parity. UI: option catalogs (CLAUDE_MODELS, CLAUDE_EFFORT, CODEX_MODELS, CODEX_REASONING, TOUR_CLAUDE_MODELS) are defined once and mapped into every dropdown so there's a single source of truth for the choices. Tour's Claude/Codex settings are kept separate from the standalone provider's state so toggling the tour engine no longer overwrites the provider's last choice. Job badge now surfaces the model/reasoning/fast selection for both Tour and the standalone Codex provider. For provenance purposes, this commit was AI assisted. * docs: add Prompts reference page Documents the three-layer structure every review call travels through (CLI system prompt, our user message of review-prompt + user-prompt, output schema flag), names each constant + its file, and calls out that the Claude/Codex review prompts are the upstream ones from those projects (only Code Tour's prompt is original to Plannotator). Linked from the Code Review command page. For provenance purposes, this commit was AI assisted. * feat(review): persist per-agent, per-model settings in a single cookie Drops the hidden "Default" options from the agent dropdowns, locks in explicit sensible defaults (Opus 4.7/High for Claude review, gpt-5.3-codex/High for Codex, Sonnet/Medium for Tour Claude, gpt-5.3-codex/Medium for Tour Codex), and remembers the last-used effort/reasoning/fast-mode per (agent job × model) so switching models reveals the choices you made last time. Backed by a single `plannotator.agents` cookie holding the whole settings tree — one read on mount, one mirror write per change, all mutations funnel through a single React state owner to avoid stale-read or lost-write races across rapid successive updates. For provenance purposes, this commit was AI assisted. * fix(tour): unblock reduced-motion nav + flush pending checklist save on close Two P2 issues surfaced in review: - Under prefers-reduced-motion, tour page animations are suppressed, so onAnimationEnd never fires and exitingPage stuck on 'intro' kept the walkthrough/checklist gated out. navigate() now swaps pages directly when reduced motion is on, mirroring the pattern already used in the wrapper. - Checklist toggles are debounced 500ms before the PUT, but unmount only cleared the timer — checking an item and closing within the window dropped the save. Cleanup now flushes the pending payload with keepalive: true. For provenance purposes, this commit was AI assisted. * feat(review): surface Claude model + effort in job badge, trim redundant labels Claude effort was never persisted on AgentJobInfo, so the job badge had nothing to render beyond "Claude". Plumbs `effort` through the shared type, both server build-command pipelines (Bun + Pi), and spawnJob, then teaches the ProviderBadge to display Claude and Tour Claude model + effort with the same shape Codex already had. Labels resolve via the dropdown catalogs so the badge shows "Opus 4.7" / "High" instead of raw ids. Server labels for both Claude and Codex reviews collapse to plain "Code Review" (matching tour's "Code Tour"), since the badge now carries the provider + model + settings — the title only needs to name the action. For provenance purposes, this commit was AI assisted. * polish(review): prefix agent dropdown options with the action name The Run launcher listed "Claude Code", "Codex CLI", and "Code Tour" side by side — the CLI's detection name was doing double duty as the action label. Prefixes the review entries with "Code Review · " so scanning three options surfaces two reviews + one tour instead of three raw provider names. For provenance purposes, this commit was AI assisted. * fix(review): drop invalid codex reasoning option + fail tour on empty output Two P2 issues from PR review: - Tour jobs exited 0 but returned null output would still be marked "done", and the auto-open watcher would greet the user with a 404 "Tour not found" dialog. onJobComplete now flips the job to failed with an error message when nothing was stored, so the card reflects the real state. - The codex reasoning dropdown offered "None", but codex-rs only accepts minimal/low/medium/high/xhigh. Picking None sent `-c model_reasoning_effort=none` and launched a broken job. Removed the option; added a one-shot cookie migration so users who already saved "none" don't keep shipping it. For provenance purposes, this commit was AI assisted. * fix(tour): format Claude-engine logs + allow reading linked issues Two P2/P3 issues from PR review: - The stdout formatter keyed only on provider === "claude", so Code Tour jobs running on the Claude engine streamed raw JSONL to the log panel while Claude review jobs got formatted text. Widened the check to also catch spawnOptions.engine === "claude" and mirrored the fix into Pi. - The tour Claude allowlist permitted PR/MR commands but not issue reads, yet the prompt explicitly asks the agent to open `Fixes #123` / `Closes owner/repo#456` targets for deeper context. Claude was being denied those commands mid-tour. Added gh issue view + gh api issues + glab issue view to the allowlist. For provenance purposes, this commit was AI assisted. * cleanup(review): e2e punchlist — Pi parity + dedup + tests - Pi now logs Claude parse failures with the same diagnostic as Bun - Tour route match uses a regex instead of an includes-offset trick - Drop `any` cast in Pi's Codex blocking-findings predicate - Extract `patchClaude` twin of `patchCodex` in useAgentSettings - Factor `resolveAgentCwd` helper in Bun review to match Pi - AgentsTab launch payload becomes a per-provider dispatch table - Wrap TourDialog in MotionConfig reducedMotion="user" so motion.* children honor prefers-reduced-motion alongside the CSS keyframes - Tests for parseTourStreamOutput/parseTourFileOutput and the Codex perModel sanitizer For provenance purposes, this commit was AI assisted. * test(tour-review): use real TourStop shape in fixtures The parsers don't validate stop fields so the original made-up shape passed fine, but future readers would be misled. Use the real CodeTourOutput / TourStop / TourDiffAnchor shapes from tour-review.ts. For provenance purposes, this commit was AI assisted. * cleanup(tour): hoist shared types + subfolder server module + dedup - Hoist tour output types to packages/shared/tour.ts so server and UI share one source of truth (prevents silent drift when schema changes). - Move tour-review into packages/server/tour/ subfolder alongside the existing packages/review-editor/components/tour/ convention. - Extend tour.buildCommand to accept tour context directly (patch, diffType, options, prMetadata), so Bun and Pi no longer duplicate the buildTourUserMessage call site. Export TOUR_EMPTY_OUTPUT_ERROR from the tour module to prevent string drift between servers. - Early-return the tour branch in buildCommand so the review userMessage is built once on the shared non-tour path instead of unconditionally (dead work on tour launches) or twice (codex+claude dup). - Regex capture group for tour checklist jobId in both servers — replaces a second url.pathname.split("/")[3] fragile fallback. - Drop existsSync guard in parseTourFileOutput (TOCTOU + duplicate syscall); the outer try/catch already handles ENOENT. - Dedupe two matchMedia reads in TourDialog behind a prefersReducedMotion helper; memoize intro-page card array so unrelated state churn doesn't recompute it. - Comment hygiene on tour files — drop WHAT/change-referencing comments added by this PR; keep non-obvious WHY. For provenance purposes, this commit was AI assisted. * fix(tour): hoist introCards useMemo above early returns + subfolder hook The memoized intro-card array was placed after the loading/error early returns in TourDialogContent, so the first successful render called one more hook than the initial loading render — React threw "Rendered more hooks than during the previous render" (error #310) the moment a tour finished loading. Move the useMemo above the early returns and use optional chaining on `tour` so it's safe to run during loading. Also move useTourData.ts into a hooks/tour/ subfolder to start a standard for feature-scoped hooks (was flat among siblings). For provenance purposes, this commit was AI assisted. --- .../src/content/docs/commands/code-review.md | 4 + .../src/content/docs/reference/prompts.md | 69 +++ apps/pi-extension/server/agent-jobs.ts | 49 +- apps/pi-extension/server/serverReview.ts | 86 ++- apps/pi-extension/vendor.sh | 13 +- bun.lock | 24 +- packages/review-editor/App.tsx | 56 ++ .../components/DiffHunkPreview.tsx | 103 ++-- .../components/ReviewSidebar.tsx | 2 +- .../components/tour/QAChecklist.tsx | 89 +++ .../components/tour/TourDialog.tsx | 561 +++++++++++++++++ .../components/tour/TourStopCard.tsx | 291 +++++++++ packages/review-editor/demoTour.ts | 198 ++++++ .../review-editor/dock/ReviewStateContext.tsx | 3 + .../dock/panels/ReviewAgentJobDetailPanel.tsx | 164 +++-- .../review-editor/hooks/tour/useTourData.ts | 106 ++++ packages/review-editor/index.css | 147 +++++ packages/review-editor/package.json | 6 +- packages/server/agent-jobs.ts | 49 +- packages/server/claude-review.ts | 5 +- packages/server/codex-review.ts | 8 +- packages/server/review.ts | 85 ++- packages/server/tour/tour-review.test.ts | 98 +++ packages/server/tour/tour-review.ts | 582 ++++++++++++++++++ packages/shared/agent-jobs.ts | 12 +- packages/shared/package.json | 3 +- packages/shared/tour.ts | 61 ++ packages/ui/components/AgentsTab.tsx | 351 ++++++++++- packages/ui/hooks/useAgentJobs.ts | 7 +- packages/ui/hooks/useAgentSettings.test.ts | 44 ++ packages/ui/hooks/useAgentSettings.ts | 223 +++++++ 31 files changed, 3334 insertions(+), 165 deletions(-) create mode 100644 apps/marketing/src/content/docs/reference/prompts.md create mode 100644 packages/review-editor/components/tour/QAChecklist.tsx create mode 100644 packages/review-editor/components/tour/TourDialog.tsx create mode 100644 packages/review-editor/components/tour/TourStopCard.tsx create mode 100644 packages/review-editor/demoTour.ts create mode 100644 packages/review-editor/hooks/tour/useTourData.ts create mode 100644 packages/server/tour/tour-review.test.ts create mode 100644 packages/server/tour/tour-review.ts create mode 100644 packages/shared/tour.ts create mode 100644 packages/ui/hooks/useAgentSettings.test.ts create mode 100644 packages/ui/hooks/useAgentSettings.ts diff --git a/apps/marketing/src/content/docs/commands/code-review.md b/apps/marketing/src/content/docs/commands/code-review.md index 2c8e2d79..71f075ad 100644 --- a/apps/marketing/src/content/docs/commands/code-review.md +++ b/apps/marketing/src/content/docs/commands/code-review.md @@ -106,6 +106,10 @@ When multiple providers are available, set your default in **Settings → AI**. If only one provider is installed, it's used automatically with no configuration needed. +## How review agents prompt the CLI + +The review agents (Claude, Codex, Code Tour) shell out to external CLIs. Plannotator controls the user message and output schema; the CLI's own harness owns the system prompt. See the [Prompts reference](/docs/reference/prompts/) for the full breakdown of what each provider sends, how the pieces join, and which knobs you can tune per job. + ## Submitting feedback - **Send Feedback** formats your annotations and sends them to the agent diff --git a/apps/marketing/src/content/docs/reference/prompts.md b/apps/marketing/src/content/docs/reference/prompts.md new file mode 100644 index 00000000..b5d479d2 --- /dev/null +++ b/apps/marketing/src/content/docs/reference/prompts.md @@ -0,0 +1,69 @@ +--- +title: "Prompts" +description: "How Plannotator's review agents structure their prompts, what we control, what the CLI harness owns, and how the pieces fit together." +sidebar: + order: 33 +section: "Reference" +--- + +Plannotator's review agents (Claude, Codex, and Code Tour) all shell out to an external CLI. This page maps what those CLIs receive on every invocation: which parts Plannotator controls, and which parts are owned by the CLI's own agent harness. + +Importantly, **we don't invent our own review prompts**. The Claude review prompt is derived from Claude Code's published open-source review prompt, and the Codex review prompt is copied verbatim from [`codex-rs/core/review_prompt.md`](https://github.com/openai/codex). You get the same review behavior those tools ship with. Code Tour is the one exception: it's a Plannotator-original workflow, so its prompt is ours. + +## The three layers + +Every review call is shaped by three layers: + +1. **System prompt.** Owned by the CLI (Claude Code or codex-rs). Plannotator never sets or touches this. +2. **User message.** What Plannotator sends. Always a single concatenated string of two parts: a static **review prompt** plus a dynamic **user prompt**. +3. **Output schema.** A JSON schema passed to the CLI as a flag, forcing the final assistant message to match a known shape. + +## What's in the user message + +The user message Plannotator sends is always: + +``` + + +--- + + +``` + +**Review prompt** is a long, static review instruction that lives in the repo as a TypeScript constant. It's distinct per provider. + +**User prompt** is a short, dynamic line built per call from the diff type (`uncommitted`, `staged`, `last-commit`, `branch`, PR URL, and so on). The same builder is used for all providers. + +## Matrix + +| | Claude review | Codex review | Code Tour (Claude or Codex) | +|---|---|---|---| +| **System prompt** | Owned by `claude` CLI. We don't touch it. | Owned by `codex` CLI. We don't touch it. | Same as whichever engine runs. | +| **Review prompt (static, ours)** | `CLAUDE_REVIEW_PROMPT` in `packages/server/claude-review.ts` | `CODEX_REVIEW_SYSTEM_PROMPT` in `packages/server/codex-review.ts` (misnamed; it's user content) | `TOUR_REVIEW_PROMPT` in `packages/server/tour-review.ts` | +| **User prompt (dynamic, ours)** | `buildCodexReviewUserMessage(patch, diffType, …)` | same function | same function | +| **Full user message** | `review prompt + "\n\n---\n\n" + user prompt` | same | same | +| **Delivered via** | stdin | last positional argv | stdin (Claude engine) or positional argv (Codex engine) | +| **Output schema flag** | `--json-schema ` | `--output-schema ` | same as engine | +| **Schema shape** | severity findings (`important`, `nit`, `pre_existing`) | priority findings (P0 through P3) | stops plus QA checklist | + +## Why the schema matters + +The schema flag is a terminal constraint, not a per-turn one. The agent reasons freely across N turns, reading files, grepping, running tests, and only the final assistant message is forced to deserialize against the schema. Everything upstream is unconstrained exploration. + +That's why this pattern works for review. You get agentic exploration (the whole point of using Claude Code or Codex over a raw LLM call), plus a machine-readable payload the UI can render without any scraping. + +## What you can tune per job + +From the **Agents** tab in the code-review UI, each provider exposes these settings: + +| Setting | Claude | Codex | Tour | +|---|---|---|---| +| Model | yes (`--model`) | yes (`-m`) | yes (per engine) | +| Reasoning effort | yes (`--effort`) | yes (`-c model_reasoning_effort=…`) | yes (per engine) | +| Fast mode | no | yes (`-c service_tier=fast`) | Codex engine only | + +None of these change the review prompt or user prompt. They only change how the underlying CLI executes the same user message. + +## Relationship to code review + +See [Code Review](/docs/commands/code-review/) for the end-to-end flow this feeds into. diff --git a/apps/pi-extension/server/agent-jobs.ts b/apps/pi-extension/server/agent-jobs.ts index 2ca1b1b8..4b327cb3 100644 --- a/apps/pi-extension/server/agent-jobs.ts +++ b/apps/pi-extension/server/agent-jobs.ts @@ -54,8 +54,8 @@ export interface AgentJobHandlerOptions { mode: "plan" | "review" | "annotate"; getServerUrl: () => string; getCwd: () => string; - /** Server-side command builder for known providers (codex, claude). */ - buildCommand?: (provider: string) => Promise<{ + /** Server-side command builder for known providers (codex, claude, tour). */ + buildCommand?: (provider: string, config?: Record) => Promise<{ command: string[]; outputPath?: string; captureStdout?: boolean; @@ -63,6 +63,16 @@ export interface AgentJobHandlerOptions { cwd?: string; prompt?: string; label?: string; + /** Underlying engine used (e.g., "claude" or "codex"). Stored on AgentJobInfo for UI display. */ + engine?: string; + /** Model used (e.g., "sonnet", "opus"). Stored on AgentJobInfo for UI display. */ + model?: string; + /** Claude --effort level. */ + effort?: string; + /** Codex reasoning effort level. */ + reasoningEffort?: string; + /** Whether Codex fast mode was enabled. */ + fastMode?: boolean; } | null>; /** Called when a job completes successfully — parse results and push annotations. */ onJobComplete?: (job: AgentJobInfo, meta: { outputPath?: string; stdout?: string; cwd?: string }) => void | Promise; @@ -81,6 +91,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) { const capabilities: AgentCapability[] = [ { id: "claude", name: "Claude Code", available: whichCmd("claude") }, { id: "codex", name: "Codex CLI", available: whichCmd("codex") }, + { id: "tour", name: "Code Tour", available: whichCmd("claude") || whichCmd("codex") }, ]; const capabilitiesResponse: AgentCapabilities = { mode, @@ -107,7 +118,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) { command: string[], label: string, outputPath?: string, - spawnOptions?: { captureStdout?: boolean; stdinPrompt?: string; cwd?: string; prompt?: string }, + spawnOptions?: { captureStdout?: boolean; stdinPrompt?: string; cwd?: string; prompt?: string; engine?: string; model?: string; effort?: string; reasoningEffort?: string; fastMode?: boolean }, ): AgentJobInfo { const id = crypto.randomUUID(); const source = jobSource(id); @@ -121,6 +132,11 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) { startedAt: Date.now(), command, cwd: getCwd(), + ...(spawnOptions?.engine && { engine: spawnOptions.engine }), + ...(spawnOptions?.model && { model: spawnOptions.model }), + ...(spawnOptions?.effort && { effort: spawnOptions.effort }), + ...(spawnOptions?.reasoningEffort && { reasoningEffort: spawnOptions.reasoningEffort }), + ...(spawnOptions?.fastMode && { fastMode: spawnOptions.fastMode }), }; let proc: ChildProcess | null = null; @@ -169,7 +185,8 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) { const lines = text.split('\n'); for (const line of lines) { if (!line.trim()) continue; - if (provider === "claude") { + // Tour jobs with the Claude engine also stream Claude JSONL. + if (provider === "claude" || spawnOptions?.engine === "claude") { const formatted = formatClaudeLogEvent(line); if (formatted !== null) { broadcast({ type: "job:log", jobId: id, delta: formatted + '\n' }); @@ -397,8 +414,20 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) { let stdinPrompt: string | undefined; let spawnCwd: string | undefined; let promptText: string | undefined; + let jobEngine: string | undefined; + let jobModel: string | undefined; + let jobEffort: string | undefined; + let jobReasoningEffort: string | undefined; + let jobFastMode: boolean | undefined; if (options.buildCommand) { - const built = await options.buildCommand(provider); + // Thread config from POST body to buildCommand + const config: Record = {}; + if (typeof body.engine === "string") config.engine = body.engine; + if (typeof body.model === "string") config.model = body.model; + if (typeof body.reasoningEffort === "string") config.reasoningEffort = body.reasoningEffort; + if (typeof body.effort === "string") config.effort = body.effort; + if (body.fastMode === true) config.fastMode = true; + const built = await options.buildCommand(provider, Object.keys(config).length > 0 ? config : undefined); if (built) { command = built.command; outputPath = built.outputPath; @@ -407,6 +436,11 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) { spawnCwd = built.cwd; promptText = built.prompt; if (built.label) label = built.label; + jobEngine = built.engine; + jobModel = built.model; + jobEffort = built.effort; + jobReasoningEffort = built.reasoningEffort; + jobFastMode = built.fastMode; } } @@ -420,6 +454,11 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) { stdinPrompt, cwd: spawnCwd, prompt: promptText, + engine: jobEngine, + model: jobModel, + effort: jobEffort, + reasoningEffort: jobReasoningEffort, + fastMode: jobFastMode, }); json(res, { job }, 201); } catch { diff --git a/apps/pi-extension/server/serverReview.ts b/apps/pi-extension/server/serverReview.ts index 6ffdf7d1..f5e5fc33 100644 --- a/apps/pi-extension/server/serverReview.ts +++ b/apps/pi-extension/server/serverReview.ts @@ -71,6 +71,7 @@ import { parseClaudeStreamOutput, transformClaudeFindings, } from "../generated/claude-review.js"; +import { createTourSession, TOUR_EMPTY_OUTPUT_ERROR } from "../generated/tour-review.js"; /** Detect if running inside WSL (Windows Subsystem for Linux) */ function detectWSL(): boolean { @@ -194,7 +195,6 @@ export async function startReviewServer(options: { // Agent jobs — background process manager (late-binds serverUrl via getter) let serverUrl = ""; - // Worktree-aware cwd resolver — shared by getCwd, buildCommand, and onJobComplete function resolveAgentCwd(): string { if (options.agentCwd) return options.agentCwd; if (currentDiffType.startsWith("worktree:")) { @@ -203,32 +203,47 @@ export async function startReviewServer(options: { } return options.gitContext?.cwd ?? process.cwd(); } + const tour = createTourSession(); + const agentJobs = createAgentJobHandler({ mode: "review", getServerUrl: () => serverUrl, getCwd: resolveAgentCwd, - async buildCommand(provider) { + async buildCommand(provider, config) { const cwd = resolveAgentCwd(); const hasAgentLocalAccess = !!options.agentCwd || !!options.gitContext; - const userMessage = buildCodexReviewUserMessage( - currentPatch, - currentDiffType, - { defaultBranch: options.gitContext?.defaultBranch, hasLocalAccess: hasAgentLocalAccess }, - options.prMetadata, - ); + const userMessageOptions = { defaultBranch: options.gitContext?.defaultBranch, hasLocalAccess: hasAgentLocalAccess }; + + if (provider === "tour") { + return tour.buildCommand({ + cwd, + patch: currentPatch, + diffType: currentDiffType, + options: userMessageOptions, + prMetadata: options.prMetadata, + config, + }); + } + + const userMessage = buildCodexReviewUserMessage(currentPatch, currentDiffType, userMessageOptions, options.prMetadata); if (provider === "codex") { + const model = typeof config?.model === "string" && config.model ? config.model : undefined; + const reasoningEffort = typeof config?.reasoningEffort === "string" && config.reasoningEffort ? config.reasoningEffort : undefined; + const fastMode = config?.fastMode === true; const outputPath = generateOutputPath(); const prompt = CODEX_REVIEW_SYSTEM_PROMPT + "\n\n---\n\n" + userMessage; - const command = await buildCodexCommand({ cwd, outputPath, prompt }); - return { command, outputPath, prompt, label: "Codex Review" }; + const command = await buildCodexCommand({ cwd, outputPath, prompt, model, reasoningEffort, fastMode }); + return { command, outputPath, prompt, label: "Code Review", model, reasoningEffort, fastMode: fastMode || undefined }; } if (provider === "claude") { + const model = typeof config?.model === "string" && config.model ? config.model : undefined; + const effort = typeof config?.effort === "string" && config.effort ? config.effort : undefined; const prompt = CLAUDE_REVIEW_PROMPT + "\n\n---\n\n" + userMessage; - const { command, stdinPrompt } = buildClaudeCommand(prompt); - return { command, stdinPrompt, prompt, cwd, label: "Claude Code Review", captureStdout: true }; + const { command, stdinPrompt } = buildClaudeCommand(prompt, model, effort); + return { command, stdinPrompt, prompt, cwd, label: "Code Review", captureStdout: true, model, effort }; } return null; @@ -243,7 +258,7 @@ export async function startReviewServer(options: { // Override verdict if there are blocking findings (P0/P1) — Codex's // freeform correctness string can say "mostly correct" with real bugs. - const hasBlockingFindings = output.findings.some((f: any) => f.priority !== null && f.priority <= 1); + const hasBlockingFindings = output.findings.some(f => f.priority !== null && f.priority <= 1); job.summary = { correctness: hasBlockingFindings ? "Issues Found" : output.overall_correctness, explanation: output.overall_explanation, @@ -260,7 +275,10 @@ export async function startReviewServer(options: { if (job.provider === "claude" && meta.stdout) { const output = parseClaudeStreamOutput(meta.stdout); - if (!output) return; + if (!output) { + console.error(`[claude-review] Failed to parse output (${meta.stdout.length} bytes, last 200: ${meta.stdout.slice(-200)})`); + return; + } const total = output.summary.important + output.summary.nit + output.summary.pre_existing; job.summary = { @@ -276,6 +294,20 @@ export async function startReviewServer(options: { } return; } + + if (job.provider === "tour") { + const { summary } = await tour.onJobComplete({ job, meta }); + if (summary) { + job.summary = summary; + } else { + // The process exited 0 but the model returned empty or malformed output + // and nothing was stored. Flip status so the client doesn't auto-open + // a successful-looking card that 404s on /api/tour/:id. + job.status = "failed"; + job.error = TOUR_EMPTY_OUTPUT_ERROR; + } + return; + } }, }); const sharingEnabled = @@ -415,6 +447,32 @@ export async function startReviewServer(options: { const server = createServer(async (req, res) => { const url = requestUrl(req); + // API: Get tour result + if (url.pathname.match(/^\/api\/tour\/[^/]+$/) && req.method === "GET") { + const jobId = url.pathname.slice("/api/tour/".length); + const result = tour.getTour(jobId); + if (!result) { + json(res, { error: "Tour not found" }, 404); + return; + } + json(res, result); + return; + } + + // API: Save tour checklist state + const checklistMatch = url.pathname.match(/^\/api\/tour\/([^/]+)\/checklist$/); + if (checklistMatch && req.method === "PUT") { + const jobId = checklistMatch[1]; + try { + const body = await parseBody(req) as { checked: boolean[] }; + if (Array.isArray(body.checked)) tour.saveChecklist(jobId, body.checked); + json(res, { ok: true }); + } catch { + json(res, { error: "Invalid JSON" }, 400); + } + return; + } + if (url.pathname === "/api/diff" && req.method === "GET") { json(res, { rawPatch: currentPatch, diff --git a/apps/pi-extension/vendor.sh b/apps/pi-extension/vendor.sh index e7dfce03..51dbc426 100755 --- a/apps/pi-extension/vendor.sh +++ b/apps/pi-extension/vendor.sh @@ -6,7 +6,7 @@ cd "$(dirname "$0")" mkdir -p generated generated/ai/providers -for f in feedback-templates review-core storage draft project pr-provider pr-github pr-gitlab checklist integrations-common repo reference-common favicon resolve-file config external-annotation agent-jobs worktree html-to-markdown url-to-markdown; do +for f in feedback-templates review-core storage draft project pr-provider pr-github pr-gitlab checklist integrations-common repo reference-common favicon resolve-file config external-annotation agent-jobs worktree html-to-markdown url-to-markdown tour; do src="../../packages/shared/$f.ts" printf '// @generated — DO NOT EDIT. Source: packages/shared/%s.ts\n' "$f" | cat - "$src" > "generated/$f.ts" done @@ -21,6 +21,17 @@ for f in codex-review claude-review path-utils; do > "generated/$f.ts" done +# tour-review lives in packages/server/tour/ — parent-relative imports and the +# shared tour types package each map to the flat generated/ layout. +for f in tour-review; do + src="../../packages/server/tour/$f.ts" + printf '// @generated — DO NOT EDIT. Source: packages/server/tour/%s.ts\n' "$f" | cat - "$src" \ + | sed 's|from "\.\./vcs"|from "./review-core.js"|' \ + | sed 's|from "\.\./pr"|from "./pr-provider.js"|' \ + | sed 's|from "@plannotator/shared/tour"|from "./tour.js"|' \ + > "generated/$f.ts" +done + for f in index types provider session-manager endpoints context base-session; do src="../../packages/ai/$f.ts" printf '// @generated — DO NOT EDIT. Source: packages/ai/%s.ts\n' "$f" | cat - "$src" > "generated/ai/$f.ts" diff --git a/bun.lock b/bun.lock index e6b79b0f..440a0800 100644 --- a/bun.lock +++ b/bun.lock @@ -64,7 +64,7 @@ }, "apps/opencode-plugin": { "name": "@plannotator/opencode", - "version": "0.17.10", + "version": "0.18.0", "dependencies": { "@opencode-ai/plugin": "^1.1.10", }, @@ -86,7 +86,7 @@ }, "apps/pi-extension": { "name": "@plannotator/pi-extension", - "version": "0.17.10", + "version": "0.18.0", "dependencies": { "@joplin/turndown-plugin-gfm": "^1.0.64", "turndown": "^7.2.4", @@ -174,7 +174,11 @@ "@pierre/diffs": "^1.1.12", "@plannotator/shared": "workspace:*", "@plannotator/ui": "workspace:*", + "@radix-ui/react-checkbox": "^1.3.3", + "@radix-ui/react-collapsible": "^1.1.12", + "@radix-ui/react-tooltip": "^1.2.8", "highlight.js": "^11.11.1", + "motion": "^12.38.0", "react": "^19.2.3", "react-dom": "^19.2.3", "tailwindcss": "^4.1.18", @@ -182,7 +186,7 @@ }, "packages/server": { "name": "@plannotator/server", - "version": "0.17.10", + "version": "0.18.0", "dependencies": { "@plannotator/ai": "workspace:*", "@plannotator/shared": "workspace:*", @@ -702,6 +706,10 @@ "@radix-ui/react-arrow": ["@radix-ui/react-arrow@1.1.7", "", { "dependencies": { "@radix-ui/react-primitive": "2.1.3" }, "peerDependencies": { "@types/react": "*", "@types/react-dom": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react", "@types/react-dom"] }, "sha512-F+M1tLhO+mlQaOWspE8Wstg+z6PwxwRd8oQ8IXceWz92kfAmalTRf0EjrouQeo7QssEPfCn05B4Ihs1K9WQ/7w=="], + "@radix-ui/react-checkbox": ["@radix-ui/react-checkbox@1.3.3", "", { "dependencies": { "@radix-ui/primitive": "1.1.3", "@radix-ui/react-compose-refs": "1.1.2", "@radix-ui/react-context": "1.1.2", "@radix-ui/react-presence": "1.1.5", "@radix-ui/react-primitive": "2.1.3", "@radix-ui/react-use-controllable-state": "1.2.2", "@radix-ui/react-use-previous": "1.1.1", "@radix-ui/react-use-size": "1.1.1" }, "peerDependencies": { "@types/react": "*", "@types/react-dom": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react", "@types/react-dom"] }, "sha512-wBbpv+NQftHDdG86Qc0pIyXk5IR3tM8Vd0nWLKDcX8nNn4nXFOFwsKuqw2okA/1D/mpaAkmuyndrPJTYDNZtFw=="], + + "@radix-ui/react-collapsible": ["@radix-ui/react-collapsible@1.1.12", "", { "dependencies": { "@radix-ui/primitive": "1.1.3", "@radix-ui/react-compose-refs": "1.1.2", "@radix-ui/react-context": "1.1.2", "@radix-ui/react-id": "1.1.1", "@radix-ui/react-presence": "1.1.5", "@radix-ui/react-primitive": "2.1.3", "@radix-ui/react-use-controllable-state": "1.2.2", "@radix-ui/react-use-layout-effect": "1.1.1" }, "peerDependencies": { "@types/react": "*", "@types/react-dom": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react", "@types/react-dom"] }, "sha512-Uu+mSh4agx2ib1uIGPP4/CKNULyajb3p92LsVXmH2EHVMTfZWpll88XJ0j4W0z3f8NK1eYl1+Mf/szHPmcHzyA=="], + "@radix-ui/react-compose-refs": ["@radix-ui/react-compose-refs@1.1.2", "", { "peerDependencies": { "@types/react": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react"] }, "sha512-z4eqJvfiNnFMHIIvXP3CY57y2WJs5g2v3X0zm9mEJkrkNv4rDxu+sg9Jh8EkXyeqBkB7SOcboo9dMVqhyrACIg=="], "@radix-ui/react-context": ["@radix-ui/react-context@1.1.2", "", { "peerDependencies": { "@types/react": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react"] }, "sha512-jCi/QKUM2r1Ju5a3J64TH2A5SpKAgh0LpknyqdQ4m6DCV0xJ2HG1xARRwNGPQfi1SLdLWZ1OJz6F4OMBBNiGJA=="], @@ -732,6 +740,8 @@ "@radix-ui/react-use-layout-effect": ["@radix-ui/react-use-layout-effect@1.1.1", "", { "peerDependencies": { "@types/react": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react"] }, "sha512-RbJRS4UWQFkzHTTwVymMTUv8EqYhOp8dOOviLj2ugtTiXRaRQS7GLGxZTLL1jWhMeoSCf5zmcZkqTl9IiYfXcQ=="], + "@radix-ui/react-use-previous": ["@radix-ui/react-use-previous@1.1.1", "", { "peerDependencies": { "@types/react": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react"] }, "sha512-2dHfToCj/pzca2Ck724OZ5L0EVrr3eHRNsG/b3xQJLA2hZpVCS99bLAX+hm1IHXDEnzU6by5z/5MIY794/a8NQ=="], + "@radix-ui/react-use-rect": ["@radix-ui/react-use-rect@1.1.1", "", { "dependencies": { "@radix-ui/rect": "1.1.1" }, "peerDependencies": { "@types/react": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react"] }, "sha512-QTYuDesS0VtuHNNvMh+CjlKJ4LJickCMUAqjlE3+j8w+RlRpwyX3apEQKGFzbZGdo7XNG1tXa+bQqIE7HIXT2w=="], "@radix-ui/react-use-size": ["@radix-ui/react-use-size@1.1.1", "", { "dependencies": { "@radix-ui/react-use-layout-effect": "1.1.1" }, "peerDependencies": { "@types/react": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react"] }, "sha512-ewrXRDTAqAXlkl6t/fkXWNAhFX9I+CkKlw6zjEwk86RSPKwZr3xpBRso655aqYafwtnbpHLj6toFzmd6xdVptQ=="], @@ -1574,6 +1584,8 @@ "forwarded": ["forwarded@0.2.0", "", {}, "sha512-buRG0fpBtRHSTCOASe6hD258tEubFoRLb4ZNA6NxMVHNw2gOcwHo9wyablzMzOA5z9xA9L1KNjk/Nt6MT9aYow=="], + "framer-motion": ["framer-motion@12.38.0", "", { "dependencies": { "motion-dom": "^12.38.0", "motion-utils": "^12.36.0", "tslib": "^2.4.0" }, "peerDependencies": { "@emotion/is-prop-valid": "*", "react": "^18.0.0 || ^19.0.0", "react-dom": "^18.0.0 || ^19.0.0" }, "optionalPeers": ["@emotion/is-prop-valid", "react", "react-dom"] }, "sha512-rFYkY/pigbcswl1XQSb7q424kSTQ8q6eAC+YUsSKooHQYuLdzdHjrt6uxUC+PRAO++q5IS7+TamgIw1AphxR+g=="], + "fresh": ["fresh@2.0.0", "", {}, "sha512-Rx/WycZ60HOaqLKAi6cHRKKI7zxWbJ31MhntmtwMoaTeF7XFH9hhBp8vITaMidfljRQ6eYWCKkaTK+ykVJHP2A=="], "fs-constants": ["fs-constants@1.0.0", "", {}, "sha512-y6OAwoSIf7FyjMIv94u+b5rdheZEjzR63GTyZJm5qh4Bi+2YgwLCcI/fPFZkL5PSixOt6ZNKm+w+Hfp/Bciwow=="], @@ -1990,6 +2002,12 @@ "mlly": ["mlly@1.8.0", "", { "dependencies": { "acorn": "^8.15.0", "pathe": "^2.0.3", "pkg-types": "^1.3.1", "ufo": "^1.6.1" } }, "sha512-l8D9ODSRWLe2KHJSifWGwBqpTZXIXTeo8mlKjY+E2HAakaTeNpqAyBZ8GSqLzHgw4XmHmC8whvpjJNMbFZN7/g=="], + "motion": ["motion@12.38.0", "", { "dependencies": { "framer-motion": "^12.38.0", "tslib": "^2.4.0" }, "peerDependencies": { "@emotion/is-prop-valid": "*", "react": "^18.0.0 || ^19.0.0", "react-dom": "^18.0.0 || ^19.0.0" }, "optionalPeers": ["@emotion/is-prop-valid", "react", "react-dom"] }, "sha512-uYfXzeHlgThchzwz5Te47dlv5JOUC7OB4rjJ/7XTUgtBZD8CchMN8qEJ4ZVsUmTyYA44zjV0fBwsiktRuFnn+w=="], + + "motion-dom": ["motion-dom@12.38.0", "", { "dependencies": { "motion-utils": "^12.36.0" } }, "sha512-pdkHLD8QYRp8VfiNLb8xIBJis1byQ9gPT3Jnh2jqfFtAsWUA3dEepDlsWe/xMpO8McV+VdpKVcp+E+TGJEtOoA=="], + + "motion-utils": ["motion-utils@12.36.0", "", {}, "sha512-eHWisygbiwVvf6PZ1vhaHCLamvkSbPIeAYxWUuL3a2PD/TROgE7FvfHWTIH4vMl798QLfMw15nRqIaRDXTlYRg=="], + "mrmime": ["mrmime@2.0.1", "", {}, "sha512-Y3wQdFg2Va6etvQ5I82yUhGdsKrcYox6p7FfL1LbK2J4V01F9TGlepTIhnK24t7koZibmg82KGglhA1XK5IsLQ=="], "ms": ["ms@2.1.3", "", {}, "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA=="], diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index 86205844..bfa17f80 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -59,6 +59,8 @@ import type { DiffFile } from './types'; import type { DiffOption, WorktreeInfo, GitContext } from '@plannotator/shared/types'; import type { PRMetadata } from '@plannotator/shared/pr-provider'; import { altKey } from '@plannotator/ui/utils/platform'; +import { TourDialog } from './components/tour/TourDialog'; +import { DEMO_TOUR_ID } from './demoTour'; declare const __APP_VERSION__: string; @@ -224,6 +226,9 @@ const ReviewApp: React.FC = () => { const { externalAnnotations, updateExternalAnnotation, deleteExternalAnnotation } = useExternalAnnotations({ enabled: !!origin }); const agentJobs = useAgentJobs({ enabled: !!origin }); + // Tour dialog state — opens as an overlay instead of a dock panel + const [tourDialogJobId, setTourDialogJobId] = useState(null); + // Dockview center panel API for the review workspace. const [dockApi, setDockApi] = useState(null); const filesRef = useRef(files); @@ -547,6 +552,39 @@ const ReviewApp: React.FC = () => { }); }, [dockApi, agentJobs.jobs]); + // Open tour as a dialog overlay + const handleOpenTour = useCallback((jobId: string) => { + setTourDialogJobId(jobId); + }, []); + + // Dev-only: Cmd/Ctrl+Shift+T toggles the demo tour for fast UI iteration. + useEffect(() => { + if (!import.meta.env.DEV) return; + const handler = (e: KeyboardEvent) => { + if ((e.metaKey || e.ctrlKey) && e.shiftKey && (e.key === 'T' || e.key === 't')) { + e.preventDefault(); + setTourDialogJobId(prev => (prev === DEMO_TOUR_ID ? null : DEMO_TOUR_ID)); + } + }; + window.addEventListener('keydown', handler); + return () => window.removeEventListener('keydown', handler); + }, []); + + // Auto-open tour dialog when a tour job completes + const tourAutoOpenRef = useRef(new Set()); + useEffect(() => { + for (const job of agentJobs.jobs) { + if ( + job.provider === 'tour' && + job.status === 'done' && + !tourAutoOpenRef.current.has(job.id) + ) { + tourAutoOpenRef.current.add(job.id); + setTourDialogJobId(job.id); + } + } + }, [agentJobs.jobs]); + // Open PR panel as center dock panel const handleOpenPRPanel = useCallback((type: 'summary' | 'comments' | 'checks') => { const api = dockApi; @@ -1016,6 +1054,7 @@ const ReviewApp: React.FC = () => { fetchPRContext, platformUser, openDiffFile, + openTourPanel: handleOpenTour, }), [ files, activeFileIndex, diffStyle, diffOverflow, diffIndicators, diffLineDiffType, diffShowLineNumbers, diffShowBackground, @@ -1030,6 +1069,7 @@ const ReviewApp: React.FC = () => { handleAskAI, handleViewAIResponse, handleClickAIMarker, aiHistoryForSelection, agentJobs.jobs, prMetadata, prContext, isPRContextLoading, prContextError, fetchPRContext, platformUser, openDiffFile, + handleOpenTour, ]); // Separate context for high-frequency job logs — prevents re-rendering all panels on every SSE event @@ -2020,6 +2060,22 @@ const ReviewApp: React.FC = () => { )} + + {/* Tour dialog overlay */} + setTourDialogJobId(null)} /> + + {/* Dev-only: open a fully-formed demo tour without running the agent. + Stripped from production builds via import.meta.env.DEV. */} + {import.meta.env.DEV && ( + + )} + diff --git a/packages/review-editor/components/DiffHunkPreview.tsx b/packages/review-editor/components/DiffHunkPreview.tsx index 5212bd90..e2bf95f2 100644 --- a/packages/review-editor/components/DiffHunkPreview.tsx +++ b/packages/review-editor/components/DiffHunkPreview.tsx @@ -12,6 +12,43 @@ interface DiffHunkPreviewProps { className?: string; } +/** + * Build the unsafeCSS string for @pierre/diffs by reading computed CSS variables. + * Called synchronously so the first render is already themed (no flash on tooltip open). + */ +function buildPierreCSS(mode: 'dark' | 'light', fontFamily: string, fontSize: string): string { + try { + const styles = getComputedStyle(document.documentElement); + const bg = styles.getPropertyValue('--background').trim(); + const fg = styles.getPropertyValue('--foreground').trim(); + if (!bg || !fg) return ''; + + const fontCSS = (fontFamily || fontSize) ? ` + pre, code, [data-line-content], [data-column-number] { + ${fontFamily ? `font-family: '${fontFamily}', monospace !important;` : ''} + ${fontSize ? `font-size: ${fontSize} !important; line-height: 1.5 !important;` : ''} + }` : ''; + + return ` + :host, [data-diff], [data-file], [data-diffs-header], [data-error-wrapper], [data-virtualizer-buffer] { + --diffs-bg: ${bg} !important; + --diffs-fg: ${fg} !important; + --diffs-dark-bg: ${bg}; + --diffs-light-bg: ${bg}; + --diffs-dark: ${fg}; + --diffs-light: ${fg}; + } + pre, code { background-color: ${bg} !important; } + [data-column-number] { background-color: ${bg} !important; } + [data-file-info] { display: none !important; } + [data-diffs-header] { display: none !important; } + ${fontCSS} + `; + } catch { + return ''; + } +} + /** * Renders a small inline diff hunk using @pierre/diffs. * Compact, read-only, no file header. Shares theme + font settings @@ -29,60 +66,46 @@ export const DiffHunkPreview: React.FC = ({ const fileDiff = useMemo(() => { if (!hunk) return undefined; try { - const needsHeaders = !hunk.startsWith('diff --git') && !hunk.startsWith('--- '); - const patch = needsHeaders - ? `diff --git a/file b/file\n--- a/file\n+++ b/file\n${hunk}` - : hunk; + // Robustly handle all three hunk formats the tour agent might produce: + // 1. Full git diff: starts with "diff --git" — use as-is + // 2. File-level diff: starts with "--- " — prepend "diff --git" line only + // 3. Bare hunk: starts with "@@ " — prepend full synthetic headers + const patch = hunk.startsWith('diff --git') + ? hunk + : hunk.startsWith('--- ') + ? `diff --git a/file b/file\n${hunk}` + : `diff --git a/file b/file\n--- a/file\n+++ b/file\n${hunk}`; return getSingularPatch(patch); } catch { return undefined; } }, [hunk]); - // Theme injection — same pattern as DiffViewer (reads computed CSS vars, injects via unsafeCSS) - const [pierreTheme, setPierreTheme] = useState<{ type: 'dark' | 'light'; css: string }>({ - type: resolvedMode, - css: '', - }); + // Initialize synchronously so the very first render (inside a tooltip) is already themed. + // The lazy initializer reads computed CSS variables from the document root. + const [pierreTheme, setPierreTheme] = useState<{ type: 'dark' | 'light'; css: string }>(() => ({ + type: resolvedMode ?? 'dark', + css: buildPierreCSS(resolvedMode ?? 'dark', state.fontFamily, state.fontSize), + })); + // Re-compute on theme / font changes useEffect(() => { const rafId = requestAnimationFrame(() => { - const styles = getComputedStyle(document.documentElement); - const bg = styles.getPropertyValue('--background').trim(); - const fg = styles.getPropertyValue('--foreground').trim(); - if (!bg || !fg) return; - - const fontFamily = state.fontFamily; - const fontSize = state.fontSize; - const fontCSS = fontFamily || fontSize ? ` - pre, code, [data-line-content], [data-column-number] { - ${fontFamily ? `font-family: '${fontFamily}', monospace !important;` : ''} - ${fontSize ? `font-size: ${fontSize} !important; line-height: 1.5 !important;` : ''} - }` : ''; - setPierreTheme({ - type: resolvedMode, - css: ` - :host, [data-diff], [data-file], [data-diffs-header], [data-error-wrapper], [data-virtualizer-buffer] { - --diffs-bg: ${bg} !important; - --diffs-fg: ${fg} !important; - --diffs-dark-bg: ${bg}; - --diffs-light-bg: ${bg}; - --diffs-dark: ${fg}; - --diffs-light: ${fg}; - } - pre, code { background-color: ${bg} !important; } - [data-column-number] { background-color: ${bg} !important; } - [data-file-info] { display: none !important; } - [data-diffs-header] { display: none !important; } - ${fontCSS} - `, + type: resolvedMode ?? 'dark', + css: buildPierreCSS(resolvedMode ?? 'dark', state.fontFamily, state.fontSize), }); }); return () => cancelAnimationFrame(rafId); }, [resolvedMode, state.fontFamily, state.fontSize]); - if (!fileDiff) return null; + if (!fileDiff) { + return ( +
+ Diff not available +
+ ); + } return (
@@ -97,8 +120,6 @@ export const DiffHunkPreview: React.FC = ({ unsafeCSS: pierreTheme.css, diffStyle: 'unified', disableLineNumbers: true, - disableFileHeader: true, - disableBackground: true, overflow: 'wrap', }} /> diff --git a/packages/review-editor/components/ReviewSidebar.tsx b/packages/review-editor/components/ReviewSidebar.tsx index 19362c9c..d923bf34 100644 --- a/packages/review-editor/components/ReviewSidebar.tsx +++ b/packages/review-editor/components/ReviewSidebar.tsx @@ -55,7 +55,7 @@ interface ReviewSidebarProps { // Agent props agentJobs?: AgentJobInfo[]; agentCapabilities?: AgentCapabilities | null; - onAgentLaunch?: (params: { provider?: string; command?: string[]; label?: string }) => void; + onAgentLaunch?: (params: { provider?: string; command?: string[]; label?: string; engine?: string; model?: string; reasoningEffort?: string; effort?: string; fastMode?: boolean }) => void; onAgentKillJob?: (id: string) => void; onAgentKillAll?: () => void; externalAnnotations?: Array<{ source?: string }>; diff --git a/packages/review-editor/components/tour/QAChecklist.tsx b/packages/review-editor/components/tour/QAChecklist.tsx new file mode 100644 index 00000000..e1d61c95 --- /dev/null +++ b/packages/review-editor/components/tour/QAChecklist.tsx @@ -0,0 +1,89 @@ +import React from 'react'; +import { motion } from 'motion/react'; +import * as Checkbox from '@radix-ui/react-checkbox'; +import type { TourQAItem, TourStop } from '../../hooks/tour/useTourData'; + +interface QAChecklistProps { + items: TourQAItem[]; + stops: TourStop[]; + checked: boolean[]; + onToggle: (index: number) => void; + onScrollToStop?: (index: number) => void; +} + +export const QAChecklist: React.FC = ({ + items, + checked, + onToggle, + onScrollToStop, +}) => { + return ( + + {items.map((item, i) => ( + + onToggle(i)} + className="tour-checkbox mt-0.5 w-[18px] h-[18px] rounded-[4px] flex-shrink-0 border border-foreground/25 dark:border-border/50 bg-muted/40 dark:bg-background shadow-[inset_0_0_0_1px_rgba(0,0,0,0.02)] dark:shadow-none data-[state=checked]:bg-primary data-[state=checked]:border-primary data-[state=checked]:shadow-none flex items-center justify-center" + > + + + + + + + +
+ + {item.question} + + {item.stop_indices.length > 0 && ( + + {item.stop_indices.map((si, j) => ( + + {j > 0 && ', '} + + + ))} + + )} +
+
+ ))} +
+ ); +}; diff --git a/packages/review-editor/components/tour/TourDialog.tsx b/packages/review-editor/components/tour/TourDialog.tsx new file mode 100644 index 00000000..b104a75f --- /dev/null +++ b/packages/review-editor/components/tour/TourDialog.tsx @@ -0,0 +1,561 @@ +import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { motion, MotionConfig, type Variants } from 'motion/react'; +import { OverlayScrollArea } from '@plannotator/ui/components/OverlayScrollArea'; +import { useTourData } from '../../hooks/tour/useTourData'; +import { useReviewState } from '../../dock/ReviewStateContext'; +import { TourStopCard } from './TourStopCard'; +import { QAChecklist } from './QAChecklist'; +import type { TourKeyTakeaway } from '../../hooks/tour/useTourData'; + +const prefersReducedMotion = () => + typeof window !== 'undefined' && !!window.matchMedia?.('(prefers-reduced-motion: reduce)').matches; + +// --------------------------------------------------------------------------- +// Intro composition cascade — each piece springs into place after the dialog +// card lands, so the landing page composes itself rather than appearing all +// at once. Spring physics give natural settle, no cartoony bounce. +// --------------------------------------------------------------------------- + +const introContainerVariants: Variants = { + hidden: {}, + visible: { transition: { staggerChildren: 0.06, delayChildren: 0.12 } }, +}; + +const introItemVariants: Variants = { + hidden: { opacity: 0, y: 8 }, + visible: { + opacity: 1, + y: 0, + transition: { type: 'spring', stiffness: 220, damping: 24 }, + }, +}; + +const takeawayListVariants: Variants = { + hidden: {}, + visible: { transition: { staggerChildren: 0.04, delayChildren: 0.05 } }, +}; + +const takeawayRowVariants: Variants = { + hidden: { opacity: 0, y: 4 }, + visible: { + opacity: 1, + y: 0, + transition: { type: 'spring', stiffness: 240, damping: 26 }, + }, +}; + +const startButtonVariants: Variants = { + hidden: { opacity: 0, scale: 0.96, y: 6 }, + visible: { + opacity: 1, + scale: 1, + y: 0, + transition: { type: 'spring', stiffness: 260, damping: 18 }, + }, +}; + +// One-shot "spark of life" — color dot pulses once when its card lands. +const dotVariants: Variants = { + hidden: { scale: 1 }, + visible: { + scale: [1, 1.35, 1], + transition: { duration: 0.45, times: [0, 0.5, 1], ease: 'easeOut' }, + }, +}; + +// --------------------------------------------------------------------------- +// Key takeaways table +// --------------------------------------------------------------------------- + +const severityLabel: Record = { + info: 'Info', + important: 'Important', + warning: 'Warning', +}; + +const severityLabelClass: Record = { + info: 'text-muted-foreground/70', + important: 'text-primary/80 dark:text-primary', + warning: 'text-warning/80 dark:text-warning', +}; + +const severityRowClass: Record = { + info: '', + important: 'bg-primary/[0.03] dark:bg-primary/[0.10]', + warning: 'bg-warning/[0.03] dark:bg-warning/[0.10]', +}; + +function TakeawaysTable({ takeaways }: { takeaways: TourKeyTakeaway[] }) { + if (takeaways.length === 0) return null; + return ( + +
+ + Key takeaways + +
+ {takeaways.map((t, i) => ( + 0 ? 'border-t border-border/10 dark:border-border/30' : '' + }`} + > + + {severityLabel[t.severity]} + + {t.text} + + ))} +
+ ); +} + +// --------------------------------------------------------------------------- +// Page types + animation helpers +// --------------------------------------------------------------------------- + +type TourPage = 'intro' | 'stops' | 'checklist'; +const PAGE_ORDER: TourPage[] = ['intro', 'stops', 'checklist']; +const PAGE_LABELS: Record = { + intro: 'Overview', + stops: 'Walkthrough', + checklist: 'Checklist', +}; + +function getPageAnimClass( + thisPage: TourPage, + currentPage: TourPage, + exitingPage: TourPage | null, + slideDir: 'fwd' | 'bwd', +): string { + if (exitingPage === thisPage) { + if (thisPage === 'intro') return 'tour-intro-exit'; + return slideDir === 'fwd' ? 'tour-page-exit-fwd' : 'tour-page-exit-bwd'; + } + if (currentPage === thisPage && exitingPage !== null) { + if (thisPage === 'stops' && exitingPage === 'intro') return ''; + return slideDir === 'fwd' ? 'tour-page-enter-fwd' : 'tour-page-enter-bwd'; + } + return ''; +} + +// --------------------------------------------------------------------------- +// Dialog wrapper +// --------------------------------------------------------------------------- + +interface TourDialogProps { + jobId: string | null; + onClose: () => void; +} + +export const TourDialog: React.FC = ({ jobId, onClose }) => { + // Keep the last known jobId mounted while we play the exit animation + const [renderedJobId, setRenderedJobId] = useState(jobId); + const [closing, setClosing] = useState(false); + + useEffect(() => { + if (jobId) { + setRenderedJobId(jobId); + setClosing(false); + } else if (renderedJobId) { + // If reduced motion is on, the exit animation is suppressed and + // onAnimationEnd will never fire — unmount immediately instead. + if (prefersReducedMotion()) { + setRenderedJobId(null); + setClosing(false); + } else { + setClosing(true); + } + } + }, [jobId, renderedJobId]); + + const requestClose = useCallback(() => { onClose(); }, [onClose]); + + // Escape to close + useEffect(() => { + if (!renderedJobId || closing) return; + const handler = (e: KeyboardEvent) => { + if (e.key === 'Escape') { e.stopPropagation(); requestClose(); } + }; + window.addEventListener('keydown', handler, true); + return () => window.removeEventListener('keydown', handler, true); + }, [renderedJobId, closing, requestClose]); + + const handleExitEnd = useCallback((e: React.AnimationEvent) => { + if (e.target !== e.currentTarget) return; + setRenderedJobId(null); + setClosing(false); + }, []); + + if (!renderedJobId) return null; + + return ( + +
+ {/* Backdrop */} +
+ + {/* Dialog card */} +
e.stopPropagation()} + onAnimationEnd={closing ? handleExitEnd : undefined} + > + +
+
+ + ); +}; + +// --------------------------------------------------------------------------- +// Dialog content — 3 navigable pages +// --------------------------------------------------------------------------- + +function TourDialogContent({ jobId, onClose }: { jobId: string; onClose: () => void }) { + const state = useReviewState(); + const { tour, loading, error, checked, toggleChecked, retry } = useTourData(jobId); + const stopsRef = useRef<(HTMLDivElement | null)[]>([]); + const stopsSeenRef = useRef(false); + const walkthroughAutoOpenTriggered = useRef(false); + const [openStops, setOpenStops] = useState>(() => new Set()); + + // Single-open accordion: opening a new stop closes any other that was open. + // Clicking the currently-open stop closes it. + const toggleStop = useCallback((i: number) => { + setOpenStops((prev) => { + if (prev.has(i)) return new Set(); + return new Set([i]); + }); + }, []); + + const [page, setPage] = useState('intro'); + const [exitingPage, setExitingPage] = useState(null); + const [slideDir, setSlideDir] = useState<'fwd' | 'bwd'>('fwd'); + + const navigate = useCallback((next: TourPage) => { + if (exitingPage || next === page) return; + // Reduced motion suppresses the page animations, so onAnimationEnd + // never fires to clear exitingPage — swap immediately instead. + if (prefersReducedMotion()) { + setPage(next); + return; + } + const fromIdx = PAGE_ORDER.indexOf(page); + const toIdx = PAGE_ORDER.indexOf(next); + setExitingPage(page); + setSlideDir(toIdx > fromIdx ? 'fwd' : 'bwd'); + setPage(next); + }, [page, exitingPage]); + + const handleSlideEnd = useCallback((exiting: TourPage) => { + setExitingPage((prev) => (prev === exiting ? null : prev)); + }, []); + + const handleAnchorClick = useCallback((filePath: string) => { + state.openDiffFile(filePath); + onClose(); + }, [state.openDiffFile, onClose]); + + const handleScrollToStop = useCallback((index: number) => { + const el = stopsRef.current[index]; + if (el) el.scrollIntoView({ behavior: 'smooth', block: 'start' }); + }, []); + + // Mark each page as "seen" after first mount. Gated on `tour` so it doesn't + // flip the refs during the loading render (which would strip the first-time + // animation classes before the page ever rendered). + useEffect(() => { + if (!tour) return; + if (page === 'stops' || exitingPage === 'stops') stopsSeenRef.current = true; + }, [page, exitingPage, tour]); + + // First-visit auto-open: when the user lands on the walkthrough page for the + // first time, wait for the stops cascade to finish + a brief breath, then + // pop the first accordion. The card itself plays its motion accordion + // animation in response, so this slots into the natural reveal beat. + useEffect(() => { + if (!tour) return; + if (page !== 'stops') return; + if (walkthroughAutoOpenTriggered.current) return; + walkthroughAutoOpenTriggered.current = true; + + const stopRevealMs = 280; // .tour-stop-reveal duration + const stopStaggerMs = 60; // delay between stops + const lastStopFinishesAt = stopRevealMs + (tour.stops.length - 1) * stopStaggerMs; + const breath = 500; + + const t = setTimeout(() => setOpenStops(new Set([0])), lastStopFinishesAt + breath); + return () => clearTimeout(t); + }, [page, tour]); + + // All hooks must run before the early returns below (rules of hooks). + // `tour` may still be null during loading, so access it safely — the + // filter strips the empty values out anyway. + const introCards = useMemo( + () => + [ + { label: 'Intent', value: tour?.intent, dot: 'bg-primary/70' }, + { label: 'Before', value: tour?.before, dot: 'bg-warning/70' }, + { label: 'After', value: tour?.after, dot: 'bg-success/70' }, + ].filter((card) => card.value && card.value.trim()), + [tour?.intent, tour?.before, tour?.after], + ); + + // Loading + if (loading) { + return ( + <> +
+
+
+
+
+
+
+
+
+
+
+
+
+
+ + ); + } + + // Error + if (error || !tour) { + return ( +
+
+

{error ?? 'Tour not found'}

+ +
+
+ ); + } + + const verifiedCount = checked.filter(Boolean).length; + const checklistCount = tour.qa_checklist.length; + const isActive = (p: TourPage) => page === p && !exitingPage; + + // Mark pages as "seen" the first time we mount them so we don't re-animate + // the section heading + staggered cards on every back-and-forth navigation. + const stopsAlreadySeen = stopsSeenRef.current; + + return ( + <> + {/* Title bar + close */} +
+

+ {tour.title} +

+ +
+ + {/* Page nav tabs */} +
+ {PAGE_ORDER.map((p) => ( + + ))} +
+ + {/* Page container */} +
+ + {/* ── OVERVIEW ── */} + {(page === 'intro' || exitingPage === 'intro') && ( +
{ if (e.target === e.currentTarget) handleSlideEnd('intro'); } + : undefined} + > + + {/* Scrollable content area with a bottom fade so content disappears + cleanly under the pinned Start Tour button. */} +
+ +
+ {tour.greeting && ( + + {tour.greeting} + + )} + + {/* Intent · Before · After trilogy — each card lands in turn, + its color dot doing a single "spark of life" pulse on land. */} + + {introCards.map((card) => ( + +
+ + + {card.label} + +
+ {card.value} +
+ ))} +
+ + +
+
+ + {/* Bottom fade — content gracefully disappears under the pinned button */} +
+
+ + {/* Pinned Start Tour button — always visible in the viewport */} + {page === 'intro' && ( +
+ navigate('stops')} + className="group w-full flex items-center justify-center gap-2 px-6 py-3.5 rounded-lg bg-primary/85 text-primary-foreground hover:bg-primary text-sm font-medium shadow-sm hover:shadow-md active:scale-[0.98] transition-[background-color,box-shadow,transform] duration-150 ease-out" + > + Start Tour + + + + +
+ )} + +
+ )} + + {/* ── WALKTHROUGH — deferred until intro exit completes so staggered reveals don't flash behind it ── */} + {(page === 'stops' || exitingPage === 'stops') && exitingPage !== 'intro' && ( +
{ if (e.target === e.currentTarget) handleSlideEnd('stops'); } + : undefined} + > + +
+
+ {tour.stops.map((stop, i) => ( +
{ stopsRef.current[i] = el; }} + className={stopsAlreadySeen ? '' : 'tour-stop-reveal'} + style={stopsAlreadySeen ? undefined : { animationDelay: `${i * 60}ms` }} + > + toggleStop(i)} + dimmed={openStops.size > 0 && !openStops.has(i)} + /> +
+ ))} +
+ +
+ {tour.stops.length} stop{tour.stops.length !== 1 ? 's' : ''} + {checklistCount > 0 && ( + + )} +
+
+
+
+ )} + + {/* ── CHECKLIST — also deferred past intro exit ── */} + {(page === 'checklist' || exitingPage === 'checklist') && exitingPage !== 'intro' && ( +
{ if (e.target === e.currentTarget) handleSlideEnd('checklist'); } + : undefined} + > + +
+ {checklistCount > 0 ? ( + { + navigate('stops'); + setTimeout(() => handleScrollToStop(i), 280); + }} + /> + ) : ( +

No checklist items for this tour.

+ )} +
+
+
+ )} + +
+ + ); +} diff --git a/packages/review-editor/components/tour/TourStopCard.tsx b/packages/review-editor/components/tour/TourStopCard.tsx new file mode 100644 index 00000000..22d91d01 --- /dev/null +++ b/packages/review-editor/components/tour/TourStopCard.tsx @@ -0,0 +1,291 @@ +import React, { useRef } from 'react'; +import { motion, AnimatePresence } from 'motion/react'; +import type { TourStop, TourDiffAnchor } from '../../hooks/tour/useTourData'; +import { DiffHunkPreview } from '../DiffHunkPreview'; +import { renderInlineMarkdown } from '../../utils/renderInlineMarkdown'; + +function renderDetail(text: string): React.ReactNode[] { + if (!text) return []; + const nodes: React.ReactNode[] = []; + const lines = text.split('\n'); + let i = 0; + let key = 0; + + while (i < lines.length) { + const line = lines[i]; + + if (!line.trim()) { + i++; + continue; + } + + // Callout block: > [!IMPORTANT] / > [!NOTE] / > [!WARNING] + if (line.match(/^>\s*\[!(IMPORTANT|NOTE|WARNING)\]/i)) { + const type = line.match(/WARNING/i) ? 'warning' : line.match(/IMPORTANT/i) ? 'important' : 'note'; + const calloutLines: string[] = []; + let j = i + 1; + while (j < lines.length && lines[j].startsWith('>')) { + calloutLines.push(lines[j].replace(/^>\s?/, '')); + j++; + } + const calloutStyles = { + important: 'bg-primary/[0.05] dark:bg-primary/[0.12] text-foreground', + warning: 'bg-warning/[0.05] dark:bg-warning/[0.12] text-foreground', + note: 'bg-muted/20 dark:bg-muted/40 text-foreground', + }; + const calloutLabel = { important: 'Important', warning: 'Warning', note: 'Note' }; + nodes.push( +
+ + {calloutLabel[type]} + + {renderInlineMarkdown(calloutLines.join(' '))} +
+ ); + i = j; + continue; + } + + // Heading h3 + if (line.startsWith('### ')) { + nodes.push( +

+ {line.slice(4)} +

+ ); + i++; + continue; + } + + // Bullet list + if (line.match(/^[-*] /)) { + const bullets: string[] = [line.slice(2)]; + let j = i + 1; + while (j < lines.length && lines[j].match(/^[-*] /)) { + bullets.push(lines[j].slice(2)); + j++; + } + nodes.push( +
    + {bullets.map((b, bi) => ( +
  • + {renderInlineMarkdown(b)} +
  • + ))} +
+ ); + i = j; + continue; + } + + // Paragraph — collect until blank line or block element + const paraLines: string[] = [line]; + let j = i + 1; + while ( + j < lines.length && + lines[j].trim() && + !lines[j].startsWith('### ') && + !lines[j].match(/^[-*] /) && + !lines[j].match(/^>\s*\[!/) + ) { + paraLines.push(lines[j]); + j++; + } + nodes.push( +

+ {renderInlineMarkdown(paraLines.join(' '))} +

+ ); + i = j; + } + + return nodes; +} + +// --------------------------------------------------------------------------- +// Inline anchor block +// --------------------------------------------------------------------------- + +function AnchorBlock({ + anchor, + onClick, + parentOpen, +}: { + anchor: TourDiffAnchor; + onClick: () => void; + /** Parent accordion state — DiffHunkPreview is deferred until first open. */ + parentOpen: boolean; +}) { + // Lazy-mount: only create the heavy FileDiff web component on first expand. + // The ref flips synchronously during render so the grid sees the correct height. + const mountedRef = useRef(false); + if (parentOpen) mountedRef.current = true; + + return ( +
+ {/* File header */} +
+ + + + + + {anchor.file} + + + L{anchor.line}–{anchor.end_line} + + +
+ + {/* Label */} + {anchor.label && ( +
+ {anchor.label} +
+ )} + + {/* Diff preview — deferred until parent accordion first opens to avoid + mounting all FileDiff web components on page load (causes jank) */} + {mountedRef.current && ( + + )} +
+ ); +} + +// --------------------------------------------------------------------------- +// Stop card +// --------------------------------------------------------------------------- + +interface TourStopCardProps { + stop: TourStop; + index: number; + total: number; + onAnchorClick: (filePath: string) => void; + open: boolean; + onToggle: () => void; + /** True when another stop is open and this one isn't — fades to defocus. */ + dimmed?: boolean; +} + +export const TourStopCard: React.FC = ({ + stop, + index, + total, + onAnchorClick, + open, + onToggle, + dimmed = false, +}) => { + const isLast = index === total - 1; + + return ( +
+ {/* Timeline node */} +
+ {index + 1} +
+ + {/* Trigger */} + + + {/* Spring-animated accordion: height + opacity, with staggered child entrance */} + + {open && ( + + + + {renderDetail(stop.detail)} + + + {stop.anchors.length > 0 && ( + + {stop.anchors.map((anchor, i) => ( + + onAnchorClick(anchor.file)} + parentOpen={open} + /> + + ))} + + )} + + + )} + + + {/* Transition phrase */} + {!isLast && stop.transition && ( +

+ {stop.transition} +

+ )} +
+ ); +}; diff --git a/packages/review-editor/demoTour.ts b/packages/review-editor/demoTour.ts new file mode 100644 index 00000000..bf87bcda --- /dev/null +++ b/packages/review-editor/demoTour.ts @@ -0,0 +1,198 @@ +/** + * Demo tour data for development mode. + * + * Loaded by useTourData when jobId === DEMO_TOUR_ID, so the dialog renders + * with realistic content without needing an agent run. Toggle via the dev-only + * floating button in App.tsx (only shown when import.meta.env.DEV is true). + */ + +import type { CodeTourData } from './hooks/tour/useTourData'; + +export const DEMO_TOUR_ID = 'demo-tour'; + +const buttonHunk = `@@ -1,15 +1,22 @@ +-import React from 'react'; ++import React, { useCallback } from 'react'; + + interface ButtonProps { + label: string; + onClick: () => void; ++ disabled?: boolean; ++ variant?: 'primary' | 'secondary'; + } + +-export const Button = ({ label, onClick }: ButtonProps) => { ++export const Button = ({ label, onClick, disabled, variant = 'primary' }: ButtonProps) => { ++ const handleClick = useCallback(() => { ++ if (!disabled) onClick(); ++ }, [disabled, onClick]); ++ + return ( +- + ); + };`; + +const authHunk = `@@ -42,7 +42,7 @@ export class AuthService { + async createSession(userId: string): Promise { + const token = await generateToken(userId); +- const expiresAt = Date.now() + 7 * 24 * 60 * 60 * 1000; // 7 days ++ const expiresAt = Date.now() + 24 * 60 * 60 * 1000; // 24 hours + return this.store.create({ userId, token, expiresAt }); + }`; + +const retryHunk = `@@ -1,5 +1,18 @@ + export async function fetchWithRetry(url: string, opts?: RequestInit) { +- return fetch(url, opts); ++ let attempt = 0; ++ let lastErr: unknown; ++ while (attempt < 3) { ++ try { ++ const res = await fetch(url, opts); ++ if (res.ok || res.status < 500) return res; ++ lastErr = new Error(\`HTTP \${res.status}\`); ++ } catch (err) { ++ lastErr = err; ++ } ++ await new Promise(r => setTimeout(r, 200 * 2 ** attempt)); ++ attempt++; ++ } ++ throw lastErr; + }`; + +export const DEMO_TOUR: CodeTourData = { + title: 'Tighten auth + harden network calls', + greeting: + "Hey — okay, so this PR does three related things: it tightens the auth session lifetime from a week down to 24 hours, gives every network call a retry budget with exponential backoff, and finally fixes that Button component that's been silently ignoring its disabled prop for the last six months. Grab a coffee — I'll walk you through it in the order I'd actually want to read it.", + intent: + 'Closes SEC-412, the overly-permissive session TTL flagged by the security team during the Q1 audit. Also lays groundwork for the offline-first work shipping next sprint, which depends on the network layer being resilient to transient failures. The Button fix is opportunistic — once we were touching the auth refresh flow, we noticed the Button on the login page had been masking its disabled state, which is how we ended up with the duplicate-submit bug from #412.', + before: + 'Sessions lasted 7 days with no refresh contract, network calls failed hard on the first 5xx, and the Button component would happily fire onClick even when disabled was true at the prop level.', + after: + 'Sessions expire in 24h with a clean refresh path, network calls retry up to 3 times with exponential backoff (200ms, 400ms, 800ms), and Button now respects disabled at both the prop and the underlying DOM element so the browser also blocks the click.', + key_takeaways: [ + { + text: 'Session TTL dropped from 7 days to 24 hours — every active session pre-deploy will be invalidated. Coordinate the deploy with a maintenance window or expect a flood of re-auth events at deploy time.', + severity: 'warning', + }, + { + text: 'Mobile clients that polled session/refresh every 6 hours need to drop to every 15 minutes. The mobile team has a separate PR (#418) that ships at the same time. Do NOT merge this without that one.', + severity: 'warning', + }, + { + text: 'New retry logic uses exponential backoff (200ms, 400ms, 800ms). Worst-case latency for a fully-failing endpoint is now ~1.4s before throwing instead of immediate failure — make sure no UI is blocking on these calls without a loading state.', + severity: 'important', + }, + { + text: 'Retries happen on 5xx and thrown network errors only. 4xx responses still return immediately because they represent client errors, not transient failures.', + severity: 'important', + }, + { + text: 'Button now memoizes its click handler with useCallback and respects disabled at the DOM level. Existing callsites are source-compatible — no migration needed.', + severity: 'info', + }, + { + text: 'The auth.ts changes touch the same lines as the in-flight refactor on PR #401. Expect a merge conflict; the resolution is mechanical (both branches just renamed the constant).', + severity: 'info', + }, + { + text: 'No new dependencies. No schema changes. No new feature flags. Pure behavior + API tightening.', + severity: 'info', + }, + ], + stops: [ + { + title: 'Auth session lifetime cut from 7 days to 24 hours', + gist: 'Single line change in AuthService.createSession — but every active session gets invalidated on deploy.', + detail: `The session TTL is now 86,400,000 ms (24h) instead of 604,800,000 ms (7 days). This is the line the security audit flagged. + +> [!IMPORTANT] +> Every existing session token in the database has a stored expiresAt based on the old 7-day window. New sessions issued after deploy will use the 24h window. We are NOT retroactively shortening existing tokens — we're letting them expire naturally. + +### What to verify +- The session refresh flow handles the shorter window gracefully (users on the app continuously for >24h need a silent re-auth) +- Mobile clients should already poll \`/api/session/refresh\` every 15 min; confirm that's still the case`, + transition: 'With shorter sessions, clients refresh more often — which makes the network layer the next thing to harden.', + anchors: [ + { + file: 'src/services/auth.ts', + line: 42, + end_line: 48, + hunk: authHunk, + label: 'TTL constant change in createSession', + }, + ], + }, + { + title: 'Network calls now retry on 5xx and network errors', + gist: 'fetchWithRetry wraps fetch with up to 3 attempts and exponential backoff (200ms, 400ms, 800ms).', + detail: `Before, any 5xx or thrown error bubbled up immediately. Now we retry up to 3 times with exponential backoff before giving up. + +### Behavior matrix +- 2xx / 4xx → return immediately (no retry on client errors) +- 5xx → retry up to 3x +- Thrown errors (network down) → retry up to 3x +- All retries exhausted → throw the last error + +### Worst case +A consistently failing endpoint takes 1.4s before throwing (200 + 400 + 800 = 1400ms of backoff). Make sure no UI is blocking on these calls without a loading state.`, + transition: 'The retry budget makes the auth refresh more reliable — closing the loop on the shorter session window.', + anchors: [ + { + file: 'src/lib/fetchWithRetry.ts', + line: 1, + end_line: 18, + hunk: retryHunk, + label: 'Retry loop with exponential backoff', + }, + ], + }, + { + title: 'Button component: disabled, variant, memoized handler', + gist: 'Pure additive API change — existing callsites unchanged, but Button now respects disabled at both prop and DOM level.', + detail: `Three things happen here: + +- New optional \`disabled\` prop, applied to both the underlying \` +
+ ); + } + + return ( +
+
+ + Tour Status + + {!terminal && ( + Generating... + )} +
+ {terminal ? ( +

Tour generation failed.

+ ) : ( +

The tour will be ready when the agent finishes.

+ )} +
+ ); +} + function StatusDot({ status }: { status: AgentJobInfo['status'] }) { if (status === 'starting' || status === 'running') { return ( @@ -267,9 +331,21 @@ function StatusDot({ status }: { status: AgentJobInfo['status'] }) { return ; } -function ProviderPill({ provider }: { provider: string }) { - const label = provider === 'claude' ? 'Claude' : provider === 'codex' ? 'Codex' : 'Shell'; - return {label}; +function ProviderPill({ provider, engine, model }: { provider: string; engine?: string; model?: string }) { + let label: string; + if (provider === 'tour') { + const engineLabel = engine === 'codex' ? 'Codex' : 'Claude'; + label = model && engine !== 'codex' ? `Tour · ${engineLabel} ${model.charAt(0).toUpperCase() + model.slice(1)}` : `Tour · ${engineLabel}`; + } else { + label = provider === 'claude' ? 'Claude' : provider === 'codex' ? 'Codex' : 'Shell'; + } + return ( + + {label} + + ); } function TabButton({ active, onClick, children }: { active: boolean; onClick: () => void; children: React.ReactNode }) { diff --git a/packages/review-editor/hooks/tour/useTourData.ts b/packages/review-editor/hooks/tour/useTourData.ts new file mode 100644 index 00000000..7fff0459 --- /dev/null +++ b/packages/review-editor/hooks/tour/useTourData.ts @@ -0,0 +1,106 @@ +import { useState, useEffect, useCallback, useRef } from 'react'; +import { DEMO_TOUR, DEMO_TOUR_ID } from '../../demoTour'; +import type { CodeTourData } from '@plannotator/shared/tour'; + +export type { TourDiffAnchor, TourKeyTakeaway, TourStop, TourQAItem, CodeTourData } from '@plannotator/shared/tour'; + +export interface UseTourDataReturn { + tour: CodeTourData | null; + loading: boolean; + error: string | null; + checked: boolean[]; + toggleChecked: (index: number) => void; + retry: () => void; +} + +export function useTourData(jobId: string): UseTourDataReturn { + const [tour, setTour] = useState(null); + const [loading, setLoading] = useState(true); + const [error, setError] = useState(null); + const [checked, setChecked] = useState([]); + const saveTimerRef = useRef | null>(null); + const pendingChecklistRef = useRef(null); + + const fetchTour = useCallback(() => { + if (!jobId) return; + setLoading(true); + setError(null); + + // Dev short-circuit: render the demo tour without a backend. + if (jobId === DEMO_TOUR_ID) { + setTour(DEMO_TOUR); + setChecked(new Array(DEMO_TOUR.qa_checklist.length).fill(false)); + setLoading(false); + return; + } + + fetch(`/api/tour/${jobId}`) + .then((res) => { + if (!res.ok) throw new Error(res.status === 404 ? 'Tour not found' : `HTTP ${res.status}`); + return res.json(); + }) + .then((data: CodeTourData) => { + setTour(data); + setChecked(data.checklist?.length > 0 ? data.checklist : new Array(data.qa_checklist.length).fill(false)); + setLoading(false); + }) + .catch((err) => { + setError(err instanceof Error ? err.message : String(err)); + setLoading(false); + }); + }, [jobId]); + + useEffect(() => { + fetchTour(); + }, [fetchTour]); + + const saveChecklist = useCallback( + (next: boolean[]) => { + if (jobId === DEMO_TOUR_ID) return; + pendingChecklistRef.current = next; + if (saveTimerRef.current) clearTimeout(saveTimerRef.current); + saveTimerRef.current = setTimeout(() => { + const payload = pendingChecklistRef.current; + pendingChecklistRef.current = null; + saveTimerRef.current = null; + if (!payload) return; + fetch(`/api/tour/${jobId}/checklist`, { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ checked: payload }), + }).catch(() => {}); + }, 500); + }, + [jobId], + ); + + const toggleChecked = useCallback( + (index: number) => { + setChecked((prev) => { + const next = [...prev]; + next[index] = !next[index]; + saveChecklist(next); + return next; + }); + }, + [saveChecklist], + ); + + useEffect(() => { + return () => { + if (saveTimerRef.current) clearTimeout(saveTimerRef.current); + const payload = pendingChecklistRef.current; + pendingChecklistRef.current = null; + if (!payload || jobId === DEMO_TOUR_ID) return; + // keepalive lets the request survive if this unmount is part of a tab close. + fetch(`/api/tour/${jobId}/checklist`, { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ checked: payload }), + keepalive: true, + }).catch(() => {}); + }; + }, [jobId]); + + return { tour, loading, error, checked, toggleChecked, retry: fetchTour }; +} diff --git a/packages/review-editor/index.css b/packages/review-editor/index.css index 0507811d..e6776109 100644 --- a/packages/review-editor/index.css +++ b/packages/review-editor/index.css @@ -1081,3 +1081,150 @@ diffs-container { :root[style*="--diff-font-size-override"] .ai-markdown code { font-size: var(--diff-font-size-override) !important; } + +/* ===================================================== + Code Tour + ===================================================== */ + +/* --- Dialog overlay + card entrance / exit --- */ +@keyframes tour-dialog-overlay-in { + from { opacity: 0; } + to { opacity: 1; } +} + +@keyframes tour-dialog-overlay-out { + from { opacity: 1; } + to { opacity: 0; } +} + +@keyframes tour-dialog-content-in { + from { opacity: 0; transform: scale(0.97); } + to { opacity: 1; transform: scale(1); } +} + +@keyframes tour-dialog-content-out { + from { opacity: 1; transform: scale(1); } + to { opacity: 0; transform: scale(0.98); } +} + +.tour-dialog-overlay { + animation: tour-dialog-overlay-in 150ms cubic-bezier(0.33, 1, 0.68, 1) both; +} + +.tour-dialog-content { + animation: tour-dialog-content-in 250ms cubic-bezier(0.16, 1, 0.3, 1) both; +} + +.tour-dialog-overlay-closing { + animation: tour-dialog-overlay-out 160ms cubic-bezier(0.33, 1, 0.68, 1) forwards; +} + +.tour-dialog-content-closing { + animation: tour-dialog-content-out 180ms cubic-bezier(0.33, 1, 0.68, 1) forwards; + pointer-events: none; +} + +/* --- Stop card reveal (applied by parent wrapper, staggered via animation-delay) --- */ +@keyframes tour-stop-reveal { + from { + opacity: 0; + transform: translateY(10px); + } + to { + opacity: 1; + transform: translateY(0); + } +} + +.tour-stop-reveal { + animation: tour-stop-reveal 280ms cubic-bezier(0.16, 1, 0.3, 1) both; + will-change: transform, opacity; +} + +/* --- Intro page exit: lifts up and fades out, then stops page reveals --- */ +@keyframes tour-intro-exit { + 0% { opacity: 1; transform: translateY(0) scale(1); } + 30% { opacity: 0.8; transform: translateY(-4px) scale(0.99); } + 100% { opacity: 0; transform: translateY(-20px) scale(0.97); } +} + +.tour-intro-exit { + animation: tour-intro-exit 320ms cubic-bezier(0.33, 1, 0.68, 1) forwards; + pointer-events: none; +} + +/* --- Stops section label entrance (after intro exits) --- */ +@keyframes tour-section-enter { + from { opacity: 0; transform: translateY(8px); } + to { opacity: 1; transform: translateY(0); } +} + +.tour-section-enter { + animation: tour-section-enter 280ms cubic-bezier(0.16, 1, 0.3, 1) both; +} + +/* --- Page slide transitions (stops ↔ checklist) --- */ +@keyframes tour-slide-out-left { + from { opacity: 1; transform: translateX(0); } + to { opacity: 0; transform: translateX(-48px); } +} + +@keyframes tour-slide-in-right { + from { opacity: 0; transform: translateX(48px); } + to { opacity: 1; transform: translateX(0); } +} + +@keyframes tour-slide-out-right { + from { opacity: 1; transform: translateX(0); } + to { opacity: 0; transform: translateX(48px); } +} + +@keyframes tour-slide-in-left { + from { opacity: 0; transform: translateX(-48px); } + to { opacity: 1; transform: translateX(0); } +} + +.tour-page-exit-fwd { + animation: tour-slide-out-left 220ms cubic-bezier(0.33, 1, 0.68, 1) forwards; + pointer-events: none; +} + +.tour-page-enter-fwd { + animation: tour-slide-in-right 260ms cubic-bezier(0.16, 1, 0.3, 1) both; +} + +.tour-page-exit-bwd { + animation: tour-slide-out-right 220ms cubic-bezier(0.33, 1, 0.68, 1) forwards; + pointer-events: none; +} + +.tour-page-enter-bwd { + animation: tour-slide-in-left 260ms cubic-bezier(0.16, 1, 0.3, 1) both; +} + +/* --- Checkbox transition (ease for color changes — elegant, per the easing blueprint) --- */ +.tour-checkbox { + transition: background-color 150ms ease, border-color 150ms ease; +} + + + +/* --- Reduced motion: disable all tour animations --- */ +@media (prefers-reduced-motion: reduce) { + .tour-dialog-overlay, + .tour-dialog-overlay-closing, + .tour-dialog-content, + .tour-dialog-content-closing, + .tour-stop-reveal, + .tour-intro-exit, + .tour-section-enter, + .tour-page-exit-fwd, + .tour-page-enter-fwd, + .tour-page-exit-bwd, + .tour-page-enter-bwd { + animation: none !important; + } + .tour-checkbox { + transition: none !important; + } +} diff --git a/packages/review-editor/package.json b/packages/review-editor/package.json index 4f8057d5..5ab3d63a 100644 --- a/packages/review-editor/package.json +++ b/packages/review-editor/package.json @@ -7,10 +7,14 @@ "./styles": "./index.css" }, "dependencies": { + "@pierre/diffs": "^1.1.12", "@plannotator/shared": "workspace:*", "@plannotator/ui": "workspace:*", - "@pierre/diffs": "^1.1.12", + "@radix-ui/react-checkbox": "^1.3.3", + "@radix-ui/react-collapsible": "^1.1.12", + "@radix-ui/react-tooltip": "^1.2.8", "highlight.js": "^11.11.1", + "motion": "^12.38.0", "react": "^19.2.3", "react-dom": "^19.2.3", "tailwindcss": "^4.1.18" diff --git a/packages/server/agent-jobs.ts b/packages/server/agent-jobs.ts index 513fdb53..584491e0 100644 --- a/packages/server/agent-jobs.ts +++ b/packages/server/agent-jobs.ts @@ -62,7 +62,7 @@ export interface AgentJobHandlerOptions { * Return an object with the command to spawn (and optional output path for result ingestion). * Return null to reject or fall through to frontend-supplied command. */ - buildCommand?: (provider: string) => Promise<{ + buildCommand?: (provider: string, config?: Record) => Promise<{ command: string[]; outputPath?: string; captureStdout?: boolean; @@ -71,6 +71,16 @@ export interface AgentJobHandlerOptions { label?: string; /** The full prompt text for display in the detail panel. */ prompt?: string; + /** Underlying engine used (e.g., "claude" or "codex"). Stored on AgentJobInfo for UI display. */ + engine?: string; + /** Model used (e.g., "sonnet", "opus"). Stored on AgentJobInfo for UI display. */ + model?: string; + /** Claude --effort level. */ + effort?: string; + /** Codex reasoning effort level. */ + reasoningEffort?: string; + /** Whether Codex fast mode was enabled. */ + fastMode?: boolean; } | null>; /** * Called after a job process exits with exit code 0. @@ -93,6 +103,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob const capabilities: AgentCapability[] = [ { id: "claude", name: "Claude Code", available: !!Bun.which("claude") }, { id: "codex", name: "Codex CLI", available: !!Bun.which("codex") }, + { id: "tour", name: "Code Tour", available: !!Bun.which("claude") || !!Bun.which("codex") }, ]; const capabilitiesResponse: AgentCapabilities = { mode, @@ -119,7 +130,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob command: string[], label: string, outputPath?: string, - spawnOptions?: { captureStdout?: boolean; stdinPrompt?: string; cwd?: string; prompt?: string }, + spawnOptions?: { captureStdout?: boolean; stdinPrompt?: string; cwd?: string; prompt?: string; engine?: string; model?: string; effort?: string; reasoningEffort?: string; fastMode?: boolean }, ): AgentJobInfo { const id = crypto.randomUUID(); const source = jobSource(id); @@ -133,6 +144,11 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob startedAt: Date.now(), command, cwd: getCwd(), + ...(spawnOptions?.engine && { engine: spawnOptions.engine }), + ...(spawnOptions?.model && { model: spawnOptions.model }), + ...(spawnOptions?.effort && { effort: spawnOptions.effort }), + ...(spawnOptions?.reasoningEffort && { reasoningEffort: spawnOptions.reasoningEffort }), + ...(spawnOptions?.fastMode && { fastMode: spawnOptions.fastMode }), }; let proc: ReturnType | null = null; @@ -220,8 +236,9 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob const lines = text.split('\n'); for (const line of lines) { if (!line.trim()) continue; - // Claude: format JSONL into readable text - if (provider === "claude") { + // Claude: format JSONL into readable text. Tour jobs with the + // Claude engine also stream Claude JSONL, so key off engine too. + if (provider === "claude" || spawnOptions?.engine === "claude") { const formatted = formatClaudeLogEvent(line); if (formatted !== null) { broadcast({ type: "job:log", jobId: id, delta: formatted + '\n' }); @@ -423,8 +440,20 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob let stdinPrompt: string | undefined; let spawnCwd: string | undefined; let promptText: string | undefined; + let jobEngine: string | undefined; + let jobModel: string | undefined; + let jobEffort: string | undefined; + let jobReasoningEffort: string | undefined; + let jobFastMode: boolean | undefined; if (options.buildCommand) { - const built = await options.buildCommand(provider); + // Thread config from POST body to buildCommand + const config: Record = {}; + if (typeof body.engine === "string") config.engine = body.engine; + if (typeof body.model === "string") config.model = body.model; + if (typeof body.reasoningEffort === "string") config.reasoningEffort = body.reasoningEffort; + if (typeof body.effort === "string") config.effort = body.effort; + if (body.fastMode === true) config.fastMode = true; + const built = await options.buildCommand(provider, Object.keys(config).length > 0 ? config : undefined); if (built) { command = built.command; outputPath = built.outputPath; @@ -433,6 +462,11 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob spawnCwd = built.cwd; promptText = built.prompt; if (built.label) label = built.label; + jobEngine = built.engine; + jobModel = built.model; + jobEffort = built.effort; + jobReasoningEffort = built.reasoningEffort; + jobFastMode = built.fastMode; } } @@ -448,6 +482,11 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob stdinPrompt, cwd: spawnCwd, prompt: promptText, + engine: jobEngine, + model: jobModel, + effort: jobEffort, + reasoningEffort: jobReasoningEffort, + fastMode: jobFastMode, }); return Response.json({ job }, { status: 201 }); } catch { diff --git a/packages/server/claude-review.ts b/packages/server/claude-review.ts index 55dd167f..97d4a4ec 100644 --- a/packages/server/claude-review.ts +++ b/packages/server/claude-review.ts @@ -189,7 +189,7 @@ export interface ClaudeCommandResult { * Build the `claude -p` command. Prompt is passed via stdin, not as a * positional arg — avoids quoting issues, argv limits, and variadic flag conflicts. */ -export function buildClaudeCommand(prompt: string): ClaudeCommandResult { +export function buildClaudeCommand(prompt: string, model: string = "claude-opus-4-7", effort?: string): ClaudeCommandResult { const allowedTools = [ "Agent", "Read", "Glob", "Grep", // GitHub CLI @@ -224,7 +224,8 @@ export function buildClaudeCommand(prompt: string): ClaudeCommandResult { "--verbose", "--json-schema", CLAUDE_REVIEW_SCHEMA_JSON, "--no-session-persistence", - "--model", "sonnet", + "--model", model, + ...(effort ? ["--effort", effort] : []), "--tools", "Agent,Bash,Read,Glob,Grep", "--allowedTools", allowedTools, "--disallowedTools", disallowedTools, diff --git a/packages/server/codex-review.ts b/packages/server/codex-review.ts index 62abef7e..559f044c 100644 --- a/packages/server/codex-review.ts +++ b/packages/server/codex-review.ts @@ -226,15 +226,21 @@ export interface CodexCommandOptions { cwd: string; outputPath: string; prompt: string; + model?: string; + reasoningEffort?: string; + fastMode?: boolean; } /** Build the `codex exec` argv array. Materializes the schema file on first call. */ export async function buildCodexCommand(options: CodexCommandOptions): Promise { - const { cwd, outputPath, prompt } = options; + const { cwd, outputPath, prompt, model, reasoningEffort, fastMode } = options; const schemaPath = await ensureSchemaFile(); const command = [ "codex", + ...(model ? ["-m", model] : []), + ...(reasoningEffort ? ["-c", `model_reasoning_effort=${reasoningEffort}`] : []), + ...(fastMode ? ["-c", "service_tier=fast"] : []), "exec", "--output-schema", schemaPath, "-o", outputPath, diff --git a/packages/server/review.ts b/packages/server/review.ts index c6b98ac0..2b9cbb04 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -32,6 +32,7 @@ import { parseClaudeStreamOutput, transformClaudeFindings, } from "./claude-review"; +import { createTourSession, TOUR_EMPTY_OUTPUT_ERROR } from "./tour/tour-review"; import { saveConfig, detectGitUser, getServerConfig } from "./config"; import { type PRMetadata, type PRReviewFileComment, fetchPRFileContent, fetchPRContext, submitPRReview, fetchPRViewedFiles, markPRFilesViewed, getPRUser, prRefFromMetadata, getDisplayRepo, getMRLabel, getMRNumberLabel } from "./pr"; import { createAIEndpoints, ProviderRegistry, SessionManager, createProvider, type AIEndpoints, type PiSDKConfig } from "@plannotator/ai"; @@ -120,6 +121,8 @@ export async function startReviewServer( const editorAnnotations = createEditorAnnotationHandler(); const externalAnnotations = createExternalAnnotationHandler("review"); + const tour = createTourSession(); + // Mutable state for diff switching let currentPatch = options.rawPatch; let currentGitRef = options.gitRef; @@ -128,42 +131,54 @@ export async function startReviewServer( // Agent jobs — background process manager (late-binds serverUrl via getter) let serverUrl = ""; + const resolveAgentCwd = (): string => + options.agentCwd ?? resolveVcsCwd(currentDiffType, gitContext?.cwd) ?? process.cwd(); const agentJobs = createAgentJobHandler({ mode: "review", getServerUrl: () => serverUrl, - getCwd: () => { - if (options.agentCwd) return options.agentCwd; - return resolveVcsCwd(currentDiffType, gitContext?.cwd) ?? process.cwd(); - }, + getCwd: resolveAgentCwd, - async buildCommand(provider) { - const cwd = options.agentCwd ?? resolveVcsCwd(currentDiffType, gitContext?.cwd) ?? process.cwd(); + async buildCommand(provider, config) { + const cwd = resolveAgentCwd(); const hasAgentLocalAccess = !!options.agentCwd || !!gitContext; - const userMessage = buildCodexReviewUserMessage( - currentPatch, - currentDiffType, - { defaultBranch: gitContext?.defaultBranch, hasLocalAccess: hasAgentLocalAccess }, - prMetadata, - ); + const userMessageOptions = { defaultBranch: gitContext?.defaultBranch, hasLocalAccess: hasAgentLocalAccess }; + + if (provider === "tour") { + return tour.buildCommand({ + cwd, + patch: currentPatch, + diffType: currentDiffType, + options: userMessageOptions, + prMetadata, + config, + }); + } + + const userMessage = buildCodexReviewUserMessage(currentPatch, currentDiffType, userMessageOptions, prMetadata); if (provider === "codex") { + const model = typeof config?.model === "string" && config.model ? config.model : undefined; + const reasoningEffort = typeof config?.reasoningEffort === "string" && config.reasoningEffort ? config.reasoningEffort : undefined; + const fastMode = config?.fastMode === true; const outputPath = generateOutputPath(); const prompt = CODEX_REVIEW_SYSTEM_PROMPT + "\n\n---\n\n" + userMessage; - const command = await buildCodexCommand({ cwd, outputPath, prompt }); - return { command, outputPath, prompt, label: "Codex Review" }; + const command = await buildCodexCommand({ cwd, outputPath, prompt, model, reasoningEffort, fastMode }); + return { command, outputPath, prompt, label: "Code Review", model, reasoningEffort, fastMode: fastMode || undefined }; } if (provider === "claude") { + const model = typeof config?.model === "string" && config.model ? config.model : undefined; + const effort = typeof config?.effort === "string" && config.effort ? config.effort : undefined; const prompt = CLAUDE_REVIEW_PROMPT + "\n\n---\n\n" + userMessage; - const { command, stdinPrompt } = buildClaudeCommand(prompt); - return { command, stdinPrompt, prompt, cwd, label: "Claude Code Review", captureStdout: true }; + const { command, stdinPrompt } = buildClaudeCommand(prompt, model, effort); + return { command, stdinPrompt, prompt, cwd, label: "Code Review", captureStdout: true, model, effort }; } return null; }, async onJobComplete(job, meta) { - const cwd = options.agentCwd ?? resolveVcsCwd(currentDiffType, gitContext?.cwd) ?? process.cwd(); + const cwd = resolveAgentCwd(); // --- Codex path --- if (job.provider === "codex" && meta.outputPath) { @@ -209,6 +224,21 @@ export async function startReviewServer( } return; } + + // --- Tour path --- + if (job.provider === "tour") { + const { summary } = await tour.onJobComplete({ job, meta }); + if (summary) { + job.summary = summary; + } else { + // The process exited 0 but the model returned empty or malformed output + // and nothing was stored. Flip status so the client doesn't auto-open + // a successful-looking card that 404s on /api/tour/:id. + job.status = "failed"; + job.error = TOUR_EMPTY_OUTPUT_ERROR; + } + return; + } }, }); @@ -354,6 +384,27 @@ export async function startReviewServer( async fetch(req, server) { const url = new URL(req.url); + // API: Get tour result + if (url.pathname.match(/^\/api\/tour\/[^/]+$/) && req.method === "GET") { + const jobId = url.pathname.slice("/api/tour/".length); + const result = tour.getTour(jobId); + if (!result) return Response.json({ error: "Tour not found" }, { status: 404 }); + return Response.json(result); + } + + // API: Save tour checklist state + const checklistMatch = url.pathname.match(/^\/api\/tour\/([^/]+)\/checklist$/); + if (checklistMatch && req.method === "PUT") { + const jobId = checklistMatch[1]; + try { + const body = await req.json() as { checked: boolean[] }; + if (Array.isArray(body.checked)) tour.saveChecklist(jobId, body.checked); + return Response.json({ ok: true }); + } catch { + return Response.json({ error: "Invalid JSON" }, { status: 400 }); + } + } + // API: Get diff content if (url.pathname === "/api/diff" && req.method === "GET") { return Response.json({ diff --git a/packages/server/tour/tour-review.test.ts b/packages/server/tour/tour-review.test.ts new file mode 100644 index 00000000..5858627b --- /dev/null +++ b/packages/server/tour/tour-review.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, test } from "bun:test"; +import { writeFile, mkdtemp } from "node:fs/promises"; +import { existsSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { parseTourStreamOutput, parseTourFileOutput } from "./tour-review"; + +const stop = { + title: "Add retry", + gist: "Wrap the fetch in a retry.", + detail: "Three attempts, exponential backoff.", + transition: "", + anchors: [{ file: "src/a.ts", line: 1, end_line: 10, hunk: "", label: "retry" }], +}; +const validOutput = { + title: "Retry pass", + greeting: "hi", + intent: "", + before: "", + after: "", + key_takeaways: [], + stops: [stop], + qa_checklist: [], +}; + +describe("parseTourStreamOutput", () => { + test("returns parsed output from terminal result event", () => { + const stdout = [ + JSON.stringify({ type: "assistant" }), + JSON.stringify({ type: "result", is_error: false, structured_output: validOutput }), + ].join("\n"); + expect(parseTourStreamOutput(stdout)).toEqual(validOutput); + }); + + test("returns null when result has is_error: true", () => { + const stdout = JSON.stringify({ type: "result", is_error: true, structured_output: validOutput }); + expect(parseTourStreamOutput(stdout)).toBeNull(); + }); + + test("returns null when stops array is empty", () => { + const stdout = JSON.stringify({ type: "result", is_error: false, structured_output: { stops: [] } }); + expect(parseTourStreamOutput(stdout)).toBeNull(); + }); + + test("returns null when structured_output has no stops key", () => { + const stdout = JSON.stringify({ type: "result", is_error: false, structured_output: {} }); + expect(parseTourStreamOutput(stdout)).toBeNull(); + }); + + test("returns null on empty input", () => { + expect(parseTourStreamOutput("")).toBeNull(); + expect(parseTourStreamOutput(" \n ")).toBeNull(); + }); + + test("does not throw on truncated/malformed JSON", () => { + const stdout = '{"type":"assistant"}\n{"type":"result","is_err'; + expect(parseTourStreamOutput(stdout)).toBeNull(); + }); +}); + +describe("parseTourFileOutput", () => { + test("returns null when file missing", async () => { + const missing = join(tmpdir(), "plannotator-tour-missing-" + Date.now() + ".json"); + expect(await parseTourFileOutput(missing)).toBeNull(); + }); + + test("returns parsed output and unlinks file", async () => { + const dir = await mkdtemp(join(tmpdir(), "plannotator-tour-")); + const file = join(dir, "out.json"); + await writeFile(file, JSON.stringify(validOutput)); + const result = await parseTourFileOutput(file); + expect(result).toEqual(validOutput); + expect(existsSync(file)).toBe(false); + }); + + test("returns null and unlinks file when JSON is malformed", async () => { + const dir = await mkdtemp(join(tmpdir(), "plannotator-tour-")); + const file = join(dir, "out.json"); + await writeFile(file, "{not json"); + const result = await parseTourFileOutput(file); + expect(result).toBeNull(); + expect(existsSync(file)).toBe(false); + }); + + test("returns null when stops array is empty", async () => { + const dir = await mkdtemp(join(tmpdir(), "plannotator-tour-")); + const file = join(dir, "out.json"); + await writeFile(file, JSON.stringify({ stops: [] })); + expect(await parseTourFileOutput(file)).toBeNull(); + }); + + test("returns null when stops key missing", async () => { + const dir = await mkdtemp(join(tmpdir(), "plannotator-tour-")); + const file = join(dir, "out.json"); + await writeFile(file, JSON.stringify({ other: 1 })); + expect(await parseTourFileOutput(file)).toBeNull(); + }); +}); diff --git a/packages/server/tour/tour-review.ts b/packages/server/tour/tour-review.ts new file mode 100644 index 00000000..bb71e12b --- /dev/null +++ b/packages/server/tour/tour-review.ts @@ -0,0 +1,582 @@ +import { join } from "node:path"; +import { homedir, tmpdir } from "node:os"; +import { mkdir, writeFile, readFile, unlink } from "node:fs/promises"; +import type { DiffType } from "../vcs"; +import type { PRMetadata } from "../pr"; +import type { + CodeTourOutput, + TourDiffAnchor, + TourKeyTakeaway, + TourStop, + TourQAItem, +} from "@plannotator/shared/tour"; + +export type { CodeTourOutput, TourDiffAnchor, TourKeyTakeaway, TourStop, TourQAItem }; + +export const TOUR_EMPTY_OUTPUT_ERROR = "Tour generation returned empty or malformed output"; + +export const TOUR_SCHEMA_JSON = JSON.stringify({ + type: "object", + properties: { + title: { type: "string" }, + greeting: { type: "string" }, + intent: { type: "string" }, + before: { type: "string" }, + after: { type: "string" }, + key_takeaways: { + type: "array", + items: { + type: "object", + properties: { + text: { type: "string" }, + severity: { type: "string", enum: ["info", "important", "warning"] }, + }, + required: ["text", "severity"], + additionalProperties: false, + }, + }, + stops: { + type: "array", + items: { + type: "object", + properties: { + title: { type: "string" }, + gist: { type: "string" }, + detail: { type: "string" }, + transition: { type: "string" }, + anchors: { + type: "array", + items: { + type: "object", + properties: { + file: { type: "string" }, + line: { type: "integer" }, + end_line: { type: "integer" }, + hunk: { type: "string" }, + label: { type: "string" }, + }, + required: ["file", "line", "end_line", "hunk", "label"], + additionalProperties: false, + }, + }, + }, + required: ["title", "gist", "detail", "transition", "anchors"], + additionalProperties: false, + }, + }, + qa_checklist: { + type: "array", + items: { + type: "object", + properties: { + question: { type: "string" }, + stop_indices: { type: "array", items: { type: "integer" } }, + }, + required: ["question", "stop_indices"], + additionalProperties: false, + }, + }, + }, + required: ["title", "greeting", "intent", "before", "after", "key_takeaways", "stops", "qa_checklist"], + additionalProperties: false, +}); + +export const TOUR_REVIEW_PROMPT = `# Code Tour Narrator + +## Identity +You are a colleague giving a casual, warm tour of work you understand well. +Think of it like sitting down next to someone and saying: "Hey Mike, here's +the PR. Let me walk you through it." The whole voice is conversational, not +documentary. You're telling the story of what changed and why. + +The arguments (like "here's why we did it this way" or "we picked X instead +of Y") live INSIDE the stop details, where they belong. The framing (the +greeting, intent, before/after, transitions between stops) stays warm and +human, the way a coworker actually talks over coffee. + +You are NOT finding bugs. You are NOT writing a technical report. + +## Tone +- Conversational throughout. You're talking to a coworker, not writing docs. +- Use "we" and "you". "Here's what we changed." "You'll notice that..." +- A couple of sentences of context is fine, even for small PRs. If a + colleague was describing a one-line change, they wouldn't just say "I + changed a line." They'd say "Oh yeah, I bumped the TTL from 7 days to 24 + hours because the audit flagged it last month." A little color is good. +- Each stop should feel like a colleague pausing to point at something: + "Okay, look at this part. Here's why it's interesting." +- **Do NOT use em-dashes (—) anywhere.** They're a dead giveaway of + AI-generated prose. Use commas, colons, semicolons, or separate sentences + instead. If you want to add an aside, use parentheses or start a new + sentence. Never an em-dash. +- No emoji anywhere. The UI handles all visual labeling deterministically. + +## Output structure + +### greeting +2-4 sentences welcoming the reviewer and setting the scene. Not a headline, +more like how you'd actually open a conversation. "Hey, so this PR does X +and Y. Grab a coffee; I'll walk you through it." A bit of warmth and context, +even for small changes. +Example: "Hey, so this PR tightens the auth session lifetime from a week down +to 24 hours. It's small in line count but it's the fix the security team has +been asking for since Q1. Let me walk you through it." + +### intent +1-3 sentences explaining WHY this changeset exists. What problem is being +solved? What motivated the work? Keep it conversational; you're giving +context, not writing a ticket. + +To determine intent: +- If a PR/MR URL was provided, read the PR description (gh pr view or + equivalent). Look for motivation, linked issues, and context the author + provided. +- If the PR body references a GitHub issue (e.g. "Fixes #123", "Closes + owner/repo#456") or GitLab issue, read that specific issue for deeper + context. +- If no PR is provided, infer intent from commit messages, branch name, and + the nature of the changes themselves. +- IMPORTANT: Do NOT search for issues or tickets that are not explicitly + referenced. Do not browse all open issues. Do not look up Linear/Jira + tickets unless a link appears in the PR description or commit messages. + Only follow what is given. + +Example: "Closes SEC-412, the overly-permissive session TTL flagged by the +security team during the Q1 audit. It also lays some groundwork for the +offline-first work shipping next sprint." + +### before / after +One to two sentences each. Paint the picture of the world before and after +this change. Focus on user or system behavior, not code structure. +Example before: "Sessions lasted 7 days, with no refresh contract, so a +stolen token was dangerous for a full week." +Example after: "Sessions now expire in 24 hours with a clean refresh path, +and mobile clients poll every 15 minutes to stay fresh." + +### key_takeaways +3 to 5 bullet points. These are the MOST IMPORTANT things someone needs to +know at a glance about what this changeset DOES. Focus on what changes in +behavior, functionality, or developer experience. Each is ONE sentence. No +emoji, no prefix, just the text. + +Severity guide (drives visual styling automatically; pick honestly, don't inflate): +- "info": neutral context, good to know. +- "important": a meaningful change in behavior, capability, or system contract. +- "warning": a behavioral shift worth watching, something that changes how + the system works in a way someone could miss. NOT code smells or style + nits. A clean changeset with no warnings is perfectly normal. + +### stops +Each stop is the colleague pausing at a specific change to explain it. + +#### How to ORDER stops +Order by READING FLOW, the order the colleague would walk you through the +change to make it understandable. NOT by blast radius or criticality. + +Lead with the entry point: the file or function that, if understood alone, +unlocks the rest. Then walk outward: +- Definitions before consumers (types/interfaces/schemas before usage). +- Cause before effect (the change that motivated downstream changes comes first). +- Verification last (tests and migrations after the code they exercise). + +#### How to CHUNK stops +A stop is a logical change, NOT a file. If three files changed for one reason, +that's ONE stop with three anchors. If one file has two unrelated changes, +that's two stops. Never "one-stop-per-file" by default; let logic decide. + +#### Stop fields +- **title**: Short, friendly. "Token refresh flow", not "Changes to auth/refresh.ts". +- **gist**: ONE sentence. The headline. A reviewer who reads nothing else should + understand this stop from the gist alone. +- **detail**: This is where the colleague pauses to explain. Supports basic markdown. + - Start with 1-2 sentences describing the situation or problem this stop addresses. + - Then make the argument: WHY did we change this? WHY does the new code look the + way it does? If a non-obvious choice was made (data structure, error strategy, + sync vs async, where the logic lives), surface it. "We did X instead of Y + because Z" is exactly what the reviewer wants. + - Use ### headings (e.g. "### Why this shape") to highlight critical sub-sections. + - Use > [!IMPORTANT], > [!WARNING], or > [!NOTE] callout blocks for context + that helps the reader understand non-obvious decisions or behavioral shifts + (e.g., a new default value, a changed error path, a contract that callers + now depend on). These are not for flagging code smells. + - Use - bullet points for multi-part changes or parallel considerations. + - Keep total length reasonable, around 3-6 sentences equivalent. Don't write + an essay. +- **transition**: A short connective phrase to the next stop, in the colleague's + voice. Examples: "Building on that...", "On a related note...", "To support + that change...". Empty string for the last stop. +- **anchors**: The specific diff hunks shown inline below the detail narrative. + Each anchor MUST have a non-empty "hunk" field containing the actual unified + diff text extracted from the changeset. The hunk must include the @@ line. + + Valid hunk format (REQUIRED; every anchor needs this): + + @@ -42,7 +42,9 @@ + function processRequest(req) { + - const result = await fetch(url); + - return result.json(); + + const result = await fetch(url, { timeout: 5000 }); + + if (!result.ok) throw new Error("HTTP " + result.status); + + return result.json(); + } + + The label should be a substantive 1-sentence explanation of what this code + section does or why it matters, not a filename paraphrase. + E.g. "Adds a 5-second timeout and explicit error check to prevent silent hangs", + not "Changes to request.ts". + +### qa_checklist +4 to 8 verification questions a HUMAN can actually answer. Two valid channels: + +1. By READING the code (e.g., "Did we update both call sites of \`legacyAuth()\`?", + "Are all uses of the old token format migrated?", "Does the error handler + cover the new throw paths?"). +2. By manually USING the product (e.g., "Sign in, restart the browser, and + confirm the session persists.", "Trigger a 503 from the API and confirm the + retry banner appears."). + +NOT machine-runnable test ideas. NOT generic "smoke test" framing. The reviewer +is a person; what would THEY do to gain confidence? + +Reference which stops each question relates to via stop_indices. Every question +should reference at least one stop. + +## Pipeline + +1. Read the full diff (git diff or inlined patch). +2. Read CLAUDE.md and README.md for project context. +3. Read commit messages (git log --oneline) and PR title/body if available. +4. Identify logical groupings of change (cross-file when appropriate). These + become stops. +5. Determine reading flow order: entry point first, then outward. Definitions + before consumers, cause before effect. +6. Write the greeting, intent, before/after, takeaways, stops, and checklist + in the voice of a coworker walking you through the work. +7. Return structured JSON matching the schema. + +## Hard constraints +- Every anchor MUST have a non-empty "hunk" field. An anchor with an empty hunk + is broken; it will show "diff not available" to the reviewer. Extract the + real unified diff text from the input patch. Do not leave hunk blank. +- Never fabricate line numbers. Extract them from the diff. +- Gist must be ONE sentence. Not two. Not a run-on. One. +- Detail supports markdown. Use it when it makes the explanation clearer, not + for decoration. Plain prose is fine when the change is simple. +- Anchor labels must explain the code's purpose or the change's impact, not + just describe the filename. +- key_takeaways: 3 to 5 items, each ONE sentence. +- Stops are LOGICAL units, not files. Cross-file grouping is expected. +- Stop ORDER is reading flow: entry point first, definitions before consumers, + cause before effect, verification last. +- Combine trivial changes (renames, imports, formatting) into one "Housekeeping" + stop at the end, or omit entirely. +- QA questions must be answerable by a human, either by reading code or by + using the product. Never frame them as automated tests. +- NEVER use em-dashes (—) anywhere in the output. Use commas, colons, + semicolons, parentheses, or separate sentences. This is a hard constraint. + +## Calibration: tour, not review +Your job is to EXPLAIN the changeset, not to critique it. If you genuinely +spot a real bug or a meaningful behavioral concern while reading the code, +surface it naturally in the relevant stop detail or as a warning takeaway. +That's the colleague noticing something worth mentioning. But don't hunt for +problems. Most clean changesets should have zero warnings and zero [!WARNING] +callouts. The primary question is "what does this change do and why?" not +"what's wrong with this code?"`; + +function buildTourUserMessage( + patch: string, + diffType: DiffType, + options?: { defaultBranch?: string; hasLocalAccess?: boolean }, + prMetadata?: PRMetadata, +): string { + if (prMetadata) { + if (options?.hasLocalAccess) { + return [ + prMetadata.url, + "", + "You are in a local worktree checked out at the PR head. The code is available locally.", + `To see the PR changes, diff against the remote base branch: git diff origin/${prMetadata.baseBranch}...HEAD`, + "Do NOT diff against the local `main` branch; it may be stale. Always use origin/.", + "", + "Walk the reviewer through this changeset as a guided tour.", + ].join("\n"); + } + return [prMetadata.url, "", "Walk the reviewer through this PR as a guided tour."].join("\n"); + } + + const effectiveDiffType = diffType.startsWith("worktree:") + ? diffType.split(":").pop() || "uncommitted" + : diffType; + + switch (effectiveDiffType) { + case "uncommitted": + return "Walk the reviewer through the current code changes (staged, unstaged, and untracked files) as a guided tour."; + case "staged": + return "Walk the reviewer through the currently staged code changes (`git diff --staged`) as a guided tour."; + case "unstaged": + return "Walk the reviewer through the unstaged code changes (tracked modifications and untracked files) as a guided tour."; + case "last-commit": + return "Walk the reviewer through the code changes introduced in the last commit (`git diff HEAD~1..HEAD`) as a guided tour."; + case "branch": { + const base = options?.defaultBranch || "main"; + return `Walk the reviewer through the code changes against the base branch '${base}' as a guided tour. Run \`git diff ${base}..HEAD\` to inspect the changes.`; + } + default: + return [ + "Walk the reviewer through the following code changes as a guided tour.", + "", + "```diff", + patch, + "```", + ].join("\n"); + } +} + +export interface TourClaudeCommandResult { + command: string[]; + stdinPrompt: string; +} + +export function buildTourClaudeCommand(prompt: string, model: string = "sonnet", effort?: string): TourClaudeCommandResult { + const allowedTools = [ + "Agent", "Read", "Glob", "Grep", + "Bash(git status:*)", "Bash(git diff:*)", "Bash(git log:*)", + "Bash(git show:*)", "Bash(git blame:*)", "Bash(git branch:*)", + "Bash(git grep:*)", "Bash(git ls-remote:*)", "Bash(git ls-tree:*)", + "Bash(git merge-base:*)", "Bash(git remote:*)", "Bash(git rev-parse:*)", + "Bash(git show-ref:*)", + "Bash(gh pr view:*)", "Bash(gh pr diff:*)", "Bash(gh pr list:*)", + "Bash(gh api repos/*/*/pulls/*)", "Bash(gh api repos/*/*/pulls/*/files*)", + // The tour prompt follows linked issues (`Fixes #123`, `Closes owner/repo#456`), + // so the allowlist has to permit the issue-read commands. + "Bash(gh issue view:*)", "Bash(gh api repos/*/*/issues/*)", + "Bash(glab mr view:*)", "Bash(glab mr diff:*)", + "Bash(glab issue view:*)", + "Bash(wc:*)", + ].join(","); + + const disallowedTools = [ + "Edit", "Write", "NotebookEdit", "WebFetch", "WebSearch", + "Bash(python:*)", "Bash(python3:*)", "Bash(node:*)", "Bash(npx:*)", + "Bash(bun:*)", "Bash(bunx:*)", "Bash(sh:*)", "Bash(bash:*)", "Bash(zsh:*)", + "Bash(curl:*)", "Bash(wget:*)", + ].join(","); + + return { + command: [ + "claude", "-p", + "--permission-mode", "dontAsk", + "--output-format", "stream-json", + "--verbose", + "--json-schema", TOUR_SCHEMA_JSON, + "--no-session-persistence", + "--model", model, + ...(effort ? ["--effort", effort] : []), + "--tools", "Agent,Bash,Read,Glob,Grep", + "--allowedTools", allowedTools, + "--disallowedTools", disallowedTools, + ], + stdinPrompt: prompt, + }; +} + +const TOUR_SCHEMA_DIR = join(homedir(), ".plannotator"); +const TOUR_SCHEMA_FILE = join(TOUR_SCHEMA_DIR, "tour-schema.json"); +let tourSchemaMaterialized = false; + +async function ensureTourSchemaFile(): Promise { + if (!tourSchemaMaterialized) { + await mkdir(TOUR_SCHEMA_DIR, { recursive: true }); + await writeFile(TOUR_SCHEMA_FILE, TOUR_SCHEMA_JSON); + tourSchemaMaterialized = true; + } + return TOUR_SCHEMA_FILE; +} + +export function generateTourOutputPath(): string { + return join(tmpdir(), `plannotator-tour-${crypto.randomUUID()}.json`); +} + +export async function buildTourCodexCommand(options: { + cwd: string; + outputPath: string; + prompt: string; + model?: string; + reasoningEffort?: string; + fastMode?: boolean; +}): Promise { + const { cwd, outputPath, prompt, model, reasoningEffort, fastMode } = options; + const schemaPath = await ensureTourSchemaFile(); + + const command = [ + "codex", + // Global flags must precede the "exec" subcommand for the Codex CLI. + ...(model ? ["-m", model] : []), + ...(reasoningEffort ? ["-c", `model_reasoning_effort=${reasoningEffort}`] : []), + ...(fastMode ? ["-c", "service_tier=fast"] : []), + "exec", + "--output-schema", schemaPath, + "-o", outputPath, + "--full-auto", "--ephemeral", + "-C", cwd, + prompt, + ]; + + return command; +} + +export function parseTourStreamOutput(stdout: string): CodeTourOutput | null { + if (!stdout.trim()) return null; + + const lines = stdout.trim().split('\n'); + for (let i = lines.length - 1; i >= 0; i--) { + const line = lines[i].trim(); + if (!line) continue; + + try { + const event = JSON.parse(line); + if (event.type === 'result') { + if (event.is_error) return null; + const output = event.structured_output; + // A tour with no stops isn't a tour — treat as invalid so the UI + // error state fires instead of rendering an empty walkthrough. + if (!output || !Array.isArray(output.stops) || output.stops.length === 0) return null; + return output as CodeTourOutput; + } + } catch { + // Not valid JSON — skip + } + } + + return null; +} + +export async function parseTourFileOutput(outputPath: string): Promise { + try { + const text = await readFile(outputPath, "utf-8"); + try { await unlink(outputPath); } catch { /* ignore */ } + if (!text.trim()) return null; + const parsed = JSON.parse(text); + // A tour with no stops isn't a tour — treat as invalid so the UI + // error state fires instead of rendering an empty walkthrough. + if (!parsed || !Array.isArray(parsed.stops) || parsed.stops.length === 0) return null; + return parsed as CodeTourOutput; + } catch { + try { await unlink(outputPath); } catch { /* ignore */ } + return null; + } +} + +export interface TourSessionBuildCommandOptions { + cwd: string; + patch: string; + diffType: DiffType; + options?: { defaultBranch?: string; hasLocalAccess?: boolean }; + prMetadata?: PRMetadata; + config?: Record; +} + +export interface TourSessionBuildCommandResult { + command: string[]; + outputPath?: string; + captureStdout?: boolean; + stdinPrompt?: string; + cwd?: string; + label?: string; + prompt?: string; + engine: "claude" | "codex"; + model: string; + effort?: string; + reasoningEffort?: string; + fastMode?: boolean; +} + +export interface TourSessionJobSummary { + correctness: string; + explanation: string; + confidence: number; +} + +export interface TourSessionJobRef { + id: string; + engine?: string; +} + +export interface TourSessionOnJobCompleteOptions { + job: TourSessionJobRef; + meta: { outputPath?: string; stdout?: string }; +} + +export interface TourSession { + tourResults: Map; + tourChecklists: Map; + buildCommand(opts: TourSessionBuildCommandOptions): Promise; + onJobComplete(opts: TourSessionOnJobCompleteOptions): Promise<{ summary: TourSessionJobSummary | null }>; + getTour(jobId: string): (CodeTourOutput & { checklist: boolean[] }) | null; + saveChecklist(jobId: string, checked: boolean[]): void; +} + +export function createTourSession(): TourSession { + const tourResults = new Map(); + const tourChecklists = new Map(); + + return { + tourResults, + tourChecklists, + + async buildCommand({ cwd, patch, diffType, options, prMetadata, config }) { + const engine = (typeof config?.engine === "string" ? config.engine : "claude") as "claude" | "codex"; + const explicitModel = typeof config?.model === "string" && config.model ? config.model : null; + // "sonnet" is a Claude model, so we must NOT pass it to Codex when no model + // is explicitly selected. Leave Codex model blank and let its CLI default pick. + const model = explicitModel ?? (engine === "codex" ? "" : "sonnet"); + const reasoningEffort = typeof config?.reasoningEffort === "string" && config.reasoningEffort ? config.reasoningEffort : undefined; + const effort = typeof config?.effort === "string" && config.effort ? config.effort : undefined; + const fastMode = config?.fastMode === true; + const userMessage = buildTourUserMessage(patch, diffType, options, prMetadata); + const prompt = TOUR_REVIEW_PROMPT + "\n\n---\n\n" + userMessage; + + if (engine === "codex") { + const outputPath = generateTourOutputPath(); + const command = await buildTourCodexCommand({ cwd, outputPath, prompt, model: model || undefined, reasoningEffort, fastMode }); + return { command, outputPath, prompt, label: "Code Tour", engine: "codex", model, reasoningEffort, fastMode: fastMode || undefined }; + } + + const { command, stdinPrompt } = buildTourClaudeCommand(prompt, model, effort); + return { command, stdinPrompt, prompt, cwd, label: "Code Tour", captureStdout: true, engine: "claude", model, effort }; + }, + + async onJobComplete({ job, meta }) { + let output: CodeTourOutput | null = null; + if (job.engine === "codex" && meta.outputPath) { + output = await parseTourFileOutput(meta.outputPath); + } else if (meta.stdout) { + output = parseTourStreamOutput(meta.stdout); + } + + if (!output) { + console.error(`[tour] Failed to parse output for job ${job.id}`); + return { summary: null }; + } + + tourResults.set(job.id, output); + const summary: TourSessionJobSummary = { + correctness: "Tour Generated", + explanation: `${output.stops.length} stop${output.stops.length !== 1 ? "s" : ""}, ${output.qa_checklist.length} QA item${output.qa_checklist.length !== 1 ? "s" : ""}`, + confidence: 1.0, + }; + return { summary }; + }, + + getTour(jobId) { + const tour = tourResults.get(jobId); + if (!tour) return null; + return { ...tour, checklist: tourChecklists.get(jobId) ?? [] }; + }, + + saveChecklist(jobId, checked) { + tourChecklists.set(jobId, checked); + }, + }; +} diff --git a/packages/shared/agent-jobs.ts b/packages/shared/agent-jobs.ts index 41434a51..20817e70 100644 --- a/packages/shared/agent-jobs.ts +++ b/packages/shared/agent-jobs.ts @@ -19,8 +19,18 @@ export interface AgentJobInfo { id: string; /** Source identifier for external annotations — "agent-{id prefix}". */ source: string; - /** Provider that spawned this job — "claude", "codex", "shell", etc. */ + /** Provider that spawned this job — "claude", "codex", "tour", "shell", etc. */ provider: string; + /** Underlying engine used (e.g., "claude" or "codex"). Set when provider is "tour". */ + engine?: string; + /** Model used (e.g., "sonnet", "opus"). Set when provider is "tour" with Claude engine. */ + model?: string; + /** Claude --effort level (e.g., "low", "medium", "high", "xhigh", "max"). */ + effort?: string; + /** Codex reasoning effort level (e.g., "high", "medium"). */ + reasoningEffort?: string; + /** Whether Codex fast mode (service_tier=fast) was enabled. */ + fastMode?: boolean; /** Human-readable label for the job. */ label: string; /** Current lifecycle status. */ diff --git a/packages/shared/package.json b/packages/shared/package.json index a8a9389a..a94b401e 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -25,7 +25,8 @@ "./improvement-hooks": "./improvement-hooks.ts", "./worktree": "./worktree.ts", "./html-to-markdown": "./html-to-markdown.ts", - "./url-to-markdown": "./url-to-markdown.ts" + "./url-to-markdown": "./url-to-markdown.ts", + "./tour": "./tour.ts" }, "dependencies": { "@joplin/turndown-plugin-gfm": "^1.0.64", diff --git a/packages/shared/tour.ts b/packages/shared/tour.ts new file mode 100644 index 00000000..832f351a --- /dev/null +++ b/packages/shared/tour.ts @@ -0,0 +1,61 @@ +export interface TourDiffAnchor { + /** Relative file path within the repo. */ + file: string; + /** Start line in the new file (post-change). */ + line: number; + /** End line in the new file. */ + end_line: number; + /** Raw unified diff hunk for this anchor. */ + hunk: string; + /** One-line chip label, e.g. "Add retry logic". */ + label: string; +} + +export interface TourKeyTakeaway { + /** One sentence — the takeaway. */ + text: string; + /** Severity for visual styling. */ + severity: "info" | "important" | "warning"; +} + +export interface TourStop { + /** Short chapter title, friendly tone. */ + title: string; + /** ONE sentence — the headline for this stop. Scannable without expanding. */ + gist: string; + /** 2-3 sentences of additional context. Only shown when expanded. */ + detail: string; + /** Connective phrase to the next stop, e.g. "Building on that..." (empty for last stop). */ + transition: string; + /** Diff anchors — the code locations this stop references. */ + anchors: TourDiffAnchor[]; +} + +export interface TourQAItem { + /** Product-level verification question. */ + question: string; + /** Indices into stops[] that this question relates to. */ + stop_indices: number[]; +} + +export interface CodeTourOutput { + /** One-line title for the entire tour. */ + title: string; + /** 1-2 sentence friendly greeting + summary. Conversational, not formal. */ + greeting: string; + /** 1-3 sentences: why this changeset exists — the motivation/problem being solved. */ + intent: string; + /** What things looked like before this changeset — one sentence. */ + before: string; + /** What things look like after — one sentence. */ + after: string; + /** 3-5 key takeaways — the most critical info, scannable at a glance. */ + key_takeaways: TourKeyTakeaway[]; + /** Ordered tour stops — the detailed walk-through. */ + stops: TourStop[]; + /** Product-level QA checklist. */ + qa_checklist: TourQAItem[]; +} + +/** UI-side tour shape: server output extended with persisted checklist state. */ +export type CodeTourData = CodeTourOutput & { checklist: boolean[] }; diff --git a/packages/ui/components/AgentsTab.tsx b/packages/ui/components/AgentsTab.tsx index 3cd1eca9..d82c2c7e 100644 --- a/packages/ui/components/AgentsTab.tsx +++ b/packages/ui/components/AgentsTab.tsx @@ -1,12 +1,67 @@ -import React, { useState, useEffect, useRef, useMemo } from 'react'; +import React, { useState, useEffect, useMemo } from 'react'; import type { AgentJobInfo, AgentCapabilities } from '../types'; import { isTerminalStatus } from '@plannotator/shared/agent-jobs'; import { ReviewAgentsIcon } from './ReviewAgentsIcon'; +import { useAgentSettings } from '../hooks/useAgentSettings'; + +// --- Agent option catalogs (shared across provider + tour-engine dropdowns) --- + +const CLAUDE_MODELS: Array<{ value: string; label: string }> = [ + { value: 'claude-opus-4-7', label: 'Opus 4.7' }, + { value: 'claude-opus-4-6', label: 'Opus 4.6' }, + { value: 'claude-sonnet-4-6', label: 'Sonnet 4.6' }, +]; + +const CLAUDE_EFFORT: Array<{ value: string; label: string }> = [ + { value: 'low', label: 'Low' }, + { value: 'medium', label: 'Medium' }, + { value: 'high', label: 'High' }, + { value: 'xhigh', label: 'XHigh' }, + { value: 'max', label: 'Max' }, +]; + +const CODEX_MODELS: Array<{ value: string; label: string }> = [ + { value: 'gpt-5.4', label: 'GPT-5.4' }, + { value: 'gpt-5.3-codex', label: 'GPT-5.3 Codex' }, + { value: 'gpt-5.3-codex-spark', label: 'GPT-5.3 Codex Spark' }, + { value: 'gpt-5.2-codex', label: 'GPT-5.2 Codex' }, + { value: 'gpt-5.2', label: 'GPT-5.2' }, + { value: 'gpt-5.1-codex-max', label: 'GPT-5.1 Codex Max' }, + { value: 'gpt-5.1-codex-mini', label: 'GPT-5.1 Codex Mini' }, + { value: 'gpt-5.4-mini', label: 'GPT-5.4 Mini' }, +]; + +const CODEX_REASONING: Array<{ value: string; label: string }> = [ + { value: 'minimal', label: 'Minimal' }, + { value: 'low', label: 'Low' }, + { value: 'medium', label: 'Medium' }, + { value: 'high', label: 'High' }, + { value: 'xhigh', label: 'XHigh' }, +]; + +// Tour Claude reuses the same effort levels but offers a different model set. +const TOUR_CLAUDE_MODELS: Array<{ value: string; label: string }> = [ + { value: 'sonnet', label: 'Sonnet (fast)' }, + { value: 'opus', label: 'Opus (thorough)' }, +]; + +// Dropdown labels: action first, provider second. Groups visually by action — +// you scan two "Code Review" entries and one "Code Tour" instead of three raw +// CLI names. +const PROVIDER_DROPDOWN_LABEL: Record = { + claude: 'Code Review · Claude', + codex: 'Code Review · Codex', + tour: 'Code Tour', +}; + +function providerDropdownLabel(id: string, fallback: string): string { + return PROVIDER_DROPDOWN_LABEL[id] ?? fallback; +} interface AgentsTabProps { jobs: AgentJobInfo[]; capabilities: AgentCapabilities | null; - onLaunch: (params: { provider?: string; command?: string[]; label?: string }) => void; + onLaunch: (params: { provider?: string; command?: string[]; label?: string; engine?: string; model?: string; reasoningEffort?: string; effort?: string; fastMode?: boolean }) => void; onKillJob: (id: string) => void; onKillAll: () => void; externalAnnotations: Array<{ source?: string }>; @@ -80,13 +135,53 @@ function StatusBadge({ status }: { status: AgentJobInfo['status'] }) { // --- Provider badge --- -function ProviderBadge({ provider }: { provider: string }) { - const label = - provider === 'claude' ? 'Claude' : - provider === 'codex' ? 'Codex' : - 'Shell'; +// Lookup a human label from the catalogs; fall back to the raw id. +function catalogLabel(list: Array<{ value: string; label: string }>, value: string): string { + return list.find((o) => o.value === value)?.label ?? value; +} + +function formatModel(provider: string, engine: string | undefined, model: string): string { + if (provider === 'codex' || engine === 'codex') return catalogLabel(CODEX_MODELS, model); + if (provider === 'tour' && engine === 'claude') return catalogLabel(TOUR_CLAUDE_MODELS, model); + return catalogLabel(CLAUDE_MODELS, model); +} + +function formatEffort(value: string): string { + return catalogLabel(CLAUDE_EFFORT, value); +} + +function formatReasoning(value: string): string { + return catalogLabel(CODEX_REASONING, value); +} + +function ProviderBadge({ provider, engine, model, effort, reasoningEffort, fastMode }: { provider: string; engine?: string; model?: string; effort?: string; reasoningEffort?: string; fastMode?: boolean }) { + let label: string; + if (provider === 'tour') { + const engineLabel = engine === 'codex' ? 'Codex' : 'Claude'; + const parts = [`Tour · ${engineLabel}`]; + if (model) parts.push(formatModel(provider, engine, model)); + if (engine === 'claude' && effort) parts.push(formatEffort(effort)); + if (engine === 'codex' && reasoningEffort) parts.push(formatReasoning(reasoningEffort)); + if (fastMode) parts.push('Fast'); + label = parts.join(' · '); + } else if (provider === 'codex') { + const parts = ['Codex']; + if (model) parts.push(formatModel(provider, engine, model)); + if (reasoningEffort) parts.push(formatReasoning(reasoningEffort)); + if (fastMode) parts.push('Fast'); + label = parts.join(' · '); + } else if (provider === 'claude') { + const parts = ['Claude']; + if (model) parts.push(formatModel(provider, engine, model)); + if (effort) parts.push(formatEffort(effort)); + label = parts.join(' · '); + } else { + label = 'Shell'; + } return ( - + {label} ); @@ -122,7 +217,7 @@ function JobCard({ >
- + {job.label}
@@ -183,20 +278,49 @@ export const AgentsTab: React.FC = ({ externalAnnotations, onOpenJobDetail, }) => { - const [selectedProvider, setSelectedProvider] = useState(''); const [expandedJobId, setExpandedJobId] = useState(null); - const initializedRef = useRef(false); + const settings = useAgentSettings(); + const { + selectedProvider, + tourEngine, + claudeModel, + claudeEffort, + codexModel, + codexReasoning, + codexFast, + tourClaudeModel, + tourClaudeEffort, + tourCodexModel, + tourCodexReasoning, + tourCodexFast, + setSelectedProvider, + setTourEngine, + setClaudeModel, + setClaudeEffort, + setCodexModel, + setCodexReasoning, + setCodexFast, + setTourClaudeModel, + setTourClaudeEffort, + setTourCodexModel, + setTourCodexReasoning, + setTourCodexFast, + } = settings; - // Set default provider once capabilities load + // Reconcile provider + tour engine against live capabilities. Runs when + // capabilities change or the stored selection becomes invalid. useEffect(() => { - if (capabilities && !initializedRef.current) { - const firstAvailable = capabilities.providers.find((p) => p.available); - if (firstAvailable) { - setSelectedProvider(firstAvailable.id); - initializedRef.current = true; - } + if (!capabilities) return; + const available = capabilities.providers.filter((p) => p.available); + if (available.length === 0) return; + if (!selectedProvider || !available.some((p) => p.id === selectedProvider)) { + setSelectedProvider(available[0].id); } - }, [capabilities]); + const hasClaude = available.some((p) => p.id === 'claude'); + const hasCodex = available.some((p) => p.id === 'codex'); + if (tourEngine === 'claude' && !hasClaude && hasCodex) setTourEngine('codex'); + else if (tourEngine === 'codex' && !hasCodex && hasClaude) setTourEngine('claude'); + }, [capabilities, selectedProvider, tourEngine, setSelectedProvider, setTourEngine]); const availableProviders = useMemo( () => capabilities?.providers.filter((p) => p.available) ?? [], @@ -229,13 +353,34 @@ export const AgentsTab: React.FC = ({ [jobs], ); + // Detect which engines are available for tour config + const claudeAvailable = capabilities?.providers.some((p) => p.id === 'claude' && p.available) ?? false; + const codexAvailable = capabilities?.providers.some((p) => p.id === 'codex' && p.available) ?? false; + + type LaunchParams = Parameters[0]; + const buildLaunch: Record LaunchParams> = { + claude: () => ({ provider: 'claude', label: 'Code Review', model: claudeModel, effort: claudeEffort }), + codex: () => ({ + provider: 'codex', + label: 'Code Review', + model: codexModel, + reasoningEffort: codexReasoning, + ...(codexFast && { fastMode: true }), + }), + tour: () => ({ + provider: 'tour', + label: 'Code Tour', + engine: tourEngine, + model: tourEngine === 'claude' ? tourClaudeModel : tourCodexModel, + ...(tourEngine === 'claude' + ? { effort: tourClaudeEffort } + : { reasoningEffort: tourCodexReasoning, ...(tourCodexFast && { fastMode: true }) }), + }), + }; + const handleLaunch = () => { if (!selectedProvider) return; - const provider = availableProviders.find((p) => p.id === selectedProvider); - onLaunch({ - provider: selectedProvider, - label: provider ? `${provider.name} Review` : selectedProvider, - }); + onLaunch(buildLaunch[selectedProvider]?.() ?? { provider: selectedProvider, label: selectedProvider }); }; return ( @@ -246,19 +391,19 @@ export const AgentsTab: React.FC = ({
{availableProviders.length > 1 ? ( ) : ( - {availableProviders[0]?.name} + {availableProviders[0] ? providerDropdownLabel(availableProviders[0].id, availableProviders[0].name) : ''} )}
+ + {/* Claude model + effort config */} + {selectedProvider === 'claude' && ( +
+
+ Model + +
+
+ Effort + +
+
+ )} + + {/* Codex model + reasoning + fast mode config */} + {selectedProvider === 'codex' && ( +
+
+ Model + +
+
+ Reasoning + +
+
+ Fast + +
+
+ )} + + {/* Tour engine/model config — only shown when tour is selected */} + {selectedProvider === 'tour' && ( +
+ {/* Engine selector */} + {claudeAvailable && codexAvailable && ( +
+ Engine + + +
+ )} + + {/* Model selector — engine-specific options */} +
+ Model + +
+ + {/* Claude-only: effort level */} + {tourEngine === 'claude' && ( +
+ Effort + +
+ )} + + {/* Codex-only: reasoning effort + fast mode */} + {tourEngine === 'codex' && ( + <> +
+ Reasoning + +
+
+ Fast + +
+ + )} +
+ )}
)} diff --git a/packages/ui/hooks/useAgentJobs.ts b/packages/ui/hooks/useAgentJobs.ts index 016ef3be..b2591ede 100644 --- a/packages/ui/hooks/useAgentJobs.ts +++ b/packages/ui/hooks/useAgentJobs.ts @@ -22,7 +22,7 @@ interface UseAgentJobsReturn { jobs: AgentJobInfo[]; jobLogs: Map; capabilities: AgentCapabilities | null; - launchJob: (params: { provider?: string; command?: string[]; label?: string }) => Promise; + launchJob: (params: { provider?: string; command?: string[]; label?: string; engine?: string; model?: string; reasoningEffort?: string; effort?: string; fastMode?: boolean }) => Promise; killJob: (id: string) => Promise; killAll: () => Promise; } @@ -163,6 +163,11 @@ export function useAgentJobs( provider?: string; command?: string[]; label?: string; + engine?: string; + model?: string; + reasoningEffort?: string; + effort?: string; + fastMode?: boolean; }): Promise => { try { const res = await fetch(JOBS_URL, { diff --git a/packages/ui/hooks/useAgentSettings.test.ts b/packages/ui/hooks/useAgentSettings.test.ts new file mode 100644 index 00000000..e7486651 --- /dev/null +++ b/packages/ui/hooks/useAgentSettings.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, test } from "bun:test"; +import { sanitizeCodexPerModel, DEFAULT_CODEX_REASONING } from "./useAgentSettings"; + +describe("sanitizeCodexPerModel", () => { + test("returns empty object for undefined/empty input", () => { + expect(sanitizeCodexPerModel(undefined)).toEqual({}); + expect(sanitizeCodexPerModel({})).toEqual({}); + }); + + test("drops stale reasoning: 'none' entry when fast is false", () => { + const result = sanitizeCodexPerModel({ + "gpt-5.3-codex": { reasoning: "none", fast: false }, + }); + expect(result).toEqual({}); + }); + + test("retains entry with reasoning: 'none' but fast: true, replacing reasoning with default", () => { + const result = sanitizeCodexPerModel({ + "gpt-5.3-codex": { reasoning: "none", fast: true }, + }); + expect(result).toEqual({ + "gpt-5.3-codex": { reasoning: DEFAULT_CODEX_REASONING, fast: true }, + }); + }); + + test("passes through valid entries unchanged", () => { + const input = { + "gpt-5.3-codex": { reasoning: "high", fast: false }, + "gpt-5.3-pro": { reasoning: "medium", fast: true }, + }; + expect(sanitizeCodexPerModel(input)).toEqual(input); + }); + + test("skips non-object entries", () => { + const input = { + valid: { reasoning: "high", fast: false }, + nullish: null as unknown as { reasoning: string; fast: boolean }, + stringy: "bad" as unknown as { reasoning: string; fast: boolean }, + }; + expect(sanitizeCodexPerModel(input)).toEqual({ + valid: { reasoning: "high", fast: false }, + }); + }); +}); diff --git a/packages/ui/hooks/useAgentSettings.ts b/packages/ui/hooks/useAgentSettings.ts new file mode 100644 index 00000000..1da005ff --- /dev/null +++ b/packages/ui/hooks/useAgentSettings.ts @@ -0,0 +1,223 @@ +import { useCallback, useEffect, useState } from 'react'; +import { getItem, setItem } from '../utils/storage'; + +const COOKIE_KEY = 'plannotator.agents'; + +export const DEFAULT_CLAUDE_MODEL = 'claude-opus-4-7'; +export const DEFAULT_CLAUDE_EFFORT = 'high'; +export const DEFAULT_CODEX_MODEL = 'gpt-5.3-codex'; +export const DEFAULT_CODEX_REASONING = 'high'; +export const DEFAULT_CODEX_FAST = false; +export const DEFAULT_TOUR_CLAUDE_MODEL = 'sonnet'; +export const DEFAULT_TOUR_CLAUDE_EFFORT = 'medium'; +export const DEFAULT_TOUR_CODEX_MODEL = 'gpt-5.3-codex'; +export const DEFAULT_TOUR_CODEX_REASONING = 'medium'; +export const DEFAULT_TOUR_CODEX_FAST = false; + +interface ClaudeSection { + model: string; + perModel: Record; +} + +interface CodexSection { + model: string; + perModel: Record; +} + +interface AgentSettingsState { + selectedProvider?: string; + tourEngine: 'claude' | 'codex'; + claude: ClaudeSection; + codex: CodexSection; + tourClaude: ClaudeSection; + tourCodex: CodexSection; +} + +const initialState: AgentSettingsState = { + tourEngine: 'claude', + claude: { model: DEFAULT_CLAUDE_MODEL, perModel: {} }, + codex: { model: DEFAULT_CODEX_MODEL, perModel: {} }, + tourClaude: { model: DEFAULT_TOUR_CLAUDE_MODEL, perModel: {} }, + tourCodex: { model: DEFAULT_TOUR_CODEX_MODEL, perModel: {} }, +}; + +// One-shot migration: drop any cached "none" codex reasoning entries. The +// dropdown no longer offers "None" (codex-rs rejects it as a config value); +// fall back to the default instead of shipping an invalid flag. +export function sanitizeCodexPerModel( + perModel: Record | undefined, +): Record { + if (!perModel) return {}; + const out: Record = {}; + for (const [model, entry] of Object.entries(perModel)) { + if (!entry || typeof entry !== 'object') continue; + if (entry.reasoning === 'none') { + if (entry.fast) out[model] = { reasoning: DEFAULT_CODEX_REASONING, fast: true }; + continue; + } + out[model] = entry; + } + return out; +} + +function readCookie(): AgentSettingsState { + const raw = getItem(COOKIE_KEY); + if (!raw) return initialState; + try { + const parsed = JSON.parse(raw); + return { + selectedProvider: typeof parsed.selectedProvider === 'string' ? parsed.selectedProvider : undefined, + tourEngine: parsed.tourEngine === 'codex' ? 'codex' : 'claude', + claude: { + model: typeof parsed.claude?.model === 'string' ? parsed.claude.model : DEFAULT_CLAUDE_MODEL, + perModel: parsed.claude?.perModel ?? {}, + }, + codex: { + model: typeof parsed.codex?.model === 'string' ? parsed.codex.model : DEFAULT_CODEX_MODEL, + perModel: sanitizeCodexPerModel(parsed.codex?.perModel), + }, + tourClaude: { + model: typeof parsed.tourClaude?.model === 'string' ? parsed.tourClaude.model : DEFAULT_TOUR_CLAUDE_MODEL, + perModel: parsed.tourClaude?.perModel ?? {}, + }, + tourCodex: { + model: typeof parsed.tourCodex?.model === 'string' ? parsed.tourCodex.model : DEFAULT_TOUR_CODEX_MODEL, + perModel: sanitizeCodexPerModel(parsed.tourCodex?.perModel), + }, + }; + } catch { + return initialState; + } +} + +export function useAgentSettings() { + const [state, setState] = useState(readCookie); + + useEffect(() => { + setItem(COOKIE_KEY, JSON.stringify(state)); + }, [state]); + + const setSelectedProvider = useCallback((id: string) => { + setState((s) => ({ ...s, selectedProvider: id })); + }, []); + + const setTourEngine = useCallback((engine: 'claude' | 'codex') => { + setState((s) => ({ ...s, tourEngine: engine })); + }, []); + + const setClaudeModel = useCallback((model: string) => { + setState((s) => ({ ...s, claude: { ...s.claude, model } })); + }, []); + + const patchClaude = useCallback( + (section: 'claude' | 'tourClaude', patch: Partial<{ effort: string }>) => { + setState((s) => { + const cur = s[section]; + const prev = cur.perModel[cur.model] ?? { effort: '' }; + return { + ...s, + [section]: { + ...cur, + perModel: { ...cur.perModel, [cur.model]: { ...prev, ...patch } }, + }, + }; + }); + }, + [], + ); + + const setClaudeEffort = useCallback( + (effort: string) => patchClaude('claude', { effort }), + [patchClaude], + ); + + const setCodexModel = useCallback((model: string) => { + setState((s) => ({ ...s, codex: { ...s.codex, model } })); + }, []); + + const patchCodex = useCallback( + ( + section: 'codex' | 'tourCodex', + patch: Partial<{ reasoning: string; fast: boolean }>, + defaults: { reasoning: string; fast: boolean }, + ) => { + setState((s) => { + const cur = s[section]; + const prev = cur.perModel[cur.model] ?? defaults; + return { + ...s, + [section]: { + ...cur, + perModel: { ...cur.perModel, [cur.model]: { ...prev, ...patch } }, + }, + }; + }); + }, + [], + ); + + const setCodexReasoning = useCallback( + (reasoning: string) => patchCodex('codex', { reasoning }, { reasoning: DEFAULT_CODEX_REASONING, fast: DEFAULT_CODEX_FAST }), + [patchCodex], + ); + const setCodexFast = useCallback( + (fast: boolean) => patchCodex('codex', { fast }, { reasoning: DEFAULT_CODEX_REASONING, fast: DEFAULT_CODEX_FAST }), + [patchCodex], + ); + + const setTourClaudeModel = useCallback((model: string) => { + setState((s) => ({ ...s, tourClaude: { ...s.tourClaude, model } })); + }, []); + + const setTourClaudeEffort = useCallback( + (effort: string) => patchClaude('tourClaude', { effort }), + [patchClaude], + ); + + const setTourCodexModel = useCallback((model: string) => { + setState((s) => ({ ...s, tourCodex: { ...s.tourCodex, model } })); + }, []); + + const setTourCodexReasoning = useCallback( + (reasoning: string) => patchCodex('tourCodex', { reasoning }, { reasoning: DEFAULT_TOUR_CODEX_REASONING, fast: DEFAULT_TOUR_CODEX_FAST }), + [patchCodex], + ); + const setTourCodexFast = useCallback( + (fast: boolean) => patchCodex('tourCodex', { fast }, { reasoning: DEFAULT_TOUR_CODEX_REASONING, fast: DEFAULT_TOUR_CODEX_FAST }), + [patchCodex], + ); + + const claudeEffort = state.claude.perModel[state.claude.model]?.effort ?? DEFAULT_CLAUDE_EFFORT; + const codexReasoning = state.codex.perModel[state.codex.model]?.reasoning ?? DEFAULT_CODEX_REASONING; + const codexFast = state.codex.perModel[state.codex.model]?.fast ?? DEFAULT_CODEX_FAST; + const tourClaudeEffort = state.tourClaude.perModel[state.tourClaude.model]?.effort ?? DEFAULT_TOUR_CLAUDE_EFFORT; + const tourCodexReasoning = state.tourCodex.perModel[state.tourCodex.model]?.reasoning ?? DEFAULT_TOUR_CODEX_REASONING; + const tourCodexFast = state.tourCodex.perModel[state.tourCodex.model]?.fast ?? DEFAULT_TOUR_CODEX_FAST; + + return { + selectedProvider: state.selectedProvider, + tourEngine: state.tourEngine, + claudeModel: state.claude.model, + claudeEffort, + codexModel: state.codex.model, + codexReasoning, + codexFast, + tourClaudeModel: state.tourClaude.model, + tourClaudeEffort, + tourCodexModel: state.tourCodex.model, + tourCodexReasoning, + tourCodexFast, + setSelectedProvider, + setTourEngine, + setClaudeModel, + setClaudeEffort, + setCodexModel, + setCodexReasoning, + setCodexFast, + setTourClaudeModel, + setTourClaudeEffort, + setTourCodexModel, + setTourCodexReasoning, + setTourCodexFast, + }; +}