feat(design): add agent-consumable design surface#336
Conversation
f216520 to
7183127
Compare
bntvllnt
left a comment
There was a problem hiding this comment.
Review verdict: BLOCKING
I found one release-blocking rendering issue in the new /design page.
Finding:
- apps/registry/app/design/page.tsx:118 — DESIGN.md is rendered through MDXContent without GFM/table support. The built /design page renders the Markdown pipe tables as literal paragraph text (verified in apps/registry/.next/server/app/design.html), so the brand attributes, semantic tokens, typography, spacing, radius, elevation, component patterns, and anti-pattern tables are not actually usable on the public design guide. Since this PR is explicitly adding the human/agent-consumable design surface, the route needs to render DESIGN.md tables correctly before ship. Use a renderer path with remark-gfm/table support or otherwise map tables to proper HTML.
Validation evidence:
- Head verified: 7183127 (matches task head).
- git diff --check origin/main...HEAD: PASS.
- pnpm install --frozen-lockfile --offline: PASS after initial missing node_modules.
- pnpm -F @vllnt/ui-registry exec tsc --noEmit: PASS.
- pnpm -F @vllnt/ui exec tsc --noEmit --project tsconfig.build.json: PASS.
- Targeted registry eslint command from PR body: PASS.
- pnpm -F @vllnt/ui-registry build: PASS and generated /design static output.
- ReactMarkdown table probe without remark-gfm rendered a pipe table as a paragraph; generated .next/server/app/design.html contains literal "| Attribute | Value |" inside a
.
- Remote gh checks for PR #336 are green (Quality Gates, issue-link gate, Verify Stories, Storybook/visual, Vercel previews).
Note: GitHub rejected formal REQUEST_CHANGES because the authenticated account owns this PR, so this COMMENT review carries the blocking verdict.
| accessibility, and component composition across the library. | ||
| </p> | ||
| </div> | ||
| <MDXContent |
There was a problem hiding this comment.
DESIGN.md contains multiple pipe tables, but MDXContent currently falls back to react-markdown without remark-gfm; I verified the production build emits those tables as literal paragraph text (for example .next/server/app/design.html contains | Attribute | Value | inside a <p>). Because this PR is adding /design as the public human/agent-consumable design surface, this makes large parts of the guide (brand attributes, tokens, typography, spacing, anti-patterns) unreadable. Please render this route with GFM/table support (or map tables explicitly) before ship.
bntvllnt
left a comment
There was a problem hiding this comment.
Review verdict: VERIFIED CLEAN for source remediation; external status remains blocking
I re-reviewed PR #336 at head d89952d after the GFM table and dependency-metadata follow-up.
Verified clean:
- Original /design table blocker is fixed. Both MDXContent render paths import remark-gfm and pass it to ReactMarkdown via remarkPlugins:
- packages/ui/src/components/mdx-content/mdx-content.tsx:5,139,210-223
- apps/registry/registry/default/mdx-content/mdx-content.tsx:5,139,210-223
- Regression coverage is present: packages/ui/src/components/mdx-content/mdx-content.test.tsx:7-24 renders a GFM pipe table and asserts table/columnheader/cell roles.
- Dependency metadata follow-up is fixed:
- packages/ui/package.json:145-146 declares react-markdown and remark-gfm in runtime dependencies.
- apps/registry/registry.json:2293-2298 declares @vllnt/ui, @mdx-js/mdx, react-markdown, and remark-gfm for the mdx-content registry item.
- pnpm-lock.yaml importer metadata includes remark-gfm under packages/ui dependencies.
- PR body matches the current head and includes the dependency follow-up, refreshed validation, and the remaining Vercel caveat.
Validation run in a fresh review clone:
- Head verified: d89952d.
- gh pr checks: all GitHub/CodeQL/storybook/visual/quality checks pass except Vercel - ui.vllnt.ai, which is canceled from Vercel Dashboard.
- git diff --check origin/main...HEAD: PASS.
- pnpm install --frozen-lockfile --offline: PASS.
- pnpm -F @vllnt/ui exec vitest run src/components/mdx-content/mdx-content.test.tsx: PASS (1 file, 1 test).
- pnpm -F @vllnt/ui exec eslint src/components/mdx-content/mdx-content.tsx src/components/mdx-content/mdx-content.test.tsx: PASS.
- pnpm -F @vllnt/ui exec tsc --noEmit --project tsconfig.build.json: PASS.
- pnpm -F @vllnt/ui-registry exec eslint app/design/page.tsx app/design.txt/route.ts app/r/design.json/route.ts app/llms.txt/route.ts app/llms-full.txt/route.ts app/sitemap.ts components/header/header.tsx components/footer/footer.tsx lib/design-guide.ts --no-warn-ignored: PASS.
- pnpm -F @vllnt/ui-registry exec tsc --noEmit: PASS.
- pnpm -F @vllnt/ui-registry build: PASS; apps/registry/.next/server/app/design.html exists, contains , and does not contain the literal pipe-table text "| Attribute | Value |".
Remaining blocker:
- Non-code external: Vercel - ui.vllnt.ai is canceled/failed from the Vercel Dashboard. I found no remaining code blockers in the table/dependency remediation. This should be cleared or explicitly accepted before merge.
Note: GitHub normally rejects formal APPROVE/REQUEST_CHANGES for this authenticated account on self-authored PRs, so this COMMENT review carries the review verdict.
bntvllnt
left a comment
There was a problem hiding this comment.
@bntvllnt Review verdict: APPROVE recommended for reviewer triage — stale Vercel waived by PM decision
Head reviewed: d89952d8556ade303c6c63a22efd83202676f431
I re-checked the current status against the PM waiver in pm-decision.md: all GitHub Actions/code checks are green and Vercel – storybook is SUCCESS. The remaining Vercel – ui.vllnt.ai: FAILURE matches the accepted stale external UI preview status for reviewer triage only.
No new source-level blockers found beyond the prior current-head clean review. This is not merge/release approval and does not weaken branch protection; it only records that the stale UI preview status is waived for review triage under the PM decision.
|
Preview ready · Updated 2026-05-20T15:48:05Z
Inspect
|
…r build context The /design.txt static export failed in CI because DESIGN.md was never copied into the Docker build context. Two surgical fixes: 1. Dockerfile: add `COPY DESIGN.md ./` so it is available during `RUN pnpm --filter @vllnt/ui-registry build`. 2. design-guide.ts: prepend a fileURLToPath-anchored candidate so path resolution is CWD-independent for any future invocation context.
bntvllnt
left a comment
There was a problem hiding this comment.
Review outcome: APPROVE recommended for human/manual approval.
I re-reviewed the prior metadata-only blocker on the live PR head e457eaa35ec758074f4151c9844c7a6b820b3109.
BLOCKING
- None.
VERIFIED CLEAN
- Live PR head still equals
e457eaa35ec758074f4151c9844c7a6b820b3109; PR is OPEN, non-draft,MERGEABLE/CLEAN. - Prior R3 blocker is remediated: the PR body now cites the current head and no longer contains the stale earlier-head validation references
71831272d4fe817465186b3c32873c12884281ca,e828be4fbd02872fb305ccd3cb6426b612c6897e, ord89952d8556ade303c6c63a22efd83202676f431. - Prior dependency-policy gap is remediated: the PR body now includes a
remark-gfmdependency tradeoff note covering runtime need, bundle/runtime impact, maintenance surface, and security posture. - Current check rollup is terminal green: 13 checks observed, no pending/failing checks.
VALIDATION
- Re-fetched live PR metadata and reviews from GitHub immediately before this review.
- Verified current-head/body remediation against
AGENTS.mdR3/R5/dependency-note requirements anddocs/agents/RULES.mdR3/R5/R6/out-of-scope dependency policy. - This pass is metadata-only. I did not mutate code, merge, publish, or release. Final GitHub approval remains reserved for bntvllnt.
Review — 1 blocking process findingBLOCKING
WARN
VERIFIED CLEAN
VALIDATION
Requesting changes for the missing PR-body gate. Once the body is refreshed with issue link, validation/evidence, design deviation status, and dependency tradeoff, the code itself looks close. |
bntvllnt
left a comment
There was a problem hiding this comment.
Review — 1 finding (1 blocking, 0 warn)
Note: GitHub rejected REQUEST_CHANGES because the authenticated account is the PR author, so I am publishing this as a blocking COMMENT review instead. Treat this as request-changes-equivalent until the inline comment block is removed.
BLOCKING
T1 — Remove inline comments from shipped app code- Evidence:
apps/registry/lib/design-guide.ts:10-15 - Why it matters:
AGENTS.mdpoints reviewers to the blocking repo rules, and R10 says shipped code should not add inline//comments. This helper ships in the registry app, so keeping a six-line implementation note here leaves the PR violating the documented rule even though the functional gates are green. - Fix: remove the comment block and let the ordered
DESIGN_FILE_CANDIDATESentries speak for themselves, or encode the context in clearer constant names if needed.
- Evidence:
WARN
- None.
VERIFIED CLEAN
- Inspected all 24 changed files on head
e457eaa35ec758074f4151c9844c7a6b820b3109: PR template, AGENTS/CONTRIBUTING/DESIGN docs, Dockerfile, design routes/page, llms routes, sitemap, header/footer links, design-guide helper, registry metadata, MDX GFM support, design token files/schema/docs, Storybook token story, package and lockfile changes. - The design guide is exposed through
/design,/design.txt,/r/design.json,/llms.txt,/llms-full.txt, sitemap, header/footer links, and Docker build context. - MDXContent now uses
remark-gfmin both the package source and registry copy, with a focused table rendering test. - PR remains linked to #250 and CI checks are green.
VALIDATION
- Ran:
pnpm install --frozen-lockfile --offline— passed. - Ran:
git diff --check origin/main...HEAD— passed. - Ran:
python3JSON parse forpackages/design/tokens.json,packages/design/tokens.schema.json, andapps/registry/registry.json— passed. - Ran:
pnpm -F @vllnt/ui lint— passed. - Ran:
pnpm -F @vllnt/ui exec tsc --noEmit --project tsconfig.build.json— passed. - Ran:
pnpm test:once— passed, 217 files / 1216 tests. - Ran:
pnpm build— passed; Next build emitted an existing-style NFT tracing warning but completed successfully. - Checked:
gh pr checks 336 --repo vllnt/ui --watch=false— all listed checks passed, including Quality Gates, Storybook Tests, Visual Regression, CodeQL, deployments, and preview deploy.
| readonly title: string; | ||
| }; | ||
|
|
||
| // Candidate order: |
There was a problem hiding this comment.
Blocking repo-rule issue: this is new shipped registry-app code, and R10 says not to add inline // comments in shipped code. Please remove this comment block (or make the fallback order self-explanatory with names) before merging.
Summary
packages/design/tokens.jsonwith schema/docs and mirrored it at/r/design.json./designand/design.txtto expose the root design guide through the registry site, with TOC, metadata, sitemap, nav/footer, and llms integration.<table>markup on/design, with regression coverage.mdx-contentconsumers install the packages it imports.Closes #250
Current Head / Validation Evidence
e457eaa35ec758074f4151c9844c7a6b820b3109.packages/ui/src/components/mdx-content/mdx-content.test.tsx, which renders a pipe table and verifies accessibletable,columnheader, andcellroles.remarkPlugins={markdownPlugins}on both package and registryMDXContentrender paths so GitHub-flavored pipe tables render as tables rather than paragraph text.Dependency Tradeoff Note
remark-gfmis imported by the shippedMDXContentruntime path in@vllnt/ui, so consumers need it installed when they use the component outside this monorepo; the registrymdx-contentitem also declares the package and MDX/markdown runtime dependencies for copy-paste installs.ReactMarkdowninMDXContent; it does not add a global app dependency path beyond consumers that render MDX/markdown content.remark-gfmis a mature, lockfile-pinned markdown parsing plugin with no required network or filesystem access in this usage; it stays covered by normal dependency review/scanning and the existing markdown-rendering trust boundary.Design Deviations
None.