Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
33 changes: 30 additions & 3 deletions packages/review-editor/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -474,6 +478,11 @@ const ReviewApp: React.FC = () => {
// so this should be addressed as a broader refactor.
const { externalAnnotations, updateExternalAnnotation, deleteExternalAnnotation } = useExternalAnnotations<CodeAnnotation>({ 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<string | null>(null);
Expand Down Expand Up @@ -745,6 +754,7 @@ const ReviewApp: React.FC = () => {
resetSession: resetAISession,
sessionId: aiSessionId,
} = aiChat;
const isAILoading = aiIsCreatingSession || aiIsStreaming;

const codeNav = useCodeNav();

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -2476,6 +2499,8 @@ const ReviewApp: React.FC = () => {
onSelectAnnotation: handleSelectAnnotation,
onNavigateToAnnotation: handleNavigateToAnnotation,
onDeleteAnnotation: handleDeleteAnnotation,
onExplainAnnotation: handleExplainAnnotation,
agentFindingSources,
descriptionAnnotations: visibleDescriptionAnnotations,
selectedDescriptionAnnotationId,
onAddDescriptionAnnotation: handleAddDescriptionAnnotation,
Expand Down Expand Up @@ -2514,7 +2539,7 @@ const ReviewApp: React.FC = () => {
aiMessages,
onAskAI: handleAskAI,
onAskAIForFile: handleAskAIForFile,
isAILoading: aiIsCreatingSession || aiIsStreaming,
isAILoading,
onViewAIResponse: handleViewAIResponse,
onClickAIMarker: handleClickAIMarker,
aiHistoryForSelection,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ function view(overrides: Partial<React.ComponentProps<typeof AllFilesCodeView>>
onEditAnnotation={() => {}}
onSelectAnnotation={() => {}}
onDeleteAnnotation={() => {}}
agentFindingSources={new Set()}
{...overrides}
/>
);
Expand Down
32 changes: 30 additions & 2 deletions packages/review-editor/components/AllFilesCodeView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,8 @@ interface AllFilesCodeViewProps {
) => void;
onSelectAnnotation: (id: string | null) => void;
onDeleteAnnotation: (id: string) => void;
onExplainAnnotation?: (id: string) => void;
agentFindingSources: ReadonlySet<string>;
// Header actions (P3). Mirror AllFilesDiffView's header surface.
onAddFileCommentForFile?: (filePath: string, text: string) => void;
viewedFiles?: Set<string>;
Expand Down Expand Up @@ -490,6 +492,8 @@ export const AllFilesCodeView: React.FC<AllFilesCodeViewProps> = ({
onEditAnnotation,
onSelectAnnotation,
onDeleteAnnotation,
onExplainAnnotation,
agentFindingSources,
onAddFileCommentForFile,
viewedFiles,
onToggleViewed,
Expand Down Expand Up @@ -530,6 +534,7 @@ export const AllFilesCodeView: React.FC<AllFilesCodeViewProps> = ({
onAddSuggestionsForFile,
onAddEditorCommentForFile,
}) => {
const explainAvailable = Boolean(onExplainAnnotation);
const mountCollapsedRef = useRef(mountCollapsed);
const seedCollapsed = mountCollapsedRef.current ?? defaultCollapsed;

Expand Down Expand Up @@ -846,7 +851,7 @@ export const AllFilesCodeView: React.FC<AllFilesCodeViewProps> = ({
// 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<DiffAnnotationMetadata>
Expand Down Expand Up @@ -875,10 +880,24 @@ export const AllFilesCodeView: React.FC<AllFilesCodeViewProps> = ({
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<DiffAnnotationMetadata>
| LineAnnotation<DiffAnnotationMetadata>,
item: CodeViewItem<DiffAnnotationMetadata>,
) => 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 /
Expand Down Expand Up @@ -2105,7 +2124,7 @@ export const AllFilesCodeView: React.FC<AllFilesCodeViewProps> = ({

// --- Custom header render slot (the full Plannotator FileHeader) -----------

const renderCustomHeader = useStableCallback((item: CodeViewItem<DiffAnnotationMetadata>) => {
const renderCustomHeaderContent = useStableCallback((item: CodeViewItem<DiffAnnotationMetadata>) => {
if (item.type !== 'diff') return null;
const filePath = itemIdToFilePath.get(item.id);
if (filePath == null) return null;
Expand Down Expand Up @@ -2211,6 +2230,9 @@ export const AllFilesCodeView: React.FC<AllFilesCodeViewProps> = ({
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.
Expand All @@ -2220,6 +2242,12 @@ export const AllFilesCodeView: React.FC<AllFilesCodeViewProps> = ({
</div>
);
});
// File-scoped findings live in the custom-header portal and need the same
// availability/loading republish as line annotations.
const renderCustomHeader = useCallback(
(item: CodeViewItem<DiffAnnotationMetadata>) => 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
Expand Down
58 changes: 58 additions & 0 deletions packages/review-editor/components/CommentActions.test.tsx
Original file line number Diff line number Diff line change
@@ -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(
<CommentActions onExplain={() => { explanationRequests += 1; }} />,
);
});

const explainButton = host.querySelector<HTMLButtonElement>('[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(
<CommentActions
onExplain={() => { explanationRequests += 1; }}
explainDisabled
/>,
);
});

const explainButton = host.querySelector<HTMLButtonElement>('[aria-label="Explain finding"]');
expect(explainButton?.disabled).toBe(true);

await act(async () => explainButton?.click());
expect(explanationRequests).toBe(0);
});
});
24 changes: 20 additions & 4 deletions packages/review-editor/components/CommentActions.tsx
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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<CommentActionsProps> = ({ onEdit, copyText, onDelete }) => {
if (!onEdit && !copyText && !onDelete) return null;
export const CommentActions: React.FC<CommentActionsProps> = ({ onEdit, onExplain, explainDisabled = false, copyText, onDelete }) => {
if (!onEdit && !onExplain && !copyText && !onDelete) return null;
return (
<div
className="flex items-center justify-end gap-1 mt-1.5 opacity-0 group-hover:opacity-100 transition-opacity"
className="flex items-center justify-end gap-1 mt-1.5 opacity-0 group-hover:opacity-100 [@media(hover:none)]:opacity-100 transition-opacity"
onClick={(e) => e.stopPropagation()}
>
{onEdit && (
Expand All @@ -38,6 +42,18 @@ export const CommentActions: React.FC<CommentActionsProps> = ({ onEdit, copyText
</svg>
</button>
)}
{onExplain && (
<button
type="button"
onClick={(e) => { e.stopPropagation(); onExplain(); }}
disabled={explainDisabled}
className={`${ACTION_BTN} hover:bg-primary/10 hover:text-primary disabled:cursor-not-allowed disabled:opacity-40`}
title="Explain finding"
aria-label="Explain finding"
>
<SparklesIcon className="w-3 h-3" />
</button>
)}
{copyText && <CopyButton text={copyText} variant="inline" />}
{onDelete && (
<button
Expand Down
12 changes: 11 additions & 1 deletion packages/review-editor/components/DiffViewer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ interface DiffViewerProps {
onEditAnnotation: (id: string, text?: string, suggestedCode?: string, originalCode?: string, conventionalLabel?: ConventionalLabel | null, decorations?: ConventionalDecoration[]) => void;
onSelectAnnotation: (id: string | null) => void;
onDeleteAnnotation: (id: string) => void;
onExplainAnnotation?: (id: string) => void;
agentFindingSources: ReadonlySet<string>;
isViewed?: boolean;
onToggleViewed?: () => void;
collapsed?: boolean;
Expand Down Expand Up @@ -225,6 +227,8 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
onEditAnnotation,
onSelectAnnotation,
onDeleteAnnotation,
onExplainAnnotation,
agentFindingSources,
isViewed = false,
onToggleViewed,
collapsed = false,
Expand Down Expand Up @@ -611,9 +615,12 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
onSelect={onSelectAnnotation}
onEdit={handleEdit}
onDelete={onDeleteAnnotation}
onExplain={onExplainAnnotation}
agentFindingSources={agentFindingSources}
explainDisabled={isAILoading}
/>
);
}, [filePath, selectedAnnotationId, onSelectAnnotation, handleEdit, onDeleteAnnotation, onClickAIMarker]);
}, [filePath, selectedAnnotationId, onSelectAnnotation, handleEdit, onDeleteAnnotation, onExplainAnnotation, isAILoading, onClickAIMarker]);

const handleGutterUtilityClick = useCallback((range: SelectedLineRange) => {
toolbarHostRef.current?.handleLineSelectionEnd(range);
Expand Down Expand Up @@ -751,6 +758,9 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
onSelect={onSelectAnnotation}
onEdit={onEditAnnotation}
onDelete={onDeleteAnnotation}
onExplain={onExplainAnnotation}
agentFindingSources={agentFindingSources}
explainDisabled={isAILoading}
/>
<div className="p-4" ref={diffContentRef}>
<div ref={splitSurfaceRef} className="relative min-w-0" style={splitGridStyle}>
Expand Down
Loading