feat(core,jsx): add ui.markdown widget (GFM subset) + streaming#415
Merged
Conversation
Adds ui.markdown(source, props?) as an experimental-tier composition widget: a zero-dependency, bounded GFM-subset parser plus a renderer that maps the AST onto existing widgets, so layout, theming, and renderer behavior are inherited rather than reimplemented. Blocks: ATX headings, paragraphs (soft joins, hard breaks), fenced and indented code with CodeEditor-tokenizer monochrome syntax emphasis, blockquotes, nested ordered/unordered/task lists, thematic breaks, and pipe tables with alignment. Inlines: strong/em/del, code spans, links, autolinks, bare URLs with punctuation trimming, escapes, and basic entities. Raw HTML renders as literal text. Safety: parsing never throws, nesting depth is bounded, and inline scanning runs on a work budget with closer memoization so adversarial input (PR bodies, agent output) cannot trigger quadratic scans. parseMarkdown/renderMarkdown and the frozen MarkdownDocument AST are exported for caller-side caching of streamed content; styling is attribute-only so output adapts to every theme. JSX exposes <Markdown source>, registered in the intrinsic factory map and pinned by the existing ui parity tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds an experimental Markdown feature: frozen AST types, a bounded deterministic parser, an AST→VNode renderer, createMarkdownStream incremental streaming with vnode caching, ui.markdown helper, JSX/intrinsic support, tests (unit/fuzz/integration), and user docs/changelog/navigation updates. ChangesMarkdown Widget Core Implementation and Wiring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Comment |
Streams agent transcripts and live logs cheaply: under append-only input every top-level block except the last is immutable, so the stream re-parses only the volatile tail block per append and caches completed blocks plus their rendered VNodes. Completed blocks keep referential identity (including the block that just stopped being last, restored via structural comparison), so reconciliation and layout stability signatures skip untouched subtrees and appends stay O(tail) instead of O(document). Chunk boundaries never change the result: CRLF pairs split across appends are carried, NUL bytes are sanitized per chunk, and fuzz tests pin document() to deep-equal parseMarkdown(source()) under random chunking, interleaved queries, resets, and unicode soup. Exports createMarkdownStream/MarkdownStream/renderMarkdownBlock from core; parseMarkdownLines stays module-internal with per-block start tracking in the block parser. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
RtlZeroMemory
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/core/src/widgets/types/base.ts (1)
402-408: ⚡ Quick winUse spacing-scale values for
blockGapinMarkdownProps.Line 407 exposes spacing as a raw
number; this diverges from the spacing-key contract and from existing spacing prop patterns in this file.♻️ Proposed API-aligned type update
export type MarkdownProps = Readonly<{ key?: string; /** Markdown source text (GitHub-Flavored-Markdown subset). */ source: string; /** Vertical spacing rows between top-level blocks. Defaults to 1. */ - blockGap?: number; + blockGap?: SpacingValue; }>;As per coding guidelines, “Spacing values use spacing keys such as
sm,md,lgrather than arbitrary values.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/widgets/types/base.ts` around lines 402 - 408, The MarkdownProps type exposes blockGap as a raw number which breaks the project's spacing-key contract; update MarkdownProps to use the shared spacing key type (e.g., the project's Spacing or SpacingKey union/type) for blockGap instead of number and adjust any related consumers (renderers or defaultProps) to accept/resolve spacing keys via the project's spacing resolver utility; specifically change the blockGap declaration inside MarkdownProps and ensure places referencing MarkdownProps (render functions or default values) convert that spacing key to an actual numeric gap using the existing spacing resolver function.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Line 11: Update the CHANGELOG entry to avoid implying a built-in streaming
mode: edit the sentence that currently reads "for caller-side caching
(streaming)" to a clarifying phrase such as "for caller-side caching
(streaming-friendly)" or "for caller-side caching; streaming mode planned" so it
accurately reflects that parseMarkdown/renderMarkdown and the frozen
MarkdownDocument AST types support caller-side caching but do not provide a
built-in streaming mode; ensure the note appears alongside references to
ui.markdown and the exported symbols parseMarkdown, renderMarkdown, and
MarkdownDocument.
---
Nitpick comments:
In `@packages/core/src/widgets/types/base.ts`:
- Around line 402-408: The MarkdownProps type exposes blockGap as a raw number
which breaks the project's spacing-key contract; update MarkdownProps to use the
shared spacing key type (e.g., the project's Spacing or SpacingKey union/type)
for blockGap instead of number and adjust any related consumers (renderers or
defaultProps) to accept/resolve spacing keys via the project's spacing resolver
utility; specifically change the blockGap declaration inside MarkdownProps and
ensure places referencing MarkdownProps (render functions or default values)
convert that spacing key to an actual numeric gap using the existing spacing
resolver function.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 71c7f238-c547-480e-aa8a-ad750c7202ce
📒 Files selected for processing (23)
CHANGELOG.mdROADMAP.mddocs/widgets/index.mddocs/widgets/markdown.mddocs/widgets/stability.mdmkdocs.ymlpackages/core/etc/core.api.mdpackages/core/src/index.tspackages/core/src/widgets/__tests__/markdown.test.tspackages/core/src/widgets/markdown/__tests__/parse.fuzz.test.tspackages/core/src/widgets/markdown/__tests__/parse.test.tspackages/core/src/widgets/markdown/ast.tspackages/core/src/widgets/markdown/index.tspackages/core/src/widgets/markdown/parse.tspackages/core/src/widgets/markdown/render.tspackages/core/src/widgets/types.tspackages/core/src/widgets/types/base.tspackages/core/src/widgets/ui.tspackages/jsx/etc/jsx.api.mdpackages/jsx/src/components.tspackages/jsx/src/createElement.tspackages/jsx/src/index.tspackages/jsx/src/types.ts
The pre-push lint run piped output through tail, which masked a non-zero exit; CI caught the two unformatted files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/core/src/widgets/markdown/stream.ts (1)
31-42: ⚡ Quick winUse an
interfacefor the publicMarkdownStreamshape.This exported object-shape type is a
Readonly<{ ... }>alias today. The repo standard for TypeScript object shapes isinterface, so this public API should follow that convention for consistency.♻️ Proposed refactor
-export type MarkdownStream = Readonly<{ - /** Appends a source chunk. Chunks may split lines, CRLF pairs, or words. */ - append: (chunk: string) => void; - /** Clears the buffer; optionally replaces it with new source. */ - reset: (source?: string) => void; - /** Full (normalized) source buffered so far. */ - source: () => string; - /** Parsed document; stable blocks keep referential identity across appends. */ - document: () => MarkdownDocument; - /** Rendered document; stable blocks reuse cached VNodes across appends. */ - vnode: () => VNode; -}>; +export interface MarkdownStream { + /** Appends a source chunk. Chunks may split lines, CRLF pairs, or words. */ + readonly append: (chunk: string) => void; + /** Clears the buffer; optionally replaces it with new source. */ + readonly reset: (source?: string) => void; + /** Full (normalized) source buffered so far. */ + readonly source: () => string; + /** Parsed document; stable blocks keep referential identity across appends. */ + readonly document: () => MarkdownDocument; + /** Rendered document; stable blocks reuse cached VNodes across appends. */ + readonly vnode: () => VNode; +}As per coding guidelines, "Prefer
interfacefor defining object shapes in TypeScript files".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/widgets/markdown/stream.ts` around lines 31 - 42, Replace the exported Readonly type alias for MarkdownStream with an exported interface named MarkdownStream that declares the same members (append, reset, source, document, vnode) as readonly method properties; locate the current symbol MarkdownStream and update its definition so the public API uses "interface MarkdownStream" (preserving parameter and return types for append, reset, source, document, and vnode) to conform to the repo convention preferring interfaces for object shapes.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/widgets/markdown/__tests__/stream.test.ts`:
- Around line 29-31: The one-line chained call in renderText should be split
across multiple lines to satisfy Biome: call createTestRenderer({ viewport: {
cols: 44, rows: 36 } }) on its own line, then call .render(vnode) on the next
line, and then call .toText() on the following line; update the function
renderText accordingly so the chain is formatted across multiple lines while
keeping the same arguments and return value.
In `@packages/core/src/widgets/markdown/stream.ts`:
- Around line 78-82: The normalization chain assigned to variable normalized
spans multiple lines and fails Biome's formatting rule; collapse the chained
string operations into a single-line expression (keep the same operations:
replace CRLF to LF, replace CR to LF, split by NUL and join with "�") so the
assignment to normalized uses one line and retains the same behavior (refer to
the normalized variable, text input, and NUL constant in stream.ts).
- Around line 129-130: The source() function currently appends a raw "\r" when
pendingCR is true which breaks normalization across CRLF boundaries; update
source() (which uses variables lines and pendingCR) to append a normalized
"\r\n" instead of "\r" so the returned buffered source always represents CRLF as
"\r\n" (i.e., change the return from lines.join("\n") + (pendingCR ? "\r" : "")
to append "\r\n" when pendingCR is true).
---
Nitpick comments:
In `@packages/core/src/widgets/markdown/stream.ts`:
- Around line 31-42: Replace the exported Readonly type alias for MarkdownStream
with an exported interface named MarkdownStream that declares the same members
(append, reset, source, document, vnode) as readonly method properties; locate
the current symbol MarkdownStream and update its definition so the public API
uses "interface MarkdownStream" (preserving parameter and return types for
append, reset, source, document, and vnode) to conform to the repo convention
preferring interfaces for object shapes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1b41ff6c-66d9-4724-a657-e99b398286b0
📒 Files selected for processing (11)
CHANGELOG.mdROADMAP.mddocs/widgets/markdown.mdpackages/core/etc/core.api.mdpackages/core/src/index.tspackages/core/src/widgets/markdown/__tests__/stream.fuzz.test.tspackages/core/src/widgets/markdown/__tests__/stream.test.tspackages/core/src/widgets/markdown/index.tspackages/core/src/widgets/markdown/parse.tspackages/core/src/widgets/markdown/render.tspackages/core/src/widgets/markdown/stream.ts
✅ Files skipped from review due to trivial changes (2)
- docs/widgets/markdown.md
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/core/src/widgets/markdown/index.ts
- packages/core/src/widgets/markdown/parse.ts
- packages/core/src/widgets/markdown/render.ts
Comment on lines
+129
to
+130
| function source(): string { | ||
| return lines.join("\n") + (pendingCR ? "\r" : ""); |
Contributor
There was a problem hiding this comment.
Normalize the pending trailing CR in source().
When the last appended chunk ends with a bare \r, source() returns that raw carriage return even though this API is documented as the normalized buffered source. That makes source() observably inconsistent on a real CRLF-split boundary.
🐛 Proposed fix
function source(): string {
- return lines.join("\n") + (pendingCR ? "\r" : "");
+ return lines.join("\n") + (pendingCR ? "\n" : "");
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/widgets/markdown/stream.ts` around lines 129 - 130, The
source() function currently appends a raw "\r" when pendingCR is true which
breaks normalization across CRLF boundaries; update source() (which uses
variables lines and pendingCR) to append a normalized "\r\n" instead of "\r" so
the returned buffered source always represents CRLF as "\r\n" (i.e., change the
return from lines.join("\n") + (pendingCR ? "\r" : "") to append "\r\n" when
pendingCR is true).
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/src/widgets/markdown/__tests__/stream.test.ts (1)
98-101: ⚡ Quick winConsider adding defensive assertions for the children array.
The test assumes
childrenexists and has at least 3 elements. While the optional chaining?.prevents crashes, ifchildrenisundefinedor has fewer elements, the assertions would pass withundefined === undefined, missing the actual cache validation.🛡️ Proposed defensive assertions
assert.ok(first.kind === "column" && second.kind === "column"); +assert.ok(first.children !== undefined && first.children.length === 3); +assert.ok(second.children !== undefined && second.children.length === 3); assert.equal(second.children?.[0], first.children?.[0], "heading vnode cached"); assert.equal(second.children?.[1], first.children?.[1], "paragraph vnode cached"); assert.notEqual(second.children?.[2], first.children?.[2], "tail re-rendered");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/widgets/markdown/__tests__/stream.test.ts` around lines 98 - 101, The test assumes first.children and second.children exist and have at least 3 entries but only uses optional chaining, which can produce false positives; add defensive assertions that first.children and second.children are defined and that their .length is >= 3 before the equality checks, then assert the individual elements (first.children[0], first.children[1], first.children[2], and corresponding second.children indexes) are not undefined where appropriate, and keep the existing comparisons (first.kind/second.kind and element equality/inequality) after those guards to ensure the cache validation is meaningful.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/core/src/widgets/markdown/__tests__/stream.test.ts`:
- Around line 98-101: The test assumes first.children and second.children exist
and have at least 3 entries but only uses optional chaining, which can produce
false positives; add defensive assertions that first.children and
second.children are defined and that their .length is >= 3 before the equality
checks, then assert the individual elements (first.children[0],
first.children[1], first.children[2], and corresponding second.children indexes)
are not undefined where appropriate, and keep the existing comparisons
(first.kind/second.kind and element equality/inequality) after those guards to
ensure the cache validation is meaningful.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 29520fda-c053-4e52-8f74-a16d38ec4146
📒 Files selected for processing (2)
packages/core/src/widgets/markdown/__tests__/stream.test.tspackages/core/src/widgets/markdown/stream.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/widgets/markdown/stream.ts
Per-side borders: a box with only vertical edges enabled (for example borderLeft alone) previously rendered nothing when shorter than two rows, because the minimum-size guard always required corner rows. Corner rows are now required only when a horizontal edge is present, so left/right bars render from h >= 1. Markdown blockquotes use this to render a GitHub-style dim left bar instead of a rounded box. app.ready(): resolves once start()/run() has finished starting and the app accepts update() calls — removes the startup race for timers, sockets, and streams that feed state while run() blocks (calling update() during startup throws ZRUI_INVALID_STATE, found while verifying the markdown stream in a live PTY). Resolves immediately when already running; rejects on startup failure or dispose before start. Wired through the inspector overlay app wrapper. Vertical-divider height hugging was investigated and deliberately not changed: stack stretch reports the loose cross limit at measure time, so hugging requires a stretch-semantics redesign out of scope here; the per-side border fix covers the bar use cases. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/src/widgets/__tests__/boxBorderSides.test.ts (1)
10-106: ⚡ Quick winTest coverage validates per-side border behavior.
The test suite effectively validates the new vertical-only h=1 capability and provides regression coverage for horizontal-only and full borders. All tests follow behavior-first design by asserting on rendered text output.
📋 Optional: Add test for top+bottom h=1 edge case
Consider adding a test case for
top=true, bottom=true, left=false, right=falseat h=1 to document the intended behavior for the overlap scenario flagged in the boxBorder.ts review:test("top and bottom without vertical edges at h=1", () => { const lines = render( ui.box( { border: "single", borderTop: true, borderRight: false, borderBottom: false, borderLeft: false, width: 6, height: 1, }, [], ), ); // Assert expected behavior (currently unclear whether this should render both edges overlapping or be prevented) });This would clarify the contract for reviewers and future maintainers.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/widgets/__tests__/boxBorderSides.test.ts` around lines 10 - 106, Add a test case named "top and bottom without vertical edges at h=1" to the existing boxBorderSides.test suite that uses ui.box(border: "single", borderTop: true, borderBottom: true, borderLeft: false, borderRight: false, width: 6, height: 1) with empty content and calls render(ui.box(...)); assert the rendered output documents the current behavior (e.g., a single-line horizontal rule at h=1 — check lines.length === 1 and that lines[0] startsWith the horizontal glyphs like "──" and does not contain vertical bar "│") so reviewers and future maintainers have an explicit test for the top+bottom overlap case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/core/src/widgets/__tests__/boxBorderSides.test.ts`:
- Around line 10-106: Add a test case named "top and bottom without vertical
edges at h=1" to the existing boxBorderSides.test suite that uses ui.box(border:
"single", borderTop: true, borderBottom: true, borderLeft: false, borderRight:
false, width: 6, height: 1) with empty content and calls render(ui.box(...));
assert the rendered output documents the current behavior (e.g., a single-line
horizontal rule at h=1 — check lines.length === 1 and that lines[0] startsWith
the horizontal glyphs like "──" and does not contain vertical bar "│") so
reviewers and future maintainers have an explicit test for the top+bottom
overlap case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 81ea0028-f7b4-4090-a3d3-3dca7328c4ab
📒 Files selected for processing (12)
CHANGELOG.mddocs/guide/lifecycle-and-updates.mddocs/widgets/markdown.mdpackages/core/etc/core.api.mdpackages/core/src/app/__tests__/appReady.test.tspackages/core/src/app/createApp.tspackages/core/src/app/inspectorOverlayHelper.tspackages/core/src/app/types.tspackages/core/src/renderer/renderToDrawlist/boxBorder.tspackages/core/src/widgets/__tests__/boxBorderSides.test.tspackages/core/src/widgets/__tests__/markdown.test.tspackages/core/src/widgets/markdown/render.ts
✅ Files skipped from review due to trivial changes (2)
- docs/guide/lifecycle-and-updates.md
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/core/src/widgets/tests/markdown.test.ts
- docs/widgets/markdown.md
- packages/core/etc/core.api.md
- packages/core/src/widgets/markdown/render.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
ui.markdown(source, props?)— an experimental-tier widget rendering a GitHub-Flavored-Markdown subset — pluscreateMarkdownStream()for append-only sources, completing the roadmap item.Architecture: composition factory. A zero-dependency parser produces a frozen
MarkdownDocumentAST; the renderer maps it onto existing widgets (text,richText,box,row,column,divider) — no new VNode kind, no new layout/render handlers, so proven layout/theming/renderer behavior is inherited.Supported
:-/:-:/-:alignment**strong**,*em*,~~del~~, code spans, links/autolinks/bare URLs (trailing punctuation trimmed), backslash escapes, basic entitiesStreaming (
createMarkdownStream)Under append-only input, every top-level block except the last is immutable. The stream:
document()deep-equalsparseMarkdown(source())at every point — chunk boundaries (split CRLF pairs, mid-word, mid-line) never change the result, pinned by fuzzSafety (untrusted input: PR bodies, agent output)
*/[/backtick floods) cannot trigger quadratic scansAPI
ui.markdown(source, { blockGap?, key? })· JSX<Markdown source>(intrinsic + component, pinned by the ui-parity tests)createMarkdownStream(options?)→append/reset/source/document/vnodeparseMarkdown/renderMarkdown/renderMarkdownBlock+ frozen AST typesDocs
Widget page (usage, props, supported-syntax table, divergences, Streaming section), catalog + stability rows (
experimental), mkdocs nav, CHANGELOG.Framework fixes (from live-PTY verification)
borderLeftalone previously drew nothing on boxes shorter than 2 rows (corner-row guard); corner rows are now required only when a horizontal edge is present. Blockquotes now render the intended GitHub-style dim left bar.app.ready()— resolves oncestart()/run()finishes starting andupdate()is accepted; removes the startup race for timers/sockets/streams feeding state whilerun()blocks (found live: earlyupdate()throwsZRUI_INVALID_STATE). Rejects on startup failure or dispose-before-start.Validation
npm run test:packages: 4,997 pass / 0 failapp.ready()lifecyclelintclean ·typecheck+typecheck:strictclean ·quality:apiregenerated (core + jsx) ·quality:depsno new violations ·quality:guardrailsOK ·docs:buildOK🤖 Generated with Claude Code