From 23659233caa27362b3515e42ef9abd473e4bb730 Mon Sep 17 00:00:00 2001
From: "glm-5.1" <glm-5.1@alk.dev>
Date: Mon, 18 May 2026 15:36:38 +0000
Subject: [PATCH] address architecture review findings and add review document

Fixes from architecture review (4 critical, 10 warnings):

Critical:
- Fix selectNode/setNode docs to accurately describe prop-key
  navigation behavior including array support and prop-key writes
- Document RenderContext/Density exported types in host-config
- Resolve ADR dual status ambiguity with clarifying note in README
  (frontmatter status = editorial, body Status = decision)
- Effect types already addressed in prior commit

Warnings addressed:
- Add Fragment re-export note to jsx-runtime section in
  build-distribution
- Document childCtx/transformCtx helper functions in transforms.md
- Document render() accepting non-root UNode in host-config
- Add Value.Hash re-entrancy constraint to reconciler.md
- Add true-passthrough constraint and h('root') special case
  to element-factory constraints
- Add _idCounter bundling caveat note

Review document added at docs/reviews/architecture-review-2026-05-18.md
with full findings, source verification table, and recommendations.
---
 docs/architecture/README.md                   |   2 +-
 docs/architecture/build-distribution.md       |   2 +
 docs/architecture/element-factory.md          |   5 +-
 docs/architecture/host-config.md              |   2 +
 docs/architecture/pointers.md                 |  23 +-
 docs/architecture/reconciler.md               |   1 +
 docs/architecture/transforms.md               |  12 +
 .../reviews/architecture-review-2026-05-18.md | 282 ++++++++++++++++++
 8 files changed, 322 insertions(+), 7 deletions(-)
 create mode 100644 docs/reviews/architecture-review-2026-05-18.md

diff --git a/docs/architecture/README.md b/docs/architecture/README.md
index 61165e8..614c61c 100644
--- a/docs/architecture/README.md
+++ b/docs/architecture/README.md
@@ -116,7 +116,7 @@ last_updated: YYYY-MM-DD
 | `stable` | API contracts are locked. Changes require a review cycle and may warrant an ADR if they affect documented decisions. | → `deprecated` when superseded. → `draft` if a fundamental redesign is needed (rare). |
 | `deprecated` | Superseded by another document. Kept for reference. Links should point to the replacement. | Removed when no longer referenced. |
 
-ADR documents use a separate `Status` field in their body: `Proposed`, `Accepted`, `Deprecated`, or `Superseded`. ADRs never revert from `Accepted`.
+ADR documents use a separate `Status` field in their body: `Proposed`, `Accepted`, `Deprecated`, or `Superseded`. ADRs never revert from `Accepted`. Note that ADR frontmatter (`status: draft`) refers to the **document's editorial status** (is the writing complete?), while the body `Status` refers to the **decision's status** (is this decision finalized?). A `Proposed` decision can have a `draft` document; an `Accepted` decision has a `stable` document.
 
 ## References
 
diff --git a/docs/architecture/build-distribution.md b/docs/architecture/build-distribution.md
index 4b74e76..6f35443 100644
--- a/docs/architecture/build-distribution.md
+++ b/docs/architecture/build-distribution.md
@@ -81,6 +81,8 @@ The `./jsx-runtime` export enables `jsxImportSource` configuration. When a consu
 
 TypeScript and other JSX transforms resolve `jsx`, `jsxs`, and `jsxDEV` from `@alkdev/ujsx/jsx-runtime`. All three are aliases for `h()` — UJSX does not distinguish between static children, dynamic children, or dev mode at the factory level.
 
+The `jsx-runtime` export also re-exports `Fragment` from `h.ts`. This means JSX consumers can import `{ Fragment }` from `@alkdev/ujsx/jsx-runtime` as well as from `@alkdev/ujsx/h` or the barrel export.
+
 ## Dependencies
 
 | Package | Version | Role | Hard/Peer? |
