fix: place diff comment box at end of selection and highlight selected lines (#23288)

site/src/components/ai-elements/tool/utils.ts

@@ -150,8 +150,91 @@ export const formatResultOutput = (result: unknown): string | null => {
 export const fileViewerCSS =
 	"pre, [data-line], [data-diffs-header] { background-color: transparent !important; }";
 
-export const diffViewerCSS =
-	"pre, [data-line], [data-diffs-header] { background-color: transparent !important; } [data-diffs-header] { border-left: 1px solid var(--border); }";
+// Selection override CSS maps the library's gold/yellow selection
+// palette to the Coder blue accent (`--content-link`) so line
+// highlighting feels native to the rest of the page.
+//
+// The library has two selection code paths: context lines use
+// `--diffs-bg-selection`, but change-addition/deletion lines
+// use a separate `color-mix()` against `--diffs-line-bg`. To
+// guarantee a uniform highlight across all line types we set
+// the CSS variables for annotations AND apply direct rules
+// with `!important` for line and gutter elements.
+const SELECTION_OVERRIDE_CSS = [
+	// Variable overrides for annotation areas and library internals.
+	":host {",
+	"  --diffs-bg-selection-override: hsl(var(--content-link) / 0.08);",
+	"  --diffs-bg-selection-number-override: hsl(var(--content-link) / 0.13);",
+	"  --diffs-selection-number-fg: hsl(var(--content-link));",
+	"}",
+	// Direct rules that override both context and change-line
+	// selection backgrounds so every selected line looks the same.
+	"[data-selected-line][data-line] {",
+	"  background-color: hsl(var(--content-link) / 0.08) !important;",
+	"}",
+	"[data-selected-line][data-column-number] {",
+	"  background-color: hsl(var(--content-link) / 0.13) !important;",
+	"  color: hsl(var(--content-link)) !important;",
+	"}",
+	// Clear the selection tint from annotation rows so the inline
+	// prompt input stands out clearly against the selected lines.
+	"[data-line-annotation][data-selected-line] [data-annotation-content] {",
+	"  background-color: transparent !important;",
+	"}",
+	"[data-line-annotation][data-selected-line]::before {",
+	"  background-color: transparent !important;",
+	"}",
+	"[data-selected-line][data-gutter-buffer='annotation'] {",
+	"  background-color: transparent !important;",
+	"}",
+].join(" ");
+
+// Restyled separators: quiet, full-width dividers that fade
+// into the background instead of drawing attention.
+const SEPARATOR_CSS = [
+	// Transparent backgrounds so separators blend with the
+	// code area rather than forming a distinct band.
+	":host {",
+	"  --diffs-bg-separator-override: transparent;",
+	"}",
+	"[data-separator-content] {",
+	"  border-radius: 0 !important;",
+	"  background-color: transparent !important;",
+	"}",
+	"[data-separator-wrapper] {",
+	"  border-radius: 0 !important;",
+	"}",
+	// Remove the inline padding that creates the inset pill look
+	// so separators span the full width of the diff.
+	"[data-unified] [data-separator='line-info'] [data-separator-wrapper] {",
+	"  padding-inline: 0 !important;",
+	"}",
+	// The first separator in a file just says "N unmodified
+	// lines" before the first hunk — that's obvious context
+	// that adds no value, so hide it entirely.
+	"[data-separator='line-info'][data-separator-first] {",
+	"  display: none !important;",
+	"}",
+	// Thin single border and muted text so collapsed-line
+	// indicators read as a quiet hint, not a landmark.
+	"[data-separator='line-info'] {",
+	"  height: 28px !important;",
+	"  border-top: 1px solid hsl(var(--border-default));",
+	"  border-bottom: 1px solid hsl(var(--border-default));",
+	"}",
+	"[data-separator-content] {",
+	"  font-size: 11px !important;",
+	"  color: hsl(var(--content-secondary)) !important;",
+	"  opacity: 0.8;",
+	"}",
+].join(" ");
+
+export const diffViewerCSS = [
+	"pre, [data-line]:not([data-selected-line]), [data-diffs-header] { background-color: transparent !important; }",
+	"[data-diffs-header] { border-left: 1px solid var(--border); }",
+	SELECTION_OVERRIDE_CSS,
+	SEPARATOR_CSS,
+].join(" ");
 
 // Theme-aware option factories shared across tool renderers.
 export function getDiffViewerOptions(isDark: boolean) {

site/src/pages/AgentsPage/DiffViewer.stories.tsx

@@ -0,0 +1,152 @@
+import type { DiffLineAnnotation, SelectedLineRange } from "@pierre/diffs";
+import { parsePatchFiles } from "@pierre/diffs";
+import type { Meta, StoryObj } from "@storybook/react-vite";
+import { fn } from "storybook/test";
+import type { DiffStyle } from "./DiffViewer";
+import { DiffViewer } from "./DiffViewer";
+import { InlinePromptInput } from "./RemoteDiffPanel";
+
+// biome-ignore format: raw diff string must preserve exact whitespace
+const sampleDiff = [
+"diff --git a/src/main.ts b/src/main.ts",
+"index abc1234..def5678 100644",
+"--- a/src/main.ts",
+"+++ b/src/main.ts",
+"@@ -1,5 +1,7 @@",
+" import { start } from \"./server\";",
+"+import { logger } from \"./logger\";",
+"",
+" const port = 3000;",
+"+logger.info(\"Starting server...\");",
+" start(port);",
+"diff --git a/src/server.ts b/src/server.ts",
+"index 1111111..2222222 100644",
+"--- a/src/server.ts",
+"+++ b/src/server.ts",
+"@@ -10,3 +10,5 @@",
+"   app.listen(port, () => {",
+"     console.log(\"Listening on port \" + port);",
+"   });",
+"+",
+"+  return app;",
+" }",
+].join("\n");
+const parsedFiles = parsePatchFiles(sampleDiff).flatMap((p) => p.files);
+const firstFileName = parsedFiles[0]?.name ?? "";
+
+const meta: Meta<typeof DiffViewer> = {
+	title: "pages/AgentsPage/DiffViewer",
+	component: DiffViewer,
+	args: {
+		parsedFiles,
+		diffStyle: "unified" satisfies DiffStyle,
+		onLineNumberClick: fn(),
+		onLineSelected: fn(),
+		onScrollToFileComplete: fn(),
+	},
+	decorators: [
+		(Story) => (
+			<div style={{ height: 500, width: 700 }}>
+				<Story />
+			</div>
+		),
+	],
+};
+export default meta;
+type Story = StoryObj<typeof DiffViewer>;
+
+export const Default: Story = {};
+
+export const SplitView: Story = {
+	args: {
+		diffStyle: "split",
+	},
+};
+
+export const Loading: Story = {
+	args: {
+		parsedFiles: [],
+		isLoading: true,
+	},
+};
+
+export const ErrorState: Story = {
+	name: "Error",
+	args: {
+		parsedFiles: [],
+		error: new Error("Failed to fetch diff"),
+	},
+};
+
+export const Empty: Story = {
+	args: {
+		parsedFiles: [],
+		emptyMessage: "No file changes to display.",
+	},
+};
+
+export const WithSelectedLines: Story = {
+	args: {
+		getSelectedLines: (fileName: string): SelectedLineRange | null => {
+			if (fileName === firstFileName) {
+				return { start: 2, end: 4, side: "additions" };
+			}
+			return null;
+		},
+	},
+};
+
+// Diff with two non-adjacent hunks in one file, producing a
+// mid-file separator that should remain visible even though
+// leading separators are hidden.
+// biome-ignore format: raw diff string must preserve exact whitespace
+const multiHunkDiff = [
+"diff --git a/src/app.ts b/src/app.ts",
+"index aaa1111..bbb2222 100644",
+"--- a/src/app.ts",
+"+++ b/src/app.ts",
+"@@ -3,4 +3,5 @@",
+" import { db } from \"./db\";",
+" import { logger } from \"./logger\";",
+"+import { metrics } from \"./metrics\";",
+" ",
+" const app = express();",
+"@@ -20,3 +21,4 @@",
+" app.listen(port, () => {",
+"   console.log(\"Listening on port \" + port);",
+"+  metrics.record(\"server.start\");",
+" });",
+].join("\n");
+const multiHunkFiles = parsePatchFiles(multiHunkDiff).flatMap((p) => p.files);
+
+export const WithMidFileSeparator: Story = {
+	args: {
+		parsedFiles: multiHunkFiles,
+	},
+};
+
+export const WithAnnotation: Story = {
+	args: {
+		getSelectedLines: (fileName: string): SelectedLineRange | null => {
+			if (fileName === firstFileName) {
+				return { start: 2, end: 4, side: "additions" };
+			}
+			return null;
+		},
+		getLineAnnotations: (fileName: string): DiffLineAnnotation<string>[] => {
+			if (fileName === firstFileName) {
+				return [
+					{
+						lineNumber: 4,
+						side: "additions",
+						metadata: "active-input",
+					},
+				];
+			}
+			return [];
+		},
+		renderAnnotation: () => (
+			<InlinePromptInput onSubmit={fn()} onCancel={fn()} />
+		),
+	},
+};

site/src/pages/AgentsPage/DiffViewer.tsx

@@ -1,5 +1,9 @@
 import { useTheme } from "@emotion/react";
-import type { DiffLineAnnotation, FileDiffMetadata } from "@pierre/diffs";
+import type {
+	DiffLineAnnotation,
+	FileDiffMetadata,
+	SelectedLineRange,
+} from "@pierre/diffs";
 import { FileDiff } from "@pierre/diffs/react";
 import { ErrorAlert } from "components/Alert/ErrorAlert";
 import {
@@ -69,6 +73,11 @@ interface DiffViewerProps {
 	 * inline widgets such as comment inputs.
 	 */
 	getLineAnnotations?: (fileName: string) => DiffLineAnnotation<string>[];
+	/**
+	 * Returns the selected line range for the given file, if any.
+	 * Used to visually highlight the lines being commented on.
+	 */
+	getSelectedLines?: (fileName: string) => SelectedLineRange | null;
 	/**
 	 * Renderer for line annotations returned by `getLineAnnotations`.
 	 */
@@ -98,15 +107,84 @@ const FILE_TREE_THRESHOLD = 1000;
  * file headers sticky and adjust metadata layout.
  */
 const STICKY_HEADER_CSS = [
+	// Layout and sticky behavior.
 	"[data-diffs-header] {",
 	"  position: sticky; top: 0; z-index: 10;",
 	"  font-size: 13px;",
+	"  min-height: 32px !important;",
+	"  padding-block: 0 !important;",
+	"  padding-inline: 12px !important;",
 	"  border-bottom: 1px solid hsl(var(--border-default));",
-	"  background-color: hsl(var(--surface-quaternary)) !important;",
+	"  background-color: hsl(var(--surface-secondary)) !important;",
 	"}",
-	"[data-diffs-header] [data-metadata] { flex-direction: row-reverse; }",
-	"@media (prefers-color-scheme: dark) {",
-	"  [data-diffs-header] { background-color: hsl(var(--surface-secondary)) !important; }",
+
+	// Keep the title in the site's sans-serif font, just a
+	// touch smaller than the surrounding header text.
+	"[data-diffs-header] [data-title] {",
+	"  font-size: 12px;",
+	"  color: hsl(var(--content-primary));",
+	"}",
+
+	// Hide the library's built-in change-type SVG icons and
+	// replace them with a single-letter badge (A/D/M/R) via
+	// CSS-generated content. The letter mirrors the file tree
+	// sidebar and works even when the tree is hidden in narrow
+	// layouts.
+	"[data-change-icon] { display: none !important; }",
+	"[data-diffs-header] [data-header-content]::before {",
+	"  font-size: 11px;",
+	"  font-weight: 600;",
+	"  flex-shrink: 0;",
+	"}",
+	"[data-diffs-header][data-change-type='new'] [data-header-content]::before {",
+	"  content: 'A';",
+	"  color: hsl(var(--git-added));",
+	"}",
+	"[data-diffs-header][data-change-type='change'] [data-header-content]::before {",
+	"  content: 'M';",
+	"  color: hsl(var(--git-modified));",
+	"}",
+	"[data-diffs-header][data-change-type='deleted'] [data-header-content]::before {",
+	"  content: 'D';",
+	"  color: hsl(var(--git-deleted));",
+	"}",
+	"[data-diffs-header][data-change-type='rename-pure'] [data-header-content]::before,",
+	"[data-diffs-header][data-change-type='rename-changed'] [data-header-content]::before {",
+	"  content: 'R';",
+	"  color: hsl(var(--git-modified));",
+	"}",
+
+	// Stat counts styled as compact pill badges matching the
+	// DiffStatBadge component used in the PR header.
+	"[data-diffs-header] [data-metadata] {",
+	"  flex-direction: row-reverse;",
+	"  gap: 0 !important;",
+	"}",
+	"[data-diffs-header] [data-additions-count],",
+	"[data-diffs-header] [data-deletions-count] {",
+	"  font-family: var(--diffs-font-family, var(--diffs-font-fallback));",
+	"  font-size: 12px;",
+	"  font-weight: 500;",
+	"  line-height: 20px;",
+	"  padding-inline: 6px;",
+	"  border-radius: 3px;",
+	"}",
+	"[data-diffs-header] [data-additions-count] {",
+	"  color: hsl(var(--git-added-bright)) !important;",
+	"  background-color: hsl(var(--surface-git-added));",
+	"}",
+	"[data-diffs-header] [data-deletions-count] {",
+	"  color: hsl(var(--git-deleted-bright)) !important;",
+	"  background-color: hsl(var(--surface-git-deleted));",
+	"}",
+	// When both counts are present, flatten the touching inner
+	// edges so they form one joined badge. DOM order is
+	// [deletions][additions]; row-reverse puts additions left.
+	"[data-deletions-count] + [data-additions-count] {",
+	"  border-radius: 3px 0 0 3px;",
+	"}",
+	"[data-deletions-count]:has(+ [data-additions-count]) {",
+	"  border-radius: 0 3px 3px 0;",
 	"}",
 ].join(" ");
 
@@ -346,12 +424,14 @@ const LazyFileDiff = memo<{
 	options: ComponentProps<typeof FileDiff>["options"];
 	lineAnnotations?: DiffLineAnnotation<string>[];
 	renderAnnotation?: (annotation: DiffLineAnnotation<string>) => ReactNode;
+	selectedLines?: SelectedLineRange | null;
 }>(
 	({
 		fileDiff,
 		options,
 		lineAnnotations,
 		renderAnnotation: renderAnnotationProp,
+		selectedLines,
 	}) => {
 		const placeholderRef = useRef<HTMLDivElement>(null);
 		const [visible, setVisible] = useState(false);
@@ -399,6 +479,7 @@ const LazyFileDiff = memo<{
 				style={DIFFS_FONT_STYLE}
 				lineAnnotations={lineAnnotations}
 				renderAnnotation={renderAnnotationProp}
+				selectedLines={selectedLines}
 			/>
 		);
 	},
@@ -406,7 +487,8 @@ const LazyFileDiff = memo<{
 		if (
 			prev.fileDiff !== next.fileDiff ||
 			prev.options !== next.options ||
-			prev.lineAnnotations !== next.lineAnnotations
+			prev.lineAnnotations !== next.lineAnnotations ||
+			prev.selectedLines !== next.selectedLines
 		) {
 			return false;
 		}
@@ -436,6 +518,7 @@ export const DiffViewer: FC<DiffViewerProps> = ({
 	onLineNumberClick,
 	onLineSelected,
 	getLineAnnotations,
+	getSelectedLines,
 	renderAnnotation,
 	scrollToFile,
 	onScrollToFileComplete,
@@ -778,6 +861,7 @@ export const DiffViewer: FC<DiffViewerProps> = ({
 										options={perFileOptions?.get(fileDiff.name) ?? fileOptions}
 										lineAnnotations={perFileAnnotations?.get(fileDiff.name)}
 										renderAnnotation={renderAnnotation}
+										selectedLines={getSelectedLines?.(fileDiff.name)}
 									/>
 								</div>
 							))}

site/src/pages/AgentsPage/InlinePromptInput.stories.tsx

@@ -0,0 +1,59 @@
+import type { Meta, StoryObj } from "@storybook/react-vite";
+import { expect, fn, userEvent, within } from "storybook/test";
+import { InlinePromptInput } from "./RemoteDiffPanel";
+
+const meta: Meta<typeof InlinePromptInput> = {
+	title: "pages/AgentsPage/InlinePromptInput",
+	component: InlinePromptInput,
+	decorators: [
+		(Story) => (
+			<div className="w-[500px] rounded-lg bg-surface-primary p-4">
+				<Story />
+			</div>
+		),
+	],
+	args: {
+		onSubmit: fn(),
+		onCancel: fn(),
+	},
+};
+export default meta;
+type Story = StoryObj<typeof InlinePromptInput>;
+
+export const Default: Story = {};
+
+export const WithText: Story = {
+	play: async ({ canvasElement }) => {
+		const canvas = within(canvasElement);
+		const textarea = canvas.getByPlaceholderText("Add a comment...");
+		await userEvent.type(textarea, "Fix the race condition on line 42");
+	},
+};
+
+export const Submitting: Story = {
+	args: {
+		onSubmit: fn(),
+	},
+	play: async ({ canvasElement, args }) => {
+		const canvas = within(canvasElement);
+		const textarea = canvas.getByPlaceholderText("Add a comment...");
+		await userEvent.type(textarea, "Fix the race condition on line 42");
+		await userEvent.keyboard("{Enter}");
+		await expect(args.onSubmit).toHaveBeenCalledWith(
+			"Fix the race condition on line 42",
+		);
+	},
+};
+
+export const Cancelling: Story = {
+	args: {
+		onCancel: fn(),
+	},
+	play: async ({ canvasElement, args }) => {
+		const canvas = within(canvasElement);
+		const textarea = canvas.getByPlaceholderText("Add a comment...");
+		await userEvent.click(textarea);
+		await userEvent.keyboard("{Escape}");
+		await expect(args.onCancel).toHaveBeenCalled();
+	},
+};

site/src/pages/AgentsPage/RemoteDiffPanel.tsx

@@ -1,11 +1,15 @@
-import type { DiffLineAnnotation, FileDiffMetadata } from "@pierre/diffs";
+import type {
+	DiffLineAnnotation,
+	FileDiffMetadata,
+	SelectedLineRange,
+} from "@pierre/diffs";
 import { parsePatchFiles } from "@pierre/diffs";
 import { chatDiffContents } from "api/queries/chats";
 import type * as TypesGen from "api/typesGenerated";
 import { Button } from "components/Button/Button";
 import {
 	ArrowLeftIcon,
-	CornerDownLeftIcon,
+	ArrowUpIcon,
 	ExternalLinkIcon,
 	GitBranchIcon,
 	GitMergeIcon,
@@ -165,7 +169,7 @@ const PullRequestStateBadge: FC<{
  * line(s). Supports multiline via Shift+Enter. Enter submits,
  * Escape dismisses.
  */
-const InlinePromptInput: FC<{
+export const InlinePromptInput: FC<{
 	onSubmit: (text: string) => void;
 	onCancel: () => void;
 }> = ({ onSubmit, onCancel }) => {
@@ -183,12 +187,12 @@ const InlinePromptInput: FC<{
 
 	return (
 		<div className="px-2 py-1.5">
-			<div className="rounded-lg border border-border-default bg-surface-secondary p-1 shadow-sm has-[textarea:focus]:ring-2 has-[textarea:focus]:ring-content-link/40">
+			<div className="rounded-lg border border-border-default/80 bg-surface-secondary/45 p-1 shadow-sm has-[textarea:focus]:ring-2 has-[textarea:focus]:ring-content-link/40">
 				<textarea
 					ref={textareaRef}
-					className="w-full resize-none border-none bg-transparent px-2.5 py-1.5 font-sans text-[13px] leading-5 text-content-primary placeholder:text-content-secondary outline-none ring-0 focus:outline-none focus:ring-0"
-					placeholder="Add a comment to include with this reference..."
-					rows={1}
+					className="w-full resize-none border-none bg-transparent px-3 py-2 font-sans text-sm leading-5 text-content-primary placeholder:text-content-secondary outline-none ring-0 focus:outline-none focus:ring-0"
+					placeholder="Add a comment..."
+					rows={2}
 					value={text}
 					onChange={(e) => setText(e.target.value)}
 					onKeyDown={(e) => {
@@ -206,11 +210,12 @@ const InlinePromptInput: FC<{
 						}
 					}}
 				/>
-				<div className="flex items-center justify-end px-1.5 pb-1">
+				<div className="flex items-end justify-between gap-2 pl-2.5 pr-1.5 pb-1.5">
+					<span className="text-xs text-content-secondary">Esc to cancel</span>
 					<Button
-						size="sm"
-						variant="subtle"
-						className="h-6 gap-1.5 px-2 text-xs text-content-secondary hover:text-content-primary"
+						size="icon"
+						variant="default"
+						className="size-7 rounded-full transition-colors [&>svg]:!size-4 [&>svg]:p-0"
 						disabled={!text.trim()}
 						onMouseDown={(e: React.MouseEvent) => {
 							// Prevent blur from firing before click.
@@ -222,8 +227,8 @@ const InlinePromptInput: FC<{
 							}
 						}}
 					>
-						<CornerDownLeftIcon className="size-3" />
-						Add to chat
+						<ArrowUpIcon />
+						<span className="sr-only">Add to chat</span>
 					</Button>
 				</div>
 			</div>
@@ -331,7 +336,11 @@ export const RemoteDiffPanel: FC<RemoteDiffPanelProps> = ({
 				side?: "additions" | "deletions";
 			} | null,
 		) => {
-			if (!range || range.start === range.end) return;
+			if (!range) {
+				setActiveCommentBox(null);
+				return;
+			}
+			if (range.start === range.end) return;
 			const side = range.side ?? "additions";
 			setActiveCommentBox({
 				fileName,
@@ -352,7 +361,7 @@ export const RemoteDiffPanel: FC<RemoteDiffPanelProps> = ({
 				return [
 					{
 						side: activeCommentBox.side,
-						lineNumber: activeCommentBox.startLine,
+						lineNumber: activeCommentBox.endLine,
 						metadata: "active-input",
 					},
 				];
@@ -362,6 +371,20 @@ export const RemoteDiffPanel: FC<RemoteDiffPanelProps> = ({
 		[activeCommentBox],
 	);
 
+	const getSelectedLines = useCallback(
+		(fileName: string): SelectedLineRange | null => {
+			if (activeCommentBox && activeCommentBox.fileName === fileName) {
+				return {
+					start: activeCommentBox.startLine,
+					end: activeCommentBox.endLine,
+					side: activeCommentBox.side,
+				};
+			}
+			return null;
+		},
+		[activeCommentBox],
+	);
+
 	const handleCancelComment = useCallback(() => {
 		setActiveCommentBox(null);
 	}, []);
@@ -492,6 +515,7 @@ export const RemoteDiffPanel: FC<RemoteDiffPanelProps> = ({
 				onLineNumberClick={handleLineNumberClick}
 				onLineSelected={handleLineSelected}
 				getLineAnnotations={getLineAnnotations}
+				getSelectedLines={getSelectedLines}
 				renderAnnotation={renderAnnotation}
 				scrollToFile={scrollTarget}
 				onScrollToFileComplete={handleScrollComplete}