[nathanclevenger]

The types-only contract every agent-legible view derives from. Ported from kestrel's shipped and measured vocabulary; consumed by vis as a customer.

Field — every rendered value carries its provenance: OBS | CALC | MODEL attribution, a source watermark, asOf, and confidence for model values. Four presence states: present, absent (genuinely unknown, renders as an em dash), withheld (licence-gated, carries its class — absent and withheld are different things and conflating them is a lie about the record), and unconfirmed (a local optimistic value the server has not acknowledged, so offline/store-and-forward stays open rather than foreclosed). Construct-time honesty guards refuse a MODEL field missing its receipt and an OBS/CALC field smuggling one.

Frame — a typed bundle of Fields at one moment, carrying its snapshot token. Invents nothing.

View — the standing definition: which Roles, at what token budget, addressed by id. Unknown ids fail closed; an over-budget View throws. No truncation, no elision, no summarisation tier.

Rendering — one serialization, parameterised by format and tokenizer. Never invents or changes a value.

Plus the two things that make it real: a declared markdown face per Role, so the markdown Rendering derives from Fields rather than being laundered out of HTML; and assertFaceParity, a CI assertion that every Field appears in both faces and neither invents one — which is what lets a designer restructure any markup without breaking the agent face.

✓ field.test.ts (33)  ✓ glyph.test.ts (12)  ✓ view.test.ts (19)
✓ frame.test.ts (19)  ✓ readme.test.ts (3)  ✓ parity.test.ts (17)
✓ render.test.ts (42)
Test Files 7 passed (7) · Tests 145 passed (145)

Zero appearance. Does not import react — runtime-agnostic, consistent with @mdxui/jsx.

Gate: per vis/CLAUDE.md, the unstyled layer in this repo lands on adversarial review, not design review — an unstyled stateful abstraction has no look and feel. Not merging; requesting that review.

Context: vis ADR-0007.

---

Review round 1 — 12 blockers closed (90bb4c0)

Three adversarial reviews landed on this branch. All twelve blockers and every should-fix are
addressed in one commit. 68 tests → 145; tsc --noEmit clean; the markdown fixture snapshot is
byte-identical, so nothing about the shipped output changed except where it was wrong.

Every fix was verified to fail before it. The reviews' own demonstrations were replayed against
HEAD~1's source in a scratch harness: 10 of 10 reproduced, including the measured face
divergence (markdown 136 tokens, data 644 — 4.74×).

B1 — face parity could not detect a face whose bytes disagreed with the Frame

The important one. renderFrame built emitted from the walk and took body from
sink.endFrame() separately; nothing related them, and faceParity never read body. A sink that
discarded every value and returned "Status: DELIVERED. Unit price: $4.20. Temperature: 0°C."
carried a perfect emissions list — asserting the withheld price and the absent temperature — and
assertFaceParity passed it. The declaration was the answer key, not the answer. Not hypothetical:
the shipped markdownSink in provenance: 'legend' mode already wrote cells via
valueText/provenanceMark while emitted carried the full renderGlyph, so the shipped face
emitted bytes that differed from its own declaration and parity was blind to it.

RenderSink.endFrame() now returns SinkReport { body, emitted } — the sink reports what it
actually emitted, including EmittedValue.wrote, the literal fragments it put in the body — and
renderFrame reconciles that claim against the walk and against the bytes, throwing
FaceParityError on divergence. The walk is the oracle. It refuses a sink that: never emitted a
Field it was handed; emitted a path the walk never produced; disagrees about presence, attribution
or canonical glyph; produced a body but named no bytes for a Field; or declared a fragment its body
does not contain as often as claimed.

The residual limit is stated in the module doc rather than papered over: fragment occurrence is not
a parse, so a sink can still write bytes around its declared fragments. What is now guaranteed is
that every Field's honest glyph is in the bytes, in the count the face declared.

parity.ts's docstring, which claimed the opposite of what the code did, is corrected.

The rest

