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
35 changes: 35 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,41 @@ Send Feedback → feedback sent to agent session
Approve → "LGTM" sent to agent session
```

### Send with additional feedback

The agent-mode review toolbar's Send Feedback is a joined split pill
`[Send Feedback | v]` (`packages/review-editor/components/ReviewSendControl.tsx`,
wired through `AgentReviewActions`'s optional `note` prop — omit it and the
incumbent `FeedbackButton` renders unchanged). The LEFT segment never changes
meaning: same label, icon, `labelBreakpoint="lg"` spans and `handleSendFeedback`
as before, except that with nothing to send it opens the panel rather than
raising the "No Annotations" dialog. The caret opens a right-anchored panel with
a multi-line note field (Enter is a newline, `Mod+Enter` submits, `Escape` closes
and KEEPS the text, outside pointerdown closes) whose own distinct action reads
**Send with additional feedback**. The two actions are always different buttons.
While the panel (or the compact dialog) is open, the header's primary Send
Feedback fades to 40% and is disabled so the panel's action is unmistakably the
submit; every close path restores it. The panel action itself always renders
full-strength — an empty-note click is a no-op that refocuses the field (on
touch, that raises the keyboard).

The note is materialized at submit time by `commitReviewNote` in
`packages/review-editor/App.tsx` as a `scope: 'general'` `CodeAnnotation`
(`filePath: ''`, `lineStart/lineEnd: 0` — the documented sentinels), so it rides
the sidebar's General group, `renderGeneralComments`'s `## General` export
section, `buildFileScopedBody`, the draft, and the `/api/feedback` annotations
array with **zero server change** (`annotations` is `unknown[]` on both runtimes
and is only counted and forwarded). It is deliberately NOT recorded in review
undo history (it lives for one submit) and NOT stamped with PR context, so it
survives an in-place PR switch or a layer/full-stack toggle. Submission waits one
render, because `feedbackMarkdown` and `handleSendFeedback` close over
`allAnnotations`. Compact/touch gets the same commit path through an additive
`note` row in the header `ActionMenu` opening `ReviewNoteDialog`; platform (PR)
mode deliberately has no caret, since `ReviewSubmissionDialog` already owns the
general-comment field there. LGTM-with-a-note is a separate, coupled phase (four
consumer call sites discard `result.feedback` on the approved path) and is not
built.

### Since-main default review view

The default code-review diff is **`since-base`** — a composite of `merge-base(base, HEAD)` vs the working tree plus untracked files ("everything a PR would show if you committed and pushed now"). It can render as a three-section **git status** panel (Committed / Changes / Untracked) via `SectionsPanel`, with a `Tree | Git status | Commits` toggle (`PanelViewToggle`). The Commits segment (git-local sessions only) is a linear `--first-parent` history rail (`CommitsPanel`): clicking a commit opens its own diff (`commit:<sha>`, vs its first parent) as the all-files view headed by the commit message rendered as markdown. The Commits view is a self-contained detour: entering it memoizes the previously active diff, exiting to Tree restores that diff verbatim (exiting to Git status resets to `since-base` as always), the memo clears whenever any non-commit diff is applied, and a reload that serves a commit-family diff with a non-Commits panel view snaps once to the session default so the commit diff cannot outlive the visit. The toggle never writes the persisted `reviewPanelView`/`defaultDiffType` pair (no server writes from a toggle click), but it does record a cookie-only last-used memo (`reviewPanelViewLastUsed`, `sections` | `tree` — never `commits`; the Commits view is session-only). A review OPENS on session choice ?? last-used memo ?? persisted `reviewPanelView` (cookie-only, written only by Settings and `ReviewSetupDialog` through `setReviewPanelView()`, which also syncs the memo so an explicit choice is never shadowed by a stale one — except the App self-heal, which passes `recordLastUsed: false` to repair the diff half of a conflicted pair without touching the memo). The first-run initializer marks review-setup-seen when it seeds the cookie-only Tree choice, not only on dismiss, so it is genuinely one-time per browser and cannot overwrite a returning reviewer's persisted or last-used view; it inherits the resolved `defaultDiffType` without a server config write. The persisted pair is coupled: the Sections view only renders `since-base`, so choosing a classic diff default snaps the persisted view to Tree and vice-versa (enforced in `ReviewSetupDialog`, the Settings Git tab, and the App first-run initializer).
Expand Down
75 changes: 75 additions & 0 deletions packages/review-editor/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { ConfirmDialog } from '@plannotator/ui/components/ConfirmDialog';
import { Settings } from '@plannotator/ui/components/Settings';
import { FeedbackButton, ApproveButton, ExitButton } from '@plannotator/ui/components/ToolbarButtons';
import { AgentReviewActions } from './components/AgentReviewActions';
import { ReviewNoteDialog, type ReviewSubmitNoteControl } from './components/ReviewSendControl';
import { useUpdateCheck } from '@plannotator/ui/hooks/useUpdateCheck';
import { storage } from '@plannotator/ui/utils/storage';
import { CompletionOverlay } from '@plannotator/ui/components/CompletionOverlay';
Expand Down Expand Up @@ -3563,6 +3564,61 @@ const ReviewApp: React.FC = () => {
}
}, [getDraftGeneration]);

