Review 5545

Review #5545 — Server-render gallery previews above the fold

imdreamrunner · MERGED · view on GitHub

Head reviewed: 7ca2a3d63f75364d86fbd5862fc84695973d5e32 Versions: Review Loop 1.4.0 · Component Audit Rubric 1.12 Lane: full · Time: 36 minutes · Verdict: approve with two non-blocking notes

This review began under 1.4.0 at gate 1 and retained that version through its critic pass.

Problem

On a cold /components visit, every preview tile was empty until hydration, intersection observation, and a showcase chunk completed. Under the stated throttling, the first preview appeared around three seconds after navigation.

Solution

The first twelve previews are now statically imported, so Next includes their rendered trees in server HTML. Later tiles remain lazy and load near the viewport. CSS owns the fixed 2× canvas/0.5 scale instead of a per-tile width measurement, the raw system-aware theme mode keeps server and first-client rendering equivalent, and the page reads showcase existence from the generated lightweight registry rather than full block source.

Four decisions: eager server rendering; CSS scaling without ResizeObserver; system-aware nested theming; lightweight showcase lookup plus a 400 px lazy margin. All are stated and route-local. The count is above the preferred two-decision shape, but splitting would leave the SSR half carrying the old page-weight problem or a first-paint dark-mode flash.

Architecture

ShowcaseThumbnail owns eager/lazy rendering; the generated showcase registry owns discovery.

• Eager server path: an exact-deployment, JavaScript-disabled probe found rendered content in exactly the first 12 of 97 showcase cards at head, versus 0 of 97 at base.
• Lazy client path: Chat Message had zero rendered children before intersection and one after scroll.
• Theme path: both the eager Button and lazy Chat Message changed computed color scheme from light to dark after the site toggle.
• Error path: three full-page runs produced no Preview unavailable.

One non-blocking ownership smell remains. eagerShowcases.ts:32-57 hand-maintains twelve component names, while data-extraction.test.ts:1039-1106 parses page source to police the duplicate. The guard prevents silent drift, so this is follow-up rather than a current defect; the build pipeline should eventually emit the static map.

Impact

A cold visitor receives twelve real previews in server HTML instead of empty muted tiles. In three alternating throttled Chromium runs, median first-preview DOM availability moved from 2,995 ms to 601 ms. Farther tiles still load on approach, and settled rendering stays equivalent.

API

No published package API changes. The docsite-internal ShowcaseThumbnail prop changes from {category, dirName} to {name}, with its only call site updated. The internal name key matches the generated registry's canonical key.

Theme targets

No theme target, token, variable, selector contract, or public theme surface changes. Server HTML resolves the nested neutral theme correctly under both light and dark OS preferences; the client toggle updates eager and lazy previews.

Breaking

• Behavior: intentional — twelve previews exist before hydration; later previews remain lazy.
• API: no public API change.
• Visual: the intended initial frame changes from an empty tile to a preview. Settled Button pixels were identical at 2560×1440 and 390×844; 1440×900 light/dark differed by at most 0.097%, confined to a four-pixel-high text strip from fractional centering.
• Theme: no; settled colors match base and system mode avoids a first-paint flash.

Performance & resources

• Effects/observers: the eager branch has none. The lazy branch keeps one cleaned-up IntersectionObserver; one ResizeObserver Effect, one state value, one callback, and layout reads are removed.
• Render/layout: eager work moves into server rendering and hydration; lazy work still waits for intersection. CSS owns scaling, with no offsetWidth read.
• Bundle: the exact local production page chunk is 59,315 B raw / 15,511 B gzip; eager showcase source totals 9,104 B.
• Transfer: head HTML is +59,669 B raw / +5,398 B compressed versus exact base. At three seconds, throttled head runs used 77 versus 88 scripts and about 1.727 MB versus 2.067 MB transferred. Initial named showcase chunks fell from nine to zero.

The small HTML increase is outweighed by the script reduction and 2.39 s median first-preview improvement. FCP medians were 840 ms head versus 816 ms base, a 24 ms difference treated as noise rather than an improvement.

Visual evidence

Manual frames were required because the intended first-rendered frame changes and the exact-head visual job was skipped.

Two exact-SHA captureWithSensors receipts held external JavaScript empty to inspect the server-only frame at 1440×900, light, LTR. The base frame is an empty muted Button tile; the head frame contains Primary, Secondary, Ghost, and Destructive buttons. Both receipts passed route, theme, color-mode, direction, viewport/media, target/state, fonts, geometry, and error sensors. The exact base-to-head source diff was banked with the capture.

Settled base/head runtime crops were supporting evidence, not canonical sensor receipts. They covered desktop light/dark, 2560×1440 light, and 390×844 light; wide/mobile were pixel-identical and desktop differed by at most 0.097%.

Accessibility & i18n

The preview remains inside inert, so newly server-rendered controls do not enter keyboard or accessibility paths. No visible or assistive string and no direction-sensitive behavior changes.

Both base and head emitted the same single disclosed React #418 hydration error in each of three runs. The change added no new error; this is inherited debt and not charged to the PR.

Judgement

Goal: met. Exact head server-renders exactly twelve previews; median first-preview availability improves 2,995→601 ms; initial named showcase chunks fall 9→0; lazy loading and theme updates still work.

Disposition: the data-boundary cleanup is a non-blocking follow-up; the stale generator pointer is an inline nit; the pre-existing hydration error remains inherited and disclosed.

Author can proceed: yes. No current user/builder defect, regression, or human-owned decision remains; the PR is already merged at these bytes.

Verdict: approve with two non-blocking notes.

The review, drafted; not posted

Thanks—SSR, hydration, lazy loading, and settled rendering check out. I left one non-blocking data-boundary note and one stale-comment nit.

Full review

[Reviewed by Robohands]

Proposed inlines:

• eagerShowcases.ts:32 — Could the generator emit this map? Hand-maintained component names cross the docsite’s build-data boundary; the source-parsing test mirrors that ownership.
• ShowcaseThumbnail.tsx:12 — This pointer survived the final refactor; the eager list now lives in eagerShowcases.ts, not generate-data.mjs.

Evidence

• Exact-head production build passed and statically generated /components.
• Docsite: 26 files / 409 tests passed; typecheck, focused ESLint, and git diff --check passed; reported CI was green.
• Exact deployment probes covered server HTML, three throttled cold loads per arm, the lazy client path, full-page fallback absence, theme updates, light/dark server colors, and settled pixels at three viewport sizes.
• Only Chromium was checked.

Independent critic

The critic failed the first draft's evidence wording, not its substance or verdict. It required saved 12/97 and Chat Message evidence, explicit labeling of unsensored settled crops, and removal of private delivery details from this public record. Those corrections are applied here; the recommended verdict remained approve.

What changed before posting

Not posted. Public actions were prohibited. The independent critic's evidence-scope and public-sanitization corrections were applied before this record was published.