From b716d8e0fbeb1403d32a9bdbcbea1b33deb10d58 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Thu, 9 Apr 2026 23:17:00 -0700 Subject: [PATCH 1/4] feat(review): configurable default diff type with first-run setup Change the default review diff type from "uncommitted" to "unstaged" (matching `git diff` semantics) and make it user-configurable via Settings > Display. A first-run dialog prompts users to choose their preferred default on their first local review session. - Add `defaultDiffType` to DiffOptions config (unstaged/uncommitted/staged) - Add `resolveDefaultDiffType()` helper in shared config - Register setting in config store with cookie + config.json sync - Add SegmentedControl in ReviewDisplayTab - Add DiffTypeSetupDialog with sequenced display (waits for AI setup) - Wire config into all three entry points (hook, opencode, pi) - Preserve P4 default ("p4-default") in hook entry point Closes #287 Co-authored-by: Hendrik Richert For provenance purposes, this commit was AI assisted. --- apps/hook/server/index.ts | 5 +- apps/opencode-plugin/commands.ts | 7 +- apps/pi-extension/plannotator-browser.ts | 3 +- packages/review-editor/App.tsx | 22 ++++ packages/shared/config.ts | 11 ++ .../ui/components/DiffTypeSetupDialog.tsx | 103 ++++++++++++++++++ packages/ui/components/Settings.tsx | 17 +++ packages/ui/config/settings.ts | 15 +++ packages/ui/utils/diffTypeSetup.ts | 18 +++ 9 files changed, 196 insertions(+), 5 deletions(-) create mode 100644 packages/ui/components/DiffTypeSetupDialog.tsx create mode 100644 packages/ui/utils/diffTypeSetup.ts diff --git a/apps/hook/server/index.ts b/apps/hook/server/index.ts index 265ba0ff..7572d43c 100644 --- a/apps/hook/server/index.ts +++ b/apps/hook/server/index.ts @@ -64,6 +64,7 @@ import { handleAnnotateServerReady, } from "@plannotator/server/annotate"; import { type DiffType, getVcsContext, runVcsDiff, gitRuntime } from "@plannotator/server/vcs"; +import { loadConfig, resolveDefaultDiffType } from "@plannotator/shared/config"; import { fetchRef, createWorktree, removeWorktree, ensureObjectAvailable } from "@plannotator/shared/worktree"; import { parsePRUrl, checkPRAuth, fetchPR, getCliName, getCliInstallUrl, getMRLabel, getMRNumberLabel, getDisplayRepo } from "@plannotator/server/pr"; import { writeRemoteShareLink } from "@plannotator/server/share-url"; @@ -379,7 +380,7 @@ if (args[0] === "sessions") { } else { // --- Local Review Mode --- gitContext = await getVcsContext(); - initialDiffType = gitContext.vcsType === "p4" ? "p4-default" : "uncommitted"; + initialDiffType = gitContext.vcsType === "p4" ? "p4-default" : resolveDefaultDiffType(loadConfig()); const diffResult = await runVcsDiff(initialDiffType, gitContext.defaultBranch); rawPatch = diffResult.patch; gitRef = diffResult.label; @@ -394,7 +395,7 @@ if (args[0] === "sessions") { gitRef, error: diffError, origin: detectedOrigin, - diffType: gitContext ? (initialDiffType ?? "uncommitted") : undefined, + diffType: gitContext ? (initialDiffType ?? "unstaged") : undefined, gitContext, prMetadata, agentCwd, diff --git a/apps/opencode-plugin/commands.ts b/apps/opencode-plugin/commands.ts index 2218c271..523f4723 100644 --- a/apps/opencode-plugin/commands.ts +++ b/apps/opencode-plugin/commands.ts @@ -20,6 +20,7 @@ import { } from "@plannotator/server/annotate"; import { getGitContext, runGitDiffWithContext } from "@plannotator/server/git"; import { parsePRUrl, checkPRAuth, fetchPR, getCliName, getMRLabel, getMRNumberLabel, getDisplayRepo } from "@plannotator/server/pr"; +import { loadConfig, resolveDefaultDiffType } from "@plannotator/shared/config"; import { resolveMarkdownFile } from "@plannotator/shared/resolve-file"; /** Shared dependencies injected by the plugin */ @@ -45,6 +46,7 @@ export async function handleReviewCommand( let rawPatch: string; let gitRef: string; let diffError: string | undefined; + let userDiffType: import("@plannotator/shared/config").DefaultDiffType | undefined; let gitContext: Awaited> | undefined; let prMetadata: Awaited>["metadata"] | undefined; @@ -78,7 +80,8 @@ export async function handleReviewCommand( client.app.log({ level: "info", message: "Opening code review UI..." }); gitContext = await getGitContext(directory); - const diffResult = await runGitDiffWithContext("uncommitted", gitContext); + userDiffType = resolveDefaultDiffType(loadConfig()); + const diffResult = await runGitDiffWithContext(userDiffType, gitContext); rawPatch = diffResult.patch; gitRef = diffResult.label; diffError = diffResult.error; @@ -89,7 +92,7 @@ export async function handleReviewCommand( gitRef, error: diffError, origin: "opencode", - diffType: isPRMode ? undefined : "uncommitted", + diffType: isPRMode ? undefined : userDiffType, gitContext, prMetadata, sharingEnabled: await getSharingEnabled(), diff --git a/apps/pi-extension/plannotator-browser.ts b/apps/pi-extension/plannotator-browser.ts index 6c6548fb..3ee5bba7 100644 --- a/apps/pi-extension/plannotator-browser.ts +++ b/apps/pi-extension/plannotator-browser.ts @@ -24,6 +24,7 @@ import { } from "./generated/pr-provider.js"; import { parseRemoteUrl } from "./generated/repo.js"; import { fetchRef, createWorktree, removeWorktree, ensureObjectAvailable } from "./generated/worktree.js"; +import { loadConfig, resolveDefaultDiffType } from "./generated/config.js"; export type AnnotateMode = "annotate" | "annotate-folder" | "annotate-last"; export interface PlanReviewDecision { @@ -336,7 +337,7 @@ export async function openCodeReview( const cwd = options.cwd ?? ctx.cwd; gitCtx = await getGitContext(cwd); const defaultBranch = options.defaultBranch ?? gitCtx.defaultBranch; - diffType = options.diffType ?? "uncommitted"; + diffType = options.diffType ?? resolveDefaultDiffType(loadConfig()); const result = await runGitDiff(diffType, defaultBranch, cwd); rawPatch = result.patch; gitRef = result.label; diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index 15f1ab6d..0c2d7a1e 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -19,6 +19,8 @@ import { getAgentSwitchSettings, getEffectiveAgentName } from '@plannotator/ui/u import { getAIProviderSettings, saveAIProviderSettings, getPreferredModel } from '@plannotator/ui/utils/aiProvider'; import { AISetupDialog } from '@plannotator/ui/components/AISetupDialog'; import { needsAISetup } from '@plannotator/ui/utils/aiSetup'; +import { DiffTypeSetupDialog } from '@plannotator/ui/components/DiffTypeSetupDialog'; +import { needsDiffTypeSetup } from '@plannotator/ui/utils/diffTypeSetup'; import { CodeAnnotation, CodeAnnotationType, SelectedLineRange, TokenAnnotationMeta, ConventionalLabel, ConventionalDecoration } from '@plannotator/ui/types'; import { useResizablePanel } from '@plannotator/ui/hooks/useResizablePanel'; import { useCodeAnnotationDraft } from '@plannotator/ui/hooks/useCodeAnnotationDraft'; @@ -362,6 +364,8 @@ const ReviewApp: React.FC = () => { }; }); const [showAISetup, setShowAISetup] = useState(false); + const [showDiffTypeSetup, setShowDiffTypeSetup] = useState(false); + const [diffTypeSetupPending, setDiffTypeSetupPending] = useState(false); const [sidebarTabOverride, setSidebarTabOverride] = useState<'ai' | undefined>(undefined); const aiChat = useAIChat({ patch: diffData?.rawPatch ?? '', @@ -662,6 +666,10 @@ const ReviewApp: React.FC = () => { } if (data.error) setDiffError(data.error); if (data.isWSL) setIsWSL(true); + // Mark diff type setup as pending on first run (local mode only) + if (data.diffType && !data.prMetadata && needsDiffTypeSetup()) { + setDiffTypeSetupPending(true); + } }) .catch(() => { // Not in API mode - use demo content @@ -676,6 +684,14 @@ const ReviewApp: React.FC = () => { .finally(() => setIsLoading(false)); }, []); + // Show diff type setup dialog only after AI setup dialog is dismissed (avoid stacking) + useEffect(() => { + if (diffTypeSetupPending && !showAISetup) { + setDiffTypeSetupPending(false); + setShowDiffTypeSetup(true); + } + }, [diffTypeSetupPending, showAISetup]); + const handleDiffStyleChange = useCallback((style: 'split' | 'unified') => { configStore.set('diffStyle', style); }, []); @@ -1907,6 +1923,12 @@ const ReviewApp: React.FC = () => { }} /> + {/* Diff type setup dialog — first-run only */} + setShowDiffTypeSetup(false)} + /> + {/* Completion overlay - shown after approve/feedback/exit */} void; +} + +export const DiffTypeSetupDialog: React.FC = ({ + isOpen, + onComplete, +}) => { + const [selected, setSelected] = useState( + () => configStore.get('defaultDiffType') as DefaultDiffType + ); + + if (!isOpen) return null; + + const handleDone = () => { + configStore.set('defaultDiffType', selected); + markDiffTypeSetupDone(); + onComplete(); + }; + + return createPortal( +
+
+ {/* Header */} +
+

Default Diff View

+

+ Choose which changes to show when you open a code review. + You can always switch between views during a session. +

+
+ + {/* Options */} +
+ {OPTIONS.map((opt) => ( + + ))} +
+ + {/* Footer */} +
+

+ You can change this later in Settings > Display. +

+ +
+
+
, + document.body + ); +}; diff --git a/packages/ui/components/Settings.tsx b/packages/ui/components/Settings.tsx index bf8e4d19..fbb62092 100644 --- a/packages/ui/components/Settings.tsx +++ b/packages/ui/components/Settings.tsx @@ -122,6 +122,11 @@ const LINE_DIFF_OPTIONS = [ { value: 'char' as const, label: 'Char' }, { value: 'none' as const, label: 'None' }, ]; +const DEFAULT_DIFF_TYPE_OPTIONS = [ + { value: 'uncommitted' as const, label: 'All Changes' }, + { value: 'unstaged' as const, label: 'Unstaged' }, + { value: 'staged' as const, label: 'Staged' }, +]; function SegmentedControl({ options, value, onChange }: { options: { value: T; label: string }[]; @@ -178,6 +183,7 @@ function ToggleSwitch({ checked, onChange, label, description }: { } const ReviewDisplayTab: React.FC = () => { + const defaultDiffType = useConfigValue('defaultDiffType'); const diffStyle = useConfigValue('diffStyle'); const diffOverflow = useConfigValue('diffOverflow'); const diffIndicators = useConfigValue('diffIndicators'); @@ -194,6 +200,17 @@ const ReviewDisplayTab: React.FC = () => { return ( <> + {/* Default Diff View */} +
+
+
Default Diff View
+
Which changes to show when opening a review
+
+ configStore.set('defaultDiffType', v)} /> +
+ +
+ {/* Font Family */}
diff --git a/packages/ui/config/settings.ts b/packages/ui/config/settings.ts index 838c8987..e1bfbfee 100644 --- a/packages/ui/config/settings.ts +++ b/packages/ui/config/settings.ts @@ -35,6 +35,21 @@ export const SETTINGS = { // --- Diff display options (namespaced under diffOptions in config.json) --- + defaultDiffType: { + defaultValue: 'unstaged' as 'uncommitted' | 'unstaged' | 'staged', + fromCookie: () => { + const v = storage.getItem('plannotator-default-diff-type'); + return v === 'uncommitted' || v === 'unstaged' || v === 'staged' ? v : undefined; + }, + toCookie: (v: string) => storage.setItem('plannotator-default-diff-type', v), + serverKey: 'diffOptions', + fromServer: (sc: Record) => { + const v = (sc.diffOptions as Record | undefined)?.defaultDiffType; + return v === 'uncommitted' || v === 'unstaged' || v === 'staged' ? v : undefined; + }, + toServer: (v: string) => ({ diffOptions: { defaultDiffType: v } }), + }, + diffStyle: { defaultValue: 'split' as 'split' | 'unified', fromCookie: () => { diff --git a/packages/ui/utils/diffTypeSetup.ts b/packages/ui/utils/diffTypeSetup.ts new file mode 100644 index 00000000..41d21c74 --- /dev/null +++ b/packages/ui/utils/diffTypeSetup.ts @@ -0,0 +1,18 @@ +/** + * Diff Type Setup Utility + * + * Tracks whether the user has seen the first-run diff type selection dialog. + * Uses cookies (not localStorage) for the same reason as all other settings. + */ + +import { storage } from './storage'; + +const STORAGE_KEY = 'plannotator-diff-type-setup-done'; + +export function needsDiffTypeSetup(): boolean { + return storage.getItem(STORAGE_KEY) !== 'true'; +} + +export function markDiffTypeSetupDone(): void { + storage.setItem(STORAGE_KEY, 'true'); +} From 55ad8ec1e6349cb142f8165489dc78900de41d4b Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Fri, 10 Apr 2026 12:18:43 -0700 Subject: [PATCH 2/4] fix(review): address code review findings for diff type setup dialog Gate dialog for P4 users, fix race condition between AI capabilities check and dialog display, and use conditional rendering to prevent stale config state. For provenance purposes, this commit was AI assisted. --- packages/review-editor/App.tsx | 19 +++++++++++-------- .../ui/components/DiffTypeSetupDialog.tsx | 4 ---- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index 0c2d7a1e..e99af130 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -364,6 +364,7 @@ const ReviewApp: React.FC = () => { }; }); const [showAISetup, setShowAISetup] = useState(false); + const [aiCheckComplete, setAiCheckComplete] = useState(false); const [showDiffTypeSetup, setShowDiffTypeSetup] = useState(false); const [diffTypeSetupPending, setDiffTypeSetupPending] = useState(false); const [sidebarTabOverride, setSidebarTabOverride] = useState<'ai' | undefined>(undefined); @@ -387,8 +388,9 @@ const ReviewApp: React.FC = () => { setShowAISetup(true); } } + setAiCheckComplete(true); }) - .catch(() => {}); + .catch(() => { setAiCheckComplete(true); }); }, []); const handleAIConfigChange = useCallback((config: { providerId?: string | null; model?: string | null }) => { @@ -667,7 +669,7 @@ const ReviewApp: React.FC = () => { if (data.error) setDiffError(data.error); if (data.isWSL) setIsWSL(true); // Mark diff type setup as pending on first run (local mode only) - if (data.diffType && !data.prMetadata && needsDiffTypeSetup()) { + if (data.diffType && !data.prMetadata && data.gitContext?.vcsType !== 'p4' && needsDiffTypeSetup()) { setDiffTypeSetupPending(true); } }) @@ -686,11 +688,11 @@ const ReviewApp: React.FC = () => { // Show diff type setup dialog only after AI setup dialog is dismissed (avoid stacking) useEffect(() => { - if (diffTypeSetupPending && !showAISetup) { + if (diffTypeSetupPending && aiCheckComplete && !showAISetup) { setDiffTypeSetupPending(false); setShowDiffTypeSetup(true); } - }, [diffTypeSetupPending, showAISetup]); + }, [diffTypeSetupPending, aiCheckComplete, showAISetup]); const handleDiffStyleChange = useCallback((style: 'split' | 'unified') => { configStore.set('diffStyle', style); @@ -1924,10 +1926,11 @@ const ReviewApp: React.FC = () => { /> {/* Diff type setup dialog — first-run only */} - setShowDiffTypeSetup(false)} - /> + {showDiffTypeSetup && ( + setShowDiffTypeSetup(false)} + /> + )} {/* Completion overlay - shown after approve/feedback/exit */} void; } export const DiffTypeSetupDialog: React.FC = ({ - isOpen, onComplete, }) => { const [selected, setSelected] = useState( () => configStore.get('defaultDiffType') as DefaultDiffType ); - if (!isOpen) return null; - const handleDone = () => { configStore.set('defaultDiffType', selected); markDiffTypeSetupDone(); From 9c04cb51187368e0ad965e9b533d29ba0a6a4b5e Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Fri, 10 Apr 2026 13:07:59 -0700 Subject: [PATCH 3/4] fix(review): import shared type and apply setup choice to active diff Import DefaultDiffType from @plannotator/shared/config instead of duplicating the type locally. Pass the selected diff type back through onComplete so the current review switches immediately when the user picks a different default. For provenance purposes, this commit was AI assisted. --- packages/review-editor/App.tsx | 5 ++++- packages/ui/components/DiffTypeSetupDialog.tsx | 9 ++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index e99af130..b3811573 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -1928,7 +1928,10 @@ const ReviewApp: React.FC = () => { {/* Diff type setup dialog — first-run only */} {showDiffTypeSetup && ( setShowDiffTypeSetup(false)} + onComplete={(selected) => { + setShowDiffTypeSetup(false); + if (selected !== diffType) handleDiffSwitch(selected); + }} /> )} diff --git a/packages/ui/components/DiffTypeSetupDialog.tsx b/packages/ui/components/DiffTypeSetupDialog.tsx index cff6d450..cd14d532 100644 --- a/packages/ui/components/DiffTypeSetupDialog.tsx +++ b/packages/ui/components/DiffTypeSetupDialog.tsx @@ -1,10 +1,9 @@ import React, { useState } from 'react'; import { createPortal } from 'react-dom'; +import type { DefaultDiffType } from '@plannotator/shared/config'; import { markDiffTypeSetupDone } from '../utils/diffTypeSetup'; import { configStore } from '../config'; -type DefaultDiffType = 'uncommitted' | 'unstaged' | 'staged'; - const OPTIONS: { value: DefaultDiffType; label: string; description: string }[] = [ { value: 'unstaged', @@ -24,20 +23,20 @@ const OPTIONS: { value: DefaultDiffType; label: string; description: string }[] ]; interface DiffTypeSetupDialogProps { - onComplete: () => void; + onComplete: (selected: DefaultDiffType) => void; } export const DiffTypeSetupDialog: React.FC = ({ onComplete, }) => { const [selected, setSelected] = useState( - () => configStore.get('defaultDiffType') as DefaultDiffType + () => configStore.get('defaultDiffType') ); const handleDone = () => { configStore.set('defaultDiffType', selected); markDiffTypeSetupDone(); - onComplete(); + onComplete(selected); }; return createPortal( From 205277f42c5b53b9846328b0debb8f10f1e89330 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Fri, 10 Apr 2026 14:47:42 -0700 Subject: [PATCH 4/4] fix(review): keep diffData in sync on diff type switch fetchDiffSwitch now updates diffData.rawPatch so clipboard copy and AI session creation use the current patch after switching diff types. For provenance purposes, this commit was AI assisted. --- packages/review-editor/App.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index b3811573..86205844 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -909,6 +909,7 @@ const ReviewApp: React.FC = () => { const nextFiles = parseDiffToFiles(data.rawPatch); dockApi?.getPanel(REVIEW_DIFF_PANEL_ID)?.api.close(); needsInitialDiffPanel.current = true; + setDiffData(prev => prev ? { ...prev, rawPatch: data.rawPatch, gitRef: data.gitRef, diffType: data.diffType } : prev); setFiles(nextFiles); setDiffType(data.diffType); setActiveFileIndex(0);