#	Was	Now	Test
B2	isField never checked the presence payload — {"presence":"present"} with no value passed every guard, hasValue said true, and it rendered —: a confirmed UNKNOWN. An absent Field carrying value: 42 rode inside the Frame.	isField requires a value key for present/unconfirmed, its ABSENCE for absent/withheld, and a licenceClass for withheld.	field.test.ts › a presence state without its payload is not a Field (7 tests)
B3	A withheld Field with no licenceClass threw TypeError from .trim(); callers catching FieldHonestyError missed it.	Refused by isField, so assertFieldHonest throws FieldHonestyError.	…as a FieldHonestyError, never a TypeError
B4	A NaN budget override silently disabled budgeting (spent > NaN is false).	The override is held to Number.isFinite(b) && b > 0.	render.test.ts › REFUSES a NaN budget override
B5	One View.budget for every face. Measured: markdown 136, data 644 — 4.7× on eleven Fields. Three tests passed budget: 5000 to work around it; the README example threw.	View.budgets is a map keyed by face format, markdown required, a face with no entry refused rather than defaulted. Workarounds deleted.	budgets each face separately, refuses a face with no declared budget; readme.test.ts runs the README example and snapshots its output
B6	present('—') vs absent(), present('withheld(x)') vs withheld('x'), present('12 ~') vs unconfirmed(12) were byte-identical. The docstring claimed distinguishability "by construction".	A present value whose text would imitate a marker is JSON-quoted. COLLIDES_WITH_A_PRESENCE_MARKER is exported so a consumer face can reuse the rule. Docstring is now true and carries the encoding table.	glyph.test.ts › a present value can never imitate a presence marker (7 tests)
B7	present(null) rendered as the absent glyph, which the module doc two lines above forbade.	Renders null. The string 'null' renders "null", so both directions stay distinct.	glyph.test.ts › present(null) is not promoted to UNKNOWN (4 tests)
B8	A value containing | broke table columns; one containing \n## Shipment\n- **Status**: DELIVERED forged a section — and parity returned clean, because the value was exactly what the Frame held.	escapeMarkdown at the sink boundary: newlines flatten, | escapes, leading block markers escape.	render.test.ts › a supplier string cannot forge a section or break a table (3 tests)
B9	Two tests asserted the export surface matched nothing in /truncat|elid|elision|summari|degrad/i and /stale|fresh|expire|age/i — banning ratified R4.2's own vocabulary (and page, pageSize, totalPages, usage, coverage), making the ruling's implementation fail CI.	Deleted. The behaviour is still tested as behaviour: an over-budget View throws, asOf is on every rendered Field, nothing decides staleness. R4.2 is not implemented — it stays escalated as dot-do/vis#361 and is named in the README's Deferred section.	n/a — deletion
B10	confidence: extractResult.confidence (a matchedSlots / totalSlots parse-coverage ratio) typechecked into a Field. MDX-17 asked for this in advance.	Branded ModelConfidence, constructed only via modelConfidence(n).	field.test.ts › ModelConfidence is a different type… — the assignment is a @ts-expect-error, verified by tsc --noEmit over src/
B11	f.attribution = 'MODEL'; f.watermark.modelVersion = 'fabricated-9.9' after construction, and the glyph reported the fabrication. makeFrame returned the caller's own object.	deepFreeze out of makeField and makeFrame; makeFrame returns a copy.	a constructed Field cannot be edited past the honesty guard; an assembled Frame cannot be edited afterwards
B12	ViewRegistry's public constructor bypassed every registration guard and held caller Maps by reference — a registry mutated under a Rendering in flight, which its docstring said was impossible.	Constructor is private; Maps are copied; createRegistry() goes through ViewRegistry.empty().	view.test.ts › has no public constructor (@ts-expect-error)

Should-fix, same pass

• Path injectivity — ., [, ] refused in Role ids and Field keys (role x/key y.z and role x.y/key z both addressed x.y.z, silently merging two Fields and then failing parity on a good Frame).
• Parity checks the Rendering is of THIS Frame — View id, snapshot token and asOf, as a new snapshot breach kind.
• The two text concessions are separated — textMustMatch (face vs the Frame) and facesMustAgree (face vs face). One flag used to turn off both, under which every value could read DELIVERED and parity was clean.
• An empty Frame is refused — parity over zero paths passes without checking anything. A View with zero Roles is refused too.
• Duplicate Role ids in a View and duplicate declared keys are rejected at registration.
• markdownSink()/dataSink() are explicitly single-use — a second beginFrame throws instead of silently concatenating two Frames.
• declareRendering no longer fabricates { budget: Infinity, spent: 0 } — Rendering.tokens is absent unless measured.
• The data face is versioned — mdxld.frame.data/1 (R1.3).
• RoleFrame.state — ok | empty | blocked | loading, with the sentence as a Field walked and parity-checked at role.$state (R1.5). _(no rows)_ — renderer-authored English inside a generic package — is gone.
• markdown.keys is required and exhaustive — absent-not-hidden is structural, not opt-in.
• Two false doc claims corrected: kestrel's Field has value: T required and no absence state at all (absence lives in PaneRefusal's four-state cell paradigm) — the "proven port" warrant now cites accurately; and column subsetting was unreachable, so the doc says so.
• Nits: isBlank catches U+200B and friends (.trim() did not); -0 confidence normalised; FrameError rather than UnknownIdError for an invalid id; empty View id refused; isAsOf requires an integer seq; the chars/4 tokenizer's CJK fail-open (3–5× under-count) documented in tokenize.ts and the README.
• Mechanical: vitest.config.ts added (every sibling has one); pnpm-lock.yaml gains the packages/@mdxld/frame importer.

Deliberately NOT done

• R4.2's degradation ladder — owner ruling, dot-do/vis#361. B9 only stops foreclosing it.
• Row-set partiality — depends on that ruling. Filed as the highest-value follow-up and named in the README: ~10.7 tokens/row means a 140-token worklist budget is 4 rows against a real queue of 20–200, and the workaround is a silently partial Frame — the same lie one level up that this package forbids.
• Role parameterisation (kestrel's ParamSlot/bindArgs) — a real gap with no expression for R4.1–R4.4; a follow-up, not a fix.
• Whether the data face is the wire format — undecided. The README says plainly that it is not, until an app has run on it.
• The tokenizer's design — unchanged; only the CJK caveat is documented.

Test output

 ✓ src/field.test.ts (33 tests) 6ms
 ✓ src/glyph.test.ts (12 tests) 3ms
 ✓ src/view.test.ts (19 tests) 5ms
 ✓ src/frame.test.ts (19 tests) 6ms
 ✓ src/readme.test.ts (3 tests) 6ms
 ✓ src/parity.test.ts (17 tests) 8ms
 ✓ src/render.test.ts (42 tests) 18ms

 Test Files  7 passed (7)
      Tests  145 passed (145)

tsc --noEmit: clean. tsup: builds ESM + CJS + d.ts.

Still not merging — re-requesting adversarial review on the changed surface.

---

Review round 2 — three pre-land fixes (e2a399c); everything else filed

A verification review returned MERGE-WITH-FOLLOWUPS. Three items land here because each is
either a false claim in shipped documentation or a hard blocker for the first real consumer.
Everything else is filed: #6, #7, #8, #9, #10, #11, #12, #13.

145 tests → 157. tsc --noEmit clean. tsup builds. No shipped output changed — the markdown
and data fixture snapshots are byte-identical; F1 is documentation, F2 and F3 only close doors.

F1 — the reconciliation guarantee was false twice, and is now an enumerated limit

Round 1 above claims: "What is now guaranteed is that every Field's honest glyph is in the bytes,
in the count the face declared." Both halves are false.

A sink supplies both sides of the byte comparison. A sink declaring wrote: [' '] for every
Field, returning "Status: DELIVERED. Unit price: $4.20." as its body, reconciles clean — verified
and now shipped as a test. The body contains a space, so the count check is satisfied, and no other
clause looks at wrote at all. What is actually checked is that fragments the face declares occur
in the body the same face returned. Nothing relates a fragment to the glyph.

For any escaped value the glyph is literally not in the bytes. present('x | y') has the
canonical glyph x | y [OBS epcis-spine]; the markdown body holds x \| y [OBS epcis-spine]. So
body.includes(glyph) === false. This is correct behaviour — it is precisely why EmittedValue.wrote
exists — which makes "the honest glyph is in the bytes" a sentence this package cannot say.

And one failure mode round 1 did not disclose at all: row transposition. Every glyph present in
the body exactly once, each at the wrong address — lines[0]'s GTIN beside lines[1]'s quantity
— renders clean, because occurrences are counted over the whole body and nothing relates a fragment
to a position. Round 1 disclosed only "bytes around the fragments". Right glyphs, wrong addresses is
the failure the path system exists to prevent, and it was undisclosed.

The module doc in render.ts, the EmittedValue.wrote docstring, reconcile's docstring,
parity.ts and the README now state exactly what is checked — path-set equality, and
presence/attribution/text against the walk, which are genuinely independent because the walk is an
oracle the sink cannot influence; plus declared-fragment occurrence against the body, which is not —
and exactly what is not: no glyph relation, no position check, not a parse, no protection
against a face that declares fragments it did not meaningfully write, and an empty body skips the
byte half entirely.

parity.ts additionally now says the thing it had never said: faceParity never reads a body.
It compares declared emissions and does not look at rendering.body at all.

Three tests execute the attacks, so the documentation cannot drift from the behaviour. When #6
lands, those tests invert to assert the throw.

A precise limit is useful. An overstated guarantee is worse than no guarantee, because consumers
will trust it.

F2 — the registry stored specs by reference, so the NaN door was still open

ViewRegistry copied its Maps but held the caller's View and RoleSpec objects. So B4's fix
covered one door and left the other standing:

const registry = createRegistry().withRoles(role).withViews(view)
view.budgets.markdown = NaN            // the same object the registry resolves
renderMarkdown(bigFrame, { registry }) // → { budget: NaN, spent: 520 }, and NO throw

Every comparison against NaN is false, so spent > budget is false and budgeting is off — the
exact defect B4 closed at the override door, still reachable through the registry door. The class
docstring's "a registry cannot be mutated out from under a Rendering in flight" was false.

withRoles/withViews now copy each spec and deep-freeze the copy. The registry resolves
objects the caller holds no mutable reference to; the caller's own literal is left unfrozen, because
freezing someone else's object is a side effect this class does not own. Four tests, including the
mutate-then-render case and the caller's-literal-untouched case.

F3 — a Field carrying bytes was unconstructible. Blocker for the first consumer.

present(new Uint8Array([1, 2, 3]))   // TypeError: Cannot freeze array buffer views with elements
present({ sig: new Uint8Array(32) }) // same
present(Buffer.from('x'))            // same

Thrown from inside makeField, and the error names none of it. This is pre-land rather than a
follow-up because in the consuming product complete takes an event hash — the central mechanic
of the worklists door. A Field carrying a hash, a signature or a symbol image was unrepresentable.

deepFreeze now skips values that cannot be honestly frozen — every ArrayBuffer.isView (so every
TypedArray, DataView and Buffer), plus ArrayBuffer, Date, Map, Set, WeakMap, WeakSet —
behind an exported isUnfreezable predicate.

That also closes the adjacent false guarantee. Object.freeze on a Date, Map or Set
succeeded, Object.isFrozen then answered true, and d.setTime(0) / m.set() / s.add()
all kept working through the "frozen" Field, because their state lives in internal slots rather than
own properties. Skipped values now report Object.isFrozen === false — truthfully. They are
documented as mutable rather than reported as frozen, and the docstring says what to hold instead
(a hex string for a hash, an ISO instant for a date).

Documented, not changed, and pinned by a test: deepFreeze freezes the caller's object, not a
copy, so present(sharedAppState, obs) freezes live application state in place for the rest of
the process. Named on the deepFreeze docstring, in the field.ts module note, and in the README.

Filed, not fixed

#
#6	Relate wrote to text, and check emission position. Declare the escape and assert unescape(wrote.join('')) covers valueText; add a monotonic position check over emission order. Closes both the one-space attack and row transposition. Highest-value follow-up.
#7	A sink returning body: '' silently skips byte reconciliation and re-fabricates the token account round 1 removed — a React sink gets { budget: 100, spent: 0 }, a measurement never taken wearing the shape of one. declareRendering is now the more honest door, which inverts the intended incentive.
#8	R1.5 is not enforced: blocked collapses into empty. RoleState.kind never enters emitted, is never reconciled and never parity-checked. A face implementing no roleState, on a blocked Role with no sentence, produces a body identical to the empty case with zero breaches. roleState appears in zero shipped tests.
#9	A data-less list Role has no legal blocked/loading Frame — both {state:{kind:'blocked'}} and {..., fields:{}} throw FrameError: carries no Fields. Table Roles get rows: [] free. The exhaustive-keys rule and the no-empty-Frame rule collide exactly on R1.5's headline case.
#10	Quadratic render cost. countOccurrences rescans the full body per fragment. 250→4000 rows: markdown 3.0→425.1 ms, data 10.3→2266.0 ms; at 5000 rows renderData + parity is 4310 ms. Fine at the 20–200 row target, one order of magnitude from unusable as a CI gate.
#11	escapeMarkdown is not injective. #foo and \#foo both → \#foo; a literal newline and the two characters \n collide; \r\n/\r/\n collapse. The same "two facts, one byte string" defect B6 fixed, reintroduced one layer out, in a package advertising the escape as "unambiguous and reversible". Also unescaped: backticks, <script>, **bold**, [text](url).
#12	Export an emission helper. Every face hand-assembles the EmittedValue literal, and wrote: [glyph] is a lie for any face that escapes, with nothing telling the author.
#13	Untested new code — tracking issue. Both roleState branches, the empty-body bypass, escapeMarkdown injectivity. (deepFreeze directly, registry-spec mutation and wrote self-certification are covered by this commit.)

Test output

 ✓ src/field.test.ts (38 tests) 6ms
 ✓ src/glyph.test.ts (12 tests) 3ms
 ✓ src/view.test.ts (23 tests) 7ms
 ✓ src/frame.test.ts (19 tests) 8ms
 ✓ src/readme.test.ts (3 tests) 7ms
 ✓ src/parity.test.ts (17 tests) 8ms
 ✓ src/render.test.ts (45 tests) 21ms

 Test Files  7 passed (7)
      Tests  157 passed (157)

Each of the eight F2/F3 tests was verified to fail against 90bb4c0's source (stash the three
changed source files, run the suite: Tests 8 failed | 98 passed). The three F1 tests pass either
way by design — they are receipts for a documentation change, and they assert the hole rather than
its absence.

