Skip to content

integrate(mdx-8je) rounds 1-3: mdxui leaves the repo (.8); one diff implementation (.12); hono serves text registers by Accept (.18) - #15

Merged
nathanclevenger merged 7 commits into
mainfrom
integrate/mdx-8je
Sep 9, 2026
Merged

nathanclevenger merged 7 commits into
mainfrom
integrate/mdx-8je

Conversation

@nathanclevenger

@nathanclevenger nathanclevenger commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

integrate(mdx-8je) round 3: hono serves text registers by Accept (mdx-8je.18)

Integration branch integrate/mdx-8je = origin/main (2b1fa0e) + round 1 (d113a9d) + round 2 (b629c8a) + this merge (5a7d7fa) + lockfile regeneration (4a2e18a). Merged by the release integrator with authority for PR #15; nothing here is deployed or published.

Merged

Issue Branch Commit Merge
mdx-8je.8 worktree-wf_db5bf2c3-d4f-2 f18c0bf d113a9d (round 1)
mdx-8je.12 worktree-wf_db5bf2c3-d4f-6 3440878 b629c8a (round 2)
mdx-8je.18 worktree-wf_db5bf2c3-d4f-7 1a5a9ca 5a7d7fa + 4a2e18a (this round)

mdx-8je.18 — @mdxe/hono: serve text registers by Accept via @mdxui/text. New src/text.ts (resolveTextRegister, renderTextRegister, textRegisterHeaders with labelled x-markdown-tokens / x-markdown-tokens-method, notAcceptable problem+json), rewritten src/format.ts (q-ordered negotiateFormat, FormatContext with decidedBy + frozen capabilities, async documentResponse / formatResponse, 406 naming the available media types, .md = md register, .mdx = raw source), ./format and ./text exports, workers-pool suite (@cloudflare/vitest-pool-workers, SELF.fetch), mdxld → workspace:^ (mdx-w89.5/.13 interim).

Conflict resolution with round 1 (mdx-8je.8)

