diff --git a/bun.lock b/bun.lock index f49daa850..962817544 100644 --- a/bun.lock +++ b/bun.lock @@ -64,7 +64,7 @@ }, "apps/opencode-plugin": { "name": "@plannotator/opencode", - "version": "0.21.1", + "version": "0.21.2", "dependencies": { "@opencode-ai/plugin": "^1.1.10", }, @@ -86,7 +86,7 @@ }, "apps/pi-extension": { "name": "@plannotator/pi-extension", - "version": "0.21.1", + "version": "0.21.2", "dependencies": { "@joplin/turndown-plugin-gfm": "^1.0.64", "@pierre/diffs": "1.2.8", @@ -215,7 +215,7 @@ }, "packages/server": { "name": "@plannotator/server", - "version": "0.21.1", + "version": "0.21.2", "dependencies": { "@pierre/diffs": "1.2.8", "@plannotator/ai": "workspace:*", diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index 0a8e7ab7b..155418361 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -85,7 +85,8 @@ import { REVIEW_ALL_FILES_PANEL_ID, REVIEW_CODE_NAV_PANEL_ID, } from './dock/reviewPanelTypes'; -import type { DiffFile } from './types'; +import type { DiffFile, AnnotationScrollTarget } from './types'; +import { annotationMatchesPrScope } from './utils/annotationScope'; import type { DiffOption, WorktreeInfo, GitContext } from '@plannotator/shared/types'; import type { PRMetadata } from '@plannotator/shared/pr-types'; import type { PRDiffScope, PRDiffScopeOption, PRStackInfo, PRStackTree } from '@plannotator/shared/pr-stack'; @@ -121,6 +122,11 @@ const ReviewApp: React.FC = () => { const [activeFileIndex, setActiveFileIndex] = useState(0); const [annotations, setAnnotations] = useState([]); const [selectedAnnotationId, setSelectedAnnotationId] = useState(null); + // Sidebar-initiated "scroll to this comment" signal. The token bumps on every + // sidebar click so re-selecting the same comment re-navigates. Selecting a + // comment in the diff sets selectedAnnotationId but NOT this — so it never + // moves the viewport. + const [scrollTargetAnnotation, setScrollTargetAnnotation] = useState(null); const [isAllFilesActive, setIsAllFilesActive] = useState(false); // Mirror ref: handlers captured by Pierre slot portals (which only republish // on item version bumps) and early-declared callbacks read the CURRENT value @@ -1511,30 +1517,39 @@ const ReviewApp: React.FC = () => { // handler is baked into Pierre slot portals, which only republish on item // version bumps — a stale captured value would yank the user out of the // all-files tab into the single-file panel when they click an annotation. + // Inline-card selection: toggle the highlight + ring only. No scroll, no file + // switch — the clicked card is already on screen. Clicking the selected card + // again (or a null id) clears it. const handleSelectAnnotation = useCallback((id: string | null) => { + // An inline selection supersedes any pending sidebar/findings navigate target, + // so a later remount (Refresh / base switch) doesn't re-scroll back to it. + setScrollTargetAnnotation(null); + setSelectedAnnotationId(prev => (!id || prev === id ? null : id)); + }, []); + + // Sidebar navigation: select AND scroll-to the comment (DiffsHub "set + + // scroll"). The token bump re-fires the panels' scroll effect even when the + // same comment is clicked twice; in single-file mode it switches to the + // owning file first so the scroll target exists. + const handleNavigateToAnnotation = useCallback((id: string | null) => { if (!id) { setSelectedAnnotationId(null); return; } - - // Find the annotation const annotation = allAnnotationsRef.current.find(a => a.id === id); - if (!annotation) { - setSelectedAnnotationId(id); + // Ignore navigation to an annotation that's gone (deleted) or filtered out of + // the active PR/diff-scope — there's nothing in the current diff to scroll to + // or highlight, so don't fake a selection. + if (!annotation || !annotationMatchesPrScope(annotation, prMetadata?.url, prDiffScope)) { return; } - - // In all-files mode, just set the selection — the panel's scroll-to-annotation - // effect handles expanding and scrolling. In single-file mode, switch to the file. if (!isAllFilesActiveRef.current) { const fileIndex = files.findIndex(f => f.path === annotation.filePath); - if (fileIndex !== -1) { - handleFileSwitch(fileIndex); - } + if (fileIndex !== -1) handleFileSwitch(fileIndex); } - setSelectedAnnotationId(id); - }, [files, handleFileSwitch]); + setScrollTargetAnnotation(prev => ({ id, token: (prev?.token ?? 0) + 1 })); + }, [files, handleFileSwitch, prMetadata, prDiffScope]); // Diff context bundled into local-mode feedback headers so the receiving // agent knows which diff the annotations are anchored to. Uses committedBase @@ -1596,6 +1611,7 @@ const ReviewApp: React.FC = () => { allAnnotations, externalAnnotations, selectedAnnotationId, + scrollTargetAnnotation, pendingSelection, onLineSelection: handleLineSelection, onAddAnnotation: handleAddAnnotation, @@ -1604,6 +1620,7 @@ const ReviewApp: React.FC = () => { onAddFileCommentForFile: handleAddFileCommentForFile, onEditAnnotation: handleEditAnnotation, onSelectAnnotation: handleSelectAnnotation, + onNavigateToAnnotation: handleNavigateToAnnotation, onDeleteAnnotation: handleDeleteAnnotation, viewedFiles, onToggleViewed: handleToggleViewed, @@ -1654,9 +1671,9 @@ const ReviewApp: React.FC = () => { diffLineDiffType, diffShowLineNumbers, diffShowBackground, diffExpandUnchanged, diffFontFamily, diffFontSize, activeDiffBase, committedBase, feedbackDiffContext, prReviewScopeLabel, prDiffScope, agentCwd, allAnnotations, externalAnnotations, - selectedAnnotationId, pendingSelection, handleLineSelection, + selectedAnnotationId, scrollTargetAnnotation, pendingSelection, handleLineSelection, handleAddAnnotation, handleAddFileComment, handleAddFileCommentForFile, handleEditAnnotation, - handleSelectAnnotation, handleDeleteAnnotation, viewedFiles, + handleSelectAnnotation, handleNavigateToAnnotation, handleDeleteAnnotation, viewedFiles, handleToggleViewed, stagedFiles, stagingFile, stageFile, canStageFiles, stageError, isSearchPending, debouncedSearchQuery, activeFileSearchMatches, activeSearchMatchId, activeSearchMatch, searchMatches, @@ -2575,6 +2592,7 @@ const ReviewApp: React.FC = () => { files={files} selectedAnnotationId={selectedAnnotationId} onSelectAnnotation={handleSelectAnnotation} + onNavigateToAnnotation={handleNavigateToAnnotation} onDeleteAnnotation={handleDeleteAnnotation} feedbackMarkdown={feedbackMarkdown} width={panelResize.width} diff --git a/packages/review-editor/components/AITab.tsx b/packages/review-editor/components/AITab.tsx index e3e865ad9..91eeae08b 100644 --- a/packages/review-editor/components/AITab.tsx +++ b/packages/review-editor/components/AITab.tsx @@ -3,6 +3,7 @@ import type { AIChatEntry, PendingPermission } from '../hooks/useAIChat'; import { renderChatMarkdown } from '../utils/renderChatMarkdown'; import { formatLineRange } from '../utils/formatLineRange'; import { formatRelativeTime } from '../utils/formatRelativeTime'; +import { FileNameChip } from './FileNameChip'; import { SparklesIcon } from '@plannotator/ui/components/SparklesIcon'; import { CountBadge } from './CountBadge'; import { CopyButton } from './CopyButton'; @@ -352,10 +353,8 @@ const QAPair = memo<{ {formatLineRange(question.lineStart, question.lineEnd)} )} - {scope === 'file' && ( - - file - + {scope === 'file' && question.filePath && ( + )} diff --git a/packages/review-editor/components/AllFilesCodeView.tsx b/packages/review-editor/components/AllFilesCodeView.tsx index 81e0f114d..e7feaf173 100644 --- a/packages/review-editor/components/AllFilesCodeView.tsx +++ b/packages/review-editor/components/AllFilesCodeView.tsx @@ -23,13 +23,16 @@ import type { import { CommentPopover } from '@plannotator/ui/components/CommentPopover'; import { usePierreTheme } from '../hooks/usePierreTheme'; import { useIsWorkerPoolReadyOrDisabled, useWorkerPoolThemeSync } from '../workerPool'; -import type { DiffFile } from '../types'; +import type { DiffFile, AnnotationScrollTarget } from '../types'; import { buildFileTree, getVisualFileOrder } from '../utils/buildFileTree'; import { buildCodeNavRequest } from '../utils/buildCodeNavRequest'; import { getDiffSelection, getLineNumberFromNode, getSideFromNode } from '../utils/diffSelection'; import { isContentConsistentWithPatch } from '../utils/patchConsistency'; import { ToolbarHost, type ToolbarHostHandle } from './ToolbarHost'; import { FileHeader } from './FileHeader'; +import { FileCommentBanner } from './FileCommentBanner'; +import { annotationMatchesPrScope, isFileScopedAnnotation, lineRangeForAnnotation } from '../utils/annotationScope'; +import { lineAnnotationMetadata } from '../utils/annotationDisplay'; import { InlineAnnotation } from './InlineAnnotation'; import { detectLanguage } from '../utils/detectLanguage'; import type { AIChatEntry } from '../hooks/useAIChat'; @@ -153,6 +156,7 @@ interface AllFilesCodeViewProps { // line annotations render through CodeView item state. annotations: CodeAnnotation[]; selectedAnnotationId: string | null; + scrollTargetAnnotation: AnnotationScrollTarget | null; pendingSelection: SelectedLineRange | null; reviewBase?: string; // Annotation / toolbar wiring (P2). Mirrors AllFilesDiffView's surface so the @@ -244,11 +248,14 @@ function hashString(value: string): string { return hash.toString(36); } -// Project a file's line annotations into Pierre's DiffLineAnnotation shape. This -// is the EXACT projection AllFilesDiffView builds (side, lineNumber = lineEnd, -// metadata = DiffAnnotationMetadata) so the two surfaces render identically. -// Filters to line-scoped annotations that belong to this file in the active -// PR/diff-scope (file-scoped comments live in the header, not the gutter). +// The first rendered line of a file's diff, used to anchor file-scoped comments. +// Pierre suppresses the header-prefix slot whenever a custom header is present +// (renderDiffChildren makes them mutually exclusive), so file comments can't +// live "between header and body" — instead they ride the line-annotation slot +// Project a file's LINE annotations into Pierre's DiffLineAnnotation shape (side, +// lineNumber = lineEnd, metadata = DiffAnnotationMetadata). File-scoped comments +// are deliberately excluded — they render in the file header (renderCustomHeader), +// not the gutter (see fileCommentsByPath). function projectFileAnnotations( annotations: CodeAnnotation[], filePath: string, @@ -260,24 +267,12 @@ function projectFileAnnotations( (a) => a.filePath === filePath && (a.scope ?? 'line') === 'line' && - (!a.prUrl || !prUrl || a.prUrl === prUrl) && - (!a.diffScope || !prDiffScope || a.diffScope === prDiffScope), + annotationMatchesPrScope(a, prUrl, prDiffScope), ) .map((ann) => ({ side: ann.side === 'new' ? ('additions' as const) : ('deletions' as const), lineNumber: ann.lineEnd, - metadata: { - annotationId: ann.id, - type: ann.type, - text: ann.text, - suggestedCode: ann.suggestedCode, - originalCode: ann.originalCode, - author: ann.author, - severity: ann.severity, - reasoning: ann.reasoning, - conventionalLabel: ann.conventionalLabel, - decorations: ann.decorations, - } as DiffAnnotationMetadata, + metadata: lineAnnotationMetadata(ann), })); } @@ -384,6 +379,7 @@ export const AllFilesCodeView: React.FC = ({ fontSize, annotations, selectedAnnotationId, + scrollTargetAnnotation, pendingSelection, reviewBase, onLineSelection, @@ -514,8 +510,12 @@ export const AllFilesCodeView: React.FC = ({ // file's patch CONTENT changes (diff type / base / whitespace / PR switch), // and is used as the CodeView `key` to force a remount + fresh seed. const fileSetKey = useMemo( - () => `${files.length}:${files.map((f, i) => `${f.path}#${patchHashes[i]}`).join('|')}`, - [files, patchHashes], + // prUrl/prDiffScope are part of the key so a pure scope switch (layer ↔ + // full-stack, same file set) remounts and re-seeds annotations through the + // current scope filter — the incremental sync bails on an unchanged + // annotations ref and can't otherwise detect the filter change. + () => `${prUrl ?? ''}:${prDiffScope ?? ''}:${files.length}:${files.map((f, i) => `${f.path}#${patchHashes[i]}`).join('|')}`, + [files, patchHashes, prUrl, prDiffScope], ); // Visual-order list of file paths (for [/] stepping). Derived from items so it @@ -659,6 +659,21 @@ export const AllFilesCodeView: React.FC = ({ toolbarHostRef.current?.startEdit(ann); }); + // Per-file file-scoped comments, namespaced to the active PR/diff-scope. These + // render in the file HEADER (renderCustomHeader, below the path) when the file + // is expanded — not in the gutter — so they read as a file-level note rather + // than a stray line comment. + const fileCommentsByPath = useMemo(() => { + const map = new Map(); + for (const a of annotations) { + if (!isFileScopedAnnotation(a) || !annotationMatchesPrScope(a, prUrl, prDiffScope)) continue; + const arr = map.get(a.filePath); + if (arr) arr.push(a); + else map.set(a.filePath, [a]); + } + return map; + }, [annotations, prUrl, prDiffScope]); + // Render a single annotation from item state. `renderAnnotation` receives both // the LineAnnotation and DiffLineAnnotation union — guard `'side' in // annotation && item.type === 'diff'` (the Diffshub pattern) so file-item @@ -678,6 +693,7 @@ export const AllFilesCodeView: React.FC = ({ = ({ const signatures = (list: CodeAnnotation[]) => { const map = new Map(); for (const a of list) { - if ((a.scope ?? 'line') !== 'line') continue; - if (a.prUrl && prUrl && a.prUrl !== prUrl) continue; - if (a.diffScope && prDiffScope && a.diffScope !== prDiffScope) continue; - const sig = JSON.stringify([ - a.id, a.lineEnd, a.side, a.type, - a.text ?? '', a.suggestedCode ?? '', a.originalCode ?? '', - a.conventionalLabel ?? '', (a.decorations ?? []).join(','), - a.severity ?? '', a.reasoning ?? '', a.author ?? '', - ]); + const scope = a.scope ?? 'line'; + if (scope !== 'line' && scope !== 'file') continue; + if (!annotationMatchesPrScope(a, prUrl, prDiffScope)) continue; + // File comments carry different render-affecting fields than line notes + // (no line/side/suggestion; they DO surface source + profile badges). + const sig = scope === 'file' + ? JSON.stringify([ + 'F', a.id, a.text ?? '', a.source ?? '', a.author ?? '', + a.reviewProfileLabel ?? '', a.conventionalLabel ?? '', + (a.decorations ?? []).join(','), a.createdAt ?? 0, a.reasoning ?? '', + ]) + : JSON.stringify([ + a.id, a.lineEnd, a.side, a.type, + a.text ?? '', a.suggestedCode ?? '', a.originalCode ?? '', + a.conventionalLabel ?? '', (a.decorations ?? []).join(','), + a.severity ?? '', a.reasoning ?? '', a.author ?? '', + a.reviewProfileLabel ?? '', a.source ?? '', a.createdAt ?? 0, + ]); map.set(a.filePath, `${map.get(a.filePath) ?? ''}${sig}\n`); } return map; @@ -1406,9 +1431,28 @@ export const AllFilesCodeView: React.FC = ({ // 2. A toolbar-originated range CodeView doesn't know about (gutter-utility // click on a not-yet-active file, draft restore) → paint it on the // active file's item. + // The controlled line highlight for a selected annotation (null for file-scoped + // / unresolved). Shared by the compose-end restore and the selection-replay + // effect so the two can't diverge. + const lineSelectionForAnnotation = useStableCallback( + (ann: CodeAnnotation | null | undefined): CodeViewLineSelection | null => { + if (!ann || isFileScopedAnnotation(ann)) return null; + const itemId = filePathToItemId.get(ann.filePath); + return itemId != null ? { id: itemId, range: lineRangeForAnnotation(ann) } : null; + }, + ); + // Mirror so the effect below can restore the selected comment's highlight when + // a compose ends, without taking selectedAnnotationId as a dep. + const selectedAnnotationIdRef = useRef(selectedAnnotationId); + selectedAnnotationIdRef.current = selectedAnnotationId; useEffect(() => { if (pendingSelection == null) { - setSelectedLines(null); + // Compose ended — restore the selected line comment's highlight instead of + // clearing, mirroring single-file's `pendingSelection ?? annotationRange`. + const ann = selectedAnnotationIdRef.current + ? annotationsRef.current.find((a) => a.id === selectedAnnotationIdRef.current) + : null; + setSelectedLines(lineSelectionForAnnotation(ann)); return; } const current = selectedLinesRef.current; @@ -1543,26 +1587,57 @@ export const AllFilesCodeView: React.FC = ({ viewer.scrollTo({ type: 'item', id: itemId, align: 'start' }); }, []); - // --- Selected-annotation navigation (P4) ----------------------------------- - - // Selecting an annotation in the sidebar must expand its owning file (if - // collapsed) and scroll to it. We expand via item state (collapsed=false + - // version bump + updateItem — the Diffshub pattern), then scrollTo the - // annotation's line range so it lands in view. rAF defers the scroll one frame - // so the expand's layout has settled before CodeView resolves the line top. - // `annotations` is read through the ref, NOT the dep list: this must fire only - // when the SELECTION changes. With `annotations` as a dep, any annotation - // change while one is selected (add/edit/delete elsewhere, an external SSE - // annotation arriving) re-runs the effect and yanks the viewport back to the - // selected annotation with zero user action. + // --- Selected-annotation highlight + navigation ---------------------------- + + // SELECTION (inline card OR sidebar) paints the line highlight + repaints the + // card ring — but NEVER scrolls. Clicking a comment in the diff must not move + // the viewport. `annotations` is read through the ref so this fires only on + // selection change, not on any add/edit/delete while one is selected. + // Read-only mirror of pendingSelection so the selection effect can yield to an + // active compose WITHOUT taking pendingSelection as a dep (which would re-run + // it on every drag delta). + const pendingSelectionRef = useRef(pendingSelection); + pendingSelectionRef.current = pendingSelection; + const prevSelectedFileRef = useRef(null); + useEffect(() => { + const ann = selectedAnnotationId + ? annotationsRef.current.find((a) => a.id === selectedAnnotationId) + : null; + const newFile = ann?.filePath ?? null; + + // Repaint the inline card's selected ring on the previously- AND + // newly-selected file: renderAnnotation only re-runs on updateItem, so a bare + // selection-state change wouldn't otherwise reach the portal'd cards. + const filesToRefresh = new Set(); + if (prevSelectedFileRef.current) filesToRefresh.add(prevSelectedFileRef.current); + if (newFile) filesToRefresh.add(newFile); + prevSelectedFileRef.current = newFile; + for (const path of filesToRefresh) { + for (const itemId of filePathToItemIds.get(path) ?? []) refreshItem(itemId); + } + + // An active compose (toolbar open) owns selectedLines — clicking/deselecting a + // comment must not clobber the in-progress range highlight. Mirrors + // single-file DiffViewer's `pendingSelection ?? selectedAnnotationRange`. + if (pendingSelectionRef.current != null) return; + + // Replay the selected line comment's range as the controlled highlight (the + // same state a drag paints); file-scoped / deselection clears it. + setSelectedLines(lineSelectionForAnnotation(ann)); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [selectedAnnotationId, filePathToItemId, filePathToItemIds, refreshItem]); + + // NAVIGATION (sidebar only) — expand the owning file if collapsed and scroll + // to the comment. Keyed on the navigation token so it fires per sidebar click + // (re-clicking the same comment re-centers it) and NEVER on a bare in-diff + // selection. rAF defers the scroll a frame so the expand's layout has settled. useEffect(() => { - if (!selectedAnnotationId) return; - const ann = annotationsRef.current.find((a) => a.id === selectedAnnotationId); + if (!scrollTargetAnnotation) return; + const ann = annotationsRef.current.find((a) => a.id === scrollTargetAnnotation.id); if (!ann) return; const itemId = filePathToItemId.get(ann.filePath); - if (itemId == null) return; const handle = viewerRef.current; - if (handle == null) return; + if (itemId == null || handle == null) return; const item = handle.getItem(itemId); if (item != null && item.collapsed === true) { @@ -1571,17 +1646,17 @@ export const AllFilesCodeView: React.FC = ({ handle.updateItem(item); } - const start = Math.min(ann.lineStart, ann.lineEnd); - const end = Math.max(ann.lineStart, ann.lineEnd); - const side = ann.side === 'new' ? ('additions' as const) : ('deletions' as const); + const isFile = isFileScopedAnnotation(ann); + const range = lineRangeForAnnotation(ann); const raf = requestAnimationFrame(() => { const viewer = viewerRef.current; if (viewer == null) return; - viewer.scrollTo({ type: 'range', id: itemId, range: { start, end, side } }); + if (isFile) viewer.scrollTo({ type: 'item', id: itemId, align: 'start' }); + else viewer.scrollTo({ type: 'range', id: itemId, range }); }); return () => cancelAnimationFrame(raf); // eslint-disable-next-line react-hooks/exhaustive-deps - }, [selectedAnnotationId, filePathToItemId]); + }, [scrollTargetAnnotation, filePathToItemId]); useEffect(() => { if (!isActive) return; @@ -1699,9 +1774,11 @@ export const AllFilesCodeView: React.FC = ({ if (file == null) return null; const collapsed = item.collapsed === true; + const fileComments = fileCommentsByPath.get(filePath) ?? []; return ( - + = ({ } onCollapseToggle={() => toggleItemCollapsed(item.id)} - /> + /> + {/* File-scoped comments live in the header (below the path), shown only + when the file is expanded. They ride the sticky header — fine for a + short guide note; long ones scroll within the banner. */} + {!collapsed && fileComments.length > 0 && ( + refreshItem(item.id)} + /> + )} + ); }); @@ -1784,6 +1878,9 @@ export const AllFilesCodeView: React.FC = ({ enableGutterUtility: true, hunkSeparators: 'line-info', stickyHeaders: true, + // Flush files together (no inter-file gap) — file boundaries already read + // via the sticky header. Keep Pierre's default 8px list edge padding. + layout: { gap: 0, paddingTop: 8, paddingBottom: 8 }, itemMetrics: { diffHeaderHeight: PANEL_HEADER_HEIGHT, hunkSeparatorHeight: HUNK_SEPARATOR_HEIGHT, diff --git a/packages/review-editor/components/CommentActions.tsx b/packages/review-editor/components/CommentActions.tsx new file mode 100644 index 000000000..99586177e --- /dev/null +++ b/packages/review-editor/components/CommentActions.tsx @@ -0,0 +1,56 @@ +import React from 'react'; +import { CopyButton } from './CopyButton'; + +interface CommentActionsProps { + /** When provided, shows the edit button (left-most). */ + onEdit?: () => void; + /** When provided, shows the copy button (middle). */ + copyText?: string; + /** When provided, shows the delete/close button (right-most). Omitted for + * read-only (e.g. externally-sourced) comments. */ + onDelete?: () => void; +} + +const ACTION_BTN = 'p-1 rounded text-muted-foreground transition-colors'; + +/** + * The single hover-revealed action row shared by every comment card (inline + * diff, sidebar, file banner). Bottom-aligned, right-justified, order + * left→right: edit · copy · delete (so the close/delete sits furthest right). + * The parent card must carry the Tailwind `group` class for the hover reveal. + */ +export const CommentActions: React.FC = ({ onEdit, copyText, onDelete }) => { + if (!onEdit && !copyText && !onDelete) return null; + return ( +
e.stopPropagation()} + > + {onEdit && ( + + )} + {copyText && } + {onDelete && ( + + )} +
+ ); +}; diff --git a/packages/review-editor/components/CommentMeta.tsx b/packages/review-editor/components/CommentMeta.tsx new file mode 100644 index 000000000..e256fc166 --- /dev/null +++ b/packages/review-editor/components/CommentMeta.tsx @@ -0,0 +1,68 @@ +import React from 'react'; +import type { ConventionalLabel, ConventionalDecoration } from '@plannotator/ui/types'; +import { isCurrentUser } from '@plannotator/ui/utils/identity'; +import { ConventionalLabelBadge } from './ConventionalLabelPicker'; +import { formatRelativeTime } from '../utils/formatRelativeTime'; + +interface CommentMetaProps { + /** Surface-specific leading element(s): severity dot, scope/file/line badge, + * collapse toggle, etc. Rendered first in the left cluster. */ + leading?: React.ReactNode; + conventionalLabel?: ConventionalLabel | null; + decorations?: ConventionalDecoration[]; + reviewProfileLabel?: string; + source?: string; + author?: string; + createdAt?: number; +} + +/** + * The single identity row shared by every comment surface — the inline diff + * card, the sidebar list, and the file-comment banner. Left cluster: leading + * badge(s) → conventional label → review-profile/source badge → author. Right: + * relative time, then any surface-specific actions. Centralizing it keeps author + * + timestamp + badge styling identical everywhere (they used to be hand-rolled + * three different ways). + */ +export const CommentMeta: React.FC = ({ + leading, + conventionalLabel, + decorations, + reviewProfileLabel, + source, + author, + createdAt, +}) => ( +
+
+ {leading} + {conventionalLabel && ( + + )} + {reviewProfileLabel ? ( + + {reviewProfileLabel} + + ) : source ? ( + + {source} + + ) : null} + {author && ( + + {author} + {isCurrentUser(author) && ' (me)'} + + )} +
+ {createdAt != null && ( + + {formatRelativeTime(createdAt)} + + )} +
+); diff --git a/packages/review-editor/components/DiffViewer.tsx b/packages/review-editor/components/DiffViewer.tsx index affd4d457..c30ceefc6 100644 --- a/packages/review-editor/components/DiffViewer.tsx +++ b/packages/review-editor/components/DiffViewer.tsx @@ -13,6 +13,10 @@ import { ToolbarHost, type ToolbarHostHandle } from './ToolbarHost'; import { OverlayScrollArea } from '@plannotator/ui/components/OverlayScrollArea'; import { useOverlayViewport } from '@plannotator/ui/hooks/useOverlayViewport'; import { FileHeader } from './FileHeader'; +import { FileCommentBanner } from './FileCommentBanner'; +import { isFileScopedAnnotation, lineRangeForAnnotation } from '../utils/annotationScope'; +import { lineAnnotationMetadata } from '../utils/annotationDisplay'; +import type { AnnotationScrollTarget } from '../types'; import { getLineNumberFromNode, getSideFromNode, getDiffSelection } from '../utils/diffSelection'; import { isContentConsistentWithPatch } from '../utils/patchConsistency'; import { InlineAnnotation } from './InlineAnnotation'; @@ -152,6 +156,7 @@ interface DiffViewerProps { fontSize?: string; annotations: CodeAnnotation[]; selectedAnnotationId: string | null; + scrollTargetAnnotation: AnnotationScrollTarget | null; pendingSelection: SelectedLineRange | null; onLineSelection: (range: SelectedLineRange | null) => void; onAddAnnotation: (type: CodeAnnotationType, text?: string, suggestedCode?: string, originalCode?: string, conventionalLabel?: ConventionalLabel, decorations?: ConventionalDecoration[], tokenMeta?: TokenAnnotationMeta) => void; @@ -203,6 +208,7 @@ export const DiffViewer: React.FC = ({ fontSize, annotations, selectedAnnotationId, + scrollTargetAnnotation, pendingSelection, onLineSelection, onAddAnnotation, @@ -406,13 +412,16 @@ export const DiffViewer: React.FC = ({ return () => viewport.removeEventListener('scroll', onScroll); }, [viewport, filePath]); - // Scroll to selected annotation when it changes + // Scroll to a comment ONLY on sidebar navigation (scrollTargetAnnotation), + // never on a bare in-diff selection — clicking a comment must not move the + // viewport. Keyed on the token so re-clicking the same sidebar row re-centers. useEffect(() => { - if (!selectedAnnotationId || !containerRef.current) return; + if (!scrollTargetAnnotation || !containerRef.current) return; + const targetId = scrollTargetAnnotation.id; const timeoutId = setTimeout(() => { const annotationEl = containerRef.current?.querySelector( - `[data-annotation-id="${selectedAnnotationId}"]` + `[data-annotation-id="${targetId}"]` ); if (annotationEl) { annotationEl.scrollIntoView({ behavior: 'smooth', block: 'center' }); @@ -420,7 +429,7 @@ export const DiffViewer: React.FC = ({ }, 100); return () => clearTimeout(timeoutId); - }, [selectedAnnotationId, viewport]); + }, [scrollTargetAnnotation, viewport]); // Apply search highlights to diff lines (including inside shadow DOM). // The query is already debounced upstream (useReviewSearch), so this runs synchronously. @@ -504,18 +513,7 @@ export const DiffViewer: React.FC = ({ .map(ann => ({ side: ann.side === 'new' ? 'additions' as const : 'deletions' as const, lineNumber: ann.lineEnd, - metadata: { - annotationId: ann.id, - type: ann.type, - text: ann.text, - suggestedCode: ann.suggestedCode, - originalCode: ann.originalCode, - author: ann.author, - severity: ann.severity, - reasoning: ann.reasoning, - conventionalLabel: ann.conventionalLabel, - decorations: ann.decorations, - } as DiffAnnotationMetadata, + metadata: lineAnnotationMetadata(ann), })); }, [annotations]); @@ -570,12 +568,13 @@ export const DiffViewer: React.FC = ({ ); - }, [filePath, onSelectAnnotation, handleEdit, onDeleteAnnotation, onClickAIMarker]); + }, [filePath, selectedAnnotationId, onSelectAnnotation, handleEdit, onDeleteAnnotation, onClickAIMarker]); const handleGutterUtilityClick = useCallback((range: SelectedLineRange) => { toolbarHostRef.current?.handleLineSelectionEnd(range); @@ -638,6 +637,24 @@ export const DiffViewer: React.FC = ({ } as React.CSSProperties; }, [diffOverflow, isSplitLayout, splitRatio]); + // File-scoped comments render below the path, above the hunks (full text, no + // truncation) — line-scoped annotations stay inline in the gutter. + const fileComments = useMemo( + () => annotations.filter(isFileScopedAnnotation), + [annotations], + ); + + // Replay a selected line/range comment's anchor as the controlled highlight so + // clicking it (inline card or sidebar) lights up its lines. A live compose + // selection (pendingSelection) wins while the toolbar is open; file-scoped + // comments have no meaningful line so they don't paint a highlight. + const selectedAnnotationRange = useMemo(() => { + if (!selectedAnnotationId) return null; + const ann = annotations.find((a) => a.id === selectedAnnotationId); + if (!ann || isFileScopedAnnotation(ann)) return null; + return lineRangeForAnnotation(ann); + }, [selectedAnnotationId, annotations]); + return (
= ({ overflowX="scroll" onViewportReady={onViewportReady} > +
{isSplitLayout && diffOverflow !== 'wrap' && ( @@ -684,7 +708,7 @@ export const DiffViewer: React.FC = ({ disableBackground={disableBackground} expandUnchanged={expandUnchanged} mergedAnnotations={mergedAnnotations} - pendingSelection={pendingSelection} + pendingSelection={pendingSelection ?? selectedAnnotationRange} onLineSelectionEnd={handlePierreLineSelectionEnd} onGutterUtilityClick={handleGutterUtilityClick} renderAnnotation={renderAnnotation} diff --git a/packages/review-editor/components/FileCommentBanner.tsx b/packages/review-editor/components/FileCommentBanner.tsx new file mode 100644 index 000000000..d0fc9227e --- /dev/null +++ b/packages/review-editor/components/FileCommentBanner.tsx @@ -0,0 +1,175 @@ +import React, { useLayoutEffect, useMemo, useRef, useState } from 'react'; +import type { CodeAnnotation } from '@plannotator/ui/types'; +import { sanitizeBlockHtml } from '@plannotator/ui/utils/sanitizeHtml'; +import { CommentMeta } from './CommentMeta'; +import { CommentActions } from './CommentActions'; +import { FileNameChip } from './FileNameChip'; +import { commentCopyText } from '../utils/annotationDisplay'; + +interface FileCommentBannerProps { + /** File-scoped comments for ONE file (already filtered to scope === 'file'). */ + comments: CodeAnnotation[]; + selectedAnnotationId: string | null; + onSelect: (id: string | null) => void; + onEdit: (id: string, text: string) => void; + onDelete: (id: string) => void; + /** Re-measure hook for the virtualized all-files host (see FileCommentCard). */ + onHeightChange?: () => void; +} + +/** First non-empty line of the comment, used as the collapsed one-line preview. */ +function firstLine(text: string): string { + for (const line of text.split('\n')) { + const trimmed = line.trim(); + if (trimmed) return trimmed.replace(/^#+\s*/, '').replace(/[*_`>]/g, ''); + } + return text.trim(); +} + +/** + * A single file-scoped comment card. Exported so the all-files view can render + * it directly inside Pierre's annotation slot (the header-prefix slot is + * suppressed whenever a custom header is present), while the single-file viewer + * renders a stack of them in {@link FileCommentBanner}. + */ +export const FileCommentCard: React.FC<{ + comment: CodeAnnotation; + isSelected: boolean; + onSelect: (id: string | null) => void; + onEdit: (id: string, text: string) => void; + onDelete: (id: string) => void; + /** Fired after our rendered height changes (expand/collapse/edit) so the + * virtualized all-files host can re-measure this item. */ + onHeightChange?: () => void; +}> = ({ comment, isSelected, onSelect, onEdit, onDelete, onHeightChange }) => { + // Default expanded (the comment IS the point of a guided review); the toggle + // lets a reviewer collapse a long note back to one line to reach the hunks. + const [collapsed, setCollapsed] = useState(false); + const [isEditing, setIsEditing] = useState(false); + const [draft, setDraft] = useState(comment.text ?? ''); + + // Tell the owner to re-measure when our height changes — in the all-files view + // these cards live in Pierre's custom-header portal, whose height isn't auto- + // observed; without this, expanding/editing overlaps the content below. + const mounted = useRef(false); + // Layout effect (before paint) so the re-measure lands without a one-frame + // overlap when the card grows/shrinks. + useLayoutEffect(() => { + if (mounted.current) onHeightChange?.(); + else mounted.current = true; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [collapsed, isEditing]); + + const html = useMemo( + () => (comment.text ? sanitizeBlockHtml(comment.text) : ''), + [comment.text], + ); + + const saveEdit = () => { + const trimmed = draft.trim(); + if (trimmed && trimmed !== comment.text) onEdit(comment.id, trimmed); + setIsEditing(false); + }; + + return ( +
onSelect(comment.id)} + > + + {/* Collapse toggle — always visible (primary affordance for reclaiming + space from a long comment), unlike the hover-revealed actions. */} + + + + } + conventionalLabel={comment.conventionalLabel} + decorations={comment.decorations} + reviewProfileLabel={comment.reviewProfileLabel} + source={comment.source} + author={comment.author} + createdAt={comment.createdAt} + /> + + {isEditing ? ( +
e.stopPropagation()}> +