render-helper: fast thumbnail lookup, thumbnails first, memoized nulls

apps/web/src/core/entries/slim-entry.ts

@@ -66,19 +66,26 @@ function pickThumbnail(entry: Entry): string | undefined {
   // entry, while the body is still here to look at.
   //
   // Two steps, because they find different things. getEntryImageRawUrl is the
-  // regex fast path over raw markdown. catchPostImage falls back to a full
-  // markdown2Html plus DOM parse, and THAT is where a video post's poster
-  // (3Speak/YouTube render as <img class="no-replace video-thumbnail">) and
-  // <center>-wrapped bare image URLs are discovered. Stopping at the fast path
-  // dropped those cards to /assets/noimage.png: measured on live posts, 4 of 29
-  // rows that carry no metadata image, concentrated in the video communities.
+  // regex fast path over raw markdown. catchPostImage in fast mode adds the
+  // cases the regex alone missed, a YouTube poster and a <center>-wrapped bare
+  // URL, still without rendering markdown. Measured on live posts, those two
+  // were 4 of 29 rows that carry no metadata image, concentrated in the video
+  // communities, and stopping at the regex dropped them to /assets/noimage.png.
   //
-  // The second call only runs when the fast path found nothing, and it is work
-  // the card itself already did before this step existed. catchPostImage(0, 0)
-  // returns the proxied /p/ URL, and re-proxying it at card size reuses the same
-  // hash rather than nesting, so the card src stays byte-identical to what it was
-  // before slimming.
-  return getEntryImageRawUrl(entry) ?? catchPostImage(entry, 0, 0, "match") ?? undefined;
+  // Fast mode matters here because this runs on the server for every row of
+  // every feed. The full lookup ends in markdown2Html plus a DOM parse, and on
+  // a long body with no image at all that is hundreds of milliseconds of
+  // synchronous CPU per row: one feed of such rows held a server's event loop
+  // for five seconds, stalling every other request on that process. The one
+  // class fast mode gives up is an ambiguous markdown image URL (one containing
+  // a parenthesis), which the card then shows without a thumbnail.
+  //
+  // catchPostImage(0, 0) returns the proxied /p/ URL, and re-proxying it at card
+  // size reuses the same hash rather than nesting, so the card src stays
+  // byte-identical to what it was before slimming.
+  return (
+    getEntryImageRawUrl(entry) ?? catchPostImage(entry, 0, 0, "match", { fast: true }) ?? undefined
+  );
 }
 
 function pickDescription(entry: Entry): string {

apps/web/src/specs/core/entries/slim-entry.spec.ts

@@ -68,23 +68,15 @@ describe("slimEntry", () => {
     expect(slimEntry(e).json_metadata?.image).toEqual(["https://images.hive.blog/in-body.png"]);
   });
 
-  it("shows the cover unslimmed and the poster slimmed, which is deliberate", () => {
-    // catchPostImage never looks at `thumbnails`: getImage() in render-helper
-    // reads json_metadata.image as a string, then as an array, then falls back
-    // to the body. Slimming puts the thumbnail first, so a post that sets the
-    // two fields to DIFFERENT urls renders its cover on an unslimmed card and
-    // its poster on a slim one.
+  it("shows the poster on both the unslimmed and the slimmed card", () => {
+    // `thumbnails` is published for exactly this purpose by 3Speak, Liketu and
+    // the editor's thumbnail picker, and a publisher who sets a dedicated poster
+    // means it. render-helper's catchPostImage reads it ahead of `image` now, the
+    // same order slimming uses, so a post that sets the two fields to DIFFERENT
+    // urls renders the poster whether or not the row was slimmed.
     //
-    // That divergence is chosen. `thumbnails` is published for exactly this
-    // purpose by 3Speak and Liketu, and a publisher who sets a dedicated poster
-    // means it. It is also unobservable in practice: across 461 live rows from
-    // trending, hot, created, promoted, tags and communities, 70 carried both
-    // fields and 0 of them disagreed.
-    //
-    // BOTH halves are asserted on purpose. Pinning only the slim side would let
-    // the divergence disappear unnoticed if render-helper ever started honouring
-    // `thumbnails`, and this test exists to make that a decision rather than a
-    // surprise.
+    // BOTH halves are asserted on purpose: if either side ever changed its order
+    // the two cards would silently disagree again.
     const meta = {
       thumbnails: ["https://images.hive.blog/poster.png"],
       image: ["https://images.hive.blog/cover.png"]
@@ -96,12 +88,9 @@ describe("slimEntry", () => {
     const card = (e: Entry) => catchPostImage(e, 320, 180, "match");
 
     expect(card(unslimmed)).toBe(
-      proxifyImageSrc("https://images.hive.blog/cover.png", 320, 180, "match")
-    );
-    expect(card(slimmed)).toBe(
       proxifyImageSrc("https://images.hive.blog/poster.png", 320, 180, "match")
     );
-    expect(card(unslimmed)).not.toBe(card(slimmed));
+    expect(card(slimmed)).toBe(card(unslimmed));
   });
 
   it("survives thumbnails that are not an array", () => {
@@ -134,18 +123,19 @@ describe("slimEntry", () => {
     expect(slimEntry(e).json_metadata?.image).toEqual(["https://images.hive.blog/real.png"]);
   });
 
-  it("keeps a video post's poster, which only the full render finds", () => {
+  it("keeps a video post's poster, which the raw-markdown regex alone misses", () => {
     // A bare YouTube URL becomes an <img class="no-replace video-thumbnail"> in
-    // the rendered post, so the raw-markdown fast path sees no image at all.
-    // Stopping there dropped these cards to the noimage placeholder.
+    // the rendered post, so getEntryImageRawUrl sees no image at all. Stopping
+    // there dropped these cards to the noimage placeholder; catchPostImage's
+    // fast mode derives the same poster without rendering.
     const e = entry({
       json_metadata: {},
       body: "Check this out\n\nhttps://www.youtube.com/watch?v=dQw4w9WgXcQ\n\nthanks"
     });
     expect(slimEntry(e).json_metadata?.image?.[0]).toBeTruthy();
   });
 
-  it("keeps a <center>-wrapped bare image URL, also full-render only", () => {
+  it("keeps a <center>-wrapped bare image URL, which the regex also missed before", () => {
     const e = entry({
       json_metadata: {},
       body: "<center>https://images.hive.blog/DQmb59qYM1czWSDDw2dRmUHJ7s97L6S6Rk3uZLyA5vCxAEr/pic.jpg</center>"

packages/render-helper/src/catch-post-image-fast.spec.ts

@@ -0,0 +1,212 @@
+import { catchPostImage, getEntryImageRawUrl } from './catch-post-image'
+import { markdown2Html } from './markdown-2-html'
+import { buildPictureSources, proxifyImageSrc } from './proxify-image-src'
+
+// Distinct author/permlink per fixture: catchPostImage memoizes per post and
+// size, process-wide, so two fixtures sharing a key would share an answer.
+let n = 0
+const entry = (body: string, json_metadata: unknown = {}) => ({
+  author: 'fast',
+  permlink: `p-${n++}`,
+  last_update: '2019-05-10T09:15:21',
+  body,
+  json_metadata
+}) as any
+
+const FAST = { fast: true }
+
+describe('catchPostImage thumbnails tier', () => {
+  it('prefers json_metadata.thumbnails[0] over image[0]', () => {
+    const e = entry('text', {
+      thumbnails: ['https://images.hive.blog/poster.png'],
+      image: ['https://images.hive.blog/cover.png']
+    })
+    expect(catchPostImage(e, 320, 180, 'match')).toBe(
+      proxifyImageSrc('https://images.hive.blog/poster.png', 320, 180, 'match')
+    )
+  })
+
+  it('accepts thumbnails published as a bare string', () => {
+    const e = entry('text', { thumbnails: 'https://images.hive.blog/single.png' })
+    expect(catchPostImage(e, 320, 180, 'match')).toBe(
+      proxifyImageSrc('https://images.hive.blog/single.png', 320, 180, 'match')
+    )
+  })
+
+  it('skips non-string members and falls through to image when thumbnails holds nothing usable', () => {
+    const junk = entry('text', { thumbnails: [null, 42, ''], image: ['https://images.hive.blog/cover.png'] })
+    expect(catchPostImage(junk, 320, 180, 'match')).toBe(
+      proxifyImageSrc('https://images.hive.blog/cover.png', 320, 180, 'match')
+    )
+    const shape = entry('text', { thumbnails: { 0: 'https://images.hive.blog/object.png' }, image: ['https://images.hive.blog/cover.png'] })
+    expect(catchPostImage(shape, 320, 180, 'match')).toBe(
+      proxifyImageSrc('https://images.hive.blog/cover.png', 320, 180, 'match')
+    )
+  })
+
+  it('proxies a gif thumbnail unsized, like a gif cover', () => {
+    const e = entry('text', { thumbnails: ['https://images.hive.blog/anim.gif'] })
+    expect(catchPostImage(e, 320, 180, 'match')).toBe(
+      proxifyImageSrc('https://images.hive.blog/anim.gif', 0, 0, 'match')
+    )
+  })
+
+  it('still reads image when there is no thumbnails field at all', () => {
+    const e = entry('text', { image: ['https://images.hive.blog/cover.png'] })
+    expect(catchPostImage(e, 320, 180, 'match')).toBe(
+      proxifyImageSrc('https://images.hive.blog/cover.png', 320, 180, 'match')
+    )
+  })
+})
+
+describe('catchPostImage fast mode', () => {
+  // Two fixtures with identical content, one per mode, so the memo cannot hand
+  // the second call the first one's answer.
+  const both = (body: string, meta: unknown = {}) => ({
+    full: catchPostImage(entry(body, meta), 600, 500, 'match'),
+    fast: catchPostImage(entry(body, meta), 600, 500, 'match', FAST)
+  })
+
+  it('agrees with the full lookup on a metadata image', () => {
+    const r = both('text', { image: ['https://images.hive.blog/cover.png'] })
+    expect(r.fast).toBe(r.full)
+    expect(r.fast).toBeTruthy()
+  })
+
+  it('agrees with the full lookup on a markdown image', () => {
+    const r = both('intro\n\n![pic](https://images.hive.blog/in-body.png)\n\nrest')
+    expect(r.fast).toBe(r.full)
+    expect(r.fast).toBe(proxifyImageSrc('https://images.hive.blog/in-body.png', 600, 500, 'match'))
+  })
+
+  it('agrees with the full lookup on an HTML img', () => {
+    const r = both('<p>hi</p><img src="https://images.hive.blog/tag.png" alt="">')
+    expect(r.fast).toBe(r.full)
+    expect(r.fast).toBeTruthy()
+  })
+
+  it('finds a <center>-wrapped bare image URL without rendering', () => {
+    const r = both('<center>https://images.hive.blog/DQmb59qYM1czWSDDw2dRmUHJ7s97L6S6Rk3uZLyA5vCxAEr/pic.jpg</center>')
+    expect(r.fast).toBe(r.full)
+    expect(r.fast).toBeTruthy()
+  })
+
+  it('derives the same YouTube poster the full render produces for a bare URL', () => {
+    const r = both('Check this out\n\nhttps://www.youtube.com/watch?v=dQw4w9WgXcQ\n\nthanks')
+    expect(r.full).toBeTruthy()
+    expect(r.fast).toBe(r.full)
+  })
+
+  it('derives the poster for youtu.be, shorts and a [url](url) link too', () => {
+    for (const body of [
+      'see https://youtu.be/dQw4w9WgXcQ now',
+      'see https://www.youtube.com/shorts/dQw4w9WgXcQ now',
+      'see [https://www.youtube.com/watch?v=dQw4w9WgXcQ](https://www.youtube.com/watch?v=dQw4w9WgXcQ) now'
+    ]) {
+      const r = both(body)
+      expect(r.full, body).toBeTruthy()
+      expect(r.fast, body).toBe(r.full)
+    }
+  })
+
+  it('keeps the full lookup precedence: a markdown image wins over an earlier video', () => {
+    // The full lookup returns the regex-found image before it would ever render
+    // the markdown and see the poster. Fast mode mirrors that, not source order.
+    const r = both('https://www.youtube.com/watch?v=dQw4w9WgXcQ\n\n![pic](https://images.hive.blog/later.png)')
+    expect(r.fast).toBe(r.full)
+    expect(r.fast).toBe(proxifyImageSrc('https://images.hive.blog/later.png', 600, 500, 'match'))
+  })
+
+  it('keeps the full lookup precedence: a markdown image before the video', () => {
+    const r = both('![pic](https://images.hive.blog/first.png)\n\nhttps://www.youtube.com/watch?v=dQw4w9WgXcQ')
+    expect(r.fast).toBe(r.full)
+    expect(r.fast).toBe(proxifyImageSrc('https://images.hive.blog/first.png', 600, 500, 'match'))
+  })
+
+  it('orders a bare image URL and a video poster by source position, as the render does', () => {
+    const posterFirst = both('https://www.youtube.com/watch?v=dQw4w9WgXcQ\n\nhttps://files.peakd.com/x/bare.png')
+    expect(posterFirst.full).toBeTruthy()
+    expect(posterFirst.fast).toBe(posterFirst.full)
+    expect(posterFirst.fast).not.toBe(proxifyImageSrc('https://files.peakd.com/x/bare.png', 600, 500, 'match'))
+
+    const bareFirst = both('https://files.peakd.com/x/bare.png\n\nhttps://www.youtube.com/watch?v=dQw4w9WgXcQ')
+    expect(bareFirst.fast).toBe(bareFirst.full)
+    expect(bareFirst.fast).toBe(proxifyImageSrc('https://files.peakd.com/x/bare.png', 600, 500, 'match'))
+  })
+
+  it('does not read a YouTube link whose label differs from its href as a poster', () => {
+    const r = both('watch [this](https://www.youtube.com/watch?v=dQw4w9WgXcQ) later')
+    expect(r.fast).toBe(r.full)
+    expect(r.fast).toBeNull()
+  })
+
+  it('ignores a YouTube URL inside a code block', () => {
+    const r = both('```\nhttps://www.youtube.com/watch?v=dQw4w9WgXcQ\n```')
+    expect(r.fast).toBe(r.full)
+    expect(r.fast).toBeNull()
+  })
+
+  it('returns null for a body with no image at all, same as the full lookup', () => {
+    const r = both('<p>lorem ipsum dolor</p> sit amet')
+    expect(r.fast).toBeNull()
+    expect(r.full).toBeNull()
+  })
+
+  it('gives up where only the markdown tier could decide (ambiguous markdown URL)', () => {
+    // The regex bails on a markdown image URL containing `(`; the full render
+    // resolves it. That is the one class fast mode knowingly hands back null for.
+    const r = both('![a](https://images.hive.blog/path_(a)_full.jpg)')
+    expect(r.full).toBeTruthy()
+    expect(r.fast).toBeNull()
+  })
+
+  it('applies to a raw markdown string as well', () => {
+    expect(catchPostImage('<center>https://images.hive.blog/x/pic.jpg</center>', 0, 0, 'match', FAST)).toBeTruthy()
+    expect(catchPostImage('https://www.youtube.com/watch?v=dQw4w9WgXcQ', 0, 0, 'match', FAST)).toBe(
+      catchPostImage('https://www.youtube.com/watch?v=dQw4w9WgXcQ', 0, 0, 'match')
+    )
+    expect(catchPostImage('![a](https://images.hive.blog/path_(a)_full.jpg)', 0, 0, 'match', FAST)).toBeNull()
+  })
+
+  it('memoizes the two modes separately', () => {
+    const e = entry('![a](https://images.hive.blog/path_(a)_full.jpg)')
+    expect(catchPostImage(e, 0, 0, 'match', FAST)).toBeNull()
+    expect(catchPostImage(e, 0, 0, 'match')).toBeTruthy()
+    expect(catchPostImage(e, 0, 0, 'match', FAST)).toBeNull()
+  })
+})
+
+describe('getEntryImageRawUrl and the LCP preload for a <center>-wrapped bare URL', () => {
+  it('finds it, and the preload avif still byte-matches the in-body <picture>', () => {
+    const firstAvif = (ss: string) => ss.split(',')[0].trim().split(/\s+/)[0].replace(/&amp;/g, '&')
+    const e = entry('<center>https://files.peakd.com/x/center-cover.png</center>')
+    const raw = getEntryImageRawUrl(e)
+    expect(raw).toBe('https://files.peakd.com/x/center-cover.png')
+    const m = markdown2Html(e, false).match(/<source type="image\/avif" srcset="([^"]+)"/)
+    expect(m).not.toBeNull()
+    expect(firstAvif(buildPictureSources(raw as string).avif)).toBe(firstAvif(m![1]))
+  })
+
+  it('does not read the text of an anchor pointing elsewhere as a bare URL', () => {
+    // `>` before a URL is allowed for wrapping tags, which would also admit an
+    // anchor's text. The renderer leaves such a link alone unless text equals
+    // href, so the scan blanks those anchors first. Applies to images and to
+    // YouTube URLs alike, and to the LCP preload (getEntryImageRawUrl).
+    const img = '<a href="https://example.com/page">https://files.peakd.com/x/linked.png</a>'
+    expect(markdown2Html(entry(img), false)).not.toContain('<img')
+    expect(getEntryImageRawUrl(entry(img))).toBeNull()
+    expect(catchPostImage(entry(img), 0, 0, 'match', FAST)).toBeNull()
+    expect(catchPostImage(entry(img), 0, 0, 'match')).toBeNull()
+
+    const yt = '<a href="https://example.com/page">https://www.youtube.com/watch?v=dQw4w9WgXcQ</a>'
+    expect(catchPostImage(entry(yt), 0, 0, 'match', FAST)).toBe(catchPostImage(entry(yt), 0, 0, 'match'))
+  })
+
+  it('still reads an anchor whose text equals its image href, which the renderer promotes', () => {
+    const u = 'https://files.peakd.com/x/self.png'
+    const body = `<a href="${u}">${u}</a>`
+    const full = catchPostImage(entry(body), 0, 0, 'match')
+    expect(catchPostImage(entry(body), 0, 0, 'match', FAST)).toBe(full)
+    expect(getEntryImageRawUrl(entry(body))).toBe(markdown2Html(entry(body), false).includes('<img') ? u : null)
+  })
+})

packages/render-helper/src/catch-post-image-memo.spec.ts

@@ -0,0 +1,46 @@
+import { vi } from 'vitest'
+
+// Counted stand-in for the markdown tier. Returning an image-free document
+// makes every lookup land on the null branch, which is the case the memo used
+// to miss: `if (item)` treated a cached null as a miss and re-rendered.
+const render = vi.fn(() => '<p>nothing to see</p>')
+vi.mock('./markdown-2-html', () => ({ markdown2Html: (...args: unknown[]) => render(...args) }))
+
+import { catchPostImage } from './catch-post-image'
+
+let n = 0
+const entry = (body: string) => ({
+  author: 'memo',
+  permlink: `p-${n++}`,
+  last_update: '2019-05-10T09:15:21',
+  body,
+  json_metadata: {}
+}) as any
+
+describe('catchPostImage memoizes null results', () => {
+  beforeEach(() => render.mockClear())
+
+  it('renders markdown once for a body with no image, not once per call', () => {
+    const e = entry('a long body with no image in it')
+    expect(catchPostImage(e, 0, 0, 'match')).toBeNull()
+    expect(catchPostImage(e, 0, 0, 'match')).toBeNull()
+    expect(catchPostImage(e, 0, 0, 'match')).toBeNull()
+    expect(render).toHaveBeenCalledTimes(1)
+  })
+
+  it('keys the memo on size and format, as before', () => {
+    const e = entry('another body with no image')
+    catchPostImage(e, 0, 0, 'match')
+    catchPostImage(e, 600, 500, 'match')
+    catchPostImage(e, 600, 500, 'match')
+    expect(render).toHaveBeenCalledTimes(2)
+  })
+
+  it('never renders markdown in fast mode', () => {
+    const e = entry('a body with no image, looked up in fast mode')
+    expect(catchPostImage(e, 0, 0, 'match', { fast: true })).toBeNull()
+    expect(catchPostImage(e, 600, 500, 'match', { fast: true })).toBeNull()
+    expect(catchPostImage(e.body, 0, 0, 'match', { fast: true })).toBeNull()
+    expect(render).not.toHaveBeenCalled()
+  })
+})