The round-2 hold ("which @mdxui/text entry is the md face") was decided for .18's contract: hono's md register renders through @mdxui/text/core render(doc, capabilities) so a served page is byte-identical to the CLI's --format md.

  • src/format.ts: .18's file. mdx-8je.8's constraint (md rendered by npm @mdxui/text, never a workspace source) holds through text.ts; @mdxui/text stays ^0.1.0, @mdxui/fumadocs ^1.9.0, no @mdxui/* workspace deps anywhere.
  • src/format.test.ts: one suite — .18's cases plus .8's (json round-trip, html fallback / custom renderer, nested extension mapping, md keeps document data). Dropped .8's txt === doc.content and */* → null (superseded by .18's plain register / negotiation) and the byte-identity with @mdxui/text/md: in @mdxui/text@0.1.0 core's md register and the /md entry are different renderers (## Type data block + JSX kept vs YAML frontmatter + JSX stripped). Known gap (dot-do/ui): hono text/markdown and workers' .md asset (still @mdxui/text/md) differ for the same document until upstream reconciles the two; recorded in the test header.
  • package.json: union — ./format + ./text exports, test runs node then the workers pool, @cloudflare/vitest-pool-workers + @cloudflare/workers-types devDeps, mdxld: workspace:^.
  • pnpm-lock.yaml: not hand-merged — regenerated with pnpm install --config.link-workspace-packages=true (committed as 4a2e18a on top of the merge); diff vs b629c8a is 5 hunks, all importers: — hono (mdxld specifier, the two new devDeps) and mdxe (mirrors its cleanly merged package.json). --frozen-lockfile install clean.
  • test/repo/mdxui-home.test.ts: the hono md witness accepts @mdxui/text/md or /core imported from format.ts or text.ts (the render path moved into text.ts); the runtime witness of the registry package is unchanged.

Verification on the integration branch (4a2e18a)

Suite Result
@mdxe/hono build green (./format, ./text entries emitted)
@mdxe/hono test — node 138 / 138 (3 files; 133 + 5 carried from .8)
@mdxe/hono test — workers pool 5 / 5
@mdxe/workers 399 / 399
@mdxe/vitest 145 / 145
mdxe 800 passed, 3 skipped (30 files)
pnpm test:repo (incl. mdxui-home, diff-home, lockfile-drift) 204 / 204
@mdxe/hono typecheck red only in src/index.test.ts (12 × TS2339: @mdxe/test-utils matcher augmentation is dropped from its dist .d.ts) — identical on main, on round 2 and on the .18 branch; CI Type Check is already red on main

Note on test:repo counts: 204 is the branch's count since round 1 (tsconfig-includes emits 3 tests per workspace package; the 17 removed @mdxui/* packages account for the earlier 255).

Remaining in epic mdx-8je (not in this PR)

.9 / .10 / .15 (gated on ai-evaluate 3.0 on npm), .1 (E2E vs auth.apis.do), .20 (mdxld publish), .17 (deferred).


integrate(mdx-8je) round 2: one diff implementation (mdx-8je.12); mdx-8je.18 held

Integration branch integrate/mdx-8je = origin/main (2b1fa0e) + round 1 (d113a9d, below) + one reviewed merge (b629c8a). A human approves and merges this PR; nothing here is deployed.

Merged

Issue Branch Commit Review
mdx-8je.12 worktree-wf_db5bf2c3-d4f-6 3440878 approved

mdx-8je.12 — @mdxld/extract: wire @mdxld/diff as the one diff implementation, 3-way mergeExtract, real extractWithAI, render→extract identity vs @mdxui/text

Intent: @mdxld/extract and @mdxld/markdown each carried their own diff/applyExtract while @mdxld/diff was never wired in; extractWithAI was a stub; nothing proved a template renders markdown its own regex can reverse. Acceptance: identity test green across the Role fixture set; one diff implementation in the repo.

What the diff does, checked against that intent and CLAUDE.md:

  • @mdxld/diff owns the structured primitives (diffPaths, applyPaths, merge3wayObjects, exported deepEqual); merge3way now returns merged content with git-style conflict regions (it returned a unified patch and printed [object Object] for conflicts). @mdxld/extract's diff/applyExtract are one-line delegations; mergeExtract(base, current, extracted) is the 3-way case where props and markdown both changed, and a path the template does not render is never treated as deleted. @mdxld/markdown drops diff/applyExtract/DiffResult and gains the one table primitive (parseTable/renderTable) that both packages now use; @mdxld/formats re-exports accordingly. Changeset present (minor x4).
  • extractWithAI is real: one generateObject call (ai-functions as an optional peer, injectable generate) for the slots the matcher cannot reverse, answers landing at the expression's data path; loop/conditional slots are captured as regions instead of breaking the scalars beside them; slot parsing is brace-balanced.
  • Tests first, as the issue asked: round-trip.test.ts proves extract(template, render(props, 'md')).data deep-equals the round-trip props for every Role that npm @mdxui/text 0.1.0 publishes (Hero/Features/Pricing/Testimonials/CTA/FAQ, 22 tests) and witnesses the published ROUND_TRIP_GAPS routed here; merge.test.ts (8) covers props-and-markdown-both-changed; ai.test.ts (8) mocks ai-functions for a loop + conditional template; @mdxld/markdown pins that it exports no diff; test/repo/diff-home.test.ts (4) is the repo-level witness: no diff/applyExtract body outside packages/@mdxld/diff and the dist re-exports produce exactly @mdxld/diff's results.
  • Deferred with a filed issue, not silently: the rename/fold of @mdxld/markdown (name collision with the departed @mdxui/markdown) is mdx-w89.17. Reasonable: round 1 already removed @mdxui/markdown from this tree, so the collision is historical.
  • Docs updated wherever they showed the removed diff/applyExtract from @mdxld/markdown (README x4, docs/ x4).

Verification on the integration branch (after merge, pnpm install --frozen-lockfile --config.link-workspace-packages=true, no lockfile drift)

Suite Result
@mdxld/diff 83 / 83
@mdxld/markdown 47 / 47
@mdxld/extract (6 files, incl. round-trip identity, merge, ai) 159 / 159
@mdxld/formats 42 / 42
pnpm test:repo (incl. diff-home) 204 / 204
typecheck (tsc --noEmit) on the four packages clean
lint on the four packages @mdxld/extract shows 4 errors, all present on main in files this diff does not touch (extract.test.ts, integration.test.ts, integration.ts); main has 5, this diff removes the entity.ts one

Lockfile: the merge conflicted only in pnpm-lock.yaml; it was regenerated (pnpm install --lockfile-only --config.link-workspace-packages=true, the same mode round 1 used, needed because main's mdxld ^1.9.2 / @mdxe/* ^3.0.0 pins are unpublished, mdx-w89.5 / mdx-w89.13). The regenerated lockfile differs from round 1's by exactly the 13 lines @mdxld/extract's new dependencies need.

Held

Issue Branch Commit Reason
mdx-8je.18 worktree-wf_db5bf2c3-d4f-7 (pushed) 1a5a9ca Changes which @mdxui/text renderer is the md face, regressing a round-1 guarantee. Round 1 (mdx-8je.8, merged above) made @mdxe/hono's md format and @mdxe/workers' .md asset render through @mdxui/text/md render(doc): frontmatter kept, JSX stripped, tested (not.toContain('<Hero')). .18 rewrites format.ts so text/markdown renders through @mdxui/text/core render(doc, capabilities) instead. Those two entries of the same npm package disagree: for a doc with <Hero title="x" /> in the body, /md gives ---\n$type: BlogPost\n…---\n\n# Hello\n\n\n\nA **bold** line… while /core md gives ## BlogPost\n\n- title: Hello\n- tags: […]\n\n---\n\n\n# Hello\n\n<Hero title="x" />\n\nA **bold** line… (data as a synthesized heading + list, JSX passed verbatim; the branch itself files this as mdx-w89.15 "upstream"). Merging would serve raw JSX on the agent register and make hono's text/markdown differ from workers' .md for the same document. On a trial merge the branch was green on its own (136 node tests, 5/5 in the workers pool via SELF.fetch, tsc clean for touched src, lint errors all pre-existing on main), and the conflicts with round 1 (format.ts, format.test.ts, package.json, lockfile) were mechanically trivial, but round 1's carried test renderDocument(doc,'md') === render(doc) from @mdxui/text/md fails under it. Human decision needed: which register is the md face (or wait for dot-do/ui to make /core md strip JSX like /md), then re-merge; the rest of .18 (q-ordered Accept, 406 problem+json naming what exists, labelled x-markdown-tokens, workers-pool test, workspace:^ fix for the unpublished pins) reads well.

integrate(mdx-8je) round 1 (post-#14): mdxui leaves the repo

Integration branch integrate/mdx-8je, fast-forwarded to origin/main (2b1fa0e), plus one reviewed merge. A human approves and merges this PR; nothing here is deployed.

Merged

Issue Branch Commit Review
mdx-8je.8 worktree-wf_db5bf2c3-d4f-2 f18c0bf approved

mdx-8je.8 — delete mdxui and @mdxui/ sources; consume @mdxui/text and @mdxui/fumadocs from npm*

Intent (un-deferred note on the issue): dot-do/ui owns mdxui and every @mdxui/*; the receipt there is done (ui-9ml4.1, @mdxui/text 0.1.0 published). Delete the sources here, repoint consumers to the registry, keep the guards green, do not publish.

What the diff does, checked against that intent and CLAUDE.md:

  • Removes packages/mdxui, packages/@mdxui/* (33 private packages), apps/storybook, the chromatic workflow, the @mdxui/editor/app submodule entry, the workspace glob, the changeset group, the publish-all entry, and the root storybook script. git ls-tree confirms no gitlink survives.
  • @mdxe/hono md format and @mdxe/workers .md asset render through @mdxui/text/md (render); @mdxe/vitest generated code imports renderMarkdown from @mdxui/text/md — verified against the installed 0.1.0 md/index.d.ts, which exports render, renderContent, and the renderMarkdown / renderMd aliases.
  • mdxe db only ever resolves an installed @mdxui/editor; @mdxai/integration drops an unused @mdxui/plan devDep.
  • Tests first: test/repo/mdxui-home.test.ts (12 tests) pins the removal, the workspace:-free dependency rule with an NPM_ALLOWED set, a source-import sweep, and a runtime witness that the installed @mdxui/text is the registry package (resolves under node_modules/.pnpm/, repository dot-do/ui) and renders a parsed document with JSX stripped. Plus hono/src/format.test.ts, workers/src/build-assets.test.ts, and vitest generated-import tests.
  • CLAUDE.md / README.md describe the new home honestly, including that @mdxui/text has only the md register today (html = hono renderPage / workers generateHtmlPage, json = JSON.stringify(doc)), which is the acceptance nuance filed as mdx-w89.7.
  • Stale-reference sweep of tracked files: only prose docs (docs/(UI Components)/mdxui/*, packages/mdxe/ECOSYSTEM.md, VERSIONING.md, some example .mdx) and comments still name the old packages; no source, tsconfig, turbo, or workflow reference remains. Doc drift is a follow-up, not a blocker.

Verification on the integration branch (after merge, pnpm install --frozen-lockfile --config.link-workspace-packages=true, no lockfile drift)

Suite Result
pnpm test:repo (repo guards, incl. mdxui-home and package-allowlist) 200 / 200
@mdxe/hono 102 / 102
@mdxe/workers 399 / 399
@mdxe/vitest 145 / 145
@mdxai/integration 96 / 96
mdxe 800 passed, 3 skipped (30 files)
builds: @mdxe/hono, @mdxe/workers, @mdxe/vitest, @mdxe/fumadocs, mdxe green
typecheck red in files this diff does not touch (see below); strictly fewer errors than main

Pre-existing, not introduced here (recorded so the reviewer does not re-derive it)

  • Lockfile interim (mdx-w89.5). origin/main's lockfile is stale against the release commit 9dab805 (e.g. hono importer records mdxld ^1.9.0 while package.json says ^1.9.2). This branch's regenerated lockfile is honest to the package.jsons; the four unpublished pins (mdxld ^1.9.2, @mdxe/hono ^3.0.0, @mdxe/fumadocs ^3.0.0 x2) resolve to link: entries. Human step after publishing the release from main: plain pnpm install, commit the lockfile.
  • Turbo ordering. Because mdxe depends on @mdxe/hono / @mdxe/fumadocs through ^3.0.0 registry ranges, turbo does not see those workspace edges: a parallel turbo run build once raced mdxe's DTS against hono's, and mdxe... did not build @mdxe/fumadocs. Both go away when the pins are published (mdx-w89.5); sequential builds are green.
  • Typecheck. @mdxe/vitest src/__tests__/index.test.ts (32 errors, lines 299-396; this diff adds lines 629-677), @mdxe/hono src/index.test.ts (12), @mdxe/workers src/local.test.ts (8), mdxe src/cli.ts (8, main has 12), mdxe src/commands/notebook.ts (3). All present on main in the same untouched files.
  • Worktree hygiene. The integration worktree carried git-ignored node_modules/dist husks under directories deleted by integrate(mdx-8je): Cloudflare-native kernel round 1 — prune to workerd-only, mdxdb root, @mdxe/cli-core + @mdxe/tui seam, sqlite pools + $init, mdxld 1.9.2 alias release #14 and by this merge; they made 25 "no longer exists" guards fail until removed. A clean checkout does not have them.

Held

Nothing held this round.

Co-Authored-By: Claude Fable 5.1 noreply@anthropic.com

nathanclevenger and others added 2 commits September 9, 2026 07:20
…xt and @mdxui/fumadocs from npm (mdx-8je.8)

dot-do/ui owns mdxui (7.0.0-alpha) and every @mdxui/* package; the format
renderers were received there (ui-9ml4.1) and @mdxui/markdown became the
md register of @mdxui/text. This repo now has no @mdxui sources.

Removed
- packages/mdxui, packages/@mdxui/* (33 packages, all already private)
- apps/storybook and .github/workflows/chromatic.yml (hosted only @mdxui kits)
- the packages/@mdxui/editor/app submodule entry, the @mdxui workspace glob,
  the mdxui changeset group, the @mdxui/fumadocs publish-all entry, the
  root `storybook` script

Re-pointed
- @mdxe/hono: `md` format renders through npm @mdxui/text/md (frontmatter +
  clean markdown, JSX stripped) instead of mdxld stringify; @mdxui/fumadocs
  stays on the registry range ^1.9.0
- @mdxe/workers: the `.md` static asset renders through @mdxui/text/md
  (no frontmatter; the .json asset carries the data)
- @mdxe/vitest: generated test code imports renderMarkdown from
  '@mdxui/text/md'; the mdxui component-factory auto-import is gone
- mdxe db command: @mdxui/editor is only ever an installed npm package
- @mdxai/integration: dropped the unused @mdxui/plan devDependency
- vitest.mdx.config.ts / tests/$.mdx: no workspace @mdxui aliases; the
  component-factory blocks (a dot-do/ui API) are removed
- README.md / CLAUDE.md describe the new home

Guards (tests first)
- test/repo/mdxui-home.test.ts: packages/mdxui, packages/@mdxui and
  apps/storybook stay gone; no workspace: dependency on mdxui/@mdxui; only
  NPM_ALLOWED (@mdxui/text, @mdxui/fumadocs) with registry ranges; no source
  import outside that set; @mdxe/hono and @mdxe/workers depend on and import
  @mdxui/text/md; the installed @mdxui/text is the registry package and its
  md register is witnessed at runtime
- packages/@mdxe/hono/src/format.test.ts, packages/@mdxe/workers/src/
  build-assets.test.ts, @mdxe/vitest generated-import tests

Lockfile: regenerated. Because the 2026-09 release (9dab805) pins mdxld
^1.9.2, @mdxe/hono ^3.0.0 and @mdxe/fumadocs ^3.0.0 that are not on npm
yet, a plain `pnpm install` fails; the lockfile was produced with
`--config.link-workspace-packages=true`, which resolves exactly those four
entries to workspace links and nothing else (mdx-w89.5). A plain install
after the publish rewrites them to registry versions.

Gaps filed under mdx-w89: .5 (unpublished release pins), .6
(@mdxui/fumadocs@1.9.0 ships workspace:* deps), .7 (acceptance names
html/json registers @mdxui/text does not have), .8 (md register keeps
inline {expr}), .9 (test:mdx lane broken by mdxdb MemoryDBClient).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…e/mdx-8je

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@claude

claude Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review

This is a large PR by file count, but almost all of it is mechanical deletion of packages/mdxui, packages/@mdxui/*, and apps/storybook (already fully accounted for in the PR description). The actual behavioral surface is small: swapping @mdxe/hono's and @mdxe/workers' markdown rendering from mdxld's stringify() to npm @mdxui/text/md's render(). That part is well done — new unit tests (format.test.ts, build-assets.test.ts), a thorough repo guard suite (test/repo/mdxui-home.test.ts) that pins removal, forbids workspace:-linked @mdxui/* deps, sweeps source imports, and even runtime-witnesses that @mdxui/text resolves from the npm registry rather than a workspace link. Docs (CLAUDE.md, README.md) were updated to match the code, not just the package moves.

Worth double-checking

  • .mdx extension now returns lossy content, not source. In packages/@mdxe/hono/src/format.ts, FORMAT_EXTENSIONS['.mdx'] = 'md' (pre-existing, untouched by this diff) routes /foo.mdx requests through the same 'md' case that used to call stringify(doc) (a faithful reconstruction of the MDX source) and now calls renderMd(doc) from @mdxui/text/md, which strips JSX and expressions. So a request for the .mdx extension specifically will now silently return JSX-stripped markdown instead of the original MDX source. That may well be intentional given the overall md-format redefinition, but it isn't called out in the PR description (which frames the change around .md/text/markdown), and format.test.ts only exercises .md/.json/no-extension in parseFormat — there's no test asserting what a .mdx-extension request actually returns post-change. Worth a one-line confirmation (or a test) that this is the desired behavior for .mdx consumers.

  • Known content-fidelity gap, already flagged. The comment in format.test.ts notes that inline {props.expr} inside a paragraph survives @mdxui/text@0.1.0's default expressionHandling: 'strip' (only flow-level expressions are stripped), and that this is tracked upstream (dot-do/ui) rather than fixed here. Good that it's surfaced rather than silently under-tested, just flagging that a raw {expr} leaking into rendered md/.md-asset output could look like a template-injection-ish oddity to consumers until that's fixed upstream — not a security issue since input is developer-authored MDX, not end-user input, but worth keeping on the radar.

  • mdxui-home.test.ts hardcodes a resolution path. The runtime witness test asserts @mdxui/text resolves under packages/@mdxe/hono/node_modules/@mdxui/text → node_modules/.pnpm/.... That's correct today given pnpm's isolated node_modules layout and @mdxe/hono listing @mdxui/text as a direct dependency, but it's a fairly tight coupling to pnpm's hoisting behavior for a repo guard — if hoisting config ever changes (e.g. shamefully-hoist), this specific test breaks even though the actual intent (registry package, not a workspace link) still holds. Not a blocker, just a maintenance note.

Nits

  • Version consistency is good: @mdxui/text: ^0.1.0 is pinned identically in root package.json, @mdxe/hono/package.json, and @mdxe/workers/package.json.
  • The mdxuiMarkdown → mdxuiText rename in @mdxe/vitest/src/index.ts and removal of the dead mdxui component-factory import branch (createComponents, getComponentMeta, etc.) is clean — no dangling references left behind (confirmed needsCreateElementFlag's mdxui.includes(...) check was removed along with the array itself).

Overall: solid, well-tested migration. The .mdx-extension fidelity change is the one item I'd want an explicit "yes, intentional" on before merge; everything else looks safe to land.

🤖 Generated with Claude Code

nathanclevenger and others added 3 commits September 9, 2026 07:54
`Accept: text/markdown` now returns the md register rendered by @mdxui/text
(byte-identical to the mdxe CLI's `--format md`) with the Markdown-for-Agents
headers — `Content-Type: text/markdown; charset=utf-8`, `Vary: Accept`,
`x-markdown-tokens` from the @mdxe/cli-core token oracle with its method
labelled in `x-markdown-tokens-method`. `Accept: text/plain` returns the plain
register. An `Accept` naming only unimplemented formats is refused with a 406
`application/problem+json` naming every media type that exists — never a
silent downgrade. Accept is read by `q` (a browser Accept is html, not xml as
the old substring match decided).

- src/text.ts: the Accept rung via resolveCapabilities({ accept }) from
  @mdxui/text/capabilities (detectedBy 'accept', agent for md / human for
  plain, harness null, never interactive), render via @mdxui/text/core,
  header stamping, the 406 problem.
- src/format.ts: q-ordered negotiateFormat, FormatContext carries the
  decision + frozen capabilities, documentResponse/formatResponse are async
  (token count). `.md` is the md register; `.mdx` is the raw source
  (text/mdx). server.ts serves every face through documentResponse.
- Tests: src/format.test.ts + src/text.test.ts (node) and
  test/workers/text-registers.test.ts under @cloudflare/vitest-pool-workers
  (headers verified via SELF.fetch in workerd). The optional js-tiktoken
  backend is stubbed in the pool (test/stubs) — see mdx-w89.14.
- Deps: @mdxui/text ^0.1.0; devDeps @cloudflare/vitest-pool-workers,
  @cloudflare/workers-types. mdxld → workspace:^.

Also: packages/mdxe devDependency mdxld and dependencies @mdxe/hono /
@mdxe/fumadocs → workspace:^. The release commit 9dab805 bumped them to
unpublished versions, so a fresh checkout could not `pnpm install` at all
(mdx-w89.13). Lockfile re-resolved accordingly.

Gaps filed (parent mdx-w89): mdx-w89.13 (unpublished specifiers),
mdx-w89.14 (token oracle optional backend under workerd), mdx-w89.15 (md
register passes body JSX through verbatim — upstream @mdxui/text).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…, 3-way mergeExtract, real extractWithAI, render→extract identity vs @mdxui/text (mdx-8je.12)

- @mdxld/diff: merge3way returns merged content with git-style conflict regions;
  new path-based diffPaths / applyPaths / merge3wayObjects, exported deepEqual
- @mdxld/extract: diff / applyExtract delegate to @mdxld/diff; new mergeExtract
  (base / current / extracted) for props and markdown both changed; extractWithAI
  implemented over ai-functions generateObject (optional peer, injectable generate);
  loop / conditional slots captured as regions instead of poisoning neighbouring
  scalars; entity tables go through @mdxld/markdown parseTable / renderTable
- @mdxld/markdown: diff / applyExtract / DiffResult removed; parseTable / renderTable
  shared with @mdxld/extract; fromMarkdown keeps empty cells positional
- @mdxld/formats: re-exports parseTable / renderTable instead of the removed diff API
- tests: round-trip identity for every Role fixture against npm @mdxui/text md
  register (ROLE_MD_TEMPLATES / MD_EXTRACTORS / ROUND_TRIP_GAPS), 3-way merge,
  extractWithAI with mocked ai-functions, test/repo/diff-home witnesses one diff
  implementation; pre-existing typecheck errors in diff/markdown test files fixed

Refs: mdx-8je.12. Follow-up mdx-w89.17 (rename/fold @mdxld/markdown once
@mdxui/markdown leaves the tree).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@nathanclevenger nathanclevenger changed the title integrate(mdx-8je) round 1: mdxui and @mdxui/* leave the repo; @mdxe/hono + @mdxe/workers render md through npm @mdxui/text (mdx-8je.8) integrate(mdx-8je) rounds 1-2: mdxui leaves the repo (mdx-8je.8); one diff implementation, 3-way mergeExtract, render→extract identity (mdx-8je.12); .18 held Sep 9, 2026
@claude

claude Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review

Focused on the actual logic changes (@mdxld/diff, @mdxld/extract, @mdxld/markdown, and the @mdxui/text re-pointing in @mdxe/hono/@mdxe/workers/@mdxe/vitest); the storybook/@mdxui/* deletions are mechanical and I skimmed only for stray references.

Overall

Solid piece of work. The "one diff implementation" consolidation is real: @mdxld/diff now owns diffPaths/applyPaths/merge3wayObjects, @mdxld/extract's diff/applyExtract are one-line delegations, and test/repo/diff-home.test.ts backs that with both a static source sweep and a runtime witness comparing dist outputs — a good pattern, since a source-regex guard alone can't catch a re-export that quietly degrades into a copy. @mdxld/markdown's new parseTable/renderTable primitive is like-for-like reused by @mdxld/extract's entity helpers. Test coverage is unusually thorough for this kind of refactor (round-trip identity against real @mdxui/text fixtures, 3-way merge cases, mocked-AI extraction, repo guards).

Potential issues

  1. balancedBraces aborts the entire scan on the first unclosed { (packages/@mdxld/extract/src/extract.ts, balancedBraces). When j >= template.length (no matching } found), the code does break out of the outer while (i < template.length) loop, not just the inner brace-matching loop — so every valid {expr} slot after the unclosed brace is silently dropped rather than just skipping the malformed one. A template with a stray { early on (e.g. inside a fenced code sample showing a JS object) would lose all subsequent slots with no error. Consider i = j + 1 continuing the outer loop instead of break, or at least documenting/asserting that templates must not contain unbalanced braces outside code fences.

  2. expressionPath only strips a fixed allowlist of chain-terminal methods (CALL_METHODS = /\.(map|filter|forEach|flatMap|reduce|join|slice|sort|some|every|find)\b/). An expression like data.title.toUpperCase() or data.items.length would resolve to the literal (wrong) path data.title.toUpperCase / data.items.length instead of data.title / data.items, since those aren't in the list. This is a narrower issue than a crash — it only affects the AI-assisted fallback path — but worth a comment noting the allowlist is deliberately narrow (or matching on "chain ends in a call at all" rather than specific names) so it doesn't quietly regress as new template patterns are used.

  3. Minor duplication: extract.ts keeps its own private getPath/setPath (used by aiTargets/extractWithAI), which duplicate the same walk logic that already exists (unexported) in @mdxld/diff. Not a correctness issue and out of scope for the "one diff implementation" guard (that's about diff/applyExtract/merge, not generic path helpers), but if @mdxld/diff ever exports getPath/setPath this file should switch to them to avoid drift.

  4. merge3way's rewrite from the diff library's Diff.merge to a hand-rolled diff3 (packages/@mdxld/diff/src/index.ts) is the highest-risk change in the PR — it replaces a well-tested third-party merge with new hunk-interval logic. The new test suite covers the standard cases well (adjacent/touching conflicts, identical changes, insert/delete on either side, trailing-newline edge cases), which gives good confidence, but this is exactly the kind of code where an adversarial fuzz/property test (random base + two edit scripts, compare against a reference 3-way merge) would be valuable given how easy it is for interval-merging logic like touches/hunk-growth to have a subtle off-by-one on inputs the hand-written unit tests don't happen to hit.

Nits

  • test/repo/diff-home.test.ts's two runtime-witness tests silently return (pass) when the package isn't built yet in the checkout. That's a reasonable CI-ordering tradeoff given the comment explaining it, but a it.skip/explicit skip reason would make "this didn't actually check anything" more visible in CI output than a green checkmark does.
  • Nice touch overall: docs (README x4, docs/) were updated in the same diff rather than left to drift, and the PR body is unusually explicit about what's deferred (mdx-8je.18 held, mdx-w89.17 filed) rather than silently dropped.

No security concerns spotted — the AI-assisted extraction path takes a caller-supplied generate/model string but only ever passes it to the (optional, explicitly peer-dependency) ai-functions call, no shell/eval/dynamic-code execution.

🤖 Generated with Claude Code

…te/mdx-8je — resolve hono format/text conflicts with mdx-8je.8

Conflicts resolved:
- packages/@mdxe/hono/src/format.ts: mdx-8je.18's contract (q-ordered negotiateFormat,
  FormatContext with decidedBy + frozen capabilities, async documentResponse/formatResponse,
  406 problem+json naming the available media types; `.md` = md register via @mdxui/text/core
  through ./text.ts, `.mdx` = raw source). mdx-8je.8's constraint (md rendered by npm
  @mdxui/text, never a workspace source) holds through text.ts.
- packages/@mdxe/hono/src/format.test.ts: one suite, mdx-8je.18's cases plus mdx-8je.8's
  (json round-trip, html fallback/custom renderer, extension mapping, md keeps document data).
  Dropped .8's `txt === doc.content` and `*/*` → null (superseded by .18's registers/negotiation)
  and the byte-identity with @mdxui/text/md: in @mdxui/text@0.1.0 core's md register and the
  /md entry are different renderers (data block + JSX kept vs YAML + JSX stripped) — gap noted
  against dot-do/ui.
- packages/@mdxe/hono/package.json: union — ./format and ./text exports, workers-pool test
  script, @cloudflare/vitest-pool-workers + workers-types devDeps, mdxld workspace:^;
  @mdxui/text ^0.1.0 and @mdxui/fumadocs ^1.9.0 stay registry ranges.
- pnpm-lock.yaml: regenerated with `pnpm install --config.link-workspace-packages=true`;
  diff vs origin/integrate/mdx-8je is the hono importer (+2 devDeps, mdxld specifier) and the
  mdxe importer mirroring its merged package.json.
- test/repo/mdxui-home.test.ts: the md witness accepts `@mdxui/text/md` or `/core` from
  format.ts or text.ts (render path moved into text.ts under mdx-8je.18).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@nathanclevenger nathanclevenger changed the title integrate(mdx-8je) rounds 1-2: mdxui leaves the repo (mdx-8je.8); one diff implementation, 3-way mergeExtract, render→extract identity (mdx-8je.12); .18 held integrate(mdx-8je) rounds 1-3: mdxui leaves the repo (.8); one diff implementation (.12); hono serves text registers by Accept (.18) Sep 9, 2026
The merge commit carried the .18 branch's lockfile verbatim; this is the
`pnpm install --config.link-workspace-packages=true` result for the merged
package.jsons. Diff vs b629c8a is the hono importer (mdxld → workspace:^,
@cloudflare/vitest-pool-workers + workers-types devDeps) and the mdxe importer.
`pnpm install --frozen-lockfile` and test/repo/lockfile-drift are green.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@nathanclevenger
nathanclevenger merged commit e88b662 into main Sep 9, 2026
1 check passed
@claude

claude Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review

This is a large, well-documented integration PR (mdxui removal + @mdxld/diff/extract/markdown consolidation). The PR description already does an unusually thorough self-audit with verification tables, so I focused on reading the actual diffs of the substantive (non-deletion) files rather than re-deriving what's already reported.

Code quality / correctness

@mdxld/diff — merge3way rewrite (packages/@mdxld/diff/src/index.ts)
Replacing the jsdiff-based patch/parse round-trip with a direct hunk-based 3-way merge is a solid improvement — it fixes the [object Object] conflict-body bug called out in the PR description and is a more standard diff3 (base/ours/theirs with ||||||| base markers). The hunk-growing/touches() logic correctly implements git's "touching changes conflict" rule. One minor nit: the trailing-newline heuristic

const trailing = oursTrailing !== baseLines.trailing ? oursTrailing : theirsTrailing

is asymmetric — if both sides change the trailing newline differently from base, ours silently wins with no conflict recorded. Very much an edge case, but worth a one-line comment or a test if it isn't already covered, since everywhere else in this file conflicting changes are explicitly surfaced rather than silently resolved.

merge3wayObjects / diffPaths / applyPaths (same file)
Clean, well-structured path-based 3-way merge with reasonable conflict semantics (text conflicts get a merge3way attempt before falling back to onConflict). clone() uses JSON.parse(JSON.stringify()), which is fine for the JSON-LD-shaped data this operates on but will silently drop undefined values / functions / Dates if ever fed richer objects — worth keeping in mind if this primitive gets reused outside the extract/markdown use case.

@mdxld/extract/src/extract.ts — brace-balanced slot parsing + extractWithAI
The switch from a flat {([^{}]+)} regex to balancedBraces() is a real correctness fix (handles template literals/nested expressions in loop bodies). The extractWithAI implementation is now real (previously a stub) and reasonably defensive — it falls back from getPath(object, path) to a literal-key lookup in case the model flattens dotted paths, and only overwrites unmatched entries the model actually answered.

Minor: getPath/setPath are reimplemented locally in extract.ts (lines ~306–336) with near-identical logic to the unexported helpers in @mdxld/diff/src/index.ts. Not a bug, and these are generic path utilities rather than "diff implementations" so the diff-home.test.ts repo guard correctly doesn't flag it, but given the PR's explicit "one diff implementation" goal it might be worth exporting these from @mdxld/diff too rather than maintaining two copies.

@mdxe/hono/src/format.ts + new text.ts
The Accept/extension content-negotiation rewrite (negotiateFormat, contextFromAccept/contextFromExtension, fail-closed 406 problem+json) is a nice improvement over the old best-effort getFormatFromAccept string-includes checks — the old version had a real bug (accept.includes('text/plain') before the more specific checks could easily mis-rank an Accept header) that's fixed by delegating to the shared parseAccept. formatResponse/serveDocument becoming async/Promise<Response> is a breaking API change but both call sites in server.ts correctly return the promise directly, and Hono supports async handlers natively, so this looks safe.

@mdxld/markdown/src/table.ts
Good consolidation of table parsing/rendering into one shared primitive consumed by both @mdxld/markdown and @mdxld/extract/entity.ts. Bonus: it also fixes a latent bug in the old fromMarkdown/parseMarkdownTable code, which unconditionally did lines.slice(2) assuming line 2 was always a separator row — the new parseTable actually checks isTableSeparator() first.

Test coverage

Test coverage for the new/changed logic looks appropriately targeted: round-trip identity tests against the published @mdxui/text fixtures, dedicated 3-way merge tests, mocked AI-extraction tests, and the test/repo/diff-home.test.ts repo-level guard that runs a runtime witness (not just a source grep) proving @mdxld/extract's diff/applyExtract/mergeExtract literally delegate to @mdxld/diff's implementations. That's a good pattern for enforcing "one implementation" invariants that a simple lint rule would miss.

Scope / process

  • The mass deletion of packages/mdxui, packages/@mdxui/*, and apps/storybook (the bulk of the diff's line count) is consistent with the stated intent (dot-do/ui now owns these) and CLAUDE.md's documented package-allowlist policy.
  • Appropriately holding back mdx-8je.18 (the .md renderer register conflict between @mdxui/text/md and @mdxui/text/core) rather than merging a regression is the right call — the PR body's reasoning for the hold is concrete and testable (shows the actual byte-level output divergence).
  • No security concerns spotted: the 406 responses/problem+json don't reflect unsanitized input beyond echoing the Accept header value (which is appropriate/expected for content negotiation errors), and the AI-extraction path only sends template/rendered content to the configured model, nothing sensitive.

Overall this reads as careful, well-tested work with good self-documentation. The nits above are minor and not blocking.

@claude

claude Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review: integrate(mdx-8je) rounds 1-3

This is a large, unusually well-self-documented integration PR (three rounds merged, each with its own intent/verification writeup). That transparency makes review much easier, but the PR is also big enough (2,400+ files touched overall per the merge commit, and GitHub's own diff view hits its 300-file cap) that I focused on the substantive new code — @mdxe/hono's text.ts/format.ts (mdx-8je.18) and @mdxld/diff/@mdxld/extract (mdx-8je.12) — rather than the mechanical @mdxui/storybook deletions, which read as a straightforward, already-reviewed removal.

Findings

1. Changeset likely under-bumps a breaking change (.changeset/one-diff-implementation.md)
The changeset marks @mdxld/diff, @mdxld/extract, @mdxld/markdown, and @mdxld/formats all as minor. But @mdxld/markdown (currently 1.9.2, i.e. already ≥1.0) has its diff, applyExtract, and DiffResult exports removed entirely (confirmed in packages/@mdxld/markdown/src/index.ts, which now just points callers at @mdxld/diff/@mdxld/extract). Under semver that's a breaking change and should be major for @mdxld/markdown at least — a minor bump lets anyone on a ^1.9.2 range pull this in and have their import silently start failing.

2. The "known gap" between hono's text/markdown and workers' .md asset is more than a footnote
packages/@mdxe/hono/src/format.test.ts (lines 9-14) and the PR body both document that, for the same MDXLD document, Accept: text/markdown on this hono face (via @mdxui/text/core) and the static .md asset served by @mdxe/workers (via @mdxui/text/md) now produce different bytes: core turns frontmatter into a ## Type heading block and keeps JSX verbatim in the body, while /md strips JSX and keeps YAML frontmatter. Two consequences worth weighing before merge rather than after:

  • Any document with components in the body (e.g. <Hero title="x" />) served via content negotiation leaks raw JSX syntax — and whatever is in the component's props — into what's supposed to be a clean, agent-consumable text/markdown response.
  • The round-2 entry in this same PR explicitly said "Human decision needed: which register is the md face" and held the merge on that basis. Round 3's writeup says the hold "was decided for .18's contract," but reads as the integrator agent making that call itself rather than recording a human sign-off. Given it's a semantic/contract decision (which of two published renderers is the source of truth for the agent-facing register), I'd want an explicit human answer to that question before this merges, not just the gap being filed as mdx-w89.15 against the upstream package.

3. Minor: CDATA injection in documentToXml (packages/@mdxe/hono/src/format.ts)

<content><![CDATA[${doc.content}]]></content>

doc.content is interpolated raw into a CDATA section. If the document body contains the literal sequence ]]>, the CDATA section terminates early and the remainder is parsed as XML markup instead of literal text. Low severity (this is the XML fallback face, and the content is presumably author-controlled MDX), but easy to fix by escaping ]]> → ]]]]><![CDATA[> before interpolating, and it's a small enough fix to bundle here given format.ts is already being rewritten in this round.

What looks solid

  • text.ts/format.ts: the q-ordered negotiateFormat, fail-closed 406 with available media types, and FormatContext.decidedBy/capabilities plumbing are clean and well-tested (format.test.ts covers q-ordering, refusal, extension-vs-Accept precedence, and the token headers).
  • @mdxld/diff's merge3way/merge3wayObjects: a reasonable git-style region-growing algorithm, conflicts are reported rather than silently dropped, and extractWithAI cleanly treats ai-functions as an optional peer (dynamic import, clear error, injectable generate) rather than a hard dependency.
  • The per-round verification tables (test counts, typecheck/lint deltas vs main) are a good practice worth keeping for future integration PRs of this shape.

Process note

Bundling three independently-scoped issues (.8, .12, .18) plus a lockfile regen into one PR/branch, with an automated integrator resolving cross-round conflicts, works here because the writeup is thorough — but it does mean a single approval is being asked to cover a semver-relevant API removal, a content-negotiation semantics decision, and a mechanical package removal all at once. Landing each round as its own PR going forward would make review and any future revert much cheaper.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant