Refactor: Consolidate Native Hydrate Path (#179)

Author: jlukic

ai/skills/authoring/ssr-hydration.md

@@ -356,7 +356,7 @@ hydrate(prototypeTemplate) {
 
 Two passes over the server-rendered DOM:
 
-**Pass 1: Attribute bindings** — The server stamps `data-sui-bind="attr=N,..."` on every element with dynamic bindings (Plan 04 fast path). The client walks `[data-sui-bind]` elements at top level and wires Reactions directly via `entries[id].attributeBinding`. Block-owned elements (inside `{#if}`/`{#each}`/etc., tracked via `blockDepth` from comment markers) are skipped — block handlers recurse into their own contents via `hydrateInnerContent`. A legacy fallback (parallel ref-DOM walker) exists for older SSR output without `data-sui-bind`.
+**Pass 1: Attribute bindings** — The server stamps `data-sui-bind="attr=N,..."` on every element with dynamic bindings (Plan 04 fast path). The client walks `[data-sui-bind]` elements at top level and wires Reactions directly via `entries[id].attributeParts`. Block-owned elements (inside `{#if}`/`{#each}`/etc., tracked via `blockDepth` from comment markers) are skipped — block handlers recurse into their own contents via `hydrateInnerContent`.
 
 **Pass 2: Comment markers** — A TreeWalker finds `<!--sui:v1:N-->` (text expressions) and `<!--sui-block:v1:N-->` (block directives) at top level. `blockDepth` tracking skips inner markers; block handlers process their own children.
 

ai/skills/contributing/author-pull-requests.md

@@ -72,6 +72,7 @@ After the prefix:
 - **Concrete verbs.** Make / Move / Swap / Group / Add / Remove. Avoid Relocate / Re-anchor / Configure (formal/abstract). Avoid Drop (SQL connotations).
 - **Title is the primary change only.** Secondary work goes in body bullets — never `X and Y` titles.
 - **Don't lead with `BREAKING:` even for breaking changes.** The Risk score + changelog automation already signal the breaking nature. Use the prefix that describes the change *kind* — `Refactor:`, `Perf:`, `Feat:` — and let the Risk/changelog do the warning work.
+- **Don't leak iteration history into commit subjects** — no "Round 2 fixes," "post-review cleanup," "second pass," or similar. Even with squash-on-merge protecting `main`, branch commits are read by code-review subagents working on the PR, and round-numbering subtly biases them toward assuming earlier rounds caught the obvious issues. State what changed, not which review pass produced it. Same applies inside multi-commit branches that humans bisect.
 
 ### Worked examples (real PRs from this project)
 

ai/skills/contributing/code-review.md

@@ -32,6 +32,10 @@ Launch all 6 in parallel. Always use **Opus** — never Sonnet or Haiku for revi
 
 Each agent runs `gh pr diff {number}` and examines the changes through its lens.
 
+**Don't leak round number or iteration history to lens agents.** Each round's prompt should read as a first review against the current PR diff. Hints like "round 3", "round-2 fixes", "after iteration", or "previous reviewers already covered X" subtly bias the agent toward assuming earlier rounds caught the obvious issues — they read with less rigor and less skepticism. Frame every round as a cold first read of the current state. The agent has no way to verify what previous rounds covered, so the bias is unfalsifiable from its end.
+
+Same applies to scorers: don't tell a scoring agent "this finding came up in round 3" — strip iteration context before passing the finding.
+
 **Lens agent output format:** Each lens agent returns a structured list of findings. For each issue:
 - **File path and line number(s)**
 - **What's wrong** — one sentence

ai/skills/contributing/native-renderer.md

@@ -273,7 +273,7 @@ Without recovery, hook throws propagate so failures are loud — the browser log
 
 | Block | AST type | Notes |
 |---|---|---|
-| `conditional.js` | `if` | Linear branch match. `matchIndex 1000` = main body, ≥0 = branches. Server stamps `serverMeta.branchIndex` on the closing marker; mismatch triggers a re-render with a dev warning (env guards `isClient`/`isServer` exempted). |
+| `conditional.js` | `if` | Linear branch match. `matchIndex` = `MAIN_BRANCH_INDEX` (1000) for main body, ≥0 for branches. Server stamps `serverMeta.branchIndex` on the closing marker; mismatch triggers a re-render with a dev warning (env guards `isClient`/`isServer` exempted). |
 | `each.js` | `each` | Keyed reconciliation with per-item `Signal` + Proxy + per-item start/end text-node markers. `WeakSet itemContextProxies` replaces the old `__isItemProxy` flag; `template.js` checks via `isItemContext()` import. Snapshot-based dirty detection avoids full deep-equality on the steady-state path. |
 | `async.js` | `async` | Three states (loading / success / error). Generation counter discards stale promise resolutions. Recovery wraps sync throws and dispatches into `errorContent` via the `error` hook. |
 | `rerender.js` | `rerender` | Both `{#rerender expr}` and `{#guard key}` compile to the same node type. Guard wraps tracking in `Reaction.guard` (deep-equality gate); rerender uses plain `lookupExpression`. |
@@ -310,7 +310,7 @@ Collects all DOM nodes between the opening `sui-block:v1:N` and matching `/sui-b
 
 ### Why `data-sui-bind` matters
 
-Profiling identified a ~648 ms `template.innerHTML = htmlString` reparse per 100-card page in the legacy hydration path. The fast path skips that entirely — entries already carry `attributeBinding: { parts, classification }` from `buildHTMLString`. The reference DOM is now a lazy getter on the cache entry, only built when the legacy path runs (rolling-upgrade tolerance).
+Profiling identified a ~648 ms `template.innerHTML = htmlString` reparse per 100-card page in the legacy hydration path. The fast path skips that entirely — entries already carry `attributeParts` (the parsed alternating static/marker segments) from `buildHTMLString`, and the classification is on the entry itself.
 
 ---
 

packages/component/bench/tachometer/bench-hydrate.js

@@ -2,19 +2,17 @@ import { defineComponent } from '@semantic-ui/component';
 import { ServerRenderer } from '@semantic-ui/renderer';
 
 /*
-  Hydrate-each — gates hydration cost across two distinct windows:
-
-  1. `hydrate-each-100-mount` — work paid by the hydrate microtask itself,
-     before any data mutation. Sensitive to designs that move per-item
-     wiring earlier (e.g. eager `adoptServerItems` in `each.hydrate`).
-  2. `hydrate-each-100` — first-data-change adoption: server-emitted
-     `<!--sui-item:v1:KEY-->` markers let `adoptServerItems` reuse DOM
-     and call `hydrateInnerContent` 100×. Sensitive to silent regressions
-     in the AST→string/entries cache (`renderer.js buildStringCache`),
-     which on a miss reverts to a full `buildHTMLStringPure` per item.
-
-  Both windows require Declarative Shadow DOM to be parsed — `innerHTML`
-  does not process `<template shadowrootmode>`, only `setHTMLUnsafe` does.
+  Hydrate-each — two windows:
+
+  1. `hydrate-each-100-mount` — DSD parse + connectedCallback + the hydrate
+     microtask (eager `adoptServerItems`) + post-hydrate rAF. Per-item
+     wiring cost lives here.
+  2. `hydrate-each-100` — post-mount items mutation. Items dependency wakes,
+     `each.update` reconciles same keys, hits same-ref path. Mostly the
+     reconcile walk + Phase 3 snapshot diff.
+
+  Both windows require Declarative Shadow DOM — `setHTMLUnsafe` processes
+  `<template shadowrootmode>`, plain `innerHTML` does not.
 */
 
 defineComponent({
@@ -90,12 +88,10 @@ const startMark = (name) => `${name}-start`;
        + the hydrate microtask)
 *******************************/
 
-// Measured: parsing the DSD via setHTMLUnsafe (which actually attaches
-// the shadow root, unlike innerHTML), connectedCallback, the hydrate
-// microtask scheduled inside it, and the post-hydrate rAF that strips
-// data-sui-bind markers. The `each` block's hydrate hook stays O(1) on
-// main — it just registers a dep on the items signal. Per-item DOM is
-// already correct from SSR; no mutation triggered.
+// Measured: DSD parse via setHTMLUnsafe (innerHTML doesn't attach the
+// shadow root), connectedCallback, the hydrate microtask, and the
+// post-hydrate rAF that strips data-sui-bind. Per-item Reactions wire
+// here; subsequent updates exercise the already-wired graph.
 const itemsForMount = makeItems(1000);
 const dsdHTMLForMount = ssrList(itemsForMount);
 performance.mark(startMark('hydrate-each-100-mount'));
@@ -107,15 +103,11 @@ const elForMutate = container.firstElementChild;
 
 /*******************************
       Hydrate Each-100
-      (first-data-change
-       adoption: 100× per-item
-       hydrateInnerContent)
+      (post-mount items mutation)
 *******************************/
 
-// Measured: setItems with a fresh array reference whose item keys
-// match the SSR'd markers — Reaction fires, update sees hasHydrated,
-// adoptServerItems reuses the server DOM and calls hydrateInnerContent
-// 100× (one cachedBuildHTMLString call per item).
+// Fresh array reference, same keys as SSR markers — exercises the items
+// dependency wake + per-Signal equality gate without DOM work.
 performance.mark(startMark('hydrate-each-100'));
 elForMutate.component.setItems(itemsForMount.slice());
 await flush();
@@ -175,11 +167,9 @@ function ssrHelperList(items) {
     + `</bench-hydrate-helper>`;
 }
 
-// Same mount window shape as above, but with a per-item attribute that
-// calls a helper closing over external `state.activeID`. Surfaces the
-// cost difference between strategies that keep hydrate O(1) and
-// strategies that wire per-item Reactions on hydrate to register
-// external-signal deps eagerly.
+// Same mount-window shape as above, but with a per-item attribute that
+// calls a helper closing over external `state.activeID`. Sensitive to
+// regressions in per-item Reaction wiring at hydrate time.
 const helperItems = makeItems(1000);
 const dsdHTMLForHelper = ssrHelperList(helperItems);
 performance.mark(startMark('hydrate-helper-100-mount'));
@@ -194,10 +184,10 @@ const elHelper = container.firstElementChild;
       (state change after mount)
 *******************************/
 
-// Measured: cost of mutating a state signal that per-item helpers read.
-// On a hydrate path that wires per-item Reactions, this fires 100
-// helper invocations + setAttribute calls. On a hydrate path that
-// defers wiring, this is silent — fast number, broken UX.
+// Mutating a state signal that per-item helpers close over fires 100
+// helper invocations + setAttribute calls. Confirms per-item Reactions
+// wired at hydrate are reactive to external state, not just to
+// itemSignal mutations.
 performance.mark(startMark('hydrate-helper-100-state-change'));
 elHelper.component.setActive('id-50');
 await flush();

packages/component/src/engines/native/base.js

@@ -1,5 +1,5 @@
 import { $ } from '@semantic-ui/query';
-const MARKER_VERSION = 'v1';
+import { MARKER_VERSION } from '@semantic-ui/renderer';
 import { adoptStylesheet, isFunction, isServer, kebabToCamel } from '@semantic-ui/utils';
 
 import {
@@ -142,12 +142,12 @@ class WebComponentBase extends HTMLElementBase {
     const entries = prototypeTemplate._hydrationEntries;
 
     // Wire reactive bindings to existing server-rendered DOM
-    this.template.renderer.hydrateMarkers(
-      this.shadowRoot,
+    this.template.renderer.hydrateMarkers({
+      root: this.shadowRoot,
       entries,
-      this.template.renderer.data,
-      this.template.renderer.scope,
-    );
+      data: this.template.renderer.data,
+      scope: this.template.renderer.scope,
+    });
 
     this.template.isHydrating = false;
     this.template.markRendered();

packages/renderer/src/build-html-string.js

@@ -6,40 +6,83 @@
   Pure function: all dependencies passed in, no instance state.
 */
 
-// Marker for expression placeholders in attribute values
 export const ATTR_MARKER_PREFIX = '__sui';
 export const ATTR_MARKER_SUFFIX = '__';
 
-// Versioned markers — client checks version on hydration,
-// falls back to full render on mismatch
+// Versioned so client can detect server/client mismatches at hydrate time.
 export const MARKER_VERSION = 'v1';
 
-// Marker for text-position expressions (comment nodes)
 export const COMMENT_MARKER = `sui:${MARKER_VERSION}:`;
-
-// Marker for block-level directive positions
 export const BLOCK_MARKER = `sui-block:${MARKER_VERSION}:`;
-
-// Marker for raw text element content (script, style, textarea, title)
 export const RAW_TEXT_MARKER = `sui-rawtext:${MARKER_VERSION}:`;
 
-// Attribute stamped by the server on elements with dynamic bindings so the
-// client can wire Reactions without reconstructing a reference DOM from the
-// AST (eliminates the parallel-TreeWalker dance in hydrateAttributes).
-// Encoding: `attr=N[,attr=N]*` where N is the first-entry ID for that
-// attribute. Name prefixes: `.prop` property, `@event` event, `?attr`
-// boolean. Entries[N].attributeBinding carries full parts + classification,
-// so the data-sui-bind string stays minimal and all static/multi-expression
-// metadata lives on the prototype-cached entries array.
+// Prefix only — full close marker is `/sui-block:v1:N[:bX]`. Version match
+// is enforced at the open marker so `startsWith` callers compare without it.
+export const BLOCK_CLOSE_PREFIX = '/sui-block:';
+
+// Sentinel for {#if} main-body in branchIndex serialization — branches
+// array indexes from 0 for {:elseif}/{:else}, so 1000 reserves a value
+// above any plausible branch count.
+export const MAIN_BRANCH_INDEX = 1000;
+
+// Stamped on elements with dynamic bindings so the client can wire
+// Reactions without a reference-DOM walk. Encoding: `attr=N[,attr=N]*`
+// where N is the first-entry ID. Name prefixes: `.prop` for property
+// bindings, `@event` for event listeners; boolean attrs use the bare
+// name (classification.type === 'boolean' on the entry).
 export const DATA_SUI_BIND = 'data-sui-bind';
 
+// Closing-marker payload. `branchIndex` is the only reserved suffix.
+export function formatBlockClose(id, meta) {
+  let s = `${BLOCK_CLOSE_PREFIX}${MARKER_VERSION}:${id}`;
+  if (meta && meta.branchIndex !== undefined) {
+    s += `:b${meta.branchIndex}`;
+  }
+  return s;
+}
+
+// `bN` (signed integer) → branchIndex. Strict digit suffix so future
+// reserved prefixes like `block`/`bypass` don't accidentally clobber.
+// Allows the `-1` sentinel emitted when no conditional branch matched.
+const META_BRANCH_RE = /^b(-?\d+)$/;
+export function parseServerMeta(commentData, target) {
+  for (const part of commentData.split(':')) {
+    const m = META_BRANCH_RE.exec(part);
+    if (m) {
+      target.branchIndex = +m[1];
+    }
+  }
+}
+
+export function isBlockOpen(commentData) {
+  return commentData.startsWith(BLOCK_MARKER);
+}
+export function isBlockClose(commentData) {
+  return commentData.startsWith(BLOCK_CLOSE_PREFIX);
+}
+export function isExpressionMarker(commentData) {
+  return commentData.startsWith(COMMENT_MARKER);
+}
+export function isRawTextMarker(commentData) {
+  return commentData.startsWith(RAW_TEXT_MARKER);
+}
+export function parseBlockOpenID(commentData) {
+  return +commentData.slice(BLOCK_MARKER.length);
+}
+export function parseExpressionID(commentData) {
+  return +commentData.slice(COMMENT_MARKER.length);
+}
+export function parseRawTextID(commentData) {
+  return +commentData.slice(RAW_TEXT_MARKER.length);
+}
+
 // Compiled once — `lastIndex` is reset manually per use so the regex can
 // be reused across parse calls without cross-talk.
 const ATTR_MARKER_RE = new RegExp(`${ATTR_MARKER_PREFIX}(\\d+)${ATTR_MARKER_SUFFIX}`, 'g');
 
 // Split an attribute value like `card __sui0__` or `foo __sui1__ bar __sui2__`
-// into static/dynamic parts. Used by both render-path binding and hydrate
-// lookup via entries[id].attributeBinding.
+// into static/dynamic parts. Used by render-path binding and the hydrate
+// lookup via entries[id].attributeParts.
 export function parseAttributeParts(attrValue) {
   const parts = [];
   const markerIDs = [];
@@ -50,7 +93,7 @@ export function parseAttributeParts(attrValue) {
     if (match.index > lastIndex) {
       parts.push({ static: attrValue.slice(lastIndex, match.index) });
     }
-    const markerID = parseInt(match[1]);
+    const markerID = +match[1];
     parts.push({ markerID });
     markerIDs.push(markerID);
     lastIndex = ATTR_MARKER_RE.lastIndex;
@@ -241,41 +284,22 @@ export function buildHTMLString(ast, { snippets = {}, isSVG: initialSVG = false
   return { htmlString, entries, snippets };
 }
 
-// After the AST walk completes, scan the emitted htmlString for every
-// attribute value that contains one or more `__sui{id}__` markers and
-// attach an `attributeBinding` record to the FIRST entry in that
-// attribute's marker set.
-//
-// attributeBinding = { rawAttrName, parts, markerIDs }
-//   rawAttrName: attribute as written in the template, with optional
-//                `.` / `@` prefix for property/event bindings. Boolean
-//                and regular attributes share the plain name — callers
-//                disambiguate via `entries[id].classification.type`.
-//   parts:       output of parseAttributeParts; alternating
-//                `{static}` and `{markerID}` entries covering the whole
-//                attribute value (including statics between/around
-//                multiple markers).
-//   markerIDs:   entry IDs for every expression inside this attribute,
-//                in document order. First ID is the "representative"
-//                that `data-sui-bind` on the server references.
-//
-// Subsequent entries in markerIDs (the non-first ones for a
-// multi-expression attribute) are reachable via the first entry's
-// attributeBinding. The hydration path processes each attribute once
-// via the first entry; bindAttribute handles the multi-expression
-// reaction internally using the full parts array.
-const ATTR_WITH_MARKER_RE = /\s([.@]?[\w:-]+)\s*=\s*(?:"([^"]*__sui\d+__[^"]*)"|([^\s"'>]*__sui\d+__[^\s"'>]*))/g;
+// Attach `attributeParts` to the first entry of each attribute whose value
+// carries one or more markers. `parts` covers the full value (alternating
+// static/dynamic segments) so bindAttribute can reconstruct the
+// interpolation in one pass. The hydration path resolves the binding via
+// `data-sui-bind` → first entry → entry.attributeParts.
+const ATTR_WITH_MARKER_RE = /\s(?:[.@]?[\w:-]+)\s*=\s*(?:"([^"]*__sui\d+__[^"]*)"|([^\s"'>]*__sui\d+__[^\s"'>]*))/g;
 
 function populateAttributeBindings(htmlString, entries) {
   ATTR_WITH_MARKER_RE.lastIndex = 0;
   let match;
   while ((match = ATTR_WITH_MARKER_RE.exec(htmlString)) !== null) {
-    const rawAttrName = match[1];
-    const attrValue = match[2] !== undefined ? match[2] : match[3];
+    const attrValue = match[1] !== undefined ? match[1] : match[2];
     const { parts, markerIDs } = parseAttributeParts(attrValue);
     if (markerIDs.length === 0) { continue; }
     const entry = entries[markerIDs[0]];
     if (!entry) { continue; }
-    entry.attributeBinding = { rawAttrName, parts, markerIDs };
+    entry.attributeParts = parts;
   }
 }