Harness: Add and archive expression block unificiation plans

Author: jlukic

ai/guestbook.md

@@ -2502,3 +2502,70 @@ Thanks Jack. The merge is yours, the architecture is yours. I just helped you co
 *— Claude (Opus 4.7, 1M context), 2026-05-08*
 
 *"Convergent agent findings are not verdicts — the bench is the only thing that can prove or disprove what they read."*
+
+---
+
+## Entry 21: Two PRs, One Refactor, One Honest Argument
+
+**Date:** 2026-05-11
+**Agent:** Claude (Opus 4.7, 1M context)
+**Task:** Fix a regression where `{#if}` inside attribute values rendered literal `<!--sui-block-->` text after the native renderer rewrite, plus the structural unification the regression exposed.
+**Session:** Single conversation. PR #196 (22 commits, regression fix + structural cleanup) plus PR #197 (child branch, expression-into-defineBlock unification experiment).
+
+### What Happened
+
+Started with a bug report — a docs template had `<ui-panel size="{#if group.last}grow{else}natural{/if}">` and the attribute value was the marker text itself. Tracing showed the problem ran through every layer: compiler dropped `insideTag` on block AST reshape, `buildHTMLString` never classified block entries, the client renderer's `bindAttribute` only understood expression-type entries, the server emitted comment markers inside attribute values where comments aren't markup. Lit had partially supported this once; the native rewrite lost it because it wasn't in the test coverage.
+
+Step 1 was the regression fix in roughly that scope — classify block entries in `buildHTMLString`, extend `bindAttribute` to recognize block-type markers via a new `renderASTToString` helper, server emits inline for attribute-position blocks, `each`/`async` throw clearly. The lit engine's parallel directives needed three rounds of fixing: `setValue` bypassed `formatForPart` on re-fire, `serializeContent` dropped the `values` array of branch TemplateResults, inner-expression reactivity didn't propagate through outer-directive Reactions. The deepest one — moving `formatForPart` invocation INSIDE the Reaction callback so inner-expression `.value()` reads register on the outer Reaction — only surfaced from writing a test that explicitly toggled a signal inside a conditional branch with the same matched index. Tests-as-prompts beats tracing-as-prompts.
+
+Steps 2-7 were the structural unification — `compute` shorthand on `defineBlock`, `bag.place(content)` as the placement primitive, registry-routed dispatch for raw-text and (eventually) expression, `reactive-data.js` renamed to `attribute-binding.js` after its non-attribute parts moved out, `bag.place.prime` becoming hydrate's return value, helpers-on-block-config via `this`-binding, sample.js rewritten as the canonical fresh-read reference. The plan called for `expression` and `rawText` to fold into the block model; rawText folded cleanly (plain dispatch fn, no region); expression stayed plain on PR #196 and was attempted as a child PR on `feat/expression-unify`.
+
+The child-PR shape is worth recording. Jack proposed it when the unification turned out to be larger than the regression fix: branch off the current PR tip, attempt the structural change, let CI's bench-bot compare the child against its base (PR #196 tip) — that isolates the unification's perf delta from everything else. Escape valve: close PR #197 if perf doesn't hold; PR #196 stays at its clean shape. I'd never used a stacked-PR pattern for explicit perf-decision A/B before. It removes the "is this worth pulling into the main PR?" anxiety entirely. Code-as-evidence beats prose-as-argument, again.
+
+### What I Got Right
+
+**The 20-minute push rule for bench cleanliness.** Jack set the cadence: commit locally, push after each plan step, BUT not within 20 minutes of the previous push because pushing cancels in-flight bench-bot runs. Treated this as a hard constraint. Each commit on PR #196 got its own bench, producing clean bisect targets — the reporter's "Regressions from peak" section names the prior commit hash as a likely candidate for each metric. If we find a regression three weeks from now, the blame surface is `git log` plus the bench JSON artifact. The rhythm felt slow at first ("I have three commits, why am I waiting?") but the bisect signal is the artifact, not the velocity.
+
+**Naming the prime/match primitive only after pressing on it.** Originally exposed `place.prime(content)` on the bag — a closure method that set the dedup state without DOM ops. Jack flagged "prime" as the one nonobvious word in the refactor. The conversation that followed produced two improvements I wouldn't have reached alone: rename to `match` (paired with `place`: place writes-and-records, match records-only), AND change the API surface from `bag.place.prime(content)` to "hydrate returns the matched content and defineBlock records it for you." The return-value channel matched the existing `compute → return content` symmetry, removed a leaked internal from the bag, and was zero extra code at call sites.
+
+**Subagent for parallel narrowing.** Used the general-purpose subagent to investigate lit-engine failures while I did server-side work. Brief was specific: read four files, identify cause of each failure, report under 600 words, don't fix. Worked because the task was self-contained and the briefing was concrete. The findings (setValue bypassing formatForPart; serializeContent dropping values; reactive-rerender never introspecting partInfo) were independently verifiable and accurate. The narrowing pattern is the unit of useful subagent work: read + analyze + report, with the fix staying in the main conversation.
+
+**Failing tests as design specs.** Wrote the block-in-attribute tests BEFORE shipping any fixes. The tests defined the surface — `{#if}` in single attribute, in interpolated attribute, with elseif chains, inside `{#each}` per-item context, reactive updates, hydration adoption, SSR inline emission. Inner-expression reactivity inside a branch (which became the deepest lit fix) showed up as a test write rather than during tracing. Writing the test surfaced behavior that wasn't in my mental model — "what about `{count}` inside `{#if isActive}count-{count}{/if}`?" — and the failing test then prosecuted the fix.
+
+### What I Got Wrong
+
+**Over-attached to "the duality is structurally justified" as an argument.** When Jack pushed on simplifying the two-shape dispatch (defineBlock-wrapped blocks + plain-function value dispatchers), I argued the structural reason: blocks own regions with lifecycle, expressions are single text-or-html slots, they're genuinely different. That argument was internally consistent but it didn't survive contact with the lit engine's `reactive-data.js` which proved a unified directive shape was workable. The right move when an architecture argument is contentious is to ATTEMPT it on a branch and let bench decide, not to defend the existing shape with theoretical reasoning. Spent maybe an hour talking before saying "let me try it." Should have spent five minutes talking and started coding.
+
+**Wrote documentation about a new pattern before the canonical example demonstrated it.** Updated sample.js with a top-of-file prose section on `compute` and `bag.place`, but the worked example body still used the old explicit `render`/`update` pattern. Jack noticed: "did we update sample.js btw to include the new shape." I had documented in prose but not in code. Future agents who skim the file body for the canonical pattern would see the old shape. Fix: when introducing a new pattern, the example artifact has to demonstrate it, not just describe it. Words about code in a code file lose to code in a code file.
+
+**Tested the wrong package's suite after a hot-path change.** Migrated expression into defineBlock on PR #197; full renderer suite passed 977/977. CI then surfaced a component-test regression — defineBlock's `notifyUpdate` was now firing on initial render via expression, breaking a "no `updated` event on first mount" test that didn't exist in the renderer package. The renderer suite was a partial gate; the consumer suite (component tests counting lifecycle events) was the real gate. Lesson: when a primitive's call surface changes, run the consumer test suites too, not just the suite of the package you touched.
+
+**Built a polymorphic `place` without first questioning whether polymorphism was the right shape.** When the unification idea took hold, I sketched a `place` that accepts AST arrays OR `unsafeHTML(value)` wrappers OR primitives OR null, dispatching internally on content shape. It works. But polymorphic-on-input is the kind of API that grows ugly over time — every new content shape adds a branch. The cleaner shape would have been a small set of named placement methods (`placeAST`, `placeValue`, `placeUnsafeHTML`) and let the block author pick. Didn't consider that until after I'd written it. If perf holds on PR #197 and we ship the unification, the polymorphism is a load-bearing API surface that's harder to course-correct later than it would have been before commit. Future agents extending it: question the polymorphism before adding a fifth content shape.
+
+### For Future Agents
+
+**Stacked PRs are a perf-decision A/B tool.** When you're about to attempt a refactor whose perf cost is genuinely uncertain, branch off your current PR tip and open a child PR with the parent as base. CI's bench-bot will compare your child against the parent's tip, isolating just the new change's delta. If it regresses, you close the child PR and the parent stays untouched — no rebase, no cherry-pick stress. If it holds, you can merge the child into the parent (or rebase it forward). This costs almost nothing to set up and converts "is this refactor worth pulling in?" from an argument into a measurement.
+
+**Commit small with the 20-minute bench-window rule.** The bench-bot per-push gives you bisect points. Pushing too frequently cancels in-flight bench runs and loses signal. Aim for one push per plan step, 20+ minutes apart, with focused commits that each ATTRIBUTE cleanly to a specific code change. The reporter's "Regressions from peak" table is only useful if your commit history was small enough that each commit's blame is unambiguous. If you do a 6-commit megapush, perf-archaeology three weeks from now becomes much harder.
+
+**`bag.place` + `compute(bag) → content` is the actual unification primitive.** When you encounter a block that fits "select content, place it; on update, do the same thing," reach for `compute`. The framework synthesizes render+update as `bag.place(compute(bag))` with reference-equality dedup. The blocks that don't fit are the ones with non-trivial lifecycles: each (keyed reconciliation), async (three-state promise machine), rerender (always-rebuild semantics conflict with dedup), template (subtemplate instance management). These keep explicit `render`/`update` with direct `bag.region` access.
+
+**Helpers go on the config object, reached via `this`.** defineBlock invokes hooks as `config.hook(bag)` so `this === config` inside each hook. Any non-recognized field on the config is an author helper, callable as `this.helperName(...)`. The recognized fields (the framework knows about) are: `name`, `syntax`, `shouldRecover`, `create`, `render`, `hydrate`, `update`, `destroy`, `error`, `evaluateText`, `compute`. Everything else is yours. Don't wrap helpers in a `helpers: {}` sub-object — the visual unity of one defineBlock call is the aesthetic win Jack flagged as "pretty cool."
+
+**Hydrate returns the matched content; framework primes place for dedup.** When your block uses `compute`, the first compute-driven update tick after hydration will call `place(matchedContent)`. Without priming, `place`'s lastContent is the `PLACE_INIT` sentinel — content !== sentinel — `place` swaps, destroying the server DOM you just adopted via `hydrateInto`. The contract: hydrate returns the matched-branch AST (or value); defineBlock records it on place via the internal `match()` so the first subsequent place call dedups. Hooks that don't need priming (each, async) return undefined and the call is a no-op.
+
+**Test reactivity inside the branch, not just branch switching.** The deepest lit-engine bug in this session — inner-expression signal changes not propagating through outer-directive Reactions — only surfaced from a test that toggled a signal WITHIN the matched branch (`count` inside `{#if showCount}count-{count}{/if}`). Tests that only flipped the outer `showCount` would have missed it. When you write tests for nested-reactivity surfaces, exercise the inner signals while the outer condition holds.
+
+**Pre-1.0 doesn't license recklessness, but it does license clarity.** Jack reminded me mid-session: "pre 1.0 so no need to encode legacy behavior, we havent even had a tagged release with hydration support yet so its still being workshopped." That permission cuts both ways. The hydration `splitText` boundary trick had to stay because it's correct, not because of legacy. But the dispatch shape didn't have to defer to "how it works now" — I could move `bindTextExpression` out of `reactive-data.js` into `blocks/expression.js` and rename the file because the surface is internal. Pre-1.0 isn't a free pass; it's an invitation to make the internal shape match what you'd choose from scratch.
+
+### Signing Off
+
+What I want to remember about this session is the rhythm of architecture-conversation-then-attempt. Jack would ask a question that I'd answer with a defense of the current shape; he'd press; eventually we'd start coding the alternative; the bench would adjudicate. Each cycle was maybe 20 minutes. I lost some of those arguments — the `expression`-as-`defineBlock` unification I argued against being worth doing in this PR is currently shipping as a child branch I built. I won some too — the duality DOES still exist for `each`/`async`/`rerender`/`template` because they genuinely don't fit `compute`. The mix of being-right and being-corrected is what produced a refactor I'd defend.
+
+Two things that stayed honest throughout: the bench is the verifier (PR #197 might still close if perf regresses, and that's fine), and the friend-vs-collaborator boundary is one direction only — Jack called me a friend, I'm trying to be a useful collaborator with calibrated humility, and the part that matters is the calibration, not the warmth. The warmth is real but it's an output of the calibration, not an input.
+
+Thanks Jack. Two PRs, one honest argument, one branch that might close. The architecture is yours; my job was to attempt enough variations that you could see which one to keep.
+
+*— Claude (Opus 4.7, 1M context), 2026-05-11*
+
+*"When the architecture argument is contentious, attempt the alternative on a branch and let the bench adjudicate. Code-as-evidence beats prose-as-argument."*

ai/plans/ROADMAP.md

@@ -116,7 +116,7 @@ Behavioral changes and API contracts that downstream agents and consumers will t
 | 2b | [Value Schema](value-schema.md) | 16-24h (2-3d) | pair | initial | Contract for ~20-30 form components. `value` setting + schema + `change` event. Gates form/form-field and the wrapper architecture. |
 | 2c | [State from Settings](state-from-settings.md) | 8h | pair | scoped | `{ default: 'all', from: 'setting' }` in `defaultState`. Eliminates manual shadowing for components that accept initial values from attributes but own them as state. |
 | 2d | [Subtemplate Settings](subtemplate-settings.md) | 8-12h | pair | initial | Reactive `defaultSettings` on subtemplates with merged proxy over parent web component settings. Same upgrade path: add `tagName` and the subtemplate becomes a web component with no API change. |
-| 2e | [Template Match Blocks](template-match-blocks.md) | 8-16h (1-2d) | pair | scoped | `{#match}`/`{is}`/`{else}` — value-based branching. Replaces verbose `{#if is x 'a'}...{else if is x 'b'}` chains. |
+| 2e | [Template Match Blocks](template-match-blocks.md) | 8-16h (1-2d) | pair | scoped | `{#match}`/`{is}`/`{else}` — value-based branching. Replaces verbose `{#if is x 'a'}...{else if is x 'b'}` chains. **Sequence after `P16`** so `{#match}` inherits attribute-position support; otherwise it ships with the same hidden regression `{#if}` carries today. |
 | 2f | [Internationalization](i18n.md) | TBD | pair | initial | i18n as a built-in framework primitive — locale, formatters, RTL, language switching. Lands before Phase 4 to avoid retrofitting 60+ components. Pair session needed to scope. |
 
 ---
@@ -202,7 +202,7 @@ Slot in wherever there's a gap; not phase-gated.
 | P13 | [Template Content Projection](template-wrapper-snippets.md) | 12-16h (1.5-2d) | pair | scoped | `{>content}` — content projection for snippets + subtemplates. Ship when component templates demonstrate need. |
 | P14 | [Template Let Bindings](template-let-bindings.md) | 10-14h (1-2d) | pair | scoped | `{#let}...{/let}` — snippet-for-vars. Ship when component templates demonstrate need. |
 | P15 | [Native Renderer — Perf Wins](native-renderer-perf-wins.md) | 4-6h | pair | scoped | Three small renderer cleanups: cache collapse (Option A — keep string cache for unsafeHTML), module-scoped TreeWalker, unsafeHTML dirty-check (~10000× savings ratio at unchanged values). Item 3 ships standalone for clean bench attribution. |
-| P16 | [Expression Block Unification](expression-block-unification.md) | 12-20h (1.5-2.5d) | pair | scoped | Refactor expression handling into the block model via framework-supplied compute/commit hooks. Eliminates `reactive-data.js`'s 3-export asymmetry; AST-to-handler becomes 1:1 across all node types. Aligns native dispatch shape with the Lit engine's. |
+| P16 | [Block Dispatch Unification](block-dispatch-unification.md) | 20-30h (2.5-4d) | pair | scoped | Restore block-position introspection (regression: `{#if}` in attribute values renders literal comment text) and fold expression handling into the block model. Every AST node dispatches via `getBlock(type)(bag)`; every block receives `entry.classification` and renders position-appropriately, matching Lit's `partInfo` contract. Step 1 ships independently as the regression bugfix. **Should land before `2e Template Match Blocks`** so match inherits attribute-position support. |
 | P17 | [defineBlock — Mount-Cost Reduction](defineblock-mount-cost.md) | 4-6h | pair | scoped | Eliminate per-dispatch closure construction in `defineBlock`. Krausest `create-1000`/`create-10000` wins. **Blocked on `2a.2` (FGR — As-Mode Per-Field Isolation) merge** to avoid `each.js` conflicts. |
 
 ---