From 9b64cbe0d0f7780520e09730e0da0cb2595f4ce7 Mon Sep 17 00:00:00 2001 From: "Leonardo R. Dias" <47978193+leoreisdias@users.noreply.github.com> Date: Mon, 3 Aug 2026 10:44:04 -0300 Subject: [PATCH 1/2] Add Explain action for agent-generated review findings --- packages/review-editor/App.tsx | 25 ++++++- .../components/AllFilesCodeView.tsx | 27 ++++++- .../components/CommentActions.test.tsx | 58 +++++++++++++++ .../components/CommentActions.tsx | 24 +++++- .../review-editor/components/DiffViewer.tsx | 8 +- .../components/FileCommentBanner.tsx | 15 +++- .../components/InlineAnnotation.tsx | 9 +++ .../components/ReviewSidebar.tsx | 7 ++ .../components/guide/GuideFileCard.tsx | 1 + .../review-editor/dock/ReviewStateContext.tsx | 1 + .../dock/panels/ReviewAgentJobDetailPanel.tsx | 28 +++++-- .../dock/panels/ReviewAllFilesDiffPanel.tsx | 1 + .../dock/panels/ReviewDiffPanel.tsx | 1 + .../utils/explainFinding.test.ts | 74 +++++++++++++++++++ .../review-editor/utils/explainFinding.ts | 57 ++++++++++++++ 15 files changed, 317 insertions(+), 19 deletions(-) create mode 100644 packages/review-editor/components/CommentActions.test.tsx create mode 100644 packages/review-editor/utils/explainFinding.test.ts create mode 100644 packages/review-editor/utils/explainFinding.ts diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index ec97c9614..0022cf487 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -41,6 +41,10 @@ import { useReviewSearch, type ReviewSearchMatch, } from './hooks/useReviewSearch'; +import { + buildExplainFindingRequest, + isAgentGeneratedFinding, +} from './utils/explainFinding'; import { useEditorAnnotations } from '@plannotator/ui/hooks/useEditorAnnotations'; import { useExternalAnnotations } from '@plannotator/ui/hooks/useExternalAnnotations'; import { useAgentJobs, jobMatchesReviewContext } from '@plannotator/ui/hooks/useAgentJobs'; @@ -711,6 +715,7 @@ const ReviewApp: React.FC = () => { resetSession: resetAISession, sessionId: aiSessionId, } = aiChat; + const isAILoading = aiIsCreatingSession || aiIsStreaming; const codeNav = useCodeNav(); @@ -813,6 +818,18 @@ const ReviewApp: React.FC = () => { handleAskAIForFile(file.path, question); }, [activeFileIndex, files, handleAskAIForFile]); + const handleExplainAnnotation = useCallback((id: string) => { + if (!aiAvailable || isAILoading) return; + const annotation = allAnnotationsRef.current.find((item) => item.id === id); + if (!annotation || !isAgentGeneratedFinding(annotation)) return; + + const file = annotation.filePath + ? files.find((item) => item.path === annotation.filePath) + : undefined; + reviewSidebar.open('ai'); + void askAI(buildExplainFindingRequest(annotation, file?.patch)); + }, [aiAvailable, askAI, files, isAILoading]); + const handleViewAIResponse = useCallback((questionId?: string) => { reviewSidebar.open('ai'); if (questionId) { @@ -2382,6 +2399,7 @@ const ReviewApp: React.FC = () => { onSelectAnnotation: handleSelectAnnotation, onNavigateToAnnotation: handleNavigateToAnnotation, onDeleteAnnotation: handleDeleteAnnotation, + onExplainAnnotation: handleExplainAnnotation, descriptionAnnotations: visibleDescriptionAnnotations, selectedDescriptionAnnotationId, onAddDescriptionAnnotation: handleAddDescriptionAnnotation, @@ -2420,7 +2438,7 @@ const ReviewApp: React.FC = () => { aiMessages, onAskAI: handleAskAI, onAskAIForFile: handleAskAIForFile, - isAILoading: aiIsCreatingSession || aiIsStreaming, + isAILoading, onViewAIResponse: handleViewAIResponse, onClickAIMarker: handleClickAIMarker, aiHistoryForSelection, @@ -2464,11 +2482,11 @@ const ReviewApp: React.FC = () => { handleSelectCommentAnnotation, handleDeleteCommentAnnotation, handleAskAIForComment, commentScrollTarget, selectedAnnotationId, scrollTargetAnnotation, pendingSelection, handleLineSelection, handleAddAnnotation, handleAddFileComment, handleAddFileCommentForFile, handleEditAnnotation, - handleSelectAnnotation, handleNavigateToAnnotation, handleDeleteAnnotation, viewedFiles, + handleSelectAnnotation, handleNavigateToAnnotation, handleDeleteAnnotation, handleExplainAnnotation, viewedFiles, handleToggleViewed, stagedFiles, stagingFile, stageFile, canStageFiles, isPathStageable, activeWorktreePath, guideRevealFile, handleGuideRevealFile, stageError, isSearchPending, debouncedSearchQuery, activeFileSearchMatches, activeSearchMatchId, activeSearchMatch, searchMatches, - aiAvailable, aiMessages, aiIsCreatingSession, aiIsStreaming, + aiAvailable, aiMessages, isAILoading, handleAskAI, handleAskAIForFile, handleViewAIResponse, handleClickAIMarker, aiHistoryForSelection, getAIHistoryForFile, agentJobs.jobs, prMetadata, prContext, prArtifacts, isPRContextLoading, prContextError, fetchPRContext, platformUser, openDiffFile, @@ -3593,6 +3611,7 @@ const ReviewApp: React.FC = () => { onSelectAnnotation={handleSelectAnnotation} onNavigateToAnnotation={handleNavigateToAnnotation} onDeleteAnnotation={handleDeleteAnnotation} + onExplainAnnotation={aiAvailable ? handleExplainAnnotation : undefined} feedbackMarkdown={feedbackMarkdown} width={panelResize.width} editorAnnotations={visibleEditorAnnotations} diff --git a/packages/review-editor/components/AllFilesCodeView.tsx b/packages/review-editor/components/AllFilesCodeView.tsx index a42997415..0cd21c50c 100644 --- a/packages/review-editor/components/AllFilesCodeView.tsx +++ b/packages/review-editor/components/AllFilesCodeView.tsx @@ -185,6 +185,7 @@ interface AllFilesCodeViewProps { ) => void; onSelectAnnotation: (id: string | null) => void; onDeleteAnnotation: (id: string) => void; + onExplainAnnotation?: (id: string) => void; // Header actions (P3). Mirror AllFilesDiffView's header surface. onAddFileCommentForFile?: (filePath: string, text: string) => void; viewedFiles?: Set; @@ -470,6 +471,7 @@ export const AllFilesCodeView: React.FC = ({ onEditAnnotation, onSelectAnnotation, onDeleteAnnotation, + onExplainAnnotation, onAddFileCommentForFile, viewedFiles, onToggleViewed, @@ -823,7 +825,7 @@ export const AllFilesCodeView: React.FC = ({ // annotation && item.type === 'diff'` (the Diffshub pattern) so file-item // annotations (none here) and metadata-less annotations are skipped. Actions // route by the OWNING item, not an active-file side channel. - const renderAnnotation = useStableCallback( + const renderAnnotationContent = useStableCallback( ( annotation: | DiffLineAnnotation @@ -852,10 +854,23 @@ export const AllFilesCodeView: React.FC = ({ onSelect={onSelectAnnotation} onEdit={handleEditAnnotation} onDelete={onDeleteAnnotation} + onExplain={onExplainAnnotation} + explainDisabled={isAILoading} /> ); }, ); + // Pierre memoizes slot portals by renderer identity. Republish them when the + // Explain action appears or changes loading state, while keeping callbacks fresh. + const renderAnnotation = useCallback( + ( + annotation: + | DiffLineAnnotation + | LineAnnotation, + item: CodeViewItem, + ) => renderAnnotationContent(annotation, item), + [renderAnnotationContent, onExplainAnnotation, isAILoading], + ); // Reset to a fresh state when the file set changes (diff switch). CodeView // itself is remounted via `fileSetKey`; this clears the React-side toolbar / @@ -2018,7 +2033,7 @@ export const AllFilesCodeView: React.FC = ({ // --- Custom header render slot (the full Plannotator FileHeader) ----------- - const renderCustomHeader = useStableCallback((item: CodeViewItem) => { + const renderCustomHeaderContent = useStableCallback((item: CodeViewItem) => { if (item.type !== 'diff') return null; const filePath = itemIdToFilePath.get(item.id); if (filePath == null) return null; @@ -2090,6 +2105,8 @@ export const AllFilesCodeView: React.FC = ({ onSelect={onSelectAnnotation} onEdit={onEditAnnotation} onDelete={onDeleteAnnotation} + onExplain={onExplainAnnotation} + explainDisabled={isAILoading} // Re-measure the item when a comment expands/collapses/edits — the // custom-header height isn't auto-observed, so without this the // content below would overlap until an unrelated refresh. @@ -2099,6 +2116,12 @@ export const AllFilesCodeView: React.FC = ({ ); }); + // File-scoped findings live in the custom-header portal and need the same + // availability/loading republish as line annotations. + const renderCustomHeader = useCallback( + (item: CodeViewItem) => renderCustomHeaderContent(item), + [renderCustomHeaderContent, onExplainAnnotation, isAILoading], + ); // Pass-through allowlist only (CODE_VIEW_DIFF_OPTION_KEYS). hunkSeparators, // stickyHeaders, itemMetrics, and the selection callbacks are CodeView-level diff --git a/packages/review-editor/components/CommentActions.test.tsx b/packages/review-editor/components/CommentActions.test.tsx new file mode 100644 index 000000000..9cfef00f0 --- /dev/null +++ b/packages/review-editor/components/CommentActions.test.tsx @@ -0,0 +1,58 @@ +import { afterEach, describe, expect, test } from 'bun:test'; +import React, { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { CommentActions } from './CommentActions'; + +const hasDom = typeof document !== 'undefined'; +let root: Root | null = null; +let host: HTMLElement | null = null; + +afterEach(async () => { + if (root !== null) await act(async () => root?.unmount()); + root = null; + host?.remove(); + host = null; +}); + +describe('CommentActions', () => { + test.skipIf(!hasDom)('invokes the learning explanation action when it is available', async () => { + let explanationRequests = 0; + host = document.createElement('div'); + document.body.appendChild(host); + + await act(async () => { + root = createRoot(host!); + root.render( + { explanationRequests += 1; }} />, + ); + }); + + const explainButton = host.querySelector('[aria-label="Explain finding"]'); + expect(explainButton).not.toBeNull(); + + await act(async () => explainButton?.click()); + expect(explanationRequests).toBe(1); + }); + + test.skipIf(!hasDom)('disables explanation requests while Ask AI is busy', async () => { + let explanationRequests = 0; + host = document.createElement('div'); + document.body.appendChild(host); + + await act(async () => { + root = createRoot(host!); + root.render( + { explanationRequests += 1; }} + explainDisabled + />, + ); + }); + + const explainButton = host.querySelector('[aria-label="Explain finding"]'); + expect(explainButton?.disabled).toBe(true); + + await act(async () => explainButton?.click()); + expect(explanationRequests).toBe(0); + }); +}); diff --git a/packages/review-editor/components/CommentActions.tsx b/packages/review-editor/components/CommentActions.tsx index 99586177e..d7b375141 100644 --- a/packages/review-editor/components/CommentActions.tsx +++ b/packages/review-editor/components/CommentActions.tsx @@ -1,9 +1,13 @@ import React from 'react'; +import { SparklesIcon } from '@plannotator/ui/components/SparklesIcon'; import { CopyButton } from './CopyButton'; interface CommentActionsProps { /** When provided, shows the edit button (left-most). */ onEdit?: () => void; + /** When provided, asks AI for a learning-oriented explanation. */ + onExplain?: () => void; + explainDisabled?: boolean; /** When provided, shows the copy button (middle). */ copyText?: string; /** When provided, shows the delete/close button (right-most). Omitted for @@ -16,14 +20,14 @@ 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). + * left→right: edit · explain · copy · delete (so 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; +export const CommentActions: React.FC = ({ onEdit, onExplain, explainDisabled = false, copyText, onDelete }) => { + if (!onEdit && !onExplain && !copyText && !onDelete) return null; return (
e.stopPropagation()} > {onEdit && ( @@ -38,6 +42,18 @@ export const CommentActions: React.FC = ({ onEdit, copyText )} + {onExplain && ( + + )} {copyText && } {onDelete && (