feat(review): send with additional feedback split control - #1449
Open
backnotprop wants to merge 3 commits into
Open
feat(review): send with additional feedback split control#1449backnotprop wants to merge 3 commits into
backnotprop wants to merge 3 commits into
Conversation
The agent-mode code review toolbar's Send Feedback becomes one joined split pill [Send Feedback | v]. The left segment is the incumbent button verbatim — same label, icon, labelBreakpoint="lg" spans, disabled/loading behavior and handleSendFeedback — with one addition: at zero annotations it opens the note panel instead of raising the "No Annotations" dialog. Nothing is removed or hidden; Close and Approve are untouched, and AgentReviewActions falls back to the plain FeedbackButton when no `note` prop is wired. The caret opens a right-anchored panel holding a multi-line auto-growing note field (rows=2, capped at 144px then scrolls) and a footer of submitHint plus a DISTINCT action button labeled "Send with additional feedback". The two actions are never the same button. Enter inserts a newline; Mod+Enter (meta or ctrl) fires the distinct action; Escape closes and KEEPS the typed text, with stopPropagation so it never reaches the review app's own Escape ladder; a pointerdown outside closes. The text lives in the control, so a keystroke never re-renders the review header. At submit the note is materialized as a scope:'general' CodeAnnotation (filePath '', lineStart/lineEnd 0 — the documented sentinels) and pushed into the annotation list, then the send waits one render because feedbackMarkdown and handleSendFeedback close over allAnnotations. That makes the sidebar's General group, renderGeneralComments's ## General export section, buildFileScopedBody and the draft pick it up for free, and it rides the existing /api/feedback annotations array with ZERO server change on either runtime. The note 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. Compact/touch shares the same commit path through an additive `note` row in the header ActionMenu opening ReviewNoteDialog; platform (PR) mode gets no caret, because ReviewSubmissionDialog already owns the general-comment field there. The control is a deliberate duplicate of the annotate twin (packages/editor/components/AnnotateSendControl.tsx) with an extraction tripwire in its module doc. LGTM-with-a-note is explicitly deferred. Tests: component DOM contract (split shape, caret, plain-Enter, Mod+Enter, distinct action, Escape-keeps-text, empty/whitespace guard, zero-annotation primary, outside click, frozen action label); the pure export trace through renderGeneralComments; a dual-runtime /api/feedback payload test asserting the note reaches waitForDecision unmodified and archives as a feedback decision; and a compact-menu toolbar-integrity test for the additive row.
Same maintainer-directed disambiguation as the annotate control: an open note panel fades the header's Send Feedback to 40% and disables it (a faded-but-clickable send would silently drop the typed note), and restores it when the panel closes. The panel's action always renders full-strength — it started disabled-gray on empty text, which put two dimmed buttons on screen and made the live one look dead; an empty-note click now refocuses the field instead.
Independent review verdict was merge-with-minor-changes; this closes the two fixable findings and the doc nit: - The compact dialog's empty-note tap was a silent dead button (the desktop panel refocuses the field, the dialog just returned). It now mirrors the desktop contract: refocus the field, which on touch also raises the keyboard. - The dialog gains the desktop panel's disabled auto-close, so it cannot sit open with a no-op action after the surface goes busy. - AGENTS.md now records the fade+disable of the header primary while the note surface is open, and the always-full-strength panel action. The archive scope-field gap (F3) stays deferred with the design doc's open question.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Brings the one-step note-with-submit affordance to code review, per the approved spec (DESIGN_review-send-with-note.md) and matching the maintainer-approved control on #1436 exactly:
scope: 'general'CodeAnnotation, riding the existing sidebar General group,renderGeneralCommentsexport,/api/feedbackpayload, and the feedback archive — zero server changes on either runtime (both handlers forward the annotations array verbatim; verified in the spec and pinned by dual-runtime payload tests).noterow in the review ActionMenu opening a dialog with the same field.Scope guarantees
Zero files changed under
packages/serverorapps/pi-extension.packages/uichanges are one additive shortcut scope file plus its barrel export. No existing button is removed or hidden — the split's left segment IS the incumbent Send Feedback.Tests
11 control DOM tests (pill anatomy, panel open/close, newline vs Mod+Enter, Esc-keeps-text, outside-click, fade+disable of the primary with restore, panel action never grayed, frozen action label), export trace through
renderGeneralComments, dual-runtime payload assertions, compact menu row. Fullpackages/review-editor+ shortcuts suite: 449 pass. Typecheck clean;apps/reviewbuild verified; reviewed live by the maintainer across three design iterations.AI-assisted (Claude) under maintainer direction.