// --- Review-level note ("Send with additional feedback") ---------------
// The note is materialized at submit time as a scope:'general' annotation so
// it rides the existing export (## General) and the /api/feedback annotations
// array with no server change. Deliberately NOT recorded in review history
// (it lives for one submit) and deliberately NOT stamped with PR context, so
// it survives an in-place PR switch or a layer/full-stack toggle.
const [compactNoteOpen, setCompactNoteOpen] = useState(false);
const [pendingNoteId, setPendingNoteId] = useState<string | null>(null);

const commitReviewNote = useCallback((text: string): string | null => {
const trimmed = text.trim();
if (!trimmed) return null;
const note: CodeAnnotation = {
id: `review-note-${Date.now()}`,
type: 'comment',
scope: 'general',
filePath: '',
lineStart: 0,
lineEnd: 0,
side: 'new',
text: trimmed,
createdAt: Date.now(),
...(identity ? { author: identity } : {}),
};
annotationsRef.current = [...annotationsRef.current, note];
setAnnotations(annotationsRef.current);
return note.id;
}, [identity]);

const handleSubmitReviewNote = useCallback((text: string) => {
if (isSendingFeedback || isApproving || isExiting || submitted) return;
const id = commitReviewNote(text);
if (!id) {
// Nothing typed: fall back to the incumbent send when there is something
// to send, and otherwise do nothing (an empty note is not a submission).
if (totalAnnotationCount > 0) void handleSendFeedback();
return;
}
setPendingNoteId(id);
}, [commitReviewNote, handleSendFeedback, isApproving, isExiting, isSendingFeedback, submitted, totalAnnotationCount]);

// feedbackMarkdown and handleSendFeedback close over allAnnotations, so the
// send has to wait for the render that carries the note.
useEffect(() => {
if (!pendingNoteId) return;
if (!allAnnotations.some(a => a.id === pendingNoteId)) return;
setPendingNoteId(null);
void handleSendFeedback();
}, [allAnnotations, handleSendFeedback, pendingNoteId]);

const reviewNoteControl = useMemo<ReviewSubmitNoteControl>(
() => ({ onSubmit: handleSubmitReviewNote }),
[handleSubmitReviewNote],
);

