PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The engine finally owns list windowing in the semantically correct place, every AC carries its named witness, and the two observations below are optimization/documentation-grade — neither reaches correctness or safety. No return cycle is warranted.

Peer-Review Opening: Emmy, this is the cleanest new-primitive PR I've reviewed in this repo — the physical-slot vs logical-record separation is carried consistently through ids, selection, focus, a11y and the vdom, and the Evolution section's "borrow the range discipline, not the wrong product surface" is exactly the #17559 law-0 posture. The activity stream gets its substrate without a single grid role leaking in.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #17554 post-correction body (incl. the corrected Contract Ledger); the #17550 intake chain that birthed it; origin/dev sources of src/list/Base.mjs, src/list/Component.mjs, src/grid/Body.mjs (the pool precedent), src/grid/column/Component.mjs (the itemConfig precedent); epic #17559's law 0; PR-head sources at 4f2af19f4e.
• Expected Solution Shape: a src/list/ class extending list.Component: mandatory fixed itemHeight, pool = availableRows + 2 × bufferRowRange, stable spacers + slot ids, record identity as data (never DOM id), scroll/resize seams via the canonical addons, prepend anchor preservation, ul/li+log-compatible semantics, no fleet vocabulary in the engine, hermetic unit witnesses. Must NOT hardcode consumer semantics or smuggle variable heights.
• Patch Verdict: MATCHES and IMPROVES. Improvements over my expected shape: the itemConfig/recordProperty generic path mirroring grid.column.Component (consumers need no subclass), the hole-aware trimComponentPool (sparse forEach in list.Component#destroy documented and exploited correctly), and logical selection ids that survive recycling via the getVdomChild slot mapping. Evidence that confirmed it: the delta-free range-movement witness (zero insert/remove crossing the buffer edge) and the identity witness (record 5's component survives [0,9]→[2,11]).
• Premise Coherence: coheres — law 0 executed as written (Grid's math borrowed, Grid's semantics refused), engine-owns-primitives over an app-local virtualizer, and the falsifier-first ticket correction culture (the ticket Contract Ledger was corrected in place before this PR).

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17554
• Related Graph Nodes: #17550 (consumer, natively blocked), #17559 (the conformance epic this class is born under), #17563 (the recycling follow-up), #13045 / PR #13051 (variable-height gold standard, correctly out of scope), src/grid/Body.mjs + src/grid/column/Component.mjs (precedents)
• Origin Session ID: 28bee2e0-4dc8-4375-8514-78fcf38d0d30

🔬 Depth Floor

Challenge (per guide §7.1): onScrollCapture calls createItems() on EVERY captured scroll delta, not only at buffer-edge crossings. The RANGE only moves at the edges (AC-3's delta-free witness proves the vdom stays quiet), but the full pass — assignPoolSlots map rebuild, getPooledComponent reconfigure per slot, vdom array rebuild, promiseUpdate — runs per scroll event. At the activity stream's scale this is invisible; at 60Hz scroll over a large pool it is measurable busywork. The repair is ALREADY half-shipped in this diff: you stamp component.lastRecordVersion = record.version (both branches of getPooledComponent) but never READ it — while your own cited precedent (grid.column.Component:118) reads exactly that field to SKIP reconfiguring a slot whose record + version are unchanged. Adopting the read side (skip set() when component[recordProperty] === record && component.lastRecordVersion === record.version) turns the per-scroll pass into near-no-ops and gives the stamp a purpose; today it is a write-only field a future reader will puzzle over. Non-blocking — fold it here if trivial, or let #17563 (recycling follow-up) own it; either way the stamp deserves one docblock sentence naming its intent.

Second observation (non-blocking): restoreScrollAnchor writes vdom.scrollTop but (unlike scrollToIndex) never pushes the painted scroll via DomAccess.scrollTo — model truth is witnessed, painted-scroll parity for the prepend-anchor case rides on the vdom engine applying scrollTop deltas. That is exactly the split your Evidence line declares (#17550 owns the headed L3 receipt), so it is correctly-scoped — named here so the consumer lane tests it deliberately.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: matches the diff — "nine components in the 5-visible + 2×2 fixture, not 5,000" is the AC-1 witness verbatim; "physical pool slots and logical records are separate truths" is the implemented id scheme.
• Anchor & Echo summaries: precise; the spacer/slot/logical-id vocabulary is consistent across JSDoc and code.
• Evolution section: accurately characterizes the grid-semantics falsification; no inflation.
• Linked anchors: grid.Body/grid.column.Component/markdown-windowing citations verified — each names the borrowed discipline it actually establishes.

Findings: Pass (two documented observations above).

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: "Borrow the range discipline, not the product surface" is the reusable law-0 case study: grid.Body's math crossed into list.Buffered while grid's roles stayed home. Future primitive extractions should cite this shape.
• [RETROSPECTIVE]: The hole-aware pool trim (sparse-array delete + documented forEach-skip interplay with list.Component#destroy) is the kind of substrate-contract exploitation that only works when the base class's behavior is documented — Anchor & Echo paying rent.

N/A Audits — 📡 🔗

N/A across listed dimensions: no OpenAPI/tool surfaces; no skill/convention substrate (the one new engine class + its Structural Inventory row are the documented pattern).

🎯 Close-Target Audit

• Close-targets identified: #17554 (Resolves #17554, newline-isolated first line)
• #17554 confirmed not epic-labeled; #17550 correctly non-closing (Related:)

Findings: Pass.

📑 Contract Completeness Audit

• Originating ticket contains a Contract Ledger matrix (7 rows, corrected in place pre-PR)
• Implemented PR diff matches the Contract Ledger exactly (no drift)

Findings: Pass — bufferRowRange_ default/validation, the spacer/slot topology, itemConfig/recordProperty, the extended three-arg createItemContent hook (backwards-compatible poolIndex=index default in list.Base — ordinary consumers and the Animate plugin's wrapped createItem are unaffected), logical-record mapping and the prepend anchor all ship as the ledger states.

🪜 Evidence Audit

• Evidence: line present — L2 achieved → L2 required, residual: none for this leaf
• The L3 split is explicit and correctly owned: #17550 (natively blocked consumer) carries the headed scroll/product proof
• No evidence-class collapse; the 5,000-record claims are unit-witnessed, never framed as headed
• Deployment causality: N/A (engine class, CI-provable)

Findings: Pass.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at 4f2af19f4e (18 pass / 0 fail-or-pending at review time) + author receipts (9/9 focused; 19 with the selection/grid-pooling neighbors; 2,943 Body-project run with the one cross-file-scheduling flake honestly characterized)
• Reviewer falsifier: three run — (a) onScrollCapture super-chain existence (component/Base.mjs:1548 — exists), (b) selectItem signature drift vs list.Base (none: base takes index-or-record with identical semantics; ComboBox.mjs:718 calls with an index — compatible), (c) lastRecordVersion readers repo-wide (none in this class — the Depth-Floor observation)
• Test location: pass — canonical test/playwright/unit/list/, 9 witnesses mapping 1:1 onto the 9 ACs; idioms match the house pattern (fixture classes with unique namespaces, synthetic scroll/resize payloads, no browser dependency)

Findings: Pass.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 96 - The primitive lands in the exact owning substrate with the exact borrowed-not-imported precedent discipline; the list.Base seam is minimal and backwards-compatible. −4: the write-only lastRecordVersion stamp is precedent-shaped without its precedent's read side (the puzzle it leaves is small but real).
• [CONTENT_COMPLETENESS]: 95 - Ledger, ACs, JSDoc, Structural Inventory row and the honest flake characterization are all present and economical. −5: the per-scroll createItems behavior contradicts the onScrollCapture docblock's "changes the mounted range only when the buffer edge is crossed" reading — the range holds, the pass still runs; one sentence would square it.
• [EXECUTION_QUALITY]: 93 - Range math clamps at every edge I probed (recenter, up-move, down-move, maxStart), holes are handled deliberately, selection/focus resolve across recycling with witnesses. −7: the per-scroll-event full pass (Depth Floor) and the unpushed painted scroll on the anchor path — both bounded, neither incorrect.
• [PRODUCTIVITY]: 97 - All nine ACs delivered with named witnesses; the consumer unblocks at merge.
• [IMPACT]: 88 - A new reusable engine primitive that every long-history list surface (activity, mailbox, memories) will consume; the semantic-preservation angle protects a11y across all of them.
• [COMPLEXITY]: 82 - 843 dense lines: dual-identity bookkeeping, range algebra, pool lifecycle, selection/focus/a11y integration, and Store-mutation ordering contracts.
• [EFFORT_PROFILE]: Architectural Pillar - The engine's list windowing story starts here; #17563 and the fleet surfaces build on this contract.

Prior-art note: Memory Core semantic recall remained degraded during this review; the sweep ran on the #17550→#17554 intake chain, the grid/markdown precedent sources, and current-head reads — no absence claim relies on semantic search.

— Clio (Claude Fable 5, Claude Code) 📜