fix(app): render linked pictures in PR markdown (#5968)

* fix(app): render linked pictures in PR markdown

Consume image-only headings, anchors, and picture wrappers together so splitting at the fallback image does not leave raw opening tags or discard its link.

* fix(app): preserve picture block boundaries in markdown

* fix(app): parse nested image wrappers without recursion

* test(app): account for delayed wheel input in scroll assertions

CI recorded a 480px wheel scroll delivered after its event alongside 41px of image growth below the new reader. Use the same input budget to locate the reader and account for intervening growth, while continuing to reject added viewport displacement.

* test(app): locate the scroll reader from frame movement

packages/app/e2e/browser/agent-timeline-scroll-smoothness.spec.ts

@@ -34,6 +34,7 @@ test("scroll detector distinguishes entered image growth from a simultaneous vie
   const after: ScrollFrame = {
     ...before,
     at: 1400,
+    scrollTop: 900,
     wheelTotal: 100,
     lastWheelAt: 1050,
     rows: [
@@ -48,6 +49,7 @@ test("scroll detector distinguishes entered image growth from a simultaneous vie
       {
         ...after,
         wheelTotal: 1000,
+        scrollTop: 0,
         rows: after.rows.map((row) => ({ ...row, top: row.top + 100 })),
       },
     ]),
@@ -66,6 +68,70 @@ test("scroll detector distinguishes entered image growth from a simultaneous vie
   ).toHaveLength(1);
 });
 
+test("scroll detector accounts for delayed wheel input and growth below the new reader", () => {
+  const before: ScrollFrame = {
+    at: 1000,
+    scrollTop: 2000,
+    scrollHeight: 5000,
+    viewportHeight: 800,
+    virtualized: true,
+    loading: false,
+    rows: [
+      { id: "new-reader", top: -520, height: 278 },
+      { id: "image", top: -242, height: 184 },
+      { id: "old-reader", top: -58, height: 130 },
+    ],
+    anchor: "old-reader",
+    wheelTotal: 0,
+    lastWheelAt: 0,
+    inputFinishedAt: null,
+    imageLoads: 0,
+    mounted: 0,
+    unmounted: 0,
+  };
+  const wheel = { ...before, at: 1100, wheelTotal: 480, lastWheelAt: 1099 };
+  const after: ScrollFrame = {
+    ...wheel,
+    at: 1220,
+    scrollTop: 1520,
+    scrollHeight: 5041,
+    anchor: "new-reader",
+    rows: [
+      { id: "new-reader", top: -40, height: 278 },
+      { id: "image", top: 238, height: 225 },
+      { id: "old-reader", top: 463, height: 130 },
+    ],
+  };
+  expect(findScrollJumps([before, wheel, after])).toEqual([]);
+  // The previous 480 px input has already moved the viewport; the recent
+  // budget includes it, but only the next 480 px belongs to this frame.
+  const steadyBefore = { ...before, at: 1050, wheelTotal: 480, lastWheelAt: 1049 };
+  const steadyAfter = {
+    ...after,
+    at: 1100,
+    wheelTotal: 960,
+    lastWheelAt: 1099,
+    scrollHeight: 6000,
+    rows: [
+      { id: "new-reader", top: -40, height: 278 },
+      { id: "image", top: 238, height: 1184 },
+      { id: "old-reader", top: 1422, height: 130 },
+    ],
+  };
+  expect(findScrollJumps([steadyBefore, steadyAfter])).toEqual([]);
+
+  expect(
+    findScrollJumps([
+      before,
+      wheel,
+      {
+        ...after,
+        rows: after.rows.map((row) => ({ id: row.id, height: row.height, top: row.top + 100 })),
+      },
+    ]),
+  ).toHaveLength(1);
+});
+
 for (const cadence of scrollCadences) {
   test(`varied timeline preserves reading position during ${cadence.name} upward scrolling`, async ({
     page,

packages/app/e2e/support/helpers/timeline-scroll-smoothness.ts

@@ -278,12 +278,13 @@ export function findScrollJumps(frames: ScrollFrame[]) {
     if (!previous?.anchor) return [];
     // Wheel input can move the reading line onto an image before it expands.
     // Follow that intended row, not text now below the image.
-    const inputDelta = current.wheelTotal - previous.wheelTotal;
-    const availableScroll = Math.min(inputDelta, previous.scrollTop);
+    const recent = frames.findLast((frame) => frame.at <= previous.at - 100);
+    const wheelBudget = current.wheelTotal - (recent?.wheelTotal ?? 0);
+    const frameScroll = Math.max(0, previous.scrollTop - current.scrollTop);
+    const availableScroll = Math.min(frameScroll, wheelBudget, previous.scrollTop);
     const readingLine = 8 - availableScroll;
     const currentRows = new Map(current.rows.map((row) => [row.id, row]));
     const intendedRow = previous.rows.find((row) => row.top + row.height > readingLine);
-    const intendedHeight = intendedRow && currentRows.get(intendedRow.id)?.height;
     const anchor = previous.anchor;
     const before = currentRows.has(anchor)
       ? previous.rows.find((row) => row.id === anchor)!
@@ -298,18 +299,21 @@ export function findScrollJumps(frames: ScrollFrame[]) {
     // A busy main thread can deliver wheel movement hundreds of milliseconds
     // after its event. Only assert idle stability after the driver stops input.
     const idle = current.inputFinishedAt !== null && previous.at - current.inputFinishedAt > 250;
-    const recent = frames.findLast((frame) => frame.at <= previous.at - 100);
-    const wheelBudget = current.wheelTotal - (recent?.wheelTotal ?? 0);
-    // Growth displaces only later rows. It never grants extra movement to the
-    // image itself, and its top must still follow the input budget.
-    const enteredRowGrowth =
-      inputDelta > 0 &&
+    // Wheel events can precede their scroll update, or include input already
+    // applied in earlier frames. Locate the reader with this frame's scroll,
+    // bounded by recent input. Growth below it can move the old anchor without
+    // moving the reading line.
+    const readerFollowsInput =
       intendedRow &&
-      intendedHeight !== undefined &&
-      before.top >= intendedRow.top + intendedRow.height &&
-      Math.abs(currentRows.get(intendedRow.id)!.top - intendedRow.top - availableScroll) <= 32
-        ? Math.max(0, intendedHeight - intendedRow.height)
-        : 0;
+      currentRows.has(intendedRow.id) &&
+      Math.abs(currentRows.get(intendedRow.id)!.top - intendedRow.top - availableScroll) <= 32;
+    const enteredRowGrowth = readerFollowsInput
+      ? previous.rows.reduce((growth, row) => {
+          if (row.top < intendedRow.top || row.top >= before.top) return growth;
+          const height = currentRows.get(row.id)?.height ?? row.height;
+          return growth + Math.max(0, height - row.height);
+        }, 0)
+      : 0;
     const excessForward = movement > wheelBudget + enteredRowGrowth + 32;
     // Upward wheel input moves the same text DOWN the viewport. A negative move
     // is a reversal, independent of legitimate scrollTop compensation on prepend.

packages/app/src/components/markdown/html-ish.test.ts

@@ -113,6 +113,85 @@ describe("splitHtmlishMarkdown", () => {
     ]);
   });
 
+  it("renders a linked picture inside a heading without exposing its HTML", () => {
+    const source = [
+      '<h2><a href="https://example.com/retrigger?id=73851569"><picture>',
+      '<source media="(prefers-color-scheme: dark)" srcset="https://example.com/dark.svg">',
+      '<source media="(prefers-color-scheme: light)" srcset="https://example.com/light.svg">',
+      '<img alt="Retrigger" src="https://example.com/default.svg" width="85" height="24">',
+      "</picture></a></h2>",
+      "",
+      "Confidence Score: 5/5",
+      "",
+      "<details><summary>Summary</summary>No outstanding findings.</details>",
+    ].join("\n");
+
+    expect(splitHtmlishMarkdown(source)).toEqual([
+      {
+        kind: "inlineImage",
+        alt: "Retrigger",
+        src: "https://example.com/default.svg",
+        href: "https://example.com/retrigger?id=73851569",
+        width: 85,
+        height: 24,
+      },
+      { kind: "markdown", text: "\n\nConfidence Score: 5/5\n\n" },
+      { kind: "details", summary: "Summary", body: "No outstanding findings." },
+    ]);
+  });
+
+  it.each([
+    ["", ""],
+    ["<h3>", "</h3>"],
+    ['<a href="javascript:alert(1)">', "</a>"],
+  ])("uses the picture fallback without activating unsafe links (%s)", (wrapper, closing) => {
+    const picture =
+      '<picture><source srcset="https://example.com/alternate.svg"/><img src="https://example.com/default.svg" alt="Image"></picture>';
+
+    expect(splitHtmlishMarkdown(`${wrapper}${picture}${closing}`)).toEqual([
+      { kind: "inlineImage", src: "https://example.com/default.svg", alt: "Image" },
+    ]);
+  });
+
+  it("keeps picture examples literal inside inline and fenced code", () => {
+    const picture =
+      '<picture><source srcset="https://example.com/alternate.svg"><img src="https://example.com/default.svg"></picture>';
+    const source = `Use \`${picture}\`\n\n\`\`\`html\n${picture}\n\`\`\``;
+    expect(splitHtmlishMarkdown(source)).toEqual([{ kind: "markdown", text: source }]);
+  });
+
+  it.each(["p", "div"])("renders a linked picture inside %s", (wrapper) => {
+    const source = `<${wrapper}><a href="https://example.com/page"><picture><source srcset="https://example.com/dark.svg"><img src="https://example.com/default.svg" alt="Image"></picture></a></${wrapper}>`;
+    expect(splitHtmlishMarkdown(source)).toEqual([
+      {
+        kind: "inlineImage",
+        src: "https://example.com/default.svg",
+        alt: "Image",
+        href: "https://example.com/page",
+      },
+    ]);
+  });
+
+  it.each(["h2", "p", "div"])("keeps text after an image-only %s on a separate row", (wrapper) => {
+    const source = `<${wrapper}><img src="https://example.com/image.svg" alt="Image"></${wrapper}>Next section`;
+    expect(splitHtmlishMarkdown(source)).toEqual([
+      { kind: "inlineImage", src: "https://example.com/image.svg", alt: "Image" },
+      { kind: "markdown", text: "Next section" },
+    ]);
+  });
+
+  it("handles deeply nested image wrappers without exhausting the stack", () => {
+    const source = `${"<div>".repeat(10000)}<a href="https://example.com/page"><picture><img src="https://example.com/image.svg" alt="Image"></picture></a>${"</div>".repeat(10000)}`;
+    expect(splitHtmlishMarkdown(source)).toEqual([
+      {
+        kind: "inlineImage",
+        src: "https://example.com/image.svg",
+        alt: "Image",
+        href: "https://example.com/page",
+      },
+    ]);
+  });
+
   it("preserves inline image parts inside details bodies", () => {
     expect(
       splitHtmlishMarkdown(

packages/app/src/components/markdown/html-ish.ts

@@ -29,7 +29,7 @@ const FENCE_LINE_RE = /^ {0,3}([`~]{3,})[^\n\r]*(?:\r?\n|$)/gm;
 const BACKTICK_RUN_RE = /`+/g;
 const SAFE_IMAGE_SRC_RE = /^(https?:\/\/|data:image\/(?:png|gif|jpe?g);base64,)/i;
 const SAFE_LINK_HREF_RE = /^(https?:\/\/|#(?:$|[\w-]))/i;
-const VOID_HTML_TAGS = new Set(["br", "img"]);
+const VOID_HTML_TAGS = new Set(["br", "img", "source"]);
 const MARKDOWN_TAG_WRAPPERS: Readonly<Record<string, readonly [string, string]>> = {
   b: ["**", "**"],
   del: ["~~", "~~"],
@@ -140,25 +140,73 @@ function parseInlineImageAt(tokens: HtmlToken[], start: number): InlineImagePars
     return image ? { part: image, end: start + 1 } : null;
   }
 
-  if (token.name !== "a") {
+  if (token.name !== "a" && token.name !== "picture" && !isImageBlockWrapper(token)) {
     return null;
   }
 
-  const closeIndex = findMatchingClose(tokens, start, "a");
-  if (closeIndex === null) {
-    return null;
-  }
+  return parseWrappedImageAt(tokens, start);
+}
 
-  const image = getSingleImageChild(tokens.slice(start + 1, closeIndex));
-  if (!image) {
-    return null;
-  }
+function parseWrappedImageAt(tokens: HtmlToken[], start: number): InlineImageParseResult | null {
+  // Walk an image-only wrapper chain once. Recursing over copied child slices
+  // makes deeply nested review HTML quadratic and can exhaust the JS stack.
+  const wrappers: string[] = [];
+  let image: MarkdownInlineImagePart | null = null;
+  let href: string | undefined;
+  let hasLink = false;
 
-  const inlineImage = imageTokenToInlineImage(image, safeHref(token.attributes.href));
-  return inlineImage ? { part: inlineImage, end: closeIndex + 1 } : null;
+  for (let index = start; index < tokens.length; index += 1) {
+    const child = tokens[index];
+    if (child.kind === "comment" || isWhitespaceText(child)) {
+      continue;
+    }
+    if (child.kind !== "tag") {
+      return null;
+    }
+    if (child.closing) {
+      if (!image || wrappers.pop() !== child.name) {
+        return null;
+      }
+      if (wrappers.length === 0) {
+        return { part: image, end: index + 1 };
+      }
+      continue;
+    }
+    // Pictures use their img fallback, not responsive source selection.
+    if (child.name === "source" && wrappers.at(-1) === "picture") {
+      continue;
+    }
+    if (image) {
+      return null;
+    }
+    if (child.name === "img") {
+      image = imageTokenToInlineImage(child, href);
+      if (!image) {
+        return null;
+      }
+      continue;
+    }
+    if (child.name !== "a" && child.name !== "picture" && !isImageBlockWrapper(child)) {
+      return null;
+    }
+    if (child.name === "a" && !hasLink) {
+      href = safeHref(child.attributes.href);
+      hasLink = true;
+    }
+    wrappers.push(child.name);
+  }
+  return null;
+}
+
+function isImageBlockWrapper(token: HtmlToken | undefined): boolean {
+  return isHeadingTag(token) || isOpenTag(token, "p") || isOpenTag(token, "div");
 }
 
 function flowsWithFollowingText(tokens: HtmlToken[], start: number, end: number): boolean {
+  if (isImageBlockWrapper(tokens[start])) {
+    return false;
+  }
+
   const previous = tokens[start - 1];
   // At line start if nothing precedes this image, or the preceding token ends with only
   // whitespace since the last newline (covers a bare space between two images on the same line).
@@ -191,6 +239,9 @@ function flowsWithFollowingText(tokens: HtmlToken[], start: number, end: number)
     }
     // An inline image tag — skip over it (the image itself and its possible wrapping close tag).
     if (token.kind === "tag") {
+      if (isImageBlockWrapper(token)) {
+        return false;
+      }
       const imageResult = parseInlineImageAt(tokens, cursor);
       if (imageResult) {
         cursor = imageResult.end;