From 91f84d4e6e9c360e6c1c291fae134957c1e16e7e Mon Sep 17 00:00:00 2001
From: Marco Beretta <81851188+berry-13@users.noreply.github.com>
Date: Fri, 28 Aug 2026 21:10:59 +0200
Subject: [PATCH] fix(client): address sixth round of Codex review findings

Recognize the batched-edit separator pair for every edit in a diff preview.
formatEditPreview writes a `--- old_text N` / `+++ new_text N` pair before each
edit's `@@`, but the parser gated on `inHunk`, which is never reset, so only
the first pair was skipped and every later one was counted as a deletion plus
an addition and rendered as a marker row. Matched by name instead, which leaves
content lines that merely begin with `-` or `+` alone.

Compose the shared Button for the popover's type-below hint, suppressing only
the fill the way Summary's quiet text buttons already do, so the focus ring and
disabled handling come from the shared recipe.

Render an answer box that carries only snippetHighlighted. It is independently
optional from snippet, and the details card opens for any answer box, so such a
result previously showed a heading with an empty body.
---
 .../Chat/Input/AskUserQuestionPopover.tsx     | 12 ++++++---
 .../Chat/Messages/Content/Parts/DiffView.tsx  | 13 ++++++++++
 .../Content/Parts/__tests__/DiffView.test.ts  | 25 +++++++++++++++++++
 .../Chat/Messages/Content/WebSearch.tsx       | 13 +++++++---
 4 files changed, 56 insertions(+), 7 deletions(-)

diff --git a/client/src/components/Chat/Input/AskUserQuestionPopover.tsx b/client/src/components/Chat/Input/AskUserQuestionPopover.tsx
index de41557741..1c967d48d0 100644
--- a/client/src/components/Chat/Input/AskUserQuestionPopover.tsx
+++ b/client/src/components/Chat/Input/AskUserQuestionPopover.tsx
@@ -205,15 +205,19 @@ function AskUserQuestionPopoverPanel({
           </div>
         )}
         <div className="flex shrink-0 items-center justify-between gap-2 p-2">
-          <button
-            type="button"
-            className="cursor-text rounded-md text-xs text-text-secondary transition-colors hover:text-text-primary focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-border-heavy"
+          {/* Shared primitive with the fill suppressed, the same way Summary's
+              quiet text buttons compose it: this reads as a hint, not a
+              control with a surface, but the focus ring and disabled handling
+              should still come from the recipe. */}
+          <Button
+            variant="ghost"
+            className="h-auto cursor-text rounded-md p-0 text-xs font-normal text-text-secondary hover:bg-transparent hover:text-text-primary"
             onClick={() => textAreaRef?.current?.focus()}
           >
             {options.length === 0
               ? localize('com_ui_ask_type_below_only')
               : localize('com_ui_ask_type_below')}
-          </button>
+          </Button>
           <div className="flex items-center gap-2">
             <Button size="sm" variant="outline" disabled={locked} onClick={() => skip()}>
               {localize('com_ui_skip')}
diff --git a/client/src/components/Chat/Messages/Content/Parts/DiffView.tsx b/client/src/components/Chat/Messages/Content/Parts/DiffView.tsx
index ecff8b7e9c..28779bca36 100644
--- a/client/src/components/Chat/Messages/Content/Parts/DiffView.tsx
+++ b/client/src/components/Chat/Messages/Content/Parts/DiffView.tsx
@@ -2,6 +2,10 @@ import { cn } from '~/utils';
 
 const HUNK_HEADER = /^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@/;
 