tsc --noEmit: clean. tsup: builds ESM + CJS + d.ts. eslint src: two errors, both pre-existing
at 90bb4c0 and untouched here (render.test.ts:349 use-isnan, render.ts beginRole's
unused role arg).

Still not merging.

[claude]

Review: @mdxld/frame - the unstyled Field/Frame/View/Rendering contract

This is a well-conceived and unusually well-documented package - the honesty-at-construction model, the single-walk/sink-adapter seam, and assertFaceParity are a genuinely elegant way to keep an agent face and a human face from drifting. Test coverage (68 tests across 5 files) is thorough and the "no dependencies, zero appearance" claims are backed by executable assertions rather than just prose. Package placement under @mdxld/* (parsing/transformation) is defensible per CLAUDE.md, given it has no rendering appearance and is consumed by @mdxui-style faces as a customer.

That said, per the PR description this is explicitly requesting adversarial review before merge, so here is the adversarial pass.

Bug: assertFieldHonest crashes with an uncaught TypeError instead of FieldHonestyError for a malformed withheld Field

This is the one I would block on. isField (src/field.ts, around line 150) only checks structural shape common to every presence state (presence, attribution, watermark.source, asOf) - it never checks that a withheld Field actually carries a licenceClass property. Then assertFieldHonest, around line 210, calls field.licenceClass.trim() to check for an empty licence class, with no undefined guard beforehand - unlike every other optional-field check in the same function, where e.g. the MODEL/modelVersion check a few lines above explicitly tests for undefined before calling trim on it.

So constructing a withheld Field the way it would arrive over a JSON boundary - an object with presence set to withheld, an attribution, a watermark with only a source, and an asOf, but with the licenceClass property missing entirely rather than empty - and passing it to assertFieldHonest throws a raw TypeError (Cannot read properties of undefined, reading trim), not the FieldHonestyError the whole module is built around.

The doc comment on assertFieldHonest specifically calls out this exact scenario as the reason the function exists: it should be called on any Field that did not come from makeField, notably one parsed from JSON, because the Fields that actually arrive dishonest are the ones that came over the wire as JSON and never met a constructor. A withheld field missing its licence class is a dishonest field by the package's own definition (see the existing test that constructs withheld with an all-whitespace licence class), but the wire-boundary path for a missing property isn't handled the way the wire-boundary path for a malformed MODEL field is.

Worse, makeFrame in frame.ts calls assertFieldHonest on every leaf specifically to catch dishonest wire values, so a Frame assembled from an untrusted or legacy payload containing this shape would crash with an unhandled TypeError rather than the documented, catchable FieldHonestyError or FrameError.

This is not covered by the existing test suite - field.test.ts's test for refusing a withheld value with no licence class goes through the withheld constructor with an all-whitespace string (not a missing property), and the "over the wire" describe block only exercises the missing-modelVersion case for MODEL, not the missing-licenceClass case for withheld.

Suggested fix: either have isField require the licenceClass property to be a non-empty string when presence is withheld, or add an undefined check before calling trim in assertFieldHonest, mirroring the modelVersion check just above it.

Design gap: a present Field holding a null value silently renders identically to absent

canonicalText in glyph.ts treats a null or undefined value as the ABSENT_GLYPH constant (the em dash). But nothing stops a caller from constructing a present Field whose value is null - PresentField's generic value type places no runtime constraint preventing that. The result is a present Field, server-confirmed and carrying provenance, rendering as an em dash - visually indistinguishable from a genuinely absent Field. That is precisely the "quiet lie" the README argues absent and withheld must never commit: a real, confirmed value silently rendered as unknown. Given the package's stated goal that there are no naked numbers and never a default, it seems worth also disallowing a null or undefined value on a present or unconfirmed Field at construction time, the same way a MODEL field without a receipt is refused.

Minor: UnknownIdError used for a validation failure, not a lookup failure

In view.ts, withRoles throws an UnknownIdError when a Role's id is empty after trimming. UnknownIdError is documented and used everywhere else - the role and view resolver methods, and the roles check in withViews - for "an id was asked for and not found." This call site is instead a registration-time validation failure (an empty id), and the two adjacent checks in the same method, for a missing markdown face and for an unknown markdown face kind, both correctly use TypeError for that kind of input error. Worth aligning for consistency, since callers may reasonably pattern-match on UnknownIdError specifically to mean "id not found, here are the valid ones," which isn't true for an empty-string id.

Minor: sinks are single-use but nothing enforces or documents it

markdownSink and dataSink close over mutable state and are only ever used for one renderFrame call in this codebase, but RenderSink is a public, documented extension point described as the way a new face gets built. Nothing stops a caller from holding a sink instance and passing it to renderFrame twice, which would silently accumulate or corrupt output rather than fail closed - a little at odds with how carefully everything else in this package fails loudly, for unknown ids, over-budget views, and dishonest fields alike. Consider either guarding against reuse, for instance by throwing if the frame-begin hook is called a second time, or documenting sinks as explicitly single-use.

Nits

• renderFrame's table branch recomputes the declared-versus-actual key check for every row to re-validate a ragged table, building a couple of Sets each time; fine at the token budgets this package targets, but would be worth hoisting if tables ever get large.
• The README's presence-state table does not spell out unconfirmed's exact trailing-space text format versus what the code actually produces - it matches today, but it's asserted only via an inline snapshot rather than a named format helper, so it would be easy to let the two drift silently on a future edit.

Overall: strong foundation, and the parity-assertion mechanism is the right idea. The withheld and licenceClass gap is the one I would actually fix before merge, since it is a correctness bug in the exact code path, JSON-boundary validation, that this package exists to make trustworthy.

[claude]

Review: feat(@mdxld/frame) — the unstyled contract for Field, Frame, View and Rendering

I read through the full source (field.ts, frame.ts, view.ts, render.ts, parity.ts, glyph.ts, tokenize.ts, errors.ts, fixtures.ts) and the test suite, and cross-checked the README's example against readme.test.ts. This is a new, self-contained package (packages/@mdxld/frame) with no other code in the monorepo depending on it yet, so blast radius is low — it fits the @mdxld scope ("parsing & transformation" / typed contract) per CLAUDE.md.

Overview

A types-only, zero-dependency, zero-appearance contract: Field (value + provenance + one of four presence states: present/absent/withheld/unconfirmed) → Frame (a materialised bundle of Fields) → View (the standing selection + per-face token budget) → Rendering (one serialization via a RenderSink). The headline mechanism is assertFaceParity, backed by a sink-reconciliation step in renderFrame that checks a face's declared emissions against both the canonical walk and its own output bytes.

Code quality & design

• Very high bar. Every non-obvious decision (why four presence states, why budgets are per-face, why paths must be injective, why sinks report bytes+claims rather than just bytes) is documented at the point of the decision, and — notably — most of those claims are backed by an actual test (readme.test.ts executing the README's own example is a good pattern worth reusing elsewhere in the repo).
• The honesty guard (assertFieldHonest) is enforced twice: once at construction (makeField) and again at makeFrame assembly, specifically to catch Fields that arrive over a JSON boundary and never touch a constructor. That's the right place to double-check it.
• deepFreeze on both Field and Frame closes a real hole (mutating a watermark after the guard ran) — good call.
• ViewRegistry's private constructor + copy-on-build (withRoles/withViews return new instances, Maps are copied rather than retained by reference) correctly closes the "caller mutates the registry mid-render" hole described in the module docstring.
• The reconcile() step in render.ts (sink declares wrote fragments, walk is the oracle) is the strongest part of this PR — it closes a real gap where a sink could discard every value and still pass a parity check built only from the walk's own emissions list, which the PR description says was previously shipped and only caught in review.

Potential issues / things worth a second look

1. escapeMarkdown (render.ts:434) is scoped to structural forgery, not visual spoofing. It escapes newlines, |, and a leading #/>/-/+. It doesn't escape *, backtick fences, or numeric list markers (1. ). In the current call sites this is safe in practice because every escaped value is emitted mid-line (after **label**: , inside a table cell, or wrapped in _..._), so a leading marker can't actually start a new markdown block. But it's worth a one-line comment noting why leading-char escaping is defensive-only today, since it would become a real gap if a future sink ever emits a Field's text at the start of a line.
2. walkFrame's docstring vs. actual guarantee (frame.ts:127-131): it claims canonical order is "Roles in View order," but walkFrame only ever sees frame.roles — the Frame's own array order — and has no reference to the View. makeFrame doesn't check or reorder against the View's declared role order, so a Frame assembled with roles out of View order would walk in that (wrong) order. This doesn't break correctness of parity or budgets (those are keyed by path, not sequence), but the doc comment overstates the guarantee. Minor, but since so much of this package's credibility rests on precise doc claims, it's worth tightening.
3. Object key order and numeric-like keys (frame.ts walkFrame, using Object.entries(role.fields ?? {})): JS reorders integer-like string keys ahead of insertion order. Given the domain (role field keys like status, gtin, coverage), this is unlikely to bite in practice — and the render path (render.ts keyOrder) sidesteps it entirely by iterating the declared markdown.keys array rather than object insertion order. Flagging only because walkFrame's "key-insertion order" claim is technically false for numeric-like keys; not a functional risk today.
4. reconcile()'s byte check is occurrence-count-only, not positional (render.ts:257-281): it verifies a declared fragment occurs in the body at least as many times as claimed, but doesn't verify where. A sink could satisfy the count while writing a fragment in the wrong location (e.g., under the wrong Role's heading). This is explicitly called out in the module docstring as a known residual limit ("this package cannot know that a body containing the honest glyph for unitPrice also contains a sentence contradicting it") — so it's a documented tradeoff rather than an oversight, just noting it as the main remaining gap for a reviewer to be aware of.

Test coverage

Excellent — 145 tests across 8 test files, and the ones I read (render.test.ts, readme.test.ts) specifically probe adversarial cases: a sink that discards values and lies about emitting them, mixed-provenance columns, empty tables (asserting no renderer-authored English like "(no rows)"), and the four presence states' distinguishability in text. The commit history shows earlier legislative tests (banning valid future vocabulary) were deleted in favor of testing actual behavior — good instinct.

Security

No injection/XSS-class concerns beyond the markdown-escaping note above (item 1), which is low severity given current call sites. No secrets, no network I/O, no dynamic code execution. Zero runtime dependencies, so no supply-chain surface added by this package itself.

Performance

Not a concern at this stage — no algorithmic red flags (reconcile's countOccurrences is O(body × fragments), fine for token-budgeted text sizes this package targets).

Process note

The PR description explicitly requests adversarial review, not design review, per vis/CLAUDE.md's gate for unstyled/stateful abstractions, and states it is not meant to merge yet. The findings above are all minor/documentation-level; I found no correctness bugs in the honesty guards, parity reconciliation, or budget enforcement logic.

[claude]

Review: @mdxld/frame — the unstyled contract for Field, Frame, View, Rendering

I read the full diff (23 files, ~4.5k additions, all-new package) plus the three self-conducted
adversarial review rounds already documented in the PR description. Note: this PR shows as
already merged — treating this as a post-hoc review per the request.

Overview

Adds packages/@mdxld/frame, a dependency-free, types-only contract: Field (a value + its
provenance: OBS|CALC|MODEL, watermark, asOf, and a branded ModelConfidence), Frame (a
typed bundle of Fields with a snapshot token), View/ViewRegistry (the standing definition of
what's shown, per-face token budgets), and Rendering (one serialization via a RenderSink
adapter over a single canonical walk). assertFaceParity is the CI-facing assertion that lets a
markdown face and a data/React face be proven to agree.

What stands out positively

• Construct-time honesty guards are real, not decorative — makeField/makeFrame deep-freeze
  their output and assertFieldHonest refuses mis-attributed provenance before a value can reach
  a Rendering. I traced this through field.ts/frame.ts and it holds together.
• The four presence states genuinely resolve a real ambiguity. absent vs withheld vs
  unconfirmed vs present, with COLLIDES_WITH_A_PRESENCE_MARKER in glyph.ts preventing a
  present string like '—' or '12 ~' from being indistinguishable from a marker — nice, and
  tested (glyph.test.ts).
• The PR description is unusually honest about the limits of its own guarantee. The render.ts
  module doc explicitly enumerates what byte-reconciliation does not check (no glyph↔fragment
  relation, no position check, wrote: [' '] trivially satisfies it, empty-body bypass). That's
  a rare and valuable practice — most PRs oversell what a new invariant covers.
• Zero runtime dependencies, ESM+CJS+d.ts build via tsup, and the markdown/data fixture snapshots
  stayed byte-identical across all three review rounds, which is good evidence the fixes were
  surgical rather than incidental behavior changes.

Issues / risks worth flagging (beyond what's already filed in the PR body)

1. countOccurrences (render.ts) is O(fragments × body length) and already filed as @mdxld/frame: quadratic render cost — countOccurrences rescans the full body per fragment (4310 ms at 5000 rows) #10 —
   at 5000 rows the PR's own numbers show renderData + parity at ~4.3s. Given this package is
   explicitly headed toward being a CI gate (assertFaceParity "belongs in CI, once per View"),
   I'd treat this as higher priority than "follow-up" once any consumer has table Roles with
   realistic row counts (the PR's own target is 20–200 rows, but nothing stops a caller from
   exceeding that until it's enforced).

2. escapeMarkdown non-injectivity (@mdxld/frame: escapeMarkdown is not injective, and leaves backticks, HTML, emphasis and links unescaped #11) is filed as a follow-up, but given B8 in round 1
   was explicitly about a supplier string forging a markdown section via \n## Shipment, and @mdxld/frame: escapeMarkdown is not injective, and leaves backticks, HTML, emphasis and links unescaped #11
   documents that #foo and \#foo collide under the current escape — this feels closer to a
   security-adjacent correctness bug (still-possible content-injection surface) than a nit, since
   the entire premise of B8's fix was "a value can't forge structure." Worth confirming the
   remaining collision doesn't reopen a variant of B8 before this becomes anyone's markdown wire
   format.

3. walkFrame/framePaths are O(n) per call and re-run on every renderFrame/faceParity
   invocation — fine at current scale, but combined with @mdxld/frame: quadratic render cost — countOccurrences rescans the full body per fragment (4310 ms at 5000 rows) #10, a consumer rendering large Frames
   in a hot path (not just CI) should be aware there's no memoization anywhere in this layer.

4. deepFreeze mutates the caller's object in place (documented, not a bug) — this is called
   out three times in the docs, which is good, but it's an easy footgun for a first consumer who
   passes sharedAppState into present() without reading the docstring. Consider whether a
   lint rule or runtime dev-warning (behind a flag) is worth it before this has multiple
   consumers, since the failure mode (frozen shared state) is a confusing one to debug from the
   call site.

5. Test coverage: the shipped counts (157 tests across 7 files) look strong for the happy
   path and honesty-guard edge cases, but per the PR's own @mdxld/frame: untested new code — six surfaces added in the contract commit have zero tests #13, roleState (both branches),
   the empty-body reconciliation bypass, and escapeMarkdown injectivity are explicitly untested.
   Since RoleState/R1.5 is presented as a shipped feature (not deferred, unlike R4.2), I'd want
   at least one test exercising blocked vs empty before calling this feature complete for a
   consumer — right now roleState has zero direct test coverage per @mdxld/frame: R1.5 is not enforced — blocked collapses into empty, and roleState appears in zero shipped tests #8/@mdxld/frame: untested new code — six surfaces added in the contract commit have zero tests #13.

Process note

The three-round adversarial review already in the PR body is genuinely rigorous — each finding
verified to reproduce against the prior commit, byte-identical fixture snapshots as a regression
guard, and explicit "deliberately NOT done" scoping. I don't have anything to add on B1–B12/F1–F3;
independent read of field.ts, frame.ts, glyph.ts, parity.ts, render.ts, and view.ts
confirms the fixes described match the code. My additions above are mostly about sequencing
(prioritize #10/#11 before this becomes a wire format or CI gate at scale) rather than new defects.

Recommendation: solid foundation, appropriately scoped as "unstyled contract" with real
fail-closed behavior. Given it's already merged, I'd suggest tracking #10 and #11 as near-term
(not just filed-and-forgotten) since both interact with claims the package makes about itself
(CI-safe, injection-resistant), rather than being purely aesthetic gaps.

[claude]

Review: @mdxld/frame

This is a strong, unusually well-documented PR — the round-1 self-review (12 blockers + should-fixes, each with a failure demonstration) already covers most of what an adversarial pass would normally surface. I read the actual source at the PR head (not just the diff) and verified the two findings below against the real code before reporting them. Per vis/CLAUDE.md's note in the description, treating this as an adversarial pass rather than a design review.

Findings

1. escapeMarkdown never escapes < — raw HTML/JSX passes into every rendered value
packages/@mdxld/frame/src/render.ts (escapeMarkdown):

export function escapeMarkdown(text: string): string {
  const flattened = text.replace(/\r\n|\r|\n/g, '\\n').replace(/\|/g, '\\|')
  return /^[#>\-+]/.test(flattened) ? `\\${flattened}` : flattened
}

This escapes |, newlines, and a handful of leading block markers (#, >, -, +), but never touches < anywhere in the string. A Field value like <img src=x onerror=alert(1)> or <script> (very plausible input for something like a shipment note or partner-supplied string — exactly the kind of hostile value B8 was written to defend against) flows verbatim into the markdown body. In an MDX-centric monorepo where <...> is parsed as JSX/HTML by design, this is a live injection vector that the B8 fix doesn't cover and no test exercises. The leading-marker escape is also incomplete relative to CommonMark's own block starters (*, `, ~, =, digit+./) for ordered lists) — not currently reachable through the two shipped sinks since they always prefix values, but escapeMarkdown is exported as a public primitive other sinks will rely on, and its docstring's "values cannot author structure" claim overstates what it actually guarantees.

Suggested fix: escape < the same way | is escaped, and extend the leading-marker class to the full CommonMark block-starter set.

2. dataSink silently drops value for a present(undefined, …) Field
packages/@mdxld/frame/src/render.ts (dataSink, entryFor):

if (f.presence === 'present' || f.presence === 'unconfirmed') entry.value = f.value
...
return { body: JSON.stringify(payload), emitted }

field.test.ts/glyph.test.ts treat present(undefined, obs) as a meaningful, distinct case from present('undefined', obs). But JSON.stringify drops object keys whose value is undefined, so a present/unconfirmed Field whose value is literally undefined loses its value key entirely in the data face's JSON body — contradicting the README's claim that "the data face IS the Frame — the Fields verbatim." Since isField/assertFieldHonest are documented as existing specifically to re-validate Fields that crossed a JSON boundary, round-tripping such a Field through the data face turns a legitimate present Field into one isField will reject (no value key). Narrow, but real and untested.

Worth a second look, lower confidence

3. faceParity([]) / assertFaceParity(frame, []) passes vacuously. The missing/invented check in parity.ts iterates over the supplied renderings array, so calling it with zero renderings (e.g. a bug that filters out every face before calling) returns zero breaches without checking anything — the same class of gap that makeFrame now explicitly closes for a zero-Field Frame. Might be worth refusing an empty renderings array the same way.

4. assertBudgets' 'markdown' in budgets check (view.ts) uses in rather than an own-property check. I don't think this is currently exploitable — the registry's copy step ({ ...view.budgets }) only copies own properties, so an inherited-only markdown key gets dropped before it would matter — but it's a slightly fragile pattern if that copy step ever changes.

Not flagged

Everything already named in the PR body as a known, tested limitation (byte-reconciliation being occurrence-counting rather than a parse, no position-checking, wrote having no required relation to text, the CJK tokenizer under-count which only ever fails open) checks out as accurately described and doesn't need repeating here. Freeze semantics, honesty guards, path injectivity, and registry immutability all held up under a second read.

Nice package — the face-parity design and the "the walk is the oracle" framing for B1 in particular is a good fix. The two findings above are worth closing before this leaves the unstyled-contract stage, given the stated purpose is exactly to be the safety rail other faces build on.