diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 76e672a1c..c8b155261 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -83,6 +83,7 @@ jobs: packages/ui/components/SkillReferenceMenu.placement.test.tsx packages/ui/components/sidebar/FileBrowser.test.ts packages/editor/editableDocumentsHook.test.tsx + packages/review-editor/components/CommentActions.test.tsx packages/review-editor/components/ReviewSubmissionDialog.ui.test.tsx packages/review-editor/components/FileHeader.edit.test.tsx packages/review-editor/edit/useEditSession.recovery.test.tsx diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index faced5acc..805e8ee79 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -43,6 +43,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'; @@ -474,6 +478,11 @@ const ReviewApp: React.FC = () => { // so this should be addressed as a broader refactor. const { externalAnnotations, updateExternalAnnotation, deleteExternalAnnotation } = useExternalAnnotations({ enabled: !!origin }); const agentJobs = useAgentJobs({ enabled: !!origin && aiUIEnabled }); + const agentFindingSourcesKey = agentJobs.jobs.map((job) => job.source).sort().join('\0'); + const agentFindingSources = useMemo( + () => new Set(agentFindingSourcesKey ? agentFindingSourcesKey.split('\0') : []), + [agentFindingSourcesKey], + ); // Tour dialog state — opens as an overlay instead of a dock panel const [tourDialogJobId, setTourDialogJobId] = useState(null); @@ -745,6 +754,7 @@ const ReviewApp: React.FC = () => { resetSession: resetAISession, sessionId: aiSessionId, } = aiChat; + const isAILoading = aiIsCreatingSession || aiIsStreaming; const codeNav = useCodeNav(); @@ -1053,6 +1063,19 @@ const ReviewApp: React.FC = () => { if (!sha) return null; return { sha, subject: commitInfo?.sha === sha ? commitInfo.subject : undefined }; }, [activeDiffBase, commitInfo]); + const handleExplainAnnotation = useCallback((id: string) => { + if (!aiAvailable || isAILoading) return; + const annotation = allAnnotationsRef.current.find((item) => item.id === id); + if (!annotation || !isAgentGeneratedFinding(annotation, agentFindingSources)) return; + + const file = annotation.filePath + ? files.find((item) => item.path === annotation.filePath) + : undefined; + reviewSidebar.open('ai'); + void askAI(buildExplainFindingRequest(annotation, file?.patch, { + activeCommitSha: activeCommitContext?.sha, + })); + }, [activeCommitContext?.sha, agentFindingSources, aiAvailable, askAI, files, isAILoading]); const activeGitButlerContext = useMemo(() => { if (!activeDiffBase.startsWith('gitbutler:')) return null; return { @@ -2476,6 +2499,8 @@ const ReviewApp: React.FC = () => { onSelectAnnotation: handleSelectAnnotation, onNavigateToAnnotation: handleNavigateToAnnotation, onDeleteAnnotation: handleDeleteAnnotation, + onExplainAnnotation: handleExplainAnnotation, + agentFindingSources, descriptionAnnotations: visibleDescriptionAnnotations, selectedDescriptionAnnotationId, onAddDescriptionAnnotation: handleAddDescriptionAnnotation, @@ -2514,7 +2539,7 @@ const ReviewApp: React.FC = () => { aiMessages, onAskAI: handleAskAI, onAskAIForFile: handleAskAIForFile, - isAILoading: aiIsCreatingSession || aiIsStreaming, + isAILoading, onViewAIResponse: handleViewAIResponse, onClickAIMarker: handleClickAIMarker, aiHistoryForSelection, @@ -2558,11 +2583,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, @@ -3707,6 +3732,8 @@ const ReviewApp: React.FC = () => { onSelectAnnotation={handleSelectAnnotation} onNavigateToAnnotation={handleNavigateToAnnotation} onDeleteAnnotation={handleDeleteAnnotation} + onExplainAnnotation={aiAvailable ? handleExplainAnnotation : undefined} + agentFindingSources={agentFindingSources} feedbackMarkdown={feedbackMarkdown} width={panelResize.width} editorAnnotations={visibleEditorAnnotations} diff --git a/packages/review-editor/components/AllFilesCodeView.lifecycle.test.tsx b/packages/review-editor/components/AllFilesCodeView.lifecycle.test.tsx index 1c957572b..5e6088213 100644 --- a/packages/review-editor/components/AllFilesCodeView.lifecycle.test.tsx +++ b/packages/review-editor/components/AllFilesCodeView.lifecycle.test.tsx @@ -131,6 +131,7 @@ function view(overrides: Partial> onEditAnnotation={() => {}} onSelectAnnotation={() => {}} onDeleteAnnotation={() => {}} + agentFindingSources={new Set()} {...overrides} /> ); diff --git a/packages/review-editor/components/AllFilesCodeView.tsx b/packages/review-editor/components/AllFilesCodeView.tsx index 8e0932672..997c5b44c 100644 --- a/packages/review-editor/components/AllFilesCodeView.tsx +++ b/packages/review-editor/components/AllFilesCodeView.tsx @@ -202,6 +202,8 @@ interface AllFilesCodeViewProps { ) => void; onSelectAnnotation: (id: string | null) => void; onDeleteAnnotation: (id: string) => void; + onExplainAnnotation?: (id: string) => void; + agentFindingSources: ReadonlySet; // Header actions (P3). Mirror AllFilesDiffView's header surface. onAddFileCommentForFile?: (filePath: string, text: string) => void; viewedFiles?: Set; @@ -490,6 +492,8 @@ export const AllFilesCodeView: React.FC = ({ onEditAnnotation, onSelectAnnotation, onDeleteAnnotation, + onExplainAnnotation, + agentFindingSources, onAddFileCommentForFile, viewedFiles, onToggleViewed, @@ -530,6 +534,7 @@ export const AllFilesCodeView: React.FC = ({ onAddSuggestionsForFile, onAddEditorCommentForFile, }) => { + const explainAvailable = Boolean(onExplainAnnotation); const mountCollapsedRef = useRef(mountCollapsed); const seedCollapsed = mountCollapsedRef.current ?? defaultCollapsed; @@ -846,7 +851,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 @@ -875,10 +880,24 @@ export const AllFilesCodeView: React.FC = ({ onSelect={onSelectAnnotation} onEdit={handleEditAnnotation} onDelete={onDeleteAnnotation} + onExplain={onExplainAnnotation} + agentFindingSources={agentFindingSources} + 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, explainAvailable, agentFindingSources, 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 / @@ -2105,7 +2124,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; @@ -2211,6 +2230,9 @@ export const AllFilesCodeView: React.FC = ({ onSelect={onSelectAnnotation} onEdit={onEditAnnotation} onDelete={onDeleteAnnotation} + onExplain={onExplainAnnotation} + agentFindingSources={agentFindingSources} + 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. @@ -2220,6 +2242,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, explainAvailable, agentFindingSources, 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 && (