+/** The separator pair `formatEditPreview` writes ahead of each batched edit,
+ *  numbered once a batch holds more than one. */
+const EDIT_SECTION_MARKER = /^(?:---|\+\+\+) (?:old_text|new_text)(?: \d+)?$/;
+
 export interface DiffLine {
   type: 'add' | 'del' | 'context' | 'hunk';
   text: string;
@@ -33,6 +37,15 @@ export function parseUnifiedDiff(diff: string): ParsedDiff {
   let inHunk = false;
 
   for (const raw of diff.replace(/\n$/, '').split('\n')) {
+    /** `formatEditPreview` emits a `--- old_text N` / `+++ new_text N` pair
+     *  before EACH batched edit's `@@`. Gating on `!inHunk` alone recognized
+     *  only the first pair, then counted every later one as a deletion plus an
+     *  addition — inflating the statistics and rendering marker rows. Matching
+     *  the synthetic pair by name catches every batch while leaving real
+     *  content lines that merely begin with `-`/`+` alone. */
+    if (EDIT_SECTION_MARKER.test(raw)) {
+      continue;
+    }
     if (!inHunk && (raw.startsWith('--- ') || raw.startsWith('+++ '))) {
       continue;
     }
diff --git a/client/src/components/Chat/Messages/Content/Parts/__tests__/DiffView.test.ts b/client/src/components/Chat/Messages/Content/Parts/__tests__/DiffView.test.ts
index 54dceac2cb..14776adee8 100644
--- a/client/src/components/Chat/Messages/Content/Parts/__tests__/DiffView.test.ts
+++ b/client/src/components/Chat/Messages/Content/Parts/__tests__/DiffView.test.ts
@@ -39,4 +39,29 @@ describe('parseUnifiedDiff', () => {
     expect(parsed.additions).toBe(1);
     expect(parsed.deletions).toBe(1);
   });
+
+  it('skips the separator pair before every batched edit', () => {
+    const parsed = parseUnifiedDiff(
+      [
+        '--- old_text 1',
+        '+++ new_text 1',
+        '@@',
+        '-first old',
+        '+first new',
+        '',
+        '--- old_text 2',
+        '+++ new_text 2',
+        '@@',
+        '-second old',
+        '+second new',
+      ].join('\n'),
+    );
+
+    /** The second edit's synthetic pair used to be counted as a real deletion
+     *  and addition, inflating both statistics and rendering marker rows. */
+    expect(parsed.deletions).toBe(2);
+    expect(parsed.additions).toBe(2);
+    expect(parsed.lines.some((line) => line.text.includes('old_text'))).toBe(false);
+    expect(parsed.lines.some((line) => line.text.includes('new_text'))).toBe(false);
+  });
 });
diff --git a/client/src/components/Chat/Messages/Content/WebSearch.tsx b/client/src/components/Chat/Messages/Content/WebSearch.tsx
index 0acfcb0fe7..a25ce7fb19 100644
--- a/client/src/components/Chat/Messages/Content/WebSearch.tsx
+++ b/client/src/components/Chat/Messages/Content/WebSearch.tsx
@@ -203,6 +203,13 @@ export default function WebSearch({
     }
     return undefined;
   }, [attachments]);
+  /** `snippet` and `snippetHighlighted` are independently optional, and
+   *  `hasDetails` opens the card for ANY answer box, so a provider result
+   *  carrying only highlights rendered a heading with an empty body. */
+  const answerText = useMemo(
+    () => answerBox?.snippet || answerBox?.snippetHighlighted?.join(' ') || undefined,
+    [answerBox],
+  );
 
   // Show favicons from the raw SERP results immediately rather than waiting for
   // each source to flip to `processed`; the agents scrape barrier would otherwise
@@ -346,16 +353,16 @@ export default function WebSearch({
                         { count: sourceCount },
                       )}
                     </div>
-                    {answerBox && (answerBox.title || answerBox.snippet) && (
+                    {answerBox && (answerBox.title || answerText) && (
                       <div className="border-t border-border-light pt-2">
                         {answerBox.title && (
                           <div className="text-sm font-medium text-text-primary">
                             {answerBox.title}
                           </div>
                         )}
-                        {answerBox.snippet && (
+                        {answerText && (
                           <div className="mt-1 text-xs leading-relaxed text-text-secondary">
-                            {answerBox.snippet}
+                            {answerText}
                           </div>
                         )}
                       </div>