// Submit reviews to one or more PRs via /api/pr-action
const handlePlatformAction = useCallback(async (action: 'approve' | 'comment', plan: ReviewSubmission, generalComment?: string) => {
setIsPlatformActioning(true);
Expand Down Expand Up @@ -3917,6 +3973,15 @@ const ReviewApp: React.FC = () => {
onSelect: () => totalAnnotationCount > 0 ? setShowExitWarning(true) : handleExit(),
disabled: compactActionBusy,
},
...(!platformMode
? [{
id: 'note' as const,
label: 'Add a note',
...(totalAnnotationCount > 0 ? { subtitle: 'Sent with your annotations' } : {}),
onSelect: () => setCompactNoteOpen(true),
disabled: compactActionBusy,
}]
: []),
...(totalAnnotationCount > 0
? [{
id: 'feedback' as const,
Expand Down Expand Up @@ -4289,6 +4354,7 @@ const ReviewApp: React.FC = () => {
onSendFeedback={handleSendFeedback}
onApprove={() => totalAnnotationCount > 0 ? setShowApproveWarning(true) : handleApprove()}
onExit={() => totalAnnotationCount > 0 ? setShowExitWarning(true) : handleExit()}
note={submitted ? undefined : reviewNoteControl}
/>
) : (
<>
Expand Down Expand Up @@ -4967,6 +5033,15 @@ const ReviewApp: React.FC = () => {
/>
)}

{/* Compact/touch review-level note composer */}
<ReviewNoteDialog
isOpen={compactNoteOpen}
onClose={() => setCompactNoteOpen(false)}
note={reviewNoteControl}
disabled={compactActionBusy || !!submitted}
annotationCount={totalAnnotationCount}
/>

{/* No annotations dialog */}
<ConfirmDialog
isOpen={showNoAnnotationsDialog}
Expand Down
27 changes: 20 additions & 7 deletions packages/review-editor/components/AgentReviewActions.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import { FeedbackButton, ApproveButton, ExitButton } from '@plannotator/ui/components/ToolbarButtons';
import { ReviewSendControl, type ReviewSubmitNoteControl } from './ReviewSendControl';

interface AgentReviewActionsProps {
totalAnnotationCount: number;
Expand All @@ -9,17 +10,20 @@ interface AgentReviewActionsProps {
onSendFeedback: () => void;
onApprove: () => void;
onExit: () => void;
/** Enables the note half of the split Send control. Omitted (a host that
* does not wire a note) falls back to the incumbent FeedbackButton. */
note?: ReviewSubmitNoteControl;
}

/**
* Toolbar actions for agent review mode (all non-platform origins).
*
* The left button flips based on whether there are annotations:
* No annotations → [Close] [Approve]
* Has annotations → [Send Feedback] [Approve]
*
* - Close (Exit): closes the session without sending feedback
* - Send Feedback: primary action when annotations exist
* - Send Feedback: the incumbent send. With a `note` it is the left segment of
* a split pill whose caret opens a review-level note composer; the segment's
* label, icon, breakpoints and handler are unchanged either way, and with no
* note wired it is the plain FeedbackButton shown only when annotations
* exist.
* - Approve: LGTM; dimmed when annotations exist (they won't be sent)
*/
export const AgentReviewActions: React.FC<AgentReviewActionsProps> = ({
Expand All @@ -30,6 +34,7 @@ export const AgentReviewActions: React.FC<AgentReviewActionsProps> = ({
onSendFeedback,
onApprove,
onExit,
note,
}) => {
const busy = isSendingFeedback || isApproving || isExiting;
const hasAnnotations = totalAnnotationCount > 0;
Expand All @@ -43,7 +48,15 @@ export const AgentReviewActions: React.FC<AgentReviewActionsProps> = ({
labelBreakpoint="lg"
/>

{hasAnnotations && (
{note ? (
<ReviewSendControl
hasFeedback={hasAnnotations}
disabled={busy}
isLoading={isSendingFeedback}
onSend={onSendFeedback}
note={note}
/>
) : hasAnnotations ? (
<FeedbackButton
onClick={onSendFeedback}
disabled={busy}
Expand All @@ -54,7 +67,7 @@ export const AgentReviewActions: React.FC<AgentReviewActionsProps> = ({
title="Send feedback"
labelBreakpoint="lg"
/>
)}
) : null}

<div className="relative group/approve inline-flex items-center">
<ApproveButton
Expand Down
46 changes: 46 additions & 0 deletions packages/review-editor/components/ReviewHeaderMenu.mobile.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,50 @@ describe('ReviewHeaderMenu compact review actions', () => {
expect(onFeedback).toHaveBeenCalledTimes(1);
expect(host?.textContent).not.toContain('Post comments');
});

// Standing toolbar-integrity rule: the additive 'note' row must not remove,
// reorder, or disable any incumbent compact action. The closed-union id edit
// is where that actually risks breaking.
test.skipIf(!hasDom)('the additive note row joins the incumbent rows without displacing them', async () => {
const onNote = mock(() => {});
host = document.createElement('div');
document.body.appendChild(host);
root = createRoot(host);

await act(async () => root?.render(
<ThemeProvider defaultTheme="dark">
<ReviewHeaderMenu
onOpenSettings={() => {}}
onOpenExport={() => {}}
onCopyAgentInstructions={() => {}}
onToggleFileTree={() => {}}
onToggleSidebar={() => {}}
isFileTreeOpen={false}
isSidebarOpen={false}
compactTouchLayout
compactActions={[
{ id: 'exit', label: 'Exit review', onSelect: () => {} },
{ id: 'note', label: 'Add a note', subtitle: 'Sent with your annotations', onSelect: onNote },
{ id: 'feedback', label: 'Send feedback', subtitle: '2 annotations', onSelect: () => {} },
{ id: 'approve', label: 'Approve', onSelect: () => {} },
]}
agentInstructionsEnabled={false}
appVersion="test"
/>
</ThemeProvider>,
));

await act(async () => host?.querySelector<HTMLButtonElement>('button[aria-label="Options"]')?.click());

const labels = ['Exit review', 'Add a note', 'Send feedback', 'Approve'];
const rows = Array.from(host?.querySelectorAll('button') ?? [])
.filter((button) => labels.some((label) => button.textContent?.includes(label)));
expect(rows.length).toBe(4);
expect(rows.map((button) => labels.find((label) => button.textContent?.includes(label)))).toEqual(labels);
expect(rows.every((button) => !button.disabled)).toBe(true);

const noteRow = rows[1];
await act(async () => noteRow.click());
expect(onNote).toHaveBeenCalledTimes(1);
});
});
9 changes: 8 additions & 1 deletion packages/review-editor/components/ReviewHeaderMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export interface CompactReviewDestination {
}

export interface CompactReviewAction {
id: 'exit' | 'feedback' | 'approve' | 'copy';
id: 'exit' | 'note' | 'feedback' | 'approve' | 'copy';
label: string;
subtitle?: string;
onSelect: () => void;
Expand Down Expand Up @@ -403,6 +403,13 @@ const CompactReviewActionIcon: React.FC<{ kind: CompactReviewAction['id'] }> = (
</svg>
);
}
if (kind === 'note') {
return (
<svg className="w-3.5 h-3.5 text-muted-foreground" fill="none" viewBox="0 0 24 24" stroke="currentColor" strokeWidth={2}>
<path strokeLinecap="round" strokeLinejoin="round" d="M11 5H6a2 2 0 00-2 2v11a2 2 0 002 2h11a2 2 0 002-2v-5m-1.414-9.414a2 2 0 112.828 2.828L11.828 15H9v-2.828l8.586-8.586z" />
</svg>
);
}
if (kind === 'copy') return <ExportIcon />;
return <CloseIcon />;
};
Loading
Loading