🤖 fix: diff padding background colors match first/last line type (#1154)

ammar-agent · web-flow · commit cd7fb3ddb493 · 2025-12-14T14:29:09.000-06:00
When a diff starts with an addition or ends with a deletion, the
top/bottom padding of the diff container now matches that line's
background color (green for additions, red for deletions).

Previously, the container used a neutral background for all padding
regardless of the first/last line types, causing a visual inconsistency.

## Changes
- Added `firstLineType`/`lastLineType` props to `DiffContainer`
- Replaced `py-1.5` padding with separate colored padding strips
- Updated `DiffRenderer` and `SelectableDiffRenderer` to pass line types
- Added `DiffPaddingColors` story to capture the behavior

---
_Generated with `mux` • Model: `anthropic:claude-opus-4-5` • Thinking:
`high`_
diff --git a/docs/AGENTS.md b/docs/AGENTS.md
@@ -170,6 +170,7 @@ Avoid mock-heavy tests that verify implementation details rather than behavior.
 
 - Prefer fixes that simplify existing code; such simplifications often do not need new tests.
 - When adding complexity, add or extend tests. If coverage requires new infrastructure, propose the harness and then add the tests there.
+- When asked to reduce LoC, focus on simplifying production logic—not stripping comments, docs, or tests.
 
 ## Mode: Exec
 
diff --git a/src/browser/components/shared/DiffRenderer.tsx b/src/browser/components/shared/DiffRenderer.tsx
@@ -216,8 +216,29 @@ const DiffIndicator: React.FC<DiffIndicatorProps> = ({
  * Used by FileEditToolCall for wrapping custom diff content
  */
 export const DiffContainer: React.FC<
-  React.PropsWithChildren<{ fontSize?: string; maxHeight?: string; className?: string }>
-> = ({ children, fontSize, maxHeight, className }) => {
+  React.PropsWithChildren<{
+    fontSize?: string;
+    maxHeight?: string;
+    className?: string;
+    /** Type of the first line in the diff (for top padding background) */
+    firstLineType?: DiffLineType;
+    /** Type of the last line in the diff (for bottom padding background) */
+    lastLineType?: DiffLineType;
+    /** Line number column widths for accurate gutter sizing */
+    lineNumberWidths?: { oldWidthCh: number; newWidthCh: number };
+    /** Whether line numbers are shown (affects gutter width calculation) */
+    showLineNumbers?: boolean;
+  }>
+> = ({
+  children,
+  fontSize,
+  maxHeight,
+  className,
+  firstLineType,
+  lastLineType,
+  lineNumberWidths,
+  showLineNumbers = true,
+}) => {
   const resolvedMaxHeight = maxHeight ?? "400px";
   const [isExpanded, setIsExpanded] = React.useState(false);
   const contentRef = React.useRef<HTMLDivElement>(null);
@@ -255,13 +276,39 @@ export const DiffContainer: React.FC<
 
   const showOverflowControls = clampContent && isOverflowing;
 
+  // Calculate gutter width to match DiffLineGutter layout:
+  // px-1 (4px) + oldWidthCh + gap-0.5 (2px) + ml-3 (12px) + newWidthCh + px-1 (4px)
+  // When line numbers hidden, gutter is just px-1 (8px total)
+  const gutterWidth =
+    showLineNumbers && lineNumberWidths
+      ? `calc(8px + ${lineNumberWidths.oldWidthCh}ch + 14px + ${lineNumberWidths.newWidthCh}ch)`
+      : "8px";
+
+  // Padding strip mirrors gutter/content background split of diff lines
+  const PaddingStrip = ({ lineType }: { lineType?: DiffLineType }) => (
+    <div className="flex h-1.5">
+      <div
+        className="shrink-0"
+        style={{
+          width: gutterWidth,
+          background: lineType ? getDiffLineGutterBackground(lineType) : undefined,
+        }}
+      />
+      <div
+        className="flex-1"
+        style={{ background: lineType ? getDiffLineBackground(lineType) : undefined }}
+      />
+    </div>
+  );
+
   return (
     <div
       className={cn(
-        "relative m-0 rounded-sm border border-border-light bg-code-bg py-1.5 [&_*]:text-[inherit]",
+        "relative m-0 rounded-sm border border-border-light bg-code-bg [&_*]:text-[inherit]",
         className
       )}
     >
+      <PaddingStrip lineType={firstLineType} />
       <div
         ref={contentRef}
         className={cn(
@@ -278,6 +325,7 @@ export const DiffContainer: React.FC<
       >
         {children}
       </div>
+      <PaddingStrip lineType={lastLineType} />
 
       {showOverflowControls && (
         <>
@@ -485,8 +533,20 @@ export const DiffRenderer: React.FC<DiffRendererProps> = ({
     );
   }
 
+  // Get first and last line types for padding background colors
+  const firstLineType = highlightedChunks[0]?.type;
+  const lastLineType = highlightedChunks[highlightedChunks.length - 1]?.type;
+
   return (
-    <DiffContainer fontSize={fontSize} maxHeight={maxHeight} className={className}>
+    <DiffContainer
+      fontSize={fontSize}
+      maxHeight={maxHeight}
+      className={className}
+      firstLineType={firstLineType}
+      lastLineType={lastLineType}
+      lineNumberWidths={lineNumberWidths}
+      showLineNumbers={showLineNumbers}
+    >
       {highlightedChunks.flatMap((chunk) =>
         chunk.lines.map((line) => {
           return (
@@ -906,8 +966,20 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
       );
     }
 
+    // Get first and last line types for padding background colors
+    const firstLineType = highlightedLineData[0]?.type;
+    const lastLineType = highlightedLineData[highlightedLineData.length - 1]?.type;
+
     return (
-      <DiffContainer fontSize={fontSize} maxHeight={maxHeight} className={className}>
+      <DiffContainer
+        fontSize={fontSize}
+        maxHeight={maxHeight}
+        className={className}
+        firstLineType={firstLineType}
+        lastLineType={lastLineType}
+        lineNumberWidths={lineNumberWidths}
+        showLineNumbers={showLineNumbers}
+      >
         {highlightedLineData.map((lineInfo, displayIndex) => {
           const isSelected = isLineSelected(displayIndex);
 
diff --git a/src/browser/stories/App.chat.stories.tsx b/src/browser/stories/App.chat.stories.tsx
@@ -755,3 +755,96 @@ export const EditingMessage: AppStory = {
     },
   },
 };
+
+/**
+ * Diff padding colors - verifies that the top/bottom padding of diff blocks
+ * matches the first/last line type (addition=green, deletion=red, context=default).
+ *
+ * This story shows three diffs:
+ * 1. Diff starting with addition (green top padding)
+ * 2. Diff ending with deletion (red bottom padding)
+ * 3. Diff with context lines at both ends (default padding)
+ */
+export const DiffPaddingColors: AppStory = {
+  render: () => (
+    <AppWithMocks
+      setup={() =>
+        setupSimpleChatStory({
+          workspaceId: "ws-diff-padding",
+          messages: [
+            createUserMessage("msg-1", "Show me different diff edge cases", {
+              historySequence: 1,
+              timestamp: STABLE_TIMESTAMP - 100000,
+            }),
+            createAssistantMessage(
+              "msg-2",
+              "Here are diffs with different first/last line types:",
+              {
+                historySequence: 2,
+                timestamp: STABLE_TIMESTAMP - 90000,
+                toolCalls: [
+                  // Diff starting with addition - top padding should be green
+                  createFileEditTool(
+                    "call-1",
+                    "src/addition-first.ts",
+                    [
+                      "--- src/addition-first.ts",
+                      "+++ src/addition-first.ts",
+                      "@@ -1,3 +1,5 @@",
+                      "+import { newModule } from './new';",
+                      "+import { anotherNew } from './another';",
+                      " export function existing() {",
+                      "   return 'unchanged';",
+                      " }",
+                    ].join("\n")
+                  ),
+                  // Diff ending with deletion - bottom padding should be red
+                  createFileEditTool(
+                    "call-2",
+                    "src/deletion-last.ts",
+                    [
+                      "--- src/deletion-last.ts",
+                      "+++ src/deletion-last.ts",
+                      "@@ -1,6 +1,3 @@",
+                      " export function keep() {",
+                      "   return 'still here';",
+                      " }",
+                      "-export function remove() {",
+                      "-  return 'goodbye';",
+                      "-}",
+                    ].join("\n")
+                  ),
+                  // Diff with context at both ends - default padding
+                  createFileEditTool(
+                    "call-3",
+                    "src/context-both.ts",
+                    [
+                      "--- src/context-both.ts",
+                      "+++ src/context-both.ts",
+                      "@@ -1,4 +1,4 @@",
+                      " function before() {",
+                      "+  console.log('added');",
+                      "-  console.log('removed');",
+                      " }",
+                    ].join("\n")
+                  ),
+                ],
+              }
+            ),
+          ],
+        })
+      }
+    />
+  ),
+  parameters: {
+    docs: {
+      description: {
+        story:
+          "Verifies diff container padding colors match first/last line types. " +
+          "The first diff should have green top padding (starts with +), " +
+          "the second should have red bottom padding (ends with -), " +
+          "and the third should have default padding (context at both ends).",
+      },
+    },
+  },
+};