diff --git a/docs/architecture/element-factory.md b/docs/architecture/element-factory.md
index 160a712..5001bca 100644
--- a/docs/architecture/element-factory.md
+++ b/docs/architecture/element-factory.md
@@ -141,10 +141,11 @@ This is documented in the schema architecture ([schema.md](schema.md) — Known
 ## Constraints
 
 - **`h()` is pure** — no side effects beyond the `_idCounter` increment in `createRoot()`. It does not call hosts, subscribe to signals, or mutate external state.
-- **Children are always flat** — `flat(Infinity)` + filter means consumers never receive nested arrays or null/false children from factory output. Hosts and transforms can assume `element.children` is a flat `UNode[]` with no null slots.
+- **`h("root", ...)` is a special case** — the string `"root"` is effectively a reserved type string. When `type === "root"`, `h()` produces a `URoot` (discriminated union with `type: "root"`), not a `UElement`. This is not a host tag — no host will ever receive `"root"` as a `createInstance` tag.
+- **Children are always flat** — `flat(Infinity)` + filter means consumers never receive nested arrays or null/false children from factory output. Hosts and transforms can assume `element.children` is a flat `UNode[]` with no null slots. Note that `true` values are NOT filtered — `{condition}` where `condition` is `true` produces a `true` `UPrimitive` child. Hosts that want `true` to render as nothing should filter it in their `createTextInstance`.
 - **Props are not deep-cloned** — `h()` spreads props shallowly. Nested objects are shared references. Consumers must not mutate element.props and expect isolation.
 - **`Fragment` produces arrays, not elements** — hosts and transforms must handle `UNode[]` return values from component renders. A Fragment does not appear in the tree.
-- **`_idCounter` is module-scoped** — each module instance has its own counter. If multiple copies of UJSX are loaded (e.g., different package versions), roots from different copies may collide on `id` values.
+- **`_idCounter` is module-scoped** — each module instance has its own counter. If multiple copies of UJSX are loaded (e.g., different package versions), roots from different copies may collide on `id` values. Bundle deduplication behavior determines whether copies share the counter.
 - **JSX aliases are identical** — `jsx`, `jsxs`, and `jsxDEV` are the same function. UJSX does not differentiate between them. Dev-mode only features (e.g., source location) are not currently supported.
 
 ## References
diff --git a/docs/architecture/host-config.md b/docs/architecture/host-config.md
index 5a834d1..c0b3d81 100644
--- a/docs/architecture/host-config.md
+++ b/docs/architecture/host-config.md
@@ -190,8 +190,10 @@ The reconciler solves this by maintaining a fiber tree alongside the instance tr
 - **`HostConfig` is the sole host integration point** — all platform-specific logic lives behind this interface. UJSX never calls DOM APIs, Three.js APIs, or any other platform API directly.
 - **`TTag` constrains element types** — hosts declare the tags they support. Attempting to render an unsupported tag is a type error at compile time. At runtime, the host receives any string as `tag` and must handle unknowns.
 - **`render()` is not idempotent** — calling `render()` twice on the same root creates two independent instance trees. It does not update the first tree.
+- **`render()` accepts any `UNode`** — if the node is a `URoot`, its children are mounted directly. If the node is any other `UNode` (element or primitive), it is wrapped in an array and mounted as a single top-level element without a root container.
 - **Function components are synchronous and transparent** — they receive props and children, return a `UNode`, and produce no host instance. The reconciler research discusses how to handle components that return different tree shapes across renders.
 - **`Context` is always present** — `createRoot()` guarantees a `Context` exists on the `Root`. If none is provided, a default is created with `density: "full"`, `target: "markdown"`, and empty `metadata`.
+- **`RenderContext` extends `ContextValue`** — the `Direction` type (`"ujsx→mdast" | "mdast→ujsx" | "ujsx→jpath" | "jpath→ujsx" | "ujsx→hast" | "hast→ujsx"`) and `RenderContext` interface (which adds `direction` to `ContextValue`) are exported from the `context` sub-path. `RenderContext` is primarily used by the transform system to specify conversion direction. `Density` (`"full" | "compact" | "minimal"`) controls rendering granularity for hosts that support adaptive output.
 - **`container` is opaque** — UJSX passes it to `createRootContext` and stores it on `Root`, but never inspects it. The host defines what it means.
 - **Mount is depth-first, post-order** — children are fully constructed before being appended to their parent. Hosts can rely on this ordering invariant.
 
diff --git a/docs/architecture/pointers.md b/docs/architecture/pointers.md
index d12f9a1..2a5f565 100644
--- a/docs/architecture/pointers.md
+++ b/docs/architecture/pointers.md
@@ -64,17 +64,32 @@ Each segment is processed sequentially against the current node:
 | Segment | Resolution | Example |
 |---------|-----------|---------|
 | Numeric (e.g., `"0"`, `"3"`) | `children[index]` on the current `UElement` | Path `["0", "2"]` → root.children[0].children[2] |
-| Non-numeric (e.g., `"title"`) | `props[segment]` on the current `UElement`, if the prop value is an object | Path `["props", "title"]` → root.props.title (if title is an object node) |
+| Non-numeric (e.g., `"title"`, `"items"`) | `props[segment]` on the current `UElement` — if the prop value is a non-null object (including arrays), navigation continues into that value; otherwise returns `undefined` | Path `["items"]` → root.props.items (if items is an object or array) |
 
-A segment that parses as a valid non-negative integer is treated as a children index. Otherwise, it is treated as a prop key. This is a simplified version of RFC 6901 JSON Pointer — no special escaping, no `~` encoding, no `/` separators. Simplicity over generality.
+A segment that parses as a valid non-negative integer is treated as a children index. Otherwise, it is treated as a prop key. If the prop value exists and is a non-null object (which includes arrays, since `typeof [] === "object"`), `selectNode` navigates into it. If the prop value is a primitive (string, number, boolean, null) or absent, `selectNode` returns `undefined`.
+
+This is a simplified version of RFC 6901 JSON Pointer — no special escaping, no `~` encoding, no `/` separators. Simplicity over generality.
 
 ### Early termination
 
 If the current node is not a `UElement` (i.e., it's a `UPrimitive` — a string, number, boolean, or null), `selectNode` returns `undefined` because primitives have no children or props. This prevents runtime errors from navigating into leaf values.
 
-### Non-element props
+### Non-element and array props
 
-When a string segment resolves to a prop value that is not an object (e.g., `props.title` is a string), `selectNode` returns `undefined`. Only object-typed prop values can be navigation targets. This is because `UPrimitive` values (strings, numbers) are terminal — they have no children to navigate into. A prop that holds a `UNode` subtree is an object and can be navigated; a prop that holds a string is a leaf.
+When a string segment resolves to a prop value that is a primitive (string, number, boolean, null), `selectNode` returns `undefined`. Only non-null object prop values — including arrays, since `typeof [] === "object"` — can be navigation targets. This means `props.items` where `items` is an array can be navigated into, but `props.title` where `title` is a string cannot.
+
+`setNode` mirrors this behavior for writes: a non-numeric string segment sets `props[segment] = value`, performing a shallow merge of that key into the element's props. This allows targeted prop updates via path-based navigation.
+
+### setNode prop-key writes
+
+Non-numeric path segments in `setNode` set values into the `props` object:
+
+```typescript
+setNode(root, ["title"], someNode)
+// Produces: { ...rootEl, props: { ...rootEl.props, title: someNode } }
+```
+
+This shallow-merges a key into `props`. The `value` must be a valid `PropValue` (or `UNode`) for the result to remain type-safe.
 
 ## setNode
 
diff --git a/docs/architecture/reconciler.md b/docs/architecture/reconciler.md
index 0214279..dfa0af2 100644
--- a/docs/architecture/reconciler.md
+++ b/docs/architecture/reconciler.md
@@ -244,6 +244,7 @@ Host implementations that currently only support mount-only rendering will start
 - **`key` is a reconciler concern, not a prop** — `key` is extracted by `h()` and never passed to components or hosts. See [ADR-004](decisions/004-key-as-first-class-field.md).
 - **Commit order is parent → child, top-down** — this ensures parent state is consistent when child updates fire.
 - **TypeBox optimizations are incremental** — the reconciler is correct without them. Each optimization layers on top.
+- **`Value.Hash` is not re-entrant** — it uses a global mutable accumulator. It must not be called from within a `computed` or `effect` that is itself triggered by a hash comparison. Hashes must be computed outside reactive computations, during the commit phase.
 - **Phase 5 (flowgraph host configs) belongs in `@alkdev/flowgraph`** — ujsx stays generic. Flowgraph is a consumer, not a modification.
 
 ## Open Questions
diff --git a/docs/architecture/transforms.md b/docs/architecture/transforms.md
index 023fd02..608f34d 100644
--- a/docs/architecture/transforms.md
+++ b/docs/architecture/transforms.md
@@ -93,6 +93,18 @@ The "first match wins" semantics mean that priority and registration order resol
 
 Maps `transform()` over an array, passing each node's index as `ctx.index`. This is a convenience for transforming child lists — it preserves the ancestor stack from `ctx` without requiring callers to manage index tracking.
 
+## Helper Functions
+
+The transform module exports two context factory functions used alongside `TransformRegistry`:
+
+### `ctx<A>(direction, ancestors?, index?, metadata?)`
+
+Creates a `TransformContext` from its arguments. The `direction` parameter is required; the rest default to `[]`, `0`, and `{}` respectively. Exported as `transformCtx` from the barrel (`@alkdev/ujsx/transform`) to avoid name collision with React's `ctx` naming.
+
+### `childCtx<A>(parent, ctx, index)`
+
+Creates a new `TransformContext` with `parent` pushed onto the ancestors stack and `index` set. This is the standard way to descend into a child node during transformation — it preserves the direction and metadata from the parent context while updating traversal state.
+
 ## Direction Definitions
 
 The six directions pair into three bi-directional channels:
diff --git a/docs/reviews/architecture-review-2026-05-18.md b/docs/reviews/architecture-review-2026-05-18.md
new file mode 100644
index 0000000..d73580d
--- /dev/null
+++ b/docs/reviews/architecture-review-2026-05-18.md
@@ -0,0 +1,282 @@
+# Architecture Review: @alkdev/ujsx
+
+**Date**: 2026-05-18  
+**Reviewer**: Architecture Reviewer (subagent)  
+**Scope**: All docs/architecture/ documents + source verification
+
+## Summary
+
+The @alkdev/ujsx architecture documentation is thorough, well-structured, and internally consistent across 15 documents. The docs accurately reflect the current source code implementation in almost all respects. The core architectural story — an HTML-agnostic, signal-driven, host-configurable reactive tree system — is clearly articulated with well-documented design decisions (ADRs). The documentation honestly catalogs known gaps and open questions alongside constraints. Key issues include: an inconsistency in how `selectNode` handles prop navigation vs. how the docs describe it, missing documentation for `childCtx`/`ctx` helper functions in the transform system, an undocumented `RenderContext` type visible in public exports, and several cross-reference paths that use different conventions (relative vs. bare). No document exceeds 265 lines.
+
+## Metrics
+
+- **Critical issues**: 4
+- **Warnings**: 10
+- **Suggestions**: 8
+- **Overall**: needs revision before stabilization
+
+---
+
+## Critical Issues
+
+### 1. `selectNode` prop navigation behavior differs from docs
+
+**Location**: `docs/architecture/pointers.md`, lines 66-68  
+**Issue**: The docs state that a non-numeric segment resolves a prop by checking `typeof propVal === "object"`, implying that only *object-typed* prop values can be navigation targets. However, the source code (`src/core/pointer.ts`, line 41) performs the check:
+
+```typescript
+if (propVal !== undefined && typeof propVal === "object" && propVal !== null)
+```
+
+This means `Array` values (which are `typeof === "object"`) also qualify as navigation targets, but the docs only mention "object nodes" and `UElement`-like structures. More critically, the docs state that navigating into `props.title` (where `title` is a string) returns `undefined`, but the *example path* `["props", "title"]` is misleading — the path `["props"]` would try to resolve `"props"` as a children index or prop key, not traverse into the `props` object. The path resolution algorithm in the docs doesn't match how the source actually works for prop access.
+
+**Recommendation**: Rewrite the path segment resolution table to accurately describe the source behavior:
+- Numeric string → `children[index]`
+- Non-numeric string → `props[segment]`, and if the value is a non-null object (including arrays), navigate into it; otherwise return `undefined`
+- Remove the confusing `["props", "title"]` example, or clarify that `"props"` is a prop key lookup, not a special accessor.
+
+### 2. `setNode` prop-key path behavior undocumented for non-array segments
+
+**Location**: `docs/architecture/pointers.md`, lines 79-109  
+**Issue**: The docs describe `setNode` in terms of children array index updates, but the source code (`src/core/pointer.ts`, lines 51-69) also handles a branch for non-numeric path segments that sets values into the `props` object:
+
+```typescript
+const props = { ...rootEl.props, [head as string]: value };
+return { ...rootEl, props } as UElement;
+```
+
+This means `setNode(root, ["title"], someNode)` would set `root.props.title = someNode`, which is a significant behavior not documented at all. The docs' "Edge cases" section mentions empty paths and paths into primitives, but not prop-key navigation for writes.
+
+**Recommendation**: Add a section documenting non-numeric path segment behavior in `setNode`, explaining that it shallow-merges a key into `props`, and note the constraint that `value` must be a valid `PropValue` (or `UNode`) for the result to remain type-safe.
+
+### 3. `RenderContext` and `Density` types are exported but undocumented
+
+**Location**: `src/mod.ts` line 12; `src/core/context.ts` lines 50-54  
+**Issue**: The barrel export in `mod.ts` re-exports `Density`, `Direction`, and `RenderContext` from `context.ts`. However, `render-context.md` does not exist, and neither `Density` nor `RenderContext` is documented anywhere in the architecture docs. The `context.ts` source file defines a `RenderContext` interface that extends `ContextValue` with a `direction` field, and a `Density` type with three string literals. The host-config doc mentions the `Context` class briefly (line 79), but does not document `Density` or `RenderContext`.
+
+**Recommendation**: Either add documentation for `RenderContext` and `Density` in a new or existing architecture doc, or add a constraint note explaining that these are exported for downstream consumer use and are not core to UJSX's internal architecture. At minimum, document how `Density` is intended to be consumed by hosts.
+
+### 4. ADR frontmatter `status: draft` contradicts ADR body `Status: Proposed`
+
+**Location**: All ADRs (`decisions/001` through `decisions/005`)  
+**Issue**: Each ADR file has YAML frontmatter with `status: draft`, which follows the document lifecycle convention defined in `README.md` (lines 104-118). However, each ADR also has an internal `**Status**: Proposed` field in its body. The README states: "ADR documents use a separate `Status` field in their body" and lists statuses as `Proposed`, `Accepted`, `Deprecated`, or `Superseded`. This creates two status fields that could diverge — the frontmatter says `draft` (a document lifecycle status) while the body says `Proposed` (an ADR decision status). If an implementer reads `status: draft` and thinks "this decision isn't finalized," they may ignore the ADR, even though ADRs document decisions that have already been made.
+
+**Recommendation**: Clarify the relationship between the two status fields. Either: (a) rename one to avoid confusion (e.g., use `doc_status` in frontmatter), (b) add a note in the README explaining that frontmatter `status` is the document's editorial status while the body `Status` is the decision's status, or (c) unify them. This is critical because the frontmatter convention is supposed to govern whether implementation should begin, and the mismatch could cause confusion about whether these decisions are stable enough to implement against.
+
+---
+
+## Warnings
+
+### 1. `Fragment` JSX runtime export missing from build-distribution docs
+
+**Location**: `docs/architecture/build-distribution.md`, lines 58-59 (sub-path exports list)  
+**Issue**: The sub-path exports table lists `jsx-runtime` but documents it as mapping to `src/core/jsx-runtime.ts`. The actual `jsx-runtime.ts` file re-exports `Fragment` in addition to `jsx`, `jsxs`, and `jsxDEV`. The `element-factory.md` documents `Fragment` as a factory function, but `build-distribution.md` doesn't mention that `Fragment` is available from the jsx-runtime sub-path. This could confuse consumers who expect Fragment to be importable from the JSX runtime but can't find it documented there.
+
+**Recommendation**: Add a note to `build-distribution.md` that `jsx-runtime` also exports `Fragment`.
+
+### 2. `childCtx` and `ctx` (as `transformCtx`) are public exports but undocumented as architecture concepts
+
+**Location**: `src/mod.ts` line 22; `src/transform/registry.ts` lines 44-63  
+**Issue**: The barrel export re-exports `childCtx`, `matchesSchema`, and `ctx as transformCtx` from the transform module. `matchesSchema` is documented in `transforms.md`, but `childCtx` and `ctx` (exported as `transformCtx`) are not documented anywhere in the architecture docs. These are convenience functions for creating transform contexts and pushing ancestors. Consumers importing from `@alkdev/ujsx/transform` would encounter them with no architectural guidance.
+
+**Recommendation**: Add documentation for `childCtx` and `ctx`/`transformCtx` in `transforms.md`, explaining their purpose as factory functions for creating `TransformContext` instances.
+
+### 3. The `render()` method processes `URoot` children, but docs and source differ in detail
+
+**Location**: `docs/architecture/host-config.md`, lines 96-103; `src/host/config.ts`, lines 97-101  
+**Issue**: The docs describe `render()` as calling `mountNode` recursively, but the actual source code does:
+
+```typescript
+const payloadChildren = isURoot(node) ? (node as URoot).children : [node];
+for (const child of payloadChildren) { mountNode(child, undefined); }
+```
+
+This means if a non-root node is passed to `render()`, it wraps it in an array and mounts it as a single top-level node without a URoot container. The documentation in host-config.md's "Mount Pipeline" section describes `URoot` nodes as transparent containers, but doesn't describe this `render()` behavior of accepting non-root nodes. The docs describe `render(node)` as "Mount the tree" without noting that non-root nodes are wrapped.
+
+**Recommendation**: Document that `render()` accepts any `UNode`, and if the node is not a `URoot`, it mounts the node directly as a single top-level element without a root container.
+
+### 4. `URoot` type casting in `h()` source differs from docs
+
+**Location**: `src/core/h.ts`, lines 9-15; `docs/architecture/element-factory.md`, lines 20-28  
+**Issue**: The docs describe `h()` as returning `UElement | URoot` depending on the type argument. The source code matches this. However, the source uses `as URoot` cast for the root case and `as UElement` for the default case. The docs don't mention this casting, which is fine, but the docs also don't mention that when `type === "root"`, the `type` argument of `h()` is widened from `UType` (which includes `string | ComponentFn`) to the `"root"` literal. This means `h("root", ...)` has special runtime behavior that bypasses the normal string-type element creation path.
+
+**Recommendation**: Add a constraint or note that `h("root", ...)` is a special case that produces a `URoot` rather than a `UElement`, and that the string `"root"` is effectively a reserved type string.
+
+### 5. Research cross-references use inconsistent formats
+
+**Location**: Multiple docs  
+**Issue**: Cross-references to research documents use different path formats across documents:
+- `README.md` uses `../research/reconciler/01-reactive-host-bridge.md` (relative from architecture/)
+- `schema.md` uses `docs/research/reconciler/00-KEY-FIELD-DESIGN.md` (project-relative)
+- `reactive-layer.md` uses `docs/research/reconciler/01-reactive-host-bridge.md` (project-relative)
+- `build-distribution.md` uses forward references without paths (just names)
+
+Since all architecture docs live in `docs/architecture/`, project-relative paths like `docs/research/...` will not resolve correctly from the architecture directory. Only the `../research/...` style works for relative links from architecture docs.
+
+**Recommendation**: Standardize all research cross-references to use consistent relative paths from the architecture docs directory (e.g., `../research/reconciler/01-reactive-host-bridge.md`).
+
+### 6. Open questions about `Value.Hash` non-reentrancy are not addressed in constraints
+
+**Location**: `docs/architecture/reconciler.md`, lines 196-199; section "Constraints"  
+**Issue**: The reconciler doc documents that `Value.Hash` uses a global mutable accumulator and is "not re-entrant" — you cannot call it from within a `computed` or `effect` that is itself triggered by a hash comparison. This is listed as a constraint on `Value.Hash` but not in the main "Constraints" section of the document. It's buried in a subsection. This is a significant constraint that could cause subtle bugs if an implementer skims the constraints section.
+
+**Recommendation**: Add this to the main "Constraints" section of `reconciler.md`: "Value.Hash is not re-entrant — it must not be called from within a reactive computation (computed/effect)."
+
+### 7. `true` not being filtered from children is a potential footgun not highlighted in element-factory constraints
+
+**Location**: `docs/architecture/element-factory.md`, lines 36-37  
+**Issue**: The docs note that `true` is not filtered (unlike `false`), and state: "This is consistent with how React treats `true` in JSX (it renders nothing in DOM but is not stripped at the factory level)." However, this means `<div>{condition}</div>` where `condition` is `true` will produce a `UElement` with `true` as a `UPrimitive` child, which most hosts will need to handle explicitly. This is only mentioned in the child normalization section, not in the Constraints section. For a universal library, this behavior could surprise hosts that expect `true` to be treated like `false`/`null`.
+
+**Recommendation**: Add to the Constraints section: "`true` passes through `h()` as a UPrimitive child. Hosts that handle `true` differently from other primitives should filter it in their `createTextInstance`."
+
+### 8. `createRoot` _idCounter is shared across all callers — no module isolation documented
+
+**Location**: `docs/architecture/element-factory.md`, lines 56-61  
+**Issue**: The docs state: "The counter is a module-level variable (`let _idCounter = 0`). It is: **Not thread-safe** — JavaScript is single-threaded, and root creation is not a concurrent operation." While this is technically correct, the more important concern is that any bundler that deduplicates the module will share the counter across all consumers. If two different packages both depend on `@alkdev/ujsx`, they may or may not share the counter depending on bundler behavior. This could lead to unexpected `root_3` from package A conflicting with `root_3` from package B.
+
+**Recommendation**: Expand the constraint note to mention that bundled consumers may or may not share the counter, and that `id` values should be treated as unique within a single module instance only, not globally unique. Consider suggesting `createRoot(uniquePrefix + counter)` for disambiguation.
+
+### 9. Missing performance/scalability discussion
+
+**Location**: Overall — no document addresses performance characteristics  
+**Issue**: No architecture document discusses performance targets, scalability considerations, or benchmarks. The reconciler doc mentions that "90% of updates bypass tree diffing" and that fiber trees are "cheap to update," but there are no quantified performance targets (latency p50/p99, throughput, memory budget). For a library that positions itself as suitable for workflow DAGs and desktop UI rendering, the absence of performance context means implementers can't validate that the architecture meets their needs.
+
+**Recommendation**: Consider adding a "Performance Model" section to one of the core docs (or the README) that outlines expected performance characteristics: tree sizes the system is designed for, update propagation latency expectations, and memory usage patterns. This doesn't need hard benchmarks yet, but should establish design targets (e.g., "designed for trees of 10-10,000 nodes" or "signal propagation should complete in <1ms for prop updates").
+
+### 10. No error handling strategy documented
+
+**Location**: Overall  
+**Issue**: No architecture document discusses what happens when things go wrong at the system level. The host-config doc has an open question about function component error propagation (line 206), and the transform doc notes that `transform()` throws if no rule matches (line 35). But there's no overarching error handling strategy: When should UJSX throw vs. emit an error event vs. silently no-op? What happens if `createInstance` throws? What happens if a signal computation throws?
+
+**Recommendation**: Add an "Error Handling" section to the README or create a separate `error-handling.md` doc that establishes UJSX's error philosophy: which errors are programmer errors (throw), which are operational (emit event), and which are silently handled (no-op with logging).
+
+---
+
+## Suggestions
+
+### 1. Consider documenting the `isUElement` null check
+
+**Location**: `docs/architecture/schema.md`, line 127  
+The type guard table documents `isUElement` as checking `typeof === "object" && "type" in node && "props" in node && "children" in node && node.type !== "root"`. The source code (`src/core/schema.ts` line 66) adds `node !== null` to the check. While `null` would fail the `"type" in node` check, explicitly documenting the null guard would make the type guard specification complete and help implementers writing host-specific guards.
+
+### 2. Add a naming convention note for `Direction` string values
+
+**Location**: `docs/architecture/transforms.md`, lines 99-106  
+The `Direction` type uses arrow characters (`ujsx→mdast`) in string literals. These are defined in `context.ts` and also appear in `RenderContext`. This is unconventional and could cause issues with IDE autocomplete or linting rules that warn on non-ASCII strings. Consider documenting why Unicode arrows were chosen, or noting that the convention should be maintained consistently.
+
+### 3. Document the `UjsxEnvelope` convenience type
+
+**Location**: `src/core/events.ts`, lines 28-31; `docs/architecture/events.md`  
+The source exports a `UjsxEnvelope` type alias that parameterizes `EventEnvelope` with `UjsxEventMap` — essentially `EventEnvelope<TKeyof, UjsxEventMap[TKeyof]>`. This convenience type is not documented in `events.md`. While not critical, it's a public export that consumers will see.
+
+### 4. Consider adding a "Quick Start" or "Concepts" doc for new readers
+
+**Location**: Overall  
+The architecture docs are detailed and reference-heavy. A new reader would benefit from a single-page "Concepts" overview that introduces UNode, h(), signals, and HostConfig in 50 lines before directing to the detailed docs. The README partially serves this role but focuses on current-state/gaps rather than a conceptual introduction.
+
+### 5. Consider documenting the `Context` class more fully
+
+**Location**: `docs/architecture/host-config.md` (partial); `src/core/context.ts`  
+The `Context` class has `get()`, `set()`, `subscribe()`, `fork()`, and a `signal` getter, but these are only briefly mentioned in host-config.md. The class is exported from the barrel and appears in the context export. A short section explaining its reactive context model would help consumers who import it directly.
+
+### 6. Standardize ADR status tracking
+
+**Location**: All ADR documents  
+All five ADRs have `Status: Proposed`, but the architectural decisions they describe are already driving implementation (e.g., ADR-003's choice of Preact signals is clearly implemented). Consider advancing ADR statuses to `Accepted` for decisions that are already in production code. The `Proposed` status on implemented decisions undermines the ADR's authority.
+
+### 7. Consider a centralized glossary
+
+**Location**: Overall  
+Terms like "fiber," "host," "instance," "primitive," and "reconciler" are defined in individual docs but would benefit from a centralized glossary. For example, "instance" means different things in different contexts (host instance vs. UElement instance), and readers may miss the distinction.
+
+### 8. Add line counts or complexity indicators to file references
+
+**Location**: All source references  
+Each architecture doc references its source file (e.g., "Source: `src/core/schema.ts`"). Adding approximate line counts (e.g., "Source: `src/core/schema.ts` (~80 lines)") would help readers calibrate their expectations about implementation complexity.
+
+---
+
+## Strengths
+
+1. **Honest gap documentation**: Every document explicitly catalogs "Known Gaps," "Open Questions," and "Constraints." This is exceptional practice — implementers can immediately see what doesn't work yet and what decisions are still pending. The README's "Known gaps" section with cross-references to relevant docs and ADRs is particularly well done.
+
+2. **Decision records (ADRs)**: All five ADRs follow a consistent format (Context, Alternatives Considered, Decision, Consequences) and document not just what was decided but *why*. The alternatives sections are substantive — they explain why each rejected option was considered and why it was rejected, not just that it was.
+
+3. **Architecture-consistent language**: The documents use consistent terminology throughout. A "UNode" always means `UPrimitive | UElement | URoot`, a "host" always refers to a `HostConfig` implementation, a "fiber" always means the reconciler's internal node type. This consistency makes cross-document references meaningful.
+
+4. **Clear separation of concerns**: The docs clearly delineate what each layer owns. The reactive layer owns signals, the host layer owns instances, the transform layer owns conversion, and the pointer layer owns targeted read/write. No layer bleeds into another's responsibilities without explicit bridging (which is documented as a planned reconciler feature).
+
+5. **"The tree is the truth" principle**: The README's stated core principle ("The tree is the truth. Hosts are interpreters.") is consistently reflected across all docs. Every feature is motivated in terms of what it means for the tree, not for any specific host.
+
+6. **Practical consumer context**: The README's "Consumer Context" section (Flowgraph, Desktop UI, OpenCode Plugin) grounds the architecture in real use cases, making it easier to evaluate trade-offs with concrete scenarios in mind.
+
+7. **Document size discipline**: No document exceeds 265 lines (reconciler.md). Each document covers exactly one topic. The README provides navigation. This makes the documentation approachable and scannable.
+
+---
+
+## Verification: Source vs. Documentation
+
+### Accuracies (docs match source)
+
+| Aspect | Doc | Source | Status |
+|--------|-----|--------|--------|
+| `UJSX` TypeBox Module schema | schema.md | src/core/schema.ts | ✅ Match |
+| TypeScript types (UPrimitive, PropValue, etc.) | schema.md | src/core/schema.ts | ✅ Match (except `key` — see below) |
+| `key` not yet on UElement | schema.md ("Known Gaps") | src/core/schema.ts (no `key` field) | ✅ Consistently documented as not yet implemented |
+| `isUElement` discriminator logic | schema.md | src/core/schema.ts:66 | ✅ Match (note: null guard in source not in docs — see suggestion) |
+| `h()` factory behavior | element-factory.md | src/core/h.ts | ✅ Match |
+| `createRoot` auto-ID counter | element-factory.md | src/core/h.ts:3,24-29 | ✅ Match |
+| `Fragment` returns `UNode[]` | element-factory.md | src/core/h.ts:43-44 | ✅ Match |
+| `jsx`/`jsxs`/`jsxDEV` aliases | element-factory.md | src/core/h.ts:47-49 | ✅ Match |
+| `ReactiveNode` interface | reactive-layer.md | src/core/reactive.ts:4-8 | ✅ Match |
+| `reactiveComponent` and `reactiveElement` behavior | reactive-layer.md | src/core/reactive.ts:10-47 | ✅ Match |
+| `ReactiveRoot` class | reactive-layer.md | src/core/reactive.ts:49-89 | ✅ Match |
+| `ReactiveRoot.render()` overwriting `renderDisposer` | reactive-layer.md ("Known Gaps") | src/core/reactive.ts:74 | ✅ Documented as a known hazard |
+| `dispose: () => {}` no-op | reactive-layer.md ("Known Gaps") | src/core/reactive.ts:25,46 | ✅ Consistently documented |
+| `HostConfig` interface | host-config.md | src/host/config.ts:5-30 | ✅ Match |
+| `Root` interface | host-config.md | src/host/config.ts:32-39 | ✅ Match |
+| `createRoot()` behavior | host-config.md | src/host/config.ts:41-109 | ✅ Match |
+| Mount-only rendering | host-config.md ("Known Gaps") | src/host/config.ts (no re-render logic) | ✅ Match |
+| `unmount()` stub behavior | host-config.md, lifecycle.md | src/host/config.ts:105-108 | ✅ Match |
+| `Direction` type values | transforms.md | src/core/context.ts:50 | ✅ Match |
+| `TransformRule` interface | transforms.md | src/transform/registry.ts:14-21 | ✅ Match |
+| `TransformRegistry` methods | transforms.md | src/transform/registry.ts:23-42 | ✅ Match |
+| `EventEnvelope` interface | events.md | src/core/events.ts:1-5 | ✅ Match |
+| `PubSubLike` interface | events.md | src/core/events.ts:7-17 | ✅ Match |
+| `UjsxEventMap` type | events.md | src/core/events.ts:19-26 | ✅ Match |
+| `createPubSubEmitter` function | events.md | src/core/events.ts:33-42 | ✅ Match |
+| `proxyEventEmitter` function | events.md | src/core/events.ts:45-51 | ✅ Match |
+| `ValuePointer` class | pointers.md | src/core/pointer.ts:5-29 | ✅ Match |
+| `selectNode` function signature | pointers.md | src/core/pointer.ts:31-49 | ✅ Match (behavior differs — see critical issue #1) |
+| `setNode` function signature | pointers.md | src/core/pointer.ts:51-69 | ✅ Match (behavior differs — see critical issue #2) |
+| Package dependencies | build-distribution.md | package.json | ✅ Match |
+| Sub-path exports | build-distribution.md | package.json exports field | ✅ Match (except Fragment in jsx-runtime — see warning #1) |
+| Barrel exports in mod.ts | build-distribution.md | src/mod.ts | ⚠️ Partial — see critical issue #3 |
+| `Context` class defaults | host-config.md (line 79, line 194) | src/core/context.ts:11-15 | ✅ Match |
+| `Date.now()` event IDs | host-config.md, events.md | src/host/config.ts:57,75,103 | ✅ Consistently documented as placeholder |
+
+### Discrepancies
+
+| Aspect | Doc Says | Source Does | Severity |
+|--------|----------|-------------|----------|
+| `selectNode` prop navigation | Only object-typed props are navigable | Source also navigates arrays (which are `typeof === "object"`) | **Critical** — doc is incomplete |
+| `setNode` prop-key writing | Not documented | Source handles non-numeric segments by setting `props[segment] = value` | **Critical** — behavior gap |
+| `RenderContext` and `Density` | Not documented | Exported from mod.ts and source | **Critical** — public API undocumented |
+| ADR dual status | Frontmatter `draft` vs body `Proposed` | Both present, semantics unclear | **Critical** — governance confusion |
+| `Fragment` in jsx-runtime | Not in exports doc | Source re-exports Fragment from jsx-runtime | **Warning** — minor coverage gap |
+| `childCtx`, `transformCtx` | Not in transforms.md | Public exports from registry.ts | **Warning** — public API gap |
+| `isUElement` null check | Not in docs | Source includes `node !== null` | **Suggestion** — minor |
+
+---
+
+## Recommendations
+
+1. **Address all four critical issues** before marking the architecture documents as `stable`: fix `selectNode`/`setNode` docs to match source, document `RenderContext`/`Density`, and resolve the ADR status ambiguity.
+
+2. **Standardize cross-reference formats** across all docs to use consistent relative paths. Pick one format and apply it everywhere.
+
+3. **Advance ADR statuses** from `Proposed` to `Accepted` for decisions that are already driving implementation (particularly ADR-002, ADR-003, and ADR-004 whose decisions are reflected in current source code).
+
+4. **Add error handling philosophy** as a cross-cutting concern. Even a brief statement like "UJSX throws for programmer errors (invalid arguments) and no-ops for operational errors (invalid paths in selectNode)" would help implementers.
+
+5. **Consider performance targets** for the reconciler design. The current docs describe algorithms without stating what "good enough" means. This will make it hard to evaluate whether the reconciler implementation meets its design goals.
\ No newline at end of file